[LXC] Fail closed when firewall rules cannot be scoped to the container

[Darren Hoehna (dhoehna)]

The problem

install_firewall_rules built the container's full deny-all chain and filled it with the network policy, then — when it did not know the container's veth interface — logged a warning and returned Ok(()).

The chain is only ever reached from FORWARD via -i <veth>. With no hook, no packet traverses it. So the caller was told the network policy had been applied while zero packets were being filtered.

That is worse than having no firewall, because it looks enforced. Nothing downstream had any way to tell the difference.

Why erroring is the right answer, and not the other two

There are exactly three things this code can do when it cannot scope the rules:

Option	Result
Return Ok (before)	Chain exists, filters nothing, caller believes it is confined
Install the rules unscoped	They would filter — and would hit every other container and the host's own traffic
Return Err (this PR)	Container does not start; nothing is silently unprotected

Erroring loses no enforcement, because there was none to lose.

Blast radius — verified, not assumed

• Only reachable when the caller explicitly asked for a firewall. apply_firewall_rules returns early unless the enforcement mode is Firewall or Both, and NetworkEnforcementMode defaults to Capabilities. A container that never wanted firewalling is untouched — and there is a test pinning exactly that.
• Rollback and teardown already handle Err correctly. apply_firewall_rules_inner converts it into a precise teardown of what was created plus residual adoption, and lxc_runner.rs:221 destroys the container rather than starting a workload that believes it is confined. No new branch was needed.
• CI already proves veth discovery works on Linux. All four E2E scripts that exercise firewall enforcement (run_lxc_network_cidr_boundary_test.sh, ..._dualstack_test.sh, ..._invalid_cidr_test.sh, ..._ipv6_cidr_test.sh) already fail if "FORWARD hook installed" is absent from the output. So in every environment where those pass today, a veth exists and this change is a no-op. It only alters the case those scripts already treat as a failure.
• No existing test pinned the old Ok(()) — all 115 still pass unchanged. That the fail-open was untested is itself the finding.

Tests

Six black-box tests, written from the documented contract by an author who did not read the implementation, so they describe the intended behavior rather than mirroring whatever the code happens to do.

They pin the refusal under both firewall-requesting modes separately, the error naming the chain, the negative control (same policy succeeds once an interface is set — without which an apply that always returned Err would pass everything else), teardown ordering relative to chain creation, and the Capabilities no-op that bounds the blast radius.

Mutation tested: 7 seeded defects, 7 caught by a failing test, 0 survivors. The seeds include restoring the old Ok(()), dropping the chain name from the message, applying the check to Firewall but not Both, inverting the interface check, skipping rollback, and swallowing the error one layer up in record_apply_outcome. Each mutant is compiled with lints silenced, so a defect caught only by the compiler counts as a harness failure rather than a pass — the tests have to answer for themselves.

The spec is attached as a #[path] child module because the fake-firewall seam is #[cfg(test)] and private, which an integration test — a separate crate — can never reach.

Scope

Slice 3 of the PR #632 re-cut (#632 was closed; its merge resolution had grown past the point of being reviewable). Follows #788 and #789.

Deliberately not fixed here: the second fail-open in the same file — build_policy_rules_logged warns "could not resolve host" and continues, so under defaultPolicy: allow a blockedHosts entry that fails DNS silently becomes reachable. Four Linux E2E scripts grep that exact string and one requires it, so changing it needs those scripts updated in the same PR. It moves to the enforcement slice.

Also noted and not fixed: host lists are only enforced under Firewall/Both, while the runner treats a non-empty host list as "needs network" — so under the default Capabilities mode a container with blockedHosts gets no firewall at all. That belongs with the enforcement work.

Refs AB#62830341.

Microsoft Reviewers: Open in CodeFlow

[azure-pipelines]

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

[Copilot]

Pull request overview

Fails closed when LXC firewall rules cannot be scoped to a veth interface.

Changes:

• Returns an enforcement error instead of silently skipping the FORWARD hook.
• Adds six tests covering failure, rollback, success, and no-op behavior.
• Review found a Bubblewrap regression from changing the shared manager.

Reviewed changes

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

File	Description
src/backends/lxc/common/src/network_iptables.rs	Adds missing-veth failure behavior and test module.
src/backends/lxc/common/src/network_iptables_veth_spec.rs	Tests fail-closed and rollback semantics.

---

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

[Darren Hoehna (dhoehna)]

Superseded by #798.

This work was split into six PRs on my initiative; it should have been one. Four of the six (#790, #792, #796, #797) were cumulatively stacked, so each one re-rendered the previous diff rather than reducing what a reviewer had to read, and they forced a merge order for no benefit.

All of the changes here are in #798, cut fresh from main as a single branch, verified as a unit: 158 + 44 tests passing, clippy and fmt clean, 14/14 E2E. Closing this in favor of that one. No content is lost.