From cb5a32693e6c6be99cc09d6b5db176d190b5f590 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 12 Jul 2026 15:53:03 +0000 Subject: [PATCH 1/3] tree: remove redundant parent_switched metric counter parent_switched was incremented on the line immediately before parent_switches at every site and never independently, so the two counters were always identical. Drop parent_switched from TreeMetrics, its snapshot, TreeStatsSnapshot, the show_tree fixture, and the fipstop render, keeping parent_switches as the sole counter. --- src/bin/fipstop/ui/tree.rs | 4 ---- src/control/snapshots/show_tree.json | 1 - src/node/handlers/mmp.rs | 2 -- src/node/metrics.rs | 2 -- src/node/stats.rs | 1 - src/node/tree.rs | 4 ---- 6 files changed, 14 deletions(-) diff --git a/src/bin/fipstop/ui/tree.rs b/src/bin/fipstop/ui/tree.rs index 2d221f2..c8119c9 100644 --- a/src/bin/fipstop/ui/tree.rs +++ b/src/bin/fipstop/ui/tree.rs @@ -174,10 +174,6 @@ fn draw_stats(frame: &mut Frame, data: &serde_json::Value, scroll: u16, focused: &helpers::nested_u64(data, "stats", "sig_failed"), ), helpers::kv_line("Stale", &helpers::nested_u64(data, "stats", "stale")), - helpers::kv_line( - "Parent Switched", - &helpers::nested_u64(data, "stats", "parent_switched"), - ), helpers::kv_line( "Loop Detected", &helpers::nested_u64(data, "stats", "loop_detected"), diff --git a/src/control/snapshots/show_tree.json b/src/control/snapshots/show_tree.json index 148ab46..675d85a 100644 --- a/src/control/snapshots/show_tree.json +++ b/src/control/snapshots/show_tree.json @@ -24,7 +24,6 @@ "loop_detected": 0, "outbound_sign_failed": 0, "parent_losses": 0, - "parent_switched": 0, "parent_switches": 0, "rate_limited": 0, "received": 0, diff --git a/src/node/handlers/mmp.rs b/src/node/handlers/mmp.rs index 74b89f2..167c687 100644 --- a/src/node/handlers/mmp.rs +++ b/src/node/handlers/mmp.rs @@ -173,7 +173,6 @@ impl Node { self.coord_cache .invalidate_via_node(our_identity.node_addr()); self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( new_parent = %self.peer_display_name(&new_parent), @@ -205,7 +204,6 @@ impl Node { self.coord_cache .invalidate_other_roots(our_identity.node_addr()); self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( new_root = %self.tree_state.root(), diff --git a/src/node/metrics.rs b/src/node/metrics.rs index 84f5ffc..948cb83 100644 --- a/src/node/metrics.rs +++ b/src/node/metrics.rs @@ -317,7 +317,6 @@ pub struct TreeMetrics { pub stale: Counter, pub ancestry_invalid: Counter, pub accepted: Counter, - pub parent_switched: Counter, pub loop_detected: Counter, pub ancestry_changed: Counter, pub sent: Counter, @@ -351,7 +350,6 @@ impl TreeMetrics { stale: self.stale.get(), ancestry_invalid: self.ancestry_invalid.get(), accepted: self.accepted.get(), - parent_switched: self.parent_switched.get(), loop_detected: self.loop_detected.get(), ancestry_changed: self.ancestry_changed.get(), sent: self.sent.get(), diff --git a/src/node/stats.rs b/src/node/stats.rs index 2315ad9..5edfe5d 100644 --- a/src/node/stats.rs +++ b/src/node/stats.rs @@ -273,7 +273,6 @@ pub struct TreeStatsSnapshot { pub stale: u64, pub ancestry_invalid: u64, pub accepted: u64, - pub parent_switched: u64, pub loop_detected: u64, pub ancestry_changed: u64, pub sent: u64, diff --git a/src/node/tree.rs b/src/node/tree.rs index f95dfc4..c99231e 100644 --- a/src/node/tree.rs +++ b/src/node/tree.rs @@ -300,7 +300,6 @@ impl Node { .invalidate_via_node(our_identity.node_addr()); self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( @@ -338,7 +337,6 @@ impl Node { self.coord_cache .invalidate_other_roots(our_identity.node_addr()); self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( new_root = %self.tree_state.root(), @@ -524,7 +522,6 @@ impl Node { .invalidate_via_node(our_identity.node_addr()); self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( @@ -560,7 +557,6 @@ impl Node { self.coord_cache .invalidate_other_roots(our_identity.node_addr()); self.reset_discovery_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( new_root = %self.tree_state.root(), From 6011d233c143b73f6f28e7da90e4a05688e30cd1 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 12 Jul 2026 15:32:04 +0000 Subject: [PATCH 2/3] discovery: keep tighter path_mtu when applying a LookupResponse An originator handling a LookupResponse unconditionally overwrote the cached path_mtu_lookup entry, so a looser (larger) estimate in a later response could clobber a tighter value already learned from a reactive MtuExceeded or PathMtuNotification. Read-and-compare before writing and keep the minimum, so a looser discovery estimate no longer loosens the clamp. Add a regression test. --- src/node/handlers/discovery.rs | 37 ++++++++++++++++++++++---------- src/node/tests/discovery.rs | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/node/handlers/discovery.rs b/src/node/handlers/discovery.rs index e3d8c7b..cf688c5 100644 --- a/src/node/handlers/discovery.rs +++ b/src/node/handlers/discovery.rs @@ -240,17 +240,32 @@ impl Node { // map used by the TUN reader/writer at TCP MSS clamp time. let fips_addr = crate::FipsAddress::from_node_addr(&target); match self.path_mtu_lookup.write() { - Ok(mut map) => { - let prior = map.insert(fips_addr, path_mtu); - debug!( - target = %self.peer_display_name(&target), - fips_addr = %fips_addr, - path_mtu = path_mtu, - prior = ?prior, - map_len = map.len(), - "Wrote path_mtu_lookup from discovery LookupResponse" - ); - } + Ok(mut map) => match map.get(&fips_addr).copied() { + Some(existing) if existing <= path_mtu => { + // Keep the tighter learned value; never loosen the + // clamp. A reactive MtuExceeded or PathMtuNotification + // tighten takes precedence over a looser discovery + // estimate (cross-carrier keep-tighter). + debug!( + target = %self.peer_display_name(&target), + fips_addr = %fips_addr, + path_mtu = path_mtu, + existing = existing, + "LookupResponse: keeping tighter existing path_mtu_lookup value" + ); + } + other => { + map.insert(fips_addr, path_mtu); + debug!( + target = %self.peer_display_name(&target), + fips_addr = %fips_addr, + path_mtu = path_mtu, + prior = ?other, + map_len = map.len(), + "Wrote path_mtu_lookup from discovery LookupResponse" + ); + } + }, Err(e) => { warn!( target = %self.peer_display_name(&target), diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index 671e911..3503f72 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -912,6 +912,45 @@ async fn test_originator_stores_path_mtu_in_cache() { ); } +#[tokio::test] +async fn test_originator_lookup_response_keeps_tighter_path_mtu_lookup() { + // Regression: a LookupResponse carrying a looser (larger) path_mtu must + // NOT clobber a tighter (smaller) value already in path_mtu_lookup that a + // reactive MtuExceeded or PathMtuNotification learned. Cross-carrier + // keep-tighter: the clamp must never loosen. + let mut node = make_node(); + let from = make_node_addr(0xAA); + + let target_identity = Identity::generate(); + let target = *target_identity.node_addr(); + let root = make_node_addr(0xF0); + let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap(); + + node.register_identity(target, target_identity.pubkey_full()); + + // Pre-seed a tighter value, as if a reactive signal already narrowed it. + let target_fips = crate::FipsAddress::from_node_addr(&target); + node.path_mtu_lookup_insert(target_fips, 1280); + + let proof_data = LookupResponse::proof_bytes(800, &target, &coords); + let proof = target_identity.sign(&proof_data); + + let mut response = LookupResponse::new(800, target, coords.clone(), proof); + // Looser discovery estimate that must be rejected in favor of the tighter + // existing entry. + response.path_mtu = 1500; + + let payload = &response.encode()[1..]; + + node.handle_lookup_response(&from, payload).await; + + assert_eq!( + node.path_mtu_lookup_get(&target_fips), + Some(1280), + "LookupResponse must not loosen a tighter existing path_mtu_lookup value" + ); +} + // ============================================================================ // Open-Discovery Sweep — cache-injection unit test // ============================================================================ From 567e6a535e76d4d27a8810c45e49eb6506a1ca2b Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 12 Jul 2026 15:56:35 +0000 Subject: [PATCH 3/3] identity: reuse one shared secp256k1 context Every sign/verify/key-derive site built a fresh context via Secp256k1::new(), which allocates a Secp256k1 and runs randomization/blinding table setup on each call. Introduce one crate-wide LazyLock> and reuse it across the local, peer, and auth sites (and their tests). Behavior-neutral: identical secp256k1 API calls, only the context lifetime changes, and the shared All context still performs the standard construction-time blinding. --- src/identity/auth.rs | 6 +++--- src/identity/local.rs | 11 ++++------- src/identity/mod.rs | 12 ++++++++++++ src/identity/peer.rs | 6 +++--- src/identity/tests.rs | 9 ++++----- 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/identity/auth.rs b/src/identity/auth.rs index 6e383b7..aedf42d 100644 --- a/src/identity/auth.rs +++ b/src/identity/auth.rs @@ -1,7 +1,7 @@ //! Authentication challenge-response protocol. use rand::Rng; -use secp256k1::{Secp256k1, XOnlyPublicKey}; +use secp256k1::XOnlyPublicKey; use sha2::{Digest, Sha256}; use super::{IdentityError, NodeAddr}; @@ -34,9 +34,9 @@ impl AuthChallenge { /// Verify a response to this challenge. pub fn verify(&self, response: &AuthResponse) -> Result { let digest = auth_challenge_digest(&self.0, response.timestamp); - let secp = Secp256k1::new(); - secp.verify_schnorr(&response.signature, &digest, &response.pubkey) + super::SECP + .verify_schnorr(&response.signature, &digest, &response.pubkey) .map_err(|_| IdentityError::SignatureVerificationFailed)?; Ok(NodeAddr::from_pubkey(&response.pubkey)) diff --git a/src/identity/local.rs b/src/identity/local.rs index f79e7cd..26db8e1 100644 --- a/src/identity/local.rs +++ b/src/identity/local.rs @@ -1,6 +1,6 @@ //! Local node identity with signing capability. -use secp256k1::{Keypair, PublicKey, Secp256k1, SecretKey, XOnlyPublicKey}; +use secp256k1::{Keypair, PublicKey, SecretKey, XOnlyPublicKey}; use std::fmt; use super::auth::{AuthResponse, auth_challenge_digest}; @@ -42,8 +42,7 @@ impl Identity { /// Create an identity from a secret key. pub fn from_secret_key(secret_key: SecretKey) -> Self { - let secp = Secp256k1::new(); - let keypair = Keypair::from_secret_key(&secp, &secret_key); + let keypair = Keypair::from_secret_key(&super::SECP, &secret_key); Self::from_keypair(keypair) } @@ -93,9 +92,8 @@ impl Identity { /// Sign arbitrary data with this identity's secret key. pub fn sign(&self, data: &[u8]) -> secp256k1::schnorr::Signature { - let secp = Secp256k1::new(); let digest = sha256(data); - secp.sign_schnorr(&digest, &self.keypair) + super::SECP.sign_schnorr(&digest, &self.keypair) } /// Create an authentication response for a challenge. @@ -103,8 +101,7 @@ impl Identity { /// The response signs: SHA256("fips-auth-v1" || challenge || timestamp) pub fn sign_challenge(&self, challenge: &[u8; 32], timestamp: u64) -> AuthResponse { let digest = auth_challenge_digest(challenge, timestamp); - let secp = Secp256k1::new(); - let signature = secp.sign_schnorr(&digest, &self.keypair); + let signature = super::SECP.sign_schnorr(&digest, &self.keypair); AuthResponse { pubkey: self.pubkey(), timestamp, diff --git a/src/identity/mod.rs b/src/identity/mod.rs index e5f7899..6093427 100644 --- a/src/identity/mod.rs +++ b/src/identity/mod.rs @@ -11,6 +11,9 @@ mod local; mod node_addr; mod peer; +use std::sync::LazyLock; + +use secp256k1::{All, Secp256k1}; use sha2::{Digest, Sha256}; use thiserror::Error; @@ -21,6 +24,15 @@ pub use local::Identity; pub use node_addr::NodeAddr; pub use peer::PeerIdentity; +/// Shared secp256k1 context reused across all identity operations. +/// +/// `Secp256k1::new()` allocates a `Secp256k1` and runs randomization / +/// blinding table setup; it is designed to be created once and reused rather +/// than rebuilt per sign / verify / key-derive call. This single `All` context +/// serves both signing and verification across the identity module and still +/// performs the standard construction-time blinding. +pub(crate) static SECP: LazyLock> = LazyLock::new(Secp256k1::new); + /// FIPS address prefix (IPv6 ULA range). pub const FIPS_ADDRESS_PREFIX: u8 = 0xfd; diff --git a/src/identity/peer.rs b/src/identity/peer.rs index 06f65f7..b997058 100644 --- a/src/identity/peer.rs +++ b/src/identity/peer.rs @@ -1,6 +1,6 @@ //! Remote peer identity (public key only, no signing capability). -use secp256k1::{Parity, PublicKey, Secp256k1, XOnlyPublicKey}; +use secp256k1::{Parity, PublicKey, XOnlyPublicKey}; use std::fmt; use super::encoding::{decode_npub, encode_npub}; @@ -107,9 +107,9 @@ impl PeerIdentity { /// Verify a signature from this peer. pub fn verify(&self, data: &[u8], signature: &secp256k1::schnorr::Signature) -> bool { - let secp = Secp256k1::new(); let digest = sha256(data); - secp.verify_schnorr(signature, &digest, &self.pubkey) + super::SECP + .verify_schnorr(signature, &digest, &self.pubkey) .is_ok() } } diff --git a/src/identity/tests.rs b/src/identity/tests.rs index 5db8d8d..546c7ff 100644 --- a/src/identity/tests.rs +++ b/src/identity/tests.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use std::net::Ipv6Addr; -use secp256k1::{Keypair, Secp256k1, SecretKey}; +use secp256k1::{Keypair, SecretKey}; use super::*; @@ -161,10 +161,10 @@ fn test_identity_sign() { let sig = identity.sign(data); // Verify the signature manually - let secp = secp256k1::Secp256k1::new(); let digest = super::sha256(data); assert!( - secp.verify_schnorr(&sig, &digest, &identity.pubkey()) + super::SECP + .verify_schnorr(&sig, &digest, &identity.pubkey()) .is_ok() ); } @@ -580,13 +580,12 @@ fn test_peer_identity_pubkey_full_even_parity_fallback() { #[test] fn test_peer_identity_pubkey_full_preserved_parity() { // Create two identities and find one with odd parity to make this test meaningful - let secp = Secp256k1::new(); let secret_bytes: [u8; 32] = [ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, ]; - let keypair = Keypair::from_seckey_slice(&secp, &secret_bytes).unwrap(); + let keypair = Keypair::from_seckey_slice(&super::SECP, &secret_bytes).unwrap(); let full_pubkey = keypair.public_key(); let peer = PeerIdentity::from_pubkey_full(full_pubkey);