Subject: [Last-Call] Genart last call review of draft-ietf-ntp-interleaved-modes-04

From: Theresa Enghardt via Datatracker <[email-masked]>
Date: Tue, 06 Apr 2021 18:43:12 -0700

Reviewer: Theresa Enghardt
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-ntp-interleaved-modes-04
Reviewer: Theresa Enghardt
Review Date: 2021-04-06
IETF LC End Date: 2021-04-14
IESG Telechat date: Not scheduled for a telechat

Summary: The draft is basically ready for publication as a Standards Track RFC,
but it has some clarity issues that need to be addressed before publication.

Major issues: None.

Minor issues:

Section 1:
The introduction describes design considerations for changing the semantics of
existing timestamps, rather than introducting additional packets. However, it
does not mention negotiation. Please add ome text to the introduction clrifying
how interleaved mode is negotiated, implicitly (if I understand correctly).
Furthermore, the introduction should mention that clients, servers, and peers
now have to infer whether a received packet is in basic mode or interleaved
mode from the timestamps themselves and their cached knowledge (if I understand
correctly). Has explicit negotiation been considered? What would be the
consequence of a client, server, or peer failing to correctly determine whether
a received packet is in basic or interleaved mode, perhaps due to an
implementation error? Please consider adding a few sentences to discuss this
scenario.

The introduction also does not mention whether a client, server, or peer that
supports interleaved mode has to operate in interleaved mode exclusively, or
whether it can switch between interleaved mode and basic mode. The document
later clarifies this, but please consider already clarifying here that an
implementation must support both modes if it wants to use interleaved mode, and
that a given sequence of messages can switch between both modes, to make the
scope clearer and the subsequent sections easier to understand.

Section 2:
Here the document first mentions the origin timestamp, where previously the
document has only talked about transmit and receive timestamps. I think it
would be good to briefly explain what the origin timestamp is, how this
document is changing its semantics, and why. Section 7.3 of RFC 5905 defines
"Origin Timestamp (org): Time at the client when the request departed for the
server", but this document says "A client request in the basic mode has an
origin timestamp equal to the transmit timestamp from the previous server
response, or is zero." If basic mode is RFC 5905, I would have expected these
definitions to match. Has the definition of origin timestamp changed from RFC
5905 to what this document terms basic mode? Please clarify.

Can a server only respond in interleaved mode if the client request was in
interleaved mode? Please clarify. (Section 3 says that a peer "MUST NOT respond
in the interleaved mode if the request was not in the interleaved mode", but I
have not found a similar statement for client/server interleaved mode.)

"Both servers and clients that support the interleaved mode MUST NOT send a
packet that has a transmit timestamp equal to the receive timestamp in order to
reliably detect whether received packets conform to the interleaved mode." I
think the document should reiterate in this section that a server or client has
to perform such detection (on each incoming packet?), and how to make this
determination.

Section 3:
"The peer A has an active association with the peer B which was specified with
an option enabling the interleaved mode" This sentence reads as if there is an
option to explicitly enable the interleaved mode. Howeve, this document does
not change the NTP packet format or add an option. Please clarify/rephrase.

Nits/editorial comments:

Section 1:
"in the user space" -> "in user space"

"would enable a traffic amplification" -> "would enable a traffic amplification
attack"

To make Section 2 easier to navigate, maybe it would help to add subsections,
e.g., "Field semantics", "Protocol operation", and "Example".

Section 3:
"The peers SHOULD compute the offset and delay using one the two sets of
timestamps specified in the client/server section" -> "[…] using one of the two
sets of timestamps […]"

---

From: Miroslav Lichvar <[email-masked]>
Date: Thu, 8 Apr 2021 17:44:07 +0200

> like any other last call comments.

Thank you for the review.

I tried to address all issues that you pointed out, with one exception
noted below. A new version of the draft is available (-05). It also
has a few smaller changes suggested by Erik Kline earlier.

> 5905 to what this document terms basic mode? Please clarify.

The description you quoted from RFC 5905 seems to be from the server's
point of view, i.e. the response has a copy of the transmit timestamp
from the request. The section "8. On-Wire Protocol" has more details
and an example of the fields in an exchange.

I added a new paragraph briefly explaining the purpose of the origin
timestamp.

> have not found a similar statement for client/server interleaved mode.)

This should be covered by the list of conditions after "When the
server receives a request from a client, it SHOULD respond in the
interleaved mode if the following conditions are met:"

I added "(i.e. the request is not detected to conform to the
interleaved mode)" to the paragraph starting with "If the conditions
are not met" to make this more clear. 

> to perform such detection (on each incoming packet?), and how to make this

I'm not sure what exactly is missing in that section. The server
has to check those two conditions for each packet, before sending a
response in interleaved mode. The client has an extended test for
bogus packets, starting with "The check for bogus packets SHOULD
compare the origin timestamp". It needs to be performed for each
packet.

> not change the NTP packet format or add an option. Please clarify/rephrase.

Symmetric interleaved mode is supposed to be enabled with an option to
prevent an interoperability issue with peers that don't support
the interleaved mode. I added a new paragraph to explain that.

Thanks,

-- 
Miroslav Lichvar