[TLS] draft-ietf-tls-dtls13-31 comments

[David Benjamin <[e-mail]>]

Hi all,

I read through draft-ietf-tls-dtls13-31 and gathered a bunch of comments. I
uploaded https://github.com/tlswg/dtls13-spec/pull/85 for the easy bits.
The remainder I figured I should send to the list. Hopefully this is a
usable format. Apologies for the very late look over it. I haven't had time
to page it in before now.

In section 4, I assume that if not negotiated, CID cannot be present and
the receiver rejects any records with it? It might be worth spelling that
out explicitly.

Section 4.2.1 reads odd to me. I assume the references to 1 and 2 are just
as an example and not a particular oddity from those epochs. Maybe stick a
"for example" or say "earlier and later epoch". It also says that during
the handshake, "implementations MUST accept packets from the old epoch",
which is ambiguous as there are several epochs in the handshake. That
sentence also runs counter to the SHOULD immediately at the start of the
paragraph, which is odd.

Section 4.5.1 talks about timing channels for the sequence number. Note
even uncompressing the sequence number (finding the right upper bytes)
introduces a timing channel. Granted, it is a much much weaker one than
record deprotection, but invalid DTLS records being non-fatal means signals
can be amplified...

Section 5 says that implementations MUST ignore ChangeCipherSpec messages.
This contradicts section 4.1, which does not include change_cipher_spec as
one of the possible record rtypes for DTLSPlaintext. It does fall through
to section 4.5.2, but 4.5.2 allows for implementations to reject these. (Is
there actually a need to ignore them in DTLS 1.3 if the middlebox hacks
aren't applicable?)

I don't understand the last paragraph of section 5.1. What's it trying to
describe? If the client got its hands on an invalid cookie, there's no
recovering that handshake. The server won't accept the cookie, and the
client won't accept a second HRR.

Section 5.3 says that the supported_versions extension is used without
modification from TLS 1.3, but that still leaves its contents ambiguous. Is
DTLS 1.3 0304 or fefc? I assume 0304 since fefc isn't mentioned. But then
how are DTLS 1.0 and DTLS 1.2 encoded? (One could imagine omitting them,
but that doesn't match how TLS 1.3 did it.) I assume they remain feff and
fefd, so we don't need to answer whether DTLS 1.0 is 0301 or 0302. Either
way, this should probably be spelled out.

This is a nitpick and exists in DTLS 1.2 too, but section 5.4 uses the term
"maximum handshake fragment size", but nowhere else in the document is the
term defined or referenced. This means the largest handshake fragment such
that the resulting DTLS record fits in a packet. (Though even that's
incomplete because you might have previously allocated a smaller fragment
and then have less room left for the next fragment. It gets further messier
when packing fragments into records and having to worry about when you are
and aren't forced to add a record boundary for epoch change...)

Section 5.9 is weird. With warning alerts gone, all alerts are fatal.
Either you've torn down all local state and don't resend alerts (I suspect
most applications just do this, for better or worse), or maybe you're
hanging around for a while to resend them. If the latter, you probably
don't want to be trying to detect that the offending record got resent,
since that involves processing records out of a connection in an error
state. It seems better to blindly retransmit on every packet until you get
bored.

Relatedly, what does close_notify mean in DTLS? This draft and DTLS 1.2
don't seem to make any mention of it. Bidirectional shutdown doesn't make
much sense. Not sure if unidirectional is meaningful, given lack of
retransmits. Maybe a best effort thing to save the peer a timeout.

Section 6.1 says:

  Implementations that receive a payload with an epoch value for which
   no corresponding cipher state can be determined MUST generate a
   "unexpected_message" alert.

This seems to be cover data from the next epoch, which contradicts section
4.2.1. (If I receive, say, EncryptedExtensions before ServerHello, I can't
determine the cipher state for that epoch.) Section 4.2.1 says you should
either buffer or drop those records, not reject them.

Section 7 defines ACKs with a uint64 record_number. I assume this is a
concatenated epoch and sequence number, but I don't see anywhere defines
this. Perhaps just write it as:

    struct {
        uint16 epoch;
        uint48 sequence_number;
    } RecordNumber;

    struct {
        RecordNumber record_numbers<0..2^16-1>;
    } ACK;

Relatedly, what is the "record sequence number" input to the AEAD (RFC
8446, section 5.3)? DTLS believes that the "sequence number" is 48 bits,
but the rest of TLS 1.3 is written expecting a 64-bit sequence number.
Using the concatenation is simplest and also matches RFC 7905, but should
be mentioned explicitly. (RFC 6347 didn't have text to this effect because
it only had AES-GCM which, in (D)TLS 1.2, used explicit IVs.)

(I will probably have other comments on the ACK bits, but I think I need to
digest it more carefully to try to understand what's going on. It does seem
a little overkill for what is ultimately a small handful of packets...)

David