From eaa01324681504b5fceaab89dd17fc9d58777fb3 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 18 Aug 2026 21:04:58 -0700 Subject: [PATCH 1/7] Enforce Bubblewrap network.enforcementMode=firewall at schema 0.8 via in-namespace iptables rules Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- docs/bwrap-support/bubblewrap-backend.md | 67 ++- .../0.8.0/networking/networking.md | 2 + .../bubblewrap/common/src/bwrap_command.rs | 119 ++++- .../bubblewrap/common/src/bwrap_runner.rs | 119 ++++- src/backends/bubblewrap/common/src/lib.rs | 6 + .../bubblewrap/common/src/network_rules.rs | 497 ++++++++++++++++++ .../bubblewrap/common/src/proxy_network.rs | 457 +++++++++++++--- src/core/wxc_common/src/config_parser.rs | 11 +- .../bubblewrap_network_firewall_cidr.json | 13 + .../bubblewrap_network_firewall_denylist.json | 14 + ...ap_network_firewall_hostname_rejected.json | 13 + tests/scripts/run_bwrap_all_tests.sh | 1 + tests/scripts/run_bwrap_firewall_test.sh | 209 ++++++++ 13 files changed, 1405 insertions(+), 123 deletions(-) create mode 100644 src/backends/bubblewrap/common/src/network_rules.rs create mode 100644 tests/configs/bubblewrap_network_firewall_cidr.json create mode 100644 tests/configs/bubblewrap_network_firewall_denylist.json create mode 100644 tests/configs/bubblewrap_network_firewall_hostname_rejected.json create mode 100644 tests/scripts/run_bwrap_firewall_test.sh diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index 7c57bd54a..b5537d13e 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -213,28 +213,52 @@ 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. Both families are covered — an IPv6 rule programs +`ip6tables` — and the terminal verdict of the unmatched family follows +`defaultPolicy`, so a v4-only allowlist under `block` does not leave IPv6 open. + +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", "2001:db8::/32"], + "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 rules are +not installed and the sandbox runs with unrestricted network access. 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. For dual-stack hosts, use +> proxy mode (below) or move to 0.8, where `ip6tables` is programmed too. + **Full allow** (`defaultPolicy: "allow"`, no host lists) — the sandbox shares the host network namespace with no restrictions. @@ -459,9 +483,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 @@ -536,10 +561,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..0d0a4cd09 100644 --- a/docs/sandbox-policy/0.8.0/networking/networking.md +++ b/docs/sandbox-policy/0.8.0/networking/networking.md @@ -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. diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 9e98b73d7..8484aaa6e 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -104,14 +104,27 @@ 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 { +/// 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 { let mut components = request.schema_version.split('.'); let major = components .next() @@ -126,7 +139,7 @@ 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 +154,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,19 +168,23 @@ 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) @@ -394,6 +415,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 { @@ -443,6 +465,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), + // Unparsable fails closed onto the legacy path for the same reason + // it does on the proxy path. + ("", ResolvedNetworkMode::FirewallFiltered), + ("not-a-version", ResolvedNetworkMode::FirewallFiltered), + ]; + + 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 +530,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 +544,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 +554,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 +563,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 +583,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 +604,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 +614,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 +624,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 +636,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 +668,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] diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index d93603bc0..57c0a7c6f 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 — @@ -145,13 +145,29 @@ 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 proxy_only || firewall_enforced { if let Err(error) = proxy_network::probe_dependencies() { return Err(ScriptResponse::error(&error)); } @@ -311,7 +327,32 @@ 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, + ) { + 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) { Ok(network) => Some(network), Err(error) => { proxy.stop(logger); @@ -1077,6 +1118,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. 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..4a8ebe2f1 --- /dev/null +++ b/src/backends/bubblewrap/common/src/network_rules.rs @@ -0,0 +1,497 @@ +// 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}; + +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)) => 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 (family, canonical) = match address.parse::() { + Ok(IpAddr::V4(ip)) => (RuleFamily::V4, ip.to_string()), + Ok(IpAddr::V6(ip)) => (RuleFamily::V6, ip.to_string()), + Err(_) => return Err(name_rejected(entry)), + }; + + let width = match family { + RuleFamily::V4 => 32, + RuleFamily::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}." + )); + } + + Ok(Self { + family, + text: format!("{canonical}/{bits}"), + }) + } +} + +/// 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 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); + } + + /// 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..08fd1646d 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,25 @@ 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. +/// 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. +fn rule_install_timeout(plan: &EgressPlan) -> Duration { + let commands = plan.command_count() as u64; + Duration::from_secs( + XTABLES_LOCK_WAIT.as_secs() * commands + STARTUP_TIMEOUT.as_secs() + commands, + ) +} 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 +88,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 +130,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 +148,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 +201,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 +606,37 @@ 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, + ) -> 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 +659,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 +706,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 +730,7 @@ impl ProxyNetworkNamespace { pid_writer: Some(pid_writer), userns: Some(userns), hosts, + rule_install_timeout: rule_install_timeout(plan), }) } @@ -767,11 +815,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(()) } @@ -1974,15 +2022,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 +2052,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 +2067,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 +2332,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!( + fixed.len() as u32, + crate::network_rules::BASE_RULE_COMMANDS, + "BASE_RULE_COMMANDS is stale, so the rule-install budget is wrong" + ); + + // 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!( - rules.len() as u32, - RULE_COMMAND_COUNT, - "RULE_COMMAND_COUNT is stale, so the rule-install budget is wrong" + dispatched.len(), + 2, + "the rule loop should have exactly a ported and an unported branch" ); - for rule in rules { + + 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,19 +2375,47 @@ 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; - assert!( - RULE_INSTALL_TIMEOUT > contended, - "a fully contended host would time out before -w could succeed" - ); - assert!( - RULE_INSTALL_TIMEOUT - contended >= STARTUP_TIMEOUT, - "the budget leaves no room for slirp startup on top of lock contention" - ); + 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); + 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.0.0.{n}")).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 - STARTUP_TIMEOUT - >= Duration::from_secs(RULE_COMMAND_COUNT as u64), - "the budget leaves no room to spawn one nsenter per rule" + rule_install_timeout(&plan_with_rule_count(50)) + > rule_install_timeout(&plan_with_rule_count(1)) ); } @@ -2479,6 +2592,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 +2643,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 +2747,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 +2783,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 +2818,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 +2867,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 +2893,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/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 517aa74f0..0483ba594 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -1088,9 +1088,10 @@ fn convert_wire_config( } } - // Bubblewrap is unprivileged by design; iptables-based enforcement - // (firewall / both) requires CAP_NET_ADMIN, which defeats the backend's - // privilege story. Reject the combination explicitly. + // The cooperative env-var proxy enforces hosts at the proxy layer, + // which is mutually exclusive with iptables-based enforcement: they are + // two different answers to the same question, and honoring both would + // mean honoring neither predictably. Rejected at every schema version. if containment == ContainmentBackend::Bubblewrap && policy.network_proxy.is_enabled() && matches!( @@ -1100,8 +1101,8 @@ fn convert_wire_config( { let msg = "Bubblewrap: network.proxy cannot be combined with \ network.enforcementMode='firewall' or 'both'. The cooperative \ - env-var proxy enforces hosts at the proxy layer; iptables-based \ - enforcement requires privilege and is mutually exclusive."; + env-var proxy enforces hosts at the proxy layer, and iptables-based \ + enforcement filters by address; the two are mutually exclusive."; return Err(WxcError::ConfigParse(msg.to_string())); } 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/scripts/run_bwrap_all_tests.sh b/tests/scripts/run_bwrap_all_tests.sh index 3209c57dc..de596c0aa 100644 --- a/tests/scripts/run_bwrap_all_tests.sh +++ b/tests/scripts/run_bwrap_all_tests.sh @@ -40,6 +40,7 @@ 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 "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..68f7953fe --- /dev/null +++ b/tests/scripts/run_bwrap_firewall_test.sh @@ -0,0 +1,209 @@ +#!/bin/bash +# Bubblewrap firewall-enforcement tests (schema 0.8+). +# +# These tests 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 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 case here therefore asserts a destination that must +# be reachable and one that must not. +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 + +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)" + +HOST_NETNS="$(readlink /proc/self/ns/net)" + +# 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 'timeout 8 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1 && echo DENY_TARGET_REACHABLE'" + }, + "network": { + "defaultPolicy": "allow", + "enforcementMode": "firewall", + "allowedHosts": ["1.1.1.0/24"] + } +} +PROBE +PROBE_OUT=$("$LXC_EXEC" --experimental --allow-testing-features "$PROBE_CONFIG" 2>&1) || true +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" + +# 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. +echo "Running Bubblewrap firewall test: schema 0.7 is unchanged..." +LEGACY_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$REPO_DIR/tests/configs/bubblewrap_network_firewall.json" 2>&1) || true +if grep -qF "not an IP address or CIDR" <<<"$LEGACY_OUT"; then + echo "$LEGACY_OUT" + echo "FAIL: schema 0.7 (inherited the 0.8 rule-address rejection)" + exit 1 +fi +echo "PASS: schema 0.7 is unchanged" + +cleanup +trap - EXIT +echo "Bubblewrap firewall enforcement tests complete." From 4092e0903dd51e403dbcf313c85e1b4a864c01d4 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Wed, 19 Aug 2026 09:28:40 -0700 Subject: [PATCH 2/7] Reject unhonorable Bubblewrap network elements at schema 0.8+ Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- docs/bwrap-support/bubblewrap-backend.md | 7 +- .../0.8.0/networking/networking.md | 17 +- .../bubblewrap/common/src/bwrap_command.rs | 153 ++++++++++--- .../bubblewrap/common/src/bwrap_runner.rs | 171 +++++++++++++- src/core/wxc_common/src/config_parser.rs | 210 +++++++++++++++++- ...network_hostrules_unenforced_rejected.json | 13 ++ .../bubblewrap_network_proxy_host_rules.json | 13 ++ tests/scripts/run_bwrap_firewall_test.sh | 24 ++ tests/scripts/run_bwrap_network_proxy_test.sh | 28 +++ tests/scripts/run_bwrap_network_test.sh | 21 ++ 10 files changed, 625 insertions(+), 32 deletions(-) create mode 100644 tests/configs/bubblewrap_network_hostrules_unenforced_rejected.json create mode 100644 tests/configs/bubblewrap_network_proxy_host_rules.json diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index b5537d13e..4cc71000b 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -276,13 +276,14 @@ 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, 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. diff --git a/docs/sandbox-policy/0.8.0/networking/networking.md b/docs/sandbox-policy/0.8.0/networking/networking.md index 0d0a4cd09..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:** @@ -406,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/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 8484aaa6e..703e298a6 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -125,14 +125,7 @@ pub(crate) enum ResolvedNetworkMode { /// 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 { - 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) + wxc_common::config_parser::schema_enforces_network_strictly(&request.schema_version) } impl ResolvedNetworkMode { @@ -189,6 +182,57 @@ impl ResolvedNetworkMode { 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 the sandbox is forced through. 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 { + wxc_common::config_parser::schema_enforces_network_strictly(&request.schema_version) +} + +/// Validate-time twin of the parser's schema-0.8 unenforced-host-list gate. +/// +/// Host lists suppress the `Isolated` mode, so a mode that does not apply them +/// leaves the sandbox on the host namespace unfiltered. The parser only sees +/// JSON configs; a Rust caller can hand `mxc_engine` an `ExecutionRequest` +/// directly and reach the runner without ever passing through it, so both +/// paths must refuse it and share the message verbatim. +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(wxc_common::config_parser::BWRAP_UNENFORCED_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. +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 @@ -219,6 +263,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, @@ -228,17 +275,18 @@ pub(crate) fn local_network_diagnostic_for_mode( network_mode.uses_private_netns(), ) { (false, false) => Some( - "WARNING: Bubblewrap: network.allowLocalNetwork=false is not enforced while the \ + "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.", + an unreachable sandbox use defaultPolicy='block' with no host rules and 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.", + "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.", ), _ => None, } @@ -441,15 +489,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 { @@ -480,10 +532,10 @@ mod tests { ("0.8.0-alpha", ResolvedNetworkMode::FirewallEnforced), ("0.9.0", ResolvedNetworkMode::FirewallEnforced), ("1.0.0", ResolvedNetworkMode::FirewallEnforced), - // Unparsable fails closed onto the legacy path for the same reason - // it does on the proxy path. + // 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::FirewallFiltered), + ("not-a-version", ResolvedNetworkMode::FirewallEnforced), ]; for (version, expected) in cases { @@ -762,6 +814,57 @@ 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 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()); + } + #[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 57c0a7c6f..9452e88d5 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -112,6 +112,21 @@ 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. + // + // The parser rejects both of these too, but only sees JSON configs; a + // Rust caller can build an `ExecutionRequest` and reach the runner + // directly, so the checks are repeated here rather than assumed. + 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 @@ -369,7 +384,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, @@ -887,7 +902,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 { @@ -1217,6 +1232,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/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 0483ba594..331f03c35 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -338,6 +338,39 @@ const CURRENT_SCHEMA_VERSION: &str = "0.8.0-alpha"; /// section or graduating one from experimental. const KNOWN_EXPERIMENTAL_BACKENDS: &[&str] = &["windows_sandbox", "wslc", "isolation_session"]; +/// Whether a schema version opts into the 0.8 network contract, under which a +/// network element the backend cannot enforce is a hard error rather than a +/// warning. +/// +/// An absent version stays lenient: `validate_schema_version` 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 -- +/// `validate_schema_version` 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) +} + +/// Rejection text for host lists no mechanism will enforce, on schema 0.8+. +/// +/// Shared with the Bubblewrap runner's validate-time twin so the CLI and +/// programmatic paths cannot drift apart in wording. +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."; + /// Validate that the schema version (semver) is supported by this binary. /// Compares major.minor only — patch and pre-release labels are ignored. fn validate_schema_version(version: &str) -> Result<(), WxcError> { @@ -1091,7 +1124,9 @@ fn convert_wire_config( // The cooperative env-var proxy enforces hosts at the proxy layer, // which is mutually exclusive with iptables-based enforcement: they are // two different answers to the same question, and honoring both would - // mean honoring neither predictably. Rejected at every schema version. + // mean honoring neither predictably. Rejected at every schema version, + // unlike the guard below. Sequenced first so a config that sets both + // gets the specific conflict rather than the 0.8 gate's advice. if containment == ContainmentBackend::Bubblewrap && policy.network_proxy.is_enabled() && matches!( @@ -1106,6 +1141,24 @@ fn convert_wire_config( return Err(WxcError::ConfigParse(msg.to_string())); } + // Host lists suppress `--unshare-net` (the runner expects iptables to + // filter instead), but under 'capabilities' with no proxy nothing does: + // the sandbox shares the host namespace unfiltered. A default-deny + // policy then yields fully open egress, so 0.8+ fails closed. + if containment == ContainmentBackend::Bubblewrap + && schema_enforces_network_strictly(&schema_version) + && matches!( + policy.network_enforcement_mode, + NetworkEnforcementMode::Capabilities + ) + && !policy.network_proxy.is_enabled() + && (!policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty()) + { + let msg = BWRAP_UNENFORCED_HOST_RULES; + logger.log_line(msg); + return Err(WxcError::ConfigParse(msg.to_string())); + } + // Seatbelt has no privileged packet-filter layer on macOS: it enforces // network policy through the sandbox profile (capabilities-style) and // ignores enforcementMode. Combining network.proxy with a firewall mode @@ -4092,6 +4145,118 @@ mod tests { ); } + #[test] + fn bubblewrap_firewall_enforcement_is_accepted_at_0_8() { + // 0.8 programs the rules into the sandbox's own network namespace, so + // firewall mode is a real enforcement mechanism and must parse. + let json = r#"{ + "version": "0.8.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "block", + "allowedHosts": ["10.0.2.2/32"], + "enforcementMode": "firewall" + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let request = load_request(&encoded, &mut logger, true).expect("firewall mode enforces"); + assert_eq!( + request.policy.allowed_hosts, + vec!["10.0.2.2/32".to_string()] + ); + } + + #[test] + fn bubblewrap_firewall_enforcement_is_accepted_before_0_8() { + // Pre-0.8 keeps its current behavior; the fail-closed gate is 0.8+ only + // so existing 0.6/0.7 callers are unaffected. + let json = r#"{ + "version": "0.7.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "block", + "allowedHosts": ["api.github.com"], + "enforcementMode": "firewall" + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let req = load_request(&encoded, &mut logger, true) + .expect("0.7 firewall configs must keep parsing"); + assert_eq!(req.schema_version, "0.7.0-alpha"); + } + + #[test] + fn bubblewrap_host_rules_without_a_mechanism_are_rejected_at_0_8() { + // Host lists suppress --unshare-net, but 'capabilities' (the default) + // with no proxy applies nothing: this ran with fully open egress. + let json = r#"{ + "version": "0.8.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "block", + "allowedHosts": ["api.github.com"] + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{}", err); + assert!( + msg.contains("require an enforcement mechanism"), + "unexpected error message: {}", + msg + ); + } + + #[test] + fn bubblewrap_host_rules_with_a_proxy_are_accepted_at_0_8() { + // The proxy is the mechanism, so the same lists are honoured. Only the + // built-in test proxy pairs with host lists -- an external one is + // rejected earlier, since MXC does not forward host policy to it. + let json = r#"{ + "version": "0.8.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "block", + "allowedHosts": ["api.github.com"], + "proxy": {"builtinTestServer": true} + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + load_request(&encoded, &mut logger, true).expect("a proxy enforces the host lists at 0.8"); + } + + #[test] + fn bubblewrap_host_rules_without_a_mechanism_are_accepted_before_0_8() { + // GHCP consumes Bubblewrap on 0.6/0.7 with exactly this shape. + let json = r#"{ + "version": "0.7.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "block", + "allowedHosts": ["api.github.com"] + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let req = load_request(&encoded, &mut logger, true) + .expect("0.7 host-list configs must keep parsing"); + assert_eq!(req.schema_version, "0.7.0-alpha"); + } + #[test] fn proxy_remote_url_with_seatbelt_and_default_block_is_rejected() { // A remote (non-loopback) proxy under default-deny would degrade the @@ -5701,6 +5866,49 @@ mod tests { assert_eq!(req.schema_version, ""); } + // 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, which + // planned work is expected to replace. + #[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: `validate_schema_version` + // 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")); + } + #[test] fn schema_version_non_semver_rejected() { let json = r#"{"process": {"commandLine": "echo hi"}, "version": "x"}"#; 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_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_firewall_test.sh b/tests/scripts/run_bwrap_firewall_test.sh index 68f7953fe..c83443d6c 100644 --- a/tests/scripts/run_bwrap_firewall_test.sh +++ b/tests/scripts/run_bwrap_firewall_test.sh @@ -189,6 +189,30 @@ if ! grep -qF "not an IP address or CIDR" <<<"$NAME_OUT"; then 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" + # 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 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." From 368a4203feca9bb6f89fa18973acb578012065e9 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Wed, 19 Aug 2026 14:52:35 -0700 Subject: [PATCH 3/7] Addressed PR comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- docs/bwrap-support/bubblewrap-backend.md | 20 ++- docs/schema.md | 7 + .../bubblewrap/common/src/bwrap_command.rs | 118 +++++++++++-- .../bubblewrap/common/src/bwrap_runner.rs | 7 + src/core/wxc_common/src/config_parser.rs | 155 +++++++++++++++++- tests/scripts/run_bwrap_firewall_test.sh | 21 ++- 6 files changed, 297 insertions(+), 31 deletions(-) diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index 4cc71000b..c29511c87 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -227,9 +227,12 @@ 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. Both families are covered — an IPv6 rule programs -`ip6tables` — and the terminal verdict of the unmatched family follows -`defaultPolicy`, so a v4-only allowlist under `block` does not leave IPv6 open. +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 explicit `blockedHosts` entry outranks any `allowedHosts` entry that covers it, including a broader CIDR: denies are installed ahead of allows in a @@ -240,7 +243,7 @@ first-match chain. "network": { "defaultPolicy": "block", "enforcementMode": "firewall", - "allowedHosts": ["10.0.2.2/32", "2001:db8::/32"], + "allowedHosts": ["10.0.2.2/32", "203.0.113.0/24"], "blockedHosts": ["10.0.2.2"] } } @@ -256,8 +259,9 @@ 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. For dual-stack hosts, use -> proxy mode (below) or move to 0.8, where `ip6tables` is programmed too. +> 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. @@ -281,7 +285,9 @@ namespace choice alone decides the outcome: | `true` | private (`--unshare-net`) | **Partially honored** — the listener is reachable only from inside the sandbox | | `true` | shared with host | Honored | -Rows 2 and 3 are rejected on schema `0.8.0-alpha` and later, and emit a +Rows 2 and 3 are rejected on schema `0.8.0-alpha` and later — at config parse, +and again in the runner for programmatic callers that bypass the parser — 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))`) 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/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 703e298a6..41ea728fc 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -187,8 +187,9 @@ impl ResolvedNetworkMode { /// /// `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 the sandbox is forced through. Every - /// other mode leaves the lists unapplied. + /// 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) } @@ -220,6 +221,24 @@ pub fn unenforced_host_rules_rejection(request: &ExecutionRequest) -> Option<&'s .then_some(wxc_common::config_parser::BWRAP_UNENFORCED_HOST_RULES) } +/// 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(wxc_common::config_parser::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. /// @@ -274,20 +293,8 @@ pub(crate) fn local_network_diagnostic_for_mode( request.policy.allow_local_network, network_mode.uses_private_netns(), ) { - (false, false) => Some( - "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.", - ), - (true, true) => Some( - "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.", - ), + (false, false) => Some(wxc_common::config_parser::BWRAP_LOCAL_NETWORK_SHARED_NS), + (true, true) => Some(wxc_common::config_parser::BWRAP_LOCAL_NETWORK_PRIVATE_NS), _ => None, } } @@ -865,6 +872,85 @@ mod tests { 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 parser cannot depend on this crate, so it re-derives the namespace + /// choice. Drift between the two would let a config be accepted by one path + /// and refused by the other, so pin them together across the mode matrix. + #[test] + fn parser_private_netns_helper_matches_resolved_mode() { + for schema in ["", "0.7.0-alpha", "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![] + }; + + let resolved = ResolvedNetworkMode::from_request(&r, proxy); + let helper = wxc_common::config_parser::bwrap_uses_private_netns( + proxy, + &r.policy.network_enforcement_mode, + &r.policy.default_network_policy, + hosts, + &r.schema_version, + ); + assert_eq!( + resolved.uses_private_netns(), + helper, + "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 9452e88d5..c7bde320c 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -120,6 +120,9 @@ impl SandboxBackend for BubblewrapScriptRunner { // The parser rejects both of these too, but only sees JSON configs; a // Rust caller can build an `ExecutionRequest` and reach the runner // directly, so the checks are repeated here rather than assumed. + 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)); } @@ -965,6 +968,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, @@ -1062,6 +1068,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)), diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 331f03c35..36c351bb1 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -371,6 +371,64 @@ pub const BWRAP_UNENFORCED_HOST_RULES: &str = drop the host lists to get the namespace-level block that defaultPolicy='block' applies \ on its own."; +/// Rejection text for an external proxy combined with host lists. +/// +/// Shared with the Bubblewrap runner's twin. Not schema-gated: the lists have +/// never been forwarded to an operator-supplied proxy. +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."; + +/// Rejection text for `allowLocalNetwork=false` under a shared namespace. +/// Shared with the Bubblewrap runner's twin. +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."; + +/// Rejection text for `allowLocalNetwork=true` under a private namespace. +/// Shared with the Bubblewrap runner's twin. +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."; + +/// Whether a Bubblewrap request resolves to a private network namespace. +/// +/// Twin of `ResolvedNetworkMode::uses_private_netns` in the backend crate, +/// which `wxc_common` must not depend on. A parity test in `bwrap_common` +/// pins the two together. +pub fn bwrap_uses_private_netns( + proxy_enabled: bool, + enforcement_mode: &NetworkEnforcementMode, + default_policy: &NetworkPolicy, + has_host_rules: bool, + schema_version: &str, +) -> bool { + let strict = schema_enforces_network_strictly(schema_version); + if proxy_enabled { + // ProxyOnly under 0.8; LegacyProxy shares the host netns. + return strict; + } + let uses_firewall = matches!( + enforcement_mode, + NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both + ); + if uses_firewall && has_host_rules { + // FirewallEnforced under 0.8; FirewallFiltered shares the host netns. + return strict; + } + // Only Isolated applies --unshare-net. + *default_policy == NetworkPolicy::Block && !has_host_rules +} + /// Validate that the schema version (semver) is supported by this binary. /// Compares major.minor only — patch and pre-release labels are ignored. fn validate_schema_version(version: &str) -> Result<(), WxcError> { @@ -1159,6 +1217,29 @@ fn convert_wire_config( return Err(WxcError::ConfigParse(msg.to_string())); } + // `allowLocalNetwork` is inbound-only and the resolved namespace decides + // the outcome. Twin of the runner's `local_network_rejection`. + if containment == ContainmentBackend::Bubblewrap + && schema_enforces_network_strictly(&schema_version) + { + let private_netns = bwrap_uses_private_netns( + policy.network_proxy.is_enabled(), + &policy.network_enforcement_mode, + &policy.default_network_policy, + !policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty(), + &schema_version, + ); + let msg = match (policy.allow_local_network, private_netns) { + (false, false) => Some(BWRAP_LOCAL_NETWORK_SHARED_NS), + (true, true) => Some(BWRAP_LOCAL_NETWORK_PRIVATE_NS), + _ => None, + }; + if let Some(msg) = msg { + logger.log_line(msg); + return Err(WxcError::ConfigParse(msg.to_string())); + } + } + // Seatbelt has no privileged packet-filter layer on macOS: it enforces // network policy through the sandbox profile (capabilities-style) and // ignores enforcementMode. Combining network.proxy with a firewall mode @@ -1279,12 +1360,7 @@ fn convert_wire_config( || !policy.blocked_hosts.is_empty() || policy.default_network_policy == NetworkPolicy::Block) { - let msg = "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."; + let msg = BWRAP_EXTERNAL_PROXY_HOST_RULES; return Err(WxcError::ConfigParse(msg.to_string())); } @@ -4191,6 +4267,73 @@ mod tests { assert_eq!(req.schema_version, "0.7.0-alpha"); } + #[test] + fn bubblewrap_rejects_an_unhonorable_allow_local_network_at_0_8() { + // Twin of the runner's `local_network_rejection`: a shared namespace + // cannot honor allowLocalNetwork=false, so it must fail at parse time + // rather than surfacing later as a backend error. + let json = r#"{ + "version": "0.8.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "allow", + "allowLocalNetwork": false + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true) + .expect_err("a shared namespace cannot honor allowLocalNetwork=false"); + assert!( + matches!(&err, WxcError::ConfigParse(m) if m.contains("allowLocalNetwork=false")), + "expected a parse-time rejection: {err:?}" + ); + } + + #[test] + fn bubblewrap_rejects_allow_local_network_true_under_a_private_namespace_at_0_8() { + let json = r#"{ + "version": "0.8.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "block", + "allowLocalNetwork": true + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true) + .expect_err("a private namespace confines the listener to the sandbox"); + assert!( + matches!(&err, WxcError::ConfigParse(m) if m.contains("allowLocalNetwork=true")), + "expected a parse-time rejection: {err:?}" + ); + } + + #[test] + fn bubblewrap_allow_local_network_mismatch_is_only_warned_before_0_8() { + // GHCP consumes proxy mode on 0.6/0.7; the gate must not reach them. + let json = r#"{ + "version": "0.7.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": { + "defaultPolicy": "allow", + "allowLocalNetwork": false + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let req = load_request(&encoded, &mut logger, true) + .expect("0.7 keeps the warning rather than rejecting"); + assert_eq!(req.schema_version, "0.7.0-alpha"); + } + #[test] fn bubblewrap_host_rules_without_a_mechanism_are_rejected_at_0_8() { // Host lists suppress --unshare-net, but 'capabilities' (the default) diff --git a/tests/scripts/run_bwrap_firewall_test.sh b/tests/scripts/run_bwrap_firewall_test.sh index c83443d6c..453d9f987 100644 --- a/tests/scripts/run_bwrap_firewall_test.sh +++ b/tests/scripts/run_bwrap_firewall_test.sh @@ -104,7 +104,7 @@ cat >"$PROBE_CONFIG" <<'PROBE' "containerId": "CLI-Bubblewrap-Firewall-Reachability-Probe", "containment": "bubblewrap", "process": { - "commandLine": "bash -c 'timeout 8 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1 && echo DENY_TARGET_REACHABLE'" + "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", @@ -113,7 +113,24 @@ cat >"$PROBE_CONFIG" <<'PROBE' } } PROBE -PROBE_OUT=$("$LXC_EXEC" --experimental --allow-testing-features "$PROBE_CONFIG" 2>&1) || true +# 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." From 23c81f0ef79e854225940e9477e115ea648f6885 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 20 Aug 2026 11:02:11 -0700 Subject: [PATCH 4/7] Add pre-0.8 compatibility e2e coverage for allowLocalNetwork and harden the legacy firewall case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- .../configs/bubblewrap_network_firewall.json | 2 +- .../bubblewrap_network_localnet_legacy.json | 12 ++ ...rap_network_localnet_private_rejected.json | 12 ++ ...wrap_network_localnet_shared_rejected.json | 12 ++ tests/scripts/run_bwrap_all_tests.sh | 1 + tests/scripts/run_bwrap_firewall_test.sh | 107 +++++++++++--- tests/scripts/run_bwrap_localnet_test.sh | 131 ++++++++++++++++++ 7 files changed, 254 insertions(+), 23 deletions(-) create mode 100644 tests/configs/bubblewrap_network_localnet_legacy.json create mode 100644 tests/configs/bubblewrap_network_localnet_private_rejected.json create mode 100644 tests/configs/bubblewrap_network_localnet_shared_rejected.json create mode 100644 tests/scripts/run_bwrap_localnet_test.sh 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_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_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/scripts/run_bwrap_all_tests.sh b/tests/scripts/run_bwrap_all_tests.sh index de596c0aa..8c1ef5897 100644 --- a/tests/scripts/run_bwrap_all_tests.sh +++ b/tests/scripts/run_bwrap_all_tests.sh @@ -41,6 +41,7 @@ run_test "Bubblewrap Denied Masking" "$SCRIPT_DIR/run_bwrap_denied_masking_test. 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 index 453d9f987..ea692afc3 100644 --- a/tests/scripts/run_bwrap_firewall_test.sh +++ b/tests/scripts/run_bwrap_firewall_test.sh @@ -1,15 +1,24 @@ #!/bin/bash -# Bubblewrap firewall-enforcement tests (schema 0.8+). +# Bubblewrap firewall-enforcement tests (schema 0.8+), and the pre-0.8 twin. # -# These tests do NOT require root: enforcement runs inside the sandbox's own +# 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. +# 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 case here therefore asserts a destination that must -# be reachable and one that must not. +# 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)" @@ -32,6 +41,77 @@ else 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..." +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 grep -qE "failed to apply iptables firewall rules|network policy error" <<<"$LEGACY_OUT"; then + # Unprivileged: host-side iptables was refused. The compatibility invariant + # above still held, which 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 for a reason that is neither a rejection nor a privilege failure)" + 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 @@ -91,8 +171,6 @@ if [ -z "$ALLOWED_PORT" ]; then fi echo " host listener is on 127.0.0.1:$ALLOWED_PORT (10.0.2.2:$ALLOWED_PORT from the sandbox)" -HOST_NETNS="$(readlink /proc/self/ns/net)" - # 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 @@ -230,21 +308,6 @@ if ! grep -qF "require an enforcement mechanism" <<<"$UNENF_OUT"; then fi echo "PASS: unenforced host rules rejected" -# 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. -echo "Running Bubblewrap firewall test: schema 0.7 is unchanged..." -LEGACY_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ - "$REPO_DIR/tests/configs/bubblewrap_network_firewall.json" 2>&1) || true -if grep -qF "not an IP address or CIDR" <<<"$LEGACY_OUT"; then - echo "$LEGACY_OUT" - echo "FAIL: schema 0.7 (inherited the 0.8 rule-address rejection)" - exit 1 -fi -echo "PASS: schema 0.7 is unchanged" - 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..cadf8a930 --- /dev/null +++ b/tests/scripts/run_bwrap_localnet_test.sh @@ -0,0 +1,131 @@ +#!/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 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." From 7de3a06135fd5866f29452d6fcb43d82162f0e75 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 20 Aug 2026 13:00:10 -0700 Subject: [PATCH 5/7] Program IPv4-mapped rule addresses with iptables so a mapped deny cannot fail open --- docs/bwrap-support/bubblewrap-backend.md | 9 ++ .../bubblewrap/common/src/bwrap_command.rs | 9 ++ .../bubblewrap/common/src/bwrap_runner.rs | 1 + .../bubblewrap/common/src/network_rules.rs | 119 ++++++++++++++++-- src/core/wxc_common/src/config_parser.rs | 28 ++++- src/core/wxc_common/src/models.rs | 9 ++ src/core/wxc_common/src/network_parser.rs | 1 + 7 files changed, 164 insertions(+), 12 deletions(-) diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index c29511c87..4e11dffae 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -234,6 +234,15 @@ 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`. A prefix shorter than `/96` also covers addresses outside that range, +which do travel as IPv6, so it stays on `ip6tables`. The LXC backend normalizes +identically, so the same policy means the same thing on both. + 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. diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 41ea728fc..4f95f5bcd 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -245,10 +245,18 @@ pub fn external_proxy_host_rules_rejection(request: &ExecutionRequest) -> Option /// Keys off proxy *enablement* rather than a resolved address because /// `builtinTestServer` has no address until the proxy starts, which is after /// validation. +/// +/// An unset `allowLocalNetwork` is never rejected. The field defaults to +/// `false`, so rejecting on the value alone would fail every 0.8 request that +/// shares the host namespace without asking for inbound policy at all. 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; } + if !request.policy.allow_local_network && !request.policy.allow_local_network_specified { + return None; + } let mode = ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()); local_network_diagnostic_for_mode(request, mode) @@ -840,6 +848,7 @@ mod tests { 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_specified = true; 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. diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index c7bde320c..82464d846 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -1249,6 +1249,7 @@ mod tests { 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.allow_local_network_specified = true; let err = BubblewrapScriptRunner::new().validate(&req).unwrap_err(); assert!( diff --git a/src/backends/bubblewrap/common/src/network_rules.rs b/src/backends/bubblewrap/common/src/network_rules.rs index 4a8ebe2f1..7a59d5fb4 100644 --- a/src/backends/bubblewrap/common/src/network_rules.rs +++ b/src/backends/bubblewrap/common/src/network_rules.rs @@ -28,7 +28,7 @@ //! accepted one cannot carry anything but digits, dots, colons and a slash. use std::fmt; -use std::net::{IpAddr, Ipv4Addr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use wxc_common::models::{ExecutionRequest, NetworkPolicy}; @@ -104,25 +104,30 @@ impl RuleAddress { family: RuleFamily::V4, text: ip.to_string(), }), - Ok(IpAddr::V6(ip)) => Ok(Self { - family: RuleFamily::V6, - 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 (family, canonical) = match address.parse::() { - Ok(IpAddr::V4(ip)) => (RuleFamily::V4, ip.to_string()), - Ok(IpAddr::V6(ip)) => (RuleFamily::V6, ip.to_string()), + let parsed = match address.parse::() { + Ok(ip) => ip, Err(_) => return Err(name_rejected(entry)), }; - let width = match family { - RuleFamily::V4 => 32, - RuleFamily::V6 => 128, + let width = match parsed { + IpAddr::V4(_) => 32, + IpAddr::V6(_) => 128, }; let bits: u8 = prefix.parse().map_err(|_| { format!( @@ -137,6 +142,16 @@ impl RuleAddress { )); } + // 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 => (RuleFamily::V6, ip.to_string(), bits), + }, + }; + Ok(Self { family, text: format!("{canonical}/{bits}"), @@ -144,6 +159,20 @@ impl RuleAddress { } } +/// 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. The LXC backend +/// normalizes the same way, so a policy means the same thing on both. +/// +/// 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 stays IPv6. +fn mapped_v4_block(ip: Ipv6Addr, bits: u8) -> Option<(Ipv4Addr, u8)> { + Some((ip.to_ipv4_mapped()?, bits.checked_sub(96)?)) +} + /// The message for an address that is neither a literal nor a CIDR. /// /// Names are called out explicitly: rejecting them is a deliberate contract @@ -331,6 +360,60 @@ mod tests { 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_stays_ipv6() { + // A /95 also covers addresses outside ::ffff:0:0/96, which do travel as + // IPv6, so it cannot be rewritten to a single IPv4 block. + let address = RuleAddress::parse("::ffff:0:0/95").expect("a v6 CIDR is valid"); + assert_eq!(address.family, RuleFamily::V6); + // Rust renders a mapped address in mixed notation, which ip6tables + // accepts; only the family matters for which table it lands in. + assert_eq!(address.text, "::ffff:0.0.0.0/95"); + } + + #[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"); @@ -423,6 +506,20 @@ mod tests { 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] diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 36c351bb1..74c6c18a7 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -1229,8 +1229,14 @@ fn convert_wire_config( !policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty(), &schema_version, ); + // Only an explicit `false` is rejected: the field defaults to + // `false`, so gating on the value alone would reject every request + // that never mentioned it. `true` needs no such guard — it cannot + // arise from the default. let msg = match (policy.allow_local_network, private_netns) { - (false, false) => Some(BWRAP_LOCAL_NETWORK_SHARED_NS), + (false, false) if policy.allow_local_network_specified => { + Some(BWRAP_LOCAL_NETWORK_SHARED_NS) + } (true, true) => Some(BWRAP_LOCAL_NETWORK_PRIVATE_NS), _ => None, }; @@ -4292,6 +4298,26 @@ mod tests { ); } + #[test] + fn bubblewrap_accepts_an_omitted_allow_local_network_at_0_8() { + // The field defaults to false, so rejecting on the value alone would + // reject every 0.8 request that shares the host namespace without + // mentioning inbound policy -- the twin test above supplies it. + let json = r#"{ + "version": "0.8.0-alpha", + "containment": "bubblewrap", + "process": {"commandLine": "echo hi"}, + "network": {"defaultPolicy": "allow"} + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let req = load_request(&encoded, &mut logger, true) + .expect("an omitted allowLocalNetwork must not be rejected"); + assert!(!req.policy.allow_local_network); + assert!(!req.policy.allow_local_network_specified); + } + #[test] fn bubblewrap_rejects_allow_local_network_true_under_a_private_namespace_at_0_8() { let json = r#"{ diff --git a/src/core/wxc_common/src/models.rs b/src/core/wxc_common/src/models.rs index 4399ad553..4d7ebe989 100644 --- a/src/core/wxc_common/src/models.rs +++ b/src/core/wxc_common/src/models.rs @@ -703,6 +703,15 @@ pub struct ContainerPolicy { /// and accept incoming connections. Independent of `default_network_policy` /// (which governs outbound traffic). pub allow_local_network: bool, + /// Whether the caller supplied `network.allowLocalNetwork` on the wire. + /// + /// Field-level twin of `network_specified`, needed because this field + /// defaults to `false`, which is also the value a caller may set + /// explicitly. Backends that reject an unenforceable `false` must not + /// reject a request that never asked for one. Parse-derived, never on the + /// wire. + #[serde(skip)] + pub allow_local_network_specified: bool, pub allowed_hosts: Vec, pub blocked_hosts: Vec, /// Outbound CIDR, protocol, and port policy. diff --git a/src/core/wxc_common/src/network_parser.rs b/src/core/wxc_common/src/network_parser.rs index b7a3fdf9a..75831a017 100644 --- a/src/core/wxc_common/src/network_parser.rs +++ b/src/core/wxc_common/src/network_parser.rs @@ -244,6 +244,7 @@ fn apply_legacy_network( } if let Some(allow) = network.allow_local_network { policy.allow_local_network = allow; + policy.allow_local_network_specified = true; } if let Some(hosts) = network.allowed_hosts { policy.allowed_hosts = hosts; From 1bdee5122930a1f439f5b3774f6ddfbfc34ed8f4 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 20 Aug 2026 21:41:49 -0700 Subject: [PATCH 6/7] Move Bubblewrap network policy validation out of the parser into the backend --- docs/bwrap-support/bubblewrap-backend.md | 50 ++- src/Cargo.lock | 1 + src/backends/bubblewrap/common/Cargo.toml | 1 + .../bubblewrap/common/src/bwrap_command.rs | 160 +++++-- .../bubblewrap/common/src/bwrap_runner.rs | 17 +- .../bubblewrap/common/src/network_rules.rs | 104 ++++- .../bubblewrap/common/src/proxy_network.rs | 181 ++++++-- src/core/wxc_common/src/config_parser.rs | 400 +----------------- 8 files changed, 452 insertions(+), 462 deletions(-) diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index 4e11dffae..ff592d5be 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 @@ -239,9 +249,24 @@ 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`. A prefix shorter than `/96` also covers addresses outside that range, -which do travel as IPv6, so it stays on `ip6tables`. The LXC backend normalizes -identically, so the same policy means the same thing on both. +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 @@ -294,8 +319,9 @@ namespace choice alone decides the outcome: | `true` | private (`--unshare-net`) | **Partially honored** — the listener is reachable only from inside the sandbox | | `true` | shared with host | Honored | -Rows 2 and 3 are rejected on schema `0.8.0-alpha` and later — at config parse, -and again in the runner for programmatic callers that bypass the parser — and +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` @@ -540,9 +566,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:** 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 4f95f5bcd..b4054a176 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -117,6 +117,25 @@ pub(crate) enum ResolvedNetworkMode { ProxyOnly, } +/// 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 @@ -125,7 +144,7 @@ pub(crate) enum ResolvedNetworkMode { /// 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 { - wxc_common::config_parser::schema_enforces_network_strictly(&request.schema_version) + schema_enforces_network_strictly(&request.schema_version) } impl ResolvedNetworkMode { @@ -198,16 +217,47 @@ impl ResolvedNetworkMode { /// 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 { - wxc_common::config_parser::schema_enforces_network_strictly(&request.schema_version) + schema_enforces_network_strictly(&request.schema_version) } -/// Validate-time twin of the parser's schema-0.8 unenforced-host-list gate. +/// 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."; + +/// 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. The parser only sees -/// JSON configs; a Rust caller can hand `mxc_engine` an `ExecutionRequest` -/// directly and reach the runner without ever passing through it, so both -/// paths must refuse it and share the message verbatim. +/// 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; @@ -217,10 +267,21 @@ pub fn unenforced_host_rules_rejection(request: &ExecutionRequest) -> Option<&'s let mode = ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()); - (has_host_rules && !mode.enforces_host_rules()) - .then_some(wxc_common::config_parser::BWRAP_UNENFORCED_HOST_RULES) + (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 @@ -236,7 +297,7 @@ pub fn external_proxy_host_rules_rejection(request: &ExecutionRequest) -> Option || !request.policy.blocked_hosts.is_empty() || request.policy.default_network_policy == NetworkPolicy::Block; - conflicts.then_some(wxc_common::config_parser::BWRAP_EXTERNAL_PROXY_HOST_RULES) + conflicts.then_some(BWRAP_EXTERNAL_PROXY_HOST_RULES) } /// Validate-time twin of [`local_network_diagnostic`]: the same mismatch, but @@ -301,8 +362,8 @@ pub(crate) fn local_network_diagnostic_for_mode( request.policy.allow_local_network, network_mode.uses_private_netns(), ) { - (false, false) => Some(wxc_common::config_parser::BWRAP_LOCAL_NETWORK_SHARED_NS), - (true, true) => Some(wxc_common::config_parser::BWRAP_LOCAL_NETWORK_PRIVATE_NS), + (false, false) => Some(BWRAP_LOCAL_NETWORK_SHARED_NS), + (true, true) => Some(BWRAP_LOCAL_NETWORK_PRIVATE_NS), _ => None, } } @@ -914,12 +975,56 @@ mod tests { assert!(external_proxy_host_rules_rejection(&r).is_none()); } - /// The parser cannot depend on this crate, so it re-derives the namespace - /// choice. Drift between the two would let a config be accepted by one path - /// and refused by the other, so pin them together across the mode matrix. + // 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 parser_private_netns_helper_matches_resolved_mode() { + 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, @@ -939,17 +1044,24 @@ mod tests { vec![] }; - let resolved = ResolvedNetworkMode::from_request(&r, proxy); - let helper = wxc_common::config_parser::bwrap_uses_private_netns( - proxy, - &r.policy.network_enforcement_mode, - &r.policy.default_network_policy, - hosts, - &r.schema_version, + // 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(), - helper, + expected, "schema={schema} proxy={proxy} mode={mode:?} \ policy={policy:?} hosts={hosts}" ); diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index 82464d846..5e9b7cbd0 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -117,9 +117,12 @@ impl SandboxBackend for BubblewrapScriptRunner { // keeps the warning, so existing configs are unaffected. Sits with the // input checks so a host without bwrap is still told what is wrong. // - // The parser rejects both of these too, but only sees JSON configs; a + // 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, so the checks are repeated here rather than assumed. + // 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)); } @@ -186,7 +189,15 @@ impl SandboxBackend for BubblewrapScriptRunner { return Err(ScriptResponse::error(&err.to_string())); } if proxy_only || firewall_enforced { - if let Err(error) = proxy_network::probe_dependencies() { + // 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)); } } diff --git a/src/backends/bubblewrap/common/src/network_rules.rs b/src/backends/bubblewrap/common/src/network_rules.rs index 7a59d5fb4..34b997cd8 100644 --- a/src/backends/bubblewrap/common/src/network_rules.rs +++ b/src/backends/bubblewrap/common/src/network_rules.rs @@ -148,6 +148,9 @@ impl RuleAddress { 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), }, }; @@ -163,16 +166,58 @@ impl RuleAddress { /// /// 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. The LXC backend -/// normalizes the same way, so a policy means the same thing on both. +/// 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 stays IPv6. +/// 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 @@ -384,14 +429,53 @@ mod tests { } #[test] - fn a_prefix_shorter_than_the_mapped_range_stays_ipv6() { - // A /95 also covers addresses outside ::ffff:0:0/96, which do travel as - // IPv6, so it cannot be rewritten to a single IPv4 block. - let address = RuleAddress::parse("::ffff:0:0/95").expect("a v6 CIDR is valid"); + 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); - // Rust renders a mapped address in mixed notation, which ip6tables - // accepts; only the family matters for which table it lands in. - assert_eq!(address.text, "::ffff:0.0.0.0/95"); + 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] diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index 08fd1646d..f06f8b2ce 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -1155,33 +1155,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 )); @@ -1190,21 +1237,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), @@ -1214,13 +1260,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 )); @@ -1228,7 +1274,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(()) @@ -1246,7 +1297,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(()); } @@ -1256,15 +1312,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() )) } @@ -2486,7 +2544,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" @@ -2503,6 +2562,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"); @@ -2512,6 +2572,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] @@ -2521,7 +2643,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" ); } @@ -2545,8 +2673,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" ); } diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 74c6c18a7..517aa74f0 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -338,97 +338,6 @@ const CURRENT_SCHEMA_VERSION: &str = "0.8.0-alpha"; /// section or graduating one from experimental. const KNOWN_EXPERIMENTAL_BACKENDS: &[&str] = &["windows_sandbox", "wslc", "isolation_session"]; -/// Whether a schema version opts into the 0.8 network contract, under which a -/// network element the backend cannot enforce is a hard error rather than a -/// warning. -/// -/// An absent version stays lenient: `validate_schema_version` 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 -- -/// `validate_schema_version` 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) -} - -/// Rejection text for host lists no mechanism will enforce, on schema 0.8+. -/// -/// Shared with the Bubblewrap runner's validate-time twin so the CLI and -/// programmatic paths cannot drift apart in wording. -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 an external proxy combined with host lists. -/// -/// Shared with the Bubblewrap runner's twin. Not schema-gated: the lists have -/// never been forwarded to an operator-supplied proxy. -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."; - -/// Rejection text for `allowLocalNetwork=false` under a shared namespace. -/// Shared with the Bubblewrap runner's twin. -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."; - -/// Rejection text for `allowLocalNetwork=true` under a private namespace. -/// Shared with the Bubblewrap runner's twin. -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."; - -/// Whether a Bubblewrap request resolves to a private network namespace. -/// -/// Twin of `ResolvedNetworkMode::uses_private_netns` in the backend crate, -/// which `wxc_common` must not depend on. A parity test in `bwrap_common` -/// pins the two together. -pub fn bwrap_uses_private_netns( - proxy_enabled: bool, - enforcement_mode: &NetworkEnforcementMode, - default_policy: &NetworkPolicy, - has_host_rules: bool, - schema_version: &str, -) -> bool { - let strict = schema_enforces_network_strictly(schema_version); - if proxy_enabled { - // ProxyOnly under 0.8; LegacyProxy shares the host netns. - return strict; - } - let uses_firewall = matches!( - enforcement_mode, - NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both - ); - if uses_firewall && has_host_rules { - // FirewallEnforced under 0.8; FirewallFiltered shares the host netns. - return strict; - } - // Only Isolated applies --unshare-net. - *default_policy == NetworkPolicy::Block && !has_host_rules -} - /// Validate that the schema version (semver) is supported by this binary. /// Compares major.minor only — patch and pre-release labels are ignored. fn validate_schema_version(version: &str) -> Result<(), WxcError> { @@ -1179,12 +1088,9 @@ fn convert_wire_config( } } - // The cooperative env-var proxy enforces hosts at the proxy layer, - // which is mutually exclusive with iptables-based enforcement: they are - // two different answers to the same question, and honoring both would - // mean honoring neither predictably. Rejected at every schema version, - // unlike the guard below. Sequenced first so a config that sets both - // gets the specific conflict rather than the 0.8 gate's advice. + // Bubblewrap is unprivileged by design; iptables-based enforcement + // (firewall / both) requires CAP_NET_ADMIN, which defeats the backend's + // privilege story. Reject the combination explicitly. if containment == ContainmentBackend::Bubblewrap && policy.network_proxy.is_enabled() && matches!( @@ -1194,58 +1100,11 @@ fn convert_wire_config( { let msg = "Bubblewrap: network.proxy cannot be combined with \ network.enforcementMode='firewall' or 'both'. The cooperative \ - env-var proxy enforces hosts at the proxy layer, and iptables-based \ - enforcement filters by address; the two are mutually exclusive."; + env-var proxy enforces hosts at the proxy layer; iptables-based \ + enforcement requires privilege and is mutually exclusive."; return Err(WxcError::ConfigParse(msg.to_string())); } - // Host lists suppress `--unshare-net` (the runner expects iptables to - // filter instead), but under 'capabilities' with no proxy nothing does: - // the sandbox shares the host namespace unfiltered. A default-deny - // policy then yields fully open egress, so 0.8+ fails closed. - if containment == ContainmentBackend::Bubblewrap - && schema_enforces_network_strictly(&schema_version) - && matches!( - policy.network_enforcement_mode, - NetworkEnforcementMode::Capabilities - ) - && !policy.network_proxy.is_enabled() - && (!policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty()) - { - let msg = BWRAP_UNENFORCED_HOST_RULES; - logger.log_line(msg); - return Err(WxcError::ConfigParse(msg.to_string())); - } - - // `allowLocalNetwork` is inbound-only and the resolved namespace decides - // the outcome. Twin of the runner's `local_network_rejection`. - if containment == ContainmentBackend::Bubblewrap - && schema_enforces_network_strictly(&schema_version) - { - let private_netns = bwrap_uses_private_netns( - policy.network_proxy.is_enabled(), - &policy.network_enforcement_mode, - &policy.default_network_policy, - !policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty(), - &schema_version, - ); - // Only an explicit `false` is rejected: the field defaults to - // `false`, so gating on the value alone would reject every request - // that never mentioned it. `true` needs no such guard — it cannot - // arise from the default. - let msg = match (policy.allow_local_network, private_netns) { - (false, false) if policy.allow_local_network_specified => { - Some(BWRAP_LOCAL_NETWORK_SHARED_NS) - } - (true, true) => Some(BWRAP_LOCAL_NETWORK_PRIVATE_NS), - _ => None, - }; - if let Some(msg) = msg { - logger.log_line(msg); - return Err(WxcError::ConfigParse(msg.to_string())); - } - } - // Seatbelt has no privileged packet-filter layer on macOS: it enforces // network policy through the sandbox profile (capabilities-style) and // ignores enforcementMode. Combining network.proxy with a firewall mode @@ -1366,7 +1225,12 @@ fn convert_wire_config( || !policy.blocked_hosts.is_empty() || policy.default_network_policy == NetworkPolicy::Block) { - let msg = BWRAP_EXTERNAL_PROXY_HOST_RULES; + let msg = "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."; return Err(WxcError::ConfigParse(msg.to_string())); } @@ -4227,205 +4091,6 @@ mod tests { ); } - #[test] - fn bubblewrap_firewall_enforcement_is_accepted_at_0_8() { - // 0.8 programs the rules into the sandbox's own network namespace, so - // firewall mode is a real enforcement mechanism and must parse. - let json = r#"{ - "version": "0.8.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "block", - "allowedHosts": ["10.0.2.2/32"], - "enforcementMode": "firewall" - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let request = load_request(&encoded, &mut logger, true).expect("firewall mode enforces"); - assert_eq!( - request.policy.allowed_hosts, - vec!["10.0.2.2/32".to_string()] - ); - } - - #[test] - fn bubblewrap_firewall_enforcement_is_accepted_before_0_8() { - // Pre-0.8 keeps its current behavior; the fail-closed gate is 0.8+ only - // so existing 0.6/0.7 callers are unaffected. - let json = r#"{ - "version": "0.7.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "block", - "allowedHosts": ["api.github.com"], - "enforcementMode": "firewall" - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let req = load_request(&encoded, &mut logger, true) - .expect("0.7 firewall configs must keep parsing"); - assert_eq!(req.schema_version, "0.7.0-alpha"); - } - - #[test] - fn bubblewrap_rejects_an_unhonorable_allow_local_network_at_0_8() { - // Twin of the runner's `local_network_rejection`: a shared namespace - // cannot honor allowLocalNetwork=false, so it must fail at parse time - // rather than surfacing later as a backend error. - let json = r#"{ - "version": "0.8.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "allow", - "allowLocalNetwork": false - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let err = load_request(&encoded, &mut logger, true) - .expect_err("a shared namespace cannot honor allowLocalNetwork=false"); - assert!( - matches!(&err, WxcError::ConfigParse(m) if m.contains("allowLocalNetwork=false")), - "expected a parse-time rejection: {err:?}" - ); - } - - #[test] - fn bubblewrap_accepts_an_omitted_allow_local_network_at_0_8() { - // The field defaults to false, so rejecting on the value alone would - // reject every 0.8 request that shares the host namespace without - // mentioning inbound policy -- the twin test above supplies it. - let json = r#"{ - "version": "0.8.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": {"defaultPolicy": "allow"} - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let req = load_request(&encoded, &mut logger, true) - .expect("an omitted allowLocalNetwork must not be rejected"); - assert!(!req.policy.allow_local_network); - assert!(!req.policy.allow_local_network_specified); - } - - #[test] - fn bubblewrap_rejects_allow_local_network_true_under_a_private_namespace_at_0_8() { - let json = r#"{ - "version": "0.8.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "block", - "allowLocalNetwork": true - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let err = load_request(&encoded, &mut logger, true) - .expect_err("a private namespace confines the listener to the sandbox"); - assert!( - matches!(&err, WxcError::ConfigParse(m) if m.contains("allowLocalNetwork=true")), - "expected a parse-time rejection: {err:?}" - ); - } - - #[test] - fn bubblewrap_allow_local_network_mismatch_is_only_warned_before_0_8() { - // GHCP consumes proxy mode on 0.6/0.7; the gate must not reach them. - let json = r#"{ - "version": "0.7.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "allow", - "allowLocalNetwork": false - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let req = load_request(&encoded, &mut logger, true) - .expect("0.7 keeps the warning rather than rejecting"); - assert_eq!(req.schema_version, "0.7.0-alpha"); - } - - #[test] - fn bubblewrap_host_rules_without_a_mechanism_are_rejected_at_0_8() { - // Host lists suppress --unshare-net, but 'capabilities' (the default) - // with no proxy applies nothing: this ran with fully open egress. - let json = r#"{ - "version": "0.8.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "block", - "allowedHosts": ["api.github.com"] - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let err = load_request(&encoded, &mut logger, true).unwrap_err(); - let msg = format!("{}", err); - assert!( - msg.contains("require an enforcement mechanism"), - "unexpected error message: {}", - msg - ); - } - - #[test] - fn bubblewrap_host_rules_with_a_proxy_are_accepted_at_0_8() { - // The proxy is the mechanism, so the same lists are honoured. Only the - // built-in test proxy pairs with host lists -- an external one is - // rejected earlier, since MXC does not forward host policy to it. - let json = r#"{ - "version": "0.8.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "block", - "allowedHosts": ["api.github.com"], - "proxy": {"builtinTestServer": true} - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - load_request(&encoded, &mut logger, true).expect("a proxy enforces the host lists at 0.8"); - } - - #[test] - fn bubblewrap_host_rules_without_a_mechanism_are_accepted_before_0_8() { - // GHCP consumes Bubblewrap on 0.6/0.7 with exactly this shape. - let json = r#"{ - "version": "0.7.0-alpha", - "containment": "bubblewrap", - "process": {"commandLine": "echo hi"}, - "network": { - "defaultPolicy": "block", - "allowedHosts": ["api.github.com"] - } - }"#; - let encoded = base64_encode(json.as_bytes()); - let mut logger = test_logger(); - - let req = load_request(&encoded, &mut logger, true) - .expect("0.7 host-list configs must keep parsing"); - assert_eq!(req.schema_version, "0.7.0-alpha"); - } - #[test] fn proxy_remote_url_with_seatbelt_and_default_block_is_rejected() { // A remote (non-loopback) proxy under default-deny would degrade the @@ -6035,49 +5700,6 @@ mod tests { assert_eq!(req.schema_version, ""); } - // 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, which - // planned work is expected to replace. - #[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: `validate_schema_version` - // 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")); - } - #[test] fn schema_version_non_semver_rejected() { let json = r#"{"process": {"commandLine": "echo hi"}, "version": "x"}"#; From a3f9f326be11418b91e2cdb109d55e5c599359c9 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 21 Aug 2026 11:09:25 -0700 Subject: [PATCH 7/7] Reject an unenforceable allowLocalNetwork deny on value, not on presence allowLocalNetwork=false under a shared namespace is refused at 0.8 because Bubblewrap cannot deliver the deny. That refusal was gated on a parse-time allow_local_network_specified flag, so it fired only for callers who wrote the field. false is the schema default *and* a deny, so an omitted field is still a request for inbound denial: a bare defaultPolicy='allow' ran with the deny silently unenforced. Default-deny is the contract, and a backend that cannot honor a restriction refuses it rather than choosing default-allow on the caller's behalf. IsolationSession already takes the strict reading of this exact field: validate_provision_network_policy requires allow_local_network by value and refuses an absent policy as "a restriction the backend cannot honor". The guard is now dropped so Bubblewrap agrees, and the flag it existed for goes with it -- that guard was its only production consumer, so models.rs and network_parser.rs leave this change with no net diff. The rejection stays actionable: a shared namespace is still reachable by acknowledging the exposure with allowLocalNetwork=true, which the error text now names. No existing config changes behavior -- both shared-namespace corpus configs set the field explicitly, and every other 0.8 Bubblewrap config resolves to a private namespace via a proxy or host rules. Coverage: the guard had none. Deleting it outright left all 202 tests passing, because the one test that looked like it covered this used defaultPolicy='block', whose private namespace short-circuits the mode arm. Two unit tests now pin the omitted and acknowledged cases (204 total; a reintroduced guard fails three), and two e2e configs pin them end to end, the acknowledged one asserting the sandbox really is in the host netns so "reject the deny" cannot be satisfied by refusing every shared-namespace request. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- docs/bwrap-support/bubblewrap-backend.md | 20 ++++- .../bubblewrap/common/src/bwrap_command.rs | 40 +++++++--- .../bubblewrap/common/src/bwrap_runner.rs | 9 ++- .../bubblewrap/common/src/proxy_network.rs | 78 +++++++++++++++++-- src/core/wxc_common/src/models.rs | 9 --- src/core/wxc_common/src/network_parser.rs | 1 - ...blewrap_network_localnet_acknowledged.json | 12 +++ ...rap_network_localnet_omitted_rejected.json | 11 +++ tests/scripts/run_bwrap_firewall_test.sh | 15 +++- tests/scripts/run_bwrap_localnet_test.sh | 33 ++++++++ 10 files changed, 192 insertions(+), 36 deletions(-) create mode 100644 tests/configs/bubblewrap_network_localnet_acknowledged.json create mode 100644 tests/configs/bubblewrap_network_localnet_omitted_rejected.json diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index ff592d5be..4b280e5fc 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -285,10 +285,14 @@ first-match chain. **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 rules are -not installed and the sandbox runs with unrestricted network access. 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. +`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 @@ -328,6 +332,14 @@ 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: diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index b4054a176..b8f252205 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -240,7 +240,8 @@ pub const BWRAP_LOCAL_NETWORK_SHARED_NS: &str = 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."; + 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 = @@ -307,17 +308,15 @@ pub fn external_proxy_host_rules_rejection(request: &ExecutionRequest) -> Option /// `builtinTestServer` has no address until the proxy starts, which is after /// validation. /// -/// An unset `allowLocalNetwork` is never rejected. The field defaults to -/// `false`, so rejecting on the value alone would fail every 0.8 request that -/// shares the host namespace without asking for inbound policy at all. The -/// warning path keeps the looser check: it only logs. +/// 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; } - if !request.policy.allow_local_network && !request.policy.allow_local_network_specified { - return None; - } let mode = ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()); local_network_diagnostic_for_mode(request, mode) @@ -909,13 +908,36 @@ mod tests { 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_specified = true; 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 diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index 5e9b7cbd0..a61b056f8 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -361,6 +361,7 @@ impl BubblewrapScriptRunner { &egress.plan(), egress.pin(), logger, + request.script_timeout, ) { Ok(network) => Some(network), Err(error) => { @@ -381,7 +382,12 @@ impl BubblewrapScriptRunner { return Err(ScriptResponse::error(&error)); } }; - match proxy_network::ProxyNetworkNamespace::start(&plan, None, logger) { + match proxy_network::ProxyNetworkNamespace::start( + &plan, + None, + logger, + request.script_timeout, + ) { Ok(network) => Some(network), Err(error) => { proxy.stop(logger); @@ -1260,7 +1266,6 @@ mod tests { 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.allow_local_network_specified = true; let err = BubblewrapScriptRunner::new().validate(&req).unwrap_err(); assert!( diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index f06f8b2ce..cc521a973 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -36,6 +36,17 @@ 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"; +/// 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. /// /// Sized so that lock contention -- the expected reason a rule stalls -- cannot @@ -49,11 +60,23 @@ const XTABLES_LOCK_PATH: &str = "/run/xtables.lock"; /// 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. -fn rule_install_timeout(plan: &EgressPlan) -> Duration { +/// +/// 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; - Duration::from_secs( + 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, @@ -624,6 +647,7 @@ impl ProxyNetworkNamespace { plan: &EgressPlan, pin: Option<&ProxyHostPin>, logger: &mut Logger, + script_timeout_ms: u32, ) -> Result { let state_dir = tempfile::Builder::new() .prefix("mxc-bwrap-proxy-") @@ -730,7 +754,7 @@ impl ProxyNetworkNamespace { pid_writer: Some(pid_writer), userns: Some(userns), hosts, - rule_install_timeout: rule_install_timeout(plan), + rule_install_timeout: rule_install_timeout(plan, script_timeout_ms), }) } @@ -2439,7 +2463,7 @@ mod tests { plan_with_rule_count(25), ] { let contended = XTABLES_LOCK_WAIT * plan.command_count(); - let budget = rule_install_timeout(&plan); + let budget = rule_install_timeout(&plan, 0); assert!( budget > contended, "a fully contended host would time out before -w could succeed" @@ -2463,7 +2487,9 @@ mod tests { schema_version: "0.8.0-alpha".into(), ..Default::default() }; - request.policy.allowed_hosts = (0..count).map(|n| format!("10.0.0.{n}")).collect(); + 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") } @@ -2472,11 +2498,49 @@ mod tests { #[test] fn rule_install_budget_grows_with_the_rule_list() { assert!( - rule_install_timeout(&plan_with_rule_count(50)) - > rule_install_timeout(&plan_with_rule_count(1)) + 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!( + Duration::from_secs(10_000 * XTABLES_LOCK_WAIT.as_secs()) > RULE_INSTALL_TIMEOUT_CAP + ); + } + + #[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"); diff --git a/src/core/wxc_common/src/models.rs b/src/core/wxc_common/src/models.rs index 4d7ebe989..4399ad553 100644 --- a/src/core/wxc_common/src/models.rs +++ b/src/core/wxc_common/src/models.rs @@ -703,15 +703,6 @@ pub struct ContainerPolicy { /// and accept incoming connections. Independent of `default_network_policy` /// (which governs outbound traffic). pub allow_local_network: bool, - /// Whether the caller supplied `network.allowLocalNetwork` on the wire. - /// - /// Field-level twin of `network_specified`, needed because this field - /// defaults to `false`, which is also the value a caller may set - /// explicitly. Backends that reject an unenforceable `false` must not - /// reject a request that never asked for one. Parse-derived, never on the - /// wire. - #[serde(skip)] - pub allow_local_network_specified: bool, pub allowed_hosts: Vec, pub blocked_hosts: Vec, /// Outbound CIDR, protocol, and port policy. diff --git a/src/core/wxc_common/src/network_parser.rs b/src/core/wxc_common/src/network_parser.rs index 75831a017..b7a3fdf9a 100644 --- a/src/core/wxc_common/src/network_parser.rs +++ b/src/core/wxc_common/src/network_parser.rs @@ -244,7 +244,6 @@ fn apply_legacy_network( } if let Some(allow) = network.allow_local_network { policy.allow_local_network = allow; - policy.allow_local_network_specified = true; } if let Some(hosts) = network.allowed_hosts { policy.allowed_hosts = hosts; 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_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/scripts/run_bwrap_firewall_test.sh b/tests/scripts/run_bwrap_firewall_test.sh index ea692afc3..7043c96c9 100644 --- a/tests/scripts/run_bwrap_firewall_test.sh +++ b/tests/scripts/run_bwrap_firewall_test.sh @@ -65,6 +65,13 @@ HOST_NETNS="$(readlink /proc/self/ns/net)" # 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=$? @@ -102,13 +109,13 @@ if [ "$LEGACY_RC" = 0 ]; then exit 1 fi echo "PASS: pre-0.8 firewall is unchanged (enforced end to end)" -elif grep -qE "failed to apply iptables firewall rules|network policy error" <<<"$LEGACY_OUT"; then - # Unprivileged: host-side iptables was refused. The compatibility invariant - # above still held, which is all this host can prove. +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 for a reason that is neither a rejection nor a privilege failure)" + echo "FAIL: pre-0.8 firewall (exited $LEGACY_RC on a host that can program iptables)" exit 1 fi diff --git a/tests/scripts/run_bwrap_localnet_test.sh b/tests/scripts/run_bwrap_localnet_test.sh index cadf8a930..a3b8002ab 100644 --- a/tests/scripts/run_bwrap_localnet_test.sh +++ b/tests/scripts/run_bwrap_localnet_test.sh @@ -96,6 +96,39 @@ 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. #