mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
Share one hop-limit rule between the forwarding pre-check and the routing core
The forwarding handler resolves a next hop only for datagrams the routing core can forward, which keeps the coordinate-cache touch in that resolution scoped to genuine forwards. Its TTL test (ttl > 1) restated the core's drop (decrement, then drop at zero) in a different form, and no test could see the two disagree: reverting the pre-check to its older ttl != 0 form passed every unit test. ttl_after_hop now holds the rule. The routing core drops on it, SessionDatagram::can_forward uses it, and a new SessionDatagramRef::can_forward is what the handler calls. A unit test pins the function, a routing test checks the core and can_forward agree for every TTL, and a handler test checks that a last-hop transit datagram does not refresh the destination's cached coordinates while a ttl=2 one does.
This commit is contained in:
@@ -55,13 +55,12 @@ impl Node {
|
||||
self.try_warm_coord_cache_ref(&datagram_ref, payload.len());
|
||||
|
||||
// Pre-resolve the next hop only for datagrams the core can actually
|
||||
// forward: not locally destined, and carrying a TTL that survives the
|
||||
// decrement (`ttl > 1` — the shell-side mirror of the core's
|
||||
// would-leave-zero drop). This keeps `find_next_hop`'s coord-cache
|
||||
// LRU-touch side effect scoped to genuine forwards, as it was when the
|
||||
// TTL test ran inline ahead of it. Warming above has already run, so
|
||||
// the resolution observes freshly cached coords.
|
||||
let next_hop = if datagram_ref.dest_addr != my_addr && datagram_ref.ttl > 1 {
|
||||
// forward: not locally destined, and passing `can_forward`, which is
|
||||
// the core's own hop-limit rule. This keeps `find_next_hop`'s
|
||||
// coord-cache LRU-touch side effect scoped to genuine forwards, as it
|
||||
// was when the TTL test ran inline ahead of it. Warming above has
|
||||
// already run, so the resolution observes freshly cached coords.
|
||||
let next_hop = if datagram_ref.dest_addr != my_addr && datagram_ref.can_forward() {
|
||||
self.resolve_next_hop(&datagram_ref.dest_addr)
|
||||
} else {
|
||||
None
|
||||
|
||||
@@ -155,6 +155,53 @@ async fn test_forwarding_ttl_two_transit_clears_the_gate() {
|
||||
);
|
||||
}
|
||||
|
||||
/// The next hop is resolved only for a datagram the core can forward, and that
|
||||
/// resolution refreshes the destination's cached coordinates. A last-hop
|
||||
/// transit datagram (ttl=1) is dropped by the core, so it must not refresh
|
||||
/// them; a ttl=2 datagram reaches the resolution and does.
|
||||
#[tokio::test]
|
||||
async fn test_forwarding_last_hop_transit_does_not_refresh_destination_coords() {
|
||||
let mut node = make_node();
|
||||
let from = make_node_addr(0xAA);
|
||||
let src = make_node_addr(0x01);
|
||||
let dest = make_node_addr(0x02);
|
||||
let root = make_node_addr(0xF0);
|
||||
let coords = TreeCoordinate::from_addrs(vec![dest, root]).unwrap();
|
||||
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap()
|
||||
.as_millis() as u64;
|
||||
let stamped_ms = now_ms - node.coord_cache().default_ttl_ms() / 2;
|
||||
node.coord_cache_mut()
|
||||
.insert_verified(dest, coords, stamped_ms);
|
||||
let last_used = |node: &Node| node.coord_cache().get_entry(&dest).unwrap().last_used();
|
||||
assert_eq!(
|
||||
last_used(&node),
|
||||
stamped_ms,
|
||||
"precondition: the entry carries the past stamp"
|
||||
);
|
||||
|
||||
for ttl in [1u8, 2] {
|
||||
let dg = SessionDatagram::new(src, dest, vec![0x10, 0x00, 0x00, 0x00]).with_ttl(ttl);
|
||||
let encoded = dg.encode();
|
||||
node.handle_session_datagram(&from, &encoded[1..], false)
|
||||
.await;
|
||||
if ttl == 1 {
|
||||
assert_eq!(
|
||||
last_used(&node),
|
||||
stamped_ms,
|
||||
"a transit ttl=1 datagram is dropped, so it must not refresh the destination's coords"
|
||||
);
|
||||
} else {
|
||||
assert!(
|
||||
last_used(&node) > stamped_ms,
|
||||
"a transit ttl=2 datagram reaches next-hop resolution, which refreshes the coords"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// --- Local delivery ---
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
+51
-1
@@ -144,6 +144,20 @@ pub struct SessionDatagramRef<'a> {
|
||||
pub payload: &'a [u8],
|
||||
}
|
||||
|
||||
/// The TTL a transit datagram leaves this node with, or `None` when it may not
|
||||
/// be transmitted because it would leave with zero.
|
||||
///
|
||||
/// Follows IP semantics: the decrement comes first, and `saturating_sub` folds
|
||||
/// an already-exhausted arrival (TTL 0) into the same outcome as a last-hop
|
||||
/// arrival (TTL 1). This is the one rule behind the routing core's hop-limit
|
||||
/// drop and both `can_forward`s.
|
||||
pub(crate) fn ttl_after_hop(ttl: u8) -> Option<u8> {
|
||||
match ttl.saturating_sub(1) {
|
||||
0 => None,
|
||||
left => Some(left),
|
||||
}
|
||||
}
|
||||
|
||||
/// SessionDatagram fixed header size: msg_type(1) + ttl(1) + path_mtu(2) + src_addr(16) + dest_addr(16).
|
||||
pub const SESSION_DATAGRAM_HEADER_SIZE: usize = 36;
|
||||
|
||||
@@ -191,7 +205,7 @@ impl SessionDatagram {
|
||||
/// True only at TTL 2 or more: at TTL 1 the decrement leaves zero, so the
|
||||
/// datagram is dropped rather than forwarded.
|
||||
pub fn can_forward(&self) -> bool {
|
||||
self.ttl > 1
|
||||
ttl_after_hop(self.ttl).is_some()
|
||||
}
|
||||
|
||||
/// Encode as link-layer message (msg_type + ttl + path_mtu + src_addr + dest_addr + payload).
|
||||
@@ -239,6 +253,12 @@ impl<'a> SessionDatagramRef<'a> {
|
||||
})
|
||||
}
|
||||
|
||||
/// Check whether this datagram would survive a transit hop, by the same
|
||||
/// rule the routing core drops on (`ttl_after_hop`).
|
||||
pub fn can_forward(&self) -> bool {
|
||||
ttl_after_hop(self.ttl).is_some()
|
||||
}
|
||||
|
||||
/// Materialize an owned datagram for forwarding/re-encoding paths.
|
||||
pub fn into_owned(self) -> SessionDatagram {
|
||||
SessionDatagram {
|
||||
@@ -424,6 +444,36 @@ mod tests {
|
||||
assert!(dg.with_ttl(255).can_forward());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ttl_after_hop_drops_only_what_would_leave_at_zero() {
|
||||
assert_eq!(
|
||||
ttl_after_hop(0),
|
||||
None,
|
||||
"an exhausted arrival leaves at zero"
|
||||
);
|
||||
assert_eq!(ttl_after_hop(1), None, "a last-hop arrival leaves at zero");
|
||||
assert_eq!(ttl_after_hop(2), Some(1));
|
||||
assert_eq!(ttl_after_hop(255), Some(254));
|
||||
|
||||
let dg = SessionDatagram::new(make_node_addr(1), make_node_addr(2), vec![0x42]);
|
||||
for ttl in 0..=u8::MAX {
|
||||
assert_eq!(
|
||||
ttl_after_hop(ttl).is_some(),
|
||||
ttl > 1,
|
||||
"ttl={ttl}: only a TTL of 2 or more survives the hop"
|
||||
);
|
||||
let owned = dg.clone().with_ttl(ttl);
|
||||
let encoded = owned.encode();
|
||||
let view = SessionDatagramRef::decode(&encoded[1..]).unwrap();
|
||||
assert_eq!(
|
||||
view.can_forward(),
|
||||
owned.can_forward(),
|
||||
"ttl={ttl}: the borrowed and owned views must agree"
|
||||
);
|
||||
assert_eq!(view.can_forward(), ttl_after_hop(ttl).is_some());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_session_datagram_decrement_ttl() {
|
||||
let base = SessionDatagram::new(make_node_addr(1), make_node_addr(2), vec![0x42]);
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
use super::limits::LimitVerdict;
|
||||
use super::state::Router;
|
||||
use super::wire::{CoordsRequired, MtuExceeded, PathBroken};
|
||||
use crate::proto::link::{SessionDatagram, SessionDatagramRef};
|
||||
use crate::proto::link::{SessionDatagram, SessionDatagramRef, ttl_after_hop};
|
||||
use crate::{NodeAddr, TreeCoordinate};
|
||||
|
||||
/// Read-only view of routing state the routing core needs.
|
||||
@@ -112,7 +112,8 @@ impl Router {
|
||||
/// datagram that would leave with a TTL of zero is not transmitted.
|
||||
///
|
||||
/// The shell pre-resolves `next_hop` only for datagrams this can actually
|
||||
/// forward (dest not local and TTL surviving the decrement), so
|
||||
/// forward (dest not local, and `SessionDatagramRef::can_forward`, which
|
||||
/// applies the same [`ttl_after_hop`] rule this drops on), so
|
||||
/// `find_next_hop`'s LRU-touch side effect stays scoped to genuine
|
||||
/// forwards. `route` still re-checks local delivery and the TTL
|
||||
/// authoritatively.
|
||||
@@ -132,15 +133,14 @@ impl Router {
|
||||
}
|
||||
|
||||
// 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 = dg.ttl.saturating_sub(1);
|
||||
if forwarded_ttl == 0 {
|
||||
// the datagram would leave with a TTL of zero. The already-exhausted
|
||||
// arrival (ttl=0) and the last-hop arrival (ttl=1) are both dropped;
|
||||
// neither is transmitted.
|
||||
let Some(forwarded_ttl) = ttl_after_hop(dg.ttl) else {
|
||||
return RouteOutcome::Drop {
|
||||
reason: DropReason::TtlExhausted,
|
||||
};
|
||||
}
|
||||
};
|
||||
|
||||
let nh = match next_hop {
|
||||
Some(nh) => nh,
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Tests for the sans-IO routing decision core.
|
||||
|
||||
use super::util::{MockPeer, MockRoutingView, make_coords, make_datagram_ref, make_next_hop};
|
||||
use crate::proto::link::SessionDatagramRef;
|
||||
use crate::proto::link::{SessionDatagramRef, ttl_after_hop};
|
||||
use crate::proto::routing::RoutingSignalType;
|
||||
use crate::proto::routing::{
|
||||
DropReason, LimitVerdict, RouteAction, RouteOutcome, Router, RoutingView, select_best_candidate,
|
||||
@@ -514,3 +514,45 @@ fn synth_mtu_exceeded_rate_limit_gate_suppresses_second_call() {
|
||||
assert!(third.action.is_some());
|
||||
assert_eq!(third.verdict, LimitVerdict::Admit);
|
||||
}
|
||||
|
||||
/// The routing core's hop-limit drop and the shell's `can_forward` pre-check
|
||||
/// agree for every TTL: a transit datagram with a next hop is dropped as
|
||||
/// TTL-exhausted exactly when `can_forward` is false, and otherwise leaves
|
||||
/// with the TTL `ttl_after_hop` gives.
|
||||
#[test]
|
||||
fn route_drops_for_hop_limit_exactly_when_can_forward_is_false() {
|
||||
let my_addr = make_node_addr(0x10);
|
||||
let nh_addr = make_node_addr(0x30);
|
||||
let rv = MockRoutingView::new(false);
|
||||
for ttl in 0..=u8::MAX {
|
||||
let mut router = Router::new();
|
||||
let dg = make_datagram_ref(ttl, make_node_addr(0x20));
|
||||
let out = router.route(
|
||||
&dg,
|
||||
&my_addr,
|
||||
false,
|
||||
Some(make_next_hop(nh_addr, 1400)),
|
||||
&rv,
|
||||
);
|
||||
match out {
|
||||
RouteOutcome::Drop {
|
||||
reason: DropReason::TtlExhausted,
|
||||
} => assert!(
|
||||
!dg.can_forward(),
|
||||
"ttl={ttl}: the core dropped a datagram the pre-check would forward"
|
||||
),
|
||||
RouteOutcome::Forward { bytes, .. } => {
|
||||
assert!(
|
||||
dg.can_forward(),
|
||||
"ttl={ttl}: the core forwarded a datagram the pre-check would not"
|
||||
);
|
||||
assert_eq!(
|
||||
Some(decode_forward(&bytes).ttl),
|
||||
ttl_after_hop(ttl),
|
||||
"ttl={ttl}: the forwarded TTL must be the shared rule's"
|
||||
);
|
||||
}
|
||||
_ => panic!("ttl={ttl}: expected Drop(TtlExhausted) or Forward"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user