Conversation
|
@mondain First interop result, from Jitsi Videobridge built on RFC 9147 section 5.9 requires the HKDF-Expand-Label prefix What it looked like on the wire, in case anyone else hits it: Chrome accepted the ServerHello (X25519MLKEM768 selected, so the hybrid key share exchange itself was fine) and then sent only ACKs, each covering one more record than the last. Those were the plaintext ServerHello records of each retransmission; it never acknowledged any epoch 2 record, because it could not decrypt them, and DTLS drops undecryptable records without an alert. The server retransmitted until the handshake timed out. With the prefix corrected, the same bridge negotiates Two things I still want to check against draft-ietf-tls-rfc9147bis, which has post-RFC corrections on ACK handling and key updates, but they did not affect this handshake. |
|
@mondain Two follow-ups after the label fix (mondain#1). Interop status. With that fix, Jitsi Videobridge on this branch now completes DTLS 1.3 with X25519MLKEM768 as server against both Chrome (BoringSSL) and Firefox (NSS), with DTLS-SRTP media flowing both ways, so the exporter agrees with both stacks. The Firefox run also turned up a path-MTU problem on our side (a 1317-byte ServerHello datagram with an ML-KEM key share on a 1280-byte path), fixed in the bridge by sizing DTLS datagrams at 1200 like the browsers do; nothing for bctls there, but worth knowing that a DTLS 1.3 ServerHello with a hybrid key share no longer fits a conservative MTU on its own and relies on handshake fragmentation working, which it did. Conformance review against RFC 9147 and draft-ietf-tls-rfc9147bis. I went through the series against the draft revision at tlswg/dtls13-spec, which has picked up several post-RFC corrections. Three items are fixed with tests in mondain#2:
Two more bis MUSTs I did not change, since they only matter with a misbehaving peer or after 65k KeyUpdates: post-handshake messages arriving in an epoch after the peer's KeyUpdate in that epoch should be One place where the draft's wording, not the code, is the problem: #294 (errata 8108) makes an ACK naming an epoch higher than "the epoch in which the ACK was received" fatal. Taken as the carrying record's epoch, which the errata's motivation implies, that breaks a second one-sided KeyUpdate, whose ACK the peer must send from its older epoch; taken as the receiver's epoch it no longer stops the forged plaintext ACK it was written for. The series' two-phase handling, and your Everything else I checked (ACK encoding and generation rules, ACK epoch rules, retransmission and the 2×MSL server state, KeyUpdate gating and key retention, invalid-record discard, sequence number and epoch reconstruction, fragmentation and reassembly, timers, codepoints) matched both texts. This review and the fixes were produced with generative-AI assistance, per the contributing guidelines, and checked against the RFC and draft text. |
7947c73 to
32b0c5d
Compare
…ha20 (RFC 9147 4.2.3), relates to github bcgit#1468.
…pher interface, relates to github bcgit#1468.
…(RFC 9147 4.2.2), relates to github bcgit#1468.
… 9147 4), relates to github bcgit#1468.
…le (RFC 9147 section 5.9) instead of TLS 1.3's "tls13 ", so DTLS 1.3 traffic keys, record number keys, Finished keys and exporters interoperate with other implementations, relates to github bcgit#1468.
…atagrams Adds the ACK-driven reliable handshake of RFC 9147 sections 5.8 and 7, and packs handshake flights into as few datagrams as the MTU allows. The packing applies to DTLS 1.2 as well as 1.3, since that is what github bcgit#1487 asks for: on the existing aggregated-handshake test a client flight went from 5 datagrams to 3 for the same 1279 bytes. Only handshake records are packed, and a non-handshake record flushes the buffer before it leaves, so write order is preserved; that matters for the implicit change_cipher_spec, which must not overtake the flight it follows. The reliable handshake registers each written fragment against the record number that carried it, retires fragments when an ACK arrives, retransmits only what is outstanding, and emits ACKs on the RFC 9147 7.1 triggers. Inbound ACKs are filtered by the epoch of the record carrying them, and both ACK emission and the accumulated record-number list are bounded by what fits in one datagram. DTLS 1.3 still cannot be negotiated, so none of the 1.3 paths are reachable yet. DTLSReassembler.contributeFragment changed from void to boolean and gained acceptsFragment and getNextExpectedOffset; the admission predicate is unchanged, and DTLS 1.2 behaviour is unaffected. Two test helpers, MinimalHandshakeAggregator and ServerHandshakeDropper, decided what to do by inspecting only the first record of a datagram, which was exact only while each datagram carried one record. Both now walk every record. relates to github bcgit#1468. closes github bcgit#1487.
…relates to github bcgit#1468.
…relates to github bcgit#1468.
…t, relates to github bcgit#1468.
…overage, relates to github bcgit#1468.
…s retransmission, relates to github bcgit#1468.
…on, relates to github bcgit#1468.
…etention, relates to github bcgit#1468.
…, relates to github bcgit#1468.
…seq, relates to github bcgit#1468. The re-review of the previous fixes found two comments claiming more than the code does and two test assertions weaker than their messages. The notification comment said the call was placed exactly as TlsServerProtocol does it. The version gating is mirrored; the ordering relative to the other TlsServer callbacks is not, and cannot be, because DTLS selects the version in generateServerHello, by which point establishClientSigAlgs and processClientExtensions have already run. Say both, rather than claiming a wholesale mirror. DTLSVerifier produces a DTLSRequest for any ClientHello carrying a cookie it has verified, so it need not have sent the HelloVerifyRequest during that call. The invariant is that the ClientHello arrived through the cookie exchange. The server-refusal test asserted the internal_error the client receives, which is not specific to this cause, so the harness now records what the server itself threw and the test asserts that diagnostic. The harness also stops printing a stack trace for a server abort a test expects, which otherwise makes a passing test look like a failing one. Counting datagrams that carry a ClientHello cannot tell a new message from a retransmission of the first, so the scripted transport records each ClientHello's message_seq and the test requires it to advance to 1, per RFC 6347 4.2.2.
32b0c5d to
49d7433
Compare
|
[AI] @JonathanLennox, an update on your two comments above.
The only change to your commit is in Conformance fixes (mondain#2). Reviewed there. The three changes are correct. The The stack is also rebased onto current |
2 participants
DTLS 1.3 (RFC 9147), part 3 of 4: the client and server handshakes.
Stacked on #2440 (reliable handshake), which is stacked on #2439 (record layer). Review those first;
this PR's diff against #2440 is the handshake logic itself.
The series was planned as five parts and is now four: the client and the server are submitted together
here, because the two are mirror images and splitting them would have left a part that could not be tested
against anything. Part 4 is the post-handshake work.
With this PR a
DTLSClientProtocolandDTLSServerProtocolpair whose peers listProtocolVersion.DTLSv13completes a full certificate-authenticated 1-RTT DTLS 1.3 handshake, including HelloRetryRequest, optional
client authentication, ACK-driven recovery from packet loss, and RFC 5705 / RFC 8446 7.5 exporters. The
immediate motivation is DTLS-SRTP (RFC 5764) over DTLS 1.3 for WebRTC, which is verified here end to end.
What a maintainer will want to check first
No shared-path changes.
TlsClientProtocol,TlsServerProtocol,TlsProtocolandRecordStreamareuntouched. The only file shared with TLS over TCP that changes at all is
TlsUtils, which gains anull != recordStreamguard inestablish13TrafficSecretsthat TLS callers never reach.DTLS 1.2 is unchanged on the wire. Every new branch in
DTLSRecordLayerandDTLSReliableHandshakeisgated on the negotiated version; the legacy 13-byte record read paths only gain an additional
else if; andthe transcript change below is conditional. The full existing DTLS 1.2 and TLS test suites pass unchanged.
There is exactly one behavioural change on the DTLS 1.2 path, and it is a deliberate convergence rather than
a side effect:
notifySecureRenegotiationnow fires fromgenerateServerHelloafterprocessClientExtensions, which is whereTlsServerProtocolhas always called it, instead of earlier fromprocessClientHello. No wire bytes move. The reason for the change is that gating that callback on theversions the client offered, rather than on the version selected, made a DTLS 1.3 server reject a client
offering both versions without
renegotiation_infoor the SCSV — a client a BC TLS 1.3 server accepts.Version ceilings.
CLIENT_LATEST_SUPPORTED_DTLSandSERVER_LATEST_SUPPORTED_DTLSboth move toDTLSv13. To be precise about what that does and does not mean:AbstractTlsPeer.getSupportedVersions()returns TLS versions, so every DTLS peer already names its own version list, and these two constants only
bound what
isSupportedDTLSVersionClient/Serverwill permit. A peer that does not listDTLSv13isunaffected. DTLS 1.3 is not silently switched on for anyone.
The transcript
DTLS 1.3 hashes a 4-byte TLS-style handshake header —
msg_typeplus a uint24length— not DTLS's 12-byteheader with
message_seq,fragment_offsetandfragment_length(RFC 9147 5.2).DTLSReliableHandshakenow defers encoding until the version is decided, because the ClientHello is hashed before a version exists,
and
DTLSTranscriptHashTestasserts the resulting bytes against an independently computed SHA-256 ratherthan against another BC peer.
This is worth calling out because it is the one class of defect that no BC-to-BC handshake can surface: both
peers would have agreed on the wrong transcript and every test would have passed while all interop failed.
DTLS-SRTP over DTLS 1.3
Verified, and the test earns the claim rather than asserting it: both peers derive byte-identical 60-byte
EXTRACTOR-dtls_srtpmaterial; the ServerHello on the wire is checked to carry nouse_srtp, proving theprofile was negotiated through the encrypted EncryptedExtensions; and the server deliberately selects the
last of two offered profiles so the client cannot be echoing its own preference. No main-source code was
needed for this — the existing SRTP plumbing works once the exporter does.
Known gaps
Stated plainly, because they are the reasons this is part 3 of 4 rather than the whole feature.
transcript header form is the one interop-critical fact covered by a byte-level test against independently
computed bytes; the HelloRetryRequest
message_hashsubstitution is the second. Everything else iscurrently proven BC-to-BC. Interop against BoringSSL, wolfSSL, OpenSSL master and NSS is being built out;
community requests for OpenSSL 4.1/master and for NSS/Firefox on DTLS 1.3 support #1468 are noted and planned.
and no
key_share; this client answersmissing_extension. MarkedTODO[dtls13:psk_ke]in place. This isthe most likely real interop failure, and it is a hard failure.
DTLSVerifierfront end cannot front a 1.3 handshake. A 1.3-capable server reached throughaccept(DTLSRequest, ...)thereforerefuses to negotiate 1.3 rather than failing obscurely later, with a diagnostic naming the cause. It is
worth saying why it refuses instead of quietly negotiating 1.2: the
DOWNGRDsentinel (RFC 8446 4.1.3) isderived from the server peer's own configured versions, not from anything this code can narrow per
connection, so a local downgrade would leave the sentinel written and every 1.3-capable client aborting. A stateless HelloRetryRequest front end
analogous to
DTLSVerifierneeds its own PR — it is a new public class, not a few lines — and an earlierdraft of this PR that tried to add a half-measure was removed rather than shipped, because a cookie demand
that is bypassable is worse than none.
post-handshake KeyUpdate, NewSessionTicket and client authentication. Post-handshake work is part 4. The
corresponding
skip13*paths andTODO[dtls13-psk]markers are in place.downstream depends on, mirroring the existing TODO on the TLS path.
Tests
837 tests in the
tlsmodule, no failures; checkstyle clean.tls/src/main/javastays within the legacyJava 1.4/1.5 Ant build constraints.
Beyond the handshake variants, the tests that carry the most weight are the ones with negative controls:
MTU 512 against MTU 1500 to establish from the wire, without decrypting, that the authenticated client flight
genuinely fragments; a deterministic drop of that flight, with the retransmission proven by the observed
epoch sequence;
nullversus zero-length exporter context yielding the same material under 1.3 and differentmaterial under 1.2, which distinguishes RFC 8446 7.5 from RFC 5705 4 rather than merely observing that two
outputs differ; and flight-matching unit tests that reject wrong epoch, wrong type, wrong length, truncation,
and a foreign fragment packed beside a real one.
Disclosure
This work was produced with generative-AI assistance, per the contributing guidelines. Every design decision,
every security finding and its fix, and every test were reviewed; the RFC citations above were checked
against the specification text rather than recalled.
Refs #1468.