mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-06 03:28:24 +00:00
Bound a remote-supplied path MTU, and give the cache a way back
The path_mtu field is an unsigned per-hop transit annotation carried outside the signed proof, and the MtuExceeded and PathBroken signals arrive unencrypted with no sender check, so any forwarder could lower it and it was accepted with no minimum. A single MtuExceeded carrying a very small value drove a session's path MTU to zero, after which every packet to that destination was answered with an ICMPv6 Packet Too Big instead of being sent: a blackhole lasting until the daemon restarted. The same value reached the SYN-time TCP MSS clamp, where anything at or below 137 saturates to a segment size of zero and the band just above it yields single digits. Ignore values below an actionable minimum rather than applying or storing them, at the three places a remote value is acted on: the path MTU state machine, the reactive MtuExceeded write, and the discovery response, whose coordinates are still cached so refusing the annotation cannot become a way to deny discovery. The MSS clamp additionally refuses to write a zero. Ignoring rather than clamping is deliberate: a clamp would fabricate an estimate the node has no basis for. Release a stored per-destination path MTU when the path is invalidated by a PathBroken report, by session idle expiry, or by handshake timeout, and reseed the link MTU read from local configuration in its place, so a directly connected peer does not lose its own measurement along with the remote claim. Locally derived MTUs are not subject to the minimum, and adaptation to hops well below the IPv6 minimum continues to work. Count and name the path MTU values ignored as below the floor Three sites ignore a remote-supplied path MTU below the actionable floor and one releases a stale entry, and an operator had no way to see any of it happening. Count each site and log the value seen with the destination, so a node being fed poison is distinguishable from a node on a quiet link. Clamp on the degenerate case, not on the remote floor The floor guard at the MSS clamp site tested the stored value alone, with no test of where it came from, so it also rejected the link MTU the node seeds from its own transport. That contradicts the contract the constant's own doc states, and it bites hardest on BLE, where the seeded value is negotiated with the peer rather than read from config: a peer whose effective MTU negotiated to 240 lost a correct 103-byte clamp and got the 1143-byte conservative ceiling, after which every full-size segment was refused by the transport with no signal and no feedback to the application. Every remote value is already refused at the three ingress guards, so a sub-floor value reaching the clamp is by construction a local one. The clamp therefore needs only to refuse the degenerate case it was really about, where no payload byte survives the arithmetic at all. Also retarget the seed-site warning, which fired on the same wrong threshold and advised checking a transport setting that a negotiated link does not have, and correct the constant's doc, which contradicted its own formula about where the single-digit band ends.
This commit is contained in:
@@ -228,6 +228,45 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Security
|
||||
|
||||
- The influence a remote party has over path MTU is now bounded, and the
|
||||
per-destination path MTU cache has a way back. The `path_mtu` field is an
|
||||
unsigned per-hop transit annotation carried outside the signed proof, and the
|
||||
`MtuExceeded` and `PathBroken` signals arrive unencrypted with no sender
|
||||
check, so any forwarder — or anyone who can reach the node — could lower it,
|
||||
and it was accepted with no minimum. A single `MtuExceeded` carrying a very
|
||||
small value drove a session's path MTU to zero, after which every packet to
|
||||
that destination was answered with an ICMPv6 Packet Too Big instead of being
|
||||
sent: a blackhole that lasted until the daemon restarted. The same value
|
||||
reached the SYN-time TCP MSS clamp, where anything at or below 137 saturates
|
||||
to a segment size of zero and the band just above it yields single digits.
|
||||
Values below an actionable minimum are now ignored rather than applied or
|
||||
stored, at the three places a remote value is acted on: the path MTU state
|
||||
machine, the reactive `MtuExceeded` write, and the discovery response, whose
|
||||
coordinates are still cached so refusing the annotation cannot become a way
|
||||
to deny discovery. The MSS clamp additionally refuses to write a zero. Each
|
||||
of the three refusals logs a warning and increments its own counter in the
|
||||
error-signal family, so an operator can tell them apart without scraping
|
||||
logs: they carry different meanings, one being an authenticated peer inside
|
||||
an established session, one an unencrypted signal anyone able to reach the
|
||||
node can send at will, and one a verified discovery response whose unsigned
|
||||
annotation a forwarder on the reverse path rewrote. Because those three
|
||||
refusals are the only way a remote value reaches the per-destination store,
|
||||
the SYN-time clamp does not apply the minimum a second time when it reads
|
||||
that store: a small value there is one the node derived from its own outgoing
|
||||
link, which is exact rather than suspect, and BLE in particular negotiates a
|
||||
link MTU per connection that lands under the minimum routinely. The clamp
|
||||
refuses only a stored value admitting no TCP payload byte at all, at 137 or
|
||||
below, where the segment size saturates to zero and the clamp would be
|
||||
skipped entirely; it logs that at trace rather than warn, since it sits on
|
||||
the per-packet path, and the peer's link promotion reports it once instead.
|
||||
A stored per-destination path MTU is released when the path is invalidated by
|
||||
a `PathBroken` report, by session idle expiry, or by handshake timeout, and
|
||||
the link MTU read from the local transport is reseeded in its place, so a
|
||||
directly connected peer does not lose its own measurement along with the
|
||||
remote claim. Locally derived MTUs are not subject to the minimum, at the
|
||||
seed or at the clamp. Legitimate narrow paths are unaffected: adaptation to
|
||||
hops well below the IPv6 minimum, which the mesh does use, continues to work.
|
||||
|
||||
- The FSP session address is now bound to the peer key the Noise handshake
|
||||
authenticated, on both the initial and the rekey path. The responder recorded
|
||||
a session under the source address carried in the datagram without ever
|
||||
|
||||
@@ -291,6 +291,9 @@ fn draw_routing_stats(
|
||||
("Coords Required", err("coords_required")),
|
||||
("Path Broken", err("path_broken")),
|
||||
("MTU Exceeded", err("mtu_exceeded")),
|
||||
("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")),
|
||||
],
|
||||
));
|
||||
right.push(Line::from(""));
|
||||
|
||||
@@ -33,8 +33,11 @@
|
||||
},
|
||||
"error_signals": {
|
||||
"coords_required": 0,
|
||||
"lookup_resp_mtu_below_floor": 0,
|
||||
"mtu_exceeded": 0,
|
||||
"path_broken": 0
|
||||
"mtu_exceeded_below_floor": 0,
|
||||
"path_broken": 0,
|
||||
"path_mtu_notif_below_floor": 0
|
||||
},
|
||||
"forwarding": {
|
||||
"decode_error_bytes": 0,
|
||||
|
||||
+75
-1
@@ -435,7 +435,20 @@ impl PathMtuState {
|
||||
/// value, spanning at least 2 * notification_interval.
|
||||
///
|
||||
/// Returns `true` if the effective MTU changed.
|
||||
///
|
||||
/// A reported value below [`MIN_ACTIONABLE_PATH_MTU`] is ignored entirely.
|
||||
/// The notification carries a remote party's claim about the path, and
|
||||
/// below that floor the claim cannot describe a usable path: acting on it
|
||||
/// drives the send gate into answering every packet with an ICMPv6 Packet
|
||||
/// Too Big instead of sending it. Returning `false` leaves whatever the
|
||||
/// local seed established and correctly reports "no change".
|
||||
///
|
||||
/// [`MIN_ACTIONABLE_PATH_MTU`]: crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU
|
||||
pub fn apply_notification(&mut self, reported_mtu: u16, now: Instant) -> bool {
|
||||
if reported_mtu < crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU {
|
||||
return false;
|
||||
}
|
||||
|
||||
if reported_mtu < self.current_mtu {
|
||||
// Decrease: immediate
|
||||
self.current_mtu = reported_mtu;
|
||||
@@ -447,7 +460,7 @@ impl PathMtuState {
|
||||
if reported_mtu > self.current_mtu {
|
||||
// Increase: track consecutive notifications
|
||||
if reported_mtu == self.pending_increase_mtu {
|
||||
self.consecutive_increase_count += 1;
|
||||
self.consecutive_increase_count = self.consecutive_increase_count.saturating_add(1);
|
||||
} else {
|
||||
// Different value: reset sequence
|
||||
self.pending_increase_mtu = reported_mtu;
|
||||
@@ -552,4 +565,65 @@ owd_window_size: 48
|
||||
assert_eq!(config.log_interval_secs, DEFAULT_LOG_INTERVAL_SECS);
|
||||
assert_eq!(config.owd_window_size, DEFAULT_OWD_WINDOW_SIZE);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_notification_ignores_a_decrease_below_the_actionable_floor() {
|
||||
// The reported value comes from a remote party over an unauthenticated
|
||||
// signal. Driving current_mtu into this band turns the TUN send gate
|
||||
// into a blackhole: every packet is answered with a Packet Too Big
|
||||
// instead of being sent. Sub-floor values are ignored outright, not
|
||||
// clamped, so the locally seeded value survives untouched.
|
||||
for reported in [0u16, 1, 137, 138, 200, 255] {
|
||||
let mut state = PathMtuState::new();
|
||||
state.seed_source_mtu(1400);
|
||||
|
||||
let changed = state.apply_notification(reported, Instant::now());
|
||||
|
||||
assert!(
|
||||
!changed,
|
||||
"reported path MTU {reported} is below the floor and must report no change"
|
||||
);
|
||||
assert_eq!(
|
||||
state.current_mtu(),
|
||||
1400,
|
||||
"reported path MTU {reported} must leave the seeded value intact"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_notification_accepts_a_decrease_at_the_actionable_floor() {
|
||||
// The floor must not swallow the smallest value the node does act on,
|
||||
// nor the legitimately narrow hops the mesh actually carries.
|
||||
for reported in [crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU, 576, 800] {
|
||||
let mut state = PathMtuState::new();
|
||||
state.seed_source_mtu(1400);
|
||||
|
||||
let changed = state.apply_notification(reported, Instant::now());
|
||||
|
||||
assert!(changed, "reported path MTU {reported} must be applied");
|
||||
assert_eq!(state.current_mtu(), reported);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_notification_increase_counter_does_not_overflow_on_a_repeated_value() {
|
||||
// The counter is a u8 and resets only when the value changes or the
|
||||
// increase is accepted. Acceptance additionally requires the sequence
|
||||
// to span two notification intervals, so a peer repeating one higher
|
||||
// value fast enough stays in the increase branch indefinitely.
|
||||
let mut state = PathMtuState::new();
|
||||
state.seed_source_mtu(1000);
|
||||
|
||||
let now = Instant::now();
|
||||
for _ in 0..600 {
|
||||
state.apply_notification(1200, now);
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
state.current_mtu(),
|
||||
1000,
|
||||
"the increase is not yet due, so the effective MTU must be unchanged"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -233,47 +233,73 @@ impl Node {
|
||||
"Discovery succeeded, proof verified, route cached"
|
||||
);
|
||||
|
||||
self.coord_cache
|
||||
.insert_with_path_mtu(target, response.target_coords, now_ms, path_mtu);
|
||||
// The annotation is unsigned and accumulates hop by hop, so any
|
||||
// forwarder on the reverse path can lower it. A value below the
|
||||
// actionable floor cannot describe a usable path, so treat it as
|
||||
// absent: cache the coordinates, which are what the proof covers,
|
||||
// and store no path MTU from this response at all.
|
||||
let path_mtu_actionable = path_mtu >= crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU;
|
||||
if path_mtu_actionable {
|
||||
self.coord_cache.insert_with_path_mtu(
|
||||
target,
|
||||
response.target_coords,
|
||||
now_ms,
|
||||
path_mtu,
|
||||
);
|
||||
} else {
|
||||
warn!(
|
||||
request_id = response.request_id,
|
||||
target = %self.peer_display_name(&target),
|
||||
path_mtu = path_mtu,
|
||||
floor = crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU,
|
||||
"LookupResponse carries a path MTU below the actionable floor; \
|
||||
caching coordinates without it"
|
||||
);
|
||||
self.metrics().errors.lookup_resp_mtu_below_floor.inc();
|
||||
self.coord_cache
|
||||
.insert(target, response.target_coords, now_ms);
|
||||
}
|
||||
|
||||
// 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) => match map.get(&fips_addr).copied() {
|
||||
Some(existing) if existing <= path_mtu => {
|
||||
// Keep the tighter learned value; never loosen the
|
||||
// clamp. A reactive MtuExceeded or PathMtuNotification
|
||||
// tighten takes precedence over a looser discovery
|
||||
// estimate (cross-carrier keep-tighter).
|
||||
debug!(
|
||||
if path_mtu_actionable {
|
||||
match self.path_mtu_lookup.write() {
|
||||
Ok(mut map) => match map.get(&fips_addr).copied() {
|
||||
Some(existing) if existing <= path_mtu => {
|
||||
// Keep the tighter learned value; never loosen the
|
||||
// clamp. A reactive MtuExceeded or PathMtuNotification
|
||||
// tighten takes precedence over a looser discovery
|
||||
// estimate (cross-carrier keep-tighter).
|
||||
debug!(
|
||||
target = %self.peer_display_name(&target),
|
||||
fips_addr = %fips_addr,
|
||||
path_mtu = path_mtu,
|
||||
existing = existing,
|
||||
"LookupResponse: keeping tighter existing path_mtu_lookup value"
|
||||
);
|
||||
}
|
||||
other => {
|
||||
map.insert(fips_addr, path_mtu);
|
||||
debug!(
|
||||
target = %self.peer_display_name(&target),
|
||||
fips_addr = %fips_addr,
|
||||
path_mtu = path_mtu,
|
||||
prior = ?other,
|
||||
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,
|
||||
existing = existing,
|
||||
"LookupResponse: keeping tighter existing path_mtu_lookup value"
|
||||
error = %e,
|
||||
"path_mtu_lookup write lock poisoned; clamp will not see this update"
|
||||
);
|
||||
}
|
||||
other => {
|
||||
map.insert(fips_addr, path_mtu);
|
||||
debug!(
|
||||
target = %self.peer_display_name(&target),
|
||||
fips_addr = %fips_addr,
|
||||
path_mtu = path_mtu,
|
||||
prior = ?other,
|
||||
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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -698,6 +724,22 @@ impl Node {
|
||||
return;
|
||||
};
|
||||
let link_mtu = transport.link_mtu(addr);
|
||||
// A locally derived MTU is deliberately exempt from the actionable
|
||||
// floor, so this seeds the value either way, and a narrow link is not
|
||||
// by itself worth reporting: BLE negotiates its MTU per connection and
|
||||
// lands below the floor routinely, where the tight clamp the seed
|
||||
// produces is exactly what the flow needs. Warn only where the link
|
||||
// admits no TCP payload byte at all, since there the SYN-time clamp
|
||||
// has nothing usable to derive and drops the peer onto the
|
||||
// conservative fallback ceiling for as long as the link stands.
|
||||
if crate::upper::icmp::mss_ceiling(link_mtu) == 0 {
|
||||
warn!(
|
||||
peer = %self.peer_display_name(peer_addr),
|
||||
link_mtu = link_mtu,
|
||||
"Link MTU leaves no room for a TCP payload byte; TCP to this peer \
|
||||
will not work until the link or the transport's mtu setting changes"
|
||||
);
|
||||
}
|
||||
let fips_addr = crate::FipsAddress::from_node_addr(peer_addr);
|
||||
let Ok(mut map) = self.path_mtu_lookup.write() else {
|
||||
warn!(
|
||||
|
||||
@@ -1081,6 +1081,23 @@ impl Node {
|
||||
return;
|
||||
};
|
||||
|
||||
// `apply_notification` refuses a sub-floor value, but it returns the
|
||||
// same `false` it returns for the ordinary "no change" case, which is
|
||||
// the common one. Test the floor here so the refusal is visible: this
|
||||
// arrives on the decrypted service-payload path, so a value this low
|
||||
// means an authenticated peer we hold a session with is sending
|
||||
// something unusable.
|
||||
if notif.path_mtu < crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU {
|
||||
warn!(
|
||||
src = %peer_name,
|
||||
reported_mtu = notif.path_mtu,
|
||||
floor = crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU,
|
||||
"PathMtuNotification reports a path MTU below the actionable floor; ignoring"
|
||||
);
|
||||
self.metrics.errors.path_mtu_notif_below_floor.inc();
|
||||
return;
|
||||
}
|
||||
|
||||
let old_mtu = mmp.path_mtu.current_mtu();
|
||||
let now = std::time::Instant::now();
|
||||
let changed = mmp.path_mtu.apply_notification(notif.path_mtu, now);
|
||||
@@ -1206,7 +1223,7 @@ 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.
|
||||
async fn handle_path_broken(&mut self, inner: &[u8]) {
|
||||
pub(in crate::node) async fn handle_path_broken(&mut self, inner: &[u8]) {
|
||||
self.metrics().errors.path_broken.inc();
|
||||
|
||||
let msg = match PathBroken::decode(inner) {
|
||||
@@ -1243,6 +1260,10 @@ impl Node {
|
||||
// Invalidate stale cached coordinates
|
||||
self.coord_cache.remove(&msg.dest_addr);
|
||||
|
||||
// The path this destination's stored MTU described is gone, so release
|
||||
// it rather than carrying it onto whatever path replaces it.
|
||||
self.path_mtu_lookup_release(&msg.dest_addr);
|
||||
|
||||
// Trigger re-discovery to get fresh coordinates, but only if we have
|
||||
// the target's identity cached — otherwise we can't verify the
|
||||
// LookupResponse proof. This avoids a race when the XK responder
|
||||
@@ -1309,6 +1330,22 @@ impl Node {
|
||||
}
|
||||
}
|
||||
|
||||
// The lookup write below is not gated on a session existing, so an
|
||||
// unencrypted MtuExceeded from anyone reaches it. Refuse to store a
|
||||
// bottleneck too small to describe a usable path; a stored value that
|
||||
// low drives the SYN-time MSS clamp into single digits or zero.
|
||||
if msg.mtu < crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU {
|
||||
warn!(
|
||||
dest = %peer_name,
|
||||
reporter = %msg.reporter,
|
||||
bottleneck_mtu = msg.mtu,
|
||||
floor = crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU,
|
||||
"MtuExceeded reports a path MTU below the actionable floor; ignoring"
|
||||
);
|
||||
self.metrics().errors.mtu_exceeded_below_floor.inc();
|
||||
return;
|
||||
}
|
||||
|
||||
// Mirror the bottleneck into the FipsAddress-keyed lookup used by
|
||||
// the TUN reader/writer at TCP MSS clamp time. Discovery's reverse-
|
||||
// path response can carry a value too generous for the actual
|
||||
|
||||
@@ -208,6 +208,7 @@ impl Node {
|
||||
info!(dest = %name, "Session handshake timed out, removing");
|
||||
self.sessions.remove(addr);
|
||||
self.pending_tun_packets.remove(addr);
|
||||
self.path_mtu_lookup_release(addr);
|
||||
}
|
||||
|
||||
// Second pass: collect resend candidates
|
||||
@@ -285,6 +286,7 @@ impl Node {
|
||||
}
|
||||
self.sessions.remove(&addr);
|
||||
self.pending_tun_packets.remove(&addr);
|
||||
self.path_mtu_lookup_release(&addr);
|
||||
debug!(
|
||||
dest = %name,
|
||||
idle_secs = timeout_ms / 1000,
|
||||
|
||||
@@ -459,6 +459,21 @@ pub struct ErrorMetrics {
|
||||
pub coords_required: Counter,
|
||||
pub path_broken: Counter,
|
||||
pub mtu_exceeded: Counter,
|
||||
/// `PathMtuNotification`s ignored for carrying a path MTU below the
|
||||
/// actionable floor. This signal arrives inside an established session
|
||||
/// on the decrypted path, so a rising count means an authenticated peer
|
||||
/// is misconfigured or misbehaving.
|
||||
pub path_mtu_notif_below_floor: Counter,
|
||||
/// `MtuExceeded` signals whose bottleneck was ignored for falling below
|
||||
/// the actionable floor. The signal is unencrypted, unauthenticated and
|
||||
/// unmetered, so a rising count on its own is the forged-signal
|
||||
/// signature; `mtu_exceeded` counts the whole population.
|
||||
pub mtu_exceeded_below_floor: Counter,
|
||||
/// `LookupResponse` path MTU annotations ignored for falling below the
|
||||
/// actionable floor. The response carried a verified proof, so a rising
|
||||
/// count means a forwarder on the reverse path is mangling the unsigned
|
||||
/// annotation.
|
||||
pub lookup_resp_mtu_below_floor: Counter,
|
||||
}
|
||||
|
||||
impl ErrorMetrics {
|
||||
@@ -468,6 +483,9 @@ impl ErrorMetrics {
|
||||
coords_required: self.coords_required.get(),
|
||||
path_broken: self.path_broken.get(),
|
||||
mtu_exceeded: self.mtu_exceeded.get(),
|
||||
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(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2545,6 +2545,50 @@ impl Node {
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop the remote-learned path MTU for a destination whose path is no
|
||||
/// longer valid, then restore what is known locally.
|
||||
///
|
||||
/// Entries in `path_mtu_lookup` come from two sources: values a remote
|
||||
/// party supplied (discovery responses, `MtuExceeded`, path MTU
|
||||
/// notifications) and the link MTU this node reads from its own transport
|
||||
/// configuration for a directly connected peer. When the path is declared
|
||||
/// broken or the session goes away, the remote-supplied value describes a
|
||||
/// path that no longer exists and must not outlive it, but the locally
|
||||
/// derived one is still true. Removing the entry and then re-running the
|
||||
/// link-peer seed keeps the second while discarding the first; a plain
|
||||
/// removal would silently drop a direct peer back to the conservative
|
||||
/// ceiling until its link re-handshakes.
|
||||
fn path_mtu_lookup_release(&self, addr: &NodeAddr) {
|
||||
let fips_addr = crate::FipsAddress::from_node_addr(addr);
|
||||
match self.path_mtu_lookup.write() {
|
||||
Ok(mut map) => {
|
||||
if map.remove(&fips_addr).is_some() {
|
||||
tracing::debug!(
|
||||
dest = %self.peer_display_name(addr),
|
||||
fips_addr = %fips_addr,
|
||||
"Released path_mtu_lookup entry for an invalidated path"
|
||||
);
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
fips_addr = %fips_addr,
|
||||
error = %e,
|
||||
"path_mtu_lookup write lock poisoned; entry not released"
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
// The write guard above must be dropped before the seed runs: it takes
|
||||
// the same lock, and `std::sync::RwLock` is not re-entrant.
|
||||
if let Some(peer) = self.peers.get(addr)
|
||||
&& let Some(transport_id) = peer.transport_id()
|
||||
&& let Some(transport_addr) = peer.current_addr().cloned()
|
||||
{
|
||||
self.seed_path_mtu_for_link_peer(addr, transport_id, &transport_addr);
|
||||
}
|
||||
}
|
||||
|
||||
/// Number of end-to-end sessions.
|
||||
pub fn session_count(&self) -> usize {
|
||||
self.sessions.len()
|
||||
|
||||
@@ -38,7 +38,11 @@
|
||||
//!
|
||||
//! - `path_mtu_lookup` is an event-driven cache (`Arc<RwLock<HashMap>>`)
|
||||
//! populated from observed path-MTU discovery traffic, not loaded from a
|
||||
//! file. There is nothing to poll. (Its read side could adopt the same
|
||||
//! file. There is nothing to poll. Release is event-driven for the same
|
||||
//! reason: an entry is dropped when the path it describes is declared
|
||||
//! invalid (a `PathBroken` report, session idle expiry, or handshake
|
||||
//! timeout) and the locally derived link MTU is reseeded in its place, so
|
||||
//! there is no expiry sweep either. (Its read side could adopt the same
|
||||
//! lock-free `ArcSwap` shape in the future, but that is an optimization, not
|
||||
//! a reload.)
|
||||
//! - `nostr_discovery` is an async spawned subsystem, not a snapshot of disk
|
||||
|
||||
@@ -343,6 +343,9 @@ pub struct ErrorSignalStatsSnapshot {
|
||||
pub coords_required: u64,
|
||||
pub path_broken: u64,
|
||||
pub mtu_exceeded: u64,
|
||||
pub path_mtu_notif_below_floor: u64,
|
||||
pub mtu_exceeded_below_floor: u64,
|
||||
pub lookup_resp_mtu_below_floor: u64,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default, Serialize)]
|
||||
|
||||
@@ -912,6 +912,95 @@ async fn test_originator_stores_path_mtu_in_cache() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_originator_ignores_sub_floor_path_mtu_but_still_caches_coords() {
|
||||
// The path_mtu annotation accumulates hop by hop outside the signed proof,
|
||||
// so any forwarder on the reverse path can lower it. A value below the
|
||||
// actionable floor must be treated as absent rather than stored — but the
|
||||
// coordinates it travelled with are proof-covered and must still land,
|
||||
// otherwise a value-poisoning vector becomes a discovery-denial one.
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
|
||||
let target_identity = Identity::generate();
|
||||
let target = *target_identity.node_addr();
|
||||
let target_fips = crate::FipsAddress::from_node_addr(&target);
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
|
||||
node.register_identity(target, target_identity.pubkey_full());
|
||||
|
||||
let proof_data = LookupResponse::proof_bytes(801, &target, &coords);
|
||||
let proof = target_identity.sign(&proof_data);
|
||||
|
||||
let mut response = LookupResponse::new(801, target, coords.clone(), proof);
|
||||
response.path_mtu = 64;
|
||||
|
||||
let payload = &response.encode()[1..];
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
|
||||
assert!(
|
||||
node.coord_cache().contains(&target, now_ms),
|
||||
"coordinates must still be cached; the proof covers them"
|
||||
);
|
||||
assert_eq!(
|
||||
node.coord_cache().get_entry(&target).unwrap().path_mtu(),
|
||||
None,
|
||||
"a sub-floor annotation must not reach the coordinate cache"
|
||||
);
|
||||
assert_eq!(
|
||||
node.path_mtu_lookup_get(&target_fips),
|
||||
None,
|
||||
"a sub-floor annotation must not reach the MSS clamp lookup"
|
||||
);
|
||||
assert_eq!(
|
||||
node.metrics().errors.lookup_resp_mtu_below_floor.get(),
|
||||
1,
|
||||
"refusing the annotation must be visible on a counter, not only in a log"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_actionable_lookup_response_path_mtu_does_not_bump_below_floor_counter() {
|
||||
// Discriminating half of the sub-floor counter check: the refusal counter
|
||||
// is only useful if an ordinary verified response leaves it alone.
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
|
||||
let target_identity = Identity::generate();
|
||||
let target = *target_identity.node_addr();
|
||||
let target_fips = crate::FipsAddress::from_node_addr(&target);
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap();
|
||||
|
||||
node.register_identity(target, target_identity.pubkey_full());
|
||||
|
||||
let proof_data = LookupResponse::proof_bytes(802, &target, &coords);
|
||||
let proof = target_identity.sign(&proof_data);
|
||||
|
||||
let mut response = LookupResponse::new(802, target, coords.clone(), proof);
|
||||
response.path_mtu = 1280;
|
||||
|
||||
let payload = &response.encode()[1..];
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
assert_eq!(
|
||||
node.path_mtu_lookup_get(&target_fips),
|
||||
Some(1280),
|
||||
"an actionable annotation must reach the MSS clamp lookup"
|
||||
);
|
||||
assert_eq!(
|
||||
node.metrics().errors.lookup_resp_mtu_below_floor.get(),
|
||||
0,
|
||||
"an actionable annotation must not bump the below-floor counter"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_originator_lookup_response_keeps_tighter_path_mtu_lookup() {
|
||||
// Regression: a LookupResponse carrying a looser (larger) path_mtu must
|
||||
|
||||
@@ -2272,6 +2272,282 @@ async fn test_handle_mtu_exceeded_keeps_tighter_existing_path_mtu_lookup() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
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.
|
||||
let mut tn = make_test_node().await;
|
||||
|
||||
let dest = NodeAddr::from_bytes([0xCC; 16]);
|
||||
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;
|
||||
|
||||
assert_eq!(
|
||||
tn.node.path_mtu_lookup_get(&dest_fips),
|
||||
None,
|
||||
"a sub-floor MtuExceeded must leave no path_mtu_lookup entry behind"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_sub_floor_mtu_exceeded_is_counted_separately_from_all_mtu_exceeded() {
|
||||
// `mtu_exceeded` counts every MtuExceeded regardless of value, so the
|
||||
// sub-floor subset is not separable from it. The signal is unencrypted,
|
||||
// unauthenticated and unmetered, so that subset climbing on its own is
|
||||
// the forged-signal signature and needs its own counter.
|
||||
use crate::node::tests::spanning_tree::make_test_node;
|
||||
|
||||
let mut tn = make_test_node().await;
|
||||
|
||||
let dest = NodeAddr::from_bytes([0xCC; 16]);
|
||||
let reporter = NodeAddr::from_bytes([0xBB; 16]);
|
||||
|
||||
assert_eq!(
|
||||
tn.node.metrics().errors.mtu_exceeded_below_floor.get(),
|
||||
0,
|
||||
"counter starts at zero on a fresh node"
|
||||
);
|
||||
|
||||
let inner = build_mtu_exceeded_inner(
|
||||
&dest,
|
||||
&reporter,
|
||||
crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU - 1,
|
||||
);
|
||||
tn.node.handle_mtu_exceeded(&inner).await;
|
||||
|
||||
assert_eq!(
|
||||
tn.node.metrics().errors.mtu_exceeded_below_floor.get(),
|
||||
1,
|
||||
"a sub-floor MtuExceeded must bump the below-floor counter"
|
||||
);
|
||||
|
||||
// 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;
|
||||
|
||||
assert_eq!(
|
||||
tn.node.metrics().errors.mtu_exceeded_below_floor.get(),
|
||||
1,
|
||||
"an actionable MtuExceeded must not bump the below-floor counter"
|
||||
);
|
||||
assert_eq!(
|
||||
tn.node.metrics().errors.mtu_exceeded.get(),
|
||||
2,
|
||||
"the all-signals counter must count both, sub-floor and actionable"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_mtu_exceeded_at_the_floor_still_writes_path_mtu_lookup() {
|
||||
use crate::node::tests::spanning_tree::make_test_node;
|
||||
|
||||
// The guard must reject only what is below the floor. Without this the
|
||||
// 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 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;
|
||||
|
||||
assert_eq!(
|
||||
tn.node.path_mtu_lookup_get(&dest_fips),
|
||||
Some(floor),
|
||||
"a bottleneck exactly at the floor is actionable and must be stored"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_forged_mtu_exceeded_of_zero_does_not_blackhole_the_session() {
|
||||
// The security property itself. MtuExceeded arrives unencrypted with no
|
||||
// sender check, so anyone who can reach this node can inject one. Applied
|
||||
// unfiltered, a reported MTU of zero drives the session's path MTU to
|
||||
// zero, and from then on the TUN send gate answers every packet with an
|
||||
// ICMPv6 Packet Too Big instead of sending it: a total blackhole for that
|
||||
// destination that survives until the daemon restarts.
|
||||
let edges = vec![(0, 1)];
|
||||
let mut nodes = run_tree_test(2, &edges, false).await;
|
||||
verify_tree_convergence(&nodes);
|
||||
populate_all_coord_caches(&mut nodes);
|
||||
|
||||
let node0_addr = *nodes[0].node.node_addr();
|
||||
let node1_addr = *nodes[1].node.node_addr();
|
||||
let node1_pubkey = nodes[1].node.identity().pubkey_full();
|
||||
|
||||
let src_fips = crate::FipsAddress::from_node_addr(&node0_addr);
|
||||
let dst_fips = crate::FipsAddress::from_node_addr(&node1_addr);
|
||||
|
||||
nodes[0]
|
||||
.node
|
||||
.initiate_session(node1_addr, node1_pubkey)
|
||||
.await
|
||||
.unwrap();
|
||||
for _ in 0..3 {
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
process_available_packets(&mut nodes).await;
|
||||
}
|
||||
assert!(
|
||||
nodes[0]
|
||||
.node
|
||||
.get_session(&node1_addr)
|
||||
.unwrap()
|
||||
.state()
|
||||
.is_established()
|
||||
);
|
||||
|
||||
// Forge the signal: an MtuExceeded claiming the path to node 1 carries
|
||||
// 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;
|
||||
|
||||
let (tun_tx, tun_rx) = std::sync::mpsc::channel();
|
||||
nodes[0].node.tun_tx = Some(tun_tx);
|
||||
|
||||
let payload = vec![0u8; 560];
|
||||
let ipv6_packet = build_ipv6_packet(&src_fips, &dst_fips, &payload);
|
||||
assert_eq!(ipv6_packet.len(), 600);
|
||||
assert!(
|
||||
ipv6_packet.len() <= nodes[0].node.effective_ipv6_mtu() as usize,
|
||||
"the packet must fit the local MTU, so any PTB comes from the forged signal"
|
||||
);
|
||||
|
||||
nodes[0].node.handle_tun_outbound(ipv6_packet).await;
|
||||
|
||||
let tun_messages: Vec<Vec<u8>> = std::iter::from_fn(|| tun_rx.try_recv().ok()).collect();
|
||||
assert!(
|
||||
tun_messages.is_empty(),
|
||||
"a forged MtuExceeded of zero must not turn ordinary packets into \
|
||||
ICMPv6 Packet Too Big; got {} message(s)",
|
||||
tun_messages.len()
|
||||
);
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_path_broken_releases_path_mtu_lookup_entry() {
|
||||
use crate::node::tests::spanning_tree::make_test_node;
|
||||
use crate::protocol::PathBroken;
|
||||
|
||||
// A PathBroken report declares the path to a destination gone. The stored
|
||||
// path MTU described that path, so it must not be carried onto whatever
|
||||
// path replaces it — otherwise a value learned once (or injected once)
|
||||
// outlives every route change until the daemon restarts.
|
||||
let mut tn = make_test_node().await;
|
||||
|
||||
let dest = NodeAddr::from_bytes([0xCC; 16]);
|
||||
let reporter = NodeAddr::from_bytes([0xBB; 16]);
|
||||
let dest_fips = crate::FipsAddress::from_node_addr(&dest);
|
||||
|
||||
tn.node.path_mtu_lookup_insert(dest_fips, 700);
|
||||
assert_eq!(tn.node.path_mtu_lookup_get(&dest_fips), Some(700));
|
||||
|
||||
// Build the body the dispatcher would hand the handler: encode() prepends
|
||||
// a 4-byte FSP prefix and a msg_type byte, both already consumed there.
|
||||
let encoded = PathBroken::new(dest, reporter).encode();
|
||||
let inner = &encoded[5..];
|
||||
assert!(
|
||||
PathBroken::decode(inner).is_ok(),
|
||||
"the test body must decode, or the handler returns early and the \
|
||||
assertion below observes nothing"
|
||||
);
|
||||
|
||||
tn.node.handle_path_broken(inner).await;
|
||||
|
||||
assert_eq!(
|
||||
tn.node.path_mtu_lookup_get(&dest_fips),
|
||||
None,
|
||||
"PathBroken must release the stored path MTU for the dead path"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_idle_session_purge_keeps_link_peer_path_mtu_seed() {
|
||||
use crate::peer::ActivePeer;
|
||||
use crate::transport::udp::UdpTransport;
|
||||
use crate::transport::{TransportHandle, packet_channel};
|
||||
|
||||
// Releasing on idle expiry must not throw away what local configuration
|
||||
// knows. Idle expiry removes an end-to-end session; the FMP link to a
|
||||
// directly connected peer stays up, and its link MTU is seeded only on
|
||||
// link promotion. A blanket removal here would drop that peer to the
|
||||
// conservative ceiling for every later flow until the link re-handshakes.
|
||||
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 (transport_packet_tx, _transport_packet_rx) = packet_channel(64);
|
||||
let transport_id = TransportId::new(1);
|
||||
let mut udp = UdpTransport::new(
|
||||
transport_id,
|
||||
Some("udp1".to_string()),
|
||||
crate::config::UdpConfig {
|
||||
bind_addr: Some("127.0.0.1:0".to_string()),
|
||||
mtu: Some(1452),
|
||||
..Default::default()
|
||||
},
|
||||
transport_packet_tx,
|
||||
);
|
||||
udp.start_async().await.unwrap();
|
||||
node.transports
|
||||
.insert(transport_id, TransportHandle::Udp(udp));
|
||||
|
||||
// A directly connected peer, seeded from its link MTU the way FMP
|
||||
// promotion seeds it, with an end-to-end session on top.
|
||||
let remote = Identity::generate();
|
||||
let remote_addr = *remote.node_addr();
|
||||
let remote_fips = crate::FipsAddress::from_node_addr(&remote_addr);
|
||||
let transport_addr = TransportAddr::from_string("127.0.0.1:2121");
|
||||
|
||||
let peer_identity = PeerIdentity::from_pubkey_full(remote.pubkey_full());
|
||||
let mut peer = ActivePeer::new(peer_identity, LinkId::new(7), 0);
|
||||
peer.set_current_addr(transport_id, transport_addr.clone());
|
||||
node.peers.insert(remote_addr, peer);
|
||||
|
||||
node.seed_path_mtu_for_link_peer(&remote_addr, transport_id, &transport_addr);
|
||||
assert_eq!(
|
||||
node.path_mtu_lookup_get(&remote_fips),
|
||||
Some(1452),
|
||||
"precondition: the direct-link seed is in place"
|
||||
);
|
||||
|
||||
let session = make_noise_session(node.identity(), &remote);
|
||||
let entry = crate::node::session::SessionEntry::new(
|
||||
remote_addr,
|
||||
remote.pubkey_full(),
|
||||
EndToEndState::Established(session),
|
||||
1000,
|
||||
true,
|
||||
);
|
||||
node.sessions.insert(remote_addr, entry);
|
||||
|
||||
node.purge_idle_sessions(1000 + 92_000);
|
||||
assert_eq!(node.session_count(), 0, "precondition: the session expired");
|
||||
|
||||
assert_eq!(
|
||||
node.path_mtu_lookup_get(&remote_fips),
|
||||
Some(1452),
|
||||
"idle expiry must leave the locally derived link MTU in place"
|
||||
);
|
||||
|
||||
for transport in node.transports.values_mut() {
|
||||
transport.stop().await.ok();
|
||||
}
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Proactive PathMtuNotification → path_mtu_lookup focused unit tests
|
||||
//
|
||||
@@ -2392,6 +2668,57 @@ fn test_handle_path_mtu_notification_no_session_no_op() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sub_floor_path_mtu_notification_is_ignored_and_counted() {
|
||||
// The state machine returns the same `false` for a sub-floor refusal as
|
||||
// for an ordinary no-change, so without a counter at the caller the
|
||||
// refusal is indistinguishable from the common case. This arrives on the
|
||||
// decrypted path, so a rising count means an authenticated peer is
|
||||
// sending unusable values.
|
||||
let mut node = make_node();
|
||||
let remote = Identity::generate();
|
||||
let remote_addr = *remote.node_addr();
|
||||
let remote_fips = crate::FipsAddress::from_node_addr(&remote_addr);
|
||||
|
||||
install_established_session_with_mmp(&mut node, &remote);
|
||||
|
||||
assert_eq!(
|
||||
node.metrics().errors.path_mtu_notif_below_floor.get(),
|
||||
0,
|
||||
"counter starts at zero on a fresh node"
|
||||
);
|
||||
|
||||
let body = build_path_mtu_notification_body(crate::upper::icmp::MIN_ACTIONABLE_PATH_MTU - 1);
|
||||
node.handle_session_path_mtu_notification(&remote_addr, &body);
|
||||
|
||||
assert_eq!(
|
||||
node.metrics().errors.path_mtu_notif_below_floor.get(),
|
||||
1,
|
||||
"a sub-floor PathMtuNotification must bump the below-floor counter"
|
||||
);
|
||||
assert_eq!(
|
||||
node.path_mtu_lookup_get(&remote_fips),
|
||||
None,
|
||||
"a sub-floor PathMtuNotification must leave no path_mtu_lookup entry"
|
||||
);
|
||||
|
||||
// The counter must discriminate: an actionable value is applied and must
|
||||
// not bump it.
|
||||
let body = build_path_mtu_notification_body(1280);
|
||||
node.handle_session_path_mtu_notification(&remote_addr, &body);
|
||||
|
||||
assert_eq!(
|
||||
node.metrics().errors.path_mtu_notif_below_floor.get(),
|
||||
1,
|
||||
"an actionable PathMtuNotification must not bump the below-floor counter"
|
||||
);
|
||||
assert_eq!(
|
||||
node.path_mtu_lookup_get(&remote_fips),
|
||||
Some(1280),
|
||||
"the actionable value must still be applied after a refused one"
|
||||
);
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Session identity binding: XK msg3 source address / static key
|
||||
// ============================================================================
|
||||
|
||||
@@ -1521,6 +1521,53 @@ async fn test_seed_path_mtu_inserts_when_empty() {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_seeded_narrow_link_mtu_reaches_the_clamp_as_a_tight_ceiling() {
|
||||
// The seed and the SYN-time MSS clamp are two halves of one mechanism: the
|
||||
// seed writes the node's own outgoing link MTU, the clamp reads it. A
|
||||
// narrow link is the case that matters, because BLE negotiates its MTU per
|
||||
// connection and lands below the remote-value floor routinely, and a
|
||||
// direct link has no forwarder to answer an over-large segment with
|
||||
// MtuExceeded. Driving the real seed rather than inserting into the map
|
||||
// pins that the clamp honours what the seed actually stores.
|
||||
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, 240).await;
|
||||
node.transports.insert(TransportId::new(1), udp);
|
||||
|
||||
let peer_addr = make_node_addr(0xEE);
|
||||
let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr);
|
||||
let transport_addr = TransportAddr::from_string("10.0.0.6:2121");
|
||||
|
||||
node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr);
|
||||
|
||||
assert_eq!(
|
||||
node.path_mtu_lookup
|
||||
.read()
|
||||
.unwrap()
|
||||
.get(&fips_addr)
|
||||
.copied(),
|
||||
Some(240),
|
||||
"the seed stores a narrow link MTU unchanged"
|
||||
);
|
||||
// 240 - 77 encap - 40 IPv6 - 20 TCP = 103. A clamp that discarded the
|
||||
// seeded value would advertise the 1143 conservative ceiling instead, and
|
||||
// every full-size segment would be refused by the transport with no
|
||||
// feedback to the TCP stack.
|
||||
assert_eq!(
|
||||
crate::upper::tun::per_flow_max_mss(&node.path_mtu_lookup, fips_addr.as_bytes(), 1360),
|
||||
103,
|
||||
"the clamp must honour the seeded link MTU, not fall back to 1143"
|
||||
);
|
||||
|
||||
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();
|
||||
|
||||
@@ -103,6 +103,25 @@ pub const FIPS_OVERHEAD: u16 = 16 + 16 + 5 + 35 + 12 + 6 + 16; // 106 bytes
|
||||
/// ```
|
||||
pub const FIPS_IPV6_OVERHEAD: u16 = 77;
|
||||
|
||||
/// Smallest remote-supplied transport path MTU this node will act on.
|
||||
///
|
||||
/// The `path_mtu` field is an unsigned per-hop transit annotation carried
|
||||
/// outside `proof_bytes`, and the `MtuExceeded` and `PathBroken` signals
|
||||
/// arrive unencrypted, so any forwarder on the path can lower it. Below this
|
||||
/// value the quantities derived from it degenerate: at a transport MTU of 137
|
||||
/// or less, [`mss_ceiling`] saturates to a TCP MSS of zero, at 138 it is a
|
||||
/// single byte, and the derived MSS stays under a hundred all the way to 236.
|
||||
/// At the floor itself the derived inner IPv6 MTU is 179 and the TCP MSS is
|
||||
/// 119, clear of both the zero cliff and that band.
|
||||
///
|
||||
/// A candidate below the floor is ignored — treated as no information at all,
|
||||
/// never applied and never stored — rather than clamped, because clamping
|
||||
/// would fabricate an estimate the node has no basis for. Locally derived link
|
||||
/// MTUs are not subject to the floor; it applies only to values a remote party
|
||||
/// supplied. A local value is exact, so the SYN-time clamp honours it however
|
||||
/// small and refuses only the zero cliff, which no provenance makes usable.
|
||||
pub const MIN_ACTIONABLE_PATH_MTU: u16 = 256;
|
||||
|
||||
/// Calculate the effective IPv6 MTU for FIPS-encapsulated traffic.
|
||||
///
|
||||
/// Given a transport MTU (e.g., UDP payload size), returns the maximum
|
||||
@@ -112,6 +131,21 @@ pub fn effective_ipv6_mtu(transport_mtu: u16) -> u16 {
|
||||
transport_mtu.saturating_sub(FIPS_IPV6_OVERHEAD)
|
||||
}
|
||||
|
||||
/// Largest TCP segment size a FIPS-encapsulated path of `transport_mtu`
|
||||
/// bytes on the wire admits: the effective inner IPv6 MTU less the 40-byte
|
||||
/// IPv6 header and the 20-byte TCP header.
|
||||
///
|
||||
/// Zero means the path has no room for even one payload byte, so no TCP
|
||||
/// segment fits and no clamp derived from it carries information. That is
|
||||
/// the one condition the SYN-time clamp treats as unusable regardless of
|
||||
/// where the MTU came from, and it is why the seed site warns; both read it
|
||||
/// from here so they cannot disagree about where the cliff is.
|
||||
pub fn mss_ceiling(transport_mtu: u16) -> u16 {
|
||||
effective_ipv6_mtu(transport_mtu)
|
||||
.saturating_sub(40)
|
||||
.saturating_sub(20)
|
||||
}
|
||||
|
||||
/// Check if we should send an ICMPv6 error for this packet.
|
||||
///
|
||||
/// Returns false if the packet is:
|
||||
|
||||
@@ -52,6 +52,12 @@ pub fn clamp_tcp_mss(ipv6_packet: &mut [u8], max_mss: u16) -> bool {
|
||||
return false;
|
||||
}
|
||||
|
||||
// A ceiling of zero carries no information and an MSS option of zero is
|
||||
// not a legal segment size. Refuse the clamp rather than write it.
|
||||
if max_mss == 0 {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Get TCP header start
|
||||
let tcp_start = 40;
|
||||
if ipv6_packet.len() < tcp_start + TCP_HEADER_MIN_LEN {
|
||||
@@ -236,6 +242,24 @@ mod tests {
|
||||
assert_eq!(mss, 1200);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamp_tcp_mss_with_zero_ceiling_leaves_mss_option_untouched() {
|
||||
// A ceiling of zero reaches here only when something upstream
|
||||
// degenerated. Writing it would put an MSS of 0 in the SYN and wedge
|
||||
// the flow, so the clamp must refuse and report that it did nothing.
|
||||
let src = [0xfd, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1];
|
||||
let dst = [0xfd, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2];
|
||||
let mut packet = make_tcp_syn_packet(src, dst, 1460);
|
||||
|
||||
let modified = clamp_tcp_mss(&mut packet, 0);
|
||||
|
||||
assert!(!modified, "a zero ceiling must not count as a clamp");
|
||||
|
||||
let tcp_start = 40;
|
||||
let mss = u16::from_be_bytes([packet[tcp_start + 22], packet[tcp_start + 23]]);
|
||||
assert_eq!(mss, 1460, "MSS option must be left exactly as it arrived");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_clamp_tcp_mss_leaves_small_mss_unchanged() {
|
||||
let src = [0xfd, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1];
|
||||
|
||||
+106
-7
@@ -71,15 +71,13 @@ pub(crate) fn per_flow_max_mss(
|
||||
addr_bytes: &[u8],
|
||||
global_max_mss: u16,
|
||||
) -> u16 {
|
||||
use super::icmp::effective_ipv6_mtu;
|
||||
use super::icmp::mss_ceiling;
|
||||
|
||||
// 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 conservative_max_mss = mss_ceiling(IPV6_MIN_MTU);
|
||||
let empty_lookup_ceiling = std::cmp::min(global_max_mss, conservative_max_mss);
|
||||
|
||||
if addr_bytes.len() != 16 {
|
||||
@@ -118,9 +116,39 @@ pub(crate) fn per_flow_max_mss(
|
||||
);
|
||||
return empty_lookup_ceiling;
|
||||
};
|
||||
let path_max_mss = effective_ipv6_mtu(path_mtu)
|
||||
.saturating_sub(40)
|
||||
.saturating_sub(20);
|
||||
let path_max_mss = mss_ceiling(path_mtu);
|
||||
// The actionable floor deliberately does not apply here. Every value a
|
||||
// remote party supplies is refused before it can reach this map, at the
|
||||
// path MTU state machine, the reactive `MtuExceeded` write and the
|
||||
// discovery response, so a small stored value is one the node derived
|
||||
// from its own outgoing link: a configured transport MTU, or the MTU a
|
||||
// BLE connection negotiated, which on that transport is routinely well
|
||||
// under the floor. Such a value is exact rather than suspect, and the
|
||||
// tight clamp it yields is the reason it is stored: discarding it would
|
||||
// advertise the conservative ceiling on a link that cannot carry it, and
|
||||
// a direct link has no forwarder to answer with `MtuExceeded`, so the
|
||||
// flow would stall with no feedback.
|
||||
//
|
||||
// What no provenance rescues is the arithmetic degenerating. At a stored
|
||||
// MTU of 137 or less not one payload byte fits alongside the IPv6 and TCP
|
||||
// headers, and `clamp_tcp_mss` refuses a ceiling of zero, which would
|
||||
// leave the SYN carrying the kernel-natural MSS instead. Fall back to the
|
||||
// conservative ceiling there; any positive result is by construction the
|
||||
// largest segment the stored MTU admits.
|
||||
//
|
||||
// `trace!`, not `warn!`, because this runs on every packet rather than
|
||||
// only on SYNs: one degenerate stored value would otherwise emit a WARN
|
||||
// per packet indefinitely and bury every other warning on the node. The
|
||||
// link promotion path warns once instead.
|
||||
if path_max_mss == 0 {
|
||||
trace!(
|
||||
fips_addr = %fips_addr,
|
||||
path_mtu,
|
||||
empty_lookup_ceiling,
|
||||
"per_flow_max_mss: stored path_mtu leaves no room for a TCP payload byte, using conservative ceiling"
|
||||
);
|
||||
return empty_lookup_ceiling;
|
||||
}
|
||||
let result = std::cmp::min(global_max_mss, path_max_mss);
|
||||
trace!(
|
||||
fips_addr = %fips_addr,
|
||||
@@ -1513,6 +1541,77 @@ mod tests {
|
||||
assert_eq!(per_flow_max_mss(&lookup, addr.as_bytes(), 1360), 1315);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn per_flow_stored_mtu_admitting_no_payload_byte_falls_back_to_conservative_ceiling() {
|
||||
// A stored MTU of 137 or less leaves nothing after the 77 bytes of
|
||||
// FIPS encapsulation and the 40 + 20 bytes of IPv6 and TCP header, so
|
||||
// the MSS arithmetic saturates to zero. Returning that zero would be
|
||||
// worse than the fallback: `clamp_tcp_mss` refuses a ceiling of zero,
|
||||
// so the SYN would go out at the kernel-natural MSS, unclamped.
|
||||
for stored in [0u16, 1, 100, 137] {
|
||||
let lookup = empty_lookup();
|
||||
let addr = fips_addr_with_node_byte(0x42);
|
||||
lookup.write().unwrap().insert(addr, stored);
|
||||
assert_eq!(
|
||||
per_flow_max_mss(&lookup, addr.as_bytes(), 1360),
|
||||
1143,
|
||||
"stored path_mtu {stored} admits no payload byte and must be ignored"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn per_flow_honors_a_locally_seeded_sub_floor_mtu_instead_of_loosening_to_the_ceiling() {
|
||||
// Only `seed_path_mtu_for_link_peer` can put a sub-floor value in this
|
||||
// map: every remote-supplied path MTU is refused at ingress, at the
|
||||
// path MTU state machine, the reactive `MtuExceeded` write and the
|
||||
// discovery response. A seeded value is therefore the node's own link
|
||||
// measurement, and BLE negotiates one per connection that lands in
|
||||
// this band routinely.
|
||||
//
|
||||
// Applying the remote-value floor here discarded it and advertised
|
||||
// 1143 instead, which a link this narrow cannot carry: every full-size
|
||||
// segment is refused by the transport, a direct link has no forwarder
|
||||
// to answer with `MtuExceeded`, and the flow stalls with no feedback.
|
||||
// The tight clamp is the whole reason the seed exists.
|
||||
//
|
||||
// 138 is the first MTU admitting a payload byte; 240 is a plausible
|
||||
// negotiated BLE value; 255 is one below the remote-value floor. The
|
||||
// whole table is evaluated before asserting, so a regression names
|
||||
// every band it broke rather than only the first.
|
||||
let want = [(138u16, 1u16), (240, 103), (255, 118)];
|
||||
let got: Vec<(u16, u16)> = want
|
||||
.iter()
|
||||
.map(|&(stored, _)| {
|
||||
let lookup = empty_lookup();
|
||||
let addr = fips_addr_with_node_byte(0x42);
|
||||
lookup.write().unwrap().insert(addr, stored);
|
||||
(stored, per_flow_max_mss(&lookup, addr.as_bytes(), 1360))
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(
|
||||
got,
|
||||
want.to_vec(),
|
||||
"each locally seeded path_mtu must clamp tight, not fall back to 1143"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn per_flow_stored_mtu_at_the_actionable_floor_is_still_honored() {
|
||||
// The smallest value the node will accept from a remote party still
|
||||
// clamps to its own arithmetic and nothing coarser: 256 - 77 - 40 - 20
|
||||
// = 119. Reintroducing the remote-value floor as a clamp-time guard
|
||||
// would leave this case passing, so it is pinned separately from the
|
||||
// sub-floor table above.
|
||||
let lookup = empty_lookup();
|
||||
let addr = fips_addr_with_node_byte(0x42);
|
||||
lookup
|
||||
.write()
|
||||
.unwrap()
|
||||
.insert(addr, super::super::icmp::MIN_ACTIONABLE_PATH_MTU);
|
||||
assert_eq!(per_flow_max_mss(&lookup, addr.as_bytes(), 1360), 119);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn per_flow_returns_conservative_ceiling_for_non_fips_addr() {
|
||||
// Non-fips IPv6 (e.g. fe80::/10 link-local) takes the empty-
|
||||
|
||||
Reference in New Issue
Block a user