mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
Pin the flap-dampening re-arm and the coordinate-gate ordering
Both fixes shipped green with no test that would fail if they were reverted. The dampening tests drive a second episode after the first lapses, which none of the six existing tests attempted; the one that looked closest engages with a zero-second duration and never re-arms. Break-checked by reverting the lapse-aware gate. The gate-ordering tests pin that an inadmissible coordinate signal never reaches the response rate limiter at all. Moving the gate below the limiter left both existing tests green, because a false return from the limiter does not short-circuit the handler, so the attacker-chosen address still lands in a map pruned only every ten seconds. Green: fmt, build, clippy and test --lib, 1526 passed.
This commit is contained in:
@@ -2865,9 +2865,11 @@ async fn test_coords_required_naming_a_dest_with_no_session_is_counted_as_an_unk
|
||||
node.handle_coords_required(&reporter, &encoded[5..]).await;
|
||||
assert_eq!(node.stats().session.unknown_session, 1);
|
||||
|
||||
// The gate runs ahead of the response rate limiter, so a second
|
||||
// identical signal is rejected the same way rather than being
|
||||
// absorbed by rate-limiter state keyed on an attacker-chosen address.
|
||||
// A second identical signal is refused the same way. This does not pin
|
||||
// the gate's position relative to the response rate limiter: should_send
|
||||
// returning false would not short-circuit the handler, so this counter
|
||||
// reaches 2 either way. The ordering is pinned by
|
||||
// test_coords_required_for_an_unbound_dest_never_reaches_the_response_rate_limiter.
|
||||
node.handle_coords_required(&reporter, &encoded[5..]).await;
|
||||
assert_eq!(node.stats().session.unknown_session, 2);
|
||||
|
||||
@@ -2899,6 +2901,51 @@ async fn test_coords_required_naming_a_dest_with_no_session_is_counted_as_an_unk
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_coords_required_for_an_unbound_dest_never_reaches_the_response_rate_limiter() {
|
||||
let mut node = make_node();
|
||||
|
||||
let dest = NodeAddr::from_bytes([0xCC; 16]);
|
||||
let reporter = NodeAddr::from_bytes([0xBB; 16]);
|
||||
|
||||
assert_eq!(
|
||||
node.coords_response_rate_limiter.len(),
|
||||
0,
|
||||
"precondition: the response rate limiter holds nothing before the signal"
|
||||
);
|
||||
|
||||
let encoded = CoordsRequired::new(dest, reporter).encode();
|
||||
node.handle_coords_required(&reporter, &encoded[5..]).await;
|
||||
|
||||
assert_eq!(node.stats().session.unknown_session, 1);
|
||||
assert_eq!(
|
||||
node.coords_response_rate_limiter.len(),
|
||||
0,
|
||||
"an inadmissible signal must be refused before should_send can insert \
|
||||
the attacker-chosen address into last_sent"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_coords_required_for_a_bound_dest_does_reach_the_response_rate_limiter() {
|
||||
let mut node = make_node();
|
||||
|
||||
let remote = Identity::generate();
|
||||
install_initiating(&mut node, &remote);
|
||||
let dest = *remote.node_addr();
|
||||
let reporter = NodeAddr::from_bytes([0xBB; 16]);
|
||||
|
||||
let encoded = CoordsRequired::new(dest, reporter).encode();
|
||||
node.handle_coords_required(&reporter, &encoded[5..]).await;
|
||||
|
||||
assert_eq!(node.stats().session.unknown_session, 0);
|
||||
assert_eq!(
|
||||
node.coords_response_rate_limiter.len(),
|
||||
1,
|
||||
"an admitted signal must still consult the response rate limiter"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_mtu_exceeded_whose_claimed_source_is_the_destination_it_names_is_dropped() {
|
||||
let mut node = make_node();
|
||||
|
||||
@@ -1816,3 +1816,80 @@ fn test_handle_parent_lost_keeps_its_published_bool_return() {
|
||||
assert!(published(&mut state, &HashMap::new()));
|
||||
assert!(state.is_root());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_flap_dampening_counter_resets_when_the_episode_lapses_not_when_it_engages() {
|
||||
// Retirement clears the switch counter when a lapsed episode is retired,
|
||||
// not when the episode is armed, so switches taken *during* an episode
|
||||
// do not carry into the next window. Telling those two apart needs an
|
||||
// episode that is genuinely live for at least one switch, and
|
||||
// `record_parent_switch` reads `Instant::now()` directly with no clock
|
||||
// seam on this line, so the shortest live episode the configuration can
|
||||
// usefully express has to be waited out in real time.
|
||||
let my_node = make_node_addr(5);
|
||||
let mut state = TreeState::new(my_node);
|
||||
state.set_flap_dampening(3, 60, 2);
|
||||
state.set_hold_down(0);
|
||||
|
||||
let peer_a = make_node_addr(1);
|
||||
let peer_b = make_node_addr(2);
|
||||
let root = make_node_addr(0);
|
||||
|
||||
state.update_peer(
|
||||
ParentDeclaration::new(peer_a, root, 1, 1000),
|
||||
make_coords(&[1, 0]),
|
||||
);
|
||||
state.update_peer(
|
||||
ParentDeclaration::new(peer_b, root, 1, 1000),
|
||||
make_coords(&[2, 0]),
|
||||
);
|
||||
|
||||
assert!(!state.set_parent(peer_a, 1, 1000));
|
||||
state.recompute_coords();
|
||||
assert!(!state.set_parent(peer_b, 2, 2000));
|
||||
state.recompute_coords();
|
||||
assert!(
|
||||
state.set_parent(peer_a, 3, 3000),
|
||||
"the third switch inside the window must arm an episode"
|
||||
);
|
||||
state.recompute_coords();
|
||||
assert!(
|
||||
state.is_flap_dampened(),
|
||||
"a two-second episode must be live immediately after it is armed"
|
||||
);
|
||||
|
||||
// In-episode accumulation: in production a mandatory switch bypasses the
|
||||
// veto and still feeds the counter. `set_parent` is the same entry point
|
||||
// those switches reach, and it must not re-arm the running episode.
|
||||
assert!(
|
||||
!state.set_parent(peer_b, 4, 4000),
|
||||
"a switch inside a live episode must not re-arm it"
|
||||
);
|
||||
state.recompute_coords();
|
||||
assert!(state.is_flap_dampened());
|
||||
|
||||
std::thread::sleep(std::time::Duration::from_millis(2_200));
|
||||
assert!(
|
||||
!state.is_flap_dampened(),
|
||||
"the episode must have lapsed after its two seconds"
|
||||
);
|
||||
|
||||
// The lapsed episode is retired on the next switch, taking the counter
|
||||
// with it: the in-episode switch must not count toward the next episode.
|
||||
assert!(
|
||||
!state.set_parent(peer_a, 5, 5000),
|
||||
"retiring a lapsed episode must clear the switch counter"
|
||||
);
|
||||
state.recompute_coords();
|
||||
assert!(
|
||||
!state.set_parent(peer_b, 6, 6000),
|
||||
"a second episode must cost a fresh threshold of switches"
|
||||
);
|
||||
state.recompute_coords();
|
||||
assert!(
|
||||
state.set_parent(peer_a, 7, 7000),
|
||||
"a second episode must arm once the fresh threshold is reached"
|
||||
);
|
||||
state.recompute_coords();
|
||||
assert!(state.is_flap_dampened());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user