diff --git a/src/node/mod.rs b/src/node/mod.rs index 93b7b93..04339a4 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -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) } diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 3c2dc20..9599a87 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -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; diff --git a/src/node/tests/spanning_tree.rs b/src/node/tests/spanning_tree.rs index 961ff67..9a49a3f 100644 --- a/src/node/tests/spanning_tree.rs +++ b/src/node/tests/spanning_tree.rs @@ -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, +) -> 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::(); @@ -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 { + // One identity per node; move the smallest-addr identity into the leaf slot. + let mut ids: Vec = (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 diff --git a/src/proto/stp/state.rs b/src/proto/stp/state.rs index fad3481..9ddc75a 100644 --- a/src/proto/stp/state.rs +++ b/src/proto/stp/state.rs @@ -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; } diff --git a/src/proto/stp/tests/state.rs b/src/proto/stp/tests/state.rs index 4284d55..b127e17 100644 --- a/src/proto/stp/tests/state.rs +++ b/src/proto/stp/tests/state.rs @@ -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);