mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
Stop a leaf node from self-electing as tree root
A leaf-profile node that held the smallest NodeAddr would self-elect as tree root, but its peers refuse a non-full node as a parent, so it formed an isolated second root and partitioned the mesh: a multi-hop session from the leaf to a non-adjacent full node then failed because the far node could not route a handshake reply back into the leaf's separate coordinate tree. Gate tree-state self-election on a new self_is_leaf flag, set from the node profile at construction. A leaf now attaches under its full upstream and holds that subtree's coordinate for its own routing. That coordinate's self < root would be rejected by the root-min wire check, but a leaf never announces it; it reaches peers only via the coordinates carried on the leaf's session frames, and the upstream already advertises the leaf in its bloom filter, so the far node routes back through the upstream. Also fix Node::with_identity to derive the node profile, leaf-only flag, and bloom state from the config, matching Node::new; it previously hardcoded the full profile and silently dropped a configured leaf or non-routing profile. Covered by sans-IO decision tests (leaf does not self-elect; leaf keeps its coordinate under a larger-rooted parent) and an end-to-end multi-hop regression, each confirmed to fail if the gate is reverted.
This commit is contained in:
+10
-3
@@ -593,6 +593,7 @@ impl Node {
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0);
|
||||
let mut tree_state = TreeState::new(node_addr, tree_now_secs);
|
||||
tree_state.set_self_is_leaf(node_profile == NodeProfile::Leaf);
|
||||
tree_state.set_parent_hysteresis(config.node.tree.parent_hysteresis);
|
||||
tree_state.set_hold_down(config.node.tree.hold_down_secs);
|
||||
tree_state.set_flap_dampening(
|
||||
@@ -743,6 +744,7 @@ impl Node {
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0);
|
||||
let mut tree_state = TreeState::new(node_addr, tree_now_secs);
|
||||
tree_state.set_self_is_leaf(config.node_profile() == NodeProfile::Leaf);
|
||||
tree_state.set_parent_hysteresis(config.node.tree.parent_hysteresis);
|
||||
tree_state.set_hold_down(config.node.tree.hold_down_secs);
|
||||
tree_state.set_flap_dampening(
|
||||
@@ -753,7 +755,11 @@ impl Node {
|
||||
tree::sign_declaration(tree_state.my_declaration_mut(), &identity)
|
||||
.expect("signing own declaration should never fail");
|
||||
|
||||
let mut bloom_state = BloomState::new(node_addr);
|
||||
let mut bloom_state = if config.is_leaf_only() {
|
||||
BloomState::leaf_only(node_addr)
|
||||
} else {
|
||||
BloomState::new(node_addr)
|
||||
};
|
||||
bloom_state.set_update_debounce_ms(config.node.bloom.update_debounce_ms);
|
||||
|
||||
let coord_cache = CoordCache::new(
|
||||
@@ -792,8 +798,8 @@ impl Node {
|
||||
identity.clone(),
|
||||
startup_epoch,
|
||||
started_at,
|
||||
false,
|
||||
NodeProfile::Full,
|
||||
config.is_leaf_only(),
|
||||
config.node_profile(),
|
||||
max_connections,
|
||||
max_peers,
|
||||
max_links,
|
||||
@@ -875,6 +881,7 @@ impl Node {
|
||||
ctx.is_leaf_only = true;
|
||||
ctx.node_profile = NodeProfile::Leaf;
|
||||
});
|
||||
node.tree_state.set_self_is_leaf(true);
|
||||
Ok(node)
|
||||
}
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ use crate::node::session::EndToEndState;
|
||||
use crate::node::tests::spanning_tree::{
|
||||
TestNode, cleanup_nodes, generate_random_edges, lock_large_network_test,
|
||||
process_available_packets, run_tree_test, run_tree_test_with_mtus, run_tree_test_with_profiles,
|
||||
verify_tree_convergence,
|
||||
run_tree_test_with_profiles_leaf_smallest, verify_tree_convergence,
|
||||
};
|
||||
use crate::proto::fsp::SessionAck;
|
||||
use crate::proto::link::SessionDatagram;
|
||||
@@ -676,6 +676,86 @@ async fn mixed_profile_nodes_converge_and_forward() {
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
/// Regression: a Leaf holding the smallest NodeAddr must not self-elect as root
|
||||
/// and partition the mesh, so a multi-hop session from the Leaf to a
|
||||
/// non-adjacent Full node still establishes and delivers.
|
||||
///
|
||||
/// Topology A(Full) — B(Full), with D(Leaf) hanging off A. D is pinned to the
|
||||
/// strictly smallest NodeAddr — the condition that made D self-elect as a second
|
||||
/// root, partitioning A/B's tree from D's and leaving B unable to route its
|
||||
/// handshake reply back to D. The multi-hop pair D->B (routed through A) is the
|
||||
/// one the Docker `mixed-profile` suite covered.
|
||||
///
|
||||
/// Only Full and Leaf profiles appear here so the elected root is always the
|
||||
/// smaller of the two Full nodes: the separate global-min-NonRouting partition
|
||||
/// (a distinct open problem for the leaf/non-routing tree-participation model) is
|
||||
/// deliberately kept out so this test isolates the Leaf fix.
|
||||
///
|
||||
/// Without the leaf gate D self-roots every run and D->B fails; with it, D
|
||||
/// attaches under A and the session establishes.
|
||||
#[tokio::test]
|
||||
async fn leaf_smallest_addr_does_not_partition_multihop() {
|
||||
use crate::proto::fmp::NodeProfile;
|
||||
|
||||
// 0=A Full, 1=B Full, 2=D Leaf (pinned smallest).
|
||||
let profiles = [NodeProfile::Full, NodeProfile::Full, NodeProfile::Leaf];
|
||||
// A-B, A-D (D is a single-upstream leaf hanging off A).
|
||||
let edges = [(0, 1), (0, 2)];
|
||||
let mut nodes = run_tree_test_with_profiles_leaf_smallest(&profiles, 2, &edges).await;
|
||||
|
||||
// The mesh must be a single tree rooted at a Full node: D (the smallest
|
||||
// addr) must NOT be its own root. A partition shows up as D self-rooted.
|
||||
assert!(
|
||||
!nodes[2].node.tree_state().is_root(),
|
||||
"the Leaf (smallest addr) must not self-elect as root"
|
||||
);
|
||||
let roots: Vec<_> = nodes.iter().map(|n| *n.node.tree_state().root()).collect();
|
||||
assert!(
|
||||
roots.iter().all(|r| *r == roots[0]),
|
||||
"all nodes must share one root (no partition); got {roots:?}"
|
||||
);
|
||||
|
||||
populate_all_coord_caches(&mut nodes);
|
||||
|
||||
// TUN receiver on B so delivered plaintext can be observed.
|
||||
let (tx, b_rx) = std::sync::mpsc::channel();
|
||||
nodes[1].node.supervisor.tun_tx = Some(tx);
|
||||
|
||||
let d_addr = *nodes[2].node.node_addr();
|
||||
let (b_addr, b_pubkey) = (
|
||||
*nodes[1].node.node_addr(),
|
||||
nodes[1].node.identity().pubkey_full(),
|
||||
);
|
||||
|
||||
// D -> B: the multi-hop pair routed through A.
|
||||
nodes[2]
|
||||
.node
|
||||
.initiate_session(b_addr, b_pubkey)
|
||||
.await
|
||||
.expect("D->B initiate_session must succeed (no partition)");
|
||||
drain_to_quiescence(&mut nodes).await;
|
||||
|
||||
let payload = b"leaf-multihop".to_vec();
|
||||
let d_fips = crate::FipsAddress::from_node_addr(&d_addr);
|
||||
let b_fips = crate::FipsAddress::from_node_addr(&b_addr);
|
||||
let ipv6 = build_ipv6_packet(&d_fips, &b_fips, &payload);
|
||||
nodes[2]
|
||||
.node
|
||||
.send_ipv6_packet(&b_addr, &ipv6)
|
||||
.await
|
||||
.expect("D->B send must succeed");
|
||||
drain_to_quiescence(&mut nodes).await;
|
||||
|
||||
let found = std::iter::from_fn(|| b_rx.try_recv().ok())
|
||||
.any(|pkt| pkt.len() >= 40 && pkt[40..] == payload[..]);
|
||||
assert!(
|
||||
found,
|
||||
"D->B multi-hop datagram must be delivered to B's TUN"
|
||||
);
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_session_100_nodes() {
|
||||
let _guard = lock_large_network_test().await;
|
||||
|
||||
@@ -98,9 +98,40 @@ pub(super) async fn make_test_node_with_profile(
|
||||
make_test_node_inner(config, 1280).await
|
||||
}
|
||||
|
||||
/// Create a test node with a specific profile AND a caller-chosen identity, so
|
||||
/// a test can pin which node holds the smallest NodeAddr (e.g. force a Leaf to
|
||||
/// be the numerically smallest node and reproduce the root-election partition
|
||||
/// deterministically).
|
||||
pub(super) async fn make_test_node_with_profile_and_identity(
|
||||
profile: crate::proto::fmp::NodeProfile,
|
||||
identity: Identity,
|
||||
) -> TestNode {
|
||||
use crate::proto::fmp::NodeProfile;
|
||||
let mut config = Config::new();
|
||||
match profile {
|
||||
NodeProfile::Leaf => config.node.leaf_only = true,
|
||||
NodeProfile::NonRouting => config.node.disable_routing = true,
|
||||
NodeProfile::Full => {}
|
||||
}
|
||||
make_test_node_inner_with_identity(config, 1280, Some(identity)).await
|
||||
}
|
||||
|
||||
/// Shared builder: a test node from an explicit `Config` and transport MTU.
|
||||
async fn make_test_node_inner(config: Config, mtu: u16) -> TestNode {
|
||||
let mut node = make_node_with(config);
|
||||
make_test_node_inner_with_identity(config, mtu, None).await
|
||||
}
|
||||
|
||||
/// Shared builder with an optional caller-chosen identity. `None` generates a
|
||||
/// fresh random identity (the default); `Some` pins it via `Node::with_identity`.
|
||||
async fn make_test_node_inner_with_identity(
|
||||
config: Config,
|
||||
mtu: u16,
|
||||
identity: Option<Identity>,
|
||||
) -> TestNode {
|
||||
let mut node = match identity {
|
||||
Some(id) => Node::with_identity(id, config).expect("build node with identity"),
|
||||
None => make_node_with(config),
|
||||
};
|
||||
let transport_id = TransportId::new(1);
|
||||
|
||||
let (tx, rx) = tokio::sync::mpsc::unbounded_channel::<ReceivedPacket>();
|
||||
@@ -815,6 +846,42 @@ pub(super) async fn run_tree_test_with_profiles(
|
||||
nodes
|
||||
}
|
||||
|
||||
/// Like `run_tree_test_with_profiles`, but pins node identities so the node at
|
||||
/// `leaf_idx` holds the strictly smallest NodeAddr in the mesh.
|
||||
///
|
||||
/// This makes the root-election partition deterministic: a Leaf (or any non-Full
|
||||
/// node) that holds the smallest NodeAddr is the one that would self-elect as a
|
||||
/// second root. Without the leaf gate the resulting mesh partitions; with it the
|
||||
/// Leaf attaches under its Full upstream and the tree stays connected.
|
||||
pub(super) async fn run_tree_test_with_profiles_leaf_smallest(
|
||||
profiles: &[crate::proto::fmp::NodeProfile],
|
||||
leaf_idx: usize,
|
||||
edges: &[(usize, usize)],
|
||||
) -> Vec<TestNode> {
|
||||
// One identity per node; move the smallest-addr identity into the leaf slot.
|
||||
let mut ids: Vec<Identity> = (0..profiles.len()).map(|_| Identity::generate()).collect();
|
||||
let smallest = (0..ids.len())
|
||||
.min_by(|&a, &b| ids[a].node_addr().cmp(ids[b].node_addr()))
|
||||
.expect("non-empty");
|
||||
ids.swap(leaf_idx, smallest);
|
||||
assert!(
|
||||
(0..ids.len()).all(|i| i == leaf_idx || ids[leaf_idx].node_addr() < ids[i].node_addr()),
|
||||
"leaf must hold the unique smallest NodeAddr"
|
||||
);
|
||||
|
||||
let mut nodes = Vec::with_capacity(profiles.len());
|
||||
for (i, &profile) in profiles.iter().enumerate() {
|
||||
nodes.push(make_test_node_with_profile_and_identity(profile, ids[i].clone()).await);
|
||||
}
|
||||
for &(i, j) in edges {
|
||||
initiate_handshake(&mut nodes, i, j).await;
|
||||
}
|
||||
let total = drain_all_packets(&mut nodes, false).await;
|
||||
assert!(total > 0, "Should have processed at least some packets");
|
||||
repair_missing_edge_handshakes(&mut nodes, edges, false).await;
|
||||
nodes
|
||||
}
|
||||
|
||||
/// Like `run_tree_test` but with per-node transport MTUs.
|
||||
///
|
||||
/// `mtus` must have one entry per node. Used for heterogeneous-MTU tests
|
||||
|
||||
+47
-3
@@ -30,6 +30,20 @@ pub struct TreeState {
|
||||
parent_hysteresis: f64,
|
||||
/// Flap-dampening / hold-down state machine.
|
||||
flap: FlapDampener,
|
||||
/// Whether this node is a Leaf-profile node.
|
||||
///
|
||||
/// A Leaf holds a single upstream Full peer, sends no tree announces, and
|
||||
/// must never self-elect as tree root: peers refuse a non-Full node as a
|
||||
/// parent (`non_full_peers()` skip), so a Leaf that self-elected would form
|
||||
/// an isolated second root and partition the mesh. When `true`, the node is
|
||||
/// excluded from root self-election and attaches under its upstream instead,
|
||||
/// holding that subtree's coordinate for its own routing (never announced, so
|
||||
/// the coordinate's `self < root` is safe). Defaults to `false`
|
||||
/// (tree-participating); the shell sets it from the node profile. NonRouting
|
||||
/// nodes keep `false` — they announce, so the same relaxation would emit a
|
||||
/// wire-invalid coordinate; a global-min NonRouting node is a separate open
|
||||
/// problem for the leaf/non-routing tree-participation model.
|
||||
self_is_leaf: bool,
|
||||
}
|
||||
|
||||
impl TreeState {
|
||||
@@ -52,6 +66,7 @@ impl TreeState {
|
||||
peer_ancestry: BTreeMap::new(),
|
||||
parent_hysteresis: 0.0,
|
||||
flap: FlapDampener::new(),
|
||||
self_is_leaf: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -187,10 +202,18 @@ impl TreeState {
|
||||
let parent_id = self.my_declaration.parent_id();
|
||||
if let Some(parent_coords) = self.peer_ancestry.get(parent_id) {
|
||||
let parent_root = *parent_coords.root_id();
|
||||
if self.my_node_addr <= parent_root {
|
||||
if !self.self_is_leaf && self.my_node_addr <= parent_root {
|
||||
// Prepending self would put a smaller-or-equal node at depth 0,
|
||||
// breaking the "advertised root = min path entry" invariant.
|
||||
// Demote to self-root rather than emit a path peers will reject.
|
||||
//
|
||||
// A Leaf is exempt: it keeps the `[self, parent, …, root]`
|
||||
// coordinate even when `self <= parent_root`, so it lives as a
|
||||
// depth-N member of its upstream's tree rather than demoting to
|
||||
// an isolated self-root (which would partition the mesh). Safe
|
||||
// because a Leaf never announces this coordinate; it reaches
|
||||
// peers only via session-carried coords, which are not subject
|
||||
// to the root-min `validate_semantics`.
|
||||
let seq = self.my_declaration.sequence();
|
||||
let ts = self.my_declaration.timestamp();
|
||||
self.my_declaration = ParentDeclaration::self_root(self.my_node_addr, seq, ts);
|
||||
@@ -218,9 +241,14 @@ impl TreeState {
|
||||
|
||||
/// Whether this node should be the tree root: either there are no peers,
|
||||
/// or our NodeAddr is `<=` every visible root.
|
||||
///
|
||||
/// A Leaf never self-elects as root while it has a peer to attach under: it
|
||||
/// cannot forward transit, peers refuse it as a parent, and a Leaf-root
|
||||
/// would partition the mesh. An isolated Leaf (no visible root) is still its
|
||||
/// own root, which is harmless.
|
||||
pub fn should_be_root(&self) -> bool {
|
||||
match self.smallest_visible_root() {
|
||||
Some(sr) => self.my_node_addr <= sr,
|
||||
Some(sr) => !self.self_is_leaf && self.my_node_addr <= sr,
|
||||
None => true,
|
||||
}
|
||||
}
|
||||
@@ -295,6 +323,14 @@ impl TreeState {
|
||||
}
|
||||
}
|
||||
|
||||
/// Mark this node as a Leaf (or not), gating it out of root self-election.
|
||||
///
|
||||
/// A Leaf attaches under its upstream Full peer and never becomes root; see
|
||||
/// the `self_is_leaf` field docs. Set from the node profile at construction.
|
||||
pub fn set_self_is_leaf(&mut self, is_leaf: bool) {
|
||||
self.self_is_leaf = is_leaf;
|
||||
}
|
||||
|
||||
/// Set the parent hysteresis factor (0.0-1.0).
|
||||
pub fn set_parent_hysteresis(&mut self, hysteresis: f64) {
|
||||
self.parent_hysteresis = hysteresis.clamp(0.0, 1.0);
|
||||
@@ -379,7 +415,15 @@ impl TreeState {
|
||||
// `self` to that peer's ancestry would put `self` at depth 0 and the
|
||||
// peer's larger root at the tail — violating "advertised root = min path
|
||||
// entry" and getting rejected by recipients' `validate_semantics`.
|
||||
if self.my_node_addr <= smallest_root {
|
||||
//
|
||||
// A Leaf is exempt: it must not self-elect as root (peers refuse it as a
|
||||
// parent, so it would partition the mesh). It falls through to select its
|
||||
// upstream Full peer as parent. The resulting
|
||||
// `self < root` coordinate is invalid on the wire, but a Leaf never
|
||||
// announces it (`send_tree_announce_to_peer` is Leaf-gated); it is used
|
||||
// only for the Leaf's own routing and propagates to peers via the
|
||||
// coords carried on its session frames, which are not root-validated.
|
||||
if !self.self_is_leaf && self.my_node_addr <= smallest_root {
|
||||
return ParentEval::None;
|
||||
}
|
||||
|
||||
|
||||
@@ -513,6 +513,86 @@ fn test_recompute_coords_demotes_when_self_smaller_than_parent_root() {
|
||||
assert_eq!(*state.my_coords().root_id(), min);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_leaf_does_not_self_elect_and_selects_full_upstream() {
|
||||
// A Leaf holding the smallest NodeAddr must NOT self-elect as root. Peers
|
||||
// refuse a non-Full node as a parent (non_full_peers skip), so a self-elected
|
||||
// Leaf becomes an isolated second root and partitions the mesh. With the leaf
|
||||
// gate it instead selects its Full upstream as parent.
|
||||
let leaf = make_node_addr(1); // smallest addr in the mesh
|
||||
let upstream = make_node_addr(3); // Full upstream, a depth-1 member of B's tree
|
||||
let real_root = make_node_addr(2); // the upstream is rooted here
|
||||
|
||||
let mut state = TreeState::new(leaf, 1000);
|
||||
state.set_self_is_leaf(true);
|
||||
state.update_peer(
|
||||
ParentDeclaration::new(upstream, real_root, 1, 1000),
|
||||
make_coords(&[3, 2]),
|
||||
);
|
||||
|
||||
let eval = state.evaluate_parent(&BTreeMap::new(), &BTreeSet::new());
|
||||
assert_eq!(
|
||||
eval.switch_target(),
|
||||
Some(upstream),
|
||||
"leaf must attach under its Full upstream, not self-elect"
|
||||
);
|
||||
assert!(
|
||||
!state.should_be_root(),
|
||||
"leaf must not claim root even as the smallest NodeAddr"
|
||||
);
|
||||
|
||||
// Break-test: the identical topology on a NON-leaf (Full) self reproduces
|
||||
// the self-election the gate suppresses — proving the gate is what fires.
|
||||
let mut full = TreeState::new(leaf, 1000);
|
||||
full.update_peer(
|
||||
ParentDeclaration::new(upstream, real_root, 1, 1000),
|
||||
make_coords(&[3, 2]),
|
||||
);
|
||||
assert_eq!(
|
||||
full.evaluate_parent(&BTreeMap::new(), &BTreeSet::new())
|
||||
.switch_target(),
|
||||
None,
|
||||
"a Full smallest-addr node self-elects (evaluate_parent returns None)"
|
||||
);
|
||||
assert!(
|
||||
full.should_be_root(),
|
||||
"a Full smallest-addr node claims root"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_leaf_keeps_coord_under_larger_rooted_parent() {
|
||||
// A Leaf attaching under a larger-rooted Full parent keeps the
|
||||
// [self, parent, root] coordinate rather than demoting to self-root, so
|
||||
// it lives as a depth-N member of the single tree. The coordinate's
|
||||
// self < root is safe because a Leaf never announces it (it reaches peers
|
||||
// only via the coords carried on its session frames, which are not
|
||||
// root-min validated). Contrast with
|
||||
// test_recompute_coords_demotes_when_self_smaller_than_parent_root, where a
|
||||
// Full self in the same position (default self_is_leaf=false) demotes.
|
||||
let leaf = make_node_addr(1);
|
||||
let upstream = make_node_addr(3);
|
||||
let real_root = make_node_addr(2);
|
||||
|
||||
let mut state = TreeState::new(leaf, 1000);
|
||||
state.set_self_is_leaf(true);
|
||||
state.update_peer(
|
||||
ParentDeclaration::new(upstream, real_root, 1, 1000),
|
||||
make_coords(&[3, 2]),
|
||||
);
|
||||
|
||||
state.set_parent(upstream, 2, 2000, 2000);
|
||||
state.recompute_coords();
|
||||
|
||||
assert!(!state.is_root(), "leaf must not demote to self-root");
|
||||
assert_eq!(state.root(), &real_root, "leaf adopts its upstream's root");
|
||||
assert_eq!(
|
||||
state.my_coords().entries().len(),
|
||||
3,
|
||||
"coordinate is [leaf, upstream, root]"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_handle_parent_lost_becomes_root() {
|
||||
let my_node = make_node_addr(5);
|
||||
|
||||
Reference in New Issue
Block a user