[ntpwg] [Gen-art] Review: draft-ietf-ntp-autokey-05.txt

[Joel M. Halpern]

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-ntp-autokey-05.txt
     Network Time Protocol Version 4 Autokey Specification
Reviewer: Joel M. Halpern
Review Date: 5-June-2009
IETF LC End Date: 12-June-2009
IESG Telechat date: N/A

Summary: While nearly ready for publication as an Informational RFC, 
this document has some issues that need to be addressed before 
publication.  The issues are described by priority below.

Major issues:
     There appears to be some confusion in the protocol description as 
to which field actually tells you what message you are handling, and 
which field tells you that this is NTP Autokey.  In teh text in section 
10, it says that the "Field Type" field is set to 2, and represents a 
version number.  Further, it says that the 6-bit Code field specifies 
the request or response.  Unfortunately, all of the subsections of 
section 10 carefully and consistently refer to having different "Field 
Type" values, not different Code values.
     Section 4 of this document is explicit that MD5 is the (singular) 
hash algorithm for generating message signatures and autokeys.  I was 
concerned about lack of algorithm agility.  Section 11.1 however 
includes a 16 bit Digest / Signature NID.
1) There is no description of how this is used / negotiated
2) There is no explanation of the relationship between this and the 
earlier assertion of MD5
3) While the text mentions that this is defined in "the OpenSSL 
library", there is no reference, and particularly no reference to any 
documentation to see what values are defined.  Since this ought to be 
implementable by someone who does not live in the OpenSSH world, a 
reference is necessary.
4) There is no mention of what happens if the value proposed by the host 
and the values supported by the server do not match.

Moderate issue:

     The IETF does not normally use "reference implementations." 
Therefore, it would be helpful if early in this document there were text 
describing what the reference implementation is, how it is found, and 
what the relationship of that reference implementation to this RFC is.

     I believe that the wording on hierarchy in section 5, paragraph 3 
is exactly the opposite of what is shown in the figure.  (And what is 
shown in the figure matches what I would expect.)  The text says:
    Secure groups can be configured as hierarchies where a TH of one
    group can be a client of one or more other groups operating at a
    lower stratum.
But in fact, what I think happens is that a client of one group operates 
as the TH of a group operating at a lower stratum.

     Section 10, the paragraph that begins "[T]he extension field parser 
initializes a pointer..." has some sort of problem.  After reading this 
several times, I am pretty sure that "the length" by which the pointer 
is increment is the length in the extension header, not the length 
computed by subtracting the NTP header from the packet length.  If so, 
please insert test (probably after the "greater than 20" check and 
before the "not multiple of 4 check") that describes something like "The 
extension header is found immediately after the NTP header.  The length 
of the extension header is checked to ensure that this length plus the 
20 bytes for the MAC header does not exceed the calculated remaining 
octets of the packet."

Minor issues:

     In figure 5, it would probably help the reader if the groups and 
hosts were not named identically.  (Even just calling the groups 
Alice-Group and Carol-Group would help.)

    In section 5, in the description of "[t]he steps in hiking the 
certificate trails...", in step 1, in the second sentence, could you 
please add language to make it clear that what the server is "exchanging 
host names and negotiating..." with is the server from whom it is 
getting information.  It took rereading the list to realize that this 
was not about the servers initiating communication with the clients who 
use them.  (Yes, I know that such initiation is impossible.  Which is 
why an extra couple of words would help.)  Part of this is because 
common terminology, and later usage in this document, is that the 
machine in this role (the "server" who is starting to exchange host 
names ...) is called the client, if I have parsed the protocol correctly.

     Is there any way section 8 could be moved earlier in the document? 
  One of the problems reading the document is that various parts early 
in the document assume properties of the system which have not been 
described yet.

     I am not sure if this is an issue that should be described in the 
security considerations section, or whether I am missing something that 
helps.  There are multiple references to the need for Identity 
mechanisms.  But those mechanisms are not part of this draft.  If I read 
things properly, if identity verification is not used then a 
man-in-the-middle can step into the stream, and assuming he can convince 
clients (possibly by packet misdirection) to talk to him, he can produce 
an apparently valid certificate chain and simply lie about the time. 
(Yes, there is mention earlier of other aspects of NTP that moderate his 
potential impact.  But if we did not care about this, then this document 
would not exist.)

Nits/editorial comments:
     It appears that section 11.6 (Security Considerations) is supposed 
to be a top-level sections, with 11.7 and 11.8 as sub-sections of that.