ietf
[Top] [All Lists]

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

2012-06-14 16:01:35
Hi Paul,

On 14 Jun 2012, at 05:02, Paul Sangster <Paul_Sangster(_at_)symantec(_dot_)com> 
wrote:

-----Original Message-----
From: Alexey Melnikov [mailto:alexey(_dot_)melnikov(_at_)isode(_dot_)com]
Sent: Monday, June 04, 2012 12:02 PM
To: apps-discuss(_at_)ietf(_dot_)org; 
draft-ietf-nea-pt-tls(_dot_)all(_at_)tools(_dot_)ietf(_dot_)org
Cc: ietf(_at_)ietf(_dot_)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


<Prev in Thread] Current Thread [Next in Thread>