fix(peer): drop non-unicast or zero-port mDNS discovery candidates
Security audit finding NET-02. A discovered `_lanspread._udp` record was accepted as a QUIC candidate as soon as it carried the current protocol version and a peer_id TXT entry; the resolved socket address itself was never inspected. Anyone on the LAN can publish mDNS records, so a forged advertisement could point every peer's handshake attempt at a multicast group (224.0.0.251), the broadcast address, 0.0.0.0/::, or port 0. That wastes a discovery slot per record and sprays QUIC Initial packets onto addresses no peer can ever answer from. `validated_candidate_endpoint` now rejects unspecified, multicast and IPv4 broadcast addresses as well as port 0 before the candidate enters the negotiation set. Loopback and link-local addresses stay admissible because single-host and DHCP-less LAN setups legitimately use them. Test plan: `just test` (new unit tests cover the address filter and the endpoint validator). Manually: start two peer-cli containers; discovery still works with real interface addresses. Claude-Session: https://claude.ai/code/session_017C3Nbgwpdm3YNwZhhFLHwg
This commit is contained in:
@@ -390,6 +390,22 @@ async fn is_self_advertisement(info: &MdnsPeerInfo, ctx: &NetworkServiceCtx) ->
|
|||||||
.is_some_and(|peer_id| *peer_id == ctx.peer_id)
|
.is_some_and(|peer_id| *peer_id == ctx.peer_id)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns whether a discovered socket address is a plausible unicast QUIC
|
||||||
|
/// endpoint. mDNS records are attacker-controlled, so multicast, broadcast,
|
||||||
|
/// unspecified, and zero-port targets are dropped before any handshake packet
|
||||||
|
/// is sent toward them.
|
||||||
|
fn is_admissible_candidate_addr(addr: std::net::SocketAddr) -> bool {
|
||||||
|
if addr.port() == 0 {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
match addr.ip() {
|
||||||
|
std::net::IpAddr::V4(ip) => {
|
||||||
|
!ip.is_unspecified() && !ip.is_multicast() && !ip.is_broadcast()
|
||||||
|
}
|
||||||
|
std::net::IpAddr::V6(ip) => !ip.is_unspecified() && !ip.is_multicast(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn validated_candidate_endpoint(info: &MdnsPeerInfo) -> Option<PeerEndpoint> {
|
fn validated_candidate_endpoint(info: &MdnsPeerInfo) -> Option<PeerEndpoint> {
|
||||||
if info.proto_ver != Some(PROTOCOL_VERSION) {
|
if info.proto_ver != Some(PROTOCOL_VERSION) {
|
||||||
log::debug!(
|
log::debug!(
|
||||||
@@ -400,6 +416,14 @@ fn validated_candidate_endpoint(info: &MdnsPeerInfo) -> Option<PeerEndpoint> {
|
|||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !is_admissible_candidate_addr(info.addr) {
|
||||||
|
log::debug!(
|
||||||
|
"Ignoring current-protocol peer advertised at non-unicast or zero-port address {}",
|
||||||
|
info.addr
|
||||||
|
);
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
let Some(peer_id) = info.peer_id else {
|
let Some(peer_id) = info.peer_id else {
|
||||||
log::debug!(
|
log::debug!(
|
||||||
"Ignoring current-protocol peer at {} without a peer_id TXT record",
|
"Ignoring current-protocol peer at {} without a peer_id TXT record",
|
||||||
@@ -451,10 +475,13 @@ mod tests {
|
|||||||
DISCOVERY_CANDIDATE_COOLDOWN,
|
DISCOVERY_CANDIDATE_COOLDOWN,
|
||||||
DiscoveryWorker,
|
DiscoveryWorker,
|
||||||
MAX_ACTIVE_DISCOVERY_CANDIDATES,
|
MAX_ACTIVE_DISCOVERY_CANDIDATES,
|
||||||
|
MdnsPeerInfo,
|
||||||
RecentCandidates,
|
RecentCandidates,
|
||||||
candidate_conflicts,
|
candidate_conflicts,
|
||||||
candidate_is_admissible,
|
candidate_is_admissible,
|
||||||
drain_service_children,
|
drain_service_children,
|
||||||
|
is_admissible_candidate_addr,
|
||||||
|
validated_candidate_endpoint,
|
||||||
};
|
};
|
||||||
|
|
||||||
fn endpoint(seed: u8, port: u16) -> PeerEndpoint {
|
fn endpoint(seed: u8, port: u16) -> PeerEndpoint {
|
||||||
@@ -464,6 +491,57 @@ mod tests {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn advertised_candidate_addresses_must_be_unicast_with_a_nonzero_port() {
|
||||||
|
for addr in [
|
||||||
|
"192.168.1.50:42424",
|
||||||
|
"10.0.0.7:1",
|
||||||
|
"127.0.0.1:42424",
|
||||||
|
"[fe80::1]:42424",
|
||||||
|
"[::1]:42424",
|
||||||
|
] {
|
||||||
|
let addr: SocketAddr = addr.parse().expect("test address parses");
|
||||||
|
assert!(is_admissible_candidate_addr(addr), "rejected {addr}");
|
||||||
|
}
|
||||||
|
for addr in [
|
||||||
|
"192.168.1.50:0",
|
||||||
|
"0.0.0.0:42424",
|
||||||
|
"224.0.0.251:42424",
|
||||||
|
"239.255.255.250:42424",
|
||||||
|
"255.255.255.255:42424",
|
||||||
|
"[::]:42424",
|
||||||
|
"[ff02::fb]:42424",
|
||||||
|
] {
|
||||||
|
let addr: SocketAddr = addr.parse().expect("test address parses");
|
||||||
|
assert!(!is_admissible_candidate_addr(addr), "accepted {addr}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn validated_candidate_endpoint_drops_non_unicast_advertisements() {
|
||||||
|
let peer_id = Some(PeerId::from_bytes([7; 32]));
|
||||||
|
let unicast = MdnsPeerInfo {
|
||||||
|
addr: "192.168.1.50:42424".parse().expect("test address parses"),
|
||||||
|
peer_id,
|
||||||
|
proto_ver: Some(lanspread_proto::PROTOCOL_VERSION),
|
||||||
|
};
|
||||||
|
assert!(validated_candidate_endpoint(&unicast).is_some());
|
||||||
|
|
||||||
|
let multicast = MdnsPeerInfo {
|
||||||
|
addr: "224.0.0.251:42424".parse().expect("test address parses"),
|
||||||
|
peer_id,
|
||||||
|
proto_ver: Some(lanspread_proto::PROTOCOL_VERSION),
|
||||||
|
};
|
||||||
|
assert!(validated_candidate_endpoint(&multicast).is_none());
|
||||||
|
|
||||||
|
let zero_port = MdnsPeerInfo {
|
||||||
|
addr: "192.168.1.50:0".parse().expect("test address parses"),
|
||||||
|
peer_id,
|
||||||
|
proto_ver: Some(lanspread_proto::PROTOCOL_VERSION),
|
||||||
|
};
|
||||||
|
assert!(validated_candidate_endpoint(&zero_port).is_none());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn active_candidate_keys_bound_both_claimed_identity_and_address() {
|
fn active_candidate_keys_bound_both_claimed_identity_and_address() {
|
||||||
let active_endpoint = endpoint(1, 12001);
|
let active_endpoint = endpoint(1, 12001);
|
||||||
|
|||||||
Reference in New Issue
Block a user