From 0e0503570a31a52bcba749afc42bdafcf329a6cf Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Sun, 21 Jun 2026 13:44:04 +0200 Subject: [PATCH 1/5] fix: address DTLS 1.3 KeyUpdate and ClientHello conformance Summary: - defer DTLS 1.3 send-key rotation until a KeyUpdate ACK arrives - ignore update_requested while a local KeyUpdate is already in flight - reject non-empty DTLS 1.3 legacy_cookie values - stop echoing DTLS 1.3 legacy_session_id values from the server - abort the client on a second HelloRetryRequest - add integration coverage for RFC 9147 conformance cases Co-Authored-By: Codex --- src/dtls13/client.rs | 14 +- src/dtls13/engine.rs | 39 +-- src/dtls13/server.rs | 15 +- src/error.rs | 10 + tests/dtls13/conformance.rs | 528 ++++++++++++++++++++++++++++++++++++ tests/dtls13/main.rs | 1 + 6 files changed, 581 insertions(+), 26 deletions(-) create mode 100644 tests/dtls13/conformance.rs diff --git a/src/dtls13/client.rs b/src/dtls13/client.rs index a6c60639..91636f51 100644 --- a/src/dtls13/client.rs +++ b/src/dtls13/client.rs @@ -463,13 +463,11 @@ impl State { // Check for HelloRetryRequest (magic random) if server_hello.is_hello_retry_request() { if client.hello_retry { - // Stale retransmission of the HRR we already processed. - // Roll back the transcript and silently discard. The CH2 - // flight will be retransmitted by the normal flight timeout - // mechanism if the server hasn't responded. - debug!("Discarding stale HRR retransmission"); client.engine.transcript.resize(transcript_len_before, 0); - return Ok(self); + return Err(Error::SecurityError( + crate::SecurityError::UnexpectedSecondHelloRetryRequest, + ) + .into()); } debug!("Received HelloRetryRequest"); @@ -1127,7 +1125,9 @@ impl State { client.engine.send_ack()?; // If peer requested us to update, schedule our own KeyUpdate - if request == KeyUpdateRequest::UpdateRequested { + if request == KeyUpdateRequest::UpdateRequested + && !client.engine.is_key_update_in_flight() + { client.pending_key_update_response = true; } diff --git a/src/dtls13/engine.rs b/src/dtls13/engine.rs index d67ccc34..1cf23085 100644 --- a/src/dtls13/engine.rs +++ b/src/dtls13/engine.rs @@ -158,6 +158,10 @@ pub struct Engine { /// Set when app_send_record_count reaches aead_encryption_threshold. needs_key_update: bool, + /// A locally sent KeyUpdate is awaiting its ACK. Send keys are rotated only + /// after that ACK arrives, so application data continues on the old epoch. + key_update_in_flight: bool, + /// Sequence number of the received close_notify alert, if any. /// Per RFC 9147 §5.10, any data with an epoch/sequence number pair /// after this must be discarded; earlier records are still valid. @@ -252,6 +256,7 @@ impl Engine { app_send_record_count: 0, aead_encryption_threshold, needs_key_update: false, + key_update_in_flight: false, close_notify_sequence: None, close_notify_reported: false, } @@ -301,7 +306,7 @@ impl Engine { } pub fn is_key_update_in_flight(&self) -> bool { - self.prev_app_send_keys.is_some() + self.key_update_in_flight } /// Returns true if the AEAD encryption limit has been reached and a @@ -1302,16 +1307,16 @@ impl Engine { } /// Process an incoming ACK record, marking acknowledged flight entries. - fn process_ack(&mut self, ack_data: &[u8]) { + fn process_ack(&mut self, ack_data: &[u8]) -> Result<(), Error> { if ack_data.len() < 2 { - return; + return Ok(()); } let record_numbers_len = u16::from_be_bytes([ack_data[0], ack_data[1]]) as usize; let entries_data = &ack_data[2..]; if entries_data.len() != record_numbers_len || record_numbers_len % 16 != 0 { - return; + return Ok(()); } let num_entries = record_numbers_len / 16; @@ -1351,17 +1356,21 @@ impl Engine { self.flight_clear_resends(); } - // If all saved records are acked and we have prev send keys (KeyUpdate in flight), - // clear them and stop the flight timer. - if self.prev_app_send_keys.is_some() + // Rotate send keys only after the peer ACKs our KeyUpdate. Until then, + // application data continues on the old/current application epoch. + if self.key_update_in_flight && !self.flight_saved_records.is_empty() && self.flight_saved_records.iter().all(|e| e.acked) { - debug!("KeyUpdate ACKed; clearing previous send keys"); + debug!("KeyUpdate ACKed; rotating send keys"); + self.update_send_keys()?; self.prev_app_send_keys = None; + self.key_update_in_flight = false; self.flight_clear_resends(); self.flight_timeout = Timeout::Disabled; } + + Ok(()) } // ========================================================================= @@ -1876,8 +1885,7 @@ impl Engine { /// Create and send a KeyUpdate handshake message. /// /// Arms the flight timer for retransmission. The KeyUpdate is sent on - /// the current app epoch, then send keys are rotated (old keys saved - /// in `prev_app_send_*` for retransmission). + /// the current app epoch. Send keys rotate only after its ACK arrives. pub fn create_key_update(&mut self, request: KeyUpdateRequest) -> Result<(), Error> { // Set up retransmission self.flight_backoff.reset(&mut self.rng); @@ -1902,12 +1910,11 @@ impl Engine { fragment.push(request.as_u8()); })?; - // Now rotate send keys (saves old keys for retransmission) - self.update_send_keys()?; + self.key_update_in_flight = true; debug!( - "KeyUpdate sent (request={:?}) on epoch {}, new send epoch {}", - request, epoch, self.app_send_epoch + "KeyUpdate sent (request={:?}) on epoch {}, awaiting ACK before rotating send keys", + request, epoch ); Ok(()) @@ -2306,7 +2313,7 @@ impl RecordHandler for Engine { match record.record().content_type { ContentType::Ack => { let fragment = record.record().fragment(record.buffer()); - self.process_ack(fragment); + self.process_ack(fragment)?; self.push_buffer(record.into_buffer()); Ok(None) } @@ -2749,7 +2756,7 @@ mod tests { malformed_ack.extend_from_slice(&7u64.to_be_bytes()); malformed_ack.push(0); - engine.process_ack(&malformed_ack); + engine.process_ack(&malformed_ack).unwrap(); assert!( !engine.flight_saved_records[0].acked, diff --git a/src/dtls13/server.rs b/src/dtls13/server.rs index 4711c8d4..8a028850 100644 --- a/src/dtls13/server.rs +++ b/src/dtls13/server.rs @@ -417,6 +417,13 @@ impl State { unreachable!() }; + if !client_hello.legacy_cookie.is_empty() { + return Err(Error::SecurityError( + crate::SecurityError::InvalidLegacyCookieInClientHello, + ) + .into()); + } + // Validate legacy_version if client_hello.legacy_version != ProtocolVersion::DTLS1_2 { return Err(Error::SecurityError( @@ -792,7 +799,7 @@ impl State { // Truncate the metadata we appended server.extension_data.resize(meta_start, 0); - let client_session_id = server.client_session_id.unwrap_or_else(SessionId::empty); + let client_session_id = SessionId::empty(); server.engine.flight_begin(2); @@ -1209,7 +1216,9 @@ impl State { server.engine.send_ack()?; // If peer requested us to update, schedule our own KeyUpdate - if request == KeyUpdateRequest::UpdateRequested { + if request == KeyUpdateRequest::UpdateRequested + && !server.engine.is_key_update_in_flight() + { server.pending_key_update_response = true; } @@ -1272,7 +1281,7 @@ fn send_hello_retry_request( selected_group: Option, ) -> Result<(), Error> { let hrr_random = Random { bytes: HRR_RANDOM }; - let client_session_id = server.client_session_id.unwrap_or_else(SessionId::empty); + let client_session_id = SessionId::empty(); server.engine.flight_begin(2); diff --git a/src/error.rs b/src/error.rs index 5856c569..c22663f7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -450,8 +450,12 @@ pub enum SecurityError { ClientHelloMustOfferNullCompression, /// The ClientHello cookie did not match the expected cookie. InvalidCookieInClientHello, + /// A DTLS 1.3 ClientHello carried a non-empty legacy_cookie field. + InvalidLegacyCookieInClientHello, /// The server attempted to send a second HelloRetryRequest. CannotSendSecondHelloRetryRequest, + /// The client received a second HelloRetryRequest. + UnexpectedSecondHelloRetryRequest, /// No common DTLS 1.3 cipher suite was found. NoCommonCipherSuite, /// No common DTLS 1.3 key exchange group was found. @@ -1134,9 +1138,15 @@ impl fmt::Display for SecurityError { write!(f, "ClientHello must offer null compression") } Self::InvalidCookieInClientHello => write!(f, "invalid cookie in ClientHello"), + Self::InvalidLegacyCookieInClientHello => { + write!(f, "ClientHello legacy_cookie must be empty") + } Self::CannotSendSecondHelloRetryRequest => { write!(f, "cannot send second HelloRetryRequest") } + Self::UnexpectedSecondHelloRetryRequest => { + write!(f, "received second HelloRetryRequest") + } Self::NoCommonCipherSuite => write!(f, "no common cipher suite found"), Self::NoCommonKeyExchangeGroup => write!(f, "no common key exchange group"), Self::HrrSelectedDisallowedCipherSuite => { diff --git a/tests/dtls13/conformance.rs b/tests/dtls13/conformance.rs new file mode 100644 index 00000000..3719dc6e --- /dev/null +++ b/tests/dtls13/conformance.rs @@ -0,0 +1,528 @@ +//! DTLS 1.3 RFC 9147 conformance regression tests. + +use std::sync::Arc; +use std::time::{Duration, Instant}; + +#[cfg(feature = "rcgen")] +use dimpl::certificate::generate_self_signed_certificate; +use dimpl::{Config, Dtls}; + +use crate::common::*; + +const RECORD_HEADER_LEN: usize = 13; +const HANDSHAKE_HEADER_LEN: usize = 12; +const HANDSHAKE_OFFSET: usize = RECORD_HEADER_LEN; +const BODY_OFFSET: usize = RECORD_HEADER_LEN + HANDSHAKE_HEADER_LEN; + +fn set_u24(buf: &mut [u8], offset: usize, value: usize) { + let value = value as u32; + buf[offset] = (value >> 16) as u8; + buf[offset + 1] = (value >> 8) as u8; + buf[offset + 2] = value as u8; +} + +fn grow_plaintext_handshake(packet: &mut [u8], added: usize) { + let record_len = u16::from_be_bytes([packet[11], packet[12]]) as usize + added; + packet[11..13].copy_from_slice(&(record_len as u16).to_be_bytes()); + + let body_len = + ((packet[14] as usize) << 16) | ((packet[15] as usize) << 8) | packet[16] as usize; + set_u24(packet, 14, body_len + added); + + let fragment_len = + ((packet[22] as usize) << 16) | ((packet[23] as usize) << 8) | packet[24] as usize; + set_u24(packet, 22, fragment_len + added); +} + +fn insert_legacy_session_id(packet: &[u8], session_id: &[u8]) -> Vec { + assert_eq!(packet[0], 22, "expected plaintext handshake record"); + assert_eq!(packet[HANDSHAKE_OFFSET], 1, "expected ClientHello"); + + let sid_len_offset = BODY_OFFSET + 2 + 32; + assert_eq!( + packet[sid_len_offset], 0, + "dimpl ClientHello starts with empty session id" + ); + + let mut out = packet.to_vec(); + out[sid_len_offset] = session_id.len() as u8; + out.splice( + sid_len_offset + 1..sid_len_offset + 1, + session_id.iter().copied(), + ); + grow_plaintext_handshake(&mut out, session_id.len()); + out +} + +fn insert_legacy_cookie(packet: &[u8], cookie: &[u8]) -> Vec { + assert_eq!(packet[0], 22, "expected plaintext handshake record"); + assert_eq!(packet[HANDSHAKE_OFFSET], 1, "expected ClientHello"); + + let sid_len_offset = BODY_OFFSET + 2 + 32; + let cookie_len_offset = sid_len_offset + 1 + packet[sid_len_offset] as usize; + assert_eq!( + packet[cookie_len_offset], 0, + "dimpl ClientHello starts with empty legacy cookie" + ); + + let mut out = packet.to_vec(); + out[cookie_len_offset] = cookie.len() as u8; + out.splice( + cookie_len_offset + 1..cookie_len_offset + 1, + cookie.iter().copied(), + ); + grow_plaintext_handshake(&mut out, cookie.len()); + out +} + +fn plaintext_server_hello_legacy_session_id_len(packet: &[u8]) -> u8 { + assert_eq!(packet[0], 22, "expected plaintext handshake record"); + assert_eq!(packet[HANDSHAKE_OFFSET], 2, "expected ServerHello/HRR"); + packet[BODY_OFFSET + 2 + 32] +} + +fn ciphertext_record_count(packets: &[Vec]) -> usize { + let mut count = 0; + for packet in packets { + let mut input = packet.as_slice(); + while !input.is_empty() { + assert!( + input[0] & 0b1110_0000 == 0b0010_0000, + "expected ciphertext record, got first byte {:#04x}", + input[0] + ); + assert!( + input[0] & 0b0000_1000 != 0, + "test expects 2-byte sequence numbers" + ); + assert!( + input[0] & 0b0000_0100 != 0, + "test expects explicit ciphertext length" + ); + let len = u16::from_be_bytes([input[3], input[4]]) as usize; + count += 1; + input = &input[5 + len..]; + } + } + count +} + +fn has_ciphertext_record_len(packets: &[Vec], wanted: usize) -> bool { + for packet in packets { + let mut input = packet.as_slice(); + while !input.is_empty() { + let len = u16::from_be_bytes([input[3], input[4]]) as usize; + if len == wanted { + return true; + } + input = &input[5 + len..]; + } + } + false +} + +#[cfg(feature = "rcgen")] +fn connected_pair(config: Arc) -> (Dtls, Dtls, Instant) { + let client_cert = generate_self_signed_certificate().expect("gen client cert"); + let server_cert = generate_self_signed_certificate().expect("gen server cert"); + + let mut now = Instant::now(); + let mut client = Dtls::new_13(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_13(config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..40 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + let client_out = drain_outputs(&mut client); + let server_out = drain_outputs(&mut server); + client_connected |= client_out.connected; + server_connected |= server_out.connected; + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + return (client, server, now); + } + now += Duration::from_millis(10); + } + + panic!("DTLS 1.3 handshake did not complete"); +} + +fn quiesce_pair(client: &mut Dtls, server: &mut Dtls, now: Instant) { + for _ in 0..8 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + let client_out = drain_outputs(client); + let server_out = drain_outputs(server); + let quiet = client_out.packets.is_empty() && server_out.packets.is_empty(); + + deliver_packets(&client_out.packets, server); + deliver_packets(&server_out.packets, client); + + if quiet { + break; + } + } +} + +#[test] +#[cfg(feature = "rcgen")] +fn server_does_not_echo_client_legacy_session_id() { + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen client cert"); + let server_cert = generate_self_signed_certificate().expect("gen server cert"); + + let config = Arc::new( + Config::builder() + .use_server_cookie(false) + .build() + .expect("build config"), + ); + let now = Instant::now(); + + let mut client = Dtls::new_13(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_13(config, server_cert, now); + server.set_active(false); + server.handle_timeout(now).expect("server timeout"); + + client.handle_timeout(now).expect("client timeout"); + let client_hello = drain_outputs(&mut client) + .packets + .into_iter() + .next() + .expect("client should emit ClientHello"); + let client_hello = insert_legacy_session_id(&client_hello, b"non-empty-session"); + + server + .handle_packet(&client_hello) + .expect("server should parse ClientHello"); + server.handle_timeout(now).expect("server timeout"); + let server_hello = drain_outputs(&mut server) + .packets + .into_iter() + .next() + .expect("server should emit ServerHello"); + + assert_eq!( + plaintext_server_hello_legacy_session_id_len(&server_hello), + 0, + "DTLS 1.3 servers MUST NOT echo legacy_session_id" + ); +} + +#[test] +#[cfg(feature = "rcgen")] +fn server_rejects_non_empty_client_legacy_cookie() { + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen client cert"); + let server_cert = generate_self_signed_certificate().expect("gen server cert"); + + let config = Arc::new( + Config::builder() + .use_server_cookie(false) + .build() + .expect("build config"), + ); + let now = Instant::now(); + + let mut client = Dtls::new_13(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_13(config, server_cert, now); + server.set_active(false); + server.handle_timeout(now).expect("server timeout"); + + client.handle_timeout(now).expect("client timeout"); + let client_hello = drain_outputs(&mut client) + .packets + .into_iter() + .next() + .expect("client should emit ClientHello"); + let client_hello = insert_legacy_cookie(&client_hello, b"legacy-cookie"); + + assert!( + server.handle_packet(&client_hello).is_err(), + "server must abort ClientHello messages with non-empty legacy_cookie" + ); +} + +#[test] +#[cfg(feature = "rcgen")] +fn client_aborts_on_distinct_second_hello_retry_request() { + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen client cert"); + let server_cert = generate_self_signed_certificate().expect("gen server cert"); + let config = dtls13_config(); + let now = Instant::now(); + + let mut client = Dtls::new_13(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_13(config, server_cert, now); + server.set_active(false); + + client.handle_timeout(now).expect("client timeout"); + let client_hello = drain_outputs(&mut client).packets; + deliver_packets(&client_hello, &mut server); + + server.handle_timeout(now).expect("server timeout"); + let hrr = drain_outputs(&mut server) + .packets + .into_iter() + .next() + .expect("server should emit HRR"); + + client + .handle_packet(&hrr) + .expect("client accepts first HRR"); + client.handle_timeout(now).expect("client sends CH2"); + let _ = drain_outputs(&mut client); + let mut second_hrr = hrr.clone(); + second_hrr[17..19].copy_from_slice(&1u16.to_be_bytes()); + + assert!( + client.handle_packet(&second_hrr).is_err(), + "client must abort on a second HRR" + ); +} + +#[test] +#[cfg(feature = "rcgen")] +fn application_data_waits_for_key_update_ack() { + let _ = env_logger::try_init(); + + let config = Arc::new( + Config::builder() + .aead_encryption_limit(3) + .build() + .expect("build config"), + ); + let (mut client, mut server, mut now) = connected_pair(config); + quiesce_pair(&mut client, &mut server, now); + + let mut key_update_out = None; + for i in 0..10 { + let msg = format!("key-update-prime-application-payload-{i}").into_bytes(); + client.send_application_data(&msg).expect("queue app data"); + let out = drain_outputs(&mut client).packets; + assert_eq!(ciphertext_record_count(&out), 1); + deliver_packets(&out, &mut server); + server.handle_timeout(now).expect("server timeout"); + let server_out = drain_outputs(&mut server); + assert_eq!(server_out.app_data, vec![msg]); + now += Duration::from_millis(10); + + client.handle_timeout(now).expect("client timeout"); + let out = drain_outputs(&mut client).packets; + if has_ciphertext_record_len(&out, 30) { + key_update_out = Some(out); + break; + } + } + + let out = key_update_out.expect("client should initiate KeyUpdate"); + assert!(!out.is_empty(), "client should send a KeyUpdate"); + let old_epoch_msg = b"sent-on-old-epoch-before-key-update-ack".to_vec(); + client + .send_application_data(&old_epoch_msg) + .expect("send app data while KeyUpdate is in flight"); + let old_epoch_data = drain_outputs(&mut client).packets; + assert!( + !old_epoch_data.is_empty(), + "application data should continue on the old epoch while KeyUpdate is in flight" + ); + deliver_packets(&old_epoch_data, &mut server); + server + .handle_timeout(now) + .expect("server receives old-epoch data"); + let server_out = drain_outputs(&mut server); + assert_eq!(server_out.app_data, vec![old_epoch_msg]); + + deliver_packets(&out, &mut server); + server + .handle_timeout(now) + .expect("server processes KeyUpdate"); + let server_out = drain_outputs(&mut server); + assert!( + server_out.app_data.is_empty(), + "processing the KeyUpdate should not produce duplicate application data" + ); + + deliver_packets(&server_out.packets, &mut client); + now += Duration::from_millis(10); + client + .handle_timeout(now) + .expect("client handles KeyUpdate ACK"); + let _ = drain_outputs(&mut client); +} + +#[test] +#[cfg(feature = "rcgen")] +fn update_requested_is_ignored_while_key_update_in_flight() { + let _ = env_logger::try_init(); + + let config = Arc::new( + Config::builder() + .aead_encryption_limit(3) + .build() + .expect("build config"), + ); + let (mut client, mut server, mut now) = connected_pair(config); + quiesce_pair(&mut client, &mut server, now); + + let mut client_key_update = None; + for i in 0..10 { + let msg = format!("client-prime-application-payload-{i}").into_bytes(); + client.send_application_data(&msg).expect("client send"); + let out = drain_outputs(&mut client).packets; + deliver_packets(&out, &mut server); + server.handle_timeout(now).expect("server timeout"); + let _ = drain_outputs(&mut server); + now += Duration::from_millis(10); + + client.handle_timeout(now).expect("client timeout"); + let out = drain_outputs(&mut client).packets; + if has_ciphertext_record_len(&out, 30) { + client_key_update = Some(out); + break; + } + } + + let mut server_key_update = None; + for i in 0..10 { + let msg = format!("server-prime-application-payload-{i}").into_bytes(); + server.send_application_data(&msg).expect("server send"); + let out = drain_outputs(&mut server).packets; + deliver_packets(&out, &mut client); + let _ = drain_outputs(&mut client); + now += Duration::from_millis(10); + + server.handle_timeout(now).expect("server timeout"); + let out = drain_outputs(&mut server).packets; + if has_ciphertext_record_len(&out, 30) { + server_key_update = Some(out); + break; + } + } + + let client_key_update = client_key_update.expect("client should initiate KeyUpdate"); + assert!( + !client_key_update.is_empty(), + "client should send its own KeyUpdate" + ); + + let server_key_update = server_key_update.expect("server should initiate KeyUpdate"); + assert!( + !server_key_update.is_empty(), + "server should send its own KeyUpdate" + ); + deliver_packets(&server_key_update, &mut client); + + client + .handle_timeout(now) + .expect("client processes peer KeyUpdate"); + let ack_only = drain_outputs(&mut client).packets; + assert_eq!( + ciphertext_record_count(&ack_only), + 1, + "client should ACK peer KeyUpdate" + ); + + now += Duration::from_millis(10); + client.handle_timeout(now).expect("client timeout"); + let response = drain_outputs(&mut client).packets; + + assert!( + response.is_empty(), + "client must not send a second KeyUpdate while the first is unacknowledged" + ); +} + +#[test] +#[cfg(feature = "rcgen")] +fn server_retransmits_final_ack_for_retransmitted_client_final_flight() { + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen client cert"); + let server_cert = generate_self_signed_certificate().expect("gen server cert"); + let config = Arc::new( + Config::builder() + .flight_retries(8) + .build() + .expect("build config"), + ); + let mut now = Instant::now(); + + let mut client = Dtls::new_13(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_13(config, server_cert, now); + server.set_active(false); + + let mut client_final_flight_seen = false; + let mut ack_dropped = false; + for round in 0..100 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + let client_out = drain_outputs(&mut client); + let server_out = drain_outputs(&mut server); + + deliver_packets(&client_out.packets, &mut server); + if client_final_flight_seen && !ack_dropped && !server_out.packets.is_empty() { + ack_dropped = true; + } else { + deliver_packets(&server_out.packets, &mut client); + } + + if !client_final_flight_seen && !client_out.packets.is_empty() && round > 2 { + client_final_flight_seen = true; + } + + if ack_dropped { + break; + } + + now += if round % 5 == 4 { + Duration::from_secs(2) + } else { + Duration::from_millis(10) + }; + } + + assert!(ack_dropped, "test should drop the server completion ACK"); + + let mut retransmitted_final_flight = Vec::new(); + for _ in 0..10 { + now += Duration::from_secs(2); + client + .handle_timeout(now) + .expect("client retransmit timeout"); + retransmitted_final_flight = drain_outputs(&mut client).packets; + if !retransmitted_final_flight.is_empty() { + break; + } + } + assert!( + !retransmitted_final_flight.is_empty(), + "client should retransmit its final flight" + ); + + deliver_packets(&retransmitted_final_flight, &mut server); + server.handle_timeout(now).expect("server timeout"); + let server_response = drain_outputs(&mut server).packets; + + assert!( + !server_response.is_empty(), + "server must retransmit its final ACK when the client final flight is retransmitted" + ); +} diff --git a/tests/dtls13/main.rs b/tests/dtls13/main.rs index 2e331868..162e8215 100644 --- a/tests/dtls13/main.rs +++ b/tests/dtls13/main.rs @@ -3,6 +3,7 @@ mod wolfssl_helper; mod common; +mod conformance; mod data; mod edge; mod fragmentation; From 28ff41a99bb545372eedac2bfe6b24f03f32722f Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Sun, 21 Jun 2026 13:49:07 +0200 Subject: [PATCH 2/5] fix: retransmit server final ack --- src/dtls13/engine.rs | 31 +++++++++++++++++++++++++------ src/dtls13/message/handshake.rs | 5 +++-- src/dtls13/server.rs | 6 ++++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/dtls13/engine.rs b/src/dtls13/engine.rs index 1cf23085..708bfe48 100644 --- a/src/dtls13/engine.rs +++ b/src/dtls13/engine.rs @@ -1253,6 +1253,10 @@ impl Engine { self.hs_recv_keys = None; } + pub fn release_application_data_retaining_handshake_keys(&mut self) { + self.release_app_data = true; + } + /// Whether a close_notify alert has been received from the peer. pub fn close_notify_received(&self) -> bool { self.close_notify_sequence.is_some() @@ -1282,6 +1286,17 @@ impl Engine { /// /// ACK format: record_numbers_length(2) + N * (epoch(8) + sequence(8)) pub fn send_ack(&mut self) -> Result<(), Error> { + self.send_ack_inner(false) + } + + pub fn send_ack_retransmittable(&mut self) -> Result<(), Error> { + if !self.received_record_numbers.is_empty() { + self.flight_clear_resends(); + } + self.send_ack_inner(true) + } + + fn send_ack_inner(&mut self, save_fragment: bool) -> Result<(), Error> { if self.received_record_numbers.is_empty() { return Ok(()); } @@ -1293,7 +1308,7 @@ impl Engine { 2 }; - self.create_ciphertext_record(ContentType::Ack, epoch, false, |fragment| { + self.create_ciphertext_record(ContentType::Ack, epoch, save_fragment, |fragment| { // record_numbers_length: 2 bytes, value = entries.len() * 16 let len = (entries.len() * 16) as u16; fragment.extend_from_slice(&len.to_be_bytes()); @@ -2361,11 +2376,6 @@ impl RecordHandler for Engine { // After KeyUpdate, epoch_bits cycles: 3→0→1→2→3→... let epoch_bits = epoch_bits as u16; - // Check handshake epoch first - if self.hs_recv_keys.is_some() && (2 & 0x03) == epoch_bits { - return 2; - } - // Check application recv epochs - return the newest (last) match // when multiple epochs share the same 2-bit value (e.g. epoch 3 and 7). let mut best = None; @@ -2374,10 +2384,19 @@ impl RecordHandler for Engine { best = Some(entry.epoch); } } + + if self.hs_recv_keys.is_some() && (2 & 0x03) == epoch_bits && !self.release_app_data { + return 2; + } + if let Some(epoch) = best { return epoch; } + if self.hs_recv_keys.is_some() && (2 & 0x03) == epoch_bits { + return 2; + } + // Default to the epoch bits value epoch_bits } diff --git a/src/dtls13/message/handshake.rs b/src/dtls13/message/handshake.rs index 7896bcf0..64d3d947 100644 --- a/src/dtls13/message/handshake.rs +++ b/src/dtls13/message/handshake.rs @@ -241,8 +241,8 @@ impl Handshake { Ok(handshake) } - // These are (unencrypted) handshakes that, when detected as - // duplicates, trigger a resend of the entire flight. + // These handshakes trigger a resend of the entire flight when detected as + // duplicates. pub fn dupe_triggers_resend(&self) -> Option { // Only trigger on the first fragment of a handshake message to avoid // multiple resends caused by fragmented duplicates of the same message. @@ -253,6 +253,7 @@ impl Handshake { let qualifies = matches!( self.header.msg_type, MessageType::ClientHello // flight 1 + | MessageType::Finished // client final flight ); qualifies.then_some(self.header.message_seq) diff --git a/src/dtls13/server.rs b/src/dtls13/server.rs index 8a028850..36aa5987 100644 --- a/src/dtls13/server.rs +++ b/src/dtls13/server.rs @@ -1133,7 +1133,7 @@ impl State { server.engine.advance_peer_handshake_seq(); // ACK the client's epoch-2 flight so it stops retransmitting - server.engine.send_ack()?; + server.engine.send_ack_retransmittable()?; // Stop flight timers - handshake complete server.engine.flight_stop_resend_timers(); @@ -1157,7 +1157,9 @@ impl State { } } - server.engine.release_application_data(); + server + .engine + .release_application_data_retaining_handshake_keys(); debug!("Handshake complete; ready for application data"); From a209bda614dccece7f24eccacc0cfb04b545666f Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Sun, 21 Jun 2026 14:02:43 +0200 Subject: [PATCH 3/5] test: update wolfssl interop dependency --- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 89cfb990..53f711b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1623,9 +1623,9 @@ checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59" [[package]] name = "wolfssl" -version = "4.1.0" +version = "7.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d92fa0dbd3219b7ace546154e915e27e61dba1267ff11e79ce71af1beeda6c1" +checksum = "a15819c1173cfd9088fcb6c31bf0412d75a51c7c9665f50b6d95cb607809d0aa" dependencies = [ "bytes", "log", @@ -1635,9 +1635,9 @@ dependencies = [ [[package]] name = "wolfssl-sys" -version = "3.1.0" +version = "4.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "906d2d45bc4afe77826d86c13c1c433a5e17bbe09dd1c0817f0a0f508a74e571" +checksum = "6b48547937d9e3ac557ff9838c0a06d0eac700512485b07e791a27b2978d450c" dependencies = [ "autotools", "bindgen", diff --git a/Cargo.toml b/Cargo.toml index 20a71a8b..f7059f60 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,4 +85,4 @@ bytes = "1" # wolfssl doesn't build on Windows [target.'cfg(not(windows))'.dev-dependencies] -wolfssl = "4.1.0" +wolfssl = "7.2.0" From 88a6d88c6ba3c8c4dd77a557b8d55ab7760c3ded Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Sun, 21 Jun 2026 14:08:33 +0200 Subject: [PATCH 4/5] fix: ack only handshake records --- src/dtls13/engine.rs | 62 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/src/dtls13/engine.rs b/src/dtls13/engine.rs index 708bfe48..5afd1901 100644 --- a/src/dtls13/engine.rs +++ b/src/dtls13/engine.rs @@ -414,7 +414,7 @@ impl Engine { // Track received record numbers for ACK generation for record in incoming.records().iter() { let seq = record.record().sequence; - if seq.epoch >= 2 { + if seq.epoch >= 2 && record.record().content_type == ContentType::Handshake { let _ = self .received_record_numbers .try_push((seq.epoch as u64, seq.sequence_number)); @@ -446,7 +446,8 @@ impl Engine { if should_replace { for record in incoming.records().iter() { let seq = record.record().sequence; - if seq.epoch >= 2 { + if seq.epoch >= 2 && record.record().content_type == ContentType::Handshake + { let _ = self .received_record_numbers .try_push((seq.epoch as u64, seq.sequence_number)); @@ -2605,6 +2606,24 @@ mod tests { packet } + fn encrypted_application_data_record(seq: u16, data: &[u8]) -> Vec { + let mut fragment = Vec::new(); + fragment.extend_from_slice(data); + fragment.push(ContentType::ApplicationData.as_u8()); + + let mut packet = Vec::new(); + packet.push( + 0b0010_0000 + | 0b0000_1000 // 2-byte sequence number. + | 0b0000_0100 // explicit length. + | 0b0000_0010, // epoch bits resolved by PassthroughRecordHandler. + ); + packet.extend_from_slice(&seq.to_be_bytes()); + packet.extend_from_slice(&(fragment.len() as u16).to_be_bytes()); + packet.extend_from_slice(&fragment); + packet + } + fn parsed_key_update(seq: u16) -> Incoming { Incoming::parse_packet( &encrypted_key_update_record(seq), @@ -2615,6 +2634,18 @@ mod tests { .expect("packet contains a record") } + fn parsed_key_update_with_app_data(key_update_seq: u16, app_seq: u16) -> Incoming { + let mut packet = encrypted_key_update_record(key_update_seq); + packet.extend_from_slice(&encrypted_application_data_record(app_seq, b"app-data")); + Incoming::parse_packet( + &packet, + &mut PassthroughRecordHandler, + Some(Dtls13CipherSuite::AES_128_GCM_SHA256), + ) + .expect("parse coalesced packet") + .expect("packet contains records") + } + /// Issue 2: Epoch-0 sequence number must have an overflow guard. /// /// Per RFC 9147 §4.2, implementations MUST NOT allow the sequence number @@ -2757,6 +2788,33 @@ mod tests { ); } + #[test] + #[cfg(feature = "rcgen")] + fn ack_tracking_ignores_non_handshake_records_in_coalesced_datagram() { + let mut engine = test_engine(); + + let incoming = parsed_key_update_with_app_data(7, 8); + assert_eq!(incoming.records().len(), 2); + assert_eq!( + incoming.records()[0].record().content_type, + ContentType::Handshake + ); + assert_eq!( + incoming.records()[1].record().content_type, + ContentType::ApplicationData + ); + + engine + .insert_incoming(incoming) + .expect("insert coalesced datagram"); + + assert_eq!( + engine.received_record_numbers.as_slice(), + &[(2, 7)], + "ACK bookkeeping must include only handshake records" + ); + } + #[test] #[cfg(feature = "rcgen")] fn malformed_ack_record_number_vector_is_ignored() { From 9c201f5211c7dafc257cd6da49f37409ab1ce5d7 Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Sun, 21 Jun 2026 14:13:43 +0200 Subject: [PATCH 5/5] docs: update changelog for DTLS conformance --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3bdafd9..6dba45b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased + * Fix DTLS 1.3 RFC 9147 conformance issues #147 * Represent DTLS wire-code identifiers as compact newtypes (breaking) #137 * Make public errors structured and fatal-only (breaking) #134