tlswg / dtls13-spec

Repo for DTLS 1.3
32 stars 25 forks source link

Martin Duke Comments #230

Closed ekr closed 3 years ago

ekr commented 3 years ago

Thanks for this document -- badly needed and easy to read.

Thanks also to Bernard Adoba for his recent tsvart review. I support his comments.

COMMENTS: Sec 2. It might be useful to introduce the term "epoch" in the glossary, for those who read this front to back.

Sec 4.2.3: "The encrypted sequence number is computed by XORing the leading bytes of the Mask with the sequence number. Decryption is accomplished by the same process."

The text is unclear if the XOR is applied to the expanded sequence number or merely the 1-2 octets on the wire. I presume it's the latter, but this should be clarified.

Sec 4.2.3: It's implied here that the sn_key rotates with the epoch. As this is different from QUIC, it's probably worth spelling out.

Sec 5.1 is a bit vague about the amplification limit; why not at least RECOMMEND 3, as we've converged on this elsewhere?

Sec 5.1. Reading between the lines, it's clear that the cookie can't be used as address verification across connections in the way that a NEW_TOKEN token is. It would be good to spell this out for clients -- use the resumption token or whatever instead.

Sec 7.2 "As noted above, the receipt of any record responding to a given flight MUST be taken as an implicit acknowledgement for the entire flight." I think this should be s/entire flight/entire previous flight?

Sec 7.2 "Upon receipt of an ACK that leaves it with only some messages from a flight having been acknowledged an implementation SHOULD retransmit the unacknowledged messages or fragments."

This language appears inconsistent with Figure 12, where Record 1 has not been acknowledged but is also not retransmitted. It appears there is an implied handling of empty ACKs that isn't written down anywhere in Sec 7.2

Sec 9. Should there be any flow control limits on new_connection_id? Or should receivers be free to simply drop CIDs they can't handle? It might be good to specify.

Finally, a really weird one. Reading this document and references to connection ID prompted to me to think how QUIC-LB could apply to DTLS. The result is here: https://github.com/quicwg/load-balancers/pull/106/files. Please note the rather unfortunate third-to-last paragraph. I'm happy to take the answer that this use case doesn't matter, since I made it up today. But if it does, it would be very helpful if (1) DTLS 1.3 clients MUST include a connection_id extension in their ClientHello, even if zero length, and/or (2) this draft updated 4.1.4 of 8446 to allow the server to include connection_id in HelloRetryRequest even if the client didn't offer it. Thoughts?

NITS: 5.2 s/select(HandshakeType)/select(msg_type). Though with pseudocode your mileage may vary as to what's clearer.

5.7 s/consitute/constitute

Sec 5.7 In table 1, why include one ACK in the diagram but not the other? It's clear from the note, but the figure is a weird omission.

hannestschofenig commented 3 years ago

Addressed in https://github.com/tlswg/dtls13-spec/pull/226