Fix handshake deadlines and retry timing

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Unreleased
 
+  * Start handshake clocks at ClientHello and honor timing settings consistently #162
   * Update OpenSSL test dependency to resolve security advisories #176
   * Remove `HandshakePending`. All errors are fatal. #175
   * Return accepted byte counts from `send_application_data` #175

README.md

@@ -102,12 +102,15 @@ use std::time::Instant;
 use dimpl::{certificate, Config, Dtls, Output};
 
 // Stub I/O to keep the example focused on the state machine
-enum Event { Udp(Vec<u8>), Timer(Instant) }
-fn wait_next_event(_next_wake: Option<Instant>) -> Event { Event::Udp(Vec::new()) }
+enum Event { Udp(Vec<u8>, Instant), Timer(Instant) }
+fn wait_next_event(_next_wake: Option<Instant>) -> Event {
+    Event::Udp(Vec::new(), Instant::now())
+}
 fn send_udp(_bytes: &[u8]) {}
 
 fn example_event_loop(mut dtls: Dtls) -> Result<(), dimpl::Error> {
     let mut next_wake: Option<Instant> = None;
+    let mut received_packet: Option<Vec<u8>> = None;
     loop {
         // Drain engine output until we have to wait for I/O or a timer
         let mut out_buf = vec![0u8; 2048];
@@ -138,9 +141,17 @@ fn example_event_loop(mut dtls: Dtls) -> Result<(), dimpl::Error> {
             }
         }
 
+        if let Some(packet) = received_packet.take() {
+            dtls.handle_packet(&packet)?;
+            continue;
+        }
+
         // Block waiting for either UDP input or the scheduled timeout
         match wait_next_event(next_wake) {
-            Event::Udp(pkt) => dtls.handle_packet(&pkt)?,
+            Event::Udp(pkt, now) => {
+                dtls.handle_timeout(now)?;
+                received_packet = Some(pkt);
+            }
             Event::Timer(now) => dtls.handle_timeout(now)?,
         }
     }

src/auto.rs

@@ -16,7 +16,7 @@
 /// and falls back to DTLS 1.2 via [`Error::Dtls12Fallback`] if the
 /// reassembled ClientHello does not offer DTLS 1.3.
 use std::sync::Arc;
-use std::time::{Duration, Instant};
+use std::time::Instant;
 
 use arrayvec::ArrayVec;
 
@@ -28,6 +28,7 @@ use crate::dtls13::message::Random;
 use crate::dtls13::message::SignatureAlgorithmsExtension;
 use crate::dtls13::message::SupportedGroupsExtension;
 use crate::dtls13::message::UseSrtpExtension;
+use crate::timer::HandshakeTimers;
 use crate::types::NamedGroup;
 use crate::{Config, CryptoError, DtlsCertificate, Error, Output, SeededRng, TimeoutError};
 // Extension type constants
@@ -269,10 +270,8 @@ pub(crate) struct ClientPending {
     needs_send: bool,
     /// Last time handle_timeout was called.
     last_now: Instant,
-    /// When to retransmit the wire_packet.
-    retransmit_at: Option<Instant>,
-    /// How many retransmits have occurred.
-    retransmit_count: usize,
+    timers: HandshakeTimers,
+    rng: SeededRng,
     closed: bool,
 }
 
@@ -284,15 +283,18 @@ impl ClientPending {
     ) -> Result<Self, Error> {
         let hybrid = HybridClientHello::new(&config)?;
         let wire_packet = hybrid.wire_packet();
+        let mut rng = SeededRng::new(config.rng_seed());
+        let mut timers = HandshakeTimers::new(&config, &mut rng);
+        timers.begin_flight(&mut rng);
         Ok(ClientPending {
             hybrid,
             config,
             certificate,
             wire_packet,
             needs_send: true,
             last_now: now,
-            retransmit_at: None,
-            retransmit_count: 0,
+            timers,
+            rng,
             closed: false,
         })
     }
@@ -302,31 +304,22 @@ impl ClientPending {
         if self.closed {
             return Ok(());
         }
-        // Arm initial retransmit timer on first call
-        if self.retransmit_at.is_none() {
-            self.retransmit_at = Some(now + Duration::from_secs(1));
-            return Ok(());
-        }
-        if let Some(deadline) = self.retransmit_at {
-            if now >= deadline {
-                if self.retransmit_count >= self.config.flight_retries() {
-                    return Err(Error::Timeout(TimeoutError::HybridClientHello));
-                }
-                self.retransmit_count += 1;
-                self.needs_send = true;
-                // Exponential backoff: 2s, 4s, 8s, ...
-                let shift = self.retransmit_count.min(5) as u32;
-                let rto = Duration::from_secs(1u64 << shift);
-                self.retransmit_at = Some(now + rto);
-            }
+        let resend = self
+            .timers
+            .handle_timeout(now, &mut self.rng)
+            .map_err(|error| {
+                Error::Timeout(match error {
+                    TimeoutError::Handshake => TimeoutError::HybridClientHello,
+                    other => other,
+                })
+            })?;
+        if resend {
+            self.needs_send = true;
         }
         Ok(())
     }
 
     pub fn poll_output<'a>(&mut self, buf: &'a mut [u8]) -> Output<'a> {
-        if self.closed {
-            return Output::Timeout(self.last_now + Duration::from_secs(10 * 365 * 24 * 60 * 60));
-        }
         if self.needs_send {
             let len = self.wire_packet.len();
             if buf.len() < len {
@@ -335,11 +328,11 @@ impl ClientPending {
             }
             self.needs_send = false;
             buf[..len].copy_from_slice(&self.wire_packet);
+            self.timers.start_handshake(self.last_now);
+            self.timers.flight_sent(self.last_now);
             return Output::Packet(&buf[..len]);
         }
-        let next = self
-            .retransmit_at
-            .unwrap_or(self.last_now + Duration::from_secs(1));
+        let next = self.timers.poll_timeout(self.last_now);
         Output::Timeout(next)
     }
 
@@ -350,12 +343,30 @@ impl ClientPending {
     pub fn close(&mut self) -> Result<(), Error> {
         self.closed = true;
         self.needs_send = false;
-        self.retransmit_at = None;
+        self.timers.stop();
         Ok(())
     }
 
-    pub fn into_parts(self) -> (HybridClientHello, Arc<Config>, DtlsCertificate, Instant) {
-        (self.hybrid, self.config, self.certificate, self.last_now)
+    pub fn into_parts(
+        self,
+    ) -> (
+        HybridClientHello,
+        Arc<Config>,
+        DtlsCertificate,
+        Instant,
+        HandshakeTimers,
+    ) {
+        // Bind the fields first: CodeQL treats a tuple holding `self.certificate`
+        // directly as sensitive data and taints every element, including `config`.
+        let Self {
+            hybrid,
+            config,
+            certificate,
+            last_now,
+            timers,
+            ..
+        } = self;
+        (hybrid, config, certificate, last_now, timers)
     }
 }
 

src/config.rs

@@ -9,6 +9,8 @@ use crate::dtls12::message::Dtls12CipherSuite;
 use crate::types::{Dtls13CipherSuite, NamedGroup};
 use crate::{ConfigError, Error};
 
+const MAX_TIMING_DURATION: Duration = Duration::from_secs(10 * 365 * 24 * 60 * 60);
+
 /// Callback for resolving PSK identities to shared secrets.
 ///
 /// Implement this trait and provide it via [`ConfigBuilder::with_psk_client`]
@@ -362,16 +364,25 @@ impl ConfigBuilder {
 
     /// Set the time of first retry.
     ///
-    /// Every flight restarts with this value.
-    /// Doubled for every retry with a ±25% jitter.
-    /// Defaults to 1 second.
+    /// Each flight starts its timer when its first packet is emitted. The base
+    /// interval doubles after every retry, with independent +/-25% jitter on
+    /// each interval. This is proportional even for sub-second values; the
+    /// minimum jittered interval is 1 nanosecond. Backoff arithmetic saturates
+    /// on overflow. Also governs DTLS 1.3 KeyUpdate flights.
+    ///
+    /// Defaults to 1 second. Must be nonzero and at most ten years (3650 days).
     pub fn flight_start_rto(mut self, rto: Duration) -> Self {
         self.flight_start_rto = rto;
         self
     }
 
     /// Set the max number of retries per flight.
     ///
+    /// Excludes the initial transmission: zero disables retransmissions.
+    /// Timer-driven and duplicate-triggered retransmissions share this budget.
+    /// Once the handshake has completed locally, the final flight is still
+    /// resent in answer to the peer's retransmitted final flight, as
+    /// RFC 6347 §4.2.4 requires. Also governs DTLS 1.3 KeyUpdate flights.
     /// Defaults to 4.
     pub fn flight_retries(mut self, retries: usize) -> Self {
         self.flight_retries = retries;
@@ -380,7 +391,22 @@ impl ConfigBuilder {
 
     /// Set the timeout for the entire handshake, regardless of flights.
     ///
-    /// Defaults to 40 seconds.
+    /// Starts when the client emits its first ClientHello packet through
+    /// [`crate::Output::Packet`], or when the server accepts its first
+    /// structurally valid ClientHello fragment, even if its body is later rejected.
+    /// A corrected ClientHello shares the same deadline for this peer.
+    /// Construction and idle time consume no budget. Uses the logical time
+    /// supplied to [`crate::Dtls::handle_timeout`].
+    ///
+    /// The absolute deadline survives Auto version selection, cookie exchanges,
+    /// later flights, and retransmissions. It is disabled after completion and
+    /// does not apply to application traffic or DTLS 1.3 KeyUpdate.
+    ///
+    /// Retry exhaustion may fail earlier; increasing this timeout does not
+    /// increase [`Self::flight_retries`]. With the defaults, an unanswered flight
+    /// exhausts retries after roughly 31 seconds, before the 40-second deadline.
+    ///
+    /// Defaults to 40 seconds. Must be nonzero and at most ten years (3650 days).
     pub fn handshake_timeout(mut self, timeout: Duration) -> Self {
         self.handshake_timeout = timeout;
         self
@@ -476,8 +502,8 @@ impl ConfigBuilder {
 
     /// Build the configuration.
     ///
-    /// This validates the crypto provider before returning the configuration.
-    /// Returns `Error::ConfigError` if the provider is invalid.
+    /// Validates timing settings and the crypto provider before returning the
+    /// configuration. Returns `Error::ConfigError` for invalid settings.
     ///
     /// The crypto provider is selected in the following priority order:
     /// 1. Explicit provider set via `with_crypto_provider()`
@@ -486,6 +512,13 @@ impl ConfigBuilder {
     /// 4. RustCrypto provider (if `rust-crypto` feature is enabled)
     /// 5. Panic if no provider is available
     pub fn build(self) -> Result<Config, Error> {
+        if self.handshake_timeout.is_zero() || self.handshake_timeout > MAX_TIMING_DURATION {
+            return Err(ConfigError::InvalidHandshakeTimeout.into());
+        }
+        if self.flight_start_rto.is_zero() || self.flight_start_rto > MAX_TIMING_DURATION {
+            return Err(ConfigError::InvalidFlightStartRto.into());
+        }
+
         let crypto_provider = self
             .crypto_provider
             .or_else(|| CryptoProvider::get_default().cloned());
@@ -693,6 +726,61 @@ impl fmt::Debug for ConfigBuilder {
 mod tests {
     use super::*;
 
+    #[test]
+    fn timing_defaults() {
+        let config = Config::default();
+        assert_eq!(config.handshake_timeout(), Duration::from_secs(40));
+        assert_eq!(config.flight_start_rto(), Duration::from_secs(1));
+        assert_eq!(config.flight_retries(), 4);
+    }
+
+    #[test]
+    fn timing_duration_bounds() {
+        for duration in [
+            Duration::ZERO,
+            MAX_TIMING_DURATION + Duration::from_nanos(1),
+            Duration::MAX,
+        ] {
+            let error = Config::builder()
+                .handshake_timeout(duration)
+                .build()
+                .expect_err("unsupported handshake timeout");
+            assert_eq!(
+                error,
+                Error::ConfigError(ConfigError::InvalidHandshakeTimeout)
+            );
+            assert_eq!(
+                error.to_string(),
+                "config error: handshake_timeout must be nonzero and at most 3650 days"
+            );
+            let error = Config::builder()
+                .flight_start_rto(duration)
+                .build()
+                .expect_err("unsupported initial RTO");
+            assert_eq!(
+                error,
+                Error::ConfigError(ConfigError::InvalidFlightStartRto)
+            );
+            assert_eq!(
+                error.to_string(),
+                "config error: flight_start_rto must be nonzero and at most 3650 days"
+            );
+        }
+        for duration in [Duration::from_nanos(1), MAX_TIMING_DURATION] {
+            for retries in [0, usize::MAX] {
+                let config = Config::builder()
+                    .handshake_timeout(duration)
+                    .flight_start_rto(duration)
+                    .flight_retries(retries)
+                    .build()
+                    .expect("supported timing boundaries");
+                assert_eq!(config.handshake_timeout(), duration);
+                assert_eq!(config.flight_start_rto(), duration);
+                assert_eq!(config.flight_retries(), retries);
+            }
+        }
+    }
+
     #[test]
     fn rejects_zero_mtu() {
         match Config::builder().mtu(0).build() {

src/dtls12/client.rs

@@ -29,6 +29,7 @@ use crate::dtls12::message::{CompressionMethod, ContentType, Cookie};
 use crate::dtls12::message::{DigitallySigned, Dtls12CipherSuite};
 use crate::dtls12::message::{ExtensionType, KeyExchangeAlgorithm, MessageType, ProtocolVersion};
 use crate::dtls12::message::{Random, SessionId, SignatureAndHashAlgorithm};
+use crate::timer::HandshakeTimers;
 use crate::{Config, DtlsCertificate, Error, InternalError, KeyingMaterial, Output};
 
 /// DTLS client
@@ -129,7 +130,8 @@ impl Client {
         config: std::sync::Arc<Config>,
         certificate: DtlsCertificate,
         now: Instant,
-    ) -> Result<Client, Error> {
+        timers: HandshakeTimers,
+    ) -> Client {
         assert!(
             !certificate.certificate.is_empty(),
             "Client certificate cannot be empty"
@@ -149,19 +151,11 @@ impl Client {
         };
         let mut engine = Engine::new(config, auth);
         engine.set_client(true);
-        // The hybrid ClientHello was sent with message_seq=0 outside this
-        // engine. Advance the counter so the with-cookie CH gets message_seq=1
-        // per RFC 6347 §4.2.2.
-        engine.set_next_handshake_seq_no(1);
-        // Inject the hybrid CH into the transcript so it matches the server's
-        // transcript when the server skips HelloVerifyRequest.
-        engine.transcript.extend_from_slice(handshake_fragment);
-        // Advance epoch-0 record sequence past the hybrid CH record.
-        engine.advance_epoch_0_sequence();
+        engine.inject_hybrid_client_hello(handshake_fragment, timers);
         let extension_data = engine.pop_buffer();
         let defragment_buffer = engine.pop_buffer();
 
-        let mut client = Client {
+        Client {
             state: State::AwaitHelloVerifyRequest,
             engine,
             random: Some(random),
@@ -177,9 +171,7 @@ impl Client {
             last_now: now,
             local_events: VecDeque::new(),
             queued_data: Vec::new(),
-        };
-        client.handle_timeout(now)?;
-        Ok(client)
+        }
     }
 
     pub fn into_server(self) -> Server {
@@ -212,6 +204,9 @@ impl Client {
     }
 
     pub fn poll_output<'a>(&mut self, buf: &'a mut [u8]) -> Output<'a> {
+        if self.state == State::SendClientHello {
+            return Output::Timeout(self.last_now);
+        }
         if let Some(event) = self.local_events.pop_front() {
             return event.into_output(buf, &self.server_certificates);
         }