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

[Danny Mayer]

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.

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.