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/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); diff --git a/src/node/handlers/lookup.rs b/src/node/handlers/lookup.rs index 64f1dad..41f7c58 100644 --- a/src/node/handlers/lookup.rs +++ b/src/node/handlers/lookup.rs @@ -283,17 +283,33 @@ 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/handlers/mmp.rs b/src/node/handlers/mmp.rs index 393603f..dc175ca 100644 --- a/src/node/handlers/mmp.rs +++ b/src/node/handlers/mmp.rs @@ -233,7 +233,6 @@ impl Node { self.coord_cache .invalidate_via_node(our_identity.node_addr()); self.reset_lookup_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( new_parent = %self.peer_display_name(&new_parent), @@ -267,7 +266,6 @@ impl Node { self.coord_cache .invalidate_other_roots(our_identity.node_addr()); self.reset_lookup_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 d5b9f61..e27d5a6 100644 --- a/src/node/metrics.rs +++ b/src/node/metrics.rs @@ -284,7 +284,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, @@ -318,7 +317,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 93e92f8..3f9955d 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/tests/discovery.rs b/src/node/tests/discovery.rs index 8ad42ba..52be3b3 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -914,6 +914,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 // ============================================================================ diff --git a/src/node/tests/spanning_tree.rs b/src/node/tests/spanning_tree.rs index 471f630..85de6ee 100644 --- a/src/node/tests/spanning_tree.rs +++ b/src/node/tests/spanning_tree.rs @@ -1228,7 +1228,7 @@ async fn test_tree_announce_self_root_promotion() { let announce = TreeAnnounce::new(decl, TreeCoordinate::from_addrs(vec![l_addr]).unwrap()); let encoded = announce.encode().unwrap(); - let switched_before = nodes[p_idx].node.metrics().tree.parent_switched.get(); + let switched_before = nodes[p_idx].node.metrics().tree.parent_switches.get(); nodes[p_idx] .node .handle_tree_announce(&l_addr, &encoded[1..]) @@ -1240,7 +1240,7 @@ async fn test_tree_announce_self_root_promotion() { ); assert_eq!(*nodes[p_idx].node.tree_state().root(), p_addr); assert_eq!( - nodes[p_idx].node.metrics().tree.parent_switched.get(), + nodes[p_idx].node.metrics().tree.parent_switches.get(), switched_before + 1 ); diff --git a/src/node/tree.rs b/src/node/tree.rs index 6d04dad..cbc2b65 100644 --- a/src/node/tree.rs +++ b/src/node/tree.rs @@ -373,7 +373,6 @@ impl Node { .invalidate_via_node(our_identity.node_addr()); self.reset_lookup_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( @@ -418,7 +417,6 @@ impl Node { self.coord_cache .invalidate_other_roots(our_identity.node_addr()); self.reset_lookup_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( new_root = %self.tree_state.root(), @@ -627,7 +625,6 @@ impl Node { .invalidate_via_node(our_identity.node_addr()); self.reset_lookup_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( @@ -670,7 +667,6 @@ impl Node { self.coord_cache .invalidate_other_roots(our_identity.node_addr()); self.reset_lookup_backoff(); - self.metrics().tree.parent_switched.inc(); self.metrics().tree.parent_switches.inc(); info!( new_root = %self.tree_state.root(), diff --git a/src/proto/stp/core.rs b/src/proto/stp/core.rs index 669d000..8181e94 100644 --- a/src/proto/stp/core.rs +++ b/src/proto/stp/core.rs @@ -35,13 +35,13 @@ pub(crate) enum TreeDecision { /// `evaluate_parent` picked a (different) parent: switch to it. Shell: /// `set_parent(new_parent, new_seq, ts)` -> flap_dampened; `recompute_coords`; /// sign; `invalidate_via_node`; reset_backoff; - /// metrics(parent_switched/parent_switches[, flap_dampened]); send_all; + /// metrics(parent_switches[, flap_dampened]); send_all; /// bloom.mark_all_updates_needed. Switch { new_parent: NodeAddr, new_seq: u64 }, /// `evaluate_parent` None && !is_root && should_be_root: self-promote to root. /// Shell: `become_root`; sign; `invalidate_other_roots`; reset_backoff; - /// metrics(parent_switched/parent_switches); send_all; + /// metrics(parent_switches); send_all; /// bloom.mark_all_updates_needed. SelfRoot,