diff --git a/src/mmp/algorithms.rs b/src/mmp/algorithms.rs index ad5ef03..55550f4 100644 --- a/src/mmp/algorithms.rs +++ b/src/mmp/algorithms.rs @@ -192,6 +192,11 @@ impl OwdTrendDetector { } } + /// Clear all samples, keeping the same capacity. + pub fn clear(&mut self) { + self.samples.clear(); + } + /// Add an OWD sample. /// /// `seq` is a monotonic sequence number (e.g., truncated frame counter). diff --git a/src/mmp/metrics.rs b/src/mmp/metrics.rs index b61574a..3562c8b 100644 --- a/src/mmp/metrics.rs +++ b/src/mmp/metrics.rs @@ -48,6 +48,22 @@ pub struct MmpMetrics { } impl MmpMetrics { + /// Reset state derived from ReceiverReport counters for rekey cutover. + /// + /// The new session starts with counter 0, so the prev_rr deltas must + /// be reset to avoid computing bogus loss/goodput from the counter + /// discontinuity. RTT (SRTT) is preserved since it remains valid. + pub fn reset_for_rekey(&mut self) { + self.prev_rr_cum_packets = 0; + self.prev_rr_cum_bytes = 0; + self.prev_rr_highest_counter = 0; + self.prev_rr_ecn_ce = 0; + self.prev_rr_reorder = 0; + self.prev_rr_time = None; + self.delivery_ratio_forward = 1.0; + // Keep srtt, etx, trends, goodput_bps — they'll refresh from data + } + pub fn new() -> Self { Self { srtt: SrttEstimator::new(), diff --git a/src/mmp/mod.rs b/src/mmp/mod.rs index 36141e6..bf2ba96 100644 --- a/src/mmp/mod.rs +++ b/src/mmp/mod.rs @@ -197,6 +197,12 @@ impl MmpPeerState { } } + /// Reset counter-dependent state for rekey cutover. + pub fn reset_for_rekey(&mut self) { + self.receiver.reset_for_rekey(); + self.metrics.reset_for_rekey(); + } + /// Current operating mode. pub fn mode(&self) -> MmpMode { self.mode @@ -256,6 +262,12 @@ impl MmpSessionState { } } + /// Reset counter-dependent state for rekey cutover. + pub fn reset_for_rekey(&mut self) { + self.receiver.reset_for_rekey(); + self.metrics.reset_for_rekey(); + } + /// Current operating mode. pub fn mode(&self) -> MmpMode { self.mode diff --git a/src/mmp/receiver.rs b/src/mmp/receiver.rs index 5794d7d..8494d0e 100644 --- a/src/mmp/receiver.rs +++ b/src/mmp/receiver.rs @@ -198,6 +198,28 @@ impl ReceiverState { } } + /// Reset counter-dependent state for rekey cutover. + /// + /// 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) { + self.highest_counter = 0; + self.cumulative_reorder_count = 0; + self.gap_tracker = GapTracker::new(); + self.interval_packets_recv = 0; + self.interval_bytes_recv = 0; + self.jitter = JitterEstimator::new(); + self.owd_trend.clear(); + self.owd_seq = 0; + self.last_sender_timestamp = 0; + self.last_recv_time = None; + self.ecn_ce_count = 0; + self.interval_has_data = false; + // Keep cumulative_packets_recv, cumulative_bytes_recv (lifetime stats) + // Keep last_report_time, report_interval (report scheduling) + } + /// Record a received frame from this peer. /// /// Called on the RX path after AEAD decryption, before message dispatch. diff --git a/src/node/handlers/encrypted.rs b/src/node/handlers/encrypted.rs index 7ba4617..219927a 100644 --- a/src/node/handlers/encrypted.rs +++ b/src/node/handlers/encrypted.rs @@ -64,13 +64,16 @@ impl Node { ); let peer = self.peers.get_mut(&node_addr).unwrap(); - if let Some(_old_our_index) = peer.handle_peer_kbit_flip() - && let (Some(transport_id), Some(new_our_index)) = - (peer.transport_id(), peer.our_index()) - { - self.peers_by_index.insert( - (transport_id, new_our_index.as_u32()), - node_addr, + if let Some(_old_our_index) = peer.handle_peer_kbit_flip() { + // New index was pre-registered in peers_by_index during + // msg1 handling (handshake.rs). Verify, don't duplicate. + debug_assert!( + peer.transport_id().is_some() + && peer.our_index().is_some() + && self.peers_by_index.contains_key( + &(peer.transport_id().unwrap(), peer.our_index().unwrap().as_u32()) + ), + "peers_by_index should contain pre-registered new index after K-bit flip" ); } } diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index d6525e5..70f6a3f 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -537,6 +537,9 @@ impl Node { "Rekey msg2 processing failed" ); if let Some(idx) = peer.abandon_rekey() { + if let Some(tid) = peer.transport_id() { + self.peers_by_index.remove(&(tid, idx.as_u32())); + } let _ = self.index_allocator.free(idx); } } diff --git a/src/node/handlers/rekey.rs b/src/node/handlers/rekey.rs index d15bd71..13cbdbc 100644 --- a/src/node/handlers/rekey.rs +++ b/src/node/handlers/rekey.rs @@ -19,6 +19,10 @@ const DRAIN_WINDOW_SECS: u64 = 10; /// a peer's rekey msg1. const REKEY_DAMPENING_SECS: u64 = 30; +/// Delay FSP initiator cutover after handshake completion to allow +/// XK msg3 to reach the responder before K-bit-flipped data arrives. +const FSP_CUTOVER_DELAY_MS: u64 = 2000; + impl Node { /// Periodic rekey check. Called from the tick loop. /// @@ -79,14 +83,16 @@ impl Node { if let Some(peer) = self.peers.get_mut(&node_addr) && let Some(_old_our_index) = peer.cutover_to_new_session() { - if let (Some(transport_id), Some(new_our_index)) = - (peer.transport_id(), peer.our_index()) - { - self.peers_by_index.insert( - (transport_id, new_our_index.as_u32()), - node_addr, - ); - } + // New index was pre-registered in peers_by_index during + // msg2 handling (handshake.rs). Verify, don't duplicate. + debug_assert!( + peer.transport_id().is_some() + && peer.our_index().is_some() + && self.peers_by_index.contains_key( + &(peer.transport_id().unwrap(), peer.our_index().unwrap().as_u32()) + ), + "peers_by_index should contain pre-registered new index after cutover" + ); info!( peer = %self.peer_display_name(&node_addr), "Rekey cutover complete (initiator), K-bit flipped" @@ -281,10 +287,14 @@ impl Node { continue; } - // 1. Initiator-side cutover: completed rekey, pending session ready + // 1. Initiator-side cutover: completed rekey, pending session ready. + // Defer cutover until msg3 has had time to reach the responder. + // Without this delay, K-bit-flipped data can arrive before + // msg3, causing decryption failures on the responder. if entry.pending_new_session().is_some() && !entry.has_rekey_in_progress() && entry.is_rekey_initiator() + && now_ms.saturating_sub(entry.rekey_completed_ms()) >= FSP_CUTOVER_DELAY_MS { sessions_to_cutover.push(*node_addr); continue; diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index c2c6700..52d1e95 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -595,6 +595,7 @@ impl Node { }; entry.set_pending_session(session); + entry.set_rekey_completed_ms(Self::now_ms()); self.sessions.insert(*src_addr, entry); debug!( diff --git a/src/node/session.rs b/src/node/session.rs index 8f1845a..d3c2468 100644 --- a/src/node/session.rs +++ b/src/node/session.rs @@ -107,6 +107,9 @@ pub(crate) struct SessionEntry { rekey_initiator: bool, /// Dampening: last time peer sent us a rekey msg1 (Unix ms). last_peer_rekey_ms: u64, + /// When the FSP rekey handshake completed (initiator sent msg3, Unix ms). + /// Used to defer cutover until msg3 has time to reach the responder. + rekey_completed_ms: u64, } impl SessionEntry { @@ -142,6 +145,7 @@ impl SessionEntry { pending_new_session: None, rekey_initiator: false, last_peer_rekey_ms: 0, + rekey_completed_ms: 0, } } @@ -369,6 +373,16 @@ impl SessionEntry { } } + /// When the FSP rekey handshake completed (initiator sent msg3). + pub(crate) fn rekey_completed_ms(&self) -> u64 { + self.rekey_completed_ms + } + + /// Record when the FSP rekey handshake completed (initiator side). + pub(crate) fn set_rekey_completed_ms(&mut self, ms: u64) { + self.rekey_completed_ms = ms; + } + /// Store a completed rekey session. pub(crate) fn set_pending_session(&mut self, session: NoiseSession) { self.pending_new_session = Some(session); @@ -408,6 +422,12 @@ impl SessionEntry { self.session_start_ms = now_ms; self.rekey_state = None; self.rekey_initiator = false; + self.rekey_completed_ms = 0; + + // Reset MMP counters to avoid metric discontinuity + if let Some(mmp) = &mut self.mmp { + mmp.reset_for_rekey(); + } true } @@ -430,6 +450,11 @@ impl SessionEntry { self.session_start_ms = now_ms; self.rekey_state = None; self.rekey_initiator = false; + + // Reset MMP counters to avoid metric discontinuity + if let Some(mmp) = &mut self.mmp { + mmp.reset_for_rekey(); + } true } diff --git a/src/peer/active.rs b/src/peer/active.rs index d66fb54..a99622c 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -880,6 +880,11 @@ impl ActivePeer { self.rekey_in_progress = false; self.reset_replay_suppressed(); + // Reset MMP counters to avoid metric discontinuity + if let Some(mmp) = &mut self.mmp { + mmp.reset_for_rekey(); + } + self.previous_our_index } @@ -909,6 +914,11 @@ impl ActivePeer { self.rekey_in_progress = false; self.reset_replay_suppressed(); + // Reset MMP counters to avoid metric discontinuity + if let Some(mmp) = &mut self.mmp { + mmp.reset_for_rekey(); + } + self.previous_our_index } diff --git a/testing/static/scripts/rekey-test.sh b/testing/static/scripts/rekey-test.sh index 5f36b49..6e75061 100755 --- a/testing/static/scripts/rekey-test.sh +++ b/testing/static/scripts/rekey-test.sh @@ -63,9 +63,6 @@ FIRST_REKEY_WAIT=40 # > REKEY_AFTER_SECS, allow margin REKEY_SETTLE=5 # settle time after rekey for cutover to complete SECOND_REKEY_WAIT=40 # wait for second cycle -# Phase 3 allows transient failures during cutover; Phase 5 must be clean -MAX_PHASE3_FAILURES=4 # allow up to 4 pair failures during first rekey - TIMEOUT=5 PASSED=0 FAILED=0 @@ -199,20 +196,11 @@ assert_min_count "Rekey cutover complete (initiator), K-bit flipped" 1 "FMP reke phase_result "FMP rekey events" echo "" -# Verify connectivity after first rekey (allow transient cutover failures) +# Verify connectivity after first rekey (strict — no failures allowed) echo "Phase 3: Post-rekey connectivity (settling ${REKEY_SETTLE}s)" sleep "$REKEY_SETTLE" ping_all -if [ "$FAILED" -le "$MAX_PHASE3_FAILURES" ]; then - if [ "$FAILED" -gt 0 ]; then - echo " (transient failures within threshold: $FAILED <= $MAX_PHASE3_FAILURES)" - fi - TOTAL_PASSED=$((TOTAL_PASSED + PASSED)) - # Don't count transient failures toward total - echo " ✓ Post-first-rekey (all 20 pairs): $PASSED passed, $FAILED transient" -else - phase_result "Post-first-rekey (all 20 pairs)" -fi +phase_result "Post-first-rekey (all 20 pairs)" echo "" # ── Phase 4: Wait for second rekey cycle ────────────────────────────── @@ -247,6 +235,8 @@ assert_zero_count "MMP link teardown" "Spurious link teardowns" assert_zero_count "Excessive decrypt failures" \ "Excessive decrypt failure removals" assert_zero_count "Rekey msg2 processing failed" "Rekey msg2 failures" +assert_zero_count "Session AEAD decryption failed" \ + "FSP decryption failures during rekey" phase_result "Log analysis" echo ""