Conversation
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.
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 <wan_ip> 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.
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.
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.
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.
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.
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<PortmapMode> for PortmapConfig`, which is the idiomatic conversion direction.
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.
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.
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.
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.
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ip portmapnftables table that survivesRouter::set_nat_mode.RouterBuilder::portmap(PortmapMode)or reconfigure at runtime withRouter::set_portmap. Off by default on every preset includingHome, so existing tests see no behavior change.portmapper(fromnet-tools) as the client and validate inbound UDP and TCP delivery after a mapping, NAT-mode regression, runtime shutdown, and delete paths.Design notes
ip portmaptable at priority-110soset_nat_mode's flush ofip natno longer drops active mappings.ServerContext::apply_after_mutationholds the registry guard across eachnftapply, serializing concurrent writers and rolling back just-created mappings when the apply fails.Security
AddPortMappingandDeletePortMappingrequire the caller's source IP to matchNewInternalClient. This stops a LAN host from hijacking inbound traffic destined for a peer or revoking another tenant's mapping, which is the same class of abuse real consumer firmware has been exploited for.client_addrfield against the packet's UDP source IP per RFC 6887 section 8.1, returningAddressMismatch.portmap/mod.rs: source-IP authorization is adequate inside the simulator but not outside.Test plan
cargo test --workspace --lib(201 passed in patchbay lib, 30 new)cargo clippy --workspace --all-targetsclean inpatchbay(pre-existing warnings inpatchbay-runnerare unrelated)cargo make format-checkcleanportmapperclient roundtrips for NAT-PMP UDP, PCP TCP, and UPnP UDPRouter::set_nat_modeduring an active mapping preserves the DNAT ruleRouter::set_portmap(None)tears the server down and unregisters all three protocols🤖 Generated with Claude Code