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 >