diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index 7c57bd54a..4b280e5fc 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -27,10 +27,20 @@ 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, 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. +- **Schema 0.8 private-namespace modes:** `slirp4netns` installed and on PATH, + plus `nsenter`, `iptables`, and `ip6tables` for the egress rules. These are + needed by **both** 0.8 modes that build a private network namespace — proxy + mode (`network.proxy`) and firewall enforcement + (`enforcementMode: "firewall"`), which share the same slirp-backed namespace + and the same dependency probe. None are required when neither applies: a + policy with no `network.proxy` and no 0.8 firewall enforcement, or a 0.6/0.7 + policy using the legacy proxy and host-rule behavior. + + > `ip6tables` is required to *deny* IPv6, not to carry it. slirp4netns is + > launched without `--enable-ipv6`, so the sandbox namespace has no IPv6 + > connectivity at all and the v6 rules exist to keep the unmatched family + > closed. An IPv6 destination is unreachable even when a rule allows it + > (see #955). ```bash # Debian/Ubuntu sudo apt install slirp4netns util-linux iptables @@ -54,7 +64,7 @@ requiring root privileges or a container runtime. ``` (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 + Both modes fail explicitly if any of these is unavailable; neither ever 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 @@ -213,28 +223,84 @@ reach in. Runs fully unprivileged. } ``` -**Per-host filtering** (`allowedHosts`/`blockedHosts`) — shares the host -network namespace and applies iptables rules via `NetworkIptablesManager` -(the same approach used by the LXC backend). **Requires root** for -iptables. - -> **IPv4 only.** Host names are resolved to IPv4 addresses only; AAAA -> records and IPv6 literals are silently dropped because `iptables` (the -> IPv4 tool) cannot accept IPv6 destinations. A host with only AAAA -> records is effectively unreachable under firewall mode. For dual-stack -> hosts, use proxy mode (below) instead. +**Per-host filtering** (`allowedHosts`/`blockedHosts`) — the behavior +depends on the schema version, because 0.8 replaced a path that did not +actually filter. + +**Schema 0.8+ — enforced.** `enforcementMode: "firewall"` puts the sandbox in +the same private, slirp-backed network namespace proxy mode uses, and programs +the rules into *that* namespace from a supervisor holding `CAP_NET_ADMIN` +inside an unprivileged user namespace. **No root required**, and the sandbox +cannot undo the rules: it drops `CAP_NET_ADMIN` before the workload starts. + +Rule addresses must be **IP literals or CIDR blocks**; a DNS name is rejected +at validation time rather than resolved on the caller's behalf. The backend +does not resolve, because the sandbox resolves names itself and a lookup that +disagreed with the one behind the rules would hand the workload an address the +chain never authorized. An IPv6 rule programs `ip6tables`, but the sandbox's +namespace has no IPv6 connectivity today — slirp4netns is launched without +`--enable-ipv6` — so an allowed IPv6 destination stays unreachable regardless +of the rule (see #955). The terminal verdict of the unmatched family still +follows `defaultPolicy`, so a v4-only allowlist under `block` does not leave +IPv6 open. + +An IPv4-mapped address such as `::ffff:203.0.113.5` is programmed as IPv4: +Linux puts a genuine IPv4 packet on the wire for one, so an `ip6tables` rule +naming it would never match and a `blockedHosts` entry under `defaultPolicy: +allow` would fail open. A mapped CIDR is translated the same way — the mapped +range is the last 32 bits of `::ffff:0:0/96`, so a `/96 + n` prefix becomes a +v4 `/n`. + +An IPv6 block **shorter** than `/96` that contains `::ffff:0:0/96` is +**rejected** rather than programmed. CIDR blocks nest or are disjoint, so such a +block always swallows the mapped range whole, and neither available reading is +safe to apply silently: leaving it on `ip6tables` unenforces the mapped half +(the same fail-open the translation above exists to prevent), while projecting +it onto IPv4 would always widen it to `0.0.0.0/0` — turning `blockedHosts: +["::/0"]` from "block all IPv6" into "block all IPv4 as well". The rejection +asks the caller to write the IPv4 side explicitly. Blocks that do not contain +the mapped range, such as `2001:db8::/32`, are unaffected. + +> **Divergence from LXC.** LXC normalizes mapped literals and `/96`-or-longer +> mapped CIDRs the same way, so those policies mean the same thing on both +> backends. It does **not** yet reject the shorter straddling blocks — there, +> such a rule stays on `ip6tables` and its mapped half goes unenforced. Until +> LXC adopts the same check, a policy using one of those blocks is the one case +> where the two backends differ. + +An explicit `blockedHosts` entry outranks any `allowedHosts` entry that covers +it, including a broader CIDR: denies are installed ahead of allows in a +first-match chain. ```json { "network": { "defaultPolicy": "block", "enforcementMode": "firewall", - "allowedHosts": ["api.github.com"], - "blockedHosts": ["evil.example.com"] + "allowedHosts": ["10.0.2.2/32", "203.0.113.0/24"], + "blockedHosts": ["10.0.2.2"] } } ``` +**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares +the host network namespace and applies rules to the *host* via +`NetworkIptablesManager`, which **requires root**. Unprivileged, the request +fails closed rather than running unfiltered: `spawn_bwrap` returns the +`apply_firewall_rules` error before `bwrap` is spawned, so no workload runs. +The unenforced case is the *privileged* one — the rules install on the host's +chains, which the sandbox does not traverse, so they filter nothing while +appearing to succeed. This is retained unchanged for existing callers and is +the reason enforcement is 0.8+ only. Names are accepted on this path and +resolved to IPv4 only. + +> **Legacy path, IPv4 only.** On schema ≤ 0.7, host names are resolved to +> IPv4 addresses only; AAAA records and IPv6 literals are silently dropped +> because `iptables` (the IPv4 tool) cannot accept IPv6 destinations. A host +> with only AAAA records is effectively unreachable. Moving to 0.8 programs +> `ip6tables` as well, but does not make such a host reachable while the +> sandbox namespace has no IPv6 (see #955); use proxy mode (below) instead. + **Full allow** (`defaultPolicy: "allow"`, no host lists) — the sandbox shares the host network namespace with no restrictions. @@ -252,16 +318,28 @@ namespace choice alone decides the outcome: | `allowLocalNetwork` | Namespace | Result | |---------------------|-----------|--------| -| `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` (default) | private (`--unshare-net`; isolated, plus 0.8 proxy and firewall modes) | 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 | -Rows 2 and 3 emit a `WARNING:` line to the runner log at preflight rather -than failing silently. Windows (AppContainer's `privateNetworkClientServer` +Rows 2 and 3 are rejected on schema `0.8.0-alpha` and later — in the backend's +validation, which every caller passes through, so a programmatic +`ExecutionRequest` is refused just like a JSON config — and +emit a +`WARNING:` line to the runner log at preflight on earlier schemas rather than +failing silently. Windows (AppContainer's `privateNetworkClientServer` capability) and macOS (Seatbelt's `(allow network-inbound (local ip))`) enforce the field at the syscall level; this divergence is Linux-specific. +Row 2 is keyed on the value, not on whether the caller wrote the field. +`false` is the schema's default *and* a deny, so an omitted `allowLocalNetwork` +is still a request for inbound denial and is rejected the same way: a bare +`defaultPolicy: "allow"` does not silently opt out of the deny it inherits. +Callers who want the shared namespace acknowledge the exposure with +`allowLocalNetwork: true` (row 4), the same acknowledgment IsolationSession +requires for this field. + ### Process Settings Standard `process` fields work as expected: @@ -459,9 +537,10 @@ request fails if its private namespace cannot be configured. - IPv6-only proxy hostnames are rejected, matching the IPv6 egress denial. - **Mutually exclusive with iptables enforcement**: setting `network.proxy` together with `network.enforcementMode` of `"firewall"` - 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.) + or `"both"` is rejected at config-parse time. Both postures build the same + private namespace, so the combination is ambiguous rather than impossible — + it is refused because there is no defined precedence between an endpoint pin + and a rule list, not because of any privilege requirement. - **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 @@ -499,9 +578,9 @@ 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 user/network-namespace supervisor, a `slirp4netns` instance and an egress rule set | +| Startup | Create → Start → Attach | Single `bwrap` exec; the 0.8 private-namespace modes (proxy and firewall enforcement) add 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`, util-linux `unshare` and `nsenter`, plus `iptables` and `ip6tables` on the `nf_tables` backend | +| Dependencies | `lxc-*` tools, templates | `bwrap`; the 0.8 private-namespace modes also need `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:** @@ -536,10 +615,12 @@ Test configs are in `tests/configs/bubblewrap_*.json`. and `/usr/local` are invisible unless explicitly listed in `readonlyPaths` / `readwritePaths`. There is no separate rootfs — the visible paths are bind-mounted from the host. -- **Network filtering** — per-host `allowedHosts`/`blockedHosts` is best - done via the cooperative env-var **network proxy** (no privilege - required, see above). The legacy iptables path - (`network.enforcementMode: "firewall"` / `"both"`) still works but - requires root and is mutually exclusive with the proxy. +- **Network filtering** — per-host `allowedHosts`/`blockedHosts` is enforced + natively on schema 0.8+ via `network.enforcementMode: "firewall"`, with + **no privilege required** (rules are programmed inside the sandbox's own + namespace; addresses must be IP literals or CIDRs). The cooperative env-var + **network proxy** remains the option when you need name-based rules. On + schema ≤ 0.7 the firewall path targets the *host* and requires root; it is + retained for compatibility but does not filter unprivileged. - **No state-aware lifecycle** — Bubblewrap implements `ScriptRunner` only (one-shot), not `StatefulSandboxBackend` diff --git a/docs/sandbox-policy/0.8.0/networking/networking.md b/docs/sandbox-policy/0.8.0/networking/networking.md index 147661277..443cda655 100644 --- a/docs/sandbox-policy/0.8.0/networking/networking.md +++ b/docs/sandbox-policy/0.8.0/networking/networking.md @@ -76,7 +76,7 @@ Throughout this document, the deny-all-except-proxy posture (the GA goal) refers - **When allowed:** Only when explicitly allowed by IP/CIDR + port + protocol rules in `egress.allow`. In model 2 there is no direct egress path, so these connections are not possible regardless of any allow rules. - **Use case:** e.g. SSH to a specific dev server, a direct TCP connection to a database, UDP to a specific endpoint, ICMP for diagnostics. - **Caveat (coarse filtering):** Rules match IP/CIDR + port + protocol, not the application protocol. A port number does not identify a service (DNS need not use 53; a database may listen on any port), so allowing or denying a port is a blunt control rather than service-level filtering. -- **Enforcement:** WFP filters (Windows process containers), network namespace + iptables (WSLc/LXC/Bubblewrap). Model 1 for macOS is not supported for GA. Seatbelt cannot filter arbitrary destinations and macOS packet filtering is not fine-grained enough for per-sandbox scenarios out of the box. +- **Enforcement:** WFP filters (Windows process containers), network namespace + iptables (WSLc/LXC/Bubblewrap). Model 1 for macOS is not supported for GA. Seatbelt cannot filter arbitrary destinations and macOS packet filtering is not fine-grained enough for per-sandbox scenarios out of the box. On Bubblewrap, model 1 egress is enforced from schema 0.8+ inside the sandbox's own network namespace — see "LXC and Bubblewrap: GA enforcement" below for what is and is not covered there. **Example:** @@ -269,6 +269,8 @@ sockets come with their own security questions and should be outlined in a separ **Elevation caveat:** Installing these filters (WFP on Windows, iptables on the Linux backends) generally requires elevation. Elevating on every sandbox launch is out of the question, so MXC applies them through a privileged broker/service rather than from the unelevated launch path. A per-platform, per-technology elevation story must be defined in a separate MXC elevation design doc and is a prerequisite for this enforcement. +> **Bubblewrap does not need this.** The caveat above holds only for filters installed on the *host*. Bubblewrap gives the sandbox its own network namespace and programs iptables inside it from a supervisor holding `CAP_NET_ADMIN` in an unprivileged user namespace — namespace-scoped capabilities are not host privilege, so the rules install with no elevation and no broker. The sandbox still cannot alter them, because it drops `CAP_NET_ADMIN` before the workload runs. Shipped at schema 0.8; see `docs/bwrap-support/bubblewrap-backend.md`. The elevation design remains a prerequisite for the host-filter backends only. + ### D3: IP literals and CIDRs only (no DNS names) **Decision:** Rule addresses must be IPv4/IPv6 literals or CIDRs. DNS names are rejected at validation time. The backend does not resolve names on behalf of callers. @@ -404,6 +406,21 @@ LXC and Bubblewrap use iptables/nftables on the container network path. Their IN and the host-to-container half of `ingress.hostLoopback`; routing and output policy enforce its container-to-host half. Model 2 permits only the proxy endpoint. +> **Implementation status (Bubblewrap).** Egress is enforced from schema 0.8+, +> but not on the path described above. Unprivileged Bubblewrap has no host-side +> veth, so no chain can be hooked into `FORWARD`. Instead the sandbox gets its +> own network namespace and a supervisor holding namespace-scoped +> `CAP_NET_ADMIN` filters on `OUTPUT` inside it (see the D2 note above). Rule +> addresses must be IP literals or CIDRs per D3; a hostname is rejected. +> Model 2 is enforced by the same chain, which opens only the proxy endpoint. +> +> Not yet covered: `ingress.default` and `ingress.hostLoopback` are parsed but +> not enforced as an `INPUT` policy. Inbound is instead a property of the +> namespace — no port forwarding is configured, so nothing outside the sandbox +> can reach in. Schema 0.6/0.7 keeps the previous warn-and-continue behavior. +> LXC is unaffected: it has a veth and runs privileged, and enforces as +> described. + ### macOS (Seatbelt): GA enforcement **Default stance:** Egress is confined to the loopback proxy port by the Seatbelt profile. diff --git a/docs/schema.md b/docs/schema.md index 9b2dcc958..19e28a7a1 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -113,6 +113,13 @@ cannot mix both formats in one request. "network": { "defaultPolicy": "block", // "allow" or "block" "enforcementMode": "firewall", // "capabilities", "firewall", or "both" + "allowedHosts": ["203.0.113.0/24"], + "blockedHosts": ["203.0.113.7"], // Denies outrank allows, including broader CIDRs + // Under bubblewrap at schema 0.8+ with + // enforcementMode "firewall", entries must be IP + // literals or CIDR blocks: DNS names are rejected at + // validation time rather than resolved. Use proxy + // mode for hostname-based control. "proxy": { "localhost": 8080 } // Loopback proxy port (processcontainer; bubblewrap; seatbelt) // (use { "builtinTestServer": true } for the bundled // testing-only proxy; requires --allow-testing-features) diff --git a/src/Cargo.lock b/src/Cargo.lock index b494dd438..4f8229949 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -218,6 +218,7 @@ version = "0.7.0" dependencies = [ "lxc_common", "nix", + "semver", "serde", "serde_json", "tempfile", diff --git a/src/backends/bubblewrap/common/Cargo.toml b/src/backends/bubblewrap/common/Cargo.toml index bbf72eed8..89e04e8e2 100644 --- a/src/backends/bubblewrap/common/Cargo.toml +++ b/src/backends/bubblewrap/common/Cargo.toml @@ -8,6 +8,7 @@ license.workspace = true wxc_common = { workspace = true } lxc_common = { workspace = true } nix = { workspace = true } +semver = "1" serde = { workspace = true } serde_json = { workspace = true } tempfile = { workspace = true } diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 9e98b73d7..b8f252205 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -104,29 +104,54 @@ pub(crate) enum ResolvedNetworkMode { /// Pre-0.8 cooperative proxy routing in the host network namespace. LegacyProxy, /// The host network namespace with per-destination iptables filtering. + /// + /// Pre-0.8 only. The rules land on the *host* chain, which the sandbox + /// never traverses, so this filters nothing; it is retained unchanged + /// because pre-0.8 callers already run under it. FirewallFiltered, + /// Per-destination iptables filtering inside a slirp-backed private + /// namespace, with no proxy. 0.8+ only. + FirewallEnforced, /// Cooperative proxy routing inside a slirp-backed private namespace, with /// egress closed to everything but the proxy. 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) +/// Whether a schema version opts into the 0.8 network contract, under which a +/// network element Bubblewrap cannot enforce is a hard error rather than a +/// warning. +/// +/// An absent version stays lenient: the parser accepts it, and programmatic +/// callers who never set one are the pre-0.8 population. A malformed non-empty +/// version is treated as strict instead of lenient -- the parser would reject +/// it outright, so the only way one reaches here is a hand-built +/// `ExecutionRequest` that skipped the parser, and a typo like `"0.8"` must not +/// silently buy pre-0.8 leniency. +pub fn schema_enforces_network_strictly(version: &str) -> bool { + if version.is_empty() { + return false; + } + semver::Version::parse(version) + .map(|parsed| (parsed.major, parsed.minor) >= (0, 8)) + .unwrap_or(true) +} + +/// Whether this schema opts into the 0.8 private network-namespace contract. +/// +/// Gates both the proxy path (`LegacyProxy` → `ProxyOnly`) and the firewall +/// path (`FirewallFiltered` → `FirewallEnforced`). Everything the private +/// namespace brings — slirp, in-namespace rules, IP/CIDR-only rule addresses — +/// is a behavior change for callers already running on 0.6/0.7, so it is +/// introduced on the dev line rather than retrofitted. +fn uses_private_network_contract(request: &ExecutionRequest) -> bool { + schema_enforces_network_strictly(&request.schema_version) } 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 if uses_private_proxy_network(request) { + return if uses_private_network_contract(request) { Self::ProxyOnly } else { Self::LegacyProxy @@ -141,7 +166,11 @@ impl ResolvedNetworkMode { !request.policy.allowed_hosts.is_empty() || !request.policy.blocked_hosts.is_empty(); if uses_firewall && has_host_rules { - Self::FirewallFiltered + if uses_private_network_contract(request) { + Self::FirewallEnforced + } else { + Self::FirewallFiltered + } } else if request.policy.default_network_policy == NetworkPolicy::Block && !has_host_rules { Self::Isolated } else { @@ -151,23 +180,146 @@ impl ResolvedNetworkMode { /// Whether current behavior gives the sandbox a private network namespace. pub(crate) fn uses_private_netns(self) -> bool { - matches!(self, Self::Isolated | Self::ProxyOnly) + matches!( + self, + Self::Isolated | Self::ProxyOnly | Self::FirewallEnforced + ) } /// Whether the runner supplies a pre-created user namespace to Bubblewrap. pub(crate) fn uses_external_userns(self) -> bool { - matches!(self, Self::ProxyOnly) + matches!(self, Self::ProxyOnly | Self::FirewallEnforced) } /// 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. + /// False for `ProxyOnly` and `FirewallEnforced`, which are *not* + /// iptables-free: they program 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_host_firewall_manager(self) -> bool { matches!(self, Self::FirewallFiltered) } + + /// Whether this mode actually applies the policy's host lists. + /// + /// `FirewallEnforced` filters by address in the sandbox's own namespace. + /// `ProxyOnly` closes egress to everything but the proxy endpoint, so the + /// lists are enforced at the proxy — true only for the builtin proxy; an + /// external one is refused by [`external_proxy_host_rules_rejection`]. + /// Every other mode leaves the lists unapplied. + pub(crate) fn enforces_host_rules(self) -> bool { + matches!(self, Self::FirewallEnforced | Self::ProxyOnly) + } +} + +/// Whether this schema opts into rejecting network elements Bubblewrap cannot +/// honor. Pre-0.8 callers keep the warning, so existing configs still run. +fn rejects_unhonorable_network(request: &ExecutionRequest) -> bool { + schema_enforces_network_strictly(&request.schema_version) +} + +/// Rejection text for host lists no mechanism will enforce, on schema 0.8+. +/// +/// Owned here rather than in the parser: this is a statement about what +/// Bubblewrap can enforce, so it belongs with the backend that enforces it. +pub const BWRAP_UNENFORCED_HOST_RULES: &str = + "Bubblewrap: network.allowedHosts and network.blockedHosts require an enforcement \ + mechanism. With enforcementMode='capabilities' (the default) and no network.proxy, \ + nothing applies them: the host lists suppress the --unshare-net full block, so the \ + sandbox shares the host network namespace with no filtering at all. Set \ + network.enforcementMode to 'firewall' to filter by address inside the sandbox's own \ + network namespace, set network.proxy to enforce host filtering at the proxy layer, or \ + drop the host lists to get the namespace-level block that defaultPolicy='block' applies \ + on its own."; + +/// Rejection text for `allowLocalNetwork=false` under a shared namespace. +pub const BWRAP_LOCAL_NETWORK_SHARED_NS: &str = + "Bubblewrap: network.allowLocalNetwork=false is not enforced while the \ + 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 host rules and no proxy, \ + which applies --unshare-net; to keep the shared namespace, acknowledge the exposure \ + with allowLocalNetwork=true."; + +/// Rejection text for `allowLocalNetwork=true` under a private namespace. +pub const BWRAP_LOCAL_NETWORK_PRIVATE_NS: &str = + "Bubblewrap: network.allowLocalNetwork=true is confined to the sandbox's own \ + network namespace. Isolated, proxy and firewall modes apply --unshare-net, so a \ + listener inside the sandbox is reachable only from within it, never from the host. \ + Use defaultPolicy='allow' with no host rules and no proxy to share the host network \ + namespace."; + +/// Reject host lists that no mechanism will apply, on schema 0.8+. +/// +/// Host lists suppress the `Isolated` mode, so a mode that does not apply them +/// leaves the sandbox on the host namespace unfiltered. This is the only layer +/// that checks it: the parser validates structure, and every caller — CLI, +/// SDK, FFI, or a Rust caller handing `mxc_engine` a hand-built +/// `ExecutionRequest` — reaches a runner, while only JSON configs reach the +/// parser. +pub fn unenforced_host_rules_rejection(request: &ExecutionRequest) -> Option<&'static str> { + if !rejects_unhonorable_network(request) { + return None; + } + let has_host_rules = + !request.policy.allowed_hosts.is_empty() || !request.policy.blocked_hosts.is_empty(); + let mode = + ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()); + + (has_host_rules && !mode.enforces_host_rules()).then_some(BWRAP_UNENFORCED_HOST_RULES) +} + +/// Rejection text for an external proxy combined with host lists. +/// +/// Duplicated (verbatim) from the parser's pre-existing check of the same +/// combination. The parser keeps its copy for JSON configs; this one covers +/// callers who hand a runner an `ExecutionRequest` directly. +pub const BWRAP_EXTERNAL_PROXY_HOST_RULES: &str = + "Bubblewrap: an external network.proxy (url/localhost) cannot be combined with \ + allowedHosts, blockedHosts, or defaultPolicy='block'. The external proxy is expected to \ + enforce its own host policy; MXC does not forward host lists to it. Use \ + 'network.proxy.builtinTestServer: true' (testing only) for MXC-enforced host filtering, \ + or remove the host policy."; + +/// Validate-time twin of the parser's external-proxy host-list rejection. +/// +/// MXC applies the host lists itself only for the builtin test proxy; an +/// operator-supplied proxy is never handed them, so accepting the combination +/// would drop the requested policy silently. Not schema-gated, matching the +/// parser. +pub fn external_proxy_host_rules_rejection(request: &ExecutionRequest) -> Option<&'static str> { + let proxy = &request.policy.network_proxy; + if !proxy.is_enabled() || proxy.builtin_test_server { + return None; + } + let conflicts = !request.policy.allowed_hosts.is_empty() + || !request.policy.blocked_hosts.is_empty() + || request.policy.default_network_policy == NetworkPolicy::Block; + + conflicts.then_some(BWRAP_EXTERNAL_PROXY_HOST_RULES) +} + +/// Validate-time twin of [`local_network_diagnostic`]: the same mismatch, but +/// only for schemas that reject it rather than warn. +/// +/// Keys off proxy *enablement* rather than a resolved address because +/// `builtinTestServer` has no address until the proxy starts, which is after +/// validation. +/// +/// Rejects on the value, not on whether the caller wrote the field. `false` is +/// the schema default *and* a deny, so an omitted `allowLocalNetwork` still +/// asks for inbound denial. Callers wanting the shared namespace acknowledge +/// the exposure with `allowLocalNetwork=true`. The warning path keeps the +/// looser check: it only logs. +pub fn local_network_rejection(request: &ExecutionRequest) -> Option<&'static str> { + if !rejects_unhonorable_network(request) { + return None; + } + let mode = + ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()); + local_network_diagnostic_for_mode(request, mode) } /// Describe a `network.allowLocalNetwork` setting Bubblewrap cannot honor, or @@ -198,6 +350,9 @@ pub fn local_network_diagnostic( } /// Describe an inbound-policy mismatch using a previously resolved mode. +/// +/// The text carries no severity prefix so it can serve both the pre-0.8 +/// warning and the 0.8+ rejection; callers add "WARNING: " when logging. pub(crate) fn local_network_diagnostic_for_mode( request: &ExecutionRequest, network_mode: ResolvedNetworkMode, @@ -206,19 +361,8 @@ pub(crate) fn local_network_diagnostic_for_mode( request.policy.allow_local_network, network_mode.uses_private_netns(), ) { - (false, false) => Some( - "WARNING: Bubblewrap: network.allowLocalNetwork=false is not enforced while the \ - 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. 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.", - ), + (false, false) => Some(BWRAP_LOCAL_NETWORK_SHARED_NS), + (true, true) => Some(BWRAP_LOCAL_NETWORK_PRIVATE_NS), _ => None, } } @@ -394,6 +538,7 @@ pub(crate) fn build_args_classified_with_mode( #[cfg(test)] mod tests { use super::*; + use wxc_common::models::ContainerPolicy; fn base_request() -> ExecutionRequest { ExecutionRequest { @@ -419,15 +564,19 @@ mod tests { ("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. + // An absent version stays legacy: programmatic callers who never + // set one are the pre-0.8 population. Anything non-empty but + // unparsable cannot have come from the parser (it validates the + // version first), so it is a hand-built request whose typo must not + // buy pre-0.8 leniency -- it resolves strictly, matching + // `schema_enforces_network_strictly`, which the rejection gates + // share. ("", ResolvedNetworkMode::LegacyProxy), ("0.8", ResolvedNetworkMode::ProxyOnly), - ("0", ResolvedNetworkMode::LegacyProxy), - ("0.8-beta", ResolvedNetworkMode::LegacyProxy), - ("v0.8.0", ResolvedNetworkMode::LegacyProxy), - ("not-a-version", ResolvedNetworkMode::LegacyProxy), + ("0", ResolvedNetworkMode::ProxyOnly), + ("0.8-beta", ResolvedNetworkMode::ProxyOnly), + ("v0.8.0", ResolvedNetworkMode::ProxyOnly), + ("not-a-version", ResolvedNetworkMode::ProxyOnly), ]; for (version, expected) in cases { @@ -443,6 +592,45 @@ mod tests { } } + #[test] + fn the_schema_gate_selects_in_netns_firewall_rules_only_from_0_8_onward() { + // The same gate governs the firewall path. Pre-0.8 callers keep + // FirewallFiltered — which filters nothing, because its rules land on + // a host chain the sandbox never traverses — because that is the + // behavior they already run under; only 0.8+ opts into rules that + // actually apply, and into the IP/CIDR-only rule addresses that come + // with them. + let cases = [ + ("0.6.0-alpha", ResolvedNetworkMode::FirewallFiltered), + ("0.7.0-alpha", ResolvedNetworkMode::FirewallFiltered), + ("0.7.99", ResolvedNetworkMode::FirewallFiltered), + ("0.8.0-alpha", ResolvedNetworkMode::FirewallEnforced), + ("0.9.0", ResolvedNetworkMode::FirewallEnforced), + ("1.0.0", ResolvedNetworkMode::FirewallEnforced), + // Absent stays legacy; a non-empty unparsable version resolves + // strictly, for the same reason it does on the proxy path. + ("", ResolvedNetworkMode::FirewallFiltered), + ("not-a-version", ResolvedNetworkMode::FirewallEnforced), + ]; + + for (version, expected) in cases { + let request = ExecutionRequest { + schema_version: version.into(), + policy: ContainerPolicy { + network_enforcement_mode: NetworkEnforcementMode::Firewall, + allowed_hosts: vec!["203.0.113.7".into()], + ..Default::default() + }, + ..base_request() + }; + assert_eq!( + ResolvedNetworkMode::from_request(&request, false), + expected, + "schema version {version:?} resolved to the wrong firewall 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 @@ -469,6 +657,7 @@ mod tests { struct NetworkPlanCase { name: &'static str, + schema_version: &'static str, default_policy: NetworkPolicy, enforcement_mode: NetworkEnforcementMode, allowed_hosts: &'static [&'static str], @@ -482,6 +671,7 @@ mod tests { let cases = [ NetworkPlanCase { name: "default block is isolated", + schema_version: "0.8.0-alpha", default_policy: NetworkPolicy::Block, enforcement_mode: NetworkEnforcementMode::Capabilities, allowed_hosts: &[], @@ -491,6 +681,7 @@ mod tests { }, NetworkPlanCase { name: "default allow is shared", + schema_version: "0.8.0-alpha", default_policy: NetworkPolicy::Allow, enforcement_mode: NetworkEnforcementMode::Capabilities, allowed_hosts: &[], @@ -499,7 +690,18 @@ mod tests { expected: ResolvedNetworkMode::Shared, }, NetworkPlanCase { - name: "firewall allow rules require filtering", + name: "firewall allow rules are enforced in-namespace at 0.8", + schema_version: "0.8.0-alpha", + default_policy: NetworkPolicy::Block, + enforcement_mode: NetworkEnforcementMode::Firewall, + allowed_hosts: &["203.0.113.7"], + blocked_hosts: &[], + proxy_active: false, + expected: ResolvedNetworkMode::FirewallEnforced, + }, + NetworkPlanCase { + name: "firewall allow rules keep the legacy host chain pre-0.8", + schema_version: "0.7.0-alpha", default_policy: NetworkPolicy::Block, enforcement_mode: NetworkEnforcementMode::Firewall, allowed_hosts: &["example.com"], @@ -508,7 +710,18 @@ mod tests { expected: ResolvedNetworkMode::FirewallFiltered, }, NetworkPlanCase { - name: "combined enforcement block rules require filtering", + name: "combined enforcement block rules are enforced at 0.8", + schema_version: "0.8.0-alpha", + default_policy: NetworkPolicy::Allow, + enforcement_mode: NetworkEnforcementMode::Both, + allowed_hosts: &[], + blocked_hosts: &["198.51.100.9"], + proxy_active: false, + expected: ResolvedNetworkMode::FirewallEnforced, + }, + NetworkPlanCase { + name: "combined enforcement block rules keep the host chain pre-0.8", + schema_version: "0.6.0-alpha", default_policy: NetworkPolicy::Allow, enforcement_mode: NetworkEnforcementMode::Both, allowed_hosts: &[], @@ -518,6 +731,7 @@ mod tests { }, NetworkPlanCase { name: "capabilities mode with host rules stays shared", + schema_version: "0.8.0-alpha", default_policy: NetworkPolicy::Block, enforcement_mode: NetworkEnforcementMode::Capabilities, allowed_hosts: &["example.com"], @@ -527,6 +741,7 @@ mod tests { }, NetworkPlanCase { name: "proxy takes precedence over isolation", + schema_version: "0.8.0-alpha", default_policy: NetworkPolicy::Block, enforcement_mode: NetworkEnforcementMode::Capabilities, allowed_hosts: &[], @@ -536,6 +751,7 @@ mod tests { }, NetworkPlanCase { name: "proxy takes precedence over firewall filtering", + schema_version: "0.8.0-alpha", default_policy: NetworkPolicy::Allow, enforcement_mode: NetworkEnforcementMode::Firewall, allowed_hosts: &[], @@ -547,7 +763,7 @@ mod tests { for case in cases { let mut request = ExecutionRequest { - schema_version: "0.8.0-alpha".into(), + schema_version: case.schema_version.into(), ..Default::default() }; request.policy.default_network_policy = case.default_policy; @@ -579,6 +795,12 @@ mod tests { assert!(!ResolvedNetworkMode::Shared.requires_host_firewall_manager()); // ProxyOnly enforces via in-netns rules, not the host manager. assert!(!ResolvedNetworkMode::ProxyOnly.requires_host_firewall_manager()); + // FirewallEnforced is the same shape as ProxyOnly minus the proxy: its + // rules are programmed from inside the sandbox's own namespace, which + // is the whole reason it filters where FirewallFiltered does not. + assert!(ResolvedNetworkMode::FirewallEnforced.uses_private_netns()); + assert!(ResolvedNetworkMode::FirewallEnforced.uses_external_userns()); + assert!(!ResolvedNetworkMode::FirewallEnforced.requires_host_firewall_manager()); } #[test] @@ -667,6 +889,211 @@ mod tests { assert!(local_network_diagnostic(&r, None).is_none()); } + // ------- allowLocalNetwork rejection (schema 0.8+) ------------------ + + #[test] + fn local_network_mismatch_is_not_rejected_before_0_8() { + // A 0.7 proxy config shares the host netns and leaves allowLocalNetwork + // at its default false. That combination must keep warning rather than + // start failing, or every existing 0.6/0.7 proxy caller breaks. + let mut r = base_request(); + r.schema_version = "0.7.0-alpha".into(); + r.policy.network_proxy.address = Some(ProxyAddress::new("127.0.0.1".into(), 8080)); + assert!(local_network_diagnostic(&r, r.policy.network_proxy.address.as_ref()).is_some()); + assert!(local_network_rejection(&r).is_none()); + } + + #[test] + fn local_network_mismatch_is_rejected_at_0_8() { + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + r.policy.default_network_policy = NetworkPolicy::Allow; + let msg = local_network_rejection(&r).expect("shared netns cannot honor the deny"); + assert!(msg.contains("allowLocalNetwork=false")); + // The text is reused verbatim as an error, so it must carry no severity. + assert!(!msg.contains("WARNING")); + } + + #[test] + fn an_omitted_local_network_is_rejected_like_an_explicit_deny_at_0_8() { + // `false` is the schema default *and* a deny, so silence still asks for + // inbound denial. + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + r.policy.default_network_policy = NetworkPolicy::Allow; + assert!(!r.policy.allow_local_network, "default is the deny posture"); + let msg = local_network_rejection(&r).expect("an omitted deny is still a deny"); + assert!(msg.contains("allowLocalNetwork=false")); + // Actionable only if it names the acknowledgment. + assert!(msg.contains("allowLocalNetwork=true")); + } + + #[test] + fn acknowledged_local_network_exposure_is_accepted_at_0_8() { + // The escape hatch from the rejection above. + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + r.policy.default_network_policy = NetworkPolicy::Allow; + r.policy.allow_local_network = true; + assert!(local_network_rejection(&r).is_none()); + } + + #[test] + fn honored_local_network_is_not_rejected_at_0_8() { + // block + no host rules + no proxy is a private netns, which satisfies + // allowLocalNetwork=false; nothing to reject. + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + assert!(local_network_rejection(&r).is_none()); + } + + #[test] + fn builtin_test_server_counts_as_an_active_proxy_when_rejecting() { + // builtinTestServer has no address until the proxy starts, which is + // after validation, so the rejection keys off enablement instead. + // defaultPolicy='allow' would be a shared namespace on its own, so the + // rejection fires only if the proxy is recognized without an address. + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + r.policy.default_network_policy = NetworkPolicy::Allow; + r.policy.allow_local_network = true; + assert!(local_network_rejection(&r).is_none()); + + r.policy.network_proxy.builtin_test_server = true; + assert!(r.policy.network_proxy.address.is_none()); + assert!(local_network_rejection(&r).is_some()); + } + + /// Without this twin a programmatic caller would sail past the parser's + /// rejection and have the host policy dropped on the floor. + #[test] + fn external_proxy_with_host_rules_is_rejected() { + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + r.policy.default_network_policy = NetworkPolicy::Allow; + r.policy.network_proxy.address = Some(ProxyAddress::new("proxy.example.com".into(), 3128)); + assert!(external_proxy_host_rules_rejection(&r).is_none()); + + r.policy.allowed_hosts = vec!["10.0.0.1".into()]; + let msg = external_proxy_host_rules_rejection(&r).expect("lists are never forwarded"); + assert!(msg.contains("external network.proxy")); + + r.policy.allowed_hosts.clear(); + r.policy.blocked_hosts = vec!["10.0.0.1".into()]; + assert!(external_proxy_host_rules_rejection(&r).is_some()); + + r.policy.blocked_hosts.clear(); + r.policy.default_network_policy = NetworkPolicy::Block; + assert!(external_proxy_host_rules_rejection(&r).is_some()); + } + + /// The builtin test proxy *is* handed the lists, so it must stay accepted. + #[test] + fn builtin_test_server_with_host_rules_is_not_rejected() { + let mut r = base_request(); + r.schema_version = "0.8.0-alpha".into(); + r.policy.network_proxy.builtin_test_server = true; + r.policy.allowed_hosts = vec!["10.0.0.1".into()]; + assert!(external_proxy_host_rules_rejection(&r).is_none()); + } + + // The predicate gates whether an unenforceable network element is a hard + // error or a warning, so a regression in it changes security behavior + // silently. Pinned directly rather than only through the call sites. + #[test] + fn strict_network_enforcement_starts_at_0_8() { + assert!(!schema_enforces_network_strictly("0.7.9")); + assert!(!schema_enforces_network_strictly("0.7.0-alpha")); + assert!(schema_enforces_network_strictly("0.8.0-alpha")); + assert!(schema_enforces_network_strictly("0.8.0")); + assert!(schema_enforces_network_strictly("0.9.0")); + } + + #[test] + fn strict_network_enforcement_holds_past_1_0() { + // A tuple compare, so a major bump must not wrap back to lenient. + assert!(schema_enforces_network_strictly("1.0.0")); + assert!(schema_enforces_network_strictly("2.3.4")); + } + + #[test] + fn a_pre_release_label_does_not_lower_the_version() { + // semver orders 0.8.0-alpha *below* 0.8.0; comparing (major, minor) + // sidesteps that, which is the whole reason for the tuple. + assert!( + semver::Version::parse("0.8.0-alpha").unwrap() + < semver::Version::parse("0.8.0").unwrap() + ); + assert!(schema_enforces_network_strictly("0.8.0-alpha")); + } + + #[test] + fn an_absent_version_is_lenient_but_a_malformed_one_is_not() { + // Absent is the pre-0.8 programmatic caller: the parser accepts it, so + // it keeps its behavior. Malformed cannot come from the parser at all + // (it rejects these), so it is a hand-built request whose typo must not + // buy leniency. + assert!(!schema_enforces_network_strictly("")); + assert!(schema_enforces_network_strictly("x")); + assert!(schema_enforces_network_strictly("0.8")); + assert!(schema_enforces_network_strictly("0.7")); + } + + /// `ResolvedNetworkMode` is now the only implementation of the namespace + /// choice — the parser's re-derived twin is gone — so the mode matrix is + /// pinned against explicit expectations rather than against a second copy + /// that could drift with it. + #[test] + fn the_resolved_mode_matches_the_namespace_it_claims_across_the_matrix() { + for schema in ["", "0.7.0-alpha", "0.8.0-alpha"] { + let strict = schema == "0.8.0-alpha"; + for proxy in [false, true] { + for mode in [ + NetworkEnforcementMode::Capabilities, + NetworkEnforcementMode::Firewall, + NetworkEnforcementMode::Both, + ] { + for policy in [NetworkPolicy::Allow, NetworkPolicy::Block] { + for hosts in [false, true] { + let mut r = base_request(); + r.schema_version = schema.into(); + r.policy.network_proxy.builtin_test_server = proxy; + r.policy.network_enforcement_mode = mode.clone(); + r.policy.default_network_policy = policy.clone(); + r.policy.allowed_hosts = if hosts { + vec!["10.0.0.1".into()] + } else { + vec![] + }; + + // Private namespaces: proxy-only and firewall- + // enforced are both 0.8-only, so either shape is + // private exactly when the schema is strict. Plus + // Isolated, the only pre-0.8 mode that unshares. + let uses_firewall = matches!( + mode, + NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both + ); + let expected = if proxy || (uses_firewall && hosts) { + strict + } else { + policy == NetworkPolicy::Block && !hosts + }; + + let resolved = ResolvedNetworkMode::from_request(&r, proxy); + assert_eq!( + resolved.uses_private_netns(), + expected, + "schema={schema} proxy={proxy} mode={mode:?} \ + policy={policy:?} hosts={hosts}" + ); + } + } + } + } + } + } + #[test] fn filesystem_policy_produces_correct_mounts() { 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 d93603bc0..a61b056f8 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, validate_network_policy_support}; use crate::{ bwrap_command::{self, ResolvedNetworkMode}, - bwrap_version, proxy_network, + bwrap_version, network_rules, proxy_network, }; /// Bubblewrap sandbox runner. Uses only shared `ContainerPolicy` fields — @@ -112,6 +112,27 @@ impl SandboxBackend for BubblewrapScriptRunner { } } + // Schema 0.8+ fails closed on a network element Bubblewrap cannot + // honor, rather than running more permissive than requested. Pre-0.8 + // keeps the warning, so existing configs are unaffected. Sits with the + // input checks so a host without bwrap is still told what is wrong. + // + // This is the only layer that checks what Bubblewrap can enforce: the + // parser validates structure, and it only sees JSON configs, while a + // Rust caller can build an `ExecutionRequest` and reach the runner + // directly. (The external-proxy case is the exception — the parser has + // rejected that combination since before this backend gained its own + // validation, so both layers refuse it.) + if let Some(reason) = bwrap_command::external_proxy_host_rules_rejection(request) { + return Err(ScriptResponse::error(reason)); + } + if let Some(reason) = bwrap_command::unenforced_host_rules_rejection(request) { + return Err(ScriptResponse::error(reason)); + } + if let Some(reason) = bwrap_command::local_network_rejection(request) { + return Err(ScriptResponse::error(reason)); + } + // Proxy-only networking derives the sandbox-visible endpoint from the // configured one and then opens exactly that address in the egress // chain. That step can reject an endpoint outright (an IPv6 loopback @@ -145,14 +166,38 @@ impl SandboxBackend for BubblewrapScriptRunner { } } + // Firewall enforcement builds its chain from the policy's host lists, + // and every rule address must be an IP literal or CIDR: the sandbox + // resolves DNS itself, so a name can be mapped to an address the chain + // never authorized and is therefore unenforceable. Rejecting here + // rather than in the parser covers both callers -- `validate` runs + // ahead of every `spawn`, while a programmatic `mxc_engine` caller + // never passes through the parser at all. + let firewall_enforced = + ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()) + == ResolvedNetworkMode::FirewallEnforced; + if firewall_enforced { + if let Err(error) = network_rules::EgressPlan::for_policy(request) { + 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 proxy_only { - if let Err(error) = proxy_network::probe_dependencies() { + if proxy_only || firewall_enforced { + // Proxy and firewall are mutually exclusive, so this names the one + // the caller actually asked for; the probe's advice is worded from + // it rather than always naming network.proxy. + let use_case = if proxy_only { + proxy_network::PrivateNetworkUse::ProxyOnlyEgress + } else { + proxy_network::PrivateNetworkUse::FirewallEnforcement + }; + if let Err(error) = proxy_network::probe_dependencies(use_case) { return Err(ScriptResponse::error(&error)); } } @@ -311,7 +356,38 @@ impl BubblewrapScriptRunner { // The workload dials the sandbox-visible address, so that is what // the egress rule must open. Some(resolved) => { - match proxy_network::ProxyNetworkNamespace::start(resolved.egress(), logger) { + let egress = resolved.egress(); + match proxy_network::ProxyNetworkNamespace::start( + &egress.plan(), + egress.pin(), + logger, + request.script_timeout, + ) { + Ok(network) => Some(network), + Err(error) => { + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } + } + } + // Firewall enforcement without a proxy: the same private namespace, + // programmed from the policy's host lists instead of a single + // endpoint. There is no hostname to pin because rule addresses are + // literals and CIDRs only. + None if network_mode == ResolvedNetworkMode::FirewallEnforced => { + let plan = match network_rules::EgressPlan::for_policy(request) { + Ok(plan) => plan, + Err(error) => { + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } + }; + match proxy_network::ProxyNetworkNamespace::start( + &plan, + None, + logger, + request.script_timeout, + ) { Ok(network) => Some(network), Err(error) => { proxy.stop(logger); @@ -328,7 +404,7 @@ impl BubblewrapScriptRunner { if let Some(warning) = bwrap_command::local_network_diagnostic_for_mode(request, network_mode) { - let _ = writeln!(logger, "{}", warning); + let _ = writeln!(logger, "WARNING: {}", warning); } let mut args = bwrap_command::build_args_classified_with_mode( request, @@ -846,7 +922,7 @@ fn resolve_through_symlinks(path: &Path) -> Option { #[cfg(test)] mod tests { use super::*; - use wxc_common::models::{ProxyAddress, ProxyConfig}; + use wxc_common::models::{NetworkEnforcementMode, ProxyAddress, ProxyConfig}; fn base_request() -> ExecutionRequest { ExecutionRequest { @@ -909,6 +985,9 @@ mod tests { 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(); + // An external proxy with the default 'block' would be refused earlier, + // by the host-policy gate; this test is about the endpoint itself. + req.policy.default_network_policy = wxc_common::models::NetworkPolicy::Allow; req.policy.network_proxy = ProxyConfig { address: Some(ProxyAddress::new("[::1]".into(), 3128)), builtin_test_server: false, @@ -1006,6 +1085,7 @@ mod tests { fn validate_rejects_a_hostname_proxy_that_would_defeat_a_denied_hosts_file() { let mut req = base_request(); req.schema_version = "0.8.0-alpha".into(); + req.policy.default_network_policy = wxc_common::models::NetworkPolicy::Allow; req.policy.denied_paths = vec!["/etc/hosts".into()]; req.policy.network_proxy = ProxyConfig { address: Some(ProxyAddress::new("proxy.example.com".into(), 3128)), @@ -1077,6 +1157,78 @@ mod tests { ); } + /// A rule address the backend cannot enforce must be refused, not silently + /// approximated. `validate` is the enforcement point rather than the parser + /// because it runs ahead of every spawn, including the programmatic + /// `mxc_engine` path that never sees the parser. + #[test] + fn validate_rejects_a_hostname_rule_address_at_0_8() { + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.network_enforcement_mode = wxc_common::models::NetworkEnforcementMode::Firewall; + req.policy.allowed_hosts = vec!["api.github.com".into()]; + + let runner = BubblewrapScriptRunner::new(); + let message = runner + .validate(&req) + .err() + .map(|err| err.error_message) + .expect("a hostname rule address must be rejected"); + + assert!( + message.contains("api.github.com") && message.contains("not an IP address or CIDR"), + "the rejection must name the offending entry and the reason: {message}" + ); + } + + #[test] + fn validate_accepts_literal_and_cidr_rule_addresses_at_0_8() { + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.network_enforcement_mode = wxc_common::models::NetworkEnforcementMode::Firewall; + req.policy.allowed_hosts = vec!["203.0.113.7".into(), "10.0.0.0/8".into()]; + req.policy.blocked_hosts = vec!["2001:db8::/32".into()]; + + let runner = BubblewrapScriptRunner::new(); + let message = runner + .validate(&req) + .err() + .map(|err| err.error_message) + .unwrap_or_default(); + + assert!( + !message.contains("not an IP address or CIDR"), + "literals and CIDRs must be accepted: {message}" + ); + } + + /// The 0.8 gate must not reach back to callers already running on 0.6/0.7, + /// whose host lists are hostnames by construction. Their runs still enforce + /// nothing -- that is the pre-existing behavior this change deliberately + /// leaves alone rather than the behavior it introduces. + #[test] + fn validate_leaves_a_pre_0_8_hostname_rule_address_untouched() { + for version in ["0.6.0-alpha", "0.7.0-alpha"] { + let mut req = base_request(); + req.schema_version = version.into(); + req.policy.network_enforcement_mode = + wxc_common::models::NetworkEnforcementMode::Firewall; + req.policy.allowed_hosts = vec!["api.github.com".into()]; + + let runner = BubblewrapScriptRunner::new(); + let message = runner + .validate(&req) + .err() + .map(|err| err.error_message) + .unwrap_or_default(); + + assert!( + !message.contains("not an IP address or CIDR"), + "schema {version} must keep parsing hostname rule addresses: {message}" + ); + } + } + /// Legacy proxy mode shares the host's network, never translates the /// endpoint and never pins a name, so the same policy has to stay /// acceptable there -- rejecting it would break callers already on 0.6/0.7. @@ -1104,6 +1256,158 @@ mod tests { ); } + #[test] + fn validate_rejects_an_unhonorable_local_network_request_at_0_8() { + // defaultPolicy='allow' shares the host netns, so allowLocalNetwork + // =false cannot be honored. Runs ahead of the bwrap probe, so this + // holds on hosts without bwrap installed. Host lists are no longer a + // vehicle for this: at 0.8 they resolve to a private namespace under + // either enforcement mechanism. + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.default_network_policy = wxc_common::models::NetworkPolicy::Allow; + + let err = BubblewrapScriptRunner::new().validate(&req).unwrap_err(); + assert!( + err.error_message.contains("allowLocalNetwork=false"), + "unexpected error: {}", + err.error_message + ); + } + + #[test] + fn validate_accepts_the_same_request_before_0_8() { + // Pre-0.8 warns at run time instead of failing, so existing callers are + // unaffected. Tolerant of a host without bwrap: it only rules out the + // local-network rejection. + let mut req = base_request(); + req.schema_version = "0.7.0-alpha".into(); + req.policy.default_network_policy = wxc_common::models::NetworkPolicy::Allow; + + if let Err(err) = BubblewrapScriptRunner::new().validate(&req) { + assert!( + !err.error_message.contains("allowLocalNetwork"), + "0.7 must not be rejected for allowLocalNetwork: {}", + err.error_message + ); + } + } + + #[test] + fn validate_accepts_firewall_enforced_host_rules_at_0_8() { + // Firewall mode is the enforcement mechanism at 0.8, so the + // unenforced-host-rules gate must not fire for it. + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.network_enforcement_mode = NetworkEnforcementMode::Firewall; + req.policy.allowed_hosts = vec!["10.0.2.2/32".into()]; + req.policy.allow_local_network = true; + + if let Err(err) = BubblewrapScriptRunner::new().validate(&req) { + assert!( + !err.error_message + .contains("require an enforcement mechanism"), + "firewall mode enforces the lists: {}", + err.error_message + ); + } + } + + #[test] + fn validate_accepts_a_firewall_mode_request_before_0_8() { + // GHCP consumes Bubblewrap on 0.6/0.7; the gate must not reach them. + let mut req = base_request(); + req.schema_version = "0.7.0-alpha".into(); + req.policy.network_enforcement_mode = NetworkEnforcementMode::Firewall; + req.policy.allowed_hosts = vec!["api.github.com".into()]; + req.policy.allow_local_network = true; + + if let Err(err) = BubblewrapScriptRunner::new().validate(&req) { + assert!( + !err.error_message.contains("enforcementMode"), + "0.7 must not be rejected for enforcementMode: {}", + err.error_message + ); + } + } + + #[test] + fn validate_rejects_host_rules_no_mechanism_will_enforce_at_0_8() { + // The gap this closes: host lists suppress --unshare-net, but under + // 'capabilities' with no proxy nothing applies them, so a default-deny + // policy ran with fully open egress on the host's namespace. + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.default_network_policy = wxc_common::models::NetworkPolicy::Block; + req.policy.allowed_hosts = vec!["api.github.com".into()]; + req.policy.allow_local_network = true; + + let err = BubblewrapScriptRunner::new().validate(&req).unwrap_err(); + assert!( + err.error_message + .contains("require an enforcement mechanism"), + "unexpected error: {}", + err.error_message + ); + } + + #[test] + fn validate_accepts_host_rules_when_a_proxy_enforces_them_at_0_8() { + // The proxy is the mechanism, so the same lists are fine with one. + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.blocked_hosts = vec!["evil.example.com".into()]; + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("127.0.0.1".into(), 3128)), + builtin_test_server: false, + }; + + if let Err(err) = BubblewrapScriptRunner::new().validate(&req) { + assert!( + !err.error_message + .contains("require an enforcement mechanism"), + "a proxy enforces the lists: {}", + err.error_message + ); + } + } + + #[test] + fn validate_accepts_host_rules_without_a_mechanism_before_0_8() { + // GHCP consumes Bubblewrap on 0.6/0.7 with exactly this shape. + let mut req = base_request(); + req.schema_version = "0.7.0-alpha".into(); + req.policy.default_network_policy = wxc_common::models::NetworkPolicy::Block; + req.policy.allowed_hosts = vec!["api.github.com".into()]; + + if let Err(err) = BubblewrapScriptRunner::new().validate(&req) { + assert!( + !err.error_message + .contains("require an enforcement mechanism"), + "0.7 must not be rejected: {}", + err.error_message + ); + } + } + + /// A malformed non-empty version cannot come from the parser, so it is a + /// hand-built request; the typo must not buy pre-0.8 leniency. + #[test] + fn validate_rejects_unenforced_host_rules_with_a_malformed_version() { + let mut req = base_request(); + req.schema_version = "0.8".into(); + req.policy.allowed_hosts = vec!["api.github.com".into()]; + req.policy.allow_local_network = true; + + let err = BubblewrapScriptRunner::new().validate(&req).unwrap_err(); + assert!( + err.error_message + .contains("require an enforcement mechanism"), + "unexpected error: {}", + err.error_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/lib.rs b/src/backends/bubblewrap/common/src/lib.rs index a74acbf8f..8484311af 100644 --- a/src/backends/bubblewrap/common/src/lib.rs +++ b/src/backends/bubblewrap/common/src/lib.rs @@ -10,6 +10,9 @@ //! - [`bwrap_version`] probes the host `bwrap` and checks it is new enough to //! understand every flag [`bwrap_command`] emits. Shared by the runner's //! `validate` and the engine's platform-support probe. +//! - `network_rules` validates rule addresses and builds the egress plan the +//! sandbox's own network namespace is programmed with. Platform-agnostic for +//! the same reason as [`bwrap_command`]. //! - [`bwrap_runner`] is gated to `target_os = "linux"` since it actually //! spawns the `bwrap` binary. @@ -17,5 +20,8 @@ pub mod bwrap_command; #[cfg(target_os = "linux")] pub mod bwrap_runner; pub mod bwrap_version; +/// Consumed only by the Linux-gated runner and proxy modules. +#[cfg(target_os = "linux")] +pub(crate) mod network_rules; #[cfg(target_os = "linux")] mod proxy_network; diff --git a/src/backends/bubblewrap/common/src/network_rules.rs b/src/backends/bubblewrap/common/src/network_rules.rs new file mode 100644 index 000000000..34b997cd8 --- /dev/null +++ b/src/backends/bubblewrap/common/src/network_rules.rs @@ -0,0 +1,678 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Egress rule model for the sandbox's private network namespace. +//! +//! Rules are programmed inside the sandbox's own network namespace by the +//! supervisor (see [`crate::proxy_network`]), so this module only has to decide +//! *what* to install. It covers both callers: +//! +//! - proxy-only egress, whose plan is a single accepted endpoint, and +//! - `enforcementMode: "firewall"`, whose plan comes from the policy's host +//! lists. +//! +//! # Addresses are IP literals and CIDRs, never names +//! +//! The 0.8 networking contract (`docs/sandbox-policy/0.8.0/networking`, D3) +//! makes rule addresses IPv4/IPv6 literals or CIDRs and rejects DNS names at +//! validation time: a name is bypassable and non-deterministic, since the +//! sandbox can resolve it itself and the answer varies by resolver, TTL and +//! cache. Domain-level control is the proxy's job — it inspects the CONNECT +//! host — not the packet filter's. +//! +//! # The rules file never contains caller text +//! +//! Every address is re-rendered from its parsed form, so what reaches the +//! supervisor's shell is the canonical output of Rust's IP address types plus +//! an integer prefix. A rejected string cannot reach the script, and an +//! accepted one cannot carry anything but digits, dots, colons and a slash. + +use std::fmt; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; + +use wxc_common::models::{ExecutionRequest, NetworkPolicy}; + +/// Which `iptables` binary carries a rule. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RuleFamily { + V4, + V6, +} + +impl RuleFamily { + /// The field the supervisor reads to pick its binary. + fn tag(self) -> &'static str { + match self { + Self::V4 => "4", + Self::V6 => "6", + } + } +} + +/// What the packet filter does with a match. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RuleVerdict { + Accept, + Drop, +} + +impl RuleVerdict { + /// The `-j` target, which is also what the supervisor validates against. + pub(crate) fn target(self) -> &'static str { + match self { + Self::Accept => "ACCEPT", + Self::Drop => "DROP", + } + } +} + +impl fmt::Display for RuleVerdict { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.target()) + } +} + +/// A validated destination: an IP literal or a CIDR block. +/// +/// Held as its canonical rendering rather than the caller's string, so the +/// supervisor never sees unvalidated input. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct RuleAddress { + family: RuleFamily, + text: String, +} + +impl RuleAddress { + /// Classify one `allowedHosts`/`blockedHosts` entry. + /// + /// # Errors + /// + /// Returns the caller-facing reason when the entry is empty, malformed, or + /// a DNS name, which D3 rejects rather than resolves. + pub(crate) fn parse(entry: &str) -> Result { + let trimmed = entry.trim(); + if trimmed.is_empty() { + return Err("Bubblewrap: an empty network rule address is not a destination".into()); + } + + if let Some((address, prefix)) = trimmed.split_once('/') { + return Self::parse_cidr(trimmed, address, prefix); + } + + match trimmed.parse::() { + Ok(IpAddr::V4(ip)) => Ok(Self { + family: RuleFamily::V4, + text: ip.to_string(), + }), + Ok(IpAddr::V6(ip)) => match ip.to_ipv4_mapped() { + Some(v4) => Ok(Self { + family: RuleFamily::V4, + text: v4.to_string(), + }), + None => Ok(Self { + family: RuleFamily::V6, + text: ip.to_string(), + }), + }, + Err(_) => Err(name_rejected(trimmed)), + } + } + + /// Validate `address/prefix`, keeping the prefix within its family's width. + fn parse_cidr(entry: &str, address: &str, prefix: &str) -> Result { + let parsed = match address.parse::() { + Ok(ip) => ip, + Err(_) => return Err(name_rejected(entry)), + }; + + let width = match parsed { + IpAddr::V4(_) => 32, + IpAddr::V6(_) => 128, + }; + let bits: u8 = prefix.parse().map_err(|_| { + format!( + "Bubblewrap: network rule '{entry}' has a malformed CIDR prefix; expected a \ + number between 0 and {width}." + ) + })?; + if bits > width { + return Err(format!( + "Bubblewrap: network rule '{entry}' has a CIDR prefix wider than its address \ + family allows; expected a number between 0 and {width}." + )); + } + + // The prefix is validated against the family the caller wrote, so a + // mapped block is only rewritten once its own notation checks out. + let (family, canonical, bits) = match parsed { + IpAddr::V4(ip) => (RuleFamily::V4, ip.to_string(), bits), + IpAddr::V6(ip) => match mapped_v4_block(ip, bits) { + Some((v4, v4_bits)) => (RuleFamily::V4, v4.to_string(), v4_bits), + None if covers_ipv4_mapped_range(ip, bits) => { + return Err(straddles_mapped_range(entry)) + } + None => (RuleFamily::V6, ip.to_string(), bits), + }, + }; + + Ok(Self { + family, + text: format!("{canonical}/{bits}"), + }) + } +} + +/// An IPv4-mapped CIDR as its IPv4 equivalent, when it has one. +/// +/// Linux puts a genuine IPv4 packet on the wire for a mapped destination, so an +/// `ip6tables` rule naming one never matches: under `defaultPolicy: allow` a +/// mapped `blockedHosts` entry would otherwise fail open. LXC normalizes +/// literals and `/96`-or-longer CIDRs the same way, so those policies mean the +/// same thing on both; it does not yet reject the straddling blocks below. +/// +/// The mapped range is the last 32 bits of `::ffff:0:0/96`, so an IPv6 prefix +/// of `96 + n` is exactly an IPv4 prefix of `n`. A shorter prefix also covers +/// addresses outside that range, which do travel as IPv6, so it has no single +/// IPv4 equivalent; [`covers_ipv4_mapped_range`] decides whether such a block +/// is merely unrelated to the mapped range or straddles it. +fn mapped_v4_block(ip: Ipv6Addr, bits: u8) -> Option<(Ipv4Addr, u8)> { + Some((ip.to_ipv4_mapped()?, bits.checked_sub(96)?)) +} + +/// Whether a sub-`/96` block contains the whole IPv4-mapped range. +/// +/// CIDR blocks nest or are disjoint, so a block shorter than `/96` either +/// contains all of `::ffff:0:0/96` or misses it entirely — there is no partial +/// overlap to split. That is why such a block cannot be projected onto a +/// narrower IPv4 rule: its mapped half is always the entire IPv4 space. +fn covers_ipv4_mapped_range(ip: Ipv6Addr, bits: u8) -> bool { + if bits >= 96 { + return false; + } + // `u128::MAX << 128` is undefined, so the /0 case is masked explicitly. + let mask = if bits == 0 { + 0 + } else { + u128::MAX << (128 - u32::from(bits)) + }; + let mapped_base = u128::from(Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0, 0)); + (u128::from(ip) & mask) == (mapped_base & mask) +} + +/// The message for an IPv6 block that swallows the IPv4-mapped range. +/// +/// Such a block cannot be honored as written. Its mapped half travels as IPv4, +/// so an `ip6tables` rule never matches it — under `defaultPolicy: allow` a +/// `blockedHosts` entry would fail open. Projecting it onto IPv4 instead would +/// silently widen the rule to all of IPv4 (the mapped half of any sub-`/96` +/// block is the whole space), turning `::/0` into "and every IPv4 address too". +/// Neither reading is safe to guess, so the caller is asked to say which they +/// meant. +fn straddles_mapped_range(entry: &str) -> String { + format!( + "Bubblewrap: network rule '{entry}' is an IPv6 block shorter than /96 that contains the \ + IPv4-mapped range '::ffff:0:0/96'. Addresses in that range travel as IPv4 packets, so an \ + ip6tables rule would not match them and the rule would be silently unenforced. Write the \ + IPv4 side explicitly instead: keep the IPv6 block for native IPv6 traffic and add the \ + IPv4 block you intend (for example '0.0.0.0/0' to cover all of IPv4)." + ) +} + +/// The message for an address that is neither a literal nor a CIDR. +/// +/// Names are called out explicitly: rejecting them is a deliberate contract +/// decision, so the error explains the alternative rather than reading as a +/// parse failure. +fn name_rejected(entry: &str) -> String { + format!( + "Bubblewrap: network rule '{entry}' is not an IP address or CIDR block. Host filtering \ + matches on addresses, not names: a sandbox resolves DNS itself, so a name can be mapped \ + to another address and is not enforceable. Use an IPv4/IPv6 literal or CIDR (for example \ + '203.0.113.4' or '10.0.0.0/8'), or route the traffic through network.proxy, which \ + filters on the requested host." + ) +} + +/// One installed rule. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct EgressRule { + verdict: RuleVerdict, + address: RuleAddress, + /// Protocol and destination port, when the rule narrows to one service. + /// Only the proxy endpoint uses it; policy host lists carry no port. + port: Option<(&'static str, u16)>, +} + +impl EgressRule { + /// The supervisor's record format: `
`. + /// + /// Five fixed fields keep the script's `read` trivial and its quoting + /// total; `-` stands in for an absent protocol/port so the field count + /// never varies. + fn render(&self) -> String { + let (protocol, port) = match self.port { + Some((protocol, port)) => (protocol.to_string(), port.to_string()), + None => ("-".to_string(), "-".to_string()), + }; + format!( + "{} {} {} {} {}", + self.address.family.tag(), + self.verdict.target(), + self.address.text, + protocol, + port + ) + } +} + +/// The complete filtering posture for one sandbox. +/// +/// Ordering is the contract: rules are appended to the chain in the order held +/// here, and the terminal verdict closes each family. Denies precede allows so +/// an explicit deny overrides a broader allow, per the 0.8 contract's D4. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct EgressPlan { + rules: Vec, + v4_terminal: RuleVerdict, + v6_terminal: RuleVerdict, +} + +/// `iptables` calls the supervisor makes regardless of rule count: per family a +/// chain creation, the loopback exemption, the terminal verdict, and the +/// `OUTPUT` hook. +pub(crate) const BASE_RULE_COMMANDS: u32 = 8; + +impl EgressPlan { + /// The proxy-only posture: one endpoint reachable, everything else dropped. + /// + /// IPv6 carries no rule and closes on `DROP`: the endpoint is IPv4, so v6 + /// egress fails closed rather than being left open. + pub(crate) fn for_proxy(ip: Ipv4Addr, port: u16) -> Self { + Self { + rules: vec![EgressRule { + verdict: RuleVerdict::Accept, + address: RuleAddress { + family: RuleFamily::V4, + text: ip.to_string(), + }, + port: Some(("tcp", port)), + }], + v4_terminal: RuleVerdict::Drop, + v6_terminal: RuleVerdict::Drop, + } + } + + /// The `enforcementMode: "firewall"` posture, from the policy's host lists. + /// + /// `defaultPolicy` sets the terminal verdict for both families, so a family + /// with no rules of its own still inherits the requested posture instead of + /// being silently open or silently closed. + /// + /// # Errors + /// + /// Returns the first address that is not a literal or CIDR. + pub(crate) fn for_policy(request: &ExecutionRequest) -> Result { + let policy = &request.policy; + let terminal = match policy.default_network_policy { + NetworkPolicy::Allow => RuleVerdict::Accept, + NetworkPolicy::Block => RuleVerdict::Drop, + }; + + // Denies first: the chain is evaluated in order and the first match + // wins, so this is what makes an explicit deny beat a broader allow. + let mut rules = Vec::with_capacity(policy.blocked_hosts.len() + policy.allowed_hosts.len()); + for entry in &policy.blocked_hosts { + rules.push(EgressRule { + verdict: RuleVerdict::Drop, + address: RuleAddress::parse(entry)?, + port: None, + }); + } + for entry in &policy.allowed_hosts { + rules.push(EgressRule { + verdict: RuleVerdict::Accept, + address: RuleAddress::parse(entry)?, + port: None, + }); + } + + Ok(Self { + rules, + v4_terminal: terminal, + v6_terminal: terminal, + }) + } + + pub(crate) fn v4_terminal(&self) -> RuleVerdict { + self.v4_terminal + } + + pub(crate) fn v6_terminal(&self) -> RuleVerdict { + self.v6_terminal + } + + /// How many `iptables` calls installing this plan takes, used to size the + /// rule-installation budget. + pub(crate) fn command_count(&self) -> u32 { + BASE_RULE_COMMANDS + self.rules.len() as u32 + } + + /// The rules file handed to the supervisor, one record per line. + /// + /// Empty when the plan is terminal-only; the supervisor's loop then runs + /// zero times and the chain is just loopback plus the terminal verdict. + pub(crate) fn render(&self) -> String { + let mut out = String::new(); + for rule in &self.rules { + out.push_str(&rule.render()); + out.push('\n'); + } + out + } +} + +#[cfg(test)] +mod tests { + use super::*; + use wxc_common::models::ContainerPolicy; + + fn request(default: NetworkPolicy, allowed: &[&str], blocked: &[&str]) -> ExecutionRequest { + ExecutionRequest { + script_code: "echo hello".into(), + policy: ContainerPolicy { + default_network_policy: default, + allowed_hosts: allowed.iter().map(|host| host.to_string()).collect(), + blocked_hosts: blocked.iter().map(|host| host.to_string()).collect(), + ..Default::default() + }, + ..Default::default() + } + } + + #[test] + fn an_ipv4_literal_is_accepted_as_a_host_rule() { + let address = RuleAddress::parse("203.0.113.4").expect("a literal is a valid rule address"); + assert_eq!(address.family, RuleFamily::V4); + assert_eq!(address.text, "203.0.113.4"); + } + + #[test] + fn an_ipv6_literal_is_accepted_and_canonicalized() { + let address = RuleAddress::parse("2001:DB8:0:0:0:0:0:1").expect("a v6 literal is valid"); + assert_eq!(address.family, RuleFamily::V6); + // Rendered from the parsed value, so the file never carries the + // caller's spelling. + assert_eq!(address.text, "2001:db8::1"); + } + + #[test] + fn an_ipv4_mapped_literal_is_programmed_as_ipv4() { + // Linux emits a genuine IPv4 packet for a mapped destination, so an + // ip6tables rule naming one never matches: under defaultPolicy 'allow' + // a mapped blockedHosts entry would fail open. + let address = RuleAddress::parse("::ffff:203.0.113.5").expect("a mapped literal is valid"); + assert_eq!(address.family, RuleFamily::V4); + assert_eq!(address.text, "203.0.113.5"); + } + + #[test] + fn an_ipv4_mapped_cidr_is_translated_to_its_ipv4_prefix() { + // The mapped range is the last 32 bits of ::ffff:0:0/96, so a v6 + // prefix of 96 + n is exactly a v4 prefix of n. + let address = RuleAddress::parse("::ffff:192.0.2.0/120").expect("a mapped CIDR is valid"); + assert_eq!(address.family, RuleFamily::V4); + assert_eq!(address.text, "192.0.2.0/24"); + + let host = RuleAddress::parse("::ffff:198.51.100.42/128").expect("a mapped /128 is valid"); + assert_eq!(host.family, RuleFamily::V4); + assert_eq!(host.text, "198.51.100.42/32"); + } + + #[test] + fn a_prefix_shorter_than_the_mapped_range_is_rejected() { + // A /95 covers all of ::ffff:0:0/96, whose members travel as IPv4 + // packets an ip6tables rule never sees. Keeping it on ip6tables would + // silently unenforce the mapped half (a fail-open for a deny rule under + // defaultPolicy 'allow'), and projecting it onto IPv4 would widen it to + // every IPv4 address. Neither is safe to guess, so it is refused. + let error = RuleAddress::parse("::ffff:0:0/95").expect_err("a straddling block is refused"); + assert!( + error.contains("shorter than /96") && error.contains("::ffff:0:0/96"), + "the message should name the mapped range and the boundary: {error}" + ); + } + + #[test] + fn the_whole_ipv6_space_is_rejected_for_the_same_reason() { + // ::/0 is the case most likely to be written by hand, and the one where + // silently projecting onto IPv4 would be most surprising: "block all + // IPv6" would become "and all IPv4 too". + let error = RuleAddress::parse("::/0").expect_err("::/0 contains the mapped range"); + assert!( + error.contains("shorter than /96"), + "unexpected message: {error}" + ); + } + + #[test] + fn a_sub_96_block_clear_of_the_mapped_range_stays_ipv6() { + // The rejection is scoped to blocks that actually contain the mapped + // range. Ordinary IPv6 CIDRs are unaffected, so native IPv6 filtering + // keeps working. + let address = RuleAddress::parse("2001:db8::/32").expect("a native v6 CIDR is valid"); + assert_eq!(address.family, RuleFamily::V6); + assert_eq!(address.text, "2001:db8::/32"); + + // Immediately below the mapped range and disjoint from it: a /95 pairs + // ::fffc/::fffd, while the mapped range sits in the ::fffe/::ffff pair. + let neighbour = RuleAddress::parse("::fffc:0:0/95").expect("a neighbouring block is valid"); + assert_eq!(neighbour.family, RuleFamily::V6); + } + + #[test] + fn the_mapped_range_itself_is_still_translated_not_rejected() { + // Exactly /96 is the boundary: it maps to 0.0.0.0/0 with no ambiguity, + // so it is translated rather than refused. + let address = RuleAddress::parse("::ffff:0:0/96").expect("the mapped range is valid"); + assert_eq!(address.family, RuleFamily::V4); + assert_eq!(address.text, "0.0.0.0/0"); + } + + #[test] + fn a_mapped_prefix_is_validated_against_the_notation_the_caller_wrote() { + // The caller wrote v6, so the width they are held to is 128 — the + // rewrite happens only once their own notation checks out. + let error = RuleAddress::parse("::ffff:192.0.2.0/129") + .expect_err("a prefix past the v6 width is malformed"); + assert!( + error.contains("wider than its address family allows"), + "{error}" + ); + } + + #[test] + fn a_non_mapped_v6_address_is_left_on_ipv6() { + // NAT64 traffic really is emitted as IPv6, so it belongs in ip6tables. + let address = RuleAddress::parse("64:ff9b::/96").expect("a NAT64 block is valid"); + assert_eq!(address.family, RuleFamily::V6); + assert_eq!(address.text, "64:ff9b::/96"); + } + + #[test] + fn a_cidr_block_keeps_its_prefix() { + let address = RuleAddress::parse("10.0.0.0/8").expect("a CIDR is a valid rule address"); + assert_eq!(address.family, RuleFamily::V4); + assert_eq!(address.text, "10.0.0.0/8"); + } + + #[test] + fn the_widest_prefix_of_each_family_is_allowed() { + assert_eq!( + RuleAddress::parse("192.0.2.1/32") + .expect("a /32 is a host route") + .text, + "192.0.2.1/32" + ); + assert_eq!( + RuleAddress::parse("2001:db8::1/128") + .expect("a /128 is a host route") + .text, + "2001:db8::1/128" + ); + } + + #[test] + fn a_prefix_wider_than_the_family_is_rejected() { + let error = RuleAddress::parse("10.0.0.0/33").expect_err("v4 has 32 bits"); + assert!(error.contains("wider than its address family"), "{error}"); + + let error = RuleAddress::parse("2001:db8::/129").expect_err("v6 has 128 bits"); + assert!(error.contains("wider than its address family"), "{error}"); + } + + #[test] + fn a_non_numeric_prefix_is_rejected() { + let error = RuleAddress::parse("10.0.0.0/eight").expect_err("a prefix must be a number"); + assert!(error.contains("malformed CIDR prefix"), "{error}"); + } + + #[test] + fn a_dns_name_is_rejected_with_the_alternatives() { + let error = RuleAddress::parse("api.github.com").expect_err("names are not enforceable"); + assert!(error.contains("not an IP address or CIDR"), "{error}"); + assert!(error.contains("network.proxy"), "{error}"); + } + + #[test] + fn an_empty_entry_is_rejected() { + let error = RuleAddress::parse(" ").expect_err("an empty entry names nothing"); + assert!(error.contains("empty network rule address"), "{error}"); + } + + /// The rules file is interpolated into a shell loop, so anything that could + /// change the record's shape has to be rejected before it gets there. + #[test] + fn shell_and_field_metacharacters_cannot_reach_the_rules_file() { + for hostile in [ + "10.0.0.1; rm -rf /", + "10.0.0.1 -j ACCEPT", + "$(id)", + "10.0.0.1\n4 ACCEPT 0.0.0.0/0 - -", + "`id`", + "10.0.0.1|id", + ] { + let error = RuleAddress::parse(hostile) + .expect_err("only literals and CIDRs may reach the supervisor"); + assert!( + error.contains("not an IP address or CIDR") || error.contains("CIDR prefix"), + "unexpected rejection for {hostile:?}: {error}" + ); + } + } + + #[test] + fn a_block_policy_closes_both_families_and_accepts_its_allowlist() { + let plan = EgressPlan::for_policy(&request(NetworkPolicy::Block, &["10.0.0.0/8"], &[])) + .expect("a CIDR allowlist is enforceable"); + + assert_eq!(plan.render(), "4 ACCEPT 10.0.0.0/8 - -\n"); + assert_eq!(plan.v4_terminal(), RuleVerdict::Drop); + assert_eq!(plan.v6_terminal(), RuleVerdict::Drop); + } + + #[test] + fn an_allow_policy_keeps_both_families_open_and_drops_its_denylist() { + let plan = EgressPlan::for_policy(&request(NetworkPolicy::Allow, &[], &["192.0.2.0/24"])) + .expect("a CIDR denylist is enforceable"); + + assert_eq!(plan.render(), "4 DROP 192.0.2.0/24 - -\n"); + assert_eq!(plan.v4_terminal(), RuleVerdict::Accept); + assert_eq!(plan.v6_terminal(), RuleVerdict::Accept); + } + + /// The reported failure: a mapped `blockedHosts` entry under + /// `defaultPolicy: allow`. Classified as IPv6 it would be programmed into + /// `ip6tables`, where it never matches the IPv4 packet Linux actually + /// emits, and the v4 chain's terminal ACCEPT would let the traffic through. + #[test] + fn a_mapped_denylist_entry_is_programmed_on_the_family_that_carries_it() { + let plan = + EgressPlan::for_policy(&request(NetworkPolicy::Allow, &[], &["::ffff:192.0.2.5"])) + .expect("a mapped denylist entry is enforceable"); + + assert_eq!(plan.render(), "4 DROP 192.0.2.5 - -\n"); + assert_eq!(plan.v4_terminal(), RuleVerdict::Accept); + } + + /// D4: an explicit deny overrides a broader allow, which in an ordered + /// chain means the deny has to be appended first. + #[test] + fn denies_precede_allows_so_the_narrower_deny_wins() { + let plan = EgressPlan::for_policy(&request( + NetworkPolicy::Block, + &["0.0.0.0/0"], + &["192.0.2.1"], + )) + .expect("an allow-with-exception policy is enforceable"); + + assert_eq!( + plan.render(), + "4 DROP 192.0.2.1 - -\n4 ACCEPT 0.0.0.0/0 - -\n" + ); + } + + #[test] + fn a_v6_rule_is_tagged_for_ip6tables() { + let plan = EgressPlan::for_policy(&request(NetworkPolicy::Block, &["2001:db8::/32"], &[])) + .expect("a v6 CIDR is enforceable"); + + assert_eq!(plan.render(), "6 ACCEPT 2001:db8::/32 - -\n"); + } + + #[test] + fn a_named_host_fails_the_whole_plan() { + let error = EgressPlan::for_policy(&request( + NetworkPolicy::Block, + &["10.0.0.1", "api.github.com"], + &[], + )) + .expect_err("one unenforceable entry invalidates the policy"); + assert!(error.contains("api.github.com"), "{error}"); + } + + #[test] + fn the_proxy_plan_opens_exactly_one_endpoint() { + let plan = EgressPlan::for_proxy(Ipv4Addr::new(10, 0, 2, 2), 3128); + + assert_eq!(plan.render(), "4 ACCEPT 10.0.2.2 tcp 3128\n"); + assert_eq!(plan.v4_terminal(), RuleVerdict::Drop); + assert_eq!(plan.v6_terminal(), RuleVerdict::Drop); + } + + /// The budget must track the rule list, or a long allowlist would be given + /// the same time as a single endpoint. + #[test] + fn the_command_count_grows_with_the_rule_list() { + assert_eq!( + EgressPlan::for_proxy(Ipv4Addr::new(10, 0, 2, 2), 3128).command_count(), + BASE_RULE_COMMANDS + 1 + ); + + let plan = EgressPlan::for_policy(&request( + NetworkPolicy::Block, + &["10.0.0.0/8", "192.0.2.0/24"], + &["203.0.113.7"], + )) + .expect("a mixed policy is enforceable"); + assert_eq!(plan.command_count(), BASE_RULE_COMMANDS + 3); + } + + #[test] + fn a_policy_with_no_host_rules_renders_no_records() { + let plan = EgressPlan::for_policy(&request(NetworkPolicy::Block, &[], &[])) + .expect("a bare block policy is enforceable"); + + assert!(plan.render().is_empty()); + assert_eq!(plan.command_count(), BASE_RULE_COMMANDS); + } +} diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index be82ad290..cc521a973 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -23,6 +23,7 @@ use wxc_common::logger::Logger; use wxc_common::models::{ContainerPolicy, ProxyAddress, ProxyHostPin}; use crate::bwrap_command::COMMAND_TAIL; +use crate::network_rules::EgressPlan; const STARTUP_TIMEOUT: Duration = Duration::from_secs(5); /// How long a single `iptables` call may block on the host's `/run/xtables.lock`. @@ -35,22 +36,48 @@ 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. +/// Ceiling on the rule-installation budget. The per-command sum below is +/// caller-scaled and host lists are uncapped, so without this a large policy +/// could authorize hours of blocked startup. Covers ~50 fully contended +/// commands. +const RULE_INSTALL_TIMEOUT_CAP: Duration = Duration::from_secs(300); +/// Floor for the deadline clamp: slirp startup plus one contended call. Setup +/// has an irreducible cost, so a shorter workload timeout bounds the workload, +/// not the sandbox that has to exist to run it. +const RULE_INSTALL_TIMEOUT_FLOOR: Duration = + Duration::from_secs(STARTUP_TIMEOUT.as_secs() + XTABLES_LOCK_WAIT.as_secs()); + +/// Budget for the rule-installation phase, which begins once slirp is up. /// -/// 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(), -); +/// Sized so that lock contention -- the expected reason a rule stalls -- cannot +/// exhaust the budget before `-w` has had a chance to succeed. A single +/// [`STARTUP_TIMEOUT`] cannot cover this: it would reject a viable sandbox the +/// moment one call waited on a busy host. +/// +/// Computed from the plan rather than fixed, so a large rule list gets a +/// proportionate budget instead of being timed out for being large. On top of +/// the lock sum it carries [`STARTUP_TIMEOUT`] plus one second per command, +/// because the same deadline also spans slirp readiness and one `nsenter` +/// launch per rule. Headroom only -- the supervisor raises a single signal for +/// both phases, so they cannot be budgeted separately. +/// +/// Bounded twice, because this phase runs before the child's own timeout +/// exists: by [`RULE_INSTALL_TIMEOUT_CAP`], and by `script_timeout` when the +/// caller set one, down to [`RULE_INSTALL_TIMEOUT_FLOOR`]. +fn rule_install_timeout(plan: &EgressPlan, script_timeout_ms: u32) -> Duration { + let commands = plan.command_count() as u64; + let budget = Duration::from_secs( + XTABLES_LOCK_WAIT.as_secs() * commands + STARTUP_TIMEOUT.as_secs() + commands, + ) + .min(RULE_INSTALL_TIMEOUT_CAP); + + if script_timeout_ms == 0 { + return budget; + } + budget + .min(Duration::from_millis(u64::from(script_timeout_ms))) + .max(RULE_INSTALL_TIMEOUT_FLOOR) +} 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. @@ -84,10 +111,10 @@ const FD_STAGING_BASE: RawFd = 10; const SUPERVISOR_SCRIPT: &str = r#" set -eu state_dir="$1" -proxy_ip="$2" -proxy_port="$3" -chain="$4" -lock_wait="$5" +chain="$2" +lock_wait="$3" +v4_terminal="$4" +v6_terminal="$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 @@ -126,10 +153,16 @@ while [ ! -s "$state_dir/slirp.internal" ]; do 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. +# Per-destination filtering, programmed here because it needs CAP_NET_ADMIN in +# the owning user namespace, which this supervisor holds (`--keep-caps`) and +# the caller does not. +# +# Both families open with their chain and the loopback exemption, then take the +# caller's rules in order, then close on the terminal verdict and the OUTPUT +# hook. Loopback is exempt because it is the sandbox's own isolated loopback, +# so it never leaves the sandbox. Ordering is the policy: the chain matches +# first-wins, and the rules file is written deny-first so an explicit deny +# overrides a broader allow. # # -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 @@ -138,14 +171,32 @@ ns="/proc/$child_pid/ns/net" # 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 +# Five fixed fields per record, every one produced by re-rendering a parsed +# address rather than echoing caller text, so nothing here can carry shell +# metacharacters. `-` stands in for an absent protocol/port. +while IFS=' ' read -r family verdict address proto port; do + [ -n "$family" ] || continue + case "$family" in + 4) tool=iptables ;; + 6) tool=ip6tables ;; + *) echo "mxc: unrecognized rule family '$family'" >&2; exit 1 ;; + esac + if [ "$proto" = "-" ]; then + nsenter --net="$ns" -- "$tool" -w "$lock_wait" \ + -A "$chain" -d "$address" -j "$verdict" + else + nsenter --net="$ns" -- "$tool" -w "$lock_wait" \ + -A "$chain" -p "$proto" -d "$address" --dport "$port" -j "$verdict" + fi +done < "$state_dir/egress.rules" +# The terminal verdict closes each family. A family with no rules of its own +# still gets it, so IPv6 under an IPv4-only rule set fails closed rather than +# being left open. +nsenter --net="$ns" -- iptables -w "$lock_wait" -A "$chain" -j "$v4_terminal" +nsenter --net="$ns" -- iptables -w "$lock_wait" -A OUTPUT -j "$chain" +nsenter --net="$ns" -- ip6tables -w "$lock_wait" -A "$chain" -j "$v6_terminal" 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 @@ -173,6 +224,11 @@ impl ProxyEgress { pub(crate) fn pin(&self) -> Option<&ProxyHostPin> { self.pin.as_ref() } + + /// The filtering posture that opens this endpoint and nothing else. + pub(crate) fn plan(&self) -> EgressPlan { + EgressPlan::for_proxy(self.ip, self.port) + } } /// The proxy as the sandbox sees it: the URL handed to the workload, and the @@ -573,23 +629,38 @@ pub(crate) struct ProxyNetworkNamespace { userns: Option, /// Hosts file mounted over `/etc/hosts`, when the endpoint is a hostname. hosts: Option, + /// Budget for the readiness signal, sized from the plan's rule count. + rule_install_timeout: Duration, } 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. + /// `plan` is the complete filtering posture the sandbox runs under once the + /// supervisor signals readiness; anything it does not accept is dropped. + /// `pin` is the hosts-file entry the sandbox needs to agree with the plan, + /// which only a hostname proxy endpoint produces. /// /// Callers reach this only after `BwrapRunner::validate` has already run /// [`probe_dependencies`], so the probe is not repeated here. - pub(crate) fn start(egress: &ProxyEgress, logger: &mut Logger) -> Result { + pub(crate) fn start( + plan: &EgressPlan, + pin: Option<&ProxyHostPin>, + logger: &mut Logger, + script_timeout_ms: u32, + ) -> Result { let state_dir = tempfile::Builder::new() .prefix("mxc-bwrap-proxy-") .tempdir() .map_err(|error| { format!("Bubblewrap: failed to create proxy-network state: {error}") })?; + // Written before the supervisor is spawned: the script reads it during + // startup, so a missing or partial file must not be possible. + let rules_path = state_dir.path().join("egress.rules"); + std::fs::write(&rules_path, plan.render()).map_err(|error| { + format!("Bubblewrap: failed to write egress rules to {rules_path:?}: {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}") @@ -612,10 +683,10 @@ impl ProxyNetworkNamespace { "mxc-bwrap-proxy-supervisor", ]) .arg(state_dir.path()) - .arg(egress.ip.to_string()) - .arg(egress.port.to_string()) .arg(EGRESS_CHAIN) .arg(XTABLES_LOCK_WAIT.as_secs().to_string()) + .arg(plan.v4_terminal().target()) + .arg(plan.v6_terminal().target()) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::from(stderr)); @@ -659,7 +730,7 @@ impl ProxyNetworkNamespace { }; logger.log_line("Bubblewrap: created rootless proxy network namespace supervisor"); - let hosts = match egress.pin() { + let hosts = match pin { Some(pin) => { let path = state_dir.path().join("hosts"); if let Err(error) = write_pinned_hosts(&path, pin) { @@ -683,6 +754,7 @@ impl ProxyNetworkNamespace { pid_writer: Some(pid_writer), userns: Some(userns), hosts, + rule_install_timeout: rule_install_timeout(plan, script_timeout_ms), }) } @@ -767,11 +839,11 @@ impl ProxyNetworkNamespace { // 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, + self.rule_install_timeout, )?; logger.log_line( - "Bubblewrap: slirp4netns configured the private proxy namespace and proxy-only \ - egress rules are in force", + "Bubblewrap: slirp4netns configured the private network namespace and the egress \ + rules are in force", ); Ok(()) } @@ -1107,33 +1179,80 @@ fn insert_hosts_bind(args: &mut Vec, hosts_path: &str) -> Result Result<(), String> { +/// What pulled this request into a private network namespace. +/// +/// The dependency probe is shared by proxy-only egress and firewall +/// enforcement, but the remedy it should suggest is not: telling a caller who +/// set `enforcementMode: "firewall"` to "omit network.proxy" names a field they +/// never set. Carries the caller's own words into every probe message. +/// +/// The two are mutually exclusive — the parser rejects a proxy combined with a +/// firewall mode — so a request always maps to exactly one. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum PrivateNetworkUse { + ProxyOnlyEgress, + FirewallEnforcement, +} + +impl PrivateNetworkUse { + /// The config element that made the private namespace necessary. + fn requirement(self) -> &'static str { + match self { + Self::ProxyOnlyEgress => "network.proxy", + Self::FirewallEnforcement => "network.enforcementMode='firewall'", + } + } + + /// What the caller can drop to stop needing these dependencies. + fn remedy(self) -> &'static str { + match self { + Self::ProxyOnlyEgress => "omit network.proxy", + Self::FirewallEnforcement => "select a different network.enforcementMode", + } + } + + /// The mechanism the installed rules implement. + fn mechanism(self) -> &'static str { + match self { + Self::ProxyOnlyEgress => "proxy-only egress", + Self::FirewallEnforcement => "firewall enforcement", + } + } +} + +pub(crate) fn probe_dependencies(use_case: PrivateNetworkUse) -> Result<(), String> { // 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 // long after they did. + // + // Caching across use cases is safe because the probe asks the same + // questions either way -- only the wording of a failure differs, and + // failures are never cached. static PROBED: OnceLock<()> = OnceLock::new(); if PROBED.get().is_some() { return Ok(()); } - probe_dependencies_uncached()?; + probe_dependencies_uncached(use_case)?; let _ = PROBED.set(()); Ok(()) } -fn probe_dependencies_uncached() -> Result<(), String> { +fn probe_dependencies_uncached(use_case: PrivateNetworkUse) -> Result<(), String> { + let requirement = use_case.requirement(); 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." + "Bubblewrap: {requirement} requires 'slirp4netns' on PATH: {error}. \ + Install slirp4netns or {}.", + use_case.remedy() ) })?; if !slirp.status.success() { return Err(format!( - "Bubblewrap: network.proxy requires a working slirp4netns installation \ + "Bubblewrap: {requirement} requires a working slirp4netns installation \ (slirp4netns --version exited with {})", slirp.status )); @@ -1142,21 +1261,20 @@ fn probe_dependencies_uncached() -> Result<(), String> { 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}") + format!("Bubblewrap: {requirement} requires util-linux 'unshare' on PATH: {error}") })?; if !unshare.status.success() || !unshare.stdout.contains("--map-current-user") || !unshare.stdout.contains("--keep-caps") { - return Err( - "Bubblewrap: proxy networking requires util-linux unshare with \ + return Err(format!( + "Bubblewrap: {requirement} requires util-linux unshare with \ --map-current-user and --keep-caps support" - .into(), - ); + )); } - // Proxy-only egress is programmed with these, so a host missing them must - // fail here rather than deep inside supervisor startup. + // The in-namespace rules are 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), @@ -1166,13 +1284,13 @@ fn probe_dependencies_uncached() -> Result<(), String> { 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}" + "Bubblewrap: {requirement} requires '{binary}' on PATH to enforce {}: {error}", + use_case.mechanism() ) })?; if !output.status.success() { return Err(format!( - "Bubblewrap: network.proxy requires a working '{binary}' installation \ + "Bubblewrap: {requirement} requires a working '{binary}' installation \ ({binary} {probe} exited with {})", output.status )); @@ -1180,7 +1298,12 @@ fn probe_dependencies_uncached() -> Result<(), String> { // 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))?; + iptables_backend_is_usable( + binary, + &output.stdout, + Path::new(XTABLES_LOCK_PATH), + use_case, + )?; } } Ok(()) @@ -1198,7 +1321,12 @@ fn probe_dependencies_uncached() -> Result<(), String> { /// 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> { +fn iptables_backend_is_usable( + binary: &str, + banner: &str, + lock: &Path, + use_case: PrivateNetworkUse, +) -> Result<(), String> { if banner.contains("nf_tables") { return Ok(()); } @@ -1208,15 +1336,17 @@ fn iptables_backend_is_usable(binary: &str, banner: &str, lock: &Path) -> Result // 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 \ + "Bubblewrap: {} 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 \ + '{}' is not writable by this user. The {} 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.", + use_case.requirement(), banner.trim(), lock.display(), + use_case.mechanism(), lock.display() )) } @@ -1974,15 +2104,20 @@ mod tests { } #[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"#); + fn script_accepts_loopback_before_applying_any_rule() { + // iptables is first-match, so the ordering *is* the policy: loopback is + // exempted first, the caller's rules are applied in the order the plan + // holds them, and only then does the terminal verdict close the chain. + // A terminal appended ahead of the loop would black-hole every rule. let loopback = script_offset(r#"iptables -A "$chain" -o lo -j ACCEPT"#); - let drop = script_offset(r#"iptables -A "$chain" -j DROP"#); + let loop_start = script_offset("while IFS=' ' read -r family verdict"); + let terminal = script_offset(r#"-A "$chain" -j "$v4_terminal""#); - assert!(loopback < accept, "loopback accept must come first"); - assert!(accept < drop, "proxy accept must precede the closing drop"); + assert!(loopback < loop_start, "loopback accept must come first"); + assert!( + loop_start < terminal, + "the caller's rules must precede the terminal verdict" + ); } #[test] @@ -1999,10 +2134,11 @@ mod tests { } #[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. + fn the_script_hardcodes_no_accept_but_loopback() { + // Every other accept must come from the plan, which is what makes the + // posture auditable in one place. A hardcoded exemption here -- a port + // 53 accept being the obvious temptation -- would be invisible to the + // rule model and to every policy test written against it. let accepts: Vec = normalised_script() .lines() .filter(|line| line.contains("-j ACCEPT")) @@ -2013,22 +2149,35 @@ mod tests { 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" + "only the sandbox's own loopback may be accepted outside the plan" ); } #[test] - fn script_fails_ipv6_closed() { - let v6_drop = script_offset(r#"ip6tables -A "$chain" -j DROP"#); + fn the_script_hardcodes_no_destination_of_its_own() { + // Destinations reach iptables only through the rules file, whose every + // address was re-rendered from a parsed one. A `-d` written into the + // script would bypass that and could carry anything. + for line in normalised_script().lines() { + assert!( + !line.contains(" -d ") || line.contains(r#"-d "$address""#), + "the script names a destination that did not come from the rules file: {line}" + ); + } + } + + #[test] + fn script_closes_ipv6_on_its_own_terminal() { + // v6 gets its own terminal verdict rather than sharing v4's chain, so + // a v4-only rule set still closes v6 instead of leaving it open. + let v6_terminal = script_offset(r#"ip6tables -A "$chain" -j "$v6_terminal""#); 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" + v6_terminal < v6_hook, + "the v6 chain must be closed before being hooked" ); } @@ -2265,20 +2414,38 @@ mod tests { /// 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 + let code: Vec<&str> = SUPERVISOR_SCRIPT .lines() - .filter(|line| { - let code = line.trim(); - !code.starts_with('#') && code.contains("tables ") - }) + .map(str::trim) + .filter(|line| !line.starts_with('#')) .collect(); + // The fixed scaffolding: per family a chain creation, the loopback + // exemption, the terminal verdict and the OUTPUT hook. Matched on the + // invocation, so the loop's `tool=iptables` assignments do not count. + let fixed: Vec<&&str> = code + .iter() + .filter(|line| line.contains("-- iptables ") || line.contains("-- ip6tables ")) + .collect(); assert_eq!( - rules.len() as u32, - RULE_COMMAND_COUNT, - "RULE_COMMAND_COUNT is stale, so the rule-install budget is wrong" + fixed.len() as u32, + crate::network_rules::BASE_RULE_COMMANDS, + "BASE_RULE_COMMANDS is stale, so the rule-install budget is wrong" ); - for rule in rules { + + // The per-rule calls, which reach iptables or ip6tables through the + // family the record selects. + let dispatched: Vec<&&str> = code + .iter() + .filter(|line| line.contains(r#"-- "$tool""#)) + .collect(); + assert_eq!( + dispatched.len(), + 2, + "the rule loop should have exactly a ported and an unported branch" + ); + + for rule in fixed.into_iter().chain(dispatched) { assert!( rule.contains(r#"-w "$lock_wait""#), "rule may fail instantly on a contended host lock: {rule}" @@ -2290,22 +2457,90 @@ mod tests { /// 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; + for plan in [ + EgressPlan::for_proxy(Ipv4Addr::new(10, 1, 2, 3), 3128), + plan_with_rule_count(0), + plan_with_rule_count(25), + ] { + let contended = XTABLES_LOCK_WAIT * plan.command_count(); + let budget = rule_install_timeout(&plan, 0); + assert!( + budget > contended, + "a fully contended host would time out before -w could succeed" + ); + assert!( + budget - contended >= STARTUP_TIMEOUT, + "the budget leaves no room for slirp startup on top of lock contention" + ); + assert!( + budget - contended - STARTUP_TIMEOUT + >= Duration::from_secs(plan.command_count() as u64), + "the budget leaves no room to spawn one nsenter per rule" + ); + } + } + + /// A plan whose rule count is what the test cares about. Sized through the + /// public constructor so the count matches what the supervisor installs. + fn plan_with_rule_count(count: usize) -> EgressPlan { + let mut request = wxc_common::models::ExecutionRequest { + schema_version: "0.8.0-alpha".into(), + ..Default::default() + }; + request.policy.allowed_hosts = (0..count) + .map(|n| format!("10.{}.{}.{}", (n >> 16) & 0xff, (n >> 8) & 0xff, n & 0xff)) + .collect(); + EgressPlan::for_policy(&request).expect("literal addresses must build a plan") + } + + /// The budget grows with the rule list, so a large policy is not timed out + /// for being large. + #[test] + fn rule_install_budget_grows_with_the_rule_list() { assert!( - RULE_INSTALL_TIMEOUT > contended, - "a fully contended host would time out before -w could succeed" + rule_install_timeout(&plan_with_rule_count(50), 0) + > rule_install_timeout(&plan_with_rule_count(1), 0) ); + } + + /// Host lists are uncapped and this budget is consumed before the child's + /// own timeout exists, so an unbounded sum would authorize hours of startup. + #[test] + fn rule_install_budget_is_capped_for_a_pathological_rule_list() { + let budget = rule_install_timeout(&plan_with_rule_count(10_000), 0); + assert_eq!(budget, RULE_INSTALL_TIMEOUT_CAP); + // The uncapped sum is what makes the cap load-bearing. assert!( - RULE_INSTALL_TIMEOUT - contended >= STARTUP_TIMEOUT, - "the budget leaves no room for slirp startup on top of lock contention" + Duration::from_secs(10_000 * XTABLES_LOCK_WAIT.as_secs()) > RULE_INSTALL_TIMEOUT_CAP ); - 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 rule_install_budget_never_outlasts_the_requested_deadline() { + // 30s workload deadline against a list whose uncapped budget exceeds it. + let plan = plan_with_rule_count(25); + let deadline = Duration::from_secs(30); + assert!(rule_install_timeout(&plan, 0) > deadline, "premise"); + assert_eq!(rule_install_timeout(&plan, 30_000), deadline); + } + + #[test] + fn the_deadline_clamp_stops_at_the_startup_floor() { + // A 1s workload timeout cannot starve slirp startup: setup has an + // irreducible cost, and clamping to it would fail every sandbox. + let plan = plan_with_rule_count(25); + assert_eq!( + rule_install_timeout(&plan, 1_000), + RULE_INSTALL_TIMEOUT_FLOOR ); } + #[test] + fn an_absent_deadline_leaves_the_budget_unclamped() { + let plan = plan_with_rule_count(5); + assert!(rule_install_timeout(&plan, 0) > RULE_INSTALL_TIMEOUT_FLOOR); + } + #[test] fn probe_gives_up_on_a_hung_binary_instead_of_blocking_forever() { let mut command = Command::new("sleep"); @@ -2373,7 +2608,8 @@ mod tests { iptables_backend_is_usable( "iptables", "iptables v1.8.10 (nf_tables)\n", - &unreachable_lock(&dir) + &unreachable_lock(&dir), + PrivateNetworkUse::ProxyOnlyEgress ) .is_ok(), "the nf_tables backend takes no lock, so an unreachable lock must not refuse it" @@ -2390,6 +2626,7 @@ mod tests { "iptables", "iptables v1.8.10 (legacy)\n", &unreachable_lock(&dir), + PrivateNetworkUse::ProxyOnlyEgress, ) .expect_err("a legacy backend with an unreachable lock cannot install rules"); @@ -2399,6 +2636,68 @@ mod tests { ); } + /// Firewall enforcement shares this probe with proxy-only egress, so a + /// missing dependency used to advise a caller to "omit network.proxy" -- + /// a field a firewall-only request never set. The advice must name what + /// the caller actually configured. + #[test] + fn a_firewall_request_is_never_advised_about_the_proxy() { + let dir = tempfile::tempdir().expect("tempdir"); + + let error = iptables_backend_is_usable( + "iptables", + "iptables v1.8.10 (legacy)\n", + &unreachable_lock(&dir), + PrivateNetworkUse::FirewallEnforcement, + ) + .expect_err("a legacy backend with an unreachable lock cannot install rules"); + + assert!( + !error.contains("network.proxy"), + "a firewall-only request must not be told about a field it never set: {error}" + ); + assert!( + error.contains("network.enforcementMode='firewall'"), + "the error must name what the caller configured: {error}" + ); + } + + /// The mirror image: the proxy wording must not drift to firewall terms. + #[test] + fn a_proxy_request_is_never_advised_about_the_enforcement_mode() { + let dir = tempfile::tempdir().expect("tempdir"); + + let error = iptables_backend_is_usable( + "iptables", + "iptables v1.8.10 (legacy)\n", + &unreachable_lock(&dir), + PrivateNetworkUse::ProxyOnlyEgress, + ) + .expect_err("a legacy backend with an unreachable lock cannot install rules"); + + assert!( + error.contains("network.proxy") && !error.contains("enforcementMode"), + "a proxy request must keep the proxy wording: {error}" + ); + } + + /// Every use case must produce a distinct, non-empty vocabulary -- an empty + /// or shared string would silently reintroduce the mismatch above. + #[test] + fn each_private_network_use_names_itself_distinctly() { + let proxy = PrivateNetworkUse::ProxyOnlyEgress; + let firewall = PrivateNetworkUse::FirewallEnforcement; + + for (a, b) in [ + (proxy.requirement(), firewall.requirement()), + (proxy.remedy(), firewall.remedy()), + (proxy.mechanism(), firewall.mechanism()), + ] { + assert!(!a.is_empty() && !b.is_empty(), "wording must not be empty"); + assert_ne!(a, b, "the two use cases must not share wording"); + } + } + /// 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] @@ -2408,7 +2707,13 @@ mod tests { fs::write(&lock, b"").expect("create lock"); assert!( - iptables_backend_is_usable("iptables", "iptables v1.8.10 (legacy)\n", &lock).is_ok(), + iptables_backend_is_usable( + "iptables", + "iptables v1.8.10 (legacy)\n", + &lock, + PrivateNetworkUse::ProxyOnlyEgress + ) + .is_ok(), "a legacy backend that can take its lock installs rules fine" ); } @@ -2432,8 +2737,13 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); assert!( - iptables_backend_is_usable("iptables", "iptables v1.6.1\n", &unreachable_lock(&dir)) - .is_err(), + iptables_backend_is_usable( + "iptables", + "iptables v1.6.1\n", + &unreachable_lock(&dir), + PrivateNetworkUse::ProxyOnlyEgress + ) + .is_err(), "a pre-1.8 build is legacy-only and must not be assumed usable" ); } @@ -2479,6 +2789,7 @@ mod tests { count=$(cat "$MXC_TEST_COUNT" 2>/dev/null || echo 0) count=$((count + 1)) printf '%s' "$count" > "$MXC_TEST_COUNT" +printf '%s\n' "$*" >> "$MXC_TEST_ARGS" if [ "${MXC_TEST_FAIL_AT:-0}" = "$count" ]; then echo "fake nsenter: forced failure on rule $count" >&2 exit 1 @@ -2529,6 +2840,31 @@ exec sleep 30 self.state_dir().join("slirp.ready").exists() } + /// The argument vector of each `nsenter` call, in the order made. + /// + /// The count alone cannot distinguish a correct chain from one whose + /// rules carry the wrong address, verdict or order, so the arguments + /// that actually reached iptables are what the policy tests assert on. + fn rule_log(&self) -> Vec { + fs::read_to_string(self.dir.path().join("rules.args")) + .unwrap_or_default() + .lines() + .map(str::to_owned) + .collect() + } + + /// The `-A ` rules, stripped to the part that expresses policy. + fn chain_rules(&self) -> Vec { + self.rule_log() + .into_iter() + .filter_map(|line| { + let (tool, rest) = line.split_once(" -w 1 -A mxc-test-chain ")?; + let tool = tool.rsplit(' ').next()?; + Some(format!("{tool} {rest}")) + }) + .collect() + } + fn stderr(&self) -> String { stderr_detail(&self.dir.path().join("supervisor.stderr")) } @@ -2608,11 +2944,24 @@ exec sleep 30 } fn spawn_fake_supervisor(fail_at: Option, slirp_dies: bool) -> FakeSupervisor { + spawn_fake_supervisor_with_plan( + &EgressPlan::for_proxy(SLIRP_HOST_GATEWAY_IP, 3128), + fail_at, + slirp_dies, + ) + } + + fn spawn_fake_supervisor_with_plan( + plan: &EgressPlan, + 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"); + fs::write(state.join("egress.rules"), plan.render()).expect("rules file"); install_stub(&bin.join("nsenter"), FAKE_NSENTER); install_stub(&bin.join("slirp4netns"), FAKE_SLIRP); @@ -2631,12 +2980,13 @@ exec sleep 30 command .args(["-c", SUPERVISOR_SCRIPT, "mxc-test-supervisor"]) .arg(&state) - .arg("10.0.2.2") - .arg("3128") .arg("mxc-test-chain") .arg("1") + .arg(plan.v4_terminal().target()) + .arg(plan.v6_terminal().target()) .env("PATH", path) .env("MXC_TEST_COUNT", dir.path().join("rules.count")) + .env("MXC_TEST_ARGS", dir.path().join("rules.args")) .env("MXC_TEST_SLIRP_PID", dir.path().join("slirp.pid")) .env( "MXC_TEST_FAIL_AT", @@ -2665,24 +3015,45 @@ exec sleep 30 } } - /// Executing the script also re-checks [`RULE_COMMAND_COUNT`] against the + /// Executing the script also re-checks the 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 plan = EgressPlan::for_proxy(SLIRP_HOST_GATEWAY_IP, 3128); let mut supervisor = spawn_fake_supervisor(None, false); supervisor.publish_sandbox_pid(); supervisor.wait_until_ready(); assert_eq!( supervisor.rule_invocations(), - RULE_COMMAND_COUNT, + plan.command_count(), "readiness was signalled after a different number of rules than the \ startup budget is sized for; stderr: {}", supervisor.stderr() ); } + /// A multi-rule policy must install every one of its rules before the + /// sandbox is released, not just the fixed scaffolding. + #[test] + fn a_policy_plan_installs_one_command_per_rule() { + for rule_count in [0, 1, 5] { + let plan = plan_with_rule_count(rule_count); + let mut supervisor = spawn_fake_supervisor_with_plan(&plan, None, false); + supervisor.publish_sandbox_pid(); + supervisor.wait_until_ready(); + + assert_eq!( + supervisor.rule_invocations(), + crate::network_rules::BASE_RULE_COMMANDS + rule_count as u32, + "a {rule_count}-rule policy installed the wrong number of commands; \ + 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. @@ -2693,8 +3064,11 @@ exec sleep 30 /// 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); + // Every position, including the per-rule ones inside the loop: a body + // that escapes `set -e` would start an unenforced sandbox. + let plan = plan_with_rule_count(3); + for rule in 1..=plan.command_count() { + let mut supervisor = spawn_fake_supervisor_with_plan(&plan, Some(rule), false); supervisor.publish_sandbox_pid(); let status = supervisor.wait_for_exit(); @@ -2716,6 +3090,120 @@ exec sleep 30 } } + /// The chain the workload actually runs under, for a policy the reviewer + /// can read off the config. Ordering is asserted as a whole sequence + /// because in a first-match chain a correct set of rules in the wrong order + /// is a different policy. + #[test] + fn a_block_policy_accepts_only_its_allowlist_and_closes_both_families() { + let mut request = wxc_common::models::ExecutionRequest::default(); + request.policy.default_network_policy = wxc_common::models::NetworkPolicy::Block; + request.policy.allowed_hosts = vec!["203.0.113.7".into(), "2001:db8::/32".into()]; + let plan = EgressPlan::for_policy(&request).expect("literals must build a plan"); + + let mut supervisor = spawn_fake_supervisor_with_plan(&plan, None, false); + supervisor.publish_sandbox_pid(); + supervisor.wait_until_ready(); + + assert_eq!( + supervisor.chain_rules(), + vec![ + "iptables -o lo -j ACCEPT".to_string(), + "ip6tables -o lo -j ACCEPT".to_string(), + "iptables -d 203.0.113.7 -j ACCEPT".to_string(), + "ip6tables -d 2001:db8::/32 -j ACCEPT".to_string(), + "iptables -j DROP".to_string(), + "ip6tables -j DROP".to_string(), + ], + "stderr: {}", + supervisor.stderr() + ); + } + + /// D4: an explicit deny beats a broader allow. In an ordered chain that is + /// purely a question of which rule is appended first, so it is asserted + /// against what reached iptables rather than against the plan alone. + #[test] + fn an_allow_policy_denies_its_blocklist_before_the_open_terminal() { + let mut request = wxc_common::models::ExecutionRequest::default(); + request.policy.default_network_policy = wxc_common::models::NetworkPolicy::Allow; + request.policy.blocked_hosts = vec!["198.51.100.0/24".into()]; + request.policy.allowed_hosts = vec!["198.51.100.9".into()]; + let plan = EgressPlan::for_policy(&request).expect("literals must build a plan"); + + let mut supervisor = spawn_fake_supervisor_with_plan(&plan, None, false); + supervisor.publish_sandbox_pid(); + supervisor.wait_until_ready(); + + let rules = supervisor.chain_rules(); + let deny = rules + .iter() + .position(|rule| rule == "iptables -d 198.51.100.0/24 -j DROP") + .expect("the deny must be installed"); + let allow = rules + .iter() + .position(|rule| rule == "iptables -d 198.51.100.9 -j ACCEPT") + .expect("the allow must be installed"); + let terminal = rules + .iter() + .position(|rule| rule == "iptables -j ACCEPT") + .expect("an allow policy must end in ACCEPT"); + + assert!( + deny < allow, + "the narrower deny must be evaluated first, or it never matches: {rules:?}" + ); + assert!( + allow < terminal, + "the open terminal must come last: {rules:?}" + ); + } + + /// The proxy posture must be byte-for-byte what it was before the rule list + /// generalised it: this path is already reviewed and already shipping. + #[test] + fn the_proxy_posture_is_unchanged_by_the_rule_list() { + let plan = EgressPlan::for_proxy(Ipv4Addr::new(10, 1, 2, 3), 3128); + let mut supervisor = spawn_fake_supervisor_with_plan(&plan, None, false); + supervisor.publish_sandbox_pid(); + supervisor.wait_until_ready(); + + assert_eq!( + supervisor.chain_rules(), + vec![ + "iptables -o lo -j ACCEPT".to_string(), + "ip6tables -o lo -j ACCEPT".to_string(), + "iptables -p tcp -d 10.1.2.3 --dport 3128 -j ACCEPT".to_string(), + "iptables -j DROP".to_string(), + "ip6tables -j DROP".to_string(), + ], + "stderr: {}", + supervisor.stderr() + ); + } + + /// An IPv4-only rule set must still close IPv6, or the sandbox keeps a + /// silent v6 exit that no rule in the config mentions. + #[test] + fn a_v4_only_policy_still_closes_ipv6() { + let mut request = wxc_common::models::ExecutionRequest::default(); + request.policy.default_network_policy = wxc_common::models::NetworkPolicy::Block; + request.policy.allowed_hosts = vec!["203.0.113.7".into()]; + let plan = EgressPlan::for_policy(&request).expect("literals must build a plan"); + + let mut supervisor = spawn_fake_supervisor_with_plan(&plan, None, false); + supervisor.publish_sandbox_pid(); + supervisor.wait_until_ready(); + + assert!( + supervisor + .chain_rules() + .contains(&"ip6tables -j DROP".to_string()), + "IPv6 was left open by a v4-only policy: {:?}", + supervisor.chain_rules() + ); + } + #[test] fn the_supervisor_gives_up_when_slirp_dies_before_signalling_readiness() { let mut supervisor = spawn_fake_supervisor(None, true); diff --git a/tests/configs/bubblewrap_network_firewall.json b/tests/configs/bubblewrap_network_firewall.json index 4771cf915..2f0c30f81 100644 --- a/tests/configs/bubblewrap_network_firewall.json +++ b/tests/configs/bubblewrap_network_firewall.json @@ -3,7 +3,7 @@ "containerId": "CLI-Bubblewrap-Network-Firewall", "containment": "bubblewrap", "process": { - "commandLine": "wget -qO- https://api.github.com/zen" + "commandLine": "bash -c 'set -u; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); echo LEGACY_FIREWALL_OK'" }, "network": { "defaultPolicy": "block", diff --git a/tests/configs/bubblewrap_network_firewall_cidr.json b/tests/configs/bubblewrap_network_firewall_cidr.json new file mode 100644 index 000000000..f7e566d59 --- /dev/null +++ b/tests/configs/bubblewrap_network_firewall_cidr.json @@ -0,0 +1,13 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Firewall-CIDR", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'set -u; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); if ! timeout 6 bash -c \"exec 3<>/dev/tcp/10.0.2.2/{{ALLOWED_PORT}}\" >/dev/null 2>&1; then echo ALLOWED_DEST_UNREACHABLE; exit 1; fi; echo ALLOWED_DEST_OK; timeout 6 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1; if [ $? = 0 ]; then echo DENIED_DEST_LEAKED; exit 1; fi; echo DENIED_DEST_BLOCKED_OK; timeout 4 bash -c \"exec 3<>/dev/tcp/127.0.0.1/9\" >/dev/null 2>&1; if [ $? = 124 ]; then echo LOOPBACK_DROPPED; exit 1; fi; echo LOOPBACK_EXEMPT_OK; capeff=$(grep \"^CapEff\" /proc/self/status | cut -f2); if [ $((0x$capeff & 0x1000)) -ne 0 ]; then echo CAP_NET_ADMIN_RETAINED; exit 1; fi; echo CAP_NET_ADMIN_DROPPED_OK; iptables -F MXC_EGRESS >/dev/null 2>&1; if [ $? = 0 ]; then echo TAMPER_FLUSH_SUCCEEDED; exit 1; fi; echo TAMPER_REFUSED_OK; 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'" + }, + "network": { + "defaultPolicy": "block", + "enforcementMode": "firewall", + "allowedHosts": ["10.0.2.2/32"] + } +} diff --git a/tests/configs/bubblewrap_network_firewall_denylist.json b/tests/configs/bubblewrap_network_firewall_denylist.json new file mode 100644 index 000000000..3fc4af860 --- /dev/null +++ b/tests/configs/bubblewrap_network_firewall_denylist.json @@ -0,0 +1,14 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Firewall-Denylist", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'set -u; if ! timeout 6 bash -c \"exec 3<>/dev/tcp/10.0.2.2/{{ALLOWED_PORT}}\" >/dev/null 2>&1; then echo OPEN_TERMINAL_UNREACHABLE; exit 1; fi; echo OPEN_TERMINAL_OK; timeout 6 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1; if [ $? = 0 ]; then echo DENY_OVERRIDDEN_BY_ALLOW; exit 1; fi; echo DENY_WINS_OK'" + }, + "network": { + "defaultPolicy": "allow", + "enforcementMode": "firewall", + "allowedHosts": ["1.1.1.0/24"], + "blockedHosts": ["1.1.1.1/32"] + } +} diff --git a/tests/configs/bubblewrap_network_firewall_hostname_rejected.json b/tests/configs/bubblewrap_network_firewall_hostname_rejected.json new file mode 100644 index 000000000..a27546450 --- /dev/null +++ b/tests/configs/bubblewrap_network_firewall_hostname_rejected.json @@ -0,0 +1,13 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Firewall-Hostname-Rejected", + "containment": "bubblewrap", + "process": { + "commandLine": "echo FIREWALL_HOSTNAME_RAN" + }, + "network": { + "defaultPolicy": "block", + "enforcementMode": "firewall", + "allowedHosts": ["api.github.com"] + } +} diff --git a/tests/configs/bubblewrap_network_hostrules_unenforced_rejected.json b/tests/configs/bubblewrap_network_hostrules_unenforced_rejected.json new file mode 100644 index 000000000..aa512d51f --- /dev/null +++ b/tests/configs/bubblewrap_network_hostrules_unenforced_rejected.json @@ -0,0 +1,13 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Host-Rules-Unenforced", + "containment": "bubblewrap", + "process": { + "commandLine": "echo SHOULD_NOT_RUN" + }, + "network": { + "defaultPolicy": "block", + "enforcementMode": "capabilities", + "allowedHosts": ["203.0.113.7"] + } +} diff --git a/tests/configs/bubblewrap_network_localnet_acknowledged.json b/tests/configs/bubblewrap_network_localnet_acknowledged.json new file mode 100644 index 000000000..d4ab0f522 --- /dev/null +++ b/tests/configs/bubblewrap_network_localnet_acknowledged.json @@ -0,0 +1,12 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-LocalNet-Acknowledged", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'set -u; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); echo LOCALNET_ACK_OK'" + }, + "network": { + "defaultPolicy": "allow", + "allowLocalNetwork": true + } +} diff --git a/tests/configs/bubblewrap_network_localnet_legacy.json b/tests/configs/bubblewrap_network_localnet_legacy.json new file mode 100644 index 000000000..532ee32d3 --- /dev/null +++ b/tests/configs/bubblewrap_network_localnet_legacy.json @@ -0,0 +1,12 @@ +{ + "version": "0.7.0-alpha", + "containerId": "CLI-Bubblewrap-LocalNet-Legacy", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'set -u; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); echo LEGACY_LOCALNET_OK'" + }, + "network": { + "defaultPolicy": "allow", + "allowLocalNetwork": false + } +} diff --git a/tests/configs/bubblewrap_network_localnet_omitted_rejected.json b/tests/configs/bubblewrap_network_localnet_omitted_rejected.json new file mode 100644 index 000000000..aeccb7526 --- /dev/null +++ b/tests/configs/bubblewrap_network_localnet_omitted_rejected.json @@ -0,0 +1,11 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-LocalNet-Omitted-Rejected", + "containment": "bubblewrap", + "process": { + "commandLine": "echo LOCALNET_SHOULD_NOT_RUN" + }, + "network": { + "defaultPolicy": "allow" + } +} diff --git a/tests/configs/bubblewrap_network_localnet_private_rejected.json b/tests/configs/bubblewrap_network_localnet_private_rejected.json new file mode 100644 index 000000000..fd897000e --- /dev/null +++ b/tests/configs/bubblewrap_network_localnet_private_rejected.json @@ -0,0 +1,12 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-LocalNet-Private-Rejected", + "containment": "bubblewrap", + "process": { + "commandLine": "echo LOCALNET_SHOULD_NOT_RUN" + }, + "network": { + "defaultPolicy": "block", + "allowLocalNetwork": true + } +} diff --git a/tests/configs/bubblewrap_network_localnet_shared_rejected.json b/tests/configs/bubblewrap_network_localnet_shared_rejected.json new file mode 100644 index 000000000..ebe6a49d2 --- /dev/null +++ b/tests/configs/bubblewrap_network_localnet_shared_rejected.json @@ -0,0 +1,12 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-LocalNet-Shared-Rejected", + "containment": "bubblewrap", + "process": { + "commandLine": "echo LOCALNET_SHOULD_NOT_RUN" + }, + "network": { + "defaultPolicy": "allow", + "allowLocalNetwork": false + } +} diff --git a/tests/configs/bubblewrap_network_proxy_host_rules.json b/tests/configs/bubblewrap_network_proxy_host_rules.json new file mode 100644 index 000000000..e53e14c14 --- /dev/null +++ b/tests/configs/bubblewrap_network_proxy_host_rules.json @@ -0,0 +1,13 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Proxy-Host-Rules", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'set -u; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); if curl -fsS --noproxy \"*\" --max-time 8 http://example.com/ >/dev/null 2>&1; then echo DIRECT_EGRESS_LEAKED; exit 1; fi; echo DIRECT_EGRESS_BLOCKED_OK; if ! curl -fsSL --max-time 15 https://api.github.com/zen >/dev/null 2>&1; then echo PROXY_PATH_UNREACHABLE; exit 1; fi; echo PROXY_PATH_OK'" + }, + "network": { + "defaultPolicy": "block", + "proxy": { "builtinTestServer": true }, + "allowedHosts": ["api.github.com"] + } +} diff --git a/tests/scripts/run_bwrap_all_tests.sh b/tests/scripts/run_bwrap_all_tests.sh index 3209c57dc..8c1ef5897 100644 --- a/tests/scripts/run_bwrap_all_tests.sh +++ b/tests/scripts/run_bwrap_all_tests.sh @@ -40,6 +40,8 @@ run_test "Bubblewrap Most-Specific Path" "$SCRIPT_DIR/run_bwrap_most_specific_te run_test "Bubblewrap Denied Masking" "$SCRIPT_DIR/run_bwrap_denied_masking_test.sh" run_test "Bubblewrap Network Block" "$SCRIPT_DIR/run_bwrap_network_test.sh" run_test "Bubblewrap Network Proxy" "$SCRIPT_DIR/run_bwrap_network_proxy_test.sh" +run_test "Bubblewrap Network Firewall" "$SCRIPT_DIR/run_bwrap_firewall_test.sh" +run_test "Bubblewrap allowLocalNetwork" "$SCRIPT_DIR/run_bwrap_localnet_test.sh" run_test "Linux Process Default" "$SCRIPT_DIR/run_linux_process_default_test.sh" echo "================================" diff --git a/tests/scripts/run_bwrap_firewall_test.sh b/tests/scripts/run_bwrap_firewall_test.sh new file mode 100644 index 000000000..7043c96c9 --- /dev/null +++ b/tests/scripts/run_bwrap_firewall_test.sh @@ -0,0 +1,320 @@ +#!/bin/bash +# Bubblewrap firewall-enforcement tests (schema 0.8+), and the pre-0.8 twin. +# +# Most cases here do NOT require root: enforcement runs inside the sandbox's own +# network namespace, where the supervisor holds CAP_NET_ADMIN through an +# unprivileged user namespace. The exception is the pre-0.8 compatibility case +# at the end, which exercises the legacy HOST-side chain and so needs privilege +# to run to completion. +# +# The point of this file is that the rules are *enforced*, not merely emitted. +# Pre-0.8 firewall mode builds a host chain the sandbox never traverses, so a +# test that only checked "the run succeeded" passed against a sandbox with no +# filtering at all. Every enforcement case here therefore asserts a destination +# that must be reachable and one that must not. +# +# The other half of the contract is that 0.8 changed nothing for existing +# callers: GHCP consumes Bubblewrap on 0.6/0.7. The 0.8 restrictions are +# therefore paired with a pre-0.8 twin asserting the old behavior still holds, +# since a test that only proves the new rejection cannot tell "correctly gated" +# from "rejects everyone". The allowLocalNetwork axis is paired the same way in +# run_bwrap_localnet_test.sh. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_DIR="$(dirname "$(dirname "$SCRIPT_DIR")")" +# An explicitly set LXC_EXEC is taken literally: falling back from it would +# silently exercise a different binary than the caller named. +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 + fi +fi + +HOST_NETNS="$(readlink /proc/self/ns/net)" + +# Schema <= 0.7 must be untouched. Those callers pass hostnames by construction +# and run today against a host chain that filters nothing; the 0.8 work must +# neither reject them nor change what they get. This asserts the compatibility +# promise, not that the legacy path is secure -- it is not, which is why the +# enforcing behavior is 0.8-only. +# +# Deliberately ahead of the slirp4netns and reachability gates below. The +# legacy path needs neither -- it shares the host namespace and this config's +# workload makes no connection -- so gating it behind them would drop the +# compatibility check entirely on hosts that skip the enforcement tests, which +# are exactly the hosts where nothing else would notice a gate leak. +# +# The outcome is deliberately tri-state rather than a flat "must succeed". +# Pre-0.8 firewall mode is the one legacy path that installs HOST-side rules, +# so `apply_firewall_rules` needs CAP_NET_ADMIN and the run legitimately fails +# unprivileged -- which is why this case used to be written as `|| true`. That +# swallowed everything, including a panic or a missing binary, so the assertion +# passed against a build that never ran the sandbox at all. Splitting the +# outcomes keeps the privilege tolerance without keeping the blind spot: a +# config rejection always fails, a privilege failure is tolerated but named, +# a success is held to the sentinel and the namespace, and anything else fails. +echo "Running Bubblewrap firewall test: pre-0.8 is unchanged..." +# Probed up front: the legacy path does not pin the iptables locale, and the +# runner's generic error wrapper would excuse any apply failure on a +# privileged runner, so neither is a dependable discriminator. +HOST_IPTABLES_OK=0 +if iptables -w 2 -S >/dev/null 2>&1; then + HOST_IPTABLES_OK=1 +fi +LEGACY_RC=0 +LEGACY_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_firewall.json" 2>&1) || LEGACY_RC=$? + +# Any of these means a 0.8 gate leaked into the legacy schema. Checked first +# and unconditionally, because this is the invariant the case exists for and +# it is the one assertion that holds with or without privilege. +for marker in \ + "not an IP address or CIDR" \ + "require an enforcement mechanism" \ + "is not enforced while the sandbox shares" \ + "Configuration parse error"; do + if grep -qF "$marker" <<<"$LEGACY_OUT"; then + echo "$LEGACY_OUT" + echo "FAIL: pre-0.8 firewall (inherited a 0.8 rejection: '$marker')" + exit 1 + fi +done + +if [ "$LEGACY_RC" = 0 ]; then + # Privileged run: the legacy path went all the way through, so the + # behavior itself is assertable. FirewallFiltered is not a private-netns + # mode, so sharing the host namespace is the pin -- if a future change + # routed pre-0.8 firewall configs through the enforcing path, the run would + # still exit 0 and only this check would notice. + if ! grep -q LEGACY_FIREWALL_OK <<<"$LEGACY_OUT"; then + echo "$LEGACY_OUT" + echo "FAIL: pre-0.8 firewall (succeeded without running the workload)" + 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: pre-0.8 firewall (expected the host namespace $HOST_NETNS, got $LEGACY_NETNS)" + exit 1 + fi + echo "PASS: pre-0.8 firewall is unchanged (enforced end to end)" +elif [ "$HOST_IPTABLES_OK" = 0 ]; then + # No privilege to program iptables, established by the probe rather than + # by the message. The invariant above is all this host can prove. + echo "PASS: pre-0.8 firewall is unchanged (host iptables needs privilege; behavior not asserted)" +else + echo "$LEGACY_OUT" + echo "FAIL: pre-0.8 firewall (exited $LEGACY_RC on a host that can program iptables)" + exit 1 +fi + +if ! command -v slirp4netns >/dev/null 2>&1; then + echo "SKIP: slirp4netns not installed; firewall enforcement needs the private namespace." + exit 0 +fi + +# A host-side listener stands in for "a destination the policy allows". It is +# reached at slirp's gateway (10.0.2.2), which maps back to the host's +# loopback -- the same translation proxy mode relies on. Using a local listener +# rather than an internet address keeps the allowed direction deterministic on +# a runner with no outbound access, where an internet probe would fail for +# reasons that have nothing to do with the policy. +TEST_PROXY="$(dirname "$LXC_EXEC")/unix-test-proxy" +if [ ! -x "$TEST_PROXY" ]; then + TEST_PROXY="$REPO_DIR/src/target/release/unix-test-proxy" +fi +if [ ! -x "$TEST_PROXY" ]; then + TEST_PROXY="$REPO_DIR/src/target/debug/unix-test-proxy" +fi +if [ ! -x "$TEST_PROXY" ]; then + echo "FAIL: unix-test-proxy not built." + exit 1 +fi + +WORK_DIR="$(mktemp -d)" +LISTENER_PID="" +cleanup() { + if [ -n "$LISTENER_PID" ]; then + kill "$LISTENER_PID" 2>/dev/null || true + wait "$LISTENER_PID" 2>/dev/null || true + fi + exec 9>&- 2>/dev/null || true + rm -rf "$WORK_DIR" +} +trap cleanup EXIT + +# The listener exits when its stdin reaches EOF; the fifo is opened read-write +# so the open does not block and the script holding it keeps the listener up. +mkfifo "$WORK_DIR/parent.pipe" +exec 9<>"$WORK_DIR/parent.pipe" +"$TEST_PROXY" --ready-file "$WORK_DIR/ready.port" --bind-address 127.0.0.1 \ + <"$WORK_DIR/parent.pipe" >"$WORK_DIR/listener.log" 2>&1 & +LISTENER_PID=$! +for _ in $(seq 1 100); do + [ -s "$WORK_DIR/ready.port" ] && break + if ! kill -0 "$LISTENER_PID" 2>/dev/null; then + cat "$WORK_DIR/listener.log" + echo "FAIL: the host listener exited before publishing its port." + exit 1 + fi + sleep 0.1 +done +ALLOWED_PORT="$(cat "$WORK_DIR/ready.port" 2>/dev/null || true)" +if [ -z "$ALLOWED_PORT" ]; then + cat "$WORK_DIR/listener.log" + echo "FAIL: the host listener did not publish a port." + exit 1 +fi +echo " host listener is on 127.0.0.1:$ALLOWED_PORT (10.0.2.2:$ALLOWED_PORT from the sandbox)" + +# A drop is only evidence of enforcement if the same destination would have +# been reachable without the rule. On a runner with no outbound access every +# "blocked" assertion below would pass while filtering nothing, so establish +# reachability first and skip rather than report a false green. +PROBE_CONFIG="$WORK_DIR/reachability_probe.json" +cat >"$PROBE_CONFIG" <<'PROBE' +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Firewall-Reachability-Probe", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'echo PROBE_WORKLOAD_STARTED; timeout 8 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1 && echo DENY_TARGET_REACHABLE; exit 0'" + }, + "network": { + "defaultPolicy": "allow", + "enforcementMode": "firewall", + "allowedHosts": ["1.1.1.0/24"] + } +} +PROBE +# The workload prints a start marker and always exits 0, so a missing +# sentinel means "no connectivity" and nothing else. A nonzero launcher exit +# or a missing start marker means the firewall path is broken, which must fail +# the suite rather than skip it -- skipping on breakage would false-green the +# very enforcement these tests exist to prove. +PROBE_RC=0 +PROBE_OUT="$("$LXC_EXEC" --experimental --allow-testing-features "$PROBE_CONFIG" 2>&1)" || PROBE_RC=$? +if [ "$PROBE_RC" -ne 0 ]; then + echo "FAIL: reachability probe exited $PROBE_RC; the firewall path itself is broken." + echo "$PROBE_OUT" + exit 1 +fi +if ! grep -q PROBE_WORKLOAD_STARTED <<<"$PROBE_OUT"; then + echo "FAIL: reachability probe workload never ran (no start marker)." + echo " The sandbox failed before executing the command line." + echo "$PROBE_OUT" + exit 1 +fi +if ! grep -q DENY_TARGET_REACHABLE <<<"$PROBE_OUT"; then + echo "SKIP: 1.1.1.1:443 is not reachable from an unfiltered sandbox on this host." + echo " The deny assertions would pass without proving anything." + exit 0 +fi +echo " 1.1.1.1:443 is reachable when allowed, so a drop is real evidence" + +run_enforced() { + local label="$1" + local config="$2" + shift 2 + echo "Running Bubblewrap firewall test: $label..." + sed -e "s/{{ALLOWED_PORT}}/$ALLOWED_PORT/g" \ + "$REPO_DIR/tests/configs/$config" >"$WORK_DIR/$config" + local out + if ! out=$("$LXC_EXEC" --experimental --allow-testing-features "$WORK_DIR/$config" 2>&1); then + echo "$out" + echo "FAIL: $label (lxc-exec returned non-zero)" + exit 1 + fi + local sentinel + for sentinel in "$@"; do + if ! grep -q "$sentinel" <<<"$out"; then + echo "$out" + echo "FAIL: $label (sentinel '$sentinel' not found in output)" + exit 1 + fi + done + echo "$out" >"$WORK_DIR/$label.out" + echo "PASS: $label" +} + +# Enforcement only happens inside the sandbox's own namespace, so a run that +# shared the host's would pass every reachability assertion while filtering +# nothing. Checked first for that reason. +run_enforced "cidr allowlist" "bubblewrap_network_firewall_cidr.json" \ + ALLOWED_DEST_OK DENIED_DEST_BLOCKED_OK LOOPBACK_EXEMPT_OK \ + CAP_NET_ADMIN_DROPPED_OK TAMPER_REFUSED_OK TAMPER_INEFFECTIVE_OK +SANDBOX_NETNS="$(sed -n 's/^SANDBOX_NETNS=//p' "$WORK_DIR/cidr allowlist.out" | tail -n 1)" +if [ -z "$SANDBOX_NETNS" ] || [ "$SANDBOX_NETNS" = "$HOST_NETNS" ]; then + echo "FAIL: cidr allowlist (sandbox did not get a private network namespace)" + exit 1 +fi +echo "PASS: firewall mode runs in a private network namespace" + +# D4: an explicit deny outranks a broader allow. The allowlist here contains +# the denied address, so a chain that appended the allow first would let it +# through. +run_enforced "denylist deny-wins" "bubblewrap_network_firewall_denylist.json" \ + OPEN_TERMINAL_OK DENY_WINS_OK + +# D3: rule addresses are literals and CIDRs. A name is refused rather than +# resolved on the caller's behalf, because the sandbox resolves DNS itself and +# could be handed an address the chain never authorized. +echo "Running Bubblewrap firewall test: hostname rule address rejected..." +NAME_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_firewall_hostname_rejected.json" 2>&1) \ + && NAME_RC=0 || NAME_RC=$? +if [ "$NAME_RC" = 0 ]; then + echo "$NAME_OUT" + echo "FAIL: hostname rule address (accepted a policy it cannot enforce)" + exit 1 +fi +if grep -q FIREWALL_HOSTNAME_RAN <<<"$NAME_OUT"; then + echo "$NAME_OUT" + echo "FAIL: hostname rule address (workload ran despite the rejection)" + exit 1 +fi +if ! grep -qF "not an IP address or CIDR" <<<"$NAME_OUT"; then + echo "$NAME_OUT" + echo "FAIL: hostname rule address (rejection did not explain the contract)" + exit 1 +fi +echo "PASS: hostname rule address rejected" + +# Host lists with no mechanism behind them: 'capabilities' does not filter and +# there is no proxy, so the lists would suppress the namespace-level block and +# leave the sandbox on the host namespace unfiltered. Must fail closed. +echo "Running Bubblewrap firewall test: unenforced host rules rejected..." +UNENF_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_hostrules_unenforced_rejected.json" 2>&1) \ + && UNENF_RC=0 || UNENF_RC=$? +if [ "$UNENF_RC" = 0 ]; then + echo "$UNENF_OUT" + echo "FAIL: unenforced host rules (accepted a policy nothing applies)" + exit 1 +fi +if grep -q SHOULD_NOT_RUN <<<"$UNENF_OUT"; then + echo "$UNENF_OUT" + echo "FAIL: unenforced host rules (workload ran despite the rejection)" + exit 1 +fi +if ! grep -qF "require an enforcement mechanism" <<<"$UNENF_OUT"; then + echo "$UNENF_OUT" + echo "FAIL: unenforced host rules (rejection did not explain the contract)" + exit 1 +fi +echo "PASS: unenforced host rules rejected" + +cleanup +trap - EXIT +echo "Bubblewrap firewall enforcement tests complete." diff --git a/tests/scripts/run_bwrap_localnet_test.sh b/tests/scripts/run_bwrap_localnet_test.sh new file mode 100644 index 000000000..a3b8002ab --- /dev/null +++ b/tests/scripts/run_bwrap_localnet_test.sh @@ -0,0 +1,164 @@ +#!/bin/bash +# Bubblewrap network.allowLocalNetwork honesty tests (schema 0.8+), and the +# pre-0.8 twin. +# +# No root, no slirp4netns and no outbound connectivity required: two of the +# three cases are validate-time outcomes and the third is a plain shared- +# namespace sandbox. That independence is the reason this lives in its own file +# rather than inside run_bwrap_firewall_test.sh, which exits early when +# slirp4netns is absent and skips when the internet is unreachable -- behind +# those gates these cases would go untested on exactly the hosts where they are +# cheapest to run. +# +# What is being asserted: +# +# Bubblewrap has no inbound-only primitive. The sandbox either shares the host +# network namespace or gets a private one, and neither can be narrowed further: +# unprivileged bwrap has no veth for iptables to match on, and seccomp cannot +# dereference the sockaddr passed to bind(), so an AF_INET-only filter is not +# expressible. allowLocalNetwork is therefore honorable only when it already +# agrees with the namespace the resolved mode picks. Both disagreeing +# combinations are rejected from 0.8; both must still be accepted before it. +# +# Not asserted here: the pre-0.8 warning text. Logger warnings are deliberately +# never written to the process's streams (wxc_common is linked into libraries +# whose host owns the terminal -- see logger.rs +# `warning_line_writes_nothing_to_stderr`), so an end-to-end run cannot observe +# them. The warning is covered by the bwrap_common unit tests +# `local_network_denied_on_shared_netns_warns` and +# `local_network_allowed_under_private_netns_warns`. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_DIR="$(dirname "$(dirname "$SCRIPT_DIR")")" +# An explicitly set LXC_EXEC is taken literally: falling back from it would +# silently exercise a different binary than the caller named. +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 + fi +fi + +HOST_NETNS="$(readlink /proc/self/ns/net)" + +# A rejection is only correct if it is the *right* rejection and the workload +# never ran: a sandbox that failed to start for an unrelated reason would +# otherwise look identical to one that fell closed on policy. +assert_rejected() { + local label="$1" + local config="$2" + local fragment="$3" + echo "Running Bubblewrap localnet test: $label..." + local out + local rc=0 + out=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/$config" 2>&1) || rc=$? + if [ "$rc" = 0 ]; then + echo "$out" + echo "FAIL: $label (accepted a policy it cannot honor)" + exit 1 + fi + if grep -q LOCALNET_SHOULD_NOT_RUN <<<"$out"; then + echo "$out" + echo "FAIL: $label (workload ran despite the rejection)" + exit 1 + fi + if ! grep -qF "$fragment" <<<"$out"; then + echo "$out" + echo "FAIL: $label (rejection did not explain the contract)" + exit 1 + fi + echo "PASS: $label" +} + +# defaultPolicy=allow with no host rules and no proxy resolves to Shared, which +# keeps the host namespace -- so allowLocalNetwork=false cannot be delivered. +assert_rejected "allowLocalNetwork=false on a shared namespace" \ + "bubblewrap_network_localnet_shared_rejected.json" \ + "is not enforced while the sandbox shares the host network namespace" + +# The mirror image: defaultPolicy=block with no host rules resolves to Isolated, +# which applies --unshare-net, so a listener is reachable only from inside the +# sandbox and allowLocalNetwork=true overstates what the caller gets. Covered +# because the two arms are separate branches; a fix to one can silently drop +# the other. +assert_rejected "allowLocalNetwork=true under a private namespace" \ + "bubblewrap_network_localnet_private_rejected.json" \ + "is confined to the sandbox's own network namespace" + +# The same rejection with the field omitted. `false` is the schema default *and* +# a deny, so silence still asks for inbound denial. Separate from the explicit +# case because a presence check would pass that one and fail this one. +assert_rejected "an omitted allowLocalNetwork on a shared namespace" \ + "bubblewrap_network_localnet_omitted_rejected.json" \ + "is not enforced while the sandbox shares the host network namespace" + +# The escape hatch from both cases above. Pinned because "reject the deny" is +# also satisfiable by rejecting every shared-namespace request, which would +# leave these callers with no expressible policy. +echo "Running Bubblewrap localnet test: acknowledged exposure is accepted..." +ACK_RC=0 +ACK_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_localnet_acknowledged.json" 2>&1) \ + || ACK_RC=$? +if [ "$ACK_RC" != 0 ]; then + echo "$ACK_OUT" + echo "FAIL: acknowledged localnet (exited $ACK_RC; the acknowledgment is not honored)" + exit 1 +fi +if ! grep -q LOCALNET_ACK_OK <<<"$ACK_OUT"; then + echo "$ACK_OUT" + echo "FAIL: acknowledged localnet (succeeded without running the workload)" + exit 1 +fi +ACK_NETNS="$(sed -n 's/^SANDBOX_NETNS=//p' <<<"$ACK_OUT" | tail -n 1)" +if [ "$ACK_NETNS" != "$HOST_NETNS" ]; then + echo "$ACK_OUT" + echo "FAIL: acknowledged localnet (expected the host netns $HOST_NETNS, got $ACK_NETNS)" + exit 1 +fi +echo "PASS: acknowledged exposure is accepted" + +# The pre-0.8 twin of the first case. GHCP consumes Bubblewrap proxy mode on +# 0.6/0.7, so the rejection above must be invisible there. +# +# The namespace assertion is what makes this a behavior pin rather than a parse +# check. Exit code alone would still pass if a future change routed pre-0.8 +# configs down the private-namespace path -- the run would succeed and silently +# hand legacy callers a sandbox with different connectivity. Three distinct +# regressions fail here: the gate leaking (nonzero exit), the workload being +# skipped (missing sentinel), and the namespace changing. +echo "Running Bubblewrap localnet test: pre-0.8 is unchanged..." +LEGACY_RC=0 +LEGACY_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_localnet_legacy.json" 2>&1) \ + || LEGACY_RC=$? +if [ "$LEGACY_RC" != 0 ]; then + echo "$LEGACY_OUT" + echo "FAIL: pre-0.8 localnet (exited $LEGACY_RC; the 0.8 rejection leaked)" + exit 1 +fi +if ! grep -q LEGACY_LOCALNET_OK <<<"$LEGACY_OUT"; then + echo "$LEGACY_OUT" + echo "FAIL: pre-0.8 localnet (succeeded without running the workload)" + 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: pre-0.8 localnet (expected the host namespace $HOST_NETNS, got $LEGACY_NETNS)" + exit 1 +fi +echo "PASS: pre-0.8 localnet is unchanged" + +echo "Bubblewrap allowLocalNetwork tests complete." diff --git a/tests/scripts/run_bwrap_network_proxy_test.sh b/tests/scripts/run_bwrap_network_proxy_test.sh index fdd85f6df..b1a83e8ee 100644 --- a/tests/scripts/run_bwrap_network_proxy_test.sh +++ b/tests/scripts/run_bwrap_network_proxy_test.sh @@ -51,6 +51,34 @@ 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" +# Regression: host lists alongside a proxy at 0.8 once suppressed --unshare-net +# while the proxy was only cooperative env vars, so `curl --noproxy '*'` walked +# straight out to a destination the allowlist never named. Proxy mode now +# confines egress to the proxy endpoint, so the direct attempt must fail while +# the proxy path still works. +echo "Running Bubblewrap proxy host-rules egress test..." +HOST_NETNS="$(readlink /proc/self/ns/net)" +if ! HOSTRULES_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_proxy_host_rules.json" 2>&1); then + echo "$HOSTRULES_OUT" + echo "FAIL: proxy host-rules egress (lxc-exec returned non-zero)" + exit 1 +fi +for sentinel in DIRECT_EGRESS_BLOCKED_OK PROXY_PATH_OK; do + if ! grep -q "$sentinel" <<<"$HOSTRULES_OUT"; then + echo "$HOSTRULES_OUT" + echo "FAIL: proxy host-rules egress (sentinel '$sentinel' not found)" + exit 1 + fi +done +HOSTRULES_NETNS="$(sed -n 's/^SANDBOX_NETNS=//p' <<<"$HOSTRULES_OUT" | tail -n 1)" +if [ -z "$HOSTRULES_NETNS" ] || [ "$HOSTRULES_NETNS" = "$HOST_NETNS" ]; then + echo "$HOSTRULES_OUT" + echo "FAIL: proxy host-rules egress (host lists suppressed the private namespace)" + exit 1 +fi +echo "PASS: proxy host-rules egress" + echo "Running Bubblewrap private proxy namespace test..." HOST_NETNS="$(readlink /proc/self/ns/net)" if ! NAMESPACE_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ diff --git a/tests/scripts/run_bwrap_network_test.sh b/tests/scripts/run_bwrap_network_test.sh index 66368d654..879e76d3b 100644 --- a/tests/scripts/run_bwrap_network_test.sh +++ b/tests/scripts/run_bwrap_network_test.sh @@ -26,3 +26,24 @@ else exit 1 fi echo "Bubblewrap network block test complete." + +# Schema 0.6 firewall mode keeps its legacy host-chain behavior. Asserts only +# that it still parses — whether iptables itself succeeds depends on privilege. +# Schema 0.8 enforcement is covered by run_bwrap_firewall_test.sh. +echo "Running Bubblewrap firewall-mode legacy test (schema 0.6)..." +set +e +LEGACY_OUTPUT=$("$LXC_EXEC" --experimental \ + "$REPO_DIR/tests/configs/bubblewrap_network_firewall.json" 2>&1) +LEGACY_STATUS=$? +set -e + +# A non-zero status here is expected without privilege (iptables itself may +# fail), but it must never be a *config* rejection -- that would mean a 0.8 +# gate leaked into the legacy schema. +if echo "$LEGACY_OUTPUT" | grep -qF "Configuration parse error"; then + echo "$LEGACY_OUTPUT" + echo "FAIL: schema 0.6 firewall mode failed config validation (status $LEGACY_STATUS)." + exit 1 +fi +echo "PASS: schema 0.6 firewall mode still accepted." +echo "Bubblewrap firewall-mode tests complete."