diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 3bd6230..16dc005 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -835,6 +835,31 @@ impl Node { return; } + // The rekey must come from the peer the session was established + // with. Compare x-only keys: the stored key's parity may be a + // synthesized even parity (npubs carry no parity), while the + // handshake learns the true point. + let rekey_pubkey = match handshake.remote_static() { + Some(pk) => *pk, + None => { + debug!("No remote static key after processing rekey XK msg3"); + entry.abandon_rekey(); + self.sessions.insert(*src_addr, entry); + return; + } + }; + if rekey_pubkey.x_only_public_key().0 != entry.remote_pubkey().x_only_public_key().0 { + warn!( + src = %self.peer_display_name(src_addr), + "FSP rekey: initiator static key differs from the established peer key" + ); + entry.abandon_rekey(); + self.sessions.insert(*src_addr, entry); + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::RekeyKeyMismatch)); + return; + } + // Complete the handshake → store as pending new session let session = match handshake.into_session() { Ok(s) => s, @@ -884,6 +909,21 @@ impl Node { } }; + // The claimed source address must be derivable from the key we just + // authenticated, or the peer is opening a session under another + // node's address. + let derived_addr = NodeAddr::from_pubkey(&remote_pubkey.x_only_public_key().0); + if derived_addr != *src_addr { + warn!( + src = %self.peer_display_name(src_addr), + derived = %derived_addr, + "SessionMsg3 source address does not match the authenticated static key" + ); + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::AddrMismatch)); + return; // Entry was already removed + } + // Register the initiator's identity for future TUN → session routing self.register_identity(*src_addr, remote_pubkey); diff --git a/src/node/reject.rs b/src/node/reject.rs index d2a44cf..89451f3 100644 --- a/src/node/reject.rs +++ b/src/node/reject.rs @@ -177,7 +177,8 @@ pub enum HandshakeReject { /// FSP session rejection reasons. /// /// `UnknownSession` and `BadState` cover the session unknown-session -/// and state-machine cluster. +/// and state-machine cluster. `AddrMismatch` and `RekeyKeyMismatch` +/// cover the peer-identity binding checks on the XK msg3 receive path. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[non_exhaustive] pub enum SessionReject { @@ -196,6 +197,18 @@ pub enum SessionReject { /// is not `AwaitingMsg3`. Tracked via /// [`SessionStats::bad_state`](crate::node::stats::SessionStats). BadState, + /// Inbound XK msg3 completed the handshake, but the initiator's + /// static key does not derive the source address the datagram + /// claimed — the peer is opening a session under another node's + /// address. Tracked via + /// [`SessionStats::addr_mismatch`](crate::node::stats::SessionStats). + AddrMismatch, + /// Inbound XK msg3 completed a responder-side rekey, but the + /// initiator's static key differs from the key the session was + /// established with — the rekey is not from the established peer. + /// Tracked via + /// [`SessionStats::rekey_key_mismatch`](crate::node::stats::SessionStats). + RekeyKeyMismatch, } /// MMP rejection reasons. diff --git a/src/node/stats.rs b/src/node/stats.rs index 5edfe5d..5d84ce8 100644 --- a/src/node/stats.rs +++ b/src/node/stats.rs @@ -32,6 +32,14 @@ pub struct SessionStats { /// before Established; SessionAck outside Initiating; SessionMsg3 /// outside AwaitingMsg3). pub bad_state: u64, + /// Inbound XK msg3 whose initiator static key does not derive the + /// source address the datagram claimed. The half-open session is + /// dropped and no identity is registered. + pub addr_mismatch: u64, + /// Inbound rekey XK msg3 whose initiator static key differs from + /// the key the session was established with. The rekey is + /// abandoned and the existing session is left intact. + pub rekey_key_mismatch: u64, } impl SessionStats { @@ -39,6 +47,8 @@ impl SessionStats { SessionStatsSnapshot { unknown_session: self.unknown_session, bad_state: self.bad_state, + addr_mismatch: self.addr_mismatch, + rekey_key_mismatch: self.rekey_key_mismatch, } } @@ -46,6 +56,8 @@ impl SessionStats { match reason { SessionReject::UnknownSession => self.unknown_session += 1, SessionReject::BadState => self.bad_state += 1, + SessionReject::AddrMismatch => self.addr_mismatch += 1, + SessionReject::RekeyKeyMismatch => self.rekey_key_mismatch += 1, } } } @@ -303,6 +315,8 @@ pub struct BloomStatsSnapshot { pub struct SessionStatsSnapshot { pub unknown_session: u64, pub bad_state: u64, + pub addr_mismatch: u64, + pub rekey_key_mismatch: u64, } #[derive(Clone, Debug, Default, Serialize)] @@ -359,6 +373,35 @@ mod tests { assert_eq!(stats.unknown_session, 0); } + #[test] + fn session_stats_record_reject_addr_mismatch() { + let mut stats = SessionStats::default(); + stats.record_reject(SessionReject::AddrMismatch); + stats.record_reject(SessionReject::AddrMismatch); + assert_eq!(stats.addr_mismatch, 2); + assert_eq!(stats.rekey_key_mismatch, 0); + assert_eq!(stats.unknown_session, 0); + } + + #[test] + fn session_stats_record_reject_rekey_key_mismatch() { + let mut stats = SessionStats::default(); + stats.record_reject(SessionReject::RekeyKeyMismatch); + assert_eq!(stats.rekey_key_mismatch, 1); + assert_eq!(stats.addr_mismatch, 0); + } + + #[test] + fn session_stats_snapshot_carries_identity_binding_counters() { + let mut stats = SessionStats::default(); + stats.record_reject(SessionReject::AddrMismatch); + stats.record_reject(SessionReject::RekeyKeyMismatch); + stats.record_reject(SessionReject::RekeyKeyMismatch); + let snap = stats.snapshot(); + assert_eq!(snap.addr_mismatch, 1); + assert_eq!(snap.rekey_key_mismatch, 2); + } + #[test] fn node_stats_record_reject_dispatches_to_session() { let mut stats = NodeStats::new(); diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 25d12c5..5f5b1cb 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -6,7 +6,7 @@ use crate::node::tests::spanning_tree::{ TestNode, cleanup_nodes, generate_random_edges, lock_large_network_test, process_available_packets, run_tree_test, run_tree_test_with_mtus, verify_tree_convergence, }; -use crate::protocol::{SessionAck, SessionDatagram}; +use crate::protocol::{SessionAck, SessionDatagram, SessionMsg3}; /// Populate all nodes' coordinate caches with each other's coords. /// @@ -2391,3 +2391,226 @@ fn test_handle_path_mtu_notification_no_session_no_op() { "PathMtuNotification with no session must not touch path_mtu_lookup" ); } + +// ============================================================================ +// Session identity binding: XK msg3 source address / static key +// ============================================================================ + +/// Helper: drive a full XK exchange against `responder_identity` and return +/// the responder's half-completed handshake plus the initiator's msg3. +/// +/// The msg3 is cryptographically valid for the responder and carries +/// `initiator_identity`'s static key, which is exactly the shape of the +/// defect: a peer that completes a real handshake while the datagram claims +/// somebody else's source address. +fn drive_xk_to_msg3( + initiator_identity: &Identity, + responder_identity: &Identity, +) -> (crate::noise::HandshakeState, Vec) { + use crate::noise::HandshakeState; + + let mut initiator = HandshakeState::new_xk_initiator( + initiator_identity.keypair(), + responder_identity.pubkey_full(), + ); + let mut responder = HandshakeState::new_xk_responder(responder_identity.keypair()); + + let mut init_epoch = [0u8; 8]; + rand::Rng::fill_bytes(&mut rand::rng(), &mut init_epoch); + initiator.set_local_epoch(init_epoch); + let mut resp_epoch = [0u8; 8]; + rand::Rng::fill_bytes(&mut rand::rng(), &mut resp_epoch); + responder.set_local_epoch(resp_epoch); + + let msg1 = initiator.write_xk_message_1().unwrap(); + responder.read_xk_message_1(&msg1).unwrap(); + let msg2 = responder.write_xk_message_2().unwrap(); + initiator.read_xk_message_2(&msg2).unwrap(); + let msg3 = initiator.write_xk_message_3().unwrap(); + + (responder, msg3) +} + +/// Helper: generate an identity whose full public key has odd parity. +fn generate_odd_parity_identity() -> Identity { + loop { + let id = Identity::generate(); + if id.pubkey_full().serialize()[0] == 0x03 { + return id; + } + } +} + +#[tokio::test] +async fn test_session_msg3_rejects_spoofed_source_address() { + let mut node = make_node(); + let attacker = Identity::generate(); + let victim = Identity::generate(); + let victim_addr = *victim.node_addr(); + + let (responder, msg3) = drive_xk_to_msg3(&attacker, node.identity()); + + // Half-open session recorded under the victim's address, as + // handle_session_setup would have done from the claimed source. + let entry = crate::node::session::SessionEntry::new( + victim_addr, + node.identity().pubkey_full(), + EndToEndState::AwaitingMsg3(responder), + 1000, + false, + ); + node.sessions.insert(victim_addr, entry); + + node.handle_session_payload(&victim_addr, &SessionMsg3::new(msg3).encode(), 1280, false) + .await; + + assert_eq!( + node.session_count(), + 0, + "session must not be installed under an address the peer's key does not derive" + ); + assert_eq!( + node.identity_cache_len(), + 0, + "identity cache must not be poisoned with the spoofed address" + ); + assert_eq!(node.stats().session.addr_mismatch, 1); +} + +#[tokio::test] +async fn test_session_msg3_accepts_matching_source_address() { + let mut node = make_node(); + let peer = Identity::generate(); + let peer_addr = *peer.node_addr(); + + let (responder, msg3) = drive_xk_to_msg3(&peer, node.identity()); + + let entry = crate::node::session::SessionEntry::new( + peer_addr, + node.identity().pubkey_full(), + EndToEndState::AwaitingMsg3(responder), + 1000, + false, + ); + node.sessions.insert(peer_addr, entry); + + node.handle_session_payload(&peer_addr, &SessionMsg3::new(msg3).encode(), 1280, false) + .await; + + assert!( + node.sessions + .get(&peer_addr) + .is_some_and(|e| e.is_established()), + "an honest initiator using its own address must still establish" + ); + assert_eq!(node.identity_cache_len(), 1); + assert_eq!(node.stats().session.addr_mismatch, 0); +} + +#[tokio::test] +async fn test_rekey_msg3_rejects_different_static_key() { + let mut node = make_node(); + let legit = Identity::generate(); + let attacker = Identity::generate(); + let peer_addr = *legit.node_addr(); + + let session = make_noise_session(node.identity(), &legit); + let mut entry = crate::node::session::SessionEntry::new( + peer_addr, + legit.pubkey_full(), + EndToEndState::Established(session), + 1000, + true, + ); + entry.mark_established(1000); + + // Responder-side rekey armed, but driven by a different identity. + let (responder, msg3) = drive_xk_to_msg3(&attacker, node.identity()); + entry.set_rekey_state(responder, false); + node.sessions.insert(peer_addr, entry); + + node.handle_session_payload(&peer_addr, &SessionMsg3::new(msg3).encode(), 1280, false) + .await; + + let entry = node + .sessions + .get(&peer_addr) + .expect("existing session must survive a spoofed rekey"); + assert!(entry.is_established()); + assert!( + entry.pending_new_session().is_none(), + "a rekey from a different static key must not become the pending session" + ); + assert!(!entry.has_rekey_in_progress()); + assert_eq!(node.stats().session.rekey_key_mismatch, 1); +} + +#[tokio::test] +async fn test_rekey_msg3_accepts_established_peer_key() { + let mut node = make_node(); + let legit = Identity::generate(); + let peer_addr = *legit.node_addr(); + + let session = make_noise_session(node.identity(), &legit); + let mut entry = crate::node::session::SessionEntry::new( + peer_addr, + legit.pubkey_full(), + EndToEndState::Established(session), + 1000, + true, + ); + entry.mark_established(1000); + + let (responder, msg3) = drive_xk_to_msg3(&legit, node.identity()); + entry.set_rekey_state(responder, false); + node.sessions.insert(peer_addr, entry); + + node.handle_session_payload(&peer_addr, &SessionMsg3::new(msg3).encode(), 1280, false) + .await; + + let entry = node.sessions.get(&peer_addr).expect("session present"); + assert!(entry.pending_new_session().is_some()); + assert_eq!(node.stats().session.rekey_key_mismatch, 0); +} + +#[tokio::test] +async fn test_rekey_msg3_accepts_odd_parity_peer_stored_as_even() { + let mut node = make_node(); + let legit = generate_odd_parity_identity(); + let peer_addr = *legit.node_addr(); + + // The stored key is the even-parity synthesis an npub-sourced peer + // identity produces; the peer's real key has odd parity. This must + // still be accepted, or every peer-initiated rekey against roughly + // half of all peers would be rejected. + let stored_pubkey = crate::identity::PeerIdentity::from_pubkey(legit.pubkey()).pubkey_full(); + assert_ne!( + stored_pubkey, + legit.pubkey_full(), + "test fixture must actually differ in parity" + ); + + let session = make_noise_session(node.identity(), &legit); + let mut entry = crate::node::session::SessionEntry::new( + peer_addr, + stored_pubkey, + EndToEndState::Established(session), + 1000, + true, + ); + entry.mark_established(1000); + + let (responder, msg3) = drive_xk_to_msg3(&legit, node.identity()); + entry.set_rekey_state(responder, false); + node.sessions.insert(peer_addr, entry); + + node.handle_session_payload(&peer_addr, &SessionMsg3::new(msg3).encode(), 1280, false) + .await; + + let entry = node.sessions.get(&peer_addr).expect("session present"); + assert!( + entry.pending_new_session().is_some(), + "a parity-normalized stored key must not reject a legitimate rekey" + ); + assert_eq!(node.stats().session.rekey_key_mismatch, 0); +}