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

[David Mills]

Danny & Co.,

I've had a couple rounds with Joel and responded to his comments. The 
marked up version is in the '06.xml at www.eecis.udel.edu/~mills and 
also via the NTP project page. Unfortunately, I can't produce a rfc text 
from xml2rfc because that program is missing a referenc to the NTP spec. 
As for the wordpad suggestion, that is in fact how I edit the rfc xml 
document, but that doesn't work for rfc txt documents which have hard 
EOLs. When I force bold text and expand the font, the line wraps become 
essentiallly unreadable.

Dave

Danny Mayer wrote:

>I will do my best to answer these and Dave Mills can chime in and
>correct my responses and I do not have all the answers handy. I'm doing
>this on a Sunday as I had no time to get to it during the week.
>
>Aside for Dave: If you can grab the plain-text copy of the draft from
>here: http://www.ietf.org/internet-drafts/draft-ietf-ntp-autokey-05.txt
>and open it in Wordpad you should be able to read it.
>
>Brian: There are references here to memo in the document. It should be
>replaced with the word document.
>
>My responses are interspersed below.
>
>Danny
>
>Joel M. Halpern wrote:
>  
>
>>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.
>>    
>>
>
>This document needs to be read in conjunction with the NTPv4 protocol
>(draft:
>http://www.ietf.org/internet-drafts/draft-ietf-ntp-ntpv4-proto-11.txt)
>which describes the extension field format. Section 12 IANA
>considerations specifies that the type for NTP Autokey are 1 through 7.
>All of them are NTP Autokey. Dave and I had a long discussion about this
>specific issue and this was what I was able to come up with to resolve
>the problem. The usage within Autokey is as a field type as described in
>paragraph 3 of Section 10 and it defines 1-7 as reserved for Autokey.
>Thus the "Field Type" field in fact performs two roles: identifying it
>as an Autokey extension and defining the type within autokey. Within
>autokey it just needs to know the type and external to autokey it just
>needs to know it's an autokey extension.
>
>Looking at this again, I may have to dig up the email on this. But I
>note that the sentence that starts with "For future versions values 1-7
>have been reserved for Autokey" (Section 10, page 22) is wrong, since
>this is true for current versions.
>
>  
>
>>     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.
>>    
>>
>
>Discussion about MD5 has been heavy recently within the dnsext working
>group but noone has come up with an exploit or other reason to worry
>about it. If you have, please send references. In any case the MAC,
>which uses MD5, is defined in the NTPv4 protocol document and any issues
>with it should be discussed against that document.
>
>  Section 11.1 however
>  
>
>>includes a 16 bit Digest / Signature NID.
>>    
>>
>
>This is specific to Autokey and has nothing to do with the MAC.
>
>  
>
>>1) There is no description of how this is used / negotiated
>>    
>>
>
>Dave needs to supply this part.
>
>  
>
>>2) There is no explanation of the relationship between this and the 
>>earlier assertion of MD5
>>    
>>
>
>There is none. MD5 is used for the MAC which is a hash for the whole NTP
>packet. This Digest/Signature NID is specific to the Autokey and its
>state and only protects the autokey information. Dave will have to
>expand on this point as I don't know how it is formed. The code says
>that the signature algorithm is bound to the digest algorithm but I
>don't know how this is applied.
>
>  
>
>>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.
>>    
>>
>
>There are references to X509 certificates but Dave will need to explain
>how this is all applied and the references to it.
>
>  
>
>>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.
>>
>>    
>>
>
>If the values do not match then the server is not authenticated. The
>client can then choose to start the authentication from the beginning,
>or drop the server as not meeting it's requirements to be authenticated.
>If we don't have that in the draft we should add language like this.
>
>  
>
>>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.
>>
>>    
>>
>
>The URL for this is: http://www.ntp.org/ which provides pointers to the
>reference implementation, documentation, research in the area, etc.
>Perhaps this could be added as an informative reference.
>
>  
>
>>     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.
>>
>>    
>>
>
>I agree.
>
>  
>
>>     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.
>>    
>>
>
>You are correct. The document just says the length instead of "extension
>field 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.)
>>    
>>
>
>Agreed. It is a bit obscure.
>
>  
>
>>    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.
>>
>>    
>>
>
>Yes this could be clarified. I think I stumbled on this one too.
>
>  
>
>>     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 agree. The Autokey Protocol Overview should come early in the document
>as an introduction to what is coming in the document. I suggest it be
>moved between sections 4 (Autokey Cryptography) and 5 (NTP Secure Groups).
>
>  
>
>>     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.)
>>    
>>
>
>If there were a MIM then it would be interrupting the exchanges and the
>packet would not validate. This is described in the last paragraph of
>Section 4. Also the client has received keys OOB that it uses to
>authenticate the server so a MIM would not be able to authenticate
>unless it had the private keys of the server in question. In addition
>NTP does not rely on a single server (at least it shouldn't) so if one
>authenticated server does not agree with the others, whether or not
>authenticated, within some overlap, it will not use the server for
>timestamps until it's within the overlap with the other servers. So for
>an attacker, they cannot just attack via one server but they need to set
>up a second one as well whose time agrees with the first within narrow
>limits. It becomes very hard to do that. Setting up a second server with
>authentication is even harder to do.
>
>  
>
>>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.
>>    
>>
>
>Agreed. It should be a new section.
>
>  
>