mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
Merge branch 'maint'
Carries up the FSP session address-to-key binding fix. The only conflict was the test module's import line: master reads the FSP message types from proto::fsp and proto::link, where maint still reads them from protocol. Resolved to master's paths, with SessionMsg3 taken from proto::fsp alongside SessionAck.
This commit is contained in:
@@ -857,6 +857,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,
|
||||
@@ -906,6 +931,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);
|
||||
|
||||
|
||||
+14
-1
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
|
||||
+224
-1
@@ -7,7 +7,7 @@ use crate::node::tests::spanning_tree::{
|
||||
lock_large_network_test, make_test_node_with_config, process_available_packets, run_tree_test,
|
||||
run_tree_test_with_mtus, verify_tree_convergence,
|
||||
};
|
||||
use crate::proto::fsp::SessionAck;
|
||||
use crate::proto::fsp::{SessionAck, SessionMsg3};
|
||||
use crate::proto::link::SessionDatagram;
|
||||
|
||||
/// Populate all nodes' coordinate caches with each other's coords.
|
||||
@@ -2539,3 +2539,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<u8>) {
|
||||
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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user