From 0f24333c0d02d6cceaaaff58301d6e556abd2094 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Mon, 16 Mar 2026 12:15:44 +0000 Subject: [PATCH] Fix post-rekey jitter spikes from drain-window frames After rekey cutover, frames encrypted with the old session can still arrive during the 10s drain window. These carry large sender timestamps from the old session, producing enormous transit deltas that spike the EWMA jitter estimator (observed 2,000-7,000ms spikes on UDP links). Add a time-based grace period (DRAIN_WINDOW_SECS + 5s = 15s) after rekey that suppresses jitter updates until drain-window frames have flushed. Baselines still update every frame so calculation resumes cleanly after grace expires. Fixes #10 --- CHANGELOG.md | 4 +++ src/mmp/mod.rs | 8 +++--- src/mmp/receiver.rs | 67 +++++++++++++++++++++++++++++++++++++++++---- src/node/session.rs | 8 ++++-- src/peer/active.rs | 6 ++-- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db08a18..ca51882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 backoff capped at 300 seconds. - Control socket permissions: non-root users couldn't connect. Daemon now chowns socket and directory to `root:fips` group at bind time. +- Post-rekey jitter spikes: old-session frames arriving via the drain window + produced 2,000–7,000ms jitter spikes that corrupted the EWMA estimator. + Added a 15-second grace period after rekey cutover that suppresses jitter + updates until drain-window frames have flushed. (#10) ## [0.1.0] - 2026-03-12 diff --git a/src/mmp/mod.rs b/src/mmp/mod.rs index bf2ba96..105b8cb 100644 --- a/src/mmp/mod.rs +++ b/src/mmp/mod.rs @@ -198,8 +198,8 @@ impl MmpPeerState { } /// Reset counter-dependent state for rekey cutover. - pub fn reset_for_rekey(&mut self) { - self.receiver.reset_for_rekey(); + pub fn reset_for_rekey(&mut self, now: Instant) { + self.receiver.reset_for_rekey(now); self.metrics.reset_for_rekey(); } @@ -263,8 +263,8 @@ impl MmpSessionState { } /// Reset counter-dependent state for rekey cutover. - pub fn reset_for_rekey(&mut self) { - self.receiver.reset_for_rekey(); + pub fn reset_for_rekey(&mut self, now: Instant) { + self.receiver.reset_for_rekey(now); self.metrics.reset_for_rekey(); } diff --git a/src/mmp/receiver.rs b/src/mmp/receiver.rs index 8494d0e..a8f201e 100644 --- a/src/mmp/receiver.rs +++ b/src/mmp/receiver.rs @@ -10,6 +10,14 @@ use crate::mmp::report::ReceiverReport; use crate::mmp::{DEFAULT_COLD_START_INTERVAL_MS, DEFAULT_OWD_WINDOW_SIZE, MAX_REPORT_INTERVAL_MS, MIN_REPORT_INTERVAL_MS}; +/// Grace period after rekey before resuming jitter calculation. +/// +/// During rekey cutover, frames from the old session may still arrive via the +/// drain window (DRAIN_WINDOW_SECS = 10s). These carry large sender timestamps +/// from the old session, producing enormous transit deltas that spike the EWMA +/// jitter estimator. We suppress jitter updates for drain window + 5s margin. +const REKEY_JITTER_GRACE_SECS: u64 = 15; + // ============================================================================ // Gap Tracker (burst loss detection) // ============================================================================ @@ -161,6 +169,11 @@ pub struct ReceiverState { /// Local time when the most recent frame was received (for dwell computation). last_recv_time: Option, + // --- Rekey grace --- + /// When set, jitter updates are suppressed until this instant passes. + /// Prevents drain-window frames from spiking the jitter estimator. + rekey_jitter_grace_until: Option, + // --- Report timing --- last_report_time: Option, report_interval: Duration, @@ -192,6 +205,7 @@ impl ReceiverState { ecn_ce_count: 0, last_sender_timestamp: 0, last_recv_time: None, + rekey_jitter_grace_until: None, last_report_time: None, report_interval: Duration::from_millis(cold_start_ms), interval_has_data: false, @@ -203,7 +217,7 @@ impl ReceiverState { /// After cutover, the new session starts with counter 0 and reset /// timestamps. Without resetting, the old `highest_counter` and /// `GapTracker.expected_next` cause false reorder/loss detection. - pub fn reset_for_rekey(&mut self) { + pub fn reset_for_rekey(&mut self, now: Instant) { self.highest_counter = 0; self.cumulative_reorder_count = 0; self.gap_tracker = GapTracker::new(); @@ -214,6 +228,8 @@ impl ReceiverState { self.owd_seq = 0; self.last_sender_timestamp = 0; self.last_recv_time = None; + self.rekey_jitter_grace_until = + Some(now + Duration::from_secs(REKEY_JITTER_GRACE_SECS)); self.ecn_ce_count = 0; self.interval_has_data = false; // Keep cumulative_packets_recv, cumulative_bytes_recv (lifetime stats) @@ -264,11 +280,18 @@ impl ReceiverState { let sender_us = (sender_timestamp_ms as i64) * 1000; // We can't get absolute µs from Instant, but we can compute the delta // between consecutive transits using relative Instant differences. - if let Some(prev_recv) = self.last_recv_time { - let recv_delta_us = now.duration_since(prev_recv).as_micros() as i64; - let send_delta_us = sender_us - (self.last_sender_timestamp as i64 * 1000); - let transit_delta = (recv_delta_us - send_delta_us) as i32; - self.jitter.update(transit_delta); + // Skip during post-rekey grace period to avoid drain-window spikes. + let in_grace = self.rekey_jitter_grace_until + .is_some_and(|deadline| now < deadline); + if !in_grace { + self.rekey_jitter_grace_until = None; // clear expired grace + if let Some(prev_recv) = self.last_recv_time { + let recv_delta_us = now.duration_since(prev_recv).as_micros() as i64; + let send_delta_us = + sender_us - (self.last_sender_timestamp as i64 * 1000); + let transit_delta = (recv_delta_us - send_delta_us) as i32; + self.jitter.update(transit_delta); + } } // OWD trend: use sender timestamp as a proxy for send time @@ -553,4 +576,36 @@ mod tests { r.update_report_interval_from_srtt(500_000); assert_eq!(r.report_interval(), Duration::from_millis(500)); } + + #[test] + fn test_rekey_jitter_grace_suppresses_spikes() { + let mut r = ReceiverState::new(32); + let t0 = Instant::now(); + + // Establish baseline with two frames so jitter starts updating + r.record_recv(1, 1000, 100, false, t0); + r.record_recv(2, 2000, 100, false, t0 + Duration::from_secs(1)); + assert_eq!(r.jitter_us(), 0); // perfect 1s spacing → 0 jitter + + // Simulate rekey: reset, then send a frame with a large old-session + // timestamp followed by a new-session timestamp near zero. + // Without grace, this would produce a huge jitter spike. + r.reset_for_rekey(t0 + Duration::from_secs(2)); + + // Frame arrives during grace period with old-session timestamp + r.record_recv(0, 120_000, 100, false, t0 + Duration::from_secs(3)); + // Next frame with new-session timestamp near zero + r.record_recv(1, 100, 100, false, t0 + Duration::from_secs(4)); + // Jitter should still be zero — updates suppressed during grace + assert_eq!(r.jitter_us(), 0); + + // After grace expires, jitter updates resume + let after_grace = t0 + Duration::from_secs(2) + + Duration::from_secs(REKEY_JITTER_GRACE_SECS + 1); + r.record_recv(2, 200, 100, false, after_grace); + r.record_recv(3, 300, 100, false, after_grace + Duration::from_millis(100)); + // Now jitter should be updating (non-zero or zero depending on timing) + // The key assertion is that it's not a multi-second spike + assert!(r.jitter_us() < 1_000_000); // less than 1 second + } } diff --git a/src/node/session.rs b/src/node/session.rs index d3c2468..f321941 100644 --- a/src/node/session.rs +++ b/src/node/session.rs @@ -5,6 +5,8 @@ //! (SessionSetup/SessionAck/SessionMsg3) carried inside SessionDatagram //! envelopes through the mesh. +use std::time::Instant; + use crate::config::SessionMmpConfig; use crate::mmp::MmpSessionState; use crate::noise::{HandshakeState, NoiseSession}; @@ -425,8 +427,9 @@ impl SessionEntry { self.rekey_completed_ms = 0; // Reset MMP counters to avoid metric discontinuity + let now = Instant::now(); if let Some(mmp) = &mut self.mmp { - mmp.reset_for_rekey(); + mmp.reset_for_rekey(now); } true } @@ -452,8 +455,9 @@ impl SessionEntry { self.rekey_initiator = false; // Reset MMP counters to avoid metric discontinuity + let now = Instant::now(); if let Some(mmp) = &mut self.mmp { - mmp.reset_for_rekey(); + mmp.reset_for_rekey(now); } true } diff --git a/src/peer/active.rs b/src/peer/active.rs index b8ee9ea..16ab281 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -887,8 +887,9 @@ impl ActivePeer { self.reset_replay_suppressed(); // Reset MMP counters to avoid metric discontinuity + let now = Instant::now(); if let Some(mmp) = &mut self.mmp { - mmp.reset_for_rekey(); + mmp.reset_for_rekey(now); } self.previous_our_index @@ -921,8 +922,9 @@ impl ActivePeer { self.reset_replay_suppressed(); // Reset MMP counters to avoid metric discontinuity + let now = Instant::now(); if let Some(mmp) = &mut self.mmp { - mmp.reset_for_rekey(); + mmp.reset_for_rekey(now); } self.previous_our_index