Add -p/--port-forward: iptables DNAT into extern-joined containers
Commit 5/6 of the network isolation feature (docs/networking-design.md).
port_forward.{h,cpp}: parse_port_forward_spec() parses
"[<network>:]<host-port>:<container-port>"; add_port_forward()
resolves the network (by name, or the container's sole extern network)
against join_networks()'s result and adds the DNAT/FORWARD rules;
remove_port_forward() undoes them. join_networks() (network_join.{h,cpp})
now returns the joined networks with their assigned IPs (was a bare
bool) so port-forward setup knows where to send traffic. -p requires
-r, is repeatable, network names may no longer contain ':' (needed to
keep the spec syntax unambiguous -- is_valid_network_name(),
network_subnet.h).
Two real corrections from testing, not assumed:
- The DNAT rule needs both nat PREROUTING and nat OUTPUT -- PREROUTING
never sees locally-generated packets (e.g. curl run on the same
host), only OUTPUT does. PREROUTING-only left the host's own real IP
connection-refused despite the container being directly reachable.
- curl localhost:<port> still doesn't work even with both chains --
a separate problem, NAT hairpinning: the container sees an inbound
packet claiming a loopback source arriving on a non-loopback
interface and drops it as martian. A net.ipv4.conf.*.route_localnet
sysctl was tried and confirmed not to fix this alone, then removed
rather than left in as dead code. Not solved here (would need scoped
source masquerading or a userland proxy); curl <host's real IP> is
the actually-relevant, verified-working path for real clients.
Also surfaced (unrelated to -p, found while testing it, not fixed
here): -x/--exec doesn't join the net namespace -- written when this
project never isolated networking at all -- so it currently sees the
host's own network stack instead of a network-isolated session's own.
Verified end-to-end as root (via a scoped doas rule): a container
serving HTTP on an extern network with -p 8080:80 was reachable via
curl <host's real IP>:8080; the rule was confirmed gone after the
session was killed.
This commit is contained in:
@@ -84,7 +84,13 @@ Source layout (all under `src/`):
|
||||
`--no-nsenter`**: reassigned here since `--network` will be far more
|
||||
heavily used; `--no-nsenter` moved to long-option-only (`options::no_nsenter`)
|
||||
rather than hunting for a new letter, matching `--kill`'s own "rare/niche
|
||||
flag, long-only is no real loss" precedent.
|
||||
flag, long-only is no real loss" precedent. `-p/--port-forward` (`'p'` was
|
||||
free) is repeatable the same accumulate-now, resolve-after-the-loop way as
|
||||
`-n` (`ParsedArgs::port_forward_specs`, raw `"[<network>:]<host-port>:
|
||||
<container-port>"` strings — actual parsing happens later, in
|
||||
`port_forward.h`, since resolving which network a spec refers to needs
|
||||
runtime join state that doesn't exist yet at parse time), but has no
|
||||
standalone use at all: rejected post-loop unless combined with `-r`.
|
||||
- `commands.{h,cpp}` — every command's implementation, plus the dispatcher.
|
||||
`dispatch_command(args, config_path, config)` (the only externally-linked
|
||||
function; everything else in this file is `.cpp`-local) is a `switch
|
||||
@@ -138,7 +144,11 @@ Source layout (all under `src/`):
|
||||
design and `config_file.{h,cpp}` below for `NetworkEntry`. Joining a network
|
||||
from `-r/--run` (repeatable `-n <name>`, `ParsedArgs::network_specs`, see
|
||||
`cli_args.{h,cpp}` above) is handled by `run_container()`, further below,
|
||||
via `network_join.{h,cpp}` (see below). `create_network_command()` rejects a duplicate name first, then
|
||||
via `network_join.{h,cpp}` (see below). `create_network_command()` rejects a
|
||||
name containing `':'` first (`is_valid_network_name()`, `network_subnet.h`
|
||||
— needed since `port_forward.h`'s `-p` syntax splits a spec on `':'`; a
|
||||
network name containing one would make that parse ambiguous), then a
|
||||
duplicate name, then
|
||||
resolves `subnet`/`subnet6`: an explicit `--subnet`/`--subnet6` is validated
|
||||
(`is_valid_ipv4_cidr()`/`is_valid_ipv6_cidr()`) and checked for overlap
|
||||
against every existing network's subnet (`ipv4_cidrs_overlap()`/
|
||||
@@ -216,7 +226,25 @@ Source layout (all under `src/`):
|
||||
`join_networks(pid, network_specs, app_config)` (`network_join.h`, see
|
||||
below) — networks are joined *before* the daemonize report is sent, so a
|
||||
`-D`-daemonized caller doesn't get control back until network setup has
|
||||
already had its chance to run.
|
||||
already had its chance to run. `join_networks()` itself early-returns (no
|
||||
namespace wait at all) when `network_specs` is empty, so calling it
|
||||
unconditionally whenever `on_bwrap_pid_known` fires for *any* reason (e.g.
|
||||
`-D/--daemonize` alone, no `-n`) doesn't cost anything. `-p`'s
|
||||
`port_forward_specs` (`cli_args.h`) are syntax/range-parsed
|
||||
(`parse_port_forward_spec()`, `port_forward.h`) up front too — `ok = false`
|
||||
on a bad spec, same as other validation failures — but *resolving* which
|
||||
network each targets can only happen after `join_networks()` returns (it
|
||||
needs to know which networks actually joined, and their assigned IPs), so
|
||||
that happens in the same `on_bwrap_pid_known` callback, right after the
|
||||
`join_networks()` call: `add_port_forward()` per spec, collecting the
|
||||
ones that actually landed into a `std::vector<ActivePortForward>` declared
|
||||
in `run_container()`'s own scope (captured by reference) — read again
|
||||
*after* `run_bwrap()` returns to `remove_port_forward()` each one. This
|
||||
two-places split (add during the callback, remove after `run_bwrap()`
|
||||
returns) mirrors how `join_networks()`'s own veths don't need an explicit
|
||||
removal step (the kernel tears them down once the session's namespace
|
||||
goes away) while port-forward rules — host-global, named, persistent
|
||||
iptables state — very much do.
|
||||
- `self_test.{h,cpp}` — `run_self_tests()` implements `-t/--test`, this
|
||||
project's own built-in self-test mode (distinct from the Meson-driven
|
||||
fixture smoke test under `tests/`, described in "Build & test commands"
|
||||
@@ -613,10 +641,18 @@ Source layout (all under `src/`):
|
||||
local subnet is already automatic once an address is assigned, no explicit
|
||||
route command needed for same-bridge reachability regardless of kind).
|
||||
Every step failure is logged specifically (which command, which network)
|
||||
and best-effort: `join_networks()` returns `true` only if every requested
|
||||
network joined, but a failure never kills the already-running session
|
||||
(network setup can only happen after `bwrap`'s own namespace exists, i.e.
|
||||
potentially after the sandboxed command is already running). Veth teardown
|
||||
and best-effort: `join_networks()` returns one `JoinedNetwork {network,
|
||||
container_ip}` per network that actually joined (in `-n` order, so shorter
|
||||
than the request list on any partial failure), never fatal to the
|
||||
already-running session (network setup can only happen after `bwrap`'s own
|
||||
namespace exists, i.e. potentially after the sandboxed command is already
|
||||
running) — this return value exists specifically for `port_forward.h`
|
||||
(see below) to resolve a `-p` spec against which networks/IPs are actually
|
||||
usable, not as a pass/fail signal on its own. An empty `network_names`
|
||||
returns immediately (no namespace wait at all), so callers that always
|
||||
invoke this once `on_bwrap_pid_known` fires for any reason (`commands.cpp`
|
||||
also fires it for `-D/--daemonize` alone, with no `-n`) don't pay for a
|
||||
wait that has nothing to do. Veth teardown
|
||||
needs no explicit code: the kernel destroys an entire veth pair (both
|
||||
ends, including the one still attached to the bridge) the instant *either*
|
||||
end's owning namespace is destroyed, so a session's veths disappear on
|
||||
@@ -630,6 +666,69 @@ Source layout (all under `src/`):
|
||||
internet through the bridge's NAT; a container joining both an `intern`
|
||||
and an `extern` network simultaneously got two working interfaces
|
||||
(`eth0`/`eth1`) with neither one breaking the other.
|
||||
|
||||
**Real, separate bug found while testing this commit, not yet fixed** (out
|
||||
of scope for the networking feature's own commit sequence, noted here so
|
||||
it isn't lost): `exec_session.cpp`'s `-x/--exec` deliberately never joins
|
||||
the `net` namespace type (see that file's own entry below — written when
|
||||
this project genuinely never isolated networking at all, so there was
|
||||
nothing to join). Now that `-r/--run` sometimes *does* isolate networking
|
||||
(whenever any `-n` was given), `-x/--exec`'ing into such a session sees the
|
||||
*host's* network stack instead of the container's — confirmed directly:
|
||||
execing into a session running a network-isolated `httpd` showed the
|
||||
host's own unrelated listening ports and failed to reach the container's
|
||||
own service on `127.0.0.1`. `exec_session.cpp` needs updating to join
|
||||
`net` too, the same way it already joins `mnt`/`uts`/`ipc`/`pid`/`cgroup`/
|
||||
`user` when they differ from the caller's own.
|
||||
- `port_forward.{h,cpp}` — implements `-p`. `parse_port_forward_spec()`
|
||||
splits `"[<network>:]<host-port>:<container-port>"` on `':'` (2 or 3
|
||||
fields; the network name is deliberately restricted to excluding `':'` --
|
||||
`is_valid_network_name()`, `network_subnet.h` -- specifically so this
|
||||
split stays unambiguous) and validates both ports are `1..65535`
|
||||
(`.cpp`-local `parse_port()`) -- pure syntax/range parsing, no knowledge of
|
||||
which networks exist or joined; that's `add_port_forward()`'s job, called
|
||||
later once `join_networks()` (`network_join.h`) has actually run.
|
||||
`add_port_forward()` resolves `spec.network` against the `JoinedNetwork`
|
||||
list -- by name if given (erroring if that network wasn't successfully
|
||||
joined, or isn't `extern`: an `intern` network's bridge has no path from
|
||||
the host at all, so forwarding into one could never work), or, if unset,
|
||||
the container's sole joined `extern` network (erroring if none or more
|
||||
than one, rather than guessing). Then adds one iptables `DNAT` rule to
|
||||
**both** `nat PREROUTING` *and* `nat OUTPUT` -- a real bug caught by
|
||||
testing, not assumed: `PREROUTING`-only left `curl <this host's own real
|
||||
IP>:<host-port>`, run *on this same host*, connection-refused, since
|
||||
`PREROUTING` only ever sees packets arriving from an actual network
|
||||
interface, never locally-generated ones (those go through `OUTPUT`
|
||||
instead) -- the same split Docker's own DNAT setup already accounts for.
|
||||
Also adds one `FORWARD ACCEPT` rule for the destination (in case of a
|
||||
default `FORWARD DROP` policy, which would otherwise silently eat the
|
||||
forwarded traffic even though the `DNAT` itself succeeded); if a later
|
||||
rule fails after an earlier one already landed, those are removed again so
|
||||
a failure doesn't leave a half-applied mapping. **Known limitation, not
|
||||
solved here, also found by testing**: `curl localhost:<host-port>` (or any
|
||||
`127.0.0.0/8` destination) specifically still doesn't work even with both
|
||||
`DNAT` chains covered -- confirmed to be a separate problem, NAT
|
||||
hairpinning: once `DNAT` rewrites the destination to the container's IP,
|
||||
the packet still carries its *original* source address (`127.0.0.1`); the
|
||||
container's own kernel sees an inbound packet claiming to be *from*
|
||||
loopback arriving on a non-loopback interface (`eth<N>`) and drops it as a
|
||||
martian source. (A `net.ipv4.conf.{all,lo}.route_localnet=1` sysctl was
|
||||
tried and confirmed *not* to fix this on its own, then removed again
|
||||
rather than left in as dead/superstitious code.) A full fix needs source
|
||||
masquerading scoped to exactly this case (matching only host-local
|
||||
traffic, not genuine external clients -- unconditionally masquerading
|
||||
would lose the real client IP for those, a regression) or a userland
|
||||
proxy, the approach Docker itself historically used for the same reason --
|
||||
out of scope here; `curl <this host's real, externally-reachable IP>:
|
||||
<host-port>` (verified working) is the actually-relevant path `-p` exists
|
||||
for. `remove_port_forward()` (`commands.cpp`'s `run_container()`, called
|
||||
for each `ActivePortForward` collected during `on_bwrap_pid_known`, after
|
||||
`run_bwrap()` returns) removes the exact same rules `add_port_forward()`
|
||||
added -- best-effort, logs a warning on failure, never fatal. **Verified
|
||||
end-to-end on this dev machine (root, via a scoped `doas` rule)**: a
|
||||
container serving HTTP on an `extern` network with `-p 8080:80` was
|
||||
reachable via `curl <host's real IP>:8080` from the host; the rule was
|
||||
confirmed gone (connection refused) after the session was killed.
|
||||
- `session_cgroup.{h,cpp}` — gives `--kill` (`kill_session.{h,cpp}`, see
|
||||
below) a reliable way to find every process a session ever started, however
|
||||
deeply forked/daemonized/reparented, by putting it in a dedicated cgroup v2
|
||||
@@ -972,7 +1071,11 @@ Source layout (all under `src/`):
|
||||
`unshare-*` keys.
|
||||
- `network_subnet.{h,cpp}` — pure CIDR arithmetic backing `-n/--network`'s
|
||||
subnet allocation and `network_bridge.{h,cpp}`'s (see below) gateway-address
|
||||
computation; no kernel/`ip`/`iptables` calls of its own. `is_valid_ipv4_cidr()`/
|
||||
computation; no kernel/`ip`/`iptables` calls of its own. `is_valid_network_name()`
|
||||
(non-empty, no `':'`) mirrors `volume_mount.h`'s `is_valid_volume_name()`
|
||||
(which rejects `'/'`) — `':'` specifically because `port_forward.h`'s `-p`
|
||||
syntax splits a spec on it; a network name containing one would make that
|
||||
parse ambiguous. `is_valid_ipv4_cidr()`/
|
||||
`is_valid_ipv6_cidr()` and `ipv4_cidrs_overlap()`/`ipv6_cidrs_overlap()` all
|
||||
build on one `.cpp`-local `parse_cidr()` (via `inet_pton()`, not hand-rolled
|
||||
parsing) producing a plain byte-vector address (4 bytes for IPv4, 16 for
|
||||
@@ -1100,8 +1203,8 @@ Build directory is `buildDir/` (already configured).
|
||||
`-l/--list-images`, `-i/--inspect`, `-x/--exec`, `--kill`, `--no-nsenter`, `-D/--daemonize`,
|
||||
`--user`, `--group`, `--hostname`, `--env`, `--env-file`, `-v/--volume`, `--list-volumes`, `--delete-volume`,
|
||||
`--delete-volume-full`, `-n/--network`, `--extern`, `--intern`, `--subnet`, `--no-ipv6`, `--subnet6`,
|
||||
`--list-networks`, `--delete-network`, `--list-processes`, `--clean-processes`, `-w/--write-config`,
|
||||
`-t/--test`, `--log-level`, `-h/--help`, `-V/--version`)
|
||||
`--list-networks`, `--delete-network`, `-p/--port-forward`, `--list-processes`, `--clean-processes`,
|
||||
`-w/--write-config`, `-t/--test`, `--log-level`, `-h/--help`, `-V/--version`)
|
||||
- Run tests: `meson test -C buildDir`
|
||||
|
||||
## Code style
|
||||
|
||||
Reference in New Issue
Block a user