Last Call Review of draft-ietf-ntp-extension-field-04
review-ietf-ntp-extension-field-04-secdir-lc-turner-2015-09-03-00

Fear not as this is just the secdir review!

I have reviewed this document as part of the security directorate’s ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written with the intent of improving security requirements and
considerations in IETF drafts. Comments not addressed in last call may be
included in AD reviews during the IESG review.  Document editors and WG chairs
should treat these comments just like any other last call comments.

draft summary: This draft updates NTPv4 Protocol and Algorithm Specification
(aka RFC 5905) s7.5, which is the section that describes extension fields, and
to paraphrase the: clarify the relationship between extension fields and MACs
and define the behavior of a host that receives an unknown extension field. 
Note that when comparing the “OLD” section to RFC 5905, you’ll should note that
the “OLD” text incorporates a verified errata (

http://www.rfc-editor.org/errata_search.php?eid=3627

).  The “NEW" text requires things like when defining an extension the
definition must specify whether it must be MACed or not, the MTI MAC, the
length of the MAC, etc.

secdir status summary: I need to clarify something in my mind, which I hope
fall into the “you missed that in this spec over here” or the “these are *NOT*
the droids you’re looking for” bucket, before I can say "ready with nits":

0) 7.5.1.1 says an extension can support multiple MACs, that the extension’s
document defines the MTI algorithm & MAC length, and that if more than one
algorithm is allowed the extension includes an indication of which one was
actually used; all great.  But, I’m trying to figure out how that fits with RFC
5905:

- In s7.3, I see "dgst (128)” in f8

- In s9.2, I see "There is no specific requirement for authentication; however,
if
   authentication is implemented, then the MD5-keyed hash algorithm
   described in [RFC1321] must be supported”

Doesn’t s7.3 limit the MAC to HMAC-MD5 and the length to 128?  I mean if you’re
going to allow an extension to override s9.2 that seems like something worth
noting in say the abstract/intro.

Thinking there’s got to be a reason for this I went off and looked at the other
NTP WG drafts … after finding the NTS & CMS-based specs, are the changes
proposed in this draft to to allow an NTP packet blob that doesn’t use the MAC
mechanism described in RFC 5905 but instead use the NTS/CMS “scheme”, i.e., an
NTP extension that is a CMS object, with no MAC in the 5905 sense - the CMS
object instead of the NTP MAC field gives you the authentication?

1) s7.5.1.2 seems to be saying if extension A specifies alg X, and extension B
specifies alg X and Y, and extension C specifies alg Y then extension A and B
can appear together as can extension B and C, but A and C can’t appear
together?   Sounds great, but what if A and C do appear together what happens?

2) Still on 7.5.12: "If there are multiple extension fields that require a MAC
they MUST
   all require use of the same algorithm and MAC length.”

So if I specify extension A with X as the MUST, and extension B with X as the
SHOULD and Y as the MUST, then I can’t include both extension A and B? 
Extension A requires X, but extension B requires Y.

3) s7.5.1.3: What’s the 24-octet limitation based on?

Minor:

0) The new s7.5 says:

  The Field Type field is specific to the defined function and is not
  elaborated here.  If a ….

0.a) I think what you’re trying to say is that the Field Type field is defined
in an IANA registry and it’s length and value are defined by the document
referred to by the registry?

0.b) Neither RFC5905 nor this document specify how the padding is done.  Is
padding determined by the document referred to by the field type?  I.e., can I
do padding with all 1s and somebody else do it with all zeros?

Maybe:

  The Field Type field defines the extension’s semantics as well as the
  extension’s syntax, i.e., length, value, and padding.  This document
  defines no extensions.

  If a …

Nits:

0) process wise: RFC 5905 has a lot of other errata; some marked verified some
marked HFDU.  Since this document updates RFC 5905, did the WG consider
including the other verified errata and resolving whether the errata marked
HFDU were worth including?  I’ll also readily admit that this could have been
found in the WG archives, but I didn’t search them.

1) s3: Might be worth noting that the “OLD” text includes the errata text.  I
can’t remember whether you can normatively or informatively point to errata
(sorry).

2) The new s7.5 includes:

  If a host receives an extension field with an
  unknown Field Type value …

I was like hmm there’s a Field Type value, which is # that goes in Field Type
and there’s the Value field for the Field Type.  Maybe dropping “value” from
the quoted sentence makes it clearer that you’re really talking about the #
that goes in Field Type and not the Value?

2) s5: Might be worth a reference to the NTP Extension Field Type IANA
registry.  Though obviously folks reading the base spec and extensions are
going to find it though.

spt

PS - RFC 5905 KoD packets are my favorite packets now.