LDP Hello Cryptographic Authentication
draft-ietf-mpls-ldp-hello-crypto-auth-10

Section 1:
"Of the above, implementations MUST include
   support for at least HMAC-SHA-256 and SHOULD include support for
   HMAC-SHA-1 and MAY include support for either of HMAC-SHA-384 or
   HMAC-SHA-512."

This makes it sound as if HMAC-SHA-384 and HMAC-SHA-512 support are mutually exclusive. I think the phrasing in Section 3 is better and should be repeated here (or, in any event, the two sections should say the same thing):

"Of the above, implementations of this specification MUST include
   support for at least HMAC-SHA-256 and SHOULD include support for
   HMAC-SHA-1 and MAY also include support for HMAC-SHA-384 and HMAC-
   SHA-512."

Section 2.2:
"This is a 32-bit unsigned integer used to uniquely identify an LDP
      SA between two LDP peers, as manually configured by the network
      operator (or, in the future, possibly by some key management
      protocol specified by the IETF)."

It would help to clarify whether you mean (1) a key management protocol that does not already exist may be defined in the future and used for this purpose, or (2) in the future SA IDs may be configured using a key management protocol that already exists.

"In order to achieve smooth key transition, KeyStartAccept SHOULD be
   less than KeyStartGenerate and KeyStopGenerate SHOULD be less than
   KeyStopAccept."

What are the conditions under which these SHOULDs might be violated?

The definition of the Length field in section 2.3 is nebulous.  Does it include the 96 bits of fixed-length fields or just the variable length Authentication Data.  It is unclear since the definition says it is the length of the "value field".

Is it worth mentioning why DTLS is not sufficient for this UDP-based protocol?

Thank you for addressing the SecDir review.  I support Stephen's comments and will watch the responses.  I don't have anything else to add.

I agree with Ted's DISCUSS in spirit, even if I disagree with it as stated.  What needs to be clear here is that the LSR receiving Hello messages needs to have a policy for each neighbor as to whether it requires authentication or does not.  If it is configured to require authentication, it MUST NOT accept unauthenticated Hello messages.  (Ted's message implies that this policy should be configured based on receipt of authenticated messages, which is a terrible idea.)

I'm also curious about Brian's "Is it worth mentioning why DTLS is not sufficient for this UDP-based protocol?" comment, but whatever you decide in conversations with him will be fine with me.

Thanks for addressing my DISCUSS point (included below FTR):

Adrian raised this as a nit, but it's really a DISCUSS: a conforming implementation of this specification could perfectly well accept an unauthenticated hello after it's accepted an authenticated hello, because there's no requirements language forbidding this behavior. One would hope that implementors would be smart and not do this, but given that this is a standard, it SHOULD be mentioned explicitly.  :)