Re: APPSDIR review of draft-ietf-nea-pt-tls-04

[Alexey Melnikov <[e-mail]>]

Hi Paul,

On 14 Jun 2012, at 05:02, Paul Sangster <[e-mail]> wrote:

>> -----Original Message-----
>> From: Alexey Melnikov [mailto:[e-mail]]
>> Sent: Monday, June 04, 2012 12:02 PM
>> To: [e-mail]; [e-mail]
>> Cc: [e-mail]
>> Subject: APPSDIR review of draft-ietf-nea-pt-tls-04
>> 
>> I have been selected as the Applications Area Directorate reviewer for
>> this draft (for background on APPSDIR, please see
>> http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectora
>> te ).
>> 
>> Please resolve these comments along with any other Last Call comments
>> you may receive. Please wait for direction from your document shepherd
>> or AD before posting a new version of the draft.  The review is not
>> copied to the IESG as the Last Call has not been announced yet.
>> 
>> Document: draft-ietf-nea-pt-tls-04
>> Title: PT-TLS: A TCP-based Posture Transport (PT) Protocol
>> Reviewer: Alexey Melnikov
>> Review Date: June 4, 2012
>> 
>> Summary: This document is almost ready for publication as a Proposed
>> Standard, although some [mostly] SASL related issues remain.
>> 
>> This document specifies PT-TLS, a TCP-based Posture Transport (PT)
>> protocol.  The PT-TLS protocol carries the Network Endpoint
>> Assessment (NEA) message exchange under the protection of a Transport
>> Layer Security (TLS) secured tunnel.
>> 
>> (Note, I've reviewed -04, but I think all of this still applies to -
>> 05.)
>> 
>> 
>> Major:
>> 
>> In 3.4.2.1: RFC 6125 use details are missing. You need to describe
>> whether CN-IDs and SRV-IDs are allowed, whether wildcards are allowed,
>> etc. I can suggest some details.
> 
> [PS:] Since you weren't happy with our first attempt to address this comment, we would like to take you up on your kind offer to provide suggestions.  Maybe we could do this off-line and included the text in -06?
> 

Yes.

>> Minor:
>> 
>> In Section 3.2: This document is not yet Internet Standard, it will be
>> Proposed Standard. Suggest saying "Publication on Standards Track"
>> instead instead of "Internet Standard". The same issue in the IANA
>> consideration section.
> 
> [PS:] This sentence will be re-written to address the gen-art comment so the new text won't have this issue.
> 
>> 
>> In 3.8.1: I think one instance of "SASL authentication messages" -->
>> "SASL authentication mechanisms". Otherwise this sentence is out of
>> place, as you are not talking about SASL messages.
> 
> [PS:] Will make it more clear the relationship between SASL messages in PT-TLS and the SASL mechanism exchanges inside the messages.  The sentence mentioned is about the PT-TLS client authentication messages.
> 
>> 
>> In 3.8.4: in SASL the server doesn't return abort as an error code, it
>> just fails the authentication exchange. I suggest removing it as a
>> choice.
> 
> [PS:] This text was attempting to support the abort as described in RFC4422 which says:
> 
> 3.5.  Aborting Authentication Exchanges
> 
>   A client or server may desire to abort an authentication exchange if
>   it is unwilling or unable to continue (or enter into).
> 
>   A client may abort the authentication exchange by sending a message,
>   the particulars of which are protocol specific, to the server,
>   indicating that the exchange is aborted.  The server may be required
>   by the protocol to return a message in response to the client's abort
>   message.
> 
>   Likewise, a server may abort the authentication exchange by sending a
>   message, the particulars of which are protocol specific, to the
>   client, indicating that the exchange is aborted.
> 
> [PS]: Is this an unacceptable way to do this?  
> 

Well, Ok, but nobody is sending a special message when a SASL server wants to abort the exchange. Existing open source API don't seem to support this either.
I suppose the client can just treat this as an authentication failure, as it can't know what caused the abort.

>> 
>> In 3.8.7: you define the Reserved field which I assume is used for
>> padding? If yes, then you will not get proper alignment for the next
>> field, as SASL mechanism names are variable length. (If you intended
>> that they are always sent as 20 bytes, then this is missing from the
>> document.)
> 
> [PS:] Sure, word alignment would have been a good reason for doing this but we thought more about byte alignment.  We also thought having these bits available could be useful later (e.g. for indicating preference or some other semantic that could impact the mechanism selection).   Since we didn't need an entire byte for the Mech Len we thought it made sense to at least byte align the Mechanism Name and the cost of 3 bits for some future purpose made sense.

Ok. Never mind.

Just to confirm: SASL mechanism names are not padded with spaces or NULs?

> 
>> 
>> In 3.8.10:
>> 
>> The Abort choice is really not needed (as per above).
>> 
>> Also, can you give me an example of when the Mechanism Failure will be
>> returned instead of just Failure?
> 
> [PS:] The document gives the example of authentication failure (incorrect credential) where mechanism failure is when the mechanism logic detects a failure in processing the authentication request that isn't related to the user's input (e.g. malloc error).
> 

Ok. Having malloc failure as an example would help.

>> In 3.9: Failed Authentication error code - how does this differ from
>> SASL Authentication result with Failure code?
> 
> [PS:] Hmm, I'm going to have to look into this more.  One way they are different is the fact that the NEA Client is not allowed to send the SASL Result TLV so can't use the Failure code but it could send a Failed Authentication error code in the PT-TLS Error Message.

Wouldn't the client just send Abort in this case? (Abort makes sense for clients)
> 
>> 
>> In 4.1.2, second block, the first bullet: I think you meant "client"
>> instead of the "server".
> 
> [PS:] Thanks, good catch
> 
>> 
>> 
>> Question (might not be an issue):
>> 
>> In 6.2: is it possible to register a vendor specific value without a
>> specification?
> 
> [PS:] In order to be placed in the IANA registry a permanent specification is required (as mentioned in section 6.1).   Of course vendors are free to use values in their name space without registering them with the IANA.

This might be a bit heavyweight and might discourage registrations. But if that is what you want...
> 
>> 
>> The same question for 6.3.
>> 
>> 
>> Nits: None
>