[LXC] Enforce the deny-all-except-proxy network policy (model 2)

.github/workflows/lxc-e2e.yml

@@ -0,0 +1,133 @@
+name: LXC E2E Tests
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  lxc-e2e:
+    name: LXC-Exec Container and Network Policy
+    runs-on: ubuntu-latest
+    timeout-minutes: 45
+
+    steps:
+      - uses: actions/checkout@v6
+
+      - name: Setup Rust toolchain
+        run: rustup update stable
+
+      - name: Point cargo at the MxcDependencies feed
+        uses: ./.github/actions/setup-cargo-feed
+
+      - name: Cache Rust build artifacts
+        uses: Swatinem/rust-cache@v2
+        with:
+          workspaces: src
+
+      - name: Install LXC and firewall tooling
+        run: |
+          sudo apt-get update
+          sudo DEBIAN_FRONTEND=noninteractive apt-get install -y \
+            lxc lxc-templates lxc-utils iptables debootstrap uidmap bridge-utils
+
+      # A bridged veth only reaches the FORWARD chain while br_netfilter is
+      # delivering bridged packets to iptables. Without it the firewall rules
+      # install cleanly and never fire, so the network policy tests would pass
+      # against a firewall that filters nothing.
+      - name: Enable bridge netfilter
+        run: |
+          sudo modprobe br_netfilter
+          sudo sysctl -w net.bridge.bridge-nf-call-iptables=1
+          sudo sysctl -w net.bridge.bridge-nf-call-ip6tables=1
+
+      # GitHub-hosted runners ship Docker, and Docker sets the IPv4 FORWARD
+      # policy to DROP. That breaks these tests twice over.
+      #
+      # First, it breaks them outright. MXC hooks its chain on traffic leaving
+      # the container (`-i <veth>` / `--physdev-in <veth>`), so an allowed
+      # request is accepted on the way out -- but the reply arrives in the
+      # opposite direction, matches no MXC rule, falls through to the policy,
+      # and is dropped. The connection times out and an explicitly allowed
+      # destination looks unreachable. Observed exactly that: DNS resolved,
+      # because dnsmasq on lxcbr0 is host-local and never traverses FORWARD,
+      # and then `wget: can't connect to remote host (140.82.116.5)`.
+      #
+      # Second, and worse, it would make the deny cases meaningless. Under a
+      # DROP policy a container with NO working MXC hook at all is also
+      # unreachable, so the enforcement and deny-precedence tests would report
+      # success against a firewall that filters nothing -- which is the precise
+      # bug this suite exists to detect, and the reason these tests carry
+      # positive controls.
+      #
+      # Setting the policy to ACCEPT restores the condition the tests were
+      # written for: the host forwards by default, so the ONLY thing that can
+      # block container traffic is a rule MXC installed. A missing hook then
+      # shows up as an unexpected success and fails the deny case loudly.
+      # A narrower conntrack RELATED,ESTABLISHED rule would fix the reply path
+      # but leave the DROP policy, and with it the vacuous pass.
+      - name: Let the host forward, so only MXC rules can block
+        run: |
+          sudo iptables -P FORWARD ACCEPT
+          sudo ip6tables -P FORWARD ACCEPT
+          sudo iptables -S FORWARD | head -5
+
+      - name: Report the environment these tests depend on
+        run: |
+          echo "--- kernel ---"
+          uname -a
+          echo "--- lxc ---"
+          lxc-create --version || echo "MISSING lxc-create"
+          echo "--- iptables ---"
+          sudo iptables --version || echo "MISSING iptables"
+          sudo ip6tables --version || echo "MISSING ip6tables"
+          echo "--- forward policy (must be ACCEPT, or deny cases pass vacuously) ---"
+          sudo iptables -S FORWARD | head -1
+          sudo ip6tables -S FORWARD | head -1
+          echo "--- bridge netfilter ---"
+          cat /proc/sys/net/bridge/bridge-nf-call-iptables || echo "MISSING bridge-nf-call-iptables"
+          cat /proc/sys/net/bridge/bridge-nf-call-ip6tables || echo "MISSING bridge-nf-call-ip6tables"
+          echo "--- host ipv6 ---"
+          cat /proc/net/if_inet6 || echo "no /proc/net/if_inet6 (IPv6 disabled)"
+
+      - name: Build lxc-exec
+        working-directory: src
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: cargo build --release -p lxc --bin lxc-exec
+
+      # MXC_LXC_TESTS_REQUIRE_EXECUTION turns an honest skip into a failure.
+      # On a developer box a missing ip6tables is a reason to run what you can.
+      # Here the runner is provisioned specifically to execute this suite, so a
+      # skip means a prerequisite disappeared and the gate would go green while
+      # testing nothing.
+      - name: Run LXC E2E suite
+        env:
+          MXC_LXC_TESTS_REQUIRE_EXECUTION: "1"
+        run: sudo --preserve-env=MXC_LXC_TESTS_REQUIRE_EXECUTION bash tests/scripts/run_lxc_all_tests.sh
+
+      - name: Show leftover firewall state on failure
+        if: failure()
+        run: |
+          echo "--- FORWARD chain ---"
+          sudo iptables -S FORWARD || true
+          sudo ip6tables -S FORWARD || true
+          echo "--- MXC chains ---"
+          sudo iptables -S | grep -E '^-N MXC-' || echo "none"
+          sudo ip6tables -S | grep -E '^-N MXC-' || echo "none"
+
+      - name: Upload logs on failure
+        if: failure() || cancelled()
+        uses: actions/upload-artifact@v6
+        with:
+          name: lxc-e2e-logs-${{ github.event.pull_request.number || github.run_number }}
+          retention-days: 7
+          path: |
+            logs/
+            **/*.log

docs/lxc-support/lxc-backend.md

@@ -116,9 +116,49 @@ Network policies are enforced with parallel `iptables` and `ip6tables` chains sc
 | `defaultPolicy: "block"` | Final DROP rule in the container chain |
 | `defaultPolicy: "allow"` | Final ACCEPT rule in the container chain |
 | `allowedHosts` | ACCEPT rules for IP literals, CIDR blocks, or resolved hostnames |
-| `blockedHosts` | DROP rules for IP literals, CIDR blocks, or resolved hostnames |
-
-`allowedHosts` and `blockedHosts` entries may be bare IPv4/IPv6 literals, IPv4/IPv6 CIDR blocks, or hostnames. Hostnames are resolved to both A and AAAA records; IPv4 destinations are applied to the `iptables` chain and IPv6 destinations are applied to the `ip6tables` chain. Entries whose CIDR prefix is out of range for its family (or otherwise malformed) are reported as unresolved and skipped, leaving the rest of the policy in force. Host-list rules match all ports and protocols; port- and protocol-specific egress rules are not supported.
+| `blockedHosts` | DROP rules for IP literals, CIDR blocks, or resolved hostnames, emitted *before* the ACCEPT rules |
+
+**A deny wins over an overlapping allow.** `iptables` evaluates a chain top to
+bottom and stops at the first match, so precedence is decided purely by
+emission order. All `blockedHosts` rules are emitted ahead of all
+`allowedHosts` rules, which means a destination named by both lists is dropped.
+Without that ordering an allow entry broad enough to cover a blocked
+destination — `0.0.0.0/0`, or a CIDR containing the blocked address — silently
+defeats the block, and the resulting chain looks fully populated while
+filtering nothing.
+
+Two limits on that guarantee are worth stating plainly, because "deny always
+wins" is not true without them:
+
+- **DNS is exempt.** The base chain accepts UDP and TCP destination port 53
+  unconditionally and is installed ahead of the generated policy rules, so
+  port-53 traffic to a blocked destination is accepted before its DROP rule is
+  reached. Narrowing that rule needs to know which resolver addresses are
+  legitimate, and no schema field carries them today.
+- **A hostname in both lists is resolved twice.** Each list entry is resolved
+  independently, so a name behind round-robin DNS can return one address for
+  the `blockedHosts` entry and a different one for the `allowedHosts` entry.
+  The guarantee holds for *addresses*, not for names. Use literal IPs or CIDRs
+  when a destination must be denied deterministically.
+
+`allowedHosts` and `blockedHosts` entries may be bare IPv4/IPv6 literals, IPv4/IPv6 CIDR blocks, or hostnames. Hostnames are resolved to both A and AAAA records; IPv4 destinations are applied to the `iptables` chain and IPv6 destinations are applied to the `ip6tables` chain. Host-list rules match all ports and protocols; port- and protocol-specific egress rules are not supported.
+
+An entry that resolves to nothing — an unknown hostname, or a CIDR prefix out
+of range for its family — cannot be turned into a rule. What that costs
+depends on the entry and on `defaultPolicy`:
+
+| Entry | `defaultPolicy` | Behavior |
+|-------|-----------------|----------|
+| `allowedHosts` | either | Reported as unresolved and skipped. Failing to write an ACCEPT rule can only make the policy more restrictive |
+| `blockedHosts` | `block` | Reported as unresolved and skipped. The closing DROP already denies the destination, so the unwritten rule was redundant |
+| `blockedHosts` | `allow` | **Fails firewall setup.** The chain ends in ACCEPT, so the unwritten DROP was the only thing that would have denied that destination, and skipping it silently converts a deny into an allow |
+
+One gap remains open and is not detected: under `defaultPolicy: "block"`, an
+`allowedHosts` entry broad enough to cover a destination whose `blockedHosts`
+rule went unwritten still reaches that destination. Detecting it would require
+the address the failed entry was *meant* to resolve to, which is by definition
+unavailable, so no check over the policy text can be complete — and a partial
+check would imply a guarantee this code cannot make.
 
 Before programming the IPv6 chain, MXC probes `ip6tables` with a read-only `ip6tables -S` and classifies the result three ways:
 
@@ -130,7 +170,40 @@ Before programming the IPv6 chain, MXC probes `ip6tables` with a read-only `ip6t
 
 Host IPv6 activity is read from `/proc/net/if_inet6`: a non-loopback interface with an IPv6 address counts as active, while loopback-only `::1` on `lo` (present even on IPv4-only hosts) does not. If that file cannot be read at all — as opposed to being absent, which means IPv6 is disabled — the state is treated as *unknown* rather than as a confirmed "IPv6 is off", so an unreadable IPv6 state fails closed instead of leaving IPv6 unfiltered.
 
-The chains are hooked into `FORWARD` for container egress by matching the host-side veth as the input interface. If MXC cannot discover the container veth, it skips the `FORWARD` hook with a warning rather than applying host-wide rules.
+The chains are hooked into `FORWARD` for container egress with **up to two
+rules per family**, because the input interface `FORWARD` sees depends on how
+the veth is attached:
+
+| Attachment | Rule that matches |
+|------------|-------------------|
+| veth routed directly by the host | `-i <veth>` |
+| veth enslaved to a bridge (the default LXC topology) | `-m physdev --physdev-in <veth>` |
+
+The two are mutually exclusive for any given packet, so nothing is counted
+twice. Installing only `-i <veth>` is what previously let a fully populated
+deny-all chain sit in the ruleset filtering nothing on the default bridged
+topology.
+
+The `physdev` rule is required only on a bridged veth. On a directly routed
+veth a host whose kernel lacks the `physdev` match logs a warning and
+continues with the interface rule alone, which is the rule that matches there;
+on a bridged veth the same failure is fatal, because `physdev` is the only
+rule that could ever match.
+
+A bridged veth additionally requires `br_netfilter` to be delivering bridged
+packets to iptables. With `/proc/sys/net/bridge/bridge-nf-call-iptables` absent
+or `0`, both hook rules install cleanly and neither ever fires. MXC reads that
+file and **fails firewall setup** rather than reporting success for a chain
+that could never be reached. When the IPv6 chain is programmed,
+`/proc/sys/net/bridge/bridge-nf-call-ip6tables` is checked separately and to
+the same standard.
+
+If MXC cannot discover the container veth at all, firewall setup **fails** and
+the partially created chains are rolled back. An unhooked chain is never
+traversed, so reporting success would hand the caller a deny-all chain that
+filters nothing — strictly worse than no firewall, because it looks enforced.
+Installing the rules host-wide instead is not an option either: unscoped, they
+would apply to every container and to the host's own traffic.
 
 Firewall state is torn down automatically with best-effort removal of the `FORWARD` hooks and both per-container chains; there is no network-policy opt-out field. Setup failures after partial creation are rolled back before returning an error, so retries do not trip over leftover chains.
 

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

@@ -205,7 +205,7 @@ impl BubblewrapScriptRunner {
                 logger,
                 "Bubblewrap: applying iptables rules for host-level network filtering"
             );
-            let mut mgr = NetworkIptablesManager::new(&container_name);
+            let mut mgr = build_firewall_manager(&container_name);
             match mgr.apply_firewall_rules(&request.policy, logger) {
                 Ok(true) => {}
                 Ok(false) => {
@@ -495,6 +495,23 @@ fn needs_iptables_rules(request: &ExecutionRequest) -> bool {
     uses_firewall && has_host_rules
 }
 
+/// Build the iptables manager for a Bubblewrap sandbox.
+///
+/// Unprivileged bwrap has no veth to scope a chain to: the sandbox either
+/// shares the host network namespace or gets a private one, and neither yields
+/// a host-side interface to match on (see `local_network_diagnostic` in
+/// `bwrap_command`). A missing veth is therefore structural here, not a failed
+/// lookup, so the manager is told not to fail closed on it. Without that, every
+/// Bubblewrap sandbox requesting firewall enforcement would refuse to start.
+///
+/// This lives in its own function so the declaration is covered by a test;
+/// inlined at the call site, deleting it broke nothing that any test could see.
+fn build_firewall_manager(container_name: &str) -> NetworkIptablesManager {
+    let mut mgr = NetworkIptablesManager::new(container_name);
+    mgr.allow_missing_veth_interface();
+    mgr
+}
+
 /// Best-effort iptables cleanup. Called on both success and error paths.
 fn cleanup_iptables(manager: &mut Option<NetworkIptablesManager>, logger: &mut Logger) {
     if let Some(ref mut mgr) = manager {
@@ -640,6 +657,21 @@ mod tests {
         }
     }
 
+    #[test]
+    fn the_firewall_manager_tolerates_the_veth_bubblewrap_never_has() {
+        // bwrap never calls set_veth_interface, so the shared manager's
+        // fail-closed path would refuse every firewall-mode sandbox at startup.
+        // The manager this backend builds must therefore have declared the
+        // absence up front.
+        let mgr = build_firewall_manager("bwrap-cov");
+
+        assert!(
+            mgr.veth_scoping_is_optional(),
+            "Bubblewrap has no veth, so the manager it builds must declare that a \
+             missing one is expected -- otherwise firewall-mode sandboxes cannot start"
+        );
+    }
+
     #[test]
     fn validate_does_not_locally_gate_builtin_test_server() {
         // The builtinTestServer gate moved to `wxc_common::validator::validate_common`