From f8666adf3814c8016d5f484ca9614f421d63586e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Apr 2026 01:32:58 +0000 Subject: [PATCH] fix(state): cap RotateChannelKey + Event.deps (SEC-V-07) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Anti-DoS caps for two attacker-controlled vectors that fan out via .clone() into per-peer state. Ref #236. Caps + tier: - MAX_EVENT_DEPS = 64. EventDag::insert rejects oversize deps with InsertError::DepsTooLong. Author-side path covered too because every event reaches `insert` before reaching state. - MAX_ENCRYPTED_KEY_BYTES = 128. EventDag::insert rejects oversize per-blob payloads inside RotateChannelKey.encrypted_keys with InsertError::EncryptedKeyTooLarge. One X25519-sealed key fits well under that. - MAX_ENCRYPTED_KEYS_OVER_MEMBERS = 4. apply_event for RotateChannelKey rejects when encrypted_keys.len() exceeds state.members.len() + epsilon. Needs state context, so it lives at apply time alongside the existing member gate. Tier choice: caps that need no state context go at the DAG insert boundary so they fire regardless of which call path produced the event. The member-count cap requires `state.members` and naturally slots into the existing apply_event Rejected branch. Runner-up: stuffing all three into Event::new. Rejected because Event::new is currently infallible and changing it cascades to ~25 production + test callsites for no extra coverage — every event already passes through dag.insert before mutating state. willow-replay matches on InsertError exhaustively; added warn arms for the two new variants. Tests (state crate, 5 added): - dag_insert_rejects_deps_over_cap - dag_insert_accepts_deps_at_cap - dag_insert_rejects_oversized_encrypted_key - apply_rotate_channel_key_rejects_excess_entries_over_member_count - apply_rotate_channel_key_accepts_at_member_count_plus_epsilon Local: cargo fmt --check, cargo clippy --workspace --all-targets -D warnings, cargo test --workspace, cargo check --target wasm32-unknown-unknown all green. https://claude.ai/code/session_01VdcwSdvqig423A5CX2fSzR --- crates/replay/src/role.rs | 9 ++ crates/state/src/dag.rs | 52 +++++++-- crates/state/src/event.rs | 25 ++++ crates/state/src/materialize.rs | 21 +++- crates/state/src/tests.rs | 196 ++++++++++++++++++++++++++++++++ 5 files changed, 295 insertions(+), 8 deletions(-) diff --git a/crates/replay/src/role.rs b/crates/replay/src/role.rs index 628f7f3d..616f6208 100644 --- a/crates/replay/src/role.rs +++ b/crates/replay/src/role.rs @@ -189,6 +189,15 @@ impl ReplayRole { Err(InsertError::PermissionDenied(reason)) => { warn!(%reason, "rejected event: permission denied"); } + Err(InsertError::DepsTooLong { got, max }) => { + warn!(got, max, "rejected event: deps over cap (SEC-V-07)"); + } + Err(InsertError::EncryptedKeyTooLarge { got, max }) => { + warn!( + got, max, + "rejected event: RotateChannelKey blob over cap (SEC-V-07)", + ); + } } } } diff --git a/crates/state/src/dag.rs b/crates/state/src/dag.rs index b88374e1..8fceefae 100644 --- a/crates/state/src/dag.rs +++ b/crates/state/src/dag.rs @@ -8,7 +8,7 @@ use std::collections::HashMap; use willow_identity::{EndpointId, Identity}; -use crate::event::{Event, EventKind}; +use crate::event::{Event, EventKind, MAX_ENCRYPTED_KEY_BYTES, MAX_EVENT_DEPS}; use crate::hash::EventHash; /// Error returned when inserting an event into the DAG fails. @@ -39,6 +39,13 @@ pub enum InsertError { vote: EventHash, proposal: EventHash, }, + /// `event.deps.len()` exceeds [`crate::event::MAX_EVENT_DEPS`]. + /// Anti-DoS cap; see SEC-V-07 (#236). + DepsTooLong { got: usize, max: usize }, + /// One entry inside `RotateChannelKey.encrypted_keys` carries a + /// blob larger than [`crate::event::MAX_ENCRYPTED_KEY_BYTES`]. + /// Anti-DoS cap; see SEC-V-07 (#236). + EncryptedKeyTooLarge { got: usize, max: usize }, /// Author lacks the required permission for this EventKind. PermissionDenied(String), } @@ -74,6 +81,13 @@ impl std::fmt::Display for InsertError { f, "Vote event {vote} must include proposal {proposal} in deps" ), + Self::DepsTooLong { got, max } => { + write!(f, "event.deps too long: {got} entries (max {max})") + } + Self::EncryptedKeyTooLarge { got, max } => write!( + f, + "RotateChannelKey encrypted blob too large: {got} bytes (max {max})" + ), Self::PermissionDenied(reason) => write!(f, "permission denied: {reason}"), } } @@ -118,12 +132,36 @@ impl EventDag { return Err(InsertError::InvalidSignature); } - // 2. Check duplicate. + // 2. Anti-DoS vector caps (SEC-V-07). + // + // A peer holding any permission could otherwise broadcast events + // with pathologically large `deps` or `encrypted_keys` blobs that + // every other peer would clone into a `BTreeMap` during apply. + // Reject at the inbound DAG boundary so over-cap events never + // even reach `applied_events` / `materialize`. + if event.deps.len() > MAX_EVENT_DEPS { + return Err(InsertError::DepsTooLong { + got: event.deps.len(), + max: MAX_EVENT_DEPS, + }); + } + if let EventKind::RotateChannelKey { encrypted_keys, .. } = &event.kind { + for (_, blob) in encrypted_keys { + if blob.len() > MAX_ENCRYPTED_KEY_BYTES { + return Err(InsertError::EncryptedKeyTooLarge { + got: blob.len(), + max: MAX_ENCRYPTED_KEY_BYTES, + }); + } + } + } + + // 3. Check duplicate. if self.events.contains_key(&event.hash) { return Err(InsertError::Duplicate); } - // 3. Genesis check: first event must be CreateServer. + // 4. Genesis check: first event must be CreateServer. // After genesis is set, reject any further CreateServer events. if self.genesis_hash.is_none() { match &event.kind { @@ -143,7 +181,7 @@ impl EventDag { return Err(InsertError::DuplicateGenesis); } - // 4. Check seq: must be latest_seq + 1. + // 5. Check seq: must be latest_seq + 1. // This also prevents equivocation: an author cannot insert two // events at the same seq number because only seq = latest + 1 // is accepted. Combined with the prev-hash check below, this @@ -157,7 +195,7 @@ impl EventDag { }); } - // 5. Check prev: must match current head (or ZERO for seq=1). + // 6. Check prev: must match current head (or ZERO for seq=1). let expected_prev = self .heads .get(&event.author) @@ -171,7 +209,7 @@ impl EventDag { }); } - // 6. Governance structural checks: Vote events must causally + // 7. Governance structural checks: Vote events must causally // depend on their proposal (via deps or prev) so topological // sort always places the proposal before the vote. if let EventKind::Vote { proposal, .. } = &event.kind { @@ -183,7 +221,7 @@ impl EventDag { } } - // 7. Insert. + // 8. Insert. let hash = event.hash; let author = event.author; self.events.insert(hash, event); diff --git a/crates/state/src/event.rs b/crates/state/src/event.rs index 885180c8..653dc9f1 100644 --- a/crates/state/src/event.rs +++ b/crates/state/src/event.rs @@ -9,6 +9,31 @@ use willow_identity::{EndpointId, Identity, Signature}; use crate::hash::EventHash; +// ───── Vector caps (anti-DoS) ────────────────────────────────────────────── +// +// These caps bound per-event memory growth so a single misbehaving peer +// holding a permission can't blow up every other peer's heap by emitting +// pathologically large vectors. See SEC-V-07 (#236). + +/// Maximum number of cross-author causal hashes an event may carry in +/// `deps`. Legitimate events reference at most a handful of recent +/// other-author heads; 64 is comfortably above that ceiling and keeps +/// the per-event payload small. +pub const MAX_EVENT_DEPS: usize = 64; + +/// Maximum byte length of a single encrypted-channel-key blob inside +/// `EventKind::RotateChannelKey.encrypted_keys`. One X25519-sealed +/// channel key fits well under 128 bytes (32-byte ciphertext + tag + +/// ephemeral pubkey = ~80 bytes); 128 leaves slack without giving a +/// hostile author room to bloat each entry. +pub const MAX_ENCRYPTED_KEY_BYTES: usize = 128; + +/// Slack added to the current member count when capping +/// `RotateChannelKey.encrypted_keys.len()`. The legitimate ceiling is +/// "one entry per current member"; epsilon absorbs benign races between +/// membership changes and key-rotation construction. +pub const MAX_ENCRYPTED_KEYS_OVER_MEMBERS: usize = 4; + // ───── Permission ────────────────────────────────────────────────────────── /// Permission types that can be granted directly by any admin. diff --git a/crates/state/src/materialize.rs b/crates/state/src/materialize.rs index 416e6987..4f057a69 100644 --- a/crates/state/src/materialize.rs +++ b/crates/state/src/materialize.rs @@ -10,7 +10,7 @@ use std::collections::{BTreeMap, BTreeSet}; use willow_identity::EndpointId; use crate::dag::EventDag; -use crate::event::{Event, EventKind, Permission, ProposedAction}; +use crate::event::{Event, EventKind, Permission, ProposedAction, MAX_ENCRYPTED_KEYS_OVER_MEMBERS}; use crate::hash::EventHash; use crate::server::{PendingProposal, ServerState}; use crate::types::{ @@ -631,6 +631,25 @@ fn apply_mutation(state: &mut ServerState, event: &Event) -> ApplyResult { if !state.members.contains_key(&event.author) { return ApplyResult::Rejected(format!("author '{}' is not a member", event.author)); } + // Anti-DoS cap (SEC-V-07). A legitimate RotateChannelKey + // carries at most one entry per current member; epsilon + // absorbs benign races between membership changes and key + // rotation. Anything beyond that is a fabricated-id flood — + // every entry would otherwise `.clone()` into the per-server + // `BTreeMap>` on every peer. + let cap = state + .members + .len() + .saturating_add(MAX_ENCRYPTED_KEYS_OVER_MEMBERS); + if encrypted_keys.len() > cap { + return ApplyResult::Rejected(format!( + "RotateChannelKey: {} encrypted_keys exceeds cap {} (members={} + epsilon={})", + encrypted_keys.len(), + cap, + state.members.len(), + MAX_ENCRYPTED_KEYS_OVER_MEMBERS, + )); + } if !encrypted_keys.is_empty() { let keys = state.channel_keys.entry(channel_id.clone()).or_default(); for (peer_id, key_bytes) in encrypted_keys { diff --git a/crates/state/src/tests.rs b/crates/state/src/tests.rs index 50f77dc2..371d692c 100644 --- a/crates/state/src/tests.rs +++ b/crates/state/src/tests.rs @@ -4516,3 +4516,199 @@ fn set_permission_legacy_unknown_string_drops_silently() { "unknown legacy permission must apply as a no-op" ); } + +// ── Issue #236 (SEC-V-07): vector caps ───────────────────────────────── +// +// `Event.deps` and `RotateChannelKey.encrypted_keys` are both attacker- +// controlled vectors that fan out via `.clone()` into per-peer state. +// Cap them at the inbound DAG boundary (deps + per-blob byte size) and +// at the materializer (encrypted_keys.len() vs current member count). + +#[test] +fn dag_insert_rejects_deps_over_cap() { + use crate::dag::InsertError; + use crate::event::MAX_EVENT_DEPS; + + let admin = Identity::generate(); + let mut dag = test_dag(&admin); + + // Build deps vector one entry over the cap. + let bad_deps: Vec = (0..=MAX_EVENT_DEPS) + .map(|i| EventHash::from_bytes(&i.to_le_bytes())) + .collect(); + assert_eq!(bad_deps.len(), MAX_EVENT_DEPS + 1); + + let bloated = dag.create_event( + &admin, + EventKind::SetProfile { + display_name: "x".into(), + }, + bad_deps, + 0, + ); + let err = dag.insert(bloated).unwrap_err(); + match err { + InsertError::DepsTooLong { got, max } => { + assert_eq!(got, MAX_EVENT_DEPS + 1); + assert_eq!(max, MAX_EVENT_DEPS); + } + other => panic!("expected DepsTooLong, got {other:?}"), + } +} + +#[test] +fn dag_insert_accepts_deps_at_cap() { + use crate::event::MAX_EVENT_DEPS; + + let admin = Identity::generate(); + let mut dag = test_dag(&admin); + + let ok_deps: Vec = (0..MAX_EVENT_DEPS) + .map(|i| EventHash::from_bytes(&i.to_le_bytes())) + .collect(); + assert_eq!(ok_deps.len(), MAX_EVENT_DEPS); + + let event = dag.create_event( + &admin, + EventKind::SetProfile { + display_name: "x".into(), + }, + ok_deps, + 0, + ); + dag.insert(event).expect("deps at cap must be accepted"); +} + +#[test] +fn dag_insert_rejects_oversized_encrypted_key() { + use crate::dag::InsertError; + use crate::event::MAX_ENCRYPTED_KEY_BYTES; + + let admin = Identity::generate(); + let mut dag = test_dag(&admin); + + // One entry whose blob is one byte over the cap. + let too_big = vec![0xab; MAX_ENCRYPTED_KEY_BYTES + 1]; + let bloated = dag.create_event( + &admin, + EventKind::RotateChannelKey { + channel_id: "ch-1".into(), + encrypted_keys: vec![(admin.endpoint_id(), too_big)], + }, + vec![], + 0, + ); + let err = dag.insert(bloated).unwrap_err(); + match err { + InsertError::EncryptedKeyTooLarge { got, max } => { + assert_eq!(got, MAX_ENCRYPTED_KEY_BYTES + 1); + assert_eq!(max, MAX_ENCRYPTED_KEY_BYTES); + } + other => panic!("expected EncryptedKeyTooLarge, got {other:?}"), + } +} + +#[test] +fn apply_rotate_channel_key_rejects_excess_entries_over_member_count() { + use crate::event::MAX_ENCRYPTED_KEYS_OVER_MEMBERS; + use crate::managed::ManagedDag; + use crate::materialize::ApplyResult; + + let admin = Identity::generate(); + let mut managed = ManagedDag::new(&admin, "S", 5000).unwrap(); + + // Create channel. + let create = managed.dag().create_event( + &admin, + EventKind::CreateChannel { + name: "general".into(), + channel_id: "ch-1".into(), + kind: crate::types::ChannelKind::Text, + ephemeral: None, + }, + vec![], + 0, + ); + managed.insert_and_apply(create).unwrap(); + + // Sole admin = 1 member. Cap = 1 + epsilon. Submit (cap + 1) entries. + let member_count = managed.state().members.len(); + assert_eq!(member_count, 1); + let cap = member_count + MAX_ENCRYPTED_KEYS_OVER_MEMBERS; + // Use real generated identities so each EndpointId is a valid curve + // point (`EndpointId::from_bytes` rejects non-curve inputs). + let bloat: Vec<(willow_identity::EndpointId, Vec)> = (0..(cap + 1)) + .map(|_| (Identity::generate().endpoint_id(), vec![0xaa])) + .collect(); + assert_eq!(bloat.len(), cap + 1); + + let rotate = managed.dag().create_event( + &admin, + EventKind::RotateChannelKey { + channel_id: "ch-1".into(), + encrypted_keys: bloat, + }, + vec![], + 10, + ); + let outcome = managed.insert_and_apply(rotate).unwrap(); + assert!( + matches!(outcome.apply_result, Some(ApplyResult::Rejected(_))), + "over-cap rotate must be rejected at apply: {:?}", + outcome.apply_result, + ); + // No channel_keys entry created — state untouched by rejected event. + assert!(!managed.state().channel_keys.contains_key("ch-1")); +} + +#[test] +fn apply_rotate_channel_key_accepts_at_member_count_plus_epsilon() { + use crate::event::MAX_ENCRYPTED_KEYS_OVER_MEMBERS; + use crate::managed::ManagedDag; + use crate::materialize::ApplyResult; + + let admin = Identity::generate(); + let mut managed = ManagedDag::new(&admin, "S", 5000).unwrap(); + + let create = managed.dag().create_event( + &admin, + EventKind::CreateChannel { + name: "general".into(), + channel_id: "ch-1".into(), + kind: crate::types::ChannelKind::Text, + ephemeral: None, + }, + vec![], + 0, + ); + managed.insert_and_apply(create).unwrap(); + + // Cap = members + epsilon. Submit exactly that many — must succeed. + let member_count = managed.state().members.len(); + let cap = member_count + MAX_ENCRYPTED_KEYS_OVER_MEMBERS; + // Use real generated identities so each EndpointId is a valid curve + // point (`EndpointId::from_bytes` rejects non-curve inputs). + let entries: Vec<(willow_identity::EndpointId, Vec)> = (0..cap) + .map(|_| (Identity::generate().endpoint_id(), vec![0xaa])) + .collect(); + + let rotate = managed.dag().create_event( + &admin, + EventKind::RotateChannelKey { + channel_id: "ch-1".into(), + encrypted_keys: entries, + }, + vec![], + 10, + ); + let outcome = managed.insert_and_apply(rotate).unwrap(); + assert!( + matches!(outcome.apply_result, Some(ApplyResult::Applied)), + "boundary case (members + epsilon) must apply: {:?}", + outcome.apply_result, + ); + assert_eq!( + managed.state().channel_keys.get("ch-1").map(|m| m.len()), + Some(cap), + ); +}