mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
Follow IP semantics for the session datagram hop limit
The forwarding path tested the hop limit before testing whether the datagram was addressed to this node, and decremented only on the forwarding branch. Two consequences followed. A datagram addressed here that arrived with a zero hop limit was dropped rather than delivered. And a forwarder receiving a transit datagram with one hop left transmitted it with zero for the next hop to discard, wasting a transmission on every expiring datagram. Delivery to the addressed node is no longer gated on the hop limit, and the decrement now happens before the drop decision, so a datagram that would leave with zero is not sent. Saturating subtraction folds the two arrivals that cannot be transmitted, already exhausted and last hop, into a single test. The reachable radius does not change, which is worth stating because it is easy to assume otherwise. The old behavior refused to deliver at zero but was willing to send at zero, and the new behavior is the mirror of that, so the two cancel: a path of h links still delivers for any source value of h or more. What this actually buys is the wasted transmission, the exhaustion counter charging at the node that makes the decision rather than the one after it, conformance with the specified semantics, and one real gain during a rolling upgrade, where an unupgraded forwarder feeding an upgraded destination delivers one hop further than either version does on its own. Coordinate cache warming moved ahead of both decisions, so a transit datagram later dropped for hop limit still contributes its plaintext coordinates. The only arrivals this newly warms from are those with a zero hop limit, and every insert they can make is already achievable with a value of one, so it grants nothing that was not already available and adds no memory growth the cap does not bound. Four tests added: delivery at zero to this node, the transit drop at one, the transit pass at two, and the one per hop decrement across a three node chain, which brackets the emitted value from both sides since it is only observable through what the next hop does. One existing test was renamed because its destination was this node, so it never exercised the transit path its name claimed, and it asserted nothing at all.
This commit is contained in:
@@ -13,6 +13,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Fixed
|
||||
|
||||
- `SessionDatagram` hop-limit handling now follows IP semantics. Delivery to
|
||||
the addressed node is no longer TTL-gated, and a forwarder decrements before
|
||||
deciding rather than after, so a datagram that would leave with a TTL of zero
|
||||
is dropped instead of transmitted. Previously the TTL check ran ahead of the
|
||||
local-delivery test, so a datagram addressed to this node that arrived with
|
||||
TTL 0 was dropped, and a forwarder receiving a transit datagram at TTL 1
|
||||
transmitted it at TTL 0 for the next hop to discard, wasting one transmission
|
||||
per expiring datagram. The reachable radius is unchanged, because the two
|
||||
behaviors compensated exactly: a path of `h` links still delivers for any
|
||||
source TTL of `h` or more. During a rolling upgrade, an unupgraded forwarder
|
||||
feeding an upgraded destination delivers one hop further than either version
|
||||
does on its own; no version mix delivers less far. The `TtlExhausted` reject
|
||||
counter now charges at the node that makes the decision rather than at the
|
||||
hop after it.
|
||||
|
||||
## [0.4.1] - 2026-07-19
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
//! SessionDatagram forwarding handler.
|
||||
//!
|
||||
//! Handles incoming SessionDatagram (0x00) link messages: decodes the
|
||||
//! envelope, enforces hop limits, performs coordinate cache warming from
|
||||
//! plaintext session-layer headers, routes to the next hop or delivers
|
||||
//! locally, and generates error signals on routing failure.
|
||||
//! envelope, performs coordinate cache warming from plaintext session-layer
|
||||
//! headers, delivers locally when the datagram is addressed to this node,
|
||||
//! otherwise enforces the transit hop limit and routes to the next hop, and
|
||||
//! generates error signals on routing failure.
|
||||
|
||||
use crate::NodeAddr;
|
||||
use crate::node::reject::ForwardingReject;
|
||||
@@ -43,26 +44,16 @@ impl Node {
|
||||
}
|
||||
};
|
||||
|
||||
// TTL enforcement: decrement for forwarding and drop only if the
|
||||
// received datagram was already exhausted.
|
||||
if datagram_ref.ttl == 0 {
|
||||
self.metrics()
|
||||
.forwarding
|
||||
.record_reject_bytes(ForwardingReject::TtlExhausted, payload.len());
|
||||
debug!(
|
||||
src = %datagram_ref.src_addr,
|
||||
dest = %datagram_ref.dest_addr,
|
||||
"SessionDatagram TTL exhausted, dropping"
|
||||
);
|
||||
return;
|
||||
}
|
||||
let forwarded_ttl = datagram_ref.ttl - 1;
|
||||
|
||||
// Coordinate cache warming from plaintext session-layer headers
|
||||
// Coordinate cache warming from plaintext session-layer headers.
|
||||
// Runs ahead of both the delivery and the TTL decisions: the coords
|
||||
// a peer put on the wire are equally valid whichever way those go.
|
||||
self.try_warm_coord_cache_ref(&datagram_ref);
|
||||
|
||||
// Local delivery: dispatch to session layer handlers without
|
||||
// materializing an owned SessionDatagram payload Vec.
|
||||
// materializing an owned SessionDatagram payload Vec. Delivery to
|
||||
// the addressed node is *not* TTL-gated — under IP semantics the
|
||||
// TTL governs forwarding, not delivery to the addressed host — so
|
||||
// this test precedes the TTL gate below.
|
||||
if datagram_ref.dest_addr == *self.node_addr() {
|
||||
self.metrics().forwarding.record_delivered(payload.len());
|
||||
self.handle_session_payload(
|
||||
@@ -75,6 +66,24 @@ impl Node {
|
||||
return;
|
||||
}
|
||||
|
||||
// TTL enforcement on the transit path: decrement first, then drop if
|
||||
// the datagram would leave with a TTL of zero. `saturating_sub` folds
|
||||
// the already-exhausted arrival (ttl=0) into the same test as the
|
||||
// last-hop arrival (ttl=1); neither is transmitted.
|
||||
let forwarded_ttl = datagram_ref.ttl.saturating_sub(1);
|
||||
if forwarded_ttl == 0 {
|
||||
self.metrics()
|
||||
.forwarding
|
||||
.record_reject_bytes(ForwardingReject::TtlExhausted, payload.len());
|
||||
debug!(
|
||||
src = %datagram_ref.src_addr,
|
||||
dest = %datagram_ref.dest_addr,
|
||||
ttl = datagram_ref.ttl,
|
||||
"SessionDatagram TTL exhausted, dropping"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let mut datagram = datagram_ref.into_owned();
|
||||
datagram.ttl = forwarded_ttl;
|
||||
|
||||
|
||||
@@ -40,22 +40,116 @@ async fn test_forwarding_hop_limit_exhausted() {
|
||||
node.handle_session_datagram(&from, &encoded[1..], false)
|
||||
.await;
|
||||
// No panic, no send (node has no peers)
|
||||
let fwd = &node.metrics().forwarding;
|
||||
assert_eq!(
|
||||
fwd.ttl_exhausted_packets.get(),
|
||||
1,
|
||||
"transit ttl=0 should be charged to TtlExhausted"
|
||||
);
|
||||
assert_eq!(
|
||||
fwd.drop_no_route_packets.get(),
|
||||
0,
|
||||
"transit ttl=0 should never reach the routing step"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_hop_limit_one_drops_at_transit() {
|
||||
// ttl=1 means after decrement it becomes 0 — the datagram can
|
||||
// still be delivered this hop but would be dropped at the next.
|
||||
// decrement_ttl returns true (1 > 0), so the handler proceeds.
|
||||
async fn test_forwarding_ttl_one_local_delivery_is_not_gated() {
|
||||
// dest == self, so this is local delivery, not transit: the TTL gate
|
||||
// does not apply and the datagram is handed to the session layer.
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
let my_addr = *node.node_addr();
|
||||
let src = make_node_addr(0x01);
|
||||
let dg = SessionDatagram::new(src, my_addr, vec![0x10, 0x00, 0x00, 0x00]).with_ttl(1);
|
||||
let encoded = dg.encode();
|
||||
// Should succeed — ttl=1 decrements to 0 but packet is still processed
|
||||
node.handle_session_datagram(&from, &encoded[1..], false)
|
||||
.await;
|
||||
let fwd = &node.metrics().forwarding;
|
||||
assert_eq!(fwd.delivered_packets.get(), 1, "ttl=1 should be delivered");
|
||||
assert_eq!(fwd.ttl_exhausted_packets.get(), 0);
|
||||
}
|
||||
|
||||
/// Acceptance: a datagram addressed to this node with ttl=0 is delivered
|
||||
/// locally. The TTL governs forwarding, not delivery to the addressed host,
|
||||
/// so the gate must sit after the local-delivery test — and the
|
||||
/// `TtlExhausted` reject must not be charged for a delivered datagram.
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_ttl_zero_local_delivery_is_not_gated() {
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
let my_addr = *node.node_addr();
|
||||
let src = make_node_addr(0x01);
|
||||
let dg = SessionDatagram::new(src, my_addr, vec![0x10, 0x00, 0x00, 0x00]).with_ttl(0);
|
||||
let encoded = dg.encode();
|
||||
node.handle_session_datagram(&from, &encoded[1..], false)
|
||||
.await;
|
||||
let fwd = &node.metrics().forwarding;
|
||||
assert_eq!(
|
||||
fwd.delivered_packets.get(),
|
||||
1,
|
||||
"ttl=0 addressed to this node must still be delivered locally"
|
||||
);
|
||||
assert_eq!(
|
||||
fwd.ttl_exhausted_packets.get(),
|
||||
0,
|
||||
"local delivery must not be charged to the TtlExhausted reject"
|
||||
);
|
||||
assert_eq!(fwd.drop_no_route_packets.get(), 0);
|
||||
}
|
||||
|
||||
/// Acceptance: a transit datagram arriving with ttl=1 would leave with ttl=0,
|
||||
/// so it is dropped here rather than transmitted. Reaching the routing step at
|
||||
/// all (`drop_no_route`) would mean it had been handed to the forwarder.
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_ttl_one_transit_dropped_before_routing() {
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
let src = make_node_addr(0x01);
|
||||
let dest = make_node_addr(0x02);
|
||||
let dg = SessionDatagram::new(src, dest, vec![0x10, 0x00, 0x00, 0x00]).with_ttl(1);
|
||||
let encoded = dg.encode();
|
||||
node.handle_session_datagram(&from, &encoded[1..], false)
|
||||
.await;
|
||||
let fwd = &node.metrics().forwarding;
|
||||
assert_eq!(
|
||||
fwd.ttl_exhausted_packets.get(),
|
||||
1,
|
||||
"transit ttl=1 must be dropped as TTL-exhausted, not forwarded"
|
||||
);
|
||||
assert_eq!(
|
||||
fwd.drop_no_route_packets.get(),
|
||||
0,
|
||||
"transit ttl=1 must not reach the routing step"
|
||||
);
|
||||
assert_eq!(fwd.forwarded_packets.get(), 0);
|
||||
assert_eq!(fwd.delivered_packets.get(), 0);
|
||||
}
|
||||
|
||||
/// The other side of the same boundary: ttl=2 clears the gate. This node has
|
||||
/// no peers, so it fails at the routing step instead — which is the evidence
|
||||
/// that the TTL gate passed it through.
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_ttl_two_transit_clears_the_gate() {
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
let src = make_node_addr(0x01);
|
||||
let dest = make_node_addr(0x02);
|
||||
let dg = SessionDatagram::new(src, dest, vec![0x10, 0x00, 0x00, 0x00]).with_ttl(2);
|
||||
let encoded = dg.encode();
|
||||
node.handle_session_datagram(&from, &encoded[1..], false)
|
||||
.await;
|
||||
let fwd = &node.metrics().forwarding;
|
||||
assert_eq!(
|
||||
fwd.ttl_exhausted_packets.get(),
|
||||
0,
|
||||
"transit ttl=2 must clear the TTL gate"
|
||||
);
|
||||
assert_eq!(
|
||||
fwd.drop_no_route_packets.get(),
|
||||
1,
|
||||
"transit ttl=2 should have reached the routing step and found no route"
|
||||
);
|
||||
}
|
||||
|
||||
// --- Local delivery ---
|
||||
@@ -392,9 +486,9 @@ async fn test_forwarding_multi_hop() {
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_hop_limit_prevents_infinite_loops() {
|
||||
// 3-node chain: 0 -- 1 -- 2
|
||||
// Send a datagram with ttl=1. It should be forwarded by node 1
|
||||
// (decrement to 0) and delivered at node 2 (local delivery). If node 2
|
||||
// tried to forward further, the 0 ttl would prevent it.
|
||||
// Send a datagram with ttl=2. Node 1 forwards it as transit (2 -> 1) and
|
||||
// node 2 delivers it locally, which is not TTL-gated. Had node 2 been
|
||||
// transit instead, the arriving ttl=1 would have stopped it there.
|
||||
let edges = vec![(0, 1), (1, 2)];
|
||||
let mut nodes = run_tree_test(3, &edges, false).await;
|
||||
verify_tree_convergence(&nodes);
|
||||
@@ -409,7 +503,7 @@ async fn test_forwarding_hop_limit_prevents_infinite_loops() {
|
||||
node2_addr,
|
||||
vec![0x10, 0x00, 0x04, 0x00, 1, 2, 3, 4],
|
||||
)
|
||||
.with_ttl(2); // Enough for 0->1 (decrement to 1) and 1->2 (decrement to 0, local delivery)
|
||||
.with_ttl(2); // Node 1 forwards with ttl=1; node 2 is the destination
|
||||
|
||||
let encoded = dg.encode();
|
||||
|
||||
@@ -428,6 +522,115 @@ async fn test_forwarding_hop_limit_prevents_infinite_loops() {
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
/// Acceptance: a transit datagram arriving with ttl=2 leaves with ttl=1.
|
||||
///
|
||||
/// Pinned on a live 3-node chain (0 -- 1 -- 2) by where the datagram stops,
|
||||
/// since the TTL that leaves node 0 is only observable through what the next
|
||||
/// hop does with it. Both injections are transit at node 0 (external source,
|
||||
/// destined for node 2), so node 1 is a forwarder in both.
|
||||
///
|
||||
/// - ttl=2 in: node 0 must emit ttl=1, which node 1 (transit) drops. If node 0
|
||||
/// emitted ttl=2 unchanged, node 1 would forward and node 2 would deliver.
|
||||
/// - ttl=3 in: node 0 emits 2, node 1 emits 1, node 2 delivers (delivery is
|
||||
/// not TTL-gated). If either hop decremented by more than one, the datagram
|
||||
/// would have died at node 1 instead.
|
||||
///
|
||||
/// Together the two pin the decrement at exactly one per hop and the drop at
|
||||
/// would-leave-zero.
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_ttl_decrement_is_one_per_hop() {
|
||||
let edges = vec![(0, 1), (1, 2)];
|
||||
let mut nodes = run_tree_test(3, &edges, false).await;
|
||||
verify_tree_convergence(&nodes);
|
||||
populate_all_coord_caches(&mut nodes);
|
||||
|
||||
let node0_addr = *nodes[0].node.node_addr();
|
||||
let node2_addr = *nodes[2].node.node_addr();
|
||||
let external_src = make_node_addr(0xEE);
|
||||
|
||||
// --- ttl=2: must die at node 1, one hop short of the destination ---
|
||||
let dg =
|
||||
SessionDatagram::new(external_src, node2_addr, vec![0x10, 0x00, 0x00, 0x00]).with_ttl(2);
|
||||
let encoded = dg.encode();
|
||||
nodes[0]
|
||||
.node
|
||||
.handle_session_datagram(&node0_addr, &encoded[1..], false)
|
||||
.await;
|
||||
|
||||
for _ in 0..3 {
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
process_available_packets(&mut nodes).await;
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
nodes[0].node.metrics().forwarding.forwarded_packets.get(),
|
||||
1,
|
||||
"node 0 should have forwarded the ttl=2 datagram"
|
||||
);
|
||||
assert_eq!(
|
||||
nodes[1]
|
||||
.node
|
||||
.metrics()
|
||||
.forwarding
|
||||
.ttl_exhausted_packets
|
||||
.get(),
|
||||
1,
|
||||
"node 1 should have received ttl=1 and dropped it as TTL-exhausted"
|
||||
);
|
||||
assert_eq!(
|
||||
nodes[1].node.metrics().forwarding.forwarded_packets.get(),
|
||||
0,
|
||||
"node 1 must not forward a datagram that would leave with ttl=0"
|
||||
);
|
||||
assert_eq!(
|
||||
nodes[2].node.metrics().forwarding.delivered_packets.get(),
|
||||
0,
|
||||
"node 2 must never see the ttl=2 datagram"
|
||||
);
|
||||
|
||||
// --- ttl=3: must survive both transit hops and be delivered at node 2 ---
|
||||
let dg =
|
||||
SessionDatagram::new(external_src, node2_addr, vec![0x10, 0x00, 0x00, 0x00]).with_ttl(3);
|
||||
let encoded = dg.encode();
|
||||
nodes[0]
|
||||
.node
|
||||
.handle_session_datagram(&node0_addr, &encoded[1..], false)
|
||||
.await;
|
||||
|
||||
for _ in 0..3 {
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
process_available_packets(&mut nodes).await;
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
nodes[0].node.metrics().forwarding.forwarded_packets.get(),
|
||||
2,
|
||||
"node 0 should have forwarded the ttl=3 datagram too"
|
||||
);
|
||||
assert_eq!(
|
||||
nodes[1].node.metrics().forwarding.forwarded_packets.get(),
|
||||
1,
|
||||
"node 1 should have forwarded the ttl=2 it received"
|
||||
);
|
||||
assert_eq!(
|
||||
nodes[1]
|
||||
.node
|
||||
.metrics()
|
||||
.forwarding
|
||||
.ttl_exhausted_packets
|
||||
.get(),
|
||||
1,
|
||||
"node 1 should not have dropped the second datagram"
|
||||
);
|
||||
assert_eq!(
|
||||
nodes[2].node.metrics().forwarding.delivered_packets.get(),
|
||||
1,
|
||||
"node 2 should have delivered the datagram that arrived with ttl=1"
|
||||
);
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_no_route_generates_error() {
|
||||
// 2-node network: 0 -- 1
|
||||
|
||||
Reference in New Issue
Block a user