fix(security): Tier C F-01/F-02/F-03 insider resource-exhaustion caps
A room ticket holder is an authenticated insider; signatures only prove keypair ownership, not a distinct human. Previously such a member could exhaust a victim's memory/tasks/dials without bound. Add caps + dedup at the gossip/core/UI boundaries (no wire/protocol change, no new deps): F-01 (gossip): cap the roster at MAX_ACTIVE_PEERS (32) — new authors are rejected when full, existing peers' updates always pass; sanitize each announced EndpointAddr (<=8 addrs, relay-URL <=256 bytes, drop Custom); replace (set_endpoint_info) instead of unioning attacker address history. F-02 (core): gate chat image auto-fetch — only roster authors qualify, (author, attachment_id) is deduped, and a 4-permit pool bounds concurrent detached fetch tasks (RAII AutoFetchGuard releases permit + dedup marker). Chat text is still shown (already sanitized); the user-initiated "Save" fetch is unchanged. Non-roster sock-puppet chat can no longer spawn tasks. F-03 (app): replace the unbounded AVATAR_HANDLE_CACHE map with a bounded, byte-equality-keyed LRU (avatar::ByteLru, cap 64) — fixes both unbounded growth from an endless stream of distinct valid avatars and the prior 64-bit-hash-collision-shows-wrong-avatar bug. Pure seams (sanitize_endpoint_addr, admit_into_roster, should_auto_fetch, ByteLru) + 6 adversarial/unit tests. 413 lib tests, clippy --all-targets clean, release build green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+130
-8
@@ -1,11 +1,11 @@
|
||||
use crate::network::{RoomState, NetError, PeerState, RoomEvent, PeerSpeakTicket};
|
||||
use iroh::{Endpoint, EndpointAddr, EndpointId, SecretKey, Signature};
|
||||
use iroh::{Endpoint, EndpointAddr, EndpointId, SecretKey, Signature, TransportAddr};
|
||||
use iroh_gossip::net::Gossip;
|
||||
use iroh_gossip::proto::TopicId;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::sync::mpsc::Receiver;
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::collections::{BTreeSet, HashMap, HashSet};
|
||||
use async_trait::async_trait;
|
||||
use tokio_stream::StreamExt;
|
||||
use serde::{Serialize, Deserialize};
|
||||
@@ -131,6 +131,57 @@ fn admit_state_mutation(
|
||||
true
|
||||
}
|
||||
|
||||
/// 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
|
||||
/// proves ownership of the generated keypair it was made with, not that the
|
||||
/// author is a distinct human. A malicious member can therefore mint many valid
|
||||
/// signed identities. Voice is full-mesh (each peer dials every other), so a real
|
||||
/// room is realistically well under this bound; the cap exists purely so a flood
|
||||
/// of sock-puppet `Announce`s can't grow our peer map / audio supervisors / dials
|
||||
/// without limit (Tier C F-01).
|
||||
const MAX_ACTIVE_PEERS: usize = 32;
|
||||
|
||||
/// Maximum transport addresses we retain from a single peer announce. iroh
|
||||
/// normally advertises a handful (a few LAN/WAN IP candidates plus one home
|
||||
/// relay); the cap stops an insider stuffing a large unique address set into each
|
||||
/// announce to inflate the address lookup and the dialer's candidate list.
|
||||
const MAX_PEER_ADDRS: usize = 8;
|
||||
|
||||
/// Maximum byte length of a relay URL we accept inside a peer address. A relay
|
||||
/// URL is normal-length; anything longer is dropped rather than retained.
|
||||
const MAX_RELAY_URL_LEN: usize = 256;
|
||||
|
||||
/// Bound an untrusted peer's advertised address set before we retain it / hand it
|
||||
/// to the address lookup and dialer (Tier C F-01). Drops transport kinds we never
|
||||
/// use (`Custom`) and over-long relay URLs, then truncates to at most
|
||||
/// [`MAX_PEER_ADDRS`] addresses. `BTreeSet` iteration is deterministic, so the
|
||||
/// kept subset is stable. Pure → unit-testable.
|
||||
fn sanitize_endpoint_addr(addr: &EndpointAddr) -> EndpointAddr {
|
||||
let addrs: BTreeSet<TransportAddr> = addr
|
||||
.addrs
|
||||
.iter()
|
||||
.filter(|a| match a {
|
||||
TransportAddr::Relay(url) => url.as_str().len() <= MAX_RELAY_URL_LEN,
|
||||
TransportAddr::Ip(_) => true,
|
||||
// `TransportAddr` is #[non_exhaustive]; we only speak IP + relay, so
|
||||
// anything else (Custom / future kinds) is dropped, not retained.
|
||||
_ => false,
|
||||
})
|
||||
.take(MAX_PEER_ADDRS)
|
||||
.cloned()
|
||||
.collect();
|
||||
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).
|
||||
/// Pure → unit-testable.
|
||||
fn admit_into_roster(roster_len: usize, is_new: bool, max_peers: usize) -> bool {
|
||||
!is_new || roster_len < max_peers
|
||||
}
|
||||
|
||||
fn peer_state_for_log(state: &PeerState) -> String {
|
||||
format!(
|
||||
"name={:?}, muted={}, addr_id={}, addrs={}, sharing={}, game={:?}",
|
||||
@@ -436,15 +487,34 @@ impl RoomState for IrohGossipState {
|
||||
let cleaned = crate::sanitize::sanitize_game_label(&g);
|
||||
(!cleaned.is_empty()).then_some(cleaned)
|
||||
});
|
||||
// Bound an insider's advertised address set
|
||||
// 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);
|
||||
let (is_new, state_changed) = {
|
||||
let admitted = {
|
||||
let mut peer_map = peers.lock().unwrap();
|
||||
let is_new = !peer_map.contains_key(&payload.author);
|
||||
let state_changed = peer_map.get(&payload.author) != Some(&state);
|
||||
if is_new || state_changed {
|
||||
peer_map.insert(payload.author, state.clone());
|
||||
// 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) {
|
||||
None
|
||||
} else {
|
||||
let state_changed = peer_map.get(&payload.author) != Some(&state);
|
||||
if is_new || state_changed {
|
||||
peer_map.insert(payload.author, state.clone());
|
||||
}
|
||||
Some((is_new, state_changed))
|
||||
}
|
||||
(is_new, state_changed)
|
||||
};
|
||||
let Some((is_new, state_changed)) = admitted else {
|
||||
crate::log_msg(&format!(
|
||||
"Gossip roster full ({MAX_ACTIVE_PEERS}); rejecting new peer {}",
|
||||
crate::short_id(&payload.author.to_string())
|
||||
));
|
||||
continue;
|
||||
};
|
||||
|
||||
if is_new {
|
||||
@@ -453,7 +523,12 @@ impl RoomState for IrohGossipState {
|
||||
crate::short_id(&payload.author.to_string()),
|
||||
peer_state_for_log(&state)
|
||||
));
|
||||
address_lookup.add_endpoint_info(state.addr.clone());
|
||||
// Replace (not union) the lookup's record for
|
||||
// this id with the authenticated, sanitized
|
||||
// address set, so leave/re-announce cycles
|
||||
// can't accumulate attacker-supplied history
|
||||
// (Tier C F-01).
|
||||
let _ = address_lookup.set_endpoint_info(state.addr.clone());
|
||||
let _ = event_tx.send(RoomEvent::PeerJoined(payload.author, state)).await;
|
||||
} else if state_changed {
|
||||
crate::log_msg(&format!(
|
||||
@@ -768,6 +843,53 @@ mod tests {
|
||||
assert!(!bootstrap.contains(&me));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn admit_into_roster_caps_new_authors_but_not_updates() {
|
||||
// New authors are admitted while there's room...
|
||||
assert!(admit_into_roster(0, true, 3));
|
||||
assert!(admit_into_roster(2, true, 3));
|
||||
// ...rejected once the roster is full...
|
||||
assert!(!admit_into_roster(3, true, 3));
|
||||
assert!(!admit_into_roster(10, true, 3));
|
||||
// ...but an existing peer's update always passes, even at/over the cap.
|
||||
assert!(admit_into_roster(3, false, 3));
|
||||
assert!(admit_into_roster(99, false, 3));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_endpoint_addr_caps_address_count() {
|
||||
use std::net::SocketAddr;
|
||||
let id = fresh_id();
|
||||
// An insider stuffs far more addresses than MAX_PEER_ADDRS into one announce.
|
||||
let many: Vec<TransportAddr> = (0..(MAX_PEER_ADDRS as u16 + 50))
|
||||
.map(|i| TransportAddr::Ip(SocketAddr::from(([127, 0, 0, 1], 1000 + i))))
|
||||
.collect();
|
||||
let addr = EndpointAddr::from_parts(id, many);
|
||||
let out = sanitize_endpoint_addr(&addr);
|
||||
assert_eq!(out.id, id);
|
||||
assert_eq!(out.addrs.len(), MAX_PEER_ADDRS);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_endpoint_addr_drops_overlong_relay_url() {
|
||||
use std::str::FromStr;
|
||||
let id = fresh_id();
|
||||
let short = iroh::RelayUrl::from_str("https://relay.example/").unwrap();
|
||||
let long = iroh::RelayUrl::from_str(&format!(
|
||||
"https://relay.example/{}",
|
||||
"a".repeat(MAX_RELAY_URL_LEN)
|
||||
))
|
||||
.unwrap();
|
||||
assert!(long.as_str().len() > MAX_RELAY_URL_LEN);
|
||||
let addr = EndpointAddr::from_parts(
|
||||
id,
|
||||
[TransportAddr::Relay(short.clone()), TransportAddr::Relay(long)],
|
||||
);
|
||||
let out = sanitize_endpoint_addr(&addr);
|
||||
let relays: Vec<_> = out.relay_urls().cloned().collect();
|
||||
assert_eq!(relays, vec![short], "over-long relay URL must be dropped");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_gossip_message_leave_round_trip() {
|
||||
let original = GossipMessage::Leave;
|
||||
|
||||
Reference in New Issue
Block a user