[LXC] Make deny rules win over allow rules and fail closed on an unresolvable block

src/backends/lxc/common/src/network_iptables_veth_spec.rs

@@ -0,0 +1,193 @@
+//! Spec for the fail-closed contract of `apply_firewall_rules`: when the
+//! firewall cannot be scoped to the container, the caller must be told the
+//! policy was not applied rather than being handed a chain that filters
+//! nothing.
+//!
+//! Attached to `network_iptables` as a child module via `#[path]`, so it can
+//! reach the `#[cfg(test)]` fake-firewall seam.
+
+use super::*;
+use wxc_common::logger::{Logger, Mode};
+use wxc_common::models::{ContainerPolicy, NetworkEnforcementMode};
+
+/// Build a policy that requests the given network enforcement mode, leaving
+/// every other field at its default.
+fn policy_requesting(mode: NetworkEnforcementMode) -> ContainerPolicy {
+    ContainerPolicy {
+        network_enforcement_mode: mode,
+        ..Default::default()
+    }
+}
+
+// A chain that is never hooked to the container's veth interface is a chain
+// no packet ever traverses. If the manager does not know which veth belongs
+// to the container, it must refuse rather than report success on a firewall
+// that filters nothing. This covers the `Firewall` half of R1; `Both` is
+// covered separately below so a fix scoped to only one enforcement mode
+// cannot pass the suite.
+#[test]
+fn apply_is_refused_when_the_container_interface_is_unknown_in_firewall_mode() {
+    let _fake = super::test_firewall::install();
+    let mut manager = NetworkIptablesManager::new("ctrl-firewall");
+    let policy = policy_requesting(NetworkEnforcementMode::Firewall);
+    let mut logger = Logger::new(Mode::Buffer);
+
+    let result = manager.apply_firewall_rules(&policy, &mut logger);
+
+    assert!(
+        result.is_err(),
+        "Firewall mode with no veth interface set must fail closed, got {:?}",
+        result
+    );
+}
+
+// Same hazard as above under `Both`, which also requests firewall
+// enforcement. A fix that only checks the interface in the `Firewall` arm
+// would leave `Both` silently unenforced, and only a dedicated test for this
+// mode would catch it.
+#[test]
+fn apply_is_refused_when_the_container_interface_is_unknown_in_both_mode() {
+    let _fake = super::test_firewall::install();
+    let mut manager = NetworkIptablesManager::new("ctrl-both");
+    let policy = policy_requesting(NetworkEnforcementMode::Both);
+    let mut logger = Logger::new(Mode::Buffer);
+
+    let result = manager.apply_firewall_rules(&policy, &mut logger);
+
+    assert!(
+        result.is_err(),
+        "Both mode with no veth interface set must fail closed, got {:?}",
+        result
+    );
+}
+
+// A caller who is told "firewall applied" while the interface was never known
+// deserves an error that says what to check. If the message drops the chain
+// name or the "will not be enforced" meaning, an operator debugging why a
+// container's traffic is unfiltered has nothing to search logs for.
+#[test]
+fn refusal_error_names_the_unenforced_chain() {
+    let _fake = super::test_firewall::install();
+    let mut manager = NetworkIptablesManager::new("acme-web");
+    let policy = policy_requesting(NetworkEnforcementMode::Firewall);
+    let mut logger = Logger::new(Mode::Buffer);
+
+    let err = manager
+        .apply_firewall_rules(&policy, &mut logger)
+        .expect_err("Firewall mode with no veth interface set must fail closed");
+
+    let chain = "MXC-acme-web";
+    assert!(
+        err.contains(chain),
+        "error must name the chain left unenforced ({chain}), got: {err}"
+    );
+
+    let lower = err.to_lowercase();
+    assert!(
+        lower.contains("not") && lower.contains("enforc"),
+        "error must convey that the policy will not be enforced, got: {err}"
+    );
+}
+
+// Negative control for R1: the only thing that changes here is that the veth
+// interface is now known. Without this test, R1's failures would prove
+// nothing about the interface check specifically -- an `apply_firewall_rules`
+// that always returned `Err` would also pass every R1 test above.
+#[test]
+fn apply_succeeds_once_the_veth_interface_is_known() {
+    let fake = super::test_firewall::install();
+    let mut manager = NetworkIptablesManager::new("ctrl-negative");
+    manager.set_veth_interface("veth-ctrl0");
+    let policy = policy_requesting(NetworkEnforcementMode::Firewall);
+    let mut logger = Logger::new(Mode::Buffer);
+    let _ = fake.forget_issued();
+
+    let result = manager.apply_firewall_rules(&policy, &mut logger);
+
+    assert!(
+        result.is_ok(),
+        "the same Firewall policy that fails with no veth interface must succeed once one is set, got {:?}",
+        result
+    );
+    assert!(
+        !fake.issued().is_empty(),
+        "a successful Firewall apply must actually issue iptables commands, not just report success"
+    );
+}
+
+// A caller who is refused must not be left holding a chain on the host: an
+// unhooked-but-still-installed chain is inert today but becomes a liability
+// the moment anything later hooks a chain by that name. The failed apply
+// must tear down what it created, not merely stop short of hooking it up.
+#[test]
+fn apply_tears_down_the_chain_it_created_when_it_fails_closed() {
+    let fake = super::test_firewall::install();
+    let mut manager = NetworkIptablesManager::new("ctrl-teardown");
+    let policy = policy_requesting(NetworkEnforcementMode::Firewall);
+    let mut logger = Logger::new(Mode::Buffer);
+    let _ = fake.forget_issued();
+
+    let result = manager.apply_firewall_rules(&policy, &mut logger);
+    assert!(
+        result.is_err(),
+        "expected the apply to fail closed so the teardown path runs, got {:?}",
+        result
+    );
+
+    let issued = fake.issued();
+    let chain = "MXC-ctrl-teardown";
+    let creation_index = issued
+        .iter()
+        .position(|cmd| cmd.iter().any(|a| a == "-N") && cmd.iter().any(|a| a == chain))
+        .unwrap_or_else(|| {
+            panic!(
+                "expected a chain-creation (-N) command naming {chain} before the failure, issued: {:?}",
+                issued
+            )
+        });
+    let teardown_index = issued
+        .iter()
+        .position(|cmd| {
+            (cmd.iter().any(|a| a == "-F") || cmd.iter().any(|a| a == "-X"))
+                && cmd.iter().any(|a| a == chain)
+        })
+        .unwrap_or_else(|| {
+            panic!(
+                "expected a teardown (-F/-X) command naming {chain} after the failed apply, issued: {:?}",
+                issued
+            )
+        });
+
+    assert!(
+        teardown_index > creation_index,
+        "teardown of {chain} must be issued after its creation, issued: {:?}",
+        issued
+    );
+}
+
+// A container that never asked for a firewall (`Capabilities` is the default
+// enforcement mode) must not be punished for an interface the caller was
+// never required to set. Any firewall command touching the host here would
+// be an unrequested side effect on a container that opted out of firewalling
+// entirely.
+#[test]
+fn capabilities_only_container_is_unaffected_by_a_missing_veth_interface() {
+    let fake = super::test_firewall::install();
+    let mut manager = NetworkIptablesManager::new("ctrl-capsonly");
+    let policy = policy_requesting(NetworkEnforcementMode::Capabilities);
+    let mut logger = Logger::new(Mode::Buffer);
+    let _ = fake.forget_issued();
+
+    let result = manager.apply_firewall_rules(&policy, &mut logger);
+
+    assert!(
+        result.is_ok(),
+        "Capabilities mode must not fail just because the veth interface is unknown, got {:?}",
+        result
+    );
+    assert!(
+        fake.issued().is_empty(),
+        "Capabilities-only enforcement must not issue any iptables commands, issued: {:?}",
+        fake.issued()
+    );
+}

tests/configs/lxc_network_deny_precedence_control.json

@@ -0,0 +1,21 @@
+{
+  "version": "0.8.0-alpha",
+  "containerId": "CLI-LXC-Net-DenyCtl",
+  "containment": "lxc",
+  "process": {
+    "commandLine": "sh -c \"wget -qO- --timeout=10 https://api.github.com/zen >/dev/null 2>&1 && echo MXC_NET_ALLOWED || echo MXC_NET_BLOCKED\""
+  },
+  "lifecycle": {
+    "destroyOnExit": true
+  },
+  "lxc": {
+    "distribution": "alpine",
+    "release": "3.23"
+  },
+  "network": {
+    "defaultPolicy": "block",
+    "enforcementMode": "firewall",
+    "allowedHosts": ["0.0.0.0/0", "::/0"],
+    "blockedHosts": []
+  }
+}

tests/configs/lxc_network_deny_precedence_overlap.json

@@ -0,0 +1,21 @@
+{
+  "version": "0.8.0-alpha",
+  "containerId": "CLI-LXC-Net-DenyWins",
+  "containment": "lxc",
+  "process": {
+    "commandLine": "sh -c \"wget -qO- --timeout=10 https://api.github.com/zen >/dev/null 2>&1 && echo MXC_NET_ALLOWED || echo MXC_NET_BLOCKED\""
+  },
+  "lifecycle": {
+    "destroyOnExit": true
+  },
+  "lxc": {
+    "distribution": "alpine",
+    "release": "3.23"
+  },
+  "network": {
+    "defaultPolicy": "block",
+    "enforcementMode": "firewall",
+    "allowedHosts": ["0.0.0.0/0", "::/0"],
+    "blockedHosts": ["0.0.0.0/0", "::/0"]
+  }
+}

tests/configs/lxc_network_enforcement_allow.json

@@ -0,0 +1,21 @@
+{
+  "version": "0.8.0-alpha",
+  "containerId": "CLI-LXC-Net-Allow",
+  "containment": "lxc",
+  "process": {
+    "commandLine": "sh -c \"wget -qO- --timeout=10 https://api.github.com/zen >/dev/null 2>&1 && echo MXC_NET_ALLOWED || echo MXC_NET_BLOCKED\""
+  },
+  "lifecycle": {
+    "destroyOnExit": true
+  },
+  "lxc": {
+    "distribution": "alpine",
+    "release": "3.23"
+  },
+  "network": {
+    "defaultPolicy": "block",
+    "enforcementMode": "firewall",
+    "allowedHosts": ["api.github.com"],
+    "blockedHosts": []
+  }
+}

tests/configs/lxc_network_enforcement_deny.json

@@ -0,0 +1,21 @@
+{
+  "version": "0.8.0-alpha",
+  "containerId": "CLI-LXC-Net-Deny",
+  "containment": "lxc",
+  "process": {
+    "commandLine": "sh -c \"wget -qO- --timeout=8 https://api.github.com/zen >/dev/null 2>&1 && echo MXC_NET_ALLOWED || echo MXC_NET_BLOCKED\""
+  },
+  "lifecycle": {
+    "destroyOnExit": true
+  },
+  "lxc": {
+    "distribution": "alpine",
+    "release": "3.23"
+  },
+  "network": {
+    "defaultPolicy": "block",
+    "enforcementMode": "firewall",
+    "allowedHosts": [],
+    "blockedHosts": []
+  }
+}

tests/scripts/run_lxc_all_tests.sh

@@ -66,6 +66,8 @@ run_test "LXC Network IPv6+CIDR" "$SCRIPT_DIR/run_lxc_network_ipv6_cidr_test.sh"
 run_test "LXC Network Invalid CIDR" "$SCRIPT_DIR/run_lxc_network_invalid_cidr_test.sh"
 run_test "LXC Network Dual-Stack Hostname" "$SCRIPT_DIR/run_lxc_network_dualstack_test.sh"
 run_test "LXC Network CIDR Boundary" "$SCRIPT_DIR/run_lxc_network_cidr_boundary_test.sh"
+run_test "LXC Network Enforcement" "$SCRIPT_DIR/run_lxc_network_enforcement_test.sh"
+run_test "LXC Network Deny Precedence" "$SCRIPT_DIR/run_lxc_network_deny_precedence_test.sh"
 run_test "LXC Timeout" "$SCRIPT_DIR/run_lxc_timeout_test.sh"
 run_test "LXC Env+Cwd" "$SCRIPT_DIR/run_lxc_env_cwd_test.sh"
 

tests/scripts/run_lxc_network_deny_precedence_test.sh

@@ -0,0 +1,96 @@
+#!/bin/bash
+# LXC deny-precedence enforcement test
+#
+# A destination named in both allowedHosts and blockedHosts must be blocked.
+# The chain is first-match-wins, so this is decided entirely by which list is
+# emitted first -- there is no separate precedence pass to assert on. That
+# makes it invisible to any test that only inspects rules individually, and it
+# is why this assertion is behavioral rather than a log grep.
+#
+# Both configs name the same destination set, 0.0.0.0/0 and ::/0, so the rules
+# are literal CIDRs rather than a hostname resolved once per list entry. A
+# hostname would be resolved separately for the allow entry and the block
+# entry, and round-robin DNS could hand back different addresses for the two,
+# which would make the outcome depend on which address wget happened to pick.
+#
+# The control run is what makes the overlap run mean anything. Without it, a
+# host with no working egress at all -- or a change that broke networking
+# outright -- would produce the same blocked verdict and look like a pass.
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+REPO_DIR="$(dirname "$(dirname "$SCRIPT_DIR")")"
+LXC_EXEC="$REPO_DIR/src/target/release/lxc-exec"
+
+if [ ! -f "$LXC_EXEC" ]; then
+    LXC_EXEC="$REPO_DIR/src/target/debug/lxc-exec"
+fi
+
+# An honest skip for a missing prerequisite: exit 77 so run_lxc_all_tests.sh
+# records SKIPPED rather than PASS. A suite that could not run must not look green.
+SKIP_EXIT=77
+skip() {
+    echo "SKIP: $1"
+    exit "$SKIP_EXIT"
+}
+
+[ "$(id -u)" -eq 0 ] || skip "requires root for iptables/ip6tables and LXC."
+command -v iptables >/dev/null 2>&1 || skip "iptables is not installed."
+command -v ip6tables >/dev/null 2>&1 || skip "ip6tables is not installed."
+command -v lxc-create >/dev/null 2>&1 || skip "LXC (lxc-create) is not installed."
+[ -f "$LXC_EXEC" ] || skip "lxc-exec binary not built; run build.sh first."
+
+OVERLAP_CONFIG="$REPO_DIR/tests/configs/lxc_network_deny_precedence_overlap.json"
+CONTROL_CONFIG="$REPO_DIR/tests/configs/lxc_network_deny_precedence_control.json"
+OVERLAP_CHAIN="MXC-CLI-LXC-Net-DenyWins"
+CONTROL_CHAIN="MXC-CLI-LXC-Net-DenyCtl"
+
+fail() {
+    echo "FAIL: $1"
+    exit 1
+}
+
+assert_firewall_chain_cleaned_up() {
+    if iptables -S "$1" >/dev/null 2>&1; then
+        fail "iptables chain '$1' was left behind after lxc-exec completed."
+    fi
+    if ip6tables -S "$1" >/dev/null 2>&1; then
+        fail "ip6tables chain '$1' was left behind after lxc-exec completed."
+    fi
+}
+
+assert_no_forward_reference() {
+    if iptables -S FORWARD 2>/dev/null | grep -Fq -- "$1"; then
+        fail "a FORWARD rule still references chain '$1' after teardown."
+    fi
+}
+
+echo "Running LXC deny-precedence enforcement test..."
+
+echo "--- control: destination allowed, nothing blocked ---"
+CONTROL_OUTPUT=$("$LXC_EXEC" --debug "$CONTROL_CONFIG" 2>&1 || true)
+echo "$CONTROL_OUTPUT"
+
+if ! echo "$CONTROL_OUTPUT" | grep -Fq "MXC_NET_ALLOWED"; then
+    fail "the control destination was unreachable with an allow-everything policy, so this host cannot distinguish a deny-precedence failure from a broken network."
+fi
+
+assert_no_forward_reference "$CONTROL_CHAIN"
+assert_firewall_chain_cleaned_up "$CONTROL_CHAIN"
+
+echo "--- overlap: same destination in both allowedHosts and blockedHosts ---"
+OVERLAP_OUTPUT=$("$LXC_EXEC" --debug "$OVERLAP_CONFIG" 2>&1 || true)
+echo "$OVERLAP_OUTPUT"
+
+if echo "$OVERLAP_OUTPUT" | grep -Fq "MXC_NET_ALLOWED"; then
+    fail "a destination present in BOTH allowedHosts and blockedHosts was reachable. Allow rules are winning over deny rules, so a blocklist entry can be silently defeated by an overlapping allowlist entry."
+fi
+if ! echo "$OVERLAP_OUTPUT" | grep -Fq "MXC_NET_BLOCKED"; then
+    fail "the overlap case produced no verdict at all; the container command did not run."
+fi
+
+assert_no_forward_reference "$OVERLAP_CHAIN"
+assert_firewall_chain_cleaned_up "$OVERLAP_CHAIN"
+
+echo "PASS: a destination in both lists was blocked, and the same destination was reachable when only allowed."
+echo "LXC deny-precedence enforcement test complete."