From ae607431ebd45d1316182c322f829a43b9b67068 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 2 May 2026 17:37:07 +0000 Subject: [PATCH] Per-destination TCP MSS clamping at the TUN boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds source-side TCP MSS clamping informed by per-destination path MTU learned via discovery, with a conservative IPv6-minimum-derived ceiling for cold flows where discovery has not yet completed. Closes the multi-hop default-config TCP wedges observed in production where a sender's local-floor MSS exceeds what some intermediate forwarder hop is willing to carry: silent drops, no PTB feedback through the userspace TUN to the kernel TCP stack, retransmits at the same too- large MSS, application connection times out. ## Architecture A new `Arc>>` field `path_mtu_lookup` on Node mirrors the per-destination path MTU in a form accessible from sync TUN reader/writer threads. A new `per_flow_max_mss` helper in `src/upper/tun.rs` reads the lookup at SYN-clamp time and returns the appropriate ceiling for the flow. Three write sites populate `path_mtu_lookup`: 1. **Discovery originator branch** of `handle_lookup_response`: the path MTU bottleneck accumulated through the reverse path lands here when a LookupResponse arrives at the originator. Same value also lands in `coord_cache` per the existing `insert_with_path_mtu` API. 2. **FMP peer-promotion seed** (`seed_path_mtu_for_link_peer`): when an FMP link-layer peer is promoted to active, the local outgoing-link MTU on the peer's transport seeds the lookup. Tighter existing values (learned via discovery) are preserved; the seed only writes when no entry exists or the existing value is looser than the link MTU. Without this seed, directly-configured peers (auto_connect / static peer config) would leave `path_mtu_lookup` empty for their FipsAddress because the FSP session establishes without ever issuing a LookupRequest. 3. **Target-edge fold at `send_lookup_response`**: when a node is the discovery target, it folds its own outgoing-link MTU to the response's next-hop into `path_mtu` before sending. Without this fold, the response leaves the target with `path_mtu = u16::MAX` and only intermediate transits min-fold; the target's first reverse-path hop is never represented in the bottleneck calculation. Refactored the existing transit- side min-fold into a shared `apply_outgoing_link_mtu_to_response` helper called from both sites. ## Read-side: per_flow_max_mss Two TUN call sites consume the lookup: - Outbound `handle_tun_packet` clamps SYN MSS using packet[24..40] (IPv6 destination) as the lookup key. - Inbound `TunWriter::run` clamps SYN-ACK MSS using packet[8..24] (IPv6 source). When the lookup contains a learned value, the helper computes `min(global_max_mss, effective_ipv6_mtu(path_mtu) - 60)` where 60 is IPv6 (40) + TCP (20) headers and `effective_ipv6_mtu` accounts for the FIPS encapsulation overhead. When the lookup is empty for a destination — the cold-flow case — the helper returns `min(global_max_mss, IPv6-minimum-derived ceiling)`. RFC 8200 mandates every IPv6 path accept ≥1280-byte packets, so the IPv6-minimum-derived MSS (1280 - 77 - 60 = 1143) fits any compliant path. Without this conservative ceiling, the first SYN to a destination with no learned path MTU exits the TUN at the kernel-natural MSS (TUN MTU - 60), and the application connection wedges silently before discovery completes for a corrected second SYN to fire. The fix is provably safe: the ceiling is taken with `min` against the local global so operators with even tighter local floors are never loosened upward. Subsequent flows pick up the actual learned per-destination value once discovery (or the FMP-promotion seed for direct peers) populates the lookup. ## Diagnostic logging All write and read sites emit instrumentation suitable for operators bisecting a wedged path: - `debug!` log on every `path_mtu_lookup` write (discovery originator path and FMP-promotion seed path), showing the FipsAddress, written value, prior value, and post-write map size. `warn!` on poisoned-lock failure path. - `trace!` log per `per_flow_max_mss` call covering every fall-through branch (wrong addr_bytes length, non-fd::/8 prefix, lookup poisoned, no entry for destination, empty-lookup conservative ceiling) and the success path. trace level filters out under normal log settings; capture with `RUST_LOG=info,fips::node::handlers::discovery=debug,fips::upper::tun=trace`. ## Tests 15 new unit tests across 3 files: - `per_flow_max_mss` (8 tests in `src/upper/tun.rs::tests`): empty-lookup conservative ceiling, empty-lookup global-smaller floor, learned-value-overrides-conservative, per-destination smaller, per-destination larger capped by global, non-fips addr, short addr slice, per-destination independence. - `seed_path_mtu_for_link_peer` (4 tests in `src/node/tests/unit.rs`): seed when empty, keep tighter existing, tighten looser existing, no-op for unknown transport. - Discovery integration (3 tests in `src/node/tests/discovery.rs`): apply_outgoing_link_mtu_to_response on unknown peer no-op, two-node target-edge fold (path_mtu reflects target-edge link), three-node chain transit min-fold (existing test, updated for target-edge inclusion). Two pre-existing discovery tests had assertions updated to account for the target-edge fold: - `test_response_path_mtu_two_node`: previously asserted `u16::MAX` (no transit to min-fold); now asserts 1280 (the test transport MTU, folded in by send_lookup_response). - `test_response_path_mtu_four_node_chain`: previously asserted 1350 (transit MTUs only); now asserts 1280 (target-edge MTU is the bottleneck). - `test_transit_forwards_when_mtu_sufficient`: previously asserted 1400 (transit MTU only); now asserts 1280 (target- edge MTU is the bottleneck). ## Verification Local CI on this commit: 29/29 suites pass, 1105 lib tests pass, clippy --all-targets --all-features -D warnings clean, cargo fmt clean. Production deploy verified via trace capture across the managed fleet: cold-flow conservative ceiling branch fires on first SYN, learned-lookup branch takes over once discovery completes, both behaviors observable end-to-end at the SYN MSS on the wire. No wire-format change. No config-format change. --- src/node/handlers/discovery.rs | 134 ++++++++++++++-- src/node/handlers/handshake.rs | 4 + src/node/lifecycle.rs | 10 +- src/node/mod.rs | 8 + src/node/tests/discovery.rs | 44 ++++-- src/node/tests/unit.rs | 135 +++++++++++++++++ src/upper/tun.rs | 270 +++++++++++++++++++++++++++++++-- 7 files changed, 568 insertions(+), 37 deletions(-) diff --git a/src/node/handlers/discovery.rs b/src/node/handlers/discovery.rs index cb478a3..d4ff004 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" ); @@ -573,6 +599,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 1ca9a4d..ae2cc45 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -1012,6 +1012,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, @@ -1107,6 +1109,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 5d1d7b4..95ca007 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -624,8 +624,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 || { @@ -641,6 +644,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( @@ -650,6 +654,7 @@ impl Node { reader_tun_tx, outbound_tx, transport_mtu, + path_mtu_lookup, shutdown_read_fd, ); }); @@ -662,6 +667,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 34c5f12..240690d 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -301,6 +301,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). @@ -616,6 +622,7 @@ impl Node { peer_aliases: HashMap::new(), peer_acl, host_map, + path_mtu_lookup: Arc::new(std::sync::RwLock::new(HashMap::new())), }) } @@ -745,6 +752,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 0179cbf..b444ad6 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -710,14 +710,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; @@ -740,21 +739,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 diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index ce90b2a..43d5944 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -1030,3 +1030,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); + } }