[Nea] Minor Comments on PT-EAP

[Stephen Hanna <[e-mail]>]

I have conducted a detailed review of PT-EAP. I have
reviewed the specification many times before but I
still found a few minor issues, noted below.

I'm surprised that there are still issues, given that
this document has been reviewed so many times by so
many people (including at least four thorough reviews
by me). But I guess there are always a few typos and
little errors in every document. Still, there are only
a few at this point and the spec overall is quite good.

PT-EAP is a valuable specification of high quality.
I support sending it to the IESG and on to RFC status.
But I'd like to see these minor issues fixed first.

Thanks,

Steve

-----------

Minor Comments on draft-ietf-nea-pt-tls-03.txt

* The front page says that the Intended Status is Informational.
  That should be Standards Track.

* Four places in the spec, the phrase "EAP TLS-based tunnel" or
  "EAP TLS-based tunnel method" appears. This could be misread
  as a reference to the EAP-TLS method. I think it would be much
  clearer to change these phrases to "TLS-based EAP tunnel" or
  "TLS-based EAP tunnel method".

* The paragraph immediately before section 1.1 says PT-EAP is
  designed to be used "under a protected tunnel" such as TEAP,
  EAP-FAST, or EAP-TTLS. In this phrase, I think the word "under"
  can be interpreted two ways. Clearly, the authors mean that
  PT-EAP is used inside a protected tunnel. But the phrase could
  also mean that PT-EAP is used as a substrate under a protected
  tunnel. In other words, that the protected tunnel would be
  encapsulated inside PT-EAP. Clearly, that's nonsense. But
  this phrase would be much clearer if we replaced the word
  "under" with "inside". Then it would say PT-EAP is designed
  to be used "inside a protected tunnel" such as TEAP, EAP-FAST,
  or EAP-TTLS.

* The second paragraph in section 3 says that this section
  includes "a flow diagram". Actually, there's no flow diagram
  in that section or anywhere else in the document. I suggest
  that we remove the words "and a flow diagram" from that
  paragraph.

* The last sentence in that paragraph is missing a word when
  it says that tls-unique "may be used to PA-TNC exchanges to
  the EAP tunnel method". I think the word "bind" needs to be
  added before "PA-TNC" in that sentence.

* The words "an PT-EAP" appear in the document three times.
  Of course, they should be "a PT-EAP".

* At the start of section 3.3, the numbers in the message
  diagram should be one space to the right. They need to
  line up with the hyphens below, not the plus signs.

* At the end of section 3.4, the word "an" should be inserted
  before the final word "attacker".

* The first paragraph of section 4 ends with two periods.
  One should be enough. Also, the penultimate sentence in
  that paragraph includes the phrase "do not evaluate".
  This should be "does not evaluate" to match the singular
  noun ("this section").

* In sections 4.1.1 and 4.1.2, several bullets start with
  "Not to". The word "to" should be deleted from those
  bullets. Otherwise, the bullets are ungrammatical when
  combined with the text that introduces the bullets. For
  example, the first bullet when combined with the text
  before the bullets now reads "The Posture Transport Client
  is trusted by the Posture Broker Client to [...] Not to
  observe, fabricate or alter the contents of the PB-TNC
  batches received from the network". Clearly, the words
  "Not to" should be simply "Not".

* Two bullets in sections 4.1.1 and 4.1.2 start with the
  words "Deliver properly security protected messages",
  which are hard to parse. Does this mean "Properly
  deliver security-protected messages" or "Ensure that
  messages delivered were properly protected from a
  security perspective"? Both meanings are sensible
  and arguably desirable but I think that the first
  meaning is already covered by the bullet that reads
  "Not to observe, fabricate or alter the contents of
  the PB-TNC batches received from the network".

  While it's clever to include both meanings in this
  text, I think we're seeking clarity in our text not
  clever double meanings that might easily be missed
  by non-native speakers so I suggest rewording these
  bullets to say "Ensure that PB-TNC batches delivered
  to and from the network are properly protected from
  a security perspective while on the network".
  
* In the first full bullet on page 10, I believe that
  "Posture Transport Client" should be "Posture Broker
  Client". Otherwise, the bullet makes no sense. Why
  would the Posture Transport Client need to expose
  the identity of the Posture Transport Server to the
  Posture Transport Client (PTC)? We'd have the PTC
  exposing something to itself. A similar comment
  applies in section 4.1.2 but in that case "Posture
  Transport Server" should be "Posture Broker Server".

* With respect to the last five bullets in section
  4.1.1, I don't think it's safe for the Posture
  Transport Server to completely trust the Posture
  Transport Client in these ways. After all, the
  Posture Transport Client is on an untrusted device.
  I suggest adding a sentence at the end of section
  4.1.1 saying "While the Posture Transport Server
  expects the Posture Transport Client to follow
  these expectations, the Posture Transport Server
  cannot truly trust the Posture Transport Client
  since it's running on a potentially untrustworthy
  machine. Therefore, the Posture Transport Server
  must assume that the Posture Transport Client may
  be infected and malicious. The Posture Transport
  Server should protect itself accordingly."

  A similar argument can be applied to section 4.1.2
  but it is not as strong there because the Posture
  Transport Server is trusted by the Posture Transport
  Client to some degree since the NEA Server has been
  authenticated and a decision has been made that it is
  trusted to receive confidential information about
  endpoint posture and perhaps to send remediation
  instructions. Still, I suggest adding the following
  text at the end of section 4.1.2:

  While the Posture Transport Client expects the Posture
  Transport Server to follow these expectations and has
  inherently placed some trust in the Posture Transport
  Server by sending information about endpoint security
  posture to that server, the Posture Transport Client
  should still protect itself against compromise of the
  Posture Transport Server or errors in implementation
  by detecting and protecting against malformed messages
  and other suspicious behavior on the part of the
  Posture Transport Server.

* In section 4.1.2, the first bullet in the second bulleted
  list should end in "Posture Transport Client" not
  "Posture Transport Server".

* Section 4.2.2 refers to "fake PT-EAP error codes".
  There are no PT-EAP error codes. I suggest that we
  say "malformed PT-EAP messages" instead.

* In the first paragraph of section 4.2.4, "the PT-EAP"
  should be just "PT-EAP". Later in that sentence,
  the word "are" should be "is" to match the subject
  of the sentence, "the message exchange".

* In section 4.2.5, please add a citation of the NEA
  Asokan Attack Analysis draft after it is mentioned.
  That is, add "[I-D.ietf-nea-asokan]". Also, please
  update the reference in the references section to
  point to the WG draft version of this document not
  the individual submission version. And in the first
  sentence of section 4.2.5, remove the extra period
  after "3.4".

* In the first sentence on top of page 14, change
  "only known" to "best known". There are other ways
  to protect against Asokan attacks. For example,
  one could use behavior analysis to detect an
  infected machine after it connects to the network.
  Still, the EMA is by far the best and most effective
  approach.

* The second paragraph of section 4.3 cites EAP-FAST
  and EAP-TTLS. Please add TEAP. 

* The third paragraph of section 4.3 cites EAP-TTLS,
  mentioning that non-EAP authentication is supported.
  Please change "TTLS" to "EAP-TTLS". Also, please
  mention TEAP here also since TEAP supports password
  authentication without using an inner EAP method.

* Section 4.3 says "Within each EAP tunnel method will
  exist a set of inner EAP method [...]". This should
  be "inner EAP methods" not "inner EAP method". Later
  in that paragraph, "the outer methods keys" should be
  "the outer method's keys".

* Section 5 refers to the "IETF EAP Method Unification
  (EMU) working group". The word "Unification" should
  be "Update" there. Later in that sentence, "the EAP
  tunnel method" should be "a Standards Track EAP
  tunnel method". We already have two EAP tunnel methods
  that are Informational RFCs and satisfy NEA's
  requirements: EAP-FAST and EAP-TTLS. What's missing
  is a Standards Track EAP tunnel method.

* The first paragraph of section 7 includes a reference
  to RFC 2434. That RFC has been obsoleted by RFC 5226.
  We should refer to the newer RFC instead. Also, we
  should add a period at the end of that paragraph.

* Many of the references are not used anywhere in the
  document. Let's delete them. They are [IEEE],
  [I-D.ietf-emu-eaptunnel-req], [RFC3478], [RFC5216],
  [RFC5996], and [TNC-Binding]. Also, [RFC2434] will
  be unused if you adopt my last suggested change.