Subject: [Ntp] draft-ietf-ntp-roughtime-05: tag change makes implementation more complex

From: JP Sugarbroad <[email-masked]>
Date: Sun, 19 Sep 2021 12:22:29 -0700

In the pre-ietf draft, short tags were padded with FF, putting them at the
end of the tag sequence. Now they're padded with 00, which puts them at the
start. So packets start with PAD rather than ending with it, noticeably
complicating packet construction.

-- 
JP Sugarbroad <[email-masked]>
"Please let me know if there's any further trouble I can give you."
    -- Unknown

---

From: Watson Ladd <[email-masked]>
Date: Mon, 20 Sep 2021 19:56:02 -0700

On Sun, Sep 19, 2021 at 12:22 PM JP Sugarbroad <[email-masked]> wrote:
>
> In the pre-ietf draft, short tags were padded with FF, putting them at the end of the tag sequence. Now they're padded with 00, which puts them at the start. So packets start with PAD rather than ending with it, noticeably complicating packet construction.

Dear JP,

I don't think this complicates packet construction much. If you know
the length of all elements ahead of time, and then compute the PAD,
nothing changes. If you are accumulating elements and putting PAD at
the end after knowing the length, it's a little bit of buffer
manipulation.

Sincerely,
Watson Ladd

> _______________________________________________
> ntp mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ntp

--
Astra mortemque praestare gradatim

---

From: JP Sugarbroad <[email-masked]>
Date: Mon, 20 Sep 2021 20:31:51 -0700

Isn't the requirement to have tags in order?

On Mon, Sep 20, 2021, 19:56 Watson Ladd <[email-masked]> wrote:

> > In the pre-ietf draft, short tags were padded with FF, putting them at
> the end of the tag sequence. Now they're padded with 00, which puts them at
> the start. So packets start with PAD rather than ending with it, noticeably

---

From: Watson Ladd <[email-masked]>
Date: Mon, 20 Sep 2021 20:43:25 -0700

> Isn't the requirement to have tags in order?

Addition is commutative. And if PAD was at the end before, it wouldn't
be if we made a tag that was three letters long and started with three
letters after PAD. An implementation that assumed that would likely be
a problem.

Could you perhaps say more about why this is a barrier to your
implementation? It's easy enough for my coauthor and I to change them
back, but we'd like to know why.

---

From: JP Sugarbroad <[email-masked]>
Date: Mon, 20 Sep 2021 21:06:51 -0700

On Mon, Sep 20, 2021, 20:43 Watson Ladd <[email-masked]> wrote:

I hear you on the other short tags, so I guess my request is to allow the
padding tag to be last. (FWIW the way the tags sort feels weird.)

The shape of a packet before was:

2, 32, "NONC", "PAD\xFF", random, padding

Notably, the size of the padding doesn't have to be calculated and is at
the end. So one can allocate a zeroed buffer, write mostly fixed data to
the start of it, and then send.

In the new version:

"ROUGHTIM", len, 3, padlen, padlen+4, "PAD\0", "VER\0", "NONC", padding,
x80000005, random

This version has padding in the middle, and the lengths are all dependent
on the length of the packet.

If padding were at the end:

"ROUGHTIM", len, 3, 4, 36, "VER\0", "NONC", "PAD\xFF", x80000005, random,
padding

Back to mostly static values. It's a small thing, but I'm on an embedded
platform and these things can matter...

- JP

---

From: Watson Ladd <[email-masked]>
Date: Mon, 20 Sep 2021 21:45:43 -0700

On Mon, Sep 20, 2021 at 9:07 PM JP Sugarbroad <[email-masked]> wrote:
> I hear you on the other short tags, so I guess my request is to allow the padding tag to be last. (FWIW the way the tags sort feels weird.)
>
> Notably, the size of the padding doesn't have to be calculated and is at the end. So one can allocate a zeroed buffer, write mostly fixed data to the start of it, and then send.
>
> In the new version:
>
> "ROUGHTIM", len, 3, padlen, padlen+4, "PAD\0", "VER\0", "NONC", padding, x80000005, random
>
> This version has padding in the middle, and the lengths are all dependent on the length of the packet.
>
> "ROUGHTIM", len, 3, 4, 36, "VER\0", "NONC", "PAD\xFF", x80000005, random, padding
>
> Back to mostly static values. It's a small thing, but I'm on an embedded platform and these things can matter...

You can still do the construction in a mostly static way. You know the
total length of the buffer ahead of time: that's what's the padding
for. So fill up the buffer with all the other data. Then you know the
padding length, and can copy that segment of the buffer backwards to
make room for the padding.

Sincerely,
Watson

---

From: Danny Mayer <[email-masked]>
Date: Tue, 21 Sep 2021 10:27:38 -0400

> back, but we'd like to know why.

It would be helpful if you explained why you changed it in the first 
place. I assume you had a good reason to do so.

Danny

---

From: Marcus Dansarie <[email-masked]>
Date: Tue, 21 Sep 2021 21:24:53 +0200

I'll hijack this thread in an attempt to answer a number of related
Roughtime questions in the same place. Apologies in advance for that.

> place. I assume you had a good reason to do so.

I was the one to initially suggest terminating the PAD tag with zeros
instead of ones (0xff) back in 2019:

> The PAD tag key is encoded as PAD\xff, while the SIG tag key is
> encoded as SIG\x00. For consistency, this should be changed so that
> both are terminated with \x00. Additionally, terminating the tag keys
> with the NULL character will make displaying them easier on many
> platforms.

In short, terminating all "ASCII-style" tags shorter than four
characters with zeros allows for unambiguous translation into 32-bit
integer tags. The entire message is archived at:
https://mailarchive.ietf.org/arch/msg/ntp/BSYmaFGoEYHNECg2Mc2qgjzQZh4/

Since a Roughtime request can currently only contain PAD, VER, and NONC
tags, where the sizes of latter two are fixed, implementations should
have no problems figuring out the size of the PAD tag beforehand if that
is necessary. I do however understand that it can be nice to have an
"add_padding" function in a Roughtime implementation, which abstracts
away the details. I have such a function in my Python implementation,
where each tag is represented by a RoughtimeTag object in a sorted list.
The list can then be transformed into the Roughtime wire format at any
time. My C implementation, in comparison, is much simpler and creates
all tags first and then adds them to a message in hard-coded order.

On 2021-09-21 06:09, JP Sugarbroad wrote:
> Perhaps a GREASE-type tag space would be beneficial?

Section 6.1. of the current draft says "Tags other than NONC and VER
SHOULD be ignored by the server." There should probably be similar text
for the client, possibly along with text in Section 8. that the server
MAY send undefined tags along with otherwise valid responses.

On 2021-09-21 06:26, JP Sugarbroad wrote:
> I notice that DTAI and DUT1 are in milliseconds, while everything else
> is in microseconds. Is the extra range of a millisecond value needed?

I was the one to suggest using milliseconds, but I can't remember any
specific reason for that. Changing DTAI and DUT1 to microseconds gives
them a range of +/- 2147 seconds. That should be enough. In any case, we
won't be around if and when that becomes a problem.

On 2021-09-21 06:47, Watson Ladd wrote:
> DUT1 is only available in 10^-1s precision. DTAI is always whole
> seconds. We could probably fix that to be seconds.

The accuracy of DUT1 as specified by IERS Bulletin C is 0.1 seconds.
However, the IERS also provides higher accuracy DUT1 values in its daily
earth orientation parameters. For example, the predicted value for today
(September 21) is -0.1094794. Bulletin C currently says -0.1 seconds. We
may or may not want to clarify the draft in this regard.

Kind regards,
Marcus

---

From: Danny Mayer <[email-masked]>
Date: Wed, 22 Sep 2021 10:59:53 -0400

> https://mailarchive.ietf.org/arch/msg/ntp/BSYmaFGoEYHNECg2Mc2qgjzQZh4/

It would make more sense to make all of the tags 4 ascii characters and 
be done with it. I also don't understand the need for PAD and that the 
message has to be at least 1024 bytes in length. The length of the 
message is included in the packet so such a requirement is unnecessary.

Danny

---

From: Watson Ladd <[email-masked]>
Date: Sat, 25 Sep 2021 09:21:59 -0700

> message is included in the packet so such a requirement is unnecessary.

NUL is an ASCII character :-P.

Padding is necessary to ensure that servers never send back replies
bigger than responses. The minimum length is to ensure that most
responses will fit. Why we do it in the message and not the packet is
partially historical: Marcus and I will at some point revisit this
along with some other choices.

Sincerely,
Watson

---

From: Danny Mayer <[email-masked]>
Date: Sun, 26 Sep 2021 12:24:23 -0400

> NUL is an ASCII character :-P.

So is VT. I'm not even sure why you want ASCII characters for tags. They 
can just as easily be represented by numbers. If you use an 8-bit size 
you can have up to 255 tags and therefore compress the amount of space 
needed in the packet.

I find this statement (in Section 6.2.5) to be a bit strange: "The MIDP 
tag value MUST be timestamp of the moment of processing." What does that 
mean? It could mean any time between receiving the packet to sending the 
packet. Since you don't appear to be sending a receive timestamp or 
sending timestamp you can't get an idea of the round-trip time. Between 
Earth and Mars that's a large number, relatively speaking but this 
proposal doesn't seem to take account of that at all. If you don't care 
about round-trip time you should say so explicitly in the draft.

> along with some other choices.

According to the diagram in Section 6, you do specify the length of the 
message. I'm not clear why you are wasting 8 bits at the beginning to 
say 'ROUGHTIM', what else would it be? You are better off using that 
space to specify roughtime versioning information.

Danny

---

From: Danny Mayer <[email-masked]>
Date: Mon, 27 Sep 2021 12:31:00 -0400

I have been reviewing the draft again and it's not ready for 
publication. Here are a number of additional issues.

1. Section 6: "Servers SHOULD support the UDP transport mode, while TCP 
transport is OPTIONAL."

Unless you specify some other transport one of these needs to be 
mandatory. "SHOULD" is probably a "MUST". There is no explanation about 
why TCP might be used instead of UDP, so that needs clarification. If 
there is a good reason to be sending multiple requests and responses 
over TCP, please explain why this is preferable.

2. The message format (Figure 1) seems bizarre. Number of pairs

The number of pairs shouldn't need 32 bits, I suspect that 16-bits is 
more than enough. You can use the rest of the 16 bits for something 
else, like version information.

3. The message format (Figure 1). You seem to have tag identifiers after 
offsets and then values. Why not have the tag followed by a length of 
the value followed by the label? Why are you going to all the trouble of 
using offsets? Even if you are going to use offsets why isn't the tag 
identifier before the offset or even a tag identifier with the offset? 
The offsets don't need to be 32-bits in size, 16-bits is more than 
enough unless you are intending to send megabytes of packets. Note that 
is seems to be assumed that the offsets are in exactly the same order as 
the tag identifier specified separately. That's a bad assumption to 
make. It's not clear why the offsets have to be in strict ascending 
order, it shouldn't matter as long as you can correlate it to the tag 
identifier.

4. TAG values

Here's another problem. Consider this from section 6.1.1: "the VER tag 
contains a list of versions". Actually the tag appears to be just a tag. 
If I interpreted the format from Figure 1 correctly, the TAG identifier 
is separate from the values and in this case the list of versions. None 
of the TAGs specified in section 6 specify the format of these values 
and what the list in the case of VER would look like. The length of 
these values are not specified anywhere except in the offset section 
which has to then interpret how long the value is which may or may not 
be a multiple of 4 bytes.

5. VER values

That is the meaning of the version? What does the version depend on? Is 
this the version of the protocol? Of the message? Of something else?

6. Secton 6.2.5 SREP

This is totally unclear. If references other tags: ROOT, MIDP, RADI, 
DUT1, DTAI, LEAP, but they seem to be part of the SREP values rather 
then part of the overall message. Are they a separate set of tags or do 
they only appear under SREP and if only under SREP then how is SREP 
formatted.

7. INDX tag

It's unclear what the need for this tag is. Section 6.2.7 says that it 
identifies the postion of the NONC but since the NONC is already in the 
message this seems to be a duplicate. The reference to the Merkle tree 
doesn't clarify this at all. Confusingly then Section 6.3.1 goes on to 
say: "The bits of INDX are ordered from least to most significant in 
this algorithm" which seems to be unrelated to the NONC.

8. CERT

The contents seem to include other tags. It's not clear why these, like 
the ones mentioned in SREP are not separately specified.

9. Hashing algorithm in Section 6.3

It's not clear why the algorithm can only be SHA-512. This really should 
be separately referenced so that if SHA-512 is considered too week in 
can be replaced by a different hashing algorithm like say HMAC-SHA-512, 
without having to update the proposed RFC.

10. Grease (Section 8)

This seems to be garbled: "Servers MUST NOT send back responses with 
incorrect times and valid signatures.  Either signature MAY be invalid 
for this application." What does that even mean?

10. Examples of each and every tag and value would be helpful. It's hard 
to understand what each tag is providing and how the values are 
formatted and interpreted.

This is enough for now.

Danny

---

From: Watson Ladd <[email-masked]>
Date: Mon, 27 Sep 2021 18:48:37 -0700

> publication. Here are a number of additional issues.

Thanks for your review! We shall revise soon.
> over TCP, please explain why this is preferable.

We can add explanatory text: TCP is for supporting Tor. I think making
UDP a MUST works, although chances are not every server will
necessarily have as good reachability over UDP.

> else, like version information.

We want to minimize the number of numeric types. There's plenty of
room for a version number in its own tag which we do.

> order, it shouldn't matter as long as you can correlate it to the tag

Nested TLV protocols are a major source of vulnerablities and
surprises. By having the tags always sorted the same way, and the
offsets in ascending order, we ensure that the data of different
sections is nonoverlapping and that identical tag->data maps have
identical representations. The 4 byte alignment is sometimes
convenient: we're not hurting for space here.

> be a multiple of 4 bytes.

Perhaps contains is not the right verb. We mean that the value
associated to the tag is the list of versions. All values are
multiples of 4 bytes. The message formats delinates the length of all
values.

> this the version of the protocol? Of the message? Of something else?

Of the protocol. This will be clarified.
> they only appear under SREP and if only under SREP then how is SREP

SREP is formated as Roughtime message. That is the tags and their
associated values are serialized in the same offset tag data format.
The rest of the paragraphs in this section define each of these
values.

> this algorithm" which seems to be unrelated to the NONC.

The NONC is inserted into a Merkel tree that contains the NONC of
other messages. This is then signed. INDX indicates where in the
Merkel tree NONC is. We will add text to clarify this.

> without having to update the proposed RFC.

Part of what is required for roughtime is interoperability between all
servers, auditors, and clients. Using only a single hash function and
signature, and then migrating entire versions, is a better approach
than permitting everything.

> for this application." What does that even mean?

We mean that servers may send back invalid signatures. This text will
be clarified.

> formatted and interpreted.

We shall add test vectors.

> This is enough for now.

Thank you for your detailed comments.

---

From: Danny Mayer <[email-masked]>
Date: Tue, 28 Sep 2021 11:43:33 -0400

> Of the protocol. This will be clarified.

That means that it's in the wrong place. In addition there can only one 
version of the protocol in the packet being sent and that belongs in the 
header. Get rid of the useless "ROUGHTIM" string in the header and 
replace it with something useful, including the version number.

Danny

---

From: JP Sugarbroad <[email-masked]>
Date: Tue, 28 Sep 2021 09:39:56 -0700

On Tue, Sep 28, 2021, 08:43 Danny Mayer <[email-masked]> wrote:

> Get rid of the useless "ROUGHTIM" string in the header and replace it with
> something useful, including the version number.
>
I don't think it's useless. We've seen a number of "tunneling" and
"punning" attacks where one protocol is mistaken for another. Having a
protocol unambiguously self-identify is useful.

- JP

---

From: Danny Mayer <[email-masked]>
Date: Tue, 28 Sep 2021 16:01:32 -0400

On 9/28/21 12:39 PM, JP Sugarbroad wrote:
> On Tue, Sep 28, 2021, 08:43 Danny Mayer <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>     Get rid of the useless "ROUGHTIM" string in the header and replace
>     it with something useful, including the version number.
>
There is nothing to prevent me creating a protocol called SMOOTHTIME and 
throwing the ROUGHTIM value in the header to fool you. How has that made 
the protocol unambiguous? That doesn't prevent tunneling, and yes I'm 
familiar with this idea and I've seen the implementations. The only 
proper way to handle this is deep packet inspection and fingerprinting. 
Nothing else really works.

Danny

---

From: "Salz, Rich" <[email-masked]>
Date: Tue, 28 Sep 2021 23:13:13 +0000

> There is nothing to prevent me creating a protocol called SMOOTHTIME and throwing the ROUGHTIM value in the header to fool you. How has that made the protocol unambiguous?

And what will happen?  Your protocol will be treated by conformant applications as ROUGHTIM.  Is that what SMOOTHTIM is?

The IETF tries not to have conflicting identifiers, and sometimes IANA is involved.

A protocol header might help here, and seems to cause no harm.

---

From: Danny Mayer <[email-masked]>
Date: Wed, 29 Sep 2021 11:11:56 -0400

On 9/28/21 7:13 PM, Salz, Rich wrote:
>
> > There is nothing to prevent me creating a protocol called SMOOTHTIME 
> and throwing the ROUGHTIM value in the header to fool you. How has 
> that made the protocol unambiguous?
>
> And what will happen?  Your protocol will be treated by conformant 
> applications as ROUGHTIM.  Is that what SMOOTHTIM is?
>
Can you point to an existing RFC where the protocol name is in the 
header? Is there something useful about it that the security folks think 
it would be a good addition? Any other references to make sense of this?

Danny

---

From: "Salz, Rich" <[email-masked]>
Date: Wed, 29 Sep 2021 15:30:02 +0000

*   Can you point to an existing RFC where the protocol name is in the header? Is there something useful about it that the security folks think it would be a good addition? Any other references to make sense of this?

Does ALPN count?  It’s a protocol identifier in the TLS clientHello message.  There’s lots of ALPN identifiers picked, https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids

For HTTP, the h2 identifier when connecting over TCP.

Any ASN1-based protocol (SNMP, PKIX, etc) has an identifier.

---

From: JP Sugarbroad <[email-masked]>
Date: Wed, 29 Sep 2021 09:16:38 -0700

On Wed, Sep 29, 2021, 08:12 Danny Mayer <[email-masked]> wrote:

> Can you point to an existing RFC where the protocol name is in the header?
> Is there something useful about it that the security folks think it would
> be a good addition? Any other references to make sense of this?
>

HTTP 1.0, SSH, SMTP, etc. And for UDP there's a whole RFC about how to
distinguish different protocols because they don't do this.

The security problem is when an attacker can direct an honest
implementation of one protocol to send traffic to an honest implementation
of a different protocol and get unexpected results. Self-identified
protocols don't have this problem.

It's not the only solution, but it is the easiest. And 8 bytes is really
nothing when we have to pad to 1k anyway to avoid amplification attacks.

- JP