diff --git a/src/node/handlers/discovery.rs b/src/node/handlers/discovery.rs index cbf4694..f91c02b 100644 --- a/src/node/handlers/discovery.rs +++ b/src/node/handlers/discovery.rs @@ -7,6 +7,7 @@ use crate::node::{Node, RecentRequest}; use crate::protocol::{LookupRequest, LookupResponse}; +use crate::transport::{TransportAddr, TransportId}; use crate::{NodeAddr, PeerIdentity}; use tracing::{debug, info, trace, warn}; @@ -138,16 +139,7 @@ impl Node { self.stats_mut().discovery.resp_forwarded += 1; // Apply path_mtu min() from the outgoing link's transport MTU - if let Some(peer) = self.peers.get(&from_peer) - && let Some(tid) = peer.transport_id() - && let Some(transport) = self.transports.get(&tid) - { - if let Some(addr) = peer.current_addr() { - response.path_mtu = response.path_mtu.min(transport.link_mtu(addr)); - } else { - response.path_mtu = response.path_mtu.min(transport.mtu()); - } - } + self.apply_outgoing_link_mtu_to_response(&mut response, &from_peer); debug!( request_id = response.request_id, @@ -217,6 +209,32 @@ impl Node { self.coord_cache .insert_with_path_mtu(target, response.target_coords, now_ms, path_mtu); + // Mirror path_mtu into the FipsAddress-keyed read-only lookup + // 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" + ); + } + Err(e) => { + warn!( + target = %self.peer_display_name(&target), + fips_addr = %fips_addr, + path_mtu = path_mtu, + error = %e, + "path_mtu_lookup write lock poisoned; clamp will not see this update" + ); + } + } + // Clean up pending lookup tracking self.pending_lookups.remove(&target); @@ -256,7 +274,8 @@ impl Node { LookupResponse::proof_bytes(request.request_id, &request.target, &our_coords); let proof = self.identity().sign(&proof_data); - let response = LookupResponse::new(request.request_id, request.target, our_coords, proof); + let mut response = + LookupResponse::new(request.request_id, request.target, our_coords, proof); // Route toward origin via reverse path. let next_hop_addr = if let Some(recent) = self.recent_requests.get(&request.request_id) { @@ -275,10 +294,17 @@ impl Node { } }; + // Fold our outgoing-link MTU into path_mtu so the target-edge link + // appears in the bottleneck calculation. Without this, the response + // leaves the target with path_mtu = u16::MAX and only intermediate + // transits min-fold; the target's first reverse-path hop is missed. + self.apply_outgoing_link_mtu_to_response(&mut response, &next_hop_addr); + debug!( request_id = request.request_id, origin = %self.peer_display_name(&request.origin), next_hop = %self.peer_display_name(&next_hop_addr), + path_mtu = response.path_mtu, "Sending LookupResponse" ); @@ -616,6 +642,92 @@ impl Node { self.recent_requests .retain(|_, entry| !entry.is_expired(current_time_ms, expiry_ms)); } + + /// Min-fold our outgoing-link MTU into a LookupResponse's `path_mtu`. + /// + /// Used at both transit-side reverse-path forward and at the target's + /// own send_lookup_response. The link MTU we apply is the MTU of the + /// transport+addr we'll use to deliver the response toward `next_hop`. + /// No-op when `next_hop` is not a directly-connected peer or its + /// transport is not registered. + pub(in crate::node) fn apply_outgoing_link_mtu_to_response( + &self, + response: &mut LookupResponse, + next_hop: &NodeAddr, + ) { + if let Some(peer) = self.peers.get(next_hop) + && let Some(tid) = peer.transport_id() + && let Some(transport) = self.transports.get(&tid) + { + let link_mtu = if let Some(addr) = peer.current_addr() { + transport.link_mtu(addr) + } else { + transport.mtu() + }; + response.path_mtu = response.path_mtu.min(link_mtu); + } + } + + /// Seed `path_mtu_lookup` for a directly-connected peer. + /// + /// Called when an FMP link-layer peer is promoted to active. The seed + /// value is the local outgoing-link MTU on the peer's transport, which + /// is the actual link constraint for direct-link traffic. Stored only + /// when no tighter value exists: discovery's reverse-path bottleneck + /// or MMP `MtuExceeded` reactive learning take precedence when smaller. + /// + /// Without this seed, configured/auto-connect peers (which establish + /// sessions without going through the discovery Lookup flow) leave + /// `path_mtu_lookup` empty for their FipsAddress, causing + /// `per_flow_max_mss` to fall back to the global ceiling and the + /// SYN-time TCP MSS clamp to over-estimate the effective path. + pub(in crate::node) fn seed_path_mtu_for_link_peer( + &self, + peer_addr: &NodeAddr, + transport_id: TransportId, + addr: &TransportAddr, + ) { + let Some(transport) = self.transports.get(&transport_id) else { + debug!( + peer = %self.peer_display_name(peer_addr), + transport_id = %transport_id, + "seed_path_mtu_for_link_peer: transport not registered, skipping seed" + ); + return; + }; + let link_mtu = transport.link_mtu(addr); + let fips_addr = crate::FipsAddress::from_node_addr(peer_addr); + let Ok(mut map) = self.path_mtu_lookup.write() else { + warn!( + peer = %self.peer_display_name(peer_addr), + "seed_path_mtu_for_link_peer: path_mtu_lookup write lock poisoned" + ); + return; + }; + match map.get(&fips_addr).copied() { + Some(existing) if existing <= link_mtu => { + // Keep the tighter learned value; never loosen the clamp. + debug!( + peer = %self.peer_display_name(peer_addr), + fips_addr = %fips_addr, + link_mtu = link_mtu, + existing = existing, + "seed_path_mtu_for_link_peer: keeping tighter existing value" + ); + } + other => { + map.insert(fips_addr, link_mtu); + debug!( + peer = %self.peer_display_name(peer_addr), + fips_addr = %fips_addr, + link_mtu = link_mtu, + prior = ?other, + map_len = map.len(), + "seed_path_mtu_for_link_peer: wrote link MTU" + ); + } + } + } } /// Tracks a pending discovery lookup with retry state. diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index 520de2c..a2a9652 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -1401,6 +1401,8 @@ impl Node { let _ = self.index_allocator.free(old_idx); } + self.seed_path_mtu_for_link_peer(&peer_node_addr, transport_id, ¤t_addr); + let mut new_peer = ActivePeer::with_session( verified_identity, link_id, @@ -1504,6 +1506,8 @@ impl Node { .get(&peer_node_addr) .map(|p| p.last_tree_announce_sent_ms()); + self.seed_path_mtu_for_link_peer(&peer_node_addr, transport_id, ¤t_addr); + let mut new_peer = ActivePeer::with_session( verified_identity, link_id, diff --git a/src/node/lifecycle.rs b/src/node/lifecycle.rs index ad51971..8627654 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -667,8 +667,11 @@ impl Node { (fds[0], fds[1]) }; - // Create writer (dups the fd for independent write access) - let (writer, tun_tx) = device.create_writer(max_mss)?; + // Create writer (dups the fd for independent write access). + // Pass path_mtu_lookup so inbound SYN-ACK clamp can read + // per-destination path MTU learned via discovery. + let (writer, tun_tx) = + device.create_writer(max_mss, self.path_mtu_lookup.clone())?; // Spawn writer thread let writer_handle = thread::spawn(move || { @@ -684,6 +687,7 @@ impl Node { // Spawn reader thread let transport_mtu = self.transport_mtu(); + let path_mtu_lookup = self.path_mtu_lookup.clone(); #[cfg(target_os = "macos")] let reader_handle = thread::spawn(move || { run_tun_reader( @@ -693,6 +697,7 @@ impl Node { reader_tun_tx, outbound_tx, transport_mtu, + path_mtu_lookup, shutdown_read_fd, ); }); @@ -705,6 +710,7 @@ impl Node { reader_tun_tx, outbound_tx, transport_mtu, + path_mtu_lookup, ); }); diff --git a/src/node/mod.rs b/src/node/mod.rs index 4564e89..358f97c 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -306,6 +306,12 @@ pub struct Node { /// Recent discovery requests (dedup + reverse-path forwarding). /// Maps request_id → RecentRequest. recent_requests: HashMap, + /// Per-destination path MTU lookup, keyed by FipsAddress (mirrors + /// `coord_cache.entries[*].path_mtu`). Sync read-only access from + /// the TUN reader/writer threads at TCP MSS clamp time so the + /// SYN/SYN-ACK clamp can use the smaller of the local-egress floor + /// and the learned per-destination path MTU. + path_mtu_lookup: Arc>>, // === Transports & Links === /// Active transports (owned by Node). @@ -628,6 +634,7 @@ impl Node { peer_aliases: HashMap::new(), peer_acl, host_map, + path_mtu_lookup: Arc::new(std::sync::RwLock::new(HashMap::new())), }) } @@ -759,6 +766,7 @@ impl Node { peer_aliases: HashMap::new(), peer_acl, host_map, + path_mtu_lookup: Arc::new(std::sync::RwLock::new(HashMap::new())), }) } diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index 863a2a8..96945fc 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -702,14 +702,13 @@ async fn test_discovery_100_nodes() { #[tokio::test] async fn test_response_path_mtu_two_node() { // Two-node topology: node0 — node1 - // Node0 initiates lookup for node1. The response should carry path_mtu - // reflecting the transport MTU (1280 in tests) clamped by transit. - // In a two-node setup: node1 (target) initializes path_mtu=u16::MAX, - // then the response is sent directly to node0. Since node1 is the - // target and sends directly, the transit logic does not apply for the - // first hop (the target sends directly). But node0 is the originator - // and doesn't apply transit MTU. So path_mtu should be u16::MAX in - // this simple case (no transit nodes to clamp it). + // Node0 initiates lookup for node1. node1 is the target and generates + // the response: send_lookup_response folds in node1's own outgoing-link + // MTU before sending, so path_mtu reflects the target-edge link + // constraint (the test transport MTU, 1280) even with no transit hops. + // Without that target-edge fold, a 2-node lookup would leave path_mtu + // at u16::MAX since no transit min-fold runs — that's the gap closed + // alongside the configured-peer seed in the B3 follow-up. let edges = vec![(0, 1)]; let mut nodes = run_tree_test(2, &edges, false).await; @@ -732,21 +731,40 @@ async fn test_response_path_mtu_two_node() { "Node 0 should have cached node 1's route" ); - // Check that path_mtu was stored in the cache entry let entry = nodes[0].node.coord_cache().get_entry(&node1_addr).unwrap(); let path_mtu = entry .path_mtu() .expect("path_mtu should be set from discovery"); - // In a 2-node setup, no transit node applies the min() so path_mtu stays u16::MAX assert_eq!( - path_mtu, - u16::MAX, - "Two-node path_mtu should be u16::MAX (no transit nodes to clamp)" + path_mtu, 1280, + "Two-node path_mtu should be the target-edge link MTU (1280 in tests)" ); cleanup_nodes(&mut nodes).await; } +#[tokio::test] +async fn test_apply_outgoing_link_mtu_to_response_unknown_peer_noop() { + // When next_hop is not a directly-connected peer (no entry in + // self.peers), apply_outgoing_link_mtu_to_response is a no-op and the + // response's path_mtu is left unchanged. Pins the early-return path. + let node = make_node(); + let unknown = make_node_addr(0x99); + + let coords = TreeCoordinate::from_addrs(vec![unknown, make_node_addr(0)]).unwrap(); + let identity = Identity::generate(); + let proof_data = LookupResponse::proof_bytes(1, &unknown, &coords); + let proof = identity.sign(&proof_data); + let mut response = LookupResponse::new(1, unknown, coords, proof); + response.path_mtu = 1500; + + node.apply_outgoing_link_mtu_to_response(&mut response, &unknown); + assert_eq!( + response.path_mtu, 1500, + "Unknown next_hop must leave path_mtu untouched" + ); +} + #[tokio::test] async fn test_response_path_mtu_three_node_chain() { // Topology: node0 — node1 — node2 @@ -928,7 +946,11 @@ async fn test_transit_forwards_when_mtu_sufficient() { // Topology: node0(1280) — node1(1400) — node2(1280) // Node0 initiates lookup for node2 with min_mtu=1280 (default TUN MTU). // Node1's transport MTU is 1400 >= 1280, so the request passes through. - // Node1 annotates path_mtu = min(u16::MAX, 1400) = 1400 on response. + // Bottleneck min-fold accumulates contributions from BOTH the target's + // own outgoing-link MTU (the target-edge fold added with the + // direct-link/target-edge gap fix) and each transit node's outgoing- + // link MTU. With node2 (target) at 1280 and node1 (transit) at 1400, + // the bottleneck is min(1280, 1400) = 1280. let mtus = [1280, 1400, 1280]; let edges = vec![(0, 1), (1, 2)]; let mut nodes = run_tree_test_with_mtus(&mtus, &edges).await; @@ -957,8 +979,8 @@ async fn test_transit_forwards_when_mtu_sufficient() { let entry = nodes[0].node.coord_cache().get_entry(&node2_addr).unwrap(); let path_mtu = entry.path_mtu().expect("path_mtu should be set"); assert_eq!( - path_mtu, 1400, - "path_mtu should reflect transit node's transport MTU (1400)" + path_mtu, 1280, + "path_mtu should be min(target-edge 1280, transit 1400) = 1280" ); cleanup_nodes(&mut nodes).await; @@ -966,16 +988,17 @@ async fn test_transit_forwards_when_mtu_sufficient() { #[tokio::test] async fn test_response_path_mtu_four_node_chain() { - // Topology: node0(1280) — node1(1400) — node2(900) — node3(1280) + // Topology: node0(1280) — node1(1500) — node2(1350) — node3(1280) // Node0 initiates lookup for node3. Response travels node3→node2→node1→node0. - // Transit nodes apply min(): node2 sees min(u16::MAX, 900) = 900, - // node1 sees min(900, 1400) = 900. - // Final path_mtu at node0 should be 900 (bottleneck at node2). + // The bottleneck min-fold now accumulates contributions from the target's + // own outgoing link MTU (target-edge fold added with the direct-link gap + // fix) AND each transit node's outgoing link MTU on the reverse path. + // node3 (target, 1280) → 1280; node2 (transit, 1350) → min(1280, 1350) = + // 1280; node1 (transit, 1500) → min(1280, 1500) = 1280. Result: 1280. // - // Note: min_mtu=1280 from TUN config. Node2's MTU (900) < 1280 would prune - // the forward request at node2, so node3 would never be reached. To test - // path_mtu annotation we need all transit links to pass the min_mtu check. - // Use MTUs above 1280 to avoid pruning but with different values to verify min(). + // Note: min_mtu=1280 from TUN config. All transit MTUs ≥ 1280 so the + // forward request is not pruned; the test exercises the response-side + // min-fold accumulation explicitly. let mtus = [1280, 1500, 1350, 1280]; let edges = vec![(0, 1), (1, 2), (2, 3)]; let mut nodes = run_tree_test_with_mtus(&mtus, &edges).await; @@ -1004,8 +1027,8 @@ async fn test_response_path_mtu_four_node_chain() { let entry = nodes[0].node.coord_cache().get_entry(&node3_addr).unwrap(); let path_mtu = entry.path_mtu().expect("path_mtu should be set"); assert_eq!( - path_mtu, 1350, - "Four-node chain path_mtu should be min of transit MTUs (1350)" + path_mtu, 1280, + "Four-node chain path_mtu = min(target-edge 1280, transits 1350+1500) = 1280" ); cleanup_nodes(&mut nodes).await; diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 5eda3d1..dcdf792 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -1033,3 +1033,138 @@ async fn test_transport_mtu_min_with_single_operational() { transport.stop().await.ok(); } } + +// path_mtu_lookup seeding for direct-link (configured) peers — closes the +// B3 coverage gap where configured/auto-connect peers never go through the +// discovery Lookup flow and so their FipsAddress was missing from +// path_mtu_lookup, causing the SYN-time TCP MSS clamp to fall back to the +// global ceiling. + +#[tokio::test] +async fn test_seed_path_mtu_inserts_when_empty() { + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp = make_udp_transport_with_mtu(1, 1452).await; + node.transports.insert(TransportId::new(1), udp); + + let peer_addr = make_node_addr(0xAA); + let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr); + let transport_addr = TransportAddr::from_string("10.0.0.2:2121"); + + node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr); + + let stored = node + .path_mtu_lookup + .read() + .unwrap() + .get(&fips_addr) + .copied(); + assert_eq!( + stored, + Some(1452), + "Empty lookup should be seeded with the link MTU" + ); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +#[tokio::test] +async fn test_seed_path_mtu_keeps_tighter_existing_value() { + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp = make_udp_transport_with_mtu(1, 1452).await; + node.transports.insert(TransportId::new(1), udp); + + let peer_addr = make_node_addr(0xBB); + let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr); + let transport_addr = TransportAddr::from_string("10.0.0.3:2121"); + + // Pre-populate with a tighter value, e.g. learned from discovery's + // reverse-path bottleneck. + node.path_mtu_lookup + .write() + .unwrap() + .insert(fips_addr, 1280); + + node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr); + + let stored = node + .path_mtu_lookup + .read() + .unwrap() + .get(&fips_addr) + .copied(); + assert_eq!( + stored, + Some(1280), + "Existing tighter value (1280) must not be loosened by direct-link seed (1452)" + ); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +#[tokio::test] +async fn test_seed_path_mtu_tightens_looser_existing_value() { + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp = make_udp_transport_with_mtu(1, 1280).await; + node.transports.insert(TransportId::new(1), udp); + + let peer_addr = make_node_addr(0xCC); + let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr); + let transport_addr = TransportAddr::from_string("10.0.0.4:2121"); + + // Pre-populate with a looser stale value. + node.path_mtu_lookup + .write() + .unwrap() + .insert(fips_addr, 1452); + + node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr); + + let stored = node + .path_mtu_lookup + .read() + .unwrap() + .get(&fips_addr) + .copied(); + assert_eq!( + stored, + Some(1280), + "Direct-link seed (1280) must overwrite looser existing value (1452)" + ); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +#[tokio::test] +async fn test_seed_path_mtu_noop_for_unknown_transport() { + let node = make_node(); + let peer_addr = make_node_addr(0xDD); + let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr); + let transport_addr = TransportAddr::from_string("10.0.0.5:2121"); + + // No transport registered — call must be a no-op, not panic. + node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(99), &transport_addr); + + let map = node.path_mtu_lookup.read().unwrap(); + assert!( + map.get(&fips_addr).is_none(), + "Seed must be a no-op when transport_id is not registered" + ); +} diff --git a/src/upper/tun.rs b/src/upper/tun.rs index 4cf7657..6822da6 100644 --- a/src/upper/tun.rs +++ b/src/upper/tun.rs @@ -13,6 +13,7 @@ use crate::FipsAddress; #[cfg(unix)] use crate::{FipsAddress, TunConfig}; +use std::collections::HashMap; #[cfg(unix)] use std::fs::File; #[cfg(unix)] @@ -23,7 +24,7 @@ use std::io::Write; use std::net::Ipv6Addr; #[cfg(unix)] use std::os::unix::io::{AsRawFd, FromRawFd}; -use std::sync::mpsc; +use std::sync::{Arc, RwLock, mpsc}; use thiserror::Error; #[cfg(unix)] use tracing::error; @@ -33,6 +34,105 @@ use tracing::{error, warn}; #[cfg(unix)] use tun::Layer; +/// Read-only handle to the per-destination path MTU map. Populated by +/// the discovery handler on `LookupResponse`; read by the TUN reader +/// (outbound clamp) and writer (inbound clamp) at TCP MSS clamp time. +/// Keyed by [`FipsAddress`] (16 bytes, the IPv6 form of a fips peer +/// address). +pub type PathMtuLookup = Arc>>; + +/// Compute the effective TCP MSS ceiling for a packet given its peer +/// address bytes (a 16-byte IPv6 destination on outbound, source on +/// inbound). Returns `min(global_max_mss, learned_path_max_mss)` when +/// the per-destination path MTU is known via discovery; otherwise +/// returns `min(global_max_mss, ipv6_minimum_safe_max_mss)`, the +/// conservative IPv6-minimum-derived ceiling. +/// +/// The conservative empty-lookup fallback exists because there is a +/// race window between TCP-SYN-out and discovery-completes-with-path- +/// MTU on cold flows. Without the floor, the first SYN exits at the +/// kernel-natural MSS (TUN MTU minus IPv6/TCP headers), which can +/// exceed what some downstream forwarder hop is willing to carry. +/// The drop is silent (no PTB feedback through the userspace TUN to +/// the kernel TCP stack), so TCP retransmits at the same too-large +/// MSS and the application's first connection wedges before discovery +/// completes for a corrected second SYN to fire. +/// +/// RFC 8200 mandates every IPv6 path accepts at least 1280-byte +/// packets, so a SYN clamped to the IPv6-minimum-derived MSS fits +/// any compliant path. Subsequent flows pick up the actual learned +/// per-destination value, which can be larger (when path supports +/// it) or smaller (when path is observed-tighter than the IPv6 min). +/// +/// Path MTU bytes-on-wire to TCP MSS: subtract 77 bytes of FIPS encap +/// overhead, then 40 bytes IPv6 + 20 bytes TCP headers. +pub(crate) fn per_flow_max_mss( + lookup: &PathMtuLookup, + addr_bytes: &[u8], + global_max_mss: u16, +) -> u16 { + use super::icmp::effective_ipv6_mtu; + + // RFC 8200 IPv6-minimum MTU (1280) → effective FIPS-encapsulated + // payload (1203) → TCP segment after IPv6+TCP headers (1143). + // Used as the conservative ceiling for empty-lookup destinations. + const IPV6_MIN_MTU: u16 = 1280; + let conservative_max_mss = effective_ipv6_mtu(IPV6_MIN_MTU) + .saturating_sub(40) + .saturating_sub(20); + let empty_lookup_ceiling = std::cmp::min(global_max_mss, conservative_max_mss); + + if addr_bytes.len() != 16 { + trace!( + len = addr_bytes.len(), + global_max_mss, + empty_lookup_ceiling, + "per_flow_max_mss: addr_bytes wrong length, fall back to conservative ceiling" + ); + return empty_lookup_ceiling; + } + let Ok(fips_addr) = FipsAddress::from_slice(addr_bytes) else { + trace!( + global_max_mss, + empty_lookup_ceiling, + "per_flow_max_mss: FipsAddress::from_slice rejected (non-fd::/8 prefix), fall back to conservative ceiling" + ); + return empty_lookup_ceiling; + }; + let Ok(map) = lookup.read() else { + trace!( + fips_addr = %fips_addr, + global_max_mss, + empty_lookup_ceiling, + "per_flow_max_mss: lookup read lock poisoned, fall back to conservative ceiling" + ); + return empty_lookup_ceiling; + }; + let Some(&path_mtu) = map.get(&fips_addr) else { + trace!( + fips_addr = %fips_addr, + global_max_mss, + empty_lookup_ceiling, + map_len = map.len(), + "per_flow_max_mss: no path_mtu_lookup entry for destination, fall back to conservative ceiling" + ); + return empty_lookup_ceiling; + }; + let path_max_mss = effective_ipv6_mtu(path_mtu) + .saturating_sub(40) + .saturating_sub(20); + let result = std::cmp::min(global_max_mss, path_max_mss); + trace!( + fips_addr = %fips_addr, + path_mtu, + path_max_mss, + global_max_mss, + result, + "per_flow_max_mss: per-destination clamp applied" + ); + result +} + /// Channel sender for packets to be written to TUN. pub type TunTx = mpsc::Sender>; @@ -224,8 +324,16 @@ impl TunDevice { /// can happen independently on separate threads. Returns the writer and /// a channel sender for submitting packets to be written. /// - /// The max_mss parameter is used for TCP MSS clamping on inbound packets. - pub fn create_writer(&self, max_mss: u16) -> Result<(TunWriter, TunTx), TunError> { + /// `max_mss` is the global TCP MSS ceiling derived from the local + /// `transport_mtu()` floor. `path_mtu_lookup` is a read-only handle to + /// the per-destination path MTU map populated by discovery; the writer + /// reads it on each inbound SYN-ACK to compute a per-flow ceiling that + /// honors learned narrow paths through the mesh. + pub fn create_writer( + &self, + max_mss: u16, + path_mtu_lookup: PathMtuLookup, + ) -> Result<(TunWriter, TunTx), TunError> { let fd = self.device.as_raw_fd(); // Duplicate the file descriptor for writing @@ -246,6 +354,7 @@ impl TunDevice { rx, name: self.name.clone(), max_mss, + path_mtu_lookup, }, tx, )) @@ -264,6 +373,7 @@ pub struct TunWriter { rx: mpsc::Receiver>, name: String, max_mss: u16, + path_mtu_lookup: PathMtuLookup, } #[cfg(unix)] @@ -279,11 +389,19 @@ impl TunWriter { debug!(name = %self.name, max_mss = self.max_mss, "TUN writer starting"); for mut packet in self.rx { + // Per-destination clamp: peer IPv6 source address (bytes 8..24) + // identifies the flow's remote end. If discovery has learned a + // smaller path MTU for that peer, tighten the ceiling. + let effective_max_mss = if packet.len() >= 24 { + per_flow_max_mss(&self.path_mtu_lookup, &packet[8..24], self.max_mss) + } else { + self.max_mss + }; // Clamp TCP MSS on inbound SYN-ACK packets - if clamp_tcp_mss(&mut packet, self.max_mss) { + if clamp_tcp_mss(&mut packet, effective_max_mss) { trace!( name = %self.name, - max_mss = self.max_mss, + max_mss = effective_max_mss, "Clamped TCP MSS in inbound SYN-ACK packet" ); } @@ -359,6 +477,7 @@ pub fn run_tun_reader( tun_tx: TunTx, outbound_tx: TunOutboundTx, transport_mtu: u16, + path_mtu_lookup: PathMtuLookup, ) { let (name, mut buf, max_mss) = tun_reader_setup(device.name(), mtu, transport_mtu); @@ -372,6 +491,7 @@ pub fn run_tun_reader( our_addr, &tun_tx, &outbound_tx, + &path_mtu_lookup, ) { break; } @@ -417,6 +537,7 @@ pub fn run_tun_reader( tun_tx: TunTx, outbound_tx: TunOutboundTx, transport_mtu: u16, + path_mtu_lookup: PathMtuLookup, shutdown_fd: std::os::unix::io::RawFd, ) { let _shutdown_fd = ShutdownFd(shutdown_fd); @@ -476,6 +597,7 @@ pub fn run_tun_reader( our_addr, &tun_tx, &outbound_tx, + &path_mtu_lookup, ) { return; // _shutdown_fd closes on drop } @@ -531,6 +653,7 @@ fn handle_tun_packet( our_addr: FipsAddress, tun_tx: &TunTx, outbound_tx: &TunOutboundTx, + path_mtu_lookup: &PathMtuLookup, ) -> bool { use super::icmp::{DestUnreachableCode, build_dest_unreachable, should_send_icmp_error}; use super::tcp_mss::clamp_tcp_mss; @@ -544,8 +667,11 @@ fn handle_tun_packet( // Check if destination is a FIPS address (fd::/8 prefix) if packet[24] == crate::identity::FIPS_ADDRESS_PREFIX { - if clamp_tcp_mss(packet, max_mss) { - trace!(name = %name, max_mss = max_mss, "Clamped TCP MSS in SYN packet"); + // Per-destination clamp: if discovery has learned a smaller path + // MTU for this destination, tighten the ceiling for this flow. + let effective_max_mss = per_flow_max_mss(path_mtu_lookup, &packet[24..40], max_mss); + if clamp_tcp_mss(packet, effective_max_mss) { + trace!(name = %name, max_mss = effective_max_mss, "Clamped TCP MSS in SYN packet"); } if outbound_tx.blocking_send(packet.to_vec()).is_err() { return false; // Channel closed, shutdown @@ -771,8 +897,14 @@ mod windows_tun { /// packets independently. Returns the writer and a channel sender for /// submitting packets to be written. /// - /// The `max_mss` parameter is used for TCP MSS clamping on inbound packets. - pub fn create_writer(&self, max_mss: u16) -> Result<(TunWriter, TunTx), TunError> { + /// `max_mss` is the global TCP MSS ceiling. `path_mtu_lookup` is a + /// read-only handle to per-destination path MTU learned via + /// discovery. + pub fn create_writer( + &self, + max_mss: u16, + path_mtu_lookup: PathMtuLookup, + ) -> Result<(TunWriter, TunTx), TunError> { let (tx, rx) = mpsc::channel(); Ok(( TunWriter { @@ -780,6 +912,7 @@ mod windows_tun { rx, name: self.name.clone(), max_mss, + path_mtu_lookup, }, tx, )) @@ -808,6 +941,7 @@ mod windows_tun { rx: mpsc::Receiver>, name: String, max_mss: u16, + path_mtu_lookup: PathMtuLookup, } impl TunWriter { @@ -816,16 +950,23 @@ mod windows_tun { /// Blocks forever, reading packets from the channel and writing them /// to the wintun session. Returns when the channel is closed. pub fn run(self) { + use super::per_flow_max_mss; use crate::upper::tcp_mss::clamp_tcp_mss; debug!(name = %self.name, max_mss = self.max_mss, "TUN writer starting"); for mut packet in self.rx { + // Per-destination clamp (peer source IPv6 = bytes 8..24) + let effective_max_mss = if packet.len() >= 24 { + per_flow_max_mss(&self.path_mtu_lookup, &packet[8..24], self.max_mss) + } else { + self.max_mss + }; // Clamp TCP MSS on inbound SYN-ACK packets - if clamp_tcp_mss(&mut packet, self.max_mss) { + if clamp_tcp_mss(&mut packet, effective_max_mss) { trace!( name = %self.name, - max_mss = self.max_mss, + max_mss = effective_max_mss, "Clamped TCP MSS in inbound SYN-ACK packet" ); } @@ -1232,4 +1373,111 @@ mod tests { // Note: TUN device creation tests require elevated privileges // and are better suited for integration tests. + + // ======================================================================== + // per_flow_max_mss — per-destination MSS clamp regression coverage + // ======================================================================== + + fn fips_addr_with_node_byte(b: u8) -> FipsAddress { + let mut bytes = [0u8; 16]; + bytes[0] = crate::identity::FIPS_ADDRESS_PREFIX; + bytes[1] = b; + FipsAddress::from_bytes(bytes).unwrap() + } + + fn empty_lookup() -> PathMtuLookup { + Arc::new(RwLock::new(HashMap::new())) + } + + #[test] + fn per_flow_empty_lookup_returns_conservative_ceiling() { + // Cold-flow first-SYN race-window guard: when no per-destination + // path_mtu has been learned yet, fall back to the IPv6-minimum- + // derived ceiling (1280 - 77 - 60 = 1143) rather than the local + // global ceiling. This ensures the first SYN to an unknown + // destination clamps small enough to traverse any RFC-8200- + // compliant IPv6 path. + let lookup = empty_lookup(); + let addr = fips_addr_with_node_byte(0x42); + assert_eq!(per_flow_max_mss(&lookup, addr.as_bytes(), 1360), 1143); + } + + #[test] + fn per_flow_empty_lookup_returns_global_when_global_smaller() { + // When the local global ceiling is already <= the conservative + // 1143 ceiling (e.g. a daemon configured with UDP-1280 only), + // the empty-lookup fallback stays at the global rather than + // expanding upward. + let lookup = empty_lookup(); + let addr = fips_addr_with_node_byte(0x42); + assert_eq!(per_flow_max_mss(&lookup, addr.as_bytes(), 1100), 1100); + } + + #[test] + fn per_flow_clamps_to_path_mtu_when_smaller() { + // Discovery learned path_mtu=1280 for this destination; global + // ceiling is 1360. Per-flow clamp should be min(1360, 1280-77-60) + // = min(1360, 1143) = 1143. + let lookup = empty_lookup(); + let addr = fips_addr_with_node_byte(0x42); + lookup.write().unwrap().insert(addr, 1280); + assert_eq!(per_flow_max_mss(&lookup, addr.as_bytes(), 1360), 1143); + } + + #[test] + fn per_flow_keeps_global_when_path_mtu_larger() { + // Discovery learned path_mtu=1452 (> global). Per-flow stays at + // global 1143 (the smaller of the two). + let lookup = empty_lookup(); + let addr = fips_addr_with_node_byte(0x42); + lookup.write().unwrap().insert(addr, 1452); + // global=1143 (UDP-1280-derived); path_max = 1452-77-60 = 1315. + assert_eq!(per_flow_max_mss(&lookup, addr.as_bytes(), 1143), 1143); + } + + #[test] + fn per_flow_learned_value_overrides_conservative_ceiling() { + // When discovery has learned a per-destination value LARGER than + // the conservative 1143 ceiling, the learned value (capped by + // the global ceiling) wins. The conservative ceiling is only the + // empty-lookup fallback; once an entry exists, the actual + // learned value governs. + let lookup = empty_lookup(); + let addr = fips_addr_with_node_byte(0x42); + lookup.write().unwrap().insert(addr, 1452); + // global=1360, path_max = 1452-77-60 = 1315; min(1360, 1315) = 1315. + // 1315 > 1143, so the conservative ceiling did NOT clamp here. + assert_eq!(per_flow_max_mss(&lookup, addr.as_bytes(), 1360), 1315); + } + + #[test] + fn per_flow_returns_conservative_ceiling_for_non_fips_addr() { + // Non-fips IPv6 (e.g. fe80::/10 link-local) takes the empty- + // lookup path. With global=1360, fall back to 1143. + let lookup = empty_lookup(); + let mut bytes = [0u8; 16]; + bytes[0] = 0xfe; + bytes[1] = 0x80; + assert_eq!(per_flow_max_mss(&lookup, &bytes, 1360), 1143); + } + + #[test] + fn per_flow_returns_conservative_ceiling_on_short_addr_slice() { + let lookup = empty_lookup(); + let bytes = [0u8; 8]; + assert_eq!(per_flow_max_mss(&lookup, &bytes, 1360), 1143); + } + + #[test] + fn per_flow_independent_per_destination() { + // Two different destinations with different path MTUs. Each + // lookup honors its own value; cross-talk would be a regression. + let lookup = empty_lookup(); + let a = fips_addr_with_node_byte(0x10); + let b = fips_addr_with_node_byte(0x20); + lookup.write().unwrap().insert(a, 1280); + lookup.write().unwrap().insert(b, 1452); + assert_eq!(per_flow_max_mss(&lookup, a.as_bytes(), 1360), 1143); + assert_eq!(per_flow_max_mss(&lookup, b.as_bytes(), 1360), 1315); + } }