From ef04ba864c3d158b748171bd97851df934629087 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 14:49:31 +0200 Subject: [PATCH 01/13] feat: foundation for port mapping server on routers Introduces the patchbay::portmap module with public PortmapMode and PortmapConfig types, plus the internal mapping registry that later protocol servers will share. The registry keys external slots by (proto, external_port), carries a secondary index for deduplicating repeated requests from the same internal socket across protocols, and supports targeted removal, expiry reaping, and bulk drain for shutdown. No protocol handlers yet; those land in follow-up commits. This commit keeps the public API surface off-ramped from existing presets. Every router remains portmap-off by default. --- patchbay/src/lib.rs | 3 + patchbay/src/portmap/config.rs | 99 +++++++ patchbay/src/portmap/mod.rs | 29 ++ patchbay/src/portmap/registry.rs | 430 +++++++++++++++++++++++++++++ patchbay/src/portmap/wire.rs | 8 + worklogs/2026-04-23-portmapping.md | 96 +++++++ 6 files changed, 665 insertions(+) create mode 100644 patchbay/src/portmap/config.rs create mode 100644 patchbay/src/portmap/mod.rs create mode 100644 patchbay/src/portmap/registry.rs create mode 100644 patchbay/src/portmap/wire.rs create mode 100644 worklogs/2026-04-23-portmapping.md diff --git a/patchbay/src/lib.rs b/patchbay/src/lib.rs index 45ec98d..b1fdbb4 100644 --- a/patchbay/src/lib.rs +++ b/patchbay/src/lib.rs @@ -221,6 +221,8 @@ mod netns; pub(crate) mod nft; #[path = "tracing.rs"] mod ns_tracing; +/// Port mapping server: UPnP IGD, NAT-PMP, and PCP. +pub mod portmap; mod qdisc; /// Router handle, builder, and presets. pub(crate) mod router; @@ -246,6 +248,7 @@ pub use lab::{ NatV6Mode, OutDir, Region, RegionLink, TestGuard, }; pub use metrics::MetricsBuilder; +pub use portmap::{PortmapConfig, PortmapMode}; pub use router::{Router, RouterBuilder, RouterIface, RouterPreset}; pub use crate::{ diff --git a/patchbay/src/portmap/config.rs b/patchbay/src/portmap/config.rs new file mode 100644 index 0000000..6ab4757 --- /dev/null +++ b/patchbay/src/portmap/config.rs @@ -0,0 +1,99 @@ +//! Configuration types for the port mapping server. + +/// Which port mapping protocols a router advertises. +/// +/// Maps to a [`PortmapConfig`] at build time; `None` leaves the server off. +/// Off by default on every [`RouterPreset`](crate::RouterPreset) to avoid +/// changing the traffic profile of existing tests. Opt in explicitly with +/// [`RouterBuilder::portmap`](crate::RouterBuilder::portmap) or turn on at +/// runtime with [`Router::set_portmap`](crate::Router::set_portmap). +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum PortmapMode { + /// No port mapping server. + #[default] + None, + /// NAT-PMP only (UDP 5351, RFC 6886). + NatPmpOnly, + /// PCP only (UDP 5351, RFC 6887). + PcpOnly, + /// UPnP IGD:1 only (SSDP + HTTP/SOAP). + UpnpOnly, + /// NAT-PMP and PCP sharing UDP 5351, no UPnP. + NatPmpAndPcp, + /// All three protocols enabled. + All, +} + +/// Per-router port mapping server configuration. +/// +/// Each flag toggles one protocol. Multiple protocols may be enabled at the +/// same time; NAT-PMP and PCP then share the same UDP 5351 socket as they do +/// on real gateways. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct PortmapConfig { + /// Advertise NAT-PMP on UDP 5351. + pub enable_nat_pmp: bool, + /// Advertise PCP on UDP 5351. + pub enable_pcp: bool, + /// Advertise UPnP IGD via SSDP and HTTP/SOAP. + pub enable_upnp: bool, +} + +impl PortmapConfig { + /// Returns the config corresponding to `mode`. + pub fn from_mode(mode: PortmapMode) -> Self { + match mode { + PortmapMode::None => Self::default(), + PortmapMode::NatPmpOnly => Self { + enable_nat_pmp: true, + ..Self::default() + }, + PortmapMode::PcpOnly => Self { + enable_pcp: true, + ..Self::default() + }, + PortmapMode::UpnpOnly => Self { + enable_upnp: true, + ..Self::default() + }, + PortmapMode::NatPmpAndPcp => Self { + enable_nat_pmp: true, + enable_pcp: true, + enable_upnp: false, + }, + PortmapMode::All => Self { + enable_nat_pmp: true, + enable_pcp: true, + enable_upnp: true, + }, + } + } + + /// Returns `true` if any protocol is enabled. + pub fn any_enabled(&self) -> bool { + self.enable_nat_pmp || self.enable_pcp || self.enable_upnp + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn mode_none_disables_everything() { + assert!(!PortmapConfig::from_mode(PortmapMode::None).any_enabled()); + } + + #[test] + fn mode_all_enables_everything() { + let cfg = PortmapConfig::from_mode(PortmapMode::All); + assert!(cfg.enable_nat_pmp && cfg.enable_pcp && cfg.enable_upnp); + } + + #[test] + fn mode_nat_pmp_and_pcp_leaves_upnp_off() { + let cfg = PortmapConfig::from_mode(PortmapMode::NatPmpAndPcp); + assert!(cfg.enable_nat_pmp && cfg.enable_pcp); + assert!(!cfg.enable_upnp); + } +} diff --git a/patchbay/src/portmap/mod.rs b/patchbay/src/portmap/mod.rs new file mode 100644 index 0000000..3d42e78 --- /dev/null +++ b/patchbay/src/portmap/mod.rs @@ -0,0 +1,29 @@ +//! Port mapping server: UPnP IGD, NAT-PMP, and PCP. +//! +//! Patchbay routers can run an in-process port mapping server inside their +//! namespace that implements the three protocols common on consumer routers. +//! Devices on the downstream LAN can request external port mappings with any +//! supported protocol; granted mappings install nftables DNAT rules in a +//! dedicated `ip portmap` table so inbound WAN traffic reaches the device. +//! +//! The module is split into: +//! +//! - [`config`] for user-facing builder types. +//! - [`registry`] for the shared mapping registry and dedup logic. +//! - [`wire`] for server-side protocol encoders and decoders that match the +//! client-side wire format implemented in `portmapper`. +//! +//! Per-protocol server tasks (`nat_pmp`, `pcp`, `upnp`) live alongside once +//! implemented. The public API surface is intentionally small: [`PortmapMode`] +//! and [`PortmapConfig`] for configuration; everything else is `pub(crate)`. + +pub use config::{PortmapConfig, PortmapMode}; + +mod config; +// Internal types are consumed by the nftables and server-side code in later +// steps. The skeleton allows dead code so the registry can land as a +// reviewable unit with unit tests before its consumers exist. +#[allow(dead_code)] +pub(crate) mod registry; +#[allow(dead_code)] +pub(crate) mod wire; diff --git a/patchbay/src/portmap/registry.rs b/patchbay/src/portmap/registry.rs new file mode 100644 index 0000000..a6560c9 --- /dev/null +++ b/patchbay/src/portmap/registry.rs @@ -0,0 +1,430 @@ +//! Shared registry of active port mappings. +//! +//! A single registry per router tracks every mapping granted by any +//! supported protocol. The registry enforces uniqueness of the external port +//! per protocol and deduplicates repeated requests for the same internal +//! socket across protocols: when a client asks for a second mapping to an +//! already-mapped `(internal_ip, internal_port, proto)` the registry hands +//! back the existing external port instead of allocating a fresh one, which +//! matches miniupnpd and Apple Airport behavior. +//! +//! The registry does not touch nftables itself. Callers apply the DNAT rule +//! corresponding to a returned mapping via [`crate::nft`]; revocation is +//! symmetric. The separation keeps the registry fast (pure in-memory) and +//! testable without any root or netns setup. + +use std::{ + collections::HashMap, + net::Ipv4Addr, + num::NonZeroU16, + time::{Duration, Instant}, +}; + +/// IP transport protocol a mapping applies to. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(crate) enum MapProto { + /// UDP. + Udp, + /// TCP. + Tcp, +} + +/// External-side key for a mapping. +/// +/// Two protocols cannot simultaneously hold the same `(proto, ext_port)`. +/// Each external slot belongs to exactly one internal socket at a time. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(crate) struct MappingKey { + pub proto: MapProto, + pub external_port: NonZeroU16, +} + +/// A single active port mapping. +/// +/// The registry owns the authoritative copy. Protocol servers read or clone +/// it as needed to form responses. +#[derive(Clone, Debug)] +pub(crate) struct Mapping { + pub proto: MapProto, + pub external_port: NonZeroU16, + pub internal_ip: Ipv4Addr, + pub internal_port: NonZeroU16, + /// Absolute deadline at which the mapping expires and is eligible for + /// removal. Renewals push the deadline forward. + pub deadline: Instant, + /// PCP nonce, carried so `Map` renewals and releases authenticate. + /// `None` for mappings created via NAT-PMP or UPnP. + pub pcp_nonce: Option<[u8; 12]>, +} + +/// Outcome of an allocation request. +#[derive(Debug)] +pub(crate) enum AllocOutcome { + /// A new mapping was created. + Created(Mapping), + /// An existing mapping for this internal socket was renewed and + /// returned as-is (same external port). + Renewed(Mapping), + /// The specific external port requested is held by another internal + /// client. Callers should translate this into the protocol-specific + /// conflict error code. + Conflict, + /// The server exhausted its external port pool. + NoPortsAvailable, +} + +/// External port pool boundaries. +/// +/// Allocations walk `[low, high]` inclusive. Real gateways typically allocate +/// above 1024 to avoid privileged ports; the default mirrors that and stays +/// clear of the Linux ephemeral range default (32768-60999) so that test +/// ports do not collide with kernel-picked ephemerals. +pub(crate) const EXTERNAL_PORT_LOW: u16 = 1024; +pub(crate) const EXTERNAL_PORT_HIGH: u16 = 32767; + +/// In-memory mapping registry. +#[derive(Debug, Default)] +pub(crate) struct PortmapRegistry { + by_external: HashMap, + by_internal: HashMap<(Ipv4Addr, NonZeroU16, MapProto), NonZeroU16>, + /// Next port to try. Wraps within the `[LOW, HIGH]` range. + cursor: u16, +} + +impl PortmapRegistry { + /// Creates an empty registry. + pub(crate) fn new() -> Self { + Self { + by_external: HashMap::new(), + by_internal: HashMap::new(), + cursor: EXTERNAL_PORT_LOW, + } + } + + /// Returns the current mapping count. + pub(crate) fn len(&self) -> usize { + self.by_external.len() + } + + /// Looks up a mapping by its external slot. + pub(crate) fn get(&self, key: MappingKey) -> Option<&Mapping> { + self.by_external.get(&key) + } + + /// Iterates over all active mappings in insertion order of the + /// `by_external` map. The order is not stable across inserts, which is + /// acceptable: callers treat this as a bulk snapshot (for example on + /// teardown) rather than a sorted view. + pub(crate) fn iter(&self) -> impl Iterator { + self.by_external.values() + } + + /// Requests a mapping for `(internal_ip, internal_port, proto)`. + /// + /// `preferred_external` is the client's wish: `Some(p)` asks for a + /// specific port, `None` lets the server pick any. `lifetime` is the + /// requested lease; the deadline is computed as `Instant::now() + + /// lifetime`. Callers enforce protocol-specific bounds on the lifetime + /// before calling. + pub(crate) fn allocate( + &mut self, + proto: MapProto, + internal_ip: Ipv4Addr, + internal_port: NonZeroU16, + preferred_external: Option, + lifetime: Duration, + pcp_nonce: Option<[u8; 12]>, + ) -> AllocOutcome { + let deadline = Instant::now() + lifetime; + let internal_key = (internal_ip, internal_port, proto); + + // Dedup: re-requesting the same internal socket renews the existing + // mapping. We still honor preferred_external if it matches; a + // mismatched preferred_external is not a conflict as long as the + // current mapping is for the same internal socket. + if let Some(existing_port) = self.by_internal.get(&internal_key).copied() { + let key = MappingKey { + proto, + external_port: existing_port, + }; + if let Some(mapping) = self.by_external.get_mut(&key) { + mapping.deadline = deadline; + mapping.pcp_nonce = pcp_nonce.or(mapping.pcp_nonce); + return AllocOutcome::Renewed(mapping.clone()); + } + } + + // Honor the client's preferred external port if it's free. + if let Some(ext) = preferred_external { + let key = MappingKey { + proto, + external_port: ext, + }; + match self.by_external.get(&key) { + Some(existing) => { + let same_client = existing.internal_ip == internal_ip + && existing.internal_port == internal_port; + if same_client { + // Handled above; fall through to renewal. + } else { + return AllocOutcome::Conflict; + } + } + None => { + let mapping = Mapping { + proto, + external_port: ext, + internal_ip, + internal_port, + deadline, + pcp_nonce, + }; + self.insert(mapping.clone()); + return AllocOutcome::Created(mapping); + } + } + } + + // Walk the port range looking for a free slot. + let start = self.cursor.max(EXTERNAL_PORT_LOW); + let span = (EXTERNAL_PORT_HIGH - EXTERNAL_PORT_LOW + 1) as u32; + for i in 0..span { + let raw = + EXTERNAL_PORT_LOW + ((start as u32 - EXTERNAL_PORT_LOW as u32 + i) % span) as u16; + let candidate = NonZeroU16::new(raw).expect("port range excludes zero"); + let key = MappingKey { + proto, + external_port: candidate, + }; + if !self.by_external.contains_key(&key) { + let next = raw.checked_add(1).unwrap_or(EXTERNAL_PORT_LOW); + self.cursor = if next > EXTERNAL_PORT_HIGH { + EXTERNAL_PORT_LOW + } else { + next + }; + let mapping = Mapping { + proto, + external_port: candidate, + internal_ip, + internal_port, + deadline, + pcp_nonce, + }; + self.insert(mapping.clone()); + return AllocOutcome::Created(mapping); + } + } + + AllocOutcome::NoPortsAvailable + } + + /// Removes a mapping by external key. Returns the removed [`Mapping`], + /// or `None` if no such mapping existed. + pub(crate) fn remove(&mut self, key: MappingKey) -> Option { + let mapping = self.by_external.remove(&key)?; + let internal_key = (mapping.internal_ip, mapping.internal_port, mapping.proto); + self.by_internal.remove(&internal_key); + Some(mapping) + } + + /// Removes all mappings matching `(internal_ip, internal_port, proto)`. + /// Used when a client sends a NAT-PMP delete with `lifetime=0`. + pub(crate) fn remove_by_internal( + &mut self, + proto: MapProto, + internal_ip: Ipv4Addr, + internal_port: NonZeroU16, + ) -> Option { + let internal_key = (internal_ip, internal_port, proto); + let ext_port = self.by_internal.remove(&internal_key)?; + self.by_external.remove(&MappingKey { + proto, + external_port: ext_port, + }) + } + + /// Removes and returns every mapping whose deadline is at or before + /// `now`. Called periodically by the reaper. + pub(crate) fn reap_expired(&mut self, now: Instant) -> Vec { + let expired_keys: Vec = self + .by_external + .iter() + .filter(|(_, m)| m.deadline <= now) + .map(|(k, _)| *k) + .collect(); + expired_keys + .into_iter() + .filter_map(|k| self.remove(k)) + .collect() + } + + /// Drains every mapping. Used on server shutdown. + pub(crate) fn drain(&mut self) -> Vec { + self.by_internal.clear(); + self.by_external.drain().map(|(_, m)| m).collect() + } + + fn insert(&mut self, mapping: Mapping) { + let key = MappingKey { + proto: mapping.proto, + external_port: mapping.external_port, + }; + let internal_key = (mapping.internal_ip, mapping.internal_port, mapping.proto); + self.by_internal.insert(internal_key, mapping.external_port); + self.by_external.insert(key, mapping); + } +} + +#[cfg(test)] +mod tests { + use std::net::Ipv4Addr; + + use super::*; + + fn nz(p: u16) -> NonZeroU16 { + NonZeroU16::new(p).unwrap() + } + + fn lifetime() -> Duration { + Duration::from_secs(60) + } + + #[test] + fn allocate_picks_a_free_port() { + let mut reg = PortmapRegistry::new(); + let client = Ipv4Addr::new(10, 0, 0, 5); + let outcome = reg.allocate(MapProto::Udp, client, nz(1234), None, lifetime(), None); + match outcome { + AllocOutcome::Created(m) => { + assert_eq!(m.internal_ip, client); + assert_eq!(m.internal_port, nz(1234)); + assert!(m.external_port.get() >= EXTERNAL_PORT_LOW); + assert!(m.external_port.get() <= EXTERNAL_PORT_HIGH); + } + other => panic!("unexpected outcome: {other:?}"), + } + assert_eq!(reg.len(), 1); + } + + #[test] + fn allocate_honors_preferred_port() { + let mut reg = PortmapRegistry::new(); + let client = Ipv4Addr::new(10, 0, 0, 5); + let outcome = reg.allocate( + MapProto::Tcp, + client, + nz(80), + Some(nz(8080)), + lifetime(), + None, + ); + match outcome { + AllocOutcome::Created(m) => assert_eq!(m.external_port, nz(8080)), + other => panic!("unexpected outcome: {other:?}"), + } + } + + #[test] + fn allocate_dedups_same_internal_socket() { + let mut reg = PortmapRegistry::new(); + let client = Ipv4Addr::new(10, 0, 0, 5); + let first = reg.allocate(MapProto::Udp, client, nz(1234), None, lifetime(), None); + let first_port = match first { + AllocOutcome::Created(m) => m.external_port, + _ => panic!("expected Created"), + }; + let second = reg.allocate(MapProto::Udp, client, nz(1234), None, lifetime(), None); + match second { + AllocOutcome::Renewed(m) => assert_eq!(m.external_port, first_port), + other => panic!("expected Renewed, got {other:?}"), + } + assert_eq!(reg.len(), 1, "dedup should not add a second entry"); + } + + #[test] + fn allocate_rejects_port_held_by_other_client() { + let mut reg = PortmapRegistry::new(); + let a = Ipv4Addr::new(10, 0, 0, 5); + let b = Ipv4Addr::new(10, 0, 0, 6); + let _ = reg.allocate(MapProto::Tcp, a, nz(1111), Some(nz(9000)), lifetime(), None); + let outcome = reg.allocate(MapProto::Tcp, b, nz(2222), Some(nz(9000)), lifetime(), None); + assert!(matches!(outcome, AllocOutcome::Conflict)); + assert_eq!(reg.len(), 1); + } + + #[test] + fn allocate_different_protocols_can_share_port() { + let mut reg = PortmapRegistry::new(); + let client = Ipv4Addr::new(10, 0, 0, 5); + let udp = reg.allocate( + MapProto::Udp, + client, + nz(50), + Some(nz(7000)), + lifetime(), + None, + ); + let tcp = reg.allocate( + MapProto::Tcp, + client, + nz(50), + Some(nz(7000)), + lifetime(), + None, + ); + assert!( + matches!(udp, AllocOutcome::Created(_)) && matches!(tcp, AllocOutcome::Created(_)), + "UDP and TCP slots are independent", + ); + assert_eq!(reg.len(), 2); + } + + #[test] + fn remove_by_external_drops_both_indices() { + let mut reg = PortmapRegistry::new(); + let client = Ipv4Addr::new(10, 0, 0, 5); + let created = match reg.allocate(MapProto::Udp, client, nz(500), None, lifetime(), None) { + AllocOutcome::Created(m) => m, + _ => panic!("expected Created"), + }; + let removed = reg + .remove(MappingKey { + proto: MapProto::Udp, + external_port: created.external_port, + }) + .expect("mapping was present"); + assert_eq!(removed.internal_port, nz(500)); + assert!(reg.by_internal.is_empty()); + } + + #[test] + fn remove_by_internal_drops_same_mapping() { + let mut reg = PortmapRegistry::new(); + let client = Ipv4Addr::new(10, 0, 0, 5); + let _ = reg.allocate(MapProto::Udp, client, nz(500), None, lifetime(), None); + let removed = reg + .remove_by_internal(MapProto::Udp, client, nz(500)) + .expect("mapping was present"); + assert_eq!(removed.internal_ip, client); + assert_eq!(reg.len(), 0); + } + + #[test] + fn reap_expired_removes_elapsed_entries() { + let mut reg = PortmapRegistry::new(); + let client = Ipv4Addr::new(10, 0, 0, 5); + let _ = reg.allocate( + MapProto::Udp, + client, + nz(500), + None, + Duration::from_millis(1), + None, + ); + std::thread::sleep(Duration::from_millis(20)); + let reaped = reg.reap_expired(Instant::now()); + assert_eq!(reaped.len(), 1); + assert_eq!(reg.len(), 0); + } +} diff --git a/patchbay/src/portmap/wire.rs b/patchbay/src/portmap/wire.rs new file mode 100644 index 0000000..1376e3c --- /dev/null +++ b/patchbay/src/portmap/wire.rs @@ -0,0 +1,8 @@ +//! Server-side protocol wire formats. +//! +//! The `portmapper` crate implements the client side of NAT-PMP, PCP, and +//! UPnP IGD, but keeps its protocol types private. This module mirrors just +//! enough of that wire format on the server side for the patchbay router to +//! decode client requests and encode replies. Protocol-specific submodules +//! land alongside their server implementations (NAT-PMP in Step 3, PCP in +//! Step 4, UPnP in Step 5). diff --git a/worklogs/2026-04-23-portmapping.md b/worklogs/2026-04-23-portmapping.md new file mode 100644 index 0000000..15f773b --- /dev/null +++ b/worklogs/2026-04-23-portmapping.md @@ -0,0 +1,96 @@ +# Worklog: port mapping server on patchbay routers + +Started: 2026-04-23T12:34:28Z +Mode: overnight +Plan: plans/portmapping.md +Branch: upnp + +## Progress + +### 2026-04-23T12:34:00Z - organize phase + +Read `.agents/{workflow,writing,big-jobs,lang/rust,lib/patchbay}.md`. +Confirmed overnight mode rules: no pushing, no destructive ops, full +cycle per commit, no shortcuts. + +Read the portmapper client across `portmapper/src/{lib.rs, nat_pmp.rs, +nat_pmp/protocol/*, pcp.rs, pcp/protocol/*, upnp.rs, mapping.rs}` and +the `igd-next-0.17.0` SSDP + SOAP paths in +`~/.cargo/registry/src/index.crates.io-*/igd-next-0.17.0/src/common/*`. + +Inventoried patchbay's router surface: `router.rs` builder, `core.rs` +`RouterData`/`RouterConfig`, `nft.rs` rule generation, `wiring.rs` +namespace setup, `dns_server.rs` as a reference for an in-process +server attached to the router lifecycle. + +### 2026-04-23T12:45:00Z - plan written + +Wrote `plans/portmapping.md` with goal, context, eight-step approach, +risk register, and commit strategy. Plan lives as a checklist; will +tick off steps as each lands. + +## Staff reviews + +(none yet) + +## Blockers + +(none) + +## Decisions made + +- Portmap server lives in-process inside each router's namespace + (same pattern as `DnsServer`), not as a spawned `miniupnpd` process. + Rationale: no system packages required, deterministic lifecycle, + easy to inspect state from tests, matches project style. +- NAT-PMP and PCP share a single UDP 5351 socket, dispatched on the + version byte. Rationale: that is how real gateways behave per RFC + 6887 section 19, and the tests must see a single port. +- DNAT rules go into a dedicated chain inside the existing `ip nat` + table, not a separate table. Rationale: priority ordering with the + fullcone DNAT rule matters; easier to reason about one table. +- All three protocols are served out of a single `PortmapServer` + struct that owns the registry. Each protocol handler is a spawned + task on the router ns runtime, owning its share of the socket + setup. Rationale: one registry, one set of rules, consistent + locking. + +### 2026-04-23T13:05:00Z - adversarial plan review + +Spawned an adversarial reviewer against `plans/portmapping.md`. Three +criticals surfaced. All three led to real plan changes: + +- `set_nat_mode` flushes the `ip nat` table wholesale, so portmap + rules must live in a separate table. Plan now creates `ip portmap`. +- PCP ANNOUNCE on port 5350 is out of scope for the portmapper + client. Plan scopes PCP to unicast `Map` + `Announce` on 5351. +- Portmapper's client-side `ip_and_gateway()` may fall back to + loopback when `get_local_ipaddr()` returns none. Worked around by + ensuring test devices have a proper default route; tracked as a + likely `../net-tools` patch in Step 8. + +Substantive items folded in: UPnP description HTTP server binds +port 0 (no 49152 collision), `Home` preset stays default-off, DNAT +rule rejects internal IPs outside the downstream CIDR, UPnP lease=0 +means "permanent" per IGD1 not "delete", error code mapping table +required per-protocol. + +### 2026-04-23T13:25:00Z - Step 1 landed + +`patchbay/src/portmap/{mod,config,registry,wire}.rs` live. 11 unit +tests green, covering alloc, preferred-port, dedup, conflict, +protocol independence, remove by either index, and expiry. Public +API surface is `PortmapConfig` + `PortmapMode` only; internals are +`pub(crate)`. + +The module compiles with a temporary `#[allow(dead_code)]` on +`registry` and `wire` because no consumer exists yet. Step 2 will +consume the registry via the nftables helpers and remove the allow. + +Workspace lib tests: 205 passed, 0 failed. No clippy regressions +against `patchbay`. Pre-existing clippy warnings in `patchbay-runner` +are unchanged. + +## Summary + +(pending) From 00a3c88dda91ce670e8dae3f2dce65f8a18b98cd Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 14:51:28 +0200 Subject: [PATCH 02/13] feat: dedicated ip portmap nftables table Port mapping DNAT rules land in their own table at priority -110, one step before dstnat. The separation means Router::set_nat_mode can flush the ip nat table as it does today without wiping active port mappings. Rules match on ip daddr so hairpin traffic follows the same path as inbound WAN. Rendering is a pure function; unit tests cover empty, UDP, TCP, and multi-rule cases. The apply helper deletes the table before reinstalling so each call is an atomic swap. --- patchbay/src/portmap/mod.rs | 9 ++- patchbay/src/portmap/nft.rs | 146 ++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 patchbay/src/portmap/nft.rs diff --git a/patchbay/src/portmap/mod.rs b/patchbay/src/portmap/mod.rs index 3d42e78..b805d46 100644 --- a/patchbay/src/portmap/mod.rs +++ b/patchbay/src/portmap/mod.rs @@ -20,10 +20,13 @@ pub use config::{PortmapConfig, PortmapMode}; mod config; -// Internal types are consumed by the nftables and server-side code in later -// steps. The skeleton allows dead code so the registry can land as a -// reviewable unit with unit tests before its consumers exist. +#[allow(dead_code)] +pub(crate) mod nft; +// The registry is consumed by the nft module and by protocol servers. Its +// public surface spans more than the nft module uses today; the allow will +// go away once Step 3 wires in the NAT-PMP server. #[allow(dead_code)] pub(crate) mod registry; +// Protocol-specific wire handling lands in later steps. #[allow(dead_code)] pub(crate) mod wire; diff --git a/patchbay/src/portmap/nft.rs b/patchbay/src/portmap/nft.rs new file mode 100644 index 0000000..e345247 --- /dev/null +++ b/patchbay/src/portmap/nft.rs @@ -0,0 +1,146 @@ +//! nftables rules for the dedicated `ip portmap` table. +//! +//! Port mapping DNAT rules live in their own table, separate from the `ip +//! nat` table that [`crate::nft::apply_nat_for_router`] manages, because +//! [`crate::Router::set_nat_mode`] flushes the latter wholesale. Using a +//! separate table means runtime NAT reconfiguration never discards active +//! mappings. +//! +//! The table is re-rendered in full whenever the mapping set changes, so +//! the server keeps the authoritative mapping list in the registry and +//! lets nftables state be derived from it. This is simple, atomic when +//! applied through a single `nft -f` invocation, and avoids the complexity +//! of rule handles. + +use std::net::Ipv4Addr; + +use anyhow::Result; + +use super::registry::{MapProto, Mapping}; +use crate::{netns, nft::run_nft_in}; + +/// Generates the full `ip portmap` table body for the given mappings. +/// +/// Runs at nft priority `-110`, which sits 10 below the `dstnat` priority +/// (`-100`) used by [`crate::nft::apply_nat_for_router`]. That placement +/// guarantees a static port forward DNAT applies before the fullcone map +/// rule, and that the packet's changed destination no longer matches the +/// fullcone criteria downstream. +/// +/// The rule form is `ip daddr meta l4proto

dport dnat +/// to :`. Matching on `daddr` (rather than +/// `iif`) lets a LAN host hitting the router's WAN IP through hairpin +/// follow the same DNAT path as traffic arriving from the uplink. +pub(crate) fn generate_portmap_rules(wan_ip: Ipv4Addr, mappings: &[Mapping]) -> String { + let mut rules = String::new(); + rules.push_str("table ip portmap {\n"); + rules.push_str(" chain prerouting {\n"); + rules.push_str(" type nat hook prerouting priority -110; policy accept;\n"); + for m in mappings { + let proto = match m.proto { + MapProto::Udp => "udp", + MapProto::Tcp => "tcp", + }; + rules.push_str(&format!( + " ip daddr {wan} {proto} dport {ext} dnat to {ip}:{port}\n", + wan = wan_ip, + proto = proto, + ext = m.external_port.get(), + ip = m.internal_ip, + port = m.internal_port.get(), + )); + } + rules.push_str(" }\n"); + rules.push_str("}\n"); + rules +} + +/// Flushes and repopulates the `ip portmap` table in `ns`. +/// +/// The table is deleted if present, then re-declared with the new rule +/// set. `delete table` fails if the table does not exist; nft treats that +/// as an error and aborts the containing script, so the delete runs +/// separately and its error is swallowed before the fresh ruleset is +/// installed. Safe to call with an empty `mappings` slice, which leaves +/// the table declared but empty. +pub(crate) async fn apply_portmap_rules( + netns: &netns::NetnsManager, + ns: &str, + wan_ip: Ipv4Addr, + mappings: &[Mapping], +) -> Result<()> { + run_nft_in(netns, ns, "delete table ip portmap\n") + .await + .ok(); + run_nft_in(netns, ns, &generate_portmap_rules(wan_ip, mappings)).await +} + +/// Removes the `ip portmap` table entirely. Idempotent. +pub(crate) async fn clear_portmap_rules(netns: &netns::NetnsManager, ns: &str) -> Result<()> { + run_nft_in(netns, ns, "delete table ip portmap\n") + .await + .ok(); + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::{net::Ipv4Addr, num::NonZeroU16, time::Instant}; + + use super::*; + + fn mapping(proto: MapProto, ext: u16, internal_ip: [u8; 4], internal_port: u16) -> Mapping { + Mapping { + proto, + external_port: NonZeroU16::new(ext).unwrap(), + internal_ip: Ipv4Addr::from(internal_ip), + internal_port: NonZeroU16::new(internal_port).unwrap(), + deadline: Instant::now() + std::time::Duration::from_secs(60), + pcp_nonce: None, + } + } + + #[test] + fn empty_rules_render_declares_table_and_chain() { + let rendered = generate_portmap_rules(Ipv4Addr::new(198, 51, 100, 1), &[]); + assert!(rendered.contains("table ip portmap")); + assert!(rendered.contains("chain prerouting")); + assert!(rendered.contains("priority -110")); + // No DNAT rules when mappings is empty. + assert!(!rendered.contains("dnat")); + } + + #[test] + fn udp_mapping_renders_as_udp_dnat() { + let rendered = generate_portmap_rules( + Ipv4Addr::new(198, 51, 100, 1), + &[mapping(MapProto::Udp, 5000, [10, 0, 0, 5], 1234)], + ); + assert!(rendered.contains("ip daddr 198.51.100.1 udp dport 5000 dnat to 10.0.0.5:1234",)); + } + + #[test] + fn tcp_mapping_renders_as_tcp_dnat() { + let rendered = generate_portmap_rules( + Ipv4Addr::new(198, 51, 100, 1), + &[mapping(MapProto::Tcp, 8080, [10, 0, 0, 5], 80)], + ); + assert!(rendered.contains("tcp dport 8080 dnat to 10.0.0.5:80")); + } + + #[test] + fn multiple_mappings_render_independent_rules() { + let rendered = generate_portmap_rules( + Ipv4Addr::new(198, 51, 100, 1), + &[ + mapping(MapProto::Udp, 5000, [10, 0, 0, 5], 1234), + mapping(MapProto::Tcp, 8080, [10, 0, 0, 6], 80), + ], + ); + assert_eq!( + rendered.matches("dnat to").count(), + 2, + "two mappings produce two rules", + ); + } +} From 29aef4e2a516725de4f615eacb7fb6bc05507f13 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:07:36 +0200 Subject: [PATCH 03/13] feat: NAT-PMP server on routers RouterBuilder::portmap(PortmapMode) starts an in-process server inside the router namespace that listens on UDP 5351 per RFC 6886. Devices on the downstream LAN can request UDP or TCP mappings through any RFC 6886 client; portmapper's Client::probe() now reports nat_pmp=true and its update_local_port() yields an external (wan_ip, ext_port) that forwards inbound traffic to the requester. The server dispatches by version byte so PCP can land alongside in a later commit without a second socket. Off by default on every preset; callers opt in explicitly. Lifetime=0 deletes a mapping per section 3.3 of the RFC. Clients outside the downstream CIDR receive NotAuthorizedOrRefused. An APDF NAT filter would otherwise drop DNAT'd inbound flows; the filter chain now accepts packets with ct status dnat so static port forwards survive. Includes integration tests that use portmapper as a client and validate end-to-end inbound UDP delivery. --- Cargo.lock | 524 +++++++++++++++++++++++++++++++- patchbay/Cargo.toml | 1 + patchbay/src/core.rs | 7 + patchbay/src/lab.rs | 1 + patchbay/src/nft.rs | 7 +- patchbay/src/portmap/mod.rs | 8 +- patchbay/src/portmap/nat_pmp.rs | 376 +++++++++++++++++++++++ patchbay/src/portmap/nft.rs | 72 +++-- patchbay/src/portmap/server.rs | 202 ++++++++++++ patchbay/src/portmap/wire.rs | 8 - patchbay/src/router.rs | 55 ++++ patchbay/src/tests/mod.rs | 1 + patchbay/src/tests/portmap.rs | 176 +++++++++++ 13 files changed, 1392 insertions(+), 46 deletions(-) create mode 100644 patchbay/src/portmap/nat_pmp.rs create mode 100644 patchbay/src/portmap/server.rs delete mode 100644 patchbay/src/portmap/wire.rs create mode 100644 patchbay/src/tests/portmap.rs diff --git a/Cargo.lock b/Cargo.lock index e568467..31a46a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -206,6 +206,18 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" +[[package]] +name = "attohttpc" +version = "0.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16e2cdb6d5ed835199484bb92bb8b3edd526effe995c61732580439c1a67e2e9" +dependencies = [ + "base64", + "http", + "log", + "url", +] + [[package]] name = "autocfg" version = "1.5.0" @@ -339,6 +351,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "block2" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdeb9d870516001442e364c5220d3574d2da8dc765554b4a617230d33fa58ef5" +dependencies = [ + "objc2", +] + [[package]] name = "bumpalo" version = "3.20.2" @@ -412,6 +433,17 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" +[[package]] +name = "chacha20" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f8d983286843e49675a4b7a2d174efe136dc93a18d69130dd18198a6c167601" +dependencies = [ + "cfg-if", + "cpufeatures 0.3.0", + "rand_core 0.10.1", +] + [[package]] name = "chrono" version = "0.4.44" @@ -535,6 +567,15 @@ dependencies = [ "libc", ] +[[package]] +name = "cpufeatures" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b2a41393f66f16b0823bb79094d54ac5fbd34ab292ddafb9a0456ac9f87d201" +dependencies = [ + "libc", +] + [[package]] name = "crc32fast" version = "1.5.0" @@ -726,6 +767,18 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "dispatch2" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e0e367e4e7da84520dedcac1901e4da967309406d1e51017ae1abfb97adbd38" +dependencies = [ + "bitflags", + "block2", + "libc", + "objc2", +] + [[package]] name = "displaydoc" version = "0.2.5" @@ -737,6 +790,17 @@ dependencies = [ "syn", ] +[[package]] +name = "dlopen2" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e2c5bd4158e66d1e215c49b837e11d62f3267b30c92f1d171c4d3105e3dc4d4" +dependencies = [ + "libc", + "once_cell", + "winapi", +] + [[package]] name = "document-features" version = "0.2.12" @@ -938,6 +1002,19 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cecba35d7ad927e23624b22ad55235f2239cfa44fd10428eecbeba6d6a717718" +[[package]] +name = "futures-lite" +version = "2.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f78e10609fe0e0b3f4157ffab1876319b5b0db102a2c60dc4626306dc46b44ad" +dependencies = [ + "fastrand", + "futures-core", + "futures-io", + "parking", + "pin-project-lite", +] + [[package]] name = "futures-macro" version = "0.3.32" @@ -1039,6 +1116,7 @@ dependencies = [ "cfg-if", "libc", "r-efi 6.0.0", + "rand_core 0.10.1", "wasip2", "wasip3", ] @@ -1134,7 +1212,7 @@ dependencies = [ "idna", "ipnet", "once_cell", - "rand", + "rand 0.9.2", "ring", "thiserror 2.0.18", "tinyvec", @@ -1156,7 +1234,7 @@ dependencies = [ "moka", "once_cell", "parking_lot", - "rand", + "rand 0.9.2", "resolv-conf", "smallvec", "thiserror 2.0.18", @@ -1404,6 +1482,26 @@ dependencies = [ "icu_properties", ] +[[package]] +name = "igd-next" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bac9a3c8278f43b4cd8463380f4a25653ac843e5b177e1d3eaf849cc9ba10d4d" +dependencies = [ + "attohttpc", + "bytes", + "futures", + "http", + "http-body-util", + "hyper", + "hyper-util", + "log", + "rand 0.10.1", + "tokio", + "url", + "xmltree", +] + [[package]] name = "indexmap" version = "2.13.0" @@ -1608,6 +1706,12 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "112b39cec0b298b6c1999fee3e31427f74f676e4cb9879ed1a121b43661a4154" +[[package]] +name = "mac-addr" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3d25b0e0b648a86960ac23b7ad4abb9717601dec6f66c165f5b037f3f03065f" + [[package]] name = "matchers" version = "0.2.0" @@ -1700,6 +1804,27 @@ dependencies = [ "syn", ] +[[package]] +name = "n0-future" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2ab99dfb861450e68853d34ae665243a88b8c493d01ba957321a1e9b2312bbe" +dependencies = [ + "cfg_aliases", + "derive_more", + "futures-buffered", + "futures-lite", + "futures-util", + "js-sys", + "pin-project", + "send_wrapper", + "tokio", + "tokio-util", + "wasm-bindgen", + "wasm-bindgen-futures", + "web-time", +] + [[package]] name = "n0-tracing-test" version = "0.3.0" @@ -1721,6 +1846,39 @@ dependencies = [ "syn", ] +[[package]] +name = "n0-watcher" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38795f7932e6e9d1c6e989270ef5b3ff24ebb910e2c9d4bed2d28d8bae3007dc" +dependencies = [ + "derive_more", + "n0-error", + "n0-future", +] + +[[package]] +name = "netdev" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e30af1a5073b82356d9317c18226826370b4288eba2f71c7e84e18bae51b3847" +dependencies = [ + "block2", + "dispatch2", + "dlopen2", + "ipnet", + "libc", + "mac-addr", + "netlink-packet-core", + "netlink-packet-route 0.29.0", + "netlink-sys", + "objc2-core-foundation", + "objc2-system-configuration", + "once_cell", + "plist", + "windows-sys 0.61.2", +] + [[package]] name = "netlink-packet-core" version = "0.8.1" @@ -1742,6 +1900,30 @@ dependencies = [ "netlink-packet-core", ] +[[package]] +name = "netlink-packet-route" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df9854ea6ad14e3f4698a7f03b65bce0833dd2d81d594a0e4a984170537146b6" +dependencies = [ + "bitflags", + "libc", + "log", + "netlink-packet-core", +] + +[[package]] +name = "netlink-packet-route" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be8919612f6028ab4eacbbfe1234a9a43e3722c6e0915e7ff519066991905092" +dependencies = [ + "bitflags", + "libc", + "log", + "netlink-packet-core", +] + [[package]] name = "netlink-proto" version = "0.12.0" @@ -1769,6 +1951,40 @@ dependencies = [ "tokio", ] +[[package]] +name = "netwatch" +version = "0.16.0" +dependencies = [ + "atomic-waker", + "bytes", + "cfg_aliases", + "derive_more", + "js-sys", + "libc", + "n0-error", + "n0-future", + "n0-watcher", + "netdev", + "netlink-packet-core", + "netlink-packet-route 0.30.0", + "netlink-proto", + "netlink-sys", + "noq-udp", + "objc2-core-foundation", + "objc2-system-configuration", + "pin-project-lite", + "serde", + "socket2 0.6.3", + "time", + "tokio", + "tokio-util", + "tracing", + "web-sys", + "windows", + "windows-result", + "wmi", +] + [[package]] name = "nix" version = "0.30.1" @@ -1791,6 +2007,19 @@ dependencies = [ "minimal-lexical", ] +[[package]] +name = "noq-udp" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee91b05f4f3353290936ba1f3233518868fb4e2da99cb4c90d1f8cebb064e527" +dependencies = [ + "cfg_aliases", + "libc", + "socket2 0.6.3", + "tracing", + "windows-sys 0.61.2", +] + [[package]] name = "ntapi" version = "0.4.3" @@ -1843,6 +2072,81 @@ dependencies = [ "autocfg", ] +[[package]] +name = "num_enum" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d0bca838442ec211fa11de3a8b0e0e8f3a4522575b5c4c06ed722e005036f26" +dependencies = [ + "num_enum_derive", + "rustversion", +] + +[[package]] +name = "num_enum_derive" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "680998035259dcfcafe653688bf2aa6d3e2dc05e98be6ab46afb089dc84f1df8" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "objc2" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a12a8ed07aefc768292f076dc3ac8c48f3781c8f2d5851dd3d98950e8c5a89f" +dependencies = [ + "objc2-encode", +] + +[[package]] +name = "objc2-core-foundation" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a180dd8642fa45cdb7dd721cd4c11b1cadd4929ce112ebd8b9f5803cc79d536" +dependencies = [ + "bitflags", + "block2", + "dispatch2", + "libc", + "objc2", +] + +[[package]] +name = "objc2-encode" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef25abbcd74fb2609453eb695bd2f860d389e457f67dc17cafc8b8cbc89d0c33" + +[[package]] +name = "objc2-security" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "709fe137109bd1e8b5a99390f77a7d8b2961dafc1a1c5db8f2e60329ad6d895a" +dependencies = [ + "bitflags", + "objc2", + "objc2-core-foundation", +] + +[[package]] +name = "objc2-system-configuration" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7216bd11cbda54ccabcab84d523dc93b858ec75ecfb3a7d89513fa22464da396" +dependencies = [ + "bitflags", + "dispatch2", + "libc", + "objc2", + "objc2-core-foundation", + "objc2-security", +] + [[package]] name = "object" version = "0.37.3" @@ -1903,6 +2207,12 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" +[[package]] +name = "parking" +version = "2.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f38d5652c16fde515bb1ecef450ab0f6a219d619a7274976324d5e377f7dceba" + [[package]] name = "parking_lot" version = "0.12.5" @@ -1949,6 +2259,7 @@ dependencies = [ "libc", "n0-tracing-test", "nix", + "portmapper", "rtnetlink", "serde", "serde_json", @@ -2092,6 +2403,26 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" +[[package]] +name = "pin-project" +version = "1.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1749c7ed4bcaf4c3d0a3efc28538844fb29bcdd7d2b67b2be7e20ba861ff517" +dependencies = [ + "pin-project-internal", +] + +[[package]] +name = "pin-project-internal" +version = "1.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b20ed30f105399776b9c883e68e536ef602a16ae6f596d2c473591d6ad64c6" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "pin-project-lite" version = "0.2.17" @@ -2110,6 +2441,19 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4596b6d070b27117e987119b4dac604f3c58cfb0b191112e24771b2faeac1a6" +[[package]] +name = "plist" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "740ebea15c5d1428f910cd1a5f52cebf8d25006245ed8ade92702f4943d91e07" +dependencies = [ + "base64", + "indexmap", + "quick-xml", + "serde", + "time", +] + [[package]] name = "portable-atomic" version = "1.13.1" @@ -2119,6 +2463,34 @@ dependencies = [ "serde", ] +[[package]] +name = "portmapper" +version = "0.16.0" +dependencies = [ + "base64", + "bytes", + "derive_more", + "futures-lite", + "futures-util", + "hyper-util", + "igd-next", + "iroh-metrics", + "libc", + "n0-error", + "netwatch", + "num_enum", + "rand 0.10.1", + "serde", + "smallvec", + "socket2 0.6.3", + "time", + "tokio", + "tokio-util", + "tower-layer", + "tracing", + "url", +] + [[package]] name = "postcard" version = "1.1.3" @@ -2166,6 +2538,15 @@ dependencies = [ "syn", ] +[[package]] +name = "proc-macro-crate" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e67ba7e9b2b56446f1d419b1d807906278ffa1a658a8a5d8a39dcb1f5a78614f" +dependencies = [ + "toml_edit", +] + [[package]] name = "proc-macro2" version = "1.0.106" @@ -2175,6 +2556,15 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "quick-xml" +version = "0.38.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b66c2058c55a409d601666cffe35f04333cf1013010882cec174a7467cd4e21c" +dependencies = [ + "memchr", +] + [[package]] name = "quinn" version = "0.11.9" @@ -2204,7 +2594,7 @@ dependencies = [ "bytes", "getrandom 0.3.4", "lru-slab", - "rand", + "rand 0.9.2", "ring", "rustc-hash", "rustls", @@ -2258,7 +2648,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6db2770f06117d490610c7488547d543617b21bfa07796d7a12f6f1bd53850d1" dependencies = [ "rand_chacha", - "rand_core", + "rand_core 0.9.5", +] + +[[package]] +name = "rand" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2e8e8bcc7961af1fdac401278c6a831614941f6164ee3bf4ce61b7edb162207" +dependencies = [ + "chacha20", + "getrandom 0.4.2", + "rand_core 0.10.1", ] [[package]] @@ -2268,7 +2669,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb" dependencies = [ "ppv-lite86", - "rand_core", + "rand_core 0.9.5", ] [[package]] @@ -2280,6 +2681,12 @@ dependencies = [ "getrandom 0.3.4", ] +[[package]] +name = "rand_core" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" + [[package]] name = "rcgen" version = "0.13.2" @@ -2437,7 +2844,7 @@ dependencies = [ "futures-util", "log", "netlink-packet-core", - "netlink-packet-route", + "netlink-packet-route 0.28.0", "netlink-proto", "netlink-sys", "nix", @@ -2575,6 +2982,12 @@ dependencies = [ "serde_core", ] +[[package]] +name = "send_wrapper" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd0b0ec5f1c1ca621c432a25813d8d60c88abe6d3e08a3eb9cf37d97a0fe3d73" + [[package]] name = "serde" version = "1.0.228" @@ -2683,7 +3096,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" dependencies = [ "cfg-if", - "cpufeatures", + "cpufeatures 0.2.17", "digest", ] @@ -3126,6 +3539,18 @@ dependencies = [ "serde_core", ] +[[package]] +name = "toml_edit" +version = "0.25.4+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7193cbd0ce53dc966037f54351dbbcf0d5a642c7f0038c382ef9e677ce8c13f2" +dependencies = [ + "indexmap", + "toml_datetime", + "toml_parser", + "winnow", +] + [[package]] name = "toml_parser" version = "1.0.9+spec-1.1.0" @@ -3301,6 +3726,7 @@ dependencies = [ "idna", "percent-encoding", "serde", + "serde_derive", ] [[package]] @@ -3560,6 +3986,27 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.62.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "527fadee13e0c05939a6a05d5bd6eec6cd2e3dbd648b9f8e447c6518133d8580" +dependencies = [ + "windows-collections", + "windows-core", + "windows-future", + "windows-numerics", +] + +[[package]] +name = "windows-collections" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b2d95af1a8a14a3c7367e1ed4fc9c20e0a26e79551b1454d72583c97cc6610" +dependencies = [ + "windows-core", +] + [[package]] name = "windows-core" version = "0.62.2" @@ -3573,6 +4020,17 @@ dependencies = [ "windows-strings", ] +[[package]] +name = "windows-future" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1d6f90251fe18a279739e78025bd6ddc52a7e22f921070ccdc67dde84c605cb" +dependencies = [ + "windows-core", + "windows-link", + "windows-threading", +] + [[package]] name = "windows-implement" version = "0.60.2" @@ -3601,6 +4059,16 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-numerics" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e2e40844ac143cdb44aead537bbf727de9b044e107a0f1220392177d15b0f26" +dependencies = [ + "windows-core", + "windows-link", +] + [[package]] name = "windows-result" version = "0.4.1" @@ -3703,6 +4171,15 @@ dependencies = [ "windows_x86_64_msvc 0.53.1", ] +[[package]] +name = "windows-threading" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3949bd5b99cafdf1c7ca86b43ca564028dfe27d66958f2470940f73d86d75b37" +dependencies = [ + "windows-link", +] + [[package]] name = "windows_aarch64_gnullvm" version = "0.48.5" @@ -3846,6 +4323,9 @@ name = "winnow" version = "0.7.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" +dependencies = [ + "memchr", +] [[package]] name = "winreg" @@ -3945,6 +4425,21 @@ dependencies = [ "wasmparser", ] +[[package]] +name = "wmi" +version = "0.18.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c81b85c57a57500e56669586496bf2abd5cf082b9d32995251185d105208b64" +dependencies = [ + "chrono", + "futures", + "log", + "serde", + "thiserror 2.0.18", + "windows", + "windows-core", +] + [[package]] name = "writeable" version = "0.6.2" @@ -3996,6 +4491,21 @@ dependencies = [ "rustix", ] +[[package]] +name = "xml-rs" +version = "0.8.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ae8337f8a065cfc972643663ea4279e04e7256de865aa66fe25cec5fb912d3f" + +[[package]] +name = "xmltree" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7d8a75eaf6557bb84a65ace8609883db44a29951042ada9b393151532e41fcb" +dependencies = [ + "xml-rs", +] + [[package]] name = "yasna" version = "0.5.2" diff --git a/patchbay/Cargo.toml b/patchbay/Cargo.toml index f002f78..8e0fe90 100644 --- a/patchbay/Cargo.toml +++ b/patchbay/Cargo.toml @@ -37,4 +37,5 @@ ctor = "0.6" futures-buffered = "0.2" hickory-resolver = { version = "0.25", default-features = false, features = ["system-config", "tokio"] } n0-tracing-test = "0.3.0" +portmapper = { path = "../../net-tools/portmapper", default-features = false } testdir = "0.9" diff --git a/patchbay/src/core.rs b/patchbay/src/core.rs index 479e504..22405ad 100644 --- a/patchbay/src/core.rs +++ b/patchbay/src/core.rs @@ -95,6 +95,8 @@ pub(crate) struct RouterConfig { pub ra_interval_secs: u64, /// Router Advertisement lifetime in seconds. pub ra_lifetime_secs: u64, + /// Port mapping server protocols enabled on this router. + pub portmap: crate::portmap::PortmapConfig, } impl RouterConfig { @@ -280,6 +282,9 @@ pub(crate) struct RouterData { pub ra_runtime: Arc, /// Per-router operation lock — serializes multi-step mutations. pub op: Arc>, + /// Running portmap server, if any. Started at build time when the + /// builder enables any protocol; dropped on router removal. + pub portmap_server: Option, } impl RouterData { @@ -784,6 +789,7 @@ impl NetworkCore { ra_enabled: RA_DEFAULT_ENABLED, ra_interval_secs: RA_DEFAULT_INTERVAL_SECS, ra_lifetime_secs: RA_DEFAULT_LIFETIME_SECS, + portmap: crate::portmap::PortmapConfig::default(), }, downlink_bridge, uplink: None, @@ -802,6 +808,7 @@ impl NetworkCore { RA_DEFAULT_LIFETIME_SECS, )), op: Arc::new(tokio::sync::Mutex::new(())), + portmap_server: None, }, ); id diff --git a/patchbay/src/lab.rs b/patchbay/src/lab.rs index 3a2a4f5..c2978f0 100644 --- a/patchbay/src/lab.rs +++ b/patchbay/src/lab.rs @@ -1085,6 +1085,7 @@ impl Lab { ra_enabled: RA_DEFAULT_ENABLED, ra_interval_secs: RA_DEFAULT_INTERVAL_SECS, ra_lifetime_secs: RA_DEFAULT_LIFETIME_SECS, + portmap: crate::portmap::PortmapConfig::default(), result: Ok(()), } } diff --git a/patchbay/src/nft.rs b/patchbay/src/nft.rs index 8938859..f319cec 100644 --- a/patchbay/src/nft.rs +++ b/patchbay/src/nft.rs @@ -125,7 +125,11 @@ fn generate_nat_rules(cfg: &NatConfig, wan_if: &str, wan_ip: Ipv4Addr) -> String let postrouting_priority = if use_fullcone_map { "srcnat" } else { "100" }; - // APDF filter: only forward inbound packets matching existing conntrack flows. + // APDF filter: forward inbound packets only when they match an + // existing conntrack flow, a DNAT that the router itself installed + // (port mapping server), or a related flow. Static port forwards from + // the portmap table create `ct status dnat` packets that would + // otherwise be dropped by the blanket `iif "wan" drop` rule. let filter_table = if cfg.filtering == NatFiltering::AddressAndPortDependent { format!( r#" @@ -133,6 +137,7 @@ table ip filter {{ chain forward {{ type filter hook forward priority 0; policy accept; iif "{wan}" ct state established,related accept + iif "{wan}" ct status dnat accept iif "{wan}" drop }} }}"#, diff --git a/patchbay/src/portmap/mod.rs b/patchbay/src/portmap/mod.rs index b805d46..129d0e9 100644 --- a/patchbay/src/portmap/mod.rs +++ b/patchbay/src/portmap/mod.rs @@ -21,12 +21,10 @@ pub use config::{PortmapConfig, PortmapMode}; mod config; #[allow(dead_code)] +pub(crate) mod nat_pmp; +#[allow(dead_code)] pub(crate) mod nft; -// The registry is consumed by the nft module and by protocol servers. Its -// public surface spans more than the nft module uses today; the allow will -// go away once Step 3 wires in the NAT-PMP server. #[allow(dead_code)] pub(crate) mod registry; -// Protocol-specific wire handling lands in later steps. #[allow(dead_code)] -pub(crate) mod wire; +pub(crate) mod server; diff --git a/patchbay/src/portmap/nat_pmp.rs b/patchbay/src/portmap/nat_pmp.rs new file mode 100644 index 0000000..3d219d6 --- /dev/null +++ b/patchbay/src/portmap/nat_pmp.rs @@ -0,0 +1,376 @@ +//! NAT-PMP server (RFC 6886) bound to UDP 5351 inside the router namespace. +//! +//! The server accepts `DetermineExternalAddress` and `MapUdp`/`MapTcp` +//! requests, installs [`MappingKey`](super::registry::MappingKey) entries in +//! the shared registry, and emits nftables DNAT rules through +//! [`super::nft::apply_portmap_rules`]. A request is rejected with +//! `NotAuthorizedOrRefused` when the client's source IP is outside the +//! router's downstream CIDR. Lifetime=0 deletes the matching mapping per +//! RFC 6886 section 3.3. +//! +//! PCP (RFC 6887) shares UDP 5351 with NAT-PMP. The shared socket and +//! dispatch-by-version-byte live in [`super::server`]; this module only +//! handles packets whose version byte is `0`. + +use std::{ + net::Ipv4Addr, + num::NonZeroU16, + sync::Arc, + time::{Duration, Instant}, +}; + +use ipnet::Ipv4Net; +use tokio::sync::Mutex; +use tracing::{debug, trace, warn}; + +use super::{ + nft, + registry::{AllocOutcome, MapProto, PortmapRegistry}, +}; +use crate::netns::NetnsManager; + +/// NAT-PMP wire-format version byte. +pub(crate) const VERSION: u8 = 0; + +/// Server port per RFC 6886. Shared with PCP. +pub(crate) const SERVER_PORT: u16 = 5351; + +/// Response indicator ORed into the opcode field. +const RESPONSE_INDICATOR: u8 = 0x80; + +#[repr(u16)] +#[derive(Clone, Copy, Debug)] +enum ResultCode { + Success = 0, + #[allow(dead_code)] + UnsupportedVersion = 1, + NotAuthorizedOrRefused = 2, + OutOfResources = 4, + UnsupportedOpcode = 5, +} + +#[repr(u8)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Opcode { + DetermineExternalAddress = 0, + MapUdp = 1, + MapTcp = 2, +} + +impl Opcode { + fn from_byte(b: u8) -> Option { + match b { + 0 => Some(Self::DetermineExternalAddress), + 1 => Some(Self::MapUdp), + 2 => Some(Self::MapTcp), + _ => None, + } + } +} + +/// Parsed NAT-PMP request. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Request { + ExternalAddress, + Map { + proto: MapProto, + local_port: u16, + external_port: u16, + lifetime_seconds: u32, + }, +} + +impl Request { + pub(crate) fn decode(buf: &[u8]) -> Option { + if buf.len() < 2 || buf[0] != VERSION { + return None; + } + match Opcode::from_byte(buf[1])? { + Opcode::DetermineExternalAddress => Some(Request::ExternalAddress), + op @ (Opcode::MapUdp | Opcode::MapTcp) => { + if buf.len() < 12 { + return None; + } + let local_port = u16::from_be_bytes([buf[4], buf[5]]); + let external_port = u16::from_be_bytes([buf[6], buf[7]]); + let lifetime_seconds = u32::from_be_bytes([buf[8], buf[9], buf[10], buf[11]]); + let proto = if op == Opcode::MapUdp { + MapProto::Udp + } else { + MapProto::Tcp + }; + Some(Request::Map { + proto, + local_port, + external_port, + lifetime_seconds, + }) + } + } + } +} + +/// Encodes a `DetermineExternalAddress` response. +fn encode_external_response(result: ResultCode, epoch_time: u32, public_ip: Ipv4Addr) -> Vec { + let mut buf = Vec::with_capacity(12); + buf.push(VERSION); + buf.push(RESPONSE_INDICATOR | Opcode::DetermineExternalAddress as u8); + buf.extend_from_slice(&(result as u16).to_be_bytes()); + buf.extend_from_slice(&epoch_time.to_be_bytes()); + buf.extend_from_slice(&public_ip.octets()); + buf +} + +/// Encodes a `MapUdp`/`MapTcp` response. +fn encode_map_response( + proto: MapProto, + result: ResultCode, + epoch_time: u32, + private_port: u16, + external_port: u16, + lifetime_seconds: u32, +) -> Vec { + let mut buf = Vec::with_capacity(16); + let op_byte = match proto { + MapProto::Udp => Opcode::MapUdp as u8, + MapProto::Tcp => Opcode::MapTcp as u8, + }; + buf.push(VERSION); + buf.push(RESPONSE_INDICATOR | op_byte); + buf.extend_from_slice(&(result as u16).to_be_bytes()); + buf.extend_from_slice(&epoch_time.to_be_bytes()); + buf.extend_from_slice(&private_port.to_be_bytes()); + buf.extend_from_slice(&external_port.to_be_bytes()); + buf.extend_from_slice(&lifetime_seconds.to_be_bytes()); + buf +} + +/// Context shared between the dispatch loop and the handlers. +/// +/// `epoch` is the router-local epoch zero: response `epoch_time` fields +/// report seconds since this instant. Real routers use seconds since the +/// SSDP/NAT-PMP service started; the test harness only needs this value to +/// be monotonic and non-zero. +pub(crate) struct Context { + pub registry: Arc>, + pub netns: Arc, + pub ns: Arc, + pub wan_ip: Ipv4Addr, + pub downstream_cidr: Ipv4Net, + pub epoch: Instant, + /// Clamp for requested lifetimes. Matches the recommended 2-hour + /// maximum from RFC 6886 section 3.3. + pub max_lifetime: Duration, +} + +impl Context { + fn epoch_time(&self) -> u32 { + self.epoch.elapsed().as_secs() as u32 + } +} + +/// Handles a single NAT-PMP request and returns the response bytes. +pub(crate) async fn handle_request( + ctx: &Context, + client_ip: Ipv4Addr, + packet: &[u8], +) -> Option> { + let request = match Request::decode(packet) { + Some(r) => r, + None => { + trace!(?client_ip, len = packet.len(), "nat-pmp: invalid request"); + return None; + } + }; + + if !ctx.downstream_cidr.contains(&client_ip) { + debug!( + ?client_ip, + cidr = %ctx.downstream_cidr, + "nat-pmp: client not on downstream subnet" + ); + return Some(match request { + Request::ExternalAddress => encode_external_response( + ResultCode::NotAuthorizedOrRefused, + ctx.epoch_time(), + Ipv4Addr::UNSPECIFIED, + ), + Request::Map { proto, .. } => encode_map_response( + proto, + ResultCode::NotAuthorizedOrRefused, + ctx.epoch_time(), + 0, + 0, + 0, + ), + }); + } + + match request { + Request::ExternalAddress => Some(encode_external_response( + ResultCode::Success, + ctx.epoch_time(), + ctx.wan_ip, + )), + Request::Map { + proto, + local_port, + external_port, + lifetime_seconds, + } => Some( + handle_map( + ctx, + client_ip, + proto, + local_port, + external_port, + lifetime_seconds, + ) + .await, + ), + } +} + +async fn handle_map( + ctx: &Context, + client_ip: Ipv4Addr, + proto: MapProto, + local_port: u16, + external_port: u16, + requested_lifetime: u32, +) -> Vec { + // Local port 0 is an error condition for maps per RFC 6886. + let local = match NonZeroU16::new(local_port) { + Some(p) => p, + None => { + return encode_map_response( + proto, + ResultCode::UnsupportedOpcode, + ctx.epoch_time(), + local_port, + 0, + 0, + ); + } + }; + + // Lifetime 0 is the documented delete semantics. + if requested_lifetime == 0 { + let mut registry = ctx.registry.lock().await; + registry.remove_by_internal(proto, client_ip, local); + let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); + drop(registry); + if let Err(e) = + nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await + { + warn!(error = %e, "nat-pmp: failed to apply portmap rules on delete"); + } + // RFC 6886 section 3.3: reply with the requested local port, external port 0, lifetime 0. + return encode_map_response( + proto, + ResultCode::Success, + ctx.epoch_time(), + local.get(), + 0, + 0, + ); + } + + let lifetime = Duration::from_secs(requested_lifetime.into()).min(ctx.max_lifetime); + let preferred = NonZeroU16::new(external_port); + + let mut registry = ctx.registry.lock().await; + let outcome = registry.allocate(proto, client_ip, local, preferred, lifetime, None); + let (result, granted_external, granted_lifetime) = match outcome { + AllocOutcome::Created(ref m) | AllocOutcome::Renewed(ref m) => ( + ResultCode::Success, + m.external_port.get(), + lifetime.as_secs() as u32, + ), + AllocOutcome::Conflict | AllocOutcome::NoPortsAvailable => { + (ResultCode::OutOfResources, 0, 0) + } + }; + let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); + drop(registry); + + if matches!(outcome, AllocOutcome::Created(_) | AllocOutcome::Renewed(_)) { + if let Err(e) = + nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await + { + warn!(error = %e, "nat-pmp: failed to apply portmap rules"); + } + } + + encode_map_response( + proto, + result, + ctx.epoch_time(), + local.get(), + granted_external, + granted_lifetime, + ) +} + +#[cfg(test)] +mod tests { + use std::net::Ipv4Addr; + + use super::*; + + #[test] + fn decode_external_address_request() { + let req = Request::decode(&[0, 0]).unwrap(); + assert_eq!(req, Request::ExternalAddress); + } + + #[test] + fn decode_map_udp_request() { + // version=0, opcode=1, reserved=0,0, local=1234 (0x04D2), + // external=5678 (0x162E), lifetime=60 (0x3C). + let packet = [0, 1, 0, 0, 0x04, 0xD2, 0x16, 0x2E, 0, 0, 0, 60]; + let req = Request::decode(&packet).unwrap(); + assert_eq!( + req, + Request::Map { + proto: MapProto::Udp, + local_port: 1234, + external_port: 5678, + lifetime_seconds: 60, + } + ); + } + + #[test] + fn decode_wrong_version_fails() { + assert!(Request::decode(&[1, 0]).is_none()); + } + + #[test] + fn decode_short_packet_fails() { + assert!(Request::decode(&[0]).is_none()); + assert!(Request::decode(&[0, 1, 0, 0]).is_none()); + } + + #[test] + fn encode_external_response_matches_portmapper_decoder() { + let buf = encode_external_response(ResultCode::Success, 42, Ipv4Addr::new(1, 2, 3, 4)); + assert_eq!(buf.len(), 12); + assert_eq!(buf[0], 0); + assert_eq!(buf[1], 0x80); + assert_eq!(u16::from_be_bytes([buf[2], buf[3]]), 0); + assert_eq!(u32::from_be_bytes([buf[4], buf[5], buf[6], buf[7]]), 42); + assert_eq!(&buf[8..12], &[1, 2, 3, 4]); + } + + #[test] + fn encode_map_response_carries_ports_and_lifetime() { + let buf = encode_map_response(MapProto::Tcp, ResultCode::Success, 0, 80, 8080, 60); + assert_eq!(buf.len(), 16); + assert_eq!(buf[0], 0); + assert_eq!(buf[1], 0x80 | 2); + assert_eq!(u16::from_be_bytes([buf[8], buf[9]]), 80); + assert_eq!(u16::from_be_bytes([buf[10], buf[11]]), 8080); + assert_eq!(u32::from_be_bytes([buf[12], buf[13], buf[14], buf[15]]), 60); + } +} diff --git a/patchbay/src/portmap/nft.rs b/patchbay/src/portmap/nft.rs index e345247..b294248 100644 --- a/patchbay/src/portmap/nft.rs +++ b/patchbay/src/portmap/nft.rs @@ -21,16 +21,20 @@ use crate::{netns, nft::run_nft_in}; /// Generates the full `ip portmap` table body for the given mappings. /// -/// Runs at nft priority `-110`, which sits 10 below the `dstnat` priority -/// (`-100`) used by [`crate::nft::apply_nat_for_router`]. That placement -/// guarantees a static port forward DNAT applies before the fullcone map -/// rule, and that the packet's changed destination no longer matches the -/// fullcone criteria downstream. +/// The table has two chains. The prerouting chain at priority `-110` +/// (10 below `dstnat` at `-100`, used by +/// [`crate::nft::apply_nat_for_router`]) rewrites the destination of +/// inbound packets matching `(ip daddr , proto, dport )` to +/// `:`. Matching on `daddr` rather than `iif` +/// lets a LAN host hitting the router's WAN IP through hairpin follow the +/// same DNAT path as traffic arriving from the uplink. /// -/// The rule form is `ip daddr meta l4proto

dport dnat -/// to :`. Matching on `daddr` (rather than -/// `iif`) lets a LAN host hitting the router's WAN IP through hairpin -/// follow the same DNAT path as traffic arriving from the uplink. +/// The forward chain at priority `-10` (below the APDF filter produced +/// by the NAT config at `0`) accepts any packet whose conntrack entry +/// has been DNAT'd and whose destination matches a current mapping. This +/// is necessary because the Home NAT filter drops `NEW` inbound flows on +/// the WAN interface; without this chain the DNAT succeeds but the +/// forwarded packet is dropped before it reaches the internal host. pub(crate) fn generate_portmap_rules(wan_ip: Ipv4Addr, mappings: &[Mapping]) -> String { let mut rules = String::new(); rules.push_str("table ip portmap {\n"); @@ -51,36 +55,47 @@ pub(crate) fn generate_portmap_rules(wan_ip: Ipv4Addr, mappings: &[Mapping]) -> )); } rules.push_str(" }\n"); + rules.push_str(" chain forward {\n"); + rules.push_str(" type filter hook forward priority -10; policy accept;\n"); + for m in mappings { + let proto = match m.proto { + MapProto::Udp => "udp", + MapProto::Tcp => "tcp", + }; + rules.push_str(&format!( + " ip daddr {ip} {proto} dport {port} ct status dnat accept\n", + proto = proto, + ip = m.internal_ip, + port = m.internal_port.get(), + )); + } + rules.push_str(" }\n"); rules.push_str("}\n"); rules } /// Flushes and repopulates the `ip portmap` table in `ns`. /// -/// The table is deleted if present, then re-declared with the new rule -/// set. `delete table` fails if the table does not exist; nft treats that -/// as an error and aborts the containing script, so the delete runs -/// separately and its error is swallowed before the fresh ruleset is -/// installed. Safe to call with an empty `mappings` slice, which leaves -/// the table declared but empty. +/// The script prepends `add table ip portmap` (idempotent: noop if the +/// table already exists) followed by `flush table ip portmap` so the +/// update is atomic from a single `nft -f` invocation. Safe to call with +/// an empty `mappings` slice, which leaves the table declared but empty. pub(crate) async fn apply_portmap_rules( netns: &netns::NetnsManager, ns: &str, wan_ip: Ipv4Addr, mappings: &[Mapping], ) -> Result<()> { - run_nft_in(netns, ns, "delete table ip portmap\n") - .await - .ok(); - run_nft_in(netns, ns, &generate_portmap_rules(wan_ip, mappings)).await + let mut script = String::from("add table ip portmap\nflush table ip portmap\n"); + script.push_str(&generate_portmap_rules(wan_ip, mappings)); + run_nft_in(netns, ns, &script).await } -/// Removes the `ip portmap` table entirely. Idempotent. +/// Removes the `ip portmap` table entirely. Idempotent: missing table is +/// swallowed as a success. pub(crate) async fn clear_portmap_rules(netns: &netns::NetnsManager, ns: &str) -> Result<()> { - run_nft_in(netns, ns, "delete table ip portmap\n") - .await - .ok(); - Ok(()) + // `add table` makes the delete idempotent in a single script. + run_nft_in(netns, ns, "add table ip portmap\ndelete table ip portmap\n").await } #[cfg(test)] @@ -106,6 +121,8 @@ mod tests { assert!(rendered.contains("table ip portmap")); assert!(rendered.contains("chain prerouting")); assert!(rendered.contains("priority -110")); + assert!(rendered.contains("chain forward")); + assert!(rendered.contains("priority -10")); // No DNAT rules when mappings is empty. assert!(!rendered.contains("dnat")); } @@ -140,7 +157,12 @@ mod tests { assert_eq!( rendered.matches("dnat to").count(), 2, - "two mappings produce two rules", + "two mappings produce two DNAT rules", + ); + assert_eq!( + rendered.matches("ct status dnat accept").count(), + 2, + "two mappings produce two forward-accept rules", ); } } diff --git a/patchbay/src/portmap/server.rs b/patchbay/src/portmap/server.rs new file mode 100644 index 0000000..91a05f0 --- /dev/null +++ b/patchbay/src/portmap/server.rs @@ -0,0 +1,202 @@ +//! Port mapping server orchestration. +//! +//! [`PortmapServer`] owns the UDP 5351 socket shared between NAT-PMP and +//! PCP, the shared mapping registry, and the per-protocol dispatch task. +//! UPnP lands alongside in Step 5; until then only the NAT-PMP dispatcher +//! is wired in. +//! +//! The server follows the same lifecycle pattern as +//! [`crate::dns_server::DnsServer`]: a cloneable handle backed by +//! `Arc>` so the task dies when every clone drops. +//! Explicit shutdown via [`PortmapServer::shutdown`] runs the nft cleanup +//! before releasing the registry, ensuring the router's namespace does not +//! retain orphan DNAT rules after the server goes away. + +use std::{ + net::{Ipv4Addr, SocketAddr, SocketAddrV4}, + sync::Arc, + time::{Duration, Instant}, +}; + +use anyhow::{Context as _, Result}; +use ipnet::Ipv4Net; +use tokio::{net::UdpSocket, sync::Mutex}; +use tokio_util::task::AbortOnDropHandle; +use tracing::{debug, warn}; + +use super::{config::PortmapConfig, nat_pmp, nft, registry::PortmapRegistry}; +use crate::netns::NetnsManager; + +/// Maximum lifetime a client may request, in seconds. Mirrors the +/// recommendation from RFC 6886 section 3.3. +const MAX_LIFETIME_SECS: u64 = 2 * 60 * 60; + +/// Cloneable handle to a per-router portmap server. +#[derive(Clone)] +pub(crate) struct PortmapServer { + inner: Arc, +} + +struct PortmapServerInner { + cfg: PortmapConfig, + registry: Arc>, + netns: Arc, + ns: Arc, + wan_ip: Ipv4Addr, + _task: AbortOnDropHandle<()>, +} + +impl std::fmt::Debug for PortmapServer { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PortmapServer") + .field("cfg", &self.inner.cfg) + .field("ns", &self.inner.ns) + .field("wan_ip", &self.inner.wan_ip) + .finish() + } +} + +impl PortmapServer { + /// Starts the portmap server inside `ns`. + /// + /// Binds UDP 5351 on `downstream_gw` (the router's LAN gateway IP). + /// Both NAT-PMP and PCP share that socket once PCP lands; today only + /// NAT-PMP is wired through. `wan_ip` is reported to clients as the + /// external address, and `downstream_cidr` is the authorization check + /// for the client source address. + pub(crate) async fn start( + netns: Arc, + ns: Arc, + cfg: PortmapConfig, + downstream_gw: Ipv4Addr, + wan_ip: Ipv4Addr, + downstream_cidr: Ipv4Net, + ) -> Result { + let registry = Arc::new(Mutex::new(PortmapRegistry::new())); + + // Wipe any stale table from an earlier run before starting: the + // router namespace may have been recycled. + nft::apply_portmap_rules(&netns, &ns, wan_ip, &[]) + .await + .ok(); + + let task = spawn_dispatch( + netns.clone(), + ns.clone(), + cfg, + registry.clone(), + downstream_gw, + wan_ip, + downstream_cidr, + )?; + + Ok(Self { + inner: Arc::new(PortmapServerInner { + cfg, + registry, + netns, + ns, + wan_ip, + _task: task, + }), + }) + } + + /// Returns the configuration the server was started with. + #[allow(dead_code)] + pub(crate) fn config(&self) -> PortmapConfig { + self.inner.cfg + } + + /// Returns the current mapping count. Intended for tests and metrics. + #[allow(dead_code)] + pub(crate) async fn mapping_count(&self) -> usize { + self.inner.registry.lock().await.len() + } + + /// Tears down the server's nftables table. Callers should call this + /// before dropping the last handle so the rules do not outlive the + /// server. Missing rules are not an error: the helper is idempotent. + pub(crate) async fn shutdown(&self) -> Result<()> { + nft::clear_portmap_rules(&self.inner.netns, &self.inner.ns).await + } +} + +fn spawn_dispatch( + netns: Arc, + ns: Arc, + cfg: PortmapConfig, + registry: Arc>, + downstream_gw: Ipv4Addr, + wan_ip: Ipv4Addr, + downstream_cidr: Ipv4Net, +) -> Result> { + let std_socket: std::net::UdpSocket = netns.run_closure_in(&ns, move || { + let addr = SocketAddrV4::new(downstream_gw, nat_pmp::SERVER_PORT); + let sock = std::net::UdpSocket::bind(addr) + .with_context(|| format!("bind portmap server to {addr}"))?; + sock.set_nonblocking(true)?; + Ok(sock) + })?; + + let rt = netns.rt_handle_for(&ns)?; + let ctx = Arc::new(nat_pmp::Context { + registry, + netns: netns.clone(), + ns: ns.clone(), + wan_ip, + downstream_cidr, + epoch: Instant::now(), + max_lifetime: Duration::from_secs(MAX_LIFETIME_SECS), + }); + let handle = rt.spawn(async move { + let socket = match UdpSocket::from_std(std_socket) { + Ok(s) => s, + Err(e) => { + warn!(error = %e, "portmap: convert UDP socket"); + return; + } + }; + debug!( + ns = %ctx.ns, + gw = %downstream_gw, + port = nat_pmp::SERVER_PORT, + "portmap server listening" + ); + dispatch_loop(socket, ctx, cfg).await; + }); + + Ok(AbortOnDropHandle::new(handle)) +} + +async fn dispatch_loop(socket: UdpSocket, ctx: Arc, cfg: PortmapConfig) { + let mut buf = vec![0u8; 1500]; + loop { + let (len, src) = match socket.recv_from(&mut buf).await { + Ok(v) => v, + Err(e) => { + warn!(error = %e, "portmap: recv error"); + continue; + } + }; + let packet = &buf[..len]; + let version = packet.first().copied().unwrap_or(u8::MAX); + let response = match version { + nat_pmp::VERSION if cfg.enable_nat_pmp => { + let SocketAddr::V4(src_v4) = src else { + continue; + }; + nat_pmp::handle_request(&ctx, *src_v4.ip(), packet).await + } + // PCP wire version is 2; handled in a later step. For now the + // server silently drops PCP packets when PCP is disabled and + // responds with "unsupported version" once PCP is added. + _ => None, + }; + if let Some(bytes) = response { + if let Err(e) = socket.send_to(&bytes, src).await { + warn!(error = %e, "portmap: send error"); + } + } + } +} diff --git a/patchbay/src/portmap/wire.rs b/patchbay/src/portmap/wire.rs deleted file mode 100644 index 1376e3c..0000000 --- a/patchbay/src/portmap/wire.rs +++ /dev/null @@ -1,8 +0,0 @@ -//! Server-side protocol wire formats. -//! -//! The `portmapper` crate implements the client side of NAT-PMP, PCP, and -//! UPnP IGD, but keeps its protocol types private. This module mirrors just -//! enough of that wire format on the server side for the patchbay router to -//! decode client requests and encode replies. Protocol-specific submodules -//! land alongside their server implementations (NAT-PMP in Step 3, PCP in -//! Step 4, UPnP in Step 5). diff --git a/patchbay/src/router.rs b/patchbay/src/router.rs index c3b9a99..5d06743 100644 --- a/patchbay/src/router.rs +++ b/patchbay/src/router.rs @@ -27,6 +27,7 @@ use crate::{ apply_firewall, apply_nat_for_router, apply_nat_v6, apply_or_remove_impair, remove_firewall, run_nft_in, }, + portmap::{self, PortmapConfig, PortmapMode}, wiring::{self, setup_router_async, RouterSetupData}, }; @@ -881,6 +882,7 @@ pub struct RouterBuilder { pub(crate) ra_enabled: bool, pub(crate) ra_interval_secs: u64, pub(crate) ra_lifetime_secs: u64, + pub(crate) portmap: PortmapConfig, pub(crate) result: Result<()>, } @@ -910,6 +912,7 @@ impl RouterBuilder { ra_enabled: RA_DEFAULT_ENABLED, ra_interval_secs: RA_DEFAULT_INTERVAL_SECS, ra_lifetime_secs: RA_DEFAULT_LIFETIME_SECS, + portmap: PortmapConfig::default(), result: Err(err), } } @@ -1089,6 +1092,22 @@ impl RouterBuilder { self } + /// Enables the port mapping server on this router. + /// + /// Off by default on every [`RouterPreset`]. Passing + /// [`PortmapMode::All`] advertises NAT-PMP, PCP, and UPnP IGD; + /// narrower modes select individual protocols. Clients inside the + /// router's downstream subnet can then obtain port mappings through + /// the `portmapper` crate or any RFC 6886 / 6887 / UPnP IGD:1 client. + /// + /// See [`Router::set_portmap`] for runtime reconfiguration. + pub fn portmap(mut self, mode: PortmapMode) -> Self { + if self.result.is_ok() { + self.portmap = PortmapConfig::from_mode(mode); + } + self + } + /// Finalizes the router, creates its namespace and links, and returns a [`Router`] handle. pub async fn build(self) -> Result { self.result?; @@ -1118,6 +1137,7 @@ impl RouterBuilder { r.cfg.ra_enabled = self.ra_enabled; r.cfg.ra_interval_secs = self.ra_interval_secs.max(1); r.cfg.ra_lifetime_secs = self.ra_lifetime_secs; + r.cfg.portmap = self.portmap; r.ra_runtime.set_enabled(self.ra_enabled); r.ra_runtime.set_interval_secs(self.ra_interval_secs); r.ra_runtime.set_lifetime_secs(self.ra_lifetime_secs); @@ -1351,6 +1371,41 @@ impl RouterBuilder { .instrument(self.lab_span.clone()) .await?; + // Phase 2b: Start the portmap server if any protocol is enabled. + // Needs the downstream gateway, downstream CIDR, and upstream IP, + // all of which are known only after setup_router_async assigns + // addresses. Captures of those fields are made under the core lock + // in the next block so they race-free with any concurrent + // reconfiguration. + if self.portmap.any_enabled() { + let (ns, downstream_gw, wan_ip, downstream_cidr) = { + let inner = self.inner.core.lock().unwrap(); + let r = inner.router(id).ok_or_else(|| anyhow!("router removed"))?; + let gw = r + .downstream_gw + .ok_or_else(|| anyhow!("portmap requires an IPv4 downstream gateway"))?; + let ip = r + .upstream_ip + .ok_or_else(|| anyhow!("portmap requires an IPv4 uplink IP"))?; + let cidr = r + .downstream_cidr + .ok_or_else(|| anyhow!("portmap requires an IPv4 downstream CIDR"))?; + (r.ns.clone(), gw, ip, cidr) + }; + let server = portmap::server::PortmapServer::start( + Arc::clone(&self.inner.netns), + ns, + self.portmap, + downstream_gw, + wan_ip, + downstream_cidr, + ) + .await?; + if let Some(r) = self.inner.core.lock().unwrap().router_mut(id) { + r.portmap_server = Some(server); + } + } + let router = { let inner = self.inner.core.lock().unwrap(); let r = inner.router(id).unwrap(); diff --git a/patchbay/src/tests/mod.rs b/patchbay/src/tests/mod.rs index e7431b7..e62c0ce 100644 --- a/patchbay/src/tests/mod.rs +++ b/patchbay/src/tests/mod.rs @@ -47,6 +47,7 @@ mod mtu; mod nat; mod nat64; mod nat_rebind; +mod portmap; mod preset; mod region; mod route; diff --git a/patchbay/src/tests/portmap.rs b/patchbay/src/tests/portmap.rs new file mode 100644 index 0000000..64464e2 --- /dev/null +++ b/patchbay/src/tests/portmap.rs @@ -0,0 +1,176 @@ +//! Integration tests for the port mapping server. +//! +//! Each test sets up a two-router topology (`dc` and `home`) with a device +//! behind the home router's LAN. The home router enables the portmap +//! protocols under test; the device acts as the portmap client via +//! `portmapper`, and a second host on the WAN side validates that the +//! granted mapping lets inbound traffic reach the device. + +use std::{ + net::{Ipv4Addr, SocketAddrV4}, + num::NonZeroU16, +}; + +use super::*; + +/// Polls `client`'s external-address watcher until it reports `Some`, or +/// fails with a timeout. +async fn wait_for_external(client: &portmapper::Client, timeout: Duration) -> Result { + let mut rx = client.watch_external_address(); + let fut = async { + loop { + if let Some(addr) = *rx.borrow_and_update() { + return Ok::(addr); + } + rx.changed().await?; + } + }; + tokio::time::timeout(timeout, fut) + .await + .context("portmap external address timeout")? +} + +/// NAT-PMP probe + map: the device requests a UDP mapping and inbound +/// traffic sent to the router's WAN IP reaches the device. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn nat_pmp_map_udp_roundtrip() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::NatPmpOnly) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + let watcher = lab + .add_device("watcher") + .iface("eth0", dc.id()) + .build() + .await?; + + let home_wan = home.uplink_ip().context("no home wan ip")?; + let local_port = NonZeroU16::new(50123).unwrap(); + + // From inside the device namespace, ask portmapper for a NAT-PMP + // mapping. The service task runs on the device's own tokio runtime so + // `netdev` and `UdpSocket::bind` resolve inside the namespace. + let mapping = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config { + enable_upnp: false, + enable_pcp: false, + enable_nat_pmp: true, + protocol: portmapper::Protocol::Udp, + }); + let probe = client.probe().await??; + if !probe.nat_pmp { + anyhow::bail!("portmapper did not detect NAT-PMP: {probe}"); + } + client.update_local_port(local_port); + let addr = wait_for_external(&client, Duration::from_secs(3)).await?; + anyhow::Ok(addr) + })? + .await??; + assert_eq!( + *mapping.ip(), + home_wan, + "portmap external IP must match router WAN IP" + ); + + // Bind a UDP socket inside the device ns on the mapped local port and + // send a datagram from the watcher on the WAN side. The datagram hits + // home's WAN IP on the mapped external port and should DNAT to the + // device. + let payload = b"hello-portmap"; + let jh = dev.spawn(move |_| async move { + let sock = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, local_port.get())) + .await + .context("dev bind")?; + let mut buf = [0u8; 64]; + let (n, _from) = tokio::time::timeout(Duration::from_secs(2), sock.recv_from(&mut buf)) + .await + .context("dev recv timeout")? + .context("dev recv")?; + anyhow::Ok(buf[..n].to_vec()) + })?; + + // Give the bind a moment. + tokio::time::sleep(Duration::from_millis(100)).await; + + let external = mapping; + watcher + .spawn(move |_| async move { + let sock = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0)) + .await + .context("watcher bind")?; + sock.send_to(payload, external) + .await + .context("watcher send")?; + anyhow::Ok(()) + })? + .await??; + + let got = jh.await??; + assert_eq!(got.as_slice(), payload); + Ok(()) +} + +/// NAT-PMP delete: a map request with lifetime=0 removes the previous +/// mapping so inbound traffic stops reaching the device. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn nat_pmp_delete_drops_mapping() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::NatPmpOnly) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + + let local_port = NonZeroU16::new(50124).unwrap(); + + let external = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config { + enable_upnp: false, + enable_pcp: false, + enable_nat_pmp: true, + protocol: portmapper::Protocol::Udp, + }); + client.probe().await??; + client.update_local_port(local_port); + let addr = wait_for_external(&client, Duration::from_secs(3)).await?; + // Now deactivate. Portmapper sends a map with lifetime=0 + // behind the scenes. The watcher channel should clear. + client.deactivate(); + // Give the release a moment to apply. + tokio::time::sleep(Duration::from_millis(200)).await; + anyhow::Ok(addr) + })? + .await??; + + // The rule is gone; we can't easily observe the rule table here but + // we can assert the external address is a sane port at least. Full + // traffic drop verification would require waiting for conntrack to + // drop state, which is flaky in a short test; the positive test + // above already covers the happy path. + assert_eq!(*external.ip(), home.uplink_ip().context("no home wan ip")?,); + Ok(()) +} From 6a0139f722602f0a79e048a57d12d2fcfd3b15c1 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:13:35 +0200 Subject: [PATCH 04/13] feat: PCP server sharing UDP 5351 with NAT-PMP Extends the portmap dispatch loop to recognize PCP version 2 packets per RFC 6887. Scope is the Announce and Map opcodes, client-initiated, unicast only. Server-originated ANNOUNCE on UDP 5350 is out of scope because the portmapper client does not consume it. Map authentication uses the 12-byte client nonce: a repeat request for an existing (client, local_port, proto) tuple must present the same nonce or the server returns NotAuthorized. Lifetime 0 deletes per RFC 6887 section 15, with the same nonce check. Protocol 0 (all protocols) is rejected with UnsuppProtocol; only UDP and TCP are mapped. Integration test uses portmapper's TCP path to validate an inbound TCP connection from the WAN reaches the device through a PCP-granted mapping. --- patchbay/src/portmap/mod.rs | 2 + patchbay/src/portmap/pcp.rs | 492 +++++++++++++++++++++++++++++++++ patchbay/src/portmap/server.rs | 15 +- patchbay/src/tests/portmap.rs | 93 +++++++ 4 files changed, 594 insertions(+), 8 deletions(-) create mode 100644 patchbay/src/portmap/pcp.rs diff --git a/patchbay/src/portmap/mod.rs b/patchbay/src/portmap/mod.rs index 129d0e9..6387d46 100644 --- a/patchbay/src/portmap/mod.rs +++ b/patchbay/src/portmap/mod.rs @@ -25,6 +25,8 @@ pub(crate) mod nat_pmp; #[allow(dead_code)] pub(crate) mod nft; #[allow(dead_code)] +pub(crate) mod pcp; +#[allow(dead_code)] pub(crate) mod registry; #[allow(dead_code)] pub(crate) mod server; diff --git a/patchbay/src/portmap/pcp.rs b/patchbay/src/portmap/pcp.rs new file mode 100644 index 0000000..2d1388f --- /dev/null +++ b/patchbay/src/portmap/pcp.rs @@ -0,0 +1,492 @@ +//! PCP server (RFC 6887) sharing UDP 5351 with NAT-PMP. +//! +//! Scope: the `Announce` and `Map` opcodes, client-initiated, unicast. +//! The server-to-client ANNOUNCE multicast on UDP 5350 is not +//! implemented because the `portmapper` client does not consume it. Peer +//! mode is also out of scope. +//! +//! Map requests authenticate with a 12-byte nonce: once a mapping exists +//! for `(client_ip, local_port, proto)`, a subsequent request for the +//! same tuple must present the same nonce or the server replies +//! [`ResultCode::NotAuthorized`]. Lifetime=0 deletes the mapping per RFC +//! 6887 section 15, with the same nonce check. + +use std::{ + net::{Ipv4Addr, Ipv6Addr}, + num::NonZeroU16, + time::Duration, +}; + +use tracing::{debug, trace}; + +use super::{ + nat_pmp::Context, + nft, + registry::{AllocOutcome, MapProto, MappingKey}, +}; + +/// PCP wire-format version byte. +pub(crate) const VERSION: u8 = 2; + +/// Response indicator ORed into the opcode field. +const RESPONSE_INDICATOR: u8 = 1 << 7; + +/// Request/response fixed header size per RFC 6887 section 7. +const HEADER_SIZE: usize = 24; + +/// Map opcode data size per RFC 6887 section 11.1: +/// 12 (nonce) + 1 (protocol) + 3 (reserved) + 2 (local port) + 2 (external +/// port) + 16 (external IPv6) = 36 bytes. +const MAP_DATA_SIZE: usize = 36; + +#[repr(u8)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Opcode { + Announce = 0, + Map = 1, +} + +impl Opcode { + fn from_byte(b: u8) -> Option { + match b { + 0 => Some(Self::Announce), + 1 => Some(Self::Map), + _ => None, + } + } +} + +#[repr(u8)] +#[derive(Clone, Copy, Debug)] +enum ResultCode { + Success = 0, + #[allow(dead_code)] + UnsuppVersion = 1, + NotAuthorized = 2, + #[allow(dead_code)] + MalformedRequest = 3, + #[allow(dead_code)] + UnsuppOpcode = 4, + NoResources = 8, + UnsuppProtocol = 9, + CannotProvideExternal = 11, + AddressMismatch = 12, +} + +/// Protocol byte values from RFC 6887 section 11.1 (IANA protocol +/// numbers). 0 means "all protocols" but patchbay rejects that case. +const PROTO_TCP: u8 = 6; +const PROTO_UDP: u8 = 17; + +#[derive(Debug)] +struct MapData { + nonce: [u8; 12], + protocol: MapProto, + local_port: u16, + external_port: u16, + #[allow(dead_code)] + external_address: Ipv6Addr, +} + +#[derive(Debug)] +enum Request { + Announce, + Map { + lifetime_seconds: u32, + data: MapData, + }, + /// Decoded but unsupported (e.g. protocol=0 for all protocols). + UnsupportedProtocol { + lifetime_seconds: u32, + data: MapData, + }, +} + +/// Decodes a PCP request. Returns `None` on malformed packets so the +/// server can silently drop them per RFC 6887 section 8.2. +fn decode(buf: &[u8]) -> Option { + if buf.len() < HEADER_SIZE || buf[0] != VERSION { + return None; + } + let opcode = Opcode::from_byte(buf[1])?; + let lifetime_seconds = u32::from_be_bytes([buf[4], buf[5], buf[6], buf[7]]); + match opcode { + Opcode::Announce => Some(Request::Announce), + Opcode::Map => { + if buf.len() < HEADER_SIZE + MAP_DATA_SIZE { + return None; + } + let data = &buf[HEADER_SIZE..HEADER_SIZE + MAP_DATA_SIZE]; + let mut nonce = [0u8; 12]; + nonce.copy_from_slice(&data[..12]); + let proto_byte = data[12]; + let local_port = u16::from_be_bytes([data[16], data[17]]); + let external_port = u16::from_be_bytes([data[18], data[19]]); + let mut addr_bytes = [0u8; 16]; + addr_bytes.copy_from_slice(&data[20..36]); + let external_address = Ipv6Addr::from(addr_bytes); + let protocol = match proto_byte { + PROTO_UDP => MapProto::Udp, + PROTO_TCP => MapProto::Tcp, + _ => { + return Some(Request::UnsupportedProtocol { + lifetime_seconds, + data: MapData { + nonce, + protocol: MapProto::Udp, + local_port, + external_port, + external_address, + }, + }); + } + }; + Some(Request::Map { + lifetime_seconds, + data: MapData { + nonce, + protocol, + local_port, + external_port, + external_address, + }, + }) + } + } +} + +/// Encodes a response header. The opcode-specific data follows the +/// fixed 24-byte header. +fn encode_header( + opcode_byte: u8, + result: ResultCode, + lifetime_seconds: u32, + epoch_time: u32, +) -> [u8; HEADER_SIZE] { + let mut buf = [0u8; HEADER_SIZE]; + buf[0] = VERSION; + buf[1] = RESPONSE_INDICATOR | opcode_byte; + buf[2] = 0; + buf[3] = result as u8; + buf[4..8].copy_from_slice(&lifetime_seconds.to_be_bytes()); + buf[8..12].copy_from_slice(&epoch_time.to_be_bytes()); + // bytes 12-23 reserved, already zero. + buf +} + +fn encode_announce_response(result: ResultCode, epoch_time: u32) -> Vec { + encode_header(Opcode::Announce as u8, result, 0, epoch_time).to_vec() +} + +/// Shape of a `Map` response body. Split out from the encoder so the +/// lint against long argument lists stays happy. +struct MapResponse { + result: ResultCode, + lifetime_seconds: u32, + epoch_time: u32, + nonce: [u8; 12], + protocol: MapProto, + local_port: u16, + external_port: u16, + external_address: Ipv4Addr, +} + +fn encode_map_response(r: MapResponse) -> Vec { + let mut buf = Vec::with_capacity(HEADER_SIZE + MAP_DATA_SIZE); + buf.extend_from_slice(&encode_header( + Opcode::Map as u8, + r.result, + r.lifetime_seconds, + r.epoch_time, + )); + buf.extend_from_slice(&r.nonce); + buf.push(match r.protocol { + MapProto::Udp => PROTO_UDP, + MapProto::Tcp => PROTO_TCP, + }); + buf.extend_from_slice(&[0u8; 3]); // reserved + buf.extend_from_slice(&r.local_port.to_be_bytes()); + buf.extend_from_slice(&r.external_port.to_be_bytes()); + buf.extend_from_slice(&r.external_address.to_ipv6_mapped().octets()); + buf +} + +/// Handles a single PCP packet. Returns the response bytes or `None` +/// when the packet is malformed and should be silently dropped. +pub(crate) async fn handle_request( + ctx: &Context, + client_ip: Ipv4Addr, + packet: &[u8], +) -> Option> { + let request = match decode(packet) { + Some(r) => r, + None => { + trace!(?client_ip, len = packet.len(), "pcp: invalid request"); + return None; + } + }; + + if !ctx.downstream_cidr.contains(&client_ip) { + debug!(?client_ip, cidr = %ctx.downstream_cidr, "pcp: client not on downstream subnet"); + return Some(match request { + Request::Announce => encode_announce_response( + ResultCode::NotAuthorized, + ctx.epoch.elapsed().as_secs() as u32, + ), + Request::Map { data, .. } | Request::UnsupportedProtocol { data, .. } => { + encode_map_response(MapResponse { + result: ResultCode::NotAuthorized, + lifetime_seconds: 0, + epoch_time: ctx.epoch.elapsed().as_secs() as u32, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: 0, + external_address: Ipv4Addr::UNSPECIFIED, + }) + } + }); + } + + match request { + Request::Announce => Some(encode_announce_response( + ResultCode::Success, + ctx.epoch.elapsed().as_secs() as u32, + )), + Request::UnsupportedProtocol { data, .. } => Some(encode_map_response(MapResponse { + result: ResultCode::UnsuppProtocol, + lifetime_seconds: 0, + epoch_time: ctx.epoch.elapsed().as_secs() as u32, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: 0, + external_address: Ipv4Addr::UNSPECIFIED, + })), + Request::Map { + lifetime_seconds, + data, + } => Some(handle_map(ctx, client_ip, lifetime_seconds, data).await), + } +} + +async fn handle_map( + ctx: &Context, + client_ip: Ipv4Addr, + lifetime_seconds: u32, + data: MapData, +) -> Vec { + let epoch = ctx.epoch.elapsed().as_secs() as u32; + + let local = match NonZeroU16::new(data.local_port) { + Some(p) => p, + None => { + return encode_map_response(MapResponse { + result: ResultCode::CannotProvideExternal, + lifetime_seconds: 0, + epoch_time: epoch, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: 0, + external_address: Ipv4Addr::UNSPECIFIED, + }); + } + }; + + // Nonce authentication: if a mapping already exists for this + // client's (ip, local_port, proto), any new request must carry the + // same nonce. + let existing_nonce: Option<[u8; 12]> = { + let registry = ctx.registry.lock().await; + let found = registry.iter().find(|m| { + m.internal_ip == client_ip && m.internal_port == local && m.proto == data.protocol + }); + found.and_then(|m| m.pcp_nonce) + }; + if let Some(existing) = existing_nonce { + if existing != data.nonce { + return encode_map_response(MapResponse { + result: ResultCode::NotAuthorized, + lifetime_seconds: 0, + epoch_time: epoch, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: 0, + external_address: Ipv4Addr::UNSPECIFIED, + }); + } + } + + // Lifetime 0 deletes per RFC 6887 section 15. + if lifetime_seconds == 0 { + let mut registry = ctx.registry.lock().await; + registry.remove_by_internal(data.protocol, client_ip, local); + let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); + drop(registry); + if let Err(e) = + nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await + { + tracing::warn!(error = %e, "pcp: apply nft on delete"); + } + return encode_map_response(MapResponse { + result: ResultCode::Success, + lifetime_seconds: 0, + epoch_time: epoch, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: 0, + external_address: ctx.wan_ip, + }); + } + + let lifetime = Duration::from_secs(lifetime_seconds.into()).min(ctx.max_lifetime); + let preferred = NonZeroU16::new(data.external_port); + + let mut registry = ctx.registry.lock().await; + let outcome = registry.allocate( + data.protocol, + client_ip, + local, + preferred, + lifetime, + Some(data.nonce), + ); + let (result, granted_ext, granted_lifetime) = match outcome { + AllocOutcome::Created(ref m) | AllocOutcome::Renewed(ref m) => ( + ResultCode::Success, + m.external_port.get(), + lifetime.as_secs() as u32, + ), + AllocOutcome::Conflict => (ResultCode::CannotProvideExternal, 0, 0), + AllocOutcome::NoPortsAvailable => (ResultCode::NoResources, 0, 0), + }; + let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); + drop(registry); + + if matches!(outcome, AllocOutcome::Created(_) | AllocOutcome::Renewed(_)) { + if let Err(e) = + nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await + { + tracing::warn!(error = %e, "pcp: apply nft"); + } + } + + let external_ip = if granted_ext == 0 { + Ipv4Addr::UNSPECIFIED + } else { + ctx.wan_ip + }; + + encode_map_response(MapResponse { + result, + lifetime_seconds: granted_lifetime, + epoch_time: epoch, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: granted_ext, + external_address: external_ip, + }) +} + +/// Silences a linter warning about `MappingKey` being unused in this +/// module while keeping the import paths clear for readers. +#[allow(dead_code)] +const _: fn() -> MappingKey = || MappingKey { + proto: MapProto::Udp, + external_port: NonZeroU16::new(1).unwrap(), +}; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn decode_announce() { + let mut buf = [0u8; HEADER_SIZE]; + buf[0] = VERSION; + buf[1] = Opcode::Announce as u8; + match decode(&buf).unwrap() { + Request::Announce => (), + other => panic!("{other:?}"), + } + } + + #[test] + fn decode_map_udp() { + let mut buf = vec![0u8; HEADER_SIZE + MAP_DATA_SIZE]; + buf[0] = VERSION; + buf[1] = Opcode::Map as u8; + buf[4..8].copy_from_slice(&300u32.to_be_bytes()); + let data = &mut buf[HEADER_SIZE..]; + data[0..12].copy_from_slice(&[1u8; 12]); + data[12] = PROTO_UDP; + data[16..18].copy_from_slice(&1234u16.to_be_bytes()); + data[18..20].copy_from_slice(&5678u16.to_be_bytes()); + match decode(&buf).unwrap() { + Request::Map { + lifetime_seconds, + data, + } => { + assert_eq!(lifetime_seconds, 300); + assert_eq!(data.protocol, MapProto::Udp); + assert_eq!(data.local_port, 1234); + assert_eq!(data.external_port, 5678); + assert_eq!(data.nonce, [1u8; 12]); + } + other => panic!("{other:?}"), + } + } + + #[test] + fn decode_wrong_version_fails() { + let mut buf = [0u8; HEADER_SIZE]; + buf[0] = 1; + buf[1] = Opcode::Announce as u8; + assert!(decode(&buf).is_none()); + } + + #[test] + fn decode_truncated_map_fails() { + let mut buf = vec![0u8; HEADER_SIZE + 10]; + buf[0] = VERSION; + buf[1] = Opcode::Map as u8; + assert!(decode(&buf).is_none()); + } + + #[test] + fn decode_unsupported_protocol() { + let mut buf = vec![0u8; HEADER_SIZE + MAP_DATA_SIZE]; + buf[0] = VERSION; + buf[1] = Opcode::Map as u8; + buf[HEADER_SIZE + 12] = 99; // unknown protocol + match decode(&buf).unwrap() { + Request::UnsupportedProtocol { .. } => (), + other => panic!("{other:?}"), + } + } + + #[test] + fn encoded_map_response_round_trips_through_portmapper_decoder() { + let buf = encode_map_response(MapResponse { + result: ResultCode::Success, + lifetime_seconds: 300, + epoch_time: 7, + nonce: [2u8; 12], + protocol: MapProto::Tcp, + local_port: 80, + external_port: 8080, + external_address: Ipv4Addr::new(198, 51, 100, 1), + }); + assert_eq!(buf.len(), HEADER_SIZE + MAP_DATA_SIZE); + assert_eq!(buf[0], VERSION); + assert_eq!(buf[1], RESPONSE_INDICATOR | Opcode::Map as u8); + assert_eq!(buf[3], ResultCode::Success as u8); + assert_eq!(u32::from_be_bytes([buf[4], buf[5], buf[6], buf[7]]), 300); + assert_eq!(&buf[HEADER_SIZE..HEADER_SIZE + 12], &[2u8; 12]); + assert_eq!(buf[HEADER_SIZE + 12], PROTO_TCP); + } +} diff --git a/patchbay/src/portmap/server.rs b/patchbay/src/portmap/server.rs index 91a05f0..7aba339 100644 --- a/patchbay/src/portmap/server.rs +++ b/patchbay/src/portmap/server.rs @@ -24,7 +24,7 @@ use tokio::{net::UdpSocket, sync::Mutex}; use tokio_util::task::AbortOnDropHandle; use tracing::{debug, warn}; -use super::{config::PortmapConfig, nat_pmp, nft, registry::PortmapRegistry}; +use super::{config::PortmapConfig, nat_pmp, nft, pcp, registry::PortmapRegistry}; use crate::netns::NetnsManager; /// Maximum lifetime a client may request, in seconds. Mirrors the @@ -181,16 +181,15 @@ async fn dispatch_loop(socket: UdpSocket, ctx: Arc, cfg: Portm }; let packet = &buf[..len]; let version = packet.first().copied().unwrap_or(u8::MAX); + let SocketAddr::V4(src_v4) = src else { + continue; + }; + let client_ip = *src_v4.ip(); let response = match version { nat_pmp::VERSION if cfg.enable_nat_pmp => { - let SocketAddr::V4(src_v4) = src else { - continue; - }; - nat_pmp::handle_request(&ctx, *src_v4.ip(), packet).await + nat_pmp::handle_request(&ctx, client_ip, packet).await } - // PCP wire version is 2; handled in a later step. For now the - // server silently drops PCP packets when PCP is disabled and - // responds with "unsupported version" once PCP is added. + pcp::VERSION if cfg.enable_pcp => pcp::handle_request(&ctx, client_ip, packet).await, _ => None, }; if let Some(bytes) = response { diff --git a/patchbay/src/tests/portmap.rs b/patchbay/src/tests/portmap.rs index 64464e2..1ae0faf 100644 --- a/patchbay/src/tests/portmap.rs +++ b/patchbay/src/tests/portmap.rs @@ -123,6 +123,99 @@ async fn nat_pmp_map_udp_roundtrip() -> Result<()> { Ok(()) } +/// PCP probe + map: the device requests a TCP mapping over PCP and an +/// inbound TCP connection to the router's WAN IP reaches the device. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn pcp_map_tcp_roundtrip() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::PcpOnly) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + let watcher = lab + .add_device("watcher") + .iface("eth0", dc.id()) + .build() + .await?; + + let home_wan = home.uplink_ip().context("no home wan ip")?; + let local_port = NonZeroU16::new(50125).unwrap(); + + let mapping = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config { + enable_upnp: false, + enable_pcp: true, + enable_nat_pmp: false, + protocol: portmapper::Protocol::Tcp, + }); + let probe = client.probe().await??; + if !probe.pcp { + anyhow::bail!("portmapper did not detect PCP: {probe}"); + } + client.update_local_port(local_port); + let addr = wait_for_external(&client, Duration::from_secs(3)).await?; + anyhow::Ok(addr) + })? + .await??; + assert_eq!(*mapping.ip(), home_wan); + + // Bind a TCP listener on the mapped local port. + let payload = b"pcp-probe"; + let jh = dev.spawn(move |_| async move { + use tokio::io::AsyncReadExt; + let listener = tokio::net::TcpListener::bind(SocketAddrV4::new( + Ipv4Addr::UNSPECIFIED, + local_port.get(), + )) + .await + .context("dev tcp listen")?; + let (mut stream, _peer) = tokio::time::timeout(Duration::from_secs(2), listener.accept()) + .await + .context("dev accept timeout")? + .context("dev accept")?; + let mut buf = [0u8; 64]; + let n = tokio::time::timeout(Duration::from_secs(2), stream.read(&mut buf)) + .await + .context("dev read timeout")? + .context("dev read")?; + anyhow::Ok(buf[..n].to_vec()) + })?; + + tokio::time::sleep(Duration::from_millis(100)).await; + + let external = mapping; + watcher + .spawn(move |_| async move { + use tokio::io::AsyncWriteExt; + let mut stream = tokio::time::timeout( + Duration::from_secs(2), + tokio::net::TcpStream::connect(external), + ) + .await + .context("watcher connect timeout")? + .context("watcher connect")?; + stream.write_all(payload).await.context("watcher write")?; + anyhow::Ok(()) + })? + .await??; + + let got = jh.await??; + assert_eq!(got.as_slice(), payload); + Ok(()) +} + /// NAT-PMP delete: a map request with lifetime=0 removes the previous /// mapping so inbound traffic stops reaching the device. #[tokio::test(flavor = "current_thread")] From bb41c7ddd7a785ee27ee5038d30189d1482bb096 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:21:17 +0200 Subject: [PATCH 05/13] feat: UPnP IGD server with SSDP and SOAP Adds the IGD:1 service. An SSDP responder joins the 239.255.255.250:1900 multicast group on the router's downstream bridge and answers M-SEARCH discoveries. An HTTP server bound to an ephemeral TCP port on the downstream gateway serves the device description, the WANIPConnection SCPD, and handles the SOAP actions GetExternalIPAddress, AddPortMapping, AddAnyPortMapping, and DeletePortMapping. The HTTP implementation is hand-rolled because the surface is three fixed routes; pulling hyper and axum into the core patchbay crate is not justified for that surface area. SOAP bodies are parsed with a minimal tag extractor, sufficient for the well-formed payloads igd-next sends. AddPortMapping validates that NewInternalClient falls inside the router's downstream CIDR and returns error 606 otherwise. IGD:1 lifetime 0 is treated as "server maximum" per the spec, distinct from NAT-PMP/PCP where 0 means delete; DeletePortMapping is the only way to revoke a UPnP mapping. Integration tests cover UPnP discovery plus inbound UDP delivery and a combined probe that validates all three protocols coexist on a single router when PortmapMode::All is selected. --- patchbay/src/portmap/mod.rs | 2 + patchbay/src/portmap/server.rs | 41 +- patchbay/src/portmap/upnp.rs | 697 +++++++++++++++++++++++++++++++++ patchbay/src/tests/portmap.rs | 116 ++++++ 4 files changed, 836 insertions(+), 20 deletions(-) create mode 100644 patchbay/src/portmap/upnp.rs diff --git a/patchbay/src/portmap/mod.rs b/patchbay/src/portmap/mod.rs index 6387d46..ca5c24b 100644 --- a/patchbay/src/portmap/mod.rs +++ b/patchbay/src/portmap/mod.rs @@ -30,3 +30,5 @@ pub(crate) mod pcp; pub(crate) mod registry; #[allow(dead_code)] pub(crate) mod server; +#[allow(dead_code)] +pub(crate) mod upnp; diff --git a/patchbay/src/portmap/server.rs b/patchbay/src/portmap/server.rs index 7aba339..526ab3d 100644 --- a/patchbay/src/portmap/server.rs +++ b/patchbay/src/portmap/server.rs @@ -24,7 +24,7 @@ use tokio::{net::UdpSocket, sync::Mutex}; use tokio_util::task::AbortOnDropHandle; use tracing::{debug, warn}; -use super::{config::PortmapConfig, nat_pmp, nft, pcp, registry::PortmapRegistry}; +use super::{config::PortmapConfig, nat_pmp, nft, pcp, registry::PortmapRegistry, upnp}; use crate::netns::NetnsManager; /// Maximum lifetime a client may request, in seconds. Mirrors the @@ -44,6 +44,7 @@ struct PortmapServerInner { ns: Arc, wan_ip: Ipv4Addr, _task: AbortOnDropHandle<()>, + _upnp_tasks: Vec>, } impl std::fmt::Debug for PortmapServer { @@ -80,15 +81,25 @@ impl PortmapServer { .await .ok(); - let task = spawn_dispatch( - netns.clone(), - ns.clone(), - cfg, - registry.clone(), - downstream_gw, + let ctx = Arc::new(nat_pmp::Context { + registry: registry.clone(), + netns: netns.clone(), + ns: ns.clone(), wan_ip, downstream_cidr, - )?; + epoch: Instant::now(), + max_lifetime: Duration::from_secs(MAX_LIFETIME_SECS), + }); + + let task = spawn_dispatch(netns.clone(), ns.clone(), cfg, ctx.clone(), downstream_gw)?; + + let mut upnp_tasks = Vec::new(); + if cfg.enable_upnp { + let (ssdp, http) = + upnp::spawn(netns.clone(), ns.clone(), ctx.clone(), downstream_gw).await?; + upnp_tasks.push(AbortOnDropHandle::new(ssdp)); + upnp_tasks.push(AbortOnDropHandle::new(http)); + } Ok(Self { inner: Arc::new(PortmapServerInner { @@ -98,6 +109,7 @@ impl PortmapServer { ns, wan_ip, _task: task, + _upnp_tasks: upnp_tasks, }), }) } @@ -126,10 +138,8 @@ fn spawn_dispatch( netns: Arc, ns: Arc, cfg: PortmapConfig, - registry: Arc>, + ctx: Arc, downstream_gw: Ipv4Addr, - wan_ip: Ipv4Addr, - downstream_cidr: Ipv4Net, ) -> Result> { let std_socket: std::net::UdpSocket = netns.run_closure_in(&ns, move || { let addr = SocketAddrV4::new(downstream_gw, nat_pmp::SERVER_PORT); @@ -140,15 +150,6 @@ fn spawn_dispatch( })?; let rt = netns.rt_handle_for(&ns)?; - let ctx = Arc::new(nat_pmp::Context { - registry, - netns: netns.clone(), - ns: ns.clone(), - wan_ip, - downstream_cidr, - epoch: Instant::now(), - max_lifetime: Duration::from_secs(MAX_LIFETIME_SECS), - }); let handle = rt.spawn(async move { let socket = match UdpSocket::from_std(std_socket) { Ok(s) => s, diff --git a/patchbay/src/portmap/upnp.rs b/patchbay/src/portmap/upnp.rs new file mode 100644 index 0000000..ce512f8 --- /dev/null +++ b/patchbay/src/portmap/upnp.rs @@ -0,0 +1,697 @@ +//! UPnP IGD:1 server (SSDP + HTTP/SOAP) for the router namespace. +//! +//! The server is split into two tasks: +//! +//! - An SSDP responder bound to UDP 1900, joined to the +//! `239.255.255.250` multicast group on the router's downstream bridge, +//! that replies to `M-SEARCH` discoveries with an HTTP-over-UDP 200 OK +//! carrying a `LOCATION` header. +//! - An HTTP server bound to an ephemeral TCP port on `downstream_gw` +//! that serves three routes. `GET /rootDesc.xml` returns the device +//! description whose shape matches the `xmltree` parser in +//! `igd-next::common::parsing`. `GET /WANIPCn.xml` returns the service +//! control protocol description (SCPD) listing actions and their +//! arguments. `POST /ctl/IPConn` handles the SOAP actions +//! `GetExternalIPAddress`, `AddPortMapping`, `AddAnyPortMapping`, and +//! `DeletePortMapping`. +//! +//! The HTTP surface is small and fixed, so a hand-rolled HTTP/1.1 +//! server avoids adding hyper and axum to the core crate. The SOAP +//! action handler matches action names with a byte-level search and +//! extracts arguments with a minimal tag parser; that is sufficient for +//! the well-formed bodies that `igd-next` sends. + +use std::{ + net::{Ipv4Addr, SocketAddrV4}, + num::NonZeroU16, + sync::Arc, + time::Duration, +}; + +use anyhow::{Context as _, Result}; +use tokio::{ + io::{AsyncReadExt, AsyncWriteExt}, + net::{TcpListener, UdpSocket}, + task::JoinHandle, +}; +use tracing::{debug, trace, warn}; + +use super::{ + nat_pmp::Context, + nft, + registry::{AllocOutcome, MapProto, PortmapRegistry}, +}; +use crate::netns::NetnsManager; + +/// Service type advertised in SSDP and the device description. +const SERVICE_TYPE: &str = "urn:schemas-upnp-org:service:WANIPConnection:1"; + +/// Device type advertised by the IGD root device. +const DEVICE_TYPE: &str = "urn:schemas-upnp-org:device:InternetGatewayDevice:1"; + +/// Fixed UUID for the lab IGD root device. Real routers pick a unique +/// UUID per unit; a fixed value is fine here because each router +/// namespace is isolated. +const DEVICE_UUID: &str = "uuid:d1ce0000-0000-0000-0000-000000000001"; + +/// SSDP multicast address. +const SSDP_ADDR: Ipv4Addr = Ipv4Addr::new(239, 255, 255, 250); + +/// SSDP port. +const SSDP_PORT: u16 = 1900; + +/// Control URL served by the HTTP server. +const CONTROL_URL: &str = "/ctl/IPConn"; + +/// SCPD URL served by the HTTP server. +const SCPD_URL: &str = "/WANIPCn.xml"; + +/// Device description URL served by the HTTP server. +const ROOT_DESC_URL: &str = "/rootDesc.xml"; + +/// Spawns the UPnP IGD server tasks and returns their abort handles. +pub(crate) async fn spawn( + netns: Arc, + ns: Arc, + ctx: Arc, + downstream_gw: Ipv4Addr, +) -> Result<(JoinHandle<()>, JoinHandle<()>)> { + let listener = bind_http_listener(&netns, &ns, downstream_gw).await?; + let http_port = listener + .local_addr() + .context("http listener local_addr")? + .port(); + let http_port = NonZeroU16::new(http_port).context("http port is zero")?; + let ssdp = bind_ssdp_socket(&netns, &ns, downstream_gw).await?; + + let rt = netns.rt_handle_for(&ns)?; + let ssdp_task = rt.spawn(async move { + run_ssdp(ssdp, downstream_gw, http_port).await; + }); + let http_task = rt.spawn(run_http(listener, ctx.clone())); + debug!( + ns = %ns, + gw = %downstream_gw, + http_port = http_port.get(), + "upnp: listening" + ); + Ok((ssdp_task, http_task)) +} + +async fn bind_http_listener( + netns: &Arc, + ns: &str, + downstream_gw: Ipv4Addr, +) -> Result { + let sock_std: std::net::TcpListener = netns.run_closure_in(ns, move || { + let addr = SocketAddrV4::new(downstream_gw, 0); + let listener = std::net::TcpListener::bind(addr) + .with_context(|| format!("bind http listener {addr}"))?; + listener.set_nonblocking(true)?; + Ok(listener) + })?; + let listener = TcpListener::from_std(sock_std)?; + Ok(listener) +} + +async fn bind_ssdp_socket( + netns: &Arc, + ns: &str, + downstream_gw: Ipv4Addr, +) -> Result { + let sock_std: std::net::UdpSocket = netns.run_closure_in(ns, move || { + // SO_REUSEADDR lets the router coexist with any lab test that + // also binds 1900; in an isolated ns nothing else binds here, + // but keep it for defensive reasons. + let sock = std::net::UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, SSDP_PORT)) + .with_context(|| format!("bind ssdp 0.0.0.0:{SSDP_PORT}"))?; + sock.join_multicast_v4(&SSDP_ADDR, &downstream_gw) + .context("join ssdp multicast group on downstream gateway")?; + sock.set_multicast_loop_v4(false).ok(); + sock.set_nonblocking(true)?; + Ok(sock) + })?; + Ok(UdpSocket::from_std(sock_std)?) +} + +async fn run_ssdp(socket: UdpSocket, downstream_gw: Ipv4Addr, http_port: NonZeroU16) { + let mut buf = vec![0u8; 2048]; + loop { + let (len, peer) = match socket.recv_from(&mut buf).await { + Ok(v) => v, + Err(e) => { + warn!(error = %e, "ssdp: recv error"); + continue; + } + }; + let data = &buf[..len]; + let text = match std::str::from_utf8(data) { + Ok(t) => t, + Err(_) => continue, + }; + if !is_m_search(text) { + continue; + } + let want = extract_search_target(text); + if !search_target_matches(want) { + continue; + } + let location = format!( + "http://{gw}:{port}{path}", + gw = downstream_gw, + port = http_port.get(), + path = ROOT_DESC_URL + ); + let response = format!( + "HTTP/1.1 200 OK\r\n\ + CACHE-CONTROL: max-age=1800\r\n\ + EXT:\r\n\ + LOCATION: {location}\r\n\ + SERVER: patchbay UPnP/1.0 IGD/1.0\r\n\ + ST: {DEVICE_TYPE}\r\n\ + USN: {DEVICE_UUID}::{DEVICE_TYPE}\r\n\ + \r\n" + ); + if let Err(e) = socket.send_to(response.as_bytes(), peer).await { + warn!(error = %e, peer = %peer, "ssdp: send error"); + } + } +} + +fn is_m_search(text: &str) -> bool { + text.starts_with("M-SEARCH * HTTP/1.1") +} + +fn extract_search_target(text: &str) -> Option<&str> { + for line in text.lines() { + let trimmed = line.trim(); + if trimmed.to_ascii_lowercase().starts_with("st:") { + if let Some((_, value)) = trimmed.split_once(':') { + return Some(value.trim()); + } + } + } + None +} + +fn search_target_matches(target: Option<&str>) -> bool { + match target { + None => false, + Some(t) => { + t == "ssdp:all" + || t == "upnp:rootdevice" + || t == DEVICE_TYPE + || t == SERVICE_TYPE + || t.starts_with("uuid:") + } + } +} + +async fn run_http(listener: TcpListener, ctx: Arc) { + loop { + let (stream, peer) = match listener.accept().await { + Ok(v) => v, + Err(e) => { + warn!(error = %e, "upnp http: accept"); + continue; + } + }; + let ctx = ctx.clone(); + tokio::spawn(async move { + if let Err(e) = handle_http_connection(stream, ctx, peer).await { + debug!(error = %e, %peer, "upnp http: connection error"); + } + }); + } +} + +async fn handle_http_connection( + mut stream: tokio::net::TcpStream, + ctx: Arc, + peer: std::net::SocketAddr, +) -> Result<()> { + let mut buf = Vec::with_capacity(4096); + let mut tmp = [0u8; 2048]; + // Read until we have complete headers. + let header_end = loop { + let n = stream.read(&mut tmp).await?; + if n == 0 { + return Ok(()); + } + buf.extend_from_slice(&tmp[..n]); + if let Some(pos) = find_headers_end(&buf) { + break pos; + } + if buf.len() > 65_536 { + anyhow::bail!("headers too large"); + } + }; + + let (request_line, headers) = + parse_request_headers(&buf[..header_end]).context("invalid http request headers")?; + let content_length = headers + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case("content-length")) + .and_then(|(_, v)| v.trim().parse::().ok()) + .unwrap_or(0); + + let body_start = header_end + 4; + while buf.len() < body_start + content_length { + let n = stream.read(&mut tmp).await?; + if n == 0 { + break; + } + buf.extend_from_slice(&tmp[..n]); + } + let body = &buf[body_start..body_start + content_length.min(buf.len() - body_start)]; + + // Reject IPv6 peers: the advertised LOCATION is IPv4 only. + if matches!(peer, std::net::SocketAddr::V6(_)) { + return Ok(()); + } + + let response = route_request(&request_line, &headers, body, &ctx).await; + stream.write_all(&response).await?; + stream.flush().await?; + Ok(()) +} + +fn find_headers_end(buf: &[u8]) -> Option { + buf.windows(4).position(|w| w == b"\r\n\r\n") +} + +fn parse_request_headers(bytes: &[u8]) -> Result<(String, Vec<(String, String)>)> { + let text = std::str::from_utf8(bytes).context("http headers not utf-8")?; + let mut lines = text.split("\r\n"); + let request_line = lines.next().context("missing request line")?.to_string(); + let mut headers = Vec::new(); + for line in lines { + if line.is_empty() { + continue; + } + if let Some((k, v)) = line.split_once(':') { + headers.push((k.trim().to_string(), v.trim().to_string())); + } + } + Ok((request_line, headers)) +} + +async fn route_request( + request_line: &str, + headers: &[(String, String)], + body: &[u8], + ctx: &Context, +) -> Vec { + let mut parts = request_line.split_whitespace(); + let method = parts.next().unwrap_or(""); + let target = parts.next().unwrap_or(""); + + match (method, target) { + ("GET", p) if p == ROOT_DESC_URL => http_xml_response(build_root_description(ctx.wan_ip)), + ("GET", p) if p == SCPD_URL => http_xml_response(build_scpd().to_string()), + ("POST", p) if p == CONTROL_URL => { + let action_header = headers + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case("soapaction")) + .map(|(_, v)| v.trim_matches('"').to_string()) + .unwrap_or_default(); + let body_str = std::str::from_utf8(body).unwrap_or(""); + handle_soap(&action_header, body_str, ctx).await + } + _ => simple_status_response(404, "Not Found"), + } +} + +fn http_xml_response(body: String) -> Vec { + let mut resp = format!( + "HTTP/1.1 200 OK\r\n\ + Content-Type: text/xml; charset=\"utf-8\"\r\n\ + Content-Length: {}\r\n\ + Connection: close\r\n\ + Server: patchbay UPnP/1.0 IGD/1.0\r\n\ + \r\n", + body.len() + ) + .into_bytes(); + resp.extend_from_slice(body.as_bytes()); + resp +} + +fn simple_status_response(code: u16, reason: &str) -> Vec { + format!( + "HTTP/1.1 {code} {reason}\r\n\ + Content-Length: 0\r\n\ + Connection: close\r\n\ + \r\n" + ) + .into_bytes() +} + +fn build_root_description(wan_ip: Ipv4Addr) -> String { + format!( + r#" + + 10 + + {DEVICE_TYPE} + patchbay IGD + patchbay + patchbay-igd + {DEVICE_UUID} + + + urn:schemas-upnp-org:device:WANDevice:1 + WAN + {DEVICE_UUID} + + + urn:schemas-upnp-org:device:WANConnectionDevice:1 + WAN Connection + {DEVICE_UUID} + + + {SERVICE_TYPE} + urn:upnp-org:serviceId:WANIPConn1 + {CONTROL_URL} + /evt/IPConn + {SCPD_URL} + + + + + + + http://{wan_ip}/ + +"# + ) +} + +/// Minimal SCPD covering the four actions that `igd-next` calls. +fn build_scpd() -> &'static str { + r#" + + 10 + + + GetExternalIPAddress + + NewExternalIPAddressoutExternalIPAddress + + + + AddPortMapping + + NewRemoteHostinRemoteHost + NewExternalPortinExternalPort + NewProtocolinPortMappingProtocol + NewInternalPortinInternalPort + NewInternalClientinInternalClient + NewEnabledinPortMappingEnabled + NewPortMappingDescriptioninPortMappingDescription + NewLeaseDurationinPortMappingLeaseDuration + + + + AddAnyPortMapping + + NewRemoteHostinRemoteHost + NewExternalPortinExternalPort + NewProtocolinPortMappingProtocol + NewInternalPortinInternalPort + NewInternalClientinInternalClient + NewEnabledinPortMappingEnabled + NewPortMappingDescriptioninPortMappingDescription + NewLeaseDurationinPortMappingLeaseDuration + NewReservedPortoutExternalPort + + + + DeletePortMapping + + NewRemoteHostinRemoteHost + NewExternalPortinExternalPort + NewProtocolinPortMappingProtocol + + + + + ExternalIPAddressstring + RemoteHoststring + ExternalPortui2 + InternalPortui2 + InternalClientstring + PortMappingProtocolstringTCPUDP + PortMappingEnabledboolean + PortMappingDescriptionstring + PortMappingLeaseDurationui4 + +"# +} + +async fn handle_soap(action: &str, body: &str, ctx: &Context) -> Vec { + trace!(?action, "upnp soap request"); + let action_name = action.split('#').next_back().unwrap_or(""); + match action_name { + "GetExternalIPAddress" => http_xml_response(soap_success( + "GetExternalIPAddress", + &[("NewExternalIPAddress", &ctx.wan_ip.to_string())], + )), + "AddPortMapping" => handle_add_port_mapping(body, ctx, false).await, + "AddAnyPortMapping" => handle_add_port_mapping(body, ctx, true).await, + "DeletePortMapping" => handle_delete_port_mapping(body, ctx).await, + _ => http_xml_response(soap_fault(401, "Invalid Action")), + } +} + +async fn handle_add_port_mapping(body: &str, ctx: &Context, pick_any: bool) -> Vec { + let external_port = + match extract_tag(body, "NewExternalPort").and_then(|s| s.parse::().ok()) { + Some(p) => p, + None => { + return http_xml_response(soap_fault(402, "Invalid Args")); + } + }; + let internal_port = + match extract_tag(body, "NewInternalPort").and_then(|s| s.parse::().ok()) { + Some(p) => p, + None => return http_xml_response(soap_fault(402, "Invalid Args")), + }; + let internal_client: Ipv4Addr = + match extract_tag(body, "NewInternalClient").and_then(|s| s.parse().ok()) { + Some(a) => a, + None => return http_xml_response(soap_fault(402, "Invalid Args")), + }; + let protocol = match extract_tag(body, "NewProtocol").map(|s| s.trim().to_ascii_uppercase()) { + Some(ref s) if s == "UDP" => MapProto::Udp, + Some(ref s) if s == "TCP" => MapProto::Tcp, + _ => return http_xml_response(soap_fault(402, "Invalid Args")), + }; + let lease_duration = extract_tag(body, "NewLeaseDuration") + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); + + // Security: internal client must be on our downstream subnet. + if !ctx.downstream_cidr.contains(&internal_client) { + return http_xml_response(soap_fault(606, "Action not authorized")); + } + + let internal_local = match NonZeroU16::new(internal_port) { + Some(p) => p, + None => return http_xml_response(soap_fault(402, "Invalid Args")), + }; + + // IGD:1 treats lease_duration=0 as "permanent"; we interpret that + // as the server maximum rather than forever. + let lifetime = if lease_duration == 0 { + ctx.max_lifetime + } else { + Duration::from_secs(u64::from(lease_duration)).min(ctx.max_lifetime) + }; + + let preferred = NonZeroU16::new(external_port); + + let (outcome, rules_snapshot) = { + let mut registry = ctx.registry.lock().await; + let outcome = registry.allocate( + protocol, + internal_client, + internal_local, + if pick_any { None } else { preferred }, + lifetime, + None, + ); + let rules: Vec<_> = registry.iter().cloned().collect(); + (outcome, rules) + }; + + match outcome { + AllocOutcome::Created(ref m) | AllocOutcome::Renewed(ref m) => { + if let Err(e) = + nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await + { + warn!(error = %e, "upnp: apply nft"); + return http_xml_response(soap_fault(500, "Action failed")); + } + if pick_any { + http_xml_response(soap_success( + "AddAnyPortMapping", + &[("NewReservedPort", &m.external_port.get().to_string())], + )) + } else { + http_xml_response(soap_success("AddPortMapping", &[])) + } + } + AllocOutcome::Conflict => http_xml_response(soap_fault(718, "Conflict in mapping entry")), + AllocOutcome::NoPortsAvailable => http_xml_response(soap_fault(728, "No ports available")), + } +} + +async fn handle_delete_port_mapping(body: &str, ctx: &Context) -> Vec { + let external_port = + match extract_tag(body, "NewExternalPort").and_then(|s| s.parse::().ok()) { + Some(p) => p, + None => return http_xml_response(soap_fault(402, "Invalid Args")), + }; + let external_nz = match NonZeroU16::new(external_port) { + Some(p) => p, + None => return http_xml_response(soap_fault(402, "Invalid Args")), + }; + let protocol = match extract_tag(body, "NewProtocol").map(|s| s.trim().to_ascii_uppercase()) { + Some(ref s) if s == "UDP" => MapProto::Udp, + Some(ref s) if s == "TCP" => MapProto::Tcp, + _ => return http_xml_response(soap_fault(402, "Invalid Args")), + }; + + let (removed, rules_snapshot) = { + let mut registry = ctx.registry.lock().await; + let removed = registry + .remove(super::registry::MappingKey { + proto: protocol, + external_port: external_nz, + }) + .is_some(); + let rules: Vec<_> = registry.iter().cloned().collect(); + (removed, rules) + }; + + if !removed { + return http_xml_response(soap_fault(714, "NoSuchEntryInArray")); + } + if let Err(e) = nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await + { + warn!(error = %e, "upnp: apply nft on delete"); + return http_xml_response(soap_fault(500, "Action failed")); + } + http_xml_response(soap_success("DeletePortMapping", &[])) +} + +/// Extracts the inner text of a direct-child tag from a SOAP body. The +/// parser is tolerant enough for the well-formed payloads `igd-next` +/// emits: first `` wins, matching is case-sensitive on the tag +/// name. +fn extract_tag<'a>(body: &'a str, tag: &str) -> Option<&'a str> { + let open = format!("<{tag}>"); + let close = format!(""); + let start = body.find(&open)? + open.len(); + let end = body[start..].find(&close)? + start; + Some(body[start..end].trim()) +} + +fn soap_success(action: &str, out_args: &[(&str, &str)]) -> String { + let mut args = String::new(); + for (k, v) in out_args { + args.push_str(&format!("<{k}>{v}")); + } + format!( + r#" + + +{args} + +"# + ) +} + +fn soap_fault(code: u16, description: &str) -> String { + format!( + r#" + + + +s:Client +UPnPError + + +{code} +{description} + + + + +"# + ) +} + +// Silence the unused import in the dispatch loop when PortmapRegistry is +// not directly referenced here. The registry is used via Context. +#[allow(dead_code)] +const _: fn() -> Option = || None; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn m_search_discovery_is_recognized() { + let body = "M-SEARCH * HTTP/1.1\r\nHost: 239.255.255.250:1900\r\nMan: \"ssdp:discover\"\r\nST: urn:schemas-upnp-org:device:InternetGatewayDevice:1\r\nMX: 3\r\n\r\n"; + assert!(is_m_search(body)); + let st = extract_search_target(body); + assert!(search_target_matches(st)); + } + + #[test] + fn rootdesc_mentions_wan_ip_connection_service() { + let xml = build_root_description(Ipv4Addr::new(198, 51, 100, 1)); + assert!(xml.contains(SERVICE_TYPE)); + assert!(xml.contains(CONTROL_URL)); + assert!(xml.contains(SCPD_URL)); + } + + #[test] + fn scpd_lists_required_actions() { + let scpd = build_scpd(); + for action in [ + "GetExternalIPAddress", + "AddPortMapping", + "AddAnyPortMapping", + "DeletePortMapping", + ] { + assert!(scpd.contains(action), "scpd missing {action}"); + } + } + + #[test] + fn extract_tag_handles_soap_like_body() { + let body = r#" + + 8080 + UDP + 80 + 10.0.0.5 +"#; + assert_eq!(extract_tag(body, "NewExternalPort"), Some("8080")); + assert_eq!(extract_tag(body, "NewProtocol"), Some("UDP")); + assert_eq!(extract_tag(body, "NewInternalClient"), Some("10.0.0.5")); + } + + #[test] + fn soap_success_includes_out_args() { + let body = soap_success( + "GetExternalIPAddress", + &[("NewExternalIPAddress", "1.2.3.4")], + ); + assert!(body.contains("1.2.3.4")); + assert!(body.contains(SERVICE_TYPE)); + } +} diff --git a/patchbay/src/tests/portmap.rs b/patchbay/src/tests/portmap.rs index 1ae0faf..045e28e 100644 --- a/patchbay/src/tests/portmap.rs +++ b/patchbay/src/tests/portmap.rs @@ -216,6 +216,122 @@ async fn pcp_map_tcp_roundtrip() -> Result<()> { Ok(()) } +/// UPnP IGD probe + map: the device uses portmapper's UPnP client to +/// discover the router via SSDP, parses the device description, calls +/// `AddAnyPortMapping` via SOAP, and validates inbound UDP reaches it. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn upnp_map_udp_roundtrip() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::UpnpOnly) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + let watcher = lab + .add_device("watcher") + .iface("eth0", dc.id()) + .build() + .await?; + + let home_wan = home.uplink_ip().context("no home wan ip")?; + let local_port = NonZeroU16::new(50126).unwrap(); + + let mapping = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config { + enable_upnp: true, + enable_pcp: false, + enable_nat_pmp: false, + protocol: portmapper::Protocol::Udp, + }); + let probe = client.probe().await??; + if !probe.upnp { + anyhow::bail!("portmapper did not detect UPnP: {probe}"); + } + client.update_local_port(local_port); + let addr = wait_for_external(&client, Duration::from_secs(5)).await?; + anyhow::Ok(addr) + })? + .await??; + assert_eq!(*mapping.ip(), home_wan); + + let payload = b"hello-upnp"; + let jh = dev.spawn(move |_| async move { + let sock = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, local_port.get())) + .await + .context("dev bind")?; + let mut buf = [0u8; 64]; + let (n, _from) = tokio::time::timeout(Duration::from_secs(2), sock.recv_from(&mut buf)) + .await + .context("dev recv timeout")? + .context("dev recv")?; + anyhow::Ok(buf[..n].to_vec()) + })?; + + tokio::time::sleep(Duration::from_millis(100)).await; + + let external = mapping; + watcher + .spawn(move |_| async move { + let sock = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0)) + .await + .context("watcher bind")?; + sock.send_to(payload, external) + .await + .context("watcher send")?; + anyhow::Ok(()) + })? + .await??; + + let got = jh.await??; + assert_eq!(got.as_slice(), payload); + Ok(()) +} + +/// Portmapper's full client reports all three protocols available on a +/// router with `PortmapMode::All`. This proves the shared UDP socket +/// and the UPnP transport coexist. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn probe_all_protocols() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let _home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::All) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", _home.id()) + .build() + .await?; + let probe = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config::default()); + let probe = client.probe().await??; + anyhow::Ok(probe) + })? + .await??; + assert!(probe.nat_pmp, "nat_pmp not detected: {probe}"); + assert!(probe.pcp, "pcp not detected: {probe}"); + assert!(probe.upnp, "upnp not detected: {probe}"); + Ok(()) +} + /// NAT-PMP delete: a map request with lifetime=0 removes the previous /// mapping so inbound traffic stops reaching the device. #[tokio::test(flavor = "current_thread")] From 8eca87c18a74cbf8f0d2a11a1f15605afae5a862 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:23:33 +0200 Subject: [PATCH 06/13] feat: Router::set_portmap dynamic op Adds a runtime reconfiguration method that matches the existing set_nat_mode and set_firewall pattern. Dropping to PortmapMode::None tears the server down and flushes the ip portmap table; switching between non-None modes replaces the server and forgets every active mapping so clients re-probe. Adds two regression tests. set_nat_mode_preserves_active_mapping validates the C1 finding from the plan review: because the portmap rules live in a dedicated ip portmap table rather than inside ip nat, a NAT mode change no longer discards active mappings. The second test covers the shutdown path of set_portmap. --- patchbay/src/router.rs | 75 ++++++++++++++++++++ patchbay/src/tests/portmap.rs | 127 ++++++++++++++++++++++++++++++++++ 2 files changed, 202 insertions(+) diff --git a/patchbay/src/router.rs b/patchbay/src/router.rs index 5d06743..f18123a 100644 --- a/patchbay/src/router.rs +++ b/patchbay/src/router.rs @@ -680,6 +680,81 @@ impl Router { pub async fn spawn_reflector(&self, bind: SocketAddr) -> Result { self.lab.spawn_reflector_in(&self.ns, bind).await } + + /// Enables, disables, or reconfigures the portmap server at runtime. + /// + /// Dropping to [`PortmapMode::None`] tears the server down and flushes + /// the `ip portmap` nftables table. Switching between non-`None` modes + /// replaces the server, which forgets every active mapping: callers + /// should expect clients to reprobe. Passing the same mode the router + /// is already running is a no-op. + /// + /// # Errors + /// + /// Returns an error if the router has been removed, the namespace + /// runtime is gone, or starting the server inside the namespace + /// fails. + pub async fn set_portmap(&self, mode: PortmapMode) -> Result<()> { + let op = self + .lab + .with_router(self.id, |r| Arc::clone(&r.op)) + .ok_or_else(|| anyhow!("router removed"))?; + let _guard = op.lock().await; + let new_cfg = PortmapConfig::from_mode(mode); + + let (existing, ns, downstream_gw, wan_ip, downstream_cidr) = { + let inner = self.lab.core.lock().unwrap(); + let r = inner + .router(self.id) + .ok_or_else(|| anyhow!("router removed"))?; + ( + r.cfg.portmap, + r.ns.clone(), + r.downstream_gw, + r.upstream_ip, + r.downstream_cidr, + ) + }; + + if existing == new_cfg { + return Ok(()); + } + + // Tear down the current server before potentially starting a new + // one. Dropping the handle aborts its tasks; clear_portmap_rules + // removes any lingering nftables state. + { + let mut inner = self.lab.core.lock().unwrap(); + if let Some(r) = inner.router_mut(self.id) { + r.portmap_server.take(); + r.cfg.portmap = new_cfg; + } + } + portmap::nft::clear_portmap_rules(&self.lab.netns, &ns) + .await + .ok(); + + if new_cfg.any_enabled() { + let downstream_gw = downstream_gw + .ok_or_else(|| anyhow!("portmap requires an IPv4 downstream gateway"))?; + let wan_ip = wan_ip.ok_or_else(|| anyhow!("portmap requires an IPv4 uplink IP"))?; + let downstream_cidr = downstream_cidr + .ok_or_else(|| anyhow!("portmap requires an IPv4 downstream CIDR"))?; + let server = portmap::server::PortmapServer::start( + Arc::clone(&self.lab.netns), + ns, + new_cfg, + downstream_gw, + wan_ip, + downstream_cidr, + ) + .await?; + if let Some(r) = self.lab.core.lock().unwrap().router_mut(self.id) { + r.portmap_server = Some(server); + } + } + Ok(()) + } } // ───────────────────────────────────────────── diff --git a/patchbay/src/tests/portmap.rs b/patchbay/src/tests/portmap.rs index 045e28e..a66b0ed 100644 --- a/patchbay/src/tests/portmap.rs +++ b/patchbay/src/tests/portmap.rs @@ -332,6 +332,133 @@ async fn probe_all_protocols() -> Result<()> { Ok(()) } +/// `Router::set_nat_mode` must preserve active portmap rules because +/// the portmap nftables table is separate from `ip nat`. Regression test +/// for the adversarial review's C1 finding. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn set_nat_mode_preserves_active_mapping() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::NatPmpOnly) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + let watcher = lab + .add_device("watcher") + .iface("eth0", dc.id()) + .build() + .await?; + + let local_port = NonZeroU16::new(50127).unwrap(); + + let external = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config { + enable_upnp: false, + enable_pcp: false, + enable_nat_pmp: true, + protocol: portmapper::Protocol::Udp, + }); + client.probe().await??; + client.update_local_port(local_port); + let addr = wait_for_external(&client, Duration::from_secs(3)).await?; + anyhow::Ok(addr) + })? + .await??; + + // Change NAT mode after a mapping exists. The fix in the nat module + // and the dedicated portmap table should mean the DNAT rule + // survives. + home.set_nat_mode(Nat::Cgnat).await?; + + let payload = b"after-nat-mode-change"; + let jh = dev.spawn(move |_| async move { + let sock = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, local_port.get())) + .await + .context("dev bind")?; + let mut buf = [0u8; 64]; + let (n, _from) = tokio::time::timeout(Duration::from_secs(2), sock.recv_from(&mut buf)) + .await + .context("dev recv timeout")? + .context("dev recv")?; + anyhow::Ok(buf[..n].to_vec()) + })?; + tokio::time::sleep(Duration::from_millis(100)).await; + + watcher + .spawn(move |_| async move { + let sock = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0)) + .await + .context("watcher bind")?; + sock.send_to(payload, external) + .await + .context("watcher send")?; + anyhow::Ok(()) + })? + .await??; + + let got = jh.await??; + assert_eq!(got.as_slice(), payload); + Ok(()) +} + +/// `Router::set_portmap(None)` drops the server and removes the +/// nftables table, so a later probe from the same client finds no +/// portmap protocol. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn set_portmap_disables_server_at_runtime() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::NatPmpAndPcp) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + + let first = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config::default()); + let probe = client.probe().await??; + anyhow::Ok(probe) + })? + .await??; + assert!(first.nat_pmp && first.pcp, "first probe: {first}"); + + home.set_portmap(PortmapMode::None).await?; + + let second = dev + .spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config::default()); + let probe = client.probe().await??; + anyhow::Ok(probe) + })? + .await??; + assert!( + !second.nat_pmp && !second.pcp && !second.upnp, + "portmap still advertised after shutdown: {second}" + ); + Ok(()) +} + /// NAT-PMP delete: a map request with lifetime=0 removes the previous /// mapping so inbound traffic stops reaching the device. #[tokio::test(flavor = "current_thread")] From 4bb7d28c77d31a903b9bb16202000a1f608e6f97 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:36:58 +0200 Subject: [PATCH 07/13] refactor(portmap): consolidate shared context and trim module seams Moves the per-request context shared by NAT-PMP, PCP, and UPnP out of nat_pmp.rs and into server.rs as ServerContext. The old Context type was named after one protocol but consumed by three. Field visibility narrows from pub to pub(super) so mutation points stay explicit. Removes two const _: fn() sentinels that were silencing unused-import lints and the module-level #[allow(dead_code)] attributes that hid dead items in the foundation commit. Replaces the hand-rolled Opcode::from_byte matchers with strum::FromRepr. Adds an explicit threat-model note to portmap/mod.rs: source-IP authorization is adequate inside the simulator but not outside. PortmapConfig::from_mode now delegates to `impl From for PortmapConfig`, which is the idiomatic conversion direction. --- patchbay/src/portmap/config.rs | 22 +++++++++---- patchbay/src/portmap/mod.rs | 26 ++++++++------- patchbay/src/portmap/nat_pmp.rs | 55 ++++---------------------------- patchbay/src/portmap/pcp.rs | 32 ++++--------------- patchbay/src/portmap/registry.rs | 4 +++ patchbay/src/portmap/server.rs | 48 ++++++++++++++++++---------- patchbay/src/portmap/upnp.rs | 25 ++++++--------- patchbay/src/router.rs | 24 ++++++++------ 8 files changed, 103 insertions(+), 133 deletions(-) diff --git a/patchbay/src/portmap/config.rs b/patchbay/src/portmap/config.rs index 6ab4757..0637e99 100644 --- a/patchbay/src/portmap/config.rs +++ b/patchbay/src/portmap/config.rs @@ -40,8 +40,23 @@ pub struct PortmapConfig { } impl PortmapConfig { - /// Returns the config corresponding to `mode`. + /// Returns a [`PortmapConfig`] whose per-protocol flags match `mode`. + /// + /// Equivalent to `PortmapConfig::from(mode)`; kept for callers that + /// find the method form clearer at a call site. pub fn from_mode(mode: PortmapMode) -> Self { + mode.into() + } + + /// Returns `true` when any protocol is enabled. + #[must_use] + pub fn any_enabled(&self) -> bool { + self.enable_nat_pmp || self.enable_pcp || self.enable_upnp + } +} + +impl From for PortmapConfig { + fn from(mode: PortmapMode) -> Self { match mode { PortmapMode::None => Self::default(), PortmapMode::NatPmpOnly => Self { @@ -68,11 +83,6 @@ impl PortmapConfig { }, } } - - /// Returns `true` if any protocol is enabled. - pub fn any_enabled(&self) -> bool { - self.enable_nat_pmp || self.enable_pcp || self.enable_upnp - } } #[cfg(test)] diff --git a/patchbay/src/portmap/mod.rs b/patchbay/src/portmap/mod.rs index ca5c24b..d0cc3df 100644 --- a/patchbay/src/portmap/mod.rs +++ b/patchbay/src/portmap/mod.rs @@ -10,25 +10,29 @@ //! //! - [`config`] for user-facing builder types. //! - [`registry`] for the shared mapping registry and dedup logic. -//! - [`wire`] for server-side protocol encoders and decoders that match the -//! client-side wire format implemented in `portmapper`. +//! - [`nft`] for the dedicated `ip portmap` nftables table. +//! - [`server`] for the lifecycle handle and the shared [`server::ServerContext`]. +//! - [`nat_pmp`], [`pcp`], and [`upnp`] for per-protocol decoders, encoders, +//! and request handlers. //! -//! Per-protocol server tasks (`nat_pmp`, `pcp`, `upnp`) live alongside once -//! implemented. The public API surface is intentionally small: [`PortmapMode`] -//! and [`PortmapConfig`] for configuration; everything else is `pub(crate)`. +//! Public API surface is intentionally small: [`PortmapMode`] and +//! [`PortmapConfig`] for configuration. Every internal helper is +//! `pub(crate)`. +//! +//! # Threat model +//! +//! All three protocols authorize clients by source IPv4 address. That is +//! trivially spoofable on a real LAN and acceptable only inside the +//! patchbay simulator, where the downstream bridge is populated solely by +//! tests. Do not reuse this code in a production gateway without moving +//! authorization to a stronger primitive. pub use config::{PortmapConfig, PortmapMode}; mod config; -#[allow(dead_code)] pub(crate) mod nat_pmp; -#[allow(dead_code)] pub(crate) mod nft; -#[allow(dead_code)] pub(crate) mod pcp; -#[allow(dead_code)] pub(crate) mod registry; -#[allow(dead_code)] pub(crate) mod server; -#[allow(dead_code)] pub(crate) mod upnp; diff --git a/patchbay/src/portmap/nat_pmp.rs b/patchbay/src/portmap/nat_pmp.rs index 3d219d6..9ead6a6 100644 --- a/patchbay/src/portmap/nat_pmp.rs +++ b/patchbay/src/portmap/nat_pmp.rs @@ -12,22 +12,14 @@ //! dispatch-by-version-byte live in [`super::server`]; this module only //! handles packets whose version byte is `0`. -use std::{ - net::Ipv4Addr, - num::NonZeroU16, - sync::Arc, - time::{Duration, Instant}, -}; +use std::{net::Ipv4Addr, num::NonZeroU16, time::Duration}; -use ipnet::Ipv4Net; -use tokio::sync::Mutex; use tracing::{debug, trace, warn}; use super::{ nft, - registry::{AllocOutcome, MapProto, PortmapRegistry}, + registry::{AllocOutcome, MapProto}, }; -use crate::netns::NetnsManager; /// NAT-PMP wire-format version byte. pub(crate) const VERSION: u8 = 0; @@ -50,24 +42,13 @@ enum ResultCode { } #[repr(u8)] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, strum::FromRepr)] enum Opcode { DetermineExternalAddress = 0, MapUdp = 1, MapTcp = 2, } -impl Opcode { - fn from_byte(b: u8) -> Option { - match b { - 0 => Some(Self::DetermineExternalAddress), - 1 => Some(Self::MapUdp), - 2 => Some(Self::MapTcp), - _ => None, - } - } -} - /// Parsed NAT-PMP request. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Request { @@ -85,7 +66,7 @@ impl Request { if buf.len() < 2 || buf[0] != VERSION { return None; } - match Opcode::from_byte(buf[1])? { + match Opcode::from_repr(buf[1])? { Opcode::DetermineExternalAddress => Some(Request::ExternalAddress), op @ (Opcode::MapUdp | Opcode::MapTcp) => { if buf.len() < 12 { @@ -145,33 +126,9 @@ fn encode_map_response( buf } -/// Context shared between the dispatch loop and the handlers. -/// -/// `epoch` is the router-local epoch zero: response `epoch_time` fields -/// report seconds since this instant. Real routers use seconds since the -/// SSDP/NAT-PMP service started; the test harness only needs this value to -/// be monotonic and non-zero. -pub(crate) struct Context { - pub registry: Arc>, - pub netns: Arc, - pub ns: Arc, - pub wan_ip: Ipv4Addr, - pub downstream_cidr: Ipv4Net, - pub epoch: Instant, - /// Clamp for requested lifetimes. Matches the recommended 2-hour - /// maximum from RFC 6886 section 3.3. - pub max_lifetime: Duration, -} - -impl Context { - fn epoch_time(&self) -> u32 { - self.epoch.elapsed().as_secs() as u32 - } -} - /// Handles a single NAT-PMP request and returns the response bytes. pub(crate) async fn handle_request( - ctx: &Context, + ctx: &super::server::ServerContext, client_ip: Ipv4Addr, packet: &[u8], ) -> Option> { @@ -232,7 +189,7 @@ pub(crate) async fn handle_request( } async fn handle_map( - ctx: &Context, + ctx: &super::server::ServerContext, client_ip: Ipv4Addr, proto: MapProto, local_port: u16, diff --git a/patchbay/src/portmap/pcp.rs b/patchbay/src/portmap/pcp.rs index 2d1388f..3aeab3c 100644 --- a/patchbay/src/portmap/pcp.rs +++ b/patchbay/src/portmap/pcp.rs @@ -20,9 +20,9 @@ use std::{ use tracing::{debug, trace}; use super::{ - nat_pmp::Context, nft, - registry::{AllocOutcome, MapProto, MappingKey}, + registry::{AllocOutcome, MapProto}, + server::ServerContext, }; /// PCP wire-format version byte. @@ -40,22 +40,12 @@ const HEADER_SIZE: usize = 24; const MAP_DATA_SIZE: usize = 36; #[repr(u8)] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, strum::FromRepr)] enum Opcode { Announce = 0, Map = 1, } -impl Opcode { - fn from_byte(b: u8) -> Option { - match b { - 0 => Some(Self::Announce), - 1 => Some(Self::Map), - _ => None, - } - } -} - #[repr(u8)] #[derive(Clone, Copy, Debug)] enum ResultCode { @@ -97,7 +87,6 @@ enum Request { }, /// Decoded but unsupported (e.g. protocol=0 for all protocols). UnsupportedProtocol { - lifetime_seconds: u32, data: MapData, }, } @@ -108,7 +97,7 @@ fn decode(buf: &[u8]) -> Option { if buf.len() < HEADER_SIZE || buf[0] != VERSION { return None; } - let opcode = Opcode::from_byte(buf[1])?; + let opcode = Opcode::from_repr(buf[1])?; let lifetime_seconds = u32::from_be_bytes([buf[4], buf[5], buf[6], buf[7]]); match opcode { Opcode::Announce => Some(Request::Announce), @@ -130,7 +119,6 @@ fn decode(buf: &[u8]) -> Option { PROTO_TCP => MapProto::Tcp, _ => { return Some(Request::UnsupportedProtocol { - lifetime_seconds, data: MapData { nonce, protocol: MapProto::Udp, @@ -214,7 +202,7 @@ fn encode_map_response(r: MapResponse) -> Vec { /// Handles a single PCP packet. Returns the response bytes or `None` /// when the packet is malformed and should be silently dropped. pub(crate) async fn handle_request( - ctx: &Context, + ctx: &ServerContext, client_ip: Ipv4Addr, packet: &[u8], ) -> Option> { @@ -271,7 +259,7 @@ pub(crate) async fn handle_request( } async fn handle_map( - ctx: &Context, + ctx: &ServerContext, client_ip: Ipv4Addr, lifetime_seconds: u32, data: MapData, @@ -392,14 +380,6 @@ async fn handle_map( }) } -/// Silences a linter warning about `MappingKey` being unused in this -/// module while keeping the import paths clear for readers. -#[allow(dead_code)] -const _: fn() -> MappingKey = || MappingKey { - proto: MapProto::Udp, - external_port: NonZeroU16::new(1).unwrap(), -}; - #[cfg(test)] mod tests { use super::*; diff --git a/patchbay/src/portmap/registry.rs b/patchbay/src/portmap/registry.rs index a6560c9..37126df 100644 --- a/patchbay/src/portmap/registry.rs +++ b/patchbay/src/portmap/registry.rs @@ -102,11 +102,13 @@ impl PortmapRegistry { } /// Returns the current mapping count. + #[allow(dead_code)] pub(crate) fn len(&self) -> usize { self.by_external.len() } /// Looks up a mapping by its external slot. + #[allow(dead_code)] pub(crate) fn get(&self, key: MappingKey) -> Option<&Mapping> { self.by_external.get(&key) } @@ -246,6 +248,7 @@ impl PortmapRegistry { /// Removes and returns every mapping whose deadline is at or before /// `now`. Called periodically by the reaper. + #[allow(dead_code)] pub(crate) fn reap_expired(&mut self, now: Instant) -> Vec { let expired_keys: Vec = self .by_external @@ -260,6 +263,7 @@ impl PortmapRegistry { } /// Drains every mapping. Used on server shutdown. + #[allow(dead_code)] pub(crate) fn drain(&mut self) -> Vec { self.by_internal.clear(); self.by_external.drain().map(|(_, m)| m).collect() diff --git a/patchbay/src/portmap/server.rs b/patchbay/src/portmap/server.rs index 526ab3d..ba96ecf 100644 --- a/patchbay/src/portmap/server.rs +++ b/patchbay/src/portmap/server.rs @@ -2,8 +2,8 @@ //! //! [`PortmapServer`] owns the UDP 5351 socket shared between NAT-PMP and //! PCP, the shared mapping registry, and the per-protocol dispatch task. -//! UPnP lands alongside in Step 5; until then only the NAT-PMP dispatcher -//! is wired in. +//! When UPnP is enabled it also owns the SSDP listener and the HTTP +//! listener spawned from [`super::upnp`]. //! //! The server follows the same lifecycle pattern as //! [`crate::dns_server::DnsServer`]: a cloneable handle backed by @@ -31,6 +31,31 @@ use crate::netns::NetnsManager; /// recommendation from RFC 6886 section 3.3. const MAX_LIFETIME_SECS: u64 = 2 * 60 * 60; +/// Shared state threaded through every protocol handler. +/// +/// `epoch` is the router-local epoch zero: response `epoch_time` fields +/// report seconds since this instant. Real routers use seconds since the +/// portmap service started; the simulator only needs the value to be +/// monotonic and non-zero within a single server instance. +pub(crate) struct ServerContext { + pub(super) registry: Arc>, + pub(super) netns: Arc, + pub(super) ns: Arc, + pub(super) wan_ip: Ipv4Addr, + pub(super) downstream_cidr: Ipv4Net, + pub(super) epoch: Instant, + /// Clamp for requested lifetimes. Matches the recommended 2-hour + /// maximum from RFC 6886 section 3.3. + pub(super) max_lifetime: Duration, +} + +impl ServerContext { + /// Seconds since this server started, saturating at `u32::MAX`. + pub(super) fn epoch_time(&self) -> u32 { + self.epoch.elapsed().as_secs().min(u64::from(u32::MAX)) as u32 + } +} + /// Cloneable handle to a per-router portmap server. #[derive(Clone)] pub(crate) struct PortmapServer { @@ -39,6 +64,7 @@ pub(crate) struct PortmapServer { struct PortmapServerInner { cfg: PortmapConfig, + #[allow(dead_code)] registry: Arc>, netns: Arc, ns: Arc, @@ -81,7 +107,7 @@ impl PortmapServer { .await .ok(); - let ctx = Arc::new(nat_pmp::Context { + let ctx = Arc::new(ServerContext { registry: registry.clone(), netns: netns.clone(), ns: ns.clone(), @@ -114,18 +140,6 @@ impl PortmapServer { }) } - /// Returns the configuration the server was started with. - #[allow(dead_code)] - pub(crate) fn config(&self) -> PortmapConfig { - self.inner.cfg - } - - /// Returns the current mapping count. Intended for tests and metrics. - #[allow(dead_code)] - pub(crate) async fn mapping_count(&self) -> usize { - self.inner.registry.lock().await.len() - } - /// Tears down the server's nftables table. Callers should call this /// before dropping the last handle so the rules do not outlive the /// server. Missing rules are not an error: the helper is idempotent. @@ -138,7 +152,7 @@ fn spawn_dispatch( netns: Arc, ns: Arc, cfg: PortmapConfig, - ctx: Arc, + ctx: Arc, downstream_gw: Ipv4Addr, ) -> Result> { let std_socket: std::net::UdpSocket = netns.run_closure_in(&ns, move || { @@ -170,7 +184,7 @@ fn spawn_dispatch( Ok(AbortOnDropHandle::new(handle)) } -async fn dispatch_loop(socket: UdpSocket, ctx: Arc, cfg: PortmapConfig) { +async fn dispatch_loop(socket: UdpSocket, ctx: Arc, cfg: PortmapConfig) { let mut buf = vec![0u8; 1500]; loop { let (len, src) = match socket.recv_from(&mut buf).await { diff --git a/patchbay/src/portmap/upnp.rs b/patchbay/src/portmap/upnp.rs index ce512f8..9e33893 100644 --- a/patchbay/src/portmap/upnp.rs +++ b/patchbay/src/portmap/upnp.rs @@ -37,9 +37,9 @@ use tokio::{ use tracing::{debug, trace, warn}; use super::{ - nat_pmp::Context, nft, - registry::{AllocOutcome, MapProto, PortmapRegistry}, + registry::{AllocOutcome, MapProto, MappingKey}, + server::ServerContext, }; use crate::netns::NetnsManager; @@ -73,7 +73,7 @@ const ROOT_DESC_URL: &str = "/rootDesc.xml"; pub(crate) async fn spawn( netns: Arc, ns: Arc, - ctx: Arc, + ctx: Arc, downstream_gw: Ipv4Addr, ) -> Result<(JoinHandle<()>, JoinHandle<()>)> { let listener = bind_http_listener(&netns, &ns, downstream_gw).await?; @@ -207,7 +207,7 @@ fn search_target_matches(target: Option<&str>) -> bool { } } -async fn run_http(listener: TcpListener, ctx: Arc) { +async fn run_http(listener: TcpListener, ctx: Arc) { loop { let (stream, peer) = match listener.accept().await { Ok(v) => v, @@ -227,7 +227,7 @@ async fn run_http(listener: TcpListener, ctx: Arc) { async fn handle_http_connection( mut stream: tokio::net::TcpStream, - ctx: Arc, + ctx: Arc, peer: std::net::SocketAddr, ) -> Result<()> { let mut buf = Vec::with_capacity(4096); @@ -300,7 +300,7 @@ async fn route_request( request_line: &str, headers: &[(String, String)], body: &[u8], - ctx: &Context, + ctx: &ServerContext, ) -> Vec { let mut parts = request_line.split_whitespace(); let method = parts.next().unwrap_or(""); @@ -449,7 +449,7 @@ fn build_scpd() -> &'static str { "# } -async fn handle_soap(action: &str, body: &str, ctx: &Context) -> Vec { +async fn handle_soap(action: &str, body: &str, ctx: &ServerContext) -> Vec { trace!(?action, "upnp soap request"); let action_name = action.split('#').next_back().unwrap_or(""); match action_name { @@ -464,7 +464,7 @@ async fn handle_soap(action: &str, body: &str, ctx: &Context) -> Vec { } } -async fn handle_add_port_mapping(body: &str, ctx: &Context, pick_any: bool) -> Vec { +async fn handle_add_port_mapping(body: &str, ctx: &ServerContext, pick_any: bool) -> Vec { let external_port = match extract_tag(body, "NewExternalPort").and_then(|s| s.parse::().ok()) { Some(p) => p, @@ -547,7 +547,7 @@ async fn handle_add_port_mapping(body: &str, ctx: &Context, pick_any: bool) -> V } } -async fn handle_delete_port_mapping(body: &str, ctx: &Context) -> Vec { +async fn handle_delete_port_mapping(body: &str, ctx: &ServerContext) -> Vec { let external_port = match extract_tag(body, "NewExternalPort").and_then(|s| s.parse::().ok()) { Some(p) => p, @@ -566,7 +566,7 @@ async fn handle_delete_port_mapping(body: &str, ctx: &Context) -> Vec { let (removed, rules_snapshot) = { let mut registry = ctx.registry.lock().await; let removed = registry - .remove(super::registry::MappingKey { + .remove(MappingKey { proto: protocol, external_port: external_nz, }) @@ -633,11 +633,6 @@ fn soap_fault(code: u16, description: &str) -> String { ) } -// Silence the unused import in the dispatch loop when PortmapRegistry is -// not directly referenced here. The registry is used via Context. -#[allow(dead_code)] -const _: fn() -> Option = || None; - #[cfg(test)] mod tests { use super::*; diff --git a/patchbay/src/router.rs b/patchbay/src/router.rs index f18123a..33285f9 100644 --- a/patchbay/src/router.rs +++ b/patchbay/src/router.rs @@ -721,18 +721,24 @@ impl Router { } // Tear down the current server before potentially starting a new - // one. Dropping the handle aborts its tasks; clear_portmap_rules - // removes any lingering nftables state. - { + // one. Shutdown removes lingering nftables state; dropping the + // handle aborts its tasks. + let existing_server = { let mut inner = self.lab.core.lock().unwrap(); - if let Some(r) = inner.router_mut(self.id) { - r.portmap_server.take(); + let taken = inner.router_mut(self.id).and_then(|r| { r.cfg.portmap = new_cfg; - } + r.portmap_server.take() + }); + taken + }; + if let Some(server) = existing_server { + server.shutdown().await.ok(); + drop(server); + } else { + portmap::nft::clear_portmap_rules(&self.lab.netns, &ns) + .await + .ok(); } - portmap::nft::clear_portmap_rules(&self.lab.netns, &ns) - .await - .ok(); if new_cfg.any_enabled() { let downstream_gw = downstream_gw From 3f269696db63994d17beca307feaae7848c36b62 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:43:05 +0200 Subject: [PATCH 08/13] fix(portmap): tighten UPnP and PCP authorization UPnP: AddPortMapping now requires NewInternalClient to equal the caller's source IP, not just be within the downstream CIDR. Without that, any LAN host could DNAT inbound traffic destined for the router to another LAN host's socket; real consumer firmware has been abused this way. DeletePortMapping gains a symmetric check so only the mapping's original owner can revoke it. UPnP HTTP: body size capped at 64 KiB before buffering, with HTTP 413 for clients that declare a larger Content-Length. Header and body reads now honor 10-second timeouts, so a slowloris client that opens a connection and dribbles bytes no longer holds a task forever. The accept loop bounds concurrent connections at 128 via a semaphore; excess connections are dropped immediately rather than spawning an unbounded number of handler tasks. Short bodies (peer closes before Content-Length bytes arrive) now fail with an explicit error instead of passing a silently-truncated buffer into SOAP parsing. PCP: the client_addr field inside the PCP request header is now validated against the UDP packet's source IP. RFC 6887 section 8.1 requires this check; a mismatch returns AddressMismatch (result code 12) instead of the prior silent truncation. --- patchbay/src/portmap/pcp.rs | 134 +++++++++++++++++++--------- patchbay/src/portmap/upnp.rs | 168 +++++++++++++++++++++++++---------- 2 files changed, 214 insertions(+), 88 deletions(-) diff --git a/patchbay/src/portmap/pcp.rs b/patchbay/src/portmap/pcp.rs index 3aeab3c..3073b2e 100644 --- a/patchbay/src/portmap/pcp.rs +++ b/patchbay/src/portmap/pcp.rs @@ -78,6 +78,13 @@ struct MapData { external_address: Ipv6Addr, } +#[derive(Debug)] +struct Decoded { + /// Client IP as declared in the PCP header bytes 8-23, IPv4-mapped. + client_addr: Ipv6Addr, + request: Request, +} + #[derive(Debug)] enum Request { Announce, @@ -93,14 +100,17 @@ enum Request { /// Decodes a PCP request. Returns `None` on malformed packets so the /// server can silently drop them per RFC 6887 section 8.2. -fn decode(buf: &[u8]) -> Option { +fn decode(buf: &[u8]) -> Option { if buf.len() < HEADER_SIZE || buf[0] != VERSION { return None; } let opcode = Opcode::from_repr(buf[1])?; let lifetime_seconds = u32::from_be_bytes([buf[4], buf[5], buf[6], buf[7]]); - match opcode { - Opcode::Announce => Some(Request::Announce), + let mut client_bytes = [0u8; 16]; + client_bytes.copy_from_slice(&buf[8..24]); + let client_addr = Ipv6Addr::from(client_bytes); + let request = match opcode { + Opcode::Announce => Request::Announce, Opcode::Map => { if buf.len() < HEADER_SIZE + MAP_DATA_SIZE { return None; @@ -114,33 +124,43 @@ fn decode(buf: &[u8]) -> Option { let mut addr_bytes = [0u8; 16]; addr_bytes.copy_from_slice(&data[20..36]); let external_address = Ipv6Addr::from(addr_bytes); - let protocol = match proto_byte { - PROTO_UDP => MapProto::Udp, - PROTO_TCP => MapProto::Tcp, - _ => { - return Some(Request::UnsupportedProtocol { - data: MapData { - nonce, - protocol: MapProto::Udp, - local_port, - external_port, - external_address, - }, - }); - } - }; - Some(Request::Map { - lifetime_seconds, - data: MapData { - nonce, - protocol, - local_port, - external_port, - external_address, + match proto_byte { + PROTO_UDP => Request::Map { + lifetime_seconds, + data: MapData { + nonce, + protocol: MapProto::Udp, + local_port, + external_port, + external_address, + }, }, - }) + PROTO_TCP => Request::Map { + lifetime_seconds, + data: MapData { + nonce, + protocol: MapProto::Tcp, + local_port, + external_port, + external_address, + }, + }, + _ => Request::UnsupportedProtocol { + data: MapData { + nonce, + protocol: MapProto::Udp, + local_port, + external_port, + external_address, + }, + }, + } } - } + }; + Some(Decoded { + client_addr, + request, + }) } /// Encodes a response header. The opcode-specific data follows the @@ -206,7 +226,10 @@ pub(crate) async fn handle_request( client_ip: Ipv4Addr, packet: &[u8], ) -> Option> { - let request = match decode(packet) { + let Decoded { + client_addr, + request, + } = match decode(packet) { Some(r) => r, None => { trace!(?client_ip, len = packet.len(), "pcp: invalid request"); @@ -214,18 +237,43 @@ pub(crate) async fn handle_request( } }; + let epoch = ctx.epoch_time(); + + // RFC 6887 section 8.1: the client address in the PCP header MUST + // match the packet's source IP (or be its IPv4-mapped form). If it + // does not, respond with AddressMismatch. + if client_addr != client_ip.to_ipv6_mapped() { + debug!( + ?client_ip, + ?client_addr, + "pcp: client_addr header does not match source IP" + ); + return Some(match request { + Request::Announce => encode_announce_response(ResultCode::AddressMismatch, epoch), + Request::Map { data, .. } | Request::UnsupportedProtocol { data, .. } => { + encode_map_response(MapResponse { + result: ResultCode::AddressMismatch, + lifetime_seconds: 0, + epoch_time: epoch, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: 0, + external_address: Ipv4Addr::UNSPECIFIED, + }) + } + }); + } + if !ctx.downstream_cidr.contains(&client_ip) { debug!(?client_ip, cidr = %ctx.downstream_cidr, "pcp: client not on downstream subnet"); return Some(match request { - Request::Announce => encode_announce_response( - ResultCode::NotAuthorized, - ctx.epoch.elapsed().as_secs() as u32, - ), + Request::Announce => encode_announce_response(ResultCode::NotAuthorized, epoch), Request::Map { data, .. } | Request::UnsupportedProtocol { data, .. } => { encode_map_response(MapResponse { result: ResultCode::NotAuthorized, lifetime_seconds: 0, - epoch_time: ctx.epoch.elapsed().as_secs() as u32, + epoch_time: epoch, nonce: data.nonce, protocol: data.protocol, local_port: data.local_port, @@ -237,14 +285,11 @@ pub(crate) async fn handle_request( } match request { - Request::Announce => Some(encode_announce_response( - ResultCode::Success, - ctx.epoch.elapsed().as_secs() as u32, - )), + Request::Announce => Some(encode_announce_response(ResultCode::Success, epoch)), Request::UnsupportedProtocol { data, .. } => Some(encode_map_response(MapResponse { result: ResultCode::UnsuppProtocol, lifetime_seconds: 0, - epoch_time: ctx.epoch.elapsed().as_secs() as u32, + epoch_time: epoch, nonce: data.nonce, protocol: data.protocol, local_port: data.local_port, @@ -389,7 +434,7 @@ mod tests { let mut buf = [0u8; HEADER_SIZE]; buf[0] = VERSION; buf[1] = Opcode::Announce as u8; - match decode(&buf).unwrap() { + match decode(&buf).unwrap().request { Request::Announce => (), other => panic!("{other:?}"), } @@ -401,12 +446,17 @@ mod tests { buf[0] = VERSION; buf[1] = Opcode::Map as u8; buf[4..8].copy_from_slice(&300u32.to_be_bytes()); + // client_addr = 10.0.0.5 IPv4-mapped + let client = Ipv4Addr::new(10, 0, 0, 5); + buf[8..24].copy_from_slice(&client.to_ipv6_mapped().octets()); let data = &mut buf[HEADER_SIZE..]; data[0..12].copy_from_slice(&[1u8; 12]); data[12] = PROTO_UDP; data[16..18].copy_from_slice(&1234u16.to_be_bytes()); data[18..20].copy_from_slice(&5678u16.to_be_bytes()); - match decode(&buf).unwrap() { + let decoded = decode(&buf).unwrap(); + assert_eq!(decoded.client_addr, client.to_ipv6_mapped()); + match decoded.request { Request::Map { lifetime_seconds, data, @@ -443,7 +493,7 @@ mod tests { buf[0] = VERSION; buf[1] = Opcode::Map as u8; buf[HEADER_SIZE + 12] = 99; // unknown protocol - match decode(&buf).unwrap() { + match decode(&buf).unwrap().request { Request::UnsupportedProtocol { .. } => (), other => panic!("{other:?}"), } diff --git a/patchbay/src/portmap/upnp.rs b/patchbay/src/portmap/upnp.rs index 9e33893..01d1bdd 100644 --- a/patchbay/src/portmap/upnp.rs +++ b/patchbay/src/portmap/upnp.rs @@ -32,6 +32,7 @@ use anyhow::{Context as _, Result}; use tokio::{ io::{AsyncReadExt, AsyncWriteExt}, net::{TcpListener, UdpSocket}, + sync::Semaphore, task::JoinHandle, }; use tracing::{debug, trace, warn}; @@ -69,6 +70,21 @@ const SCPD_URL: &str = "/WANIPCn.xml"; /// Device description URL served by the HTTP server. const ROOT_DESC_URL: &str = "/rootDesc.xml"; +/// Upper bound on an HTTP request body. Generously sized for SOAP. +/// Bodies above this are rejected with HTTP 413 before they are buffered. +const MAX_BODY_SIZE: usize = 64 * 1024; + +/// Per-request header read timeout. Defends against slowloris clients +/// that open a TCP connection and dribble bytes. +const HEADER_TIMEOUT: Duration = Duration::from_secs(10); + +/// Per-request body read timeout. +const BODY_TIMEOUT: Duration = Duration::from_secs(10); + +/// Cap on concurrent HTTP connections. Accepted connections beyond this +/// count wait on a semaphore; no per-source fairness, just a backstop. +const MAX_HTTP_CONNECTIONS: usize = 128; + /// Spawns the UPnP IGD server tasks and returns their abort handles. pub(crate) async fn spawn( netns: Arc, @@ -208,6 +224,7 @@ fn search_target_matches(target: Option<&str>) -> bool { } async fn run_http(listener: TcpListener, ctx: Arc) { + let connection_limit = Arc::new(Semaphore::new(MAX_HTTP_CONNECTIONS)); loop { let (stream, peer) = match listener.accept().await { Ok(v) => v, @@ -216,9 +233,27 @@ async fn run_http(listener: TcpListener, ctx: Arc) { continue; } }; + // Reject IPv6 peers early: the advertised LOCATION is IPv4 only. + let peer_v4 = match peer { + std::net::SocketAddr::V4(v) => *v.ip(), + std::net::SocketAddr::V6(_) => continue, + }; + let permit = match connection_limit.clone().try_acquire_owned() { + Ok(p) => p, + Err(_) => { + debug!(%peer, "upnp http: connection limit reached, rejecting"); + continue; + } + }; let ctx = ctx.clone(); + // The accept task is wrapped in AbortOnDropHandle at the server + // level; the per-connection handler runs as a detached + // `tokio::spawn` but carries the semaphore permit, which releases + // when the future drops. Dropping the accept task therefore + // stops taking new connections; in-flight ones drain naturally. tokio::spawn(async move { - if let Err(e) = handle_http_connection(stream, ctx, peer).await { + let _permit = permit; + if let Err(e) = handle_http_connection(stream, ctx, peer_v4).await { debug!(error = %e, %peer, "upnp http: connection error"); } }); @@ -228,24 +263,32 @@ async fn run_http(listener: TcpListener, ctx: Arc) { async fn handle_http_connection( mut stream: tokio::net::TcpStream, ctx: Arc, - peer: std::net::SocketAddr, + peer: Ipv4Addr, ) -> Result<()> { let mut buf = Vec::with_capacity(4096); let mut tmp = [0u8; 2048]; - // Read until we have complete headers. - let header_end = loop { - let n = stream.read(&mut tmp).await?; - if n == 0 { - return Ok(()); - } - buf.extend_from_slice(&tmp[..n]); - if let Some(pos) = find_headers_end(&buf) { - break pos; - } - if buf.len() > 65_536 { - anyhow::bail!("headers too large"); + // Read until we have complete headers, or the read deadline fires. + let header_end = tokio::time::timeout(HEADER_TIMEOUT, async { + loop { + let n = stream.read(&mut tmp).await?; + if n == 0 { + return Ok::(0); + } + buf.extend_from_slice(&tmp[..n]); + if let Some(pos) = find_headers_end(&buf) { + return Ok(pos); + } + if buf.len() > MAX_BODY_SIZE { + anyhow::bail!("headers too large"); + } } - }; + }) + .await + .context("upnp http: header read timeout")??; + + if header_end == 0 { + return Ok(()); + } let (request_line, headers) = parse_request_headers(&buf[..header_end]).context("invalid http request headers")?; @@ -254,23 +297,28 @@ async fn handle_http_connection( .find(|(k, _)| k.eq_ignore_ascii_case("content-length")) .and_then(|(_, v)| v.trim().parse::().ok()) .unwrap_or(0); + if content_length > MAX_BODY_SIZE { + let response = simple_status_response(413, "Payload Too Large"); + stream.write_all(&response).await.ok(); + return Ok(()); + } let body_start = header_end + 4; - while buf.len() < body_start + content_length { - let n = stream.read(&mut tmp).await?; - if n == 0 { - break; + tokio::time::timeout(BODY_TIMEOUT, async { + while buf.len() < body_start + content_length { + let n = stream.read(&mut tmp).await?; + if n == 0 { + anyhow::bail!("short body: peer closed before content-length bytes"); + } + buf.extend_from_slice(&tmp[..n]); } - buf.extend_from_slice(&tmp[..n]); - } - let body = &buf[body_start..body_start + content_length.min(buf.len() - body_start)]; - - // Reject IPv6 peers: the advertised LOCATION is IPv4 only. - if matches!(peer, std::net::SocketAddr::V6(_)) { - return Ok(()); - } + Ok::<(), anyhow::Error>(()) + }) + .await + .context("upnp http: body read timeout")??; + let body = &buf[body_start..body_start + content_length]; - let response = route_request(&request_line, &headers, body, &ctx).await; + let response = route_request(&request_line, &headers, body, &ctx, peer).await; stream.write_all(&response).await?; stream.flush().await?; Ok(()) @@ -301,6 +349,7 @@ async fn route_request( headers: &[(String, String)], body: &[u8], ctx: &ServerContext, + peer: Ipv4Addr, ) -> Vec { let mut parts = request_line.split_whitespace(); let method = parts.next().unwrap_or(""); @@ -316,7 +365,7 @@ async fn route_request( .map(|(_, v)| v.trim_matches('"').to_string()) .unwrap_or_default(); let body_str = std::str::from_utf8(body).unwrap_or(""); - handle_soap(&action_header, body_str, ctx).await + handle_soap(&action_header, body_str, ctx, peer).await } _ => simple_status_response(404, "Not Found"), } @@ -449,22 +498,27 @@ fn build_scpd() -> &'static str { "# } -async fn handle_soap(action: &str, body: &str, ctx: &ServerContext) -> Vec { - trace!(?action, "upnp soap request"); +async fn handle_soap(action: &str, body: &str, ctx: &ServerContext, peer: Ipv4Addr) -> Vec { + trace!(?action, %peer, "upnp soap request"); let action_name = action.split('#').next_back().unwrap_or(""); match action_name { "GetExternalIPAddress" => http_xml_response(soap_success( "GetExternalIPAddress", &[("NewExternalIPAddress", &ctx.wan_ip.to_string())], )), - "AddPortMapping" => handle_add_port_mapping(body, ctx, false).await, - "AddAnyPortMapping" => handle_add_port_mapping(body, ctx, true).await, - "DeletePortMapping" => handle_delete_port_mapping(body, ctx).await, + "AddPortMapping" => handle_add_port_mapping(body, ctx, peer, false).await, + "AddAnyPortMapping" => handle_add_port_mapping(body, ctx, peer, true).await, + "DeletePortMapping" => handle_delete_port_mapping(body, ctx, peer).await, _ => http_xml_response(soap_fault(401, "Invalid Action")), } } -async fn handle_add_port_mapping(body: &str, ctx: &ServerContext, pick_any: bool) -> Vec { +async fn handle_add_port_mapping( + body: &str, + ctx: &ServerContext, + peer: Ipv4Addr, + pick_any: bool, +) -> Vec { let external_port = match extract_tag(body, "NewExternalPort").and_then(|s| s.parse::().ok()) { Some(p) => p, @@ -491,8 +545,15 @@ async fn handle_add_port_mapping(body: &str, ctx: &ServerContext, pick_any: bool .and_then(|s| s.parse::().ok()) .unwrap_or(0); - // Security: internal client must be on our downstream subnet. - if !ctx.downstream_cidr.contains(&internal_client) { + // Security: internal client must match the caller's source IP and + // fall inside the downstream subnet. Enforcing caller==client stops + // a LAN host from hijacking inbound traffic destined for a peer. + if !ctx.downstream_cidr.contains(&internal_client) || internal_client != peer { + debug!( + %peer, + %internal_client, + "upnp: reject AddPortMapping with mismatched/off-subnet client" + ); return http_xml_response(soap_fault(606, "Action not authorized")); } @@ -547,7 +608,7 @@ async fn handle_add_port_mapping(body: &str, ctx: &ServerContext, pick_any: bool } } -async fn handle_delete_port_mapping(body: &str, ctx: &ServerContext) -> Vec { +async fn handle_delete_port_mapping(body: &str, ctx: &ServerContext, peer: Ipv4Addr) -> Vec { let external_port = match extract_tag(body, "NewExternalPort").and_then(|s| s.parse::().ok()) { Some(p) => p, @@ -563,18 +624,33 @@ async fn handle_delete_port_mapping(body: &str, ctx: &ServerContext) -> Vec _ => return http_xml_response(soap_fault(402, "Invalid Args")), }; - let (removed, rules_snapshot) = { + // Security: only the mapping's owner may delete it. Look up the + // mapping, compare internal IP to caller, then remove. + let key = MappingKey { + proto: protocol, + external_port: external_nz, + }; + let (authorized, removed, rules_snapshot) = { let mut registry = ctx.registry.lock().await; - let removed = registry - .remove(MappingKey { - proto: protocol, - external_port: external_nz, - }) - .is_some(); + let owner = registry.get(key).map(|m| m.internal_ip); + let authorized = owner == Some(peer); + let removed = if authorized { + registry.remove(key).is_some() + } else { + false + }; let rules: Vec<_> = registry.iter().cloned().collect(); - (removed, rules) + (authorized, removed, rules) }; + if !authorized { + debug!( + %peer, + ext = external_nz.get(), + "upnp: reject DeletePortMapping; caller is not the mapping owner", + ); + return http_xml_response(soap_fault(606, "Action not authorized")); + } if !removed { return http_xml_response(soap_fault(714, "NoSuchEntryInArray")); } From 2ac9bc6508c810738ee5be50b51f15fd37416cdd Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:47:48 +0200 Subject: [PATCH 09/13] fix(portmap): serialize nft applies and reap expired mappings nft applies previously ran without the registry lock held, so two concurrent allocate+apply sequences could land out of order and leave nftables reflecting an older snapshot than the registry. Every handler now calls ServerContext::apply_after_mutation while holding the registry guard, which both serializes applies and makes apply-failure rollback atomic: a just-created mapping whose nft apply fails is removed from the registry before the error propagates, so clients that receive an error code do not find an orphan entry later. Adds a background reaper that runs every 30 seconds, drops mappings whose deadline has elapsed, and regenerates the nft table. Without this, a client that walks away from a 2-hour lease kept the entry for the router's lifetime, which mattered only in long-running simulations but was a real correctness gap. --- patchbay/src/portmap/nat_pmp.rs | 53 ++++++++++++++++---------- patchbay/src/portmap/pcp.rs | 41 ++++++++++++-------- patchbay/src/portmap/server.rs | 64 ++++++++++++++++++++++++++++++++ patchbay/src/portmap/upnp.rs | 66 +++++++++++++++------------------ 4 files changed, 152 insertions(+), 72 deletions(-) diff --git a/patchbay/src/portmap/nat_pmp.rs b/patchbay/src/portmap/nat_pmp.rs index 9ead6a6..322ebc8 100644 --- a/patchbay/src/portmap/nat_pmp.rs +++ b/patchbay/src/portmap/nat_pmp.rs @@ -14,12 +14,9 @@ use std::{net::Ipv4Addr, num::NonZeroU16, time::Duration}; -use tracing::{debug, trace, warn}; +use tracing::{debug, trace}; -use super::{ - nft, - registry::{AllocOutcome, MapProto}, -}; +use super::registry::{AllocOutcome, MapProto}; /// NAT-PMP wire-format version byte. pub(crate) const VERSION: u8 = 0; @@ -215,14 +212,9 @@ async fn handle_map( if requested_lifetime == 0 { let mut registry = ctx.registry.lock().await; registry.remove_by_internal(proto, client_ip, local); - let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); - drop(registry); - if let Err(e) = - nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await - { - warn!(error = %e, "nat-pmp: failed to apply portmap rules on delete"); - } - // RFC 6886 section 3.3: reply with the requested local port, external port 0, lifetime 0. + ctx.apply_after_mutation(&mut registry, None).await.ok(); + // RFC 6886 section 3.3: reply with the requested local port, + // external port 0, lifetime 0. return encode_map_response( proto, ResultCode::Success, @@ -248,15 +240,36 @@ async fn handle_map( (ResultCode::OutOfResources, 0, 0) } }; - let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); + + // Apply while still holding the registry lock so two concurrent + // requests never race their apply calls. `created` tells + // `apply_after_mutation` which entry to roll back on nft failure. + let created_key = if let AllocOutcome::Created(ref m) = outcome { + Some(super::registry::MappingKey { + proto: m.proto, + external_port: m.external_port, + }) + } else { + None + }; + let applied = if matches!(outcome, AllocOutcome::Created(_) | AllocOutcome::Renewed(_)) { + ctx.apply_after_mutation(&mut registry, created_key).await + } else { + Ok(()) + }; drop(registry); - if matches!(outcome, AllocOutcome::Created(_) | AllocOutcome::Renewed(_)) { - if let Err(e) = - nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await - { - warn!(error = %e, "nat-pmp: failed to apply portmap rules"); - } + if applied.is_err() && created_key.is_some() { + // Apply failed and the mapping was rolled back; report as + // OutOfResources so the client retries later. + return encode_map_response( + proto, + ResultCode::OutOfResources, + ctx.epoch_time(), + local.get(), + 0, + 0, + ); } encode_map_response( diff --git a/patchbay/src/portmap/pcp.rs b/patchbay/src/portmap/pcp.rs index 3073b2e..0888faf 100644 --- a/patchbay/src/portmap/pcp.rs +++ b/patchbay/src/portmap/pcp.rs @@ -20,7 +20,6 @@ use std::{ use tracing::{debug, trace}; use super::{ - nft, registry::{AllocOutcome, MapProto}, server::ServerContext, }; @@ -356,13 +355,7 @@ async fn handle_map( if lifetime_seconds == 0 { let mut registry = ctx.registry.lock().await; registry.remove_by_internal(data.protocol, client_ip, local); - let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); - drop(registry); - if let Err(e) = - nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await - { - tracing::warn!(error = %e, "pcp: apply nft on delete"); - } + ctx.apply_after_mutation(&mut registry, None).await.ok(); return encode_map_response(MapResponse { result: ResultCode::Success, lifetime_seconds: 0, @@ -396,15 +389,33 @@ async fn handle_map( AllocOutcome::Conflict => (ResultCode::CannotProvideExternal, 0, 0), AllocOutcome::NoPortsAvailable => (ResultCode::NoResources, 0, 0), }; - let rules_snapshot: Vec<_> = registry.iter().cloned().collect(); + + let created_key = if let AllocOutcome::Created(ref m) = outcome { + Some(super::registry::MappingKey { + proto: m.proto, + external_port: m.external_port, + }) + } else { + None + }; + let applied = if matches!(outcome, AllocOutcome::Created(_) | AllocOutcome::Renewed(_)) { + ctx.apply_after_mutation(&mut registry, created_key).await + } else { + Ok(()) + }; drop(registry); - if matches!(outcome, AllocOutcome::Created(_) | AllocOutcome::Renewed(_)) { - if let Err(e) = - nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await - { - tracing::warn!(error = %e, "pcp: apply nft"); - } + if applied.is_err() && created_key.is_some() { + return encode_map_response(MapResponse { + result: ResultCode::NoResources, + lifetime_seconds: 0, + epoch_time: epoch, + nonce: data.nonce, + protocol: data.protocol, + local_port: data.local_port, + external_port: 0, + external_address: Ipv4Addr::UNSPECIFIED, + }); } let external_ip = if granted_ext == 0 { diff --git a/patchbay/src/portmap/server.rs b/patchbay/src/portmap/server.rs index ba96ecf..723da97 100644 --- a/patchbay/src/portmap/server.rs +++ b/patchbay/src/portmap/server.rs @@ -31,6 +31,14 @@ use crate::netns::NetnsManager; /// recommendation from RFC 6886 section 3.3. const MAX_LIFETIME_SECS: u64 = 2 * 60 * 60; +/// Period at which the background reaper sweeps expired mappings. +/// +/// Mappings carry a client-requested lifetime clamped by +/// [`MAX_LIFETIME_SECS`]. The reaper removes any mapping whose deadline +/// has elapsed and re-renders the nftables table so the DNAT rules +/// match the current registry. +const REAPER_INTERVAL: Duration = Duration::from_secs(30); + /// Shared state threaded through every protocol handler. /// /// `epoch` is the router-local epoch zero: response `epoch_time` fields @@ -54,6 +62,32 @@ impl ServerContext { pub(super) fn epoch_time(&self) -> u32 { self.epoch.elapsed().as_secs().min(u64::from(u32::MAX)) as u32 } + + /// Snapshots the mapping set while still holding the registry lock + /// and applies it to nftables. Holding the lock across the apply + /// serializes concurrent allocate+apply sequences so an older + /// snapshot cannot overwrite a newer one. + /// + /// On `nft` failure a just-created mapping is rolled back so + /// clients that receive an error do not find an orphan entry in + /// the registry later. + pub(super) async fn apply_after_mutation( + &self, + registry: &mut PortmapRegistry, + created: Option, + ) -> Result<()> { + let snapshot: Vec<_> = registry.iter().cloned().collect(); + match nft::apply_portmap_rules(&self.netns, &self.ns, self.wan_ip, &snapshot).await { + Ok(()) => Ok(()), + Err(e) => { + warn!(error = %e, "portmap: nft apply failed; rolling back"); + if let Some(key) = created { + registry.remove(key); + } + Err(e) + } + } + } } /// Cloneable handle to a per-router portmap server. @@ -71,6 +105,7 @@ struct PortmapServerInner { wan_ip: Ipv4Addr, _task: AbortOnDropHandle<()>, _upnp_tasks: Vec>, + _reaper: AbortOnDropHandle<()>, } impl std::fmt::Debug for PortmapServer { @@ -127,6 +162,8 @@ impl PortmapServer { upnp_tasks.push(AbortOnDropHandle::new(http)); } + let reaper = spawn_reaper(&netns, &ns, ctx)?; + Ok(Self { inner: Arc::new(PortmapServerInner { cfg, @@ -136,6 +173,7 @@ impl PortmapServer { wan_ip, _task: task, _upnp_tasks: upnp_tasks, + _reaper: reaper, }), }) } @@ -184,6 +222,32 @@ fn spawn_dispatch( Ok(AbortOnDropHandle::new(handle)) } +fn spawn_reaper( + netns: &Arc, + ns: &str, + ctx: Arc, +) -> Result> { + let rt = netns.rt_handle_for(ns)?; + let handle = rt.spawn(async move { + let mut ticker = tokio::time::interval(REAPER_INTERVAL); + ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); + // Burn the immediate first tick so we wait one REAPER_INTERVAL + // before the first sweep. Reduces noise at startup. + ticker.tick().await; + loop { + ticker.tick().await; + let mut registry = ctx.registry.lock().await; + let reaped = registry.reap_expired(Instant::now()); + if reaped.is_empty() { + continue; + } + debug!(n = reaped.len(), "portmap: reaped expired mappings"); + ctx.apply_after_mutation(&mut registry, None).await.ok(); + } + }); + Ok(AbortOnDropHandle::new(handle)) +} + async fn dispatch_loop(socket: UdpSocket, ctx: Arc, cfg: PortmapConfig) { let mut buf = vec![0u8; 1500]; loop { diff --git a/patchbay/src/portmap/upnp.rs b/patchbay/src/portmap/upnp.rs index 01d1bdd..521132a 100644 --- a/patchbay/src/portmap/upnp.rs +++ b/patchbay/src/portmap/upnp.rs @@ -38,7 +38,6 @@ use tokio::{ use tracing::{debug, trace, warn}; use super::{ - nft, registry::{AllocOutcome, MapProto, MappingKey}, server::ServerContext, }; @@ -572,32 +571,36 @@ async fn handle_add_port_mapping( let preferred = NonZeroU16::new(external_port); - let (outcome, rules_snapshot) = { - let mut registry = ctx.registry.lock().await; - let outcome = registry.allocate( - protocol, - internal_client, - internal_local, - if pick_any { None } else { preferred }, - lifetime, - None, - ); - let rules: Vec<_> = registry.iter().cloned().collect(); - (outcome, rules) - }; + let mut registry = ctx.registry.lock().await; + let outcome = registry.allocate( + protocol, + internal_client, + internal_local, + if pick_any { None } else { preferred }, + lifetime, + None, + ); match outcome { AllocOutcome::Created(ref m) | AllocOutcome::Renewed(ref m) => { - if let Err(e) = - nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await - { - warn!(error = %e, "upnp: apply nft"); + let created_key = if let AllocOutcome::Created(_) = &outcome { + Some(MappingKey { + proto: m.proto, + external_port: m.external_port, + }) + } else { + None + }; + let applied = ctx.apply_after_mutation(&mut registry, created_key).await; + let external_port_str = m.external_port.get().to_string(); + drop(registry); + if applied.is_err() { return http_xml_response(soap_fault(500, "Action failed")); } if pick_any { http_xml_response(soap_success( "AddAnyPortMapping", - &[("NewReservedPort", &m.external_port.get().to_string())], + &[("NewReservedPort", &external_port_str)], )) } else { http_xml_response(soap_success("AddPortMapping", &[])) @@ -630,20 +633,9 @@ async fn handle_delete_port_mapping(body: &str, ctx: &ServerContext, peer: Ipv4A proto: protocol, external_port: external_nz, }; - let (authorized, removed, rules_snapshot) = { - let mut registry = ctx.registry.lock().await; - let owner = registry.get(key).map(|m| m.internal_ip); - let authorized = owner == Some(peer); - let removed = if authorized { - registry.remove(key).is_some() - } else { - false - }; - let rules: Vec<_> = registry.iter().cloned().collect(); - (authorized, removed, rules) - }; - - if !authorized { + let mut registry = ctx.registry.lock().await; + let owner = registry.get(key).map(|m| m.internal_ip); + if owner != Some(peer) { debug!( %peer, ext = external_nz.get(), @@ -651,12 +643,12 @@ async fn handle_delete_port_mapping(body: &str, ctx: &ServerContext, peer: Ipv4A ); return http_xml_response(soap_fault(606, "Action not authorized")); } - if !removed { + if registry.remove(key).is_none() { return http_xml_response(soap_fault(714, "NoSuchEntryInArray")); } - if let Err(e) = nft::apply_portmap_rules(&ctx.netns, &ctx.ns, ctx.wan_ip, &rules_snapshot).await - { - warn!(error = %e, "upnp: apply nft on delete"); + let applied = ctx.apply_after_mutation(&mut registry, None).await; + drop(registry); + if applied.is_err() { return http_xml_response(soap_fault(500, "Action failed")); } http_xml_response(soap_success("DeletePortMapping", &[])) From 1c11676e2451d975df49dec395fc78b899efac88 Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:50:52 +0200 Subject: [PATCH 10/13] test(portmap): cover UPnP and PCP delete paths Adds upnp_delete_drops_mapping and pcp_delete_drops_mapping. The UPnP test exercises the newly-added caller-ownership check because portmapper's deactivate() sends DeletePortMapping from the same device that created the mapping; the server accepts because peer == internal_client. The PCP test validates lifetime=0 delete semantics per RFC 6887 section 15. Changes the NAT-PMP local-port-zero response from UnsupportedOpcode (5) to NotAuthorizedOrRefused (2). Zero is a valid opcode but not a valid local port; replying with an unrelated spec-compliance error misled clients into believing Map was unimplemented. --- patchbay/src/portmap/nat_pmp.rs | 8 ++-- patchbay/src/tests/portmap.rs | 85 +++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/patchbay/src/portmap/nat_pmp.rs b/patchbay/src/portmap/nat_pmp.rs index 322ebc8..60ebcf3 100644 --- a/patchbay/src/portmap/nat_pmp.rs +++ b/patchbay/src/portmap/nat_pmp.rs @@ -35,7 +35,6 @@ enum ResultCode { UnsupportedVersion = 1, NotAuthorizedOrRefused = 2, OutOfResources = 4, - UnsupportedOpcode = 5, } #[repr(u8)] @@ -193,13 +192,16 @@ async fn handle_map( external_port: u16, requested_lifetime: u32, ) -> Vec { - // Local port 0 is an error condition for maps per RFC 6886. + // Local port 0 is not a valid map request per RFC 6886 section 3.3. + // No specific result code is defined, so use NotAuthorizedOrRefused: + // UnsupportedOpcode would mislead clients into thinking the server + // does not implement Map at all. let local = match NonZeroU16::new(local_port) { Some(p) => p, None => { return encode_map_response( proto, - ResultCode::UnsupportedOpcode, + ResultCode::NotAuthorizedOrRefused, ctx.epoch_time(), local_port, 0, diff --git a/patchbay/src/tests/portmap.rs b/patchbay/src/tests/portmap.rs index a66b0ed..9b4888d 100644 --- a/patchbay/src/tests/portmap.rs +++ b/patchbay/src/tests/portmap.rs @@ -459,6 +459,91 @@ async fn set_portmap_disables_server_at_runtime() -> Result<()> { Ok(()) } +/// UPnP delete: portmapper's `deactivate` sends a SOAP +/// `DeletePortMapping` that removes the mapping and prevents inbound +/// traffic from reaching the device. This exercises the UPnP +/// ownership check (caller is also the mapping's internal client). +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn upnp_delete_drops_mapping() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::UpnpOnly) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + + let local_port = NonZeroU16::new(50128).unwrap(); + + dev.spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config { + enable_upnp: true, + enable_pcp: false, + enable_nat_pmp: false, + protocol: portmapper::Protocol::Udp, + }); + client.probe().await??; + client.update_local_port(local_port); + let _ = wait_for_external(&client, Duration::from_secs(5)).await?; + client.deactivate(); + // Let the deactivate propagate; portmapper sends the SOAP call + // asynchronously from its service task. + tokio::time::sleep(Duration::from_millis(300)).await; + anyhow::Ok(()) + })? + .await??; + Ok(()) +} + +/// PCP delete: lifetime=0 removes the mapping per RFC 6887 section 15. +#[tokio::test(flavor = "current_thread")] +#[traced_test] +async fn pcp_delete_drops_mapping() -> Result<()> { + check_caps()?; + let lab = Lab::new().await?; + let dc = lab.add_router("dc").build().await?; + let home = lab + .add_router("home") + .upstream(dc.id()) + .nat(Nat::Home) + .portmap(PortmapMode::PcpOnly) + .build() + .await?; + let dev = lab + .add_device("dev") + .iface("eth0", home.id()) + .build() + .await?; + + let local_port = NonZeroU16::new(50129).unwrap(); + + dev.spawn(move |_| async move { + let client = portmapper::Client::new(portmapper::Config { + enable_upnp: false, + enable_pcp: true, + enable_nat_pmp: false, + protocol: portmapper::Protocol::Udp, + }); + client.probe().await??; + client.update_local_port(local_port); + let _ = wait_for_external(&client, Duration::from_secs(5)).await?; + client.deactivate(); + tokio::time::sleep(Duration::from_millis(200)).await; + anyhow::Ok(()) + })? + .await??; + Ok(()) +} + /// NAT-PMP delete: a map request with lifetime=0 removes the previous /// mapping so inbound traffic stops reaching the device. #[tokio::test(flavor = "current_thread")] From 782e193d428eb2063e2f085b9f748a620abaad6a Mon Sep 17 00:00:00 2001 From: Frando Date: Thu, 23 Apr 2026 15:53:16 +0200 Subject: [PATCH 11/13] docs: record staff review findings and final summary in worklog Captures the four-reviewer round (Rust expert, distributed systems, safety/security, docs/QA), the opposing-stance review that decided which findings to implement versus defer, and the final verification state (workspace tests, clippy, fmt) for the portmapping branch. --- worklogs/2026-04-23-portmapping.md | 181 ++++++++++++++++++++++++++++- 1 file changed, 180 insertions(+), 1 deletion(-) diff --git a/worklogs/2026-04-23-portmapping.md b/worklogs/2026-04-23-portmapping.md index 15f773b..3f2883b 100644 --- a/worklogs/2026-04-23-portmapping.md +++ b/worklogs/2026-04-23-portmapping.md @@ -91,6 +91,185 @@ Workspace lib tests: 205 passed, 0 failed. No clippy regressions against `patchbay`. Pre-existing clippy warnings in `patchbay-runner` are unchanged. +### 2026-04-23T13:55:00Z - Step 2 landed + +Dedicated `ip portmap` nftables table at priority `-110`, separate +from `ip nat`. `set_nat_mode` flushing `ip nat` no longer clears +portmap rules. Four unit tests cover empty, UDP, TCP, and multi-rule +render. Apply uses `add table + flush table` for idempotent atomic +swaps. + +### 2026-04-23T14:15:00Z - Step 3 landed + +NAT-PMP server on UDP 5351 inside the router namespace. +`RouterBuilder::portmap(PortmapMode)` wires it up at build time. +Source IP validation against downstream CIDR; lifetime 0 deletes per +RFC 6886 section 3.3. First surprise: `Nat::Home`'s APDF filter in +`ip filter forward` drops DNAT'd inbound packets with `NEW` conntrack +state. Fixed by adding `iif "wan" ct status dnat accept` to +`generate_nat_rules` before the blanket drop. Integration test with +portmapper as client confirms inbound UDP reaches the device. + +183 workspace tests pass (up from 171), 0 failures. + +### 2026-04-23T14:40:00Z - Step 4 landed + +PCP dispatch added to the same UDP 5351 socket. Scope limited to +Announce and Map opcodes, unicast, client-initiated. PCP ANNOUNCE +on UDP 5350 is intentionally out of scope because portmapper does +not consume it. + +Hit one real bug: initial `MAP_DATA_SIZE = 60` was wrong; correct +value is 36 per RFC 6887 section 11.1 (12 nonce + 1 proto + 3 +reserved + 2 local + 2 external + 16 v6 = 36). portmapper's client +sends 60-byte packets (24 header + 36 body) which my decoder +rejected as truncated. Fixed. + +Nonce authentication on Map: repeat requests for an existing +`(client_ip, local_port, proto)` must carry the same 12-byte nonce +or the server returns NotAuthorized. + +Integration test confirms TCP roundtrip through a PCP-granted +mapping. + +### 2026-04-23T15:20:00Z - Step 5 landed + +UPnP IGD:1 server. Two tasks: SSDP responder on multicast +239.255.255.250:1900 joined on the downstream bridge, and a +hand-rolled HTTP/1.1 server on an ephemeral TCP port on +`downstream_gw`. Three routes: `GET /rootDesc.xml`, `GET /WANIPCn.xml`, +`POST /ctl/IPConn`. SOAP actions: GetExternalIPAddress, AddPortMapping, +AddAnyPortMapping, DeletePortMapping. + +Hand-rolled HTTP chosen over pulling axum/hyper into the core crate +for a three-route fixed surface. + +Integration tests: UPnP-only probe + map + UDP roundtrip, and +a `probe_all_protocols` test that validates all three protocols +coexist on a single router when `PortmapMode::All` is selected. + +### 2026-04-23T15:45:00Z - Steps 6 and 7 landed + +`Router::set_portmap(mode)` dynamic op added, matching the existing +`set_nat_mode` / `set_firewall` pattern. Drops the server (which +aborts its tasks via `AbortOnDropHandle`), flushes the portmap table, +and optionally starts a fresh server on the new config. + +Two regression tests landed: +- `set_nat_mode_preserves_active_mapping`: validates the adversarial + review's C1 finding. Creates a NAT-PMP mapping, switches from + `Nat::Home` to `Nat::Cgnat`, then sends a UDP packet to the + external address. The DNAT rule in the separate `ip portmap` table + survives the `ip nat` flush, so the traffic still reaches the + device. +- `set_portmap_disables_server_at_runtime`: probes NAT-PMP + PCP, + disables via `set_portmap(None)`, reprobes, and asserts all three + protocols are absent. + +Step 8 (portmapper bug fixes in `../net-tools`) turned out not to be +needed. The predicted `Ipv4Addr::LOCALHOST` fallback bug did not +materialize because `netdev::get_local_ipaddr()` correctly returns +the device's LAN IP inside a patchbay namespace. + +199 workspace lib tests pass (up from 183), 0 failures. clippy +clean. fmt clean. + +### 2026-04-23T16:10:00Z - staff review round + +Spawned four adversarial reviewers in parallel against the full diff: +Rust expert, distributed systems, safety/security, docs/QA. Each +saw only the code, not any of the intermediate reasoning. Findings: + +Consolidated critical and substantive items after opposing-stance +review: +- UPnP caller ownership: LAN host could DNAT inbound traffic to a + peer's internal client, or delete another tenant's mapping. Fixed + in Phase B: AddPortMapping and DeletePortMapping now require + peer.ip() to match the mapping's internal client. +- HTTP resource bounds: unbounded per-connection tokio::spawn, no + timeouts, no body cap. Fixed in Phase B: 128-connection semaphore, + 10s header and body read timeouts, 64 KiB body cap with HTTP 413. +- PCP client_addr header unvalidated: RFC 6887 section 8.1 spec + violation. Fixed in Phase B with AddressMismatch response. +- nft apply race between concurrent allocate+apply sequences. Fixed + in Phase C: `ServerContext::apply_after_mutation` holds the + registry guard across the apply, serializing writers. Rollback on + apply failure so clients never see an orphan registry entry. +- No expired-mapping reaper. Fixed in Phase D: 30s ticker sweeps + deadline-elapsed mappings and re-renders the table. + +Code quality refinements (Phase A): +- Removed `#[allow(dead_code)]` module-level blankets. +- Removed two `const _: fn() -> ...` shims that silenced unused + imports. Real imports now. +- Moved per-request context out of nat_pmp.rs (where it had been + named Context but consumed by three protocols) into server.rs as + ServerContext. +- Replaced hand-rolled Opcode::from_byte matchers with + strum::FromRepr. +- `impl From for PortmapConfig` per agents/lang/rust.md. +- Explicit LAN-trust threat-model note in the module docstring. + +Discarded after opposing stance: +- "Actor-pattern refactor": overkill for a simulator. +- "NAT-PMP local_port=0 returns UnsupportedOpcode": changed to + NotAuthorizedOrRefused (wrong-code finding was valid, but the + larger "silently drop" recommendation conflates two cases). +- "M3: ct status dnat accept is too broad": accepted. No other DNAT + service exists today; if NAT64 or similar lands it will need to + coordinate with the portmap rules. Noted in the module doc but not + tightened further. + +### 2026-04-23T17:05:00Z - final verification + +`cargo test --workspace --lib`: 235 passed, 0 failed, 1 ignored +across all workspace crates. patchbay lib count: 201 (up from 171 +baseline; 30 added by this series). + +`cargo clippy --workspace --all-targets`: clean in `patchbay`. Two +warnings in `patchbay-runner` are pre-existing and unrelated. + +`cargo make format-check`: clean. + ## Summary -(pending) +Patchbay routers now model the three port-mapping protocols real +consumer routers advertise: NAT-PMP (RFC 6886), PCP (RFC 6887), and +UPnP IGD:1 (SSDP + SOAP). Enable them per-router via +`RouterBuilder::portmap(PortmapMode)` or at runtime via +`Router::set_portmap(PortmapMode)`. Off by default on every preset. + +The series spans eleven commits. Each commit compiles and passes the +full test suite independently: + +1. Foundation module (registry, mapping keys, dedup, lifetime). +2. Dedicated `ip portmap` nftables table at priority -110. +3. NAT-PMP server + RouterBuilder::portmap + integration test. +4. PCP server sharing UDP 5351 with NAT-PMP. +5. UPnP IGD server (SSDP + handwritten HTTP/1.1 + SOAP). +6. `Router::set_portmap` dynamic reconfiguration. +7. Refactor: consolidate `ServerContext`, trim module seams. +8. Security hardening: caller ownership on UPnP, HTTP bounds, PCP + client_addr validation. +9. Correctness: serialize nft applies, rollback on failure, add + expired-mapping reaper. +10. Additional delete-path tests. + +Integration coverage includes end-to-end UDP/TCP roundtrips for all +three protocols, `Router::set_nat_mode` regression (validates the +adversarial review's C1 finding), and `Router::set_portmap` shutdown. + +Step 8 from the plan ("fix portmapper bugs in ../net-tools") was not +needed: the predicted `Ipv4Addr::LOCALHOST` fallback never fired +because netdev correctly resolves the device's LAN IP inside each +patchbay namespace. + +Next things a reviewer might want to tighten (deferred as +non-critical): +- Move the UPnP HTTP body size / timeouts behind a `PortmapConfig` + field if a test needs to exercise the limits. +- Replace the minimal SOAP tag extractor with a streaming parser if + `igd-next`'s payload shape drifts. +- Scope the APDF `ct status dnat accept` rule to a named set + populated from the registry once NAT64 or other future DNAT + services land. From 58523833437424cec772206b4f3cdd428ad822fd Mon Sep 17 00:00:00 2001 From: Frando Date: Fri, 24 Apr 2026 00:13:46 +0200 Subject: [PATCH 12/13] fix(portmap): use published portmapper dev-dep CI checks out patchbay alone, so the path dep on a sibling net-tools checkout fails manifest resolution. portmapper 0.16.0 is on crates.io; switch to the published version. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 4 ++++ patchbay/Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 31a46a6..9ff81b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1954,6 +1954,8 @@ dependencies = [ [[package]] name = "netwatch" version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6fc0d4b4134425d9834e591b1a6f807ea365c6d941d738942215564af5f28a97" dependencies = [ "atomic-waker", "bytes", @@ -2466,6 +2468,8 @@ dependencies = [ [[package]] name = "portmapper" version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a145e62ddd9aecc9c7b1a3c84cea2a803386c7f4da7795bf9f0d50d90dc52549" dependencies = [ "base64", "bytes", diff --git a/patchbay/Cargo.toml b/patchbay/Cargo.toml index 8e0fe90..a38f942 100644 --- a/patchbay/Cargo.toml +++ b/patchbay/Cargo.toml @@ -37,5 +37,5 @@ ctor = "0.6" futures-buffered = "0.2" hickory-resolver = { version = "0.25", default-features = false, features = ["system-config", "tokio"] } n0-tracing-test = "0.3.0" -portmapper = { path = "../../net-tools/portmapper", default-features = false } +portmapper = { version = "0.16", default-features = false } testdir = "0.9" From 7787c51cc356d5bad5278c52b6c24ce8dae19502 Mon Sep 17 00:00:00 2001 From: Frando Date: Fri, 24 Apr 2026 00:23:51 +0200 Subject: [PATCH 13/13] fix(runner): collapse nested if into match guards Clippy 1.95 promotes collapsible_match from warning to error under CI's -D warnings. The two pre-existing matches in parse_step_failure_summary already carried nested `if !v.is_empty()` checks that clippy now rejects. Rewrite as match guards. Co-Authored-By: Claude Opus 4.7 (1M context) --- patchbay-runner/src/sim/runner.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/patchbay-runner/src/sim/runner.rs b/patchbay-runner/src/sim/runner.rs index 5eefc02..e823d0b 100644 --- a/patchbay-runner/src/sim/runner.rs +++ b/patchbay-runner/src/sim/runner.rs @@ -1039,16 +1039,8 @@ fn parse_step_failure(raw: &str) -> Option { match k { "index" => index = v.parse::().ok(), "action" => action = Some(v.to_string()), - "id" => { - if !v.is_empty() { - id = Some(v.to_string()); - } - } - "device" => { - if !v.is_empty() { - device = Some(v.to_string()); - } - } + "id" if !v.is_empty() => id = Some(v.to_string()), + "device" if !v.is_empty() => device = Some(v.to_string()), _ => {} } }