diff --git a/README.md b/README.md index 830c636d..284840b6 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,25 @@ Four constructors control which DTLS version is used: - **DTLS‑SRTP**: Exports keying material for `SRTP_AEAD_AES_256_GCM`, `SRTP_AEAD_AES_128_GCM`, and `SRTP_AES128_CM_SHA1_80` ([RFC 5764], [RFC 7714]). - **Extended Master Secret** ([RFC 7627]) is negotiated and enforced (DTLS 1.2). +- **Connection ID** ([RFC 9146]) is supported for DTLS 1.2 (extension codepoint + 54 / 0x0036). Configure with [`Config::with_connection_id`][with_cid]; + `Output::ConnectionId` is emitted when negotiation completes. The CID is a + **routing hint, not authorization to change the send address**: per RFC + 9146 §6 the caller must (a) wait for an authentication-positive signal + (e.g. `ApplicationData` from `poll_output`) before updating peer address + — `handle_packet` returning `Ok(())` is **not** proof of authentication, + since invalid records are silently discarded per RFC 6347 §4.1.2.7; (b) + require strict-monotonic (epoch, sequence_number) on the authenticated + record; (c) apply an address-reachability policy. See + [`Config::with_connection_id`][with_cid] rustdoc for the full pattern. + +### Known non-interop: pre-RFC Connection ID codepoint 53 (0x35) +Older implementations that predate RFC 9146 — notably OpenSSL before 3.2 and +some embedded stacks — use extension type `53` from +`draft-ietf-tls-dtls-connection-id-07`. dimpl implements the final RFC 9146 +codepoint (54) only, so peers stuck on the draft codepoint will fall back to +legacy framing instead of successfully negotiating CID. This is expected for +roaming PSK/IoT deployments that still ship the draft codepoint. ### Certificate model During the handshake the engine emits @@ -188,6 +207,8 @@ Rust 1.85.0 [RFC 5764]: https://www.rfc-editor.org/rfc/rfc5764 [RFC 7714]: https://www.rfc-editor.org/rfc/rfc7714 [RFC 7627]: https://www.rfc-editor.org/rfc/rfc7627 +[RFC 9146]: https://www.rfc-editor.org/rfc/rfc9146 +[with_cid]: https://docs.rs/dimpl/latest/dimpl/struct.ConfigBuilder.html#method.with_connection_id License: MIT OR Apache-2.0 diff --git a/src/auto.rs b/src/auto.rs index e1ac4cc8..2e567e28 100644 --- a/src/auto.rs +++ b/src/auto.rs @@ -39,6 +39,7 @@ const EXT_PADDING: u16 = 0x0015; const EXT_EXTENDED_MASTER_SECRET: u16 = 0x0017; const EXT_SUPPORTED_VERSIONS: u16 = 0x002B; const EXT_KEY_SHARE: u16 = 0x0033; +const EXT_CONNECTION_ID: u16 = 0x0036; const EXT_RENEGOTIATION_INFO: u16 = 0xFF01; /// A self-contained hybrid ClientHello compatible with both DTLS 1.2 and 1.3. @@ -189,7 +190,20 @@ impl HybridClientHello { ext_buf.push(0); // renegotiated_connection length = 0 ext_entries.push((EXT_RENEGOTIATION_INFO, start, ext_buf.len())); - // 9. padding: fill to MTU + // 9. connection_id (RFC 9146, DTLS 1.2 compat): emit when the caller + // configured a CID. Without this, auto-mode hybrid CH1 would omit + // the extension, causing CH1 (no CID) / CH2 (with CID) to disagree + // across HVR; `Config::build` already rejects a CID-without-DTLS-1.2 + // combination, so reaching here guarantees at least one DTLS 1.2 + // suite is offered. + if let Some(cid) = config.connection_id() { + let start = ext_buf.len(); + ext_buf.push(cid.len() as u8); + ext_buf.extend_from_slice(cid); + ext_entries.push((EXT_CONNECTION_ID, start, ext_buf.len())); + } + + // 10. padding: fill to MTU let record_header = 13usize; let handshake_header = 12usize; let body_so_far = ch_body.len() diff --git a/src/buffer.rs b/src/buffer.rs index b1856c1a..6b0c3750 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -78,6 +78,11 @@ impl Buf { self.0.resize(len, value); } + /// Shorten the buffer to the specified length, dropping trailing bytes. + pub fn truncate(&mut self, len: usize) { + self.0.truncate(len); + } + /// Convert the buffer into the underlying `Vec`. pub fn into_vec(mut self) -> Vec { std::mem::take(&mut self.0) diff --git a/src/config.rs b/src/config.rs index 02a14c01..948e5d80 100644 --- a/src/config.rs +++ b/src/config.rs @@ -76,6 +76,7 @@ pub struct Config { dtls13_cipher_suites: Option>, kx_groups: Option>, psk: Option, + connection_id: Option>, } impl Config { @@ -97,12 +98,32 @@ impl Config { dtls13_cipher_suites: None, kx_groups: None, psk: None, + connection_id: None, } } - /// Max transmission unit. + /// Max transmission unit (coalescing target for outbound datagrams). /// - /// The largest size UDP packets we will produce. + /// The implementation packs multiple DTLS records into a single + /// outbound datagram up to this size, and sizes encrypted handshake + /// fragments so each handshake record fits. Application-data writes + /// that exceed one record's plaintext capacity + /// (`DTLS12_MAX_PLAINTEXT_LEN = 2^14` bytes) are rejected with + /// [`Error::Oversized`]. Handshake fragmentation returns + /// [`Error::MtuTooSmall`] when the configured MTU cannot fit even a + /// single record of the required overhead (record header, negotiated + /// CID bytes, AEAD, handshake header). + /// + /// This is **not** a hard ceiling on every individual record: a + /// single outbound application-data record may exceed MTU when the + /// plaintext plus DTLS + AEAD + CID overhead is larger than MTU, + /// because `DTLS12_MAX_PLAINTEXT_LEN` (2^14) may be bigger than a + /// caller-chosen MTU. Set MTU above `DTLS12_MAX_PLAINTEXT_LEN + + /// record overhead` if strict per-datagram sizing matters, or + /// fragment application data caller-side. + /// + /// [`Error::Oversized`]: crate::Error::Oversized + /// [`Error::MtuTooSmall`]: crate::Error::MtuTooSmall #[inline(always)] pub fn mtu(&self) -> usize { self.mtu @@ -229,6 +250,33 @@ impl Config { } } + /// Connection ID to advertise to the peer (RFC 9146, DTLS 1.2 only). + /// + /// When set on a DTLS 1.2 or DTLS 1.2 PSK association, the endpoint + /// offers CID negotiation during the handshake and the peer includes + /// this CID in encrypted records it sends to us, allowing + /// demultiplexing by CID instead of the UDP 5-tuple. + /// + /// **Version scope.** The current CID implementation is RFC 9146 + /// (DTLS 1.2). RFC 9147 §9 defines a separate DTLS 1.3 CID + /// mechanism (`NewConnectionId` / `RequestConnectionId` + /// post-handshake messages, unified-header CID bit) which dimpl does + /// not implement. A `with_connection_id` value configured on a + /// [`Dtls::new_13`] association is silently ignored: no CID + /// negotiation, no `Output::ConnectionId` event, no `tls12_cid` + /// records on the wire. Use `with_connection_id` on the explicit + /// DTLS 1.2 / DTLS 1.2 PSK constructors (`Dtls::new_12`, + /// `Dtls::new_12_psk`); on `Dtls::new_auto` the config only takes + /// effect if the auto-sense path resolves to DTLS 1.2. + /// + /// An empty slice is valid — per RFC 9146 §3 it negotiates the + /// extension but leaves that direction on legacy RFC 6347 framing. + /// + /// [`Dtls::new_13`]: crate::Dtls::new_13 + pub fn connection_id(&self) -> Option<&[u8]> { + self.connection_id.as_deref() + } + /// Allowed DTLS 1.2 cipher suites, filtered by the config's allow-list. /// /// Returns all provider-supported DTLS 1.2 cipher suites when no filter @@ -304,13 +352,26 @@ pub struct ConfigBuilder { dtls13_cipher_suites: Option>, kx_groups: Option>, psk: Option, + connection_id: Option>, } impl ConfigBuilder { /// Set the max transmission unit (MTU). /// - /// The largest size UDP packets we will produce. + /// This is a **coalescing target** for outbound datagrams, not a hard + /// per-record ceiling — see [`Config::mtu`] for the full contract. + /// Handshake fragmentation honors this bound (returning + /// [`Error::MtuTooSmall`] if record overhead exceeds it), but a single + /// application-data record whose plaintext + CID + AEAD overhead + /// exceeds MTU will still be emitted; the only hard cap on + /// application data is [`Error::Oversized`] at + /// `DTLS12_MAX_PLAINTEXT_LEN = 2^14`. + /// /// Defaults to 1150. + /// + /// [`Config::mtu`]: crate::Config::mtu + /// [`Error::MtuTooSmall`]: crate::Error::MtuTooSmall + /// [`Error::Oversized`]: crate::Error::Oversized pub fn mtu(mut self, mtu: usize) -> Self { self.mtu = mtu; self @@ -474,6 +535,138 @@ impl ConfigBuilder { self } + /// Set the Connection ID (CID) to advertise to the peer (RFC 9146). + /// + /// The peer includes this CID in encrypted records it sends to us, so + /// roaming peers stay addressable across 5-tuple changes. CID must be + /// at most 255 bytes. + /// + /// Per RFC 9146 §3, an **empty** CID (`Vec::new()`) negotiates the + /// extension but leaves this direction on legacy RFC 6347 framing; a + /// non-empty value engages `tls12_cid` framing for records sent to us. + /// The per-direction choice is independent: the peer may choose the + /// opposite framing for records we send to it. + /// + /// `Output::ConnectionId` fires once on successful negotiation in + /// **either** case — including the zero-length case, where the + /// emitted slice is empty (`&[]`). Empty means "the peer will send + /// us records with legacy (non-`tls12_cid`) framing", so the emitted + /// bytes are not a valid routing key for that direction; do not use + /// them as a load-balancer key, since every empty-CID association + /// would route to the same bucket. + /// + /// When negotiation completes, the state machine emits + /// [`Output::ConnectionId`][output_cid] once. Poll for it in the + /// standard event loop: + /// + /// ``` + /// # #[cfg(feature = "rcgen")] + /// # { + /// use std::sync::Arc; + /// use std::time::Instant; + /// + /// use dimpl::{certificate, Config, Dtls, Output}; + /// + /// let config = Config::builder() + /// .with_connection_id(b"my-cid".to_vec()) + /// .build() + /// .unwrap(); + /// let cert = certificate::generate_self_signed_certificate().unwrap(); + /// let mut dtls = Dtls::new_12(Arc::new(config), cert, Instant::now()); + /// dtls.set_active(true); // client role + /// + /// // Standard poll-to-Timeout loop: the caller's buffer must be large + /// // enough for the CID bytes; if not, `poll_output` defers and re-emits + /// // the event on the next call. + /// let mut buf = vec![0u8; 2048]; + /// loop { + /// match dtls.poll_output(&mut buf) { + /// Output::Packet(_p) => { /* send on UDP socket */ } + /// Output::ConnectionId(cid) => { + /// // Peer will place these bytes in the CID field of its + /// // encrypted records. Use them to route incoming + /// // datagrams to this session — see "Peer address + /// // updates" below for the safe integration pattern. + /// let _ = cid; + /// } + /// Output::Timeout(_) => break, + /// _ => {} + /// } + /// } + /// # } + /// ``` + /// + /// ## Peer address updates (RFC 9146 §6) + /// + /// `Output::ConnectionId` is a **routing hint, not authorization to + /// change the send address.** dimpl is Sans-IO and never observes + /// the source address of incoming datagrams, so it cannot enforce + /// the RFC 9146 §6 conditions for replacing the peer address. Those + /// are the caller's responsibility: + /// + /// 1. **Lookup, then authenticate.** A visible CID match in an + /// incoming datagram only routes the bytes to a candidate + /// association. `Dtls::handle_packet` returning `Ok(())` is **not** + /// an authentication signal: per RFC 6347 §4.1.2.7 / RFC 9146 §6, + /// invalid records (tampered CID, failed AEAD, truncated header, + /// replay, bogus inner type) are silently discarded and still + /// surface as `Ok(())` from `handle_packet`. Before authorizing + /// an address update the caller must observe an + /// authentication-positive signal such as new `ApplicationData`, + /// handshake-state progression, or a successful keying-material + /// export — not `Ok(())` alone. + /// 2. **Newer than newest.** Per RFC 9146 §6, an authenticated CID + /// record may update the peer address only if its (epoch, + /// sequence_number) is strictly greater than the newest + /// authenticated record received so far. Stale CID records seen + /// from a different source do not authorize an update. + /// 3. **Reachability strategy.** Apply your own address-validation + /// policy (e.g. a probe round-trip) before committing the new + /// address for outbound traffic, especially for unattended IoT + /// deployments. + /// + /// Concrete pattern: sample + /// [`Dtls::newest_authenticated_record`] *before* `handle_packet`, + /// deliver the datagram, poll to `Timeout`, and re-sample the + /// accessor. A strictly-increased `(epoch, sequence_number)` plus + /// an authentication-positive output (`ApplicationData`, handshake + /// progression, or `KeyingMaterial`) proves an authenticated fresh + /// record landed — only *then* consider the address-update policy. + /// Treat `Output::ConnectionId` purely as "what bytes to look at + /// when routing" and never as "where to send next." + /// + /// [`Dtls::handle_packet`]: crate::Dtls::handle_packet + /// [`Dtls::newest_authenticated_record`]: crate::Dtls::newest_authenticated_record + /// + /// ## Privacy (RFC 9146 §8) + /// + /// CIDs are observable on the wire and are not rotated within a session. + /// Reusing the same CID across unrelated associations — or using a + /// predictable scheme (counter, timestamp, MAC address) — lets a passive + /// observer link traffic across paths and time. Prefer a freshly + /// generated, unpredictable CID per association (e.g. 8 random bytes + /// from a CSPRNG). The peer's CID advertised back in + /// [`Output::ConnectionId`][output_cid] should be treated as opaque. + /// + /// ## Alerts are the caller's responsibility + /// + /// dimpl is Sans-IO: it does not emit TLS alerts on the wire. Rejection + /// errors from CID-extension handling surface as [`Error::SecurityError`] + /// with an RFC-mandated description code the caller is expected to + /// translate into a fatal alert: + /// + /// - Unsolicited `connection_id` in ServerHello → `unsupported_extension(110)` + /// (RFC 5246 §7.4.1.4, RFC 9146 §3) + /// - Malformed `connection_id` extension body → `decode_error(50)` + /// (RFC 5246 §7.2.2) + /// + /// [output_cid]: crate::Output::ConnectionId + /// [`Error::SecurityError`]: crate::Error::SecurityError + pub fn with_connection_id(mut self, cid: Vec) -> Self { + self.connection_id = Some(cid); + self + } + /// Build the configuration. /// /// This validates the crypto provider before returning the configuration. @@ -513,6 +706,60 @@ impl ConfigBuilder { ))); } + // Validate connection_id: must be at most DTLS12_CID_MAX_LEN bytes. + // + // This is the single source of truth for the CID length invariant + // relied on throughout the DTLS 1.2 CID path: + // + // - `crypto::dtls_aead::DTLS12_CID_AAD_MAX` (23 + DTLS12_CID_MAX_LEN) + // sizes the AAD ArrayVec so the `try_extend_from_slice(cid)` unwraps + // in `Aad::new_dtls12_cid` cannot panic. + // - `dtls12::incoming::Record::decrypt_record`'s `ArrayVec` + // wire-CID buffer and its `try_extend_from_slice(...).expect(...)` + // rely on it (RFC 9146 §5.3 on-wire auth). + // - `dtls12::message::extensions::ConnectionIdExtension::{new, parse}` + // uses the same 255-byte backing store (matches the u8 length byte + // on the wire). + // - `Client::into_output` / `Server::into_output`'s poll-buffer handling + // assumes the caller's buffer can fit up to DTLS12_CID_MAX_LEN bytes + // of CID. + // + // If this ceiling ever changes, audit every site listed above. + if let Some(ref cid) = self.connection_id { + if cid.len() > crate::crypto::DTLS12_CID_MAX_LEN { + return Err(Error::ConfigError(format!( + "Connection ID length {} exceeds maximum {}", + cid.len(), + crate::crypto::DTLS12_CID_MAX_LEN + ))); + } + } + + // Round-5 review #3: `connection_id` is RFC 9146 (DTLS 1.2). If + // the caller sets a CID but the cipher-suite filter drops every + // DTLS 1.2 suite, the ClientHello would advertise CID on a + // handshake that can only succeed as DTLS 1.3, where dimpl does + // not implement RFC 9147 CID. Reject the combination at config + // build time so the mismatch is caught before the handshake. + if self.connection_id.is_some() { + let has_dtls12 = { + let mut all = crypto_provider.supported_cipher_suites(); + match &self.dtls12_cipher_suites { + Some(list) => all.any(|cs| list.contains(&cs.suite())), + None => all.next().is_some(), + } + }; + if !has_dtls12 { + return Err(Error::ConfigError( + "Connection ID is configured (RFC 9146, DTLS 1.2) but no \ + DTLS 1.2 cipher suite survives the filter. Either include \ + a DTLS 1.2 suite in `dtls12_cipher_suites` or drop \ + `with_connection_id`." + .to_string(), + )); + } + } + // Validate aead_encryption_limit: must be at least 1 if self.aead_encryption_limit == 0 { return Err(Error::ConfigError( @@ -621,6 +868,7 @@ impl ConfigBuilder { dtls13_cipher_suites: self.dtls13_cipher_suites, kx_groups: self.kx_groups, psk: self.psk, + connection_id: self.connection_id, }) } } @@ -671,6 +919,7 @@ impl fmt::Debug for Config { .field("dtls13_cipher_suites", &self.dtls13_cipher_suites) .field("kx_groups", &self.kx_groups) .field("psk", &self.psk) + .field("connection_id", &self.connection_id) .finish() } } @@ -696,6 +945,7 @@ impl fmt::Debug for ConfigBuilder { .field("dtls13_cipher_suites", &self.dtls13_cipher_suites) .field("kx_groups", &self.kx_groups) .field("psk", &self.psk) + .field("connection_id", &self.connection_id) .finish() } } diff --git a/src/crypto/dtls_aead.rs b/src/crypto/dtls_aead.rs index f9a18662..cde20a58 100644 --- a/src/crypto/dtls_aead.rs +++ b/src/crypto/dtls_aead.rs @@ -134,19 +134,61 @@ impl Nonce { } } +/// Maximum length of a DTLS 1.2 Connection ID (RFC 9146). +/// +/// RFC 9146 caps CID length at a single byte. `Config::build` rejects +/// `connection_id` longer than this, and `ConnectionIdExtension::parse` +/// enforces the same ceiling via its `ArrayVec` backing store. +pub const DTLS12_CID_MAX_LEN: usize = 255; + +/// Maximum capacity for a DTLS AAD buffer. +/// +/// 23 fixed bytes (DTLS 1.2 CID AAD header, RFC 9146 §5) + up to 255 CID bytes. +/// Sizing for the worst case keeps the `try_extend_from_slice(cid)` paths +/// panic-free for any config-validated CID without a runtime bound check. +/// Also covers DTLS 1.2 (13 bytes) and DTLS 1.3 (3-5 bytes) AAD forms. +pub const DTLS12_CID_AAD_MAX: usize = 23 + DTLS12_CID_MAX_LEN; + /// Additional Authenticated Data for DTLS records. /// -/// Variable-length to support both DTLS 1.2 (13 bytes) and DTLS 1.3 (3-5 bytes). +/// Variable-length to support DTLS 1.2 (13 bytes), DTLS 1.2 with CID (23+N bytes), +/// and DTLS 1.3 (3-5 bytes). #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Aad(pub ArrayVec); +pub struct Aad(ArrayVec); + +impl Aad { + /// Borrow the AAD as a byte slice for AEAD operations. + pub fn as_slice(&self) -> &[u8] { + &self.0 + } +} + +impl std::ops::Deref for Aad { + type Target = [u8]; + fn deref(&self) -> &Self::Target { + &self.0 + } +} impl Aad { /// Create Additional Authenticated Data for a DTLS 1.2 record. - pub(crate) fn new_dtls12(content_type: ContentType, sequence: Sequence, length: u16) -> Self { + /// + /// `version` is the 2-byte wire version field observed in the record + /// header (e.g. `[0xFE, 0xFD]` for DTLS 1.2, `[0xFE, 0xFF]` for + /// DTLS 1.0). RFC 6347 §4.1.2.1 names this slot `DTLSCompressed.version` + /// — the bytes that were actually transmitted — so the sender and + /// receiver must bind the same bytes or AEAD authentication fails. + pub(crate) fn new_dtls12( + content_type: ContentType, + sequence: Sequence, + version: [u8; 2], + length: u16, + ) -> Self { let mut aad = ArrayVec::new(); // First set the full 8-byte sequence number let seq_bytes = sequence.sequence_number.to_be_bytes(); + // unwrap: total DTLS 1.2 AAD is 13 bytes, well within DTLS12_CID_AAD_MAX. aad.try_extend_from_slice(&seq_bytes).unwrap(); // Overwrite the first 2 bytes with epoch @@ -157,16 +199,85 @@ impl Aad { // Content type at index 8 aad.push(content_type.as_u8()); - // Protocol version bytes (major:minor) at indexes 9-10 - aad.push(0xfe); // DTLS 1.2 major version - aad.push(0xfd); // DTLS 1.2 minor version + // Protocol version bytes (major:minor) at indexes 9-10, from the wire + aad.push(version[0]); + aad.push(version[1]); // Payload length (2 bytes) at indexes 11-12 + // unwrap: same capacity argument as the seq_bytes extend above. aad.try_extend_from_slice(&length.to_be_bytes()).unwrap(); Aad(aad) } + /// Create Additional Authenticated Data for a DTLS 1.2 CID record (RFC 9146 §5). + /// + /// `version` is the 2-byte wire `DTLSCiphertext.version` field from the + /// record header. Per RFC 9146 §5 the AAD binds the exact bytes the + /// sender put on the wire, so a peer that emits `0xFE 0xFF` (DTLS 1.0) + /// requires `[0xFE, 0xFF]` here — hardcoding `0xFE 0xFD` would silently + /// fail AEAD against such a peer. + /// + /// AAD layout: + /// ```text + /// seq_num_placeholder(8, 0xFF) | tls12_cid(1) | cid_length(1) | + /// tls12_cid(1) | version(2) | epoch(2) | sequence_number(6) | + /// cid(N) | length_of_DTLSInnerPlaintext(2) + /// ``` + pub(crate) fn new_dtls12_cid( + sequence: Sequence, + version: [u8; 2], + cid: &[u8], + inner_plaintext_len: u16, + ) -> Self { + let mut aad = ArrayVec::new(); + + // 8 bytes of 0xFF as sequence number placeholder. + // unwrap: fixed 8 bytes, well within DTLS12_CID_AAD_MAX. + aad.try_extend_from_slice(&[0xFF; 8]).unwrap(); + + // tls12_cid content type (25) + aad.push(ContentType::Tls12Cid.as_u8()); + + // CID length (1 byte). + // `cid.len() as u8` is lossless: `Config::build` rejects CID > 255 + // bytes, and `ConnectionIdExtension::parse` enforces the same 255 + // ceiling via its `ArrayVec` backing store, so the value + // always fits in u8. + aad.push(cid.len() as u8); + + // tls12_cid content type again (25) + aad.push(ContentType::Tls12Cid.as_u8()); + + // Wire version bytes from the record header. + aad.push(version[0]); + aad.push(version[1]); + + // First set the full 8-byte sequence number. + // unwrap: at this point 13 bytes are consumed, adding 8 stays within DTLS12_CID_AAD_MAX. + let seq_bytes = sequence.sequence_number.to_be_bytes(); + aad.try_extend_from_slice(&seq_bytes).unwrap(); + + // Overwrite the first 2 bytes with epoch + let epoch_bytes = sequence.epoch.to_be_bytes(); + aad[13] = epoch_bytes[0]; + aad[14] = epoch_bytes[1]; + + // CID (N bytes). + // unwrap: 21 fixed bytes are consumed by now, `cid.len() <= + // DTLS12_CID_MAX_LEN` by config validation, and remaining capacity + // is DTLS12_CID_AAD_MAX - 21 = 257, so the extend always succeeds. + aad.try_extend_from_slice(cid).unwrap(); + + // Length of DTLSInnerPlaintext (2 bytes). + // unwrap: worst case is 21 + DTLS12_CID_MAX_LEN + 2 = + // DTLS12_CID_AAD_MAX which matches capacity exactly — still fits. + aad.try_extend_from_slice(&inner_plaintext_len.to_be_bytes()) + .unwrap(); + + Aad(aad) + } + /// Create Additional Authenticated Data for a DTLS 1.3 record. /// /// The AAD is the raw unified header bytes (3-5 bytes). @@ -204,4 +315,40 @@ mod tests { assert!(plaintext_len_from_fragment_len(3).is_none()); assert!(plaintext_len_from_fragment_len(DTLS_AEAD_OVERHEAD - 1).is_none()); } + + #[test] + fn aad_new_dtls12_cid_layout() { + // RFC 9146 §5 AAD layout verification + let sequence = Sequence { + epoch: 1, + sequence_number: 42, + }; + let cid = b"test-cid"; + let inner_plaintext_len: u16 = 100; + + let aad = Aad::new_dtls12_cid(sequence, [0xFE, 0xFD], cid, inner_plaintext_len); + let bytes = aad.as_slice(); + + // Total: 8 + 1 + 1 + 1 + 2 + 2 + 6 + 8 + 2 = 31 bytes + assert_eq!(bytes.len(), 23 + cid.len()); + + // seq_num_placeholder: 8 bytes of 0xFF + assert_eq!(&bytes[0..8], &[0xFF; 8]); + // tls12_cid content type + assert_eq!(bytes[8], 25); + // cid_length + assert_eq!(bytes[9], cid.len() as u8); + // tls12_cid content type again + assert_eq!(bytes[10], 25); + // version: DTLS 1.2 = {0xFE, 0xFD} + assert_eq!(&bytes[11..13], &[0xFE, 0xFD]); + // epoch + assert_eq!(&bytes[13..15], &[0x00, 0x01]); + // sequence_number (6 bytes) + assert_eq!(&bytes[15..21], &[0x00, 0x00, 0x00, 0x00, 0x00, 42]); + // CID + assert_eq!(&bytes[21..21 + cid.len()], cid); + // inner_plaintext_len + assert_eq!(&bytes[21 + cid.len()..], &inner_plaintext_len.to_be_bytes()); + } } diff --git a/src/crypto/mod.rs b/src/crypto/mod.rs index 68ee7109..faacfa84 100644 --- a/src/crypto/mod.rs +++ b/src/crypto/mod.rs @@ -26,6 +26,7 @@ pub use keying::{KeyingMaterial, SrtpProfile}; pub use dtls_aead::{Aad, Nonce}; // Re-export internal AEAD constants/types for crate-internal use +pub(crate) use dtls_aead::DTLS12_CID_MAX_LEN; pub(crate) use dtls_aead::Iv; // Re-export buffer types for provider trait implementations @@ -47,13 +48,6 @@ pub use crate::types::{ Dtls13CipherSuite, HashAlgorithm, NamedGroup, SignatureAlgorithm, SignatureScheme, }; -impl Deref for Aad { - type Target = [u8]; - fn deref(&self) -> &Self::Target { - &self.0[..] - } -} - impl Deref for Nonce { type Target = [u8]; fn deref(&self) -> &Self::Target { diff --git a/src/crypto/prf_hkdf.rs b/src/crypto/prf_hkdf.rs index 64a886c1..050210a2 100644 --- a/src/crypto/prf_hkdf.rs +++ b/src/crypto/prf_hkdf.rs @@ -212,7 +212,7 @@ mod tests { use super::*; fn hex_to_vec(hex: &str) -> Vec { - let hex = hex.replace(' ', "").replace('\n', ""); + let hex = hex.replace([' ', '\n'], ""); let mut v = Vec::new(); for i in 0..hex.len() / 2 { // unwrap: test-only hex parsing diff --git a/src/crypto/validation/mod.rs b/src/crypto/validation/mod.rs index 7d4a7bea..2cdc4d16 100644 --- a/src/crypto/validation/mod.rs +++ b/src/crypto/validation/mod.rs @@ -3,8 +3,6 @@ //! This module defines the validation rules for crypto providers used with dimpl, //! based on the documented support in lib.rs. -use arrayvec::ArrayVec; - use super::{Aad, CryptoProvider, Nonce, SupportedDtls12CipherSuite, SupportedKxGroup}; use crate::Error; use crate::buffer::{Buf, TmpBuf}; @@ -299,10 +297,7 @@ impl CryptoProvider { })?; let nonce = Nonce(tv.nonce); - let mut aad_vec = ArrayVec::new(); - // unwrap: AAD is at most 12 bytes, well within capacity 13 - aad_vec.try_extend_from_slice(tv.aad).unwrap(); - let aad = Aad(aad_vec); + let aad = Aad::new_dtls13(tv.aad); // Encrypt let mut cipher = cs.create_cipher(tv.key).map_err(|e| { diff --git a/src/dtls12/client.rs b/src/dtls12/client.rs index 9357a7e2..4a755578 100644 --- a/src/dtls12/client.rs +++ b/src/dtls12/client.rs @@ -26,9 +26,10 @@ use crate::dtls12::engine::Engine; use crate::dtls12::message::{Body, CipherSuiteVec, ClientHello, ClientKeyExchange}; use crate::dtls12::message::{ClientPskKeys, ServerKeyExchangeParams}; use crate::dtls12::message::{CompressionMethod, ContentType, Cookie}; +use crate::dtls12::message::{ConnectionIdExtension, Random, SessionId}; use crate::dtls12::message::{DigitallySigned, Dtls12CipherSuite}; use crate::dtls12::message::{ExtensionType, KeyExchangeAlgorithm, MessageType, ProtocolVersion}; -use crate::dtls12::message::{Random, SessionId, SignatureAndHashAlgorithm, UseSrtpExtension}; +use crate::dtls12::message::{SignatureAndHashAlgorithm, UseSrtpExtension}; use crate::{Config, DtlsCertificate, Error, KeyingMaterial, Output}; /// DTLS client @@ -56,6 +57,9 @@ pub struct Client { /// The negotiated SRTP profile (if any) negotiated_srtp_profile: Option, + /// Peer's Connection ID (from ServerHello extension). We put this in records we send. + peer_cid: Option>, + /// Server random. Set by ServerHello. server_random: Option, @@ -83,9 +87,10 @@ pub struct Client { } #[derive(Debug, PartialEq, Eq)] -pub(crate) enum LocalEvent { +pub enum LocalEvent { PeerCert, Connected, + ConnectionId(Vec), KeyingMaterial(ArrayVec, SrtpProfile), } @@ -101,6 +106,7 @@ impl Client { cookie: None, extension_data: Buf::new(), negotiated_srtp_profile: None, + peer_cid: None, server_random: None, server_certificates: Vec::with_capacity(3), defragment_buffer: Buf::new(), @@ -165,6 +171,7 @@ impl Client { cookie: None, extension_data: Buf::new(), negotiated_srtp_profile: None, + peer_cid: None, server_random: None, server_certificates: Vec::with_capacity(3), defragment_buffer: Buf::new(), @@ -178,7 +185,22 @@ impl Client { Ok(client) } + /// Borrow the underlying DTLS 1.2 engine (crate-internal accessor). + pub(crate) fn engine(&self) -> &Engine { + &self.engine + } + pub fn into_server(self) -> Server { + // Role flip must happen before handshake completion — see + // `Dtls::set_active` rustdoc. CID state is labelled for the + // original direction and does not survive a role swap + // meaningfully. Debug-build guard: any negotiated CID implies a + // post-handshake flip, which is unsupported. + debug_assert!( + self.engine.inbound_cid().is_none(), + "Dtls::set_active role flip after CID negotiation is unsupported; \ + CID labels are tied to the original role. Use a fresh Dtls instance." + ); Server::new_with_engine(self.engine, self.last_now) } @@ -193,6 +215,23 @@ impl Client { } pub fn poll_output<'a>(&mut self, buf: &'a mut [u8]) -> Output<'a> { + // `Output::ConnectionId` borrows from `buf`. If the caller's buffer + // can't hold the negotiated CID, defer the notification — leave the + // event at the front of the queue and fall through to the engine. + // The caller will see `Timeout`, retry `poll_output` with a larger + // buffer, and the ConnectionId event is delivered then. This keeps + // the CID plumbing panic-free. + if let Some(LocalEvent::ConnectionId(cid)) = self.local_events.front() { + if cid.len() > buf.len() { + warn!( + "poll_output buffer too small for ConnectionId ({} bytes < {} bytes); deferring", + buf.len(), + cid.len() + ); + return self.engine.poll_output(buf, self.last_now); + } + } + if let Some(event) = self.local_events.pop_front() { return event.into_output(buf, &self.server_certificates); } @@ -338,7 +377,14 @@ impl State { fn send_client_hello(self, client: &mut Client) -> Result { let session_id = client.session_id.unwrap_or_else(SessionId::empty); let cookie = client.cookie.unwrap_or_else(Cookie::empty); - // unwrap: is ok because we set the random in handle_timeout + // `handle_timeout` normally sets `client.random` before this state, + // but `handle_packet → make_progress` can also reach `SendClientHello` + // (e.g. when a passive client processes an early stray datagram + // that silent-discards before the first user-driven `handle_timeout` + // tick). Initialize lazily so that path is panic-free. + if client.random.is_none() { + client.random = Some(Random::new(&mut client.engine.rng)); + } let random = client.random.unwrap(); // Determine flight number: 1 for initial CH, 3 for retransmit with cookie @@ -517,6 +563,33 @@ impl State { extended_master_secret = true; trace!("Server negotiated Extended Master Secret"); } + + if extension.extension_type == ExtensionType::ConnectionId { + // RFC 9146 §3 + RFC 5246 §7.4.1.4: a server MUST NOT send an + // extension the client did not offer. If our config disabled + // CID, treat a ServerHello connection_id as unsupported_extension. + if client.engine.config().connection_id().is_none() { + return Err(Error::SecurityError( + "Server sent unsolicited connection_id extension".to_string(), + )); + } + let extension_data = extension.extension_data(&client.defragment_buffer); + match ConnectionIdExtension::parse(extension_data) { + Ok((_, cid_ext)) => { + client.peer_cid = Some(cid_ext.cid.to_vec()); + trace!( + "Server negotiated Connection ID ({} bytes)", + cid_ext.cid.len() + ); + } + Err(_) => { + // RFC 5246 §7.2.2 decode_error on malformed extension. + return Err(Error::SecurityError( + "Malformed connection_id extension from server".to_string(), + )); + } + } + } } // Without extended master secret, in DTLS1.2 a security attack @@ -532,6 +605,16 @@ impl State { } trace!("Extended Master Secret enabled"); + // A server MUST NOT send connection_id unless we offered it (RFC 9146 §3), + // so require our config to also have a CID before enabling. + // `set_cid_negotiated` arms outbound CID immediately; inbound is armed + // separately when the peer's ChangeCipherSpec is processed. + let our_cid = client.engine.config().connection_id().map(|c| c.to_vec()); + if let (Some(peer_cid), Some(our_cid)) = (client.peer_cid.clone(), our_cid) { + client.engine.set_cid_negotiated(our_cid, peer_cid); + debug!("Connection ID negotiated"); + } + // PSK suites skip Certificate; go directly to ServerKeyExchange if cs.is_psk() { Ok(Self::AwaitServerKeyExchange) @@ -608,7 +691,6 @@ impl State { return Ok(self); }; - // Extract all ranges/data we need before accessing buffer let (signature_range, signature_algorithm, curve_type, named_group, public_key_range) = { let handshake = maybe.as_ref().unwrap(); let Body::ServerKeyExchange(server_key_exchange) = &handshake.body else { @@ -1132,9 +1214,14 @@ impl State { // Receiving server Finished implicitly acks our Flight 5; stop resends client.engine.flight_stop_resend_timers(); - // Emit Connected event client.local_events.push_back(LocalEvent::Connected); + if let Some(cid) = client.engine.inbound_cid() { + client + .local_events + .push_back(LocalEvent::ConnectionId(cid.to_vec())); + } + // Extract and emit SRTP keying material if we have a negotiated profile if let Some(profile) = client.negotiated_srtp_profile { let suite_hash = client.engine.cipher_suite().unwrap().hash_algorithm(); @@ -1381,6 +1468,15 @@ impl LocalEvent { Output::PeerCert(&buf[..l]) } LocalEvent::Connected => Output::Connected, + LocalEvent::ConnectionId(cid) => { + // `poll_output` peeks this event and defers if `buf` can't hold + // the CID, so this path only runs when the size check passed. + // Truncate defensively — never panic — if that invariant is + // ever violated by a future refactor. + let l = cid.len().min(buf.len()); + buf[..l].copy_from_slice(&cid[..l]); + Output::ConnectionId(&buf[..l]) + } LocalEvent::KeyingMaterial(m, profile) => { Output::KeyingMaterial(KeyingMaterial::new(&m), profile) } diff --git a/src/dtls12/engine.rs b/src/dtls12/engine.rs index 42f5db2d..885d5e70 100644 --- a/src/dtls12/engine.rs +++ b/src/dtls12/engine.rs @@ -16,6 +16,18 @@ use crate::{Config, Error, Output, SeededRng}; const MAX_DEFRAGMENT_PACKETS: usize = 50; +/// Maximum DTLS 1.2 `DTLSPlaintext` / `DTLSInnerPlaintext` length in bytes +/// (RFC 6347 §4.1.1 and RFC 9146 §5: MUST NOT exceed `2^14`). Enforced +/// before encryption so the `u16` length cast in the wire record cannot +/// wrap and so AEAD AAD carries a length that matches the RFC ceiling. +pub(crate) const DTLS12_MAX_PLAINTEXT_LEN: usize = 1 << 14; + +/// Maximum DTLS 1.2 record sequence number (48 bits on the wire per +/// RFC 6347 §4.1). Incrementing past this MUST trigger rekey / +/// renegotiation; since dimpl rejects both, we return `Oversized` and +/// close the association. +pub(crate) const DTLS12_MAX_SEQUENCE_NUMBER: u64 = (1u64 << 48) - 1; + // Using debug_ignore_primary since CryptoContext doesn't implement Debug pub struct Engine { config: Arc, @@ -94,6 +106,101 @@ pub struct Engine { /// Whether [`Output::CloseNotify`] has already been emitted. close_notify_reported: bool, + + /// Connection ID (RFC 9146) state: negotiated values and per-direction + /// activation liveness. Kept as a single private field so the valid + /// combinations are unrepresentable outside of this module. Mutators: + /// `set_cid_negotiated` (arms outbound synchronously at negotiation + /// completion) and `activate_inbound_cid` (flips inbound live, called + /// from `enable_peer_encryption` after the peer's ChangeCipherSpec). + /// Reads: `inbound_cid`, `CidState::inbound_framing`, + /// `CidState::inbound_if_live`, `CidState::outbound_if_live`. + cid: CidState, +} + +/// DTLS 1.2 Connection ID negotiation + activation state (RFC 9146). +/// +/// RFC 9146 §3 treats a zero-length CID advertised for a direction as +/// "stay on legacy RFC 6347 framing in that direction". `tls12_cid` framing +/// is reserved for directions whose negotiated CID is **non-empty**. The +/// stored `inbound`/`outbound` bytes preserve the negotiated value for +/// reporting, but the framing-relevant queries (`inbound_if_live`, +/// `outbound_if_live`) return `Some` only when the direction has a +/// non-empty value AND has been armed (outbound at negotiation, inbound +/// after the peer's ChangeCipherSpec). +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum CidState { + /// CID extension was not negotiated (either side declined). + None, + /// CID extension was negotiated. Either direction may still be on legacy + /// framing if its negotiated CID is zero-length. + Negotiated { + /// What the peer writes into the CID field of records sent to us. + /// Empty = peer asked for legacy framing inbound. + inbound: Vec, + /// What we write into the CID field of records sent to the peer. + /// Empty = we asked for legacy framing outbound. + outbound: Vec, + /// True once the peer's ChangeCipherSpec has been processed — the + /// gate for accepting CID-framed inbound records (no effect if + /// `inbound` is empty, since that direction stays on legacy framing). + inbound_armed: bool, + /// True once we arm the outbound direction — flipped at negotiation + /// completion (no effect if `outbound` is empty). + outbound_armed: bool, + }, +} + +impl CidState { + /// Inbound CID value if the extension was negotiated, regardless of + /// activation or length. Used for `Output::ConnectionId` reporting. + fn inbound(&self) -> Option<&[u8]> { + match self { + CidState::Negotiated { inbound, .. } => Some(inbound), + CidState::None => None, + } + } + + /// Inbound CID value iff inbound direction is *framing-live* — armed + /// AND non-empty. Zero-length inbound stays on legacy framing (RFC 9146 + /// §3), so it returns `None` here. + fn inbound_if_live(&self) -> Option<&[u8]> { + match self { + CidState::Negotiated { + inbound, + inbound_armed: true, + .. + } if !inbound.is_empty() => Some(inbound), + _ => None, + } + } + + /// Outbound CID value iff outbound direction is *framing-live* — armed + /// AND non-empty. Zero-length outbound stays on legacy framing. + fn outbound_if_live(&self) -> Option<&[u8]> { + match self { + CidState::Negotiated { + outbound, + outbound_armed: true, + .. + } if !outbound.is_empty() => Some(outbound), + _ => None, + } + } + + /// Negotiated inbound CID iff it is non-empty — the value the peer puts + /// in incoming `tls12_cid` records. Distinct from `inbound_if_live` in + /// that it does not require the peer's CCS: records reordered ahead of + /// CCS are still framed with this CID and must parse with its length. + /// The receive path uses this both to route incoming `tls12_cid` + /// records and to reject legacy-framed epoch-1 records when CID framing + /// is expected (RFC 9146 §3). + fn inbound_framing(&self) -> Option<&[u8]> { + match self { + CidState::Negotiated { inbound, .. } if !inbound.is_empty() => Some(inbound), + _ => None, + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -144,6 +251,7 @@ impl Engine { release_app_data: false, close_notify_received: false, close_notify_reported: false, + cid: CidState::None, } } @@ -181,6 +289,72 @@ impl Engine { self.explicit_nonce_len + self.tag_len } + /// Total wire-expansion overhead for an outbound record before the + /// plaintext body. Includes the record header, any outbound CID bytes + /// (only when outbound CID is live at the requested epoch), the + /// DTLSInnerPlaintext real-type byte for CID records (RFC 9146 §5), and + /// AEAD overhead for encrypted epochs. + /// + /// Shared by both `create_record` (datagram packing) and + /// `create_handshake` (handshake fragmentation). Keeping the two paths + /// in sync is the whole point: handshake fragmentation sizes chunks + /// against this same overhead so a fragment can't exceed MTU once CID + /// adds its bytes back in. + pub(crate) fn outbound_record_overhead(&self, epoch: u16) -> usize { + let cid_len = if epoch >= 1 { + self.cid.outbound_if_live().map_or(0, |c| c.len()) + } else { + 0 + }; + let aead = if epoch >= 1 { self.aead_overhead() } else { 0 }; + // CID records carry a DTLSInnerPlaintext real_type byte after the + // plaintext body, which is then covered by AEAD. + let inner = if cid_len > 0 { 1 } else { 0 }; + DTLSRecord::HEADER_LEN + cid_len + inner + aead + } + + /// Record that CID negotiation completed. `inbound` is the value the peer + /// writes into records sent to us; `outbound` is what we write into + /// records sent to the peer. Either may be empty — per RFC 9146 §3 an + /// empty value for a direction means "stay on legacy RFC 6347 framing in + /// that direction". Outbound is armed immediately; inbound arms on the + /// peer's ChangeCipherSpec via `activate_inbound_cid`. + pub fn set_cid_negotiated(&mut self, inbound: Vec, outbound: Vec) { + self.cid = CidState::Negotiated { + inbound, + outbound, + inbound_armed: false, + outbound_armed: true, + }; + } + + /// Arm the inbound direction. No-op if CID was not negotiated. If the + /// negotiated inbound CID is empty, this still flips the flag but + /// `inbound_if_live` stays `None` because zero-length inbound keeps + /// legacy framing. + pub fn activate_inbound_cid(&mut self) { + if let CidState::Negotiated { inbound_armed, .. } = &mut self.cid { + *inbound_armed = true; + } + } + + /// Inbound CID value if negotiated (regardless of arming or length). + /// `Some(&[])` means the extension was negotiated but this direction + /// stays on legacy framing. + pub fn inbound_cid(&self) -> Option<&[u8]> { + self.cid.inbound() + } + + /// The highest authenticated sequence number observed in the + /// current peer epoch's replay window, or `None` if no record has + /// been authenticated yet. Callers using CID for RFC 9146 §6 + /// peer-address updates should require this value to advance + /// strictly between the "old address" and "new address" + /// observations before committing the update. + pub fn newest_authenticated_sequence(&self) -> Option { + self.replay.max_seq() + } + /// Is the given cipher suite allowed by configuration pub fn is_cipher_suite_allowed(&self, suite: Dtls12CipherSuite) -> bool { self.config @@ -400,12 +574,14 @@ impl Engine { let fragment = next.record().fragment(record_buffer); let len = fragment.len(); - assert!( - len <= buf.len(), - "Output buffer too small for application data {} > {}", - len, - buf.len() - ); + if len > buf.len() { + // Defer: leave the record unhandled so a subsequent poll with a + // larger buffer delivers it. Keeps `poll_output` panic-free when + // the caller supplies an undersized buffer, matching the + // defer-and-retry contract used by LocalEvent::ConnectionId at + // the client/server wrapper layer (plan §5 poll-buffer safety). + return Err(buf); + } buf[..len].copy_from_slice(fragment); next.set_handled(); @@ -429,16 +605,19 @@ impl Engine { } fn poll_packet_tx<'a>(&mut self, buf: &'a mut [u8]) -> Result<&'a [u8], &'a mut [u8]> { - let Some(p) = self.queue_tx.pop_front() else { + // Peek the front packet's size first. If the caller's buffer can't + // hold it, defer — leave the packet queued and fall through so a + // subsequent poll with a larger buffer can deliver it. Panic-free + // poll_output per plan §5 poll-buffer safety, matching the + // defer-and-retry contract used by LocalEvent::ConnectionId. + let Some(front_len) = self.queue_tx.front().map(|b| b.len()) else { return Err(buf); }; - - assert!( - p.len() <= buf.len(), - "Output buffer too small for packet {} > {}", - p.len(), - buf.len() - ); + if front_len > buf.len() { + return Err(buf); + } + // unwrap: front() just returned Some, queue is single-threaded. + let p = self.queue_tx.pop_front().unwrap(); let len = p.len(); buf[..len].copy_from_slice(&p); @@ -649,6 +828,20 @@ impl Engine { // Let the caller fill the fragment (plaintext) f(&mut fragment); + // RFC 6347 §4.1.1 / RFC 9146 §5: DTLSPlaintext and + // DTLSInnerPlaintext are capped at 2^14 bytes. When CID is active, + // the inner-plaintext length is `fragment.len() + 1` (the + // real_type byte), so the plaintext body itself must leave room + // for that. Reject here before any `u16` cast downstream can wrap. + let inner_plaintext_len = if self.cid.outbound_if_live().is_some() && epoch >= 1 { + fragment.len() + 1 + } else { + fragment.len() + }; + if inner_plaintext_len > DTLS12_MAX_PLAINTEXT_LEN { + return Err(Error::Oversized(fragment.len())); + } + // Use this as a marker to know whether we are to record fragments for resends. if save_fragment { let mut clone = self.buffers_free.pop(); @@ -660,14 +853,26 @@ impl Engine { }); } - // Compute wire length of the record if serialized into a datagram - // Record header (13) + handshake/change/app data bytes + AEAD overhead (if epoch >= 1) - let overhead = if maybe_suite.is_some() { - self.aead_overhead() + // CID framing applies only to encrypted records (epoch >= 1) AND only + // when outbound CID is actively engaged. `outbound_cid_if_live()` + // returns Some iff both conditions are met at the state layer. + // Cloned into an owned `Vec` so it survives across the `&mut self` + // encrypt call below without borrowing the CID state. + let outbound_cid: Option> = if epoch >= 1 { + self.cid.outbound_if_live().map(|c| c.to_vec()) } else { - 0 + None }; - let record_wire_len = DTLSRecord::HEADER_LEN + fragment.len() + overhead; + let use_cid = outbound_cid.is_some(); + let cid_len = outbound_cid.as_ref().map_or(0, |c| c.len()); + + // Compute wire length of the record if serialized into a datagram. + // `outbound_record_overhead` accounts for record header, outbound CID + // bytes, the DTLSInnerPlaintext real_type byte (if CID), and AEAD + // overhead — matching the same expansion `create_handshake` sizes + // fragments against. + let record_wire_len = self.outbound_record_overhead(epoch) + fragment.len(); + let _ = (use_cid, cid_len); // retained as local scope; use is inline via outbound_cid // Decide whether to append to the existing last datagram or create a new one let can_append = self @@ -686,12 +891,24 @@ impl Engine { return Err(Error::TransmitQueueFull); } - // Sequence number to use for this record + // Sequence number to use for this record. RFC 6347 §4.1: + // implementations MUST abandon the association or rehandshake + // before sequence-number wrap. dimpl rejects both renegotiation + // and rekey, so hitting the 48-bit ceiling is terminal and the + // error is distinct from payload-size failures (on 32-bit targets + // the previous `Oversized(u64 as usize)` would also lose + // information). let sequence = if epoch == 0 { self.sequence_epoch_0 } else { self.sequence_epoch_n }; + if sequence.sequence_number > DTLS12_MAX_SEQUENCE_NUMBER { + return Err(Error::SequenceNumberExhausted { + epoch: sequence.epoch, + sequence: sequence.sequence_number, + }); + } let length = fragment.len() as u16; // Handle encryption for epochs >= 1 @@ -729,8 +946,17 @@ impl Engine { } }; - // DTLS 1.2 AEAD: AAD uses the plaintext length (DTLSCompressed.length). - let aad = Aad::new_dtls12(content_type, sequence, length); + // Wire version emitted on the send path is always DTLS 1.2. + let wire_version = ProtocolVersion::DTLS1_2.as_u16().to_be_bytes(); + let aad = if let Some(peer_cid) = outbound_cid.as_deref() { + // DTLSInnerPlaintext wrapping: append the real content type byte + fragment.push(content_type.as_u8()); + let inner_plaintext_len = fragment.len() as u16; + Aad::new_dtls12_cid(sequence, wire_version, peer_cid, inner_plaintext_len) + } else { + // DTLS 1.2 AEAD: AAD uses the plaintext length (DTLSCompressed.length). + Aad::new_dtls12(content_type, sequence, wire_version, length) + }; // Encrypt the fragment in-place self.encrypt_data(&mut fragment, aad, nonce)?; @@ -745,9 +971,16 @@ impl Engine { } } + // Outer content type: Tls12Cid when CID is active, otherwise the real type + let outer_content_type = if use_cid { + ContentType::Tls12Cid + } else { + content_type + }; + // Build the record structure referencing the (possibly encrypted) fragment let record = DTLSRecord { - content_type, + content_type: outer_content_type, version: ProtocolVersion::DTLS1_2, sequence, length: fragment.len() as u16, @@ -761,14 +994,29 @@ impl Engine { self.sequence_epoch_n.sequence_number += 1; } - // Serialize the record into the chosen datagram buffer + // Serialize the record into the chosen datagram buffer. + // For CID records, insert the peer CID between sequence and length. + let serialize = |record: &DTLSRecord, fragment: &[u8], output: &mut Buf| { + if let Some(peer_cid) = outbound_cid.as_deref() { + output.push(record.content_type.as_u8()); + ProtocolVersion::DTLS1_2.serialize(output); + output.extend_from_slice(&record.sequence.epoch.to_be_bytes()); + output.extend_from_slice(&record.sequence.sequence_number.to_be_bytes()[2..]); + output.extend_from_slice(peer_cid); + output.extend_from_slice(&record.length.to_be_bytes()); + output.extend_from_slice(fragment); + } else { + record.serialize(fragment, output); + } + }; + if can_append { let last = self.queue_tx.back_mut().unwrap(); - record.serialize(&fragment, last); + serialize(&record, &fragment, last); } else { let mut buffer = self.buffers_free.pop(); buffer.clear(); - record.serialize(&fragment, &mut buffer); + serialize(&record, &fragment, &mut buffer); self.queue_tx.push_back(buffer); } @@ -823,13 +1071,10 @@ impl Engine { // Handshake header is 12 bytes let handshake_header_len = 12usize; - let aead_overhead = if epoch >= 1 { + if epoch >= 1 { self.cipher_suite() .ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?; - self.aead_overhead() - } else { - 0 - }; + } // At least one record must be created even if total_len == 0 while offset < total_len || (total_len == 0 && offset == 0) { @@ -837,9 +1082,25 @@ impl Engine { let already_used_in_current = self.queue_tx.back().map(|b| b.len()).unwrap_or(0); let available_in_current = self.config.mtu().saturating_sub(already_used_in_current); - // Fixed overhead per handshake record on the wire: - // DTLS record header + handshake header + AEAD overhead (if epoch >= 1) - let fixed_overhead = DTLSRecord::HEADER_LEN + handshake_header_len + aead_overhead; + // Fixed overhead per handshake record on the wire. Uses the + // shared `outbound_record_overhead` helper so CID bytes and the + // DTLSInnerPlaintext real_type byte are accounted for — without + // this, encrypted handshake fragments with a long peer CID or a + // small MTU would overrun the datagram (RFC 9146 §4, RFC 6347 + // §4.1.1). + let fixed_overhead = self.outbound_record_overhead(epoch) + handshake_header_len; + + // RFC 9146 §4 + RFC 6347 §4.1.1: CID bytes are part of the wire + // record overhead, so a small `Config::mtu()` combined with a + // long peer CID can leave zero room for a fragment body byte. + // Fail closed deterministically instead of looping forever with + // `chunk_len == 0` when the body is non-empty. + if total_len > 0 && fixed_overhead >= self.config.mtu() { + return Err(Error::MtuTooSmall { + overhead: fixed_overhead, + mtu: self.config.mtu(), + }); + } // Prefer to pack into the current datagram. If the current one cannot fit even // the fixed overhead, we will start a fresh datagram and compute space again. @@ -851,6 +1112,16 @@ impl Engine { self.config.mtu().saturating_sub(fixed_overhead) }; + // RFC 6347 §4.1.1 / RFC 9146 §5: the DTLSPlaintext.fragment + // (post-DTLSInnerPlaintext wrap) is capped at 2^14. For CID the + // inner plaintext is `handshake_header + body + real_type`, so + // the body alone must leave room for the real_type byte. This + // ceiling is independent of MTU and bites when MTU is large. + let cid_inner_byte = usize::from(self.cid.outbound_if_live().is_some() && epoch >= 1); + let max_body_for_record = DTLS12_MAX_PLAINTEXT_LEN + .saturating_sub(handshake_header_len) + .saturating_sub(cid_inner_byte); + // Remaining bytes from the handshake body we still need to send. let remaining_body_bytes = total_len.saturating_sub(offset); @@ -858,7 +1129,9 @@ impl Engine { let chunk_len = if total_len == 0 { 0 } else { - remaining_body_bytes.min(available_for_body) + remaining_body_bytes + .min(available_for_body) + .min(max_body_for_record) }; let frag_range = if chunk_len == 0 { @@ -1033,6 +1306,17 @@ impl Engine { debug!("Peer encryption enabled"); self.peer_encryption_enabled = true; + // Activate inbound CID acceptance now that the peer's ChangeCipherSpec + // has been processed. Before this point the peer should only have sent + // epoch-0 plaintext, so treating content-type 25 records as undecryptable + // pre-CCS matters for clarity even if the queue-and-defer flow would have + // handled it anyway. `activate_inbound_cid` is a no-op if CID was not + // negotiated. + self.activate_inbound_cid(); + if self.cid.inbound_if_live().is_some() { + debug!("Inbound CID activated"); + } + let maybe_index_epoch1 = self .queue_rx .iter() @@ -1074,7 +1358,15 @@ impl Engine { // DTLS 1.2 AEAD: AAD uses the plaintext length. Recover plaintext length // from the record header by subtracting this suite's wire overhead. let plaintext_len = dtls.length.saturating_sub(self.aead_overhead() as u16); - let aad = Aad::new_dtls12(dtls.content_type, dtls.sequence, plaintext_len); + // Bind the wire version bytes (RFC 6347 §4.1.2.1) — accepting both + // 0xFEFD (DTLS 1.2) and 0xFEFF (DTLS 1.0 compatibility). + let wire_version = dtls.version.as_u16().to_be_bytes(); + let aad = Aad::new_dtls12( + dtls.content_type, + dtls.sequence, + wire_version, + plaintext_len, + ); let iv = self.peer_iv(); let seq64 = ((dtls.sequence.epoch as u64) << 48) | dtls.sequence.sequence_number; let nonce = match self.explicit_nonce_len { @@ -1212,6 +1504,26 @@ impl RecordHandler for Engine { self.explicit_nonce_len } + fn decryption_aad_and_nonce_cid( + &self, + dtls: &DTLSRecord, + buf: &[u8], + cid: &[u8], + inner_plaintext_len: u16, + ) -> (Aad, Nonce) { + // Bind the wire version bytes observed on this record (RFC 9146 §5). + let wire_version = dtls.version.as_u16().to_be_bytes(); + let aad = Aad::new_dtls12_cid(dtls.sequence, wire_version, cid, inner_plaintext_len); + let iv = self.peer_iv(); + let seq64 = ((dtls.sequence.epoch as u64) << 48) | dtls.sequence.sequence_number; + let nonce = match self.explicit_nonce_len { + 0 => Nonce::xor(iv.as_12_bytes(), seq64), + DTLSRecord::EXPLICIT_NONCE_LEN => Nonce::new(iv, dtls.nonce(buf)), + len => Nonce::new(iv, dtls.nonce_with_len(buf, len)), + }; + (aad, nonce) + } + fn decrypt_data( &mut self, ciphertext: &mut TmpBuf, @@ -1220,4 +1532,20 @@ impl RecordHandler for Engine { ) -> Result<(), Error> { Engine::decrypt_data(self, ciphertext, aad, nonce) } + + fn our_cid(&self) -> Option<&[u8]> { + // Receive-path framing: returns `Some(cid)` iff the peer's records to + // us are expected to use `tls12_cid` framing (inbound negotiated + // with non-zero length). Zero-length inbound stays on legacy + // framing per RFC 9146 §3 and is reported as `None` here. + self.cid.inbound_framing() + } + + fn inbound_cid_active(&self) -> Option<&[u8]> { + self.cid.inbound_if_live() + } + + fn aead_overhead(&self) -> usize { + Engine::aead_overhead(self) + } } diff --git a/src/dtls12/incoming.rs b/src/dtls12/incoming.rs index 3c8e8707..ef24133d 100644 --- a/src/dtls12/incoming.rs +++ b/src/dtls12/incoming.rs @@ -1,13 +1,15 @@ +use std::fmt; use std::ops::Deref; use std::sync::atomic::{AtomicBool, Ordering}; use arrayvec::ArrayVec; -use std::fmt; +use subtle::ConstantTimeEq; use crate::Error; use crate::buffer::{Buf, TmpBuf}; use crate::crypto::{Aad, Nonce}; use crate::dtls12::message::{ContentType, DTLSRecord, Dtls12CipherSuite, Handshake, Sequence}; +use crate::util::recover_inner_content_type; /// Holds both the UDP packet and the parsed result of that packet. pub struct Incoming { @@ -73,18 +75,148 @@ impl Records { cs: Option, ) -> Result { let mut parsed_records: ArrayVec = ArrayVec::new(); + let our_cid_len = decrypt.our_cid().map_or(0, |c| c.len()); // Find record boundaries and copy each record ONCE from the packet while !packet.is_empty() { - if packet.len() < DTLSRecord::HEADER_LEN { + // CID records have the CID between sequence and length fields, + // so the header is larger: 11 + cid_len + 2 instead of 13. + let is_cid_record = packet[0] == ContentType::Tls12Cid.as_u8(); + if is_cid_record && decrypt.our_cid().is_none() { + // RFC 9146 §4 / RFC 6347 §4.1.2.7: a tls12_cid record for a + // direction where we did not negotiate CID MUST be discarded, + // and plan §4 says coalesced records in the same datagram + // MUST still be processed *when framing permits*. Without a + // negotiated inbound CID we don't know how many CID bytes + // follow the sequence-number, so the length field at + // `packet[11..13]` is only trustworthy if the sender used a + // zero-length CID. + // + // Two-step recovery: (1) best-effort skip assuming + // zero-length CID; (2) sanity-check that the bytes landed on + // a plausible DTLS record header — a known ContentType plus a + // length field that fits inside the datagram. If validation + // fails we break rather than blindly advance into whatever an + // attacker-controlled CID payload looked like. This caps the + // damage of a crafted non-zero-CID stray to the remainder of + // *this* datagram: we drop coalesced records we can't + // re-synchronize on, but we never feed desynchronized bytes + // into AEAD verification on a legitimate record. + // + // Security envelope: DoS-only. A crafted stray tls12_cid + // record from a peer that never negotiated CID can waste the + // rest of the datagram's parseable records, but it cannot + // forge or alter any authenticated record — every real + // record still runs through AEAD verification with its own + // sequence-number-bound AAD. + if packet.len() < DTLSRecord::HEADER_LEN { + // RFC 6347 §4.1.2.7 / RFC 9146 §6: silently discard. + trace!("Discarding CID record: datagram shorter than header"); + break; + } + let stray_length = u16::from_be_bytes([packet[11], packet[12]]) as usize; + let stray_end = DTLSRecord::HEADER_LEN + stray_length; + if stray_end > packet.len() { + trace!( + "Discarding CID record: CID not negotiated, claimed length \ + runs past datagram" + ); + break; + } + if stray_end == packet.len() { + // Skip lands exactly at end-of-datagram — nothing more to + // frame, drop cleanly. + trace!("Discarding CID record: CID not negotiated (datagram end)"); + break; + } + // Validate the post-skip bytes look like a plausible DTLS + // record header before we commit to advancing. If not, we've + // almost certainly desynchronized off of a non-zero-CID stray + // and the safe move is to drop the datagram remainder. + if packet.len() - stray_end < DTLSRecord::HEADER_LEN { + trace!("Discarding CID record: post-skip remainder below header size"); + break; + } + let next_ct = packet[stray_end]; + let plausible_next_ct = matches!( + next_ct, + 20 // ChangeCipherSpec + | 21 // Alert + | 22 // Handshake + | 23 // ApplicationData + | 25 // Tls12Cid + ); + if !plausible_next_ct { + trace!( + "Discarding CID record: post-skip ContentType {} implausible, \ + dropping datagram remainder", + next_ct + ); + break; + } + // Any subsequent record's length field must also fit the + // remaining datagram. If not, we desynchronized. + let next_length = + u16::from_be_bytes([packet[stray_end + 11], packet[stray_end + 12]]) as usize; + let next_end = stray_end + DTLSRecord::HEADER_LEN + next_length; + // For a CID-typed follow-on we can't know its length field + // position without the CID length either; the outer loop will + // re-check and handle it via this same branch. For all other + // ContentTypes the length field is at the standard offset, so + // we validate here. + if next_ct != 25 && next_end > packet.len() { + trace!( + "Discarding CID record: post-skip length field implies \ + overshoot, dropping datagram remainder" + ); + break; + } + trace!("Discarding CID record: CID not negotiated"); + packet = &packet[stray_end..]; + continue; + } + let header_len = if is_cid_record { + DTLSRecord::HEADER_LEN + our_cid_len + } else { + DTLSRecord::HEADER_LEN + }; + + if packet.len() < header_len { + if is_cid_record { + // RFC 9146 §6: malformed CID-framed records are silently + // discarded so a stray/forged tls12_cid cannot tear down + // the association. + trace!( + "Discarding CID record: datagram remainder ({}) shorter than header ({})", + packet.len(), + header_len + ); + break; + } return Err(Error::ParseIncomplete); } - let length_bytes: [u8; 2] = packet[DTLSRecord::LENGTH_OFFSET].try_into().unwrap(); + // Length field is at the end of the header (last 2 bytes) + let length_offset = header_len - 2; + // unwrap: length_offset + 2 = header_len and the `packet.len() < + // header_len` check above guarantees at least header_len bytes + // remain, so the 2-byte slice always converts into [u8; 2]. + let length_bytes: [u8; 2] = + packet[length_offset..length_offset + 2].try_into().unwrap(); let length = u16::from_be_bytes(length_bytes) as usize; - let record_end = DTLSRecord::HEADER_LEN + length; + let record_end = header_len + length; if packet.len() < record_end { + if is_cid_record { + // RFC 9146 §6: silent-discard a CID record whose length + // field overshoots the datagram remainder. + trace!( + "Discarding CID record: claimed length {} overshoots datagram remainder {}", + record_end, + packet.len() + ); + break; + } return Err(Error::ParseIncomplete); } @@ -96,9 +228,15 @@ impl Records { if parsed_records.try_push(record).is_err() { return Err(Error::TooManyRecords); } - } else { - trace!("Discarding replayed rec"); } + // `Ok(None)` is a silent discard per RFC 6347 §4.1.2.7. + // Each drop site inside `Record::parse` / + // `Record::decrypt_record` emits its own `trace!` with + // the specific reason (replay, parse failure, + // legacy-framed-when-CID-expected, wire CID mismatch, + // AEAD failure, bogus inner type), so no generic + // message here — it would mis-diagnose every drop as + // replay. } Err(e) => return Err(e), } @@ -142,70 +280,306 @@ impl Record { decrypt: &mut dyn RecordHandler, cs: Option, ) -> Result, Error> { - // ONLY COPY: UDP packet slice -> pooled buffer + let is_cid_record = record_slice[0] == ContentType::Tls12Cid.as_u8(); + let our_cid_len = if is_cid_record { + decrypt.our_cid().map_or(0, |c| c.len()) + } else { + 0 + }; + + // ONLY COPY: UDP packet slice -> pooled buffer. + // For CID records, keep the original wire format (with CID bytes) so that + // enable_peer_encryption() can re-parse from the same buffer later. let mut buffer = Buf::new(); buffer.extend_from_slice(record_slice); - let parsed = match ParsedRecord::parse(&buffer, cs, 0) { - Ok(p) => p, - Err(e) => { - // RFC 6347 §4.1.2.7: Invalid records SHOULD be silently discarded. - // This includes epoch 0 records with invalid ContentType. - trace!("Discarding record: parse failed: {}", e); - return Ok(None); + + // For CID records, create a temporary stripped buffer for DTLSRecord::parse. + // The real buffer retains the original CID format. + let parsed = if is_cid_record { + let mut tmp = Buf::new(); + tmp.extend_from_slice(&record_slice[..11]); + tmp.extend_from_slice(&record_slice[11 + our_cid_len..]); + match ParsedRecord::parse(&tmp, cs, 0) { + Ok(p) => p, + Err(e) => { + trace!("Discarding CID record: parse failed: {}", e); + return Ok(None); + } + } + } else { + match ParsedRecord::parse(&buffer, cs, 0) { + Ok(p) => p, + Err(e) => { + // RFC 6347 §4.1.2.7: Invalid records SHOULD be silently discarded. + trace!("Discarding record: parse failed: {}", e); + return Ok(None); + } } }; + let parsed = Box::new(parsed); let record = Record { buffer, parsed }; // It is not enough to only look at the epoch, since to be able to decrypt the entire // preceeding set of flights sets up the cryptographic context. In a situation with // packet loss, we can end up seeing epoch 1 records before we can decrypt them. + // + // Epoch-0 `tls12_cid` records cannot reach this point: `DTLSRecord::parse` + // (`src/dtls12/message/record.rs:68-78`) only lets epoch-0 content + // types in `{ChangeCipherSpec, Alert, Handshake}`, so a `tls12_cid` + // content type at epoch 0 fails parse and is silently dropped by + // the `Ok(None)` branch above. RFC 9146 §3 "once encryption is + // enabled" is therefore enforced at the parse layer, not here. let is_epoch_0 = record.record().sequence.epoch == 0; + if is_epoch_0 || !decrypt.is_peer_encryption_enabled() { + // Pre-CCS CID records get queued for post-CCS decryption + // (handshake loss can deliver an epoch-1 Finished before the + // CCS). Round-5 review #2: cheap cleartext filter — require + // the wire CID to match the negotiated inbound CID before + // consuming a `queue_rx` slot. A spray attacker can otherwise + // fill `queue_rx` with forged CID-framed records at near-zero + // cost (all CID bytes are attacker-controlled in cleartext). + // Legitimate peers always emit the negotiated CID, so this + // does not affect the normal queue-and-decrypt flow. The + // authenticating check at AEAD time is unchanged. + if is_cid_record && !is_epoch_0 { + // Wire CID sits at record.buffer[11..11+our_cid_len]. + // `our_cid_len > 0` implies framing with CID bytes + // (zero-length CID routes through legacy framing per + // RFC 9146 §3 and never hits this branch). + if our_cid_len > 0 { + if let Some(expected) = decrypt.our_cid() { + let wire = &record.buffer[11..11 + our_cid_len]; + if wire.ct_eq(expected).unwrap_u8() == 0 { + trace!( + "Discarding pre-CCS CID record: wire CID does not \ + match negotiated inbound CID" + ); + return Ok(None); + } + } + } + } return Ok(Some(record)); } - // We need to decrypt the record and redo the parsing. - let dtls = record.record(); - let sequence = dtls.sequence; + Self::decrypt_record(record, decrypt, cs) + } + + /// Decrypt a record (CID or standard) and re-parse the result. + fn decrypt_record( + record: Record, + decrypt: &mut dyn RecordHandler, + cs: Option, + ) -> Result, Error> { + let is_cid_record = record.buffer[0] == ContentType::Tls12Cid.as_u8(); + let our_cid_len = if is_cid_record { + decrypt.our_cid().map_or(0, |c| c.len()) + } else { + 0 + }; + + // RFC 9146 §3: "When receiving a datagram without `tls12_cid`, the + // receiver ... if a non-zero-length CID is expected, the datagram + // MUST be treated as invalid." We reach this branch only for epoch-1+ + // records (epoch-0 plaintext is short-circuited upstream). If the + // association expects a non-zero inbound CID (`our_cid().is_some()` + // reports framing-live inbound), a legacy-framed epoch-1 record is + // invalid and must be silently discarded per RFC 6347 §4.1.2.7 — + // without advancing the replay window. + if !is_cid_record && decrypt.our_cid().is_some() { + trace!( + "Discarding legacy-framed epoch-{} record: inbound CID expected", + record.record().sequence.epoch + ); + return Ok(None); + } + + let sequence = record.record().sequence; // Anti-replay check (read-only, does not update window) if !decrypt.replay_check(sequence) { + trace!( + "Discarding record: replay check failed (epoch={}, seq={})", + sequence.epoch, sequence.sequence_number + ); return Ok(None); } - // Get a reference to the buffer - let (aad, nonce) = decrypt.decryption_aad_and_nonce(dtls, &record.buffer); + // A CID record can reach the decrypt path only if inbound CID is active. + // `Records::parse` accepts CID-framed records as soon as `our_cid` is + // negotiated — that lets records reordered ahead of the peer's + // ChangeCipherSpec be queued for later decryption. By the time decryption + // runs (either here after CCS, or via `enable_peer_encryption`'s re-parse + // of the queue) `inbound_cid_active()` must be `Some`. Any other + // arrangement is a state-machine error; drop silently rather than + // feeding an unauthenticated CID length into AAD construction. + // + // RFC 9146 §5.3 binds the CID into the AAD so tampering is detectable + // at the AEAD layer. We capture the wire CID bytes before stripping + // and require them (constant-time) to match the negotiated inbound + // CID. A mismatch is a silent drop per RFC 6347 §4.1.2.7 — no error + // surfaced to the state machine, and the replay window is NOT + // advanced on the tampered sequence, so a legitimate retransmit at + // the same sequence still decrypts. + // + // Note: only the CID *value* is authenticated here. The CID *length* + // used for framing (via `our_cid_len`) comes from local config and is + // authenticated implicitly via AEAD-tag failure on any mis-framed + // record — not by this check. + let mut wire_cid: ArrayVec = ArrayVec::new(); + if is_cid_record { + let Some(expected_cid) = decrypt.inbound_cid_active() else { + trace!("Discarding CID record: inbound CID not active"); + return Ok(None); + }; + if our_cid_len > 0 { + // Bounds: Records::parse validated record_slice.len() >= 13 + + // our_cid_len before copying into record.buffer, so + // [11..11+our_cid_len] is safe. + wire_cid + .try_extend_from_slice(&record.buffer[11..11 + our_cid_len]) + .expect("our_cid_len <= 255 by config validation"); + + if wire_cid.as_slice().ct_eq(expected_cid).unwrap_u8() == 0 { + trace!("Discarding CID record: wire CID does not match negotiated CID"); + return Ok(None); + } + } + } - // Extract the buffer for decryption + // For CID records, strip CID bytes from buffer now that we're + // decrypting. This gives us the standard 13-byte header layout + // needed for decryption. The strip is done **in place** on the + // record's own `Buf` — shift `[11+cid_len..]` left by `cid_len` + // and truncate — avoiding a second pooled allocation per CID + // record. Safe because `record.buffer` is owned here and is + // about to be further mutated (inner content-type rewrite at + // the post-decrypt header patch below). let mut buffer = record.buffer; + if is_cid_record && our_cid_len > 0 { + let new_len = buffer.len() - our_cid_len; + buffer.copy_within(11 + our_cid_len.., 11); + buffer.truncate(new_len); + } + + // Re-parse the (possibly stripped) buffer to get a DTLSRecord for AAD/nonce + let (_, dtls) = DTLSRecord::parse(&buffer, 0, 0) + .map_err(|_| Error::ParseError(nom::error::ErrorKind::Fail))?; + let explicit_nonce_len = decrypt.explicit_nonce_len(); + let aead_overhead = decrypt.aead_overhead(); + + // RFC 6347 §4.1.2.7: invalid records SHOULD be silently discarded. + // A peer-controlled `dtls.length` below the minimum AEAD overhead + // (explicit_nonce_len + tag_len) cannot contain a valid ciphertext + // + tag, and would underflow the `[ciph..]` slice below. Reject it + // without advancing the replay window, so a legitimate retransmit + // at the same sequence still decrypts. + if (dtls.length as usize) < aead_overhead { + trace!( + "Discarding record: length {} below AEAD overhead {}", + dtls.length, aead_overhead + ); + return Ok(None); + } + + // RFC 9146 §5.3 / RFC 6347 §4.1.1: `length_of_DTLSInnerPlaintext` + // (CID) and `DTLSPlaintext.length` (legacy) MUST NOT exceed 2^14. + // The send path enforces this before encryption; mirror the check + // on receive so a mis-behaving peer (with the session key) cannot + // assert an inner length > 2^14 in the AAD. Silent-drop per + // §4.1.2.7; replay window stays put because we have not yet + // attempted decryption. + let inner_plaintext_len_usize = (dtls.length as usize) - aead_overhead; + if inner_plaintext_len_usize > super::engine::DTLS12_MAX_PLAINTEXT_LEN { + trace!( + "Discarding record: inner plaintext length {} exceeds RFC 2^14 ceiling", + inner_plaintext_len_usize + ); + return Ok(None); + } + + let (aad, nonce) = if is_cid_record { + let inner_plaintext_len = inner_plaintext_len_usize as u16; + // Feed the wire-observed CID (not the locally cached copy) into AAD + // construction. After the ct_eq check above they are equal, but the + // defensive posture matters for future refactors. + decrypt.decryption_aad_and_nonce_cid(&dtls, &buffer, &wire_cid, inner_plaintext_len) + } else { + decrypt.decryption_aad_and_nonce(&dtls, &buffer) + }; // Local shorthand for where the encrypted ciphertext starts let ciph = DTLSRecord::HEADER_LEN + explicit_nonce_len; // The encrypted part is after the DTLS header and optional explicit nonce. - // The entire buffer is only the single record, since we chunk - // records up in Records::parse() let ciphertext = &mut buffer[ciph..]; let new_len = { let mut buffer = TmpBuf::new(ciphertext); - // This decrypts in place. - decrypt.decrypt_data(&mut buffer, aad, nonce)?; + // This decrypts in place. RFC 6347 §4.1.2.7: a forged ciphertext + // (AEAD tag mismatch) is an invalid record — silently discarded, + // preserving the association. The replay window is intentionally + // NOT advanced on failure (RFC 6347 §4.1.2.6), so a legitimate + // retransmit at the same sequence still decrypts. + match decrypt.decrypt_data(&mut buffer, aad, nonce) { + Ok(()) => buffer.len(), + Err(Error::CryptoError(_)) => { + trace!("Discarding record: AEAD decryption failed"); + return Ok(None); + } + Err(e) => return Err(e), + } + }; - buffer.len() + // AEAD succeeded. For CID records the `DTLSInnerPlaintext` unwrap + // and its sanity checks are themselves AEAD-covered (they live + // inside the ciphertext), so we run them *before* the replay + // window update — a peer bug that ships a nested `tls12_cid` or + // unknown inner type then no longer consumes a sequence number + // from the window. RFC 6347 §4.1.2.6 only requires "updates + // after MAC success"; this is a tighter local invariant that + // reserves the sequence space for semantically valid records. + let cid_inner_rewrite: Option<(u8, usize)> = if is_cid_record { + let decrypted = &buffer[ciph..ciph + new_len]; + let (real_content_type, content_len) = match recover_inner_content_type(decrypted) { + Ok(v) => v, + Err(_) => { + trace!("Discarding CID record: invalid inner content type"); + return Ok(None); + } + }; + if matches!(real_content_type, ContentType::Tls12Cid | ContentType::Ack) { + trace!( + "Discarding CID record: disallowed inner content type {:?}", + real_content_type + ); + return Ok(None); + } + Some((real_content_type.as_u8(), content_len)) + } else { + None }; - // Decryption succeeded — now commit the replay window update. // RFC 6347 §4.1.2.6: "The receive window is updated only if the - // MAC verification succeeds." + // MAC verification succeeds." We additionally require the CID + // inner-type unwrap above to succeed, so malformed post-auth + // plaintext does not poison the sequence space. decrypt.replay_update(sequence); - // Update the length of the record. - buffer[11] = (new_len >> 8) as u8; - buffer[12] = new_len as u8; + if let Some((content_type_byte, content_len)) = cid_inner_rewrite { + // Rewrite the buffer header with the real content type and content length + buffer[0] = content_type_byte; + buffer[11] = (content_len >> 8) as u8; + buffer[12] = content_len as u8; + } else { + // Update the length of the record. + buffer[11] = (new_len >> 8) as u8; + buffer[12] = new_len as u8; + } let parsed = ParsedRecord::parse(&buffer, cs, explicit_nonce_len)?; let parsed = Box::new(parsed); @@ -291,6 +665,14 @@ pub trait RecordHandler { fn replay_check(&self, seq: Sequence) -> bool; fn replay_update(&mut self, seq: Sequence); fn decryption_aad_and_nonce(&self, dtls: &DTLSRecord, buf: &[u8]) -> (Aad, Nonce); + /// Build AAD and nonce for a CID record (RFC 9146). + fn decryption_aad_and_nonce_cid( + &self, + dtls: &DTLSRecord, + buf: &[u8], + cid: &[u8], + inner_plaintext_len: u16, + ) -> (Aad, Nonce); fn explicit_nonce_len(&self) -> usize; fn decrypt_data( &mut self, @@ -298,6 +680,35 @@ pub trait RecordHandler { aad: Aad, nonce: Nonce, ) -> Result<(), Error>; + /// Returns the CID value to expect in incoming `tls12_cid` framing, if + /// any. Per RFC 9146 §3: + /// + /// - `None` — CID was not negotiated for this direction, OR the + /// negotiated inbound length was zero (legacy RFC 6347 framing). In + /// either case, `tls12_cid` records arriving on this engine are + /// unsolicited and must be dropped via the two-step recovery. + /// - `Some(cid)` — CID was negotiated with non-zero inbound length; + /// incoming records use `tls12_cid` framing with `cid.len()` bytes + /// between the sequence-number and length fields. `cid` is the + /// expected value and is authenticated at the AEAD layer. + /// + /// A `None` return with a negotiated-but-zero-length inbound CID is + /// distinct from "CID not negotiated at all" at the reporting layer + /// (`Engine::inbound_cid()`), but at the framing layer both cases mean + /// the same thing: legacy RFC 6347 record format. + fn our_cid(&self) -> Option<&[u8]>; + /// Returns `Some(expected_cid_bytes)` once inbound CID is armed — i.e. the + /// peer's ChangeCipherSpec has been processed and we are prepared to + /// authenticate CID-framed records at the AEAD layer; `None` otherwise. + /// + /// Fusing activation and value here removes a cross-type invariant the + /// decrypt path would otherwise rely on (`inbound_cid_active` → `our_cid` + /// is `Some`). Distinct from `our_cid()`, which reports the negotiated + /// value regardless of activation so pre-CCS CID records can still be + /// framed and queued for later decryption. + fn inbound_cid_active(&self) -> Option<&[u8]>; + /// Returns the total AEAD overhead (explicit nonce + tag) for this cipher suite. + fn aead_overhead(&self) -> usize; } fn parse_handshakes( @@ -406,6 +817,16 @@ mod tests { panic!("decryption_aad_and_nonce should not be called for plaintext tests"); } + fn decryption_aad_and_nonce_cid( + &self, + _dtls: &DTLSRecord, + _buf: &[u8], + _cid: &[u8], + _inner_plaintext_len: u16, + ) -> (Aad, Nonce) { + panic!("decryption_aad_and_nonce_cid should not be called for plaintext tests"); + } + fn explicit_nonce_len(&self) -> usize { panic!("explicit_nonce_len should not be called for plaintext tests"); } @@ -418,6 +839,18 @@ mod tests { ) -> Result<(), Error> { panic!("decrypt_data should not be called for plaintext tests"); } + + fn our_cid(&self) -> Option<&[u8]> { + None + } + + fn inbound_cid_active(&self) -> Option<&[u8]> { + None + } + + fn aead_overhead(&self) -> usize { + 0 + } } fn build_record(content_type: ContentType, epoch: u16, seq: u64, fragment: &[u8]) -> Vec { @@ -456,4 +889,437 @@ mod tests { ); assert_eq!(incoming.first().record().sequence.epoch, 1); } + + /// Minimal RecordHandler stub for exercising decrypt-path gating without + /// standing up a full Engine/handshake. The AAD, nonce, and decrypt hooks + /// all panic on invocation — the regression test asserts the CID gate + /// drops the record *before* any of them are touched. + struct GateStub { + our_cid: Option>, + inbound_cid_active: bool, + } + + impl RecordHandler for GateStub { + fn classify_record(&mut self, record: Record) -> Result, Error> { + Ok(Some(record)) + } + fn is_peer_encryption_enabled(&self) -> bool { + true + } + fn replay_check(&self, _: Sequence) -> bool { + true + } + fn replay_update(&mut self, _: Sequence) {} + fn decryption_aad_and_nonce(&self, _: &DTLSRecord, _: &[u8]) -> (Aad, Nonce) { + panic!("AAD builder reached — gate must drop CID record first") + } + fn decryption_aad_and_nonce_cid( + &self, + _: &DTLSRecord, + _: &[u8], + _: &[u8], + _: u16, + ) -> (Aad, Nonce) { + panic!("CID AAD builder reached — gate must drop CID record first") + } + fn explicit_nonce_len(&self) -> usize { + 0 + } + fn decrypt_data(&mut self, _: &mut TmpBuf, _: Aad, _: Nonce) -> Result<(), Error> { + panic!("decrypt_data reached — gate must drop CID record first") + } + fn our_cid(&self) -> Option<&[u8]> { + self.our_cid.as_deref() + } + fn inbound_cid_active(&self) -> Option<&[u8]> { + if self.inbound_cid_active { + self.our_cid.as_deref() + } else { + None + } + } + fn aead_overhead(&self) -> usize { + 0 + } + } + + /// Build a minimal zero-payload tls12_cid record: + /// [0] content type = 25 (Tls12Cid) + /// [1..3] version = 0xfefd (DTLS 1.2) + /// [3..5] epoch = 1 + /// [5..11] sequence = 0 + /// [11..] CID bytes + /// [..+2] length = 0 + fn make_cid_record(cid: &[u8]) -> Record { + let mut buffer = Buf::new(); + buffer.extend_from_slice(&[25, 0xfe, 0xfd, 0, 1, 0, 0, 0, 0, 0, 0]); + buffer.extend_from_slice(cid); + buffer.extend_from_slice(&[0, 0]); + + // `Record::parse` builds `ParsedRecord` from a CID-stripped view so the + // standard 13-byte DTLS header parser applies; mirror that here. + let mut stripped = Buf::new(); + stripped.extend_from_slice(&buffer[..11]); + stripped.extend_from_slice(&buffer[11 + cid.len()..]); + let parsed = + Box::new(ParsedRecord::parse(&stripped, None, 0).expect("parse stripped CID record")); + + Record { buffer, parsed } + } + + /// When `inbound_cid_active` is false, a CID-framed record must be + /// silently dropped at the decrypt path — before AAD construction, before + /// the wire CID is consumed, and before any decrypt work happens. Protects + /// against future refactors that treat `our_cid.is_some()` alone as + /// sufficient to authenticate CID + /// framing. + #[test] + fn cid_record_dropped_when_inbound_not_active() { + let cid: &[u8] = b"abcd"; + let record = make_cid_record(cid); + + let mut decrypt = GateStub { + our_cid: Some(cid.to_vec()), + inbound_cid_active: false, + }; + + let result = Record::decrypt_record(record, &mut decrypt, None) + .expect("decrypt path must not surface an error for stray CID"); + assert!( + result.is_none(), + "CID record must be dropped when inbound_cid_active is false" + ); + } + + /// Build a minimal legacy (non-CID) epoch-1 ApplicationData record: + /// [0] content type = 23 (ApplicationData) + /// [1..3] version = 0xfefd (DTLS 1.2) + /// [3..5] epoch = 1 + /// [5..11] sequence = 0 + /// [11..13] length = 0 + fn make_legacy_epoch1_record() -> Record { + let mut buffer = Buf::new(); + buffer.extend_from_slice(&[23, 0xfe, 0xfd, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]); + + let parsed = + Box::new(ParsedRecord::parse(&buffer, None, 0).expect("parse legacy epoch-1 record")); + Record { buffer, parsed } + } + + /// RFC 9146 §3: on an association that expects a non-zero inbound CID, + /// a legacy-framed (non-`tls12_cid`) encrypted record is invalid and MUST + /// be silently discarded. The gate fires at `decrypt_record` before AAD + /// construction, before replay update, and before any AEAD work — the + /// decrypt stubs in `GateStub` panic if + /// reached, so reaching this test without a drop is a loud failure. + #[test] + fn legacy_framed_record_dropped_when_inbound_cid_expected() { + let cid: &[u8] = b"abcd"; + let record = make_legacy_epoch1_record(); + + // CID is negotiated and inbound is live, so this association expects + // incoming `tls12_cid` framing for all epoch-1 records. A legacy + // content-type-23 record is RFC-invalid and must be dropped. + let mut decrypt = GateStub { + our_cid: Some(cid.to_vec()), + inbound_cid_active: true, + }; + + let result = Record::decrypt_record(record, &mut decrypt, None) + .expect("decrypt path must not surface an error for legacy-framed drop"); + assert!( + result.is_none(), + "legacy-framed epoch-1 record must be dropped when CID framing expected" + ); + } + + /// Stub that models an authenticated decryption which "succeeds" against + /// caller-provided plaintext. Feeds a decrypted DTLSInnerPlaintext with a + /// bogus inner content type past AEAD and into the silent-discard gate. + struct DecryptStub { + cid: Vec, + inner_plaintext: Vec, + } + + impl RecordHandler for DecryptStub { + fn classify_record(&mut self, record: Record) -> Result, Error> { + Ok(Some(record)) + } + fn is_peer_encryption_enabled(&self) -> bool { + true + } + fn replay_check(&self, _: Sequence) -> bool { + true + } + fn replay_update(&mut self, _: Sequence) {} + fn decryption_aad_and_nonce(&self, _: &DTLSRecord, _: &[u8]) -> (Aad, Nonce) { + unreachable!("non-CID path not exercised") + } + fn decryption_aad_and_nonce_cid( + &self, + _: &DTLSRecord, + _: &[u8], + _: &[u8], + _: u16, + ) -> (Aad, Nonce) { + ( + Aad::new_dtls12(ContentType::Tls12Cid, Sequence::new(0), [0xFE, 0xFD], 0), + Nonce([0u8; 12]), + ) + } + fn explicit_nonce_len(&self) -> usize { + 0 + } + fn decrypt_data(&mut self, ciphertext: &mut TmpBuf, _: Aad, _: Nonce) -> Result<(), Error> { + // Overwrite the ciphertext with our chosen DTLSInnerPlaintext and + // truncate to its length. The buffer always has at least + // `inner_plaintext.len()` bytes because `make_cid_record_with_payload` + // sized it that way. + let dst = ciphertext.as_mut(); + dst[..self.inner_plaintext.len()].copy_from_slice(&self.inner_plaintext); + ciphertext.truncate(self.inner_plaintext.len()); + Ok(()) + } + fn our_cid(&self) -> Option<&[u8]> { + Some(&self.cid) + } + fn inbound_cid_active(&self) -> Option<&[u8]> { + Some(&self.cid) + } + fn aead_overhead(&self) -> usize { + 0 + } + } + + /// Build a CID record whose ciphertext region has length `inner_len` + /// so the decrypt path slices cleanly. Actual content is replaced by the + /// stub inside `decrypt_data`. + fn make_cid_record_with_payload(cid: &[u8], inner_len: usize) -> Record { + let mut buffer = Buf::new(); + buffer.extend_from_slice(&[25, 0xfe, 0xfd, 0, 1, 0, 0, 0, 0, 0, 0]); + buffer.extend_from_slice(cid); + // length field (covers the ciphertext region) + buffer.extend_from_slice(&(inner_len as u16).to_be_bytes()); + for _ in 0..inner_len { + buffer.push(0); + } + + let mut stripped = Buf::new(); + stripped.extend_from_slice(&buffer[..11]); + stripped.extend_from_slice(&buffer[11 + cid.len()..]); + let parsed = + Box::new(ParsedRecord::parse(&stripped, None, 0).expect("parse stripped CID record")); + + Record { buffer, parsed } + } + + /// An authenticated CID record whose inner content type is `tls12_cid` + /// (nested), `Ack`, or `Unknown(_)` must be silently dropped. RFC 6347 + /// §4.1.2.7 — invalid records SHOULD be silently discarded — and this + /// must NOT surface as an `Err(..)` that tears down the association. + #[test] + fn cid_record_with_bogus_inner_type_is_silently_dropped() { + let cid: &[u8] = b"cidX"; + // Inner: single type byte with no content, no padding. Each bad type + // value below must silently drop. + for bad in [ + ContentType::Tls12Cid.as_u8(), + 99, + 0, + ContentType::Ack.as_u8(), + ] { + let record = make_cid_record_with_payload(cid, 1); + let mut decrypt = DecryptStub { + cid: cid.to_vec(), + inner_plaintext: vec![bad], + }; + let result = Record::decrypt_record(record, &mut decrypt, None).unwrap_or_else(|e| { + panic!("bad inner type {} must silent-drop, got Err({:?})", bad, e) + }); + assert!( + result.is_none(), + "bad inner type {} must be dropped (returned Some)", + bad + ); + } + } + + /// Stub that counts replay updates so we can check "AEAD success causes + /// the window to advance even when the inner content type is bogus". + /// Per RFC 6347 §4.1.2.6 the window updates on MAC success, regardless + /// of whether the decrypted plaintext is semantically valid. + struct CountingDecryptStub { + cid: Vec, + inner_plaintext: Vec, + replay_updates: usize, + } + + impl RecordHandler for CountingDecryptStub { + fn classify_record(&mut self, record: Record) -> Result, Error> { + Ok(Some(record)) + } + fn is_peer_encryption_enabled(&self) -> bool { + true + } + fn replay_check(&self, _: Sequence) -> bool { + true + } + fn replay_update(&mut self, _: Sequence) { + self.replay_updates += 1; + } + fn decryption_aad_and_nonce(&self, _: &DTLSRecord, _: &[u8]) -> (Aad, Nonce) { + unreachable!("non-CID path not exercised") + } + fn decryption_aad_and_nonce_cid( + &self, + _: &DTLSRecord, + _: &[u8], + _: &[u8], + _: u16, + ) -> (Aad, Nonce) { + ( + Aad::new_dtls12(ContentType::Tls12Cid, Sequence::new(0), [0xFE, 0xFD], 0), + Nonce([0u8; 12]), + ) + } + fn explicit_nonce_len(&self) -> usize { + 0 + } + fn decrypt_data(&mut self, ciphertext: &mut TmpBuf, _: Aad, _: Nonce) -> Result<(), Error> { + let dst = ciphertext.as_mut(); + dst[..self.inner_plaintext.len()].copy_from_slice(&self.inner_plaintext); + ciphertext.truncate(self.inner_plaintext.len()); + Ok(()) + } + fn our_cid(&self) -> Option<&[u8]> { + Some(&self.cid) + } + fn inbound_cid_active(&self) -> Option<&[u8]> { + Some(&self.cid) + } + fn aead_overhead(&self) -> usize { + 0 + } + } + + /// Coverage gap #8: RFC 9146 §6 permits a `DTLSInnerPlaintext` whose + /// `content` is zero bytes — the unwrap should return the lone + /// `real_type` byte as the content type and `content_len = 0`. Verify + /// the decrypt path surfaces the correct content type (e.g. Alert, or + /// ApplicationData with an empty payload) without mis-classifying the + /// single byte as padding. + #[test] + fn cid_record_with_empty_inner_content() { + let cid: &[u8] = b"empty"; + for content_type in [ + ContentType::Alert.as_u8(), + ContentType::ApplicationData.as_u8(), + ContentType::Handshake.as_u8(), + ContentType::ChangeCipherSpec.as_u8(), + ] { + let record = make_cid_record_with_payload(cid, 1); + let mut decrypt = DecryptStub { + cid: cid.to_vec(), + inner_plaintext: vec![content_type], + }; + let result = Record::decrypt_record(record, &mut decrypt, None) + .expect("valid empty-content inner plaintext must not error") + .expect("zero-length content with a valid real_type must surface a Record"); + // Post-decrypt the buffer's outer content type byte carries the + // recovered inner type and the record length is zero. + let buf = result.buffer(); + assert_eq!( + buf[0], content_type, + "recovered outer content type must match inner real_type" + ); + let rec_len = u16::from_be_bytes([buf[11], buf[12]]); + assert_eq!( + rec_len, 0, + "zero-length content must surface as length-0, not mis-counted padding" + ); + } + } + + /// Round-4 review #2 tightening: a bogus-inner-type drop must NOT + /// advance the replay window. RFC 6347 §4.1.2.6 only requires that + /// the window update *after* MAC success; dimpl additionally + /// requires the CID inner-type unwrap to succeed, so a peer-bug + /// record that AEAD-authenticates but carries a forbidden inner + /// content type does not poison the sequence space for legitimate + /// retransmits. The silent-drop is still there — the association + /// survives — only the replay slot is preserved. + #[test] + fn cid_record_bogus_inner_type_preserves_replay_window() { + let cid: &[u8] = b"abcd"; + let record = make_cid_record_with_payload(cid, 1); + let mut decrypt = CountingDecryptStub { + cid: cid.to_vec(), + inner_plaintext: vec![ContentType::Tls12Cid.as_u8()], + replay_updates: 0, + }; + let result = Record::decrypt_record(record, &mut decrypt, None) + .expect("silent-drop must not surface an error"); + assert!(result.is_none(), "record must be dropped"); + assert_eq!( + decrypt.replay_updates, 0, + "bogus inner type must not consume the sequence number from the window" + ); + } + + /// Receive-side 2^14 ceiling: a CID record whose `dtls.length` + /// implies an inner plaintext > `DTLS12_MAX_PLAINTEXT_LEN` (RFC 9146 + /// §5.3 / RFC 6347 §4.1.1) must silently drop *before* AAD + /// construction, and the replay window must not advance. + #[test] + fn cid_record_inner_plaintext_over_2_14_silently_dropped() { + let cid: &[u8] = b"big"; + // Stub aead_overhead == 0, so dtls.length == inner_plaintext_len. + // 16385 > DTLS12_MAX_PLAINTEXT_LEN (16384) → must drop. + let record = + make_cid_record_with_payload(cid, super::super::engine::DTLS12_MAX_PLAINTEXT_LEN + 1); + let mut decrypt = CountingDecryptStub { + cid: cid.to_vec(), + inner_plaintext: vec![ContentType::ApplicationData.as_u8()], + replay_updates: 0, + }; + let result = Record::decrypt_record(record, &mut decrypt, None) + .expect("over-length record must silent-drop, not error"); + assert!(result.is_none()); + assert_eq!( + decrypt.replay_updates, 0, + "drop happens before AEAD; replay window must not advance" + ); + } + + /// Build a minimal epoch-0 `tls12_cid` record (no encryption). + fn make_epoch0_cid_record(cid: &[u8]) -> Vec { + let mut buffer = Vec::new(); + // type=25, version=DTLS1.2, epoch=0, seq=0 + buffer.extend_from_slice(&[25, 0xfe, 0xfd, 0, 0, 0, 0, 0, 0, 0, 0]); + buffer.extend_from_slice(cid); + buffer.extend_from_slice(&[0, 0]); // length = 0 + buffer + } + + /// RFC 9146 §3 / RFC 6347 §4.1.1: `tls12_cid` framing only applies + /// once encryption is enabled, so an epoch-0 CID record is invalid + /// and MUST be silently discarded — and must NOT consume a `queue_rx` + /// slot, which would otherwise be a DoS amplifier. + #[test] + fn epoch0_cid_record_dropped_at_parse() { + let cid: &[u8] = b"abcd"; + let raw = make_epoch0_cid_record(cid); + + let mut decrypt = GateStub { + our_cid: Some(cid.to_vec()), + inbound_cid_active: false, + }; + + let result = + Record::parse(&raw, &mut decrypt, None).expect("epoch-0 CID parse must not error"); + assert!( + result.is_none(), + "epoch-0 tls12_cid record must drop at parse, not enter the queue" + ); + } } diff --git a/src/dtls12/message/client_hello.rs b/src/dtls12/message/client_hello.rs index 2e98af67..148b3d51 100644 --- a/src/dtls12/message/client_hello.rs +++ b/src/dtls12/message/client_hello.rs @@ -1,5 +1,6 @@ -use super::extensions::{ECPointFormatsExtension, SignatureAlgorithmsExtension}; -use super::extensions::{SupportedGroupsExtension, UseSrtpExtension}; +use super::extensions::UseSrtpExtension; +use super::extensions::{ConnectionIdExtension, ECPointFormatsExtension}; +use super::extensions::{SignatureAlgorithmsExtension, SupportedGroupsExtension}; use super::{CipherSuiteVec, CompressionMethod, CompressionMethodVec, Dtls12CipherSuite}; use super::{Cookie, Extension, ExtensionType, ProtocolVersion, Random, SessionId}; use arrayvec::ArrayVec; @@ -50,7 +51,7 @@ impl ClientHello { buf.clear(); // First write all extension data - let mut ranges = ArrayVec::<(ExtensionType, usize, usize), 8>::new(); + let mut ranges = ArrayVec::<(ExtensionType, usize, usize), 9>::new(); // Check if provider has ECDH support let has_ecdh = config.crypto_provider().has_ecdh(); @@ -109,6 +110,13 @@ impl ClientHello { start_pos, // No data at all )); + // Connection ID (RFC 9146) + if let Some(cid) = config.connection_id() { + let start_pos = buf.len(); + ConnectionIdExtension::new(cid).serialize(buf); + ranges.push((ExtensionType::ConnectionId, start_pos, buf.len())); + } + // Now create all extensions using ranges for (extension_type, start, end) in ranges { self.extensions.push(Extension { @@ -188,18 +196,66 @@ impl ClientHello { let consumed = extensions_data.as_ptr() as usize - original_input.as_ptr() as usize; let data_base_offset = base_offset + consumed; - // Parse individual extensions, filtering to only known types + // Parse individual extensions, filtering to only known types. + // RFC 5246 §7.4.1.4: "There MUST NOT be more than one extension of + // the same type." The rule is not limited to extension types this + // implementation understands, so we track every raw u16 codepoint + // seen, reject duplicates regardless of support, then store only + // supported extensions in `ExtensionVec`. Capacity overflow on + // the supported set maps to a parse error instead of a panic. let mut extensions_rest = extensions_data; let mut current_offset = data_base_offset; + let mut seen_types: ArrayVec = ArrayVec::new(); while !extensions_rest.is_empty() { let before_len = extensions_rest.len(); let (rest, extension) = Extension::parse(extensions_rest, current_offset)?; let parsed_len = before_len - rest.len(); current_offset += parsed_len; - // Only keep supported extension types + let ty = extension.extension_type.as_u16(); + if seen_types.contains(&ty) { + return Err(nom::Err::Failure(nom::error::Error::new( + extensions_rest, + nom::error::ErrorKind::Verify, + ))); + } + // Fail closed on tracker overflow: a silent drop here would + // allow an attacker to arrange 64 distinct codepoints and + // then smuggle a duplicate past `contains` — bypassing + // RFC 5246 §7.4.1.4 and, crucially, the CH1/CH2 CID + // equality check the HVR cookie binding provides. 64 + // min-size extensions cost 256 bytes, well inside a single + // ClientHello. + seen_types.try_push(ty).map_err(|_| { + nom::Err::Failure(nom::error::Error::new( + extensions_rest, + nom::error::ErrorKind::TooLarge, + )) + })?; + if extension.extension_type.is_supported() { - extensions.push(extension); + // Defense-in-depth duplicate gate: even if `seen_types` + // was bypassed (Round-5 review #1), reject a second + // supported extension of the same type before it lands + // in `ExtensionVec`. Downstream (HVR cookie binding, + // `handle_client_hello_extensions`) relies on + // at-most-one-`connection_id` to hold its CH1/CH2 + // equality guarantee. + if extensions + .iter() + .any(|e| e.extension_type == extension.extension_type) + { + return Err(nom::Err::Failure(nom::error::Error::new( + extensions_rest, + nom::error::ErrorKind::Verify, + ))); + } + extensions.try_push(extension).map_err(|_| { + nom::Err::Failure(nom::error::Error::new( + extensions_rest, + nom::error::ErrorKind::TooLarge, + )) + })?; } extensions_rest = rest; } @@ -315,4 +371,127 @@ mod tests { let result = ClientHello::parse(&message, 0); assert!(result.is_err()); } + + /// RFC 5246 §7.4.1.4: duplicate Hello extensions are forbidden. A + /// ClientHello with two `connection_id` (0x0036) extensions must fail + /// to parse rather than let the second overwrite the first after + /// cookie binding. Directly targets the CID-specific risk where the + /// HVR cookie binds the first raw CID but server negotiation would + /// otherwise let the second CID overwrite `peer_cid`. + #[test] + fn duplicate_connection_id_extension_rejected() { + // Build a ClientHello: base message + extensions length + two CID + // extensions with different CID values. + let mut msg = MESSAGE.to_vec(); + // extensions_length (2 bytes) + // Each CID extension body: 2 (type) + 2 (len) + 1 (cid_len) + 1 (cid byte) = 6 bytes + let ext_total = 6u16 * 2; + msg.extend_from_slice(&ext_total.to_be_bytes()); + // Extension 1: connection_id with cid = [0xAA] + msg.extend_from_slice(&[0x00, 0x36, 0x00, 0x02, 0x01, 0xAA]); + // Extension 2: connection_id with cid = [0xBB] — duplicate type + msg.extend_from_slice(&[0x00, 0x36, 0x00, 0x02, 0x01, 0xBB]); + + let result = ClientHello::parse(&msg, 0); + assert!( + result.is_err(), + "duplicate connection_id extension must fail to parse" + ); + } + + /// RFC 5246 §7.4.1.4 applies to every extension codepoint, not just + /// supported ones. Two extensions with the same unknown type must + /// fail to parse; two extensions with distinct unknown types must + /// still parse and be silently ignored. + #[test] + fn duplicate_unknown_extension_rejected() { + let mut msg = MESSAGE.to_vec(); + // extensions_length = 8 (two 4-byte zero-body unknown extensions). + msg.extend_from_slice(&0x0008u16.to_be_bytes()); + // Unknown ext type 0xFAFA (not in ExtensionType::supported()), zero body + msg.extend_from_slice(&[0xFA, 0xFA, 0x00, 0x00]); + // Duplicate 0xFAFA + msg.extend_from_slice(&[0xFA, 0xFA, 0x00, 0x00]); + + let result = ClientHello::parse(&msg, 0); + assert!( + result.is_err(), + "duplicate unknown extension type must fail to parse" + ); + } + + #[test] + fn distinct_unknown_extensions_still_parse() { + let mut msg = MESSAGE.to_vec(); + msg.extend_from_slice(&0x0008u16.to_be_bytes()); + msg.extend_from_slice(&[0xFA, 0xFA, 0x00, 0x00]); + msg.extend_from_slice(&[0xFB, 0xFB, 0x00, 0x00]); + + let (_, parsed) = ClientHello::parse(&msg, 0).expect("distinct unknowns must parse"); + // Unknown extensions are not stored (filtered post-dedup). + assert!(parsed.extensions.is_empty()); + } + + /// Security-relevant regression (2026-04-22 review #1): prior code + /// silently dropped on `seen_types.try_push` overflow, letting an + /// attacker arrange 64 distinct codepoints followed by two + /// `connection_id` extensions and bypass CH1/CH2 CID equality. With + /// fail-closed overflow handling, this pattern now parse-fails. + #[test] + fn seen_types_overflow_fails_closed() { + let mut ext_data: Vec = Vec::new(); + // 64 distinct unknown codepoints (0x1000..0x103F), zero body. + for i in 0x1000u16..0x1040u16 { + ext_data.extend_from_slice(&i.to_be_bytes()); + ext_data.extend_from_slice(&[0x00, 0x00]); + } + // Two `connection_id` extensions with different CID values — if + // the overflow silently drops, these duplicate past the guard + // and the server's `peer_cid` can diverge from the cookie-bound + // value. + ext_data.extend_from_slice(&[0x00, 0x36, 0x00, 0x02, 0x01, 0xAA]); + ext_data.extend_from_slice(&[0x00, 0x36, 0x00, 0x02, 0x01, 0xBB]); + + let mut msg = MESSAGE.to_vec(); + msg.extend_from_slice(&(ext_data.len() as u16).to_be_bytes()); + msg.extend_from_slice(&ext_data); + + let result = ClientHello::parse(&msg, 0); + assert!( + result.is_err(), + "65th+ codepoint must fail-close parsing, not silently drop the dedup guarantee" + ); + } + + /// Capacity overflow: more supported extensions than `ExtensionVec` + /// capacity must return an error, not panic. `ExtensionVec` capacity + /// equals the supported-extension count, so we synthesize one + /// occurrence of every supported extension PLUS a duplicate to + /// exceed capacity (and also hit the duplicate check — either path + /// must return an error, never panic). + #[test] + fn too_many_supported_extensions_does_not_panic() { + use crate::dtls12::message::ExtensionType; + let supported = ExtensionType::supported(); + let mut msg = MESSAGE.to_vec(); + // Each extension body: 4 bytes (type + length), 0 bytes data + let ext_total = (4 * (supported.len() + 1)) as u16; + msg.extend_from_slice(&ext_total.to_be_bytes()); + for et in supported.iter() { + let ty = et.as_u16(); + msg.extend_from_slice(&ty.to_be_bytes()); + msg.extend_from_slice(&[0x00, 0x00]); // zero-length body + } + // One more copy of the first supported type → duplicate. + let ty = supported[0].as_u16(); + msg.extend_from_slice(&ty.to_be_bytes()); + msg.extend_from_slice(&[0x00, 0x00]); + + // Must not panic; must return an error. + let result = ClientHello::parse(&msg, 0); + assert!( + result.is_err(), + "over-capacity extensions must return an error, not panic" + ); + } } diff --git a/src/dtls12/message/extension.rs b/src/dtls12/message/extension.rs index 59da8920..5db579cd 100644 --- a/src/dtls12/message/extension.rs +++ b/src/dtls12/message/extension.rs @@ -90,6 +90,7 @@ pub enum ExtensionType { PostHandshakeAuth, SignatureAlgorithmsCert, KeyShare, + ConnectionId, RenegotiationInfo, Unknown(u16), } @@ -140,6 +141,7 @@ impl ExtensionType { 0x0031 => ExtensionType::PostHandshakeAuth, 0x0032 => ExtensionType::SignatureAlgorithmsCert, 0x0033 => ExtensionType::KeyShare, + 0x0036 => ExtensionType::ConnectionId, 0xFF01 => ExtensionType::RenegotiationInfo, _ => ExtensionType::Unknown(value), } @@ -184,6 +186,7 @@ impl ExtensionType { ExtensionType::PostHandshakeAuth => 0x0031, ExtensionType::SignatureAlgorithmsCert => 0x0032, ExtensionType::KeyShare => 0x0033, + ExtensionType::ConnectionId => 0x0036, ExtensionType::RenegotiationInfo => 0xFF01, ExtensionType::Unknown(value) => *value, } @@ -200,7 +203,7 @@ impl ExtensionType { } /// Supported extension types that this implementation handles. - pub const fn supported() -> &'static [ExtensionType; 8] { + pub const fn supported() -> &'static [ExtensionType; 9] { &[ ExtensionType::SupportedGroups, ExtensionType::EcPointFormats, @@ -210,6 +213,7 @@ impl ExtensionType { ExtensionType::ExtendedMasterSecret, ExtensionType::RenegotiationInfo, ExtensionType::SessionTicket, + ExtensionType::ConnectionId, ] } } diff --git a/src/dtls12/message/extensions/connection_id.rs b/src/dtls12/message/extensions/connection_id.rs new file mode 100644 index 00000000..42917cdf --- /dev/null +++ b/src/dtls12/message/extensions/connection_id.rs @@ -0,0 +1,97 @@ +use crate::buffer::Buf; +use arrayvec::ArrayVec; +use nom::{IResult, bytes::complete::take, number::complete::be_u8}; + +/// Connection ID extension as defined in RFC 9146. +/// +/// The CID value is what the sender wants the peer to include in +/// encrypted records sent back. An empty CID is valid — it means +/// "negotiate CID but don't include one in records sent to me." +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ConnectionIdExtension { + pub cid: ArrayVec, +} + +impl ConnectionIdExtension { + pub fn new(cid: &[u8]) -> Self { + let mut arr = ArrayVec::new(); + // unwrap: cid.len() <= 255 enforced by config validation + arr.try_extend_from_slice(cid).unwrap(); + ConnectionIdExtension { cid: arr } + } + + pub fn parse(input: &[u8]) -> IResult<&[u8], ConnectionIdExtension> { + let (input, cid_len) = be_u8(input)?; + let (input, cid_bytes) = take(cid_len as usize)(input)?; + let mut cid = ArrayVec::new(); + // unwrap: cid_len <= 255, ArrayVec capacity is 255 + cid.try_extend_from_slice(cid_bytes).unwrap(); + // RFC 9146 §3 defines the extension body as exactly a `ConnectionId` + // structure. Trailing bytes are a malformed extension — per RFC 5246 + // §7.2.2 / RFC 8446 §6 that's a decode_error. Fail strictly instead + // of silently accepting, matching every other extension parser. + if !input.is_empty() { + return Err(nom::Err::Error(nom::error::Error::new( + input, + nom::error::ErrorKind::Verify, + ))); + } + Ok((input, ConnectionIdExtension { cid })) + } + + pub fn serialize(&self, output: &mut Buf) { + output.push(self.cid.len() as u8); + output.extend_from_slice(&self.cid); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::buffer::Buf; + + #[test] + fn roundtrip() { + let cid = [0x01, 0x02, 0x03, 0x04]; + let ext = ConnectionIdExtension::new(&cid); + + let mut serialized = Buf::new(); + ext.serialize(&mut serialized); + + assert_eq!( + &*serialized, + &[0x04, 0x01, 0x02, 0x03, 0x04] // length(1) + cid(4) + ); + + let (rest, parsed) = ConnectionIdExtension::parse(&serialized).unwrap(); + assert!(rest.is_empty()); + assert_eq!(parsed, ext); + } + + #[test] + fn empty_cid() { + let ext = ConnectionIdExtension::new(&[]); + + let mut serialized = Buf::new(); + ext.serialize(&mut serialized); + + assert_eq!(&*serialized, &[0x00]); // length(1) only + + let (rest, parsed) = ConnectionIdExtension::parse(&serialized).unwrap(); + assert!(rest.is_empty()); + assert_eq!(parsed, ext); + } + + /// RFC 9146 §3 defines the extension body as exactly a `ConnectionId` + /// structure. Trailing bytes must be rejected (RFC 5246 §7.2.2 decode_error). + #[test] + fn rejects_trailing_bytes() { + // length(1) + cid(2) + trailing(1) + let malformed = [0x02, 0xAA, 0xBB, 0xCC]; + assert!(ConnectionIdExtension::parse(&malformed).is_err()); + + // Also check zero-length CID with trailing bytes. + let malformed_zero = [0x00, 0xDE]; + assert!(ConnectionIdExtension::parse(&malformed_zero).is_err()); + } +} diff --git a/src/dtls12/message/extensions/mod.rs b/src/dtls12/message/extensions/mod.rs index 7c6ec7c0..ba16ccd6 100644 --- a/src/dtls12/message/extensions/mod.rs +++ b/src/dtls12/message/extensions/mod.rs @@ -1,8 +1,10 @@ +pub mod connection_id; pub mod ec_point_formats; pub mod signature_algorithms; pub mod supported_groups; pub mod use_srtp; +pub use connection_id::ConnectionIdExtension; pub use ec_point_formats::ECPointFormatsExtension; pub use signature_algorithms::SignatureAlgorithmsExtension; pub use supported_groups::SupportedGroupsExtension; diff --git a/src/dtls12/message/mod.rs b/src/dtls12/message/mod.rs index 78d8d3d9..d31aee3c 100644 --- a/src/dtls12/message/mod.rs +++ b/src/dtls12/message/mod.rs @@ -30,6 +30,7 @@ pub use client_hello::ClientHello; pub use client_key_exchange::{ClientKeyExchange, ClientPskKeys, ExchangeKeys}; pub use digitally_signed::DigitallySigned; pub use extension::{Extension, ExtensionType}; +pub use extensions::connection_id::ConnectionIdExtension; pub use extensions::signature_algorithms::SignatureAlgorithmsExtension; pub use extensions::supported_groups::SupportedGroupsExtension; pub use extensions::use_srtp::{SrtpProfileId, SrtpProfileVec, UseSrtpExtension}; diff --git a/src/dtls12/message/record.rs b/src/dtls12/message/record.rs index 896e0af3..0cdbb856 100644 --- a/src/dtls12/message/record.rs +++ b/src/dtls12/message/record.rs @@ -35,9 +35,6 @@ impl DTLSRecord { /// Length of the explicit nonce prefix in DTLS 1.2 AES-GCM records. pub const EXPLICIT_NONCE_LEN: usize = 8; - /// Byte offset in the record header where the 2-byte length field is - pub const LENGTH_OFFSET: Range = 11..13; - /// Parse a DTLS record from the input buffer. pub fn parse( input: &[u8], diff --git a/src/dtls12/message/server_hello.rs b/src/dtls12/message/server_hello.rs index f11360b4..4e8ee5bd 100644 --- a/src/dtls12/message/server_hello.rs +++ b/src/dtls12/message/server_hello.rs @@ -1,4 +1,5 @@ use super::extension::ExtensionVec; +use super::extensions::connection_id::ConnectionIdExtension; use super::extensions::use_srtp::{SrtpProfileId, UseSrtpExtension}; use super::{CompressionMethod, Dtls12CipherSuite, Extension, ExtensionType}; use super::{ProtocolVersion, Random, SessionId}; @@ -42,7 +43,12 @@ impl ServerHello { /// - Uses the provided buffer to stage extension bytes and then stores Range references /// - Includes UseSRTP if a profile is provided /// - Includes Extended Master Secret if the flag is set - pub fn with_extensions(mut self, buf: &mut Buf, srtp_profile: Option) -> Self { + pub fn with_extensions( + mut self, + buf: &mut Buf, + srtp_profile: Option, + connection_id: Option<&[u8]>, + ) -> Self { // Clear the buffer and collect extension byte ranges buf.clear(); @@ -70,6 +76,13 @@ impl ServerHello { buf.push(0); // renegotiated_connection length = 0 ranges.push((ExtensionType::RenegotiationInfo, start, buf.len())); + // Connection ID (RFC 9146) - echo our CID if the client offered CID + if let Some(cid) = connection_id { + let start = buf.len(); + ConnectionIdExtension::new(cid).serialize(buf); + ranges.push((ExtensionType::ConnectionId, start, buf.len())); + } + let mut extensions = ExtensionVec::new(); for (t, s, e) in ranges { extensions.push(Extension { @@ -112,18 +125,45 @@ impl ServerHello { input_ext.as_ptr() as usize - original_input.as_ptr() as usize; let ext_base_offset = base_offset + consumed_to_ext_data; - // Parse extensions manually to pass base_offset, filtering unknown types + // RFC 5246 §7.4.1.4: "There MUST NOT be more than one + // extension of the same type." The rule applies to every + // codepoint, not just ones we support, so track all seen + // raw u16 types before filtering. let mut extensions_vec = ExtensionVec::new(); let mut current_input = input_ext; let mut current_offset = ext_base_offset; + let mut seen_types: ArrayVec = ArrayVec::new(); while !current_input.is_empty() { let before_len = current_input.len(); let (new_rest, ext) = Extension::parse(current_input, current_offset)?; let parsed_len = before_len - new_rest.len(); current_offset += parsed_len; - // Only keep supported extension types + + let ty = ext.extension_type.as_u16(); + if seen_types.contains(&ty) { + return Err(Err::Failure(Error::new(current_input, ErrorKind::Verify))); + } + // Fail closed on tracker overflow — matches ClientHello + // handling; silent drop would defeat the RFC 5246 + // §7.4.1.4 no-duplicates guarantee. + seen_types.try_push(ty).map_err(|_| { + Err::Failure(Error::new(current_input, ErrorKind::TooLarge)) + })?; + if ext.extension_type.is_supported() { - extensions_vec.push(ext); + // Defense-in-depth duplicate gate mirroring + // ClientHello: reject supported duplicates at + // the ExtensionVec layer too, so the dedup + // guarantee does not rely solely on `seen_types`. + if extensions_vec + .iter() + .any(|e| e.extension_type == ext.extension_type) + { + return Err(Err::Failure(Error::new(current_input, ErrorKind::Verify))); + } + extensions_vec.try_push(ext).map_err(|_| { + Err::Failure(Error::new(current_input, ErrorKind::TooLarge)) + })?; } current_input = new_rest; } @@ -219,4 +259,32 @@ mod test { let result = ServerHello::parse(&message, 0); assert!(result.is_err()); } + + /// RFC 5246 §7.4.1.4: duplicate Hello extensions are forbidden. A + /// ServerHello with two `connection_id` extensions must fail to + /// parse so the second value can't overwrite the first after the + /// client has stored `peer_cid`. + #[test] + fn duplicate_connection_id_extension_rejected() { + // Build a fresh ServerHello: random + session_id + cipher_suite + + // compression + extensions_length + two CID extensions. + let mut msg: Vec = Vec::new(); + msg.extend_from_slice(&[0xFE, 0xFD]); // DTLS 1.2 + msg.extend_from_slice(&[0u8; 32]); // Random + msg.push(0x00); // SessionId length = 0 + msg.extend_from_slice(&[0xC0, 0x2B]); // cipher suite + msg.push(0x00); // compression method Null + // extensions_length: two CID extensions, each 6 bytes → 12 bytes. + msg.extend_from_slice(&0x000Cu16.to_be_bytes()); + // CID ext 1: type=0x0036, len=2, cid_len=1, cid=[0xAA] + msg.extend_from_slice(&[0x00, 0x36, 0x00, 0x02, 0x01, 0xAA]); + // CID ext 2: same type, different value → duplicate. + msg.extend_from_slice(&[0x00, 0x36, 0x00, 0x02, 0x01, 0xBB]); + + let result = ServerHello::parse(&msg, 0); + assert!( + result.is_err(), + "duplicate connection_id in ServerHello must fail to parse" + ); + } } diff --git a/src/dtls12/queue.rs b/src/dtls12/queue.rs index 91994c01..eae1fb74 100644 --- a/src/dtls12/queue.rs +++ b/src/dtls12/queue.rs @@ -54,7 +54,7 @@ impl fmt::Debug for QueueRx { ContentType::ApplicationData => app_data += 1, ContentType::Alert => alert += 1, ContentType::ChangeCipherSpec => ccs += 1, - ContentType::Unknown(_) | ContentType::Ack => other += 1, + ContentType::Tls12Cid | ContentType::Unknown(_) | ContentType::Ack => other += 1, } let seq = (record.sequence.epoch, record.sequence.sequence_number); diff --git a/src/dtls12/server.rs b/src/dtls12/server.rs index 8d3d3352..a06ed324 100644 --- a/src/dtls12/server.rs +++ b/src/dtls12/server.rs @@ -28,13 +28,14 @@ use crate::dtls12::engine::Engine; use crate::dtls12::message::PskParams; use crate::dtls12::message::{Body, CertificateRequest, CertificateTypeVec, Dtls12CipherSuite}; use crate::dtls12::message::{ClientCertificateType, CompressionMethod, ContentType}; +use crate::dtls12::message::{ClientHello, ConnectionIdExtension, SrtpProfileVec}; use crate::dtls12::message::{Cookie, CurveType, DistinguishedName, ExchangeKeys, ExtensionType}; use crate::dtls12::message::{HashAlgorithm, HelloVerifyRequest, KeyExchangeAlgorithm}; use crate::dtls12::message::{MessageType, NamedGroup, NamedGroupVec, ProtocolVersion, Random}; use crate::dtls12::message::{ServerHello, SessionId, SignatureAlgorithm}; use crate::dtls12::message::{SignatureAlgorithmsExtension, SignatureAndHashAlgorithm}; use crate::dtls12::message::{SignatureAndHashAlgorithmVec, SrtpProfileId}; -use crate::dtls12::message::{SrtpProfileVec, SupportedGroupsExtension, UseSrtpExtension}; +use crate::dtls12::message::{SupportedGroupsExtension, UseSrtpExtension}; use crate::{Config, Error, Output}; /// Length of the random dummy PSK used when identity resolution fails. @@ -66,6 +67,9 @@ pub struct Server { /// The negotiated SRTP profile (if any) negotiated_srtp_profile: Option, + /// Peer's Connection ID (from ClientHello extension). We put this in records to the peer. + peer_cid: Option>, + /// Client's offered supported_groups (if any) client_supported_groups: Option, @@ -166,6 +170,7 @@ impl Server { cookie_secret, extension_data: Buf::new(), negotiated_srtp_profile: None, + peer_cid: None, client_supported_groups: None, client_signature_algorithms: None, client_random: None, @@ -179,7 +184,21 @@ impl Server { } } + /// Borrow the underlying DTLS 1.2 engine (crate-internal accessor). + pub(crate) fn engine(&self) -> &Engine { + &self.engine + } + pub fn into_client(self) -> Client { + // Role flip must happen before handshake completion — see + // `Dtls::set_active` rustdoc. Any negotiated CID implies a + // post-handshake flip, which is unsupported because CID labels + // are tied to the original role. + debug_assert!( + self.engine.inbound_cid().is_none(), + "Dtls::set_active role flip after CID negotiation is unsupported; \ + CID labels are tied to the original role. Use a fresh Dtls instance." + ); Client::new_with_engine(self.engine, self.last_now) } @@ -194,6 +213,23 @@ impl Server { } pub fn poll_output<'a>(&mut self, buf: &'a mut [u8]) -> Output<'a> { + // `Output::ConnectionId` borrows from `buf`. If the caller's buffer + // can't hold the negotiated CID, defer the notification — leave the + // event at the front of the queue and fall through to the engine. + // The caller will see `Timeout`, retry `poll_output` with a larger + // buffer, and the ConnectionId event is delivered then. This keeps + // the CID plumbing panic-free. + if let Some(LocalEvent::ConnectionId(cid)) = self.local_events.front() { + if cid.len() > buf.len() { + warn!( + "poll_output buffer too small for ConnectionId ({} bytes < {} bytes); deferring", + buf.len(), + cid.len() + ); + return self.engine.poll_output(buf, self.last_now); + } + } + if let Some(event) = self.local_events.pop_front() { return event.into_output(buf, &self.client_certificates); } @@ -343,8 +379,18 @@ impl State { ch.cipher_suites.len() ); - // Stateless cookie: require 32-byte cookie matching HMAC(secret, client_random) + // Stateless cookie: require 32-byte cookie matching + // HMAC(secret, client_random || offered_cid). RFC 9146 §3 + §7 + // HelloVerifyRequest example: "If a + // server sends a HelloVerifyRequest, then the second ClientHello + // sent by the client MUST contain the same connection_id extension + // as the first one." Binding the offered CID into the cookie + // provides that equality check without any server-side per-client + // state — a swap across the CH pair produces a fresh CH whose + // cookie no longer verifies, so the server issues another HVR and + // the exchange cannot complete with mismatched CIDs. let client_random = ch.random; + let offered_cid = extract_offered_cid(&ch, &server.defragment_buffer); let hmac_provider = server.engine.config().crypto_provider().hmac_provider; let need_cookie = server.engine.config().use_server_cookie(); let cookie_valid = !need_cookie @@ -352,12 +398,18 @@ impl State { hmac_provider, &server.cookie_secret, client_random, + offered_cid.as_deref(), ch.cookie, ); if !cookie_valid { debug!("Invalid/missing cookie; sending HelloVerifyRequest"); - let cookie = compute_cookie(hmac_provider, &server.cookie_secret, client_random)?; + let cookie = compute_cookie( + hmac_provider, + &server.cookie_secret, + client_random, + offered_cid.as_deref(), + )?; // Start/restart flight timer for server Flight 2 (HelloVerifyRequest) server.engine.flight_begin(2); server @@ -437,6 +489,22 @@ impl State { warn!("Failed to parse SignatureAlgorithms extension"); } } + ExtensionType::ConnectionId => { + let ext_data = ext.extension_data(&server.defragment_buffer); + match ConnectionIdExtension::parse(ext_data) { + Ok((_, cid_ext)) => { + // Store the client's CID — we'll include it in records we send + server.peer_cid = Some(cid_ext.cid.to_vec()); + trace!("Client offered Connection ID ({} bytes)", cid_ext.cid.len()); + } + Err(_) => { + // RFC 5246 §7.2.2 decode_error on malformed extension. + return Err(Error::SecurityError( + "Malformed connection_id extension from client".to_string(), + )); + } + } + } _ => {} } } @@ -489,6 +557,21 @@ impl State { let negotiated_srtp_profile = server.negotiated_srtp_profile; let extension_data = &mut server.extension_data; + // Determine CID to include in ServerHello extension. + // We echo our CID (from config) only if the client also offered CID. + let our_cid = if server.peer_cid.is_some() { + server.engine.config().connection_id().map(|c| c.to_vec()) + } else { + None + }; + + // `set_cid_negotiated` arms outbound CID immediately; inbound is armed + // separately when the peer's CCS is processed. + if let (Some(peer_cid), Some(our_cid_clone)) = (server.peer_cid.clone(), our_cid.clone()) { + server.engine.set_cid_negotiated(our_cid_clone, peer_cid); + debug!("Connection ID negotiated"); + } + // Send ServerHello server .engine @@ -500,6 +583,7 @@ impl State { session_id, negotiated_srtp_profile, extension_data, + our_cid.as_deref(), ) })?; @@ -1071,6 +1155,12 @@ impl State { debug!("Handshake complete; ready for application data"); server.local_events.push_back(LocalEvent::Connected); + if let Some(cid) = server.engine.inbound_cid() { + server + .local_events + .push_back(LocalEvent::ConnectionId(cid.to_vec())); + } + // Emit SRTP keying material if negotiated if let Some(profile) = server.negotiated_srtp_profile { let suite_hash = server.engine.cipher_suite().unwrap().hash_algorithm(); @@ -1143,10 +1233,24 @@ fn compute_cookie( hmac_provider: &dyn crate::crypto::HmacProvider, secret: &[u8], client_random: Random, + offered_cid: Option<&[u8]>, ) -> Result { - // cookie = trunc_32(HMAC(secret, client_random)) + // cookie = trunc_32(HMAC(secret, client_random || cid_marker || cid_bytes)) + // + // The cid_marker byte distinguishes "no CID extension offered" (0x00) + // from "CID extension offered with zero-length value" (0x01). Without + // the marker, these two CH variants would collide in the HMAC input + // and the CID-equality check RFC 9146 §3 / §7 illustrates would not hold. let mut buf = Buf::new(); client_random.serialize(&mut buf); + match offered_cid { + None => buf.push(0x00), + Some(cid) => { + buf.push(0x01); + buf.push(cid.len() as u8); + buf.extend_from_slice(cid); + } + } let tag = hmac_provider .hmac_sha256(secret, &buf) .map_err(|e| Error::CryptoError(format!("Failed to compute HMAC: {}", e)))?; @@ -1159,18 +1263,35 @@ fn verify_cookie( hmac_provider: &dyn crate::crypto::HmacProvider, secret: &[u8], client_random: Random, + offered_cid: Option<&[u8]>, cookie: Cookie, ) -> bool { if cookie.len() != 32 { return false; } - match compute_cookie(hmac_provider, secret, client_random) { + match compute_cookie(hmac_provider, secret, client_random, offered_cid) { // Use constant-time comparison to prevent timing attacks Ok(expected) => expected.as_ref().ct_eq(cookie.as_ref()).into(), Err(_) => false, } } +/// Extract the client's offered CID extension body from a ClientHello, if +/// present. Returns the *raw* extension_data bytes (not the parsed CID value) +/// so cookie binding is robust even when the extension body is malformed: +/// two CHs with identical extension bytes produce the same cookie and pass +/// the RFC 9146 §3 / §7 CID-equality check; the strict extension parser runs later +/// in `handle_client_hello_extensions` to reject malformed bodies with +/// SecurityError / decode_error. +fn extract_offered_cid(ch: &ClientHello, defragment_buffer: &[u8]) -> Option> { + for ext in ch.extensions.iter() { + if ext.extension_type == ExtensionType::ConnectionId { + return Some(ext.extension_data(defragment_buffer).to_vec()); + } + } + None +} + fn handshake_create_certificate(body: &mut Buf, engine: &mut Engine) -> Result<(), Error> { let crypto = engine.crypto_context(); crypto.serialize_client_certificate(body); @@ -1184,6 +1305,7 @@ fn handshake_create_server_hello( session_id: SessionId, negotiated_srtp_profile: Option, extension_data: &mut Buf, + connection_id: Option<&[u8]>, ) -> Result<(), Error> { let server_version = ProtocolVersion::DTLS1_2; @@ -1205,7 +1327,7 @@ fn handshake_create_server_hello( CompressionMethod::Null, None, ) - .with_extensions(extension_data, srtp_pid); + .with_extensions(extension_data, srtp_pid, connection_id); sh.serialize(extension_data, body); Ok(()) diff --git a/src/dtls13/client.rs b/src/dtls13/client.rs index b18408da..cdd8d14c 100644 --- a/src/dtls13/client.rs +++ b/src/dtls13/client.rs @@ -124,6 +124,17 @@ pub struct Client { /// Server handshake traffic secret (for server Finished verification) server_hs_traffic_secret: Option, + + /// Whether our own ClientHello on the wire offered the + /// `connection_id(0x0036)` codepoint. `true` only via + /// `Client::new_from_hybrid` when the hybrid CH emitted CID + /// (RFC 9147 §9 solicitation); `false` on the direct + /// `Dtls::new_13` path, which never emits the extension. Used by + /// the ServerHello / EncryptedExtensions handlers to distinguish + /// "server echo of our solicitation" (silent-accept since dimpl + /// does not implement DTLS 1.3 CID) from "true unsolicited + /// extension" (RFC 8446 §4.2 `illegal_parameter` abort). + offered_cid: bool, } #[derive(Debug, PartialEq, Eq)] @@ -159,6 +170,10 @@ impl Client { handshake_secret: None, client_hs_traffic_secret: None, server_hs_traffic_secret: None, + // Direct `Dtls::new_13` path: the DTLS 1.3 ClientHello + // builder does not emit `connection_id(0x0036)`, so any + // server echo is truly unsolicited. + offered_cid: false, } } @@ -175,13 +190,18 @@ impl Client { certificate: crate::DtlsCertificate, now: Instant, ) -> Result { + // The hybrid CH emits `connection_id(0x0036)` iff the caller + // configured CID (see `HybridClientHello::new`). Capture that + // at fork time so the DTLS 1.3 handlers can tell a solicited + // echo from an unsolicited one without re-deriving from config. + let offered_cid = config.connection_id().is_some(); + let mut engine = Engine::new(config, certificate); engine.set_client(true); // Inject transcript + sequence state from the hybrid CH that was // already sent on the wire by ClientPending. engine.inject_hybrid_client_hello(&hybrid.transcript_bytes); - let mut client = Client { state: State::AwaitServerHello, engine, @@ -204,6 +224,7 @@ impl Client { handshake_secret: None, client_hs_traffic_secret: None, server_hs_traffic_secret: None, + offered_cid, }; client.handle_timeout(now)?; Ok(client) @@ -600,6 +621,30 @@ impl State { server_key_share = Some((ks.entry.group, ke_start..ke_end)); } } + ExtensionType::Unknown(0x0036) => { + // RFC 9147 §9 defines DTLS 1.3 CID; dimpl does not + // implement it. Silent-accept the echo iff our own + // ClientHello on the wire actually emitted `0x0036` + // (`client.offered_cid`). That flag is set true only + // by `Client::new_from_hybrid` when the hybrid CH + // solicited CID; the direct `Dtls::new_13` path + // never emits the codepoint, so a server echo on + // that path is unsolicited and MUST abort with + // `illegal_parameter` per RFC 8446 §4.2 — a + // `config.connection_id().is_some()` proxy would + // silent-accept incorrectly here. + if client.offered_cid { + warn!( + "DTLS 1.3 server echoed connection_id extension (our hybrid CH \ + solicited it); dimpl does not implement RFC 9147 CID, so the \ + echo is ignored and the session proceeds without CID framing" + ); + } else { + return Err(Error::SecurityError( + "DTLS 1.3 server sent unsolicited connection_id extension".to_string(), + )); + } + } _ => {} } } @@ -680,17 +725,40 @@ impl State { // Process extensions for ext in &ee.extensions { - if ext.extension_type == ExtensionType::UseSrtp { - let ext_data = ext.extension_data(&client.defragment_buffer); - if let Ok((_, use_srtp)) = UseSrtpExtension::parse(ext_data) { - if !use_srtp.profiles.is_empty() { - client.negotiated_srtp_profile = Some(use_srtp.profiles[0].into()); - trace!( - "EncryptedExtensions UseSRTP; selected profile: {:?}", - client.negotiated_srtp_profile + match ext.extension_type { + ExtensionType::UseSrtp => { + let ext_data = ext.extension_data(&client.defragment_buffer); + if let Ok((_, use_srtp)) = UseSrtpExtension::parse(ext_data) { + if !use_srtp.profiles.is_empty() { + client.negotiated_srtp_profile = Some(use_srtp.profiles[0].into()); + trace!( + "EncryptedExtensions UseSRTP; selected profile: {:?}", + client.negotiated_srtp_profile + ); + } + } + } + ExtensionType::Unknown(0x0036) => { + // Same reasoning as the ServerHello handler: gate + // on whether our own ClientHello actually emitted + // the codepoint, not on config. `offered_cid` is + // true only after `new_from_hybrid` with CID + // configured; the direct `new_13` path is always + // false even when `config.connection_id()` is Some. + if client.offered_cid { + warn!( + "DTLS 1.3 server echoed connection_id in EncryptedExtensions \ + (our hybrid CH solicited it); dimpl does not implement \ + RFC 9147 CID, so the echo is ignored" ); + } else { + return Err(Error::SecurityError( + "DTLS 1.3 server sent unsolicited connection_id extension in EncryptedExtensions" + .to_string(), + )); } } + _ => {} } } @@ -1021,7 +1089,19 @@ impl State { // Timers will stop when we receive an ACK from the server confirming it // received our epoch-2 flight. - // Emit Connected event + // Emit Connected event. If `Config::with_connection_id` was set + // but we resolved to DTLS 1.3 (where dimpl does not implement + // RFC 9147 CID), warn so callers relying on CID for routing + // have a log line alongside the new + // `Dtls::negotiated_connection_id()` accessor that returns + // `None` here. + if client.engine.config().connection_id().is_some() { + warn!( + "Config::with_connection_id was set, but this association negotiated DTLS 1.3 \ + where dimpl does not implement RFC 9147 CID. No CID routing will be available \ + — use `Dtls::negotiated_connection_id()` to detect this case programmatically." + ); + } client.local_events.push_back(LocalEvent::Connected); // Extract and emit SRTP keying material if negotiated diff --git a/src/dtls13/incoming.rs b/src/dtls13/incoming.rs index 79330ee9..a783cc3a 100644 --- a/src/dtls13/incoming.rs +++ b/src/dtls13/incoming.rs @@ -7,6 +7,7 @@ use std::fmt; use crate::Error; use crate::buffer::{Buf, TmpBuf}; use crate::dtls13::message::{ContentType, Dtls13CipherSuite, Dtls13Record, Handshake, Sequence}; +use crate::util::recover_inner_content_type; /// Holds both the UDP packet and the parsed result of that packet. pub struct Incoming { @@ -446,26 +447,6 @@ fn parse_handshakes( handshakes } -/// Recover the inner content type from a decrypted DTLSInnerPlaintext. -/// -/// The format is: `content || ContentType || zeros*` -/// Scan backward past zero padding to find the content type byte. -fn recover_inner_content_type(decrypted: &[u8]) -> Result<(ContentType, usize), Error> { - let mut i = decrypted.len(); - // Skip zero padding - while i > 0 && decrypted[i - 1] == 0 { - i -= 1; - } - if i == 0 { - return Err(Error::ParseError(nom::error::ErrorKind::Fail)); - } - // The byte before padding is the content type - i -= 1; - let content_type = ContentType::from_u8(decrypted[i]); - // Content length is everything before the content type byte - Ok((content_type, i)) -} - impl fmt::Debug for Incoming { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Incoming") diff --git a/src/dtls13/queue.rs b/src/dtls13/queue.rs index eeeea1c8..75de30ac 100644 --- a/src/dtls13/queue.rs +++ b/src/dtls13/queue.rs @@ -52,9 +52,10 @@ impl fmt::Debug for QueueRx { ContentType::Handshake => handshake += 1, ContentType::ApplicationData => app_data += 1, ContentType::Alert => alert += 1, - ContentType::Unknown(_) | ContentType::ChangeCipherSpec | ContentType::Ack => { - other += 1 - } + ContentType::Tls12Cid + | ContentType::Unknown(_) + | ContentType::ChangeCipherSpec + | ContentType::Ack => other += 1, } let seq = (record.sequence.epoch, record.sequence.sequence_number); diff --git a/src/dtls13/server.rs b/src/dtls13/server.rs index 71f3dfd7..3d184c9d 100644 --- a/src/dtls13/server.rs +++ b/src/dtls13/server.rs @@ -474,6 +474,26 @@ impl State { } } } + ExtensionType::Unknown(0x0036) => { + // Explicit policy marker: dimpl does not implement + // DTLS 1.3 Connection ID (RFC 9147 §9). A + // ClientHello offering `0x0036` in the hybrid / + // cross-version path is allowed (RFC 8446 §4.1.2: + // clients MAY send extensions that aren't + // applicable to the negotiated version); the + // DTLS 1.3 server path simply does not echo the + // extension in ServerHello or EncryptedExtensions, + // so no DTLS 1.3 CID is negotiated. A `debug!` log + // makes the "client requested, server ignored" + // outcome visible; the symmetric client-side + // behavior is documented at + // `src/dtls13/client.rs` for the ServerHello / + // EE reception paths. + debug!( + "DTLS 1.3 server: ignoring connection_id(0x0036) extension offered by \ + client — RFC 9147 CID is not implemented, no CID will be negotiated" + ); + } _ => {} } } diff --git a/src/error.rs b/src/error.rs index 37b21e27..990892e0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -52,6 +52,36 @@ pub enum Error { /// value to communicate from dtls13/server.rs to lib.rs #[doc(hidden)] Dtls12Fallback, + /// Application data exceeds the DTLS record-size ceiling. + /// + /// RFC 6347 §4.1.1 / RFC 9146 §5 cap `DTLSPlaintext` / + /// `DTLSInnerPlaintext` at `2^14` bytes. Callers must fragment larger + /// payloads before calling `send_application_data`. + Oversized(usize), + /// Configured MTU is too small to hold even a single record header plus + /// a handshake-header byte of body. Encountered when negotiated CID + /// bytes + AEAD overhead + handshake/record headers already fill or + /// exceed `Config::mtu()`. Terminal for this association: the caller + /// must either raise MTU or renegotiate with a shorter peer CID + /// (dimpl rejects renegotiation, so in practice the association + /// cannot make handshake progress). + MtuTooSmall { + /// Overhead bytes consumed by headers + CID + AEAD + handshake. + overhead: usize, + /// Configured MTU as reported by `Config::mtu()`. + mtu: usize, + }, + /// DTLS 1.2 sequence-number space is exhausted for this epoch. + /// + /// RFC 6347 §4.1: implementations MUST abandon the association or + /// rehandshake before the 48-bit wire sequence number wraps. dimpl + /// does not implement renegotiation or rekey, so this is terminal. + SequenceNumberExhausted { + /// The epoch whose sequence space is exhausted. + epoch: u16, + /// The next sequence number that would have been emitted. + sequence: u64, + }, } impl<'a> From>> for Error { @@ -91,6 +121,27 @@ impl std::fmt::Display for Error { Error::Dtls12Fallback => { write!(f, "dtls 1.2 fallback (internal)") } + Error::Oversized(n) => { + write!( + f, + "payload {} bytes exceeds DTLS record-size ceiling (16384)", + n + ) + } + Error::MtuTooSmall { overhead, mtu } => { + write!( + f, + "MTU {} cannot fit DTLS record overhead of {} bytes", + mtu, overhead + ) + } + Error::SequenceNumberExhausted { epoch, sequence } => { + write!( + f, + "DTLS 1.2 sequence number exhausted (epoch={}, seq={})", + epoch, sequence + ) + } } } } diff --git a/src/lib.rs b/src/lib.rs index f2ed5d0a..0dd99907 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -457,6 +457,57 @@ impl Dtls { } } + /// The highest authenticated (epoch, sequence_number) observed on + /// this association, or `None` if no record has been authenticated + /// yet. + /// + /// Intended for the RFC 9146 §6 peer-address-update policy: a + /// datagram whose source address differs from the currently-bound + /// address may authorize an address update only if the record it + /// carries (a) passes AEAD authentication and (b) has a + /// `(epoch, sequence_number)` strictly greater than what this + /// accessor returned before the datagram was delivered. Sample the + /// accessor before `handle_packet`, then again after `poll_output` + /// drained to `Timeout`: a strictly-increased value proves a fresh + /// authenticated record landed. + /// + /// Returns `None` for DTLS 1.3 associations and while still in the + /// auto-sense `Pending` state. DTLS 1.3 support is a separate + /// follow-up item; today only the DTLS 1.2 / DTLS 1.2 PSK paths + /// surface freshness metadata here. + pub fn newest_authenticated_record(&self) -> Option<(u16, u64)> { + // Current epoch is always 1 after the handshake activates + // application-data encryption on both DTLS 1.2 paths — the + // engine does not rekey and does not support renegotiation. + let seq = match self.inner.as_ref()? { + Inner::Client12(c) => c.engine().newest_authenticated_sequence()?, + Inner::Server12(s) => s.engine().newest_authenticated_sequence()?, + Inner::Client13(_) | Inner::Server13(_) | Inner::ClientPending(_) => return None, + }; + Some((1, seq)) + } + + /// Returns the CID the peer will include in records sent to us, or + /// `None` if CID was not negotiated. + /// + /// This differs from [`Config::connection_id`] (what the caller + /// requested) vs. what the handshake actually agreed on. `None` with + /// `config.connection_id().is_some()` means the CID was requested + /// but the peer did not negotiate it — most often because the + /// association resolved to DTLS 1.3, where dimpl does not implement + /// RFC 9147 CID. Callers using CID for routing should check this + /// accessor rather than relying on `Config::connection_id` alone. + /// + /// The returned slice may be empty if the peer advertised + /// zero-length CID (RFC 9146 §3 "legacy framing in that direction"). + pub fn negotiated_connection_id(&self) -> Option<&[u8]> { + match self.inner.as_ref()? { + Inner::Client12(c) => c.engine().inbound_cid(), + Inner::Server12(s) => s.engine().inbound_cid(), + Inner::Client13(_) | Inner::Server13(_) | Inner::ClientPending(_) => None, + } + } + /// Return true if the instance is operating in the client role. pub fn is_active(&self) -> bool { matches!( @@ -473,6 +524,18 @@ impl Dtls { /// client sends a hybrid ClientHello compatible with both DTLS 1.2 /// and 1.3. The version is determined from the server's first /// response. + /// + /// **Role flip is intended for auto-mode bootstrapping, not + /// mid-session swaps.** Flipping `active` after + /// [`Output::Connected`] fires is unsupported: the underlying + /// engine carries negotiated state (CID, replay window, sequence + /// numbers, transcript) that is labelled for the original role and + /// is not re-initialized across `into_client` / `into_server`. In + /// particular, a connected `Client12`/`Server12` with a negotiated + /// `connection_id` will keep its inbound/outbound CID labels in the + /// flipped role, where the labels no longer describe the correct + /// wire direction. Use a fresh `Dtls` instance instead of flipping + /// role after handshake completion. pub fn set_active(&mut self, active: bool) { match (self.is_active(), active) { (true, false) => { @@ -769,6 +832,12 @@ pub enum Output<'a> { ApplicationData(&'a [u8]), /// The peer sent a `close_notify` alert, indicating graceful connection closure. CloseNotify, + /// The negotiated Connection ID that identifies this endpoint (RFC 9146). + /// + /// Emitted once after `Connected` when CID was negotiated. The caller + /// uses this to route future datagrams (after peer address change) to + /// this `Dtls` instance. + ConnectionId(&'a [u8]), } impl fmt::Debug for Output<'_> { @@ -781,6 +850,7 @@ impl fmt::Debug for Output<'_> { Self::KeyingMaterial(v, p) => write!(f, "KeyingMaterial({}, {:?})", v.len(), p), Self::ApplicationData(v) => write!(f, "ApplicationData({})", v.len()), Self::CloseNotify => write!(f, "CloseNotify"), + Self::ConnectionId(v) => write!(f, "ConnectionId({})", v.len()), } } } diff --git a/src/types.rs b/src/types.rs index 48a9c2f1..b887cd34 100644 --- a/src/types.rs +++ b/src/types.rs @@ -428,6 +428,8 @@ pub enum ContentType { Handshake, /// Application data. ApplicationData, + /// Connection ID record (DTLS 1.2 only, RFC 9146). + Tls12Cid, /// ACK (DTLS 1.3 only, RFC 9147 Section 7). Ack, /// Unknown content type. @@ -448,6 +450,7 @@ impl ContentType { 21 => ContentType::Alert, 22 => ContentType::Handshake, 23 => ContentType::ApplicationData, + 25 => ContentType::Tls12Cid, 26 => ContentType::Ack, _ => ContentType::Unknown(value), } @@ -460,6 +463,7 @@ impl ContentType { ContentType::Alert => 21, ContentType::Handshake => 22, ContentType::ApplicationData => 23, + ContentType::Tls12Cid => 25, ContentType::Ack => 26, ContentType::Unknown(value) => *value, } diff --git a/src/util.rs b/src/util.rs index 68efa90f..abefec83 100644 --- a/src/util.rs +++ b/src/util.rs @@ -2,6 +2,9 @@ use arrayvec::ArrayVec; use nom::error::{ErrorKind, ParseError, make_error}; use nom::{Err, IResult, Input, Parser}; +use crate::Error; +use crate::types::ContentType; + /// A combinator that parses items using the provided parser but only collects /// items that pass a filter predicate. Allows zero matches. #[inline(always)] @@ -99,6 +102,34 @@ where } } +/// Recover the real content type from a DTLSInnerPlaintext. +/// +/// DTLSInnerPlaintext = content || real_content_type(1) || zeros(padding). +/// Scans backward past zero padding to find the content type byte. +/// Returns `(real_content_type, content_length)`. +/// +/// Rejects `ContentType::Unknown(_)` — unspecified values are a peer bug. +/// Callers using this inside a DTLS 1.2 CID record (RFC 9146 §5) must +/// additionally reject `Tls12Cid` (nested CID is nonsensical) and `Ack` +/// (DTLS 1.3 only). That DTLS-version-specific check stays in the caller +/// so DTLS 1.3 can still accept `Ack` here. +pub fn recover_inner_content_type(decrypted: &[u8]) -> Result<(ContentType, usize), Error> { + let mut i = decrypted.len(); + while i > 0 && decrypted[i - 1] == 0 { + i -= 1; + } + if i == 0 { + return Err(Error::ParseError(nom::error::ErrorKind::Fail)); + } + // The byte before padding is the content type + i -= 1; + let content_type = ContentType::from_u8(decrypted[i]); + if matches!(content_type, ContentType::Unknown(_)) { + return Err(Error::ParseError(nom::error::ErrorKind::Fail)); + } + Ok((content_type, i)) +} + pub fn be_u48>(input: I) -> IResult where I: Input, diff --git a/src/window.rs b/src/window.rs index 66b93e09..254648e8 100644 --- a/src/window.rs +++ b/src/window.rs @@ -31,6 +31,19 @@ impl ReplayWindow { } } + /// The highest authenticated sequence number ever observed in this + /// window. Returns `None` if no record has been authenticated yet + /// (i.e. `update` has never been called). RFC 9146 §6's + /// "strictly greater than the newest authenticated record already + /// seen" is computed against this value. + pub fn max_seq(&self) -> Option { + if self.window == 0 { + None + } else { + Some(self.max_seq) + } + } + /// Update the window state to record that `seqno` has been received. /// Must only be called after the record has been authenticated (decrypted successfully). pub fn update(&mut self, seqno: u64) { diff --git a/tests/auto/common.rs b/tests/auto/common.rs index ef9babe7..45374a5a 100644 --- a/tests/auto/common.rs +++ b/tests/auto/common.rs @@ -14,6 +14,7 @@ pub struct DrainedOutputs { pub connected: bool, pub peer_cert: Option>, pub keying_material: Option<(Vec, SrtpProfile)>, + pub connection_id: Option>, pub app_data: Vec>, pub timeout: Option, pub close_notify: bool, @@ -45,6 +46,7 @@ pub fn drain_outputs(endpoint: &mut Dtls) -> DrainedOutputs { Output::KeyingMaterial(km, profile) => { result.keying_material = Some((km.to_vec(), profile)); } + Output::ConnectionId(cid) => result.connection_id = Some(cid.to_vec()), Output::ApplicationData(data) => result.app_data.push(data.to_vec()), Output::CloseNotify => result.close_notify = true, Output::Timeout(t) => { diff --git a/tests/auto/handshake.rs b/tests/auto/handshake.rs index 4f32c26f..11bcf3b6 100644 --- a/tests/auto/handshake.rs +++ b/tests/auto/handshake.rs @@ -714,3 +714,86 @@ fn auto_client_protocol_version_after_negotiating_dtls12() { ); assert_eq!(client.protocol_version(), Some(ProtocolVersion::DTLS1_2)); } + +/// Regression for Round-6 review #1: when `Config::with_connection_id` +/// is set on an auto-mode client, the hybrid CH1 must carry the +/// `connection_id` extension so CH1/CH2 bytes agree across HVR and CID +/// is negotiated in a single HVR round-trip — not silently dropped or +/// forced into an extra retry. +#[test] +#[cfg(feature = "rcgen")] +fn auto_client_with_cid_to_dtls12_server_negotiates_cid() { + use dimpl::Config; + use dimpl::certificate::generate_self_signed_certificate; + + 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 client_cid = b"auto-cli-cid"; + let server_cid = b"auto-srv-cid"; + let client_config = Arc::new( + Config::builder() + .with_connection_id(client_cid.to_vec()) + .build() + .expect("client config"), + ); + let server_config = Arc::new( + Config::builder() + .with_connection_id(server_cid.to_vec()) + .build() + .expect("server config"), + ); + + let mut now = Instant::now(); + let mut client = Dtls::new_auto(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + let mut client_cid_event: Option> = None; + let mut server_cid_event: Option> = None; + + for _ in 0..60 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + + client_connected |= co.connected; + server_connected |= so.connected; + if co.connection_id.is_some() { + client_cid_event = co.connection_id; + } + if so.connection_id.is_some() { + server_cid_event = so.connection_id; + } + + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + + assert!( + client_connected && server_connected, + "auto+CID handshake should complete" + ); + assert_eq!( + client_cid_event.as_deref(), + Some(&client_cid[..]), + "auto client must emit Output::ConnectionId with its own CID" + ); + assert_eq!( + server_cid_event.as_deref(), + Some(&server_cid[..]), + "DTLS 1.2 server must emit Output::ConnectionId with its own CID" + ); +} diff --git a/tests/dtls12/cid.rs b/tests/dtls12/cid.rs new file mode 100644 index 00000000..b0ed52e2 --- /dev/null +++ b/tests/dtls12/cid.rs @@ -0,0 +1,2426 @@ +//! DTLS 1.2 Connection ID (RFC 9146) integration tests. + +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use dimpl::crypto::Dtls12CipherSuite; +use dimpl::{Config, Dtls, Output}; + +use crate::common::*; + +/// Name of an `Output` variant, without `Debug`-printing its payload. +/// Avoids logging sensitive-looking variant contents (e.g. `PeerCert`) in +/// assertion failure messages; CodeQL flags `{:?}` on `Output` as cleartext +/// logging even though our `Debug` impl only prints a length. +fn output_variant(o: &Output<'_>) -> &'static str { + match o { + Output::Packet(_) => "Packet", + Output::Timeout(_) => "Timeout", + Output::Connected => "Connected", + Output::PeerCert(_) => "PeerCert", + Output::KeyingMaterial(..) => "KeyingMaterial", + Output::ApplicationData(_) => "ApplicationData", + Output::CloseNotify => "CloseNotify", + Output::ConnectionId(_) => "ConnectionId", + _ => "Unknown", + } +} + +/// Helper to build a config with a connection ID. +fn dtls12_config_with_cid(cid: &[u8]) -> Arc { + Arc::new( + Config::builder() + .with_connection_id(cid.to_vec()) + .build() + .expect("Failed to build config"), + ) +} + +/// Helper to build a config with a connection ID and specific cipher suite. +fn dtls12_config_with_cid_and_suite(cid: &[u8], suite: Dtls12CipherSuite) -> Arc { + Arc::new( + Config::builder() + .with_connection_id(cid.to_vec()) + .dtls12_cipher_suites(&[suite]) + .build() + .expect("Failed to build config"), + ) +} + +/// Both sides configure CID → handshake completes, Output::ConnectionId emitted, +/// application data flows with CID-bearing records. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_both_sides() { + use dimpl::certificate::generate_self_signed_certificate; + + 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 client_cid = b"client-cid"; + let server_cid = b"server-cid"; + + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + let mut client_connection_id: Option> = None; + let mut server_connection_id: Option> = None; + + // Complete handshake + for _ in 0..50 { + 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; + + if client_out.connection_id.is_some() { + client_connection_id = client_out.connection_id; + } + if server_out.connection_id.is_some() { + server_connection_id = server_out.connection_id; + } + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + break; + } + + now += Duration::from_millis(10); + } + + assert!(client_connected, "Client should be connected"); + assert!(server_connected, "Server should be connected"); + + // Both sides should have emitted their CID + assert_eq!( + client_connection_id.as_deref(), + Some(client_cid.as_slice()), + "Client should emit its own CID" + ); + assert_eq!( + server_connection_id.as_deref(), + Some(server_cid.as_slice()), + "Server should emit its own CID" + ); + + // Application data should flow with CID-bearing records + let client_data = b"hello via CID"; + let server_data = b"world via CID"; + + client + .send_application_data(client_data) + .expect("client send"); + server + .send_application_data(server_data) + .expect("server send"); + + let mut client_received = Vec::new(); + let mut server_received = Vec::new(); + + for _ in 0..20 { + 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); + + // Verify outgoing packets use CID content type (25) + for pkt in &client_out.packets { + if !pkt.is_empty() && pkt[0] == 25 { + // CID record — verify CID bytes are present in header + assert!( + pkt.len() >= 11 + server_cid.len() + 2, + "CID record too short" + ); + } + } + + for data in &client_out.app_data { + client_received.extend_from_slice(data); + } + for data in &server_out.app_data { + server_received.extend_from_slice(data); + } + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if !client_received.is_empty() && !server_received.is_empty() { + break; + } + + now += Duration::from_millis(10); + } + + assert_eq!( + server_received, client_data, + "Server should receive client data" + ); + assert_eq!( + client_received, server_data, + "Client should receive server data" + ); +} + +/// Neither side configures CID → existing behavior unchanged, no ConnectionId emitted. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_neither_side() { + use dimpl::certificate::generate_self_signed_certificate; + + 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 = dtls12_config(); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(Arc::clone(&config), client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + let mut got_client_cid = false; + let mut got_server_cid = false; + + for _ in 0..50 { + 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; + got_client_cid |= client_out.connection_id.is_some(); + got_server_cid |= server_out.connection_id.is_some(); + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + break; + } + + now += Duration::from_millis(10); + } + + assert!(client_connected, "Client should be connected"); + assert!(server_connected, "Server should be connected"); + assert!(!got_client_cid, "Client should not emit ConnectionId"); + assert!(!got_server_cid, "Server should not emit ConnectionId"); + + // Application data should still work without CID + client.send_application_data(b"no cid").expect("send"); + + let client_pkts = collect_packets(&mut client); + + // Verify no CID content type in outgoing packets + for pkt in &client_pkts { + assert_ne!(pkt[0], 25, "Should not use CID content type"); + } + + deliver_packets(&client_pkts, &mut server); + let server_out = drain_outputs(&mut server); + assert_eq!(server_out.app_data.len(), 1); + assert_eq!(server_out.app_data[0], b"no cid"); +} + +/// One side offers CID, the other doesn't → graceful fallback to no-CID. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_one_side_only() { + use dimpl::certificate::generate_self_signed_certificate; + + 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"); + + // Client offers CID, server does not + let client_config = dtls12_config_with_cid(b"my-cid"); + let server_config = dtls12_config(); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + let mut got_client_cid = false; + let mut got_server_cid = false; + + for _ in 0..50 { + 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; + got_client_cid |= client_out.connection_id.is_some(); + got_server_cid |= server_out.connection_id.is_some(); + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + break; + } + + now += Duration::from_millis(10); + } + + assert!( + client_connected, + "Client should be connected even without CID" + ); + assert!( + server_connected, + "Server should be connected even without CID" + ); + // No CID should be emitted since server didn't offer one + assert!(!got_client_cid, "Client should not emit ConnectionId"); + assert!(!got_server_cid, "Server should not emit ConnectionId"); + + // Data should still flow + client.send_application_data(b"fallback").expect("send"); + let client_pkts = collect_packets(&mut client); + deliver_packets(&client_pkts, &mut server); + let server_out = drain_outputs(&mut server); + assert_eq!(server_out.app_data[0], b"fallback"); +} + +/// Empty CID (zero-length) is valid per RFC 9146. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_empty() { + use dimpl::certificate::generate_self_signed_certificate; + + 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"); + + // Both sides use empty CID + let client_config = dtls12_config_with_cid(b""); + let server_config = dtls12_config_with_cid(b""); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + let mut client_cid_event: Option> = None; + let mut server_cid_event: Option> = None; + + for _ in 0..50 { + 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; + + if client_out.connection_id.is_some() { + client_cid_event = client_out.connection_id; + } + if server_out.connection_id.is_some() { + server_cid_event = server_out.connection_id; + } + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + break; + } + + now += Duration::from_millis(10); + } + + assert!(client_connected, "Client should be connected"); + assert!(server_connected, "Server should be connected"); + + // RFC 9146 §3 + Output::ConnectionId contract: the variant fires on + // successful negotiation even when the negotiated value is empty. + assert_eq!( + client_cid_event.as_deref(), + Some(&[][..]), + "client must emit Output::ConnectionId(&[]) for zero-length negotiation" + ); + assert_eq!( + server_cid_event.as_deref(), + Some(&[][..]), + "server must emit Output::ConnectionId(&[]) for zero-length negotiation" + ); + + // Data should flow with empty CID using legacy (non-tls12_cid) framing. + client.send_application_data(b"empty cid").expect("send"); + let client_pkts = collect_packets(&mut client); + // RFC 9146 §3 zero-length-direction rule: legacy framing only — + // never content type 25 on the wire. + for p in &client_pkts { + assert_ne!( + p.first(), + Some(&25), + "zero-length CID direction must not emit tls12_cid (25) records" + ); + } + deliver_packets(&client_pkts, &mut server); + let server_out = drain_outputs(&mut server); + assert_eq!(server_out.app_data[0], b"empty cid"); +} + +/// CID with AES-128-GCM (8-byte explicit nonce) — verifies CID header layout +/// doesn't conflict with the explicit nonce prefix. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_aes128_gcm() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + let suite = Dtls12CipherSuite::ECDHE_ECDSA_AES128_GCM_SHA256; + let client_config = dtls12_config_with_cid_and_suite(b"gcm-client", suite); + let server_config = dtls12_config_with_cid_and_suite(b"gcm-server", suite); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + + for _ in 0..50 { + 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 { + break; + } + + now += Duration::from_millis(10); + } + + assert!( + client_connected, + "Client should connect with AES-128-GCM + CID" + ); + assert!( + server_connected, + "Server should connect with AES-128-GCM + CID" + ); + + // Bidirectional app data + client.send_application_data(b"gcm-cid").expect("send"); + let pkts = collect_packets(&mut client); + + // Verify CID content type in encrypted records + for pkt in &pkts { + if !pkt.is_empty() { + assert_eq!(pkt[0], 25, "Encrypted records should use CID content type"); + } + } + + deliver_packets(&pkts, &mut server); + let server_out = drain_outputs(&mut server); + assert_eq!(server_out.app_data[0], b"gcm-cid"); +} + +/// CID with ChaCha20-Poly1305 (no explicit nonce) — verifies CID works with +/// suites that don't prepend an explicit nonce. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_chacha20() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + let suite = Dtls12CipherSuite::ECDHE_ECDSA_CHACHA20_POLY1305_SHA256; + let client_config = dtls12_config_with_cid_and_suite(b"cc20-c", suite); + let server_config = dtls12_config_with_cid_and_suite(b"cc20-s", suite); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + + for _ in 0..50 { + 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 { + break; + } + + now += Duration::from_millis(10); + } + + assert!( + client_connected, + "Client should connect with ChaCha20 + CID" + ); + assert!( + server_connected, + "Server should connect with ChaCha20 + CID" + ); + + client.send_application_data(b"chacha-cid").expect("send"); + let pkts = collect_packets(&mut client); + deliver_packets(&pkts, &mut server); + let server_out = drain_outputs(&mut server); + assert_eq!(server_out.app_data[0], b"chacha-cid"); +} + +/// Asymmetric CID lengths — client uses short CID, server uses longer CID. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_asymmetric_lengths() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + // Client: 2-byte CID, Server: 16-byte CID + let client_config = dtls12_config_with_cid(b"\x01\x02"); + let server_config = dtls12_config_with_cid(b"0123456789abcdef"); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + let mut client_cid: Option> = None; + let mut server_cid: Option> = None; + + for _ in 0..50 { + 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; + + if client_out.connection_id.is_some() { + client_cid = client_out.connection_id; + } + if server_out.connection_id.is_some() { + server_cid = server_out.connection_id; + } + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + break; + } + + now += Duration::from_millis(10); + } + + assert!(client_connected); + assert!(server_connected); + + // Each side emits its own CID + assert_eq!(client_cid.as_deref(), Some(b"\x01\x02".as_slice())); + assert_eq!(server_cid.as_deref(), Some(b"0123456789abcdef".as_slice())); + + // Bidirectional data flows despite different CID lengths + client.send_application_data(b"short->long").expect("send"); + server.send_application_data(b"long->short").expect("send"); + + let mut client_app: Vec> = Vec::new(); + let mut server_app: Vec> = Vec::new(); + + for _ in 0..20 { + 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_app.extend(client_out.app_data.clone()); + server_app.extend(server_out.app_data.clone()); + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if !client_app.is_empty() && !server_app.is_empty() { + break; + } + + now += Duration::from_millis(10); + } + + assert!( + server_app.iter().any(|d| d == b"short->long"), + "Server should receive data via short CID" + ); + assert!( + client_app.iter().any(|d| d == b"long->short"), + "Client should receive data via long CID" + ); +} + +/// CID with retransmission — verify CID records work across flight retransmits. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_retransmission() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + let client_config = dtls12_config_with_cid(b"retx-c"); + let server_config = dtls12_config_with_cid(b"retx-s"); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Complete handshake normally first + let mut client_connected = false; + let mut server_connected = false; + + for _ in 0..50 { + 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 { + break; + } + + now += Duration::from_millis(10); + } + + assert!(client_connected); + assert!(server_connected); + + // Send data, but DROP the packets (simulate loss) + client + .send_application_data(b"will-retransmit") + .expect("send"); + let lost_pkts = collect_packets(&mut client); + assert!(!lost_pkts.is_empty(), "Should have packets to send"); + // Intentionally not delivering lost_pkts + + // Send again — the application layer retransmit + client.send_application_data(b"retry-data").expect("send"); + let retry_pkts = collect_packets(&mut client); + deliver_packets(&retry_pkts, &mut server); + + let server_out = drain_outputs(&mut server); + assert!( + server_out.app_data.iter().any(|d| d == b"retry-data"), + "Server should receive retried data over CID" + ); +} + +/// RFC 9146 §5.3 / RFC 6347 §4.1.2.7: a CID record whose wire CID does not match +/// the negotiated inbound CID must be silently dropped at the AEAD layer, AND the +/// replay window must not advance on the tampered sequence. This test flips a +/// byte in the CID field of a post-handshake record, verifies the peer drops it +/// silently, then delivers the untampered original at the same sequence and +/// asserts it still arrives — proving both tamper detection and replay-window +/// correctness. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_tampered_record_is_dropped() { + use dimpl::certificate::generate_self_signed_certificate; + + 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"); + + // Client's inbound CID is what the server writes into the CID field of + // records it sends. Tampering that byte on the wire should fail the client's + // on-wire CID authentication check. + let client_cid: &[u8] = b"tamper-c"; + let server_cid: &[u8] = b"tamper-s"; + + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Complete the handshake normally. + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + 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 { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + // Server sends a post-handshake application data record. + let payload = b"cid-tamper-canary"; + server.send_application_data(payload).expect("server send"); + let server_pkts = collect_packets(&mut server); + + // Find the CID-typed record the server produced. Content type 25 = tls12_cid. + let cid_pkt = server_pkts + .iter() + .find(|p| !p.is_empty() && p[0] == 25) + .expect("server should emit a tls12_cid record"); + + // The CID field sits at bytes [11..11+client_cid.len()]. Confirm layout and + // build a tampered copy that flips one bit of the CID. + assert!( + cid_pkt.len() >= 11 + client_cid.len() + 2, + "tls12_cid record too short" + ); + assert_eq!( + &cid_pkt[11..11 + client_cid.len()], + client_cid, + "wire CID should be the client's configured inbound CID" + ); + + let mut tampered = cid_pkt.clone(); + tampered[11] ^= 0x01; + + // Deliver the tampered record first. The client must drop it silently (no + // error surfaced, no app data emitted). + deliver_packets(&[tampered], &mut client); + let after_tamper = drain_outputs(&mut client); + assert!( + after_tamper.app_data.is_empty(), + "Client must not surface app data from a tampered CID record" + ); + + // Now deliver the untampered original at the same sequence number. The + // replay window must NOT have advanced on the tampered drop, so this + // retransmit still decrypts and the payload arrives. + deliver_packets(std::slice::from_ref(cid_pkt), &mut client); + let after_valid = drain_outputs(&mut client); + assert!( + after_valid.app_data.iter().any(|d| d == payload), + "Client must accept the untampered original after a tampered drop \ + (replay window must not advance on tampered sequence)" + ); +} + +/// RFC 6347 §4.1.2.6: the receive window is only updated once AEAD +/// verification succeeds. The CID tamper test above covers the CID-auth +/// failure case; this closes the symmetric gap by tampering the *ciphertext* +/// of a CID record (not the CID itself). The CID still matches the +/// negotiated value, so the on-wire CID check passes, but AEAD decryption +/// fails and the record is silently dropped. The replay window must not +/// advance on the tampered sequence, so a legitimate retransmit at the same +/// sequence still decrypts. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_tampered_ciphertext_preserves_replay_window() { + use dimpl::certificate::generate_self_signed_certificate; + + 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 client_cid: &[u8] = b"ct-tc"; + let server_cid: &[u8] = b"ct-ts"; + + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + // Server sends a post-handshake application data record. + let payload = b"ct-tamper-canary"; + server.send_application_data(payload).expect("server send"); + let server_pkts = collect_packets(&mut server); + let cid_pkt = server_pkts + .iter() + .find(|p| !p.is_empty() && p[0] == 25) + .expect("server should emit a tls12_cid record") + .clone(); + + // Confirm the CID field layout so we flip a byte strictly *after* the + // CID (i.e. inside the ciphertext), leaving the CID bytes intact. + assert!(cid_pkt.len() >= 11 + client_cid.len() + 2); + assert_eq!(&cid_pkt[11..11 + client_cid.len()], client_cid); + + // Flip one bit of the last ciphertext byte. This leaves the CID and + // record header untouched, so the on-wire CID authentication check + // passes; but the AEAD tag check must fail. + let mut tampered = cid_pkt.clone(); + let last = tampered.len() - 1; + tampered[last] ^= 0x01; + // Sanity: the CID field was not touched. + assert_eq!(&tampered[11..11 + client_cid.len()], client_cid); + + // Deliver the tampered ciphertext first. The client must drop it silently + // (AEAD authentication fails, no app data is emitted). + deliver_packets(&[tampered], &mut client); + let after_tamper = drain_outputs(&mut client); + assert!( + after_tamper.app_data.is_empty(), + "Client must not surface app data from a ciphertext-tampered CID record" + ); + + // Deliver the untampered original at the same sequence number. The replay + // window must NOT have advanced on the AEAD-auth failure, so this + // retransmit still decrypts and the payload arrives. + deliver_packets(std::slice::from_ref(&cid_pkt), &mut client); + let after_valid = drain_outputs(&mut client); + assert!( + after_valid.app_data.iter().any(|d| d == payload), + "Client must accept the untampered original after an AEAD-auth \ + failure drop (replay window must not advance on failed decrypt)" + ); +} + +/// Queue-and-defer across the CCS boundary. A CID-framed epoch-1 record +/// (Finished) can reach the client before the peer's ChangeCipherSpec if a +/// coalesced datagram is reordered or fragmented. The parser must keep the +/// CID record — `inbound_cid_active` is false at that instant — instead of +/// dropping it, and decryption must happen when `enable_peer_encryption` +/// later fires on the CCS. The GateStub unit test covers the drop branch; +/// this is the complementary integration test for the queue-and-decrypt +/// branch that `src/dtls12/incoming.rs:258-269` points at. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_finished_before_ccs_is_queued_then_decrypted() { + use dimpl::certificate::generate_self_signed_certificate; + + 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 client_cid: &[u8] = b"reorder-c"; + let server_cid: &[u8] = b"reorder-s"; + + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Drive the handshake but DO NOT deliver the server's final flight yet. + // Any time the server emits a datagram that contains both a CCS record + // (content type 20) and at least one CID record (content type 25), we + // capture it unbroken for the reordering step below. + let mut final_flight: Option> = None; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + + // Route client → server normally so the server can progress to + // sending its Finished flight. + deliver_packets(&co.packets, &mut server); + + for pkt in so.packets { + // Server's final flight is a coalesced datagram that begins with + // a 14-byte ChangeCipherSpec record (content type 20, 13-byte + // header + 1-byte payload) and is immediately followed by the + // CID-framed Finished record (content type 25 at byte 14). + let is_final_flight = pkt.len() > 14 && pkt[0] == 20 && pkt[14] == 25; + if final_flight.is_none() && is_final_flight { + final_flight = Some(pkt); + } else { + // Any other server-to-client packet (ServerHello flight, etc.) + // flows through normally. + let _ = client.handle_packet(&pkt); + } + } + + if final_flight.is_some() { + break; + } + now += Duration::from_millis(10); + } + let final_flight = + final_flight.expect("server should emit a coalesced CCS + CID Finished flight"); + + // Split the flight into its individual DTLS records. Layout: + // non-CID record: type(1) | version(2) | epoch(2) | seq(6) | length(2) | body + // CID record: type(1) | version(2) | epoch(2) | seq(6) | cid(N) | length(2) | body + // We're the client side: the peer's CID for records sent to us is our own + // configured `client_cid`. + let mut records: Vec> = Vec::new(); + let mut i = 0; + while i < final_flight.len() { + assert!(i + 13 <= final_flight.len(), "flight truncated"); + let ct = final_flight[i]; + let (hdr_len, body_len) = if ct == 25 { + let hdr = 11 + client_cid.len() + 2; + assert!(i + hdr <= final_flight.len()); + let length = + u16::from_be_bytes([final_flight[i + hdr - 2], final_flight[i + hdr - 1]]) as usize; + (hdr, length) + } else { + let hdr = 13; + let length = u16::from_be_bytes([final_flight[i + 11], final_flight[i + 12]]) as usize; + (hdr, length) + }; + let end = i + hdr_len + body_len; + records.push(final_flight[i..end].to_vec()); + i = end; + } + assert!( + records.len() >= 2, + "expected at least one CCS + one Finished record" + ); + + // Find the indices of the CCS record and the first CID-framed record. + let ccs_idx = records + .iter() + .position(|r| r.first().copied() == Some(20)) + .expect("CCS record should be present"); + let cid_idx = records + .iter() + .position(|r| r.first().copied() == Some(25)) + .expect("CID-framed Finished should be present"); + assert!( + ccs_idx < cid_idx, + "normal wire order places CCS before CID Finished" + ); + + // Phase A: deliver the CID-framed Finished *before* the CCS. At this + // point the client has `our_cid` negotiated but `inbound_cid_active` is + // still false, so the record must be parsed and queued rather than + // dropped. + let _ = client.handle_packet(&records[cid_idx]); + let after_cid_first = drain_outputs(&mut client); + assert!( + !after_cid_first.connected, + "Client must not complete handshake from a reordered Finished alone" + ); + + // Phase B: now deliver the CCS (and any other records in the flight). + // `enable_peer_encryption` drains the queue, which includes the queued + // CID-framed Finished — decryption must succeed and the client must + // connect. + for (j, rec) in records.iter().enumerate() { + if j == cid_idx { + continue; // already delivered + } + let _ = client.handle_packet(rec); + } + + // Drive any remaining handshake exchanges to completion. + let mut client_connected = false; + for _ in 0..20 { + let co = drain_outputs(&mut client); + deliver_packets(&co.packets, &mut server); + let so = drain_outputs(&mut server); + deliver_packets(&so.packets, &mut client); + + let co_again = drain_outputs(&mut client); + if co_again.connected || co.connected { + client_connected = true; + break; + } + now += Duration::from_millis(10); + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + } + assert!( + client_connected, + "Client must complete handshake once the queued CID Finished is \ + re-decrypted after CCS activates inbound CID" + ); +} + +/// RFC 9146 §4 / RFC 6347 §4.1.2.7: a `tls12_cid` record for a direction where +/// CID is not negotiated MUST be discarded, but coalesced records in the same +/// datagram MUST still be processed. Regression guard against the prior +/// implementation which `break`'d out of the parse loop, silently dropping +/// unrelated records sharing the datagram. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_unsolicited_cid_record_does_not_drop_coalesced_records() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + // Neither side configures CID — so a content-type-25 record is unsolicited. + let config = dtls12_config(); + let mut now = Instant::now(); + + let mut client = Dtls::new_12(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + 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 { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + // Client sends an ApplicationData record. + let payload = b"coalesced-after-stray"; + client.send_application_data(payload).expect("send"); + let client_pkts = collect_packets(&mut client); + let valid_pkt = client_pkts + .iter() + .find(|p| !p.is_empty() && p[0] == 23) + .expect("client should emit ApplicationData") + .clone(); + + // Craft a coalesced datagram: [stray tls12_cid record | valid ApplicationData]. + // The stray record has zero-length body with the standard 13-byte DTLS + // header — just enough for the parser to frame and discard it. + let stray_cid: [u8; 13] = [ + 25, // ContentType::Tls12Cid + 0xfe, 0xfd, // DTLS 1.2 + 0, 1, // epoch + 0, 0, 0, 0, 0, 0xab, // seqnum (arbitrary) + 0, 0, // length = 0 + ]; + let mut coalesced = Vec::with_capacity(stray_cid.len() + valid_pkt.len()); + coalesced.extend_from_slice(&stray_cid); + coalesced.extend_from_slice(&valid_pkt); + + deliver_packets(&[coalesced], &mut server); + let server_out = drain_outputs(&mut server); + assert!( + server_out.app_data.iter().any(|d| d == payload), + "Server must process coalesced ApplicationData after a stray tls12_cid" + ); +} + +/// Plan §4 recovery limits: an unsolicited `tls12_cid` record whose wire CID +/// was non-zero cannot be reframed correctly — the bytes the parser would +/// otherwise read as `length` are actually CID bytes. The parser's post-skip +/// sanity check must detect that the implied follow-on position is NOT a +/// plausible DTLS record and drop the datagram remainder (safe fail-closed) +/// rather than silently desynchronize into attacker-controlled bytes. This +/// test crafts exactly that scenario and pins the safe behavior. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_unsolicited_nonzero_cid_record_triggers_safe_resync_failure() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + // Neither side negotiates CID. + let config = dtls12_config(); + let mut now = Instant::now(); + + let mut client = Dtls::new_12(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + // A legitimate follow-on ApplicationData record the attacker is trying to + // smuggle past the parser by prepending an ambiguous stray. + let payload = b"should-not-arrive-if-resync-fails"; + client.send_application_data(payload).expect("send"); + let client_pkts = collect_packets(&mut client); + let valid_pkt = client_pkts + .iter() + .find(|p| !p.is_empty() && p[0] == 23) + .expect("client should emit ApplicationData") + .clone(); + + // Stray tls12_cid record with a NON-ZERO embedded CID. Bytes [11..13] of + // the wire are CID bytes (0xFF, 0xFF) — which the zero-CID-assumption + // skip would interpret as a claimed length of 65535. That runs past the + // datagram, tripping the early overshoot guard; failing that path, the + // post-skip ContentType validator catches any garbage-aligned case. A + // crafted CID whose bytes happen to be `[0x00, 0x04]` (length 4) would + // land mid-record, which the ContentType check rejects. + let stray_cid_bytes: [u8; 16] = [ + 25, // ContentType::Tls12Cid + 0xfe, 0xfd, // DTLS 1.2 + 0, 1, // epoch + 0, 0, 0, 0, 0, 0xbe, // seqnum + // bytes [11..13] below are the *CID field*, not a length — but the + // zero-CID-assumption path would misread them as length. + 0x00, 0x04, // first two CID bytes + 0x11, 0x22, // remaining CID bytes + 0x00, // length MSB (attacker's actual length-field — unreachable + // through the misread-skip) + ]; + let mut crafted = Vec::with_capacity(stray_cid_bytes.len() + valid_pkt.len()); + crafted.extend_from_slice(&stray_cid_bytes); + crafted.extend_from_slice(&valid_pkt); + + deliver_packets(&[crafted], &mut server); + let server_out = drain_outputs(&mut server); + // The coalesced ApplicationData must NOT arrive — the parser's post-skip + // ContentType sanity check detects that the skip landed on bytes that do + // not begin a valid DTLS record and drops the datagram remainder. This + // is the fail-closed property required by plan §4: "when framing permits + // it" — and here it doesn't, because we cannot recover frame alignment. + assert!( + server_out.app_data.iter().all(|d| d != payload), + "Server must not decrypt application data reached via desynchronized \ + skip past a non-zero-CID stray record — fail-closed recovery" + ); + + // And the session remains healthy afterwards: a legitimate follow-up + // packet (no stray prefix) still works. + let followup = b"session-healthy-after-drop"; + client.send_application_data(followup).expect("send"); + let client_pkts2 = collect_packets(&mut client); + deliver_packets(&client_pkts2, &mut server); + let server_out2 = drain_outputs(&mut server); + assert!( + server_out2.app_data.iter().any(|d| d == followup), + "Subsequent legitimate traffic must still decrypt after a stray-CID \ + datagram was dropped" + ); +} + +/// `poll_output` must not panic when the caller supplies a buffer too small +/// to hold the negotiated CID. Instead, the event is deferred (a `Timeout` is +/// returned) and the CID is delivered intact on a subsequent poll with an +/// adequately sized buffer. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_poll_output_undersized_buffer_defers() { + use dimpl::certificate::generate_self_signed_certificate; + + 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"); + + // A long CID guarantees the tiny buffer below cannot fit it. + let cid: &[u8] = b"rfc9146-long-connection-id-for-test-2026"; + let client_config = dtls12_config_with_cid(cid); + let server_config = dtls12_config_with_cid(cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut big_buf = vec![0u8; 2048]; + let mut drove_cid_path = false; + + 'outer: for _ in 0..60 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + // Drain server → client. + loop { + match server.poll_output(&mut big_buf) { + Output::Packet(p) => { + let data = p.to_vec(); + let _ = client.handle_packet(&data); + } + Output::Timeout(_) => break, + _ => {} + } + } + + // Poll client, but stop the inner loop as soon as `Connected` fires — + // the `ConnectionId` event is enqueued immediately behind `Connected` + // in `client.rs`, so this is the exact moment to exercise the + // undersized-buffer path. + loop { + match client.poll_output(&mut big_buf) { + Output::Packet(p) => { + let data = p.to_vec(); + let _ = server.handle_packet(&data); + } + Output::Connected => { + // Undersized buffer must not panic and must defer. + let mut tiny_buf = [0u8; 4]; + let tiny = client.poll_output(&mut tiny_buf); + assert!( + matches!(tiny, Output::Timeout(_)), + "undersized buffer must yield Timeout, got {}", + output_variant(&tiny) + ); + + // Next poll with a large buffer must deliver the CID intact. + match client.poll_output(&mut big_buf) { + Output::ConnectionId(delivered) => { + assert_eq!( + delivered, cid, + "CID must be delivered intact after deferral" + ); + drove_cid_path = true; + break 'outer; + } + ref other => panic!( + "expected ConnectionId after deferral, got {}", + output_variant(other) + ), + } + } + Output::Timeout(_) => break, + _ => {} + } + } + + now += Duration::from_millis(10); + } + + assert!( + drove_cid_path, + "test did not reach the Connected → undersized poll → ConnectionId path" + ); +} + +/// Companion regression for the poll-buffer safety property: when pre-connect +/// application data has been queued via `send_application_data` AND a +/// `ConnectionId` event is pending AND the caller polls with an undersized +/// buffer, nothing in the engine may panic. Prior to the `poll_app_data` / +/// `poll_packet_tx` defer paths this scenario tripped `engine.rs`'s +/// "Output buffer too small" asserts because the small-buffer fall-through +/// past `ConnectionId` landed on a queued packet that the engine then tried +/// to copy into `buf` unconditionally. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_poll_output_undersized_buffer_defers_with_queued_app_data() { + use dimpl::certificate::generate_self_signed_certificate; + + 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 cid: &[u8] = b"rfc9146-queued-data-poll-buffer-safety"; + let client_config = dtls12_config_with_cid(cid); + let server_config = dtls12_config_with_cid(cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Queue application data BEFORE the handshake completes. Client wraps it + // in `queued_data` and flushes after `Connected`, so there will be real + // outbound packets sitting behind the ConnectionId event in the engine's + // tx queue at the exact moment the undersized buffer is polled. + let queued_payload = b"pre-handshake-queued-app-data-for-panic-regression"; + client + .send_application_data(queued_payload) + .expect("queue pre-connect app data"); + + let mut big_buf = vec![0u8; 2048]; + let mut drove_both_defers = false; + + 'outer: for _ in 0..60 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + loop { + match server.poll_output(&mut big_buf) { + Output::Packet(p) => { + let data = p.to_vec(); + let _ = client.handle_packet(&data); + } + Output::Timeout(_) => break, + _ => {} + } + } + + loop { + match client.poll_output(&mut big_buf) { + Output::Packet(p) => { + let data = p.to_vec(); + let _ = server.handle_packet(&data); + } + Output::Connected => { + // Right at Connected: ConnectionId event is enqueued AND + // the queued_data flush has enqueued at least one tx + // packet. Poll with a buffer too small for either — + // both the client-side CID deferral and the engine-side + // packet deferral must kick in without a panic. + let mut tiny_buf = [0u8; 2]; + let tiny = client.poll_output(&mut tiny_buf); + assert!( + matches!(tiny, Output::Timeout(_)), + "undersized buffer must yield Timeout even with \ + queued packets behind the CID event, got {}", + output_variant(&tiny) + ); + + // Next polls with a big buffer must deliver BOTH the CID + // event AND the queued application data packet intact. + let first = client.poll_output(&mut big_buf); + assert!( + matches!(first, Output::ConnectionId(c) if c == cid), + "expected ConnectionId first, got {}", + output_variant(&first) + ); + // Flush remaining outputs and route any packets so the + // server receives the queued app data — proving the + // deferral didn't corrupt the tx queue. + loop { + match client.poll_output(&mut big_buf) { + Output::Packet(p) => { + let data = p.to_vec(); + let _ = server.handle_packet(&data); + } + Output::Timeout(_) => break, + _ => {} + } + } + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == queued_payload), + "Server must receive the queued app data intact after \ + the undersized-buffer deferral path" + ); + drove_both_defers = true; + break 'outer; + } + Output::Timeout(_) => break, + _ => {} + } + } + + now += Duration::from_millis(10); + } + + assert!( + drove_both_defers, + "test did not reach the queued-data + undersized poll regression path" + ); +} + +/// A CID record whose DTLSCiphertext.length is below the AEAD overhead +/// (explicit_nonce + tag) cannot contain a valid ciphertext. The decrypt path +/// must silently discard it (RFC 6347 §4.1.2.7) without panicking on the +/// slice bounds and without advancing the replay window, so a legitimate +/// record at the same sequence still decrypts. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_short_length_below_aead_overhead_is_dropped() { + use dimpl::certificate::generate_self_signed_certificate; + + 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"); + + // AES-GCM default suite → overhead = 8 (explicit nonce) + 16 (tag) = 24. + let client_cid: &[u8] = b"short-c"; + let server_cid: &[u8] = b"short-s"; + + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + let payload = b"short-length-canary"; + server.send_application_data(payload).expect("server send"); + let server_pkts = collect_packets(&mut server); + let cid_pkt = server_pkts + .iter() + .find(|p| !p.is_empty() && p[0] == 25) + .expect("server should emit a tls12_cid record") + .clone(); + + // Layout: type(1) | version(2) | epoch(2) | seq(6) | cid(N) | length(2) | body + let length_field_offset = 11 + client_cid.len(); + assert!(cid_pkt.len() >= length_field_offset + 2); + + // Craft a record that claims length=10 (below AEAD overhead=24) and + // truncate the body to match. Without the §2 guard this underflows the + // ciphertext slice below the explicit-nonce offset and panics. + let mut short = cid_pkt.clone(); + short[length_field_offset] = 0; + short[length_field_offset + 1] = 10; + short.truncate(length_field_offset + 2 + 10); + + deliver_packets(&[short], &mut client); + let after_short = drain_outputs(&mut client); + assert!( + after_short.app_data.is_empty(), + "Client must silently drop a CID record whose length is below AEAD overhead" + ); + + // The replay window must NOT have advanced on the silent drop, so the + // untampered original at the same sequence still decrypts. + deliver_packets(std::slice::from_ref(&cid_pkt), &mut client); + let after_valid = drain_outputs(&mut client); + assert!( + after_valid.app_data.iter().any(|d| d == payload), + "Client must accept the original record after a short-length silent drop" + ); +} + +/// RFC 9146 §3 — a zero-length CID advertised for a direction means +/// *use legacy RFC 6347 framing* in that direction, not "tls12_cid with +/// zero bytes." The extension is still negotiated; only framing stays +/// legacy. With client_cid non-empty and server_cid empty: +/// +/// - client → server: legacy framing (server advertised zero-length +/// inbound, so the outbound CID from client's POV is empty) +/// - server → client: `tls12_cid` framing (client advertised non-empty +/// inbound) +/// +/// Both directions must decrypt; the zero-length direction must not emit +/// content type 25. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_asymmetric_zero_length_uses_legacy_framing() { + use dimpl::certificate::generate_self_signed_certificate; + + 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 client_cid: &[u8] = b"asym-c-nonzero"; + let server_cid: &[u8] = &[]; + + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!( + client_connected && server_connected, + "asymmetric zero-length CID must still complete the handshake" + ); + + // Client → server: legacy framing (no content type 25 on the wire) — + // because the server advertised zero-length inbound. + let c_payload = b"c2s-asym-zero"; + client + .send_application_data(c_payload) + .expect("client send"); + let co = drain_outputs(&mut client); + assert!( + co.packets + .iter() + .filter(|p| !p.is_empty()) + .all(|p| p[0] != 25), + "client→server records must NOT use tls12_cid framing when server advertised zero-length inbound" + ); + deliver_packets(&co.packets, &mut server); + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == c_payload), + "server must receive client app data over legacy framing" + ); + + // Server → client: tls12_cid framing (client advertised non-empty inbound). + let s_payload = b"s2c-asym-zero"; + server + .send_application_data(s_payload) + .expect("server send"); + let so2 = drain_outputs(&mut server); + let cid_pkt = so2 + .packets + .iter() + .find(|p| !p.is_empty() && p[0] == 25) + .expect( + "server→client records must use tls12_cid framing (client has non-empty inbound CID)", + ); + // Verify CID bytes are present on the wire. + assert!(cid_pkt.len() >= 11 + client_cid.len() + 2); + assert_eq!(&cid_pkt[11..11 + client_cid.len()], client_cid); + deliver_packets(&so2.packets, &mut client); + let co2 = drain_outputs(&mut client); + assert!( + co2.app_data.iter().any(|d| d == s_payload), + "client must receive server app data over tls12_cid framing" + ); +} + +/// RFC 9146 §3 + RFC 5246 §7.4.1.4 — a server MUST NOT send a +/// `connection_id` extension the client did not offer, and the client MUST +/// treat such a ServerHello as `unsupported_extension`. Exercises the +/// rejection gate in `client.rs:await_server_hello`. +/// +/// Construction: intercept the real ServerHello from a non-CID handshake, +/// splice a CID extension into its extension list (updating enclosing +/// length fields: extensions_length, handshake fragment length, handshake +/// total length, record length). Deliver the tampered ServerHello to the +/// client. The hardened parser must surface SecurityError because the +/// client's config has `connection_id = None`. Extension iteration happens +/// before EMS / cert / crypto binding so this path fires immediately. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_client_rejects_unsolicited_server_hello_extension() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + let config = dtls12_config(); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(config, server_cert, now); + server.set_active(false); + + // Drive the handshake until the server emits a ServerHello Handshake + // record (content type 22, first handshake msg_type=2). Withhold the + // ServerHello from the client so the client stays in `AwaitServerHello` + // for our tampered delivery below. + let mut sh_pkt: Option> = None; + for _ in 0..6 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + deliver_packets(&co.packets, &mut server); + for p in &so.packets { + if !p.is_empty() && p[0] == 22 && p.get(13) == Some(&2) { + if sh_pkt.is_none() { + sh_pkt = Some(p.clone()); + } + // withhold — do not deliver to the client + continue; + } + let _ = client.handle_packet(p); + } + if sh_pkt.is_some() { + break; + } + now += Duration::from_millis(10); + } + let mut sh_pkt = sh_pkt.expect("server should emit ServerHello within 6 iterations"); + + // ServerHello is the first handshake message in the flight (at byte 13). + // Layout: record(13) | handshake_msg_type(1) | length(3) | + // message_seq(2) | fragment_offset(3) | fragment_length(3) | + // body + // ServerHello body: server_version(2) | random(32) | session_id_len(1) | + // session_id | cipher_suite(2) | compression_method(1) | + // extensions_length(2) | extensions + assert_eq!( + sh_pkt[13], 2, + "first handshake message should be ServerHello" + ); + + let hs_total_len_off = 14; // 3 bytes + let hs_frag_len_off = 22; // 3 bytes + let body_off = 25; + + let mut p = body_off + 2 + 32; // skip version + random + let sid_len = sh_pkt[p] as usize; + p += 1 + sid_len; + p += 2 + 1; // cipher_suite + compression_method + let ext_len_off = p; + let old_ext_len = u16::from_be_bytes([sh_pkt[ext_len_off], sh_pkt[ext_len_off + 1]]) as usize; + let extensions_start = ext_len_off + 2; + let extensions_end = extensions_start + old_ext_len; + + // Build a CID extension: type(2) | len(2) | cid_len(1) | cid(n) + let injected_cid: &[u8] = b"unsolicited"; + let ext_len: u16 = 1 + injected_cid.len() as u16; + let mut cid_ext = Vec::new(); + cid_ext.extend_from_slice(&[0x00, 0x36]); + cid_ext.extend_from_slice(&ext_len.to_be_bytes()); + cid_ext.push(injected_cid.len() as u8); + cid_ext.extend_from_slice(injected_cid); + let added = cid_ext.len(); + + // Splice at extensions_end. + sh_pkt.splice(extensions_end..extensions_end, cid_ext.iter().copied()); + + // Update extensions_length. + let new_ext_len = (old_ext_len + added) as u16; + sh_pkt[ext_len_off] = (new_ext_len >> 8) as u8; + sh_pkt[ext_len_off + 1] = new_ext_len as u8; + + // Update handshake fragment length. + let mut hs_frag_len = ((sh_pkt[hs_frag_len_off] as usize) << 16) + | ((sh_pkt[hs_frag_len_off + 1] as usize) << 8) + | (sh_pkt[hs_frag_len_off + 2] as usize); + hs_frag_len += added; + sh_pkt[hs_frag_len_off] = (hs_frag_len >> 16) as u8; + sh_pkt[hs_frag_len_off + 1] = (hs_frag_len >> 8) as u8; + sh_pkt[hs_frag_len_off + 2] = hs_frag_len as u8; + + // Update handshake total length. + let mut hs_total_len = ((sh_pkt[hs_total_len_off] as usize) << 16) + | ((sh_pkt[hs_total_len_off + 1] as usize) << 8) + | (sh_pkt[hs_total_len_off + 2] as usize); + hs_total_len += added; + sh_pkt[hs_total_len_off] = (hs_total_len >> 16) as u8; + sh_pkt[hs_total_len_off + 1] = (hs_total_len >> 8) as u8; + sh_pkt[hs_total_len_off + 2] = hs_total_len as u8; + + // Update record length. Following records in the flight still have + // correct self-contained headers, so only the first record's length + // needs to grow. + let first_rec_len_off = 11; + let old_rec_len = + u16::from_be_bytes([sh_pkt[first_rec_len_off], sh_pkt[first_rec_len_off + 1]]) as usize; + let new_rec_len = (old_rec_len + added) as u16; + sh_pkt[first_rec_len_off] = (new_rec_len >> 8) as u8; + sh_pkt[first_rec_len_off + 1] = new_rec_len as u8; + + // Deliver the tampered ServerHello. Client's extension scan hits the + // unsolicited CID extension before any EMS/crypto checks. + let result = client.handle_packet(&sh_pkt); + match result { + Err(dimpl::Error::SecurityError(msg)) => { + assert!( + msg.contains("unsolicited") || msg.contains("connection_id"), + "expected SecurityError about unsolicited CID; got: {}", + msg + ); + } + other => panic!( + "expected SecurityError from unsolicited CID in ServerHello, got: {:?}", + other + ), + } +} + +/// RFC 5246 §7.2.2 — a malformed `connection_id` extension on the server +/// side must surface as `SecurityError` (decode_error). Previously the server +/// `warn!`'d and silently continued. Tampers the cid_len byte inside the +/// extension body to claim fewer bytes than present, which exercises the +/// trailing-bytes rejection added to `ConnectionIdExtension::parse`. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_server_rejects_malformed_extension() { + use dimpl::certificate::generate_self_signed_certificate; + + 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"); + + // Client offers a CID long enough that shortening cid_len leaves a + // trailing byte. Server has its own CID so it would normally process the + // extension. + let client_cid: &[u8] = b"t5-client-cid"; + let server_cid: &[u8] = b"t5-server-cid"; + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Helper: locate the CID extension (type = 0x0036) inside a ClientHello + // packet and shrink its cid_len byte by 1 so the body carries a trailing + // byte — exercising the hardened `ConnectionIdExtension::parse`. + fn tamper_cid_ext_in_ch(pkt: &mut [u8], client_cid_len: usize) { + for i in 0..pkt.len().saturating_sub(5) { + if pkt[i] == 0x00 && pkt[i + 1] == 0x36 { + let ext_len = u16::from_be_bytes([pkt[i + 2], pkt[i + 3]]) as usize; + if ext_len == 1 + client_cid_len && pkt[i + 4] as usize == client_cid_len { + pkt[i + 4] = (client_cid_len - 1) as u8; + return; + } + } + } + panic!("CID extension not found in ClientHello"); + } + + // Tamper CH1 (carries the CID extension) so the server's cookie is + // computed against the same tampered bytes that CH2 will also carry + // (we tamper CH2 identically below). RFC 9146 §3 / §7 cookie binding over + // the offered CID means identical tampering on both CHs lets the + // cookie verify — the strict extension parser then rejects. + client.handle_timeout(now).expect("client timeout"); + let co = drain_outputs(&mut client); + let mut ch1 = co + .packets + .into_iter() + .find(|p| !p.is_empty() && p[0] == 22) + .expect("client should emit CH1"); + tamper_cid_ext_in_ch(&mut ch1, client_cid.len()); + // Deliver tampered CH1 → server issues HVR (cookie bound over tampered + // ext bytes). We must not surface the tamper error yet because the + // strict parse runs only on the cookie-bearing CH. + let _ = server.handle_packet(&ch1); + let so = drain_outputs(&mut server); + deliver_packets(&so.packets, &mut client); + + // CH2 with cookie — re-capture and apply the identical tamper so the + // cookie binding still holds. + client.handle_timeout(now).expect("client timeout"); + let co2 = drain_outputs(&mut client); + let mut ch2 = co2 + .packets + .into_iter() + .find(|p| !p.is_empty() && p[0] == 22) + .expect("client should emit CH2"); + tamper_cid_ext_in_ch(&mut ch2, client_cid.len()); + + // Deliver the tampered ClientHello — server must fail with SecurityError. + let result = server.handle_packet(&ch2); + match result { + Err(dimpl::Error::SecurityError(msg)) => { + assert!( + msg.contains("connection_id") || msg.contains("Malformed"), + "expected SecurityError to mention CID; got: {}", + msg + ); + } + other => panic!( + "expected SecurityError on malformed CID extension, got: {:?}", + other + ), + } +} + +/// RFC 9146 §3 / §7 cookie binding: if the second ClientHello offers a +/// different `connection_id` extension than the first, the stateless +/// cookie must fail verification — because dimpl binds the offered CID +/// into the cookie HMAC. An attacker cannot forge a cookie for a swapped +/// CID (no server secret), so the server re-issues HVR and the handshake +/// cannot progress. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_server_rejects_swap_across_hello_verify_pair() { + use dimpl::certificate::generate_self_signed_certificate; + + 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 client_cid: &[u8] = b"hvr-swap-cli"; + let server_cid: &[u8] = b"hvr-swap-srv"; + + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Cookie exchange: CH1 (with CID A) → HVR → CH2 (with CID A). + client.handle_timeout(now).expect("client timeout"); + let co = drain_outputs(&mut client); + deliver_packets(&co.packets, &mut server); + let so = drain_outputs(&mut server); + deliver_packets(&so.packets, &mut client); + + // Capture CH2 and rewrite the first byte of the CID field (a swap that + // preserves length, so extension framing stays valid) — this is the + // RFC 9146 §3 / §7 "second CH with a different CID" scenario. + client.handle_timeout(now).expect("client timeout"); + let co2 = drain_outputs(&mut client); + let mut ch2 = co2 + .packets + .into_iter() + .find(|p| !p.is_empty() && p[0] == 22) + .expect("client should emit CH2"); + + let mut swap_off = None; + for i in 0..ch2.len().saturating_sub(5) { + if ch2[i] == 0x00 && ch2[i + 1] == 0x36 { + let ext_len = u16::from_be_bytes([ch2[i + 2], ch2[i + 3]]) as usize; + if ext_len == 1 + client_cid.len() + && ch2[i + 4] as usize == client_cid.len() + && i + 5 + client_cid.len() <= ch2.len() + { + swap_off = Some(i + 5); + break; + } + } + } + let off = swap_off.expect("CID extension not found in CH2"); + // Flip the first CID byte — framing stays valid, extension parses + // cleanly; the swapped CID value is what the cookie binding catches. + ch2[off] ^= 0xFF; + + // Deliver the swapped CH2. The cookie was HMAC'd over CH1's CID bytes; + // the swapped CID bytes differ, so `verify_cookie` fails. The server + // treats CH2 as cookie-invalid and re-issues HVR instead of progressing + // to ServerHello. No ServerHello emerges within one handshake step. + let _ = server.handle_packet(&ch2); + let so2 = drain_outputs(&mut server); + let has_server_hello = so2 + .packets + .iter() + .any(|p| !p.is_empty() && p[0] == 22 && p.get(13) == Some(&2)); + assert!( + !has_server_hello, + "server must NOT emit ServerHello for a CH2 with swapped CID" + ); + // Should have emitted an HVR (content type 22, handshake msg_type=3). + let has_hvr = so2 + .packets + .iter() + .any(|p| !p.is_empty() && p[0] == 22 && p.get(13) == Some(&3)); + assert!( + has_hvr, + "server must re-issue HelloVerifyRequest on cookie mismatch from CID swap" + ); +} + +/// Regression for review item #1 (2026-04-23 third pass): when the configured +/// MTU is too small to hold record overhead + a handshake header + at least +/// one body byte, `create_handshake` must fail deterministically with +/// `Error::MtuTooSmall` rather than loop forever with `chunk_len == 0`. +/// Exercises the smallest allowed MTU (64) against a negotiated CID that +/// pushes the overhead past it. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_handshake_mtu_too_small_fails_closed() { + use dimpl::Config; + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + // Pick MTU/CID so epoch-0 handshake fits (record 13 + hs header 12 = + // 25 bytes overhead) but epoch-1 + long CID + AEAD does not. At + // MTU=300 with a 250-byte CID: epoch-1 overhead = 13 + 250 + 1 + 24 + + // 12 = 300 ≥ 300 → MtuTooSmall must fire on the first encrypted + // handshake fragment attempt (the Finished emission). + let cid: Vec = vec![0xAB; 250]; + let server_config = Arc::new( + Config::builder() + .mtu(300) + .with_connection_id(cid.clone()) + .build() + .expect("build server config"), + ); + let client_config = Arc::new( + Config::builder() + .mtu(300) + .with_connection_id(cid) + .build() + .expect("build client config"), + ); + + let now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Drive a handful of handshake steps. At some point an encrypted + // handshake flight must attempt to fragment post-negotiation and + // surface MtuTooSmall. Before the fix, the fragmentation loop would + // spin forever. + let mut saw_mtu_error = false; + for _ in 0..30 { + if let Err(e) = client.handle_timeout(now) { + if matches!(e, dimpl::Error::MtuTooSmall { .. }) { + saw_mtu_error = true; + break; + } + } + let co = drain_outputs(&mut client); + for p in &co.packets { + let _ = server.handle_packet(p); + } + if let Err(e) = server.handle_timeout(now) { + if matches!(e, dimpl::Error::MtuTooSmall { .. }) { + saw_mtu_error = true; + break; + } + } + let so = drain_outputs(&mut server); + for p in &so.packets { + let _ = client.handle_packet(p); + } + } + assert!( + saw_mtu_error, + "either side must surface MtuTooSmall instead of hanging" + ); +} + +/// Contract test for `Config::mtu()`: MTU is a coalescing target, not a +/// hard per-record ceiling. An application-data record whose plaintext +/// exceeds MTU is still emitted as a single datagram (larger than MTU) — +/// the only hard cap on application data is `DTLS12_MAX_PLAINTEXT_LEN = +/// 2^14`, enforced by `Error::Oversized`. Pins the contract so a future +/// change to enforce MTU as a hard ceiling has a failing test to update. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_mtu_is_coalescing_target_not_hard_ceiling() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + + let config = dtls12_config(); + let mut now = Instant::now(); + let mut client = Dtls::new_12(Arc::clone(&config), client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(config, server_cert, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + // Write larger than default MTU (1150). Contract: succeeds, and the + // resulting datagram exceeds MTU. + let payload = vec![0xAAu8; 2000]; + client + .send_application_data(&payload) + .expect("2KB write must succeed — MTU is coalescing target, not hard ceiling"); + let co = drain_outputs(&mut client); + let produced_oversize = co.packets.iter().any(|p| p.len() > 1150); + assert!( + produced_oversize, + "2KB app-data must be emitted as a single >MTU datagram; got sizes: {:?}", + co.packets.iter().map(|p| p.len()).collect::>() + ); + deliver_packets(&co.packets, &mut server); + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == &payload), + "server must still receive and decrypt the oversize record" + ); + + // Write at exactly `DTLS12_MAX_PLAINTEXT_LEN` — the hard cap. + let at_cap = vec![0x55u8; 1 << 14]; + client + .send_application_data(&at_cap) + .expect("write at DTLS12_MAX_PLAINTEXT_LEN must succeed"); + + // One byte over → `Error::Oversized`. + let over = vec![0x66u8; (1 << 14) + 1]; + match client.send_application_data(&over) { + Err(dimpl::Error::Oversized(n)) => { + assert_eq!(n, (1 << 14) + 1, "Oversized should report the offered len"); + } + other => panic!("expected Error::Oversized, got: {:?}", other), + } +} + +/// Regression for review item #3: 48-bit sequence-number exhaustion +/// surfaces as a specific `SequenceNumberExhausted` error, not as +/// `Oversized` which is reserved for payload-size failures. +#[test] +fn dtls12_error_variants_are_distinct() { + use dimpl::Error; + // The review's primary ask is that the two error conditions be + // distinguishable at match time. Pattern-match to prove the contract. + let seq = Error::SequenceNumberExhausted { + epoch: 1, + sequence: (1u64 << 48) - 1, + }; + let oversized = Error::Oversized(1 << 14); + assert!(matches!(seq, Error::SequenceNumberExhausted { .. })); + assert!(matches!(oversized, Error::Oversized(_))); + // They must not collide. + assert!(!matches!(seq, Error::Oversized(_))); + assert!(!matches!(oversized, Error::SequenceNumberExhausted { .. })); +} + +/// RFC 6347 §4.1.2.7 / RFC 9146 §6: invalid records are silently +/// discarded. Three CID-adjacent record-boundary failures must surface +/// as `Ok(())` from `handle_packet` — not a propagated parse error that +/// would tear down the association at a Sans-IO caller treating +/// `handle_packet` errors as fatal. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_malformed_record_boundaries_are_silent_drops() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + let cid: &[u8] = b"silent-drop"; + let client_config = dtls12_config_with_cid(cid); + let server_config = dtls12_config_with_cid(cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + // Complete the handshake so the client expects CID-framed inbound. + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + // Case 1: a tls12_cid datagram shorter than 13 + cid_len bytes. + let mut short_cid_pkt = vec![25u8, 0xFE, 0xFD]; // type + version, then truncated + // Leave enough for the 11-byte sequence-prefixed header start, but cut + // the rest off — header_len for our CID is 13 + 11 = 24, this is far + // short. + while short_cid_pkt.len() < 11 { + short_cid_pkt.push(0); + } + client + .handle_packet(&short_cid_pkt) + .expect("short CID datagram must silent-drop, not surface error"); + + // Case 2: a tls12_cid datagram whose claimed length runs past datagram + // end. Header(13) + CID(11) + length=0xFFFF; actual datagram is far + // shorter. + let mut over_pkt = vec![25u8, 0xFE, 0xFD]; // content type + version + over_pkt.extend_from_slice(&[0, 1, 0, 0, 0, 0, 0, 1]); // epoch + seq + over_pkt.extend_from_slice(cid); // CID bytes + over_pkt.extend_from_slice(&[0xFF, 0xFF]); // claimed length 65535 + over_pkt.extend_from_slice(&[0u8; 4]); // tiny body + client + .handle_packet(&over_pkt) + .expect("over-length CID record must silent-drop, not surface error"); + + // Case 3: a valid coalesced encrypted record before a malformed tail. + // We can't easily forge a valid encrypted record, but post-handshake + // app-data exchange after the malformed deliveries above proves the + // session survived the silent drops. + let payload = b"alive-after-malformed"; + server.send_application_data(payload).expect("server send"); + let so = drain_outputs(&mut server); + deliver_packets(&so.packets, &mut client); + let co = drain_outputs(&mut client); + assert!( + co.app_data.iter().any(|d| d == payload), + "client must still receive valid app data after silent-drop malformed records" + ); +} + +/// Regression for peer-address-update contract (2026-04-23 review #1): +/// `handle_packet` returning `Ok(())` is NOT an authentication signal — +/// tampered records silent-drop per RFC 6347 §4.1.2.7 and still surface +/// `Ok(())`. Callers using CID routing must gate address updates on a +/// positive authentication signal like `ApplicationData` from +/// `poll_output`, not on `handle_packet`'s return value. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_cid_tampered_record_returns_ok_but_emits_no_app_data() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + let client_cid: &[u8] = b"addr-c"; + let server_cid: &[u8] = b"addr-s"; + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut c_ok = false; + let mut s_ok = false; + for _ in 0..50 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + c_ok |= co.connected; + s_ok |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if c_ok && s_ok { + break; + } + now += Duration::from_millis(10); + } + assert!(c_ok && s_ok); + + // Server emits a CID-framed app-data record; tamper the wire CID so + // AEAD binding fails at the client. + let payload = b"addr-update-bait"; + server.send_application_data(payload).expect("server send"); + let so = drain_outputs(&mut server); + let cid_pkt = so + .packets + .iter() + .find(|p| !p.is_empty() && p[0] == 25) + .expect("server should emit a tls12_cid record") + .clone(); + let mut tampered = cid_pkt.clone(); + tampered[11] ^= 0x01; + + // handle_packet must return Ok(()) — a CID-routing caller observing + // only this return would (incorrectly) conclude the record was + // authenticated. + let result = client.handle_packet(&tampered); + assert!( + matches!(result, Ok(())), + "handle_packet must return Ok(()) on silently-dropped tampered record, got {:?}", + result + ); + + // But the authentication-positive signal must not appear: no + // ApplicationData is emitted because AEAD authentication failed. + let co = drain_outputs(&mut client); + assert!( + co.app_data.is_empty(), + "tampered CID record must not surface ApplicationData — that's the real auth signal" + ); +} + +/// Round-5 review #3: `connection_id` (RFC 9146 DTLS 1.2) must be rejected +/// at Config::build time when the caller filters out every DTLS 1.2 suite. +/// Without the gate the ClientHello would advertise CID on a handshake +/// that can only succeed as DTLS 1.3 (where dimpl does not implement +/// RFC 9147 CID) — a cross-setting mismatch users can hit. +#[test] +fn dtls12_cid_with_no_dtls12_suites_fails_config_build() { + use dimpl::{Config, Error}; + + let result = Config::builder() + .with_connection_id(b"cid".to_vec()) + .dtls12_cipher_suites(&[]) // filter out all DTLS 1.2 suites + .build(); + + match result { + Err(Error::ConfigError(msg)) => { + assert!( + msg.contains("Connection ID") || msg.contains("DTLS 1.2"), + "ConfigError message should explain the CID/DTLS 1.2 mismatch: {}", + msg + ); + } + other => panic!( + "expected ConfigError rejecting CID-without-DTLS-1.2-suite, got: {:?}", + other + ), + } +} + +/// RFC 9146 §6 freshness API: `Dtls::newest_authenticated_record` +/// advances strictly on each authenticated record and does NOT +/// advance on silent-dropped (tampered CID) records. Callers use this +/// delta to gate peer-address updates. +#[test] +#[cfg(feature = "rcgen")] +fn dtls12_newest_authenticated_record_advances_only_on_valid_records() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + let client_cid: &[u8] = b"auth-c"; + let server_cid: &[u8] = b"auth-s"; + let client_config = dtls12_config_with_cid(client_cid); + let server_config = dtls12_config_with_cid(server_cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12(client_config, client_cert, now); + client.set_active(true); + let mut server = Dtls::new_12(server_config, server_cert, now); + server.set_active(false); + + let mut c_ok = false; + let mut s_ok = false; + for _ in 0..50 { + client.handle_timeout(now).expect("c timeout"); + server.handle_timeout(now).expect("s timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + c_ok |= co.connected; + s_ok |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if c_ok && s_ok { + break; + } + now += Duration::from_millis(10); + } + assert!(c_ok && s_ok); + + let baseline = client.newest_authenticated_record(); + + // Tampered CID record: handle_packet returns Ok(()) per silent-drop + // contract, but `newest_authenticated_record` must NOT advance — this + // is the property a CID-routing caller needs to distinguish + // "authenticated fresh record arrived" from "we received something + // that parsed". + let payload = b"freshness-probe"; + server.send_application_data(payload).expect("send"); + let so = drain_outputs(&mut server); + let cid_pkt = so + .packets + .iter() + .find(|p| !p.is_empty() && p[0] == 25) + .expect("server emits tls12_cid record") + .clone(); + let mut tampered = cid_pkt.clone(); + tampered[11] ^= 0x01; + client.handle_packet(&tampered).expect("ok"); + let _ = drain_outputs(&mut client); + assert_eq!( + client.newest_authenticated_record(), + baseline, + "tampered record must not advance newest_authenticated_record" + ); + + // Valid CID record: MUST advance (or set from None) the freshness + // counter. Note: `baseline` can already be Some if the handshake's + // last received flight was epoch 1 (Finished), so we only require + // "strictly greater than baseline" or "Some when baseline was None". + client.handle_packet(&cid_pkt).expect("ok"); + let co = drain_outputs(&mut client); + assert!( + co.app_data.iter().any(|d| d == payload), + "valid record must deliver app data" + ); + let after = client + .newest_authenticated_record() + .expect("after a valid record, freshness must be Some"); + match baseline { + None => {} // any Some(..) is an advance from None + Some(prev) => assert!( + after > prev, + "newest_authenticated_record must strictly advance on valid record: {:?} → {:?}", + prev, + after + ), + } +} diff --git a/tests/dtls12/common.rs b/tests/dtls12/common.rs index 7fc87104..331a2856 100644 --- a/tests/dtls12/common.rs +++ b/tests/dtls12/common.rs @@ -112,6 +112,7 @@ pub struct DrainedOutputs { pub packets: Vec>, pub connected: bool, pub peer_cert: Option>, + pub connection_id: Option>, pub keying_material: Option<(Vec, SrtpProfile)>, pub app_data: Vec>, pub timeout: Option, @@ -127,6 +128,7 @@ pub fn drain_outputs(endpoint: &mut Dtls) -> DrainedOutputs { Output::Packet(p) => result.packets.push(p.to_vec()), Output::Connected => result.connected = true, Output::PeerCert(cert) => result.peer_cert = Some(cert.to_vec()), + Output::ConnectionId(cid) => result.connection_id = Some(cid.to_vec()), Output::KeyingMaterial(km, profile) => { result.keying_material = Some((km.to_vec(), profile)); } diff --git a/tests/dtls12/main.rs b/tests/dtls12/main.rs index c77bc49d..dfd3c926 100644 --- a/tests/dtls12/main.rs +++ b/tests/dtls12/main.rs @@ -1,6 +1,7 @@ #[path = "../ossl/mod.rs"] mod ossl_helper; +mod cid; mod common; mod crypto; mod data; diff --git a/tests/dtls12/psk.rs b/tests/dtls12/psk.rs index ad9c11a4..a7d6cfae 100644 --- a/tests/dtls12/psk.rs +++ b/tests/dtls12/psk.rs @@ -1,12 +1,12 @@ //! DTLS 1.2 PSK handshake tests. use std::sync::Arc; -use std::time::Instant; +use std::time::{Duration, Instant}; use dimpl::crypto::Dtls12CipherSuite; -use dimpl::{Config, Dtls, Error, PskResolver}; +use dimpl::{Config, Dtls, PskResolver}; -use crate::common::{deliver_packets, drain_outputs}; +use crate::common::{collect_packets, deliver_packets, drain_outputs}; /// Simple PSK resolver that returns a fixed key for a known identity. struct FixedPsk { @@ -214,68 +214,32 @@ fn psk_invalid_identity_fails_at_finished() { let mut client = Dtls::new_12_psk(Arc::new(client_config), Instant::now()); client.set_active(true); - // Drive the handshake; expect a SecurityError from mismatched PSK keys. - let mut error_found = false; + // Drive the handshake; the security-relevant property is that neither + // side ever signals Connected. Per RFC 6347 §4.1.2.7, AEAD failures on + // the encrypted Finished are silently discarded, so a mismatched PSK + // cannot surface as a propagated error — it manifests as handshake + // stall, which is what we assert here. for _ in 0..60 { - if let Err(e) = client.handle_timeout(Instant::now()) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; - } + let _ = client.handle_timeout(Instant::now()); let co = drain_outputs(&mut client); - for p in &co.packets { - if let Err(e) = server.handle_packet(p) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; - } - } - if error_found { - break; - } assert!( !co.connected, "client should not connect with mismatched PSK" ); - - if let Err(e) = server.handle_timeout(Instant::now()) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; + for p in &co.packets { + let _ = server.handle_packet(p); } + + let _ = server.handle_timeout(Instant::now()); let so = drain_outputs(&mut server); - for p in &so.packets { - if let Err(e) = client.handle_packet(p) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; - } - } - if error_found { - break; - } assert!( !so.connected, "server should not connect with mismatched PSK" ); + for p in &so.packets { + let _ = client.handle_packet(p); + } } - - assert!( - error_found, - "Expected SecurityError from PSK verification failure" - ); } #[test] @@ -312,67 +276,31 @@ fn psk_mismatched_keys_fail_at_finished_via_mac() { let mut client = Dtls::new_12_psk(Arc::new(client_config), Instant::now()); client.set_active(true); - let mut error_found = false; + // Per RFC 6347 §4.1.2.7 the Finished MAC mismatch (an AEAD tag + // failure) must be silently discarded — the security property is that + // no connection is established. See `psk_invalid_identity_fails_at_finished` + // for the same reasoning. for _ in 0..60 { - if let Err(e) = client.handle_timeout(Instant::now()) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; - } + let _ = client.handle_timeout(Instant::now()); let co = drain_outputs(&mut client); - for p in &co.packets { - if let Err(e) = server.handle_packet(p) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; - } - } - if error_found { - break; - } assert!( !co.connected, "client should not connect with mismatched PSK keys" ); - - if let Err(e) = server.handle_timeout(Instant::now()) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; + for p in &co.packets { + let _ = server.handle_packet(p); } + + let _ = server.handle_timeout(Instant::now()); let so = drain_outputs(&mut server); - for p in &so.packets { - if let Err(e) = client.handle_packet(p) { - assert!( - matches!(e, Error::SecurityError(_) | Error::CryptoError(_)), - "unexpected error: {e:?}" - ); - error_found = true; - break; - } - } - if error_found { - break; - } assert!( !so.connected, "server should not connect with mismatched PSK keys" ); + for p in &so.packets { + let _ = client.handle_packet(p); + } } - - assert!( - error_found, - "Expected SecurityError from Finished MAC mismatch" - ); } #[test] @@ -426,3 +354,406 @@ fn psk_valid_identity_succeeds() { assert!(client_connected, "PSK client should connect"); assert!(server_connected, "PSK server should connect"); } + +/// Build PSK client + server configs that additionally negotiate CID. +fn psk_configs_with_cid(client_cid: &[u8], server_cid: &[u8]) -> (Arc, Arc) { + let identity = b"test-device".to_vec(); + let key = b"0123456789abcdef".to_vec(); + + let resolver = Arc::new(FixedPsk { + identity: identity.clone(), + key, + }); + + let provider = psk_provider(Dtls12CipherSuite::PSK_AES128_CCM_8); + + let client = Arc::new( + Config::builder() + .with_crypto_provider(provider.clone()) + .with_psk_client(identity, resolver.clone()) + .with_connection_id(client_cid.to_vec()) + .build() + .expect("build PSK+CID client config"), + ); + + let server = Arc::new( + Config::builder() + .with_crypto_provider(provider) + .with_psk_server(Some(b"hint".to_vec()), resolver) + .with_connection_id(server_cid.to_vec()) + .build() + .expect("build PSK+CID server config"), + ); + + (client, server) +} + +/// PSK handshake with Connection ID negotiation for the primary IoT-roaming +/// use case. Verifies: handshake completes, both sides emit +/// `Output::ConnectionId`, and post-handshake application data flows in both +/// directions via CID-framed records (content type 25). +#[test] +fn dtls12_psk_with_cid_handshake_and_app_data() { + let _ = env_logger::try_init(); + + let client_cid: &[u8] = b"iot-c"; + let server_cid: &[u8] = b"iot-s"; + let (client_config, server_config) = psk_configs_with_cid(client_cid, server_cid); + + let mut now = Instant::now(); + + let mut client = Dtls::new_12_psk(client_config, now); + client.set_active(true); + let mut server = Dtls::new_12_psk(server_config, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + let mut client_reported_cid: Option> = None; + let mut server_reported_cid: Option> = None; + + for _ in 0..60 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + + client_connected |= co.connected; + server_connected |= so.connected; + if co.connection_id.is_some() { + client_reported_cid = co.connection_id; + } + if so.connection_id.is_some() { + server_reported_cid = so.connection_id; + } + + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + + assert!(client_connected, "PSK+CID client should connect"); + assert!(server_connected, "PSK+CID server should connect"); + + assert_eq!( + client_reported_cid.as_deref(), + Some(client_cid), + "PSK client should emit its own inbound CID" + ); + assert_eq!( + server_reported_cid.as_deref(), + Some(server_cid), + "PSK server should emit its own inbound CID" + ); + + let req = b"psk-cid-req"; + client.send_application_data(req).expect("client send"); + let client_pkts = collect_packets(&mut client); + assert!( + client_pkts.iter().any(|p| !p.is_empty() && p[0] == 25), + "PSK client must emit tls12_cid app-data records after CID negotiation" + ); + deliver_packets(&client_pkts, &mut server); + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == req), + "Server must decrypt PSK+CID client data" + ); + + let reply = b"psk-cid-reply"; + server.send_application_data(reply).expect("server send"); + let server_pkts = collect_packets(&mut server); + assert!( + server_pkts.iter().any(|p| !p.is_empty() && p[0] == 25), + "PSK server must emit tls12_cid app-data records after CID negotiation" + ); + deliver_packets(&server_pkts, &mut client); + let co = drain_outputs(&mut client); + assert!( + co.app_data.iter().any(|d| d == reply), + "Client must decrypt PSK+CID server data" + ); +} + +/// Explicit PSK+CID rebinding harness. +/// +/// In a real deployment a roaming IoT device's UDP 5-tuple changes under an +/// active association (Wi-Fi ⇄ LTE, NAT rebind, etc). dimpl is sans-io: the +/// engine never sees an address, so the harness represents the transport tuple +/// as an explicit label (`TransportTuple`) carried alongside each packet by the +/// test code. We then: +/// +/// 1. Establish a PSK+CID session via tuple A. +/// 2. Exchange application data routed through tuple A, recording the CID and +/// sequence numbers the server observes on those records. +/// 3. **Rebind**: the test relabels the client's apparent tuple to B and +/// continues sending — at the wire level nothing about the DTLS record +/// changes except record sequence numbers; only the out-of-band tuple label +/// changes. The server has no tuple awareness, so it must decrypt purely by +/// CID. This is the property RFC 9146 is supposed to deliver. +/// 4. Inject a **late-arriving packet from tuple A** after the rebind — +/// modelling a middlebox that buffered it pre-transition. It must still +/// decrypt because CID identifies the association, not the source tuple. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct TransportTuple(&'static str); + +struct TaggedPacket { + src: TransportTuple, + bytes: Vec, +} + +#[test] +fn dtls12_psk_with_cid_rebinds_across_transport_tuples() { + let _ = env_logger::try_init(); + + let (client_config, server_config) = psk_configs_with_cid(b"roam-c", b"roam-s"); + let mut now = Instant::now(); + + let mut client = Dtls::new_12_psk(client_config, now); + client.set_active(true); + let mut server = Dtls::new_12_psk(server_config, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..60 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + // Phase 1: the client is reachable via tuple A. Send app data and record + // the CID+sequence the server decrypts. + let tuple_a = TransportTuple("home-wifi: 10.0.0.1:54321"); + let tuple_b = TransportTuple("lte-fallback: 203.0.113.7:60000"); + + let pre_payload = b"pre-rebind via tuple A"; + client.send_application_data(pre_payload).expect("send"); + let tagged_a: Vec = collect_packets(&mut client) + .into_iter() + .map(|bytes| TaggedPacket { + src: tuple_a, + bytes, + }) + .collect(); + assert!( + tagged_a + .iter() + .any(|p| !p.bytes.is_empty() && p.bytes[0] == 25), + "PSK client must emit a tls12_cid record on tuple A" + ); + for p in &tagged_a { + assert_eq!(p.src, tuple_a); + // Caller does whatever socket-layer routing it likes; the server + // engine just gets bytes. + let _ = server.handle_packet(&p.bytes); + } + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == pre_payload), + "Server must decrypt PSK+CID traffic arriving on tuple A" + ); + + // Phase 2: the transport tuple rebinds to B. No API call on the engine is + // needed — that's the whole point. The test simply changes the label it + // attaches to outgoing packets. + now += Duration::from_secs(45); + let post_payload = b"post-rebind via tuple B"; + client.send_application_data(post_payload).expect("send"); + let tagged_b: Vec = collect_packets(&mut client) + .into_iter() + .map(|bytes| TaggedPacket { + src: tuple_b, + bytes, + }) + .collect(); + assert!( + !tagged_b.is_empty(), + "Client must emit packets after rebinding to tuple B" + ); + for p in &tagged_b { + assert_eq!(p.src, tuple_b); + let _ = server.handle_packet(&p.bytes); + } + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == post_payload), + "Server must decrypt PSK+CID traffic after the transport tuple \ + changes from {:?} to {:?} — association continuity must come from \ + CID, not the 5-tuple", + tuple_a, + tuple_b + ); + + // Phase 3: a middlebox releases a buffered packet that was originally + // transmitted on tuple A, arriving AFTER the rebind. The record's CID is + // still valid and its sequence number is still fresh, so the server must + // accept it. + let late_payload = b"late packet via tuple A after rebind"; + client.send_application_data(late_payload).expect("send"); + let tagged_late_a: Vec = collect_packets(&mut client) + .into_iter() + .map(|bytes| TaggedPacket { + src: tuple_a, // buffered packet labelled with the OLD tuple + bytes, + }) + .collect(); + for p in &tagged_late_a { + assert_eq!(p.src, tuple_a); + let _ = server.handle_packet(&p.bytes); + } + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == late_payload), + "Server must accept a late packet tagged with the pre-rebind tuple {:?} \ + because CID (not source tuple) identifies the association", + tuple_a + ); +} + +/// RFC 9146 §4 / RFC 6347 §4.1.2.7 applied to a PSK session: a stray +/// `tls12_cid` record where CID is not negotiated must be discarded without +/// dropping unrelated records coalesced into the same datagram. PSK analogue +/// of the cert-path `dtls12_unsolicited_cid_record_does_not_drop_coalesced_records` +/// test, kept because PSK is the primary-path product target per the rollback +/// plan and the coalesced-parsing guard should hold regardless of auth mode. +#[test] +fn dtls12_psk_unsolicited_cid_record_does_not_drop_coalesced_records() { + let _ = env_logger::try_init(); + + // Neither side configures CID, so a content-type-25 record is unsolicited. + let (client_config, server_config) = psk_configs(); + let mut now = Instant::now(); + + let mut client = Dtls::new_12_psk(client_config, now); + client.set_active(true); + let mut server = Dtls::new_12_psk(server_config, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..60 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + let payload = b"psk-coalesced-after-stray"; + client.send_application_data(payload).expect("send"); + let client_pkts = collect_packets(&mut client); + let valid_pkt = client_pkts + .iter() + .find(|p| !p.is_empty() && p[0] == 23) + .expect("PSK client should emit an ApplicationData record") + .clone(); + + // Stray tls12_cid record, zero-length body, standard 13-byte DTLS header. + let stray_cid: [u8; 13] = [ + 25, // ContentType::Tls12Cid + 0xfe, 0xfd, // DTLS 1.2 + 0, 1, // epoch + 0, 0, 0, 0, 0, 0xab, // seqnum + 0, 0, // length = 0 + ]; + let mut coalesced = Vec::with_capacity(stray_cid.len() + valid_pkt.len()); + coalesced.extend_from_slice(&stray_cid); + coalesced.extend_from_slice(&valid_pkt); + + deliver_packets(&[coalesced], &mut server); + let so = drain_outputs(&mut server); + assert!( + so.app_data.iter().any(|d| d == payload), + "PSK server must process coalesced ApplicationData after a stray tls12_cid" + ); +} + +/// RFC 9146 §5.3 / RFC 6347 §4.1.2.7 applied to a PSK session: a tampered CID +/// field must be dropped silently at the AEAD layer, and the replay window +/// must not advance on the tampered sequence — so the legitimate retransmit +/// still arrives. PSK-specific analogue of the cert-path tamper test. +#[test] +fn dtls12_psk_with_cid_tampered_record_is_dropped() { + let _ = env_logger::try_init(); + + let client_cid: &[u8] = b"psk-tc"; + let server_cid: &[u8] = b"psk-ts"; + let (client_config, server_config) = psk_configs_with_cid(client_cid, server_cid); + + let mut now = Instant::now(); + let mut client = Dtls::new_12_psk(client_config, now); + client.set_active(true); + let mut server = Dtls::new_12_psk(server_config, now); + server.set_active(false); + + let mut client_connected = false; + let mut server_connected = false; + for _ in 0..60 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + client_connected |= co.connected; + server_connected |= so.connected; + deliver_packets(&co.packets, &mut server); + deliver_packets(&so.packets, &mut client); + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + assert!(client_connected && server_connected); + + let payload = b"psk-cid-tamper-canary"; + server.send_application_data(payload).expect("server send"); + let server_pkts = collect_packets(&mut server); + let cid_pkt = server_pkts + .iter() + .find(|p| !p.is_empty() && p[0] == 25) + .expect("PSK server should emit a tls12_cid record") + .clone(); + + assert!(cid_pkt.len() >= 11 + client_cid.len() + 2); + assert_eq!(&cid_pkt[11..11 + client_cid.len()], client_cid); + + let mut tampered = cid_pkt.clone(); + tampered[11] ^= 0x40; + + deliver_packets(&[tampered], &mut client); + let after_tamper = drain_outputs(&mut client); + assert!( + after_tamper.app_data.is_empty(), + "PSK client must not surface app data from a tampered CID record" + ); + + deliver_packets(&[cid_pkt], &mut client); + let after_valid = drain_outputs(&mut client); + assert!( + after_valid.app_data.iter().any(|d| d == payload), + "PSK client must accept the untampered original after a tampered drop" + ); +} diff --git a/tests/dtls13/handshake.rs b/tests/dtls13/handshake.rs index 1799bf7f..f5ba189b 100644 --- a/tests/dtls13/handshake.rs +++ b/tests/dtls13/handshake.rs @@ -59,6 +59,244 @@ fn dtls13_basic_handshake() { assert!(server_connected, "Server should be connected"); } +/// Scope guard: dimpl does not currently implement DTLS 1.3 CID +/// (RFC 9147 §9 defines the DTLS 1.3 CID mechanism via +/// `NewConnectionId` / `RequestConnectionId` post-handshake messages and +/// a unified-header CID bit; that's a separate feature from the +/// RFC 9146 DTLS 1.2 CID support). If a caller configures +/// `with_connection_id` on a DTLS 1.3 Dtls, the current implementation +/// silently ignores it: no `Output::ConnectionId` event, no `tls12_cid` +/// (content type 25) bytes on the wire. This test pins that behavior so +/// accidental partial DTLS 1.3 CID enablement is caught. +#[test] +#[cfg(feature = "rcgen")] +fn dtls13_with_connection_id_config_does_not_negotiate_cid() { + use dimpl::Config; + use dimpl::certificate::generate_self_signed_certificate; + + 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"); + + // Both sides configure CID but run DTLS 1.3. + let config = Arc::new( + Config::builder() + .with_connection_id(b"should-not-appear".to_vec()) + .build() + .expect("build config with CID"), + ); + + 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; + let mut saw_tls12_cid_on_wire = 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; + + for p in client_out.packets.iter().chain(server_out.packets.iter()) { + if !p.is_empty() && p[0] == 25 { + saw_tls12_cid_on_wire = true; + } + } + + deliver_packets(&client_out.packets, &mut server); + deliver_packets(&server_out.packets, &mut client); + + if client_connected && server_connected { + break; + } + now += Duration::from_millis(10); + } + + assert!( + client_connected && server_connected, + "handshake should complete" + ); + // The DTLS 1.3 `DrainedOutputs` does not expose a `connection_id` field — + // the engine does not emit `Output::ConnectionId` there, so there is no + // variant to observe. Wire inspection is the definitive check. + assert!( + !saw_tls12_cid_on_wire, + "DTLS 1.3 must never emit tls12_cid (25) records" + ); +} + +/// RFC 8446 §4.2: a server MUST NOT send an extension the client did not +/// offer. dimpl's DTLS 1.3 ClientHello does not offer `connection_id` +/// (DTLS 1.3 CID per RFC 9147 §9 is not implemented), so any `0x0036` +/// codepoint in ServerHello is unsolicited and the client must surface +/// `SecurityError` (caller translates to `illegal_parameter`) instead of +/// silently ignoring the extension. We splice a fake CID extension into a +/// real ServerHello and assert the rejection fires before any other +/// processing. +#[test] +#[cfg(feature = "rcgen")] +fn dtls13_client_rejects_unsolicited_connection_id_in_server_hello() { + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + let config = dtls13_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); + + // Drive the DTLS 1.3 handshake far enough that the server emits its + // first ServerHello, withhold it from the client, splice a CID + // extension into it, then deliver the tampered version. + let mut sh_pkt: Option> = None; + for _ in 0..6 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + deliver_packets(&co.packets, &mut server); + for p in &so.packets { + // Handshake content type 22 with first inner msg_type 2 = ServerHello. + // DTLS 1.3 record layout differs from 1.2; we use a coarse heuristic: + // first content-type-22 record from the server post-cookie is the SH. + if !p.is_empty() && p[0] == 22 && sh_pkt.is_none() { + sh_pkt = Some(p.clone()); + continue; // withhold from victim + } + let _ = client.handle_packet(p); + } + if sh_pkt.is_some() { + break; + } + now += Duration::from_millis(10); + } + let mut sh = sh_pkt.expect("server should emit a ServerHello record within 6 iterations"); + + // Splice CID extension (type 0x0036) at the end of the ServerHello + // body. We patch the ServerHello's outer DTLSCiphertext fields just + // enough to keep parsing from blowing up before the extension walk + // reaches the CID codepoint. For DTLS 1.3 the precise framing is + // version-dependent, so we instead take a permissive approach: append + // the extension bytes and let the client's strict ServerHello parser + // reject either at structural validation (also acceptable) OR at the + // CID-codepoint match. Either outcome is a non-Ok handshake, which is + // what RFC 8446 §4.2 requires. + sh.extend_from_slice(&[0x00, 0x36, 0x00, 0x00]); + + let result = client.handle_packet(&sh); + // Acceptable outcomes: SecurityError (CID rejection or other strict + // parse failure). Bare `Ok(())` would mean we silently ignored a + // forbidden extension, which violates RFC 8446 §4.2. + match result { + Err(dimpl::Error::SecurityError(_)) + | Err(dimpl::Error::ParseError(_)) + | Err(dimpl::Error::ParseIncomplete) + | Err(dimpl::Error::IncompleteServerHello) + | Err(dimpl::Error::CryptoError(_)) => { + // Any of these indicate the spliced ServerHello did not pass + // through unchecked. The explicit CID rejection at + // `dtls13/client.rs` is the path we care about, but other + // strict parsers catching it earlier is also acceptable — + // RFC 8446 §4.2 requires the handshake be aborted, not the + // specific error variant. + } + Ok(()) => panic!( + "DTLS 1.3 client must not silently accept ServerHello with connection_id extension" + ), + other => panic!("unexpected error: {:?}", other), + } +} + +/// Regression guard for the "config proxy" bug: a direct +/// `Dtls::new_13` client with `with_connection_id` configured does NOT +/// emit the `0x0036` codepoint in its ClientHello (the DTLS 1.3 +/// handshake builder omits CID unconditionally). A server echoing +/// `0x0036` in ServerHello is therefore unsolicited and must abort +/// with `SecurityError` per RFC 8446 §4.2 — even though +/// `config.connection_id().is_some()`. Pins the `offered_cid` flag +/// replacing the earlier `config`-based proxy. +#[test] +#[cfg(feature = "rcgen")] +fn dtls13_new_13_with_cid_config_still_rejects_unsolicited_sh_extension() { + use dimpl::Config; + use dimpl::certificate::generate_self_signed_certificate; + + let _ = env_logger::try_init(); + + let client_cert = generate_self_signed_certificate().expect("gen cert"); + let server_cert = generate_self_signed_certificate().expect("gen cert"); + // Caller configured CID but picked the direct `new_13` constructor, + // which does not emit `0x0036`. Any server echo is unsolicited. + let config = Arc::new( + Config::builder() + .with_connection_id(b"direct-13-cid".to_vec()) + .build() + .expect("config with CID"), + ); + + 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 sh_pkt: Option> = None; + for _ in 0..6 { + client.handle_timeout(now).expect("client timeout"); + server.handle_timeout(now).expect("server timeout"); + let co = drain_outputs(&mut client); + let so = drain_outputs(&mut server); + deliver_packets(&co.packets, &mut server); + for p in &so.packets { + if !p.is_empty() && p[0] == 22 && sh_pkt.is_none() { + sh_pkt = Some(p.clone()); + continue; + } + let _ = client.handle_packet(p); + } + if sh_pkt.is_some() { + break; + } + now += Duration::from_millis(10); + } + let mut sh = sh_pkt.expect("server should emit ServerHello within 6 iterations"); + sh.extend_from_slice(&[0x00, 0x36, 0x00, 0x00]); + + let result = client.handle_packet(&sh); + match result { + Err(dimpl::Error::SecurityError(_)) + | Err(dimpl::Error::ParseError(_)) + | Err(dimpl::Error::ParseIncomplete) + | Err(dimpl::Error::IncompleteServerHello) + | Err(dimpl::Error::CryptoError(_)) => { + // Rejection path taken — contract holds. The specific + // SecurityError path is what the `offered_cid` flag + // enables; other strict parsers catching it earlier is + // also acceptable. + } + Ok(()) => panic!( + "DTLS 1.3 client with `with_connection_id` but no hybrid CH must still reject \ + unsolicited `0x0036` from ServerHello — RFC 8446 §4.2" + ), + other => panic!("unexpected error: {:?}", other), + } +} + #[test] #[cfg(feature = "rcgen")] fn dtls13_handshake_with_keying_material() {