diff --git a/CHANGELOG.md b/CHANGELOG.md index b27e449..4efeef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -274,6 +274,47 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- UDP transport with `advertise_on_nostr: true` + `public: true` + + a wildcard `bind_addr` (e.g. `0.0.0.0:2121`) is now advertised + with its STUN-discovered public IPv4 instead of being silently + dropped from the published Kind 37195 advert. Previously the + advert builder filtered the wildcard out (since `0.0.0.0` is + not a valid endpoint), but emitted no log explaining what + happened — operators saw the daemon up, both flags set, and + no UDP endpoint in the advert. The fix runs a one-shot STUN + observation against an ephemeral socket on the daemon's + configured `stun_servers` and combines the reflexive IPv4 with + the configured listener port for the advert (`udp::`). + Successful STUN observations are cached per-transport for one + `advert_refresh_secs` cycle (default 30 min) so we don't re-STUN + every refresh. Failed observations are cached for only 60s, so + a transient STUN flake at startup retries within ~a minute and + grows the advert with UDP as soon as STUN starts working — + rather than waiting the full 30-min cycle. Per-server STUN + response timeout is 5s for the advert-publish path (vs. 2s for + the latency-sensitive per-traversal path), giving slow + first-call STUN time to complete without giving up. On STUN + failure, the wildcard-bind path still skips, but now logs a + loud `warn!` pointing at the operator-side fixes (set + `external_addr`, bind to a specific IP, or ensure `stun_servers` + reachable). Restores zero-config public-IP autodiscovery on + AWS EIP / GCP / Azure setups where binding to the public IP + directly is impossible (1:1 NAT) +- New `external_addr` field on `transports.udp.*` and + `transports.tcp.*` for explicit advertise-as override. Accepts + either a bare IP (`"54.183.70.180"` — the configured `bind_addr` + port is appended) or a full `host:port` + (`"54.183.70.180:8443"`). Takes precedence over both the bound + address and any STUN-derived autodiscovery. Required for TCP + on cloud-NAT setups (AWS EIP, GCP/Azure external IPs) where + binding to the public IP directly fails with `EADDRNOTAVAIL` + (the EIP isn't on a host interface). Optional but useful for + UDP as a deterministic alternative to STUN — operators who + want to skip STUN egress (or whose STUN is blocked) can + specify it explicitly. Without `external_addr`, TCP with a + wildcard `bind_addr` + `advertise_on_nostr: true` now logs a + loud `warn!` pointing at the two fixes instead of silently + skipping - Nostr-discovery now tolerates ±60s of clock skew on offer/answer freshness checks so a responder whose wall clock leads the initiator's by less than that no longer silently rejects every diff --git a/src/config/transport.rs b/src/config/transport.rs index af996f6..2281ee4 100644 --- a/src/config/transport.rs +++ b/src/config/transport.rs @@ -4,9 +4,31 @@ //! transport-specific configuration structs. use std::collections::HashMap; +use std::net::{IpAddr, SocketAddr}; use serde::{Deserialize, Serialize}; +/// Parse an `external_addr` config string against a known bind port, +/// producing the absolute `SocketAddr` to advertise on Nostr. +/// +/// Accepts either a bare IP (`"54.183.70.180"` or `"[::1]"`) — in which +/// case the bind port is appended — or a full `host:port` form +/// (`"54.183.70.180:443"` or `"[::1]:443"`). Returns `None` on any parse +/// error. IPv6 must use bracket notation when supplying a port. +fn parse_external_advert_addr(raw: &str, bind_port: u16) -> Option { + if let Ok(sa) = raw.parse::() { + return Some(sa); + } + let ip: IpAddr = raw.parse().ok()?; + Some(SocketAddr::new(ip, bind_port)) +} + +/// Extract the port from a `bind_addr` string. Returns `None` if the +/// string can't be parsed (e.g. a bare hostname without port). +fn parse_bind_port(raw: &str) -> Option { + raw.parse::().ok().map(|sa| sa.port()) +} + /// Default UDP bind address. const DEFAULT_UDP_BIND_ADDR: &str = "0.0.0.0:2121"; @@ -54,6 +76,16 @@ pub struct UdpConfig { /// Default: false. #[serde(default, skip_serializing_if = "Option::is_none")] pub public: Option, + /// Optional explicit public address to advertise when `public: true` + /// is set. Takes precedence over both the bound address and any + /// STUN-derived autodiscovery. Accepts either a bare IP + /// (`"54.183.70.180"` — the configured `bind_addr` port is appended) + /// or a full `host:port` (`"54.183.70.180:443"`). Useful when the + /// public IP isn't on a local interface (e.g. AWS EIP / cloud 1:1 + /// NAT) and the operator wants to skip STUN autodiscovery for a + /// deterministic value. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub external_addr: Option, /// Outbound-only mode. When true, the transport binds to a kernel- /// assigned ephemeral port (`0.0.0.0:0`) instead of the configured /// `bind_addr`, refuses inbound handshake msg1, and is never @@ -119,6 +151,16 @@ impl UdpConfig { self.public.unwrap_or(false) } + /// Parse `external_addr` against the configured `bind_addr` port, + /// returning the absolute `SocketAddr` to advertise on Nostr. + /// Returns `None` if `external_addr` is unset or malformed, or if + /// the port cannot be inferred. + pub fn external_advert_addr(&self) -> Option { + let raw = self.external_addr.as_deref()?; + let bind_port = parse_bind_port(self.bind_addr())?; + parse_external_advert_addr(raw, bind_port) + } + /// Whether this transport runs in outbound-only mode. Default: false. pub fn outbound_only(&self) -> bool { self.outbound_only.unwrap_or(false) @@ -366,6 +408,16 @@ pub struct TcpConfig { /// Default: false. #[serde(default, skip_serializing_if = "Option::is_none")] pub advertise_on_nostr: Option, + + /// Optional explicit public address to advertise. Required when + /// `bind_addr` is wildcard (e.g. `"0.0.0.0:443"`) and + /// `advertise_on_nostr: true`, since TCP has no STUN equivalent + /// for autodiscovery. Accepts either a bare IP (`"54.183.70.180"` + /// — the configured `bind_addr` port is appended) or a full + /// `host:port`. Common pattern on AWS EIP / cloud 1:1 NAT setups + /// where the public IP isn't bindable on the host. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub external_addr: Option, } impl TcpConfig { @@ -410,6 +462,16 @@ impl TcpConfig { pub fn advertise_on_nostr(&self) -> bool { self.advertise_on_nostr.unwrap_or(false) } + + /// Parse `external_addr` against the configured `bind_addr` port, + /// returning the absolute `SocketAddr` to advertise on Nostr. + /// Returns `None` if `external_addr` is unset or malformed, or if + /// `bind_addr` is unset / unparseable so no port can be inferred. + pub fn external_advert_addr(&self) -> Option { + let raw = self.external_addr.as_deref()?; + let bind_port = parse_bind_port(self.bind_addr.as_deref()?)?; + parse_external_advert_addr(raw, bind_port) + } } // ============================================================================ @@ -804,3 +866,103 @@ impl TransportsConfig { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_external_addr_accepts_bare_ipv4_with_appended_bind_port() { + let sa = parse_external_advert_addr("54.183.70.180", 2121).unwrap(); + assert_eq!(sa.to_string(), "54.183.70.180:2121"); + } + + #[test] + fn parse_external_addr_accepts_full_ipv4_socket_addr() { + let sa = parse_external_advert_addr("54.183.70.180:443", 2121).unwrap(); + assert_eq!(sa.to_string(), "54.183.70.180:443"); + // Explicit port wins over the bind port we passed in. + } + + #[test] + fn parse_external_addr_accepts_bare_ipv6_with_appended_bind_port() { + let sa = parse_external_advert_addr("2001:db8::1", 443).unwrap(); + assert_eq!(sa.to_string(), "[2001:db8::1]:443"); + } + + #[test] + fn parse_external_addr_accepts_bracketed_ipv6_with_explicit_port() { + let sa = parse_external_advert_addr("[2001:db8::1]:8443", 443).unwrap(); + assert_eq!(sa.to_string(), "[2001:db8::1]:8443"); + } + + #[test] + fn parse_external_addr_rejects_garbage() { + assert!(parse_external_advert_addr("not-an-ip", 443).is_none()); + assert!(parse_external_advert_addr("", 443).is_none()); + } + + #[test] + fn udp_external_advert_addr_combines_with_bind_port_default() { + let cfg = UdpConfig { + external_addr: Some("54.183.70.180".to_string()), + ..UdpConfig::default() + }; + // bind_addr unset, so default DEFAULT_UDP_BIND_ADDR (0.0.0.0:2121) applies. + let sa = cfg.external_advert_addr().unwrap(); + assert_eq!(sa.to_string(), "54.183.70.180:2121"); + } + + #[test] + fn udp_external_advert_addr_with_explicit_full_socket_addr_overrides_bind_port() { + let cfg = UdpConfig { + bind_addr: Some("0.0.0.0:2121".to_string()), + external_addr: Some("54.183.70.180:9999".to_string()), + ..UdpConfig::default() + }; + let sa = cfg.external_advert_addr().unwrap(); + assert_eq!(sa.to_string(), "54.183.70.180:9999"); + } + + #[test] + fn udp_external_advert_addr_returns_none_when_unset() { + let cfg = UdpConfig::default(); + assert!(cfg.external_advert_addr().is_none()); + } + + #[test] + fn tcp_external_advert_addr_requires_bind_port() { + let cfg = TcpConfig { + external_addr: Some("54.183.70.180".to_string()), + ..TcpConfig::default() + }; + // bind_addr unset → no port to combine with → None. + assert!(cfg.external_advert_addr().is_none()); + + let cfg = TcpConfig { + bind_addr: Some("0.0.0.0:443".to_string()), + external_addr: Some("54.183.70.180".to_string()), + ..TcpConfig::default() + }; + let sa = cfg.external_advert_addr().unwrap(); + assert_eq!(sa.to_string(), "54.183.70.180:443"); + } + + #[test] + fn tcp_external_advert_addr_with_full_socket_addr_independent_of_bind() { + let cfg = TcpConfig { + bind_addr: Some("0.0.0.0:443".to_string()), + external_addr: Some("54.183.70.180:8443".to_string()), + ..TcpConfig::default() + }; + let sa = cfg.external_advert_addr().unwrap(); + assert_eq!(sa.to_string(), "54.183.70.180:8443"); + } + + #[test] + fn parse_bind_port_extracts_from_socket_addr_strings() { + assert_eq!(parse_bind_port("0.0.0.0:2121"), Some(2121)); + assert_eq!(parse_bind_port("[::]:443"), Some(443)); + assert_eq!(parse_bind_port("not-a-socket-addr"), None); + } +} diff --git a/src/discovery/nostr/runtime.rs b/src/discovery/nostr/runtime.rs index a08bf88..4a64300 100644 --- a/src/discovery/nostr/runtime.rs +++ b/src/discovery/nostr/runtime.rs @@ -1,6 +1,7 @@ use std::collections::{HashMap, HashSet}; +use std::net::SocketAddr; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; use nostr::nips::nip17; use nostr::nips::nip19::ToBech32; @@ -56,6 +57,25 @@ fn endpoint_summary(endpoints: &[OverlayEndpointAdvert]) -> String { .join(",") } +/// 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. +struct CachedPublicUdpAddr { + /// Most recent STUN observation. `None` means the last attempt failed + /// (recorded so we don't re-spam STUN every refresh tick on broken + /// network conditions). + addr: Option, + fetched_at: Instant, +} + +/// Cache lifetime for a *failed* STUN observation. Held briefly so that +/// transient flakes (slow startup network, momentary STUN-server +/// blip) get retried within ~a minute and the advert grows its UDP +/// endpoint as soon as STUN starts working — rather than waiting a +/// full `advert_refresh_secs` (30 min) for the success-path TTL to +/// expire. Successful results use the longer per-config TTL. +const PUBLIC_UDP_ADDR_FAILURE_TTL: Duration = Duration::from_secs(60); + pub struct NostrDiscovery { client: Client, keys: nostr::Keys, @@ -74,6 +94,10 @@ pub struct NostrDiscovery { notify_task: Mutex>>, advertise_task: Mutex>>, failure_state: FailureState, + /// STUN-derived public address per advert-eligible UDP transport + /// (keyed by `TransportId.as_u32()`). Populated on demand by + /// `learn_public_udp_addr()` and refreshed by TTL. + public_udp_addr_cache: RwLock>, } impl NostrDiscovery { @@ -133,6 +157,7 @@ impl NostrDiscovery { notify_task: Mutex::new(None), advertise_task: Mutex::new(None), failure_state, + public_udp_addr_cache: RwLock::new(HashMap::new()), }); runtime.subscribe().await?; @@ -204,6 +229,108 @@ impl NostrDiscovery { .collect() } + /// Discover (or return cached) the public-Internet address for an + /// advert-eligible UDP transport bound to a wildcard. Used by + /// `build_overlay_advert` to avoid emitting `udp:0.0.0.0:port`, + /// which is invalid as an advertised endpoint. Result is the + /// reflexive IP (from STUN against the daemon's first + /// `stun_servers` reachable) combined with the configured + /// `advertise_port`. + /// + /// Asymmetric cache TTL: a successful observation is cached for + /// `advert_refresh_secs` (default 1800 = same as advert refresh) + /// so we don't re-STUN every refresh tick. A failed observation + /// is cached for `PUBLIC_UDP_ADDR_FAILURE_TTL` (60s) so we retry + /// soon after a transient STUN flake at startup, instead of + /// blocking advertise-as-public for half an hour. Once a success + /// is cached, subsequent ticks are zero-overhead. + pub async fn learn_public_udp_addr( + &self, + transport_id_key: u32, + advertise_port: u16, + ) -> Option { + if let Some(entry) = self + .public_udp_addr_cache + .read() + .await + .get(&transport_id_key) + { + let ttl = if entry.addr.is_some() { + Duration::from_secs(self.config.advert_refresh_secs.max(60)) + } else { + PUBLIC_UDP_ADDR_FAILURE_TTL + }; + if entry.fetched_at.elapsed() < ttl { + return entry.addr; + } + } + let resolved = self.stun_observe_public_ip(advertise_port).await; + let mut cache = self.public_udp_addr_cache.write().await; + cache.insert( + transport_id_key, + CachedPublicUdpAddr { + addr: resolved, + fetched_at: Instant::now(), + }, + ); + resolved + } + + /// Run a one-shot STUN observation against an ephemeral UDP socket + /// to learn this host's public IPv4 (or IPv6, if the local STUN + /// server returns one). Returns `:`, + /// or `None` if STUN failed or no `stun_servers` are configured. + /// + /// The STUN-reported port is the ephemeral source port and is + /// discarded — what we want to advertise is the bound listener + /// port, which the kernel preserves through 1:1 NAT (AWS EIP, + /// GCP/Azure external IPs) and which the operator has explicitly + /// chosen via `bind_addr`. + async fn stun_observe_public_ip(&self, advertise_port: u16) -> Option { + if self.config.stun_servers.is_empty() { + return None; + } + let socket = match std::net::UdpSocket::bind("0.0.0.0:0") { + Ok(s) => s, + Err(err) => { + debug!(error = %err, "public-udp-addr: ephemeral bind failed"); + return None; + } + }; + if let Err(err) = socket.set_nonblocking(true) { + debug!(error = %err, "public-udp-addr: set_nonblocking failed"); + return None; + } + let observed = match super::stun::observe_traversal_addresses( + &socket, + &self.config.stun_servers, + false, + super::stun::ADVERT_STUN_TIMEOUT, + ) + .await + { + Ok((reflexive, _local, stun_server)) => { + debug!( + stun = %stun_server.as_deref().unwrap_or("-"), + reflexive = %reflexive + .as_ref() + .map(|a| format!("{}:{}", a.ip, a.port)) + .unwrap_or_else(|| "-".into()), + "public-udp-addr: STUN observation" + ); + reflexive + } + Err(err) => { + debug!(error = %err, "public-udp-addr: STUN failed"); + return None; + } + }; + observed.and_then(|addr| { + let parsed_ip: std::net::IpAddr = addr.ip.parse().ok()?; + Some(SocketAddr::new(parsed_ip, advertise_port)) + }) + } + /// Stale-advert re-check (B6). Called by lifecycle on the /// streak-threshold transition. Actively re-queries the peer's /// Kind 37195 advert from `advert_relays`; evicts the cache entry @@ -663,6 +790,7 @@ impl NostrDiscovery { &base_socket, &self.config.stun_servers, self.config.share_local_candidates, + super::stun::TRAVERSAL_STUN_TIMEOUT, ) .await?; debug!( @@ -852,6 +980,7 @@ impl NostrDiscovery { &base_socket, &self.config.stun_servers, self.config.share_local_candidates, + super::stun::TRAVERSAL_STUN_TIMEOUT, ) .await?; let accepted = reflexive_address.is_some() || !local_addresses.is_empty(); @@ -1317,6 +1446,7 @@ impl NostrDiscovery { notify_task: Mutex::new(None), advertise_task: Mutex::new(None), failure_state, + public_udp_addr_cache: RwLock::new(HashMap::new()), } } diff --git a/src/discovery/nostr/stun.rs b/src/discovery/nostr/stun.rs index c51743b..3aa7a14 100644 --- a/src/discovery/nostr/stun.rs +++ b/src/discovery/nostr/stun.rs @@ -11,10 +11,23 @@ use super::types::{BootstrapError, TraversalAddress}; // Local interface discovery remains best-effort and may still be incomplete // on dual-stack, NAT64, or heavily firewalled hosts. +/// Default per-server STUN response wait used by the per-traversal flow. +/// Latency-sensitive: keep tight so a misbehaving STUN server doesn't +/// stretch every traversal attempt. +pub(super) const TRAVERSAL_STUN_TIMEOUT: Duration = Duration::from_secs(2); + +/// Per-server STUN response wait used by the advert-publish path's +/// public-IP discovery. Longer than `TRAVERSAL_STUN_TIMEOUT` because +/// it's a one-shot at startup (cached afterward) and we'd rather block +/// the first advert build by a few seconds than skip UDP advertising +/// over a slow first response. Returns immediately on success. +pub(super) const ADVERT_STUN_TIMEOUT: Duration = Duration::from_secs(5); + pub(super) async fn observe_traversal_addresses( socket: &std::net::UdpSocket, stun_servers: &[String], share_local_candidates: bool, + per_server_timeout: Duration, ) -> Result< ( Option, @@ -39,7 +52,7 @@ pub(super) async fn observe_traversal_addresses( let mut last_error = None; for stun_server in stun_servers { - match perform_stun(socket, stun_server).await { + match perform_stun(socket, stun_server, per_server_timeout).await { Ok(mapped) => { debug!( stun_server = %stun_server, @@ -70,6 +83,7 @@ pub(super) async fn observe_traversal_addresses( async fn perform_stun( socket: &std::net::UdpSocket, stun_server: &str, + response_timeout: Duration, ) -> Result, BootstrapError> { let endpoint = parse_stun_url(stun_server)?; let txn_id = random_txn_id(); @@ -80,7 +94,7 @@ async fn perform_stun( let udp = UdpSocket::from_std(socket.try_clone()?)?; udp.send_to(&request, addr).await?; let mut buf = [0u8; 2048]; - let deadline = tokio::time::Instant::now() + Duration::from_secs(2); + let deadline = tokio::time::Instant::now() + response_timeout; loop { let result = tokio::time::timeout_at(deadline, udp.recv_from(&mut buf)).await; let Ok(Ok((len, _remote))) = result else { diff --git a/src/node/lifecycle.rs b/src/node/lifecycle.rs index 55b546d..5d7105f 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -1487,7 +1487,10 @@ impl Node { ) } - fn build_overlay_advert(&self) -> Option { + async fn build_overlay_advert( + &self, + bootstrap: &std::sync::Arc, + ) -> Option { if !self.config.node.discovery.nostr.enabled { return None; } @@ -1509,13 +1512,49 @@ impl Node { continue; } if cfg.is_public() { - if let Some(addr) = handle.local_addr() - && !addr.ip().is_unspecified() - { + // Precedence: + // 1. operator-supplied `external_addr` (skips STUN) + // 2. non-wildcard `local_addr` (operator bound to + // a specific public IP directly) + // 3. STUN auto-discovery against ephemeral socket + // 4. loud warn + omit endpoint + if let Some(explicit) = cfg.external_advert_addr() { endpoints.push(OverlayEndpointAdvert { transport: OverlayTransportKind::Udp, - addr: addr.to_string(), + addr: explicit.to_string(), }); + } else { + match handle.local_addr() { + Some(addr) if !addr.ip().is_unspecified() => { + endpoints.push(OverlayEndpointAdvert { + transport: OverlayTransportKind::Udp, + addr: addr.to_string(), + }); + } + Some(addr) => { + let key = handle.transport_id().as_u32(); + let port = addr.port(); + if let Some(public) = + bootstrap.learn_public_udp_addr(key, port).await + { + endpoints.push(OverlayEndpointAdvert { + transport: OverlayTransportKind::Udp, + addr: public.to_string(), + }); + } else { + warn!( + transport_id = key, + bind_addr = %addr, + "advert: udp public=true bound to wildcard but \ + STUN observation failed; advertising no UDP \ + endpoint. Either set transports.udp.external_addr, \ + bind to a specific public IP, or ensure \ + node.discovery.nostr.stun_servers is reachable" + ); + } + } + None => {} + } } } else { endpoints.push(OverlayEndpointAdvert { @@ -1532,13 +1571,38 @@ impl Node { if !cfg.advertise_on_nostr() { continue; } - if let Some(addr) = handle.local_addr() - && !addr.ip().is_unspecified() - { + // Precedence: + // 1. operator-supplied `external_addr` (only path that + // works on cloud-NAT setups where the public IP is + // not on a host interface). + // 2. non-wildcard `local_addr` (operator bound to a + // specific public IP directly). + // 3. loud warn + omit endpoint (no TCP STUN equivalent). + if let Some(explicit) = cfg.external_advert_addr() { endpoints.push(OverlayEndpointAdvert { transport: OverlayTransportKind::Tcp, - addr: addr.to_string(), + addr: explicit.to_string(), }); + } else { + match handle.local_addr() { + Some(addr) if !addr.ip().is_unspecified() => { + endpoints.push(OverlayEndpointAdvert { + transport: OverlayTransportKind::Tcp, + addr: addr.to_string(), + }); + } + Some(addr) => { + warn!( + bind_addr = %addr, + "advert: tcp advertise_on_nostr=true bound to wildcard \ + and no transports.tcp.external_addr set; advertising no \ + TCP endpoint. Either set external_addr to the public \ + IP (recommended for cloud 1:1-NAT setups) or bind \ + explicitly to the public IP" + ); + } + None => {} + } } } "tor" => { @@ -1577,7 +1641,7 @@ impl Node { &self, bootstrap: &std::sync::Arc, ) -> Result<(), crate::discovery::nostr::BootstrapError> { - let advert = self.build_overlay_advert(); + let advert = self.build_overlay_advert(bootstrap).await; bootstrap.update_local_advert(advert).await }