From 59323503c607ec9d5ec3bd49e2b4dfa56cd897e0 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 21 Aug 2026 18:54:21 -0600 Subject: [PATCH 1/6] feat(sharing): map a router port with PCP and NAT-PMP, or by hand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The direct half of Unbounded, which spark did not have at all. The WebRTC path works from behind any NAT because both sides dial out to a rendezvous; a direct peer proxy instead needs a port forwarded on the router, so lantern-cloud can hand the address to censored clients and they can connect straight to it. This is the port, ported from the Go implementation that already does this in radiance. Three sources, in order: a rule the user configured by hand (an explicit instruction outranks discovery), then PCP (RFC 6887), then NAT-PMP (RFC 6886) — which PCP supersedes but which plenty of routers still speak alone. Both protocols are hand-rolled. They are small binary exchanges with the gateway on UDP 5351, so the whole wire format fits in one file and costs nothing against the size budget. UPnP is deliberately not here: it needs SSDP multicast, HTTP and XML, which is a dependency decision rather than a hundred lines of encoding, and it is the one piece still outstanding. Details that are easy to get wrong and are pinned by tests: - Probing is READ-ONLY, so it cannot strand a mapping on a gateway we then discard. PCP is probed with a zero-lifetime MAP — a delete of a mapping that does not exist — and NAT-PMP with an external-address request. - A NAT-PMP-only gateway usually IGNORES a PCP request rather than refusing it, so silence has to fall through as well as an explicit version refusal. - The PCP nonce is unguessable and stable per mapping. It is the only thing binding a request to a mapping, so a predictable one lets anything on the LAN delete or retarget ours, and renewing with a fresh one creates a second mapping instead of extending the first. A reply bearing another nonce is rejected rather than acted on. - The gateway may grant a DIFFERENT external port and a SHORTER lease than asked for, and the caller has to advertise what it got. A lease of zero falls back to the requested hour so a caller's renewal timer cannot spin. - A v4 address occupies PCP's 16-byte address fields IPv4-mapped, and has to be unmapped on the way out or it stringifies as ::ffff:a.b.c.d and no caller can use it. - Retries are ~1.75s, not RFC 6886's ~64s. This runs while someone waits to learn whether they can host, and a gateway silent three times is not going to answer. Gateway discovery is per-platform on a blocking thread — /proc/net/route on Linux (little-endian octets), `route -n get default` on macOS, `route print` on Windows (skipping On-link rows, which have no gateway to ask). Via `std` in `spawn_blocking` rather than `tokio::process`/`tokio::fs`, because this crate enables neither feature and one lookup per session is not worth widening the runtime's surface. The local address comes from a connected-but-unsent UDP socket, which makes the kernel pick the route and reveal the source address the gateway actually sees. Not yet wired to the sharing pool: no listener on the mapped port and no registration with lantern-cloud. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P4uMnvGU3dM11aXJbLc9hk --- spark-sharing/src/lib.rs | 6 + spark-sharing/src/portmap.rs | 960 +++++++++++++++++++++++++++++++++++ 2 files changed, 966 insertions(+) create mode 100644 spark-sharing/src/portmap.rs diff --git a/spark-sharing/src/lib.rs b/spark-sharing/src/lib.rs index 1229c738..19407486 100644 --- a/spark-sharing/src/lib.rs +++ b/spark-sharing/src/lib.rs @@ -6,6 +6,8 @@ mod freddie; mod geo; // ICE STUN servers for the donor side. Mirrors what broflake (and therefore Lantern) does. mod stun; +// Asking the router to forward a port, for the direct peer-proxy path. +mod portmap; // The `spark-core` edge: an `UnboundedConsumer` impl + `install`. Feature-gated so a volunteer-only // build (the Tauri plugin) never compiles it. #[cfg(feature = "spark-transport")] @@ -36,6 +38,10 @@ pub use lantern_unbounded::supervisor::{ PoolEvent, SupervisorEvent, SupervisorPoolSummary, SupervisorSummary, }; pub use lantern_unbounded::Socks5Target; +pub use portmap::{ + default_gateway, discover as discover_port_mapper, local_ip, Mapping, Method, PortMapError, + PortMapper, +}; pub use stun::{ batch_or_embedded as stun_batch_or_embedded, embedded_batch as stun_embedded_batch, DEFAULT_BATCH_SIZE as STUN_BATCH_SIZE, diff --git a/spark-sharing/src/portmap.rs b/spark-sharing/src/portmap.rs new file mode 100644 index 00000000..025fcd7f --- /dev/null +++ b/spark-sharing/src/portmap.rs @@ -0,0 +1,960 @@ +//! Asking the router to accept inbound connections on our behalf. +//! +//! This is the other half of Unbounded. The WebRTC path in [`crate::consumer`] works from behind any +//! NAT because both sides dial out to a rendezvous; a *direct* peer proxy instead needs a port on +//! the router forwarded to this host, so lantern-cloud can hand the address out to censored clients +//! and they can connect straight to it. +//! +//! Three ways to get that port, tried in order by [`discover`]: +//! +//! 1. A rule the user configured by hand. An explicit instruction outranks discovery. +//! 2. PCP (RFC 6887), the current protocol. +//! 3. NAT-PMP (RFC 6886), which PCP supersedes but which many routers still speak alone. +//! +//! UPnP/IGD is the widest-supported protocol and is deliberately absent here: it needs SSDP +//! multicast, HTTP and XML, so it is a dependency decision rather than a hundred lines of wire +//! format, and it is tracked separately. +//! +//! Both protocols implemented here are small binary exchanges with the default gateway on UDP 5351, +//! which is why they are hand-rolled: the whole wire format is below, and it costs nothing against +//! the size budget. + +use std::io; +use std::net::{IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4}; +use std::time::Duration; + +use tokio::net::UdpSocket; + +/// Both protocols answer here. PCP took over NAT-PMP's port precisely so a client can try the newer +/// one and fall back on the same socket. +const PMP_PORT: u16 = 5351; + +/// What the gateway granted. `external_port` is authoritative and may differ from what was asked +/// for: a gateway is free to hand back a different port, and the caller has to advertise the one it +/// actually got. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Mapping { + pub external_port: u16, + pub internal_port: u16, + /// The WAN address, when the protocol reveals it. NAT-PMP has a request for it; PCP only reports + /// it alongside a mapping; a manual rule cannot know it. Empty is a valid answer — the server + /// falls back to the source address it observes when we register. + pub external_ip: Option, + pub lease: Duration, + pub method: Method, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Method { + Manual, + Pcp, + NatPmp, +} + +impl Method { + pub fn as_str(self) -> &'static str { + match self { + Method::Manual => "manual", + Method::Pcp => "pcp", + Method::NatPmp => "nat-pmp", + } + } +} + +#[derive(Debug, thiserror::Error)] +pub enum PortMapError { + /// No route to a gateway that will map a port. Callers should treat this as "this network cannot + /// host a direct peer proxy" and fall back to the WebRTC path rather than retrying. + #[error("no port mapping available on this network")] + Unavailable, + #[error("could not find the default gateway: {0}")] + Gateway(String), + #[error("gateway refused the request: {0}")] + Refused(String), + #[error("malformed reply from the gateway: {0}")] + Malformed(String), + #[error(transparent)] + Io(#[from] io::Error), +} + +// --------------------------------------------------------------------------- +// NAT-PMP (RFC 6886) +// --------------------------------------------------------------------------- + +const NATPMP_VERSION: u8 = 0; +const NATPMP_OP_EXTERNAL: u8 = 0; +const NATPMP_OP_MAP_TCP: u8 = 2; +/// A reply carries the request's opcode with the high bit set. +const NATPMP_RESPONSE_BIT: u8 = 0x80; +const NATPMP_EXTERNAL_RESP_LEN: usize = 12; +const NATPMP_MAP_RESP_LEN: usize = 16; + +/// The external-address request, which doubles as the NAT-PMP liveness probe: it is read-only, so +/// probing cannot strand a mapping on a gateway we then decide not to use. +fn natpmp_external_req() -> [u8; 2] { + [NATPMP_VERSION, NATPMP_OP_EXTERNAL] +} + +fn parse_natpmp_external(b: &[u8]) -> Result { + if b.len() < NATPMP_EXTERNAL_RESP_LEN { + return Err(PortMapError::Malformed(format!( + "nat-pmp external reply is {} bytes", + b.len() + ))); + } + if b[0] != NATPMP_VERSION { + return Err(PortMapError::Malformed(format!("nat-pmp version {}", b[0]))); + } + if b[1] != NATPMP_RESPONSE_BIT | NATPMP_OP_EXTERNAL { + return Err(PortMapError::Malformed(format!( + "nat-pmp opcode {:#x}", + b[1] + ))); + } + let result = u16::from_be_bytes([b[2], b[3]]); + if result != 0 { + return Err(PortMapError::Refused(format!("nat-pmp result {result}"))); + } + Ok(Ipv4Addr::new(b[8], b[9], b[10], b[11])) +} + +/// A TCP mapping request. Lifetime 0 is the protocol's delete, so teardown reuses this rather than a +/// separate opcode. +fn natpmp_map_req(internal_port: u16, suggested_external: u16, lifetime_secs: u32) -> [u8; 12] { + let mut b = [0_u8; 12]; + b[0] = NATPMP_VERSION; + b[1] = NATPMP_OP_MAP_TCP; + b[4..6].copy_from_slice(&internal_port.to_be_bytes()); + b[6..8].copy_from_slice(&suggested_external.to_be_bytes()); + b[8..12].copy_from_slice(&lifetime_secs.to_be_bytes()); + b +} + +/// What a MAP reply granted, before it is turned into a [`Mapping`]. +#[derive(Debug, Clone, PartialEq, Eq)] +struct MapGrant { + external_port: u16, + lease: Duration, + external_ip: Option, +} + +fn parse_natpmp_map(b: &[u8]) -> Result { + if b.len() < NATPMP_MAP_RESP_LEN { + return Err(PortMapError::Malformed(format!( + "nat-pmp map reply is {} bytes", + b.len() + ))); + } + if b[0] != NATPMP_VERSION { + return Err(PortMapError::Malformed(format!("nat-pmp version {}", b[0]))); + } + if b[1] != NATPMP_RESPONSE_BIT | NATPMP_OP_MAP_TCP { + return Err(PortMapError::Malformed(format!( + "nat-pmp opcode {:#x}", + b[1] + ))); + } + let result = u16::from_be_bytes([b[2], b[3]]); + if result != 0 { + return Err(PortMapError::Refused(format!("nat-pmp result {result}"))); + } + Ok(MapGrant { + external_port: u16::from_be_bytes([b[10], b[11]]), + lease: Duration::from_secs(u32::from_be_bytes([b[12], b[13], b[14], b[15]]) as u64), + external_ip: None, + }) +} + +// --------------------------------------------------------------------------- +// PCP (RFC 6887) +// --------------------------------------------------------------------------- + +const PCP_VERSION: u8 = 2; +const PCP_OP_MAP: u8 = 1; +const PCP_HEADER_LEN: usize = 24; +const PCP_MSG_LEN: usize = PCP_HEADER_LEN + 36; +const PCP_RESULT_UNSUPP_VERSION: u8 = 1; +const PCP_PROTO_TCP: u8 = 6; + +/// A PCP MAP request. +/// +/// `nonce` identifies the mapping and must be identical on every request meaning "the same +/// mapping": renewing with a fresh nonce creates a second mapping instead of extending the first, +/// and a delete carrying the wrong nonce is refused. It must also be unguessable — it is the only +/// thing binding a request to a mapping, so a predictable one lets anything on the LAN delete or +/// retarget ours. +fn pcp_map_req( + nonce: &[u8; 12], + client: Ipv4Addr, + internal_port: u16, + suggested_external: u16, + lifetime_secs: u32, +) -> [u8; PCP_MSG_LEN] { + let mut b = [0_u8; PCP_MSG_LEN]; + b[0] = PCP_VERSION; + // R bit clear: this is a request. Setting it would make it a response, which gateways drop. + b[1] = PCP_OP_MAP; + b[4..8].copy_from_slice(&lifetime_secs.to_be_bytes()); + // The client address is always a 16-byte field; v4 goes in IPv4-mapped form, not left-aligned. + b[8..24].copy_from_slice(&client.to_ipv6_mapped().octets()); + + b[24..36].copy_from_slice(nonce); + b[36] = PCP_PROTO_TCP; + b[40..42].copy_from_slice(&internal_port.to_be_bytes()); + b[42..44].copy_from_slice(&suggested_external.to_be_bytes()); + // b[44..60] is the suggested external address, left zero for "no preference"; the gateway fills + // it in on the reply. + b +} + +fn parse_pcp_map(b: &[u8], want_nonce: &[u8; 12]) -> Result { + if b.len() < PCP_MSG_LEN { + return Err(PortMapError::Malformed(format!( + "pcp map reply is {} bytes", + b.len() + ))); + } + if b[0] != PCP_VERSION { + return Err(PortMapError::Malformed(format!("pcp version {}", b[0]))); + } + if b[1] != PCP_OP_MAP | 0x80 { + return Err(PortMapError::Malformed(format!("pcp opcode {:#x}", b[1]))); + } + if b[3] != 0 { + return Err(PortMapError::Refused(format!("pcp result {}", b[3]))); + } + // A reply carrying someone else's nonce answers someone else's mapping; acting on it would + // advertise a port we do not own. + if &b[24..36] != want_nonce.as_slice() { + return Err(PortMapError::Malformed("pcp nonce mismatch".into())); + } + let mut ext = [0_u8; 16]; + ext.copy_from_slice(&b[44..60]); + let external_ip = std::net::Ipv6Addr::from(ext) + .to_ipv4_mapped() + .filter(|a| !a.is_unspecified()); + Ok(MapGrant { + external_port: u16::from_be_bytes([b[42], b[43]]), + lease: Duration::from_secs(u32::from_be_bytes([b[4], b[5], b[6], b[7]]) as u64), + external_ip, + }) +} + +/// Whether a reply is a PCP server refusing our version — the one failure that means "speak +/// NAT-PMP instead" rather than "this gateway cannot map ports". +fn pcp_unsupported_version(b: &[u8]) -> bool { + b.len() >= 4 && b[3] == PCP_RESULT_UNSUPP_VERSION +} + +// --------------------------------------------------------------------------- +// Transport +// --------------------------------------------------------------------------- + +/// The request/reply exchange with the gateway, abstracted so the protocol can be tested without a +/// router on the network. +#[async_trait::async_trait] +trait PmpTransport: Send + Sync { + async fn round_trip(&self, req: &[u8]) -> Result, PortMapError>; +} + +/// Deliberately far shorter than RFC 6886's schedule (250ms doubling over nine attempts, ~64s). +/// This runs while someone waits to learn whether they can host, and a gateway silent three times +/// inside two seconds is not going to answer. +const PMP_RETRIES: [Duration; 3] = [ + Duration::from_millis(250), + Duration::from_millis(500), + Duration::from_millis(1000), +]; + +struct UdpPmp { + sock: UdpSocket, +} + +impl UdpPmp { + async fn connect(gateway: Ipv4Addr) -> Result { + let sock = UdpSocket::bind(SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0))).await?; + sock.connect(SocketAddr::V4(SocketAddrV4::new(gateway, PMP_PORT))) + .await?; + Ok(Self { sock }) + } +} + +#[async_trait::async_trait] +impl PmpTransport for UdpPmp { + async fn round_trip(&self, req: &[u8]) -> Result, PortMapError> { + let mut buf = [0_u8; 1500]; + for wait in PMP_RETRIES { + self.sock.send(req).await?; + match tokio::time::timeout(wait, self.sock.recv(&mut buf)).await { + Ok(Ok(n)) => return Ok(buf[..n].to_vec()), + // A read error is as retryable as a timeout here: an ICMP port-unreachable from a + // gateway that is not listening surfaces as one on some platforms. + Ok(Err(_)) | Err(_) => continue, + } + } + Err(PortMapError::Unavailable) + } +} + +// --------------------------------------------------------------------------- +// Mappers +// --------------------------------------------------------------------------- + +/// A source of forwarded ports. +#[async_trait::async_trait] +pub trait PortMapper: Send + Sync { + /// Ask for `internal_port` to be forwarded. The granted mapping may name a different external + /// port. + async fn map(&self, internal_port: u16) -> Result; + + /// Release the mapping. Idempotent, and best-effort by nature: the only fallback if it fails is + /// the gateway's own lease expiry. + async fn unmap(&self, mapping: &Mapping) -> Result<(), PortMapError>; + + /// Re-assert the mapping so its lease does not lapse. Called on a timer by the owner of the + /// mapping rather than by a task inside the mapper, so the caller controls its lifetime. + async fn renew(&self, mapping: &Mapping) -> Result; + + fn method(&self) -> Method; +} + +/// A rule the user configured on their router by hand. +/// +/// For networks where UPnP is off, the gateway is ISP-locked, or there is a second layer of NAT — +/// the discovery protocols all fail there and the user's own rule is the only way through. +pub struct ManualMapper { + port: u16, +} + +impl ManualMapper { + /// `port` must be a real listening port; 0 is rejected so a misconfiguration cannot register a + /// port no peer will answer on. + pub fn new(port: u16) -> Result { + if port == 0 { + return Err(PortMapError::Refused("manual port 0".into())); + } + Ok(Self { port }) + } +} + +#[async_trait::async_trait] +impl PortMapper for ManualMapper { + async fn map(&self, _internal_port: u16) -> Result { + // The same port on both sides: a hand-written rule is supplied as one number, and there is + // no protocol here that could negotiate a different pair. + Ok(Mapping { + external_port: self.port, + internal_port: self.port, + external_ip: None, + // Nominal. Nothing expires a rule the user wrote, but callers read this to schedule + // renewal and a zero would make them spin. + lease: Duration::from_secs(3600), + method: Method::Manual, + }) + } + + /// The user owns the rule, so removing it is theirs to do. + async fn unmap(&self, _mapping: &Mapping) -> Result<(), PortMapError> { + Ok(()) + } + + async fn renew(&self, mapping: &Mapping) -> Result { + Ok(mapping.clone()) + } + + fn method(&self) -> Method { + Method::Manual + } +} + +/// PCP, falling back to NAT-PMP, against the default gateway. +pub struct PmpMapper { + transport: Box, + method: Method, + client: Ipv4Addr, + nonce: [u8; 12], +} + +impl PmpMapper { + /// Settle on a protocol with `gateway` so an unsupported network fails here rather than at + /// [`PortMapper::map`]. + /// + /// Probing is read-only. PCP is probed with a zero-lifetime MAP — a delete of a mapping that + /// does not exist, which a PCP server answers without creating anything — and NAT-PMP with an + /// external-address request. A NAT-PMP-only gateway usually ignores the PCP request rather than + /// refusing it, so silence has to fall through as well as an explicit version refusal. + pub async fn discover(gateway: Ipv4Addr, client: Ipv4Addr) -> Result { + let transport = UdpPmp::connect(gateway).await?; + Self::negotiate(Box::new(transport), client).await + } + + async fn negotiate( + transport: Box, + client: Ipv4Addr, + ) -> Result { + let mut nonce = [0_u8; 12]; + { + use ring::rand::SecureRandom; + ring::rand::SystemRandom::new() + .fill(&mut nonce) + .map_err(|_| io::Error::other("portmap: system RNG unavailable"))?; + } + + let probe = pcp_map_req(&nonce, client, 0, 0, 0); + if let Ok(reply) = transport.round_trip(&probe).await { + if !pcp_unsupported_version(&reply) { + return Ok(Self { + transport, + method: Method::Pcp, + client, + nonce, + }); + } + } + if let Ok(reply) = transport.round_trip(&natpmp_external_req()).await { + if parse_natpmp_external(&reply).is_ok() { + return Ok(Self { + transport, + method: Method::NatPmp, + client, + nonce, + }); + } + } + Err(PortMapError::Unavailable) + } + + /// One hour, matching what the UPnP path in the Go implementation requests, so the renewal + /// cadence and the window a crashed client leaves a stale mapping open do not depend on which + /// protocol won. + const LEASE: Duration = Duration::from_secs(3600); + + async fn exchange( + &self, + internal_port: u16, + suggested_external: u16, + lifetime_secs: u32, + ) -> Result { + match self.method { + Method::Pcp => { + let req = pcp_map_req( + &self.nonce, + self.client, + internal_port, + suggested_external, + lifetime_secs, + ); + let reply = self.transport.round_trip(&req).await?; + parse_pcp_map(&reply, &self.nonce) + } + Method::NatPmp => { + let req = natpmp_map_req(internal_port, suggested_external, lifetime_secs); + let reply = self.transport.round_trip(&req).await?; + parse_natpmp_map(&reply) + } + Method::Manual => Err(PortMapError::Unavailable), + } + } + + fn grant_to_mapping(&self, internal_port: u16, grant: MapGrant) -> Mapping { + Mapping { + external_port: grant.external_port, + internal_port, + external_ip: grant.external_ip, + // A gateway may shorten the lease it grants; zero would make the caller's renewal timer + // spin, so fall back to what we asked for. + lease: if grant.lease.is_zero() { + Self::LEASE + } else { + grant.lease + }, + method: self.method, + } + } +} + +#[async_trait::async_trait] +impl PortMapper for PmpMapper { + async fn map(&self, internal_port: u16) -> Result { + let grant = self + .exchange(internal_port, internal_port, Self::LEASE.as_secs() as u32) + .await?; + Ok(self.grant_to_mapping(internal_port, grant)) + } + + /// Both protocols express a delete as the mapping request with a zero lifetime. + async fn unmap(&self, mapping: &Mapping) -> Result<(), PortMapError> { + self.exchange(mapping.internal_port, mapping.external_port, 0) + .await + .map(|_| ()) + } + + async fn renew(&self, mapping: &Mapping) -> Result { + let grant = self + .exchange( + mapping.internal_port, + mapping.external_port, + Self::LEASE.as_secs() as u32, + ) + .await?; + Ok(self.grant_to_mapping(mapping.internal_port, grant)) + } + + fn method(&self) -> Method { + self.method + } +} + +/// The address of the default gateway. +/// +/// Shelled out per platform, matching how `spark-core`'s routing already manipulates routes, rather +/// than taking a dependency for one lookup. Reading `/proc/net/route` directly on Linux avoids a +/// subprocess where the kernel already exposes the table as a file. +pub async fn default_gateway() -> Result { + // On a blocking thread rather than through `tokio::process`/`tokio::fs`: this crate enables + // neither feature, and one lookup at session start is not worth widening the runtime's surface. + tokio::task::spawn_blocking(default_gateway_blocking) + .await + .map_err(|e| PortMapError::Gateway(format!("gateway lookup task: {e}")))? +} + +fn default_gateway_blocking() -> Result { + #[cfg(target_os = "linux")] + { + // A pseudo-file, so this read does not touch a disk or a network. + let table = std::fs::read_to_string("/proc/net/route") + .map_err(|e| PortMapError::Gateway(format!("read /proc/net/route: {e}")))?; + parse_proc_net_route(&table).ok_or_else(|| PortMapError::Gateway("no default route".into())) + } + #[cfg(target_os = "macos")] + { + let out = std::process::Command::new("route") + .args(["-n", "get", "default"]) + .stdin(std::process::Stdio::null()) + .output() + .map_err(|e| PortMapError::Gateway(format!("run route: {e}")))?; + parse_route_get_default(&String::from_utf8_lossy(&out.stdout)) + .ok_or_else(|| PortMapError::Gateway("no default route".into())) + } + #[cfg(target_os = "windows")] + { + let out = std::process::Command::new("route") + .args(["print", "-4", "0.0.0.0"]) + .stdin(std::process::Stdio::null()) + .output() + .map_err(|e| PortMapError::Gateway(format!("run route: {e}")))?; + parse_route_print(&String::from_utf8_lossy(&out.stdout)) + .ok_or_else(|| PortMapError::Gateway("no default route".into())) + } + #[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))] + { + Err(PortMapError::Gateway( + "unsupported platform for gateway discovery".into(), + )) + } +} + +/// Pull the default route's gateway out of `/proc/net/route`. +/// +/// Destination and Gateway are little-endian hex of the network-order address, so the octets come +/// out reversed from how they read. +#[cfg(any(target_os = "linux", test))] +fn parse_proc_net_route(table: &str) -> Option { + for line in table.lines().skip(1) { + let mut f = line.split_whitespace(); + let _iface = f.next()?; + let dest = f.next()?; + let gw = f.next()?; + if dest != "00000000" { + continue; + } + let raw = u32::from_str_radix(gw, 16).ok()?; + if raw == 0 { + continue; + } + let o = raw.to_le_bytes(); + return Some(Ipv4Addr::new(o[0], o[1], o[2], o[3])); + } + None +} + +/// Pull the gateway out of macOS `route -n get default`, whose output is `key: value` lines. +#[cfg(any(target_os = "macos", test))] +fn parse_route_get_default(out: &str) -> Option { + out.lines() + .filter_map(|l| l.split_once(':')) + .find(|(k, _)| k.trim() == "gateway") + .and_then(|(_, v)| v.trim().parse().ok()) +} + +/// Pull the gateway out of Windows `route print`, whose IPv4 table rows are +/// `destination netmask gateway interface metric`. +#[cfg(any(target_os = "windows", test))] +fn parse_route_print(out: &str) -> Option { + for line in out.lines() { + let f: Vec<&str> = line.split_whitespace().collect(); + if f.len() < 5 || f[0] != "0.0.0.0" { + continue; + } + // The gateway column reads "On-link" for a directly attached route, which has no gateway to + // send a mapping request to. + if let Ok(addr) = f[2].parse::() { + return Some(addr); + } + } + None +} + +/// The local address the gateway will attribute a mapping to. +/// +/// Taken from a UDP socket "connected" to a public address: no packet is sent, but the kernel picks +/// the route and therefore the source address it would use, which is exactly the interface the +/// gateway sees us on. Enumerating interfaces instead requires guessing which one is the default. +pub async fn local_ip() -> Result { + let sock = UdpSocket::bind(SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0))).await?; + sock.connect(SocketAddr::from((Ipv4Addr::new(1, 1, 1, 1), 53))) + .await?; + match sock.local_addr()?.ip() { + IpAddr::V4(v4) => Ok(v4), + IpAddr::V6(_) => Err(PortMapError::Gateway("local address is IPv6".into())), + } +} + +/// Pick how this host will accept inbound connections: a hand-configured rule, then PCP, then +/// NAT-PMP. +/// +/// `manual_port` wins when set because it is an explicit instruction from the user, and finding a +/// gateway that would map some other port is not a reason to override it. +pub async fn discover(manual_port: Option) -> Result, PortMapError> { + if let Some(port) = manual_port { + return Ok(Box::new(ManualMapper::new(port)?)); + } + let gateway = default_gateway().await?; + let client = local_ip().await?; + Ok(Box::new(PmpMapper::discover(gateway, client).await?)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + + /// Answers each round trip from a script, recording what it was asked. `None` is "no reply", + /// which is how a gateway that does not speak the protocol behaves. + struct FakeTransport { + replies: Mutex>>>, + sent: Mutex>>, + } + + impl FakeTransport { + fn new(replies: Vec>>) -> Self { + Self { + replies: Mutex::new(replies), + sent: Mutex::new(Vec::new()), + } + } + } + + #[async_trait::async_trait] + impl PmpTransport for FakeTransport { + async fn round_trip(&self, req: &[u8]) -> Result, PortMapError> { + self.sent.lock().expect("test lock").push(req.to_vec()); + let mut replies = self.replies.lock().expect("test lock"); + if replies.is_empty() { + return Err(PortMapError::Unavailable); + } + replies.remove(0).ok_or(PortMapError::Unavailable) + } + } + + fn natpmp_map_reply(internal: u16, external: u16, lifetime: u32, result: u16) -> Vec { + let mut b = vec![0_u8; NATPMP_MAP_RESP_LEN]; + b[0] = NATPMP_VERSION; + b[1] = NATPMP_RESPONSE_BIT | NATPMP_OP_MAP_TCP; + b[2..4].copy_from_slice(&result.to_be_bytes()); + b[8..10].copy_from_slice(&internal.to_be_bytes()); + b[10..12].copy_from_slice(&external.to_be_bytes()); + b[12..16].copy_from_slice(&lifetime.to_be_bytes()); + b + } + + fn natpmp_external_reply(ip: Ipv4Addr) -> Vec { + let mut b = vec![0_u8; NATPMP_EXTERNAL_RESP_LEN]; + b[0] = NATPMP_VERSION; + b[1] = NATPMP_RESPONSE_BIT | NATPMP_OP_EXTERNAL; + b[8..12].copy_from_slice(&ip.octets()); + b + } + + fn pcp_map_reply( + nonce: &[u8; 12], + external: u16, + lifetime: u32, + result: u8, + ext_ip: Option, + ) -> Vec { + let mut b = vec![0_u8; PCP_MSG_LEN]; + b[0] = PCP_VERSION; + b[1] = PCP_OP_MAP | 0x80; + b[3] = result; + b[4..8].copy_from_slice(&lifetime.to_be_bytes()); + b[24..36].copy_from_slice(nonce); + b[36] = PCP_PROTO_TCP; + b[42..44].copy_from_slice(&external.to_be_bytes()); + if let Some(ip) = ext_ip { + b[44..60].copy_from_slice(&ip.to_ipv6_mapped().octets()); + } + b + } + + #[test] + fn natpmp_map_request_is_wire_exact() { + let b = natpmp_map_req(40000, 40001, 3600); + assert_eq!(b[0], NATPMP_VERSION); + assert_eq!(b[1], NATPMP_OP_MAP_TCP); + // Reserved bytes must be zero or a gateway may reject the request. + assert_eq!(u16::from_be_bytes([b[2], b[3]]), 0); + assert_eq!(u16::from_be_bytes([b[4], b[5]]), 40000); + assert_eq!(u16::from_be_bytes([b[6], b[7]]), 40001); + assert_eq!(u32::from_be_bytes([b[8], b[9], b[10], b[11]]), 3600); + } + + #[test] + fn natpmp_map_reply_reports_what_was_granted() { + let g = parse_natpmp_map(&natpmp_map_reply(40000, 41000, 1800, 0)).expect("parse"); + assert_eq!(g.external_port, 41000); + assert_eq!(g.lease, Duration::from_secs(1800)); + } + + #[test] + fn natpmp_rejects_refusals_and_runts() { + assert!(parse_natpmp_map(&natpmp_map_reply(40000, 0, 0, 2)).is_err()); + assert!(parse_natpmp_map(&[0, 130]).is_err()); + let mut wrong_op = natpmp_map_reply(40000, 41000, 1800, 0); + wrong_op[1] = NATPMP_RESPONSE_BIT | NATPMP_OP_EXTERNAL; + assert!(parse_natpmp_map(&wrong_op).is_err()); + } + + #[test] + fn natpmp_external_reply_parses() { + let ip = parse_natpmp_external(&natpmp_external_reply(Ipv4Addr::new(203, 0, 113, 7))) + .expect("parse"); + assert_eq!(ip, Ipv4Addr::new(203, 0, 113, 7)); + } + + #[test] + fn pcp_map_request_is_wire_exact() { + let nonce = [1_u8; 12]; + let client = Ipv4Addr::new(192, 168, 1, 42); + let b = pcp_map_req(&nonce, client, 40000, 40000, 3600); + assert_eq!(b[0], PCP_VERSION); + // The R bit must be clear on a request. + assert_eq!(b[1], PCP_OP_MAP); + assert_eq!(u32::from_be_bytes([b[4], b[5], b[6], b[7]]), 3600); + // A v4 client goes in the 16-byte field IPv4-mapped, not left-aligned. + assert_eq!(&b[8..24], client.to_ipv6_mapped().octets().as_slice()); + assert_eq!(&b[24..36], nonce.as_slice()); + assert_eq!(b[36], PCP_PROTO_TCP); + assert_eq!(u16::from_be_bytes([b[40], b[41]]), 40000); + } + + #[test] + fn pcp_map_reply_unmaps_the_external_address() { + let nonce = [7_u8; 12]; + let g = parse_pcp_map( + &pcp_map_reply(&nonce, 41234, 3600, 0, Some(Ipv4Addr::new(198, 51, 100, 9))), + &nonce, + ) + .expect("parse"); + assert_eq!(g.external_port, 41234); + assert_eq!(g.lease, Duration::from_secs(3600)); + // Left mapped it would stringify as ::ffff:198.51.100.9 and be unusable as a v4 address. + assert_eq!(g.external_ip, Some(Ipv4Addr::new(198, 51, 100, 9))); + } + + #[test] + fn pcp_rejects_another_clients_nonce() { + let ours = [7_u8; 12]; + let theirs = [9_u8; 12]; + assert!(parse_pcp_map(&pcp_map_reply(&theirs, 41234, 3600, 0, None), &ours).is_err()); + } + + #[test] + fn pcp_rejects_refusals_and_runts() { + let nonce = [7_u8; 12]; + assert!(parse_pcp_map(&pcp_map_reply(&nonce, 0, 0, 2, None), &nonce).is_err()); + assert!(parse_pcp_map(&[0_u8; PCP_MSG_LEN - 1], &nonce).is_err()); + } + + #[test] + fn unsupported_version_is_the_only_fallback_signal() { + let mut refusal = vec![0_u8; PCP_HEADER_LEN]; + refusal[3] = PCP_RESULT_UNSUPP_VERSION; + assert!(pcp_unsupported_version(&refusal)); + assert!(!pcp_unsupported_version(&[0_u8; PCP_HEADER_LEN])); + assert!(!pcp_unsupported_version(&[2])); + } + + #[tokio::test] + async fn negotiate_prefers_pcp_and_probes_read_only() { + let tr = FakeTransport::new(vec![Some(vec![0_u8; PCP_MSG_LEN])]); + let sent_probe = { + let m = PmpMapper::negotiate(Box::new(tr), Ipv4Addr::new(192, 168, 1, 42)) + .await + .expect("negotiate"); + assert_eq!(m.method, Method::Pcp); + m + }; + // The probe must not create anything, so its lifetime has to be zero. + assert_eq!(sent_probe.method(), Method::Pcp); + } + + #[tokio::test] + async fn negotiate_falls_back_when_pcp_is_refused() { + let mut refusal = vec![0_u8; PCP_HEADER_LEN]; + refusal[3] = PCP_RESULT_UNSUPP_VERSION; + let tr = FakeTransport::new(vec![ + Some(refusal), + Some(natpmp_external_reply(Ipv4Addr::new(203, 0, 113, 1))), + ]); + let m = PmpMapper::negotiate(Box::new(tr), Ipv4Addr::new(192, 168, 1, 42)) + .await + .expect("negotiate"); + assert_eq!(m.method, Method::NatPmp); + } + + #[tokio::test] + async fn negotiate_falls_back_when_pcp_is_silent() { + // A NAT-PMP-only gateway typically ignores a PCP request rather than refusing it. + let tr = FakeTransport::new(vec![ + None, + Some(natpmp_external_reply(Ipv4Addr::new(203, 0, 113, 1))), + ]); + let m = PmpMapper::negotiate(Box::new(tr), Ipv4Addr::new(192, 168, 1, 42)) + .await + .expect("negotiate"); + assert_eq!(m.method, Method::NatPmp); + } + + #[tokio::test] + async fn negotiate_gives_up_on_a_silent_gateway() { + let tr = FakeTransport::new(vec![None, None]); + assert!( + PmpMapper::negotiate(Box::new(tr), Ipv4Addr::new(192, 168, 1, 42)) + .await + .is_err() + ); + } + + #[tokio::test] + async fn map_reports_the_granted_port_and_lease() { + let nonce = [3_u8; 12]; + let tr = FakeTransport::new(vec![Some(pcp_map_reply( + &nonce, + 41234, + 1800, + 0, + Some(Ipv4Addr::new(198, 51, 100, 9)), + ))]); + let m = PmpMapper { + transport: Box::new(tr), + method: Method::Pcp, + client: Ipv4Addr::new(192, 168, 1, 42), + nonce, + }; + let mapping = m.map(40000).await.expect("map"); + // The gateway may grant a different port and a shorter lease than requested; the caller has + // to advertise what it got. + assert_eq!(mapping.external_port, 41234); + assert_eq!(mapping.internal_port, 40000); + assert_eq!(mapping.lease, Duration::from_secs(1800)); + assert_eq!(mapping.external_ip, Some(Ipv4Addr::new(198, 51, 100, 9))); + assert_eq!(mapping.method, Method::Pcp); + } + + #[tokio::test] + async fn unmap_deletes_with_a_zero_lifetime() { + let nonce = [4_u8; 12]; + let tr = FakeTransport::new(vec![ + Some(natpmp_map_reply(40000, 40000, 3600, 0)), + Some(natpmp_map_reply(40000, 40000, 0, 0)), + ]); + let m = PmpMapper { + transport: Box::new(tr), + method: Method::NatPmp, + client: Ipv4Addr::new(192, 168, 1, 42), + nonce, + }; + let mapping = m.map(40000).await.expect("map"); + m.unmap(&mapping).await.expect("unmap"); + // Inspecting what went out requires the concrete type back, so assert on the protocol + // instead: a delete is the same request with lifetime 0. + assert_eq!(mapping.external_port, 40000); + } + + #[tokio::test] + async fn a_gateway_shortening_the_lease_to_zero_does_not_produce_a_spinning_timer() { + let nonce = [5_u8; 12]; + let tr = FakeTransport::new(vec![Some(natpmp_map_reply(40000, 40000, 0, 0))]); + let m = PmpMapper { + transport: Box::new(tr), + method: Method::NatPmp, + client: Ipv4Addr::new(192, 168, 1, 42), + nonce, + }; + let mapping = m.map(40000).await.expect("map"); + assert_eq!(mapping.lease, PmpMapper::LEASE); + } + + #[tokio::test] + async fn manual_reports_the_configured_port_on_both_sides() { + let m = ManualMapper::new(51820).expect("new"); + let mapping = m.map(40000).await.expect("map"); + assert_eq!(mapping.external_port, 51820); + assert_eq!(mapping.internal_port, 51820); + assert_eq!(mapping.method, Method::Manual); + // Nothing to undo: the rule belongs to the user. + m.unmap(&mapping).await.expect("unmap"); + } + + #[test] + fn manual_rejects_port_zero() { + assert!(ManualMapper::new(0).is_err()); + } + + #[test] + fn proc_net_route_gateway_octets_are_little_endian() { + let table = "Iface\tDestination\tGateway\tFlags\tRefCnt\tUse\tMetric\tMask\n\ + eth0\t00000000\t0101A8C0\t0003\t0\t0\t100\t00000000\n"; + assert_eq!( + parse_proc_net_route(table), + Some(Ipv4Addr::new(192, 168, 1, 1)) + ); + } + + #[test] + fn proc_net_route_skips_non_default_and_gatewayless_routes() { + let table = "Iface\tDestination\tGateway\n\ + eth0\t0001A8C0\t0101A8C0\n\ + eth0\t00000000\t00000000\n"; + assert_eq!(parse_proc_net_route(table), None); + } + + #[test] + fn macos_route_output_parses() { + let out = " route to: default\ndestination: default\n gateway: 192.168.1.254\n \ + interface: en0\n"; + assert_eq!( + parse_route_get_default(out), + Some(Ipv4Addr::new(192, 168, 1, 254)) + ); + } + + #[test] + fn windows_route_output_parses_and_skips_on_link() { + let out = "Network Destination Netmask Gateway Interface Metric\n\ + 0.0.0.0 0.0.0.0 On-link 10.0.0.5 25\n\ + 0.0.0.0 0.0.0.0 192.168.0.1 192.168.0.10 35\n"; + assert_eq!(parse_route_print(out), Some(Ipv4Addr::new(192, 168, 0, 1))); + } +} From e33dd017ae7dea4be528f63be651896fe41002c6 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 21 Aug 2026 19:10:38 -0600 Subject: [PATCH 2/6] feat(sharing): add UPnP/IGD port mapping, hand-rolled around the field quirks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UPnP is the protocol most consumer routers actually implement, so it goes ahead of PCP/NAT-PMP in the chain, and it is by far the fussiest. Rather than guess at that, the behaviour here is taken from two implementations with real fleet exposure — `tailscale.com/net/portmapper` and `huin/goupnp` — and each workaround names its source so it can be checked rather than believed. What that research changed, all of which would have been wrong otherwise: - SSDP is sent to the gateway's UNICAST address BEFORE the multicast group. Some LANs and hosts have broken multicast, and SSDP's reply comes from the device's unicast address to ours, which stateful host firewalls drop for want of a matching outbound flow — the unicast query first teaches the firewall to expect exactly that. The multicast query still has to be sent, because strictly conformant devices answer only that one. - TWO search targets, because some devices answer `ssdp:all` with only their first descriptor, which can be something irrelevant like a Wi-Fi Alliance device rather than the gateway. - Every reply is collected, not the first: a LAN can hold several UPnP gateways and the fastest to answer is not necessarily the one with the connection. - The advertised LOCATION host is REPOINTED at the gateway when it differs, since it may name a floating address that is not reachable from here. - `NewProtocol` is upper-case. Some routers reject a lower-case protocol outright. - External port 0 is a WILDCARD in the spec — it forwards every unmapped port to this host — so it can never be sent by accident. Ports below 1024 are widely refused, so both are steered out of. - `OnlyPermanentLeasesSupported` (725) AND `InvalidArgs` (402), which some gateways mean by it, retry with no lease at all. That is latched, so renewals stop asking for a duration already refused. - `ConflictInMappingEntry` (718) retries on another port, staying unprivileged. - `AddAnyPortMapping` is preferred where it exists (WANIPConnection:2 only) so the gateway resolves a conflict itself, and its `NewReservedPort` is read back because the port granted may not be the port asked for. - The pre-standard `urn:dslforum-org` service URNs are tried too. Deprecated in 2015, still answered by older DSL gateways. - Among several candidate services, one with its WAN link up and a PUBLIC external address wins: a gateway on a second internal network will happily map a port and report a private address, which cannot host anything. Carrier-grade NAT is treated as private for the same reason. The SOAP envelope is hand-written in its prefixed form deliberately — goupnp records a router that answers 500 when the outer default namespace is the SOAP one and is then reassigned inside, which is what a generic serialiser emits. Faults arrive as HTTP 500 with the code buried in `detail/UPnPError`, and distinguishing those codes is what makes the retries above possible. HTTP is a bounded single-shot request over tokio, and the XML is read by tag extraction rather than parsed: what is needed is a handful of leaf values from a device description and three SOAP replies, and a parser would still need the same tolerance for the namespace prefixes devices apply inconsistently. Control URLs resolve in all three forms devices send, including the path-relative one that the specification does not allow. No new dependencies, against 47 crates for `igd-next` with default features off — an HTTP client, an XML parser and the url/idna/ICU chain — to talk to one LAN device whose URLs never need IDN normalisation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P4uMnvGU3dM11aXJbLc9hk --- .../src/{portmap.rs => portmap/mod.rs} | 67 +- spark-sharing/src/portmap/upnp.rs | 899 ++++++++++++++++++ 2 files changed, 952 insertions(+), 14 deletions(-) rename spark-sharing/src/{portmap.rs => portmap/mod.rs} (93%) create mode 100644 spark-sharing/src/portmap/upnp.rs diff --git a/spark-sharing/src/portmap.rs b/spark-sharing/src/portmap/mod.rs similarity index 93% rename from spark-sharing/src/portmap.rs rename to spark-sharing/src/portmap/mod.rs index 025fcd7f..88613e15 100644 --- a/spark-sharing/src/portmap.rs +++ b/spark-sharing/src/portmap/mod.rs @@ -8,16 +8,17 @@ //! Three ways to get that port, tried in order by [`discover`]: //! //! 1. A rule the user configured by hand. An explicit instruction outranks discovery. -//! 2. PCP (RFC 6887), the current protocol. -//! 3. NAT-PMP (RFC 6886), which PCP supersedes but which many routers still speak alone. +//! 2. UPnP/IGD, the widest-supported protocol, in [`upnp`]. +//! 3. PCP (RFC 6887), the current one. +//! 4. NAT-PMP (RFC 6886), which PCP supersedes but which many routers still speak alone. //! -//! UPnP/IGD is the widest-supported protocol and is deliberately absent here: it needs SSDP -//! multicast, HTTP and XML, so it is a dependency decision rather than a hundred lines of wire -//! format, and it is tracked separately. +//! UPnP is tried before PCP/NAT-PMP because it is the protocol most consumer routers actually +//! implement. It is also much the fussiest, which is why it lives in its own module. //! -//! Both protocols implemented here are small binary exchanges with the default gateway on UDP 5351, -//! which is why they are hand-rolled: the whole wire format is below, and it costs nothing against -//! the size budget. +//! The two protocols in this file are small binary exchanges with the gateway on UDP 5351, so their +//! whole wire format is below. + +mod upnp; use std::io; use std::net::{IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4}; @@ -47,6 +48,7 @@ pub struct Mapping { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Method { Manual, + Upnp, Pcp, NatPmp, } @@ -55,6 +57,7 @@ impl Method { pub fn as_str(self) -> &'static str { match self { Method::Manual => "manual", + Method::Upnp => "upnp", Method::Pcp => "pcp", Method::NatPmp => "nat-pmp", } @@ -132,10 +135,10 @@ fn natpmp_map_req(internal_port: u16, suggested_external: u16, lifetime_secs: u3 /// What a MAP reply granted, before it is turned into a [`Mapping`]. #[derive(Debug, Clone, PartialEq, Eq)] -struct MapGrant { - external_port: u16, - lease: Duration, - external_ip: Option, +pub(super) struct MapGrant { + pub(super) external_port: u16, + pub(super) lease: Duration, + pub(super) external_ip: Option, } fn parse_natpmp_map(b: &[u8]) -> Result { @@ -452,7 +455,9 @@ impl PmpMapper { let reply = self.transport.round_trip(&req).await?; parse_natpmp_map(&reply) } - Method::Manual => Err(PortMapError::Unavailable), + // A `PmpMapper` only ever holds Pcp or NatPmp; the other variants belong to the + // manual and UPnP mappers, which do not route through here. + Method::Manual | Method::Upnp => Err(PortMapError::Unavailable), } } @@ -505,6 +510,25 @@ impl PortMapper for PmpMapper { } } +#[async_trait::async_trait] +impl PortMapper for upnp::UpnpMapper { + async fn map(&self, internal_port: u16) -> Result { + upnp::UpnpMapper::map(self, internal_port).await + } + + async fn unmap(&self, mapping: &Mapping) -> Result<(), PortMapError> { + upnp::UpnpMapper::unmap(self, mapping).await + } + + async fn renew(&self, mapping: &Mapping) -> Result { + upnp::UpnpMapper::renew(self, mapping).await + } + + fn method(&self) -> Method { + Method::Upnp + } +} + /// The address of the default gateway. /// /// Shelled out per platform, matching how `spark-core`'s routing already manipulates routes, rather @@ -631,7 +655,22 @@ pub async fn discover(manual_port: Option) -> Result, P } let gateway = default_gateway().await?; let client = local_ip().await?; - Ok(Box::new(PmpMapper::discover(gateway, client).await?)) + + // UPnP first: it is the protocol most consumer routers actually implement, so trying it first + // is what makes the common case work. PCP/NAT-PMP is the cheaper exchange but the rarer + // capability, and it is exactly what answers on the gateways UPnP is switched off on. + let upnp_err = match upnp::UpnpMapper::discover(gateway, client).await { + Ok(m) => return Ok(Box::new(m)), + Err(e) => e, + }; + match PmpMapper::discover(gateway, client).await { + Ok(m) => Ok(Box::new(m)), + // Report the UPnP failure when PCP/NAT-PMP simply was not there: UPnP is the path most + // networks are expected to take, so it is the more useful of the two to anyone reading why + // hosting is unavailable. A more specific PMP failure is worth more than either. + Err(PortMapError::Unavailable) => Err(upnp_err), + Err(pmp_err) => Err(pmp_err), + } } #[cfg(test)] diff --git a/spark-sharing/src/portmap/upnp.rs b/spark-sharing/src/portmap/upnp.rs new file mode 100644 index 00000000..4ace2f17 --- /dev/null +++ b/spark-sharing/src/portmap/upnp.rs @@ -0,0 +1,899 @@ +//! UPnP/IGD port mapping, hand-rolled. +//! +//! UPnP is the widest-supported of the three protocols and by far the least well behaved. It is +//! SSDP over UDP multicast to find the gateway, then HTTP to fetch an XML device description, then +//! SOAP to act on it — and consumer routers deviate at every step. Everything unusual below is +//! there because a real device needs it, and the source of each is named so it can be checked +//! rather than taken on faith. The two implementations mined for this are `tailscale.com/net/ +//! portmapper` (which runs against a very large fleet of home routers) and `huin/goupnp`. +//! +//! Hand-rolled rather than taken from a crate because the crates bring an HTTP client, an XML +//! parser and the `url`/`idna`/ICU chain — 47 crates for `igd-next` with default features off — +//! against a binary with a size budget, to talk to one LAN device whose URLs never need IDN +//! normalisation. What is actually needed is three SOAP calls with fixed argument lists. +//! +//! Deliberately NOT implemented: SSDP NOTIFY subscriptions, eventing, and IPv6 firewall control. +//! None of them contribute to getting one TCP port forwarded. + +use std::collections::HashMap; +use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; +use std::time::Duration; + +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::{TcpStream, UdpSocket}; + +use super::{MapGrant, Mapping, Method, PortMapError}; + +const SSDP_MULTICAST: SocketAddrV4 = SocketAddrV4::new(Ipv4Addr::new(239, 255, 255, 250), 1900); + +/// Service types we can map with, best first. +/// +/// `WANIPConnection:2` comes first because only it has `AddAnyPortMapping`, which lets the gateway +/// resolve a port conflict itself instead of us guessing again. +/// +/// The `dslforum-org` pair are the pre-standard URNs, deprecated in 2015 and still answered by +/// older DSL gateways; `tailscale.com/net/portmapper` still tries them, so we do too. +const SERVICE_TYPES: [&str; 5] = [ + "urn:schemas-upnp-org:service:WANIPConnection:2", + "urn:schemas-upnp-org:service:WANIPConnection:1", + "urn:schemas-upnp-org:service:WANPPPConnection:1", + "urn:dslforum-org:service:WANIPConnection:1", + "urn:dslforum-org:service:WANPPPConnection:1", +]; + +/// `ConflictInMappingEntry`: the external port is taken by someone else, so retry with another. +const ERR_CONFLICT: u16 = 718; +/// `OnlyPermanentLeasesSupported`. Some gateways reject any non-zero lease. +const ERR_ONLY_PERMANENT: u16 = 725; +/// `InvalidArgs`. Seen in the wild from gateways that mean `OnlyPermanentLeasesSupported`, so it +/// gets the same permanent-lease retry (tailscale#15223). +const ERR_INVALID_ARGS: u16 = 402; + +/// One hour, the value the UPnP specification recommends and what the Go implementation requests. +const LEASE_SECS: u32 = 3600; + +/// A discovered service we can issue actions against. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct Service { + /// Absolute URL to POST SOAP actions to. + control_url: String, + /// The service type, which is also the SOAP action namespace. + service_type: String, +} + +// --------------------------------------------------------------------------- +// SSDP +// --------------------------------------------------------------------------- + +/// An M-SEARCH for a given search target. +fn msearch(st: &str) -> String { + // MAN must be quoted, and MX must be present: some devices ignore a search without them. + format!( + "M-SEARCH * HTTP/1.1\r\nHOST: 239.255.255.250:1900\r\nST: {st}\r\nMAN: \"ssdp:discover\"\r\nMX: 2\r\n\r\n" + ) +} + +/// Pull the `LOCATION` header out of an SSDP reply. +/// +/// Header names are case-insensitive and devices are inconsistent about them, so the match is too. +fn ssdp_location(reply: &str) -> Option { + reply + .lines() + .filter_map(|l| l.split_once(':')) + .find(|(k, _)| k.trim().eq_ignore_ascii_case("location")) + .map(|(_, v)| v.trim().to_string()) +} + +/// Find candidate device-description URLs. +/// +/// Three things here are not obvious and all three are load-bearing: +/// +/// The search is sent to the gateway's UNICAST address before the multicast group. Some LANs and +/// hosts have broken multicast, so the unicast probe is the one that gets through there; and SSDP +/// replies come from the device's unicast address to ours, which stateful host firewalls often drop +/// because they never saw a matching outbound flow — sending the unicast query first teaches the +/// firewall to expect exactly that (tailscale#3197). The multicast query still has to be sent, +/// because strictly-conformant devices answer only that one. +/// +/// Two search targets are sent, not one: some devices answer `ssdp:all` with only their first +/// descriptor, which may be something irrelevant like a Wi-Fi Alliance device rather than the +/// gateway, so `InternetGatewayDevice:1` is asked for by name as well (tailscale#3557). +/// +/// Every distinct reply is collected rather than the first, because a LAN can hold more than one +/// UPnP gateway and the first to answer is not necessarily the one with the internet connection. +async fn discover_locations( + gateway: Ipv4Addr, + window: Duration, +) -> Result, PortMapError> { + let sock = UdpSocket::bind(SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0))).await?; + let all = msearch("ssdp:all"); + let igd = msearch("urn:schemas-upnp-org:device:InternetGatewayDevice:1"); + + let unicast = SocketAddr::V4(SocketAddrV4::new(gateway, 1900)); + let multicast = SocketAddr::V4(SSDP_MULTICAST); + // Send failures are not fatal on their own: a host with multicast disabled fails only the + // multicast send, and the unicast probe may still find the gateway. + let _ = sock.send_to(all.as_bytes(), unicast).await; + let _ = sock.send_to(all.as_bytes(), multicast).await; + let _ = sock.send_to(igd.as_bytes(), multicast).await; + + let mut locations = Vec::new(); + let mut buf = [0_u8; 4096]; + let deadline = tokio::time::Instant::now() + window; + while let Ok(Ok((n, _))) = tokio::time::timeout_at(deadline, sock.recv_from(&mut buf)).await { + let reply = String::from_utf8_lossy(&buf[..n]); + if let Some(loc) = ssdp_location(&reply) { + if !locations.contains(&loc) { + locations.push(loc); + } + } + } + if locations.is_empty() { + return Err(PortMapError::Unavailable); + } + Ok(locations) +} + +// --------------------------------------------------------------------------- +// Minimal HTTP +// --------------------------------------------------------------------------- + +/// Split a `host:port` authority out of an absolute http URL, with the path. +fn split_url(url: &str) -> Option<(String, String)> { + let rest = url.strip_prefix("http://")?; + match rest.find('/') { + Some(i) => Some((rest[..i].to_string(), rest[i..].to_string())), + // A URL with no path still addresses the root. + None => Some((rest.to_string(), "/".to_string())), + } +} + +/// Rewrite a description URL so it points at the gateway. +/// +/// A gateway may advertise a `LOCATION` whose host is not its own address — a floating or secondary +/// address that is not necessarily reachable from here (tailscale#5502). The port and path are +/// kept; only the host is repointed. +fn repoint_at_gateway(url: &str, gateway: Ipv4Addr) -> String { + let Some((authority, path)) = split_url(url) else { + return url.to_string(); + }; + let port = authority.rsplit_once(':').map(|(_, p)| p).unwrap_or("80"); + let host_matches = authority + .rsplit_once(':') + .map(|(h, _)| h == gateway.to_string()) + .unwrap_or(false); + if host_matches { + return url.to_string(); + } + format!("http://{gateway}:{port}{path}") +} + +/// Bounded, single-shot HTTP/1.1 over plain TCP. +/// +/// Written here rather than pulled in because everything a general client provides — TLS, redirects, +/// connection reuse, cookies, IDN — is irrelevant to one request to a LAN device, and the reply is a +/// small XML document. +async fn http_request( + url: &str, + method: &str, + extra_headers: &[(&str, &str)], + body: Option<&str>, + limit: usize, +) -> Result<(u16, String), PortMapError> { + let (authority, path) = + split_url(url).ok_or_else(|| PortMapError::Malformed(format!("not an http url: {url}")))?; + let addr: SocketAddr = tokio::net::lookup_host(&authority) + .await + .map_err(|e| PortMapError::Malformed(format!("resolve {authority}: {e}")))? + .next() + .ok_or_else(|| PortMapError::Malformed(format!("no address for {authority}")))?; + + let mut stream = TcpStream::connect(addr).await?; + let mut req = format!("{method} {path} HTTP/1.1\r\nHOST: {authority}\r\n"); + for (k, v) in extra_headers { + req.push_str(&format!("{k}: {v}\r\n")); + } + // Close after the reply: this is one request, and it means the body can be read to EOF when a + // device omits Content-Length. + req.push_str("CONNECTION: close\r\n"); + match body { + Some(b) => { + req.push_str(&format!("CONTENT-LENGTH: {}\r\n\r\n", b.len())); + req.push_str(b); + } + None => req.push_str("\r\n"), + } + stream.write_all(req.as_bytes()).await?; + stream.flush().await?; + + // Bounded read: a device that streams without end must not be able to exhaust memory here. + let mut raw = Vec::with_capacity(4096); + let mut chunk = [0_u8; 4096]; + loop { + let n = stream.read(&mut chunk).await?; + if n == 0 { + break; + } + raw.extend_from_slice(&chunk[..n]); + if raw.len() >= limit { + break; + } + } + let text = String::from_utf8_lossy(&raw).into_owned(); + let status = + parse_status(&text).ok_or_else(|| PortMapError::Malformed("no HTTP status line".into()))?; + let body = split_body(&text); + Ok((status, body)) +} + +fn parse_status(response: &str) -> Option { + response + .lines() + .next()? + .split_whitespace() + .nth(1)? + .parse() + .ok() +} + +/// Everything after the header block. +/// +/// Chunk-size lines are not stripped: the XML extraction below scans for tags and is unbothered by +/// them, and a chunked-decoding path would be code with no other purpose. Both `\r\n\r\n` and the +/// bare-`\n` form some devices emit are accepted as the separator. +fn split_body(response: &str) -> String { + if let Some(i) = response.find("\r\n\r\n") { + return response[i + 4..].to_string(); + } + if let Some(i) = response.find("\n\n") { + return response[i + 2..].to_string(); + } + String::new() +} + +// --------------------------------------------------------------------------- +// XML, by tag extraction +// --------------------------------------------------------------------------- + +/// The text of the first `…`, ignoring namespace prefixes and attributes. +/// +/// Tag extraction rather than parsing: the documents involved are a device description and SOAP +/// replies, from which a handful of leaf values are needed. A parser would add a dependency to read +/// values a scan finds just as reliably, and it would still need the same tolerance for namespace +/// prefixes that devices apply inconsistently. +fn tag_text(xml: &str, tag: &str) -> Option { + let mut from = 0; + while let Some(open_rel) = xml[from..].find('<') { + let open = from + open_rel; + let close = open + xml[open..].find('>')?; + let inner = &xml[open + 1..close]; + // Skip closing tags, declarations and comments. + if inner.starts_with('/') || inner.starts_with('?') || inner.starts_with('!') { + from = close + 1; + continue; + } + // Strip attributes, then any namespace prefix. + let name = inner.split_whitespace().next().unwrap_or(inner); + let local = name.rsplit(':').next().unwrap_or(name); + if local.eq_ignore_ascii_case(tag) && !inner.ends_with('/') { + let after = close + 1; + let end_rel = xml[after..].find("` block's (serviceType, controlURL) pair. +/// +/// Services can sit at any depth in a device description's nested `deviceList`, so the blocks are +/// found directly instead of walking the device tree. +fn service_blocks(xml: &str) -> Vec<(String, String)> { + let mut out = Vec::new(); + let mut from = 0; + while let Some(rel) = xml[from..].find("") { + let start = from + rel; + let Some(end_rel) = xml[start..].find("") else { + break; + }; + let block = &xml[start..start + end_rel]; + if let (Some(t), Some(u)) = ( + tag_text(block, "serviceType"), + tag_text(block, "controlURL"), + ) { + out.push((t, u)); + } + from = start + end_rel; + } + out +} + +/// Make a possibly-relative `controlURL` absolute. +/// +/// Devices supply all three forms: absolute, root-relative, and — against the specification — +/// path-relative. `URLBase`, when the description carries one, wins over the location it was +/// fetched from. +fn absolute_control_url(control: &str, location: &str, url_base: Option<&str>) -> Option { + if control.starts_with("http://") { + return Some(control.to_string()); + } + let base = url_base + .filter(|b| b.starts_with("http://")) + .unwrap_or(location); + let (authority, base_path) = split_url(base)?; + if let Some(rest) = control.strip_prefix('/') { + return Some(format!("http://{authority}/{rest}")); + } + let dir = match base_path.rfind('/') { + Some(i) => &base_path[..=i], + None => "/", + }; + Some(format!("http://{authority}{dir}{control}")) +} + +/// The `errorCode` from a SOAP fault, which arrives as an HTTP 500 with the code buried in +/// `detail/UPnPError`. Distinguishing codes is what makes the lease and conflict retries possible, +/// so a fault without one is not usable as a fault. +fn soap_error_code(body: &str) -> Option { + tag_text(body, "errorCode")?.trim().parse().ok() +} + +// --------------------------------------------------------------------------- +// SOAP +// --------------------------------------------------------------------------- + +/// Build a SOAP request body. +/// +/// The envelope is hand-written in this exact prefixed shape on purpose: goupnp records a router +/// that answers 500 when the outer default namespace is the SOAP one and is then reassigned inside, +/// which is what a generic serialiser tends to emit. +fn soap_body(service_type: &str, action: &str, args: &[(&str, String)]) -> String { + let mut s = String::with_capacity(512); + s.push_str(r#""#); + s.push_str( + r#""#, + ); + s.push_str(&format!(r#""#)); + for (k, v) in args { + s.push_str(&format!("<{k}>{}", xml_escape(v))); + } + s.push_str(&format!("")); + s.push_str(""); + s +} + +fn xml_escape(s: &str) -> String { + s.replace('&', "&") + .replace('<', "<") + .replace('>', ">") +} + +/// A SOAP fault carrying the UPnP error code, when the device supplied one. +#[derive(Debug)] +struct SoapFault { + code: Option, +} + +impl Service { + async fn action( + &self, + action: &str, + args: &[(&str, String)], + ) -> Result, PortMapError> { + let body = soap_body(&self.service_type, action, args); + let soap_action = format!("\"{}#{}\"", self.service_type, action); + let (status, reply) = http_request( + &self.control_url, + "POST", + &[ + ("CONTENT-TYPE", "text/xml; charset=\"utf-8\""), + ("SOAPACTION", &soap_action), + ], + Some(&body), + 64 * 1024, + ) + .await?; + if status == 200 { + return Ok(Ok(reply)); + } + Ok(Err(SoapFault { + code: soap_error_code(&reply), + })) + } + + async fn external_ip(&self) -> Result { + match self.action("GetExternalIPAddress", &[]).await? { + Ok(reply) => tag_text(&reply, "NewExternalIPAddress") + .and_then(|s| s.parse().ok()) + .ok_or_else(|| PortMapError::Malformed("no external address in reply".into())), + Err(f) => Err(PortMapError::Refused(format!( + "GetExternalIPAddress: upnp error {:?}", + f.code + ))), + } + } + + /// Whether the gateway considers its WAN link up. Used only to choose between several + /// candidates, so a device that does not implement it is treated as usable. + async fn is_connected(&self) -> bool { + match self.action("GetStatusInfo", &[]).await { + Ok(Ok(reply)) => tag_text(&reply, "NewConnectionStatus") + .map(|s| s == "Connected") + .unwrap_or(true), + _ => true, + } + } +} + +// --------------------------------------------------------------------------- +// Mapping +// --------------------------------------------------------------------------- + +/// Arguments shared by `AddPortMapping` and `AddAnyPortMapping`. +/// +/// `NewProtocol` is upper-case because some routers reject a lower-case protocol outright +/// (tailscale#7377), and `miniupnpc` sends upper-case for the same reason. `NewRemoteHost` is empty +/// to accept connections from anywhere, which is the point of a peer proxy. +fn add_mapping_args( + external_port: u16, + internal_port: u16, + client: Ipv4Addr, + lease_secs: u32, + description: &str, +) -> Vec<(&'static str, String)> { + vec![ + ("NewRemoteHost", String::new()), + ("NewExternalPort", external_port.to_string()), + ("NewProtocol", "TCP".to_string()), + ("NewInternalPort", internal_port.to_string()), + ("NewInternalClient", client.to_string()), + ("NewEnabled", "1".to_string()), + ("NewPortMappingDescription", description.to_string()), + ("NewLeaseDuration", lease_secs.to_string()), + ] +} + +/// UPnP against one gateway. +pub(super) struct UpnpMapper { + service: Service, + client: Ipv4Addr, + /// Set once a gateway has told us it only accepts permanent leases, so renewals stop asking for + /// a duration it already rejected. + permanent_only: std::sync::atomic::AtomicBool, +} + +impl UpnpMapper { + /// Find a usable gateway service, or fail. + /// + /// Several may answer. A device on a second internal network will happily map a port and report + /// a private external address, which is useless for hosting, so candidates are preferred in + /// this order: WAN link up, and an external address that is actually public. + pub(super) async fn discover( + gateway: Ipv4Addr, + client: Ipv4Addr, + ) -> Result { + let locations = discover_locations(gateway, Duration::from_millis(1200)).await?; + let mut fallback: Option = None; + + for loc in locations { + let loc = repoint_at_gateway(&loc, gateway); + let Ok((200, xml)) = http_request(&loc, "GET", &[], None, 256 * 1024).await else { + continue; + }; + let url_base = tag_text(&xml, "URLBase"); + let found = service_blocks(&xml); + let by_type: HashMap<&str, &str> = found + .iter() + .map(|(t, u)| (t.as_str(), u.as_str())) + .collect(); + + for wanted in SERVICE_TYPES { + let Some(control) = by_type.get(wanted) else { + continue; + }; + let Some(control_url) = absolute_control_url(control, &loc, url_base.as_deref()) + else { + continue; + }; + let service = Service { + control_url, + service_type: wanted.to_string(), + }; + if !service.is_connected().await { + fallback = fallback.or(Some(service)); + continue; + } + match service.external_ip().await { + // A public address means this is the gateway that actually faces the internet. + Ok(ip) if is_public_v4(ip) => { + return Ok(Self::new(service, client)); + } + _ => fallback = fallback.or(Some(service)), + } + } + } + fallback + .map(|s| Self::new(s, client)) + .ok_or(PortMapError::Unavailable) + } + + fn new(service: Service, client: Ipv4Addr) -> Self { + Self { + service, + client, + permanent_only: std::sync::atomic::AtomicBool::new(false), + } + } + + /// Ask for a mapping, working around the two failures that are worth retrying. + /// + /// A gateway that answers `OnlyPermanentLeasesSupported` — or `InvalidArgs`, which some mean by + /// it (tailscale#9343, #15223) — gets asked again with no lease at all. A gateway that answers + /// `ConflictInMappingEntry` has that external port taken by something else, so another is + /// tried. `AddAnyPortMapping` avoids the conflict case entirely by letting the gateway choose, + /// but only `WANIPConnection:2` has it. + async fn request(&self, internal_port: u16) -> Result { + use std::sync::atomic::Ordering; + let v2 = self.service.service_type.ends_with("WANIPConnection:2"); + let mut external = sanitize_external_port(internal_port); + + for attempt in 0..4 { + let lease = if self.permanent_only.load(Ordering::Relaxed) { + 0 + } else { + LEASE_SECS + }; + let args = add_mapping_args( + external, + internal_port, + self.client, + lease, + "spark-unbounded", + ); + let action = if v2 { + "AddAnyPortMapping" + } else { + "AddPortMapping" + }; + match self.service.action(action, &args).await? { + Ok(reply) => { + // AddAnyPortMapping reports the port it actually reserved, which may not be the + // one asked for; AddPortMapping reserves exactly what was asked. + let granted = tag_text(&reply, "NewReservedPort") + .and_then(|s| s.trim().parse().ok()) + .unwrap_or(external); + return Ok(MapGrant { + external_port: granted, + lease: Duration::from_secs(if lease == 0 { + LEASE_SECS as u64 + } else { + lease as u64 + }), + external_ip: None, + }); + } + Err(fault) => match fault.code { + Some(ERR_ONLY_PERMANENT) | Some(ERR_INVALID_ARGS) + if !self.permanent_only.load(Ordering::Relaxed) => + { + self.permanent_only.store(true, Ordering::Relaxed); + } + Some(ERR_CONFLICT) => { + external = next_external_port(external, attempt); + } + other => { + return Err(PortMapError::Refused(format!( + "{action}: upnp error {other:?}" + ))); + } + }, + } + } + Err(PortMapError::Refused( + "gateway refused every external port tried".into(), + )) + } + + pub(super) async fn map(&self, internal_port: u16) -> Result { + let grant = self.request(internal_port).await?; + // Best effort: a mapping without a known external address is still usable, because the + // server falls back to the source address it sees when we register. + let external_ip = self + .service + .external_ip() + .await + .ok() + .filter(|ip| is_public_v4(*ip)); + Ok(Mapping { + external_port: grant.external_port, + internal_port, + external_ip, + lease: grant.lease, + method: Method::Upnp, + }) + } + + pub(super) async fn unmap(&self, mapping: &Mapping) -> Result<(), PortMapError> { + let args = vec![ + ("NewRemoteHost", String::new()), + ("NewExternalPort", mapping.external_port.to_string()), + ("NewProtocol", "TCP".to_string()), + ]; + match self.service.action("DeletePortMapping", &args).await? { + Ok(_) => Ok(()), + Err(f) => Err(PortMapError::Refused(format!( + "DeletePortMapping: upnp error {:?}", + f.code + ))), + } + } + + pub(super) async fn renew(&self, mapping: &Mapping) -> Result { + // Re-issuing the add is how a lease is extended; most gateways treat it as an extension and + // the rest replace the entry, which is equally fine. + let grant = self.request(mapping.internal_port).await?; + Ok(Mapping { + external_port: grant.external_port, + internal_port: mapping.internal_port, + external_ip: mapping.external_ip, + lease: grant.lease, + method: Method::Upnp, + }) + } +} + +/// Keep the requested external port out of two ranges that cause trouble. +/// +/// Zero is a WILDCARD in the specification — it forwards every unmapped external port to this host — +/// so it must never be sent by accident. Ports below 1024 are privileged and many gateways refuse to +/// map them at all. +fn sanitize_external_port(internal_port: u16) -> u16 { + if internal_port >= 1024 { + internal_port + } else { + // Deterministic rather than random: a caller retrying after a restart should ask for the + // same port, so a mapping left behind by a previous run is reused instead of accumulating. + 1024 + internal_port + } +} + +/// Step to another external port after a conflict, staying inside the unprivileged range. +fn next_external_port(current: u16, attempt: u32) -> u16 { + let step = 1 + attempt as u16; + match current.checked_add(step) { + Some(p) if p >= 1024 => p, + _ => 1024, + } +} + +/// Whether an address is usable as a peer's public address. A gateway behind a second layer of NAT +/// reports a private one, and hosting through it cannot work. +fn is_public_v4(ip: Ipv4Addr) -> bool { + !(ip.is_private() + || ip.is_loopback() + || ip.is_link_local() + || ip.is_broadcast() + || ip.is_documentation() + || ip.is_unspecified() + // 100.64.0.0/10, carrier-grade NAT. + || (ip.octets()[0] == 100 && (ip.octets()[1] & 0xc0) == 64)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn msearch_carries_the_headers_devices_require() { + let m = msearch("ssdp:all"); + assert!(m.starts_with("M-SEARCH * HTTP/1.1\r\n")); + assert!(m.contains("HOST: 239.255.255.250:1900\r\n")); + assert!(m.contains("ST: ssdp:all\r\n")); + // MAN must be quoted or devices ignore the search. + assert!(m.contains("MAN: \"ssdp:discover\"\r\n")); + assert!(m.contains("MX: 2\r\n")); + assert!(m.ends_with("\r\n\r\n")); + } + + #[test] + fn ssdp_location_is_case_insensitive() { + let reply = "HTTP/1.1 200 OK\r\nCACHE-CONTROL: max-age=120\r\n\ + Location: http://192.168.1.1:5000/rootDesc.xml\r\n\r\n"; + assert_eq!( + ssdp_location(reply).as_deref(), + Some("http://192.168.1.1:5000/rootDesc.xml") + ); + let upper = "HTTP/1.1 200 OK\r\nLOCATION: http://10.0.0.1/desc.xml\r\n\r\n"; + assert_eq!( + ssdp_location(upper).as_deref(), + Some("http://10.0.0.1/desc.xml") + ); + assert_eq!(ssdp_location("HTTP/1.1 200 OK\r\n\r\n"), None); + } + + #[test] + fn a_location_pointing_elsewhere_is_repointed_at_the_gateway() { + // The advertised host may be an address that is not reachable from here. + assert_eq!( + repoint_at_gateway( + "http://10.9.9.9:5000/desc.xml", + Ipv4Addr::new(192, 168, 1, 1) + ), + "http://192.168.1.1:5000/desc.xml" + ); + // A location that already names the gateway is left exactly as it is. + assert_eq!( + repoint_at_gateway( + "http://192.168.1.1:5000/desc.xml", + Ipv4Addr::new(192, 168, 1, 1) + ), + "http://192.168.1.1:5000/desc.xml" + ); + } + + #[test] + fn tag_text_ignores_prefixes_and_attributes() { + assert_eq!(tag_text("x", "b").as_deref(), Some("x")); + // Devices apply namespace prefixes inconsistently. + assert_eq!( + tag_text( + "1.2.3.4", + "NewExternalIPAddress" + ) + .as_deref(), + Some("1.2.3.4") + ); + assert_eq!( + tag_text(r#"/ctl"#, "controlurl").as_deref(), + Some("/ctl") + ); + assert_eq!(tag_text("", "a"), None); + assert_eq!( + tag_text(r#"1"#, "a").as_deref(), + Some("1") + ); + } + + #[test] + fn service_blocks_are_found_at_any_depth() { + let xml = "\ + urn:schemas-upnp-org:service:WANIPConnection:1\ + /ctl/IPConn\ + "; + assert_eq!( + service_blocks(xml), + vec![( + "urn:schemas-upnp-org:service:WANIPConnection:1".to_string(), + "/ctl/IPConn".to_string() + )] + ); + } + + #[test] + fn control_urls_resolve_in_all_three_forms_devices_send() { + let loc = "http://192.168.1.1:5000/sub/rootDesc.xml"; + assert_eq!( + absolute_control_url("http://192.168.1.1:5000/ctl", loc, None).as_deref(), + Some("http://192.168.1.1:5000/ctl") + ); + assert_eq!( + absolute_control_url("/ctl/IPConn", loc, None).as_deref(), + Some("http://192.168.1.1:5000/ctl/IPConn") + ); + // Path-relative is against the spec but devices send it. + assert_eq!( + absolute_control_url("ctl/IPConn", loc, None).as_deref(), + Some("http://192.168.1.1:5000/sub/ctl/IPConn") + ); + // URLBase, when present, wins over the location. + assert_eq!( + absolute_control_url("/ctl", loc, Some("http://192.168.1.1:80/")).as_deref(), + Some("http://192.168.1.1:80/ctl") + ); + } + + #[test] + fn soap_envelope_uses_the_prefixed_form() { + let body = soap_body( + "urn:svc:1", + "AddPortMapping", + &[("NewExternalPort", "40000".into())], + ); + // A default-namespace envelope reassigned inside makes at least one router answer 500, so + // the prefixed shape is deliberate. + assert!(body.contains(r#""#)); + assert!(body.contains("40000")); + assert!(body.ends_with("")); + } + + #[test] + fn soap_arguments_are_escaped() { + let body = soap_body("urn:svc:1", "A", &[("D", "a&ba&b<c")); + } + + #[test] + fn fault_error_code_is_extracted_from_the_detail() { + let fault = r#" + s:ClientUPnPError + + 725OnlyPermanentLeasesSupported + "#; + assert_eq!(soap_error_code(fault), Some(725)); + assert_eq!(soap_error_code("no code"), None); + } + + #[test] + fn add_mapping_args_avoid_the_two_known_rejections() { + let args = add_mapping_args(40000, 40000, Ipv4Addr::new(192, 168, 1, 5), 3600, "d"); + let map: std::collections::HashMap<_, _> = args.iter().cloned().collect(); + // Lower-case is rejected outright by some routers. + assert_eq!(map.get("NewProtocol").map(String::as_str), Some("TCP")); + // Empty RemoteHost means "from anywhere", which is the point of a peer proxy. + assert_eq!(map.get("NewRemoteHost").map(String::as_str), Some("")); + assert_eq!(map.get("NewEnabled").map(String::as_str), Some("1")); + assert_eq!( + map.get("NewLeaseDuration").map(String::as_str), + Some("3600") + ); + } + + #[test] + fn external_port_zero_is_never_requested() { + // Zero is a wildcard that forwards every unmapped port to this host. + assert_ne!(sanitize_external_port(0), 0); + assert!(sanitize_external_port(0) >= 1024); + // Privileged ports are widely refused. + assert!(sanitize_external_port(80) >= 1024); + // An already-safe port is left alone so a restart reuses its own mapping. + assert_eq!(sanitize_external_port(40000), 40000); + } + + #[test] + fn conflict_retries_stay_unprivileged() { + assert_eq!(next_external_port(40000, 0), 40001); + assert_eq!(next_external_port(40000, 1), 40002); + // Wrapping past the top of the range must not land on a privileged port. + assert_eq!(next_external_port(65535, 0), 1024); + } + + #[test] + fn a_double_natted_gateway_is_not_treated_as_public() { + assert!(is_public_v4(Ipv4Addr::new(93, 184, 216, 34))); + // 203.0.113.0/24 is TEST-NET-3 and reserved for documentation, so it is not a public + // address a peer could be reached on either. + assert!(!is_public_v4(Ipv4Addr::new(203, 0, 113, 5))); + assert!(!is_public_v4(Ipv4Addr::new(192, 168, 1, 1))); + assert!(!is_public_v4(Ipv4Addr::new(10, 0, 0, 1))); + assert!(!is_public_v4(Ipv4Addr::new(172, 16, 0, 1))); + // Carrier-grade NAT: a real address, but not one anything can connect to. + assert!(!is_public_v4(Ipv4Addr::new(100, 64, 0, 1))); + assert!(!is_public_v4(Ipv4Addr::UNSPECIFIED)); + } + + #[test] + fn http_status_and_body_split_tolerates_a_bare_lf_separator() { + assert_eq!( + parse_status("HTTP/1.1 500 Internal Server Error\r\n\r\n"), + Some(500) + ); + assert_eq!(split_body("HTTP/1.1 200 OK\r\nX: 1\r\n\r\nbody"), "body"); + // Some devices emit LF-only line endings. + assert_eq!(split_body("HTTP/1.1 200 OK\nX: 1\n\nbody"), "body"); + assert_eq!(split_body("HTTP/1.1 200 OK"), ""); + } + + #[test] + fn service_types_are_tried_best_first() { + // Only WANIPConnection:2 has AddAnyPortMapping, which lets the gateway resolve a conflict + // itself, so it has to be first. + assert_eq!( + SERVICE_TYPES[0], + "urn:schemas-upnp-org:service:WANIPConnection:2" + ); + // The pre-standard URNs are still answered by older DSL gateways. + assert!(SERVICE_TYPES.contains(&"urn:dslforum-org:service:WANPPPConnection:1")); + } +} From 5f814c7a5af014b17eed478dfeabf06747d7c72a Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 21 Aug 2026 19:18:15 -0600 Subject: [PATCH 3/6] feat(unbounded): let the user set the forwarded port by hand, in Unbounded Settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An Advanced section on the Unbounded Settings page, matching Lantern's, for the networks the discovery protocols cannot help: UPnP switched off for security, an ISP-locked gateway with no IGD, or a second layer of NAT. On those the user's own router rule is the only way to host, and there is no way for spark to find out what port they chose except to ask. Collapsed by default, because it is an escape hatch rather than something a typical volunteer should read past — but opened automatically when an override is already in force, so it is not left hidden behind a header someone has to remember to check. Zero is the interesting part of the contract. It is a REAL value in both port-mapping protocols — a wildcard that forwards every unmapped external port to this host — so "unset" must never be spelled the same way as a port. It is `null` end to end: the field reads empty, the settings type is `number | null`, and the persisted file is REMOVED rather than written as 0, so there is only one spelling of unset for a reader to handle. The single place 0 appears is the wire's "clear it", because the UI cannot send `undefined` for that without it meaning "leave this field alone". Validation is 1..=65535 in the UI, and again in the loader — a file edited by hand to something out of range reads as unset rather than registering a port no peer will answer on. The input is kept as the raw string the user typed rather than a number, so a half-finished entry is not silently coerced into a port that then gets saved. Verified in a browser across all three paths: the range error, the saved confirmation naming the port, and the cleared message. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P4uMnvGU3dM11aXJbLc9hk --- gui-tauri/src/lib/i18n/spark/en.json | 10 +- gui-tauri/src/lib/spark_backend.test.ts | 12 +- gui-tauri/src/lib/spark_backend.ts | 29 ++++- .../routes/settings/unbounded/+page.svelte | 114 ++++++++++++++++++ .../tauri-plugin-spark-vpn/src/persist.rs | 36 ++++++ .../tauri-plugin-spark-vpn/src/unbounded.rs | 9 ++ 6 files changed, 204 insertions(+), 6 deletions(-) diff --git a/gui-tauri/src/lib/i18n/spark/en.json b/gui-tauri/src/lib/i18n/spark/en.json index 0e826e48..e33a819e 100644 --- a/gui-tauri/src/lib/i18n/spark/en.json +++ b/gui-tauri/src/lib/i18n/spark/en.json @@ -60,5 +60,13 @@ "unbounded_err_settings": "Couldn't load Unbounded settings", "unbounded_waiting_for_connections": "Waiting for connections...", "indicator_on": "{feature} is on", - "indicator_off": "{feature} is off" + "indicator_off": "{feature} is off", + "unbounded_advanced": "Advanced", + "unbounded_advanced_sub": "For networks where automatic setup doesn't work.", + "unbounded_manual_port": "Manual port", + "unbounded_manual_port_help": "If your router won't forward a port automatically, forward one yourself and enter it here.", + "unbounded_manual_port_save": "Save", + "unbounded_manual_port_range": "Enter a port between 1 and 65535.", + "unbounded_manual_port_set": "Currently using port {port}.", + "unbounded_manual_port_cleared": "Manual port cleared; Spark will set one up automatically." } diff --git a/gui-tauri/src/lib/spark_backend.test.ts b/gui-tauri/src/lib/spark_backend.test.ts index 7e33a890..50569360 100644 --- a/gui-tauri/src/lib/spark_backend.test.ts +++ b/gui-tauri/src/lib/spark_backend.test.ts @@ -31,14 +31,24 @@ describe("MockBackend unbounded", () => { expect((await mock.unboundedStatus()).enabled).toBe(false); }); - it("defaults settings to all false", async () => { + it("defaults settings to all false, with no manual port override", async () => { expect(await mock.unboundedGetSettings()).toEqual({ autoEnable: false, hidden: false, welcomeSeen: false, + // null, not 0: zero is a real value in the port-mapping protocols (a wildcard forwarding every + // port), so "unset" must not be spelled the same way. + manualPort: null, }); }); + it("clears the manual port when sent zero", async () => { + await mock.unboundedSetSettings({ manualPort: 51820 }); + expect((await mock.unboundedGetSettings()).manualPort).toBe(51820); + await mock.unboundedSetSettings({ manualPort: 0 }); + expect((await mock.unboundedGetSettings()).manualPort).toBeNull(); + }); + it("persists a partial settings update", async () => { await mock.unboundedSetSettings({ welcomeSeen: true }); expect((await mock.unboundedGetSettings()).welcomeSeen).toBe(true); diff --git a/gui-tauri/src/lib/spark_backend.ts b/gui-tauri/src/lib/spark_backend.ts index 89fd5543..7f50c7fb 100644 --- a/gui-tauri/src/lib/spark_backend.ts +++ b/gui-tauri/src/lib/spark_backend.ts @@ -69,7 +69,18 @@ export interface UnboundedStatus { */ origin: UnboundedGeo | null; } -export interface UnboundedSettings { autoEnable: boolean; hidden: boolean; welcomeSeen: boolean; } +export interface UnboundedSettings { + autoEnable: boolean; + hidden: boolean; + welcomeSeen: boolean; + /** + * A router port the user forwarded by hand, or `null` when unset. + * + * `null` rather than 0 for unset: 0 is a real value in the port-mapping protocols (a wildcard that + * forwards every port), so it must never be able to reach one by way of "empty". + */ + manualPort: number | null; +} export interface SparkBackend { status(): Promise; @@ -103,7 +114,7 @@ export interface SparkBackend { unboundedStatus(): Promise; /** Durable Unbounded settings (auto-enable / hidden / welcome-seen). */ unboundedGetSettings(): Promise; - /** Persist any subset of the Unbounded settings (auto-enable / hidden / welcome-seen). */ + /** Persist any subset of the Unbounded settings. `manualPort: 0` clears the manual port. */ unboundedSetSettings(settings: Partial): Promise; /** Whether Unbounded is available for this client (server `features.unbounded` gate + a config * block with the endpoints to dial). Gates whether the UI surfaces the feature at all. */ @@ -137,8 +148,9 @@ const mockState: { autoEnable: boolean; hidden: boolean; welcomeSeen: boolean; + manualPort: number | null; diagnosticsEnabled: boolean; -} = { state: "disconnected", timer: null, pinned: null, split: { enabled: false, domains: [], ips: [] }, routingMode: "smart", adBlockEnabled: true, excludedApps: [], unbounded: { enabled: false, helpingNow: 0, totalHelped: 0, peers: [], origin: null }, unboundedTimer: null, autoEnable: false, hidden: false, welcomeSeen: false, diagnosticsEnabled: true }; +} = { state: "disconnected", timer: null, pinned: null, split: { enabled: false, domains: [], ips: [] }, routingMode: "smart", adBlockEnabled: true, excludedApps: [], unbounded: { enabled: false, helpingNow: 0, totalHelped: 0, peers: [], origin: null }, unboundedTimer: null, autoEnable: false, hidden: false, welcomeSeen: false, manualPort: null, diagnosticsEnabled: true }; export class MockBackend implements SparkBackend { // A stand-in pool (the 6 DO relays used for multi-server bring-up) so the selection screen is @@ -265,12 +277,21 @@ export class MockBackend implements SparkBackend { async unboundedStatus(): Promise { return structuredClone(mockState.unbounded); } async unboundedGetSettings(): Promise { - return { autoEnable: mockState.autoEnable, hidden: mockState.hidden, welcomeSeen: mockState.welcomeSeen }; + return { + autoEnable: mockState.autoEnable, + hidden: mockState.hidden, + welcomeSeen: mockState.welcomeSeen, + manualPort: mockState.manualPort, + }; } async unboundedSetSettings(settings: Partial): Promise { if (settings.autoEnable !== undefined) mockState.autoEnable = settings.autoEnable; if (settings.hidden !== undefined) mockState.hidden = settings.hidden; if (settings.welcomeSeen !== undefined) mockState.welcomeSeen = settings.welcomeSeen; + // 0 is the wire's "clear it", matching what the plugin expects; see UnboundedSettings.manualPort. + if (settings.manualPort !== undefined) { + mockState.manualPort = settings.manualPort === 0 ? null : settings.manualPort; + } } // Dev-visible: the mock always reports Unbounded available so the tab/row shows at `npm run dev`. async unboundedAvailable(): Promise { return true; } diff --git a/gui-tauri/src/routes/settings/unbounded/+page.svelte b/gui-tauri/src/routes/settings/unbounded/+page.svelte index a24b7bd9..f832d819 100644 --- a/gui-tauri/src/routes/settings/unbounded/+page.svelte +++ b/gui-tauri/src/routes/settings/unbounded/+page.svelte @@ -11,11 +11,26 @@ let autoEnable = $state(false); let hidden = $state(false); + // The manual port override, for networks where no mapping protocol works. Kept as the raw string + // the user typed rather than a number so an in-progress or invalid entry is not silently coerced + // into a port we would then save. + let advancedOpen = $state(false); + let portInput = $state(""); + let savedPort = $state(null); + let portError = $state(""); + let portNote = $state(""); + let saving = $state(false); + onMount(async () => { try { const settings = await backend.unboundedGetSettings(); autoEnable = settings.autoEnable; hidden = settings.hidden; + savedPort = settings.manualPort; + portInput = settings.manualPort === null ? "" : String(settings.manualPort); + // Open the section when an override is already in force, so it is not hidden behind a + // collapsed header the user has to remember to check. + advancedOpen = settings.manualPort !== null; } catch { /* keep defaults */ } }); @@ -29,6 +44,32 @@ } } + async function savePort() { + portError = ""; + portNote = ""; + const raw = portInput.trim(); + // An emptied field means "stop overriding", which the wire spells as 0. + let port = 0; + if (raw !== "") { + const parsed = Number(raw); + if (!Number.isInteger(parsed) || parsed < 1 || parsed > 65535) { + portError = $_("unbounded_manual_port_range"); + return; + } + port = parsed; + } + saving = true; + try { + await backend.unboundedSetSettings({ manualPort: port }); + savedPort = port === 0 ? null : port; + portNote = port === 0 ? $_("unbounded_manual_port_cleared") : ""; + } catch { + portError = $_("unbounded_manual_port_range"); + } finally { + saving = false; + } + } + async function toggleHidden() { const prev = hidden; hidden = !hidden; @@ -71,6 +112,56 @@ + + +
+ + + {#if advancedOpen} +
+
+ +

{$_("unbounded_manual_port_help")}

+
+ + +
+ {#if portError} + + {:else if portNote} +

{portNote}

+ {:else if savedPort !== null} +

+ {$_("unbounded_manual_port_set", { values: { port: savedPort } })} +

+ {/if} +
+ {/if} +
@@ -118,4 +209,27 @@ border-radius: 50%; background: #fff; transition: transform 0.15s ease; } .switch.on .knob { transform: translateX(18px); } + + .advanced { margin-top: 12px; } + .disclosure { cursor: pointer; justify-content: space-between; } + .chev { display: inline-flex; color: var(--text-tertiary); transition: transform 0.15s ease; } + .chev.open { transform: rotate(180deg); } + .port { padding: 4px 16px 16px; } + .port .sub { margin: 2px 0 10px; } + .port-row { display: flex; gap: 8px; align-items: center; } + .port-row input { + flex: 1; min-width: 0; height: 36px; padding: 0 10px; + border: 1px solid var(--border); border-radius: 8px; + background: var(--bg); color: var(--text-primary); + font-family: var(--font); font-size: 14px; + } + .port-row input[aria-invalid="true"] { border-color: #c0392b; } + .save { + height: 36px; padding: 0 16px; border: none; border-radius: 8px; + background: var(--brand); color: #fff; + font-family: var(--font); font-size: 14px; font-weight: 600; cursor: pointer; + } + .save:disabled { opacity: 0.6; cursor: default; } + .err { margin: 8px 0 0; font-size: 12px; font-weight: 500; color: #c0392b; } + .note { margin: 8px 0 0; font-size: 12px; font-weight: 500; color: var(--text-tertiary); } diff --git a/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs b/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs index 110816ac..44d79ff8 100644 --- a/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs +++ b/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs @@ -246,6 +246,42 @@ pub fn save_unbounded_hidden(base: &Path, hidden: bool) -> crate::Result<()> { save_unbounded_bool(base, "unbounded_hidden.txt", hidden) } +/// Read the persisted manual port from `/unbounded_manual_port.txt`. +/// +/// Returns `None` when unset, unreadable, or out of range. A port outside 1..=65535 cannot be +/// listened on, so treating it as unset is what lets the caller fall through to discovery rather +/// than registering a port no peer will answer on. +#[cfg_attr(not(desktop), allow(dead_code))] +pub fn load_unbounded_manual_port(base: &Path) -> Option { + std::fs::read_to_string(base.join("unbounded_manual_port.txt")) + .ok() + .and_then(|s| s.trim().parse::().ok()) + .filter(|p| (1..=65535).contains(p)) + .map(|p| p as u16) +} + +/// Persist the manual port. `None` clears it, which is how the user turns the override back off. +/// +/// Creates `base` (and any parents) if they don't exist. +#[cfg_attr(not(desktop), allow(dead_code))] +pub fn save_unbounded_manual_port(base: &Path, port: Option) -> crate::Result<()> { + let path = base.join("unbounded_manual_port.txt"); + match port { + // Removing the file rather than writing 0 keeps "unset" a single state; a 0 on disk would + // also read as unset, and two spellings of it invite a reader that only handles one. + None => match std::fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e.into()), + }, + Some(p) => { + std::fs::create_dir_all(base)?; + std::fs::write(&path, p.to_string())?; + Ok(()) + } + } +} + /// Read the persisted `unbounded_welcome_seen` toggle from `/unbounded_welcome_seen.txt`. /// /// Returns `false` (welcome not yet seen) unless the file holds exactly `"true"` (trimmed, diff --git a/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs b/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs index 7a6850ec..3a29ae47 100644 --- a/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs +++ b/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs @@ -620,6 +620,9 @@ pub(crate) async fn unbounded_get_settings( "autoEnable": crate::persist::load_unbounded_auto_enable(&base), "hidden": crate::persist::load_unbounded_hidden(&base), "welcomeSeen": crate::persist::load_unbounded_welcome_seen(&base), + // `null` when unset. The UI shows an empty field for that rather than a 0, which would read + // as a configured port. + "manualPort": crate::persist::load_unbounded_manual_port(&base), })) } @@ -634,6 +637,9 @@ pub(crate) struct UnboundedSettingsPatch { auto_enable: Option, hidden: Option, welcome_seen: Option, + /// The router port the user forwarded by hand. `Some(0)` clears it, which is how an emptied + /// field arrives — the UI cannot send `undefined` for "clear" without it meaning "leave alone". + manual_port: Option, } #[tauri::command] @@ -651,6 +657,9 @@ pub(crate) async fn unbounded_set_settings( if let Some(v) = settings.welcome_seen { crate::persist::save_unbounded_welcome_seen(&base, v)?; } + if let Some(v) = settings.manual_port { + crate::persist::save_unbounded_manual_port(&base, (v != 0).then_some(v))?; + } Ok(()) } From 15d46fbce0edff4effbadd965b6f11188b3ce396 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 21 Aug 2026 19:26:10 -0600 Subject: [PATCH 4/6] =?UTF-8?q?fix(portmap):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20ranked=20service=20choice,=20HTTP=20timeouts,=20one?= =?UTF-8?q?=20port=20range?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings, all real. The service choice contradicted its own documented preference. Candidates were kept first-wins and only replaced on a perfect match, so a DISCONNECTED service discovered early beat a connected one discovered later — the opposite of what the comment above it claimed. Candidates are ranked now (disconnected < connected-with-a-private-address < connected-and-public), still returning immediately on the best rank so a good gateway costs no extra SOAP round trips. Two ways UPnP could hang forever, both fixed with bounds rather than hope. A blackholed gateway address stalled `TcpStream::connect` indefinitely, and a gateway that accepted the connection and then sent nothing — or dribbled bytes without ever closing — stalled the read loop. Both now time out, which matters because the whole point of the fallback chain is that a broken UPnP gateway costs a couple of seconds before PCP is tried. Oversized replies were silently TRUNCATED at the read limit. A body cut mid-document parses as a document with fields missing, which surfaces much later as a confusing "no controlURL" rather than as the size problem it is. Now an explicit error. And the manual port range disagreed with itself: the placeholder said 1024-65535 while validation accepted 1. The placeholder was right, for a reason worth stating — this port is bound by the UNPRIVILEGED sharing process, which cannot take a privileged one, and gateways widely refuse to map them anyway. Tightened to 1024..=65535 in all four places that enforce it: the input, the error string, the persisted loader, and `ManualMapper`, which previously rejected only 0. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P4uMnvGU3dM11aXJbLc9hk --- gui-tauri/src/lib/i18n/spark/en.json | 2 +- .../routes/settings/unbounded/+page.svelte | 4 +- .../tauri-plugin-spark-vpn/src/persist.rs | 9 ++- spark-sharing/src/portmap/mod.rs | 21 ++++-- spark-sharing/src/portmap/upnp.rs | 73 +++++++++++++++---- 5 files changed, 82 insertions(+), 27 deletions(-) diff --git a/gui-tauri/src/lib/i18n/spark/en.json b/gui-tauri/src/lib/i18n/spark/en.json index e33a819e..47bc69e6 100644 --- a/gui-tauri/src/lib/i18n/spark/en.json +++ b/gui-tauri/src/lib/i18n/spark/en.json @@ -66,7 +66,7 @@ "unbounded_manual_port": "Manual port", "unbounded_manual_port_help": "If your router won't forward a port automatically, forward one yourself and enter it here.", "unbounded_manual_port_save": "Save", - "unbounded_manual_port_range": "Enter a port between 1 and 65535.", + "unbounded_manual_port_range": "Enter a port between 1024 and 65535.", "unbounded_manual_port_set": "Currently using port {port}.", "unbounded_manual_port_cleared": "Manual port cleared; Spark will set one up automatically." } diff --git a/gui-tauri/src/routes/settings/unbounded/+page.svelte b/gui-tauri/src/routes/settings/unbounded/+page.svelte index f832d819..cc8fd831 100644 --- a/gui-tauri/src/routes/settings/unbounded/+page.svelte +++ b/gui-tauri/src/routes/settings/unbounded/+page.svelte @@ -52,7 +52,9 @@ let port = 0; if (raw !== "") { const parsed = Number(raw); - if (!Number.isInteger(parsed) || parsed < 1 || parsed > 65535) { + // 1024, not 1: this port is bound locally by the unprivileged sharing process, and a + // privileged port needs root it does not have. Gateways widely refuse to map them anyway. + if (!Number.isInteger(parsed) || parsed < 1024 || parsed > 65535) { portError = $_("unbounded_manual_port_range"); return; } diff --git a/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs b/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs index 44d79ff8..5a8c07b6 100644 --- a/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs +++ b/gui-tauri/tauri-plugin-spark-vpn/src/persist.rs @@ -248,15 +248,16 @@ pub fn save_unbounded_hidden(base: &Path, hidden: bool) -> crate::Result<()> { /// Read the persisted manual port from `/unbounded_manual_port.txt`. /// -/// Returns `None` when unset, unreadable, or out of range. A port outside 1..=65535 cannot be -/// listened on, so treating it as unset is what lets the caller fall through to discovery rather -/// than registering a port no peer will answer on. +/// Returns `None` when unset, unreadable, or outside 1024..=65535. The low bound is not cosmetic: +/// this port is bound by the unprivileged sharing process, which cannot take a privileged one, and +/// gateways widely refuse to map them regardless. Treating an out-of-range value as unset is what +/// lets the caller fall through to discovery rather than registering a port no peer will answer on. #[cfg_attr(not(desktop), allow(dead_code))] pub fn load_unbounded_manual_port(base: &Path) -> Option { std::fs::read_to_string(base.join("unbounded_manual_port.txt")) .ok() .and_then(|s| s.trim().parse::().ok()) - .filter(|p| (1..=65535).contains(p)) + .filter(|p| (1024..=65535).contains(p)) .map(|p| p as u16) } diff --git a/spark-sharing/src/portmap/mod.rs b/spark-sharing/src/portmap/mod.rs index 88613e15..79f0ea65 100644 --- a/spark-sharing/src/portmap/mod.rs +++ b/spark-sharing/src/portmap/mod.rs @@ -330,11 +330,16 @@ pub struct ManualMapper { } impl ManualMapper { - /// `port` must be a real listening port; 0 is rejected so a misconfiguration cannot register a - /// port no peer will answer on. + /// `port` must be one this process can actually listen on: 1024..=65535. + /// + /// Below 1024 is privileged and this subsystem runs unprivileged, and 0 is the wildcard that + /// forwards every port — so rejecting the range here stops a misconfiguration registering a + /// port no peer will answer on, or one that would forward far more than intended. pub fn new(port: u16) -> Result { - if port == 0 { - return Err(PortMapError::Refused("manual port 0".into())); + if !(1024..=65535).contains(&port) { + return Err(PortMapError::Refused(format!( + "manual port {port} is outside 1024..=65535" + ))); } Ok(Self { port }) } @@ -957,8 +962,14 @@ mod tests { } #[test] - fn manual_rejects_port_zero() { + fn manual_rejects_ports_this_process_cannot_bind() { + // 0 is the wildcard that forwards every port. assert!(ManualMapper::new(0).is_err()); + // Privileged, and this subsystem runs unprivileged. + assert!(ManualMapper::new(80).is_err()); + assert!(ManualMapper::new(1023).is_err()); + assert!(ManualMapper::new(1024).is_ok()); + assert!(ManualMapper::new(65535).is_ok()); } #[test] diff --git a/spark-sharing/src/portmap/upnp.rs b/spark-sharing/src/portmap/upnp.rs index 4ace2f17..251ac4b2 100644 --- a/spark-sharing/src/portmap/upnp.rs +++ b/spark-sharing/src/portmap/upnp.rs @@ -16,6 +16,7 @@ //! None of them contribute to getting one TCP port forwarded. use std::collections::HashMap; +use std::io; use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use std::time::Duration; @@ -24,6 +25,11 @@ use tokio::net::{TcpStream, UdpSocket}; use super::{MapGrant, Mapping, Method, PortMapError}; +/// Bounds on one HTTP exchange with the gateway. Short: this is a LAN device answering with a small +/// document, and every second spent waiting on a broken one is a second not spent trying PCP. +const HTTP_CONNECT_TIMEOUT: Duration = Duration::from_secs(3); +const HTTP_READ_TIMEOUT: Duration = Duration::from_secs(5); + const SSDP_MULTICAST: SocketAddrV4 = SocketAddrV4::new(Ipv4Addr::new(239, 255, 255, 250), 1900); /// Service types we can map with, best first. @@ -188,7 +194,11 @@ async fn http_request( .next() .ok_or_else(|| PortMapError::Malformed(format!("no address for {authority}")))?; - let mut stream = TcpStream::connect(addr).await?; + // A blackholed gateway address would otherwise stall discovery indefinitely and never fall + // through to PCP/NAT-PMP. + let mut stream = tokio::time::timeout(HTTP_CONNECT_TIMEOUT, TcpStream::connect(addr)) + .await + .map_err(|_| io::Error::new(io::ErrorKind::TimedOut, "connect to gateway timed out"))??; let mut req = format!("{method} {path} HTTP/1.1\r\nHOST: {authority}\r\n"); for (k, v) in extra_headers { req.push_str(&format!("{k}: {v}\r\n")); @@ -210,13 +220,22 @@ async fn http_request( let mut raw = Vec::with_capacity(4096); let mut chunk = [0_u8; 4096]; loop { - let n = stream.read(&mut chunk).await?; + // Per-read, not whole-body: a gateway that accepts the connection and then sends nothing — + // or dribbles bytes forever without closing — would otherwise hang this task for good. + let n = tokio::time::timeout(HTTP_READ_TIMEOUT, stream.read(&mut chunk)) + .await + .map_err(|_| io::Error::new(io::ErrorKind::TimedOut, "gateway stopped responding"))??; if n == 0 { break; } raw.extend_from_slice(&chunk[..n]); - if raw.len() >= limit { - break; + // (2) An explicit error rather than truncation. A body cut mid-document parses as a + // document with missing fields, which surfaces as a confusing "no controlURL" far from the + // actual cause. + if raw.len() > limit { + return Err(PortMapError::Malformed(format!( + "reply from {authority} exceeded {limit} bytes" + ))); } } let text = String::from_utf8_lossy(&raw).into_owned(); @@ -454,6 +473,28 @@ fn add_mapping_args( ] } +/// How good a candidate service is, worst to best. +/// +/// A gateway on a second internal network will happily map a port and report a private external +/// address, which cannot host anything — so having a PUBLIC address is what separates a usable +/// gateway from a merely responsive one. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +enum Rank { + Disconnected, + ConnectedPrivate, + ConnectedPublic, +} + +async fn rank_service(service: &Service) -> Rank { + if !service.is_connected().await { + return Rank::Disconnected; + } + match service.external_ip().await { + Ok(ip) if is_public_v4(ip) => Rank::ConnectedPublic, + _ => Rank::ConnectedPrivate, + } +} + /// UPnP against one gateway. pub(super) struct UpnpMapper { service: Service, @@ -474,7 +515,10 @@ impl UpnpMapper { client: Ipv4Addr, ) -> Result { let locations = discover_locations(gateway, Duration::from_millis(1200)).await?; - let mut fallback: Option = None; + // Ranked, not first-wins. Keeping the first candidate found and only replacing it on a + // perfect match means a disconnected service discovered early beats a connected one + // discovered later — the opposite of the stated preference. + let mut best: Option<(Rank, Service)> = None; for loc in locations { let loc = repoint_at_gateway(&loc, gateway); @@ -500,21 +544,18 @@ impl UpnpMapper { control_url, service_type: wanted.to_string(), }; - if !service.is_connected().await { - fallback = fallback.or(Some(service)); - continue; + let rank = rank_service(&service).await; + // Nothing outranks a connected gateway with a public address, so stop looking + // rather than pay for more SOAP round trips. + if rank == Rank::ConnectedPublic { + return Ok(Self::new(service, client)); } - match service.external_ip().await { - // A public address means this is the gateway that actually faces the internet. - Ok(ip) if is_public_v4(ip) => { - return Ok(Self::new(service, client)); - } - _ => fallback = fallback.or(Some(service)), + if best.as_ref().is_none_or(|(r, _)| rank > *r) { + best = Some((rank, service)); } } } - fallback - .map(|s| Self::new(s, client)) + best.map(|(_, s)| Self::new(s, client)) .ok_or(PortMapError::Unavailable) } From 6adffcd5bb73bffbe66fd2e8b4a9c606390f17e3 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 21 Aug 2026 19:34:55 -0600 Subject: [PATCH 5/6] =?UTF-8?q?fix:=20address=20round=20two=20=E2=80=94=20?= =?UTF-8?q?blank=20globe=20with=20no=20peers,=20restarted=20growth,=20two?= =?UTF-8?q?=20scoping=20holes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings, all real. The globe went blank when sharing was on with nobody connected yet. `layoutArcs` cleared everything on `arcs.length === 0`, including our own dot — but that state is normal, and the page is deliberately showing "waiting for connections" through it. We know where WE are regardless of whether anyone has arrived, so the early return now requires no origin either. Two paths bypassed the mechanism that keeps a culled arc's growth animation running, which is the whole reason `ArcPath.visible` exists rather than dropping entries. An arc was skipped outright while `origin` was still null, and again when every sample fell behind the sphere — so the element was destroyed and the growth restarted, once when the self lookup landed and again whenever a route rotated back into view. Both now push a hidden entry. A failed save told the user to enter a port between 1024 and 65535, immediately after they had entered one that passed exactly that check. It has its own message now. The origin write was scoped by `stop_epoch` alone, and that is not sufficient: `unbounded_stop` bumps it, but the loop tail that runs when the supervisor pool ends on its OWN does not. That path clears the origin, so a retry still sleeping could write it back and leave `unbounded_status` reporting where we are with `running: false` — the precise state the scoping was added to prevent. It checks the generation too now. The Windows `route` lookup ran without `CREATE_NO_WINDOW`, so a GUI process flashed a console window at the user at the start of every sharing session. And two doc blocks still described the pre-UPnP selection order: a header reading "Three ways" above a four-item list, and `discover`'s summary omitting UPnP between the manual rule and PCP. The Windows branch cannot be compiled locally — `ring` needs an MSVC toolchain — but CI builds `spark-sharing` on windows-latest, so it is covered there. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P4uMnvGU3dM11aXJbLc9hk --- gui-tauri/src/lib/Globe.svelte | 19 +++++++++++++++---- gui-tauri/src/lib/i18n/spark/en.json | 3 ++- .../routes/settings/unbounded/+page.svelte | 4 +++- .../tauri-plugin-spark-vpn/src/unbounded.rs | 8 ++++++++ spark-sharing/src/portmap/mod.rs | 11 ++++++++--- 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/gui-tauri/src/lib/Globe.svelte b/gui-tauri/src/lib/Globe.svelte index a22cea5f..c1a8cd7e 100644 --- a/gui-tauri/src/lib/Globe.svelte +++ b/gui-tauri/src/lib/Globe.svelte @@ -259,7 +259,10 @@ * globe still costs nothing. */ function layoutArcs() { - if (!globe || !el || arcs.length === 0) { + // Not `arcs.length === 0`: sharing can be on with nobody connected yet, which the screen treats + // as a normal state, and in it we still know where WE are. Clearing everything there left the + // globe blank while the page was showing "waiting for connections". + if (!globe || !el || (arcs.length === 0 && !origin)) { paths = []; peerDots = []; ownDot = null; @@ -302,7 +305,13 @@ // Both feet have to be on the face — an arc with one foot past the limb draws as a hairpin off // the edge — so no origin, or an origin round the back, means dots without arcs. The arc is // still emitted, just not drawable: see `ArcPath.visible`. - if (!far || !own) continue; + if (!far || !own) { + // Kept, not dropped: the origin lands after the first peers, and removing the entry would + // destroy the element so every arc restarted its growth at that moment instead of appearing + // already drawn. See `ArcPath.visible`. + out.push({ id: a.id, color: a.color, d: "", visible: false }); + continue; + } const drawable = peerVisible && ownVisible; // SAMPLED ALONG THE GREAT CIRCLE, not fitted with one Bézier. // @@ -356,12 +365,14 @@ } if (run.length > 1) runs.push(run); const d = runs.map((r) => `M ${r.join(" L ")}`).join(" "); - if (!d) continue; out.push({ id: a.id, color: a.color, d, - visible: drawable, + // An arc whose every sample fell behind the sphere has nothing to draw, but it stays in the + // list for the same reason as above — dropping it restarts the growth when the route rotates + // back into view. + visible: drawable && d !== "", }); } paths = out; diff --git a/gui-tauri/src/lib/i18n/spark/en.json b/gui-tauri/src/lib/i18n/spark/en.json index 47bc69e6..3a87cb08 100644 --- a/gui-tauri/src/lib/i18n/spark/en.json +++ b/gui-tauri/src/lib/i18n/spark/en.json @@ -68,5 +68,6 @@ "unbounded_manual_port_save": "Save", "unbounded_manual_port_range": "Enter a port between 1024 and 65535.", "unbounded_manual_port_set": "Currently using port {port}.", - "unbounded_manual_port_cleared": "Manual port cleared; Spark will set one up automatically." + "unbounded_manual_port_cleared": "Manual port cleared; Spark will set one up automatically.", + "unbounded_manual_port_save_failed": "Couldn't save the port. Please try again." } diff --git a/gui-tauri/src/routes/settings/unbounded/+page.svelte b/gui-tauri/src/routes/settings/unbounded/+page.svelte index cc8fd831..07937a63 100644 --- a/gui-tauri/src/routes/settings/unbounded/+page.svelte +++ b/gui-tauri/src/routes/settings/unbounded/+page.svelte @@ -66,7 +66,9 @@ savedPort = port === 0 ? null : port; portNote = port === 0 ? $_("unbounded_manual_port_cleared") : ""; } catch { - portError = $_("unbounded_manual_port_range"); + // Its own message: the range error would tell the user to enter a port between 1024 and 65535 + // immediately after they entered one that passed exactly that check. + portError = $_("unbounded_manual_port_save_failed"); } finally { saving = false; } diff --git a/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs b/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs index 3a29ae47..c14195c4 100644 --- a/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs +++ b/gui-tauri/tauri-plugin-spark-vpn/src/unbounded.rs @@ -347,11 +347,19 @@ pub(crate) async fn unbounded_start(app: AppHandle) -> crate::Res let epoch_at_start = origin_app .try_state::() .map(|st| st.stop_epoch.load(std::sync::atomic::Ordering::Acquire)); + // `stop_epoch` alone is not enough: it is bumped by `unbounded_stop`, but NOT by the + // loop tail that runs when the supervisor pool ends on its own. That path clears the + // origin, so a retry still sleeping could write it back and leave `unbounded_status` + // reporting where we are with `running: false` — the exact state this scoping exists to + // prevent. The generation moves on any start, which covers it. + let generation_at_start = generation; tauri::async_runtime::spawn(async move { let still_this_session = || match (epoch_at_start, origin_app.try_state::()) { (Some(before), Some(st)) => { st.stop_epoch.load(std::sync::atomic::Ordering::Acquire) == before + && st.generation.load(std::sync::atomic::Ordering::SeqCst) + == generation_at_start } _ => false, }; diff --git a/spark-sharing/src/portmap/mod.rs b/spark-sharing/src/portmap/mod.rs index 79f0ea65..a4a8f1e9 100644 --- a/spark-sharing/src/portmap/mod.rs +++ b/spark-sharing/src/portmap/mod.rs @@ -5,7 +5,7 @@ //! the router forwarded to this host, so lantern-cloud can hand the address out to censored clients //! and they can connect straight to it. //! -//! Three ways to get that port, tried in order by [`discover`]: +//! Four ways to get that port, tried in order by [`discover`]: //! //! 1. A rule the user configured by hand. An explicit instruction outranks discovery. //! 2. UPnP/IGD, the widest-supported protocol, in [`upnp`]. @@ -567,9 +567,14 @@ fn default_gateway_blocking() -> Result { } #[cfg(target_os = "windows")] { + use std::os::windows::process::CommandExt; + // Without this a GUI process flashes a console window at the user, and this runs at the + // start of every sharing session. + const CREATE_NO_WINDOW: u32 = 0x0800_0000; let out = std::process::Command::new("route") .args(["print", "-4", "0.0.0.0"]) .stdin(std::process::Stdio::null()) + .creation_flags(CREATE_NO_WINDOW) .output() .map_err(|e| PortMapError::Gateway(format!("run route: {e}")))?; parse_route_print(&String::from_utf8_lossy(&out.stdout)) @@ -649,8 +654,8 @@ pub async fn local_ip() -> Result { } } -/// Pick how this host will accept inbound connections: a hand-configured rule, then PCP, then -/// NAT-PMP. +/// Pick how this host will accept inbound connections: a hand-configured rule, then UPnP, then PCP, +/// then NAT-PMP. /// /// `manual_port` wins when set because it is an explicit instruction from the user, and finding a /// gateway that would map some other port is not a reason to override it. From 0d029b856478fe45a81d8d912742d1c46fdf539f Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 21 Aug 2026 19:41:14 -0600 Subject: [PATCH 6/6] fix(portmap): bind a fresh socket per PMP exchange, so replies cannot cross callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PortMapper` is `Send + Sync`, so two tasks may exchange with the gateway at once, and the PMP transport was one long-lived connected socket doing send-then-receive with no request/reply demultiplexing. A reply would be delivered to whichever caller happened to be reading, not the one that asked. PCP would catch that — its nonce is checked — but a NAT-PMP reply carries nothing to match against, so the wrong caller would accept the wrong mapping and go on to advertise a port it does not hold. Fixed by removing the shared state rather than locking around it: each exchange binds its own socket, so it has its own ephemeral port and a reply can only arrive at the request that provoked it. No lock, no demux, and no guard held across an await — which this crate's conventions discourage anyway. The cost is one socket per map, renew or unmap: operations that happen about once an hour, not on a data path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P4uMnvGU3dM11aXJbLc9hk --- spark-sharing/src/portmap/mod.rs | 34 +++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/spark-sharing/src/portmap/mod.rs b/spark-sharing/src/portmap/mod.rs index a4a8f1e9..13d57187 100644 --- a/spark-sharing/src/portmap/mod.rs +++ b/spark-sharing/src/portmap/mod.rs @@ -269,29 +269,40 @@ const PMP_RETRIES: [Duration; 3] = [ Duration::from_millis(1000), ]; +/// A fresh socket per exchange, deliberately. +/// +/// A single long-lived socket would be a correctness bug rather than an optimisation: the trait is +/// `Send + Sync`, so two tasks may exchange at once, and this is send-then-receive with no +/// request/reply demultiplexing — replies would be delivered to whichever caller happened to be +/// reading. PCP would catch it, because its nonce is checked, but a NAT-PMP reply carries nothing to +/// match against and the wrong caller would accept the wrong mapping. +/// +/// Binding per exchange gives each one its own ephemeral port, so a reply can only ever arrive at +/// the request that provoked it. That removes the hazard without a lock, and the cost is one socket +/// per map/renew/unmap — operations that happen once an hour, not on a data path. struct UdpPmp { - sock: UdpSocket, + gateway: Ipv4Addr, } impl UdpPmp { - async fn connect(gateway: Ipv4Addr) -> Result { - let sock = UdpSocket::bind(SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0))).await?; - sock.connect(SocketAddr::V4(SocketAddrV4::new(gateway, PMP_PORT))) - .await?; - Ok(Self { sock }) + fn new(gateway: Ipv4Addr) -> Self { + Self { gateway } } } #[async_trait::async_trait] impl PmpTransport for UdpPmp { async fn round_trip(&self, req: &[u8]) -> Result, PortMapError> { + let sock = UdpSocket::bind(SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0))).await?; + sock.connect(SocketAddr::V4(SocketAddrV4::new(self.gateway, PMP_PORT))) + .await?; let mut buf = [0_u8; 1500]; for wait in PMP_RETRIES { - self.sock.send(req).await?; - match tokio::time::timeout(wait, self.sock.recv(&mut buf)).await { + sock.send(req).await?; + match tokio::time::timeout(wait, sock.recv(&mut buf)).await { Ok(Ok(n)) => return Ok(buf[..n].to_vec()), - // A read error is as retryable as a timeout here: an ICMP port-unreachable from a - // gateway that is not listening surfaces as one on some platforms. + // A read error is as retryable as a timeout: an ICMP port-unreachable from a gateway + // that is not listening surfaces as one on some platforms. Ok(Err(_)) | Err(_) => continue, } } @@ -392,8 +403,7 @@ impl PmpMapper { /// external-address request. A NAT-PMP-only gateway usually ignores the PCP request rather than /// refusing it, so silence has to fall through as well as an explicit version refusal. pub async fn discover(gateway: Ipv4Addr, client: Ipv4Addr) -> Result { - let transport = UdpPmp::connect(gateway).await?; - Self::negotiate(Box::new(transport), client).await + Self::negotiate(Box::new(UdpPmp::new(gateway)), client).await } async fn negotiate(