Re: [Ntp] I-D Action: draft-ietf-ntp-using-nts-for-ntp-18.txt

[Marcus Dansarie <[e-mail]>]

Thanks for your comments! See my comments inline.

Kind regards,
Marcus


On 2019-04-22 10:06, Hal Murray wrote:
> 
> Typo, page 20:
>   Duplicate "Nonetheless, "

Thanks! (I submitted a pull request for this last Wednesday.)

> 
> Page 6, single round trip
>   That's after the TLS setup.  Right?  How many round trips does that take?

Correct, that's one round trip excluding the TLS setup. The setup is
expected to take two or three round trips depending on version.
(Theoretically, PSK could bring this down to one round trip, but I do
not envisage it being used in this application.)

> Page 13: cookies
>   Do all cookies have to have the same length?

It is not explicitly required by the draft. I can't see any "sane"
reason for a server to send cookies of different length in the same reply.

A practical reason for changing cookie length is if a server updates its
(implementation dependent) cookie format. It could then start replying
with cookies of different length than before. This could possibly create
problems if the new cookies are longer than the old ones. To avoid
sending replies that are longer than the requests, the server would have
to send zero new cookies as reply to requests containing only one cookie
placeholder in the hope that the client's next request contains two
cookie placeholders, thus enabling it to send one cookie back.
Hopefully, as the old cookies are depleted the client then starts
sending larger cookie placeholder records.

> Page 17, Nonce length and Nonce in general.
>   I don't understand this area.  (My version of "understand" is that I could 
> explain it to somebody.)
> 
>   It seems really really ugly to have an Additional Padding field that 
> requires a layer of magic to compute the length.  Am I missing something 
> simple?

The additional padding field ensures that the server always can select
the nonce length it wishes without having to send a reply larger than
the request. The somewhat complicated algorithm is due to the
specifications in RFC 5116 and the requirement that the field length be
divisible by four.

> 
>   If I poke around in AEAD RFCs, I find text about having a counter to make 
> sure the nonce is unique.  Is that appropriate or necessary?  It's really 
> really hard to keep from reusing a counter if you consider disks getting 
> restored from backup and such.
> 
>   Are random number generators good enough?  How do I figure out how many bits 
> I need?

Correctly implemented CPRNGs are good enough and is probably best
practice in this case. Still, implementors have to be careful that the
PRNG is initialized with random data and that initialization data is not
accidentaly restored from backups or similar. 128 bits is a good rule of
thumb for lowest acceptable nonce length. We would then expect a
collision (i.e. same nonce generated twice) with 50% probability after
about 2^64 generated nonces.


>   How nonce-reuse resistant are the algorithms we are using?

This is discussed in RFC 5297 section 1.3.2.

> Why not require the client to use the same length as the server?  Maybe pad 
> the client nonce with 0s if generating randomness is expensive and it doesn't 
> need as long a nonce as the server.

When the client first sends a reply, it doesn't know what length the
server wishes to use in its reply. Padding a short nonce with zeros
would not increase security. A client that wishes to do so in order to
avoid adding an additional padding field is free to do so. It is not
against the language in the draft.


> Did anybody test this?  (I doubt if our code will do the right thing.)

Test what?

> Should we include a table of AEAD algorithms and the Nonce size?

This is in the RFCs specifying the algorithms. See
https://www.iana.org/assignments/aead-parameters/aead-parameters.xhtml


> Page 19:
>   Why MUST the Unique Identifier be in the clear?

In case a server receives a request containing a cookie it cannot parse,
for example because it is encrypted with a key the server does not have,
it will reply with a NTSN KoD. Requiring inclusion of the unique
identifier stops off-path attackers from faking such replies.

> Page 21:
>   What is  an AEAD tag?  I couldn't find any other references.

It is data generated by the AEAD algorithm that cryptographically
authenticates the message.

> Page 23:  KoD/NTSN
>   They can be forged.  What's the current policy on processing KoDs?

This is discussed beginning on paragraph 4 on page 23  ("If the NTP
server has previously...").

> Page 23, last paragraph, save AEAD alg, S2C, C2S, and cookie to avoid NTS-KE 
> on NTP restart.
>   I think that paragraph would be better in an appendix.
>   In addition to avoiding the load on the NTS-KE server, it also avoids the 
> time delay that would add to the startup sequence.

The primary motivation for including the recommendation/requirement is
that the NTS system should be dependent on the KE servers as little as
possible. Ideally, an NTS client should only perform a KE once in its
existence.

> Page 24:  "AEAD key"
>   AEAD is used in two contexts - one for the wire and another for cookies.  It 
> would be nice if that word wasn't reused here.  I don't have any good 
> wordsmith suggestions.
> 

Ah, I think I see what you mean: should we be more clear that the
server's key for encrypting/decrypting cookies is different from the S2C
and C2S keys?

> Page 36, checking certificates before you know the time.
> 
>   Does the NTP_PHASE_MAX test work with systems that have been sleeping in low 
> power mode?
> 
>   "by picking a random time in the week preceding"...
>   At a quick read, it's not obvious if that's an example of what to do or what 
> not to do.  Is the idea that the certificate will get updated at least a week 
> in advance, so asking a week in advance will get the new certificate?

Yes, that's the idea. I just realized that we made that assumption
without explicitly stating it.

> Page 37/42, [Shpiner]
>   The reference on page 42 says "Mizraha, T" - no mention of Shpiner.

This is due to an error in the XML source. I submitted a pull request
fixing it last Wednesday.