diff --git a/src/avatar.rs b/src/avatar.rs index 0b4df60..019de4d 100644 --- a/src/avatar.rs +++ b/src/avatar.rs @@ -217,8 +217,13 @@ impl ByteLru { use std::hash::{Hash, Hasher}; let mut hasher = std::collections::hash_map::DefaultHasher::new(); bytes.hash(&mut hasher); - let hash = hasher.finish(); + self.get_or_insert_hashed(hasher.finish(), bytes, build) + } + /// Inner seam with the content `hash` supplied explicitly. Production callers + /// use [`get_or_insert`]; tests use this to force a hash collision (different + /// bytes, same hash) and exercise the byte-equality guard. + fn get_or_insert_hashed(&mut self, hash: u64, bytes: &[u8], build: impl FnOnce() -> V) -> V { if let Some(idx) = self .entries .iter() @@ -288,6 +293,21 @@ mod tests { assert_eq!(ins(&mut lru, b"b"), 4); } + #[test] + fn byte_lru_byte_equality_survives_a_hash_collision() { + // Force the SAME 64-bit hash for two DIFFERENT byte strings (the case a + // bare-hash cache would alias — Tier C F-03 collision bug). + let mut lru: ByteLru = ByteLru::new(4); + assert_eq!(lru.get_or_insert_hashed(42, b"alice", || 1), 1); + // `bob` collides on the hash but differs in bytes → a MISS, built fresh, + // NOT aliased to alice's value. + assert_eq!(lru.get_or_insert_hashed(42, b"bob", || 2), 2); + // Both coexist; each re-lookup returns its own value (build closure unused). + assert_eq!(lru.get_or_insert_hashed(42, b"alice", || 99), 1); + assert_eq!(lru.get_or_insert_hashed(42, b"bob", || 99), 2); + assert_eq!(lru.len(), 2); + } + #[test] fn initials_takes_first_two_words() { assert_eq!(initials("Alice"), "A"); diff --git a/src/network/gossip.rs b/src/network/gossip.rs index 8f6c3ec..fb9db63 100644 --- a/src/network/gossip.rs +++ b/src/network/gossip.rs @@ -131,6 +131,27 @@ fn admit_state_mutation( true } +/// Size at which we prune stale entries from the replay-tracking map (Tier C +/// F-01 audit). `admit_state_mutation` records `(author, kind)` for every signed +/// mutation, so an insider sending validly signed `Leave`s from unlimited +/// generated keys would otherwise grow it for the room's lifetime. A mutation +/// older than the freshness window can never be the deciding `last_ts` for an +/// in-window message — `verify_gossip`'s timestamp check rejects such a replay +/// first — so dropping those entries cannot weaken replay protection; it bounds +/// the map to roughly the authors seen within one freshness window. +const STATE_MUTATIONS_SOFT_CAP: usize = 256; + +/// Drop replay-tracking entries whose timestamp is older than `window_ms` before +/// `now_ms` (see [`STATE_MUTATIONS_SOFT_CAP`]). Pure → unit-testable. +fn prune_stale_mutations( + seen: &mut HashMap<(EndpointId, StateMutationKind), u64>, + now_ms: u64, + window_ms: u64, +) { + let floor = now_ms.saturating_sub(window_ms); + seen.retain(|_, last_ts| *last_ts >= floor); +} + /// Maximum number of distinct peers we hold in a room roster at once. /// /// Everyone with the room ticket is an authenticated *insider*: a signature only @@ -174,12 +195,23 @@ fn sanitize_endpoint_addr(addr: &EndpointAddr) -> EndpointAddr { EndpointAddr { id: addr.id, addrs } } -/// Whether an `Announce` may enter the roster. A NEW author is admitted only -/// while the roster is below [`MAX_ACTIVE_PEERS`]; updates to an already-present -/// peer always pass (so a full room's members can keep changing mute/avatar/etc). +/// Whether an `Announce` may enter the roster. Only a brand-new author +/// (`subject_to_cap`) is gated by [`MAX_ACTIVE_PEERS`]; updates to an +/// already-present peer AND re-announces from a peer mid-reconnect (which +/// already held a slot) always pass — exempting reconnects keeps a full room +/// from rejecting a legitimately reconnecting member and orphaning its recovery +/// state (Tier C F-01 audit). Pure → unit-testable. +fn admit_into_roster(roster_len: usize, subject_to_cap: bool, max_peers: usize) -> bool { + !subject_to_cap || roster_len < max_peers +} + +/// Whether a received `Announce`'s author is gated by the roster cap. A peer +/// already in the roster (`is_new == false`, an ordinary update) or one +/// mid-reconnect (`is_reconnecting`, it already held a slot) is exempt; only a +/// brand-new author counts against [`MAX_ACTIVE_PEERS`] (Tier C F-01 audit). /// Pure → unit-testable. -fn admit_into_roster(roster_len: usize, is_new: bool, max_peers: usize) -> bool { - !is_new || roster_len < max_peers +fn announce_subject_to_cap(is_new: bool, is_reconnecting: bool) -> bool { + is_new && !is_reconnecting } fn peer_state_for_log(state: &PeerState) -> String { @@ -441,6 +473,18 @@ impl RoomState for IrohGossipState { continue; } + // Keep the replay-tracking map bounded: prune entries + // older than the freshness window once it grows past the + // soft cap (Tier C F-01 audit). Stale entries can't gate + // an in-window message, so this never weakens replay + // protection. + if state_mutations_seen.len() > STATE_MUTATIONS_SOFT_CAP { + prune_stale_mutations( + &mut state_mutations_seen, + now_millis(), + GOSSIP_FRESHNESS_MS, + ); + } if !admit_state_mutation( &mut state_mutations_seen, payload.author, @@ -491,15 +535,25 @@ impl RoomState for IrohGossipState { // before we retain it / hand it to the dialer // (Tier C F-01). state.addr = sanitize_endpoint_addr(&state.addr); - disconnected_peers.lock().unwrap().remove(&payload.author); + // A peer reconnecting from a transient drop sits + // in `disconnected_peers` (not the live roster); + // it already held a slot, so it must be re-admitted + // regardless of the cap, and its disconnect marker + // cleared ONLY once re-admitted — clearing it before + // a possible reject would orphan its recovery state + // (Tier C F-01 audit). + let is_reconnecting = + disconnected_peers.lock().unwrap().contains(&payload.author); let admitted = { let mut peer_map = peers.lock().unwrap(); let is_new = !peer_map.contains_key(&payload.author); // Cap the roster so a flood of signed // sock-puppet identities can't grow our // memory/tasks/dials without bound (Tier C - // F-01). Existing peers' updates always pass. - if !admit_into_roster(peer_map.len(), is_new, MAX_ACTIVE_PEERS) { + // F-01). Existing-peer updates and reconnects + // are exempt; only brand-new authors are gated. + let subject_to_cap = announce_subject_to_cap(is_new, is_reconnecting); + if !admit_into_roster(peer_map.len(), subject_to_cap, MAX_ACTIVE_PEERS) { None } else { let state_changed = peer_map.get(&payload.author) != Some(&state); @@ -516,6 +570,10 @@ impl RoomState for IrohGossipState { )); continue; }; + // Admitted — now it is safe to clear any reconnect + // marker (a rejected announce above leaves it intact + // so a later signed Leave still cleans up). + disconnected_peers.lock().unwrap().remove(&payload.author); if is_new { crate::log_msg(&format!( @@ -541,6 +599,11 @@ impl RoomState for IrohGossipState { } GossipMessage::Leave => { crate::log_msg(&format!("Gossip peer leave request from author={:?}", payload.author)); + // Drop this id's address-lookup entry so cycling + // distinct identities through Announce→Leave can't + // grow the lookup for the room's lifetime (Tier C + // F-01 audit). Re-announce re-populates it. + let _ = address_lookup.remove_endpoint_info(payload.author); let removed = peers.lock().unwrap().remove(&payload.author).is_some(); let was_disconnected = disconnected_peers .lock() @@ -856,6 +919,37 @@ mod tests { assert!(admit_into_roster(99, false, 3)); } + #[test] + fn reconnecting_and_existing_peers_are_exempt_from_the_cap() { + // A brand-new author counts against the cap... + assert!(announce_subject_to_cap(/* is_new */ true, /* is_reconnecting */ false)); + // ...but an ordinary update from an in-roster peer does not... + assert!(!announce_subject_to_cap(false, false)); + // ...and neither does a re-announce from a peer mid-reconnect, even + // though it was removed from the live roster (the F-01-audit fix: a full + // room must not reject a legitimately reconnecting member). + assert!(!announce_subject_to_cap(true, true)); + // Combined with admit_into_roster: a reconnecting author passes at a full + // roster, a brand-new one does not. + assert!(admit_into_roster(3, announce_subject_to_cap(true, true), 3)); + assert!(!admit_into_roster(3, announce_subject_to_cap(true, false), 3)); + } + + #[test] + fn prune_stale_mutations_drops_only_out_of_window_entries() { + let a = fresh_id(); + let b = fresh_id(); + let mut seen = HashMap::new(); + seen.insert((a, StateMutationKind::Announce), 10_000u64); + seen.insert((b, StateMutationKind::Leave), 250_000u64); + // now = 300_000, window = 120_000 → floor 180_000. The 10_000 entry is + // stale (and could never gate an in-window message), the 250_000 is live. + prune_stale_mutations(&mut seen, 300_000, GOSSIP_FRESHNESS_MS); + assert_eq!(seen.len(), 1); + assert!(seen.contains_key(&(b, StateMutationKind::Leave))); + assert!(!seen.contains_key(&(a, StateMutationKind::Announce))); + } + #[test] fn sanitize_endpoint_addr_caps_address_count() { use std::net::SocketAddr;