diff --git a/CHANGELOG.md b/CHANGELOG.md index e158d41..e6d60df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Apply ±15s symmetric jitter per session to the FMP and FSP rekey + timer trigger. Eliminates the steady-state dual-initiation race + in symmetric-start meshes; previously the smaller-NodeAddr + tie-breaker resolved correctness after every cycle's collision. + `node.rekey.after_secs` becomes the nominal interval rather than + a floor; mean is preserved. - Rekey integration test (`testing/static/scripts/rekey-test.sh`): bumped Phase 1 baseline-convergence headroom from 36s to 60s. Eliminates the intermittent GitHub-runner Phase 1 timeout that diff --git a/src/node/handlers/rekey.rs b/src/node/handlers/rekey.rs index 44f4c8b..35cb3fe 100644 --- a/src/node/handlers/rekey.rs +++ b/src/node/handlers/rekey.rs @@ -74,7 +74,11 @@ impl Node { .map(|s| s.current_send_counter()) .unwrap_or(0); - if elapsed >= rekey_after_secs || counter >= rekey_after_messages { + // Apply per-session symmetric jitter to desynchronize + // dual-initiation in symmetric-start meshes. + let effective_after_secs = + rekey_after_secs.saturating_add_signed(peer.rekey_jitter_secs()); + if elapsed >= effective_after_secs || counter >= rekey_after_messages { peers_to_rekey.push(*node_addr); } } @@ -323,7 +327,11 @@ impl Node { let elapsed_secs = now_ms.saturating_sub(entry.session_start_ms()) / 1000; let counter = entry.send_counter(); - if elapsed_secs >= rekey_after_secs || counter >= rekey_after_messages { + // Apply per-session symmetric jitter to desynchronize + // dual-initiation in symmetric-start meshes. + let effective_after_secs = + rekey_after_secs.saturating_add_signed(entry.rekey_jitter_secs()); + if elapsed_secs >= effective_after_secs || counter >= rekey_after_messages { sessions_to_rekey.push(*node_addr); } } diff --git a/src/node/mod.rs b/src/node/mod.rs index 284db6e..cb64298 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -24,6 +24,13 @@ pub(crate) mod wire; use self::discovery_rate_limit::{DiscoveryBackoff, DiscoveryForwardRateLimiter}; use self::rate_limit::HandshakeRateLimiter; use self::routing_error_rate_limit::RoutingErrorRateLimiter; + +/// Half-range of the symmetric jitter applied to the per-session rekey timer. +/// Each session draws an offset uniformly from `[-REKEY_JITTER_SECS, +/// +REKEY_JITTER_SECS]` seconds at construction. Desynchronizes +/// dual-initiation in symmetric-start meshes; the configured +/// `node.rekey.after_secs` remains the nominal interval (mean preserved). +pub(crate) const REKEY_JITTER_SECS: i64 = 15; use self::wire::{ FLAG_CE, FLAG_KEY_EPOCH, FLAG_SP, build_encrypted, build_established_header, prepend_inner_header, diff --git a/src/node/session.rs b/src/node/session.rs index e596cf2..d78e701 100644 --- a/src/node/session.rs +++ b/src/node/session.rs @@ -10,9 +10,16 @@ use std::time::Instant; use crate::NodeAddr; use crate::config::SessionMmpConfig; use crate::mmp::MmpSessionState; +use crate::node::REKEY_JITTER_SECS; use crate::noise::{HandshakeState, NoiseSession}; +use rand::RngExt; use secp256k1::PublicKey; +/// Draw a fresh per-session rekey jitter from `[-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]`. +fn draw_rekey_jitter() -> i64 { + rand::rng().random_range(-REKEY_JITTER_SECS..=REKEY_JITTER_SECS) +} + /// State machine for an end-to-end session. /// /// `Established` is intentionally larger than the handshake variants: @@ -121,6 +128,12 @@ pub(crate) struct SessionEntry { /// 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, + /// Per-session symmetric jitter applied to the rekey timer trigger. + /// Drawn once at construction (and at each cutover) uniformly from + /// `[-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]`. Desynchronizes + /// dual-initiation in symmetric-start meshes; mean interval is + /// preserved. + rekey_jitter_secs: i64, } impl SessionEntry { @@ -157,6 +170,7 @@ impl SessionEntry { rekey_initiator: false, last_peer_rekey_ms: 0, rekey_completed_ms: 0, + rekey_jitter_secs: draw_rekey_jitter(), } } @@ -398,6 +412,16 @@ impl SessionEntry { self.rekey_completed_ms } + /// Per-session symmetric rekey-timer jitter offset (seconds). + /// + /// Drawn at session construction and at each rekey cutover; uniform + /// over `[-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]`. Callers add this + /// to the configured `node.rekey.after_secs` to obtain the effective + /// trigger interval for this session. + pub(crate) fn rekey_jitter_secs(&self) -> i64 { + self.rekey_jitter_secs + } + /// 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; @@ -443,6 +467,7 @@ impl SessionEntry { self.rekey_state = None; self.rekey_initiator = false; self.rekey_completed_ms = 0; + self.rekey_jitter_secs = draw_rekey_jitter(); // Reset MMP counters to avoid metric discontinuity let now = Instant::now(); @@ -471,6 +496,7 @@ impl SessionEntry { self.session_start_ms = now_ms; self.rekey_state = None; self.rekey_initiator = false; + self.rekey_jitter_secs = draw_rekey_jitter(); // Reset MMP counters to avoid metric discontinuity let now = Instant::now(); diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index decab10..40907aa 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -67,6 +67,68 @@ fn test_session_entry_new_initiating() { assert_eq!(entry.last_activity(), 1000); } +#[test] +fn test_session_entry_rekey_jitter_in_range() { + use crate::node::REKEY_JITTER_SECS; + use crate::noise::HandshakeState; + + // Every newly constructed SessionEntry's jitter must lie in the + // symmetric range [-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]. + for _ in 0..100 { + let identity_a = Identity::generate(); + let identity_b = Identity::generate(); + let handshake = + HandshakeState::new_initiator(identity_a.keypair(), identity_b.pubkey_full()); + let entry = crate::node::session::SessionEntry::new( + *identity_b.node_addr(), + identity_b.pubkey_full(), + EndToEndState::Initiating(handshake), + 1000, + true, + ); + let j = entry.rekey_jitter_secs(); + assert!( + (-REKEY_JITTER_SECS..=REKEY_JITTER_SECS).contains(&j), + "jitter {} outside [-{}, +{}]", + j, + REKEY_JITTER_SECS, + REKEY_JITTER_SECS + ); + } +} + +#[test] +fn test_session_entry_rekey_jitter_mean_near_zero() { + use crate::noise::HandshakeState; + + // Sanity check that the distribution is roughly symmetric and not + // stuck at one extreme. With N=200 draws from a uniform ~30-second + // range, the empirical mean should be well under 5 in absolute value. + let mut sum: i64 = 0; + let n: i64 = 200; + for _ in 0..n { + let identity_a = Identity::generate(); + let identity_b = Identity::generate(); + let handshake = + HandshakeState::new_initiator(identity_a.keypair(), identity_b.pubkey_full()); + let entry = crate::node::session::SessionEntry::new( + *identity_b.node_addr(), + identity_b.pubkey_full(), + EndToEndState::Initiating(handshake), + 1000, + true, + ); + sum += entry.rekey_jitter_secs(); + } + let mean = sum / n; + assert!( + mean.abs() < 5, + "empirical mean {} not within 5 of 0 over {} samples", + mean, + n + ); +} + #[test] fn test_session_entry_touch() { use crate::noise::HandshakeState; diff --git a/src/peer/active.rs b/src/peer/active.rs index 942c278..2ef0c2d 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -5,15 +5,22 @@ use crate::bloom::BloomFilter; use crate::mmp::{MmpConfig, MmpPeerState}; +use crate::node::REKEY_JITTER_SECS; use crate::noise::{HandshakeState as NoiseHandshakeState, NoiseError, NoiseSession}; use crate::transport::{LinkId, LinkStats, TransportAddr, TransportId}; use crate::tree::{ParentDeclaration, TreeCoordinate}; use crate::utils::index::SessionIndex; use crate::{FipsAddress, NodeAddr, PeerIdentity}; +use rand::RngExt; use secp256k1::XOnlyPublicKey; use std::fmt; use std::time::Instant; +/// Draw a fresh per-session rekey jitter from `[-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]`. +fn draw_rekey_jitter() -> i64 { + rand::rng().random_range(-REKEY_JITTER_SECS..=REKEY_JITTER_SECS) +} + /// Connectivity state for an active peer. /// /// This is simpler than the full PeerState since authentication is complete. @@ -156,6 +163,12 @@ pub struct ActivePeer { // === Rekey (Key Rotation) === /// When the current Noise session was established (for rekey timer). session_established_at: Instant, + /// Per-session symmetric jitter applied to the rekey timer trigger. + /// Drawn once at construction (and at each cutover) uniformly from + /// `[-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]`. Desynchronizes + /// dual-initiation in symmetric-start meshes; mean interval is + /// preserved. + rekey_jitter_secs: i64, /// Current K-bit epoch value (alternates each rekey). current_k_bit: bool, /// Previous session kept alive during drain window after cutover. @@ -220,6 +233,7 @@ impl ActivePeer { replay_suppressed_count: 0, consecutive_decrypt_failures: 0, session_established_at: now, + rekey_jitter_secs: draw_rekey_jitter(), current_k_bit: false, previous_session: None, previous_our_index: None, @@ -300,6 +314,7 @@ impl ActivePeer { replay_suppressed_count: 0, consecutive_decrypt_failures: 0, session_established_at: now, + rekey_jitter_secs: draw_rekey_jitter(), current_k_bit: false, previous_session: None, previous_our_index: None, @@ -783,6 +798,16 @@ impl ActivePeer { self.session_established_at } + /// Per-session symmetric rekey-timer jitter offset (seconds). + /// + /// Drawn at session construction and at each rekey cutover; uniform + /// over `[-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]`. Callers add this + /// to the configured `node.rekey.after_secs` to obtain the effective + /// trigger interval for this session. + pub fn rekey_jitter_secs(&self) -> i64 { + self.rekey_jitter_secs + } + /// Current K-bit epoch value. pub fn current_k_bit(&self) -> bool { self.current_k_bit @@ -887,6 +912,7 @@ impl ActivePeer { self.session_established_at = Instant::now(); self.session_start = Instant::now(); self.rekey_in_progress = false; + self.rekey_jitter_secs = draw_rekey_jitter(); self.reset_replay_suppressed(); // Reset MMP counters to avoid metric discontinuity @@ -922,6 +948,7 @@ impl ActivePeer { self.session_established_at = Instant::now(); self.session_start = Instant::now(); self.rekey_in_progress = false; + self.rekey_jitter_secs = draw_rekey_jitter(); self.reset_replay_suppressed(); // Reset MMP counters to avoid metric discontinuity @@ -1252,4 +1279,44 @@ mod tests { assert_eq!(peer.increment_decrypt_failures(), 1); assert_eq!(peer.consecutive_decrypt_failures(), 1); } + + #[test] + fn test_rekey_jitter_in_range() { + // Every newly constructed peer's jitter must lie in the + // symmetric range [-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]. + for _ in 0..100 { + let identity = make_peer_identity(); + let peer = ActivePeer::new(identity, LinkId::new(1), 1000); + let j = peer.rekey_jitter_secs(); + assert!( + (-REKEY_JITTER_SECS..=REKEY_JITTER_SECS).contains(&j), + "jitter {} outside [-{}, +{}]", + j, + REKEY_JITTER_SECS, + REKEY_JITTER_SECS + ); + } + } + + #[test] + fn test_rekey_jitter_mean_near_zero() { + // Sanity check that the distribution is roughly symmetric and + // not stuck at one extreme. With N=200 draws from a uniform + // ~30-second-wide range, the empirical mean should be well + // under 5 in absolute value with overwhelming probability. + let mut sum: i64 = 0; + let n: i64 = 200; + for _ in 0..n { + let identity = make_peer_identity(); + let peer = ActivePeer::new(identity, LinkId::new(1), 1000); + sum += peer.rekey_jitter_secs(); + } + let mean = sum / n; + assert!( + mean.abs() < 5, + "empirical mean {} not within 5 of 0 over {} samples", + mean, + n + ); + } }