diff --git a/CHANGELOG.md b/CHANGELOG.md index 321a0ec3..f87f6c51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -376,6 +376,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 download now checks a per-architecture pinned SHA-256, with the hash provenance recorded honestly, upstream publishing no checksum document. +- The three routing signals (`CoordsRequired`, `PathBroken`, `MtuExceeded`) are + no longer acted on unless this node has itself bound the destination address + they name, either by initiating a session toward it or by completing the + Noise handshake that binds an address to a peer's static key. These signals + carry no end-to-end authentication, so until now any admitted mesh member + could send one naming any address and have its effects applied: a path-MTU + clamp written for an arbitrary address, a cached-coordinate flush for an + arbitrary address, and a discovery and warmup cycle for an arbitrary address. + The `MtuExceeded` case was the sharpest, because its write into the + address-keyed path-MTU lookup that the TUN reader consults at TCP MSS clamp + time sat outside the session guard and so required no session, no peer + relationship and no prior state at all. A half-open session created by an + inbound handshake that has not yet proved its address does not admit these + signals, so a forged session opening cannot be used to unlock them. Signals + from a genuine on-path forwarder are unaffected: the reporter may be any node + at any distance. This does not make the sender authentic, which nothing + short of a wire format change can do. Rejected signals are counted as + unknown-session rejections, and additionally on four new error-signal + counters visible through `show routing`, `show metrics` and the fipstop + routing pane: `unbound_coords`, `unbound_broken` and `unbound_mtu` give the + refused count per signal type, against the existing per-type arrival + counters as the denominator, and `unbound_forged` counts the subset whose + claimed source and destination pairing no honest forwarder could produce. + The drop log line now carries the signal type and the refusal class. + ## [0.4.1] - 2026-07-19 ### Changed diff --git a/src/bin/fipstop/ui/routing.rs b/src/bin/fipstop/ui/routing.rs index 10dd5492..819541e4 100644 --- a/src/bin/fipstop/ui/routing.rs +++ b/src/bin/fipstop/ui/routing.rs @@ -294,6 +294,10 @@ fn draw_routing_stats( ("PMTU Notif < Floor", err("path_mtu_notif_below_floor")), ("MTU Exceeded < Floor", err("mtu_exceeded_below_floor")), ("Lookup PMTU < Floor", err("lookup_resp_mtu_below_floor")), + ("Coords Required Refused", err("unbound_coords")), + ("Path Broken Refused", err("unbound_broken")), + ("MTU Exceeded Refused", err("unbound_mtu")), + ("Forged Pairing", err("unbound_forged")), ], )); right.push(Line::from("")); diff --git a/src/bin/fipstop/ui/snapshots.rs b/src/bin/fipstop/ui/snapshots.rs index 8df2fc8c..b108cc66 100644 --- a/src/bin/fipstop/ui/snapshots.rs +++ b/src/bin/fipstop/ui/snapshots.rs @@ -1151,7 +1151,7 @@ fn routing_focused_pane_scrolls() { // column is the taller of the two, so scrolling fully to the bottom would // over-scroll the right column past Congestion; this offset lands the // Congestion region inside the short window instead. - app1.scroll_offsets.insert((Tab::Routing, 2), 24); + app1.scroll_offsets.insert((Tab::Routing, 2), 28); let buf1 = testkit::render(100, 20, |frame, area| { super::routing::draw(frame, &app1, area); }); diff --git a/src/control/snapshots/show_routing.json b/src/control/snapshots/show_routing.json index 437701a2..9741572e 100644 --- a/src/control/snapshots/show_routing.json +++ b/src/control/snapshots/show_routing.json @@ -37,7 +37,11 @@ "mtu_exceeded": 0, "mtu_exceeded_below_floor": 0, "path_broken": 0, - "path_mtu_notif_below_floor": 0 + "path_mtu_notif_below_floor": 0, + "unbound_broken": 0, + "unbound_coords": 0, + "unbound_forged": 0, + "unbound_mtu": 0 }, "forwarding": { "decode_error_bytes": 0, diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 497fe561..bb70c0d9 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -55,6 +55,35 @@ struct PipelinedSend<'a> { dest_coords: Option<&'a crate::tree::TreeCoordinate>, } +/// Outcome of the routing-signal admission test. +/// +/// `Unbound` and `Forged` are both refusals, kept apart because they mean +/// different things to an operator. `Unbound` is consistent with a benign +/// race — a signal arriving just after a local session teardown. `Forged` +/// is not consistent with any honest emitter, so it is the sharper +/// indicator and is counted separately. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum SignalVerdict { + /// The named destination is an address this node bound itself. + Admit, + /// The src/dest pairing is structurally impossible for a legitimate + /// emitter. + Forged, + /// No qualifying session entry exists for the named destination. + Unbound, +} + +impl SignalVerdict { + /// Short stable label for the `verdict` log field. + fn label(self) -> &'static str { + match self { + Self::Admit => "admit", + Self::Forged => "forged", + Self::Unbound => "unbound", + } + } +} + impl Node { /// Handle a locally-delivered session datagram payload. /// @@ -106,13 +135,13 @@ impl Node { let error_body = &inner[1..]; match SessionMessageType::from_byte(error_type) { Some(SessionMessageType::CoordsRequired) => { - self.handle_coords_required(error_body).await; + self.handle_coords_required(src_addr, error_body).await; } Some(SessionMessageType::PathBroken) => { - self.handle_path_broken(error_body).await; + self.handle_path_broken(src_addr, error_body).await; } Some(SessionMessageType::MtuExceeded) => { - self.handle_mtu_exceeded(error_body).await; + self.handle_mtu_exceeded(src_addr, error_body).await; } _ => { debug!(error_type, "Unknown plaintext error signal type"); @@ -1156,13 +1185,58 @@ impl Node { } } + /// Whether a routing signal naming `dest`, arriving in a datagram + /// claiming source `src`, may be acted on, and if not, which kind of + /// refusal it is. + /// + /// `src` is the SessionDatagram's `src_addr`: a plain wire field, + /// authenticated only hop-by-hop by FMP Noise and never end to end. It + /// is therefore logged, not trusted. What is enforced here is that this + /// node has bound `dest` itself, either by initiating toward it or by + /// completing Noise XK, which binds the address to the peer's static + /// key (see the address-mismatch check in `handle_session_msg3`). A + /// responder entry that is still awaiting msg3 does NOT qualify: it is + /// keyed on an address the sender merely claimed, so admitting it would + /// let one forged SessionSetup unlock a signal about any address. + /// + /// The first two clauses reject nothing legitimate, and so return + /// `Forged` rather than `Unbound`. A datagram whose destination is this + /// node takes the deliver-local branch before any forwarding, so no node + /// ever emits a signal naming us as `dest`; and the emitter is by + /// construction a transit node for the datagram it is reporting on, so + /// it is never itself that datagram's destination. + /// + /// This narrows who can be targeted; it does not authenticate the + /// sender, which nothing short of a wire format change can do. + fn signal_verdict(&self, src: &NodeAddr, dest: &NodeAddr) -> SignalVerdict { + if dest == self.node_addr() || src == dest { + return SignalVerdict::Forged; + } + if self + .sessions + .get(dest) + .is_some_and(|e| e.is_established() || e.is_initiator()) + { + SignalVerdict::Admit + } else { + SignalVerdict::Unbound + } + } + /// Handle a CoordsRequired error signal from a transit router. /// /// The router couldn't route our packet because it lacks cached /// coordinates for the destination. Send a standalone CoordsWarmup /// immediately (rate-limited), trigger discovery, and reset the /// warmup counter for subsequent data packets. - async fn handle_coords_required(&mut self, inner: &[u8]) { + /// + /// `src_addr` is the datagram's claimed source and is not + /// end-to-end authenticated; see `signal_admissible`. + pub(in crate::node) async fn handle_coords_required( + &mut self, + src_addr: &NodeAddr, + inner: &[u8], + ) { self.metrics().errors.coords_required.inc(); let msg = match CoordsRequired::decode(inner) { @@ -1173,6 +1247,20 @@ impl Node { } }; + let verdict = self.signal_verdict(src_addr, &msg.dest_addr); + if verdict != SignalVerdict::Admit { + debug!(src = %src_addr, dest = %msg.dest_addr, reporter = %msg.reporter, + signal = "CoordsRequired", verdict = verdict.label(), + "Routing signal names an address this node has not bound; dropping"); + self.metrics().errors.unbound.coords.inc(); + if verdict == SignalVerdict::Forged { + self.metrics().errors.unbound.forged.inc(); + } + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::UnknownSession)); + return; + } + debug!( dest = %msg.dest_addr, reporter = %msg.reporter, @@ -1223,7 +1311,10 @@ impl Node { /// The router has coordinates but still can't route to the destination. /// Send a standalone CoordsWarmup immediately (rate-limited), invalidate /// cached coordinates, trigger re-discovery, and reset the warmup counter. - pub(in crate::node) async fn handle_path_broken(&mut self, inner: &[u8]) { + /// + /// `src_addr` is the datagram's claimed source and is not + /// end-to-end authenticated; see `signal_admissible`. + pub(in crate::node) async fn handle_path_broken(&mut self, src_addr: &NodeAddr, inner: &[u8]) { self.metrics().errors.path_broken.inc(); let msg = match PathBroken::decode(inner) { @@ -1234,6 +1325,20 @@ impl Node { } }; + let verdict = self.signal_verdict(src_addr, &msg.dest_addr); + if verdict != SignalVerdict::Admit { + debug!(src = %src_addr, dest = %msg.dest_addr, reporter = %msg.reporter, + signal = "PathBroken", verdict = verdict.label(), + "Routing signal names an address this node has not bound; dropping"); + self.metrics().errors.unbound.broken.inc(); + if verdict == SignalVerdict::Forged { + self.metrics().errors.unbound.forged.inc(); + } + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::UnknownSession)); + return; + } + debug!( dest = %msg.dest_addr, reporter = %msg.reporter, @@ -1293,7 +1398,10 @@ impl Node { /// A transit router couldn't forward our packet because it exceeded the /// next-hop transport MTU. Apply the reported bottleneck MTU to our /// PathMtuState for the affected session, causing an immediate decrease. - pub(in crate::node) async fn handle_mtu_exceeded(&mut self, inner: &[u8]) { + /// + /// `src_addr` is the datagram's claimed source and is not + /// end-to-end authenticated; see `signal_admissible`. + pub(in crate::node) async fn handle_mtu_exceeded(&mut self, src_addr: &NodeAddr, inner: &[u8]) { self.metrics().errors.mtu_exceeded.inc(); let msg = match MtuExceeded::decode(inner) { @@ -1304,6 +1412,20 @@ impl Node { } }; + let verdict = self.signal_verdict(src_addr, &msg.dest_addr); + if verdict != SignalVerdict::Admit { + debug!(src = %src_addr, dest = %msg.dest_addr, reporter = %msg.reporter, + signal = "MtuExceeded", verdict = verdict.label(), + "Routing signal names an address this node has not bound; dropping"); + self.metrics().errors.unbound.mtu.inc(); + if verdict == SignalVerdict::Forged { + self.metrics().errors.unbound.forged.inc(); + } + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::UnknownSession)); + return; + } + let peer_name = self.peer_display_name(&msg.dest_addr); debug!( dest = %peer_name, @@ -1352,6 +1474,14 @@ impl Node { // forward path; the reactive signal from a forwarder that actually // dropped a packet is authoritative for "what fits". Keep the // tighter of existing-or-new — never loosen the clamp. + // + // Unlike the proactive PathMtuNotification handler, this mirror is + // deliberately not skipped when the session-side MTU was unchanged. + // The lookup is seeded independently of `mmp.path_mtu` (discovery's + // reverse-path response and the FMP-promotion seed both write it), + // so an unchanged session MTU says nothing about whether the lookup + // still holds a looser value. The admission gate above, not this + // block, is what restricts which addresses can be written. let fips_addr = crate::FipsAddress::from_node_addr(&msg.dest_addr); match self.path_mtu_lookup.write() { Ok(mut map) => match map.get(&fips_addr).copied() { diff --git a/src/node/metrics.rs b/src/node/metrics.rs index 415266d4..a7352b55 100644 --- a/src/node/metrics.rs +++ b/src/node/metrics.rs @@ -453,6 +453,33 @@ impl CongestionMetrics { } } +/// Routing signals refused by the sender-binding admission gate, split by +/// signal type. +/// +/// The sibling counters on `ErrorMetrics` count arrivals, incremented before +/// the gate runs; these count the subset that was refused. Read together they +/// give the refused fraction per signal type, which is what separates a node +/// nobody is talking to from a node with a genuinely broken path from a node +/// being fed forged signals. +#[derive(Default)] +pub struct UnboundSignals { + /// `CoordsRequired` refused because this node has not bound the + /// destination address the signal names. + pub coords: Counter, + /// `PathBroken` refused because this node has not bound the destination + /// address the signal names. + pub broken: Counter, + /// `MtuExceeded` refused because this node has not bound the destination + /// address the signal names. + pub mtu: Counter, + /// Subset of the above whose src/dest pairing is structurally impossible + /// for a legitimate emitter: the signal names this node as the + /// destination, or claims a source equal to the destination it names. + /// Neither can arise from an honest on-path forwarder, so any count here + /// is a fabricated signal rather than ordinary session churn. + pub forged: Counter, +} + /// Error-signal metric counters. #[derive(Default)] pub struct ErrorMetrics { @@ -474,6 +501,7 @@ pub struct ErrorMetrics { /// count means a forwarder on the reverse path is mangling the unsigned /// annotation. pub lookup_resp_mtu_below_floor: Counter, + pub unbound: UnboundSignals, } impl ErrorMetrics { @@ -486,6 +514,10 @@ impl ErrorMetrics { path_mtu_notif_below_floor: self.path_mtu_notif_below_floor.get(), mtu_exceeded_below_floor: self.mtu_exceeded_below_floor.get(), lookup_resp_mtu_below_floor: self.lookup_resp_mtu_below_floor.get(), + unbound_coords: self.unbound.coords.get(), + unbound_broken: self.unbound.broken.get(), + unbound_mtu: self.unbound.mtu.get(), + unbound_forged: self.unbound.forged.get(), } } } diff --git a/src/node/session.rs b/src/node/session.rs index cd566335..956d3ea3 100644 --- a/src/node/session.rs +++ b/src/node/session.rs @@ -321,7 +321,6 @@ impl SessionEntry { } /// Whether this node initiated the Noise handshake. - #[cfg_attr(not(test), allow(dead_code))] pub(crate) fn is_initiator(&self) -> bool { self.is_initiator } diff --git a/src/node/stats.rs b/src/node/stats.rs index a88154c4..1a89e468 100644 --- a/src/node/stats.rs +++ b/src/node/stats.rs @@ -346,6 +346,10 @@ pub struct ErrorSignalStatsSnapshot { pub path_mtu_notif_below_floor: u64, pub mtu_exceeded_below_floor: u64, pub lookup_resp_mtu_below_floor: u64, + pub unbound_coords: u64, + pub unbound_broken: u64, + pub unbound_mtu: u64, + pub unbound_forged: u64, } #[derive(Clone, Debug, Default, Serialize)] diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 3c0bf00e..ff61a8ed 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, SessionMsg3}; +use crate::protocol::{CoordsRequired, PathBroken, SessionAck, SessionDatagram, SessionMsg3}; /// Populate all nodes' coordinate caches with each other's coords. /// @@ -2198,13 +2198,53 @@ fn build_mtu_exceeded_inner(dest: &NodeAddr, reporter: &NodeAddr, mtu: u16) -> V buf } +/// Install the half-open entry an inbound SessionSetup creates: keyed on an +/// address the sender merely claimed, awaiting msg3, not initiated by us. +/// +/// This is the shape an attacker manufactures with one forged handshake +/// opening, so a routing signal naming `claimed` must not be admitted by it. +fn install_halfopen(node: &mut Node, claimed: NodeAddr) { + use crate::noise::HandshakeState; + + let handshake = HandshakeState::new_xk_responder(node.identity().keypair()); + let placeholder = node.identity().keypair().public_key(); + let entry = crate::node::session::SessionEntry::new( + claimed, + placeholder, + EndToEndState::AwaitingMsg3(handshake), + 1000, + false, + ); + node.sessions.insert(claimed, entry); +} + +/// Install the entry `initiate_session` creates: an address this node chose +/// itself, with the handshake still in flight and MMP not yet initialized. +fn install_initiating(node: &mut Node, remote: &Identity) { + use crate::noise::HandshakeState; + + let handshake = + HandshakeState::new_xk_initiator(node.identity().keypair(), remote.pubkey_full()); + let remote_addr = *remote.node_addr(); + let entry = crate::node::session::SessionEntry::new( + remote_addr, + remote.pubkey_full(), + EndToEndState::Initiating(handshake), + 1000, + true, + ); + node.sessions.insert(remote_addr, entry); +} + #[tokio::test] async fn test_handle_mtu_exceeded_writes_path_mtu_lookup_when_empty() { use crate::node::tests::spanning_tree::make_test_node; let mut tn = make_test_node().await; - let dest = NodeAddr::from_bytes([0xCC; 16]); + let remote = Identity::generate(); + install_established_session_with_mmp(&mut tn.node, &remote); + let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); let dest_fips = crate::FipsAddress::from_node_addr(&dest); @@ -2214,7 +2254,7 @@ async fn test_handle_mtu_exceeded_writes_path_mtu_lookup_when_empty() { ); let inner = build_mtu_exceeded_inner(&dest, &reporter, 1280); - tn.node.handle_mtu_exceeded(&inner).await; + tn.node.handle_mtu_exceeded(&reporter, &inner).await; assert_eq!( tn.node.path_mtu_lookup_get(&dest_fips), @@ -2229,7 +2269,9 @@ async fn test_handle_mtu_exceeded_tightens_existing_path_mtu_lookup() { let mut tn = make_test_node().await; - let dest = NodeAddr::from_bytes([0xCC; 16]); + let remote = Identity::generate(); + install_established_session_with_mmp(&mut tn.node, &remote); + let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); let dest_fips = crate::FipsAddress::from_node_addr(&dest); @@ -2238,7 +2280,7 @@ async fn test_handle_mtu_exceeded_tightens_existing_path_mtu_lookup() { tn.node.path_mtu_lookup_insert(dest_fips, 1500); let inner = build_mtu_exceeded_inner(&dest, &reporter, 1280); - tn.node.handle_mtu_exceeded(&inner).await; + tn.node.handle_mtu_exceeded(&reporter, &inner).await; assert_eq!( tn.node.path_mtu_lookup_get(&dest_fips), @@ -2253,7 +2295,9 @@ async fn test_handle_mtu_exceeded_keeps_tighter_existing_path_mtu_lookup() { let mut tn = make_test_node().await; - let dest = NodeAddr::from_bytes([0xCC; 16]); + let remote = Identity::generate(); + install_established_session_with_mmp(&mut tn.node, &remote); + let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); let dest_fips = crate::FipsAddress::from_node_addr(&dest); @@ -2263,7 +2307,7 @@ async fn test_handle_mtu_exceeded_keeps_tighter_existing_path_mtu_lookup() { tn.node.path_mtu_lookup_insert(dest_fips, 1280); let inner = build_mtu_exceeded_inner(&dest, &reporter, 1500); - tn.node.handle_mtu_exceeded(&inner).await; + tn.node.handle_mtu_exceeded(&reporter, &inner).await; assert_eq!( tn.node.path_mtu_lookup_get(&dest_fips), @@ -2276,18 +2320,22 @@ async fn test_handle_mtu_exceeded_keeps_tighter_existing_path_mtu_lookup() { async fn test_handle_mtu_exceeded_below_floor_leaves_path_mtu_lookup_untouched() { use crate::node::tests::spanning_tree::make_test_node; - // MtuExceeded is an unencrypted signal and the lookup write below it is - // not gated on a session existing, so anyone can reach it. A bottleneck - // this small cannot describe a real path; storing it would drive the - // SYN-time MSS clamp to a single-digit or zero segment size. + // MtuExceeded is an unencrypted signal that any admitted member can send + // for any destination this node has bound. A bottleneck this small cannot + // describe a real path; storing it would drive the SYN-time MSS clamp to a + // single-digit or zero segment size. The session is installed so the + // admission gate lets the signal through and the floor is what refuses it; + // without one this would pass whether or not the floor exists. let mut tn = make_test_node().await; - let dest = NodeAddr::from_bytes([0xCC; 16]); + let remote = Identity::generate(); + install_initiating(&mut tn.node, &remote); + let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); let dest_fips = crate::FipsAddress::from_node_addr(&dest); let inner = build_mtu_exceeded_inner(&dest, &reporter, 100); - tn.node.handle_mtu_exceeded(&inner).await; + tn.node.handle_mtu_exceeded(&reporter, &inner).await; assert_eq!( tn.node.path_mtu_lookup_get(&dest_fips), @@ -2306,7 +2354,11 @@ async fn test_sub_floor_mtu_exceeded_is_counted_separately_from_all_mtu_exceeded let mut tn = make_test_node().await; - let dest = NodeAddr::from_bytes([0xCC; 16]); + // Bound the destination so the admission gate admits the signal and the + // floor is what classifies it. + let remote = Identity::generate(); + install_initiating(&mut tn.node, &remote); + let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); assert_eq!( @@ -2320,7 +2372,7 @@ async fn test_sub_floor_mtu_exceeded_is_counted_separately_from_all_mtu_exceeded &reporter, crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU - 1, ); - tn.node.handle_mtu_exceeded(&inner).await; + tn.node.handle_mtu_exceeded(&reporter, &inner).await; assert_eq!( tn.node.metrics().errors.mtu_exceeded_below_floor.get(), @@ -2331,7 +2383,7 @@ async fn test_sub_floor_mtu_exceeded_is_counted_separately_from_all_mtu_exceeded // The counter must discriminate: an actionable bottleneck is stored and // must bump only the all-signals counter. let inner = build_mtu_exceeded_inner(&dest, &reporter, 1280); - tn.node.handle_mtu_exceeded(&inner).await; + tn.node.handle_mtu_exceeded(&reporter, &inner).await; assert_eq!( tn.node.metrics().errors.mtu_exceeded_below_floor.get(), @@ -2353,13 +2405,15 @@ async fn test_handle_mtu_exceeded_at_the_floor_still_writes_path_mtu_lookup() { // floor could be widened arbitrarily and the test above would not notice. let mut tn = make_test_node().await; - let dest = NodeAddr::from_bytes([0xCD; 16]); + let remote = Identity::generate(); + install_initiating(&mut tn.node, &remote); + let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); let dest_fips = crate::FipsAddress::from_node_addr(&dest); let floor = crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU; let inner = build_mtu_exceeded_inner(&dest, &reporter, floor); - tn.node.handle_mtu_exceeded(&inner).await; + tn.node.handle_mtu_exceeded(&reporter, &inner).await; assert_eq!( tn.node.path_mtu_lookup_get(&dest_fips), @@ -2410,7 +2464,7 @@ async fn test_forged_mtu_exceeded_of_zero_does_not_blackhole_the_session() { // nothing at all, reported by a node that is not on the path. let reporter = NodeAddr::from_bytes([0xEE; 16]); let inner = build_mtu_exceeded_inner(&node1_addr, &reporter, 0); - nodes[0].node.handle_mtu_exceeded(&inner).await; + nodes[0].node.handle_mtu_exceeded(&reporter, &inner).await; let (tun_tx, tun_rx) = std::sync::mpsc::channel(); nodes[0].node.tun_tx = Some(tun_tx); @@ -2447,7 +2501,13 @@ async fn test_path_broken_releases_path_mtu_lookup_entry() { // outlives every route change until the daemon restarts. let mut tn = make_test_node().await; - let dest = NodeAddr::from_bytes([0xCC; 16]); + // The signal is only acted on for a destination this node has itself + // bound, so the release is reachable only behind an installed session. + // Without one the admission gate refuses the signal and this test would + // observe the entry surviving for the wrong reason. + let remote = Identity::generate(); + install_initiating(&mut tn.node, &remote); + let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); let dest_fips = crate::FipsAddress::from_node_addr(&dest); @@ -2464,7 +2524,7 @@ async fn test_path_broken_releases_path_mtu_lookup_entry() { assertion below observes nothing" ); - tn.node.handle_path_broken(inner).await; + tn.node.handle_path_broken(&reporter, inner).await; assert_eq!( tn.node.path_mtu_lookup_get(&dest_fips), @@ -2548,6 +2608,368 @@ async fn test_idle_session_purge_keeps_link_peer_path_mtu_seed() { } } +// ============================================================================ +// Routing-signal admission: the named destination must be an address this +// node bound itself, either by initiating toward it or by completing the +// handshake that binds an address to a peer's static key. These signals carry +// no end-to-end authentication, so without that gate any mesh member can name +// any address and have the effects applied. +// ============================================================================ + +#[tokio::test] +async fn test_mtu_exceeded_naming_a_dest_with_no_session_does_not_touch_path_mtu_lookup() { + let mut node = make_node(); + + let dest = NodeAddr::from_bytes([0xCC; 16]); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + let dest_fips = crate::FipsAddress::from_node_addr(&dest); + + assert!( + node.path_mtu_lookup_get(&dest_fips).is_none(), + "lookup should start empty for this destination" + ); + + let inner = build_mtu_exceeded_inner(&dest, &reporter, 1280); + node.handle_mtu_exceeded(&reporter, &inner).await; + + assert_eq!( + node.path_mtu_lookup_get(&dest_fips), + None, + "a signal naming an address with no session must not write the clamp" + ); + assert_eq!(node.stats().session.unknown_session, 1); + + let errors = &node.metrics().errors; + assert_eq!( + errors.unbound.mtu.get(), + 1, + "the refusal must be counted against the MtuExceeded counter" + ); + assert_eq!( + errors.unbound.coords.get(), + 0, + "an MtuExceeded refusal must not bump the CoordsRequired counter" + ); + assert_eq!( + errors.unbound.broken.get(), + 0, + "an MtuExceeded refusal must not bump the PathBroken counter" + ); + assert_eq!( + errors.unbound.forged.get(), + 0, + "an absent session is an unbound refusal, not a forged pairing" + ); + assert_eq!( + errors.mtu_exceeded.get(), + 1, + "the arrival counter is the denominator and counts refused arrivals too" + ); +} + +#[tokio::test] +async fn test_mtu_exceeded_naming_a_dest_whose_entry_is_an_unauthenticated_responder_handshake_is_dropped() + { + let mut node = make_node(); + + let dest = NodeAddr::from_bytes([0xCC; 16]); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + let dest_fips = crate::FipsAddress::from_node_addr(&dest); + + // One forged SessionSetup naming `dest` would leave exactly this entry. + install_halfopen(&mut node, dest); + + let inner = build_mtu_exceeded_inner(&dest, &reporter, 1280); + node.handle_mtu_exceeded(&reporter, &inner).await; + + assert_eq!( + node.path_mtu_lookup_get(&dest_fips), + None, + "a half-open entry keyed on a claimed address must not admit the signal" + ); + assert_eq!(node.stats().session.unknown_session, 1); + + let errors = &node.metrics().errors; + assert_eq!( + errors.unbound.mtu.get(), + 1, + "a half-open entry is an unbound refusal for MtuExceeded" + ); + assert_eq!( + errors.unbound.forged.get(), + 0, + "a half-open entry is a plausible pairing, not a forged one" + ); +} + +#[tokio::test] +async fn test_mtu_exceeded_for_a_session_we_initiated_seeds_path_mtu_lookup_before_establishment() { + let mut node = make_node(); + + let remote = Identity::generate(); + install_initiating(&mut node, &remote); + let dest = *remote.node_addr(); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + let dest_fips = crate::FipsAddress::from_node_addr(&dest); + + let inner = build_mtu_exceeded_inner(&dest, &reporter, 1280); + node.handle_mtu_exceeded(&reporter, &inner).await; + + assert_eq!( + node.path_mtu_lookup_get(&dest_fips), + Some(1280), + "an address we chose ourselves must still seed the clamp during handshake" + ); + assert_eq!( + node.metrics().errors.unbound.mtu.get(), + 0, + "an admitted signal must not be counted as refused" + ); +} + +#[tokio::test] +async fn test_mtu_exceeded_from_a_third_party_forwarder_still_tightens_an_active_session() { + let mut node = make_node(); + + let remote = Identity::generate(); + install_established_session_with_mmp(&mut node, &remote); + let dest = *remote.node_addr(); + // A real transit reporter is neither us nor the destination. + let reporter = NodeAddr::from_bytes([0xBB; 16]); + let dest_fips = crate::FipsAddress::from_node_addr(&dest); + + let inner = build_mtu_exceeded_inner(&dest, &reporter, 1280); + node.handle_mtu_exceeded(&reporter, &inner).await; + + assert_eq!( + node.path_mtu_lookup_get(&dest_fips), + Some(1280), + "an on-path forwarder's report must still tighten the clamp" + ); + assert_eq!( + node.sessions + .get(&dest) + .and_then(|e| e.mmp()) + .map(|m| m.path_mtu.current_mtu()), + Some(1280), + "the session-side path MTU must also decrease" + ); +} + +#[tokio::test] +async fn test_path_broken_naming_a_dest_with_no_session_does_not_flush_cached_coords() { + let mut node = make_node(); + + let dest = NodeAddr::from_bytes([0xCC; 16]); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + let coords = node.tree_state().my_coords().clone(); + node.coord_cache_mut().insert(dest, coords, 1000); + + let encoded = PathBroken::new(dest, reporter).encode(); + node.handle_path_broken(&reporter, &encoded[5..]).await; + + assert!( + node.coord_cache().get(&dest, 1000).is_some(), + "a signal naming an address with no session must not flush its coords" + ); + assert_eq!(node.stats().session.unknown_session, 1); + + let errors = &node.metrics().errors; + assert_eq!( + errors.unbound.broken.get(), + 1, + "the refusal must be counted against the PathBroken counter" + ); + assert_eq!( + errors.unbound.mtu.get(), + 0, + "a PathBroken refusal must not bump the MtuExceeded counter" + ); + assert_eq!( + errors.unbound.coords.get(), + 0, + "a PathBroken refusal must not bump the CoordsRequired counter" + ); + assert_eq!( + errors.unbound.forged.get(), + 0, + "an absent session is an unbound refusal, not a forged pairing" + ); +} + +#[tokio::test] +async fn test_path_broken_naming_a_dest_whose_entry_is_an_unauthenticated_responder_handshake_does_not_flush_cached_coords() + { + let mut node = make_node(); + + let dest = NodeAddr::from_bytes([0xCC; 16]); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + let coords = node.tree_state().my_coords().clone(); + node.coord_cache_mut().insert(dest, coords, 1000); + + // One forged SessionSetup naming `dest` would leave exactly this entry. + install_halfopen(&mut node, dest); + + let encoded = PathBroken::new(dest, reporter).encode(); + node.handle_path_broken(&reporter, &encoded[5..]).await; + + assert!( + node.coord_cache().get(&dest, 1000).is_some(), + "a half-open entry keyed on a claimed address must not admit the signal" + ); + assert_eq!(node.stats().session.unknown_session, 1); + + let errors = &node.metrics().errors; + assert_eq!( + errors.unbound.broken.get(), + 1, + "a half-open entry is an unbound refusal for PathBroken" + ); + assert_eq!( + errors.unbound.forged.get(), + 0, + "a half-open entry is a plausible pairing, not a forged one" + ); +} + +#[tokio::test] +async fn test_path_broken_for_a_session_we_initiated_still_flushes_cached_coords() { + let mut node = make_node(); + + let remote = Identity::generate(); + install_initiating(&mut node, &remote); + let dest = *remote.node_addr(); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + let coords = node.tree_state().my_coords().clone(); + node.coord_cache_mut().insert(dest, coords, 1000); + + let encoded = PathBroken::new(dest, reporter).encode(); + node.handle_path_broken(&reporter, &encoded[5..]).await; + + assert!( + node.coord_cache().get(&dest, 1000).is_none(), + "handshake-time recovery must still flush coords for an address we chose" + ); +} + +#[tokio::test] +async fn test_coords_required_naming_a_dest_with_no_session_is_counted_as_an_unknown_session_reject() + { + let mut node = make_node(); + + let dest = NodeAddr::from_bytes([0xCC; 16]); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + + let encoded = CoordsRequired::new(dest, reporter).encode(); + node.handle_coords_required(&reporter, &encoded[5..]).await; + assert_eq!(node.stats().session.unknown_session, 1); + + // The gate runs ahead of the response rate limiter, so a second + // identical signal is rejected the same way rather than being + // absorbed by rate-limiter state keyed on an attacker-chosen address. + node.handle_coords_required(&reporter, &encoded[5..]).await; + assert_eq!(node.stats().session.unknown_session, 2); + + let errors = &node.metrics().errors; + assert_eq!( + errors.unbound.coords.get(), + 2, + "both refusals must be counted against the CoordsRequired counter" + ); + assert_eq!( + errors.unbound.broken.get(), + 0, + "a CoordsRequired refusal must not bump the PathBroken counter" + ); + assert_eq!( + errors.unbound.mtu.get(), + 0, + "a CoordsRequired refusal must not bump the MtuExceeded counter" + ); + assert_eq!( + errors.unbound.forged.get(), + 0, + "an absent session is an unbound refusal, not a forged pairing" + ); + assert_eq!( + errors.coords_required.get(), + 2, + "the arrival counter is the denominator and counts refused arrivals too" + ); +} + +#[tokio::test] +async fn test_mtu_exceeded_whose_claimed_source_is_the_destination_it_names_is_dropped() { + let mut node = make_node(); + + let remote = Identity::generate(); + install_established_session_with_mmp(&mut node, &remote); + let dest = *remote.node_addr(); + let dest_fips = crate::FipsAddress::from_node_addr(&dest); + + // The emitter of a routing signal is by construction a transit node for + // the datagram it is reporting on, so it is never that datagram's own + // destination. A signal claiming otherwise is malformed. + let inner = build_mtu_exceeded_inner(&dest, &dest, 1280); + node.handle_mtu_exceeded(&dest, &inner).await; + + assert_eq!( + node.path_mtu_lookup_get(&dest_fips), + None, + "a signal whose claimed source is the destination it names must be dropped" + ); + assert_eq!(node.stats().session.unknown_session, 1); + + let errors = &node.metrics().errors; + assert_eq!( + errors.unbound.mtu.get(), + 1, + "the refusal must still be counted against the MtuExceeded counter" + ); + assert_eq!( + errors.unbound.forged.get(), + 1, + "a src equal to the dest it names is a structurally impossible pairing" + ); +} + +#[tokio::test] +async fn test_coords_required_naming_this_node_as_the_destination_counts_a_forged_pairing() { + let mut node = make_node(); + + // A datagram addressed to this node is delivered locally before any + // forwarding, so no honest transit router ever emits a signal naming + // us as the destination. This clause can only be reached by fabrication. + let dest = *node.node_addr(); + let reporter = NodeAddr::from_bytes([0xBB; 16]); + + let encoded = CoordsRequired::new(dest, reporter).encode(); + node.handle_coords_required(&reporter, &encoded[5..]).await; + + assert_eq!(node.stats().session.unknown_session, 1); + let errors = &node.metrics().errors; + assert_eq!( + errors.unbound.coords.get(), + 1, + "the refusal must be counted against the CoordsRequired counter" + ); + assert_eq!( + errors.unbound.forged.get(), + 1, + "a signal naming this node as the destination is a forged pairing" + ); + assert_eq!( + errors.unbound.broken.get(), + 0, + "a CoordsRequired refusal must not bump the PathBroken counter" + ); + assert_eq!( + errors.unbound.mtu.get(), + 0, + "a CoordsRequired refusal must not bump the MtuExceeded counter" + ); +} + // ============================================================================ // Proactive PathMtuNotification → path_mtu_lookup focused unit tests //