Subject: secdir review for draft-ietf-ntp-checksum-trailer-04.txt

From: Sandra Murphy <[email-masked]>
Date: Thu, 3 Mar 2016 11:47:27 -0500

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 primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.

The document draft-ietf-ntp-checksum-trailer-04.txt defines a new
extension header for NTP that will allow a hardware based timestamping
engine to modify the timestamp in the NTP packet and then add a
correction to the packet's UDP checksum to account for the modified
timestamp.  Putting the correction in an extension header allows for
serial processing of the packet - no need to backtrack from the
timestamp to the UDP checksum preceding in the packet and correct that
field.

I found the document adequate for the most part.

(1)

It seems there's an impossibility of protecting the NTP packet with
this extension.  This extension header MUST NOT be used with an NTP
MAC, and so is usable only in those situations where NTP is used in an
"unauthenticated mode".  One might imagine that NTP running in an
unauthenticated mode could be run in an ipsec tunnel for protection.
However, the description of the timestamping engine makes it seem that
the engine modifies the packet directly before transmission.  I.e.,
the packet would be modified after the ipsec protections were applied.
Certainly the "intermediate nodes" mentioned in 3.2.2 would be adding
the extension header after the source applied the ipsec protection.
If my suppositions are correct, then using this extension header means
neither an authenticated NTP nor a protected tunnel could be used.
All the intruder attacks mentioned in RFC5905 security considerations
section would be possible.

(2)

RFC5905 and draft-ietf-ntp-extension-field-07.txt both define the
general extension header format with the padding following the value.
This draft defines this extension header as the value following the
padding.  That conflict should be addressed.

(3)

Section 1 describes intermediate entities (like the timestamping
engine), and section 3.2.2 says:

  An intermediate node that receives and alters an NTP packet
  containing a Checksum Complement extension MAY use the Checksum
  Complement to maintain a correct UDP checksum value.

However, section 4 says:

                                                              The
  extension is intended to be used internally in an NTP client or
  server, and not intended to be used by intermediate switches and
  routers that reside between the client and the server.

That seems to contradict the earlier statements, particularly section
3.2.2.  Are the intermediate nodes of section 3.2.2 not "switches and
routers"?  This is probably just a wording issue, but it should be
more clear.

--Sandy

---

From: Tal Mizrahi <[email-masked]>
Date: Thu, 10 Mar 2016 15:00:02 +0000

Hi Sandra,

Thanks for the comments.

In the current draft (07) I believe that all your comments have been addressed.
https://tools.ietf.org/html/draft-ietf-ntp-checksum-trailer-07

Detailed responses below.

>(1)
>
>It seems there's an impossibility of protecting the NTP packet with this
>extension.  This extension header MUST NOT be used with an NTP MAC, and
>so is usable only in those situations where NTP is used in an "unauthenticated
>mode".  One might imagine that NTP running in an unauthenticated mode
>could be run in an ipsec tunnel for protection.
>However, the description of the timestamping engine makes it seem that the
>engine modifies the packet directly before transmission.  I.e., the packet
>would be modified after the ipsec protections were applied.
>Certainly the "intermediate nodes" mentioned in 3.2.2 would be adding the
>extension header after the source applied the ipsec protection.
>If my suppositions are correct, then using this extension header means neither
>an authenticated NTP nor a protected tunnel could be used.
>All the intruder attacks mentioned in RFC5905 security considerations section
>would be possible.

Agreed. 
This issue is discussed in the security considerations section. Based on your comment the text has been slightly revised to clarify this point further. Here is the updated text:

   The concept described in this document is intended to be used only in
   unauthenticated mode. As discussed in Section 3.4. , if a
   cryptographic security mechanism is used, then the Checksum
   Complement does not simplify the implementation compared to using the
   conventional Checksum, and therefore the Checksum Complement is not
   used.

>(2)
>
>RFC5905 and draft-ietf-ntp-extension-field-07.txt both define the general
>extension header format with the padding following the value.
>This draft defines this extension header as the value following the padding.
>That conflict should be addressed.

Agreed. 
Based on this comment the field name has been changed from "Padding" to "MBZ".

>(3)
>
>Section 1 describes intermediate entities (like the timestamping engine), and
>section 3.2.2 says:
>
>That seems to contradict the earlier statements, particularly section 3.2.2.
>Are the intermediate nodes of section 3.2.2 not "switches and routers"?  This
>is probably just a wording issue, but it should be more clear.

The point is well taken.
The use of the word "intermediate" was a bit confusing in a few locations in the document. The text has been rephrased in these cases in order to clarify the meaning.
Specifically, the sentence from Section 4 that you mentioned above was rephrased to the following:

   The
   extension is intended to be used internally in an NTP client or
   server. This extension is not intended to be used by switches and
   routers that reside between the client and the server.

Best regards,
Tal.

>-----Original Message-----
>From: ietf [mailto:[email-masked]] On Behalf Of Sandra Murphy
>Sent: Thursday, March 03, 2016 6:47 PM
>To: [email-masked]; [email-masked]; The IETF
>Subject: secdir review for draft-ietf-ntp-checksum-trailer-04.txt
>
>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 primarily for the benefit of the security area
>directors.  Document editors and WG chairs should treat these comments just
>like any other last call comments.
>
>The document draft-ietf-ntp-checksum-trailer-04.txt defines a new extension
>header for NTP that will allow a hardware based timestamping engine to
>modify the timestamp in the NTP packet and then add a correction to the
>packet's UDP checksum to account for the modified timestamp.  Putting the
>correction in an extension header allows for serial processing of the packet -
>no need to backtrack from the timestamp to the UDP checksum preceding in
>the packet and correct that field.
>
>It seems there's an impossibility of protecting the NTP packet with this
>extension.  This extension header MUST NOT be used with an NTP MAC, and
>so is usable only in those situations where NTP is used in an "unauthenticated
>mode".  One might imagine that NTP running in an unauthenticated mode
>could be run in an ipsec tunnel for protection.
>However, the description of the timestamping engine makes it seem that the
>engine modifies the packet directly before transmission.  I.e., the packet
>would be modified after the ipsec protections were applied.
>Certainly the "intermediate nodes" mentioned in 3.2.2 would be adding the
>extension header after the source applied the ipsec protection.
>If my suppositions are correct, then using this extension header means neither
>an authenticated NTP nor a protected tunnel could be used.
>All the intruder attacks mentioned in RFC5905 security considerations section
>would be possible.
>
>(2)
>
>RFC5905 and draft-ietf-ntp-extension-field-07.txt both define the general
>extension header format with the padding following the value.
>This draft defines this extension header as the value following the padding.
>That conflict should be addressed.
>
>(3)
>
>Section 1 describes intermediate entities (like the timestamping engine), and
>section 3.2.2 says:
>
>That seems to contradict the earlier statements, particularly section 3.2.2.
>Are the intermediate nodes of section 3.2.2 not "switches and routers"?  This
>is probably just a wording issue, but it should be more clear.
>
>--Sandy