diff --git a/CHANGELOG.md b/CHANGELOG.md index 84a653d..cccc88e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Nostr discovery: filter unroutable direct UDP/TCP advert endpoints. + Publisher and validator now retain only endpoints that parse as + concrete socket addresses with routable IPs and nonzero ports. + `udp:nat` rendezvous endpoints and Tor endpoints pass through + unchanged. Adverts that collapse to zero usable endpoints after + filtering are rejected with a clear "missing publicly routable + endpoints" error. Before this change, misconfigured nodes could + publish RFC1918, loopback, link-local, CGNAT 100.64/10, IPv6 ULA, + or IPv6 link-local endpoints into Nostr discovery, and consumers + would cache and dial them; in mixed LAN/VPN/NAT environments, that + could prefer a misleading one-way private path over the intended + `udp:nat` bootstrap. - Coord cache invalidation made surgical at parent-position-change and root-change sites. Replaces the previous unconditional `CoordCache::clear()` calls with two targeted methods: diff --git a/src/discovery/nostr/runtime.rs b/src/discovery/nostr/runtime.rs index 26f3b4b..d5852f9 100644 --- a/src/discovery/nostr/runtime.rs +++ b/src/discovery/nostr/runtime.rs @@ -57,6 +57,67 @@ fn endpoint_summary(endpoints: &[OverlayEndpointAdvert]) -> String { .join(",") } +fn is_unroutable_direct_advert_ip(ip: std::net::IpAddr) -> bool { + match ip { + std::net::IpAddr::V4(v4) => { + v4.is_private() + || v4.is_loopback() + || v4.is_link_local() + || v4.is_unspecified() + || v4.is_multicast() + || v4.is_broadcast() + || v4.is_documentation() + || (v4.octets()[0] == 100 && (v4.octets()[1] & 0xc0) == 64) + } + std::net::IpAddr::V6(v6) => { + v6.is_loopback() + || v6.is_unspecified() + || v6.is_unique_local() + || v6.is_multicast() + || (v6.segments()[0] & 0xffc0) == 0xfe80 + } + } +} + +fn endpoint_advert_is_publicly_usable(endpoint: &OverlayEndpointAdvert) -> bool { + let addr = endpoint.addr.trim(); + if addr.is_empty() { + return false; + } + + if endpoint.transport == super::types::OverlayTransportKind::Udp + && addr.eq_ignore_ascii_case("nat") + { + return true; + } + if addr.eq_ignore_ascii_case("nat") { + return false; + } + + match endpoint.transport { + super::types::OverlayTransportKind::Udp | super::types::OverlayTransportKind::Tcp => { + let Ok(socket_addr) = addr.parse::() else { + let Some((host, port)) = addr.rsplit_once(':') else { + return false; + }; + let host = host.trim().trim_start_matches('[').trim_end_matches(']'); + if host.is_empty() || port.trim().parse::().ok().is_none_or(|p| p == 0) { + return false; + } + if host.eq_ignore_ascii_case("localhost") { + return false; + } + return host + .parse::() + .ok() + .is_none_or(|ip| !is_unroutable_direct_advert_ip(ip)); + }; + socket_addr.port() != 0 && !is_unroutable_direct_advert_ip(socket_addr.ip()) + } + super::types::OverlayTransportKind::Tor => true, + } +} + /// Cached STUN-derived public address for an advert-eligible UDP transport /// bound to a wildcard. Lives on `NostrDiscovery` so the freshness window /// survives advert refresh cycles. @@ -758,6 +819,7 @@ impl NostrDiscovery { advert.identifier = ADVERT_IDENTIFIER.to_string(); advert.version = ADVERT_VERSION; + advert.endpoints.retain(endpoint_advert_is_publicly_usable); // Defensive: build_overlay_advert returns None on empty endpoints, // so this is only reachable from non-lifecycle callers. if advert.endpoints.is_empty() { @@ -1300,12 +1362,11 @@ impl NostrDiscovery { "missing required endpoints".to_string(), )); } - for endpoint in &advert.endpoints { - if endpoint.addr.trim().is_empty() { - return Err(BootstrapError::InvalidAdvert( - "endpoint addr cannot be empty".to_string(), - )); - } + advert.endpoints.retain(endpoint_advert_is_publicly_usable); + if advert.endpoints.is_empty() { + return Err(BootstrapError::InvalidAdvert( + "missing publicly routable endpoints".to_string(), + )); } let has_nat = advert.has_udp_nat_endpoint(); diff --git a/src/discovery/nostr/tests.rs b/src/discovery/nostr/tests.rs index b412058..d28201c 100644 --- a/src/discovery/nostr/tests.rs +++ b/src/discovery/nostr/tests.rs @@ -39,7 +39,7 @@ fn can_reach(local_nat: NatType, remote_nat: NatType) -> bool { fn signed_overlay_advert_event(created_at_secs: u64, expiration_secs: Option) -> nostr::Event { let keys = nostr::Keys::generate(); - let content = r#"{"identifier":"fips-overlay-v1","version":1,"endpoints":[{"transport":"tcp","addr":"203.0.113.10:443"}]}"#; + let content = r#"{"identifier":"fips-overlay-v1","version":1,"endpoints":[{"transport":"tcp","addr":"8.8.8.8:443"}]}"#; let mut builder = EventBuilder::new(Kind::Custom(ADVERT_KIND), content) .custom_created_at(Timestamp::from(created_at_secs)); if let Some(expiration_secs) = expiration_secs { @@ -118,6 +118,57 @@ fn rejects_invalid_overlay_adverts() { assert!(NostrDiscovery::validate_overlay_advert(wrong_identifier).is_err()); } +#[test] +fn validate_overlay_advert_filters_unroutable_direct_endpoints() { + let advert = OverlayAdvert { + identifier: ADVERT_IDENTIFIER.to_string(), + version: ADVERT_VERSION, + endpoints: vec![ + OverlayEndpointAdvert { + transport: OverlayTransportKind::Tcp, + addr: "192.168.1.10:443".to_string(), + }, + OverlayEndpointAdvert { + transport: OverlayTransportKind::Udp, + addr: "100.64.1.2:2121".to_string(), + }, + OverlayEndpointAdvert { + transport: OverlayTransportKind::Tcp, + addr: "8.8.8.8:443".to_string(), + }, + ], + signal_relays: None, + stun_servers: None, + }; + + let validated = NostrDiscovery::validate_overlay_advert(advert).unwrap(); + assert_eq!(validated.endpoints.len(), 1); + assert_eq!(validated.endpoints[0].addr, "8.8.8.8:443"); +} + +#[test] +fn validate_overlay_advert_rejects_only_unroutable_direct_endpoints() { + let advert = OverlayAdvert { + identifier: ADVERT_IDENTIFIER.to_string(), + version: ADVERT_VERSION, + endpoints: vec![ + OverlayEndpointAdvert { + transport: OverlayTransportKind::Tcp, + addr: "127.0.0.1:443".to_string(), + }, + OverlayEndpointAdvert { + transport: OverlayTransportKind::Udp, + addr: "10.0.0.2:2121".to_string(), + }, + ], + signal_relays: None, + stun_servers: None, + }; + + let err = NostrDiscovery::validate_overlay_advert(advert).unwrap_err(); + assert!(err.to_string().contains("missing publicly routable")); +} + #[test] fn advert_freshness_rejects_expired_events() { let now_secs = Timestamp::now().as_secs();