Security hardening: log redaction + 5 trust-boundary fixes (S10, T1/T2/T5/T6/T7)
Codex (gpt-5.5) implementer branch, senior-reviewed. - S10 (High): redact capabilities/chat from logs; create log 0600 + chmod existing; rotate at 5 MiB. New short_id/short_bytes_hex/redact_for_log seams. - T1 (P2): friends-ALPN authorizes (handler(from)) before reading any peer bytes; unauthorized conns closed pre-read (DoS relief). - T2 (P2): per-(author,kind) replay gate on state-changing gossip (Announce/ Leave) only; Chat bypasses it, preserving the S2 no-monotonic-ts decision. - T5 (P2): cap inbound Opus datagrams at 4 + MAX_OPUS_PAYLOAD (4000). - T6 (P2): bind friend-Pong room ticket host to the authenticated responder (interpret_pong/probe now thread the remote id) — blocks Join-button redirect/phishing. Non-regressive given the W7 P3 restamp design. - T7 (P3): sanitize_ticket caps/validates PeerState.sharing at gossip ingest so invalid offers never render a Watch button. 302 lib tests pass (was 291), clippy --all-targets clean, release builds. Tests-green only; DoS relief + 2-machine replay/redirect behavior want a field test. W7 P7 (n0 DNS privacy) reviewed read-only — findings to triage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+145
-7
@@ -43,7 +43,7 @@ impl std::fmt::Debug for GossipPayload {
|
||||
f.debug_struct("GossipPayload")
|
||||
.field("author", &self.author)
|
||||
.field("ts", &self.ts)
|
||||
.field("msg", &self.msg)
|
||||
.field("msg_kind", &gossip_message_kind(&self.msg))
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
@@ -83,6 +83,56 @@ enum GossipReject {
|
||||
AnnounceAddressMismatch,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||
enum StateMutationKind {
|
||||
Announce,
|
||||
Leave,
|
||||
}
|
||||
|
||||
fn gossip_message_kind(msg: &GossipMessage) -> &'static str {
|
||||
match msg {
|
||||
GossipMessage::Announce(_) => "Announce",
|
||||
GossipMessage::Leave => "Leave",
|
||||
GossipMessage::Chat { .. } => "Chat",
|
||||
}
|
||||
}
|
||||
|
||||
fn state_mutation_kind(msg: &GossipMessage) -> Option<StateMutationKind> {
|
||||
match msg {
|
||||
GossipMessage::Announce(_) => Some(StateMutationKind::Announce),
|
||||
GossipMessage::Leave => Some(StateMutationKind::Leave),
|
||||
GossipMessage::Chat { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn admit_state_mutation(
|
||||
seen: &mut HashMap<(EndpointId, StateMutationKind), u64>,
|
||||
author: EndpointId,
|
||||
msg: &GossipMessage,
|
||||
ts: u64,
|
||||
) -> bool {
|
||||
let Some(kind) = state_mutation_kind(msg) else {
|
||||
return true;
|
||||
};
|
||||
let key = (author, kind);
|
||||
if seen.get(&key).is_some_and(|last_ts| ts <= *last_ts) {
|
||||
return false;
|
||||
}
|
||||
seen.insert(key, ts);
|
||||
true
|
||||
}
|
||||
|
||||
fn peer_state_for_log(state: &PeerState) -> String {
|
||||
format!(
|
||||
"name={:?}, muted={}, addr_id={}, addrs={}, sharing={}",
|
||||
state.name,
|
||||
state.is_muted,
|
||||
crate::short_id(&state.addr.id.to_string()),
|
||||
state.addr.addrs.len(),
|
||||
state.sharing.is_some()
|
||||
)
|
||||
}
|
||||
|
||||
/// Authenticate a received payload against the room topic and local clock. The
|
||||
/// signature must validate for the claimed `author` (closing the author-spoofing
|
||||
/// vector, security S2), and the timestamp must be within `window_ms` of
|
||||
@@ -191,11 +241,21 @@ impl RoomState for IrohGossipState {
|
||||
self_state: PeerState,
|
||||
extra_bootstrap: Vec<EndpointAddr>,
|
||||
) -> Result<(), NetError> {
|
||||
crate::log_msg(&format!("RoomState::join: self_id={:?}, self_name={:?}, ticket={}", self_state.addr.id, self_state.name, ticket_str));
|
||||
crate::log_msg(&format!(
|
||||
"RoomState::join: self_id={}, self_name={:?}, ticket={}",
|
||||
crate::short_id(&self_state.addr.id.to_string()),
|
||||
self_state.name,
|
||||
crate::redact_for_log(ticket_str)
|
||||
));
|
||||
let ticket = ticket_str.parse::<PeerSpeakTicket>()?;
|
||||
let topic_id = TopicId::from_bytes(ticket.topic_id);
|
||||
|
||||
crate::log_msg(&format!("Parsed ticket. host_id={:?}, host_addrs={:?}, topic={:?}", ticket.host_addr.id, ticket.host_addr.addrs, topic_id));
|
||||
crate::log_msg(&format!(
|
||||
"Parsed ticket. host_id={}, host_addrs={}, topic={}",
|
||||
crate::short_id(&ticket.host_addr.id.to_string()),
|
||||
ticket.host_addr.addrs.len(),
|
||||
crate::short_bytes_hex(&ticket.topic_id)
|
||||
));
|
||||
|
||||
// Stop any currently running topic
|
||||
let _ = self.leave().await;
|
||||
@@ -242,6 +302,7 @@ impl RoomState for IrohGossipState {
|
||||
|
||||
let handle = tokio::spawn(async move {
|
||||
crate::log_msg(&format!("Spawned gossip topic loop for self_id={:?}", self_id));
|
||||
let mut state_mutations_seen = HashMap::new();
|
||||
|
||||
// Broadcast initial state
|
||||
let initial_payload = {
|
||||
@@ -291,7 +352,26 @@ impl RoomState for IrohGossipState {
|
||||
continue;
|
||||
}
|
||||
|
||||
crate::log_msg(&format!("Gossip Event::Received from author={:?}, payload={:?}", payload.author, payload.msg));
|
||||
if !admit_state_mutation(
|
||||
&mut state_mutations_seen,
|
||||
payload.author,
|
||||
&payload.msg,
|
||||
payload.ts,
|
||||
) {
|
||||
crate::log_msg(&format!(
|
||||
"Gossip dropped replayed state mutation author={}, kind={}, ts={}",
|
||||
crate::short_id(&payload.author.to_string()),
|
||||
gossip_message_kind(&payload.msg),
|
||||
payload.ts
|
||||
));
|
||||
continue;
|
||||
}
|
||||
|
||||
crate::log_msg(&format!(
|
||||
"Gossip Event::Received author={}, kind={}",
|
||||
crate::short_id(&payload.author.to_string()),
|
||||
gossip_message_kind(&payload.msg)
|
||||
));
|
||||
|
||||
match payload.msg {
|
||||
GossipMessage::Announce(mut state) => {
|
||||
@@ -305,6 +385,10 @@ impl RoomState for IrohGossipState {
|
||||
// monogram, so a malformed/oversized/bomb
|
||||
// image can't crash or exhaust us (W4).
|
||||
state.avatar = state.avatar.sanitize_incoming();
|
||||
// Screen-share tickets are capabilities and
|
||||
// peer-supplied: cap/validate once at ingest
|
||||
// so invalid offers never render a Watch button.
|
||||
state.sharing = state.sharing.and_then(crate::screenshare::sanitize_ticket);
|
||||
let (is_new, state_changed) = {
|
||||
let mut peer_map = peers.lock().unwrap();
|
||||
let is_new = !peer_map.contains_key(&payload.author);
|
||||
@@ -316,11 +400,19 @@ impl RoomState for IrohGossipState {
|
||||
};
|
||||
|
||||
if is_new {
|
||||
crate::log_msg(&format!("Gossip new peer joined: {:?}, state: {:?}", payload.author, state));
|
||||
crate::log_msg(&format!(
|
||||
"Gossip new peer joined: {}, state: {}",
|
||||
crate::short_id(&payload.author.to_string()),
|
||||
peer_state_for_log(&state)
|
||||
));
|
||||
address_lookup.add_endpoint_info(state.addr.clone());
|
||||
let _ = event_tx.send(RoomEvent::PeerJoined(payload.author, state)).await;
|
||||
} else if state_changed {
|
||||
crate::log_msg(&format!("Gossip peer state updated: {:?}, state: {:?}", payload.author, state));
|
||||
crate::log_msg(&format!(
|
||||
"Gossip peer state updated: {}, state: {}",
|
||||
crate::short_id(&payload.author.to_string()),
|
||||
peer_state_for_log(&state)
|
||||
));
|
||||
let _ = event_tx.send(RoomEvent::PeerUpdated(payload.author, state)).await;
|
||||
}
|
||||
}
|
||||
@@ -396,7 +488,10 @@ impl RoomState for IrohGossipState {
|
||||
}
|
||||
|
||||
async fn update_self_state(&self, self_state: PeerState) -> Result<(), NetError> {
|
||||
crate::log_msg(&format!("RoomState::update_self_state: state={:?}", self_state));
|
||||
crate::log_msg(&format!(
|
||||
"RoomState::update_self_state: state: {}",
|
||||
peer_state_for_log(&self_state)
|
||||
));
|
||||
*self.self_state.lock().unwrap() = Some(self_state.clone());
|
||||
|
||||
let sender_opt = self.active_sender.lock().unwrap().clone();
|
||||
@@ -495,6 +590,7 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::network::PeerState;
|
||||
use iroh::SecretKey;
|
||||
use std::collections::HashMap;
|
||||
|
||||
fn sample_peer_state_for(id: EndpointId) -> PeerState {
|
||||
let addr = iroh::EndpointAddr::from(id);
|
||||
@@ -750,4 +846,46 @@ mod tests {
|
||||
Err(GossipReject::AnnounceAddressMismatch)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn state_mutation_replay_gate_drops_replayed_leave_and_announce() {
|
||||
let author = fresh_id();
|
||||
let mut seen = HashMap::new();
|
||||
|
||||
assert!(admit_state_mutation(&mut seen, author, &GossipMessage::Leave, 10));
|
||||
assert!(!admit_state_mutation(&mut seen, author, &GossipMessage::Leave, 10));
|
||||
assert!(!admit_state_mutation(&mut seen, author, &GossipMessage::Leave, 9));
|
||||
assert!(admit_state_mutation(&mut seen, author, &GossipMessage::Leave, 11));
|
||||
|
||||
let announce = GossipMessage::Announce(sample_peer_state_for(author));
|
||||
assert!(admit_state_mutation(&mut seen, author, &announce, 10));
|
||||
assert!(!admit_state_mutation(&mut seen, author, &announce, 10));
|
||||
assert!(!admit_state_mutation(&mut seen, author, &announce, 9));
|
||||
assert!(admit_state_mutation(&mut seen, author, &announce, 12));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn state_mutation_replay_gate_leaves_chat_ordering_untouched() {
|
||||
let author = fresh_id();
|
||||
let mut seen = HashMap::new();
|
||||
let later_chat = GossipMessage::Chat { name: "A".into(), text: "later".into(), ts: 200 };
|
||||
let earlier_chat = GossipMessage::Chat { name: "A".into(), text: "earlier".into(), ts: 100 };
|
||||
|
||||
assert!(admit_state_mutation(&mut seen, author, &later_chat, 200));
|
||||
assert!(admit_state_mutation(&mut seen, author, &earlier_chat, 100));
|
||||
assert!(admit_state_mutation(&mut seen, author, &later_chat, 200));
|
||||
assert!(seen.is_empty(), "chat must not populate the state-mutation replay map");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn state_mutation_replay_gate_is_per_author_and_kind() {
|
||||
let author = fresh_id();
|
||||
let other = fresh_id();
|
||||
let mut seen = HashMap::new();
|
||||
let announce = GossipMessage::Announce(sample_peer_state_for(author));
|
||||
|
||||
assert!(admit_state_mutation(&mut seen, author, &GossipMessage::Leave, 5));
|
||||
assert!(admit_state_mutation(&mut seen, author, &announce, 5));
|
||||
assert!(admit_state_mutation(&mut seen, other, &GossipMessage::Leave, 5));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user