Hi Paul,

On 14 Jun 2012, at 05:02, Paul Sangster <paul_sangs...@symantec.com> wrote:

>> -----Original Message-----
>> From: Alexey Melnikov [mailto:alexey.melni...@isode.com]
>> Sent: Monday, June 04, 2012 12:02 PM
>> To: apps-disc...@ietf.org; draft-ietf-nea-pt-tls....@tools.ietf.org
>> Cc: ietf@ietf.org
>> 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
> 

Reply via email to