From a5e7f460d50e63757db008da056abbf4e4305b0c Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 14 Aug 2026 13:58:57 -0700 Subject: [PATCH 1/3] Centralize Bubblewrap network mode selection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- .../bubblewrap/common/src/bwrap_command.rs | 190 ++++++++++++++++-- .../bubblewrap/common/src/bwrap_runner.rs | 37 ++-- 2 files changed, 192 insertions(+), 35 deletions(-) diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 8fed24071..1dfc847d8 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -10,7 +10,7 @@ use std::collections::HashSet; use wxc_common::filesystem_resolve::FsIntent; -use wxc_common::models::{ExecutionRequest, NetworkPolicy, ProxyAddress}; +use wxc_common::models::{ExecutionRequest, NetworkEnforcementMode, NetworkPolicy, ProxyAddress}; use wxc_common::proxy_env::{is_managed_proxy_key, PROXY_SET_KEYS}; /// Read-only host paths bind-mounted into every Bubblewrap sandbox as the @@ -87,17 +87,52 @@ const BASELINE_RO_BIND_PATHS: &[&str] = &[ "/mnt/wsl/resolv.conf", ]; -/// Whether the sandbox gets its own network namespace (`--unshare-net`) rather -/// than sharing the host's. -/// -/// Full isolation applies only when the default policy denies outbound, no -/// per-host rules need iptables on the shared namespace, and no loopback proxy -/// has to stay reachable. -fn uses_private_netns(request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>) -> bool { - request.policy.default_network_policy == NetworkPolicy::Block - && request.policy.allowed_hosts.is_empty() - && request.policy.blocked_hosts.is_empty() - && proxy_address.is_none() +/// The networking behavior Bubblewrap applies for one execution. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ResolvedNetworkMode { + /// A private network namespace with no external connectivity. + Isolated, + /// The host network namespace without MXC firewall filtering. + Shared, + /// The host network namespace with per-destination iptables filtering. + FirewallFiltered, + /// Cooperative proxy routing; later work moves this into a private + /// namespace and adds proxy-only egress enforcement. + ProxyOnly, +} + +impl ResolvedNetworkMode { + /// Classify the internal request using the proxy's resolved runtime state. + pub(crate) fn from_request(request: &ExecutionRequest, proxy_active: bool) -> Self { + if proxy_active { + return Self::ProxyOnly; + } + + let uses_firewall = matches!( + request.policy.network_enforcement_mode, + NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both + ); + let has_host_rules = + !request.policy.allowed_hosts.is_empty() || !request.policy.blocked_hosts.is_empty(); + + if uses_firewall && has_host_rules { + Self::FirewallFiltered + } else if request.policy.default_network_policy == NetworkPolicy::Block && !has_host_rules { + Self::Isolated + } else { + Self::Shared + } + } + + /// Whether current behavior gives the sandbox a private network namespace. + pub(crate) fn uses_private_netns(self) -> bool { + matches!(self, Self::Isolated) + } + + /// Whether current behavior installs iptables filtering rules. + pub(crate) fn requires_iptables(self) -> bool { + matches!(self, Self::FirewallFiltered) + } } /// Describe a `network.allowLocalNetwork` setting Bubblewrap cannot honor, or @@ -122,10 +157,19 @@ fn uses_private_netns(request: &ExecutionRequest, proxy_address: Option<&ProxyAd pub fn local_network_diagnostic( request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>, +) -> Option<&'static str> { + let network_mode = ResolvedNetworkMode::from_request(request, proxy_address.is_some()); + local_network_diagnostic_for_mode(request, network_mode) +} + +/// Describe an inbound-policy mismatch using a previously resolved mode. +pub(crate) fn local_network_diagnostic_for_mode( + request: &ExecutionRequest, + network_mode: ResolvedNetworkMode, ) -> Option<&'static str> { match ( request.policy.allow_local_network, - uses_private_netns(request, proxy_address), + network_mode.uses_private_netns(), ) { (false, false) => Some( "WARNING: Bubblewrap: network.allowLocalNetwork=false is not enforced while the \ @@ -179,6 +223,17 @@ pub fn build_args_classified( request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>, denied_files: &HashSet, +) -> Vec { + let network_mode = ResolvedNetworkMode::from_request(request, proxy_address.is_some()); + build_args_classified_with_mode(request, proxy_address, denied_files, network_mode) +} + +/// Build Bubblewrap arguments using a previously resolved network mode. +pub(crate) fn build_args_classified_with_mode( + request: &ExecutionRequest, + proxy_address: Option<&ProxyAddress>, + denied_files: &HashSet, + network_mode: ResolvedNetworkMode, ) -> Vec { // -- Namespace isolation (all unshared by default) --------------------- let mut args = vec![ @@ -197,7 +252,7 @@ pub fn build_args_classified( // applies iptables rules separately. When a network proxy is active we // also keep the host network namespace so the sandbox can reach the // loopback proxy. - if uses_private_netns(request, proxy_address) { + if network_mode.uses_private_netns() { args.push("--unshare-net".into()); } @@ -309,7 +364,6 @@ pub fn build_args_classified( #[cfg(test)] mod tests { use super::*; - use wxc_common::models::{ExecutionRequest, NetworkPolicy, ProxyAddress}; fn base_request() -> ExecutionRequest { ExecutionRequest { @@ -328,6 +382,112 @@ mod tests { assert!(args.contains(&"--unshare-uts".to_string())); } + struct NetworkPlanCase { + name: &'static str, + default_policy: NetworkPolicy, + enforcement_mode: NetworkEnforcementMode, + allowed_hosts: &'static [&'static str], + blocked_hosts: &'static [&'static str], + proxy_active: bool, + expected: ResolvedNetworkMode, + } + + #[test] + fn classifies_current_network_policy_modes() { + let cases = [ + NetworkPlanCase { + name: "default block is isolated", + default_policy: NetworkPolicy::Block, + enforcement_mode: NetworkEnforcementMode::Capabilities, + allowed_hosts: &[], + blocked_hosts: &[], + proxy_active: false, + expected: ResolvedNetworkMode::Isolated, + }, + NetworkPlanCase { + name: "default allow is shared", + default_policy: NetworkPolicy::Allow, + enforcement_mode: NetworkEnforcementMode::Capabilities, + allowed_hosts: &[], + blocked_hosts: &[], + proxy_active: false, + expected: ResolvedNetworkMode::Shared, + }, + NetworkPlanCase { + name: "firewall allow rules require filtering", + default_policy: NetworkPolicy::Block, + enforcement_mode: NetworkEnforcementMode::Firewall, + allowed_hosts: &["example.com"], + blocked_hosts: &[], + proxy_active: false, + expected: ResolvedNetworkMode::FirewallFiltered, + }, + NetworkPlanCase { + name: "combined enforcement block rules require filtering", + default_policy: NetworkPolicy::Allow, + enforcement_mode: NetworkEnforcementMode::Both, + allowed_hosts: &[], + blocked_hosts: &["example.com"], + proxy_active: false, + expected: ResolvedNetworkMode::FirewallFiltered, + }, + NetworkPlanCase { + name: "capabilities mode with host rules stays shared", + default_policy: NetworkPolicy::Block, + enforcement_mode: NetworkEnforcementMode::Capabilities, + allowed_hosts: &["example.com"], + blocked_hosts: &[], + proxy_active: false, + expected: ResolvedNetworkMode::Shared, + }, + NetworkPlanCase { + name: "proxy takes precedence over isolation", + default_policy: NetworkPolicy::Block, + enforcement_mode: NetworkEnforcementMode::Capabilities, + allowed_hosts: &[], + blocked_hosts: &[], + proxy_active: true, + expected: ResolvedNetworkMode::ProxyOnly, + }, + NetworkPlanCase { + name: "proxy takes precedence over firewall filtering", + default_policy: NetworkPolicy::Allow, + enforcement_mode: NetworkEnforcementMode::Firewall, + allowed_hosts: &[], + blocked_hosts: &["example.com"], + proxy_active: true, + expected: ResolvedNetworkMode::ProxyOnly, + }, + ]; + + for case in cases { + let mut request = ExecutionRequest::default(); + request.policy.default_network_policy = case.default_policy; + request.policy.network_enforcement_mode = case.enforcement_mode; + request.policy.allowed_hosts = case + .allowed_hosts + .iter() + .map(|host| (*host).into()) + .collect(); + request.policy.blocked_hosts = case + .blocked_hosts + .iter() + .map(|host| (*host).into()) + .collect(); + + let actual = ResolvedNetworkMode::from_request(&request, case.proxy_active); + assert_eq!(actual, case.expected, "{}", case.name); + } + } + + #[test] + fn resolved_network_mode_capabilities_match_current_behavior() { + assert!(ResolvedNetworkMode::Isolated.uses_private_netns()); + assert!(!ResolvedNetworkMode::ProxyOnly.uses_private_netns()); + assert!(ResolvedNetworkMode::FirewallFiltered.requires_iptables()); + assert!(!ResolvedNetworkMode::Shared.requires_iptables()); + } + #[test] fn network_block_adds_unshare_net() { let mut r = base_request(); diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index fb6c97127..9fb8c5088 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -35,7 +35,7 @@ use std::time::Duration; use lxc_common::network_iptables::NetworkIptablesManager; use wxc_common::interruptible_reader::{wrap_pipe, InterruptibleReader, ReadCanceller}; use wxc_common::logger::Logger; -use wxc_common::models::{ExecutionRequest, NetworkEnforcementMode, ScriptResponse}; +use wxc_common::models::{ExecutionRequest, ScriptResponse}; use wxc_common::sandbox_process::{ boxed_closer, cancel_and_join_discard, group_kill, spawn_discard, take_boxed_read, take_boxed_write, wait_with_timeout, SandboxBackend, SandboxProcess, StdioMode, StreamCloser, @@ -44,7 +44,10 @@ use wxc_common::sandbox_process::{ use wxc_common::unix_proxy_coordinator::UnixProxyCoordinator; use wxc_common::validator::validate_common; -use crate::{bwrap_command, bwrap_version}; +use crate::{ + bwrap_command::{self, ResolvedNetworkMode}, + bwrap_version, +}; /// Bubblewrap sandbox runner. Uses only shared `ContainerPolicy` fields — /// no backend-specific config struct required. @@ -213,13 +216,22 @@ impl BubblewrapScriptRunner { } } + let network_mode = ResolvedNetworkMode::from_request(request, proxy.is_active()); + // 2. Build the bwrap argument vector. `denied_files` is the file-mask // subset classified during symlink resolution (see // [`resolve_denied_paths`]). - if let Some(warning) = bwrap_command::local_network_diagnostic(request, proxy.address()) { + if let Some(warning) = + bwrap_command::local_network_diagnostic_for_mode(request, network_mode) + { let _ = writeln!(logger, "{}", warning); } - let args = bwrap_command::build_args_classified(request, proxy.address(), denied_files); + let args = bwrap_command::build_args_classified_with_mode( + request, + proxy.address(), + denied_files, + network_mode, + ); let _ = writeln!( logger, "Bubblewrap: spawning bwrap with {} args", @@ -229,7 +241,7 @@ impl BubblewrapScriptRunner { // 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 = needs_iptables_rules(request) && !proxy.is_active(); + let needs_iptables = network_mode.requires_iptables(); let container_name = if request.container_id.is_empty() { format!("bwrap-{:08x}", std::process::id()) } else { @@ -516,21 +528,6 @@ impl Drop for BubblewrapSandboxProcess { } } -/// Returns `true` when the request has per-host network rules that require -/// iptables. Pure `"block"` with no host lists uses `--unshare-net` instead. -fn needs_iptables_rules(request: &ExecutionRequest) -> bool { - let uses_firewall = matches!( - request.policy.network_enforcement_mode, - NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both - ); - let has_host_rules = - !request.policy.allowed_hosts.is_empty() || !request.policy.blocked_hosts.is_empty(); - - // Only invoke iptables when there are actual per-host rules to apply and - // the enforcement mode includes firewall. - uses_firewall && has_host_rules -} - /// Build the iptables manager for a Bubblewrap sandbox. /// /// Unprivileged bwrap has no veth: the sandbox either shares the host network From 75c4f84258a27e79deaa50ce079a94b732b9dde0 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 14 Aug 2026 16:51:44 -0700 Subject: [PATCH 2/3] Add schema-gated private networking for Bubblewrap proxy Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- .../templates/SDK.Integration.Test.Job.yml | 4 +- .github/copilot-instructions.md | 2 +- .github/workflows/Build.Linux.Job.yml | 2 +- docs/bwrap-support/bubblewrap-backend.md | 54 ++- src/Cargo.lock | 1 + src/backends/bubblewrap/common/Cargo.toml | 3 +- .../bubblewrap/common/src/bwrap_command.rs | 139 ++++-- .../bubblewrap/common/src/bwrap_runner.rs | 88 +++- src/backends/bubblewrap/common/src/lib.rs | 2 + .../bubblewrap/common/src/proxy_network.rs | 433 ++++++++++++++++++ .../bubblewrap_network_proxy_namespace.json | 12 + tests/scripts/run_bwrap_network_proxy_test.sh | 32 +- 12 files changed, 704 insertions(+), 68 deletions(-) create mode 100644 src/backends/bubblewrap/common/src/proxy_network.rs create mode 100644 tests/configs/bubblewrap_network_proxy_namespace.json diff --git a/.azure-pipelines/templates/SDK.Integration.Test.Job.yml b/.azure-pipelines/templates/SDK.Integration.Test.Job.yml index f14c546fd..b2b32dbc6 100644 --- a/.azure-pipelines/templates/SDK.Integration.Test.Job.yml +++ b/.azure-pipelines/templates/SDK.Integration.Test.Job.yml @@ -99,8 +99,8 @@ jobs: - ${{ if eq(target.os, 'linux') }}: - script: | sudo apt-get update -qq - sudo apt-get install -y -qq lxc lxc-utils dnsmasq-base iptables bubblewrap - displayName: Install LXC and Bubblewrap + sudo apt-get install -y -qq lxc lxc-utils dnsmasq-base iptables bubblewrap slirp4netns + displayName: Install LXC, Bubblewrap, and slirp4netns - script: sudo MXC_SKIP_LXC_NETWORK_TESTS=1 MXC_DEBUG=${{ parameters.debug }} npm test workingDirectory: $(integrationDirectory) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3a1ad851a..e00d601c2 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`. 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 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`. | ### Config flow diff --git a/.github/workflows/Build.Linux.Job.yml b/.github/workflows/Build.Linux.Job.yml index 66c804c46..6988b0a0b 100644 --- a/.github/workflows/Build.Linux.Job.yml +++ b/.github/workflows/Build.Linux.Job.yml @@ -85,7 +85,7 @@ jobs: working-directory: ${{ github.workspace }} run: | sudo apt-get update - sudo apt-get install -y bubblewrap + sudo apt-get install -y bubblewrap slirp4netns # Ubuntu 24.04 runners restrict unprivileged user namespaces via # AppArmor, which blocks `bwrap --unshare-user`. Relax it so the # sandbox can start (no-op on kernels without this knob). diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index 6232845d8..ff5597d26 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -27,6 +27,22 @@ 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 + the legacy proxy behavior. + ```bash + # Debian/Ubuntu + sudo apt install slirp4netns + + # Fedora/RHEL + sudo dnf install slirp4netns + + # Alpine + apk add slirp4netns + ``` + 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`. - User namespaces must be enabled: ```bash # Check: should print "1" @@ -219,7 +235,7 @@ namespace choice alone decides the outcome: | `allowLocalNetwork` | Namespace | Result | |---------------------|-----------|--------| -| `false` (default) | private (`--unshare-net`) | Honored at the sandbox boundary — nothing outside can reach in. `bind()`/`listen()` still succeed on the sandbox's own loopback, so its processes can talk to each other; that is already inside the caller's trust boundary | +| `false` (default) | private (`--unshare-net`, including 0.8 proxy mode) | Honored at the sandbox boundary — nothing outside can reach in. `bind()`/`listen()` still succeed on the sandbox's own loopback, so its processes can talk to each other; that is already inside the caller's trust boundary | | `false` | shared with host | **Not honored** — the process can bind/listen on host-local addresses | | `true` | private (`--unshare-net`) | **Partially honored** — the listener is reachable only from inside the sandbox | | `true` | shared with host | Honored | @@ -244,12 +260,19 @@ Standard `process` fields work as expected: } ``` -## Network proxy (cooperative, unprivileged) +## Network proxy (private namespace, unprivileged) Bubblewrap supports an **unprivileged, cooperative network proxy** that enforces `allowedHosts` / `blockedHosts` at the proxy layer instead of via -iptables. This is the **recommended** way to do per-host filtering on -Bubblewrap because it requires **no root and no `CAP_NET_ADMIN`**. +host-level iptables. The workload runs in a private network namespace and +reaches the proxy through rootless `slirp4netns` routing. This requires no root +privileges. + +This private-network behavior applies to schema **0.8 and later**. Policies +using schema 0.6 or 0.7 retain the existing shared-host-network proxy behavior +for compatibility and do not require `slirp4netns`. An absent schema version is +also treated as legacy. The runner never silently falls back: a 0.8 proxy +request fails if its private namespace cannot be configured. ### How it works @@ -258,17 +281,19 @@ Bubblewrap because it requires **no root and no `CAP_NET_ADMIN`**. `unix-test-proxy` binary is used (`builtinTestServer: true`, testing-only and gated behind `--allow-testing-features`); in production callers supply their own proxy via `localhost: ` or `url: `. -2. The sandbox is then started **without** `--unshare-net` so the sandbox - shares the host network namespace and can reach the loopback proxy. -3. The command builder sets `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY`, +2. The runner creates a same-UID user-namespace supervisor, starts Bubblewrap + 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. +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 - `NO_PROXY` / `no_proxy`, are stripped before injection). The runner deliberately - does **not** set `NO_PROXY`: since the sandbox shares the host netns, - a `NO_PROXY=localhost,127.0.0.1` entry would let cooperating clients - bypass the proxy for host-loopback destinations, defeating - `allowedHosts` / `blockedHosts` enforcement for those targets. -4. Cooperative tools (curl, wget, Python `requests`, Node `https`, etc.) + `NO_PROXY` / `no_proxy`, are stripped before injection). The runner + deliberately does **not** set `NO_PROXY`, because exempt destinations would + 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. @@ -309,7 +334,8 @@ Bubblewrap because it requires **no root and no `CAP_NET_ADMIN`**. `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) are **not enforced**. + 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 diff --git a/src/Cargo.lock b/src/Cargo.lock index f0c9c9b62..45fa4991d 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -222,6 +222,7 @@ dependencies = [ "serde_json", "tempfile", "thiserror", + "url", "wxc_common", ] diff --git a/src/backends/bubblewrap/common/Cargo.toml b/src/backends/bubblewrap/common/Cargo.toml index e337d810e..bbf72eed8 100644 --- a/src/backends/bubblewrap/common/Cargo.toml +++ b/src/backends/bubblewrap/common/Cargo.toml @@ -10,7 +10,8 @@ lxc_common = { workspace = true } nix = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +tempfile = { workspace = true } thiserror = { workspace = true } +url = { workspace = true } [dev-dependencies] -tempfile = { workspace = true } diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 1dfc847d8..3d6b68eaf 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -94,18 +94,36 @@ pub(crate) enum ResolvedNetworkMode { Isolated, /// The host network namespace without MXC firewall filtering. Shared, + /// Pre-0.8 cooperative proxy routing in the host network namespace. + LegacyProxy, /// The host network namespace with per-destination iptables filtering. FirewallFiltered, - /// Cooperative proxy routing; later work moves this into a private - /// namespace and adds proxy-only egress enforcement. + /// Cooperative proxy routing inside a slirp-backed private namespace. + /// Later work adds proxy-only egress enforcement. ProxyOnly, } +/// Whether this schema opts into the 0.8 private proxy-network contract. +fn uses_private_proxy_network(request: &ExecutionRequest) -> bool { + let mut components = request.schema_version.split('.'); + let major = components + .next() + .and_then(|value| value.parse::().ok()); + let minor = components + .next() + .and_then(|value| value.parse::().ok()); + matches!((major, minor), (Some(major), Some(minor)) if major > 0 || minor >= 8) +} + impl ResolvedNetworkMode { /// Classify the internal request using the proxy's resolved runtime state. pub(crate) fn from_request(request: &ExecutionRequest, proxy_active: bool) -> Self { if proxy_active { - return Self::ProxyOnly; + return if uses_private_proxy_network(request) { + Self::ProxyOnly + } else { + Self::LegacyProxy + }; } let uses_firewall = matches!( @@ -126,10 +144,16 @@ impl ResolvedNetworkMode { /// Whether current behavior gives the sandbox a private network namespace. pub(crate) fn uses_private_netns(self) -> bool { - matches!(self, Self::Isolated) + matches!(self, Self::Isolated | Self::ProxyOnly) + } + + /// Whether the runner supplies a pre-created user namespace to Bubblewrap. + pub(crate) fn uses_external_userns(self) -> bool { + matches!(self, Self::ProxyOnly) } /// Whether current behavior installs iptables filtering rules. + #[cfg(any(target_os = "linux", test))] pub(crate) fn requires_iptables(self) -> bool { matches!(self, Self::FirewallFiltered) } @@ -173,16 +197,16 @@ pub(crate) fn local_network_diagnostic_for_mode( ) { (false, false) => Some( "WARNING: Bubblewrap: network.allowLocalNetwork=false is not enforced while the \ - sandbox shares the host network namespace (defaultPolicy='allow' or network.proxy). \ + sandbox shares the host network namespace (defaultPolicy='allow'). \ The sandboxed process can still bind, listen and accept on host-local addresses. For \ an unreachable sandbox use defaultPolicy='block' with no proxy, which applies \ --unshare-net.", ), (true, true) => Some( "WARNING: Bubblewrap: network.allowLocalNetwork=true is confined to the sandbox's own \ - network namespace. defaultPolicy='block' with no proxy applies --unshare-net, so a \ - listener inside the sandbox is reachable only from within it, never from the host. \ - Use defaultPolicy='allow' to share the host network namespace.", + network namespace. Isolated and proxy modes apply --unshare-net, so a listener inside \ + the sandbox is reachable only from within it, never from the host. Use \ + defaultPolicy='allow' without a proxy to share the host network namespace.", ), _ => None, } @@ -205,11 +229,11 @@ pub fn build_args(request: &ExecutionRequest, proxy_address: Option<&ProxyAddres /// The returned vector does **not** include the `bwrap` binary name itself — /// callers pass it to `Command::new("bwrap").args(&args)`. /// -/// `proxy_address` is the loopback address of the network proxy launched by -/// the Bubblewrap runner (if the request has `network.proxy` configured). +/// `proxy_address` is the proxy endpoint visible from the sandbox (if the +/// request has `network.proxy` configured). /// When `Some`, the builder: -/// - drops `--unshare-net` (the sandbox needs to reach the loopback proxy on -/// the host's network namespace), +/// - emits `--unshare-net` and expects the runner to provide `--userns FD` +/// plus slirp-backed connectivity, /// - strips any caller-supplied `HTTP_PROXY` / `HTTPS_PROXY` / `ALL_PROXY` / /// `FTP_PROXY` / `NO_PROXY` entries from `request.env`, /// - emits `--setenv` for the proxy keys (all but `NO_PROXY`) pointing at the @@ -236,22 +260,19 @@ pub(crate) fn build_args_classified_with_mode( network_mode: ResolvedNetworkMode, ) -> Vec { // -- Namespace isolation (all unshared by default) --------------------- - let mut args = vec![ - "--unshare-user", - "--unshare-pid", - "--unshare-ipc", - "--unshare-uts", - ] - .into_iter() - .map(String::from) - .collect::>(); - - // Network: use --unshare-net for full block when no per-host rules are - // configured AND no proxy is active. When allowedHosts / blockedHosts - // are present the runner leaves the network namespace shared and - // applies iptables rules separately. When a network proxy is active we - // also keep the host network namespace so the sandbox can reach the - // loopback proxy. + let mut args = Vec::new(); + if !network_mode.uses_external_userns() { + args.push("--unshare-user".into()); + } + args.extend( + ["--unshare-pid", "--unshare-ipc", "--unshare-uts"] + .into_iter() + .map(String::from), + ); + + // Network: full-block and proxy modes use a private namespace. Proxy mode + // receives rootless connectivity from the runner's slirp supervisor. + // Per-host firewall mode continues to share the host namespace. if network_mode.uses_private_netns() { args.push("--unshare-net".into()); } @@ -338,13 +359,8 @@ pub(crate) fn build_args_classified_with_mode( // are NOT enforced -- this is a documented limitation of the // unprivileged proxy model. // - // We deliberately do NOT set NO_PROXY here. Bubblewrap with a proxy - // keeps the host network namespace shared, so without a NO_PROXY entry - // a cooperating client doing `CONNECT 127.0.0.1:5432` (e.g. local - // Postgres) still goes via the proxy, where the configured - // allowed/blocked-hosts policy applies. Exempting loopback via - // NO_PROXY would silently bypass that filtering for host-loopback - // destinations. + // We deliberately do NOT set NO_PROXY here. Exempting any destination + // would let cooperating clients bypass the configured proxy policy. if let Some(addr) = proxy_address { let url = addr.to_url(); for key in PROXY_SET_KEYS { @@ -461,7 +477,10 @@ mod tests { ]; for case in cases { - let mut request = ExecutionRequest::default(); + let mut request = ExecutionRequest { + schema_version: "0.8.0-alpha".into(), + ..Default::default() + }; request.policy.default_network_policy = case.default_policy; request.policy.network_enforcement_mode = case.enforcement_mode; request.policy.allowed_hosts = case @@ -483,7 +502,10 @@ mod tests { #[test] fn resolved_network_mode_capabilities_match_current_behavior() { assert!(ResolvedNetworkMode::Isolated.uses_private_netns()); - assert!(!ResolvedNetworkMode::ProxyOnly.uses_private_netns()); + assert!(ResolvedNetworkMode::ProxyOnly.uses_private_netns()); + 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()); } @@ -544,12 +566,20 @@ mod tests { } #[test] - fn local_network_denied_with_proxy_warns() { + fn local_network_denied_with_legacy_proxy_warns() { let r = base_request(); let addr = ProxyAddress::new("127.0.0.1".into(), 8080); assert!(local_network_diagnostic(&r, Some(&addr)).is_some()); } + #[test] + fn local_network_denied_with_0_8_proxy_is_not_warned() { + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + let addr = ProxyAddress::new("127.0.0.1".into(), 8080); + assert!(local_network_diagnostic(&r, Some(&addr)).is_none()); + } + #[test] fn local_network_allowed_under_private_netns_warns() { let mut r = base_request(); @@ -828,15 +858,39 @@ mod tests { // ------- Network proxy env-var injection tests ---------------------- #[test] - fn proxy_active_omits_unshare_net_even_when_default_blocks() { + fn proxy_active_uses_private_network_and_external_user_namespace() { let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); r.policy.default_network_policy = NetworkPolicy::Block; let addr = ProxyAddress::new("127.0.0.1".into(), 12345); let args = build_args(&r, Some(&addr)); assert!( - !args.contains(&"--unshare-net".to_string()), - "proxy active must keep host netns so loopback proxy is reachable" + args.contains(&"--unshare-net".to_string()), + "proxy mode must use a private network namespace" ); + assert!( + !args.contains(&"--unshare-user".to_string()), + "the runner supplies proxy mode's pre-created user namespace" + ); + } + + #[test] + fn pre_0_8_proxy_keeps_existing_shared_network_behavior() { + for version in ["", "0.6.0-alpha", "0.7.0-alpha"] { + let mut request = base_request(); + request.schema_version = version.into(); + let address = ProxyAddress::new("127.0.0.1".into(), 12345); + let args = build_args(&request, Some(&address)); + + assert!( + args.contains(&"--unshare-user".to_string()), + "{version:?} should retain Bubblewrap's existing user namespace setup" + ); + assert!( + !args.contains(&"--unshare-net".to_string()), + "{version:?} should retain shared-network proxy behavior" + ); + } } #[test] @@ -870,8 +924,7 @@ mod tests { fn proxy_active_does_not_exempt_loopback_via_no_proxy() { // Setting NO_PROXY=localhost,127.0.0.1 would let cooperating HTTP // clients bypass the proxy for host-loopback destinations. - // Bubblewrap+proxy keeps the host netns shared, so that bypass - // would silently defeat allowedHosts/blockedHosts for loopback. + // Any bypass would silently defeat allowedHosts/blockedHosts. let r = base_request(); let addr = ProxyAddress::new("127.0.0.1".into(), 7777); let args = build_args(&r, Some(&addr)); diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index 9fb8c5088..1c2aecd0e 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -46,7 +46,7 @@ use wxc_common::validator::validate_common; use crate::{ bwrap_command::{self, ResolvedNetworkMode}, - bwrap_version, + bwrap_version, proxy_network, }; /// Bubblewrap sandbox runner. Uses only shared `ContainerPolicy` fields — @@ -117,6 +117,13 @@ impl SandboxBackend for BubblewrapScriptRunner { 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 let Err(error) = proxy_network::probe_dependencies() { + return Err(ScriptResponse::error(&error)); + } + } Ok(()) } @@ -217,6 +224,38 @@ impl BubblewrapScriptRunner { } let network_mode = ResolvedNetworkMode::from_request(request, proxy.is_active()); + let sandbox_proxy_address = if network_mode == ResolvedNetworkMode::ProxyOnly { + match proxy.address() { + Some(address) => match proxy_network::sandbox_proxy_address(address) { + Ok(address) => Some(address), + Err(error) => { + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } + }, + None => { + proxy.stop(logger); + return Err(ScriptResponse::error( + "Bubblewrap: proxy mode was selected without a resolved proxy address.", + )); + } + } + } else { + None + }; + 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) { + Ok(network) => Some(network), + Err(error) => { + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } + } + } else { + None + }; // 2. Build the bwrap argument vector. `denied_files` is the file-mask // subset classified during symlink resolution (see @@ -226,12 +265,23 @@ impl BubblewrapScriptRunner { { let _ = writeln!(logger, "{}", warning); } - let args = bwrap_command::build_args_classified_with_mode( + let mut args = bwrap_command::build_args_classified_with_mode( request, - proxy.address(), + proxy_address, denied_files, network_mode, ); + let mut network_startup = match proxy_network.as_ref() { + Some(network) => match network.configure_bwrap(&mut args) { + Ok(startup) => Some(startup), + Err(error) => { + proxy_network.take(); + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } + }, + None => None, + }; let _ = writeln!( logger, "Bubblewrap: spawning bwrap with {} args", @@ -310,6 +360,7 @@ impl BubblewrapScriptRunner { Err(error) => { let mut fw_manager = fw_manager; cleanup_iptables(&mut fw_manager, logger); + proxy_network.take(); proxy.stop(logger); return Err(ScriptResponse::error(&format!( "Bubblewrap: failed to spawn bwrap: {}", @@ -318,6 +369,31 @@ impl BubblewrapScriptRunner { } }; + if let Some(mut startup) = network_startup.take() { + startup.child_spawned(); + let startup_result = startup + .child_pid(&mut child) + .and_then(|child_pid| { + proxy_network + .as_mut() + .ok_or_else(|| { + "Bubblewrap: proxy network lifecycle disappeared during startup" + .to_string() + })? + .attach(child_pid, logger) + }) + .and_then(|()| startup.release()); + if let Err(error) = startup_result { + let _ = child.kill(); + let _ = child.wait(); + let mut fw_manager = fw_manager; + cleanup_iptables(&mut fw_manager, logger); + proxy_network.take(); + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } + } + let (stdin, stdout, stderr) = match stdio { StdioMode::Pipes => (child.stdin.take(), child.stdout.take(), child.stderr.take()), StdioMode::Inherit => (None, None, None), @@ -336,6 +412,7 @@ impl BubblewrapScriptRunner { let _ = child.wait(); let mut fw_manager = fw_manager; cleanup_iptables(&mut fw_manager, logger); + proxy_network.take(); proxy.stop(logger); let error = out_result.err().or(err_result.err()); return Err(ScriptResponse::error(&format!( @@ -359,6 +436,7 @@ impl BubblewrapScriptRunner { stderr_canceller, group, proxy, + proxy_network, fw_manager, timeout, }) @@ -381,6 +459,7 @@ struct BwrapChild { /// killing bwrap (pid 1 of the namespace) alone tears the sandbox down. group: bool, proxy: UnixProxyCoordinator, + proxy_network: Option, fw_manager: Option, timeout: Option, } @@ -390,6 +469,9 @@ impl BwrapChild { /// the manager level. fn cleanup(&mut self, logger: &mut Logger) { cleanup_iptables(&mut self.fw_manager, logger); + if let Some(mut network) = self.proxy_network.take() { + network.stop(logger); + } self.proxy.stop(logger); } } diff --git a/src/backends/bubblewrap/common/src/lib.rs b/src/backends/bubblewrap/common/src/lib.rs index 0cb83efd2..a74acbf8f 100644 --- a/src/backends/bubblewrap/common/src/lib.rs +++ b/src/backends/bubblewrap/common/src/lib.rs @@ -17,3 +17,5 @@ pub mod bwrap_command; #[cfg(target_os = "linux")] pub mod bwrap_runner; pub mod bwrap_version; +#[cfg(target_os = "linux")] +mod proxy_network; diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs new file mode 100644 index 000000000..70470cfd6 --- /dev/null +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -0,0 +1,433 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Rootless private networking for Bubblewrap proxy mode. + +use std::fs::{self, File, OpenOptions}; +use std::io::{ErrorKind, Read, Write}; +use std::os::fd::{AsRawFd, OwnedFd, RawFd}; +use std::process::{Child, Command, Stdio}; +use std::thread; +use std::time::{Duration, Instant}; + +use nix::fcntl::{fcntl, FcntlArg, FdFlag, OFlag}; +use nix::unistd::pipe2; +use tempfile::TempDir; +use wxc_common::logger::Logger; +use wxc_common::models::ProxyAddress; + +const STARTUP_TIMEOUT: Duration = Duration::from_secs(5); +const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(2); +const SLIRP_HOST_GATEWAY: &str = "10.0.2.2"; +const SUPERVISOR_SCRIPT: &str = r#" +set -eu +state_dir="$1" +ready_fd="$2" +exit_fd="$3" +printf ready > "$state_dir/userns.ready" +while [ ! -s "$state_dir/child.pid" ]; do + sleep 0.01 +done +child_pid="$(cat "$state_dir/child.pid")" +exec slirp4netns --configure --mtu=65520 \ + --ready-fd "$ready_fd" --exit-fd "$exit_fd" \ + "$child_pid" tap0 +"#; + +/// Runtime file descriptors Bubblewrap needs while establishing its child. +pub(crate) struct BwrapStartup { + info_reader: File, + info_writer: Option, + gate_reader: Option, + gate_writer: Option, +} + +impl BwrapStartup { + /// Close the parent copies of the descriptors inherited by Bubblewrap. + pub(crate) fn child_spawned(&mut self) { + self.info_writer.take(); + self.gate_reader.take(); + } + + /// Wait for Bubblewrap to report the host-visible PID of its sandbox child. + pub(crate) fn child_pid(&mut self, child: &mut Child) -> Result { + set_nonblocking(self.info_reader.as_raw_fd())?; + let deadline = Instant::now() + STARTUP_TIMEOUT; + let mut json = Vec::new(); + let mut chunk = [0_u8; 512]; + + loop { + match self.info_reader.read(&mut chunk) { + Ok(0) => {} + Ok(count) => json.extend_from_slice(&chunk[..count]), + Err(error) if error.kind() == ErrorKind::WouldBlock => {} + Err(error) => { + return Err(format!( + "Bubblewrap: failed to read bwrap child information: {error}" + )); + } + } + + if let Ok(value) = serde_json::from_slice::(&json) { + if let Some(pid) = value.get("child-pid").and_then(|pid| pid.as_u64()) { + return u32::try_from(pid).map_err(|_| { + format!("Bubblewrap: bwrap reported an out-of-range child PID: {pid}") + }); + } + return Err(format!( + "Bubblewrap: bwrap child information omitted 'child-pid': {value}" + )); + } + + if let Some(status) = child + .try_wait() + .map_err(|error| format!("Bubblewrap: failed to inspect bwrap startup: {error}"))? + { + return Err(format!( + "Bubblewrap: bwrap exited before publishing child information ({status})" + )); + } + if Instant::now() >= deadline { + return Err("Bubblewrap: timed out waiting for bwrap child information".into()); + } + thread::sleep(Duration::from_millis(10)); + } + } + + /// Allow Bubblewrap to execute the workload after networking is ready. + pub(crate) fn release(mut self) -> Result<(), String> { + let writer = self + .gate_writer + .take() + .ok_or_else(|| "Bubblewrap: workload startup gate is already closed".to_string())?; + File::from(writer) + .write_all(&[1]) + .map_err(|error| format!("Bubblewrap: failed to release workload startup: {error}")) + } +} + +/// A same-UID user-namespace supervisor and its `slirp4netns` process. +pub(crate) struct ProxyNetworkNamespace { + state_dir: TempDir, + supervisor: Child, + exit_writer: Option, + userns: File, +} + +impl ProxyNetworkNamespace { + /// Create the capability-retaining namespace supervisor. + pub(crate) fn start(logger: &mut Logger) -> Result { + probe_dependencies()?; + + let state_dir = tempfile::Builder::new() + .prefix("mxc-bwrap-proxy-") + .tempdir() + .map_err(|error| { + format!("Bubblewrap: failed to create proxy-network state: {error}") + })?; + let stderr_path = state_dir.path().join("supervisor.stderr"); + 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}") + })?; + clear_cloexec(ready.as_raw_fd())?; + + let (exit_reader, exit_writer) = + pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; + clear_cloexec(exit_reader.as_raw_fd())?; + + let mut command = Command::new("unshare"); + command + .args([ + "--user", + "--map-current-user", + "--keep-caps", + "--", + "sh", + "-c", + SUPERVISOR_SCRIPT, + "mxc-bwrap-proxy-supervisor", + ]) + .arg(state_dir.path()) + .arg(ready.as_raw_fd().to_string()) + .arg(exit_reader.as_raw_fd().to_string()) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::from(stderr)); + + let mut supervisor = command.spawn().map_err(|error| { + format!("Bubblewrap: failed to start proxy-network supervisor: {error}") + })?; + drop(exit_reader); + drop(ready); + + if let Err(error) = wait_for_file( + state_dir.path().join("userns.ready"), + &mut supervisor, + &stderr_path, + "user namespace", + ) { + terminate_child(&mut supervisor); + return Err(error); + } + + let userns_path = format!("/proc/{}/ns/user", supervisor.id()); + let userns = match File::open(&userns_path) { + Ok(file) => file, + Err(error) => { + terminate_child(&mut supervisor); + return Err(format!( + "Bubblewrap: failed to open proxy user namespace {userns_path}: {error}" + )); + } + }; + clear_cloexec(userns.as_raw_fd())?; + logger.log_line("Bubblewrap: created rootless proxy network namespace supervisor"); + + Ok(Self { + state_dir, + supervisor, + exit_writer: Some(exit_writer), + userns, + }) + } + + /// Add the dynamic namespace and startup-barrier descriptors to bwrap. + pub(crate) fn configure_bwrap(&self, args: &mut Vec) -> Result { + let (info_reader, info_writer) = + pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; + let (gate_reader, gate_writer) = + pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; + clear_cloexec(info_writer.as_raw_fd())?; + clear_cloexec(gate_reader.as_raw_fd())?; + + let runtime_args = [ + "--userns".to_string(), + self.userns.as_raw_fd().to_string(), + "--info-fd".to_string(), + info_writer.as_raw_fd().to_string(), + "--block-fd".to_string(), + gate_reader.as_raw_fd().to_string(), + ]; + args.splice(0..0, runtime_args); + + Ok(BwrapStartup { + info_reader: File::from(info_reader), + info_writer: Some(info_writer), + gate_reader: Some(gate_reader), + gate_writer: Some(gate_writer), + }) + } + + /// Give the supervisor the Bubblewrap child PID and wait for slirp readiness. + pub(crate) fn attach(&mut self, child_pid: u32, logger: &mut Logger) -> Result<(), String> { + fs::write( + self.state_dir.path().join("child.pid"), + child_pid.to_string(), + ) + .map_err(|error| format!("Bubblewrap: failed to publish bwrap child PID: {error}"))?; + + wait_for_file( + self.state_dir.path().join("slirp.ready"), + &mut self.supervisor, + &self.state_dir.path().join("supervisor.stderr"), + "slirp4netns", + )?; + logger.log_line("Bubblewrap: slirp4netns configured the private proxy namespace"); + Ok(()) + } + + /// Stop slirp and reap the namespace supervisor. + pub(crate) fn stop(&mut self, logger: &mut Logger) { + self.exit_writer.take(); + let deadline = Instant::now() + SHUTDOWN_TIMEOUT; + loop { + match self.supervisor.try_wait() { + Ok(Some(_)) => 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", + ); + terminate_child(&mut self.supervisor); + return; + } + Err(error) => { + logger.log_line(&format!( + "WARNING: Bubblewrap: failed to inspect slirp4netns shutdown: {error}" + )); + terminate_child(&mut self.supervisor); + return; + } + } + } + } +} + +impl Drop for ProxyNetworkNamespace { + fn drop(&mut self) { + let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); + self.stop(&mut logger); + } +} + +/// 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 is_loopback = host.eq_ignore_ascii_case("localhost") + || host + .parse::() + .is_ok_and(|ip| ip.is_loopback()); + if !is_loopback { + return Ok(address.clone()); + } + + if let Some(original_url) = &address.original_url { + let mut url = url::Url::parse(original_url).map_err(|error| { + format!("Bubblewrap: failed to translate proxy URL for private networking: {error}") + })?; + url.set_host(Some(SLIRP_HOST_GATEWAY)).map_err(|_| { + "Bubblewrap: failed to translate proxy URL host for private networking".to_string() + })?; + return Ok(ProxyAddress::from_url( + url.as_str(), + SLIRP_HOST_GATEWAY.to_string(), + address.port(), + )); + } + + Ok(ProxyAddress::new( + SLIRP_HOST_GATEWAY.to_string(), + address.port(), + )) +} + +pub(crate) fn probe_dependencies() -> 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." + ) + })?; + if !slirp.status.success() { + return Err(format!( + "Bubblewrap: network.proxy requires a working slirp4netns installation \ + (slirp4netns --version exited with {})", + slirp.status + )); + } + + 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); + if !unshare.status.success() + || !help.contains("--map-current-user") + || !help.contains("--keep-caps") + { + return Err( + "Bubblewrap: proxy networking requires util-linux unshare with \ + --map-current-user and --keep-caps support" + .into(), + ); + } + Ok(()) +} + +fn wait_for_file( + path: impl AsRef, + child: &mut Child, + stderr_path: &std::path::Path, + component: &str, +) -> Result<(), String> { + let path = path.as_ref(); + let deadline = Instant::now() + STARTUP_TIMEOUT; + loop { + if fs::metadata(path).is_ok_and(|metadata| metadata.len() > 0) { + return Ok(()); + } + 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() + )); + } + if Instant::now() >= deadline { + return Err(format!( + "Bubblewrap: timed out waiting for {component} startup" + )); + } + thread::sleep(Duration::from_millis(10)); + } +} + +fn clear_cloexec(fd: RawFd) -> Result<(), String> { + fcntl(fd, FcntlArg::F_SETFD(FdFlag::empty())) + .map(|_| ()) + .map_err(|error| format!("Bubblewrap: failed to make descriptor inheritable: {error}")) +} + +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}"))?; + let flags = OFlag::from_bits_truncate(flags); + fcntl(fd, FcntlArg::F_SETFL(flags | OFlag::O_NONBLOCK)) + .map(|_| ()) + .map_err(|error| format!("Bubblewrap: failed to make descriptor nonblocking: {error}")) +} + +fn terminate_child(child: &mut Child) { + let _ = child.kill(); + let _ = child.wait(); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn translates_loopback_proxy_to_slirp_gateway() { + let address = ProxyAddress::new("127.0.0.1".into(), 8080); + let translated = sandbox_proxy_address(&address).unwrap(); + + assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); + assert_eq!(translated.port(), 8080); + assert_eq!(translated.to_url(), "http://10.0.2.2:8080"); + } + + #[test] + fn translates_loopback_url_without_losing_url_components() { + let address = ProxyAddress::from_url("http://localhost:3128/", "localhost".into(), 3128); + let translated = sandbox_proxy_address(&address).unwrap(); + + assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); + assert_eq!(translated.port(), 3128); + assert_eq!(translated.to_url(), "http://10.0.2.2:3128/"); + } + + #[test] + fn leaves_remote_proxy_unchanged() { + let address = + ProxyAddress::from_url("https://proxy.example:8443", "proxy.example".into(), 8443); + let translated = sandbox_proxy_address(&address).unwrap(); + + assert_eq!(translated.to_url(), address.to_url()); + } +} diff --git a/tests/configs/bubblewrap_network_proxy_namespace.json b/tests/configs/bubblewrap_network_proxy_namespace.json new file mode 100644 index 000000000..54494980d --- /dev/null +++ b/tests/configs/bubblewrap_network_proxy_namespace.json @@ -0,0 +1,12 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Proxy-Namespace", + "containment": "bubblewrap", + "process": { + "commandLine": "set -e; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); curl -fsSL https://api.github.com/zen > /dev/null; echo PROXY_NAMESPACE_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 908b70ec7..3f0d5bc4a 100644 --- a/tests/scripts/run_bwrap_network_proxy_test.sh +++ b/tests/scripts/run_bwrap_network_proxy_test.sh @@ -1,8 +1,8 @@ #!/bin/bash -# Bubblewrap network-proxy sandbox tests (cooperative env-var proxy). +# Bubblewrap network-proxy sandbox tests. # -# These tests do NOT require root: the builtin test proxy runs as the -# current user and the sandbox reaches it via loopback. +# These tests do NOT require root. Proxy mode uses a private network namespace +# with rootless slirp4netns routing to the host-side builtin proxy. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -41,4 +41,30 @@ run_one "builtin proxy" "bubblewrap_network_proxy_builtin.json" "PROXY_OK" run_one "proxy allowlist" "bubblewrap_network_proxy_allowlist.json" "BLOCKED_OK" run_one "proxy blocklist" "bubblewrap_network_proxy_blocklist.json" "BLOCKED_OK" +echo "Running Bubblewrap private proxy namespace test..." +HOST_NETNS="$(readlink /proc/self/ns/net)" +if ! NAMESPACE_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_proxy_namespace.json" 2>&1); then + echo "$NAMESPACE_OUT" + echo "FAIL: private proxy namespace (lxc-exec returned non-zero)" + exit 1 +fi +SANDBOX_NETNS="$(sed -n 's/^SANDBOX_NETNS=//p' <<<"$NAMESPACE_OUT" | tail -n 1)" +if [ -z "$SANDBOX_NETNS" ]; then + echo "$NAMESPACE_OUT" + echo "FAIL: private proxy namespace (namespace identity not reported)" + exit 1 +fi +if [ "$SANDBOX_NETNS" = "$HOST_NETNS" ]; then + echo "$NAMESPACE_OUT" + echo "FAIL: private proxy namespace (sandbox shares host network namespace)" + exit 1 +fi +if ! grep -q "PROXY_NAMESPACE_OK" <<<"$NAMESPACE_OUT"; then + echo "$NAMESPACE_OUT" + echo "FAIL: private proxy namespace (proxy request did not complete)" + exit 1 +fi +echo "PASS: private proxy namespace" + echo "Bubblewrap network proxy tests complete." From 2ea464b9a405334edb86b1913c51660007a21dc3 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 18 Aug 2026 11:19:12 -0700 Subject: [PATCH 3/3] Addressed PR comments --- .github/workflows/Build.Linux.Job.yml | 13 +- docs/bwrap-support/bubblewrap-backend.md | 41 +++- .../bubblewrap/common/src/bwrap_command.rs | 60 +++++ .../bubblewrap/common/src/bwrap_runner.rs | 28 ++- .../bubblewrap/common/src/proxy_network.rs | 221 +++++++++++++++--- .../bubblewrap_network_proxy_legacy.json | 12 + .../bubblewrap_network_proxy_namespace.json | 2 +- tests/scripts/run_bwrap_network_proxy_test.sh | 165 ++++++++++++- 8 files changed, 498 insertions(+), 44 deletions(-) create mode 100644 tests/configs/bubblewrap_network_proxy_legacy.json diff --git a/.github/workflows/Build.Linux.Job.yml b/.github/workflows/Build.Linux.Job.yml index 6988b0a0b..ee20e54b8 100644 --- a/.github/workflows/Build.Linux.Job.yml +++ b/.github/workflows/Build.Linux.Job.yml @@ -85,7 +85,7 @@ jobs: working-directory: ${{ github.workspace }} run: | sudo apt-get update - sudo apt-get install -y bubblewrap slirp4netns + sudo apt-get install -y bubblewrap slirp4netns iptables # Ubuntu 24.04 runners restrict unprivileged user namespaces via # AppArmor, which blocks `bwrap --unshare-user`. Relax it so the # sandbox can start (no-op on kernels without this knob). @@ -119,6 +119,17 @@ jobs: run: cargo build --locked --release --target ${{ matrix.target }} -p unix_test_proxy + # Exercises the real binary against the real dependencies: launches the + # slirp4netns supervisor, joins its user namespace, and asserts the + # sandbox lands in a private network namespace. Needs unix-test-proxy + # (builtinTestServer) alongside lxc-exec, which the two build steps + # above place in the same target directory. + - name: Test Bubblewrap proxy networking (end-to-end) + working-directory: ${{ github.workspace }} + env: + LXC_EXEC: ${{ github.workspace }}/src/target/${{ matrix.target }}/release/lxc-exec + run: bash tests/scripts/run_bwrap_network_proxy_test.sh + - name: Upload binaries uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index ff5597d26..d1fc2684d 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -330,6 +330,41 @@ request fails if its private namespace cannot be configured. ### Caveats +- **Host loopback moves to the gateway address (breaking change in 0.8+ + proxy mode)**: the sandbox gets its own network namespace, so inside it + `127.0.0.1` now means *the sandbox itself*, not the host. A config that + reaches a host-local service by loopback address — a database on + `127.0.0.1:5432`, a metadata endpoint, a second proxy — silently stops + connecting to the host and starts connecting to nothing. Under the schema + 0.6/0.7 legacy proxy path the sandbox shared the **host's own network + 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 + `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. +- **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 + workload — `bwrap` keeps it across its own `fork`/`exec` and offers no flag + to close it. Re-entering the namespace with `setns` requires + `CAP_SYS_ADMIN`, which the sandbox cannot hold: `bwrap` empties the + capability bounding set before `exec`, so the workload runs with + `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 @@ -382,9 +417,9 @@ resolution. |--------|-----|------------| | Privileges | Root required | Unprivileged (user namespaces) | | Rootfs | Downloads distro rootfs | Bind-mounts host filesystem | -| Startup | Create → Start → Attach | Single `bwrap` exec | -| Network isolation | iptables + veth | `--unshare-net` or iptables | -| Dependencies | `lxc-*` tools, templates | Single `bwrap` binary | +| Startup | Create → Start → Attach | Single `bwrap` exec (proxy mode adds a namespace supervisor) | +| 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 | **When to use Bubblewrap:** diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 3d6b68eaf..f0cc5aeb5 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -264,6 +264,11 @@ pub(crate) fn build_args_classified_with_mode( if !network_mode.uses_external_userns() { args.push("--unshare-user".into()); } + // SECURITY: proxy mode joins the supervisor's user namespace rather than + // unsharing, leaving that descriptor open in the workload. It is inert only + // because bwrap empties the capability sets before exec — asserted by + // run_bwrap_network_proxy_test.sh, explained in + // docs/bwrap-support/bubblewrap-backend.md. args.extend( ["--unshare-pid", "--unshare-ipc", "--unshare-uts"] .into_iter() @@ -389,6 +394,61 @@ mod tests { } } + #[test] + fn the_schema_gate_selects_the_private_namespace_only_from_0_8_onward() { + // The gate decides whether a proxy run gets the 0.8 private namespace + // or keeps the legacy shared-host-network behavior GHCP depends on, so + // its boundaries are pinned explicitly. + let cases = [ + ("0.6.0-alpha", ResolvedNetworkMode::LegacyProxy), + ("0.7.0-alpha", ResolvedNetworkMode::LegacyProxy), + ("0.7.99", ResolvedNetworkMode::LegacyProxy), + ("0.8.0", ResolvedNetworkMode::ProxyOnly), + ("0.8.0-alpha", ResolvedNetworkMode::ProxyOnly), + ("0.8.0-beta", ResolvedNetworkMode::ProxyOnly), + ("0.9.0", ResolvedNetworkMode::ProxyOnly), + ("0.10.0", ResolvedNetworkMode::ProxyOnly), + ("1.0.0", ResolvedNetworkMode::ProxyOnly), + ("2.1.0", ResolvedNetworkMode::ProxyOnly), + // Anything unparsable fails closed onto the legacy path: the old + // behavior is the compatible one, so an unreadable version must + // never silently opt a caller into the new namespace model. + ("", ResolvedNetworkMode::LegacyProxy), + ("0.8", ResolvedNetworkMode::ProxyOnly), + ("0", ResolvedNetworkMode::LegacyProxy), + ("0.8-beta", ResolvedNetworkMode::LegacyProxy), + ("v0.8.0", ResolvedNetworkMode::LegacyProxy), + ("not-a-version", ResolvedNetworkMode::LegacyProxy), + ]; + + for (version, expected) in cases { + let request = ExecutionRequest { + schema_version: version.into(), + ..base_request() + }; + assert_eq!( + ResolvedNetworkMode::from_request(&request, true), + expected, + "schema version {version:?} resolved to the wrong network mode" + ); + } + } + + #[test] + fn the_schema_gate_does_not_apply_when_no_proxy_is_active() { + // Without an active proxy the version is irrelevant — a 0.8 request + // still classifies on policy alone (default policy is block, so this + // lands on the plain isolated namespace, not the proxy one). + let request = ExecutionRequest { + schema_version: "0.8.0".into(), + ..base_request() + }; + assert_eq!( + ResolvedNetworkMode::from_request(&request, false), + ResolvedNetworkMode::Isolated + ); + } + #[test] fn basic_args_contain_namespace_flags() { let args = build_args(&base_request(), None); diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index 1c2aecd0e..a9600b25e 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -275,7 +275,7 @@ impl BubblewrapScriptRunner { Some(network) => match network.configure_bwrap(&mut args) { Ok(startup) => Some(startup), Err(error) => { - proxy_network.take(); + stop_proxy_network(&mut proxy_network, logger); proxy.stop(logger); return Err(ScriptResponse::error(&error)); } @@ -354,13 +354,16 @@ impl BubblewrapScriptRunner { if group { command.process_group(0); } + if let Some(startup) = network_startup.as_ref() { + startup.prepare_command(&mut command); + } let mut child = match command.spawn() { Ok(process) => process, Err(error) => { let mut fw_manager = fw_manager; cleanup_iptables(&mut fw_manager, logger); - proxy_network.take(); + stop_proxy_network(&mut proxy_network, logger); proxy.stop(logger); return Err(ScriptResponse::error(&format!( "Bubblewrap: failed to spawn bwrap: {}", @@ -371,6 +374,9 @@ impl BubblewrapScriptRunner { if let Some(mut startup) = network_startup.take() { startup.child_spawned(); + if let Some(network) = proxy_network.as_mut() { + network.userns_handed_off(); + } let startup_result = startup .child_pid(&mut child) .and_then(|child_pid| { @@ -388,7 +394,7 @@ impl BubblewrapScriptRunner { let _ = child.wait(); let mut fw_manager = fw_manager; cleanup_iptables(&mut fw_manager, logger); - proxy_network.take(); + stop_proxy_network(&mut proxy_network, logger); proxy.stop(logger); return Err(ScriptResponse::error(&error)); } @@ -412,7 +418,7 @@ impl BubblewrapScriptRunner { let _ = child.wait(); let mut fw_manager = fw_manager; cleanup_iptables(&mut fw_manager, logger); - proxy_network.take(); + stop_proxy_network(&mut proxy_network, logger); proxy.stop(logger); let error = out_result.err().or(err_result.err()); return Err(ScriptResponse::error(&format!( @@ -634,6 +640,20 @@ fn cleanup_iptables(manager: &mut Option, logger: &mut L } } +/// Tear down the proxy network namespace against the caller's logger. +/// +/// `Drop` would also stop it, but only through a throwaway in-memory logger, so +/// warnings about slirp needing forced termination are lost on exactly the +/// startup paths that already failed. +fn stop_proxy_network( + network: &mut Option, + logger: &mut Logger, +) { + if let Some(mut network) = network.take() { + network.stop(logger); + } +} + /// Outcome of resolving and classifying `deniedPaths` in a single pass. struct DeniedPlan { /// Rewritten denied-path list. `Some` only when at least one entry differs diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index 70470cfd6..157e300e1 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -6,7 +6,9 @@ use std::fs::{self, File, OpenOptions}; use std::io::{ErrorKind, Read, Write}; use std::os::fd::{AsRawFd, OwnedFd, RawFd}; +use std::os::unix::process::CommandExt; use std::process::{Child, Command, Stdio}; +use std::sync::OnceLock; use std::thread; use std::time::{Duration, Instant}; @@ -24,11 +26,20 @@ set -eu state_dir="$1" ready_fd="$2" exit_fd="$3" +pid_fd="$4" printf ready > "$state_dir/userns.ready" -while [ ! -s "$state_dir/child.pid" ]; do - sleep 0.01 -done -child_pid="$(cat "$state_dir/child.pid")" +# 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 +exec 3<&- +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 @@ -40,9 +51,18 @@ pub(crate) struct BwrapStartup { info_writer: Option, gate_reader: Option, gate_writer: Option, + /// Descriptors bwrap must inherit, cleared of `FD_CLOEXEC` in the child + /// only. See [`inherit_descriptors`]. + inheritable: Vec, } impl BwrapStartup { + /// Arrange for bwrap -- and only bwrap -- to inherit the startup + /// descriptors. + pub(crate) fn prepare_command(&self, command: &mut Command) { + inherit_descriptors(command, self.inheritable.clone()); + } + /// Close the parent copies of the descriptors inherited by Bubblewrap. pub(crate) fn child_spawned(&mut self) { self.info_writer.take(); @@ -111,14 +131,20 @@ pub(crate) struct ProxyNetworkNamespace { state_dir: TempDir, supervisor: Child, exit_writer: Option, - userns: File, + /// Write end of the pipe carrying the sandbox PID to the supervisor. + /// Dropping it without writing ends the supervisor's wait at EOF. + pid_writer: Option, + /// Handle to the supervisor's user namespace, passed to bwrap as + /// `--userns`. Released once bwrap owns it; see [`Self::userns_handed_off`]. + userns: Option, } impl ProxyNetworkNamespace { /// Create the capability-retaining namespace supervisor. + /// + /// 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 { - probe_dependencies()?; - let state_dir = tempfile::Builder::new() .prefix("mxc-bwrap-proxy-") .tempdir() @@ -136,11 +162,11 @@ impl ProxyNetworkNamespace { .map_err(|error| { format!("Bubblewrap: failed to create slirp readiness file: {error}") })?; - clear_cloexec(ready.as_raw_fd())?; let (exit_reader, exit_writer) = pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; - clear_cloexec(exit_reader.as_raw_fd())?; + let (pid_reader, pid_writer) = + pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; let mut command = Command::new("unshare"); command @@ -157,14 +183,26 @@ impl ProxyNetworkNamespace { .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()) .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( + &mut command, + vec![ + ready.as_raw_fd(), + exit_reader.as_raw_fd(), + pid_reader.as_raw_fd(), + ], + ); let mut supervisor = command.spawn().map_err(|error| { format!("Bubblewrap: failed to start proxy-network supervisor: {error}") })?; drop(exit_reader); + drop(pid_reader); drop(ready); if let Err(error) = wait_for_file( @@ -187,14 +225,14 @@ impl ProxyNetworkNamespace { )); } }; - clear_cloexec(userns.as_raw_fd())?; logger.log_line("Bubblewrap: created rootless proxy network namespace supervisor"); Ok(Self { state_dir, supervisor, exit_writer: Some(exit_writer), - userns, + pid_writer: Some(pid_writer), + userns: Some(userns), }) } @@ -204,12 +242,15 @@ impl ProxyNetworkNamespace { pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; let (gate_reader, gate_writer) = pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; - clear_cloexec(info_writer.as_raw_fd())?; - clear_cloexec(gate_reader.as_raw_fd())?; + + let userns = self + .userns + .as_ref() + .ok_or_else(|| "Bubblewrap: proxy user namespace is already handed off".to_string())?; let runtime_args = [ "--userns".to_string(), - self.userns.as_raw_fd().to_string(), + userns.as_raw_fd().to_string(), "--info-fd".to_string(), info_writer.as_raw_fd().to_string(), "--block-fd".to_string(), @@ -218,6 +259,11 @@ impl ProxyNetworkNamespace { args.splice(0..0, runtime_args); Ok(BwrapStartup { + inheritable: vec![ + userns.as_raw_fd(), + info_writer.as_raw_fd(), + gate_reader.as_raw_fd(), + ], info_reader: File::from(info_reader), info_writer: Some(info_writer), gate_reader: Some(gate_reader), @@ -225,13 +271,27 @@ impl ProxyNetworkNamespace { }) } + /// Drop this process's handle to the user namespace once bwrap holds it. + /// + /// The namespace itself stays alive through the supervisor, which is a + /// member of it. Releasing here bounds the window in which a concurrent + /// spawn could pick the descriptor up to the bwrap spawn itself, rather + /// than the whole sandbox lifetime. + pub(crate) fn userns_handed_off(&mut self) { + self.userns.take(); + } + /// Give the supervisor the Bubblewrap child PID and wait for slirp readiness. pub(crate) fn attach(&mut self, child_pid: u32, logger: &mut Logger) -> Result<(), String> { - fs::write( - self.state_dir.path().join("child.pid"), - child_pid.to_string(), - ) - .map_err(|error| format!("Bubblewrap: failed to publish bwrap child PID: {error}"))?; + let mut writer = self + .pid_writer + .take() + .map(File::from) + .ok_or_else(|| "Bubblewrap: sandbox PID was already published".to_string())?; + writer + .write_all(format!("{child_pid}\n").as_bytes()) + .map_err(|error| format!("Bubblewrap: failed to publish bwrap child PID: {error}"))?; + drop(writer); wait_for_file( self.state_dir.path().join("slirp.ready"), @@ -245,6 +305,9 @@ impl ProxyNetworkNamespace { /// Stop slirp and reap the namespace supervisor. pub(crate) fn stop(&mut self, logger: &mut Logger) { + // Release the PID pipe too: a supervisor still waiting for the sandbox + // PID sees EOF and exits rather than lingering. + self.pid_writer.take(); self.exit_writer.take(); let deadline = Instant::now() + SHUTDOWN_TIMEOUT; loop { @@ -282,11 +345,14 @@ 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 is_loopback = host.eq_ignore_ascii_case("localhost") + // `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. + let is_host_local = host.eq_ignore_ascii_case("localhost") || host .parse::() - .is_ok_and(|ip| ip.is_loopback()); - if !is_loopback { + .is_ok_and(|ip| ip.is_loopback() || ip.is_unspecified()); + if !is_host_local { return Ok(address.clone()); } @@ -311,6 +377,21 @@ pub(crate) fn sandbox_proxy_address(address: &ProxyAddress) -> Result Result<(), String> { + // Probing costs two 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 + // long after they did. + static PROBED: OnceLock<()> = OnceLock::new(); + if PROBED.get().is_some() { + return Ok(()); + } + probe_dependencies_uncached()?; + let _ = PROBED.set(()); + Ok(()) +} + +fn probe_dependencies_uncached() -> Result<(), String> { let slirp = Command::new("slirp4netns") .arg("--version") .output() @@ -370,18 +451,44 @@ fn wait_for_file( )); } 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" + "Bubblewrap: timed out waiting for {component} startup after {STARTUP_TIMEOUT:?} \ + ({detail})" )); } thread::sleep(Duration::from_millis(10)); } } -fn clear_cloexec(fd: RawFd) -> Result<(), String> { - fcntl(fd, FcntlArg::F_SETFD(FdFlag::empty())) - .map(|_| ()) - .map_err(|error| format!("Bubblewrap: failed to make descriptor inheritable: {error}")) +/// Hand `fds` to one specific child, without exposing them process-wide. +/// +/// `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. Clearing it in the forked child +/// instead gives them to the intended child and no one else. +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 + // this closure allocates nothing -- `fds` is captured by move and holds + // plain integers. + unsafe { + command.pre_exec(move || { + for fd in &fds { + fcntl(*fd, FcntlArg::F_SETFD(FdFlag::empty())).map_err(std::io::Error::from)?; + } + Ok(()) + }); + } } fn set_nonblocking(fd: RawFd) -> Result<(), String> { @@ -430,4 +537,64 @@ mod tests { assert_eq!(translated.to_url(), address.to_url()); } + + /// A proxy bound to the wildcard address is reachable on the host's + /// loopback, which the sandbox's private namespace cannot see, so it needs + /// the same gateway rewrite `127.0.0.1` gets. + #[test] + fn translates_wildcard_proxy_to_slirp_gateway() { + let address = ProxyAddress::new("0.0.0.0".into(), 8080); + let translated = sandbox_proxy_address(&address).unwrap(); + + assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); + assert_eq!(translated.port(), 8080); + } + + #[test] + fn translates_bracketed_ipv6_wildcard_proxy_to_slirp_gateway() { + let address = ProxyAddress::from_url("http://[::]:3128/", "[::]".into(), 3128); + let translated = sandbox_proxy_address(&address).unwrap(); + + assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); + assert_eq!(translated.port(), 3128); + assert_eq!(translated.to_url(), "http://10.0.2.2:3128/"); + } + + /// The descriptor must reach the child that was prepared and no other. The + /// obvious alternative -- clearing `FD_CLOEXEC` on the parent's copy -- + /// passes the first assertion and fails the other two. + #[test] + fn inherited_descriptors_reach_only_the_prepared_child() { + let (reader, _writer) = pipe2(OFlag::O_CLOEXEC).expect("pipe"); + let fd = reader.as_raw_fd(); + let probe = format!("test -e /proc/self/fd/{fd} && echo present || echo absent"); + + let mut prepared = Command::new("sh"); + prepared.arg("-c").arg(&probe); + inherit_descriptors(&mut prepared, vec![fd]); + let prepared_out = prepared.output().expect("spawn prepared child"); + + let bystander = Command::new("sh") + .arg("-c") + .arg(&probe) + .output() + .expect("spawn bystander child"); + + assert_eq!( + String::from_utf8_lossy(&prepared_out.stdout).trim(), + "present", + "the prepared child did not inherit the descriptor" + ); + assert_eq!( + String::from_utf8_lossy(&bystander.stdout).trim(), + "absent", + "an unrelated child inherited the descriptor" + ); + + let flags = FdFlag::from_bits_truncate(fcntl(fd, FcntlArg::F_GETFD).expect("F_GETFD")); + assert!( + flags.contains(FdFlag::FD_CLOEXEC), + "the parent's own descriptor was left inheritable" + ); + } } diff --git a/tests/configs/bubblewrap_network_proxy_legacy.json b/tests/configs/bubblewrap_network_proxy_legacy.json new file mode 100644 index 000000000..74d2504bc --- /dev/null +++ b/tests/configs/bubblewrap_network_proxy_legacy.json @@ -0,0 +1,12 @@ +{ + "version": "0.7.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Proxy-Legacy", + "containment": "bubblewrap", + "process": { + "commandLine": "set -e; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); echo SANDBOX_PROXY=$HTTP_PROXY; curl -fsSL https://api.github.com/zen > /dev/null; echo LEGACY_PROXY_OK" + }, + "network": { + "defaultPolicy": "allow", + "proxy": { "builtinTestServer": true } + } +} diff --git a/tests/configs/bubblewrap_network_proxy_namespace.json b/tests/configs/bubblewrap_network_proxy_namespace.json index 54494980d..c0d2a5f8e 100644 --- a/tests/configs/bubblewrap_network_proxy_namespace.json +++ b/tests/configs/bubblewrap_network_proxy_namespace.json @@ -3,7 +3,7 @@ "containerId": "CLI-Bubblewrap-Network-Proxy-Namespace", "containment": "bubblewrap", "process": { - "commandLine": "set -e; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); curl -fsSL https://api.github.com/zen > /dev/null; echo PROXY_NAMESPACE_OK" + "commandLine": "set -e; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); echo SANDBOX_CAPBND=$(grep ^CapBnd: /proc/self/status | cut -f2); echo SANDBOX_CAPEFF=$(grep ^CapEff: /proc/self/status | cut -f2); echo SANDBOX_CAPPRM=$(grep ^CapPrm: /proc/self/status | cut -f2); curl -fsSL https://api.github.com/zen > /dev/null; echo PROXY_NAMESPACE_OK" }, "network": { "defaultPolicy": "allow", diff --git a/tests/scripts/run_bwrap_network_proxy_test.sh b/tests/scripts/run_bwrap_network_proxy_test.sh index 3f0d5bc4a..fe0876983 100644 --- a/tests/scripts/run_bwrap_network_proxy_test.sh +++ b/tests/scripts/run_bwrap_network_proxy_test.sh @@ -7,15 +7,25 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" REPO_DIR="$(dirname "$(dirname "$SCRIPT_DIR")")" -LXC_EXEC="$REPO_DIR/src/target/release/lxc-exec" - -if [ ! -f "$LXC_EXEC" ]; then - LXC_EXEC="$REPO_DIR/src/target/debug/lxc-exec" -fi +# CI builds to a target-triple subdirectory, so allow the caller to point at a +# specific binary instead of guessing. An explicitly set LXC_EXEC is taken +# literally: falling back from it would silently exercise a different binary +# than the caller named -- a stale debug build passing while release is broken. +if [ -n "${LXC_EXEC:-}" ]; then + if [ ! -f "$LXC_EXEC" ]; then + echo "Error: LXC_EXEC is set to '$LXC_EXEC', which does not exist." + exit 1 + fi +else + LXC_EXEC="$REPO_DIR/src/target/release/lxc-exec" + if [ ! -f "$LXC_EXEC" ]; then + LXC_EXEC="$REPO_DIR/src/target/debug/lxc-exec" + fi -if [ ! -f "$LXC_EXEC" ]; then - echo "Error: lxc-exec not found. Run build.sh first." - exit 1 + if [ ! -f "$LXC_EXEC" ]; then + echo "Error: lxc-exec not found. Run build.sh first." + exit 1 + fi fi run_one() { @@ -67,4 +77,143 @@ if ! grep -q "PROXY_NAMESPACE_OK" <<<"$NAMESPACE_OUT"; then fi echo "PASS: private proxy namespace" +# Proxy mode has bwrap join the supervisor's user namespace instead of creating +# its own, and that namespace descriptor stays open in the workload (bwrap keeps +# it across its own fork+exec and upstream offers no way to close it). Re-entering +# it via setns requires CAP_SYS_ADMIN, so containment rests entirely on bwrap +# emptying the capability sets before exec. Assert that here: if a future bwrap +# ever leaves a non-empty bounding set, proxy mode must stop sharing the +# supervisor's user namespace, and this test is what catches it. +echo "Running Bubblewrap proxy-namespace capability drop test..." +for cap_field in CAPBND CAPEFF CAPPRM; do + cap_value="$(sed -n "s/^SANDBOX_${cap_field}=//p" <<<"$NAMESPACE_OUT" | tail -n 1)" + if [ -z "$cap_value" ]; then + echo "$NAMESPACE_OUT" + echo "FAIL: capability drop ($cap_field not reported by the sandbox)" + exit 1 + fi + if [ "$cap_value" != "0000000000000000" ]; then + echo "$NAMESPACE_OUT" + echo "FAIL: capability drop ($cap_field is $cap_value, expected 0000000000000000)" + exit 1 + fi +done +echo "PASS: proxy-namespace capability drop" + +# Schema <= 0.7 must keep the pre-0.8 proxy behavior: GHCP consumes Bubblewrap +# proxy mode on 0.6/0.7, so the private-namespace work must be invisible there. +# The unit tests only inspect generated arguments; this runs the legacy path for +# real and asserts all three properties that make it compatible. +# +# slirp4netns is deliberately shadowed with a failing stub for this case: the +# legacy path must neither probe nor use it, so the run succeeding with a broken +# slirp4netns is what proves the dependency is 0.8-only. +echo "Running Bubblewrap legacy (schema 0.7) proxy compatibility test..." +STUB_DIR="$(mktemp -d)" +trap 'rm -rf "$STUB_DIR"' EXIT +printf '#!/bin/sh\necho "slirp4netns must not be used on the legacy proxy path" >&2\nexit 1\n' \ + > "$STUB_DIR/slirp4netns" +chmod +x "$STUB_DIR/slirp4netns" + +if ! LEGACY_OUT=$(PATH="$STUB_DIR:$PATH" "$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_proxy_legacy.json" 2>&1); then + echo "$LEGACY_OUT" + echo "FAIL: legacy proxy (lxc-exec returned non-zero)" + exit 1 +fi + +LEGACY_NETNS="$(sed -n 's/^SANDBOX_NETNS=//p' <<<"$LEGACY_OUT" | tail -n 1)" +if [ "$LEGACY_NETNS" != "$HOST_NETNS" ]; then + echo "$LEGACY_OUT" + echo "FAIL: legacy proxy (expected the host network namespace $HOST_NETNS, got $LEGACY_NETNS)" + exit 1 +fi + +LEGACY_PROXY="$(sed -n 's/^SANDBOX_PROXY=//p' <<<"$LEGACY_OUT" | tail -n 1)" +case "$LEGACY_PROXY" in + *127.0.0.1*) ;; + *) + echo "$LEGACY_OUT" + echo "FAIL: legacy proxy (proxy address was rewritten away from loopback: $LEGACY_PROXY)" + exit 1 + ;; +esac + +if ! grep -q "LEGACY_PROXY_OK" <<<"$LEGACY_OUT"; then + echo "$LEGACY_OUT" + echo "FAIL: legacy proxy (proxied request did not complete)" + exit 1 +fi +echo "PASS: legacy (schema 0.7) proxy compatibility" + +# The supervisor blocks on a parent-owned pipe waiting for the sandbox PID. If +# the executor dies in that window the read must hit EOF and the supervisor must +# exit; the earlier file-polling loop had no exit condition and leaked a process +# holding a live user namespace. +# +# bwrap is shadowed with a stub that never reports a PID, which holds the +# executor in its startup wait and widens that window from microseconds to +# seconds so the kill lands inside it deterministically. +echo "Running Bubblewrap supervisor orphan-reaping test..." +BWRAP_STUB_DIR="$(mktemp -d)" +trap 'rm -rf "$STUB_DIR" "$BWRAP_STUB_DIR"' EXIT +cat > "$BWRAP_STUB_DIR/bwrap" < "$BWRAP_STUB_DIR/stub.pid" +exec sleep 300 +STUB +chmod +x "$BWRAP_STUB_DIR/bwrap" + +SUPERVISOR_PATTERN="mxc-bwrap-proxy-supervisor" +PATH="$BWRAP_STUB_DIR:$PATH" "$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_proxy_namespace.json" >/dev/null 2>&1 & +ORPHAN_EXEC_PID=$! + +SUPERVISOR_SEEN=0 +for _ in $(seq 1 100); do + if pgrep -f "$SUPERVISOR_PATTERN" >/dev/null 2>&1; then + SUPERVISOR_SEEN=1 + break + fi + sleep 0.05 +done + +if [ "$SUPERVISOR_SEEN" -ne 1 ]; then + kill -9 "$ORPHAN_EXEC_PID" 2>/dev/null || true + wait "$ORPHAN_EXEC_PID" 2>/dev/null || true + pkill -f "$SUPERVISOR_PATTERN" 2>/dev/null || true + echo "FAIL: supervisor orphan reaping (the supervisor never started)" + exit 1 +fi + +kill -9 "$ORPHAN_EXEC_PID" 2>/dev/null || true +wait "$ORPHAN_EXEC_PID" 2>/dev/null || true + +SUPERVISOR_REAPED=0 +for _ in $(seq 1 100); do + if ! pgrep -f "$SUPERVISOR_PATTERN" >/dev/null 2>&1; then + SUPERVISOR_REAPED=1 + break + fi + sleep 0.05 +done + +# The stub bwrap outlives the SIGKILLed executor by design; the real backend +# runs it as pid 1 of a pid namespace, so only this stub needs reaping. It +# records its own pid because it `exec`s sleep, leaving nothing for pkill to +# match on its command line. +if [ -f "$BWRAP_STUB_DIR/stub.pid" ]; then + kill -9 "$(cat "$BWRAP_STUB_DIR/stub.pid")" 2>/dev/null || true +fi + +if [ "$SUPERVISOR_REAPED" -ne 1 ]; then + pkill -f "$SUPERVISOR_PATTERN" 2>/dev/null || true + echo "FAIL: supervisor orphan reaping (the supervisor survived the executor)" + exit 1 +fi +echo "PASS: supervisor orphan reaping" + echo "Bubblewrap network proxy tests complete."