diff --git a/CHANGELOG.md b/CHANGELOG.md index a9298f1d..f07f7d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 non-positive rate is rejected at config validation rather than silently refusing every session. +- `node.limits.max_sessions`, defaulting to 1024, which bounds the end-to-end + session table. Zero means unlimited, which restores the previous behaviour + exactly and is the way to back the change out on a running node. The default + is four times the adjacent `node.session.pending_max_destinations`. A + session entry measures 6608 bytes of inline state plus heap, so the table + holds to roughly 7 MB, and a test pins that per-entry figure so the + arithmetic behind the default fails loudly if an entry grows. Existing + configurations parse unchanged, the key being optional. + - `node.rate_limit.established_handshake_burst` and `node.rate_limit.established_handshake_rate`, the parameters of the new established-link msg1 token bucket, which meters link-layer msg1 rather @@ -1189,6 +1198,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 time, so a peer admitted during a window that has already happened keeps its link. +- The end-to-end session table now has a bound. It was the one remotely-grown + map with none: an inbound SessionSetup naming an address nobody had seen + inserted an entry, and the two existing limits did not reach it, the setup + limiter governing the arrival rate rather than the population and the idle + purge only reaching entries a peer stops using. One neighbour sending setups + at the permitted rate could hold roughly 1440 half-open entries at any + moment and grow the table without limit by keeping them warm. Setups that + would grow the table past `node.limits.max_sessions` are now refused, ahead + of the setup limiter, so a full table costs no token, no responder handshake + and no ack; a refused setup emits nothing at all, which is indistinguishable + from loss to the sender and is already covered by its own msg1 resend + schedule. The test is whether admitting would grow the table, not whether + the sender is a stranger, so a resent setup for an entry already present is + still served and an in-flight handshake is not broken. Unauthenticated + half-open entries are additionally held to half the table, so a handshake + flood cannot deny the whole of it to peers that complete; that share is sized + to leave a reconnect storm, where every peer initiates at once after a + restart or a healed partition, room to land. Locally originated sessions are + capped at the same ceiling, answered with ICMPv6 destination unreachable so + the application gets an immediate error rather than a silent drop. The cap + refuses rather than evicts: the setup that triggers the decision is + unauthenticated at that point, so evicting would hand a stranger a way to + tear down sessions it has nothing to do with. Refusals are counted as + `table_full` and `half_open_full` in the session reject family. What stays + open is per-neighbour fairness among established sessions: one hostile + neighbour that completes handshakes and keeps each session warm can occupy + the table and hold new session establishment closed for as long as it keeps + doing so, which is a denial of new sessions rather than the unbounded memory + growth it replaces. + - An accepted inbound TCP connection no longer holds a slot indefinitely without sending anything. The cap was tested at accept and the pool insert and counter bump followed with no read in between, while the frame reader's diff --git a/src/config/node.rs b/src/config/node.rs index eead8709..38b614c6 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -28,6 +28,20 @@ pub struct LimitsConfig { /// Max pending inbound handshakes (`node.limits.max_pending_inbound`). #[serde(default = "LimitsConfig::default_max_pending_inbound")] pub max_pending_inbound: usize, + /// Max end-to-end sessions (`node.limits.max_sessions`), `0` = unlimited. + /// + /// The session table is the only remotely-grown map with no bound: an + /// inbound SessionSetup from an address nobody has seen inserts an + /// entry, and the idle purge only reaches entries a peer stops using. + /// The default of 1024 is four times the adjacent + /// `node.session.pending_max_destinations`. One entry measures 6608 + /// bytes of inline state plus heap, so the table holds to roughly 7 MB + /// and a test pins the per-entry figure the default rests on. Raising it + /// raises the memory an attacker can make this node hold; lowering it + /// refuses new sessions sooner on a node that legitimately talks + /// end-to-end to many others, such as a gateway. + #[serde(default = "LimitsConfig::default_max_sessions")] + pub max_sessions: usize, } impl Default for LimitsConfig { @@ -37,6 +51,7 @@ impl Default for LimitsConfig { max_peers: 128, max_links: 256, max_pending_inbound: 1000, + max_sessions: 1024, } } } @@ -54,6 +69,9 @@ impl LimitsConfig { fn default_max_pending_inbound() -> usize { 1000 } + fn default_max_sessions() -> usize { + 1024 + } } /// Rate limiting (`node.rate_limit.*`). diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 3601dd71..6e5e2ec9 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -64,6 +64,20 @@ fn link_wire_len(encoded_len: usize) -> usize { encoded_len + LINK_FRAME_OVERHEAD } +/// Divisor giving the share of the session table that unauthenticated +/// half-open entries may hold, as `max_sessions / DIVISOR`. +/// +/// Two means a reconnect storm, where every peer that had a session +/// initiates at once after a restart or a healed partition, still fits in +/// half the table; a tighter share bites four times sooner and is felt by +/// a hub before it is felt by an attacker. Half-open entries are reaped +/// after `handshake_timeout_secs` while established ones survive +/// `idle_timeout_secs`, so they turn over faster than the share suggests. +/// Lowering the divisor raises the share, which lets a handshake flood +/// crowd out peers that complete; raising it refuses legitimate initiators +/// sooner in a storm. +const HALF_OPEN_SHARE_DIVISOR: usize = 2; + /// Inputs to `try_send_session_data_pipelined` — the FSP+FMP pipelined /// fast path that hands both AEAD operations to the encrypt worker /// in a single dispatch. @@ -517,6 +531,19 @@ impl Node { // no limit at all. A setup naming an established peer cannot grow the // table and is metered separately, so that a stranger flood over a // shared link cannot stop that peer's rekey from arming. + // Population cap, ahead of the limiter so a full table costs no + // token, no responder handshake and no ack. The predicate is "would + // admitting this grow the table", not "is this a stranger": `class` + // is Stranger for an existing Initiating or AwaitingMsg3 entry too, + // and refusing those would break in-flight legitimate handshakes and + // the duplicate-ack resend. Same shape as the pending-destination cap + // in `queue_pending_packet`. Refuse rather than evict: msg1 is + // unauthenticated here, so evicting would hand a stranger a teardown + // primitive it does not have. + if !self.admit_new_session(src_addr) { + return; + } + let class = if self .sessions .get(src_addr) @@ -1798,6 +1825,59 @@ impl Node { /// Creates a Noise XK handshake as initiator, wraps msg1 in a /// SessionSetup, encapsulates in a SessionDatagram, and routes /// toward the destination. + /// Whether a session for `addr` may be created, given the table cap. + /// + /// Returns true when an entry already exists, since admitting it cannot + /// grow the table. Counts its own refusals, so the two reasons are + /// distinguishable without turning on debug logging. + pub(in crate::node) fn admit_new_session(&mut self, addr: &NodeAddr) -> bool { + let max_sessions = self.config().node.limits.max_sessions; + if max_sessions == 0 || self.sessions.contains_key(addr) { + return true; + } + + if self.sessions.len() >= max_sessions { + debug!( + src = %self.peer_display_name(addr), + sessions = self.sessions.len(), + max_sessions = max_sessions, + "Session table full, refusing to create a session" + ); + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::TableFull)); + return false; + } + + // Half-open entries are unauthenticated and are reaped after + // `handshake_timeout_secs`, so they are the cheap half of the table + // to fill. Holding them to a share keeps room for peers that + // complete. The outer length test makes the scan unreachable below + // the share, and the table is itself bounded by the cap above. + // At least one, or a table capped at one would admit no inbound + // session at all rather than one. + let half_open_share = (max_sessions / HALF_OPEN_SHARE_DIVISOR).max(1); + if self.sessions.len() >= half_open_share { + let half_open = self + .sessions + .values() + .filter(|e| e.is_awaiting_msg3()) + .count(); + if half_open >= half_open_share { + debug!( + src = %self.peer_display_name(addr), + half_open = half_open, + half_open_share = half_open_share, + "Half-open session share exhausted, refusing to create a session" + ); + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::HalfOpenFull)); + return false; + } + } + + true + } + pub(in crate::node) async fn initiate_session( &mut self, dest_addr: NodeAddr, @@ -2639,6 +2719,17 @@ impl Node { return; } + // No session, so this one would grow the table. Answer the local + // application the way an unroutable destination is answered rather + // than returning an error from `initiate_session`: the caller reads + // an error as "no route" and responds with a discovery lookup and a + // queued packet, which is outbound traffic on a node already at its + // limit. + if !self.admit_new_session(&dest_addr) { + self.send_icmpv6_dest_unreachable(&ipv6_packet); + return; + } + // No session: initiate one and queue the packet. // If session initiation fails (no route), trigger discovery and // queue the packet for retry when discovery completes. @@ -2769,6 +2860,10 @@ impl Node { return; } + if !self.admit_new_session(&dest_addr) { + return; + } + match self.initiate_session(dest_addr, dest_pubkey).await { Ok(()) => { debug!(dest = %self.peer_display_name(&dest_addr), "Session initiated after discovery"); diff --git a/src/node/reject.rs b/src/node/reject.rs index 40499f32..ea58e467 100644 --- a/src/node/reject.rs +++ b/src/node/reject.rs @@ -258,6 +258,18 @@ pub enum SessionReject { /// before any handshake state was created or any ack sent. Tracked via /// [`SessionStats::setup_rate_limited`](crate::node::stats::SessionStats). SetupRateLimited, + /// A session would have been created but the table is at + /// `node.limits.max_sessions`. Refused rather than evicted: the + /// deciding message is unauthenticated at this point, so evicting + /// would hand a stranger a way to tear down established sessions. + /// Tracked via + /// [`SessionStats::table_full`](crate::node::stats::SessionStats). + TableFull, + /// A session would have been created but unauthenticated half-open + /// entries already hold their share of the table. Bounds what a + /// handshake flood can deny an established peer. Tracked via + /// [`SessionStats::half_open_full`](crate::node::stats::SessionStats). + HalfOpenFull, } /// MMP rejection reasons. diff --git a/src/node/stats.rs b/src/node/stats.rs index 52d0f399..d6028f62 100644 --- a/src/node/stats.rs +++ b/src/node/stats.rs @@ -79,6 +79,14 @@ pub struct SessionStats { /// A setup message was refused by the per-link-peer setup limiter, /// before any handshake state was created or any ack sent. pub setup_rate_limited: u64, + /// A session would have been created but the table is at + /// `node.limits.max_sessions`. A sustained rate means either the cap + /// is sized below what this node legitimately carries, or something is + /// holding the table full. + pub table_full: u64, + /// A session would have been created but unauthenticated half-open + /// entries already hold their share of the table. + pub half_open_full: u64, } impl SessionStats { @@ -96,6 +104,8 @@ impl SessionStats { pending_replaced: self.pending_replaced, ack_handshake_failed: self.ack_handshake_failed, setup_rate_limited: self.setup_rate_limited, + table_full: self.table_full, + half_open_full: self.half_open_full, } } @@ -110,6 +120,8 @@ impl SessionStats { SessionReject::RekeyPending => self.rekey_pending += 1, SessionReject::AckHandshakeFailed => self.ack_handshake_failed += 1, SessionReject::SetupRateLimited => self.setup_rate_limited += 1, + SessionReject::TableFull => self.table_full += 1, + SessionReject::HalfOpenFull => self.half_open_full += 1, } } } @@ -392,6 +404,8 @@ pub struct SessionStatsSnapshot { pub pending_replaced: u64, pub ack_handshake_failed: u64, pub setup_rate_limited: u64, + pub table_full: u64, + pub half_open_full: u64, } #[derive(Clone, Debug, Default, Serialize)] diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 2fe5eedd..78676c67 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -4206,6 +4206,258 @@ async fn test_a_drained_stranger_bucket_still_admits_a_setup_naming_an_establish cleanup_nodes(&mut nodes).await; } +// ============================================================================ +// Integration tests: the session-table population cap +// ============================================================================ + +/// Build a two-node routable mesh with the session table capped for a test +/// and the setup limiter opened wide, so the cap is the only thing refusing. +async fn make_session_capped_pair(max_sessions: usize) -> Vec { + let configs = (0..2) + .map(|_| { + let mut config = Config::new(); + config.node.rekey.enabled = false; + config.node.limits.max_sessions = max_sessions; + config.node.rate_limit.session_setup_burst = 10_000; + config.node.rate_limit.session_setup_rate = 10_000.0; + config + }) + .collect(); + let mut nodes = run_tree_test_with_configs(configs, &[(0, 1)]).await; + verify_tree_convergence(&nodes); + populate_all_coord_caches(&mut nodes); + nodes +} + +#[tokio::test] +async fn test_forged_setups_stop_growing_the_session_table_once_the_cap_is_reached() { + // The table was the one remotely-grown map with no bound: each setup from + // an address nobody has seen inserted an entry, and neither existing limit + // reached it, the setup limiter governing arrival rate rather than + // population and the idle purge only reaching entries a peer stops using. + const MAX: usize = 8; + let share = MAX / 2; + let mut nodes = make_session_capped_pair(MAX).await; + + for _ in 0..share { + deliver_forged_setup_over_link(&mut nodes).await; + } + assert_eq!( + nodes[1].node.sessions.len(), + share, + "the admissible entries must be admitted, or this test would pass for \ + the wrong reason" + ); + assert_eq!(nodes[1].node.stats().session.half_open_full, 0); + + // Every SessionAck goes out through `send_session_datagram`, the only + // thing bumping this counter on a node with no transit traffic. A refused + // setup must not move it. + let originated = nodes[1].node.metrics().forwarding.originated_packets.get(); + + for _ in 0..4 { + deliver_forged_setup_over_link(&mut nodes).await; + } + + assert_eq!( + nodes[1].node.sessions.len(), + share, + "a table at its bound must stop growing" + ); + assert_eq!( + nodes[1].node.stats().session.half_open_full, + 4, + "each refusal must be counted; the DEBUG line is invisible by default" + ); + assert_eq!( + nodes[1].node.metrics().forwarding.originated_packets.get(), + originated, + "a refused setup must emit nothing at all" + ); + + cleanup_nodes(&mut nodes).await; +} + +#[tokio::test] +async fn test_a_setup_that_would_grow_a_full_table_is_refused_and_counted() { + // The table-full arm specifically: one established entry against a cap of + // one, so the half-open share is not what refuses. + let mut nodes = make_session_capped_pair(1).await; + establish_pair_session(&mut nodes).await; + assert_eq!( + nodes[1].node.sessions.len(), + 1, + "precondition: the table is full with the established peer" + ); + + let originated = nodes[1].node.metrics().forwarding.originated_packets.get(); + deliver_forged_setup_over_link(&mut nodes).await; + + assert_eq!( + nodes[1].node.sessions.len(), + 1, + "a full table must not grow for a stranger" + ); + assert_eq!(nodes[1].node.stats().session.table_full, 1); + assert_eq!( + nodes[1].node.metrics().forwarding.originated_packets.get(), + originated, + "a refused setup must cost no ack" + ); + + cleanup_nodes(&mut nodes).await; +} + +#[tokio::test] +async fn test_a_full_session_table_still_serves_a_setup_naming_an_existing_entry() { + // The guard against writing the cap as "refuse strangers". A setup for an + // entry already present cannot grow the table, and refusing it would break + // the duplicate-ack resend an initiator depends on. + let mut nodes = make_session_capped_pair(1).await; + establish_pair_session(&mut nodes).await; + + let node0_addr = *nodes[0].node.node_addr(); + let node1_addr = *nodes[1].node.node_addr(); + let refused_before = nodes[1].node.stats().session.table_full; + let originated = nodes[1].node.metrics().forwarding.originated_packets.get(); + + // A setup naming the established peer: the shape an inbound rekey has. + let setup = forge_setup_from_stranger(&nodes); + let datagram = SessionDatagram::new(node0_addr, node1_addr, setup).with_ttl(64); + let encoded = datagram.encode(); + nodes[1] + .node + .handle_session_datagram(&node0_addr, &encoded[1..], false) + .await; + + assert_eq!( + nodes[1].node.stats().session.table_full, + refused_before, + "a setup that cannot grow the table must not be refused by the cap" + ); + assert!( + nodes[1].node.metrics().forwarding.originated_packets.get() > originated, + "and it must still be answered" + ); + + cleanup_nodes(&mut nodes).await; +} + +#[tokio::test] +async fn test_a_full_session_table_does_not_evict_an_established_session() { + // Pins refuse-not-evict. The setup that triggers the decision is + // unauthenticated at that point, so evicting would hand a stranger a way + // to tear down a session it has nothing to do with. + let mut nodes = make_session_capped_pair(1).await; + establish_pair_session(&mut nodes).await; + let node0_addr = *nodes[0].node.node_addr(); + + for _ in 0..4 { + deliver_forged_setup_over_link(&mut nodes).await; + } + + assert!( + nodes[1] + .node + .get_session(&node0_addr) + .expect("the established session must survive a flood at the cap") + .is_established(), + "a stranger's setup must never cost an established peer its session" + ); + + cleanup_nodes(&mut nodes).await; +} + +#[tokio::test] +async fn test_the_session_table_admits_again_after_the_handshake_reaper_drains_it() { + // The cap is a ceiling, not a latch: half-open entries are reaped after + // `handshake_timeout_secs` and the room they free must be usable. + const MAX: usize = 8; + let share = MAX / 2; + let mut nodes = make_session_capped_pair(MAX).await; + + for _ in 0..(share + 2) { + deliver_forged_setup_over_link(&mut nodes).await; + } + assert!( + nodes[1].node.stats().session.half_open_full > 0, + "precondition: the table is refusing before the reaper runs" + ); + + let timeout_ms = nodes[1] + .node + .config() + .node + .rate_limit + .handshake_timeout_secs + * 1000; + let now_ms = Node::now_ms(); + nodes[1] + .node + .resend_pending_session_handshakes(now_ms + timeout_ms + 1) + .await; + assert_eq!( + nodes[1].node.sessions.len(), + 0, + "precondition: the reaper freed the half-open entries" + ); + + deliver_forged_setup_over_link(&mut nodes).await; + assert_eq!( + nodes[1].node.sessions.len(), + 1, + "room freed by the reaper must be usable, or the cap is a latch" + ); + + cleanup_nodes(&mut nodes).await; +} + +#[tokio::test] +async fn test_half_open_setups_cannot_consume_more_than_their_share_of_the_table() { + // Half-open entries are unauthenticated and cheap to create, so they are + // held to a share of the table rather than being allowed to fill it and + // deny it to every peer that would complete a handshake. + const MAX: usize = 16; + let share = MAX / 2; + let mut nodes = make_session_capped_pair(MAX).await; + + for _ in 0..(share + 2) { + deliver_forged_setup_over_link(&mut nodes).await; + } + + assert_eq!( + nodes[1].node.sessions.len(), + share, + "half-open entries must stop at their share, well below the table cap" + ); + assert_eq!(nodes[1].node.stats().session.half_open_full, 2); + assert_eq!( + nodes[1].node.stats().session.table_full, + 0, + "the table itself is not full, so the refusals must be attributed to \ + the share rather than to the cap" + ); + + cleanup_nodes(&mut nodes).await; +} + +#[test] +fn test_session_entry_size_stays_within_the_budget_the_cap_is_derived_from() { + // The default `max_sessions` is derived from what one entry costs. + // Measured at 6608 bytes of inline state when the cap was written, plus + // heap for the MMP window and handshake payloads, so 1024 sessions is + // roughly 7 MB. This is what fires if a large field is added later and + // the arithmetic behind that default stops holding. + const BUDGET: usize = 8192; + assert!( + std::mem::size_of::() <= BUDGET, + "SessionEntry is {} bytes, over the {} the max_sessions default \ + assumes; re-derive the default or shrink the entry", + std::mem::size_of::(), + BUDGET + ); +} + // ============================================================================ // Integration tests: a forged SessionAck against an in-flight initiation // ============================================================================