Skip to content

[Bubblewrap] Enforce proxy-only egress for private networking - #931

Merged
Soham Das (SohamDas2021) merged 3 commits into
mainfrom
user/sodas/bwrap-proxy-egress-enforcement
Aug 20, 2026
Merged

Soham Das (SohamDas2021) merged 3 commits into
mainfrom
user/sodas/bwrap-proxy-egress-enforcement

Conversation

@SohamDas2021

@SohamDas2021 Soham Das (SohamDas2021) commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

📖 Description

Stacked on #930 (user/sodas/bwrap-network-model2).

The base PR moves schema-0.8+ Bubblewrap proxy sandboxes into a private network namespace, but slirp4netns routes everything out of it — so the proxy is still only cooperative. A workload that ignores HTTP_PROXY/HTTPS_PROXY, or opens a raw socket, reaches the network directly. This PR makes the boundary real: inside the namespace, the proxy endpoint is the only reachable destination. Everything else is dropped, IPv4 and IPv6.

How it works

The supervisor (already introduced in the base PR) gains firewall responsibility:

  1. slirp4netns moves from exec to a backgrounded child with its own internal readiness fd, so the supervisor regains control once routing is up.
  2. The supervisor installs an MXC_EGRESS chain via nsenter --net=/proc/<pid>/ns/net, hooked from OUTPUT: accept lo, accept tcp -d <proxy-ip> --dport <proxy-port>, DROP everything else. The IPv6 chain accepts lo and drops the rest.
  3. Only after every rule is installed does it write the external ready signal that releases the workload. The ordering is the security property — the sandbox can never observe an unenforced network. set -e means a failed rule kills the supervisor rather than proceeding.

bwrap joins the supervisor's user namespace and drops capabilities, so the workload sits inside the namespace that owns its own firewall but cannot modify it. That assumption is load-bearing, so it is now both documented and asserted by a test (below).

DNS is deliberately closed

No :53 accept rule is installed. An unscoped port-53 allowance is a standing DNS-tunnel exfiltration path, and this mirrors the policy already shipped for LXC model 2. Name resolution happens through the proxy.

Limitations

  • Proxy endpoints must be a literal IPv4 address + port. A firewall rule needs an address, and resolving a hostname at rule-install time is a TOCTOU hole. Hostname and routable IPv6 endpoints are rejected at validation with an actionable error; localhost / 127.0.0.0/8 / 0.0.0.0 / :: are rewritten to slirp's gateway, while ::1 is rejected (an IPv6-loopback listener cannot accept the gateway's IPv4 connection). This is a narrowing of what the base PR accepts — hostname support requires porting LXC's ProxyHostPin (already present in
    wxc_common::models) and is the immediate follow-up.
  • New host prerequisites for this mode: nsenter, iptables, ip6tables, added to the existing validate-time probe. None require root. iptables/ip6tables must also resolve to the nf_tables backend unless /run/xtables.lock is writable — the legacy backend opens that lock before touching any table, which an unprivileged same-uid supervisor cannot do on a stock host. validate refuses such a host rather than letting the supervisor die at the first rule.
  • The 9 rule installations are serialized nsenter + iptables spawns, each passing -w, under a dedicated rule-installation budget rather than the 5s startup timeout. Batching them via iptables-restore was considered and dropped: -w plus the separate budget closes the correctness half, leaving a latency refactor that would trade per-rule failure attribution for one opaque batch failure.

Follow-ups filed: #935 (slirp liveness during the run), #936 (AF_VSOCK and other non-IP families), #937 (network.proxy enforced on LXC/bwrap-0.8 but cooperative on Seatbelt/WSLc).

🔍 Validation

Automated:

  • cargo test -p bwrap_common — 112 pass, incl. 37 new tests covering endpoint validation (translation, ::1, hostname/IPv6, port-0, legacy-schema untouched, builtinTestServer exemption), iptables backend selection, and supervisor behaviour. The supervisor tests execute the script under a real sh with stubbed nsenter/slirp4netns — rule ordering, readiness-written-last, every rule position failing ⇒ no readiness, slirp dying early, and the exact ACCEPT set (which fails if a :53 rule is ever added).
  • cargo clippy -p bwrap_common --all-targets -- -D warnings, cargo fmt --check — clean.
  • node scripts/versioning/validate-configs.js — 233 configs.

Manual E2E on Ubuntu 24.04 (WSL2), unprivileged, no sudo:

  • tests/scripts/run_bwrap_network_proxy_test.sh — 8/8 pass. The bubblewrap_network_proxy_egress_denied.json case asserts seven properties in one run: the proxy is reachable over a raw socket first (a positive control, so a blocked direct connection is attributable to the DROP rule rather than a dead network), direct egress blocked, loopback still exempt, CAP_NET_ADMIN dropped, an in-sandbox flush refused with a non-zero status, tampering ineffective, and the proxy still reachable at the end (so an offline host fails rather than passing vacuously).
  • No leaked slirp4netns / bwrap processes after teardown.

Every new invariant was checked non-vacuous by injecting the matching regression (|| true on a rule, a swallowed rule failure, readiness moved ahead of the rules, the backend check stubbed to pass) and confirming the test fails. The first two are caught only by the executing supervisor tests — the text-based ones stayed green against an unenforced sandbox.

✅ Checklist

  • Signed the Contributor License Agreement
  • Linked to an issue
  • Updated documentation (if applicable)
  • Updated Copilot instructions (if build, architecture, or conventions changed)
  • If this PR changes Cargo.lock, the dependency-feed-check check passes

📋 Issue Type

  • Bug fix
  • Feature
  • Task

GitHub Actions runs the PR validation build automatically. The ADO pipeline
(MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHub
Actions build; it runs on merge to main, and Microsoft reviewers with write access can trigger it
on a PR with /azp run. See docs/pull-requests.md.

If the dependency-feed-check check fails on a new dependency, the crate must be added to
the feed before the PR can pass. See docs/pull-requests.md
for the steps.

@SohamDas2021
Soham Das (SohamDas2021) requested a review from a team as a code owner August 18, 2026 06:44
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enforces proxy-only Bubblewrap egress by installing namespace-local IPv4/IPv6 firewall rules before releasing workloads.

Changes:

  • Adds fail-closed MXC_EGRESS firewall setup and dependency checks.
  • Validates proxy endpoints and updates network-mode handling.
  • Adds automated/E2E coverage and documentation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/backends/bubblewrap/common/src/proxy_network.rs Implements namespace firewall enforcement.
src/backends/bubblewrap/common/src/bwrap_runner.rs Resolves and applies proxy egress endpoints.
src/backends/bubblewrap/common/src/bwrap_command.rs Clarifies host versus namespace firewall modes.
tests/configs/bubblewrap_network_proxy_egress_denied.json Adds egress and tampering checks.
tests/scripts/run_bwrap_network_proxy_test.sh Runs the new enforcement test.
docs/bwrap-support/bubblewrap-backend.md Documents requirements and enforcement behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/backends/bubblewrap/common/src/bwrap_runner.rs
Comment thread tests/configs/bubblewrap_network_proxy_egress_denied.json Outdated
Comment thread docs/bwrap-support/bubblewrap-backend.md
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-proxy-egress-enforcement branch from ccfc317 to 2fd1dd7 Compare August 18, 2026 18:54
Copilot AI review requested due to automatic review settings August 18, 2026 18:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/backends/bubblewrap/common/src/bwrap_runner.rs:257

  • This endpoint check is only reached during spawn_bwrap. ScriptRunner::run returns success immediately after validate_runner for a dry run, while this backend's validate only probes the dependencies, so --dry-run incorrectly accepts hostname and routable IPv6 proxy endpoints that execution rejects. Perform the endpoint validation in validate as well (with the existing loopback translation and builtin-proxy handling).
                .ok_or_else(|| {
                    "Bubblewrap: proxy-only networking requires a resolved proxy address"
                        .to_string()
                })
                .and_then(proxy_network::ProxyEgress::from_address)

src/backends/bubblewrap/common/src/bwrap_command.rs:102

  • The proxy-env section later in this same builder (lines 364–369) still says clients using raw sockets are “NOT enforced.” That now contradicts this mode's contract for schema 0.8 and can mislead future maintenance. Update that comment to distinguish cooperative routing from the new enforced egress boundary, while retaining the legacy-version qualification.
    /// Cooperative proxy routing inside a slirp-backed private namespace, with
    /// egress closed to everything but the proxy.

docs/bwrap-support/bubblewrap-backend.md:395

  • This change knowingly leaves the user-facing parser diagnostic false: config_parser.rs:1168-1182 still says Bubblewrap iptables requires privilege and calls this a cooperative-only model, which this PR disproves. Update that diagnostic to state the actual reason firewall/both remains incompatible, rather than documenting an incorrect error message.
  or `"both"` is rejected at config-parse time. (The rejection message cites a
  root requirement that proxy mode has since disproved; see the firewall
  section.)

src/backends/bubblewrap/common/src/proxy_network.rs:59

  • The fixed descriptor 9 can collide with any of the dynamically allocated descriptors passed to the supervisor. For example, if ready.as_raw_fd() is 9, this redirection replaces the external readiness file with slirp.internal; the final readiness write then goes to the wrong file and startup always times out. Allocate the internal readiness file/FD in Rust and pass its distinct descriptor to the script instead of hard-coding an FD number.
exec 9> "$state_dir/slirp.internal"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

src/backends/bubblewrap/common/src/proxy_network.rs:509

  • This translates every 127.0.0.0/8 address, but slirp's 10.0.2.2 gateway maps specifically to host 127.0.0.1. A configured proxy bound only to 127.0.0.2 therefore passes validation, is advertised as supported in the updated docs, and is rewritten to an endpoint where nothing listens. Reject non-127.0.0.1 loopback literals (or add forwarding that preserves them) instead of treating the whole loopback range as reachable through the gateway.
        || parsed.is_some_and(|ip| ip.is_loopback() || ip.is_unspecified());

src/backends/bubblewrap/common/src/proxy_network.rs:39

  • The deadline is exactly the sum of the nine maximum -w waits, but attach starts it before slirp readiness and each nsenter/iptables process also consumes execution and spawn time. Thus the fully contended case this budget claims to support necessarily exceeds the deadline and rejects a viable sandbox. Include slirp startup and command overhead (or enforce one explicit end-to-end deadline inside the supervisor) rather than sizing to the bare lock waits.
const RULE_INSTALL_TIMEOUT: Duration =
    Duration::from_secs(XTABLES_LOCK_WAIT.as_secs() * RULE_COMMAND_COUNT as u64);

tests/configs/bubblewrap_network_proxy_egress_denied.json:6

  • This assertion can pass without any MXC egress rule: on hosts that already block direct Internet access (a common reason to require a proxy), the connection to 1.1.1.1:443 fails even if slirp remains unrestricted. The later proxied GitHub request only proves proxy connectivity, not that this direct destination was reachable before enforcement. Use a host-side control listener on a non-proxy port reachable through 10.0.2.2 (or establish an equivalent positive precondition) and assert that the sandbox cannot reach it.
    "commandLine": "bash -c 'set -u; pu=\"${HTTP_PROXY:-${http_proxy:-}}\"; if [ -z \"$pu\" ]; then echo NO_PROXY_ENV; exit 1; fi; pa=\"${pu#*://}\"; pa=\"${pa%%/*}\"; ph=\"${pa%%:*}\"; pp=\"${pa##*:}\"; if [ \"$pp\" = \"$ph\" ]; then pp=80; fi; if ! timeout 6 bash -c \"exec 3<>/dev/tcp/$ph/$pp\" >/dev/null 2>&1; then echo CONTROL_PROXY_UNREACHABLE; exit 1; fi; echo CONTROL_PROXY_REACHABLE_OK; timeout 6 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1; rc=$?; if [ \"$rc\" = 0 ]; then echo DIRECT_EGRESS_LEAKED; exit 1; fi; echo DIRECT_EGRESS_BLOCKED_OK; timeout 4 bash -c \"exec 3<>/dev/tcp/127.0.0.1/9\" >/dev/null 2>&1; lrc=$?; if [ \"$lrc\" = 124 ]; then echo LOOPBACK_DROPPED; exit 1; fi; echo LOOPBACK_EXEMPT_OK; if ! command -v iptables >/dev/null 2>&1; then echo NO_IPTABLES_BINARY; exit 1; fi; capeff=$(grep \"^CapEff\" /proc/self/status | cut -f2); if [ -z \"$capeff\" ]; then echo NO_CAPEFF; exit 1; fi; if [ $((0x$capeff & 0x1000)) -ne 0 ]; then echo CAP_NET_ADMIN_RETAINED; exit 1; fi; echo CAP_NET_ADMIN_DROPPED_OK; terr=$(iptables -F MXC_EGRESS 2>&1); trc=$?; if [ \"$trc\" = 0 ]; then echo \"TAMPER_FLUSH_SUCCEEDED: $terr\"; exit 1; fi; echo \"TAMPER_REFUSED_OK rc=$trc\"; timeout 6 bash -c \"exec 3<>/dev/tcp/1.1.1.1/443\" >/dev/null 2>&1; if [ $? = 0 ]; then echo TAMPER_DISABLED_EGRESS; exit 1; fi; echo TAMPER_INEFFECTIVE_OK; if ! curl -fsSL --max-time 15 https://api.github.com/zen >/dev/null; then echo PROXY_UNREACHABLE; exit 1; fi; echo PROXY_STILL_OK'"

Comment thread src/backends/bubblewrap/common/src/proxy_network.rs Outdated
Copilot AI review requested due to automatic review settings August 18, 2026 22:23
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-proxy-egress-enforcement branch from b20acf1 to 081e737 Compare August 18, 2026 22:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/backends/bubblewrap/common/src/proxy_network.rs:43

  • The parent starts this timeout before slirp startup, but the budget is exactly the sum of the nine maximum -w waits. If each command legitimately waits close to five seconds, slirp startup and process-spawn overhead push readiness past 45 seconds, so concurrent provisioning can be killed even though every rule command is still within its own timeout. Reserve the startup allowance (and therefore some overhead) in this deadline.
const RULE_INSTALL_TIMEOUT: Duration =
    Duration::from_secs(XTABLES_LOCK_WAIT.as_secs() * RULE_COMMAND_COUNT as u64);

src/backends/bubblewrap/common/src/proxy_network.rs:514

  • :: does not guarantee a dual-stack listener: IPV6_V6ONLY may be enabled per socket or via net.ipv6.bindv6only. Rewriting an external [::] proxy to the IPv4 gateway therefore accepts a valid IPv6-only listener during validation but makes it unreachable at runtime. Reject :: with the other IPv6 endpoints (matching this PR's IPv4-only contract), or establish an IPv4 listener before translating it; update the tests and docs that currently promise this translation.
    // `::` 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());

src/backends/bubblewrap/common/src/bwrap_command.rs:102

  • This updated mode contract conflicts with the proxy-environment comment at lines 364–369, which still says raw sockets are “NOT enforced” and calls that a documented limitation. Scope that statement to LegacyProxy or explain that ProxyOnly enforces the destination with its in-namespace firewall, so future changes do not rely on the obsolete security model.
    /// Cooperative proxy routing inside a slirp-backed private namespace, with
    /// egress closed to everything but the proxy.

@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-proxy-egress-enforcement branch from 081e737 to 46b10dc Compare August 19, 2026 03:39
Base automatically changed from user/sodas/bwrap-network-model2 to main August 19, 2026 19:14
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-proxy-egress-enforcement branch from 46b10dc to c1f93e1 Compare August 19, 2026 19:14
Copilot AI review requested due to automatic review settings August 20, 2026 07:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/backends/bubblewrap/common/src/proxy_network.rs:723

  • When the lock file is absent, creating it requires both write and search (X_OK) permission on the parent directory. Checking only W_OK can accept a legacy backend that will then fail at its first rule installation on a write-only/non-searchable directory.
    access(target, AccessFlags::W_OK).is_ok()

src/backends/bubblewrap/common/src/proxy_network.rs:43

  • attach starts this timeout before slirp4netns has signalled its internal readiness, but the budget contains only the nine 5-second rule waits. A valid launch where slirp consumes part of its startup allowance and the rules then contend for the lock can therefore time out before the final rule succeeds. Include the slirp startup allowance (and command overhead), or use separate slirp-ready and rule-installation deadlines.

This issue also appears on line 723 of the same file.

const RULE_INSTALL_TIMEOUT: Duration =
    Duration::from_secs(XTABLES_LOCK_WAIT.as_secs() * RULE_COMMAND_COUNT as u64);

src/backends/bubblewrap/common/src/proxy_network.rs:468

  • slirp4netns is now a background child of the shell supervisor, so this forced path kills only the shell (Child::kill sends SIGKILL). SIGKILL bypasses the shell's TERM/INT trap, leaving a wedged slirp—and potentially the current nsenter/iptables child—running as an orphan. Terminate the whole supervisor process tree (for example, with a dedicated process group and TERM/KILL escalation) before returning.
                    logger.log_line(&format!(
                        "WARNING: Bubblewrap: slirp4netns did not stop promptly; terminating it \
                         ({})",
                        stderr_detail(&self.state_dir.path().join("supervisor.stderr"))
                    ));
                    terminate_child(&mut self.supervisor);

Comment on lines +318 to +330
through slirp's `10.0.2.2` host gateway. Once slirp is up, the supervisor
programs a default-DROP `MXC_EGRESS` chain into that namespace via
`nsenter`, permitting only loopback and the proxy endpoint (IPv6 gets a
DROP-only chain). The workload is released only after every rule is
installed, so it can never run with egress open. A failure to program any
rule aborts the supervisor rather than starting an unenforced sandbox.

Bubblewrap joins the supervisor's user namespace (`--userns`) rather than
creating its own, so the sandbox lives in the namespace that owns the
rule-bearing network namespace. This relies on Bubblewrap dropping
capabilities in the sandboxed process — the runner passes no `--cap-add` —
which is what prevents the workload from holding the `CAP_NET_ADMIN` needed
to flush the chain.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: nice! I do something similar for Windows in AppInfoSvc. That said, I'd be interested in when we clean up things. In Windows we clean up the Windows Filtering Platform rules I add once all processes in the Job object has gone away.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup here is structural rather than bookkeeping: the rules live inside the sandbox's own network namespace, not the host's tables. When the last process in that namespace exits, the kernel tears the namespace down and every rule goes with it — so there is nothing to reconcile, and no way for a rule to outlive the sandbox or affect anything else on the host. Drop for ProxyNetworkNamespace still explicitly reaps the supervisor, slirp4netns and the temp state dir.

The closer analogue to your WFP case is the legacy firewall mode, which does write host-global rules and needs explicit removal — one of the reasons 0.8 moved the rules inside the namespace.

let proxy_only =
ResolvedNetworkMode::from_request(request, request.policy.network_proxy.is_enabled())
== ResolvedNetworkMode::ProxyOnly;
if proxy_only && !request.policy.network_proxy.builtin_test_server {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: In 0.8 networking policy we no longer have the builtin_test_server property. It was only used by us for testing, so it being in the schema was bad on my part. Also now that we're going into a world of in proc SDKs the builtin_test_server won't actually do anything since there won't be a test proxy exe.

What this means for us is that for our testing we'd spin up the exe first either manually or via a test script, then run our test scenario.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and useful to know. It is still in the 0.8 contract today (src/core/mxc_config_contract/src/dev/network.rs), so this PR keeps the carve-out.

When it goes away the change is a simplification rather than a redesign: the only reason builtin_test_server is special here is that it has no address until it is started, so its endpoint has to be validated at runtime in run instead of at policy time. If every proxy is operator-supplied — spun up by a test script beforehand, as you describe — that runtime check disappears and validation collapses into the single policy-time path. Happy to fold that into whichever PR removes the property.

// 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!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: thinking about this some more should we have specific errors related to this proxy_network.rs file? Or is the string in the ScriptResponse enough. Also wondering if any of this code can be reused for lxc or if we're doing something different in there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On errors: the policy-time rejections already reach callers as config errors, so those are typed enough to act on. The distinction the string currently flattens is host-capability vs user-config — "this host has no slirp4netns/nsenter" is an operator problem, "your proxy is bound to ::1" is a caller problem, and an SDK consumer cannot tell them apart today. I would rather do that as a follow-up so it covers the dependency probes too, instead of just this file.

On LXC: reuse runs the other way. LXC has real CAP_NET_ADMIN, so it already has the richer rule layer — NetworkIptablesManager plus network_ingress.rs, with proxy-aware closing DROP and deny-precedence. What is Bubblewrap-specific is manufacturing the authority at all (userns + slirp4netns + nsenter), because it is rootless. Longer term Bubblewrap should adopt LXC's rule builder rather than the reverse; the namespace-bringup half is not reusable in either direction.

/// Cooperative proxy routing inside a slirp-backed private namespace.
/// Later work adds proxy-only egress enforcement.
/// Cooperative proxy routing inside a slirp-backed private namespace, with
/// egress closed to everything but the proxy.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: The existing config warning now contradicts this schema 0.8+ guarantee.

config_parser.rs:1314-1329 still warns every Bubblewrap proxy + defaultPolicy='block' request that raw-socket clients bypass the proxy and reach the host network. Its condition does not check the schema version, while schema 0.8+ resolves this case to ProxyOnly and installs the default-DROP chain described here.

Could we scope that warning to the legacy proxy path (schema 0.6/0.7), or otherwise suppress it for schema 0.8+ ProxyOnly requests?

@SohamDas2021 Soham Das (SohamDas2021) Aug 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, and agreed the warning is wrong under 0.8. Tracked in #970 (Priority3) rather than fixed here.

Comment thread src/backends/bubblewrap/common/src/proxy_network.rs Outdated
@SohamDas2021
Soham Das (SohamDas2021) requested review from a team and Copilot August 20, 2026 18:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/backends/bubblewrap/common/src/proxy_network.rs:729

  • When the lock file is absent, creating it requires both write and search (X_OK) permission on the parent directory. Checking only W_OK can accept a legacy backend during validation and then let the first rule fail during supervisor startup. Check both permissions for the parent while retaining the file check for an existing lock.
fn lock_is_writable(lock: &Path) -> bool {
    let target = if lock.exists() {
        lock
    } else {
        match lock.parent() {
            Some(parent) => parent,
            None => return false,
        }
    };
    access(target, AccessFlags::W_OK).is_ok()

src/backends/bubblewrap/common/src/bwrap_command.rs:103

  • The variant documentation says everything except the proxy is closed, but the implementation intentionally permits loopback and only filters IPv4/IPv6. Describe that narrower guarantee to avoid propagating an inaccurate security invariant to callers and future changes.
    /// Cooperative proxy routing inside a slirp-backed private namespace, with
    /// egress closed to everything but the proxy.
    ProxyOnly,

docs/bwrap-support/bubblewrap-backend.md:415

  • This caveat overstates the containment guarantee. The new rules cover only AF_INET/AF_INET6; as tracked in #936, non-IP families such as AF_VSOCK remain outside iptables. Qualify the claim so users do not interpret proxy-only mode as all-family egress containment.
  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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/backends/bubblewrap/common/src/proxy_network.rs:514

  • ip.is_loopback() also matches addresses such as 127.0.0.2, but slirp's 10.0.2.2 special gateway maps to host 127.0.0.1; it does not preserve the original loopback alias. A proxy URL bound only to 127.0.0.2 therefore passes validation, is rewritten to 10.0.2.2, and dials the wrong host address. Restrict this rewrite to 127.0.0.1 (plus localhost/supported wildcards) and reject other 127/8 literals, unless explicit forwarding that preserves the alias is added.
    let is_host_local = host.eq_ignore_ascii_case("localhost")
        || parsed.is_some_and(|ip| ip.is_loopback() || ip.is_unspecified());

src/backends/bubblewrap/common/src/proxy_network.rs:43

  • This deadline also covers slirp startup, but it is sized to exactly the nine per-command -w 5 budgets. If slirp takes any nonzero time and repeated lock contention makes several successful rule commands wait near their ceilings, the parent can time out before the final command/readiness write, rejecting an otherwise viable concurrent launch. Either wait for slirp.internal with STARTUP_TIMEOUT before starting this rule-installation deadline, or include the startup phase and command overhead in this budget.

This issue also appears on line 513 of the same file.

const RULE_INSTALL_TIMEOUT: Duration =
    Duration::from_secs(XTABLES_LOCK_WAIT.as_secs() * RULE_COMMAND_COUNT as u64);

docs/bwrap-support/bubblewrap-backend.md:419

  • This overstates supported loopback endpoints: slirp's 10.0.2.2 gateway targets host 127.0.0.1, so rewriting (for example) a proxy bound only to 127.0.0.2 loses that alias and makes the proxy unreachable. Document only 127.0.0.1 as supported and reject other 127/8 literals, unless the implementation adds forwarding that preserves the selected alias.
  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

Copilot AI review requested due to automatic review settings August 20, 2026 18:59
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-proxy-egress-enforcement branch from b670f79 to 2946b5a Compare August 20, 2026 18:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

docs/bwrap-support/bubblewrap-backend.md:434

  • This caveat documents a known-incorrect validation diagnostic instead of correcting it. config_parser.rs:1178-1181 still says Bubblewrap's proxy is cooperative and that iptables requires privilege, but this PR installs iptables rules from the unprivileged supervisor. Keep rejecting the combination if it remains unsupported, but make the parser report the actual incompatibility rather than directing users based on behavior this PR removes.
- **Mutually exclusive with iptables enforcement**: setting
  `network.proxy` together with `network.enforcementMode` of `"firewall"`
  or `"both"` is rejected at config-parse time. (The rejection message cites a
  root requirement that proxy mode has since disproved; see the firewall
  section.)

src/backends/bubblewrap/common/src/bwrap_command.rs:103

  • The new enforced semantics leave the old cooperative-path warning active. config_parser.rs:1314-1330 still tells every Bubblewrap proxy request with defaultPolicy='block' that raw sockets bypass the proxy, including schema 0.8 requests now protected by this mode; lines 365-369 in this file repeat the same obsolete claim. Gate that warning to the legacy proxy schemas and update the inline comment so 0.8 users are not given a false security warning.
    /// Cooperative proxy routing inside a slirp-backed private namespace, with
    /// egress closed to everything but the proxy.
    ProxyOnly,

src/backends/bubblewrap/common/src/proxy_network.rs:729

  • When the lock file is absent, checking only W_OK on its parent is insufficient: creating a named file in a directory also requires execute/search permission. A write-only directory therefore passes this probe even though legacy iptables cannot create xtables.lock, so validation accepts the host and startup then fails. Require W_OK | X_OK for the parent-directory case.
    access(target, AccessFlags::W_OK).is_ok()

@SohamDas2021
Soham Das (SohamDas2021) requested review from Branden Bonaby (bbonaby) and removed request for a team August 20, 2026 19:05
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3
Copilot AI review requested due to automatic review settings August 20, 2026 19:46
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-proxy-egress-enforcement branch from 2946b5a to 613dc25 Compare August 20, 2026 19:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/backends/bubblewrap/common/src/proxy_network.rs:107

  • slirp4netns is now a background child, but every timeout path still calls terminate_child, which sends SIGKILL only to the supervisor shell. In particular, if shutdown times out because slirp ignored the exit-fd closure, killing the shell leaves that same slirp process orphaned; an attach timeout can likewise leave an in-flight nsenter/iptables child behind. Put the supervisor and all descendants in a dedicated process group and terminate/reap the group, or otherwise retain and kill the descendant PIDs from Rust.
slirp4netns --configure --mtu=65520 \
    --ready-fd 9 --exit-fd 4 \
    "$child_pid" tap0 &
slirp_pid=$!
trap 'kill "$slirp_pid" 2>/dev/null || true' TERM INT

docs/bwrap-support/bubblewrap-backend.md:434

  • The runtime rejection still says proxy enforcement is cooperative and that iptables requires privilege (config_parser.rs:1091-1104), both of which this PR makes false for schema 0.8. Leaving a caveat in the docs does not fix the actionable error users receive. Update that parser comment/message (and its tests) to explain the actual reason the two modes remain mutually exclusive, then remove this stale-message note.
  or `"both"` is rejected at config-parse time. (The rejection message cites a
  root requirement that proxy mode has since disproved; see the firewall
  section.)

docs/bwrap-support/bubblewrap-backend.md:415

  • This overstates the enforcement boundary. The new chains govern only AF_INET/AF_INET6; the linked #936 documents that AF_VSOCK remains available and bypasses these rules. Scope this guarantee to IP egress and document the known non-IP socket-family limitation so operators do not read “only loopback and the proxy endpoint” as a complete egress guarantee.
  and additionally programs a default-DROP egress chain inside the sandbox's
  private network namespace. Clients that ignore the env vars (raw sockets,
  custom HTTP clients) can no longer reach the network directly: only loopback
  and the proxy endpoint are permitted. DNS is deliberately **not** opened —
  the proxy resolves on the workload's behalf — so the proxy endpoint must be

depends on the schema version, because 0.8 replaced a path that did not
actually filter.

**Schema 0.8+ — enforced.** `enforcementMode: "firewall"` puts the sandbox in

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: in 1.0 enforcementMode won't exist (it's still in the 0.8 schema I'm adding in my 0.8 PR), but when we remove the legacy network schema it won't exist. Firewall will be implicit. Just giving you an FYI for the future, that said, I don't think that changes your messaging below .

Comment on lines +288 to +290
Rows 2 and 3 are rejected on schema `0.8.0-alpha` and later — at config parse,
and again in the runner for programmatic callers that bypass the parser — and
emit a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: because we're doing this legacy + new networking schema together for 0.8 they should technically be able to still do them when using 0.8 with the legacy network details. I guess unless you're changing the legacy behavior as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants