From 831a69620749abb86ac15e2b693e44fd492bdb7a Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 18 Aug 2026 00:20:02 -0700 Subject: [PATCH 1/6] Support hostname proxy endpoints under Bubblewrap proxy-only egress Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- docs/bwrap-support/bubblewrap-backend.md | 42 +- .../bubblewrap/common/src/bwrap_runner.rs | 68 +- .../bubblewrap/common/src/proxy_network.rs | 621 +++++++++++++++--- .../bubblewrap_network_proxy_hostname.json | 12 + tests/scripts/run_bwrap_network_proxy_test.sh | 82 +++ 5 files changed, 680 insertions(+), 145 deletions(-) create mode 100644 tests/configs/bubblewrap_network_proxy_hostname.json diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index 26551fa56..7f4747644 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -411,22 +411,44 @@ request fails if its private namespace cannot be configured. private network namespace. Clients that ignore the env vars (raw sockets, custom HTTP clients) can no longer reach the network directly: only loopback and the proxy endpoint are permitted. DNS is deliberately **not** opened — - the proxy resolves on the workload's behalf — so the proxy endpoint must be - an IPv4 literal. IPv6 egress is denied outright. - - Consequently the only accepted host-local proxy endpoints are `localhost`, - `127.0.0.0/8` and the wildcards `0.0.0.0` / `::`, each rewritten to - `10.0.2.2`. `::1` is **rejected at validation time**: a proxy bound only to - the IPv6 loopback cannot accept the IPv4 connection slirp's gateway - produces, so translating it would hand the sandbox an address nothing - answers on. Bind such a proxy to `127.0.0.1` or to a dual-stack wildcard - instead. + the proxy resolves on the workload's behalf. IPv6 egress is denied outright. + + Host-local proxy endpoints — `localhost`, `127.0.0.0/8` and the wildcards + `0.0.0.0` / `::` — are rewritten to `10.0.2.2`. `::1` is **rejected at + validation time**: a proxy bound only to the IPv6 loopback cannot accept the + IPv4 connection slirp's gateway produces, so translating it would hand the + sandbox an address nothing answers on. Bind such a proxy to `127.0.0.1` or to + a dual-stack wildcard instead. + On schema **0.6/0.7** the legacy behavior applies: the sandbox shares the host network namespace, no egress rules are installed, and only the cooperative env-var routing is in effect — a client that ignores `HTTP_PROXY`/`HTTPS_PROXY` reaches the network directly. For strict whole-network isolation on those versions, omit `network.proxy` so the runner can apply `--unshare-net` instead. +- **Hostname proxy endpoints are pinned, not resolved in the sandbox**: because + DNS is closed, a hostname in `network.proxy.url` cannot be resolved by the + workload. The runner resolves it **once on the host** before the sandbox + starts, opens the egress chain for that address, and pins + `
` as the first line of a generated `/etc/hosts` that is + bind-mounted read-only over the sandbox's copy. The workload therefore sees + the URL exactly as configured, so `Host` headers and proxy-auth realms match. + Consequences worth knowing: + - The name is resolved **once**, at start. A proxy whose address changes + mid-run is not followed. + - Only the pinned address is opened in the egress chain, so a resolver that + bypasses `/etc/hosts` (for example a client that speaks DNS directly, which + is itself blocked) cannot reach a different address. The failure is closed. + - IP **literals** are rewritten rather than pinned, per the rules above. A + hostname that resolves to a loopback address is likewise pinned to the + gateway. + - `localhost` is always rewritten, never pinned. It is reserved to loopback + (RFC 6761) and a pin is a sandbox-wide mapping, so pinning it would + redirect the workload's own loopback traffic to the host. + - The generated `/etc/hosts` preserves the host's existing entries after the + pin line, so `localhost` and friends keep working. A `readwritePaths` entry + covering `/etc/hosts` is overridden by the pin mount. + - 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 diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index 0e047328f..093da3bb8 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -112,13 +112,19 @@ impl SandboxBackend for BubblewrapScriptRunner { } } - // Proxy-only networking rewrites the configured endpoint to slirp's - // host gateway and then opens exactly that address in the egress - // chain. Both steps can reject an endpoint (an IPv6 loopback the - // gateway cannot reach, a routable IPv6 literal the IPv4-only rules - // cannot express, a zero port). Running them here means the caller - // learns at policy time instead of after a namespace, a slirp instance - // and a rule set have already been built and torn down. + // 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 (an IPv6 loopback the + // gateway cannot reach, a routable or IPv6-only endpoint the IPv4-only + // rules cannot express, a zero port, a hostname that does not resolve). + // Running it here means the caller learns at policy time instead of + // after a namespace, a slirp instance and a rule set have already been + // built and torn down. + // + // A hostname costs a DNS lookup here, which `run` repeats -- the + // resolved address is deliberately not carried over, since the one the + // egress chain opens must come from the same lookup that produces the + // pin the sandbox is given. // // The builtin test server has no address until it is started (the // parser leaves a port-0 placeholder), so its endpoint stays on the @@ -128,9 +134,7 @@ impl SandboxBackend for BubblewrapScriptRunner { == ResolvedNetworkMode::ProxyOnly; if proxy_only && !request.policy.network_proxy.builtin_test_server { if let Some(address) = request.policy.network_proxy.address.as_ref() { - let egress = proxy_network::sandbox_proxy_address(address) - .and_then(|translated| proxy_network::ProxyEgress::from_address(&translated)); - if let Err(error) = egress { + if let Err(error) = proxy_network::SandboxProxy::resolve(address) { return Err(ScriptResponse::error(&error)); } } @@ -247,10 +251,10 @@ impl BubblewrapScriptRunner { } let network_mode = ResolvedNetworkMode::from_request(request, proxy.is_active()); - let sandbox_proxy_address = if network_mode == ResolvedNetworkMode::ProxyOnly { + let sandbox_proxy = if network_mode == ResolvedNetworkMode::ProxyOnly { match proxy.address() { - Some(address) => match proxy_network::sandbox_proxy_address(address) { - Ok(address) => Some(address), + Some(address) => match proxy_network::SandboxProxy::resolve(address) { + Ok(resolved) => Some(resolved), Err(error) => { proxy.stop(logger); return Err(ScriptResponse::error(&error)); @@ -266,34 +270,24 @@ impl BubblewrapScriptRunner { } else { None }; - let proxy_address = sandbox_proxy_address.as_ref().or_else(|| proxy.address()); + let proxy_address = sandbox_proxy + .as_ref() + .map(|resolved| resolved.address()) + .or_else(|| proxy.address()); - let mut proxy_network = if network_mode == ResolvedNetworkMode::ProxyOnly { + let mut proxy_network = match sandbox_proxy.as_ref() { // The workload dials the sandbox-visible address, so that is what // the egress rule must open. - let egress = match sandbox_proxy_address - .as_ref() - .ok_or_else(|| { - "Bubblewrap: proxy-only networking requires a resolved proxy address" - .to_string() - }) - .and_then(proxy_network::ProxyEgress::from_address) - { - Ok(egress) => egress, - Err(error) => { - proxy.stop(logger); - return Err(ScriptResponse::error(&error)); - } - }; - match proxy_network::ProxyNetworkNamespace::start(&egress, logger) { - Ok(network) => Some(network), - Err(error) => { - proxy.stop(logger); - return Err(ScriptResponse::error(&error)); + Some(resolved) => { + match proxy_network::ProxyNetworkNamespace::start(resolved.egress(), logger) { + Ok(network) => Some(network), + Err(error) => { + proxy.stop(logger); + return Err(ScriptResponse::error(&error)); + } } } - } else { - None + None => None, }; // 2. Build the bwrap argument vector. `denied_files` is the file-mask @@ -311,7 +305,7 @@ impl BubblewrapScriptRunner { network_mode, ); let mut network_startup = match proxy_network.as_ref() { - Some(network) => match network.configure_bwrap(&mut args) { + Some(network) => match network.configure_bwrap(&mut args, logger) { Ok(startup) => Some(startup), Err(error) => { stop_proxy_network(&mut proxy_network, logger); diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index 0cf37cee0..9ff658eb9 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -5,9 +5,10 @@ use std::fs::{self, File}; use std::io::{ErrorKind, Read, Seek, SeekFrom, Write}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, ToSocketAddrs}; use std::os::fd::{AsRawFd, OwnedFd, RawFd}; use std::os::unix::process::CommandExt; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Child, Command, ExitStatus, Stdio}; use std::sync::OnceLock; use std::thread; @@ -17,7 +18,7 @@ use nix::fcntl::{fcntl, FcntlArg, FdFlag, OFlag}; use nix::unistd::{access, dup2, pipe2, AccessFlags}; use tempfile::TempDir; use wxc_common::logger::Logger; -use wxc_common::models::ProxyAddress; +use wxc_common::models::{ProxyAddress, ProxyHostPin}; const STARTUP_TIMEOUT: Duration = Duration::from_secs(5); /// How long a single `iptables` call may block on the host's `/run/xtables.lock`. @@ -51,6 +52,11 @@ const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(2); /// which returns in milliseconds, so only a genuinely wedged binary trips it. const PROBE_TIMEOUT: Duration = Duration::from_secs(5); const SLIRP_HOST_GATEWAY: &str = "10.0.2.2"; +/// The gateway as an address, for rules and pins. Kept in step with +/// [`SLIRP_HOST_GATEWAY`] by [`tests::gateway_constants_agree`]. +const SLIRP_HOST_GATEWAY_IP: Ipv4Addr = Ipv4Addr::new(10, 0, 2, 2); +/// Path the hosts-file pin is mounted over inside the sandbox. +const SANDBOX_HOSTS_PATH: &str = "/etc/hosts"; /// Egress chain installed inside the sandbox's own network namespace. const EGRESS_CHAIN: &str = "MXC_EGRESS"; /// Descriptor numbers the supervisor script hardcodes. They must stay single @@ -144,36 +150,201 @@ wait "$slirp_pid" /// The single destination a proxy-only sandbox may reach. /// -/// Must be an IPv4 literal: rules are IPv4 `iptables`, and DNS is closed inside -/// the sandbox, so a hostname could not be resolved even if a rule existed for -/// its address. LXC solves that with a hosts-file pin; Bubblewrap has no -/// equivalent yet and fails closed instead. +/// The address is IPv4 because the rule is emitted with IPv4 `iptables`. A +/// hostname endpoint is resolved on the host and carried here as its address +/// plus the [`ProxyHostPin`] the sandbox needs to agree with it: DNS is closed +/// inside the sandbox, so a hosts-file pin is the only way the workload can +/// reach a name the firewall has authorized. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ProxyEgress { - ip: std::net::Ipv4Addr, + ip: Ipv4Addr, port: u16, + pin: Option, } impl ProxyEgress { - /// Derive the permitted endpoint from the sandbox-visible proxy address. - pub(crate) fn from_address(address: &ProxyAddress) -> Result { - let host = address.host().trim_matches(['[', ']']); - let ip = host.parse::().map_err(|_| { - format!( - "Bubblewrap: proxy-only egress requires an IPv4 proxy endpoint, but the \ - sandbox-visible proxy host is '{host}'. Proxy-only egress is enforced with \ - IPv4 iptables rules and DNS is closed inside the sandbox, so a hostname or \ - IPv6 endpoint cannot be reached. Use a loopback or IPv4 proxy address." - ) - })?; - if address.port() == 0 { + /// The hosts-file pin the sandbox needs, if the endpoint is a hostname. + pub(crate) fn pin(&self) -> Option<&ProxyHostPin> { + self.pin.as_ref() + } +} + +/// The proxy as the sandbox sees it: the URL handed to the workload, and the +/// single endpoint the egress chain opens for it. +/// +/// Both come from one host-side lookup. Resolving twice can disagree -- DNS +/// round-robin reorders, or a TTL expires between the calls -- and a sandbox +/// pinned to an address the chain did not authorize cannot reach its proxy at +/// all. +#[derive(Debug)] +pub(crate) struct SandboxProxy { + address: ProxyAddress, + egress: ProxyEgress, +} + +impl SandboxProxy { + /// Derive the sandbox-visible proxy from the configured one. + /// + /// A hostname keeps its URL and is pinned, rather than being rewritten to + /// an address: the workload then presents the `Host` header and proxy-auth + /// realm the operator configured. An IP literal has no name to pin, so a + /// loopback literal is rewritten to slirp's gateway instead. + pub(crate) fn resolve(configured: &ProxyAddress) -> Result { + Self::resolve_with(configured, resolve_ipv4) + } + + /// [`Self::resolve`] against an injected resolver. + /// + /// Resolution is the one step that depends on the host's DNS, so it is a + /// parameter: the decisions layered on top of it are then testable without + /// a lookup whose answer the test does not control. + fn resolve_with( + configured: &ProxyAddress, + resolve: impl Fn(&str, u16) -> Result, + ) -> Result { + if configured.port() == 0 { return Err("Bubblewrap: proxy-only egress requires a non-zero proxy port".to_string()); } + let host = configured.host().trim_matches(['[', ']']); + + // `localhost` is reserved to loopback (RFC 6761), so it is rewritten + // rather than pinned. A pin is a sandbox-wide mapping: pointing this + // name at the gateway would redirect the workload's own loopback + // traffic to the host. + let is_reserved_loopback_name = host.eq_ignore_ascii_case("localhost"); + + if let Ok(ip) = host.parse::() { + // A literal cannot be pinned, so loopback and the wildcard are + // reached by rewriting the URL to the address slirp maps back to + // the host. `0.0.0.0` names the host just as `127.0.0.1` does. + return if ip.is_loopback() || ip.is_unspecified() { + Ok(Self::rewritten(configured)?) + } else { + Ok(Self { + address: configured.clone(), + egress: ProxyEgress { + ip, + port: configured.port(), + pin: None, + }, + }) + }; + } + + if let Ok(ip) = host.parse::() { + // A dual-stack wildcard listener accepts the gateway's IPv4 + // connection, so `::` can be rewritten. `::1` cannot: it listens + // on the IPv6 loopback only, so the rewrite would hand the sandbox + // an address nothing answers on -- failing at connect time rather + // than at policy time. + if ip.is_unspecified() { + return Self::rewritten(configured); + } + if ip.is_loopback() { + return Err(format!( + "Bubblewrap: proxy address '{}' uses the IPv6 loopback, which the private \ + network namespace cannot reach; bind the proxy to 127.0.0.1 or a dual-stack \ + wildcard address instead. The egress rule is emitted with IPv4 iptables.", + configured.host() + )); + } + return Err(ipv6_unsupported(host)); + } + + if is_reserved_loopback_name { + return Self::rewritten(configured); + } + + let ip = sandbox_facing_ip(resolve(host, configured.port())?); + let pin = configured + .host_pin(IpAddr::V4(ip)) + .map_err(|error| format!("Bubblewrap: {error}"))?; + + Ok(Self { + address: configured.clone(), + egress: ProxyEgress { + ip, + port: configured.port(), + pin, + }, + }) + } + + /// The proxy reached by rewriting its URL to slirp's gateway. + fn rewritten(configured: &ProxyAddress) -> Result { Ok(Self { - ip, - port: address.port(), + address: rewrite_to_gateway(configured)?, + egress: ProxyEgress { + ip: SLIRP_HOST_GATEWAY_IP, + port: configured.port(), + pin: None, + }, }) } + + /// The proxy URL the workload is given. + pub(crate) fn address(&self) -> &ProxyAddress { + &self.address + } + + /// The endpoint the egress chain opens. + pub(crate) fn egress(&self) -> &ProxyEgress { + &self.egress + } +} + +/// The address the sandbox reaches a host-resolved endpoint at. +/// +/// slirp gives the sandbox its own loopback, so a name that resolves to the +/// host's loopback is pinned to the gateway instead: pinning it verbatim would +/// aim the workload at itself. +fn sandbox_facing_ip(resolved: Ipv4Addr) -> Ipv4Addr { + if resolved.is_loopback() { + SLIRP_HOST_GATEWAY_IP + } else { + resolved + } +} + +/// Resolve `host` to the address the egress chain will open. +/// +/// Only the first IPv4 answer is used, and it is the same one the sandbox is +/// pinned to. Opening the rest would widen the chain to addresses the sandbox +/// can no longer select: with DNS closed, the pin is its only resolution path. +fn resolve_ipv4(host: &str, port: u16) -> Result { + let resolved: Vec = (host, port) + .to_socket_addrs() + .map_err(|error| { + format!( + "Bubblewrap: could not resolve proxy host '{host}': {error}. The proxy endpoint \ + is resolved on the host because DNS is closed inside the sandbox." + ) + })? + .map(|addr| addr.ip()) + .collect(); + + if let Some(IpAddr::V4(ip)) = resolved.iter().find(|ip| ip.is_ipv4()) { + return Ok(*ip); + } + + // A name with AAAA records and no A records is the same unenforceable case + // as an IPv6 literal, so say so rather than claiming it does not resolve. + if resolved.is_empty() { + Err(format!( + "Bubblewrap: proxy host '{host}' did not resolve to any address." + )) + } else { + Err(ipv6_unsupported(host)) + } +} + +/// The error for an endpoint that only IPv6 can reach. +fn ipv6_unsupported(host: &str) -> String { + format!( + "Bubblewrap: proxy-only egress requires an IPv4 proxy endpoint, but '{host}' is reachable \ + over IPv6 only. The egress rule is emitted with IPv4 iptables, so an IPv6 endpoint would \ + be silently dropped. Use an IPv4 proxy address." + ) } /// Runtime file descriptors Bubblewrap needs while establishing its child. @@ -268,6 +439,8 @@ pub(crate) struct ProxyNetworkNamespace { /// Handle to the supervisor's user namespace, passed to bwrap as /// `--userns`. Released once bwrap owns it; see [`Self::userns_handed_off`]. userns: Option, + /// Hosts file mounted over `/etc/hosts`, when the endpoint is a hostname. + hosts: Option, } impl ProxyNetworkNamespace { @@ -354,17 +527,39 @@ impl ProxyNetworkNamespace { }; logger.log_line("Bubblewrap: created rootless proxy network namespace supervisor"); + let hosts = match egress.pin() { + Some(pin) => { + let path = state_dir.path().join("hosts"); + if let Err(error) = write_pinned_hosts(&path, pin) { + terminate_child(&mut supervisor); + return Err(error); + } + logger.log_line(&format!( + "Bubblewrap: pinned proxy host '{}' to {} for the sandbox", + pin.hostname(), + pin.ip() + )); + Some(path) + } + None => None, + }; + Ok(Self { state_dir, supervisor, exit_writer: Some(exit_writer), pid_writer: Some(pid_writer), userns: Some(userns), + hosts, }) } /// Add the dynamic namespace and startup-barrier descriptors to bwrap. - pub(crate) fn configure_bwrap(&self, args: &mut Vec) -> Result { + pub(crate) fn configure_bwrap( + &self, + args: &mut Vec, + logger: &mut Logger, + ) -> Result { let (info_reader, info_writer) = pipe2(OFlag::O_CLOEXEC).map_err(|error| format!("Bubblewrap: pipe failed: {error}"))?; let (gate_reader, gate_writer) = @@ -375,6 +570,18 @@ impl ProxyNetworkNamespace { .as_ref() .ok_or_else(|| "Bubblewrap: proxy user namespace is already handed off".to_string())?; + if let Some(hosts) = &self.hosts { + let path = hosts + .to_str() + .ok_or_else(|| "Bubblewrap: proxy hosts pin path is not valid UTF-8".to_string())?; + if insert_hosts_bind(args, path)? { + logger.log_line( + "Bubblewrap: the proxy host pin overrides an earlier mount of \ + /etc/hosts; the sandbox sees the pinned file", + ); + } + } + let runtime_args = [ "--userns".to_string(), userns.as_raw_fd().to_string(), @@ -492,35 +699,11 @@ impl Drop for ProxyNetworkNamespace { } } -/// Return the proxy endpoint visible through slirp's host gateway. -pub(crate) fn sandbox_proxy_address(address: &ProxyAddress) -> Result { - let host = address.host().trim_matches(['[', ']']); - let parsed = host.parse::().ok(); - - // slirp's gateway reaches the host's *IPv4* loopback only. A proxy bound - // to `::1` listens on the IPv6 loopback exclusively, so rewriting it to - // the gateway would hand the sandbox an address nothing answers on -- - // failing at connect time rather than at policy time. Reject it instead. - if matches!(parsed, Some(std::net::IpAddr::V6(ip)) if ip.is_loopback()) { - return Err(format!( - "Bubblewrap: proxy address '{}' uses the IPv6 loopback, which the private \ - network namespace cannot reach; bind the proxy to 127.0.0.1 or a dual-stack \ - wildcard address instead", - address.host() - )); - } - - // `0.0.0.0` / `::` name the host itself just as `127.0.0.1` does: a proxy - // bound to the wildcard is reachable on the host's loopback, which the - // sandbox's private namespace cannot see. Both need the gateway rewrite. - // `::` is safe to rewrite to IPv4 because a dual-stack wildcard listener - // accepts IPv4 connections, which `::1` does not. - let is_host_local = host.eq_ignore_ascii_case("localhost") - || parsed.is_some_and(|ip| ip.is_loopback() || ip.is_unspecified()); - if !is_host_local { - return Ok(address.clone()); - } - +/// Rewrite a loopback or wildcard proxy URL to slirp's host gateway. +/// +/// Only IP literals reach here. A hostname is pinned instead, so that the URL +/// the workload receives is the one the operator configured. +fn rewrite_to_gateway(address: &ProxyAddress) -> Result { if let Some(original_url) = &address.original_url { let mut url = url::Url::parse(original_url).map_err(|error| { format!("Bubblewrap: failed to translate proxy URL for private networking: {error}") @@ -597,6 +780,62 @@ fn run_probe(mut command: Command, label: &str) -> Result { Ok(ProbeOutput { status, stdout }) } +/// Write the sandbox's `/etc/hosts`: the pin, then the host's own entries. +/// +/// The pin goes first because the first match wins in a hosts file. A host +/// whose `/etc/hosts` already maps the proxy name -- to a stale address, or to +/// one the egress chain never authorized -- would otherwise shadow the pin and +/// leave the sandbox unable to reach its proxy. +/// +/// The host's entries are kept rather than discarded so the sandbox retains +/// the mappings a workload expects (`localhost` above all). They are read +/// before the file is created, so a read failure cannot leave a half-written +/// pin behind. +fn write_pinned_hosts(path: &PathBuf, pin: &ProxyHostPin) -> Result<(), String> { + // The host file is optional: a host without one simply contributes no + // entries, which is not a reason to refuse to pin. + let existing = match fs::read_to_string(SANDBOX_HOSTS_PATH) { + Ok(contents) => contents, + Err(error) if error.kind() == ErrorKind::NotFound => String::new(), + Err(error) => { + return Err(format!( + "Bubblewrap: failed to read {SANDBOX_HOSTS_PATH} while pinning the proxy host: \ + {error}" + )) + } + }; + + let contents = format!("{}\n{}", pin.hosts_line(), existing); + fs::write(path, contents) + .map_err(|error| format!("Bubblewrap: failed to write the proxy hosts pin: {error}")) +} + +/// Splice the pinned-hosts bind in just before the command separator. +/// +/// bwrap applies mounts in argument order and the last mount at a path wins, +/// so the bind must come after every baseline and user-policy mount for the +/// pin to survive -- including one that would otherwise expose the host's own +/// `/etc/hosts`. Returns `true` when an earlier mount already targeted +/// `/etc/hosts`, so the caller can report that the pin overrides it. +fn insert_hosts_bind(args: &mut Vec, hosts_path: &str) -> Result { + let separator = args + .iter() + .position(|arg| arg == "--") + .ok_or_else(|| "Bubblewrap: argument list has no command separator".to_string())?; + let overrides = args[..separator] + .iter() + .any(|arg| arg == SANDBOX_HOSTS_PATH); + args.splice( + separator..separator, + [ + "--ro-bind".to_string(), + hosts_path.to_string(), + SANDBOX_HOSTS_PATH.to_string(), + ], + ); + Ok(overrides) +} + pub(crate) fn probe_dependencies() -> 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 @@ -880,51 +1119,138 @@ mod tests { use super::*; #[test] - fn egress_opens_the_translated_loopback_proxy() { - // Rules must open the translated address, not the original loopback. + fn gateway_constants_agree() { + // The rule and the pin are written from the address; the URL rewrite + // from the string. A drift between them would open one endpoint and + // point the workload at another. + assert_eq!(SLIRP_HOST_GATEWAY_IP.to_string(), SLIRP_HOST_GATEWAY); + } + + #[test] + fn egress_opens_the_gateway_for_a_loopback_literal() { + // A literal has no name to pin, so the URL is rewritten and the rule + // must open the rewritten address, not the original loopback. let address = ProxyAddress::new("127.0.0.1".into(), 8080); - let translated = sandbox_proxy_address(&address).unwrap(); - let egress = ProxyEgress::from_address(&translated).unwrap(); + let resolved = SandboxProxy::resolve(&address).unwrap(); - assert_eq!(egress.ip.to_string(), SLIRP_HOST_GATEWAY); - assert_eq!(egress.port, 8080); + assert_eq!(resolved.address().host(), SLIRP_HOST_GATEWAY); + assert_eq!(resolved.egress().ip.to_string(), SLIRP_HOST_GATEWAY); + assert_eq!(resolved.egress().port, 8080); + assert!(resolved.egress().pin().is_none()); } #[test] fn egress_accepts_a_routable_ipv4_proxy() { let address = ProxyAddress::new("10.1.2.3".into(), 3128); - let egress = ProxyEgress::from_address(&address).unwrap(); + let resolved = SandboxProxy::resolve(&address).unwrap(); + + assert_eq!(resolved.address().host(), "10.1.2.3"); + assert_eq!(resolved.egress().ip.to_string(), "10.1.2.3"); + assert_eq!(resolved.egress().port, 3128); + assert!(resolved.egress().pin().is_none()); + } - assert_eq!(egress.ip.to_string(), "10.1.2.3"); - assert_eq!(egress.port, 3128); + /// A resolver that answers every name with `ip`, so pin decisions can be + /// tested without a lookup the test does not control. + fn resolver(ip: [u8; 4]) -> impl Fn(&str, u16) -> Result { + move |_, _| Ok(Ipv4Addr::from(ip)) } #[test] - fn egress_rejects_a_hostname_proxy() { - // DNS is closed, so a name the workload cannot resolve must fail loudly - // rather than yield a rule that is never reachable. + fn pins_a_hostname_and_keeps_its_url() { + // The workload must present the configured name -- proxy-auth realms + // and Host headers are keyed on it -- so the name is pinned rather + // than rewritten to an address. let address = ProxyAddress::from_url( - "http://proxy.corp.example:3128", + "http://proxy.corp.example:3128/", "proxy.corp.example".into(), 3128, ); - let error = ProxyEgress::from_address(&address).unwrap_err(); + let resolved = SandboxProxy::resolve_with(&address, resolver([10, 1, 2, 3])).unwrap(); - assert!( - error.contains("proxy.corp.example"), - "error should name the offending host: {error}" + assert_eq!( + resolved.address().to_url(), + "http://proxy.corp.example:3128/" ); - assert!( - error.contains("IPv4"), - "error should explain the IPv4 requirement: {error}" + + let pin = resolved.egress().pin().expect("hostname must be pinned"); + assert_eq!(pin.hostname(), "proxy.corp.example"); + assert_eq!(pin.ip().to_string(), "10.1.2.3"); + assert_eq!(resolved.egress().ip.to_string(), "10.1.2.3"); + } + + #[test] + fn pins_a_loopback_hostname_to_the_gateway() { + // slirp gives the sandbox its own loopback, so a name resolving to the + // host's loopback must be pinned to the gateway -- pinning it verbatim + // would aim the workload at itself. + let address = ProxyAddress::from_url("http://proxy.local:3128", "proxy.local".into(), 3128); + let resolved = SandboxProxy::resolve_with(&address, resolver([127, 0, 1, 1])).unwrap(); + + let pin = resolved.egress().pin().expect("hostname must be pinned"); + assert_eq!(pin.ip().to_string(), SLIRP_HOST_GATEWAY); + assert_eq!(resolved.egress().ip.to_string(), SLIRP_HOST_GATEWAY); + } + + #[test] + fn rewrites_localhost_instead_of_pinning_it() { + // `localhost` is reserved to loopback (RFC 6761). A pin is sandbox + // wide, so pinning it would redirect the workload's own loopback + // traffic to the host. + let address = ProxyAddress::from_url("http://localhost:3128/", "localhost".into(), 3128); + let resolved = + SandboxProxy::resolve_with(&address, |_, _| panic!("localhost must not be resolved")) + .unwrap(); + + assert_eq!(resolved.address().host(), SLIRP_HOST_GATEWAY); + assert!(resolved.egress().pin().is_none()); + assert_eq!(resolved.egress().ip.to_string(), SLIRP_HOST_GATEWAY); + } + + #[test] + fn pins_a_routable_hostname_to_its_resolved_address() { + // Only a loopback answer is redirected to the gateway; a routable one + // is reached directly through slirp. + assert_eq!( + sandbox_facing_ip(Ipv4Addr::new(10, 1, 2, 3)), + Ipv4Addr::new(10, 1, 2, 3) + ); + assert_eq!( + sandbox_facing_ip(Ipv4Addr::new(127, 0, 0, 1)), + SLIRP_HOST_GATEWAY_IP ); } #[test] fn egress_rejects_an_ipv6_proxy() { // Rules are IPv4-only, so an IPv6 endpoint would never be opened. + let loopback = ProxyAddress::new("[::1]".into(), 8080); + let error = SandboxProxy::resolve(&loopback).unwrap_err(); + assert!( + error.contains("IPv4"), + "error should explain the IPv4 requirement: {error}" + ); + let routable = ProxyAddress::new("2001:db8::1".into(), 8080); - assert!(ProxyEgress::from_address(&routable).is_err()); + assert!(SandboxProxy::resolve(&routable).is_err()); + } + + #[test] + fn egress_rejects_an_unresolvable_hostname() { + // `.invalid` is reserved by RFC 2606 and never resolves, so a name + // that cannot be pinned must fail loudly rather than yield a rule the + // sandbox can never reach. + let address = ProxyAddress::from_url( + "http://proxy.corp.invalid:3128", + "proxy.corp.invalid".into(), + 3128, + ); + let error = SandboxProxy::resolve(&address).unwrap_err(); + + assert!( + error.contains("proxy.corp.invalid"), + "error should name the offending host: {error}" + ); } /// `::1` used to be rewritten to the IPv4 gateway, which handed the sandbox @@ -935,7 +1261,7 @@ mod tests { fn rejects_an_ipv6_loopback_proxy_instead_of_translating_it() { for host in ["[::1]", "::1"] { let address = ProxyAddress::new(host.into(), 8080); - let error = sandbox_proxy_address(&address).unwrap_err(); + let error = SandboxProxy::resolve(&address).unwrap_err(); assert!( error.contains("IPv6 loopback"), @@ -949,15 +1275,15 @@ mod tests { #[test] fn ipv6_loopback_rejection_leaves_the_ipv6_wildcard_translatable() { let address = ProxyAddress::new("[::]".into(), 8080); - let translated = sandbox_proxy_address(&address).unwrap(); + let translated = SandboxProxy::resolve(&address).unwrap(); - assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); + assert_eq!(translated.address().host(), SLIRP_HOST_GATEWAY); } #[test] fn egress_rejects_a_zero_port() { let address = ProxyAddress::new("10.0.2.2".into(), 0); - let error = ProxyEgress::from_address(&address).unwrap_err(); + let error = SandboxProxy::resolve(&address).unwrap_err(); assert!( error.contains("non-zero"), @@ -976,6 +1302,102 @@ mod tests { SUPERVISOR_SCRIPT.replace(r#" -w "$lock_wait""#, "") } + /// The pin a hostname proxy produces, for the hosts-file tests. + fn test_pin() -> ProxyHostPin { + ProxyAddress::new("proxy.example".into(), 3128) + .host_pin(IpAddr::V4(SLIRP_HOST_GATEWAY_IP)) + .expect("hostname must be pinnable") + .expect("a hostname needs a pin") + } + + #[test] + fn pinned_hosts_file_puts_the_pin_first() { + // First match wins in a hosts file, so a host entry for the same name + // must not be able to shadow the pin. + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hosts"); + write_pinned_hosts(&path, &test_pin()).unwrap(); + + let contents = fs::read_to_string(&path).unwrap(); + assert_eq!( + contents.lines().next().unwrap(), + format!("{SLIRP_HOST_GATEWAY} proxy.example") + ); + } + + #[test] + fn pinned_hosts_file_keeps_the_host_entries() { + // Dropping them would cost the sandbox `localhost`, which workloads + // and the loopback exemption both rely on. + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hosts"); + write_pinned_hosts(&path, &test_pin()).unwrap(); + + let written = fs::read_to_string(&path).unwrap(); + let host_file = fs::read_to_string(SANDBOX_HOSTS_PATH).unwrap_or_default(); + for line in host_file.lines().filter(|line| !line.trim().is_empty()) { + assert!( + written.contains(line), + "host entry should be preserved: {line}" + ); + } + } + + #[test] + fn hosts_bind_is_applied_after_policy_mounts() { + // bwrap applies mounts in order and the last at a path wins, so the + // pin must land past every policy mount -- including one that would + // otherwise expose the host's own /etc/hosts. + let mut args = vec![ + "--ro-bind-try".to_string(), + "/etc".to_string(), + "/etc".to_string(), + "--bind".to_string(), + "/etc/hosts".to_string(), + "/etc/hosts".to_string(), + "--".to_string(), + "sh".to_string(), + ]; + insert_hosts_bind(&mut args, "/tmp/pin/hosts").unwrap(); + + let pin = args + .windows(3) + .position(|window| window == ["--ro-bind", "/tmp/pin/hosts", SANDBOX_HOSTS_PATH]) + .expect("pin bind should be present"); + let policy = args + .windows(3) + .position(|window| window == ["--bind", "/etc/hosts", "/etc/hosts"]) + .expect("policy mount should be retained"); + let separator = args.iter().position(|arg| arg == "--").unwrap(); + + assert!(pin > policy, "pin must be applied after the policy mount"); + assert!(pin < separator, "pin must precede the command separator"); + } + + #[test] + fn hosts_bind_reports_an_overridden_policy_mount() { + // Silently shadowing a user's own /etc/hosts mount would make the + // sandbox's view of the file inexplicable from the config alone. + let mut args = vec![ + "--bind".to_string(), + "/custom/hosts".to_string(), + "/etc/hosts".to_string(), + "--".to_string(), + ]; + assert!(insert_hosts_bind(&mut args, "/tmp/pin/hosts").unwrap()); + + let mut untouched = vec!["--ro-bind-try".to_string(), "--".to_string()]; + assert!(!insert_hosts_bind(&mut untouched, "/tmp/pin/hosts").unwrap()); + } + + #[test] + fn hosts_bind_requires_a_command_separator() { + // Appending blindly to an argument list with no separator would pass + // the bind to the workload as arguments and leave it unpinned. + let mut args = vec!["--ro-bind-try".to_string(), "/etc".to_string()]; + assert!(insert_hosts_bind(&mut args, "/tmp/pin/hosts").is_err()); + } + /// Byte offset of `needle` in the normalised supervisor script. fn script_offset(needle: &str) -> usize { normalised_script() @@ -1072,32 +1494,35 @@ mod tests { } #[test] - fn translates_loopback_proxy_to_slirp_gateway() { + fn translates_loopback_literal_to_slirp_gateway() { let address = ProxyAddress::new("127.0.0.1".into(), 8080); - let translated = sandbox_proxy_address(&address).unwrap(); + let resolved = SandboxProxy::resolve(&address).unwrap(); - assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); - assert_eq!(translated.port(), 8080); - assert_eq!(translated.to_url(), "http://10.0.2.2:8080"); + assert_eq!(resolved.address().host(), SLIRP_HOST_GATEWAY); + assert_eq!(resolved.address().port(), 8080); + assert_eq!(resolved.address().to_url(), "http://10.0.2.2:8080"); } #[test] fn translates_loopback_url_without_losing_url_components() { - let address = ProxyAddress::from_url("http://localhost:3128/", "localhost".into(), 3128); - let translated = sandbox_proxy_address(&address).unwrap(); + let address = + ProxyAddress::from_url("http://user:pass@127.0.0.1:3128/", "127.0.0.1".into(), 3128); + let resolved = SandboxProxy::resolve(&address).unwrap(); - assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); - assert_eq!(translated.port(), 3128); - assert_eq!(translated.to_url(), "http://10.0.2.2:3128/"); + assert_eq!(resolved.address().host(), SLIRP_HOST_GATEWAY); + assert_eq!(resolved.address().port(), 3128); + assert_eq!( + resolved.address().to_url(), + "http://user:pass@10.0.2.2:3128/" + ); } #[test] - fn leaves_remote_proxy_unchanged() { - let address = - ProxyAddress::from_url("https://proxy.example:8443", "proxy.example".into(), 8443); - let translated = sandbox_proxy_address(&address).unwrap(); + fn leaves_a_routable_literal_proxy_unchanged() { + let address = ProxyAddress::from_url("https://10.1.2.3:8443", "10.1.2.3".into(), 8443); + let resolved = SandboxProxy::resolve(&address).unwrap(); - assert_eq!(translated.to_url(), address.to_url()); + assert_eq!(resolved.address().to_url(), address.to_url()); } /// A proxy bound to the wildcard address is reachable on the host's @@ -1106,20 +1531,20 @@ mod tests { #[test] fn translates_wildcard_proxy_to_slirp_gateway() { let address = ProxyAddress::new("0.0.0.0".into(), 8080); - let translated = sandbox_proxy_address(&address).unwrap(); + let translated = SandboxProxy::resolve(&address).unwrap(); - assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); - assert_eq!(translated.port(), 8080); + assert_eq!(translated.address().host(), SLIRP_HOST_GATEWAY); + assert_eq!(translated.address().port(), 8080); } #[test] fn translates_bracketed_ipv6_wildcard_proxy_to_slirp_gateway() { let address = ProxyAddress::from_url("http://[::]:3128/", "[::]".into(), 3128); - let translated = sandbox_proxy_address(&address).unwrap(); + let translated = SandboxProxy::resolve(&address).unwrap(); - assert_eq!(translated.host(), SLIRP_HOST_GATEWAY); - assert_eq!(translated.port(), 3128); - assert_eq!(translated.to_url(), "http://10.0.2.2:3128/"); + assert_eq!(translated.address().host(), SLIRP_HOST_GATEWAY); + assert_eq!(translated.address().port(), 3128); + assert_eq!(translated.address().to_url(), "http://10.0.2.2:3128/"); } /// The descriptor must reach the child that was prepared and no other. The diff --git a/tests/configs/bubblewrap_network_proxy_hostname.json b/tests/configs/bubblewrap_network_proxy_hostname.json new file mode 100644 index 000000000..38aeb26a5 --- /dev/null +++ b/tests/configs/bubblewrap_network_proxy_hostname.json @@ -0,0 +1,12 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-Bubblewrap-Network-Proxy-Hostname", + "containment": "bubblewrap", + "process": { + "commandLine": "bash -c 'set -u; if ! getent hosts \"{{PROXY_HOST}}\" >/dev/null; then echo PIN_MISSING; exit 1; fi; echo PIN_PRESENT_OK; if ! curl -fsSL --max-time 15 https://api.github.com/zen >/dev/null; then echo PIN_PROXY_UNREACHABLE; exit 1; fi; echo PIN_PROXY_OK; timeout 6 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1; rc=$?; if [ \"$rc\" = 0 ]; then echo PIN_DIRECT_EGRESS_LEAKED; exit 1; fi; echo PIN_DIRECT_BLOCKED_OK'" + }, + "network": { + "defaultPolicy": "allow", + "proxy": { "url": "http://{{PROXY_HOST}}:{{PROXY_PORT}}" } + } +} diff --git a/tests/scripts/run_bwrap_network_proxy_test.sh b/tests/scripts/run_bwrap_network_proxy_test.sh index 7b426c15b..6c8449506 100644 --- a/tests/scripts/run_bwrap_network_proxy_test.sh +++ b/tests/scripts/run_bwrap_network_proxy_test.sh @@ -233,4 +233,86 @@ for sentinel in CONTROL_PROXY_REACHABLE_OK DIRECT_EGRESS_BLOCKED_OK LOOPBACK_EXE done echo "PASS: proxy-only egress enforcement" +# Hostname proxy endpoints. The endpoint is resolved on the host and pinned +# into the sandbox's /etc/hosts, because DNS is closed inside the sandbox. +# +# The host's own name is used because it resolves everywhere without editing +# /etc/hosts (which would need root), and it resolves to loopback -- the case +# the pin has to redirect to slirp's gateway rather than carry through. +echo "Running Bubblewrap hostname proxy pin test..." +PROXY_HOST="$(hostname)" +if ! getent hosts "$PROXY_HOST" >/dev/null; then + echo "SKIP: hostname proxy pin (host name '$PROXY_HOST' does not resolve)" +else + TEST_PROXY="$REPO_DIR/src/target/release/unix-test-proxy" + if [ ! -x "$TEST_PROXY" ]; then + TEST_PROXY="$REPO_DIR/src/target/debug/unix-test-proxy" + fi + if [ ! -x "$TEST_PROXY" ]; then + echo "FAIL: hostname proxy pin (unix-test-proxy not built)" + exit 1 + fi + + PIN_DIR="$(mktemp -d)" + PIN_PROXY_PID="" + cleanup_pin() { + if [ -n "$PIN_PROXY_PID" ]; then + kill "$PIN_PROXY_PID" 2>/dev/null || true + wait "$PIN_PROXY_PID" 2>/dev/null || true + fi + exec 9>&- 2>/dev/null || true + rm -rf "$PIN_DIR" + } + trap cleanup_pin EXIT + + # The proxy exits when its stdin reaches EOF, which is how the coordinator + # stops an orphan. Driving it from a script means supplying that pipe here: + # the fifo is opened read-write so the open does not block, and the script + # holding it keeps the proxy alive for the run. + mkfifo "$PIN_DIR/parent.pipe" + exec 9<>"$PIN_DIR/parent.pipe" + + # The proxy binds an OS-assigned port and publishes it, so the config is + # generated per run rather than committed with a fixed port. + "$TEST_PROXY" --ready-file "$PIN_DIR/ready.port" --bind-address 127.0.0.1 \ + <"$PIN_DIR/parent.pipe" >"$PIN_DIR/proxy.log" 2>&1 & + PIN_PROXY_PID=$! + for _ in $(seq 1 100); do + [ -s "$PIN_DIR/ready.port" ] && break + if ! kill -0 "$PIN_PROXY_PID" 2>/dev/null; then + cat "$PIN_DIR/proxy.log" + echo "FAIL: hostname proxy pin (test proxy exited before publishing its port)" + exit 1 + fi + sleep 0.1 + done + PROXY_PORT="$(cat "$PIN_DIR/ready.port" 2>/dev/null || true)" + if [ -z "$PROXY_PORT" ]; then + cat "$PIN_DIR/proxy.log" + echo "FAIL: hostname proxy pin (test proxy did not publish a port)" + exit 1 + fi + + sed -e "s/{{PROXY_HOST}}/$PROXY_HOST/g" -e "s/{{PROXY_PORT}}/$PROXY_PORT/g" \ + "$REPO_DIR/tests/configs/bubblewrap_network_proxy_hostname.json" \ + >"$PIN_DIR/hostname.json" + + if ! PIN_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$PIN_DIR/hostname.json" 2>&1); then + echo "$PIN_OUT" + echo "FAIL: hostname proxy pin (lxc-exec returned non-zero)" + exit 1 + fi + for sentinel in PIN_PRESENT_OK PIN_PROXY_OK PIN_DIRECT_BLOCKED_OK; do + if ! grep -q "$sentinel" <<<"$PIN_OUT"; then + echo "$PIN_OUT" + echo "FAIL: hostname proxy pin (sentinel '$sentinel' not found in output)" + exit 1 + fi + done + cleanup_pin + trap - EXIT + echo "PASS: hostname proxy pin" +fi + echo "Bubblewrap network proxy tests complete." From cfba3d5a34dfa70b07d78ee060ac7c6126af13ab Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 18 Aug 2026 16:05:53 -0700 Subject: [PATCH 2/6] Document Bubblewrap proxy enforcement and hostname pinning in the schema reference Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- docs/schema.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/schema.md b/docs/schema.md index 31215484d..0625f95f9 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -126,6 +126,18 @@ cannot mix both formats in one request. // The chain hooks FORWARD, so traffic addressed to the // bridge gateway itself is delivered locally via INPUT // and is outside what this chain governs. + // Under Bubblewrap on schema 0.8+ the proxy is likewise + // enforced, in the sandbox's own network namespace: + // egress is dropped except the proxy endpoint, and DNS + // is not opened. A url-form hostname is resolved on the + // host and pinned into the sandbox's /etc/hosts, since + // the sandbox has no resolver of its own. `localhost`, + // 127.0.0.0/8 and the wildcards 0.0.0.0 / :: are + // rewritten to the slirp gateway; `::1` is rejected, + // because an IPv6-loopback listener cannot accept the + // IPv4 connection that gateway produces. On schema + // 0.6/0.7 Bubblewrap keeps the cooperative-only + // behavior (no egress rules). }, "ui": { From f513b299eb7a85445dad66937075b968ffac1c31 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 18 Aug 2026 17:39:38 -0700 Subject: [PATCH 3/6] 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 | 12 +- docs/schema.md | 5 +- .../bubblewrap/common/src/bwrap_command.rs | 11 +- .../bubblewrap/common/src/bwrap_runner.rs | 161 ++++++++++- .../bubblewrap/common/src/proxy_network.rs | 240 +++++++++++++++- tests/scripts/run_bwrap_network_proxy_test.sh | 260 +++++++++++++----- 6 files changed, 595 insertions(+), 94 deletions(-) diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index 7f4747644..7c57bd54a 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -446,8 +446,16 @@ request fails if its private namespace cannot be configured. (RFC 6761) and a pin is a sandbox-wide mapping, so pinning it would redirect the workload's own loopback traffic to the host. - The generated `/etc/hosts` preserves the host's existing entries after the - pin line, so `localhost` and friends keep working. A `readwritePaths` entry - covering `/etc/hosts` is overridden by the pin mount. + pin line, so `localhost` and friends keep working. A `readwritePaths` or + `readonlyPaths` entry covering `/etc/hosts` is overridden by the pin mount, + with a warning — that narrows the caller's access rather than widening it. + - A **`deniedPaths`** entry covering `/etc/hosts` (directly or via an + ancestor such as `/etc`) is **rejected** instead. The pin is applied after + every policy mount, so honouring it would hand back a readable file + populated from the host's own `/etc/hosts` — the opposite of the requested + denial. Give the proxy an IP address instead, which needs no pin. A more + specific grant beneath the denial (for example denying `/etc` while listing + `/etc/hosts` under `readonlyPaths`) takes effect and is accepted. - 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"` diff --git a/docs/schema.md b/docs/schema.md index 0625f95f9..9b2dcc958 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -135,7 +135,10 @@ cannot mix both formats in one request. // 127.0.0.0/8 and the wildcards 0.0.0.0 / :: are // rewritten to the slirp gateway; `::1` is rejected, // because an IPv6-loopback listener cannot accept the - // IPv4 connection that gateway produces. On schema + // IPv4 connection that gateway produces. Because the + // pin outranks every filesystem mount, a `deniedPaths` + // entry covering /etc/hosts is rejected rather than + // silently overridden. On schema // 0.6/0.7 Bubblewrap keeps the cooperative-only // behavior (no egress rules). }, diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index a569d22b9..9e98b73d7 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -13,6 +13,13 @@ use wxc_common::filesystem_resolve::FsIntent; use wxc_common::models::{ExecutionRequest, NetworkEnforcementMode, NetworkPolicy, ProxyAddress}; use wxc_common::proxy_env::{is_managed_proxy_key, PROXY_SET_KEYS}; +/// The fixed prefix of the command bwrap is asked to run. +/// +/// `build_args` appends this last, so its position identifies where the +/// options end -- unlike a bare `--` token, which a caller-supplied +/// environment value can also be. Shared so the two cannot drift apart. +pub(crate) const COMMAND_TAIL: [&str; 3] = ["--", "sh", "-c"]; + /// Read-only host paths bind-mounted into every Bubblewrap sandbox as the /// deny-by-default baseline. Mirrors the seatbelt backend's /// `SYSTEM_READ_ALLOW` (`src/backends/seatbelt/common/src/profile_builder.rs`): @@ -378,9 +385,7 @@ pub(crate) fn build_args_classified_with_mode( } // -- Command ----------------------------------------------------------- - args.push("--".into()); - args.push("sh".into()); - args.push("-c".into()); + args.extend(COMMAND_TAIL.iter().map(|arg| arg.to_string())); args.push(request.script_code.clone()); args diff --git a/src/backends/bubblewrap/common/src/bwrap_runner.rs b/src/backends/bubblewrap/common/src/bwrap_runner.rs index 093da3bb8..d93603bc0 100644 --- a/src/backends/bubblewrap/common/src/bwrap_runner.rs +++ b/src/backends/bubblewrap/common/src/bwrap_runner.rs @@ -114,17 +114,16 @@ impl SandboxBackend for BubblewrapScriptRunner { // 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 (an IPv6 loopback the - // gateway cannot reach, a routable or IPv6-only endpoint the IPv4-only - // rules cannot express, a zero port, a hostname that does not resolve). - // Running it here means the caller learns at policy time instead of - // after a namespace, a slirp instance and a rule set have already been - // built and torn down. + // chain. That step can reject an endpoint outright (an IPv6 loopback + // the gateway cannot reach, a routable or IPv6-only endpoint the + // IPv4-only rules cannot express, a zero port), and those verdicts + // follow from the configured address alone, so they are reported here + // instead of after a proxy has been started. // - // A hostname costs a DNS lookup here, which `run` repeats -- the - // resolved address is deliberately not carried over, since the one the - // egress chain opens must come from the same lookup that produces the - // pin the sandbox is given. + // A hostname is deliberately left to `run`: its verdict needs a lookup, + // and the answer is the pin the sandbox is given, so resolving here + // would either resolve twice or pin an address the egress chain never + // opened. // // The builtin test server has no address until it is started (the // parser leaves a port-0 placeholder), so its endpoint stays on the @@ -134,9 +133,15 @@ impl SandboxBackend for BubblewrapScriptRunner { == ResolvedNetworkMode::ProxyOnly; if proxy_only && !request.policy.network_proxy.builtin_test_server { if let Some(address) = request.policy.network_proxy.address.as_ref() { - if let Err(error) = proxy_network::SandboxProxy::resolve(address) { + if let Err(error) = proxy_network::SandboxProxy::check_without_resolving(address) { return Err(ScriptResponse::error(&error)); } + if let Err(error) = + proxy_network::check_hosts_pin_against_policy(address, &request.policy) + { + return Err(ScriptResponse::error(&error)); + } + // Repeated in `spawn` against the normalized policy. } } @@ -212,11 +217,38 @@ impl SandboxBackend for BubblewrapScriptRunner { } None => request, }; + // The masks are built from the list above, not from the one the caller + // wrote, so the pin conflict has to be judged against it too. + if let Err(msg) = check_pin_against_denied_hosts(request) { + return Err(ScriptResponse::error(&msg)); + } let child = self.spawn_bwrap(request, &plan.files, logger, stdio)?; Ok(Box::new(BubblewrapSandboxProcess::new(child))) } } +/// Reject a hostname proxy pin that would defeat a denied `/etc/hosts`. +/// +/// Runs in both `validate` (against the policy as written, so the caller hears +/// about it early) and `spawn` (against the normalized policy, which is what +/// the masks are built from). One is not enough: `resolve_denied_paths` +/// rewrites `/etc/../etc/hosts` -- or any symlinked spelling -- to +/// `/etc/hosts`, which the written form never matches, and the pin is spliced +/// after every policy mount, so it hands back the file the policy masked. +fn check_pin_against_denied_hosts(request: &ExecutionRequest) -> Result<(), String> { + let proxy_only = + ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled()) + == ResolvedNetworkMode::ProxyOnly; + // The builtin test server is never a hostname, so it is never pinned. + if !proxy_only || request.policy.network_proxy.builtin_test_server { + return Ok(()); + } + match request.policy.network_proxy.address.as_ref() { + Some(address) => proxy_network::check_hosts_pin_against_policy(address, &request.policy), + None => Ok(()), + } +} + impl BubblewrapScriptRunner { /// Set up networking and spawn `bwrap`, returning a [`BwrapChild`] wrapped /// by the [`SandboxProcess`] handle. With [`StdioMode::Pipes`] the child's @@ -872,7 +904,7 @@ mod tests { /// Proxy-only mode rewrites the endpoint to slirp's gateway and opens /// exactly that address, so an endpoint neither step can express has to be - /// refused at policy time -- not after a namespace and rule set were built. + /// refused at policy time -- before a proxy is started. #[test] fn validate_rejects_an_ipv6_loopback_proxy_endpoint_before_the_environment_probe() { let mut req = base_request(); @@ -967,6 +999,111 @@ mod tests { ); } + /// A hostname endpoint is reached by pinning it over `/etc/hosts`, and the + /// pin outranks every policy mount -- so a policy that denies the file must + /// refuse the combination rather than silently hand the file back. + #[test] + 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.denied_paths = vec!["/etc/hosts".into()]; + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("proxy.example.com".into(), 3128)), + builtin_test_server: false, + }; + + let runner = BubblewrapScriptRunner::new(); + let err = runner.validate(&req).unwrap_err(); + + assert!( + err.error_message.contains("deniedPaths"), + "the conflict must be refused at policy time: {}", + err.error_message + ); + } + + /// `validate` compares the denied paths as written, so a spelling that only + /// becomes `/etc/hosts` after normalization slips past it. `spawn` builds + /// the masks from the normalized list, so the mask lands on `/etc/hosts` + /// and the pin -- spliced after every policy mount -- then hands the file + /// back. Regression test: the check must also see the normalized policy. + #[test] + fn a_dotdot_spelling_of_a_denied_hosts_file_still_refuses_the_pin() { + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("proxy.example.com".into(), 3128)), + builtin_test_server: false, + }; + + // As written, this matches nothing the check looks for. + req.policy.denied_paths = vec!["/etc/../etc/hosts".into()]; + assert!( + check_pin_against_denied_hosts(&req).is_ok(), + "precondition: the written form is not recognized" + ); + + // Normalized -- what `spawn` actually builds the masks from. + let mut normalized = req.clone(); + normalized.policy.denied_paths = vec!["/etc/hosts".into()]; + let err = check_pin_against_denied_hosts(&normalized) + .expect_err("the normalized policy must refuse the pin"); + assert!(err.contains("deniedPaths"), "{err}"); + } + + /// The rejection is scoped to endpoints that actually produce a pin. An IP + /// literal is reached without touching `/etc/hosts`, so denying the file + /// stays compatible -- and that is the escape hatch the message offers. + #[test] + fn validate_accepts_an_ip_proxy_endpoint_alongside_a_denied_hosts_file() { + let mut req = base_request(); + req.schema_version = "0.8.0-alpha".into(); + req.policy.denied_paths = vec!["/etc/hosts".into()]; + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("10.1.2.3".into(), 3128)), + builtin_test_server: false, + }; + + let runner = BubblewrapScriptRunner::new(); + let message = runner + .validate(&req) + .err() + .map(|err| err.error_message) + .unwrap_or_default(); + + assert!( + !message.contains("deniedPaths"), + "an endpoint that needs no pin must not inherit the rejection: {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. + #[test] + fn validate_leaves_a_legacy_schema_hosts_denial_untouched() { + let mut req = base_request(); + req.schema_version = "0.7.0-alpha".into(); + req.policy.denied_paths = vec!["/etc/hosts".into()]; + req.policy.network_proxy = ProxyConfig { + address: Some(ProxyAddress::new("proxy.example.com".into(), 3128)), + builtin_test_server: false, + }; + + let runner = BubblewrapScriptRunner::new(); + let message = runner + .validate(&req) + .err() + .map(|err| err.error_message) + .unwrap_or_default(); + + assert!( + !message.contains("deniedPaths"), + "legacy proxy mode pins nothing, so it must not inherit the \ + private-namespace rejection: {message}" + ); + } + #[test] fn validate_rejects_empty_script_before_environment_probe() { // Empty script_code is a user-input error and must be surfaced diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index 9ff658eb9..41f6f6c9d 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -3,6 +3,7 @@ //! Rootless private networking for Bubblewrap proxy mode. +use std::cell::Cell; use std::fs::{self, File}; use std::io::{ErrorKind, Read, Seek, SeekFrom, Write}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, ToSocketAddrs}; @@ -17,8 +18,11 @@ use std::time::{Duration, Instant}; use nix::fcntl::{fcntl, FcntlArg, FdFlag, OFlag}; use nix::unistd::{access, dup2, pipe2, AccessFlags}; use tempfile::TempDir; +use wxc_common::filesystem_resolve::{resolve_mount_order, FsIntent}; use wxc_common::logger::Logger; -use wxc_common::models::{ProxyAddress, ProxyHostPin}; +use wxc_common::models::{ContainerPolicy, ProxyAddress, ProxyHostPin}; + +use crate::bwrap_command::COMMAND_TAIL; const STARTUP_TIMEOUT: Duration = Duration::from_secs(5); /// How long a single `iptables` call may block on the host's `/run/xtables.lock`. @@ -193,6 +197,36 @@ impl SandboxProxy { Self::resolve_with(configured, resolve_ipv4) } + /// Apply every rejection [`Self::resolve`] can reach without a lookup. + /// + /// A hostname's verdict depends on what it resolves to, and that answer is + /// also the pin the sandbox is given -- so it has to come from the single + /// lookup `run` performs, not from a second one here. Deferring is detected + /// rather than predicted: the injected resolver records that it was asked. + pub(crate) fn check_without_resolving(configured: &ProxyAddress) -> Result<(), String> { + let (needs_lookup, checked) = Self::inspect_without_resolving(configured); + if needs_lookup { + return Ok(()); + } + checked + } + + /// Run the static checks, reporting whether a lookup would have followed. + /// + /// A lookup is needed exactly when the endpoint is a hostname, which is + /// also exactly when a hosts pin is created -- an IP literal resolves to + /// `Ok(None)` from [`ProxyAddress::host_pin`]. Callers use the flag to + /// reason about the pin without duplicating that classification. + fn inspect_without_resolving(configured: &ProxyAddress) -> (bool, Result<(), String>) { + let asked_to_resolve = Cell::new(false); + let checked = Self::resolve_with(configured, |_, _| { + asked_to_resolve.set(true); + Err(String::new()) + }); + + (asked_to_resolve.get(), checked.map(|_| ())) + } + /// [`Self::resolve`] against an injected resolver. /// /// Resolution is the one step that depends on the host's DNS, so it is a @@ -297,9 +331,11 @@ impl SandboxProxy { /// /// slirp gives the sandbox its own loopback, so a name that resolves to the /// host's loopback is pinned to the gateway instead: pinning it verbatim would -/// aim the workload at itself. +/// aim the workload at itself. `0.0.0.0` is an answer `/etc/hosts` can produce +/// and names the host the same way, so it is translated alongside loopback -- +/// matching the IP-literal path, which rewrites both. fn sandbox_facing_ip(resolved: Ipv4Addr) -> Ipv4Addr { - if resolved.is_loopback() { + if resolved.is_loopback() || resolved.is_unspecified() { SLIRP_HOST_GATEWAY_IP } else { resolved @@ -810,6 +846,58 @@ fn write_pinned_hosts(path: &PathBuf, pin: &ProxyHostPin) -> Result<(), String> .map_err(|error| format!("Bubblewrap: failed to write the proxy hosts pin: {error}")) } +/// Reject a hostname endpoint whose pin would defeat a denied `/etc/hosts`. +/// +/// The pin is spliced after every filesystem-policy mount so it survives them +/// (see [`insert_hosts_bind`]), which means it would also survive a denial -- +/// handing back a readable file, populated from the host's own `/etc/hosts`, +/// that the policy asked to mask. Refusing at policy time is the only honest +/// outcome: dropping the pin instead would leave the proxy name unresolvable +/// and fail at connect time, long after the caller could act on it. +/// +/// Only a denial is refused. A `readonlyPaths` or `readwritePaths` entry is +/// still overridden with a warning: shadowing a mount the caller asked to +/// *read* narrows their access rather than widening it. +pub(crate) fn check_hosts_pin_against_policy( + configured: &ProxyAddress, + policy: &ContainerPolicy, +) -> Result<(), String> { + let (needs_pin, _) = SandboxProxy::inspect_without_resolving(configured); + if !needs_pin || !hosts_file_is_denied(policy) { + return Ok(()); + } + + Err(format!( + "Bubblewrap: the proxy endpoint '{}' is a hostname, which is reached by pinning it in \ + the sandbox's {SANDBOX_HOSTS_PATH}, but the filesystem policy denies that path. The pin \ + is applied after every policy mount, so honouring it would expose the file the policy \ + masks. Remove {SANDBOX_HOSTS_PATH} from deniedPaths, or give the proxy an IP address, \ + which needs no pin.", + configured.host() + )) +} + +/// Whether the filesystem policy masks the sandbox's hosts file. +/// +/// The plan is ordered shallow-to-deep and bwrap applies the last mount at a +/// path, so the deepest entry covering the file is the one that takes effect: +/// an ancestor denial counts, and a more specific grant beneath it wins back. +fn hosts_file_is_denied(policy: &ContainerPolicy) -> bool { + resolve_mount_order(policy) + .iter() + .rfind(|mount| covers_hosts_file(&mount.path)) + .is_some_and(|mount| mount.intent == FsIntent::Denied) +} + +/// Whether `path` is the sandbox hosts file or a directory holding it. +fn covers_hosts_file(path: &str) -> bool { + let path = path.trim_end_matches('/'); + path == SANDBOX_HOSTS_PATH + || SANDBOX_HOSTS_PATH + .strip_prefix(path) + .is_some_and(|rest| rest.starts_with('/')) +} + /// Splice the pinned-hosts bind in just before the command separator. /// /// bwrap applies mounts in argument order and the last mount at a path wins, @@ -817,11 +905,23 @@ fn write_pinned_hosts(path: &PathBuf, pin: &ProxyHostPin) -> Result<(), String> /// pin to survive -- including one that would otherwise expose the host's own /// `/etc/hosts`. Returns `true` when an earlier mount already targeted /// `/etc/hosts`, so the caller can report that the pin overrides it. +/// Index of the separator that ends bwrap's options and begins the command. +/// +/// Scanning for the first `--` would find a caller-controlled value instead: +/// `process.env` of `FOO=--` is emitted as `--setenv FOO --` ahead of the real +/// separator, and splicing there would shift the pin's three arguments into +/// that option's operands. Scanning for the *last* one is no better -- the +/// script is arbitrary text. The command is appended last with a fixed shape, +/// so that trailing shape is what identifies it. +fn command_separator(args: &[String]) -> Result { + args.len() + .checked_sub(COMMAND_TAIL.len() + 1) + .filter(|&separator| args[separator..separator + COMMAND_TAIL.len()] == COMMAND_TAIL[..]) + .ok_or_else(|| "Bubblewrap: argument list has no command separator".to_string()) +} + fn insert_hosts_bind(args: &mut Vec, hosts_path: &str) -> Result { - let separator = args - .iter() - .position(|arg| arg == "--") - .ok_or_else(|| "Bubblewrap: argument list has no command separator".to_string())?; + let separator = command_separator(args)?; let overrides = args[..separator] .iter() .any(|arg| arg == SANDBOX_HOSTS_PATH); @@ -1207,6 +1307,67 @@ mod tests { assert_eq!(resolved.egress().ip.to_string(), SLIRP_HOST_GATEWAY); } + /// The pre-flight check exists to fail fast, not to duplicate work: a name + /// must survive it untouched so `run` performs the only lookup, whose + /// answer is also the pin the sandbox is given. + #[test] + fn the_pre_flight_check_defers_every_verdict_that_needs_a_lookup() { + // `.invalid` is reserved as never-resolvable (RFC 2606), so a real + // lookup here could only fail -- passing proves none was made. + let named = ProxyAddress::new("proxy.this-name-cannot-exist.invalid".into(), 3128); + assert!( + SandboxProxy::check_without_resolving(&named).is_ok(), + "a hostname's verdict belongs to the lookup in `run`" + ); + } + + #[test] + fn the_pre_flight_check_still_rejects_what_no_lookup_could_rescue() { + // Decided by the configured address alone, so deferring these would + // only move the same rejection past a started proxy. + for unusable in [ + ProxyAddress::new("[::1]".into(), 3128), + ProxyAddress::new("2001:db8::1".into(), 3128), + ProxyAddress::new("127.0.0.1".into(), 0), + ] { + assert!( + SandboxProxy::check_without_resolving(&unusable).is_err(), + "endpoint '{}' is unusable regardless of DNS", + unusable.host() + ); + } + } + + /// The effective intent is the deepest entry covering the file, so an + /// ancestor denial masks it and a grant beneath that denial wins it back. + #[test] + fn a_denied_ancestor_counts_as_denying_the_hosts_file() { + let denied_parent = ContainerPolicy { + denied_paths: vec!["/etc".into()], + ..Default::default() + }; + assert!(hosts_file_is_denied(&denied_parent)); + + let regranted = ContainerPolicy { + denied_paths: vec!["/etc".into()], + readonly_paths: vec![SANDBOX_HOSTS_PATH.into()], + ..Default::default() + }; + assert!( + !hosts_file_is_denied(®ranted), + "a more specific grant beneath the denial takes effect" + ); + + let unrelated = ContainerPolicy { + denied_paths: vec!["/etc/hostname".into(), "/etc/hosts.allow".into()], + ..Default::default() + }; + assert!( + !hosts_file_is_denied(&unrelated), + "a sibling with a shared prefix must not be mistaken for the file" + ); + } + #[test] fn pins_a_routable_hostname_to_its_resolved_address() { // Only a loopback answer is redirected to the gateway; a routable one @@ -1219,6 +1380,12 @@ mod tests { sandbox_facing_ip(Ipv4Addr::new(127, 0, 0, 1)), SLIRP_HOST_GATEWAY_IP ); + // `/etc/hosts` can map a name to `0.0.0.0`, which names the host the + // same way loopback does -- and which the IP-literal path rewrites. + assert_eq!( + sandbox_facing_ip(Ipv4Addr::UNSPECIFIED), + SLIRP_HOST_GATEWAY_IP + ); } #[test] @@ -1355,9 +1522,8 @@ mod tests { "--bind".to_string(), "/etc/hosts".to_string(), "/etc/hosts".to_string(), - "--".to_string(), - "sh".to_string(), ]; + args.extend(command_tail()); insert_hosts_bind(&mut args, "/tmp/pin/hosts").unwrap(); let pin = args @@ -1368,7 +1534,7 @@ mod tests { .windows(3) .position(|window| window == ["--bind", "/etc/hosts", "/etc/hosts"]) .expect("policy mount should be retained"); - let separator = args.iter().position(|arg| arg == "--").unwrap(); + let separator = command_separator(&args).unwrap(); assert!(pin > policy, "pin must be applied after the policy mount"); assert!(pin < separator, "pin must precede the command separator"); @@ -1382,11 +1548,12 @@ mod tests { "--bind".to_string(), "/custom/hosts".to_string(), "/etc/hosts".to_string(), - "--".to_string(), ]; + args.extend(command_tail()); assert!(insert_hosts_bind(&mut args, "/tmp/pin/hosts").unwrap()); - let mut untouched = vec!["--ro-bind-try".to_string(), "--".to_string()]; + let mut untouched = vec!["--ro-bind-try".to_string()]; + untouched.extend(command_tail()); assert!(!insert_hosts_bind(&mut untouched, "/tmp/pin/hosts").unwrap()); } @@ -1398,6 +1565,55 @@ mod tests { assert!(insert_hosts_bind(&mut args, "/tmp/pin/hosts").is_err()); } + /// The command bwrap is asked to run, as `build_args` appends it. + fn command_tail() -> Vec { + COMMAND_TAIL + .iter() + .map(|arg| arg.to_string()) + .chain(["echo hello".to_string()]) + .collect() + } + + /// Environment values are caller-controlled and reach the argument vector + /// verbatim, so one that happens to be `--` must not be mistaken for the + /// separator: splicing there would consume the pin's arguments as that + /// `--setenv`'s operands and leave the sandbox reading the host's hosts + /// file. + #[test] + fn a_caller_supplied_separator_value_is_not_mistaken_for_the_command() { + let mut args = vec![ + "--setenv".to_string(), + "FOO".to_string(), + "--".to_string(), + "--ro-bind-try".to_string(), + "/etc".to_string(), + "/etc".to_string(), + ]; + args.extend(command_tail()); + let decoy = 2; + + insert_hosts_bind(&mut args, "/tmp/pin/hosts").unwrap(); + + assert_eq!( + args[..=decoy], + ["--setenv", "FOO", "--"], + "the environment value must survive the splice intact" + ); + let pin = args + .windows(3) + .position(|window| window == ["--ro-bind", "/tmp/pin/hosts", SANDBOX_HOSTS_PATH]) + .expect("pin bind should be present"); + assert!( + pin > decoy, + "the pin must be spliced at the command, not at the decoy value" + ); + assert_eq!( + args[args.len() - COMMAND_TAIL.len() - 1..], + ["--", "sh", "-c", "echo hello"], + "the command must remain last" + ); + } + /// Byte offset of `needle` in the normalised supervisor script. fn script_offset(needle: &str) -> usize { normalised_script() diff --git a/tests/scripts/run_bwrap_network_proxy_test.sh b/tests/scripts/run_bwrap_network_proxy_test.sh index 6c8449506..fdd85f6df 100644 --- a/tests/scripts/run_bwrap_network_proxy_test.sh +++ b/tests/scripts/run_bwrap_network_proxy_test.sh @@ -235,84 +235,216 @@ echo "PASS: proxy-only egress enforcement" # Hostname proxy endpoints. The endpoint is resolved on the host and pinned # into the sandbox's /etc/hosts, because DNS is closed inside the sandbox. -# -# The host's own name is used because it resolves everywhere without editing -# /etc/hosts (which would need root), and it resolves to loopback -- the case -# the pin has to redirect to slirp's gateway rather than carry through. echo "Running Bubblewrap hostname proxy pin test..." PROXY_HOST="$(hostname)" -if ! getent hosts "$PROXY_HOST" >/dev/null; then - echo "SKIP: hostname proxy pin (host name '$PROXY_HOST' does not resolve)" -else +# The host's own name is used because it resolves everywhere without editing +# /etc/hosts (which would need root). Where it points is not fixed, though: a +# workstation maps it to loopback, a cloud runner to a routable address. Both +# are cases the pin must handle, so the proxy is bound to whatever the name +# actually resolves to rather than assuming loopback -- binding 127.0.0.1 while +# the name resolves routably leaves the sandbox dialing a closed port. +PROXY_BIND="$(getent ahostsv4 "$PROXY_HOST" 2>/dev/null | awk 'NR==1 {print $1}')" +if [ -z "$PROXY_BIND" ]; then + echo "FAIL: hostname proxy pin (host name '$PROXY_HOST' does not resolve to an IPv4 address)" + exit 1 +fi +# A loopback answer is redirected to slirp's gateway, and the gateway lands on +# the host's 127.0.0.1 -- not on the exact loopback address the name carries +# (Ubuntu maps the machine name to 127.0.1.1). Binding there is what the +# sandbox can actually reach, and is where a real proxy would listen. +case "$PROXY_BIND" in + 127.*) PROXY_BIND=127.0.0.1 ;; +esac +echo " host name '$PROXY_HOST' resolves to $PROXY_BIND" +# The proxy lives beside lxc-exec, wherever that came from: CI overrides +# LXC_EXEC with a --target build under src/target//release, which the +# repo-relative fallbacks below do not cover. Every other case in this file +# reaches the proxy through the coordinator, which already resolves it that +# way -- this is the one that spawns it directly. +TEST_PROXY="$(dirname "$LXC_EXEC")/unix-test-proxy" +if [ ! -x "$TEST_PROXY" ]; then TEST_PROXY="$REPO_DIR/src/target/release/unix-test-proxy" - if [ ! -x "$TEST_PROXY" ]; then - TEST_PROXY="$REPO_DIR/src/target/debug/unix-test-proxy" - fi - if [ ! -x "$TEST_PROXY" ]; then - echo "FAIL: hostname proxy pin (unix-test-proxy not built)" - exit 1 - fi +fi +if [ ! -x "$TEST_PROXY" ]; then + TEST_PROXY="$REPO_DIR/src/target/debug/unix-test-proxy" +fi +if [ ! -x "$TEST_PROXY" ]; then + echo "FAIL: hostname proxy pin (unix-test-proxy not built)" + exit 1 +fi - PIN_DIR="$(mktemp -d)" - PIN_PROXY_PID="" - cleanup_pin() { - if [ -n "$PIN_PROXY_PID" ]; then - kill "$PIN_PROXY_PID" 2>/dev/null || true - wait "$PIN_PROXY_PID" 2>/dev/null || true - fi - exec 9>&- 2>/dev/null || true - rm -rf "$PIN_DIR" - } - trap cleanup_pin EXIT +PIN_DIR="$(mktemp -d)" +PIN_PROXY_PID="" +cleanup_pin() { + if [ -n "$PIN_PROXY_PID" ]; then + kill "$PIN_PROXY_PID" 2>/dev/null || true + wait "$PIN_PROXY_PID" 2>/dev/null || true + fi + exec 9>&- 2>/dev/null || true + rm -rf "$PIN_DIR" +} +trap cleanup_pin EXIT - # The proxy exits when its stdin reaches EOF, which is how the coordinator - # stops an orphan. Driving it from a script means supplying that pipe here: - # the fifo is opened read-write so the open does not block, and the script - # holding it keeps the proxy alive for the run. - mkfifo "$PIN_DIR/parent.pipe" - exec 9<>"$PIN_DIR/parent.pipe" +# The proxy exits when its stdin reaches EOF, which is how the coordinator +# stops an orphan. Driving it from a script means supplying that pipe here: +# the fifo is opened read-write so the open does not block, and the script +# holding it keeps the proxy alive for the run. +mkfifo "$PIN_DIR/parent.pipe" +exec 9<>"$PIN_DIR/parent.pipe" - # The proxy binds an OS-assigned port and publishes it, so the config is - # generated per run rather than committed with a fixed port. - "$TEST_PROXY" --ready-file "$PIN_DIR/ready.port" --bind-address 127.0.0.1 \ - <"$PIN_DIR/parent.pipe" >"$PIN_DIR/proxy.log" 2>&1 & - PIN_PROXY_PID=$! - for _ in $(seq 1 100); do - [ -s "$PIN_DIR/ready.port" ] && break - if ! kill -0 "$PIN_PROXY_PID" 2>/dev/null; then - cat "$PIN_DIR/proxy.log" - echo "FAIL: hostname proxy pin (test proxy exited before publishing its port)" - exit 1 - fi - sleep 0.1 - done - PROXY_PORT="$(cat "$PIN_DIR/ready.port" 2>/dev/null || true)" - if [ -z "$PROXY_PORT" ]; then +# The proxy binds an OS-assigned port and publishes it, so the config is +# generated per run rather than committed with a fixed port. +"$TEST_PROXY" --ready-file "$PIN_DIR/ready.port" --bind-address "$PROXY_BIND" \ + <"$PIN_DIR/parent.pipe" >"$PIN_DIR/proxy.log" 2>&1 & +PIN_PROXY_PID=$! +for _ in $(seq 1 100); do + [ -s "$PIN_DIR/ready.port" ] && break + if ! kill -0 "$PIN_PROXY_PID" 2>/dev/null; then cat "$PIN_DIR/proxy.log" - echo "FAIL: hostname proxy pin (test proxy did not publish a port)" + echo "FAIL: hostname proxy pin (test proxy exited before publishing its port)" exit 1 fi + sleep 0.1 +done +PROXY_PORT="$(cat "$PIN_DIR/ready.port" 2>/dev/null || true)" +if [ -z "$PROXY_PORT" ]; then + cat "$PIN_DIR/proxy.log" + echo "FAIL: hostname proxy pin (test proxy did not publish a port)" + exit 1 +fi - sed -e "s/{{PROXY_HOST}}/$PROXY_HOST/g" -e "s/{{PROXY_PORT}}/$PROXY_PORT/g" \ - "$REPO_DIR/tests/configs/bubblewrap_network_proxy_hostname.json" \ - >"$PIN_DIR/hostname.json" +sed -e "s/{{PROXY_HOST}}/$PROXY_HOST/g" -e "s/{{PROXY_PORT}}/$PROXY_PORT/g" \ + "$REPO_DIR/tests/configs/bubblewrap_network_proxy_hostname.json" \ + >"$PIN_DIR/hostname.json" - if ! PIN_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ - "$PIN_DIR/hostname.json" 2>&1); then +if ! PIN_OUT=$("$LXC_EXEC" --experimental --allow-testing-features \ + "$PIN_DIR/hostname.json" 2>&1); then + echo "$PIN_OUT" + echo "FAIL: hostname proxy pin (lxc-exec returned non-zero)" + exit 1 +fi +for sentinel in PIN_PRESENT_OK PIN_PROXY_OK PIN_DIRECT_BLOCKED_OK; do + if ! grep -q "$sentinel" <<<"$PIN_OUT"; then echo "$PIN_OUT" - echo "FAIL: hostname proxy pin (lxc-exec returned non-zero)" + echo "FAIL: hostname proxy pin (sentinel '$sentinel' not found in output)" exit 1 fi - for sentinel in PIN_PRESENT_OK PIN_PROXY_OK PIN_DIRECT_BLOCKED_OK; do - if ! grep -q "$sentinel" <<<"$PIN_OUT"; then - echo "$PIN_OUT" - echo "FAIL: hostname proxy pin (sentinel '$sentinel' not found in output)" - exit 1 - fi - done - cleanup_pin - trap - EXIT - echo "PASS: hostname proxy pin" +done +cleanup_pin +trap - EXIT +echo "PASS: hostname proxy pin" + +# The pin outranks every filesystem-policy mount, so a policy that denies +# /etc/hosts has to be refused rather than silently handed the file back. +echo "Running Bubblewrap hostname pin vs denied /etc/hosts test..." +DENY_DIR="$(mktemp -d)" +trap 'rm -rf "$DENY_DIR"' EXIT +cat >"$DENY_DIR/denied.json" <&1) && DENY_RC=0 || DENY_RC=$? +if [ "$DENY_RC" = 0 ]; then + echo "$DENY_OUT" + echo "FAIL: hostname pin vs denied /etc/hosts (accepted a policy it cannot honour)" + exit 1 +fi +if grep -q PIN_DENIED_HOSTS_RAN <<<"$DENY_OUT"; then + echo "$DENY_OUT" + echo "FAIL: hostname pin vs denied /etc/hosts (workload ran despite the rejection)" + exit 1 +fi +if ! grep -qF "deniedPaths" <<<"$DENY_OUT"; then + echo "$DENY_OUT" + echo "FAIL: hostname pin vs denied /etc/hosts (rejection did not name the conflict)" + exit 1 +fi +rm -rf "$DENY_DIR" +trap - EXIT +echo "PASS: hostname pin vs denied /etc/hosts" + +# The same denial spelled with `..` normalizes to /etc/hosts before the masks +# are built, so it must be refused too -- checking only the written form let it +# through and the pin handed the file back. +echo "Running Bubblewrap hostname pin vs dot-dot denied /etc/hosts test..." +DOTDOT_DIR="$(mktemp -d)" +trap 'rm -rf "$DOTDOT_DIR"' EXIT +cat >"$DOTDOT_DIR/dotdot.json" <&1) && DOTDOT_RC=0 || DOTDOT_RC=$? +if [ "$DOTDOT_RC" = 0 ]; then + echo "$DOTDOT_OUT" + echo "FAIL: dot-dot denied /etc/hosts (accepted a policy it cannot honour)" + exit 1 +fi +if grep -q "localhost" <<<"$DOTDOT_OUT"; then + echo "$DOTDOT_OUT" + echo "FAIL: dot-dot denied /etc/hosts (the pin exposed the masked file)" + exit 1 +fi +if ! grep -qF "deniedPaths" <<<"$DOTDOT_OUT"; then + echo "$DOTDOT_OUT" + echo "FAIL: dot-dot denied /etc/hosts (rejection did not name the conflict)" + exit 1 +fi +rm -rf "$DOTDOT_DIR" +trap - EXIT +echo "PASS: hostname pin vs dot-dot denied /etc/hosts" + +# The same policy on the legacy schema pins nothing, so it must still run -- +# 0.6/0.7 behaviour is unchanged by the rejection above. +echo "Running Bubblewrap legacy (schema 0.7) denied /etc/hosts compatibility test..." +LEGACY_DIR="$(mktemp -d)" +trap 'rm -rf "$LEGACY_DIR"' EXIT +cat >"$LEGACY_DIR/legacy.json" <&1) || true +if grep -qF "deniedPaths" <<<"$LEGACY_OUT"; then + echo "$LEGACY_OUT" + echo "FAIL: legacy denied /etc/hosts (0.7 inherited the 0.8 rejection)" + exit 1 +fi +if ! grep -q LEGACY_DENIED_HOSTS_OK <<<"$LEGACY_OUT"; then + echo "$LEGACY_OUT" + echo "FAIL: legacy denied /etc/hosts (workload did not run)" + exit 1 fi +rm -rf "$LEGACY_DIR" +trap - EXIT +echo "PASS: legacy (schema 0.7) denied /etc/hosts compatibility" echo "Bubblewrap network proxy tests complete." From c3a2244aa41847183c94d5de25bd345c8fc5191e Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 20 Aug 2026 01:01:54 -0700 Subject: [PATCH 4/6] Addressed pr comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- .../bubblewrap/common/src/proxy_network.rs | 182 ++++++++++++++++-- 1 file changed, 168 insertions(+), 14 deletions(-) diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index 41f6f6c9d..8e470d86d 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -816,18 +816,22 @@ fn run_probe(mut command: Command, label: &str) -> Result { Ok(ProbeOutput { status, stdout }) } -/// Write the sandbox's `/etc/hosts`: the pin, then the host's own entries. +/// Write the sandbox's `/etc/hosts`: the pin, then the host's own entries with +/// every competing mapping for the pinned name removed. /// -/// The pin goes first because the first match wins in a hosts file. A host -/// whose `/etc/hosts` already maps the proxy name -- to a stale address, or to -/// one the egress chain never authorized -- would otherwise shadow the pin and -/// leave the sandbox unable to reach its proxy. +/// Ordering alone is *not* enough, which is why the name is stripped rather +/// than merely outranked. glibc's `files` backend collects **every** line that +/// matches a name and hands the whole set to `getaddrinfo`, which then re-sorts +/// it by RFC 6724 destination-address rules. Those rules promote a loopback +/// address above a global one, so a leftover `127.0.0.1 ` entry is +/// returned *ahead* of a pin written on line 1. A client walking the result in +/// order then dials an address the egress chain never authorized -- or, worse, +/// dials back into the sandbox's own loopback. /// -/// The host's entries are kept rather than discarded so the sandbox retains -/// the mappings a workload expects (`localhost` above all). They are read -/// before the file is created, so a read failure cannot leave a half-written -/// pin behind. -fn write_pinned_hosts(path: &PathBuf, pin: &ProxyHostPin) -> Result<(), String> { +/// The host's other entries are kept so the sandbox retains the mappings a +/// workload expects (`localhost` above all). They are read before the file is +/// created, so a read failure cannot leave a half-written pin behind. +fn write_pinned_hosts(path: &Path, pin: &ProxyHostPin) -> Result<(), String> { // The host file is optional: a host without one simply contributes no // entries, which is not a reason to refuse to pin. let existing = match fs::read_to_string(SANDBOX_HOSTS_PATH) { @@ -841,11 +845,82 @@ fn write_pinned_hosts(path: &PathBuf, pin: &ProxyHostPin) -> Result<(), String> } }; - let contents = format!("{}\n{}", pin.hosts_line(), existing); + let contents = format!( + "{}\n{}", + pin.hosts_line(), + strip_host_from_hosts(&existing, pin.hostname()) + ); fs::write(path, contents) .map_err(|error| format!("Bubblewrap: failed to write the proxy hosts pin: {error}")) } +/// Remove `hostname` from every entry in a hosts file, dropping a line that has +/// no names left. +/// +/// Only the name is removed, never the whole line: an entry like +/// `127.0.0.1 localhost ` still has to keep resolving `localhost`. +/// Comments and blank lines pass through untouched so the sandbox's file stays +/// recognizable, and a trailing comment on a mapping line is preserved. +/// +/// Matching is ASCII-case-insensitive because DNS names are, so a host file +/// spelling the proxy name in a different case would otherwise survive and +/// reintroduce exactly the competing mapping this removes. +fn strip_host_from_hosts(contents: &str, hostname: &str) -> String { + let mut out = String::with_capacity(contents.len()); + for line in contents.lines() { + let (body, comment) = match line.split_once('#') { + Some((body, comment)) => (body, Some(comment)), + None => (line, None), + }; + + let mut fields = body.split_whitespace(); + let Some(address) = fields.next() else { + out.push_str(line); + out.push('\n'); + continue; + }; + + // Lines that never mention the pinned name are passed through byte for + // byte. Rewriting them would normalize the operator's tabs and column + // alignment for no benefit. + let names: Vec<&str> = fields.collect(); + if !names.iter().any(|name| name.eq_ignore_ascii_case(hostname)) { + out.push_str(line); + out.push('\n'); + continue; + } + + let kept: Vec<&str> = names + .into_iter() + .filter(|name| !name.eq_ignore_ascii_case(hostname)) + .collect(); + + // Every name on the line was the pinned one, so the mapping is gone. + // Keep a trailing comment rather than silently deleting the operator's + // text along with the entry. + if kept.is_empty() { + if let Some(comment) = comment { + out.push('#'); + out.push_str(comment); + out.push('\n'); + } + continue; + } + + out.push_str(address); + for name in kept { + out.push(' '); + out.push_str(name); + } + if let Some(comment) = comment { + out.push_str(" #"); + out.push_str(comment); + } + out.push('\n'); + } + out +} + /// Reject a hostname endpoint whose pin would defeat a denied `/etc/hosts`. /// /// The pin is spliced after every filesystem-policy mount so it survives them @@ -1479,8 +1554,8 @@ mod tests { #[test] fn pinned_hosts_file_puts_the_pin_first() { - // First match wins in a hosts file, so a host entry for the same name - // must not be able to shadow the pin. + // Ordering is not sufficient on its own (see the duplicate-name tests + // below), but the pin still leads the file so the intent is legible. let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("hosts"); write_pinned_hosts(&path, &test_pin()).unwrap(); @@ -1492,6 +1567,74 @@ mod tests { ); } + /// The reason the name is stripped rather than merely outranked: glibc + /// returns *every* matching hosts line to `getaddrinfo`, which re-sorts + /// them by RFC 6724 and promotes a loopback address above the pin. A + /// surviving duplicate is therefore tried first, sending the workload to an + /// address the egress chain never authorized -- or back into the sandbox. + #[test] + fn a_competing_mapping_for_the_pinned_name_is_removed() { + let stripped = strip_host_from_hosts( + "127.0.0.1 localhost\n127.0.0.1 proxy.example\n10.9.9.9 proxy.example\n", + "proxy.example", + ); + assert_eq!(stripped, "127.0.0.1 localhost\n"); + } + + #[test] + fn stripping_the_pinned_name_keeps_the_other_names_on_its_line() { + // Dropping the whole line would cost the sandbox `localhost`. + let stripped = + strip_host_from_hosts("127.0.0.1 localhost proxy.example\n", "proxy.example"); + assert_eq!(stripped, "127.0.0.1 localhost\n"); + } + + #[test] + fn stripping_the_pinned_name_ignores_case() { + // DNS names are case-insensitive, so a differently cased duplicate + // would otherwise survive and reintroduce the competing mapping. + let stripped = strip_host_from_hosts("127.0.0.1 Proxy.EXAMPLE\n", "proxy.example"); + assert_eq!(stripped, ""); + } + + #[test] + fn stripping_leaves_unrelated_lines_byte_for_byte() { + // Real hosts files are tab-aligned; normalizing them would churn the + // sandbox's file for no benefit. + let original = "127.0.0.1\tlocalhost\n\n# a comment\n::1\tip6-localhost ip6-loopback\n"; + assert_eq!(strip_host_from_hosts(original, "proxy.example"), original); + } + + #[test] + fn stripping_an_entire_entry_keeps_its_trailing_comment() { + let stripped = + strip_host_from_hosts("10.9.9.9 proxy.example # operator note\n", "proxy.example"); + assert_eq!(stripped, "# operator note\n"); + } + + #[test] + fn the_written_hosts_file_has_exactly_one_mapping_for_the_pinned_name() { + // End-to-end twin of the unit tests above, against the real host file. + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hosts"); + write_pinned_hosts(&path, &test_pin()).unwrap(); + + let written = fs::read_to_string(&path).unwrap(); + let matches = written + .lines() + .filter(|line| { + let body = line.split('#').next().unwrap_or(""); + body.split_whitespace() + .skip(1) + .any(|name| name.eq_ignore_ascii_case("proxy.example")) + }) + .count(); + assert_eq!( + matches, 1, + "exactly one mapping may survive for the pinned name:\n{written}" + ); + } + #[test] fn pinned_hosts_file_keeps_the_host_entries() { // Dropping them would cost the sandbox `localhost`, which workloads @@ -1502,7 +1645,18 @@ mod tests { let written = fs::read_to_string(&path).unwrap(); let host_file = fs::read_to_string(SANDBOX_HOSTS_PATH).unwrap_or_default(); - for line in host_file.lines().filter(|line| !line.trim().is_empty()) { + for line in host_file.lines().filter(|line| { + // A line mapping the pinned name is *expected* to be rewritten; + // every other line must survive untouched. + !line.trim().is_empty() + && !line + .split('#') + .next() + .unwrap_or("") + .split_whitespace() + .skip(1) + .any(|name| name.eq_ignore_ascii_case("proxy.example")) + }) { assert!( written.contains(line), "host entry should be preserved: {line}" From 2e595996a0d03b9fec20ea8983e5b2c70bbc4d96 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 20 Aug 2026 13:44:55 -0700 Subject: [PATCH 5/6] Reject proxy addresses in the slirp reserved range so they cannot reach host loopback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- .../bubblewrap/common/src/proxy_network.rs | 97 ++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index 8e470d86d..f85d0a5e6 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -59,6 +59,8 @@ const SLIRP_HOST_GATEWAY: &str = "10.0.2.2"; /// The gateway as an address, for rules and pins. Kept in step with /// [`SLIRP_HOST_GATEWAY`] by [`tests::gateway_constants_agree`]. const SLIRP_HOST_GATEWAY_IP: Ipv4Addr = Ipv4Addr::new(10, 0, 2, 2); +/// The network slirp gives the sandbox, for diagnostics. +const SLIRP_NETWORK: &str = "10.0.2.0/24"; /// Path the hosts-file pin is mounted over inside the sandbox. const SANDBOX_HOSTS_PATH: &str = "/etc/hosts"; /// Egress chain installed inside the sandbox's own network namespace. @@ -254,6 +256,7 @@ impl SandboxProxy { return if ip.is_loopback() || ip.is_unspecified() { Ok(Self::rewritten(configured)?) } else { + reject_slirp_reserved(ip, host)?; Ok(Self { address: configured.clone(), egress: ProxyEgress { @@ -289,7 +292,9 @@ impl SandboxProxy { return Self::rewritten(configured); } - let ip = sandbox_facing_ip(resolve(host, configured.port())?); + let resolved = resolve(host, configured.port())?; + reject_slirp_reserved(resolved, host)?; + let ip = sandbox_facing_ip(resolved); let pin = configured .host_pin(IpAddr::V4(ip)) .map_err(|error| format!("Bubblewrap: {error}"))?; @@ -342,6 +347,32 @@ fn sandbox_facing_ip(resolved: Ipv4Addr) -> Ipv4Addr { } } +/// Reject an endpoint the host resolved into the sandbox's own slirp network. +/// +/// Every address in [`SLIRP_NETWORK`] is on-link inside the namespace, so it +/// does not name the machine the host meant: [`SLIRP_HOST_GATEWAY`] is the +/// route to host loopback, `.100` is the sandbox itself, and the rest have no +/// neighbour at all. Opening the gateway is the sharp case -- the egress rule +/// would grant the workload an unrelated host-loopback service on the proxy +/// port. Applied to what the host answered, so the deliberate loopback and +/// wildcard translations in [`sandbox_facing_ip`] still reach the gateway. +fn reject_slirp_reserved(resolved: Ipv4Addr, host: &str) -> Result<(), String> { + if !SLIRP_NETWORK_OCTETS.eq(&resolved.octets()[..3]) { + return Ok(()); + } + Err(format!( + "Bubblewrap: proxy endpoint '{host}' is {resolved}, which is inside the sandbox's own \ + network ({SLIRP_NETWORK}); there that address is slirp's own, not the host's \ + {resolved}, so the egress rule would open an unrelated service. Bind the proxy to \ + 127.0.0.1 -- which is translated to the gateway deliberately -- or give it an address \ + outside {SLIRP_NETWORK}." + )) +} + +/// Leading octets of [`SLIRP_NETWORK`], asserted against it by +/// [`tests::gateway_constants_agree`]. +const SLIRP_NETWORK_OCTETS: [u8; 3] = [10, 0, 2]; + /// Resolve `host` to the address the egress chain will open. /// /// Only the first IPv4 answer is used, and it is the same one the sandbox is @@ -1299,6 +1330,70 @@ mod tests { // from the string. A drift between them would open one endpoint and // point the workload at another. assert_eq!(SLIRP_HOST_GATEWAY_IP.to_string(), SLIRP_HOST_GATEWAY); + // The reserved-range check is written from the octets, the diagnostic + // from the string; the gateway must fall inside the range it names. + assert!(SLIRP_NETWORK.starts_with(&format!( + "{}.{}.{}.", + SLIRP_NETWORK_OCTETS[0], SLIRP_NETWORK_OCTETS[1], SLIRP_NETWORK_OCTETS[2] + ))); + assert_eq!(SLIRP_HOST_GATEWAY_IP.octets()[..3], SLIRP_NETWORK_OCTETS); + } + + /// A hostname answering with the gateway is the sharp case: the address is + /// routable on the host, so nothing else flags it, but inside the namespace + /// it is the route to host loopback. Pinning it would hand the workload an + /// unrelated host service on the proxy port. + #[test] + fn egress_rejects_a_hostname_resolving_into_the_sandbox_network() { + let address = ProxyAddress::new("proxy.example".into(), 3128); + let error = SandboxProxy::resolve_with(&address, resolver([10, 0, 2, 2])) + .expect_err("the slirp gateway must not be pinned as if it were a host address"); + + assert!( + error.contains("10.0.2.0/24") && error.contains("127.0.0.1"), + "error should name the reserved range and the supported alternative: {error}" + ); + } + + /// The sandbox's own tap address, and a plain neighbour, are wrong for the + /// same reason -- so the whole range is rejected, not just the gateway. + #[test] + fn egress_rejects_every_address_in_the_sandbox_network() { + for octets in [[10, 0, 2, 3], [10, 0, 2, 100], [10, 0, 2, 50]] { + let address = ProxyAddress::new("proxy.example".into(), 3128); + assert!( + SandboxProxy::resolve_with(&address, resolver(octets)).is_err(), + "{octets:?} is on-link inside the namespace and cannot name a host endpoint" + ); + + let literal = ProxyAddress::new(Ipv4Addr::from(octets).to_string(), 3128); + assert!( + SandboxProxy::resolve(&literal).is_err(), + "{octets:?} must be rejected as a literal too, not only as an answer" + ); + } + } + + /// The rejection must not swallow the translation it sits next to: a + /// loopback answer is *meant* to become the gateway. + #[test] + fn rejecting_the_sandbox_network_leaves_the_loopback_translation_intact() { + let address = ProxyAddress::new("proxy.example".into(), 3128); + let resolved = SandboxProxy::resolve_with(&address, resolver([127, 0, 0, 1])) + .expect("a loopback answer is translated, not rejected"); + + assert_eq!(resolved.egress().ip, SLIRP_HOST_GATEWAY_IP); + } + + /// The neighbouring /24 is ordinary host space, so the check must not + /// widen past the network slirp actually assigns. + #[test] + fn egress_accepts_an_address_just_outside_the_sandbox_network() { + let address = ProxyAddress::new("proxy.example".into(), 3128); + let resolved = SandboxProxy::resolve_with(&address, resolver([10, 0, 3, 2])) + .expect("10.0.3.2 is a normal routable answer"); + + assert_eq!(resolved.egress().ip, Ipv4Addr::new(10, 0, 3, 2)); } #[test] From 6cb4bfcc2ac3a430d800e13758a597a52017ca1c Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 20 Aug 2026 14:52:40 -0700 Subject: [PATCH 6/6] Bound the proxy host lookup so an unresponsive resolver cannot stall startup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3 --- .../bubblewrap/common/src/proxy_network.rs | 109 +++++++++++++++++- 1 file changed, 106 insertions(+), 3 deletions(-) diff --git a/src/backends/bubblewrap/common/src/proxy_network.rs b/src/backends/bubblewrap/common/src/proxy_network.rs index f85d0a5e6..be82ad290 100644 --- a/src/backends/bubblewrap/common/src/proxy_network.rs +++ b/src/backends/bubblewrap/common/src/proxy_network.rs @@ -11,7 +11,7 @@ use std::os::fd::{AsRawFd, OwnedFd, RawFd}; use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; use std::process::{Child, Command, ExitStatus, Stdio}; -use std::sync::OnceLock; +use std::sync::{mpsc, OnceLock}; use std::thread; use std::time::{Duration, Instant}; @@ -373,12 +373,77 @@ fn reject_slirp_reserved(resolved: Ipv4Addr, host: &str) -> Result<(), String> { /// [`tests::gateway_constants_agree`]. const SLIRP_NETWORK_OCTETS: [u8; 3] = [10, 0, 2]; -/// Resolve `host` to the address the egress chain will open. +/// Bound on the host lookup performed by [`resolve_ipv4`]. +/// +/// Sized so a resolver whose first nameserver is dead still answers: glibc +/// defaults to a 5s timeout with 2 attempts per server, so a single failed +/// server costs ~10s before the next is tried. A black-holed resolver would +/// otherwise run to ~30s or more across three servers. +const RESOLVE_TIMEOUT: Duration = Duration::from_secs(15); + +/// Thread name for the lookup [`resolve_ipv4`] bounds. +const RESOLVE_TIMEOUT_LABEL: &str = "mxc-proxy-resolve"; + +/// Run `work` on its own thread and wait at most `timeout` for its answer. +/// +/// Used for calls that cannot be cancelled: on expiry the thread is abandoned +/// and its result discarded, which bounds the caller's wait even though the +/// work itself runs to completion. `label` names the thread for diagnostics. +fn with_deadline( + label: &str, + timeout: Duration, + work: impl FnOnce() -> T + Send + 'static, +) -> Result, std::io::Error> { + let (sender, receiver) = mpsc::channel(); + thread::Builder::new() + .name(label.to_string()) + .spawn(move || { + // Abandoned on expiry: the receiver is gone, so this send fails + // and the answer is discarded rather than blocking the thread. + let _ = sender.send(work()); + })?; + + Ok(receiver.recv_timeout(timeout)) +} + +/// Resolve `host` to the address the egress chain will open, without waiting +/// on the resolver indefinitely. +/// +/// This runs during setup, before the sandbox starts, so it is outside the +/// script timeout: an unbounded lookup would stall a run that never began. +/// `getaddrinfo` cannot be cancelled, so the lookup is moved to a thread and +/// abandoned once the bound expires -- the wait is bounded even though the +/// query itself keeps running until the resolver gives up. +fn resolve_ipv4(host: &str, port: u16) -> Result { + let query = host.to_string(); + let waited = with_deadline(RESOLVE_TIMEOUT_LABEL, RESOLVE_TIMEOUT, move || { + lookup_ipv4(&query, port) + }) + .map_err(|error| { + format!("Bubblewrap: could not start the lookup of proxy host '{host}': {error}") + })?; + + match waited { + Ok(resolved) => resolved, + Err(mpsc::RecvTimeoutError::Timeout) => Err(format!( + "Bubblewrap: resolving proxy host '{host}' exceeded {}s. The proxy endpoint is \ + resolved on the host because DNS is closed inside the sandbox, so an unresponsive \ + host resolver blocks the sandbox from starting. Check the host's DNS configuration, \ + or give the proxy as an IPv4 address to skip resolution.", + RESOLVE_TIMEOUT.as_secs() + )), + Err(mpsc::RecvTimeoutError::Disconnected) => Err(format!( + "Bubblewrap: the lookup of proxy host '{host}' ended without an answer." + )), + } +} + +/// The blocking lookup behind [`resolve_ipv4`]. /// /// Only the first IPv4 answer is used, and it is the same one the sandbox is /// pinned to. Opening the rest would widen the chain to addresses the sandbox /// can no longer select: with DNS closed, the pin is its only resolution path. -fn resolve_ipv4(host: &str, port: u16) -> Result { +fn lookup_ipv4(host: &str, port: u16) -> Result { let resolved: Vec = (host, port) .to_socket_addrs() .map_err(|error| { @@ -1324,6 +1389,44 @@ fn terminate_child(child: &mut Child) { mod tests { use super::*; + /// The reported concern: a hostname proxy resolves through the host's + /// resolver during setup, before the script timeout applies, so an + /// unresponsive resolver would stall a run that never started. + #[test] + fn work_that_outlives_its_deadline_is_abandoned() { + let started = Instant::now(); + let waited = with_deadline("mxc-test-deadline", Duration::from_millis(50), || { + thread::sleep(Duration::from_secs(30)); + "an answer that arrives too late" + }) + .expect("the worker thread starts"); + + assert!( + matches!(waited, Err(mpsc::RecvTimeoutError::Timeout)), + "the wait must end on the deadline rather than on the work" + ); + assert!( + started.elapsed() < Duration::from_secs(5), + "the caller waited {:?}, so the deadline did not bound it", + started.elapsed() + ); + } + + #[test] + fn work_that_finishes_within_its_deadline_returns_its_answer() { + let waited = with_deadline("mxc-test-deadline", Duration::from_secs(30), || 7) + .expect("the worker thread starts"); + + assert_eq!(waited, Ok(7)); + } + + /// The bound is only useful if it is longer than a working resolver's + /// slow path; see [`RESOLVE_TIMEOUT`]. + #[test] + fn the_resolver_bound_survives_one_dead_nameserver() { + assert!(RESOLVE_TIMEOUT > Duration::from_secs(10)); + } + #[test] fn gateway_constants_agree() { // The rule and the pin are written from the address; the URL rewrite