diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6427bb4e1..1ea6a4c15 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -185,7 +185,7 @@ The Rust workspace (`src/`) implements multiple sandboxing backends behind the ` | WSLc | `wxc-exec.exe` | Windows | `backends/wslc/common/src/` — feature-gated behind `wslc`, experimental, uses the WSLc SDK (`wslcsdk.dll`, loaded at runtime) to run Linux containers in a WSL2 VM. Supports both one-shot (`WSLContainerRunner`, via `ScriptRunner` + streaming `SandboxBackend`) and state-aware (`state_aware.rs` `WslcStateAwareRunner`, via `StatefulSandboxBackend`) modes. Because the WSLc SDK has **no cross-process re-attach**, state-aware keeps the session (VM) + container warm across separate `wxc-exec` phase processes behind a persistent per-user daemon (`wxc-wslc-daemon.exe`, `backends/wslc/daemon/`) that owns the live `WslcSession`/`WslcContainer` handles; phase processes are thin named-pipe clients (`daemon_client.rs`). The daemon runs all SDK calls on one apartment-affine worker thread (so exec is currently serialized across sandboxes — see `docs/wsl/wslc-state-aware.md`). Honors `readwritePaths`/`readonlyPaths` at provision (→ container volumes) + `network.defaultPolicy` (`Block`→`None`, `Allow`→`Bridged`; networking is all-or-nothing — no per-host filtering, since the container lacks `CAP_NET_ADMIN`); rejects `deniedPaths` nested under a mount and rejects proxy/host-filtering at provision. exec honors `network.proxy` **url-form only** (injected as `HTTP_PROXY`/`HTTPS_PROXY`); start/stop/deprovision reject all policy. ID prefix `wslc` (`wslc:<32-hex>`). Idle-timeout is env-overridable via `MXC_WSLC_DAEMON_IDLE_TIMEOUT_SECS`/`MXC_WSLC_DAEMON_IDLE_POLL_SECS`. See `docs/wsl/wslc-state-aware.md`. | | LXC | `lxc-exec` | Linux | `core/lxc/src/main.rs` + `backends/lxc/common/` | | Seatbelt | `mxc-exec-mac` | macOS | `core/mxc_darwin/src/main.rs` + `backends/seatbelt/common/` — uses macOS App Sandbox (Seatbelt) profiles for process containment. Requires schema `0.7.0-alpha`+. Supports `network.proxy` via the same cooperative env-var model as Bubblewrap (injects `HTTP_PROXY`/`HTTPS_PROXY` into the sandbox, reusing `wxc_common::unix_proxy_coordinator`; `builtinTestServer` spawns the shared `unix-test-proxy`). See `docs/macos-support/seatbelt-backend.md`. | -| Bubblewrap | `lxc-exec` | Linux | `backends/bubblewrap/common/src/bwrap_runner.rs` — unprivileged sandboxing via Linux user namespaces and `bwrap`. Experimental — requires `--experimental`. Uses shared filesystem/network policy fields; per-host network filtering via `NetworkIptablesManager` from `backends/lxc/common`. For schema 0.8+, proxy mode uses a private network namespace with rootless `slirp4netns` routing and fails if `slirp4netns` is unavailable; schema 0.6/0.7 and absent-version requests retain the legacy shared-network proxy behavior. See `docs/bwrap-support/bubblewrap-backend.md`. | +| Bubblewrap | `lxc-exec` | Linux | `backends/bubblewrap/common/src/bwrap_runner.rs` — unprivileged sandboxing via Linux user namespaces and `bwrap`. Experimental — requires `--experimental`. Uses shared filesystem/network policy fields; per-host network filtering via `NetworkIptablesManager` from `backends/lxc/common`. For schema 0.8+, proxy mode uses a private network namespace with rootless `slirp4netns` routing and a default-DROP egress chain that permits only loopback and the translated proxy endpoint (installed via `nsenter` + `iptables`/`ip6tables` from a supervisor that holds the namespaces); it requires `slirp4netns`, util-linux `unshare`/`nsenter` and `iptables`/`ip6tables` on PATH and fails validation if any is unavailable. `iptables`/`ip6tables` must also resolve to the `nf_tables` backend, unless `/run/xtables.lock` is writable by the calling user: the legacy backend opens that lock unconditionally, and the rules are installed by an unprivileged same-uid supervisor that cannot open a root-owned one — `validate` refuses such a host rather than letting the supervisor die at the first rule. The host proxy endpoint is rewritten to slirp's gateway `10.0.2.2`, so `127.0.0.1`/`0.0.0.0`/`::` are translated while `::1` is rejected (an IPv6-loopback listener cannot accept the gateway's IPv4 connection). Schema 0.6/0.7 and absent-version requests retain the legacy shared-network proxy behavior. See `docs/bwrap-support/bubblewrap-backend.md`. | ### Config flow diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index d1fc2684d..26551fa56 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -27,22 +27,39 @@ requiring root privileges or a container runtime. newer** is required. Platform detection probes `bwrap --version` and reports the backend as unavailable — with the detected version — when the host is below that floor. -- **Schema 0.8 proxy mode only:** `slirp4netns` installed and on PATH. It is - not required when `network.proxy` is omitted or when a 0.6/0.7 policy uses +- **Schema 0.8 proxy mode only:** `slirp4netns` installed and on PATH, plus + `nsenter`, `iptables`, and `ip6tables` for the proxy-only egress rules. None + are required when `network.proxy` is omitted or when a 0.6/0.7 policy uses the legacy proxy behavior. ```bash # Debian/Ubuntu - sudo apt install slirp4netns + sudo apt install slirp4netns util-linux iptables # Fedora/RHEL - sudo dnf install slirp4netns + sudo dnf install slirp4netns util-linux iptables # Alpine - apk add slirp4netns + apk add slirp4netns util-linux iptables ``` - Proxy mode fails explicitly if `slirp4netns` is unavailable; it never falls - back to sharing the host network namespace. The host must also provide the - util-linux `unshare` command with `--map-current-user` and `--keep-caps`. + `iptables` must resolve to the **`nf_tables` backend** (the default on + Debian 10+, Ubuntu 20.10+, RHEL 8+, and Alpine). The legacy backend opens + `/run/xtables.lock` before touching any table, and the rules are installed by + an unprivileged supervisor that keeps the caller's uid — so on a stock host, + where `/run` is root-owned, it cannot take that lock. `validate` refuses such + a host with a message naming the backend, rather than letting the supervisor + die at the first rule. If a host is pinned to legacy, switch it: + ```bash + sudo update-alternatives --set iptables /usr/sbin/iptables-nft + sudo update-alternatives --set ip6tables /usr/sbin/ip6tables-nft + ``` + (A legacy backend is still accepted where the lock *is* writable — for + example when running as root — since it works there.) + Proxy mode fails explicitly if any of these is unavailable; it never falls + back to sharing the host network namespace or to running without egress + rules. The host must also provide the util-linux `unshare` command with + `--map-current-user` and `--keep-caps`. No root is needed: `iptables` runs + against the sandbox's own network namespace, where the supervisor holds + `CAP_NET_ADMIN`. - User namespaces must be enabled: ```bash # Check: should print "1" @@ -276,6 +293,19 @@ request fails if its private namespace cannot be configured. ### How it works +0. Before anything is launched, `validate` probes the host tools this mode + depends on — `slirp4netns`, `unshare` (checked for `--map-current-user` and + `--keep-caps`), `nsenter`, `iptables`, and `ip6tables` — so a host that is + missing one fails immediately with a message naming it, rather than partway + through supervisor startup. For `iptables`/`ip6tables` presence is not + enough: the probe also reads the backend from the version banner and refuses + a legacy backend whose `/run/xtables.lock` this user cannot open, because + the unprivileged supervisor would otherwise die at the first rule. Each + probe is bounded by a short timeout: a + wedged binary is reported as hung and named, instead of stalling every + proxy-mode execution on the host indefinitely. A successful probe is cached + for the life of the process; failures are not, so installing the missing + tool takes effect without a restart. 1. When `network.proxy` is set, the runner launches an unprivileged HTTP proxy on loopback (`127.0.0.1:N`). For tests, the bundled `unix-test-proxy` binary is used (`builtinTestServer: true`, @@ -285,8 +315,19 @@ request fails if its private namespace cannot be configured. with `--unshare-net`, and keeps the workload behind a startup barrier. 3. The supervisor attaches `slirp4netns` to Bubblewrap's private network namespace. Host-loopback proxy endpoints are presented to the sandbox - through slirp's `10.0.2.2` host gateway. The workload starts only after - slirp reports that `tap0` is configured. + through slirp's `10.0.2.2` host gateway. Once slirp is up, the supervisor + programs a default-DROP `MXC_EGRESS` chain into that namespace via + `nsenter`, permitting only loopback and the proxy endpoint (IPv6 gets a + DROP-only chain). The workload is released only after every rule is + installed, so it can never run with egress open. A failure to program any + rule aborts the supervisor rather than starting an unenforced sandbox. + + Bubblewrap joins the supervisor's user namespace (`--userns`) rather than + creating its own, so the sandbox lives in the namespace that owns the + rule-bearing network namespace. This relies on Bubblewrap dropping + capabilities in the sandboxed process — the runner passes no `--cap-add` — + which is what prevents the workload from holding the `CAP_NET_ADMIN` needed + to flush the chain. 4. The command builder sets `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY`, `FTP_PROXY`, and their lowercase variants inside the sandbox via `bwrap --setenv` (caller-supplied values for these keys, including @@ -295,13 +336,14 @@ request fails if its private namespace cannot be configured. bypass the configured proxy policy. 5. Cooperative tools (curl, wget, Python `requests`, Node `https`, etc.) honor the env vars and traffic flows through the proxy, which applies - the `allowedHosts` / `blockedHosts` lists. + the `allowedHosts` / `blockedHosts` lists. Non-cooperating clients are not + merely unrouted — their traffic is dropped by the egress chain. ### Example: builtin test proxy with allowlist ```json { - "version": "0.6.0-alpha", + "version": "0.8.0-alpha", "platform": "linux", "containment": "bubblewrap", "process": { @@ -319,7 +361,7 @@ request fails if its private namespace cannot be configured. ```json { - "version": "0.6.0-alpha", + "version": "0.8.0-alpha", "containment": "bubblewrap", "process": { "commandLine": "curl -fsSL https://example.com" }, "network": { @@ -328,6 +370,11 @@ request fails if its private namespace cannot be configured. } ``` +> Both examples declare `0.8.0-alpha` deliberately: the private-namespace and +> egress-enforcement behavior described above is selected by the schema version, +> so the same config on `0.6`/`0.7` runs the legacy shared-host-network proxy +> path instead. + ### Caveats - **Host loopback moves to the gateway address (breaking change in 0.8+ @@ -340,21 +387,14 @@ request fails if its private namespace cannot be configured. namespace**, so `127.0.0.1` did reach the host; that is the behavior changing here. - Host-local services remain reachable, but at slirp's gateway address + The configured proxy remains reachable, but at slirp's gateway address `10.0.2.2` instead — which is exactly how the runner rewrites a - `localhost` proxy endpoint so the sandbox can still find it. - - **This reachability is not limited to the configured proxy.** slirp runs - without `--disable-host-loopback`, so the workload can open a connection - to *any* service bound to host loopback via `10.0.2.2:` — a local - database, a metadata endpoint, an unrelated daemon. That is a deliberate - exception to the private-network boundary and a more sensitive one than - generic outbound internet egress, because host-loopback services often - assume that only host-local callers can reach them. `--disable-host-loopback` - would close the path, but it would also break the proxy rewrite above, so - the gateway stays reachable until a single-port forwarding mechanism - replaces it. Restricting egress to the configured proxy endpoint is the - job of the proxy-only enforcement work that builds on this change. + `localhost` proxy endpoint so the sandbox can still find it. Other + host-local services do **not** come along: slirp itself runs without + `--disable-host-loopback`, so the gateway can in principle carry traffic to + any host-loopback port, but the egress chain admits only the single + `10.0.2.2:` destination and drops the rest. The host-loopback + surface is therefore the proxy endpoint alone. - **The supervisor's user namespace is visible to the sandbox**: in proxy mode `bwrap` joins the supervisor's user namespace via `--userns` rather than creating its own, and the namespace descriptor stays open in the @@ -365,21 +405,33 @@ request fails if its private namespace cannot be configured. `CapBnd`/`CapEff`/`CapPrm` all zero. The end-to-end test suite asserts those are zero, because that assumption is what makes the exposed descriptor inert. -- **Cooperative model**: the runner enforces by injecting - `HTTP_PROXY` / `HTTPS_PROXY` into the sandbox environment, so only - well-behaved clients that honor those vars are routed through the - proxy. Tools that bypass them (raw sockets, custom HTTP clients, - statically-linked binaries that ignore the env) can still use slirp's direct - egress and are **not yet enforced**. - This applies to **both** the builtin test proxy and external (BYO) - proxy modes — the limitation is in the env-var injection mechanism, - not in the proxy itself; a BYO proxy can do whatever it likes for - cooperating clients. For strict whole-network isolation, omit - `network.proxy` so the runner can apply `--unshare-net` instead. +- **Cooperative routing, enforced egress (schema 0.8+)**: the runner injects + `HTTP_PROXY` / `HTTPS_PROXY` so cooperating clients route through the proxy, + and additionally programs a default-DROP egress chain inside the sandbox's + private network namespace. Clients that ignore the env vars (raw sockets, + custom HTTP clients) can no longer reach the network directly: only loopback + and the proxy endpoint are permitted. DNS is deliberately **not** opened — + the proxy resolves on the workload's behalf — so the proxy endpoint must be + an IPv4 literal. IPv6 egress is denied outright. + + Consequently the only accepted host-local proxy endpoints are `localhost`, + `127.0.0.0/8` and the wildcards `0.0.0.0` / `::`, each rewritten to + `10.0.2.2`. `::1` is **rejected at validation time**: a proxy bound only to + the IPv6 loopback cannot accept the IPv4 connection slirp's gateway + produces, so translating it would hand the sandbox an address nothing + answers on. Bind such a proxy to `127.0.0.1` or to a dual-stack wildcard + instead. + On schema **0.6/0.7** the legacy behavior applies: the sandbox shares the + host network namespace, no egress rules are installed, and only the + cooperative env-var routing is in effect — a client that ignores + `HTTP_PROXY`/`HTTPS_PROXY` reaches the network directly. For strict + whole-network isolation on those versions, omit `network.proxy` so the + runner can apply `--unshare-net` instead. - **Mutually exclusive with iptables enforcement**: setting `network.proxy` together with `network.enforcementMode` of `"firewall"` - or `"both"` is rejected at config-parse time because iptables-based - enforcement requires root and would defeat the proxy's privilege story. + or `"both"` is rejected at config-parse time. (The rejection message cites a + root requirement that proxy mode has since disproved; see the firewall + section.) - **External proxy delegates policy**: when `network.proxy` uses `localhost: ` or `url: ` (not `builtinTestServer`), the external proxy is responsible for any host filtering. The runner does @@ -417,10 +469,10 @@ resolution. |--------|-----|------------| | Privileges | Root required | Unprivileged (user namespaces) | | Rootfs | Downloads distro rootfs | Bind-mounts host filesystem | -| Startup | Create → Start → Attach | Single `bwrap` exec (proxy mode adds a namespace supervisor) | +| Startup | Create → Start → Attach | Single `bwrap` exec; proxy mode adds a user/network-namespace supervisor, a `slirp4netns` instance and an egress rule set | | Network isolation | iptables + veth | `--unshare-net`, private netns + slirp4netns, or iptables | -| Dependencies | `lxc-*` tools, templates | `bwrap`; proxy mode also needs `slirp4netns` and util-linux `unshare` | -| Lifecycle | Create/destroy containers | Process dies on exit | +| Dependencies | `lxc-*` tools, templates | `bwrap`; proxy mode also needs `slirp4netns`, util-linux `unshare` and `nsenter`, plus `iptables` and `ip6tables` on the `nf_tables` backend | +| Lifecycle | Create/destroy containers | Process dies on exit; proxy mode's supervisor is reaped with it | **When to use Bubblewrap:** - Quick sandboxing without root access diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index f0cc5aeb5..a569d22b9 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -98,8 +98,8 @@ pub(crate) enum ResolvedNetworkMode { LegacyProxy, /// The host network namespace with per-destination iptables filtering. FirewallFiltered, - /// Cooperative proxy routing inside a slirp-backed private namespace. - /// Later work adds proxy-only egress enforcement. + /// Cooperative proxy routing inside a slirp-backed private namespace, with + /// egress closed to everything but the proxy. ProxyOnly, } @@ -152,9 +152,13 @@ impl ResolvedNetworkMode { matches!(self, Self::ProxyOnly) } - /// Whether current behavior installs iptables filtering rules. + /// Whether the mode uses the host-side firewall manager. + /// + /// False for `ProxyOnly`, which is *not* iptables-free: it programs rules + /// directly into the sandbox's own network namespace from the supervisor + /// (see `proxy_network`) rather than going through the host manager. #[cfg(any(target_os = "linux", test))] - pub(crate) fn requires_iptables(self) -> bool { + pub(crate) fn requires_host_firewall_manager(self) -> bool { matches!(self, Self::FirewallFiltered) } } @@ -566,8 +570,10 @@ mod tests { assert!(ResolvedNetworkMode::ProxyOnly.uses_external_userns()); assert!(!ResolvedNetworkMode::Isolated.uses_external_userns()); assert!(!ResolvedNetworkMode::LegacyProxy.uses_private_netns()); - assert!(ResolvedNetworkMode::FirewallFiltered.requires_iptables()); - assert!(!ResolvedNetworkMode::Shared.requires_iptables()); + assert!(ResolvedNetworkMode::FirewallFiltered.requires_host_firewall_manager()); + assert!(!ResolvedNetworkMode::Shared.requires_host_firewall_manager()); + // ProxyOnly enforces via in-netns rules, not the host manager. + assert!(!ResolvedNetworkMode::ProxyOnly.requires_host_firewall_manager()); } #[test] diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index 16e458a51..0e047328f 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -112,15 +112,37 @@ impl SandboxBackend for BubblewrapScriptRunner { } } + // Proxy-only networking rewrites the configured endpoint to slirp's + // host gateway and then opens exactly that address in the egress + // chain. Both steps can reject an endpoint (an IPv6 loopback the + // gateway cannot reach, a routable IPv6 literal the IPv4-only rules + // cannot express, a zero port). Running them here means the caller + // learns at policy time instead of after a namespace, a slirp instance + // and a rule set have already been built and torn down. + // + // The builtin test server has no address until it is started (the + // parser leaves a port-0 placeholder), so its endpoint stays on the + // runtime check in `run`; this only covers an operator-supplied proxy. + let proxy_only = + ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()) + == ResolvedNetworkMode::ProxyOnly; + if proxy_only && !request.policy.network_proxy.builtin_test_server { + if let Some(address) = request.policy.network_proxy.address.as_ref() { + let egress = proxy_network::sandbox_proxy_address(address) + .and_then(|translated| proxy_network::ProxyEgress::from_address(&translated)); + if let Err(error) = egress { + return Err(ScriptResponse::error(&error)); + } + } + } + // `bwrap` must be present *and* new enough for every flag the argument // builder emits — an old binary would otherwise fail at spawn time with // an opaque "unknown option" error. if let Err(err) = bwrap_version::probe_bwrap() { return Err(ScriptResponse::error(&err.to_string())); } - if ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()) - == ResolvedNetworkMode::ProxyOnly - { + if proxy_only { if let Err(error) = proxy_network::probe_dependencies() { return Err(ScriptResponse::error(&error)); } @@ -247,7 +269,23 @@ impl BubblewrapScriptRunner { let proxy_address = sandbox_proxy_address.as_ref().or_else(|| proxy.address()); let mut proxy_network = if network_mode == ResolvedNetworkMode::ProxyOnly { - match proxy_network::ProxyNetworkNamespace::start(logger) { + // The workload dials the sandbox-visible address, so that is what + // the egress rule must open. + let egress = match sandbox_proxy_address + .as_ref() + .ok_or_else(|| { + "Bubblewrap: proxy-only networking requires a resolved proxy address" + .to_string() + }) + .and_then(proxy_network::ProxyEgress::from_address) + { + Ok(egress) => egress, + Err(error) => { + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } + }; + match proxy_network::ProxyNetworkNamespace::start(&egress, logger) { Ok(network) => Some(network), Err(error) => { proxy.stop(logger); @@ -289,10 +327,10 @@ impl BubblewrapScriptRunner { args.len() ); - // 3. Determine whether iptables network rules are needed. When the - // cooperative proxy is active we skip iptables entirely (host - // enforcement happens at the proxy layer). - let needs_iptables = network_mode.requires_iptables(); + // 3. Determine whether the host-side firewall manager is needed. Proxy + // mode does not use it: it programs its own rules inside the + // sandbox's network namespace instead (see proxy_network). + let needs_iptables = network_mode.requires_host_firewall_manager(); let container_name = if request.container_id.is_empty() { format!("bwrap-{:08x}", std::process::id()) } else { @@ -838,6 +876,103 @@ mod tests { assert!(runner.validate(&req).is_ok()); } + /// Proxy-only mode rewrites the endpoint to slirp's gateway and opens + /// exactly that address, so an endpoint neither step can express has to be + /// refused at policy time -- not after a namespace and rule set were built. + #[test] + fn validate_rejects_an_ipv6_loopback_proxy_endpoint_before_the_environment_probe() { + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("[::1]".into(), 3128)), + builtin_test_server: false, + }; + + let runner = BubblewrapScriptRunner::new(); + let err = runner.validate(&req).unwrap_err(); + + assert!( + err.error_message.contains("IPv6 loopback"), + "an endpoint the gateway cannot reach must be refused by validate: {}", + err.error_message + ); + } + + #[test] + fn validate_rejects_a_routable_ipv6_proxy_endpoint_before_the_environment_probe() { + // The egress rules are IPv4-only, so this endpoint could never be + // opened -- `run` would discover that only after starting slirp. + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("2001:db8::1".into(), 3128)), + builtin_test_server: false, + }; + + let runner = BubblewrapScriptRunner::new(); + let err = runner.validate(&req).unwrap_err(); + + assert!( + !err.error_message.contains("bwrap"), + "the endpoint check must run ahead of the environment probe: {}", + err.error_message + ); + } + + /// The check is scoped to the private-namespace mode. A legacy-schema + /// request shares the host's network, never translates the endpoint and + /// installs no rules, so the same address must stay acceptable there -- + /// tightening it would break callers already on 0.6/0.7. + #[test] + fn validate_leaves_a_legacy_schema_proxy_endpoint_untouched() { + let mut req = base_request(); + req.schema_version = "0.7.0-alpha".into(); + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("[::1]".into(), 3128)), + builtin_test_server: false, + }; + + let runner = BubblewrapScriptRunner::new(); + let message = runner + .validate(&req) + .err() + .map(|err| err.error_message) + .unwrap_or_default(); + + assert!( + !message.contains("IPv6 loopback"), + "legacy proxy mode does not translate the endpoint, so it must not \ + inherit the private-namespace rejection: {message}" + ); + } + + /// The parser leaves a port-0 placeholder address on a `builtinTestServer` + /// request; the real endpoint is only known once the server is started. The + /// endpoint check must therefore skip it, or every builtin-proxy sandbox is + /// rejected at validation with "requires a non-zero proxy port". + #[test] + fn validate_does_not_apply_the_endpoint_check_to_the_builtin_test_server() { + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("127.0.0.1".into(), 0)), + builtin_test_server: true, + }; + + let runner = BubblewrapScriptRunner::new(); + let message = runner + .validate(&req) + .err() + .map(|err| err.error_message) + .unwrap_or_default(); + + assert!( + !message.contains("non-zero proxy port"), + "the builtin proxy's placeholder port must not be validated as an \ + operator-supplied endpoint: {message}" + ); + } + #[test] fn validate_rejects_empty_script_before_environment_probe() { // Empty script_code is a user-input error and must be surfaced diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index 157e300e1..0cf37cee0 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -3,35 +3,89 @@ //! Rootless private networking for Bubblewrap proxy mode. -use std::fs::{self, File, OpenOptions}; -use std::io::{ErrorKind, Read, Write}; +use std::fs::{self, File}; +use std::io::{ErrorKind, Read, Seek, SeekFrom, Write}; use std::os::fd::{AsRawFd, OwnedFd, RawFd}; use std::os::unix::process::CommandExt; -use std::process::{Child, Command, Stdio}; +use std::path::Path; +use std::process::{Child, Command, ExitStatus, Stdio}; use std::sync::OnceLock; use std::thread; use std::time::{Duration, Instant}; use nix::fcntl::{fcntl, FcntlArg, FdFlag, OFlag}; -use nix::unistd::pipe2; +use nix::unistd::{access, dup2, pipe2, AccessFlags}; use tempfile::TempDir; use wxc_common::logger::Logger; use wxc_common::models::ProxyAddress; const STARTUP_TIMEOUT: Duration = Duration::from_secs(5); +/// How long a single `iptables` call may block on the host's `/run/xtables.lock`. +/// +/// Only the legacy backend takes that lock, and `nsenter --net` leaves the mount +/// namespace alone, so there it is shared with every other sandbox. Waiting is +/// what makes concurrent provisioning work; the ceiling keeps a wedged lock from +/// hanging startup. `nf_tables` takes no lock and ignores the wait. +const XTABLES_LOCK_WAIT: Duration = Duration::from_secs(5); +/// Lock file the legacy `iptables` backend opens before touching any table. +/// `nf_tables` does not use it. See [`iptables_backend_is_usable`]. +const XTABLES_LOCK_PATH: &str = "/run/xtables.lock"; +/// Number of `iptables`/`ip6tables` calls the supervisor makes. Only used to +/// size the rule-installation budget, so it is asserted against the script. +const RULE_COMMAND_COUNT: u32 = 9; +/// Work the rule phase does beyond waiting on the lock: slirp coming up, plus +/// one `nsenter` launch per rule. Headroom only — the supervisor raises a single +/// signal for both phases, so they cannot be budgeted separately. +const RULE_INSTALL_OVERHEAD: Duration = + Duration::from_secs(STARTUP_TIMEOUT.as_secs() + RULE_COMMAND_COUNT as u64); +/// Budget for bringing slirp up and installing every egress rule. +/// +/// Covers the fully contended lock case plus [`RULE_INSTALL_OVERHEAD`], so a host +/// that legitimately consumes most of its `-w` allowance is not cut off just +/// short of finishing. +const RULE_INSTALL_TIMEOUT: Duration = Duration::from_secs( + XTABLES_LOCK_WAIT.as_secs() * RULE_COMMAND_COUNT as u64 + RULE_INSTALL_OVERHEAD.as_secs(), +); const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(2); +/// Ceiling for a single dependency probe. Generous next to a `--version` call, +/// which returns in milliseconds, so only a genuinely wedged binary trips it. +const PROBE_TIMEOUT: Duration = Duration::from_secs(5); const SLIRP_HOST_GATEWAY: &str = "10.0.2.2"; +/// Egress chain installed inside the sandbox's own network namespace. +const EGRESS_CHAIN: &str = "MXC_EGRESS"; +/// Descriptor numbers the supervisor script hardcodes. They must stay single +/// digit and below [`FD_STAGING_BASE`]: dash cannot name a descriptor >= 10 in +/// a redirection, which is the whole reason the parent pins them. +const SUPERVISOR_PID_FD: RawFd = 3; +const SUPERVISOR_EXIT_FD: RawFd = 4; +/// Descriptors are staged above every target before being landed, so a source +/// already sitting on a target cannot be clobbered mid-remap. +const FD_STAGING_BASE: RawFd = 10; +/// Brings up the private network, then closes it down to the proxy before the +/// workload runs. +/// +/// Ordering is the security property: the readiness signal is written only +/// after every rule is installed, and `set -e` turns a failed rule into +/// supervisor death rather than an unenforced sandbox. Rules are applied here +/// because that needs `CAP_NET_ADMIN` in the owning user namespace, which the +/// supervisor holds (`--keep-caps`) and the caller does not. const SUPERVISOR_SCRIPT: &str = r#" set -eu state_dir="$1" -ready_fd="$2" -exit_fd="$3" -pid_fd="$4" +proxy_ip="$2" +proxy_port="$3" +chain="$4" +lock_wait="$5" +# Inherited descriptors are remapped to fixed numbers by the parent (see +# `remap_descriptors`): 3 is the parent's PID pipe, 4 is slirp's exit pipe. +# They are hardcoded rather than passed in argv because /bin/sh is dash on +# Debian/Ubuntu, which rejects a *variable* descriptor >= 10 ("Bad fd number") +# at parse time -- a failure that depends on how many files the parent happens +# to hold open, so it surfaces nondeterministically and never in unit tests. printf ready > "$state_dir/userns.ready" # Block on the parent-owned PID pipe rather than polling for a file: if the # parent dies before it can publish the PID, the read ends at EOF and this # supervisor exits instead of spinning forever as an orphan. -eval "exec 3<&$pid_fd" if ! IFS= read -r child_pid <&3; then child_pid="${child_pid:-}" fi @@ -40,11 +94,88 @@ if [ -z "$child_pid" ]; then echo "mxc: parent exited before publishing the sandbox PID" >&2 exit 1 fi -exec slirp4netns --configure --mtu=65520 \ - --ready-fd "$ready_fd" --exit-fd "$exit_fd" \ - "$child_pid" tap0 + +# slirp signals readiness internally so the supervisor, not slirp, decides when +# the sandbox is ready -- the rules below must be in place first. Descriptor 9 +# is safe to hardcode: the parent hands this child exactly 0-4, so nothing else +# can be sitting on it. +exec 9> "$state_dir/slirp.internal" +slirp4netns --configure --mtu=65520 \ + --ready-fd 9 --exit-fd 4 \ + "$child_pid" tap0 & +slirp_pid=$! +trap 'kill "$slirp_pid" 2>/dev/null || true' TERM INT +while [ ! -s "$state_dir/slirp.internal" ]; do + if ! kill -0 "$slirp_pid" 2>/dev/null; then + echo "slirp4netns exited before signalling readiness" >&2 + exit 1 + fi + sleep 0.01 +done + +ns="/proc/$child_pid/ns/net" +# Deny-all-except-proxy. Loopback is exempt: it is the sandbox's own isolated +# loopback, so it never leaves the sandbox. Port 53 is deliberately not opened +# -- the proxy resolves on the workload's behalf, so an accept would only be a +# DNS-tunnel exfil path out of a proxy-only posture. +# +# -w matters only on the legacy backend, the one that takes a lock: nsenter +# enters the network namespace but not the mount namespace, so concurrent +# sandboxes contend for the *host's* /run/xtables.lock, and set -e turns a lost +# race into a dead supervisor. nf_tables takes no lock and ignores the wait. +# A backend that cannot take the lock at all is refused by probe_dependencies. +nsenter --net="$ns" -- iptables -w "$lock_wait" -N "$chain" +nsenter --net="$ns" -- iptables -w "$lock_wait" -A "$chain" -o lo -j ACCEPT +nsenter --net="$ns" -- iptables -w "$lock_wait" -A "$chain" -p tcp -d "$proxy_ip" --dport "$proxy_port" -j ACCEPT +nsenter --net="$ns" -- iptables -w "$lock_wait" -A "$chain" -j DROP +nsenter --net="$ns" -- iptables -w "$lock_wait" -A OUTPUT -j "$chain" +# The proxy rule is IPv4 only, so v6 carries its closing DROP alone: IPv6 +# egress fails closed rather than being left open. +nsenter --net="$ns" -- ip6tables -w "$lock_wait" -N "$chain" +nsenter --net="$ns" -- ip6tables -w "$lock_wait" -A "$chain" -o lo -j ACCEPT +nsenter --net="$ns" -- ip6tables -w "$lock_wait" -A "$chain" -j DROP +nsenter --net="$ns" -- ip6tables -w "$lock_wait" -A OUTPUT -j "$chain" + +# Signalled by path, not through a descriptor: this is a plain file the parent +# polls, so it needs no shell redirection and cannot hit dash's fd limit. +printf ready > "$state_dir/slirp.ready" +wait "$slirp_pid" "#; +/// The single destination a proxy-only sandbox may reach. +/// +/// Must be an IPv4 literal: rules are IPv4 `iptables`, and DNS is closed inside +/// the sandbox, so a hostname could not be resolved even if a rule existed for +/// its address. LXC solves that with a hosts-file pin; Bubblewrap has no +/// equivalent yet and fails closed instead. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ProxyEgress { + ip: std::net::Ipv4Addr, + port: u16, +} + +impl ProxyEgress { + /// Derive the permitted endpoint from the sandbox-visible proxy address. + pub(crate) fn from_address(address: &ProxyAddress) -> Result { + let host = address.host().trim_matches(['[', ']']); + let ip = host.parse::().map_err(|_| { + format!( + "Bubblewrap: proxy-only egress requires an IPv4 proxy endpoint, but the \ + sandbox-visible proxy host is '{host}'. Proxy-only egress is enforced with \ + IPv4 iptables rules and DNS is closed inside the sandbox, so a hostname or \ + IPv6 endpoint cannot be reached. Use a loopback or IPv4 proxy address." + ) + })?; + if address.port() == 0 { + return Err("Bubblewrap: proxy-only egress requires a non-zero proxy port".to_string()); + } + Ok(Self { + ip, + port: address.port(), + }) + } +} + /// Runtime file descriptors Bubblewrap needs while establishing its child. pub(crate) struct BwrapStartup { info_reader: File, @@ -142,9 +273,12 @@ pub(crate) struct ProxyNetworkNamespace { impl ProxyNetworkNamespace { /// Create the capability-retaining namespace supervisor. /// + /// `egress` is the only destination the sandbox can reach once the + /// supervisor signals readiness; everything else is dropped. + /// /// Callers reach this only after `BwrapRunner::validate` has already run /// [`probe_dependencies`], so the probe is not repeated here. - pub(crate) fn start(logger: &mut Logger) -> Result { + pub(crate) fn start(egress: &ProxyEgress, logger: &mut Logger) -> Result { let state_dir = tempfile::Builder::new() .prefix("mxc-bwrap-proxy-") .tempdir() @@ -155,14 +289,6 @@ impl ProxyNetworkNamespace { let stderr = File::create(&stderr_path).map_err(|error| { format!("Bubblewrap: failed to create proxy-network diagnostics: {error}") })?; - let ready = OpenOptions::new() - .create(true) - .append(true) - .open(state_dir.path().join("slirp.ready")) - .map_err(|error| { - format!("Bubblewrap: failed to create slirp readiness file: {error}") - })?; - let (exit_reader, exit_writer) = pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; let (pid_reader, pid_writer) = @@ -181,20 +307,21 @@ impl ProxyNetworkNamespace { "mxc-bwrap-proxy-supervisor", ]) .arg(state_dir.path()) - .arg(ready.as_raw_fd().to_string()) - .arg(exit_reader.as_raw_fd().to_string()) - .arg(pid_reader.as_raw_fd().to_string()) + .arg(egress.ip.to_string()) + .arg(egress.port.to_string()) + .arg(EGRESS_CHAIN) + .arg(XTABLES_LOCK_WAIT.as_secs().to_string()) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::from(stderr)); - // These stay CLOEXEC in this process, so a concurrent spawn from - // another thread cannot inherit them; only the supervisor gets them. - inherit_descriptors( + // Both pipes stay CLOEXEC in this process, so a concurrent spawn from + // another thread cannot inherit them; only the supervisor gets them, + // and only at the fixed numbers its script names. + remap_descriptors( &mut command, - vec![ - ready.as_raw_fd(), - exit_reader.as_raw_fd(), - pid_reader.as_raw_fd(), + [ + (pid_reader.as_raw_fd(), SUPERVISOR_PID_FD), + (exit_reader.as_raw_fd(), SUPERVISOR_EXIT_FD), ], ); @@ -203,13 +330,13 @@ impl ProxyNetworkNamespace { })?; drop(exit_reader); drop(pid_reader); - drop(ready); if let Err(error) = wait_for_file( state_dir.path().join("userns.ready"), &mut supervisor, &stderr_path, - "user namespace", + "user namespace startup", + STARTUP_TIMEOUT, ) { terminate_child(&mut supervisor); return Err(error); @@ -297,9 +424,16 @@ impl ProxyNetworkNamespace { self.state_dir.path().join("slirp.ready"), &mut self.supervisor, &self.state_dir.path().join("supervisor.stderr"), - "slirp4netns", + // Names both phases: this signal is written after slirp is up *and* + // every egress rule is installed, so attributing a stall to + // slirp alone would send the reader to the wrong place. + "slirp4netns startup and egress rule installation", + RULE_INSTALL_TIMEOUT, )?; - logger.log_line("Bubblewrap: slirp4netns configured the private proxy namespace"); + logger.log_line( + "Bubblewrap: slirp4netns configured the private proxy namespace and proxy-only \ + egress rules are in force", + ); Ok(()) } @@ -312,14 +446,30 @@ impl ProxyNetworkNamespace { let deadline = Instant::now() + SHUTDOWN_TIMEOUT; loop { match self.supervisor.try_wait() { - Ok(Some(_)) => return, + Ok(Some(status)) => { + // A clean teardown closes the exit pipe and slirp leaves + // with success. Anything else means it died on its own -- + // and since slirp carries the sandbox's only route, that + // is otherwise indistinguishable from the workload's own + // network calls failing. Say so, with whatever it wrote. + if !status.success() { + logger.log_line(&format!( + "WARNING: Bubblewrap: proxy network supervisor exited with {status} \ + ({})", + stderr_detail(&self.state_dir.path().join("supervisor.stderr")) + )); + } + return; + } Ok(None) if Instant::now() < deadline => { thread::sleep(Duration::from_millis(10)); } Ok(None) => { - logger.log_line( - "WARNING: Bubblewrap: slirp4netns did not stop promptly; terminating it", - ); + logger.log_line(&format!( + "WARNING: Bubblewrap: slirp4netns did not stop promptly; terminating it \ + ({})", + stderr_detail(&self.state_dir.path().join("supervisor.stderr")) + )); terminate_child(&mut self.supervisor); return; } @@ -345,13 +495,28 @@ impl Drop for ProxyNetworkNamespace { /// Return the proxy endpoint visible through slirp's host gateway. pub(crate) fn sandbox_proxy_address(address: &ProxyAddress) -> Result { let host = address.host().trim_matches(['[', ']']); + let parsed = host.parse::().ok(); + + // slirp's gateway reaches the host's *IPv4* loopback only. A proxy bound + // to `::1` listens on the IPv6 loopback exclusively, so rewriting it to + // the gateway would hand the sandbox an address nothing answers on -- + // failing at connect time rather than at policy time. Reject it instead. + if matches!(parsed, Some(std::net::IpAddr::V6(ip)) if ip.is_loopback()) { + return Err(format!( + "Bubblewrap: proxy address '{}' uses the IPv6 loopback, which the private \ + network namespace cannot reach; bind the proxy to 127.0.0.1 or a dual-stack \ + wildcard address instead", + address.host() + )); + } + // `0.0.0.0` / `::` name the host itself just as `127.0.0.1` does: a proxy // bound to the wildcard is reachable on the host's loopback, which the // sandbox's private namespace cannot see. Both need the gateway rewrite. + // `::` is safe to rewrite to IPv4 because a dual-stack wildcard listener + // accepts IPv4 connections, which `::1` does not. let is_host_local = host.eq_ignore_ascii_case("localhost") - || host - .parse::() - .is_ok_and(|ip| ip.is_loopback() || ip.is_unspecified()); + || parsed.is_some_and(|ip| ip.is_loopback() || ip.is_unspecified()); if !is_host_local { return Ok(address.clone()); } @@ -376,8 +541,64 @@ pub(crate) fn sandbox_proxy_address(address: &ProxyAddress) -> Result Result { + let mut child = command + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .map_err(|error| format!("{error}"))?; + + let deadline = Instant::now() + PROBE_TIMEOUT; + let status = loop { + match child.try_wait() { + Ok(Some(status)) => break status, + Ok(None) if Instant::now() < deadline => { + thread::sleep(Duration::from_millis(10)); + } + Ok(None) => { + // Leaving it running would keep the pipe open and strand the + // process for the lifetime of the host process. + terminate_child(&mut child); + return Err(format!( + "Bubblewrap: '{label}' did not respond within {PROBE_TIMEOUT:?}; \ + the host's {label} installation appears to be hung" + )); + } + Err(error) => { + terminate_child(&mut child); + return Err(format!("failed to inspect '{label}': {error}")); + } + } + }; + + let mut stdout = String::new(); + if let Some(mut pipe) = child.stdout.take() { + let _ = pipe.read_to_string(&mut stdout); + } + Ok(ProbeOutput { status, stdout }) +} + pub(crate) fn probe_dependencies() -> Result<(), String> { - // Probing costs two subprocess spawns, and the host's tooling does not + // Probing costs five subprocess spawns, and the host's tooling does not // change under a running process often enough to pay that on every // sandbox. Cache the *success* only: a failure is usually "the operator // has not installed slirp4netns yet", and caching that would keep failing @@ -392,15 +613,14 @@ pub(crate) fn probe_dependencies() -> Result<(), String> { } fn probe_dependencies_uncached() -> Result<(), String> { - let slirp = Command::new("slirp4netns") - .arg("--version") - .output() - .map_err(|error| { - format!( - "Bubblewrap: network.proxy requires 'slirp4netns' on PATH: {error}. \ - Install slirp4netns or omit network.proxy." - ) - })?; + let mut slirp_command = Command::new("slirp4netns"); + slirp_command.arg("--version"); + let slirp = run_probe(slirp_command, "slirp4netns").map_err(|error| { + format!( + "Bubblewrap: network.proxy requires 'slirp4netns' on PATH: {error}. \ + Install slirp4netns or omit network.proxy." + ) + })?; if !slirp.status.success() { return Err(format!( "Bubblewrap: network.proxy requires a working slirp4netns installation \ @@ -409,16 +629,14 @@ fn probe_dependencies_uncached() -> Result<(), String> { )); } - let unshare = Command::new("unshare") - .arg("--help") - .output() - .map_err(|error| { - format!("Bubblewrap: proxy networking requires util-linux 'unshare' on PATH: {error}") - })?; - let help = String::from_utf8_lossy(&unshare.stdout); + let mut unshare_command = Command::new("unshare"); + unshare_command.arg("--help"); + let unshare = run_probe(unshare_command, "unshare").map_err(|error| { + format!("Bubblewrap: proxy networking requires util-linux 'unshare' on PATH: {error}") + })?; if !unshare.status.success() - || !help.contains("--map-current-user") - || !help.contains("--keep-caps") + || !unshare.stdout.contains("--map-current-user") + || !unshare.stdout.contains("--keep-caps") { return Err( "Bubblewrap: proxy networking requires util-linux unshare with \ @@ -426,17 +644,135 @@ fn probe_dependencies_uncached() -> Result<(), String> { .into(), ); } + + // Proxy-only egress is programmed with these, so a host missing them must + // fail here rather than deep inside supervisor startup. + for (binary, probe, has_backend) in [ + ("nsenter", "--version", false), + ("iptables", "--version", true), + ("ip6tables", "--version", true), + ] { + let mut command = Command::new(binary); + command.arg(probe); + let output = run_probe(command, binary).map_err(|error| { + format!( + "Bubblewrap: network.proxy requires '{binary}' on PATH to enforce proxy-only \ + egress: {error}" + ) + })?; + if !output.status.success() { + return Err(format!( + "Bubblewrap: network.proxy requires a working '{binary}' installation \ + ({binary} {probe} exited with {})", + output.status + )); + } + // Presence is not enough: the binary can work while its backend is one + // this supervisor cannot drive. + if has_backend { + iptables_backend_is_usable(binary, &output.stdout, Path::new(XTABLES_LOCK_PATH))?; + } + } Ok(()) } +/// Whether `binary`'s backend can install rules from the unprivileged supervisor. +/// +/// The supervisor runs under `unshare --user --map-current-user`, so it keeps the +/// caller's uid: its capabilities apply inside the new user namespace, not to +/// root-owned files in the initial one. The legacy backend opens +/// [`XTABLES_LOCK_PATH`] before touching any table, so on a stock host (root-owned +/// `/run`, no lock file) it fails with `EACCES` and `set -e` kills the supervisor +/// at the first rule. `nf_tables` takes no lock. +/// +/// The banner alone cannot decide this: legacy *does* work where the lock is +/// reachable (as root, or with a writable lock). So the backend picks the +/// question, and for legacy the lock itself is tested. +fn iptables_backend_is_usable(binary: &str, banner: &str, lock: &Path) -> Result<(), String> { + if banner.contains("nf_tables") { + return Ok(()); + } + if lock_is_writable(lock) { + return Ok(()); + } + + // A pre-1.8 banner carries no marker at all; those builds are legacy-only. + Err(format!( + "Bubblewrap: network.proxy requires an iptables backend the sandbox supervisor can \ + drive without privilege, but '{binary}' resolves to the legacy backend ({}) and \ + '{}' is not writable by this user. Proxy-only egress rules are installed by an \ + unprivileged supervisor in a user namespace, which keeps the caller's uid, so the \ + root-owned lock is unreachable and every rule would fail. Select the nf_tables \ + backend (for example: update-alternatives --set {binary} /usr/sbin/{binary}-nft), \ + or make '{}' writable.", + banner.trim(), + lock.display(), + lock.display() + )) +} + +/// Whether the legacy backend could take its lock as the current user. +/// +/// The lock is created on first use, so an absent file makes the *directory* the +/// thing that must be writable. `access` tests the real uid — the uid the +/// supervisor itself runs under. +fn lock_is_writable(lock: &Path) -> bool { + let target = if lock.exists() { + lock + } else { + match lock.parent() { + Some(parent) => parent, + None => return false, + } + }; + access(target, AccessFlags::W_OK).is_ok() +} + +/// Cap on how much component stderr is quoted into a diagnostic. +/// +/// A wedged component can write without bound, and this is read from `Drop`, so +/// the read itself must be bounded rather than trimmed after the fact. +const MAX_STDERR_TAIL: u64 = 2048; + +/// Last [`MAX_STDERR_TAIL`] bytes of `path`, phrased for an error message. +/// +/// The tail, not the head: whatever actually killed the component is written +/// last, and a truncated head would quote its startup banner instead. +fn stderr_detail(path: &std::path::Path) -> String { + let text = read_stderr_tail(path); + let text = text.trim(); + if text.is_empty() { + return "no stderr output".to_string(); + } + format!("stderr: {text}") +} + +fn read_stderr_tail(path: &std::path::Path) -> String { + let Ok(mut file) = File::open(path) else { + return String::new(); + }; + let len = file.metadata().map(|metadata| metadata.len()).unwrap_or(0); + if len > MAX_STDERR_TAIL && file.seek(SeekFrom::End(-(MAX_STDERR_TAIL as i64))).is_err() { + return String::new(); + } + let mut buffer = Vec::new(); + if file.take(MAX_STDERR_TAIL).read_to_end(&mut buffer).is_err() { + return String::new(); + } + // Seeking to a byte offset can land mid-character; lossy keeps the rest + // readable instead of discarding the whole tail. + String::from_utf8_lossy(&buffer).into_owned() +} + fn wait_for_file( path: impl AsRef, child: &mut Child, stderr_path: &std::path::Path, component: &str, + timeout: Duration, ) -> Result<(), String> { let path = path.as_ref(); - let deadline = Instant::now() + STARTUP_TIMEOUT; + let deadline = Instant::now() + timeout; loop { if fs::metadata(path).is_ok_and(|metadata| metadata.len() > 0) { return Ok(()); @@ -444,26 +780,18 @@ fn wait_for_file( if let Some(status) = child.try_wait().map_err(|error| { format!("Bubblewrap: failed to inspect {component} startup: {error}") })? { - let stderr = fs::read_to_string(stderr_path).unwrap_or_default(); return Err(format!( "Bubblewrap: {component} exited during startup ({status}): {}", - stderr.trim() + stderr_detail(stderr_path) )); } if Instant::now() >= deadline { // Include whatever the component wrote to stderr: on a timeout it // is usually the only evidence of *why* startup stalled, and the // process is still alive so no exit status will explain it. - let stderr = fs::read_to_string(stderr_path).unwrap_or_default(); - let stderr = stderr.trim(); - let detail = if stderr.is_empty() { - "no stderr output".to_string() - } else { - format!("stderr: {stderr}") - }; return Err(format!( - "Bubblewrap: timed out waiting for {component} startup after {STARTUP_TIMEOUT:?} \ - ({detail})" + "Bubblewrap: timed out waiting for {component} after {timeout:?} ({})", + stderr_detail(stderr_path) )); } thread::sleep(Duration::from_millis(10)); @@ -476,6 +804,11 @@ fn wait_for_file( /// the descriptors to every concurrent `Command::spawn` -- a real window, since /// this crate is reachable from the SDK and FFI. Clearing it in the forked child /// instead gives them to the intended child and no one else. +/// +/// The number is left as the OS assigned it, which is safe here because bwrap +/// receives it in argv (`--info-fd N`) and parses it as an integer. A child +/// that must *redirect* to the descriptor from a shell needs +/// [`remap_descriptors`] instead. fn inherit_descriptors(command: &mut Command, fds: Vec) { // SAFETY: `pre_exec` runs between fork and exec, where only // async-signal-safe work is permitted. `fcntl` is async-signal-safe and @@ -491,6 +824,43 @@ fn inherit_descriptors(command: &mut Command, fds: Vec) { } } +/// Hand `mapping` to one specific child at fixed, known descriptor numbers. +/// +/// Two problems are solved together. `FD_CLOEXEC` is per-process, so clearing +/// it on the parent's copy would leak the descriptors to every concurrent +/// `Command::spawn` -- a real window, since this crate is reachable from the +/// SDK and FFI. And an OS-assigned descriptor number cannot be named from +/// `/bin/sh`: dash rejects `>&$fd` and `<&$fd` for any descriptor >= 10 with +/// `Bad fd number` at *parse* time, so interpolating a raw number breaks the +/// supervisor as soon as the parent holds enough open files. +/// +/// `dup2` in the forked child solves both: it clears `FD_CLOEXEC` on the new +/// descriptor only, and it puts that descriptor on a number the script can +/// hardcode. The originals stay CLOEXEC and close at exec. +fn remap_descriptors(command: &mut Command, mapping: [(RawFd, RawFd); N]) { + // SAFETY: `pre_exec` runs between fork and exec, where only + // async-signal-safe work is permitted. `fcntl` and `dup2` are both + // async-signal-safe and this closure allocates nothing -- `mapping` is a + // fixed-size array of plain integers captured by move, and the staging + // buffer is a fixed-size array too. + unsafe { + command.pre_exec(move || { + // Stage every source above the target range before landing any of + // them: a direct `dup2` could otherwise clobber a source that + // happens to already sit on a later target. + let mut staged = [-1; N]; + for (index, (source, _)) in mapping.iter().enumerate() { + staged[index] = fcntl(*source, FcntlArg::F_DUPFD_CLOEXEC(FD_STAGING_BASE)) + .map_err(std::io::Error::from)?; + } + for (index, (_, target)) in mapping.iter().enumerate() { + dup2(staged[index], *target).map_err(std::io::Error::from)?; + } + Ok(()) + }); + } +} + fn set_nonblocking(fd: RawFd) -> Result<(), String> { let flags = fcntl(fd, FcntlArg::F_GETFL) .map_err(|error| format!("Bubblewrap: failed to read descriptor flags: {error}"))?; @@ -509,6 +879,198 @@ fn terminate_child(child: &mut Child) { mod tests { use super::*; + #[test] + fn egress_opens_the_translated_loopback_proxy() { + // Rules must open the translated address, not the original loopback. + let address = ProxyAddress::new("127.0.0.1".into(), 8080); + let translated = sandbox_proxy_address(&address).unwrap(); + let egress = ProxyEgress::from_address(&translated).unwrap(); + + assert_eq!(egress.ip.to_string(), SLIRP_HOST_GATEWAY); + assert_eq!(egress.port, 8080); + } + + #[test] + fn egress_accepts_a_routable_ipv4_proxy() { + let address = ProxyAddress::new("10.1.2.3".into(), 3128); + let egress = ProxyEgress::from_address(&address).unwrap(); + + assert_eq!(egress.ip.to_string(), "10.1.2.3"); + assert_eq!(egress.port, 3128); + } + + #[test] + fn egress_rejects_a_hostname_proxy() { + // DNS is closed, so a name the workload cannot resolve must fail loudly + // rather than yield a rule that is never reachable. + let address = ProxyAddress::from_url( + "http://proxy.corp.example:3128", + "proxy.corp.example".into(), + 3128, + ); + let error = ProxyEgress::from_address(&address).unwrap_err(); + + assert!( + error.contains("proxy.corp.example"), + "error should name the offending host: {error}" + ); + assert!( + error.contains("IPv4"), + "error should explain the IPv4 requirement: {error}" + ); + } + + #[test] + fn egress_rejects_an_ipv6_proxy() { + // Rules are IPv4-only, so an IPv6 endpoint would never be opened. + let routable = ProxyAddress::new("2001:db8::1".into(), 8080); + assert!(ProxyEgress::from_address(&routable).is_err()); + } + + /// `::1` used to be rewritten to the IPv4 gateway, which handed the sandbox + /// an address the proxy never listens on: an IPv6-loopback listener does + /// not accept the IPv4 connection slirp's gateway produces. Rejecting at + /// policy time beats failing at connect time. + #[test] + fn rejects_an_ipv6_loopback_proxy_instead_of_translating_it() { + for host in ["[::1]", "::1"] { + let address = ProxyAddress::new(host.into(), 8080); + let error = sandbox_proxy_address(&address).unwrap_err(); + + assert!( + error.contains("IPv6 loopback"), + "error should name the unreachable address family: {error}" + ); + } + } + + /// The rejection must not swallow `::`: a dual-stack wildcard listener does + /// accept the gateway's IPv4 connection, so it still translates. + #[test] + fn ipv6_loopback_rejection_leaves_the_ipv6_wildcard_translatable() { + let address = ProxyAddress::new("[::]".into(), 8080); + let translated = sandbox_proxy_address(&address).unwrap(); + + assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); + } + + #[test] + fn egress_rejects_a_zero_port() { + let address = ProxyAddress::new("10.0.2.2".into(), 0); + let error = ProxyEgress::from_address(&address).unwrap_err(); + + assert!( + error.contains("non-zero"), + "error should explain the port requirement: {error}" + ); + } + + /// The supervisor script with the lock-wait flag elided. + /// + /// The flag is an operational detail, not part of the egress policy these + /// tests pin. Normalising it away keeps a change in lock handling from + /// failing every ordering assertion -- while + /// `every_rule_command_waits_for_the_xtables_lock` still asserts it is + /// present on all of them. + fn normalised_script() -> String { + SUPERVISOR_SCRIPT.replace(r#" -w "$lock_wait""#, "") + } + + /// Byte offset of `needle` in the normalised supervisor script. + fn script_offset(needle: &str) -> usize { + normalised_script() + .find(needle) + .unwrap_or_else(|| panic!("supervisor script should contain {needle:?}")) + } + + #[test] + fn script_accepts_the_proxy_before_dropping() { + // iptables is first-match: a DROP appended ahead of the proxy ACCEPT + // would black-hole the proxy itself. + let accept = script_offset(r#"-p tcp -d "$proxy_ip" --dport "$proxy_port" -j ACCEPT"#); + let loopback = script_offset(r#"iptables -A "$chain" -o lo -j ACCEPT"#); + let drop = script_offset(r#"iptables -A "$chain" -j DROP"#); + + assert!(loopback < accept, "loopback accept must come first"); + assert!(accept < drop, "proxy accept must precede the closing drop"); + } + + #[test] + fn script_signals_readiness_only_after_rules_are_installed() { + // The caller releases the workload on this signal, so emitting it early + // would let the workload run with egress wide open. + let last_rule = script_offset(r#"ip6tables -A OUTPUT -j "$chain""#); + let ready = script_offset(r#"printf ready > "$state_dir/slirp.ready""#); + + assert!( + last_rule < ready, + "readiness must be signalled after the final rule" + ); + } + + #[test] + fn script_does_not_open_dns() { + // Deliberate: the proxy resolves on the workload's behalf, so an + // unscoped port 53 accept would only be an exfil path. Pinning the full + // accept set catches any widening, not just DNS. + let accepts: Vec = normalised_script() + .lines() + .filter(|line| line.contains("-j ACCEPT")) + .map(str::to_owned) + .collect(); + + assert_eq!( + accepts, + vec![ + r#"nsenter --net="$ns" -- iptables -A "$chain" -o lo -j ACCEPT"#, + r#"nsenter --net="$ns" -- iptables -A "$chain" -p tcp -d "$proxy_ip" --dport "$proxy_port" -j ACCEPT"#, + r#"nsenter --net="$ns" -- ip6tables -A "$chain" -o lo -j ACCEPT"#, + ], + "only loopback and the proxy endpoint may be accepted" + ); + } + + #[test] + fn script_fails_ipv6_closed() { + let v6_drop = script_offset(r#"ip6tables -A "$chain" -j DROP"#); + let v6_hook = script_offset(r#"ip6tables -A OUTPUT -j "$chain""#); + + assert!(v6_drop < v6_hook, "v6 chain must drop before being hooked"); + assert!( + !normalised_script().contains(r#"ip6tables -A "$chain" -p tcp"#), + "v6 must not carry a proxy accept" + ); + } + + #[test] + fn script_hooks_both_chains_into_output() { + // An unhooked chain is never consulted and enforces nothing. + script_offset(r#"iptables -A OUTPUT -j "$chain""#); + script_offset(r#"ip6tables -A OUTPUT -j "$chain""#); + } + + #[test] + fn script_installs_rules_synchronously() { + // Offset-based ordering assertions only hold if the rules run inline. + // Backgrounding any of them would satisfy those tests while destroying + // the guarantee that rules precede readiness. + let rules_start = script_offset(r#"nsenter --net="$ns" -- iptables -N "$chain""#); + let ready = script_offset(r#"printf ready > "$state_dir/slirp.ready""#); + let script = normalised_script(); + let region = &script[rules_start..ready]; + + for line in region.lines().filter(|line| line.contains("nsenter")) { + assert!( + !line.trim_end().ends_with('&'), + "rule must not be backgrounded: {line}" + ); + assert!( + !line.contains('('), + "rule must not run in a subshell: {line}" + ); + } + } + #[test] fn translates_loopback_proxy_to_slirp_gateway() { let address = ProxyAddress::new("127.0.0.1".into(), 8080); @@ -597,4 +1159,608 @@ mod tests { "the parent's own descriptor was left inheritable" ); } + + /// Regression for the defect this scheme exists to prevent: dash rejects a + /// redirection to a *variable* descriptor >= 10 at parse time, so the old + /// `eval "... >&$fd"` form failed based on how many files the parent + /// happened to hold open. Remapping must land the pipe on a fixed low + /// number regardless of what the OS originally assigned. + #[test] + fn remapped_descriptors_land_on_fixed_numbers_from_any_source() { + // Force a high source number -- the exact case the old scheme broke on. + let (reader, writer) = pipe2(OFlag::O_CLOEXEC).expect("pipe"); + let high = fcntl(reader.as_raw_fd(), FcntlArg::F_DUPFD_CLOEXEC(20)).expect("dup high"); + assert!(high >= 20, "test needs a two-digit source descriptor"); + + File::from(writer).write_all(b"4242\n").expect("write pid"); + + let mut command = Command::new("sh"); + command + .arg("-c") + // Reads through the *literal* descriptor the script hardcodes. + .arg(format!( + "IFS= read -r v <&{SUPERVISOR_PID_FD}; printf %s \"$v\"" + )); + remap_descriptors(&mut command, [(high, SUPERVISOR_PID_FD)]); + + let out = command.output().expect("spawn remapped child"); + assert!( + out.status.success(), + "remapped child failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + "4242", + "the pipe did not arrive on the fixed descriptor" + ); + } + + /// A source already sitting on another mapping's target must survive the + /// remap. A naive sequential `dup2` clobbers it and this catches that. + #[test] + fn remapping_survives_a_source_already_on_a_target_number() { + let (a_reader, a_writer) = pipe2(OFlag::O_CLOEXEC).expect("pipe"); + let (b_reader, b_writer) = pipe2(OFlag::O_CLOEXEC).expect("pipe"); + let a = a_reader.as_raw_fd(); + let b = b_reader.as_raw_fd(); + + File::from(a_writer).write_all(b"AAA\n").expect("write a"); + File::from(b_writer).write_all(b"BBB\n").expect("write b"); + + // The second mapping's source is the first mapping's target, so a + // sequential `dup2` would overwrite `b` before it had been landed. + let mapping = [(a, b), (b, SUPERVISOR_PID_FD)]; + + let mut command = Command::new("sh"); + command.arg("-c").arg(format!( + "printf 'first=%s second=%s' \ + \"$(cat /proc/self/fd/{b})\" \"$(cat /proc/self/fd/{SUPERVISOR_PID_FD})\"" + )); + remap_descriptors(&mut command, mapping); + + let out = command.output().expect("spawn remapped child"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + "first=AAA second=BBB", + "a source sitting on another target was clobbered during remap" + ); + } + + /// The supervisor runs under `/bin/sh`, which is dash on Debian/Ubuntu. + /// Parse it with the real shell so a construct dash rejects cannot ship. + #[test] + fn supervisor_script_parses_under_the_shell_that_runs_it() { + let out = Command::new("sh") + .arg("-n") + .arg("-c") + .arg(SUPERVISOR_SCRIPT) + .output() + .expect("spawn sh"); + + assert!( + out.status.success(), + "supervisor script is not valid /bin/sh: {}", + String::from_utf8_lossy(&out.stderr) + ); + } + + /// The script may only name descriptors the parent actually pins. Two + /// forms are banned: `>&$var`, which broke on dash for fd >= 10, and + /// `{var}>`, the varfd allocation the review proposed as a fix — dash + /// *parses* that as a plain word, so it fails silently rather than loudly + /// and `sh -n` cannot catch it. + #[test] + fn supervisor_script_never_redirects_to_a_variable_descriptor() { + for line in SUPERVISOR_SCRIPT.lines() { + let code = line.trim(); + if code.starts_with('#') { + continue; + } + assert!( + !code.contains(">&$") && !code.contains("<&$"), + "variable descriptor redirection breaks dash for fd >= 10: {code}" + ); + assert!( + !code.contains("}>") && !code.contains("}<"), + "varfd allocation is a bash-ism; dash parses it as a word: {code}" + ); + } + } + + /// Every rule call must wait for the shared host lock. One unguarded call + /// is enough to fail a concurrent launch under `set -e`. + #[test] + fn every_rule_command_waits_for_the_xtables_lock() { + let rules: Vec<&str> = SUPERVISOR_SCRIPT + .lines() + .filter(|line| { + let code = line.trim(); + !code.starts_with('#') && code.contains("tables ") + }) + .collect(); + + assert_eq!( + rules.len() as u32, + RULE_COMMAND_COUNT, + "RULE_COMMAND_COUNT is stale, so the rule-install budget is wrong" + ); + for rule in rules { + assert!( + rule.contains(r#"-w "$lock_wait""#), + "rule may fail instantly on a contended host lock: {rule}" + ); + } + } + + /// Equality with the lock sum is not enough: the same deadline also spans + /// slirp startup and one process launch per rule. + #[test] + fn rule_install_budget_covers_every_command_blocking_on_the_lock() { + let contended = XTABLES_LOCK_WAIT * RULE_COMMAND_COUNT; + assert!( + RULE_INSTALL_TIMEOUT > contended, + "a fully contended host would time out before -w could succeed" + ); + assert!( + RULE_INSTALL_TIMEOUT - contended >= STARTUP_TIMEOUT, + "the budget leaves no room for slirp startup on top of lock contention" + ); + assert!( + RULE_INSTALL_TIMEOUT - contended - STARTUP_TIMEOUT + >= Duration::from_secs(RULE_COMMAND_COUNT as u64), + "the budget leaves no room to spawn one nsenter per rule" + ); + } + + #[test] + fn probe_gives_up_on_a_hung_binary_instead_of_blocking_forever() { + let mut command = Command::new("sleep"); + command.arg("120"); + + let started = Instant::now(); + let error = run_probe(command, "wedged-tool").expect_err("a hung probe must not succeed"); + let elapsed = started.elapsed(); + + assert!( + error.contains("wedged-tool"), + "the error must name the tool that hung, got: {error}" + ); + // The point of the change: bounded, not merely eventual. `sleep 120` + // would blow this budget by two orders of magnitude if unbounded. + assert!( + elapsed < PROBE_TIMEOUT * 3, + "probe took {elapsed:?}, expected to give up near {PROBE_TIMEOUT:?}" + ); + } + + #[test] + fn probe_returns_stdout_of_a_well_behaved_binary() { + let mut command = Command::new("sh"); + command.args(["-c", "echo probe-stdout-marker"]); + + let probe = run_probe(command, "echo").expect("a fast probe must succeed"); + + assert!( + probe.status.success(), + "expected success, got {:?}", + probe.status + ); + assert!( + probe.stdout.contains("probe-stdout-marker"), + "stdout was not captured, got: {:?}", + probe.stdout + ); + } + + #[test] + fn probe_reports_a_failing_binary_without_treating_it_as_a_hang() { + let mut command = Command::new("sh"); + command.args(["-c", "exit 3"]); + + let probe = run_probe(command, "failing").expect("a clean non-zero exit is not an error"); + + assert!(!probe.status.success(), "expected a non-zero exit status"); + } + + /// A path whose parent does not exist, standing in for the root-owned `/run` + /// an unprivileged supervisor meets. Keeps the test deterministic and + /// root-free while hitting the same branch. + fn unreachable_lock(dir: &TempDir) -> std::path::PathBuf { + dir.path().join("no-such-dir").join("xtables.lock") + } + + /// Verified live: with the lock absent and `/run` unwritable, `iptables-nft + /// -N` still succeeds. + #[test] + fn the_nft_backend_is_accepted_even_when_the_lock_is_unreachable() { + let dir = tempfile::tempdir().expect("tempdir"); + + assert!( + iptables_backend_is_usable( + "iptables", + "iptables v1.8.10 (nf_tables)\n", + &unreachable_lock(&dir) + ) + .is_ok(), + "the nf_tables backend takes no lock, so an unreachable lock must not refuse it" + ); + } + + /// The regression this check exists for: legacy opens the lock + /// unconditionally, so the supervisor dies at the first rule. + #[test] + fn the_legacy_backend_is_refused_when_it_could_not_take_the_lock() { + let dir = tempfile::tempdir().expect("tempdir"); + + let error = iptables_backend_is_usable( + "iptables", + "iptables v1.8.10 (legacy)\n", + &unreachable_lock(&dir), + ) + .expect_err("a legacy backend with an unreachable lock cannot install rules"); + + assert!( + error.contains("iptables") && error.contains("nf_tables"), + "the error must name the binary and the backend to switch to: {error}" + ); + } + + /// Legacy is refused for being unable to take its lock, not for being + /// legacy: it works as root, and rejecting that would break a valid host. + #[test] + fn the_legacy_backend_is_accepted_when_the_lock_is_writable() { + let dir = tempfile::tempdir().expect("tempdir"); + let lock = dir.path().join("xtables.lock"); + fs::write(&lock, b"").expect("create lock"); + + assert!( + iptables_backend_is_usable("iptables", "iptables v1.8.10 (legacy)\n", &lock).is_ok(), + "a legacy backend that can take its lock installs rules fine" + ); + } + + /// The lock is created on first use, so the check must ask about the + /// directory rather than report the absent file as a failure. + #[test] + fn an_absent_lock_in_a_writable_directory_is_usable() { + let dir = tempfile::tempdir().expect("tempdir"); + + assert!( + lock_is_writable(&dir.path().join("xtables.lock")), + "the lock is created on demand, so a writable directory is enough" + ); + } + + /// Pre-1.8 builds print no backend marker and are legacy-only, so an + /// unmarked banner must not be assumed usable. + #[test] + fn a_banner_without_a_backend_marker_is_treated_as_legacy() { + let dir = tempfile::tempdir().expect("tempdir"); + + assert!( + iptables_backend_is_usable("iptables", "iptables v1.6.1\n", &unreachable_lock(&dir)) + .is_err(), + "a pre-1.8 build is legacy-only and must not be assumed usable" + ); + } + + #[test] + fn stderr_detail_keeps_the_tail_and_stays_bounded() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("supervisor.stderr"); + // The interesting line is last; everything before it is noise that must + // not push it out of the quoted region. + let mut noise = "x".repeat(MAX_STDERR_TAIL as usize * 4); + noise.push_str("\nfatal: the-actual-failure\n"); + fs::write(&path, &noise).expect("write stderr"); + + let detail = stderr_detail(&path); + + assert!( + detail.contains("fatal: the-actual-failure"), + "the tail (where the failure is) was dropped: {detail}" + ); + assert!( + detail.len() <= MAX_STDERR_TAIL as usize + 64, + "quoted stderr was not bounded, got {} bytes", + detail.len() + ); + } + + #[test] + fn stderr_detail_reports_absence_rather_than_an_empty_string() { + let dir = tempfile::tempdir().expect("tempdir"); + let missing = dir.path().join("does-not-exist"); + assert_eq!(stderr_detail(&missing), "no stderr output"); + + let empty = dir.path().join("empty.stderr"); + fs::write(&empty, " \n").expect("write stderr"); + assert_eq!(stderr_detail(&empty), "no stderr output"); + } + + /// Counts the rule commands and fails the one named by `MXC_TEST_FAIL_AT`. + /// The real `nsenter` needs a live namespace and `CAP_SYS_ADMIN`; the + /// script only cares whether it succeeded. + const FAKE_NSENTER: &str = r#"#!/bin/sh +count=$(cat "$MXC_TEST_COUNT" 2>/dev/null || echo 0) +count=$((count + 1)) +printf '%s' "$count" > "$MXC_TEST_COUNT" +if [ "${MXC_TEST_FAIL_AT:-0}" = "$count" ]; then + echo "fake nsenter: forced failure on rule $count" >&2 + exit 1 +fi +exit 0 +"#; + + /// Signals readiness on the descriptor the supervisor opened for it (9), + /// then idles so the supervisor's `wait` has something to wait on. + const FAKE_SLIRP: &str = r#"#!/bin/sh +echo $$ > "$MXC_TEST_SLIRP_PID" +if [ "${MXC_TEST_SLIRP_DIES:-0}" = "1" ]; then + echo "fake slirp4netns: exiting before readiness" >&2 + exit 1 +fi +printf ready >&9 +exec sleep 30 +"#; + + /// The supervisor script running for real against fake tools. + /// + /// The six text-matching tests above assert what the script *says*; they + /// cannot assert what it *does*. This runs it under the same `sh` a host + /// would use, with `nsenter`/`slirp4netns` replaced by stubs that can fail + /// on demand, so the fail-closed ordering can be observed rather than read. + struct FakeSupervisor { + dir: TempDir, + child: Child, + pid_writer: Option, + _exit_writer: OwnedFd, + } + + impl FakeSupervisor { + fn state_dir(&self) -> std::path::PathBuf { + self.dir.path().join("state") + } + + /// How many rule commands actually ran. + fn rule_invocations(&self) -> u32 { + fs::read_to_string(self.dir.path().join("rules.count")) + .ok() + .and_then(|value| value.trim().parse().ok()) + .unwrap_or(0) + } + + /// Whether the supervisor told the parent the sandbox is enforced. + fn signalled_ready(&self) -> bool { + self.state_dir().join("slirp.ready").exists() + } + + fn stderr(&self) -> String { + stderr_detail(&self.dir.path().join("supervisor.stderr")) + } + + /// Hand over a sandbox PID, as the parent does once bwrap is up. + fn publish_sandbox_pid(&mut self) { + let mut writer = self.pid_writer.take().expect("pid writer"); + writer.write_all(b"4242\n").expect("publish pid"); + } + + /// Close the PID pipe without writing, as a parent that died would. + fn abandon_without_publishing_pid(&mut self) { + drop(self.pid_writer.take()); + } + + fn wait_for_exit(&mut self) -> ExitStatus { + let deadline = Instant::now() + Duration::from_secs(20); + while Instant::now() < deadline { + if let Some(status) = self.child.try_wait().expect("try_wait") { + return status; + } + thread::sleep(Duration::from_millis(10)); + } + panic!( + "supervisor did not exit; stderr: {}, rules run: {}", + self.stderr(), + self.rule_invocations() + ); + } + + fn wait_until_ready(&mut self) { + let deadline = Instant::now() + Duration::from_secs(20); + while Instant::now() < deadline { + if self.signalled_ready() { + return; + } + if let Some(status) = self.child.try_wait().expect("try_wait") { + panic!( + "supervisor exited ({status}) without signalling readiness; \ + stderr: {}, rules run: {}", + self.stderr(), + self.rule_invocations() + ); + } + thread::sleep(Duration::from_millis(10)); + } + panic!( + "supervisor never signalled readiness; stderr: {}", + self.stderr() + ); + } + } + + impl Drop for FakeSupervisor { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + // `set -e` bypasses the script's TERM trap, so a stub left running + // by an aborted supervisor has to be reaped here. + if let Ok(pid) = fs::read_to_string(self.dir.path().join("slirp.pid")) { + let pid = pid.trim(); + if !pid.is_empty() { + let _ = Command::new("kill") + .args(["-9", pid]) + .stderr(Stdio::null()) + .status(); + } + } + } + } + + fn install_stub(path: &std::path::Path, body: &str) { + fs::write(path, body).expect("write stub"); + let mut perms = fs::metadata(path).expect("stat stub").permissions(); + std::os::unix::fs::PermissionsExt::set_mode(&mut perms, 0o755); + fs::set_permissions(path, perms).expect("chmod stub"); + } + + fn spawn_fake_supervisor(fail_at: Option, slirp_dies: bool) -> FakeSupervisor { + let dir = tempfile::tempdir().expect("tempdir"); + let bin = dir.path().join("bin"); + let state = dir.path().join("state"); + fs::create_dir_all(&bin).expect("bin dir"); + fs::create_dir_all(&state).expect("state dir"); + install_stub(&bin.join("nsenter"), FAKE_NSENTER); + install_stub(&bin.join("slirp4netns"), FAKE_SLIRP); + + let stderr = File::create(dir.path().join("supervisor.stderr")).expect("stderr"); + let (exit_reader, exit_writer) = pipe2(OFlag::O_CLOEXEC).expect("exit pipe"); + let (pid_reader, pid_writer) = pipe2(OFlag::O_CLOEXEC).expect("pid pipe"); + + // The stubs shadow the real tools; the rest of PATH still supplies the + // coreutils the script uses (`sleep`, `cat`). + let path = format!( + "{}:{}", + bin.display(), + std::env::var("PATH").unwrap_or_default() + ); + let mut command = Command::new("sh"); + command + .args(["-c", SUPERVISOR_SCRIPT, "mxc-test-supervisor"]) + .arg(&state) + .arg("10.0.2.2") + .arg("3128") + .arg("mxc-test-chain") + .arg("1") + .env("PATH", path) + .env("MXC_TEST_COUNT", dir.path().join("rules.count")) + .env("MXC_TEST_SLIRP_PID", dir.path().join("slirp.pid")) + .env( + "MXC_TEST_FAIL_AT", + fail_at.map(|n| n.to_string()).unwrap_or_default(), + ) + .env("MXC_TEST_SLIRP_DIES", if slirp_dies { "1" } else { "0" }) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::from(stderr)); + remap_descriptors( + &mut command, + [ + (pid_reader.as_raw_fd(), SUPERVISOR_PID_FD), + (exit_reader.as_raw_fd(), SUPERVISOR_EXIT_FD), + ], + ); + let child = command.spawn().expect("spawn supervisor"); + drop(exit_reader); + drop(pid_reader); + + FakeSupervisor { + dir, + child, + pid_writer: Some(File::from(pid_writer)), + _exit_writer: exit_writer, + } + } + + /// Executing the script also re-checks [`RULE_COMMAND_COUNT`] against the + /// number of commands that actually run, which the text-offset tests can + /// only approximate. + #[test] + fn the_supervisor_signals_readiness_only_after_every_rule_is_installed() { + let mut supervisor = spawn_fake_supervisor(None, false); + supervisor.publish_sandbox_pid(); + supervisor.wait_until_ready(); + + assert_eq!( + supervisor.rule_invocations(), + RULE_COMMAND_COUNT, + "readiness was signalled after a different number of rules than the \ + startup budget is sized for; stderr: {}", + supervisor.stderr() + ); + } + + /// The fail-closed guarantee of the whole feature: a rule that does not + /// install must take the supervisor down *before* readiness, so the parent + /// never releases a sandbox whose egress is unenforced. + /// + /// Asserting it for every rule position is the point. A future edit that + /// moves one command into a pipeline, an `if` condition or a subshell + /// escapes `set -e` and would start an unenforced sandbox while every + /// text-matching test above stayed green. + #[test] + fn a_failed_rule_kills_the_supervisor_instead_of_signalling_readiness() { + for rule in 1..=RULE_COMMAND_COUNT { + let mut supervisor = spawn_fake_supervisor(Some(rule), false); + supervisor.publish_sandbox_pid(); + let status = supervisor.wait_for_exit(); + + assert!( + !status.success(), + "rule {rule} failed but the supervisor exited successfully; stderr: {}", + supervisor.stderr() + ); + assert!( + !supervisor.signalled_ready(), + "rule {rule} failed yet the sandbox was signalled ready -- it would \ + have run with unenforced egress" + ); + assert_eq!( + supervisor.rule_invocations(), + rule, + "rule {rule} failed but the script carried on installing rules" + ); + } + } + + #[test] + fn the_supervisor_gives_up_when_slirp_dies_before_signalling_readiness() { + let mut supervisor = spawn_fake_supervisor(None, true); + supervisor.publish_sandbox_pid(); + let status = supervisor.wait_for_exit(); + + assert!(!status.success(), "a dead slirp must fail the startup"); + assert!(!supervisor.signalled_ready()); + assert!( + supervisor.stderr().contains("before signalling readiness"), + "the failure must say slirp never came up: {}", + supervisor.stderr() + ); + assert_eq!( + supervisor.rule_invocations(), + 0, + "rules were installed against a namespace with no connectivity" + ); + } + + /// The blocking read on the PID pipe exists so a supervisor whose parent + /// died exits instead of spinning as an orphan holding a namespace. + #[test] + fn the_supervisor_exits_when_the_parent_never_publishes_the_sandbox_pid() { + let mut supervisor = spawn_fake_supervisor(None, false); + supervisor.abandon_without_publishing_pid(); + let status = supervisor.wait_for_exit(); + + assert!( + !status.success(), + "an unpublished PID must fail the startup" + ); + assert!(!supervisor.signalled_ready()); + assert!( + supervisor.stderr().contains("before publishing"), + "the failure must name the missing PID: {}", + supervisor.stderr() + ); + assert_eq!(supervisor.rule_invocations(), 0); + } } diff --git a/tests/configs/bubblewrap_network_proxy_egress_denied.json b/tests/configs/bubblewrap_network_proxy_egress_denied.json new file mode 100644 index 000000000..3834801de --- /dev/null +++ b/tests/configs/bubblewrap_network_proxy_egress_denied.json @@ -0,0 +1,12 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Proxy-Egress-Denied", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'set -u; pu=\"${HTTP_PROXY:-${http_proxy:-}}\"; if [ -z \"$pu\" ]; then echo NO_PROXY_ENV; exit 1; fi; pa=\"${pu#*://}\"; pa=\"${pa%%/*}\"; ph=\"${pa%%:*}\"; pp=\"${pa##*:}\"; if [ \"$pp\" = \"$ph\" ]; then pp=80; fi; if ! timeout 6 bash -c \"exec 3<>/dev/tcp/$ph/$pp\" >/dev/null 2>&1; then echo CONTROL_PROXY_UNREACHABLE; exit 1; fi; echo CONTROL_PROXY_REACHABLE_OK; timeout 6 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1; rc=$?; if [ \"$rc\" = 0 ]; then echo DIRECT_EGRESS_LEAKED; exit 1; fi; echo DIRECT_EGRESS_BLOCKED_OK; timeout 4 bash -c \"exec 3<>/dev/tcp/127.0.0.1/9\" >/dev/null 2>&1; lrc=$?; if [ \"$lrc\" = 124 ]; then echo LOOPBACK_DROPPED; exit 1; fi; echo LOOPBACK_EXEMPT_OK; if ! command -v iptables >/dev/null 2>&1; then echo NO_IPTABLES_BINARY; exit 1; fi; capeff=$(grep \"^CapEff\" /proc/self/status | cut -f2); if [ -z \"$capeff\" ]; then echo NO_CAPEFF; exit 1; fi; if [ $((0x$capeff & 0x1000)) -ne 0 ]; then echo CAP_NET_ADMIN_RETAINED; exit 1; fi; echo CAP_NET_ADMIN_DROPPED_OK; terr=$(iptables -F MXC_EGRESS 2>&1); trc=$?; if [ \"$trc\" = 0 ]; then echo \"TAMPER_FLUSH_SUCCEEDED: $terr\"; exit 1; fi; echo \"TAMPER_REFUSED_OK rc=$trc\"; timeout 6 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1; if [ $? = 0 ]; then echo TAMPER_DISABLED_EGRESS; exit 1; fi; echo TAMPER_INEFFECTIVE_OK; if ! curl -fsSL --max-time 15 https://api.github.com/zen >/dev/null; then echo PROXY_UNREACHABLE; exit 1; fi; echo PROXY_STILL_OK'" + }, + "network": { + "defaultPolicy": "allow", + "proxy": { "builtinTestServer": true } + } +} diff --git a/tests/scripts/run_bwrap_network_proxy_test.sh b/tests/scripts/run_bwrap_network_proxy_test.sh index fe0876983..7b426c15b 100644 --- a/tests/scripts/run_bwrap_network_proxy_test.sh +++ b/tests/scripts/run_bwrap_network_proxy_test.sh @@ -216,4 +216,21 @@ if [ "$SUPERVISOR_REAPED" -ne 1 ]; then fi echo "PASS: supervisor orphan reaping" +echo "Running Bubblewrap proxy-only egress enforcement test..." +if ! EGRESS_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_proxy_egress_denied.json" 2>&1); then + echo "$EGRESS_OUT" + echo "FAIL: proxy-only egress (lxc-exec returned non-zero)" + exit 1 +fi +for sentinel in CONTROL_PROXY_REACHABLE_OK DIRECT_EGRESS_BLOCKED_OK LOOPBACK_EXEMPT_OK \ + CAP_NET_ADMIN_DROPPED_OK TAMPER_REFUSED_OK TAMPER_INEFFECTIVE_OK PROXY_STILL_OK; do + if ! grep -q "$sentinel" <<<"$EGRESS_OUT"; then + echo "$EGRESS_OUT" + echo "FAIL: proxy-only egress (sentinel '$sentinel' not found in output)" + exit 1 + fi +done +echo "PASS: proxy-only egress enforcement" + echo "Bubblewrap network proxy tests complete."