From 6117179b1b82f7717727b7dd1eecd34ab17e6729 Mon Sep 17 00:00:00 2001 From: Gudge Date: Tue, 6 Oct 2026 14:15:06 -0700 Subject: [PATCH] Retire pre-v0.9 LXC network paths This PR removes unreachable LXC host-list and proxy routing and lowers directional CIDRs through typed rules. Reused containers check for stale proxy pins inside the container before launching workloads. Details * Reject unsupported network policy before container creation. * Remove old host routing while retaining in-container pin cleanup. * Document the supported directional-only LXC posture. Tests * cargo fmt --all -- --check and cargo test --workspace --quiet: passed. * cargo check --workspace --all-targets --all-features --quiet: passed. * cargo clippy --workspace --all-targets --all-features --quiet -- -D warnings: passed. * LXC units: 284 passed on Windows, 303 passed on Linux. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7570a662-a429-46d2-aa24-47fae6655cc4 Generated-with: gpt-6-sol --- docs/backends/lxc/lxc-backend.md | 28 +- .../src/backends/lxc/common/lxc_bindings.rs | 34 +- .../src/backends/lxc/common/lxc_runner.rs | 817 +++----- .../backends/lxc/common/network_ingress.rs | 422 +--- .../network_ingress_permissive_spec_tests.rs | 169 -- .../backends/lxc/common/network_iptables.rs | 1696 +++-------------- .../network_iptables_deny_precedence_spec.rs | 563 ------ .../common/network_iptables_ga_egress_spec.rs | 95 +- .../lxc/common/network_iptables_proxy_spec.rs | 535 ------ .../src/backends/lxc/common/signal_cleanup.rs | 8 +- 10 files changed, 667 insertions(+), 3700 deletions(-) delete mode 100644 src/mxc-sdk/src/backends/lxc/common/network_ingress_permissive_spec_tests.rs delete mode 100644 src/mxc-sdk/src/backends/lxc/common/network_iptables_deny_precedence_spec.rs delete mode 100644 src/mxc-sdk/src/backends/lxc/common/network_iptables_proxy_spec.rs diff --git a/docs/backends/lxc/lxc-backend.md b/docs/backends/lxc/lxc-backend.md index f7d6cd7f6..d3ff17f04 100644 --- a/docs/backends/lxc/lxc-backend.md +++ b/docs/backends/lxc/lxc-backend.md @@ -4,9 +4,10 @@ The LXC backend provides Linux container isolation using [LXC (Linux Containers)](https://linuxcontainers.org/lxc/). -For exact `0.9.0-alpha`, networking is directional-only: use `network.egress` -and `network.ingress`. LXC rejects `runtimeConfig.networkProxy`, so a v0.9 LXC -request has no proxy surface at all — see [Proxy](#proxy) below. +For every registered contract from `0.9.0-alpha` onward, networking is +directional-only: use `network.egress` and `network.ingress`. LXC rejects +`runtimeConfig.networkProxy`, so a supported LXC request has no proxy surface +at all — see [Proxy](#proxy) below. Legacy host lists and enforcement-mode fields in older examples are not accepted. Do not relabel an old request as v0.9 without migrating its policy. See [the schema migration reference](../../schema.md). @@ -143,9 +144,6 @@ in `process.env` replaces it. > configuration, not just project input. Pass `"HOME=…"` to point elsewhere > when the workspace is untrusted. -Below 0.9 only `process.env` is passed through and `inheritDefaultEnv` is -rejected. - Shells like bash also have a fallback `PATH`, so a truly empty environment is not reachable through `process.env`. @@ -173,9 +171,15 @@ The legacy `network.proxy` fields also have no supported exact contract. Choose a backend that supports a loopback proxy if the workload needs one; relabeling a retired LXC proxy request as v0.9 cannot make its policy enforceable. +The retired host-list, enforcement-mode, local-network, and `network.proxy` +fields are no longer representable in typed Rust requests. LXC still rejects +the supported `runtimeConfig.networkProxy` field before creating a container. +Omitting `network.egress` or `network.ingress` means default deny for that +direction; retired fields never supply a fallback. + ### No network at all -A request that permits nothing and names no proxy keeps its own loopback and reaches nothing else. +A request that permits nothing keeps its own loopback and reaches nothing else. ## Usage @@ -247,7 +251,11 @@ LXC reads a run's network section only when the container starts. Omit `containerId` for a generated name. The claim is released when the handle drops. **Teardown is owed on every terminal path,** including a drop without `wait`: -the proxy pin and the network chains come down, and the container is released. +the network chains come down, and the container is released. A reused container +also has any stale proxy hosts-file pin from an earlier run cleared before its +new workload launches. MXC checks for marked lines inside the running +container using `lxc-attach` and removes them if present. A failed check +refuses to run the workload. A teardown failure is reported through `Sandbox::warnings`, which a bare drop leaves no handle to read. @@ -336,8 +344,8 @@ The zone query should answer the zone you assigned. and anything it starts run unfiltered in both directions for that interval. The requested command is attached afterwards. A connection opened during that window keeps working once the rules land, because the chains accept established flows. -- **A filtered container cannot renew a DHCP lease.** The chains permit loopback, - established flows, and DNS, with no carve-out for DHCP. A container that +- **A filtered container cannot renew a DHCP lease.** The chains permit loopback + and established flows, with no implicit carve-out for DHCP or DNS. A container that outlives its lease loses its address; one that finishes within the lease period is unaffected. - **A container that needs a network waits for an IPv4 address.** A dual-stack diff --git a/src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs b/src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs index ef4aba6aa..93ff101a4 100644 --- a/src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs +++ b/src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs @@ -1429,13 +1429,11 @@ mod tests { } #[test] - fn proxy_disabled_keeps_caller_proxy_env_and_still_clears_inherited_env() { - use crate::mxc_common::{models::ProxyConfig, proxy_env::apply_proxy_env}; - let mut env = vec![ + fn caller_proxy_env_is_preserved_and_host_env_is_cleared() { + let env = vec![ "HTTP_PROXY=http://caller-proxy.example:9999".to_string(), "PATH=/usr/bin".to_string(), ]; - apply_proxy_env(&mut env, &ProxyConfig::default()); let args = build_attach_args_with_env_control(&env, "", "cmd", true); assert!( args.iter().any(|a| a == "--clear-env"), @@ -1452,34 +1450,6 @@ mod tests { ); } - #[test] - fn proxy_enabled_emits_clear_env_and_proxy_keys_in_attach_args() { - use crate::mxc_common::{ - models::{ProxyAddress, ProxyConfig}, - proxy_env::apply_proxy_env, - }; - let proxy = ProxyConfig { - address: Some(ProxyAddress::new("10.0.0.5".to_string(), 3128)), - builtin_test_server: false, - }; - let mut env = vec!["PATH=/usr/bin".to_string()]; - apply_proxy_env(&mut env, &proxy); - let args = build_attach_args_with_env_control(&env, "", "cmd", true); - assert!( - args.iter().any(|a| a == "--clear-env"), - "proxy enabled must emit --clear-env; got {args:?}" - ); - assert!( - args.iter() - .any(|a| a.starts_with("--set-var=HTTP_PROXY=http://") && a.contains(":3128")), - "proxy enabled must set HTTP_PROXY (with port 3128); got {args:?}" - ); - assert!( - args.iter().any(|a| a == "--set-var=PATH=/usr/bin"), - "PATH must survive the proxy-env merge; got {args:?}" - ); - } - // The attach paths drop this capability only when chains are installed: the // drop is privileged, so applying it to every run costs an unprivileged // caller the whole execution. diff --git a/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs b/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs index 556404352..132e8e6c2 100644 --- a/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs +++ b/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs @@ -11,7 +11,7 @@ use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ ContainerPolicy, ExecutionRequest, LifecycleConfig, LxcConfig, NetworkEnforcementMode, - ScriptResponse, + NetworkPolicy, ScriptResponse, }; use crate::mxc_common::sandbox_process::{SandboxBackend, SandboxProcess, StdioMode}; use crate::mxc_common::script_runner::ScriptRunner; @@ -34,7 +34,7 @@ use crate::lxc_common::filesystem_mounts; use crate::lxc_common::lxc_bindings::{ContainerFirewall, LxcContainer, StartNetwork}; use crate::lxc_common::network_ingress::IngressManager; use crate::lxc_common::network_iptables::{ - needs_network, plan_network, uses_directional_keys, EgressHookPoint, NetworkIptablesManager, + needs_network, plan_network, EgressHookPoint, NetworkIptablesManager, }; use crate::lxc_common::signal_cleanup; @@ -306,8 +306,7 @@ impl LxcScriptRunner { /// reporting whatever it could not release. #[cfg_attr(not(target_os = "linux"), allow(dead_code))] fn abandon_prepared(&self, prepared: &mut PreparedSandbox, logger: &mut Logger) -> Vec { - // The pin and the chains are both reached through the container, so - // they come out before it does. + // Namespace-scoped chains come out before the container does. let mut failures = prepared.remove_in_container_state(self.cleanup_policy, logger); let release = self.release_kind(prepared.container_created); @@ -333,7 +332,7 @@ impl LxcScriptRunner { failures } - fn set_up_network_rules_for_v07( + fn set_up_network_rules( &self, container_name: &str, hook_point: EgressHookPoint, @@ -342,36 +341,13 @@ impl LxcScriptRunner { logger: &mut Logger, ) -> Result<(NetworkIptablesManager, Option), String> { let mut fw_manager = NetworkIptablesManager::new(container_name, hook_point); - Self::egress_apply_outcome(fw_manager.apply_legacy_rules(policy, logger))?; + Self::egress_apply_outcome(fw_manager.apply_firewall_rules(policy, logger))?; fw_manager.set_preserve_policy(!self.cleanup_policy); let mut ingress_manager = None; if let Some(pid) = netns_pid { let mut mgr = IngressManager::new(container_name, pid); - Self::ingress_apply_outcome(mgr.apply_legacy_rules(policy, logger))?; - mgr.set_preserve_policy(!self.cleanup_policy); - ingress_manager = Some(mgr); - } - - Ok((fw_manager, ingress_manager)) - } - - fn set_up_network_rules_for_v08( - &self, - container_name: &str, - hook_point: EgressHookPoint, - netns_pid: Option, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result<(NetworkIptablesManager, Option), String> { - let mut fw_manager = NetworkIptablesManager::new(container_name, hook_point); - Self::egress_apply_outcome(fw_manager.apply_directional_rules(policy, logger))?; - fw_manager.set_preserve_policy(!self.cleanup_policy); - - let mut ingress_manager = None; - if let Some(pid) = netns_pid { - let mut mgr = IngressManager::new(container_name, pid); - Self::ingress_apply_outcome(mgr.apply_directional_rules(policy, logger))?; + Self::ingress_apply_outcome(mgr.apply_firewall_rules(policy, logger))?; mgr.set_preserve_policy(!self.cleanup_policy); ingress_manager = Some(mgr); } @@ -456,6 +432,7 @@ impl LxcScriptRunner { request: &ExecutionRequest, logger: &mut Logger, ) -> Result { + self.validate_runner(request)?; let normalized; let request = match crate::mxc_common::filesystem_object::normalize_object_conflicts( &request.policy, @@ -485,35 +462,9 @@ impl LxcScriptRunner { let name_claim = self.claim_container_name()?; let container_name = name_claim.name().to_owned(); - // A process's argv is world-readable through /proc//cmdline. - // Refuse credential-bearing proxy URLs before they become lxc-attach - // `--set-var` arguments. - if let Some(url) = request - .policy - .network_proxy - .address - .as_ref() - .map(|address| address.to_url()) - { - if crate::mxc_common::proxy_env::proxy_url_has_credentials(&url) { - return Err(ScriptResponse::error(&format!( - "LXC: network.proxy.url must not carry credentials ('{}'). LXC passes the \ - proxy URL to lxc-attach as a --set-var command-line argument, and process \ - arguments are world-readable through /proc//cmdline, so the password \ - would be visible to every local user while the command runs. Use a proxy \ - that does not require inline credentials, or supply them to the proxy \ - itself rather than through the URL.", - crate::mxc_common::proxy_env::redact_proxy_url(&url) - ))); - } - } - // The policy lowers to the same rules with or without a container, so // one that cannot be programmed is refused before a container exists. - if let Err(msg) = NetworkIptablesManager::validate_egress_lowering( - &request.policy, - uses_directional_keys(&request.policy), - ) { + if let Err(msg) = NetworkIptablesManager::validate_egress_lowering(&request.policy) { return Err(ScriptResponse::error(&msg)); } @@ -650,23 +601,13 @@ impl LxcScriptRunner { } } - let setup = if uses_directional_keys(&request.policy) { - self.set_up_network_rules_for_v08( - &container_name, - hook_point, - netns_pid, - &request.policy, - logger, - ) - } else { - self.set_up_network_rules_for_v07( - &container_name, - hook_point, - netns_pid, - &request.policy, - logger, - ) - }; + let setup = self.set_up_network_rules( + &container_name, + hook_point, + netns_pid, + &request.policy, + logger, + ); let (fw_manager, ingress_manager) = match setup { Ok(managers) => managers, @@ -676,7 +617,6 @@ impl LxcScriptRunner { } }; - let mut pinned = false; let firewall = container_firewall( fw_manager.rules_applied(), ingress_manager @@ -684,67 +624,27 @@ impl LxcScriptRunner { .is_some_and(|mgr| mgr.rules_applied()), ); - // A proxied chain opens no port 53; without this pin the container has - // no resolver to reach its proxy. - if let Some(pin) = fw_manager.proxy_host_pin() { - let command = Self::build_hosts_pin_command(&pin.hosts_line()); - let _ = writeln!( - logger, - "Pinning proxy host {} to {} in the container's /etc/hosts.", - pin.hostname(), - pin.ip() - ); - let pin_outcome = container.attach_capture( - &command, - "/", - &[], - true, - Some(HOSTS_COMMAND_TIMEOUT), - firewall, - ); - let pin_error = match pin_outcome { - Ok((0, _, _)) => None, - - Ok((code, _, stderr)) => { - Some(Self::hosts_command_failure("writing", code, &stderr)) - } - Err(e) => Some(e.to_string()), - }; - if let Some(reason) = pin_error { - self.release_after_failure(&container, container_created, logger); - return Err(ScriptResponse::error(&format!( - "Failed to pin the network proxy host inside the container: {}. \ - The proxy would be unreachable, so the script was not run.", - reason - ))); - } - pinned = true; - } else if !container_created { - let clear_stale_pin_command = Self::build_hosts_unpin_command(); - let stale_pin_error = match container.attach_capture( - &clear_stale_pin_command, - "/", - &[], - true, - Some(HOSTS_COMMAND_TIMEOUT), - firewall, - ) { - Ok((0, _, _)) => None, - Ok((code, _, stderr)) => { - Some(Self::hosts_command_failure("clearing", code, &stderr)) - } - Err(e) => Some(e.to_string()), - }; - - if let Some(reason) = stale_pin_error { - self.release_after_failure(&container, container_created, logger); - return Err(ScriptResponse::error(&format!( - "Failed to clear a stale network proxy pin from the container's \ - /etc/hosts: {}. The script was not run, because it could have resolved \ - the pinned hostname to an address this policy did not authorize.", - reason - ))); - } + // A reused container can carry a pin left by an older run. + let stale_pin_error = Self::clear_stale_hosts_pin(container_created, |command| { + container + .attach_capture( + command, + "/", + &[], + true, + Some(HOSTS_COMMAND_TIMEOUT), + firewall, + ) + .map_err(|e| e.to_string()) + }); + if let Err(reason) = stale_pin_error { + self.release_after_failure(&container, container_created, logger); + return Err(ScriptResponse::error(&format!( + "Failed to clear a stale network proxy pin from the container's \ + /etc/hosts: {}. The script was not run, because it could have resolved \ + the pinned hostname to an address this policy did not authorize.", + reason + ))); } // `script_timeout == 0` means "no timeout" per the SDK contract. @@ -753,8 +653,7 @@ impl LxcScriptRunner { } else { Some(Duration::from_millis(u64::from(request.script_timeout))) }; - let mut exec_env = resolved_env(request); - crate::mxc_common::proxy_env::apply_proxy_env(&mut exec_env, &request.policy.network_proxy); + let exec_env = resolved_env(request); Ok(PreparedSandbox { fw_manager, @@ -762,7 +661,6 @@ impl LxcScriptRunner { container, container_created, firewall, - pinned, released: false, force_stopped: false, stop_failed: false, @@ -809,25 +707,10 @@ impl LxcScriptRunner { response } - fn build_hosts_pin_command(hosts_line: &str) -> String { - // LXC may bind-mount `/etc/hosts`; rewrite it in place. BusyBox - // images have `grep` and `printf`, and kept lines stay in a shell - // variable to avoid following a predictable scratch-file symlink. - // Shell command substitution strips trailing newlines; `printf` writes - // one back only when content survived the filter. - format!( - "{}{{ if [ -n \"$kept\" ]; then printf '%s\\n' \"$kept\"; fi; \ - printf '%s {marker}\\n' '{hosts_line}'; }} > /etc/hosts", - Self::hosts_read_prologue(), - marker = HOSTS_PIN_MARKER, - hosts_line = hosts_line - ) - } - fn hosts_read_prologue() -> String { - // Read `/etc/hosts` before the redirect opens and truncates it. `grep` - // status 1 means no line was selected, missing `grep` reports 127, and - // `grep` reports absent and unreadable files as status 2. + // Check for a marker before opening the redirect. A marker-free file + // must not be rewritten. `grep` status 1 means no match, while a + // missing tool or unreadable file must fail before launching the script. format!( "if [ -h /etc/hosts ]; then \ printf 'mxc: refusing to rewrite /etc/hosts: it is a symbolic link\\n' >&2; \ @@ -835,6 +718,16 @@ impl LxcScriptRunner { fi; \ kept=''; \ if [ -e /etc/hosts ]; then \ + if grep -q '{marker}' /etc/hosts 2>/dev/null; then :; \ + else \ + status=$?; \ + if [ \"$status\" -gt 1 ]; then \ + printf 'mxc: refusing to rewrite /etc/hosts: reading it exited %s\\n' \ + \"$status\" >&2; \ + exit \"$status\"; \ + fi; \ + exit 0; \ + fi; \ kept=$(grep -v '{marker}' /etc/hosts 2>/dev/null); \ status=$?; \ if [ \"$status\" -gt 1 ]; then \ @@ -842,6 +735,7 @@ impl LxcScriptRunner { \"$status\" >&2; \ exit \"$status\"; \ fi; \ + else exit 0; \ fi; ", marker = HOSTS_PIN_MARKER ) @@ -854,12 +748,26 @@ impl LxcScriptRunner { ) } - fn hosts_command_failure(verb: &str, exit_code: i32, stderr: &str) -> String { + fn clear_stale_hosts_pin( + container_created: bool, + attach: impl FnOnce(&str) -> Result<(i32, String, String), String>, + ) -> Result<(), String> { + if container_created { + return Ok(()); + } + let (code, _, stderr) = attach(&Self::build_hosts_unpin_command())?; + if code != 0 { + return Err(Self::hosts_command_failure(code, &stderr)); + } + Ok(()) + } + + fn hosts_command_failure(exit_code: i32, stderr: &str) -> String { let detail = stderr.trim(); if detail.is_empty() { - return format!("{} /etc/hosts exited with {}", verb, exit_code); + return format!("clearing /etc/hosts exited with {}", exit_code); } - format!("{} /etc/hosts exited with {}: {}", verb, exit_code, detail) + format!("clearing /etc/hosts exited with {}: {}", exit_code, detail) } } @@ -874,7 +782,6 @@ struct PreparedSandbox { container_created: bool, firewall: ContainerFirewall, - pinned: bool, released: bool, #[cfg_attr(not(target_os = "linux"), allow(dead_code))] @@ -894,9 +801,7 @@ struct PreparedSandbox { } impl PreparedSandbox { - /// Remove the state that can only come out while the container is up: the - /// pin is rewritten by a command inside it, and the chains are addressed - /// through its init PID. + /// Remove chains addressed through the container's init PID while it is up. fn remove_in_container_state( &mut self, cleanup_policy: bool, @@ -925,33 +830,6 @@ impl PreparedSandbox { } } - if self.pinned && cleanup_policy { - let clear_run_pin_command = LxcScriptRunner::build_hosts_unpin_command(); - let unpin_error = match self.container.attach_capture( - &clear_run_pin_command, - "/", - &[], - true, - Some(HOSTS_COMMAND_TIMEOUT), - self.firewall, - ) { - Ok((0, _, _)) => None, - Ok((code, _, stderr)) => Some(LxcScriptRunner::hosts_command_failure( - "clearing", code, &stderr, - )), - Err(e) => Some(e.to_string()), - }; - - match unpin_error { - Some(reason) => { - let failure = format!("failed to clear the proxy host pin: {}", reason); - let _ = writeln!(logger, "Warning: {}", failure); - failures.push(failure); - } - None => self.pinned = false, - } - } - // These chains are addressed by the container's init PID, so they have // to come out before the container does. if self.fw_manager.rules_applied() && cleanup_policy { @@ -982,7 +860,7 @@ impl PreparedSandbox { failures } - /// Run the completion-path release — pin, then rules, then container — + /// Run the completion-path release — rules, then container — /// repeating only the steps an earlier call failed to complete. fn tear_down( &mut self, @@ -992,10 +870,8 @@ impl PreparedSandbox { ) -> Vec { let mut failures = Vec::new(); - // Destroying takes the network namespace and the rootfs down with it, - // so the chains in one and the pin in the other are already going. - // Chains that were never hooked live on the host and still have to - // come out below. + // Destroying takes the network namespace down with it. Chains that + // were never hooked live on the host and still have to come out below. let destroying = destroy_on_exit && !self.released; let skipped_in_container = destroying && self.fw_manager.is_hooked(); if !skipped_in_container { @@ -1056,11 +932,6 @@ impl PreparedSandbox { /// Give up the state that needed the container, now that it is gone. fn note_namespace_gone(&mut self) { - // Rewriting the pin takes a command inside a running container, and - // there is no longer one; a container kept for reuse has the stale pin - // cleared by the next run. - self.pinned = false; - // These chains went down with the network namespace, and the init PID // that addressed them can be recycled into another namespace, where the // same chain name may belong to a later run. @@ -1073,12 +944,6 @@ impl PreparedSandbox { } } -pub const LXC_CAPABILITIES_MODE_UNSUPPORTED: &str = - "LXC: network.enforcementMode='capabilities' (the default) selects Windows AppContainer \ - capability SIDs, which LXC has no mechanism for. Accepting it would enforce the policy by \ - some means other than the one named. Use the supported network.egress / network.ingress \ - fields instead; no registered LXC contract accepts network.enforcementMode."; - pub const LXC_RUNTIME_PROXY_UNSUPPORTED: &str = "LXC: runtimeConfig.networkProxy is not supported. It must name a loopback endpoint, which \ inside the container's own network namespace is the container rather than the host. LXC \ @@ -1094,13 +959,22 @@ pub const LXC_INHERIT_STDIO_UNSUPPORTED: &str = pub const LXC_STREAMING_LINUX_ONLY: &str = "LXC: sandboxes can only be launched on Linux."; -fn asks_for_capabilities_enforcement(request: &ExecutionRequest) -> bool { - !uses_directional_keys(&request.policy) - && request.policy.network_mode_specified - && matches!( - request.policy.network_enforcement_mode, - NetworkEnforcementMode::Capabilities - ) +fn retired_network_field(policy: &ContainerPolicy) -> Option<&'static str> { + if policy.default_network_policy != NetworkPolicy::Block { + Some("network.defaultPolicy") + } else if policy.network_enforcement_mode != NetworkEnforcementMode::Capabilities { + Some("network.enforcementMode") + } else if policy.allow_local_network { + Some("network.allowLocalNetwork") + } else if !policy.allowed_hosts.is_empty() { + Some("network.allowedHosts") + } else if !policy.blocked_hosts.is_empty() { + Some("network.blockedHosts") + } else if policy.network_proxy.is_enabled() { + Some("network.proxy") + } else { + None + } } fn lxc_network_policy_support() -> NetworkPolicySupport { @@ -1115,10 +989,13 @@ impl ScriptRunner for LxcScriptRunner { if request.policy.runtime_network_proxy_specified { return Err(ScriptResponse::error(LXC_RUNTIME_PROXY_UNSUPPORTED)); } - validate_network_policy_support(request, lxc_network_policy_support())?; - if asks_for_capabilities_enforcement(request) { - return Err(ScriptResponse::error(LXC_CAPABILITIES_MODE_UNSUPPORTED)); + if let Some(field) = retired_network_field(&request.policy) { + return Err(ScriptResponse::error(&format!( + "LXC: {field} is retired and cannot be enforced; use network.egress / \ + network.ingress instead." + ))); } + validate_network_policy_support(request, lxc_network_policy_support())?; Ok(()) } @@ -1717,6 +1594,22 @@ mod tests { } } + fn egress_policy_for_tests() -> ContainerPolicy { + ContainerPolicy { + network_egress: Some(NetworkEgressPolicy { + allow: vec![crate::mxc_common::models::NetworkRule { + to: vec![crate::mxc_common::models::NetworkPeer { + cidr: "192.0.2.10/32".parse().expect("test CIDR"), + except: Vec::new(), + }], + ..Default::default() + }], + ..Default::default() + }), + ..Default::default() + } + } + #[test] fn a_failed_egress_apply_tears_down_its_debris_even_under_preserve_policy() { let fake = crate::lxc_common::network_iptables::test_firewall::install(); @@ -1743,14 +1636,10 @@ mod tests { ..LifecycleConfig::default() }, ); - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - allowed_hosts: vec!["192.0.2.10".to_string()], - ..Default::default() - }; + let policy = egress_policy_for_tests(); let mut logger = Logger::new(Mode::Buffer); - let outcome = runner.set_up_network_rules_for_v07( + let outcome = runner.set_up_network_rules( "preserve-rollback-test", EgressHookPoint::Unhooked, None, @@ -1805,10 +1694,8 @@ mod tests { ); } - /// A password in the URL would reach `lxc-attach` as an argument, and - /// process arguments are world-readable through `/proc//cmdline`. #[test] - fn prepare_refuses_a_proxy_url_carrying_credentials() { + fn prepare_refuses_a_retired_proxy_url_carrying_credentials() { let runner = LxcScriptRunner::new( &LxcConfig { distribution: "alpine".to_string(), @@ -1833,11 +1720,11 @@ mod tests { let refusal = runner .prepare(&request, &mut logger) .err() - .expect("a proxy URL carrying a password must not become an lxc-attach argument"); + .expect("retired proxy URLs must not reach lxc-attach"); assert!( - refusal.error_message.contains("must not carry credentials"), - "the refusal must name the cause; got: {:?}", + refusal.error_message.contains("network.proxy"), + "the refusal must name the retired field; got: {:?}", refusal.error_message ); assert!( @@ -1876,7 +1763,6 @@ mod tests { fn tear_down_repeats_only_the_steps_that_failed() { let _fake = crate::lxc_common::network_iptables::test_firewall::install(); let mut prepared = prepared_with_applied_rules("tear-down-idempotent"); - prepared.pinned = true; let mut first = Logger::new(Mode::Buffer); prepared.tear_down(true, true, &mut first); @@ -1895,15 +1781,8 @@ mod tests { second.get_buffer() ); - // The fixture names a container that was never created, so the unpin and - // the destroy both fail here and both must be attempted again. - assert!( - first.get_buffer().contains("proxy host pin") - && second.get_buffer().contains("proxy host pin"), - "an unpin that failed must be retried; logs: {:?} / {:?}", - first.get_buffer(), - second.get_buffer() - ); + // The fixture names a container that was never created, so destroying + // it fails on both attempts. assert!( second.get_buffer().contains("Destroying container"), "a destroy that failed must be retried; log: {:?}", @@ -1916,7 +1795,6 @@ mod tests { let fake = crate::lxc_common::network_iptables::test_firewall::install(); let mut prepared = prepared_with_applied_rules("stop-refused"); apply_hooked_egress_rules(&mut prepared, 4242); - prepared.pinned = true; // The fixture names a container that was never created, so `lxc-stop` // has nothing to stop and reports the failure this test needs. @@ -1950,11 +1828,7 @@ mod tests { /// manager drives `nsenter` through a runner the iptables fake does not /// intercept, so its own module owns that coverage. fn apply_hooked_egress_rules(prepared: &mut PreparedSandbox, netns_pid: u32) { - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - allowed_hosts: vec!["192.0.2.10".to_string()], - ..Default::default() - }; + let policy = egress_policy_for_tests(); let mut logger = Logger::new(Mode::Buffer); prepared.fw_manager = NetworkIptablesManager::new( @@ -1963,7 +1837,7 @@ mod tests { ); prepared .fw_manager - .apply_legacy_rules(&policy, &mut logger) + .apply_firewall_rules(&policy, &mut logger) .expect("the fake firewall accepts every command"); } @@ -1972,7 +1846,6 @@ mod tests { let _fake = crate::lxc_common::network_iptables::test_firewall::install(); let mut prepared = prepared_with_applied_rules("namespace-gone"); apply_hooked_egress_rules(&mut prepared, 4242); - prepared.pinned = true; assert!( prepared.fw_manager.rules_applied(), @@ -1985,10 +1858,6 @@ mod tests { !prepared.fw_manager.rules_applied(), "the netns took the chain with it, so its dead init PID must never be nsentered" ); - assert!( - !prepared.pinned, - "the pin cannot be rewritten without a running container" - ); } #[test] @@ -2170,21 +2039,11 @@ mod tests { let name = format!("mxc-{}-{}-{}", tag, std::process::id(), uuid_simple()); let runner = LxcScriptRunner::new(&LxcConfig::default(), &name, &LifecycleConfig::default()); - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - allowed_hosts: vec!["192.0.2.10".to_string()], - ..Default::default() - }; + let policy = egress_policy_for_tests(); let mut logger = Logger::new(Mode::Buffer); let (fw_manager, ingress_manager) = runner - .set_up_network_rules_for_v07( - &name, - EgressHookPoint::Unhooked, - None, - &policy, - &mut logger, - ) + .set_up_network_rules(&name, EgressHookPoint::Unhooked, None, &policy, &mut logger) .expect("the fake firewall accepts every command"); assert!( fw_manager.rules_applied(), @@ -2197,7 +2056,6 @@ mod tests { container: LxcContainer::new(&name, Some("/var/lib/lxc")), container_created: false, firewall: ContainerFirewall::Absent, - pinned: false, released: false, force_stopped: false, stop_failed: false, @@ -2211,29 +2069,55 @@ mod tests { } #[test] - fn a_07_network_section_left_on_capabilities_is_refused() { + fn a_retired_enforcement_mode_is_refused() { let policy = ContainerPolicy { - network_mode_specified: true, + network_enforcement_mode: NetworkEnforcementMode::Firewall, ..Default::default() }; let refusal = validating_runner() .validate_runner(&request_with_policy(policy)) - .expect_err("LXC has no capability-SID mechanism to enforce through"); - - assert_eq!(refusal.error_message, LXC_CAPABILITIES_MODE_UNSUPPORTED); - } - #[test] - fn a_07_network_section_naming_the_firewall_is_accepted() { - let policy = ContainerPolicy { - network_mode_specified: true, - network_enforcement_mode: NetworkEnforcementMode::Firewall, - ..Default::default() - }; + .expect_err("a retired enforcement mode cannot configure LXC"); + + assert!(refusal.error_message.contains("network.enforcementMode")); + } + #[test] + fn retired_fields_are_refused_before_container_creation() { + type RetiredFieldCase = (&'static str, fn(&mut ContainerPolicy)); + let cases: &[RetiredFieldCase] = &[ + ("network.defaultPolicy", |p| { + p.default_network_policy = NetworkPolicy::Allow + }), + ("network.enforcementMode", |p| { + p.network_enforcement_mode = NetworkEnforcementMode::Both + }), + ("network.allowLocalNetwork", |p| { + p.allow_local_network = true + }), + ("network.allowedHosts", |p| { + p.allowed_hosts.push("192.0.2.10".into()) + }), + ("network.blockedHosts", |p| { + p.blocked_hosts.push("192.0.2.10".into()) + }), + ("network.proxy", |p| { + p.network_proxy.builtin_test_server = true + }), + ]; - assert!(validating_runner() - .validate_runner(&request_with_policy(policy)) - .is_ok()); + for &(field, set_field) in cases { + let mut policy = egress_policy_for_tests(); + set_field(&mut policy); + let runner = runner_for_guard_tests(field); + let mut logger = Logger::new(Mode::Buffer); + let response = runner.run_internal(&request_with_policy(policy), &mut logger); + assert!( + response.error_message.contains(field), + "{field} must fail before creating a container: {}", + response.error_message + ); + assert!(!logger.get_buffer().contains("Creating LXC container")); + } } #[test] @@ -2244,7 +2128,7 @@ mod tests { } #[test] - fn a_08_directional_network_section_is_accepted() { + fn a_directional_network_section_is_accepted() { let policy = ContainerPolicy { network_egress: Some(NetworkEgressPolicy::default()), ..Default::default() @@ -2623,52 +2507,14 @@ mod tests { } #[test] - fn the_hosts_pin_command_writes_the_requested_mapping() { - let command = LxcScriptRunner::build_hosts_pin_command("10.0.0.5 proxy.example.com"); - - assert!( - command.contains("'10.0.0.5 proxy.example.com'"), - "the command must carry the mapping verbatim, got: {command}" - ); - assert!( - command.contains("/etc/hosts"), - "the command must target /etc/hosts, got: {command}" - ); - } - - #[test] - fn the_hosts_pin_command_strips_its_own_previous_entries_first() { - let command = LxcScriptRunner::build_hosts_pin_command("10.0.0.5 proxy.example.com"); - - assert!( - command.contains(&format!("grep -v '{}'", HOSTS_PIN_MARKER)), - "the command must remove prior pins before writing; got: {command}" - ); + fn a_failed_hosts_command_reports_a_reason_it_can_actually_supply() { + let reason = LxcScriptRunner::hosts_command_failure(1, ""); assert!( - command.matches(HOSTS_PIN_MARKER).count() >= 2, - "the written line must carry the marker that the strip looks for; got: {command}" + reason.contains('1'), + "the reason must name the exit code: {reason}" ); - } - - #[test] - fn a_failed_hosts_command_reports_a_reason_it_can_actually_supply() { - for verb in ["writing", "clearing"] { - let reason = LxcScriptRunner::hosts_command_failure(verb, 1, ""); - - assert!( - reason.contains('1'), - "the reason must name the exit code; got: {reason}" - ); - assert!( - !reason.trim_end().ends_with(':'), - "the reason must not trail a separator promising more; got: {reason:?}" - ); - assert_eq!( - reason.trim_end(), - reason, - "the reason must not trail whitespace where a stderr used to go; got: {reason:?}" - ); - } + assert!(!reason.trim_end().ends_with(':')); + assert_eq!(reason.trim_end(), reason); } /// A symlinked `/etc/hosts` and an unreadable one differ only in what the @@ -2676,7 +2522,6 @@ mod tests { #[test] fn a_failed_hosts_command_carries_the_helpers_own_diagnosis() { let reason = LxcScriptRunner::hosts_command_failure( - "writing", 4, "mxc: refusing to rewrite /etc/hosts: it is a symbolic link\n", ); @@ -2697,14 +2542,44 @@ mod tests { } #[test] - fn a_failed_hosts_command_distinguishes_the_step_that_failed() { - let pinning = LxcScriptRunner::hosts_command_failure("writing", 2, ""); - let clearing = LxcScriptRunner::hosts_command_failure("clearing", 2, ""); + fn new_containers_do_not_attach_to_check_for_stale_hosts_pins() { + LxcScriptRunner::clear_stale_hosts_pin(true, |_| { + panic!("a newly created container has no previous MXC pin") + }) + .expect("new containers need no migration"); + } - assert_ne!( - pinning, clearing, - "the two steps must not produce the same reason; got: {pinning:?}" - ); + #[test] + fn reused_containers_attach_to_check_for_stale_hosts_pins() { + let mut attached = false; + LxcScriptRunner::clear_stale_hosts_pin(false, |command| { + attached = true; + assert!(command.contains(HOSTS_PIN_MARKER)); + Ok((0, String::new(), String::new())) + }) + .expect("a successful attach check should clear the pin"); + assert!(attached); + } + + #[test] + fn reused_containers_refuse_failed_unpins() { + let mut calls = 0; + for result in [ + Ok((4, String::new(), "symbolic link".to_string())), + Err("attach failed".to_string()), + ] { + let outcome = LxcScriptRunner::clear_stale_hosts_pin(false, |command| { + calls += 1; + assert!(command.contains(HOSTS_PIN_MARKER)); + result + }); + match calls { + 1 => assert!(outcome.unwrap_err().contains("symbolic link")), + 2 => assert_eq!(outcome, Err("attach failed".to_string())), + _ => unreachable!(), + } + } + assert_eq!(calls, 2, "failed unpins must reach the container"); } #[test] @@ -2712,14 +2587,15 @@ mod tests { let command = LxcScriptRunner::build_hosts_unpin_command(); assert!( - command.contains(&format!("grep -v '{}'", HOSTS_PIN_MARKER)), + command.contains(&format!("grep -q '{}'", HOSTS_PIN_MARKER)) + && command.contains(&format!("grep -v '{}'", HOSTS_PIN_MARKER)), "the command must filter out every marked line; got: {command}" ); assert_eq!( command.matches(HOSTS_PIN_MARKER).count(), - 1, - "the marker should appear only in the filter; got: {command}" + 2, + "the marker should appear only in the probe and filter; got: {command}" ); } @@ -2737,78 +2613,29 @@ mod tests { ); } - #[test] - fn the_hosts_pin_command_rewrites_the_file_in_place_rather_than_replacing_it() { - let command = LxcScriptRunner::build_hosts_pin_command("10.0.0.5 proxy.example.com"); - - assert!( - command.contains("> /etc/hosts"), - "the command must redirect into the existing file, got: {command}" - ); - assert!( - !command.contains("mv "), - "the command must not replace the inode, got: {command}" - ); - } - - #[test] - fn the_hosts_pin_command_uses_only_busybox_available_tools() { - let command = LxcScriptRunner::build_hosts_pin_command("10.0.0.5 proxy.example.com"); - - for forbidden in ["sed ", "awk ", "tee ", "sponge "] { - assert!( - !command.contains(forbidden), - "the command must not depend on {forbidden:?}, got: {command}" - ); - } - } - #[test] fn the_hosts_commands_stage_nothing_in_a_container_writable_directory() { - let commands = [ - LxcScriptRunner::build_hosts_pin_command("10.0.0.5 proxy.example.com"), - LxcScriptRunner::build_hosts_unpin_command(), - ]; - - for command in commands { - for scratch in ["/tmp/", "/var/tmp/", "/dev/shm/", "/run/"] { - assert!( - !command.contains(scratch), - "the command must not stage under {scratch:?}, got: {command}" - ); - } - assert_eq!( - command.matches("> /etc/hosts").count(), - 1, - "/etc/hosts must be the only redirect target; got: {command}" + let command = LxcScriptRunner::build_hosts_unpin_command(); + for scratch in ["/tmp/", "/var/tmp/", "/dev/shm/", "/run/"] { + assert!( + !command.contains(scratch), + "the command must not stage under {scratch:?}, got: {command}" ); } + assert_eq!(command.matches("> /etc/hosts").count(), 1); } #[test] fn the_hosts_commands_build_their_content_before_truncating_the_target() { - let commands = [ - LxcScriptRunner::build_hosts_pin_command("10.0.0.5 proxy.example.com"), - LxcScriptRunner::build_hosts_unpin_command(), - ]; - - for command in commands { - let capture = command.find("kept=$(").unwrap_or_else(|| { - panic!("the command must stage into a variable; got: {command}") - }); - let redirect = command - .find("> /etc/hosts") - .unwrap_or_else(|| panic!("the command must target /etc/hosts; got: {command}")); - - assert!( - capture < redirect, - "the content must be captured before /etc/hosts is truncated; got: {command}" - ); - assert!( - !command[redirect..].contains("grep"), - "no file-reading command may run after the target is truncated; got: {command}" - ); - } + let command = LxcScriptRunner::build_hosts_unpin_command(); + let capture = command + .find("kept=$(") + .unwrap_or_else(|| panic!("the command must stage into a variable; got: {command}")); + let redirect = command + .find("> /etc/hosts") + .unwrap_or_else(|| panic!("the command must target /etc/hosts; got: {command}")); + assert!(capture < redirect); + assert!(!command[redirect..].contains("grep")); } use crate::mxc_common::models::{ @@ -2906,7 +2733,7 @@ mod tests { runner .validate_runner(&egress_only_directional_request()) .is_ok(), - "a 0.8 config stating only network.egress must reach the backend; rejecting it \ + "a directional config stating only network.egress must reach the backend; rejecting it \ here would make every directional egress policy unusable on LXC" ); } @@ -2977,10 +2804,8 @@ mod tests { let response = runner.run_internal(&request, &mut logger); assert!( - response - .error_message - .contains("must not carry credentials"), - "the runner must refuse a credential-bearing proxy URL even when the parser \ + response.error_message.contains("network.proxy"), + "the runner must refuse a retired proxy URL even when the parser \ never saw the request, got: {}", response.error_message ); @@ -3011,7 +2836,7 @@ mod tests { } #[test] - fn a_credential_free_proxy_url_is_not_refused_by_the_credential_guard() { + fn a_credential_free_legacy_proxy_is_still_refused() { let runner = runner_for_guard_tests("credential-free"); let request = request_with_proxy_url("http://proxy.example.com:8080"); let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); @@ -3019,16 +2844,14 @@ mod tests { let response = runner.run_internal(&request, &mut logger); assert!( - !response - .error_message - .contains("must not carry credentials"), - "a proxy URL without userinfo must clear the credential guard, got: {}", + response.error_message.contains("network.proxy"), + "a proxy URL without userinfo is still retired, got: {}", response.error_message ); } #[test] - fn the_credential_refusal_happens_before_any_container_work() { + fn the_legacy_proxy_refusal_happens_before_any_container_work() { let runner = runner_for_guard_tests("refusal-ordering"); let request = request_with_proxy_url("http://alice:hunter2@proxy.example.com:8080"); let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); @@ -3352,8 +3175,6 @@ mod hosts_command_execution { const ORIGINAL: &str = "127.0.0.1 localhost\n::1 ip6-localhost\n10.0.0.9 build.internal\n"; - const PIN_LINE: &str = "10.0.0.5 proxy.example.com"; - struct Scratch { dir: PathBuf, } @@ -3403,6 +3224,27 @@ mod hosts_command_execution { ) } + fn path_with_failing_filter(&self) -> String { + use std::os::unix::fs::PermissionsExt; + + let bin = self.dir.join("bin"); + std::fs::create_dir_all(&bin).expect("the shim directory should be creatable"); + let grep = bin.join("grep"); + std::fs::write( + &grep, + "#!/bin/sh\nif [ \"$1\" = -q ]; then exit 0; fi\nexit 2\n", + ) + .expect("the shim should be writable"); + std::fs::set_permissions(&grep, std::fs::Permissions::from_mode(0o755)) + .expect("the shim should be executable"); + + format!( + "{}:{}", + bin.display(), + std::env::var("PATH").unwrap_or_default() + ) + } + fn path_without_grep(&self) -> String { let empty = self.dir.join("empty"); std::fs::create_dir_all(&empty).expect("the empty directory should be creatable"); @@ -3437,81 +3279,48 @@ mod hosts_command_execution { .expect("the shell should exit rather than be signalled") } - fn pin(hosts: &Path) -> String { - retarget(&LxcScriptRunner::build_hosts_pin_command(PIN_LINE), hosts) - } - fn unpin(hosts: &Path) -> String { retarget(&LxcScriptRunner::build_hosts_unpin_command(), hosts) } #[test] - fn pinning_adds_the_mapping_and_keeps_every_line_the_image_shipped() { - let scratch = Scratch::new("keeps"); - scratch.write_hosts(ORIGINAL); - - let code = run(&pin(&scratch.hosts()), None); - let after = scratch.read_hosts(); - - assert_eq!(code, 0, "pinning a readable file should succeed"); - for line in ORIGINAL.lines() { - assert!( - after.contains(line), - "the pin dropped {line:?}; file is now:\n{after}" - ); - } - assert!( - after.contains(&format!("{PIN_LINE} {HOSTS_PIN_MARKER}")), - "the pin never landed; file is now:\n{after}" - ); - } - - #[test] - fn re_pinning_replaces_the_previous_entry_instead_of_stacking_on_it() { - let scratch = Scratch::new("repin"); + fn marker_free_hosts_is_not_rewritten_on_reuse() { + let scratch = Scratch::new("nomarker"); scratch.write_hosts(ORIGINAL); - - assert_eq!(run(&pin(&scratch.hosts()), None), 0); - assert_eq!(run(&pin(&scratch.hosts()), None), 0); - let after = scratch.read_hosts(); - + let file = std::fs::File::options() + .write(true) + .open(scratch.hosts()) + .expect("fixture file should open"); + let original_time = std::time::SystemTime::UNIX_EPOCH + Duration::from_secs(10); + file.set_modified(original_time) + .expect("fixture timestamp should be settable"); + + assert_eq!(run(&unpin(&scratch.hosts()), None), 0); + assert_eq!(scratch.read_hosts(), ORIGINAL); assert_eq!( - after.matches(HOSTS_PIN_MARKER).count(), - 1, - "a second pin should replace the first, not stack; file is now:\n{after}" - ); - assert!( - after.contains("10.0.0.9 build.internal"), - "re-pinning dropped an unrelated entry; file is now:\n{after}" + std::fs::metadata(scratch.hosts()) + .expect("fixture should still exist") + .modified() + .expect("timestamp should be readable"), + original_time, + "a marker-free file must not be rewritten" ); } #[test] - fn a_failed_read_leaves_the_file_byte_for_byte_as_it_was() { - let scratch = Scratch::new("failread"); - scratch.write_hosts(ORIGINAL); - let path = scratch.path_with_failing_grep(2); - - let code = run(&pin(&scratch.hosts()), Some(&path)); - - assert_eq!( - scratch.read_hosts(), - ORIGINAL, - "a failed read truncated the file it could not read" - ); - assert_ne!( - code, 0, - "a failed read must fail the command, not report a pin it never made" - ); + fn missing_hosts_file_is_not_created_on_reuse() { + let scratch = Scratch::new("missing"); + assert_eq!(run(&unpin(&scratch.hosts()), None), 0); + assert!(!scratch.hosts().exists()); } #[test] - fn a_missing_grep_leaves_the_file_byte_for_byte_as_it_was() { + fn a_missing_grep_while_unpinning_leaves_the_file_byte_for_byte_as_it_was() { let scratch = Scratch::new("nogrep"); scratch.write_hosts(ORIGINAL); let path = scratch.path_without_grep(); - let code = run(&pin(&scratch.hosts()), Some(&path)); + let code = run(&unpin(&scratch.hosts()), Some(&path)); assert_eq!( scratch.read_hosts(), @@ -3538,45 +3347,37 @@ mod hosts_command_execution { } #[test] - fn a_file_of_nothing_but_previous_pins_is_rewritten_rather_than_refused() { - let scratch = Scratch::new("allmarked"); - scratch.write_hosts(&format!("10.0.0.4 proxy.example.com {HOSTS_PIN_MARKER}\n")); - - let code = run(&pin(&scratch.hosts()), None); - let after = scratch.read_hosts(); + fn a_failed_filter_after_a_matching_probe_does_not_truncate_hosts() { + let scratch = Scratch::new("failedfilter"); + let original = format!("{ORIGINAL}10.0.0.4 proxy.example.com {HOSTS_PIN_MARKER}\n"); + scratch.write_hosts(&original); + let path = scratch.path_with_failing_filter(); - assert_eq!( - code, 0, - "a file of only stale pins should still be pinnable" - ); - assert_eq!( - after.trim(), - format!("{PIN_LINE} {HOSTS_PIN_MARKER}"), - "the stale pin should be gone and the new one present" - ); + assert_ne!(run(&unpin(&scratch.hosts()), Some(&path)), 0); + assert_eq!(scratch.read_hosts(), original); } #[test] - fn an_image_with_no_hosts_file_is_pinned_rather_than_refused() { - let scratch = Scratch::new("nofile"); + fn unpinning_a_file_of_nothing_but_previous_pins_leaves_it_empty() { + let scratch = Scratch::new("allmarked"); + scratch.write_hosts(&format!("10.0.0.4 proxy.example.com {HOSTS_PIN_MARKER}\n")); - let code = run(&pin(&scratch.hosts()), None); + let code = run(&unpin(&scratch.hosts()), None); + let after = scratch.read_hosts(); assert_eq!( code, 0, - "a missing hosts file should be created, not refused" - ); - assert_eq!( - scratch.read_hosts().trim(), - format!("{PIN_LINE} {HOSTS_PIN_MARKER}") + "a file of only stale pins should still be clearable" ); + assert_eq!(after.trim(), "", "the stale pin should be removed"); } #[test] fn unpinning_removes_the_pin_and_keeps_everything_else() { let scratch = Scratch::new("unpin"); - scratch.write_hosts(ORIGINAL); - assert_eq!(run(&pin(&scratch.hosts()), None), 0); + scratch.write_hosts(&format!( + "{ORIGINAL}10.0.0.5 proxy.example.com {HOSTS_PIN_MARKER}\n" + )); let code = run(&unpin(&scratch.hosts()), None); let after = scratch.read_hosts(); @@ -3595,40 +3396,22 @@ mod hosts_command_execution { } #[test] - fn pinning_refuses_a_dangling_symlink_instead_of_creating_its_target() { + fn unpinning_refuses_a_dangling_symlink_instead_of_creating_its_target() { let scratch = Scratch::new("dangling"); let target = scratch.dir.join("attacker-named"); std::os::unix::fs::symlink(&target, scratch.hosts()) .expect("the scratch symlink should be creatable"); - let code = run(&pin(&scratch.hosts()), None); + let code = run(&unpin(&scratch.hosts()), None); assert_ne!(code, 0, "writing through a symlink should be refused"); assert!( !target.exists(), - "the refused pin still created {}", + "the refused unpin still created {}", target.display() ); } - #[test] - fn pinning_refuses_a_symlink_rather_than_writing_through_it() { - let scratch = Scratch::new("symlink"); - let target = scratch.dir.join("elsewhere"); - std::fs::write(&target, ORIGINAL).expect("the target should be writable"); - std::os::unix::fs::symlink(&target, scratch.hosts()) - .expect("the scratch symlink should be creatable"); - - let code = run(&pin(&scratch.hosts()), None); - - assert_ne!(code, 0, "writing through a symlink should be refused"); - assert_eq!( - std::fs::read_to_string(&target).expect("the target should still be readable"), - ORIGINAL, - "the refused pin still rewrote the symlink target" - ); - } - #[test] fn unpinning_refuses_a_symlink_rather_than_emptying_its_target() { let scratch = Scratch::new("unpinsymlink"); @@ -3656,7 +3439,7 @@ mod hosts_command_execution { let output = std::process::Command::new("/bin/sh") .arg("-c") - .arg(pin(&scratch.hosts())) + .arg(unpin(&scratch.hosts())) .output() .expect("/bin/sh should be executable"); diff --git a/src/mxc-sdk/src/backends/lxc/common/network_ingress.rs b/src/mxc-sdk/src/backends/lxc/common/network_ingress.rs index 4905fd9df..23da80707 100644 --- a/src/mxc-sdk/src/backends/lxc/common/network_ingress.rs +++ b/src/mxc-sdk/src/backends/lxc/common/network_ingress.rs @@ -8,11 +8,10 @@ use std::process::Command; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ContainerPolicy, NetworkAction, NetworkIngressPolicy}; +use crate::mxc_common::models::{ContainerPolicy, NetworkAction}; use crate::lxc_common::network_iptables::{ - ingress_chain_name_for, plan_network, uses_directional_keys, HostIpv6State, Ip6tablesStatus, - NetworkIptablesManager, + ingress_chain_name_for, plan_network, HostIpv6State, Ip6tablesStatus, NetworkIptablesManager, }; #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -302,22 +301,8 @@ impl IngressManager { } } - fn stated_ingress( - policy: &ContainerPolicy, - uses_directional_keys: bool, - ) -> Option<&NetworkIngressPolicy> { - if uses_directional_keys { - policy.network_ingress.as_ref() - } else { - None - } - } - - fn permissive_inbound_field( - policy: &ContainerPolicy, - uses_directional_keys: bool, - ) -> Option<&'static str> { - if let Some(ingress) = Self::stated_ingress(policy, uses_directional_keys) { + fn permissive_inbound_field(policy: &ContainerPolicy) -> Option<&'static str> { + if let Some(ingress) = policy.network_ingress.as_ref() { if ingress.default == NetworkAction::Allow { return Some("network.ingress.default"); } @@ -327,13 +312,12 @@ impl IngressManager { return None; } - policy.allow_local_network.then_some("allowLocalNetwork") + None } - fn apply_rules_in_dialect( + pub fn apply_firewall_rules( &mut self, policy: &ContainerPolicy, - uses_directional_keys: bool, logger: &mut Logger, ) -> Result { if !plan_network(policy).installs_firewall() { @@ -341,7 +325,7 @@ impl IngressManager { return Ok(true); } - if let Some(field) = Self::permissive_inbound_field(policy, uses_directional_keys) { + if let Some(field) = Self::permissive_inbound_field(policy) { return Err(format!( "{field} asks for permissive inbound, which is not yet implemented for \ the LXC firewall path. LXC has a single inbound chain and the policy \ @@ -357,19 +341,9 @@ impl IngressManager { "Creating inbound iptables chain: {}", self.chain_name )); - let inbound_field = if uses_directional_keys { - "network.ingress" - } else { - "allowLocalNetwork" - }; - logger.log_line(&format!( - "Inbound ({inbound_field}) policy: {}", - if policy.allow_local_network { - "ACCEPT new inbound connections" - } else { - "DROP new inbound connections (default-deny)" - } - )); + logger.log_line( + "Inbound (network.ingress) policy: DROP new inbound connections (default-deny)", + ); let mut runner = NsenterRunner; @@ -393,52 +367,17 @@ impl IngressManager { } }; - let ipv4_rules = Self::build_ingress_rules( - &self.chain_name, - policy, - uses_directional_keys, - IpFamily::V4, - ); + let ipv4_rules = Self::build_ingress_rules(&self.chain_name, IpFamily::V4); self.install_family(IpFamily::V4, &ipv4_rules, &mut runner, logger)?; if ipv6_enabled { - let ipv6_rules = Self::build_ingress_rules( - &self.chain_name, - policy, - uses_directional_keys, - IpFamily::V6, - ); + let ipv6_rules = Self::build_ingress_rules(&self.chain_name, IpFamily::V6); self.install_family(IpFamily::V6, &ipv6_rules, &mut runner, logger)?; } Ok(true) } - pub fn apply_legacy_rules( - &mut self, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result { - self.apply_rules_in_dialect(policy, false, logger) - } - - pub fn apply_directional_rules( - &mut self, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result { - self.apply_rules_in_dialect(policy, true, logger) - } - - pub fn apply_firewall_rules( - &mut self, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result { - let dialect = uses_directional_keys(policy); - self.apply_rules_in_dialect(policy, dialect, logger) - } - fn install_family( &mut self, family: IpFamily, @@ -531,12 +470,7 @@ impl IngressManager { result } - fn build_ingress_rules( - chain: &str, - policy: &ContainerPolicy, - uses_directional_keys: bool, - family: IpFamily, - ) -> IngressRules { + fn build_ingress_rules(chain: &str, family: IpFamily) -> IngressRules { fn argv(args: &[&str]) -> Vec { args.iter().map(|s| s.to_string()).collect() } @@ -573,21 +507,8 @@ impl IngressManager { } } - let inbound_verb = - if Self::permissive_inbound_field(policy, uses_directional_keys).is_some() { - accept - } else { - drop - }; body.push(argv(&[ - "-A", - chain, - "-m", - "state", - "--state", - "NEW", - "-j", - inbound_verb, + "-A", chain, "-m", "state", "--state", "NEW", "-j", drop, ])); body.push(argv(&["-A", chain, "-j", drop])); @@ -817,17 +738,11 @@ impl Drop for IngressManager { } } -#[cfg(test)] -#[path = "network_ingress_permissive_spec_tests.rs"] -mod permissive_spec_tests; - #[cfg(test)] mod tests { use super::*; use crate::lxc_common::network_iptables::plan_network; - use crate::mxc_common::models::{ - NetworkAction, NetworkEnforcementMode, NetworkIngressPolicy, NetworkPolicy, - }; + use crate::mxc_common::models::{NetworkAction, NetworkIngressPolicy}; fn directional_ingress( default: NetworkAction, @@ -845,10 +760,10 @@ mod tests { #[test] fn a_deny_deny_directional_ingress_asks_for_nothing_permissive() { assert_eq!( - IngressManager::permissive_inbound_field( - &directional_ingress(NetworkAction::Deny, NetworkAction::Deny), - true - ), + IngressManager::permissive_inbound_field(&directional_ingress( + NetworkAction::Deny, + NetworkAction::Deny + )), None, "the GA ingress defaults are what the existing default-deny chain already \ enforces, so they must not be refused" @@ -858,10 +773,10 @@ mod tests { #[test] fn a_permissive_directional_ingress_default_is_named_in_the_refusal() { assert_eq!( - IngressManager::permissive_inbound_field( - &directional_ingress(NetworkAction::Allow, NetworkAction::Deny), - true - ), + IngressManager::permissive_inbound_field(&directional_ingress( + NetworkAction::Allow, + NetworkAction::Deny + )), Some("network.ingress.default"), "an unenforceable value must be refused by the name the operator wrote" ); @@ -870,29 +785,15 @@ mod tests { #[test] fn a_permissive_directional_host_loopback_is_named_in_the_refusal() { assert_eq!( - IngressManager::permissive_inbound_field( - &directional_ingress(NetworkAction::Deny, NetworkAction::Allow), - true - ), + IngressManager::permissive_inbound_field(&directional_ingress( + NetworkAction::Deny, + NetworkAction::Allow + )), Some("network.ingress.hostLoopback"), "an unenforceable value must be refused by the name the operator wrote" ); } - #[test] - fn a_legacy_permissive_request_is_still_named_allow_local_network() { - let policy = ContainerPolicy { - allow_local_network: true, - ..Default::default() - }; - - assert_eq!( - IngressManager::permissive_inbound_field(&policy, false), - Some("allowLocalNetwork"), - "the 0.7 refusal must keep naming the 0.7 field" - ); - } - #[test] fn a_bare_directional_config_asks_for_nothing_permissive() { let policy = ContainerPolicy { @@ -901,54 +802,25 @@ mod tests { }; assert_eq!( - IngressManager::permissive_inbound_field(&policy, true), + IngressManager::permissive_inbound_field(&policy), None, "the parser's fill-in denies inbound, which the default chain already enforces" ); } #[test] - fn the_inbound_log_line_names_the_schema_that_asked_for_the_posture() { - for (directional, expected, absent) in [ - (true, "network.ingress", "allowLocalNetwork"), - (false, "allowLocalNetwork", "network.ingress"), - ] { - let mut policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - network_enforcement_mode: NetworkEnforcementMode::Firewall, - allowed_hosts: vec!["example.com".to_string()], - ..Default::default() - }; - if directional { - policy.network_egress = Some(crate::mxc_common::models::NetworkEgressPolicy { - default: NetworkAction::Allow, - ..Default::default() - }); - policy.network_ingress = Some(NetworkIngressPolicy::default()); - } - - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let mut manager = IngressManager::new("log-field-test", 1); - let _ = manager.apply_firewall_rules(&policy, &mut logger); - let logged = logger.get_buffer().to_string(); - - assert!( - logged.contains(expected), - "directional={directional}: inbound log must name {expected}, got: {logged}" - ); - assert!( - !logged.contains(absent), - "directional={directional}: inbound log must not name {absent}, got: {logged}" - ); - } - } - - fn policy_with(allow_local: bool, default: NetworkPolicy) -> ContainerPolicy { - ContainerPolicy { - allow_local_network: allow_local, - default_network_policy: default, + fn the_inbound_log_line_names_directional_policy() { + let mut policy = directional_ingress(NetworkAction::Deny, NetworkAction::Deny); + policy.network_egress = Some(crate::mxc_common::models::NetworkEgressPolicy { + default: NetworkAction::Allow, ..Default::default() - } + }); + let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); + let mut manager = IngressManager::new("log-field-test", 1); + let _ = manager.apply_firewall_rules(&policy, &mut logger); + assert!(logger + .get_buffer() + .contains("Inbound (network.ingress) policy: DROP")); } fn is(rule: &[String], want: &[&str]) -> bool { @@ -1052,25 +924,16 @@ mod tests { ); } - fn full_sequence( - policy: &ContainerPolicy, - uses_directional_keys: bool, - family: IpFamily, - ) -> Vec> { - let rules = - IngressManager::build_ingress_rules(TEST_CHAIN, policy, uses_directional_keys, family); + fn full_sequence(family: IpFamily) -> Vec> { + let rules = IngressManager::build_ingress_rules(TEST_CHAIN, family); let mut seq = vec![vec!["-N".to_string(), TEST_CHAIN.to_string()]]; seq.extend(rules.body.iter().cloned()); seq.push(rules.hook.clone()); seq } - fn build(allow_local: bool) -> Vec> { - full_sequence( - &policy_with(allow_local, NetworkPolicy::Block), - false, - IpFamily::V4, - ) + fn build() -> Vec> { + full_sequence(IpFamily::V4) } struct FakeRunner Result<(), RunError>> { @@ -1206,36 +1069,15 @@ mod tests { } #[test] - fn loopback_always_accepts_regardless_of_allow_local() { - for allow in [true, false] { - let rules = build(allow); - assert!( - has(&rules, &["-A", "MXC-t", "-i", "lo", "-j", "ACCEPT"]), - "loopback must be an unconditional ACCEPT (allow_local={allow})" - ); - assert!( - !has(&rules, &["-A", "MXC-t", "-i", "lo", "-j", "DROP"]), - "loopback must never be DROP (allow_local={allow})" - ); - } - } - - #[test] - fn allow_local_true_accepts_new_inbound() { - let rules = build(true); - assert!(has( - &rules, - &["-A", "MXC-t", "-m", "state", "--state", "NEW", "-j", "ACCEPT"] - )); - assert!(!has( - &rules, - &["-A", "MXC-t", "-m", "state", "--state", "NEW", "-j", "DROP"] - )); + fn loopback_always_accepts() { + let rules = build(); + assert!(has(&rules, &["-A", "MXC-t", "-i", "lo", "-j", "ACCEPT"])); + assert!(!has(&rules, &["-A", "MXC-t", "-i", "lo", "-j", "DROP"])); } #[test] fn allow_local_false_drops_new_inbound() { - let rules = build(false); + let rules = build(); assert!(has( &rules, &["-A", "MXC-t", "-m", "state", "--state", "NEW", "-j", "DROP"] @@ -1248,7 +1090,7 @@ mod tests { #[test] fn established_precedes_new_inbound_decision() { - let rules = build(false); + let rules = build(); let est = pos( &rules, &[ @@ -1275,47 +1117,21 @@ mod tests { } #[test] - fn new_inbound_precedes_terminal_default() { - let rules = build(true); - let new = pos( - &rules, - &[ - "-A", "MXC-t", "-m", "state", "--state", "NEW", "-j", "ACCEPT", - ], - ) - .expect("NEW rule must be emitted"); - let def = - pos(&rules, &["-A", "MXC-t", "-j", "DROP"]).expect("terminal default must be emitted"); - assert!( - new < def, - "NEW-inbound accept must precede the terminal default DROP" - ); - } - - #[test] - fn terminal_default_is_always_drop_regardless_of_egress_policy() { - for default in [NetworkPolicy::Block, NetworkPolicy::Allow] { - let rules = full_sequence(&policy_with(false, default.clone()), false, IpFamily::V4); - assert!( - has(&rules, &["-A", "MXC-t", "-j", "DROP"]), - "terminal must be DROP (egress default={default:?})" - ); - assert!( - !has(&rules, &["-A", "MXC-t", "-j", "ACCEPT"]), - "terminal must never be a bare ACCEPT (egress default={default:?})" - ); - } + fn terminal_default_is_always_drop() { + let rules = full_sequence(IpFamily::V4); + assert!(has(&rules, &["-A", "MXC-t", "-j", "DROP"])); + assert!(!has(&rules, &["-A", "MXC-t", "-j", "ACCEPT"])); } #[test] fn input_hook_present_with_netns() { - let rules = build(true); + let rules = build(); assert!(has(&rules, &["-I", "INPUT", "-j", "MXC-t"])); } #[test] fn no_egress_dest_or_dns_rules_in_ingress_chain() { - let rules = build(false); + let rules = build(); assert!( !rules.iter().any(|r| r.iter().any(|a| a == "-d")), "ingress chain must not emit -d destination rules" @@ -1328,24 +1144,20 @@ mod tests { #[test] fn chain_is_created_first() { - let rules = build(false); + let rules = build(); assert!( is(&rules[0], &["-N", "MXC-t"]), "chain must be created first" ); } - fn build_family(allow_local: bool, family: IpFamily) -> Vec> { - full_sequence( - &policy_with(allow_local, NetworkPolicy::Block), - false, - family, - ) + fn build_family(family: IpFamily) -> Vec> { + full_sequence(family) } #[test] fn ipv4_full_sequence_is_pinned_exactly() { - let rules = build_family(false, IpFamily::V4); + let rules = build_family(IpFamily::V4); let want: &[&[&str]] = &[ &["-N", "MXC-t"], &["-A", "MXC-t", "-i", "lo", "-j", "ACCEPT"], @@ -1380,20 +1192,15 @@ mod tests { #[test] fn ipv4_emits_no_icmpv6_rules() { - for allow in [true, false] { - let rules = build_family(allow, IpFamily::V4); - assert!( - !rules - .iter() - .any(|r| r.iter().any(|a| a == "--icmpv6-type" || a == "icmpv6")), - "IPv4 chain must not emit any ICMPv6 rule (allow_local={allow})" - ); - } + let rules = build_family(IpFamily::V4); + assert!(!rules + .iter() + .any(|r| r.iter().any(|a| a == "--icmpv6-type" || a == "icmpv6"))); } #[test] fn ipv6_permits_neighbor_discovery_types() { - let rules = build_family(false, IpFamily::V6); + let rules = build_family(IpFamily::V6); for (num, name) in [ ("133", "router-solicitation"), ("134", "router-advertisement"), @@ -1421,7 +1228,7 @@ mod tests { #[test] fn ipv6_permits_mld_and_essential_error_types() { - let rules = build_family(false, IpFamily::V6); + let rules = build_family(IpFamily::V6); for (num, name) in [ ("130", "multicast-listener-query"), ("131", "multicast-listener-report"), @@ -1453,7 +1260,7 @@ mod tests { #[test] fn ipv6_icmpv6_accepts_precede_new_and_terminal_drop() { - let rules = build_family(false, IpFamily::V6); + let rules = build_family(IpFamily::V6); let new = pos( &rules, &["-A", "MXC-t", "-m", "state", "--state", "NEW", "-j", "DROP"], @@ -1491,7 +1298,7 @@ mod tests { #[test] fn ipv6_does_not_blanket_accept_icmpv6_or_new_inbound() { - let rules = build_family(false, IpFamily::V6); + let rules = build_family(IpFamily::V6); assert!( !has(&rules, &["-A", "MXC-t", "-p", "icmpv6", "-j", "ACCEPT"]), "IPv6 chain must not blanket-accept all ICMPv6" @@ -1527,20 +1334,6 @@ mod tests { ); } - #[test] - fn ipv6_new_decision_follows_allow_local_toggle() { - let allow = build_family(true, IpFamily::V6); - assert!(has( - &allow, - &["-A", "MXC-t", "-m", "state", "--state", "NEW", "-j", "ACCEPT"] - )); - let deny = build_family(false, IpFamily::V6); - assert!(has( - &deny, - &["-A", "MXC-t", "-m", "state", "--state", "NEW", "-j", "DROP"] - )); - } - #[test] fn ingress_chain_name_is_distinct_from_egress() { let name = "my-container"; @@ -1560,12 +1353,7 @@ mod tests { } fn permissive_firewall_policy() -> ContainerPolicy { - ContainerPolicy { - allow_local_network: true, - default_network_policy: NetworkPolicy::Block, - network_enforcement_mode: NetworkEnforcementMode::Firewall, - ..Default::default() - } + directional_ingress(NetworkAction::Allow, NetworkAction::Deny) } #[test] @@ -1577,7 +1365,7 @@ mod tests { let result = mgr.apply_firewall_rules(&permissive_firewall_policy(), &mut logger); assert!( result.is_err(), - "allowLocalNetwork: true must be refused (pid={pid})" + "network.ingress.default: allow must be refused (pid={pid})" ); let msg = result.unwrap_err(); assert!( @@ -1604,7 +1392,7 @@ mod tests { directional_ingress(NetworkAction::Allow, NetworkAction::Deny), "network.ingress.default", ), - (permissive_firewall_policy(), "allowLocalNetwork"), + (permissive_firewall_policy(), "network.ingress.default"), ] { let mut mgr = IngressManager::new("lan-inbound", 42); let msg = mgr @@ -1819,12 +1607,7 @@ mod tests { #[test] fn builder_returns_body_and_hook_separately() { for family in [IpFamily::V4, IpFamily::V6] { - let rules = IngressManager::build_ingress_rules( - TEST_CHAIN, - &policy_with(false, NetworkPolicy::Block), - false, - family, - ); + let rules = IngressManager::build_ingress_rules(TEST_CHAIN, family); assert_eq!( rules.hook, vec![ @@ -1865,12 +1648,7 @@ mod tests { let container = "install-order-container"; let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); let mut mgr = IngressManager::new(container, pid); - let rules = IngressManager::build_ingress_rules( - mgr.chain_name(), - &policy_with(false, NetworkPolicy::Block), - false, - IpFamily::V4, - ); + let rules = IngressManager::build_ingress_rules(mgr.chain_name(), IpFamily::V4); let mut runner = FakeRunner { calls: Vec::new(), @@ -2141,12 +1919,7 @@ mod tests { let pid = 7u32; let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); let mut mgr = IngressManager::new("partial-container", pid); - let rules = IngressManager::build_ingress_rules( - mgr.chain_name(), - &policy_with(false, NetworkPolicy::Block), - false, - IpFamily::V4, - ); + let rules = IngressManager::build_ingress_rules(mgr.chain_name(), IpFamily::V4); let mut runner = FakeRunner { calls: Vec::new(), @@ -2197,12 +1970,7 @@ mod tests { let pid = 7u32; let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); let mut mgr = IngressManager::new("reset-spawn-fail-container", pid); - let rules = IngressManager::build_ingress_rules( - mgr.chain_name(), - &policy_with(false, NetworkPolicy::Block), - false, - IpFamily::V4, - ); + let rules = IngressManager::build_ingress_rules(mgr.chain_name(), IpFamily::V4); let mut runner = FakeRunner { calls: Vec::new(), @@ -2237,12 +2005,7 @@ mod tests { let pid = 7u32; let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); let mut mgr = IngressManager::new("reset-exhaustion-container", pid); - let rules = IngressManager::build_ingress_rules( - mgr.chain_name(), - &policy_with(false, NetworkPolicy::Block), - false, - IpFamily::V4, - ); + let rules = IngressManager::build_ingress_rules(mgr.chain_name(), IpFamily::V4); let mut runner = FakeRunner { calls: Vec::new(), @@ -2283,12 +2046,7 @@ mod tests { #[test] fn ipv6_full_body_sequence_is_pinned_exactly() { - let rules = IngressManager::build_ingress_rules( - TEST_CHAIN, - &policy_with(false, NetworkPolicy::Block), - false, - IpFamily::V6, - ); + let rules = IngressManager::build_ingress_rules(TEST_CHAIN, IpFamily::V6); let want: Vec> = vec![ vec!["-A", "MXC-t", "-i", "lo", "-j", "ACCEPT"], @@ -2486,37 +2244,9 @@ mod tests { } } - fn legacy_firewall_mode_with_permissive_egress( - mode: NetworkEnforcementMode, - ) -> ContainerPolicy { - ContainerPolicy { - network_enforcement_mode: mode, - default_network_policy: NetworkPolicy::Allow, - ..Default::default() - } - } - - #[test] - fn a_firewall_mode_config_with_nothing_to_restrict_outbound_still_installs_the_inbound_chain() { - for mode in [ - NetworkEnforcementMode::Firewall, - NetworkEnforcementMode::Both, - ] { - let label = format!("{mode:?}"); - let policy = legacy_firewall_mode_with_permissive_egress(mode); - - assert!( - plan_network(&policy).installs_firewall(), - "{label}: a config naming a firewall enforcement mode is owed the inbound \ - deny chain even with nothing to restrict outbound" - ); - } - } - #[test] fn a_stated_directional_posture_installs_the_inbound_chain() { let mut policy = directional_ingress(NetworkAction::Allow, NetworkAction::Deny); - policy.default_network_policy = NetworkPolicy::Allow; policy.network_egress = Some(Default::default()); assert!(plan_network(&policy).installs_firewall()); diff --git a/src/mxc-sdk/src/backends/lxc/common/network_ingress_permissive_spec_tests.rs b/src/mxc-sdk/src/backends/lxc/common/network_ingress_permissive_spec_tests.rs deleted file mode 100644 index 17504549e..000000000 --- a/src/mxc-sdk/src/backends/lxc/common/network_ingress_permissive_spec_tests.rs +++ /dev/null @@ -1,169 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -use super::*; -use crate::mxc_common::logger::Mode; -use crate::mxc_common::models::NetworkEnforcementMode; - -const UNOCCUPIABLE_NETNS_PID: u32 = u32::MAX; - -/// Linux rejects PIDs at or above 2^22. -const PID_MAX_LIMIT: u32 = 1 << 22; - -const _: () = assert!( - UNOCCUPIABLE_NETNS_PID > PID_MAX_LIMIT, - "the netns PID these tests use must exceed the kernel's PID_MAX_LIMIT; otherwise a \ - privileged Linux run can enter a real process's network namespace and reset its \ - iptables chains" -); - -fn make_logger() -> Logger { - Logger::new(Mode::Buffer) -} - -fn firewall_policy(allow_local: bool) -> ContainerPolicy { - ContainerPolicy { - allow_local_network: allow_local, - network_enforcement_mode: NetworkEnforcementMode::Firewall, - ..Default::default() - } -} - -#[test] -fn permissive_inbound_in_a_container_netns_is_refused_not_installed() { - let policy = firewall_policy(true); - let mut mgr = IngressManager::new("test-container-refused", UNOCCUPIABLE_NETNS_PID); - let mut logger = make_logger(); - - let result = mgr.apply_firewall_rules(&policy, &mut logger); - - assert!( - result.is_err(), - "allow_local_network=true: expected Err (not-yet-implemented refusal), got {:?}", - result - ); - - let msg = result.unwrap_err(); - - assert!( - msg.contains("not yet implemented"), - "error message must contain \"not yet implemented\", got: {:?}", - msg - ); - assert!( - msg.contains("allowLocalNetwork"), - "error message must contain \"allowLocalNetwork\", got: {:?}", - msg - ); - assert!( - msg.contains("over-broad accept"), - "error message must contain \"over-broad accept\", got: {:?}", - msg - ); -} - -#[test] -fn permissive_inbound_in_both_mode_is_refused_not_installed() { - let policy = ContainerPolicy { - allow_local_network: true, - network_enforcement_mode: NetworkEnforcementMode::Both, - ..Default::default() - }; - let mut mgr = IngressManager::new("test-container-refused-both", UNOCCUPIABLE_NETNS_PID); - let mut logger = make_logger(); - - let result = mgr.apply_firewall_rules(&policy, &mut logger); - - assert!( - result.is_err(), - "allow_local_network=true, mode=Both: expected Err, got {:?}", - result - ); - let msg = result.unwrap_err(); - assert!( - msg.contains("not yet implemented"), - "mode=Both: message must contain \"not yet implemented\", got: {:?}", - msg - ); - assert!( - msg.contains("allowLocalNetwork"), - "mode=Both: message must contain \"allowLocalNetwork\", got: {:?}", - msg - ); - assert!( - msg.contains("over-broad accept"), - "mode=Both: message must contain \"over-broad accept\", got: {:?}", - msg - ); -} - -#[test] -fn permissive_inbound_refusal_does_not_set_rules_applied() { - let policy = firewall_policy(true); - let mut mgr = IngressManager::new("test-container-refused-state", UNOCCUPIABLE_NETNS_PID); - let mut logger = make_logger(); - - let result = mgr.apply_firewall_rules(&policy, &mut logger); - - assert!( - result.is_err(), - "allow_local_network=true: expected Err (refusal), got {:?}", - result - ); - - assert!( - !mgr.rules_applied(), - "rules_applied() must be false after a refusal — no rules were installed so cleanup \ - must not run, got rules_applied()=true" - ); -} - -#[test] -fn default_deny_with_netns_is_not_the_permissive_refusal() { - let policy = firewall_policy(false); - let mut mgr = IngressManager::new("test-container-deny-with-netns", UNOCCUPIABLE_NETNS_PID); - let mut logger = make_logger(); - - let result = mgr.apply_firewall_rules(&policy, &mut logger); - - if let Err(ref msg) = result { - assert!( - !msg.contains("not yet implemented"), - "allow_local_network=false: must not return the not-yet-implemented refusal, got: {:?}", - msg - ); - assert!( - !msg.contains("over-broad accept"), - "allow_local_network=false: must not return the over-broad-accept refusal, got: {:?}", - msg - ); - eprintln!( - "WARNING: default_deny_with_netns_is_not_the_permissive_refusal got Err \ - (expected off Linux — no iptables binary / no /proc). The non-error branch was \ - not exercised. Re-run on Linux to verify the happy path." - ); - } -} - -#[test] -fn permissive_inbound_is_refused_under_every_accepted_mode() { - for mode in [ - NetworkEnforcementMode::Firewall, - NetworkEnforcementMode::Both, - ] { - let policy = ContainerPolicy { - allow_local_network: true, - network_enforcement_mode: mode.clone(), - ..Default::default() - }; - let mut mgr = IngressManager::new("test-container-perm-modes", UNOCCUPIABLE_NETNS_PID); - let mut logger = make_logger(); - - let result = mgr.apply_firewall_rules(&policy, &mut logger); - - assert!( - result.is_err(), - "allow_local_network=true, mode={mode:?}: expected refusal, got {result:?}" - ); - } -} diff --git a/src/mxc-sdk/src/backends/lxc/common/network_iptables.rs b/src/mxc-sdk/src/backends/lxc/common/network_iptables.rs index 307911b72..983452633 100644 --- a/src/mxc-sdk/src/backends/lxc/common/network_iptables.rs +++ b/src/mxc-sdk/src/backends/lxc/common/network_iptables.rs @@ -1,16 +1,16 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::net::{IpAddr, Ipv6Addr, ToSocketAddrs}; +use std::net::IpAddr; use std::process::Command; use crate::mxc_common::hashing::sha256; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ - ContainerPolicy, NetworkAction, NetworkCidr, NetworkEgressPolicy, NetworkPeer, NetworkPolicy, - NetworkPort, NetworkProtocol, NetworkRule, ProxyAddress, ProxyHostPin, + ContainerPolicy, NetworkAction, NetworkCidr, NetworkEgressPolicy, NetworkPeer, NetworkPort, + NetworkProtocol, NetworkRule, }; -use crate::mxc_common::network_blocks::{self, AddressBlock, IpFamily, MAX_EGRESS_ENTRIES}; +use crate::mxc_common::network_blocks::{self, IpFamily, MAX_EGRESS_ENTRIES}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum NetworkPlan { @@ -29,37 +29,17 @@ impl NetworkPlan { } } -pub(crate) fn uses_directional_keys(policy: &ContainerPolicy) -> bool { - policy.network_egress.is_some() || policy.network_ingress.is_some() -} - pub(crate) fn plan_network(policy: &ContainerPolicy) -> NetworkPlan { - if uses_directional_keys(policy) { - plan_directional(policy) - } else { - plan_legacy(policy) - } -} - -fn plan_directional(policy: &ContainerPolicy) -> NetworkPlan { - if policy.network_proxy.is_enabled() { - return NetworkPlan::Filtered; - } - - if !policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty() { - return NetworkPlan::Filtered; - } - let egress_permits_nothing = match policy.network_egress.as_ref() { Some(egress) => egress.default == NetworkAction::Deny && egress.allow.is_empty(), - None => matches!(policy.default_network_policy, NetworkPolicy::Block), + None => true, }; let ingress_permits_nothing = match policy.network_ingress.as_ref() { Some(ingress) => { ingress.default == NetworkAction::Deny && ingress.host_loopback == NetworkAction::Deny } - None => !policy.allow_local_network, + None => true, }; if egress_permits_nothing && ingress_permits_nothing { @@ -69,33 +49,10 @@ fn plan_directional(policy: &ContainerPolicy) -> NetworkPlan { } } -fn plan_legacy(policy: &ContainerPolicy) -> NetworkPlan { - if policy.network_proxy.is_enabled() { - return NetworkPlan::Filtered; - } - - if !policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty() { - return NetworkPlan::Filtered; - } - - if matches!(policy.default_network_policy, NetworkPolicy::Block) && !policy.allow_local_network - { - return NetworkPlan::Isolated; - } - - NetworkPlan::Filtered -} - pub(crate) fn needs_network(policy: &ContainerPolicy) -> bool { !plan_network(policy).omits_interface() } -#[derive(Debug, Clone, PartialEq, Eq)] -struct ProxyEndpoint { - ip: String, - port: u16, -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum RuleAction { Allow, @@ -105,7 +62,7 @@ enum RuleAction { // iptables applies first-match-wins; entry order is precedence. #[derive(Debug, Clone, PartialEq, Eq)] struct EgressEntry { - destination: String, + destination: NetworkCidr, action: RuleAction, matching: RuleMatch, } @@ -144,18 +101,6 @@ struct PortRange { end: u16, } -#[derive(Debug, Default, Clone, PartialEq, Eq)] -struct ResolvedDestinations { - ipv4: Vec, - ipv6: Vec, -} - -impl ResolvedDestinations { - fn is_empty(&self) -> bool { - self.ipv4.is_empty() && self.ipv6.is_empty() - } -} - #[derive(Debug, Default, Clone, PartialEq, Eq)] struct FirewallRuleArgs { ipv4: Vec>, @@ -272,7 +217,6 @@ pub struct NetworkIptablesManager { preserve_policy: bool, hook_point: EgressHookPoint, created: CreatedResources, - proxy_pin: Option, } // iptables rejects chain names of 29 characters or more. @@ -348,7 +292,6 @@ impl NetworkIptablesManager { preserve_policy: false, hook_point, created: CreatedResources::default(), - proxy_pin: None, } } @@ -375,135 +318,6 @@ impl NetworkIptablesManager { self.rules_applied = false; } - // The hosts-file pin a proxied container needs before it runs. - // DNS round-robin can answer the same name differently on a later lookup. - pub fn proxy_host_pin(&self) -> Option<&ProxyHostPin> { - self.proxy_pin.as_ref() - } - - fn covers_every_address(destination: &str) -> bool { - destination - .split_once('/') - .and_then(|(_, prefix)| prefix.trim().parse::().ok()) - .is_some_and(|prefix| prefix == 0) - } - - fn resolve_host(host: &str) -> ResolvedDestinations { - // Winsock resolves an empty entry formatted as `:0` to every local - // interface address. glibc rejects `:0`. - if host.trim().is_empty() { - return ResolvedDestinations::default(); - } - - // Linux emits IPv4-mapped destinations as IPv4 packets. - // They must be programmed with iptables. - let rewritten = Self::ipv4_mapped_destination(host); - let host = rewritten.as_deref().unwrap_or(host); - - if host.contains('/') { - return match Self::destination_family(host) { - Some(IpFamily::V4) => ResolvedDestinations { - ipv4: vec![host.to_string()], - ipv6: Vec::new(), - }, - Some(IpFamily::V6) => ResolvedDestinations { - ipv4: Vec::new(), - ipv6: vec![host.to_string()], - }, - None => ResolvedDestinations::default(), - }; - } - - if let Ok(addr) = host.parse::() { - return match addr { - IpAddr::V4(_) => ResolvedDestinations { - ipv4: vec![host.to_string()], - ipv6: Vec::new(), - }, - IpAddr::V6(_) => ResolvedDestinations { - ipv4: Vec::new(), - ipv6: vec![host.to_string()], - }, - }; - } - - if let Ok(addrs) = format!("{}:0", host).to_socket_addrs() { - return Self::bucket_resolved_addrs(addrs.map(|addr| addr.ip())); - } - ResolvedDestinations::default() - } - - // Split A records, AAAA records, and IPv4-mapped AAAA records into the - // table that can match each packet. - fn bucket_resolved_addrs>(addrs: I) -> ResolvedDestinations { - let mut resolved = ResolvedDestinations::default(); - for ip in addrs { - match ip { - IpAddr::V4(ip) => resolved.ipv4.push(ip.to_string()), - - // A resolver can return a AAAA record in mapped form. - // It travels as IPv4 on the wire. - IpAddr::V6(ip) => match ip.to_ipv4_mapped() { - Some(v4) => resolved.ipv4.push(v4.to_string()), - None => resolved.ipv6.push(ip.to_string()), - }, - } - } - resolved - } - - // Rewrite an IPv4-mapped IPv6 destination to its embedded IPv4 form. - // Linux puts a genuine IPv4 packet on the wire for a mapped destination. - // - // CIDRs inside `::ffff:0:0/96` are handled too: the mapped range is the - // final 32 bits of that /96. A prefix shorter than 96 covers addresses - // outside the mapped range and stays IPv6. - fn ipv4_mapped_destination(destination: &str) -> Option { - let Some((network, prefix)) = destination.split_once('/') else { - return destination - .parse::() - .ok()? - .to_ipv4_mapped() - .map(|v4| v4.to_string()); - }; - - if prefix.is_empty() || !prefix.bytes().all(|b| b.is_ascii_digit()) { - return None; - } - let mapped = network.parse::().ok()?.to_ipv4_mapped()?; - let v4_prefix = prefix.parse::().ok()?.checked_sub(96)?; - if v4_prefix > 32 { - return None; - } - Some(format!("{}/{}", mapped, v4_prefix)) - } - - fn destination_family(destination: &str) -> Option { - if let Some((network, prefix)) = destination.split_once('/') { - // `u8::from_str` accepts a leading `+`. iptables canonicalizes - // `10.0.0.0/+24` to `10.0.0.0/24`. - if network.is_empty() - || prefix.is_empty() - || !prefix.bytes().all(|b| b.is_ascii_digit()) - { - return None; - } - - let addr = network.parse::().ok()?; - let prefix = prefix.parse::().ok()?; - return match addr { - IpAddr::V4(_) if prefix <= 32 => Some(IpFamily::V4), - IpAddr::V6(_) if prefix <= 128 => Some(IpFamily::V6), - _ => None, - }; - } - - match destination.parse::().ok()? { - IpAddr::V4(_) => Some(IpFamily::V4), - IpAddr::V6(_) => Some(IpFamily::V6), - } - } - fn rule_action_arg(action: &RuleAction) -> &'static str { match action { RuleAction::Allow => "ACCEPT", @@ -539,216 +353,36 @@ impl NetworkIptablesManager { ] } - // The resolver address is not known when a closed chain's allow list is - // written. Port 53 is opened until the container can resolve names. - fn build_dns_resolution_rule_args(chain_name: &str) -> Vec> { - vec![ - vec![ - "-A", chain_name, "-p", "udp", "--dport", "53", "-j", "ACCEPT", - ], - vec![ - "-A", chain_name, "-p", "tcp", "--dport", "53", "-j", "ACCEPT", - ], - ] - .into_iter() - .map(|args| args.into_iter().map(String::from).collect()) - .collect() - } - - fn default_policy_action(default_policy: NetworkPolicy, proxy_enabled: bool) -> &'static str { - if proxy_enabled { - return "DROP"; - } + fn default_policy_action(default_policy: NetworkAction) -> &'static str { match default_policy { - NetworkPolicy::Block => "DROP", - NetworkPolicy::Allow => "ACCEPT", + NetworkAction::Deny => "DROP", + NetworkAction::Allow => "ACCEPT", } } - fn build_default_policy_rule_arg( - chain_name: &str, - policy: NetworkPolicy, - proxy_enabled: bool, - ) -> Vec { - let default_action = Self::default_policy_action(policy, proxy_enabled); + fn build_default_policy_rule_arg(chain_name: &str, policy: NetworkAction) -> Vec { + let default_action = Self::default_policy_action(policy); vec!["-A", chain_name, "-j", default_action] .into_iter() .map(String::from) .collect() } - // Proxy rules are IPv4 only and must not run through `ip6tables`. - // A proxied IPv6 chain reaches its closing DROP with only loopback allowed. - fn build_proxy_chain_rule_args( - chain_name: &str, - endpoints: &[ProxyEndpoint], - ) -> Vec> { - endpoints - .iter() - .map(|endpoint| { - vec![ - "-A".to_string(), - chain_name.to_string(), - "-p".to_string(), - "tcp".to_string(), - "-d".to_string(), - endpoint.ip.clone(), - "--dport".to_string(), - endpoint.port.to_string(), - "-j".to_string(), - "ACCEPT".to_string(), - ] - }) - .collect() - } - - fn host_is_ipv6_literal(host: &str) -> bool { - let candidate = crate::mxc_common::models::unbracket_host(host); - matches!(candidate.parse::(), Ok(IpAddr::V6(_))) - } - - fn ipv6_proxy_unsupported(host: &str) -> String { - format!( - "IPv6 network proxy endpoints are not supported: the proxy firewall rule is \ - emitted with IPv4 iptables only, so '{}' cannot be enforced and would be \ - silently dropped. Use an IPv4 proxy address.", - host - ) - } - - // Cap on how many resolved proxy addresses the chain will open. - // A round-robin or CDN answer is unbounded, and every address becomes - // its own ACCEPT rule and `iptables` process on the container-start path. - const MAX_PROXY_ENDPOINTS: usize = 16; - - fn bound_proxy_addresses<'a>( - host: &str, - addresses: &'a [String], - logger: &mut Logger, - ) -> &'a [String] { - if addresses.len() <= Self::MAX_PROXY_ENDPOINTS { - return addresses; - } - - logger.log_line(&format!( - "Warning: proxy host '{}' resolved to {} addresses; opening the first {} only. \ - The container is pinned to the first, so it still reaches the proxy.", - host, - addresses.len(), - Self::MAX_PROXY_ENDPOINTS - )); - - &addresses[..Self::MAX_PROXY_ENDPOINTS] - } - - // Resolve the proxy with one lookup. DNS round-robin can answer one - // name differently on a later lookup. - fn resolve_proxy_endpoints( - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result<(Vec, Option), String> { - if !policy.network_proxy.is_enabled() { - return Ok((Vec::new(), None)); - } - - let address = policy.network_proxy.address.as_ref().ok_or_else(|| { - "Network proxy is enabled but no proxy address is configured".to_string() - })?; - - if address.port() == 0 { - return Err("Network proxy port must be between 1 and 65535".to_string()); - } - - if Self::host_is_ipv6_literal(address.host()) { - return Err(Self::ipv6_proxy_unsupported(address.host())); - } - - let resolved = Self::resolve_host(address.host()); - if resolved.ipv4.is_empty() { - if !resolved.ipv6.is_empty() { - return Err(Self::ipv6_proxy_unsupported(address.host())); - } - return Err(format!( - "Could not resolve network proxy host '{}'", - address.host() - )); - } - - let endpoints: Vec = - Self::bound_proxy_addresses(address.host(), &resolved.ipv4, logger) - .iter() - .map(|ip| { - logger.log_line(&format!( - "Allowing network proxy egress: {}:{} ({})", - address.host(), - address.port(), - ip - )); - ProxyEndpoint { - ip: ip.clone(), - port: address.port(), - } - }) - .collect(); - - let pin = Self::build_proxy_host_pin(address, &endpoints[0].ip, logger)?; - Ok((endpoints, pin)) - } - - // A proxied chain opens no port 53. The hosts-file pin lets the - // container find the proxy without selecting an address the chain never allowed. - fn build_proxy_host_pin( - address: &ProxyAddress, - ip: &str, - logger: &mut Logger, - ) -> Result, String> { - let parsed: IpAddr = ip.parse().map_err(|_| { - format!( - "Network proxy host '{}' resolved to '{}', which is not an IP address", - address.host(), - ip - ) - })?; - - let pin = address - .host_pin(parsed) - .map_err(|e| format!("Cannot pin network proxy host: {}", e))?; - - if let Some(pin) = pin.as_ref() { - logger.log_line(&format!( - "Pinning network proxy '{}' to resolved address {} inside the container.", - pin.hostname(), - pin.ip() - )); - } - - Ok(pin) - } - - fn build_resolved_destination_rule_args( + fn build_destination_rule_args( chain_name: &str, - destinations: &ResolvedDestinations, + destination: &NetworkCidr, action: &RuleAction, matching: RuleMatch, ) -> FirewallRuleArgs { let mut args = FirewallRuleArgs::default(); - for destination in &destinations.ipv4 { - args.ipv4.push(Self::build_single_rule_args( - chain_name, - destination, - action, - matching, - IpFamily::V4, - )); - } - for destination in &destinations.ipv6 { - args.ipv6.push(Self::build_single_rule_args( - chain_name, - destination, - action, - matching, - IpFamily::V6, - )); + if let Some((family, block)) = network_blocks::resolve(destination) { + let destination = format!("{}/{}", block.address(family), block.prefix()); + let rule = + Self::build_single_rule_args(chain_name, &destination, action, matching, family); + match family { + IpFamily::V4 => args.ipv4.push(rule), + IpFamily::V6 => args.ipv6.push(rule), + } } args } @@ -800,96 +434,34 @@ impl NetworkIptablesManager { } #[cfg(test)] - fn build_host_rule_args(chain_name: &str, host: &str, action: &RuleAction) -> FirewallRuleArgs { - let destinations = Self::resolve_host(host); - Self::build_resolved_destination_rule_args( - chain_name, - &destinations, - action, - RuleMatch::AnyTraffic, - ) - } - - #[cfg(test)] - fn build_policy_rule_args( - chain_name: &str, - policy: &ContainerPolicy, - uses_directional_keys: bool, - ) -> FirewallRuleArgs { + fn build_policy_rule_args(chain_name: &str, policy: &ContainerPolicy) -> FirewallRuleArgs { let mut logger = crate::mxc_common::logger::Logger::new(crate::mxc_common::logger::Mode::Buffer); - Self::build_policy_rules_logged(chain_name, policy, uses_directional_keys, &mut logger) - .expect( - "test policy should not pair an accepting default with an unresolvable block entry", - ) - } - - fn stated_egress( - policy: &ContainerPolicy, - uses_directional_keys: bool, - ) -> Option<&NetworkEgressPolicy> { - if uses_directional_keys { - policy.network_egress.as_ref() - } else { - None - } + Self::build_policy_rules_logged(chain_name, policy, &mut logger).expect( + "test policy should not pair an accepting default with an unresolvable block entry", + ) } - fn effective_default_policy( - policy: &ContainerPolicy, - uses_directional_keys: bool, - ) -> NetworkPolicy { - match Self::stated_egress(policy, uses_directional_keys) { - Some(egress) => match egress.default { - NetworkAction::Allow => NetworkPolicy::Allow, - NetworkAction::Deny => NetworkPolicy::Block, - }, - None => policy.default_network_policy.clone(), - } + fn effective_default_policy(policy: &ContainerPolicy) -> NetworkAction { + policy + .network_egress + .as_ref() + .map_or(NetworkAction::Deny, |egress| egress.default) } /// Lower the egress policy for its refusals alone, discarding the rules. - pub(crate) fn validate_egress_lowering( - policy: &ContainerPolicy, - uses_directional_keys: bool, - ) -> Result<(), String> { - Self::lower_egress(policy, uses_directional_keys).map(|_| ()) + pub(crate) fn validate_egress_lowering(policy: &ContainerPolicy) -> Result<(), String> { + Self::lower_egress(policy).map(|_| ()) } - fn lower_egress( - policy: &ContainerPolicy, - uses_directional_keys: bool, - ) -> Result, String> { - match Self::stated_egress(policy, uses_directional_keys) { + fn lower_egress(policy: &ContainerPolicy) -> Result, String> { + match policy.network_egress.as_ref() { Some(egress) => Self::lower_directional_egress(egress), - None => Ok(Self::lower_legacy_hosts(policy)), + None => Ok(Vec::new()), } } - // Block entries come first. Under iptables first-match-wins, exchanging - // the two halves reverses overlapping allow and block lists. - fn lower_legacy_hosts(policy: &ContainerPolicy) -> Vec { - policy - .blocked_hosts - .iter() - .map(|host| (host, RuleAction::Deny)) - .chain( - policy - .allowed_hosts - .iter() - .map(|host| (host, RuleAction::Allow)), - ) - .map(|(host, action)| EgressEntry { - destination: host.clone(), - action, - - matching: RuleMatch::AnyTraffic, - }) - .collect() - } - - // Deny rules precede allow rules for the same first-match-wins reason - // as the legacy lowering. + // Deny rules precede allow rules under iptables first-match-wins. fn lower_directional_egress(egress: &NetworkEgressPolicy) -> Result, String> { let mut entries = Vec::new(); let mut remaining = MAX_EGRESS_ENTRIES; @@ -944,23 +516,22 @@ impl NetworkIptablesManager { /// /// An `iptables` rule matches one destination block and cannot carry an /// exclusion, so the exclusion is subtracted from the peer instead. - fn peer_destinations(peer: &NetworkPeer) -> Result, String> { + fn peer_destinations(peer: &NetworkPeer) -> Result, String> { // Passing an out-of-range prefix through unchanged keeps it on the path // that reports a destination resolving to no address. let Some((family, blocks)) = network_blocks::peer_blocks(peer)? else { - return Ok(vec![Self::cidr_destination(&peer.cidr)]); + return Ok(vec![peer.cidr.clone()]); }; Ok(blocks .into_iter() - .map(|block| Self::block_destination(block, family)) + .map(|block| NetworkCidr { + address: block.address(family), + prefix_length: block.prefix(), + }) .collect()) } - fn block_destination(block: AddressBlock, family: IpFamily) -> String { - format!("{}/{}", block.address(family), block.prefix()) - } - // A v4 chain and a v6 chain are programmed separately. Neither wildcard // covers the other. fn every_destination_peers() -> Vec { @@ -982,10 +553,6 @@ impl NetworkIptablesManager { ] } - fn cidr_destination(cidr: &NetworkCidr) -> String { - network_blocks::cidr_text(cidr) - } - fn lower_port_selectors(ports: &[NetworkPort]) -> Vec { if ports.is_empty() { return vec![RuleMatch::AnyTraffic]; @@ -1028,30 +595,26 @@ impl NetworkIptablesManager { } } - // DNS round-robin can answer one name differently on repeated lookups. - // Each lowered entry is resolved once before logging and rule generation share the result. + // Destinations are typed CIDRs; the shared block arithmetic determines + // their firewall family, including IPv4-mapped IPv6 addresses. fn build_policy_rules_logged( chain_name: &str, policy: &ContainerPolicy, - uses_directional_keys: bool, logger: &mut Logger, ) -> Result { - let default_permits = matches!( - Self::effective_default_policy(policy, uses_directional_keys), - NetworkPolicy::Allow - ); + let default_permits = Self::effective_default_policy(policy) == NetworkAction::Allow; let mut args = FirewallRuleArgs::default(); - let mut unresolved_denies: Vec<&str> = Vec::new(); - let mut catch_all_allows: Vec<&str> = Vec::new(); - let entries = Self::lower_egress(policy, uses_directional_keys)?; + let mut unresolved_denies: Vec = Vec::new(); + let mut catch_all_allows: Vec = Vec::new(); + let entries = Self::lower_egress(policy)?; for entry in &entries { - let host = entry.destination.as_str(); + let host = network_blocks::cidr_text(&entry.destination); let action = entry.action; - let destinations = Self::resolve_host(host); - if destinations.is_empty() { + let family = network_blocks::resolve(&entry.destination); + if family.is_none() { if default_permits && matches!(action, RuleAction::Deny) { return Err(format!( - "blocked host '{}' resolved to no address, so no rule can be \ + "network.egress deny destination '{}' resolved to no address, so no rule can be \ programmed to deny it, and the default network policy accepts \ what no rule matches; refusing to apply a policy that would \ leave it reachable", @@ -1059,22 +622,21 @@ impl NetworkIptablesManager { )); } if matches!(action, RuleAction::Deny) { - unresolved_denies.push(host); + unresolved_denies.push(host.clone()); } - logger.log_line(&format!("Warning: could not resolve host '{}'", host)); + logger.log_line(&format!( + "Warning: could not resolve destination '{}'", + host + )); } else if matches!(action, RuleAction::Allow) && matches!(entry.matching, RuleMatch::AnyTraffic) - && destinations - .ipv4 - .iter() - .chain(destinations.ipv6.iter()) - .any(|dest| Self::covers_every_address(dest)) + && entry.destination.prefix_length == 0 { - catch_all_allows.push(host); + catch_all_allows.push(host.clone()); } - let rule_args = Self::build_resolved_destination_rule_args( + let rule_args = Self::build_destination_rule_args( chain_name, - &destinations, + &entry.destination, &action, entry.matching, ); @@ -1088,8 +650,8 @@ impl NetworkIptablesManager { } if !unresolved_denies.is_empty() && !catch_all_allows.is_empty() { return Err(format!( - "blocked host(s) {} resolved to no address, so no rule can be programmed \ - to deny them, while allowed host(s) {} accept every address and are \ + "network.egress deny destination(s) {} resolved to no address, so no rule can be programmed \ + to deny them, while allow destination(s) {} accept every address and are \ evaluated before the chain's closing DROP; whatever the blocked host \ resolves to for the container is therefore accepted, so deny precedence \ cannot hold. Fix or remove the unresolvable blocked host, or narrow the \ @@ -1354,10 +916,9 @@ impl NetworkIptablesManager { Ok(()) } - fn apply_rules_in_dialect( + pub fn apply_firewall_rules( &mut self, policy: &ContainerPolicy, - uses_directional_keys: bool, logger: &mut Logger, ) -> Result { let plan = plan_network(policy); @@ -1375,43 +936,10 @@ impl NetworkIptablesManager { )); } - let (proxy_endpoints, proxy_pin) = Self::resolve_proxy_endpoints(policy, logger)?; - self.proxy_pin = proxy_pin; - - let outcome = self.apply_firewall_rules_inner( - policy, - uses_directional_keys, - &proxy_endpoints, - logger, - ); + let outcome = self.apply_firewall_rules_inner(policy, logger); self.record_apply_outcome(outcome, logger) } - pub fn apply_legacy_rules( - &mut self, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result { - self.apply_rules_in_dialect(policy, false, logger) - } - - pub fn apply_directional_rules( - &mut self, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result { - self.apply_rules_in_dialect(policy, true, logger) - } - - pub fn apply_firewall_rules( - &mut self, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result { - let dialect = uses_directional_keys(policy); - self.apply_rules_in_dialect(policy, dialect, logger) - } - fn record_apply_outcome( &mut self, outcome: Result, @@ -1450,18 +978,10 @@ impl NetworkIptablesManager { fn apply_firewall_rules_inner( &self, policy: &ContainerPolicy, - uses_directional_keys: bool, - proxy_endpoints: &[ProxyEndpoint], logger: &mut Logger, ) -> Result { let mut created = CreatedResources::default(); - match self.install_firewall_rules( - policy, - uses_directional_keys, - proxy_endpoints, - logger, - &mut created, - ) { + match self.install_firewall_rules(policy, logger, &mut created) { Ok(()) => Ok(created), Err(e) => { let residual = self.teardown_created(&self.chain_name, &created, logger); @@ -1473,8 +993,6 @@ impl NetworkIptablesManager { fn install_firewall_rules( &self, policy: &ContainerPolicy, - uses_directional_keys: bool, - proxy_endpoints: &[ProxyEndpoint], logger: &mut Logger, created: &mut CreatedResources, ) -> Result<(), String> { @@ -1483,15 +1001,6 @@ impl NetworkIptablesManager { self.chain_name )); - if !proxy_endpoints.is_empty() && !policy.blocked_hosts.is_empty() { - return Err( - "network.proxy cannot be combined with blockedHosts: the proxy can fetch a \ - blocked destination on the container's behalf, so the block list would not \ - be enforced" - .to_string(), - ); - } - let ipv6_enabled = match self.ip6tables_status(logger) { Ip6tablesStatus::Available => true, Ip6tablesStatus::KernelIpv6Disabled => false, @@ -1514,84 +1023,27 @@ impl NetworkIptablesManager { Self::publish_created(created); } - let proxy_mode = !proxy_endpoints.is_empty(); - - if proxy_mode { - // No port 53 accept: the container resolves the proxy through the - // hosts-file pin, and an unscoped one would be a standing DNS-tunnel - // exfil path. No ESTABLISHED,RELATED accept: every packet to the - // proxy already matches an endpoint ACCEPT. - let loopback_rules = vec![Self::build_loopback_accept_rule_args(&self.chain_name)]; - self.run_iptables_rule_args(&loopback_rules, logger)?; - if ipv6_enabled { - self.run_ip6tables_rule_args(&loopback_rules, logger)?; - } - let proxy_rules = Self::build_proxy_chain_rule_args(&self.chain_name, proxy_endpoints); - self.run_iptables_rule_args(&proxy_rules, logger)?; - for rule in loopback_rules.iter().chain(proxy_rules.iter()) { - logger.log_line(&format!("Programmed iptables rule: {}", rule.join(" "))); - } - if !policy.allowed_hosts.is_empty() { - logger.log_line( - "Warning: network.proxy is configured, so allowedHosts is not programmed; \ - the container may reach the proxy and nothing else.", - ); - } - if ipv6_enabled { - logger.log_line( - "IPv6 egress is denied outright while a proxy is configured: the proxy \ - endpoint is IPv4, so the IPv6 chain carries its loopback accept and \ - its closing DROP.", - ); - } - } else { - let base_rules = Self::build_base_chain_rule_args(&self.chain_name); - - let mut tail_rules: Vec> = Vec::new(); - - // Only a closed chain naming hosts to allow gets DNS. An open chain - // needs no grant, and the accept would sit ahead of the deny rules. - let closed_with_named_hosts = matches!( - Self::effective_default_policy(policy, uses_directional_keys), - NetworkPolicy::Block - ) && !policy.allowed_hosts.is_empty(); - if !uses_directional_keys && closed_with_named_hosts { - tail_rules.extend(Self::build_dns_resolution_rule_args(&self.chain_name)); - } - - self.run_iptables_rule_args(&base_rules, logger)?; - if !tail_rules.is_empty() { - self.run_iptables_rule_args(&tail_rules, logger)?; - } - if ipv6_enabled { - self.run_ip6tables_rule_args(&base_rules, logger)?; - if !tail_rules.is_empty() { - self.run_ip6tables_rule_args(&tail_rules, logger)?; - } - } + let base_rules = Self::build_base_chain_rule_args(&self.chain_name); + self.run_iptables_rule_args(&base_rules, logger)?; + if ipv6_enabled { + self.run_ip6tables_rule_args(&base_rules, logger)?; + } - let policy_rules = Self::build_policy_rules_logged( - &self.chain_name, - policy, - uses_directional_keys, - logger, - )?; - self.run_iptables_rule_args(&policy_rules.ipv4, logger)?; - if ipv6_enabled { - self.run_ip6tables_rule_args(&policy_rules.ipv6, logger)?; - } else if !policy_rules.ipv6.is_empty() { - logger.log_line(&format!( - "Warning: {} IPv6 firewall rule(s) not applied because ip6tables \ - is unavailable; IPv6 egress is unfiltered on this host.", - policy_rules.ipv6.len() - )); - } + let policy_rules = Self::build_policy_rules_logged(&self.chain_name, policy, logger)?; + self.run_iptables_rule_args(&policy_rules.ipv4, logger)?; + if ipv6_enabled { + self.run_ip6tables_rule_args(&policy_rules.ipv6, logger)?; + } else if !policy_rules.ipv6.is_empty() { + logger.log_line(&format!( + "Warning: {} IPv6 firewall rule(s) not applied because ip6tables \ + is unavailable; IPv6 egress is unfiltered on this host.", + policy_rules.ipv6.len() + )); } let default_rule = Self::build_default_policy_rule_arg( &self.chain_name, - Self::effective_default_policy(policy, uses_directional_keys), - proxy_mode, + Self::effective_default_policy(policy), ); let default_args: Vec<&str> = default_rule.iter().map(String::as_str).collect(); let default_action = default_args.last().copied().unwrap_or("ACCEPT"); @@ -1748,14 +1200,6 @@ impl Drop for NetworkIptablesManager { } } -#[cfg(test)] -#[path = "network_iptables_deny_precedence_spec.rs"] -mod deny_precedence_spec; - -#[cfg(test)] -#[path = "network_iptables_proxy_spec.rs"] -mod proxy_spec; - #[cfg(test)] #[path = "network_iptables_ga_egress_spec.rs"] mod ga_egress_spec; @@ -1872,15 +1316,15 @@ pub(crate) mod test_firewall { mod tests { use super::*; use crate::mxc_common::logger::{Logger, Mode}; - use crate::mxc_common::models::{ - ContainerPolicy, NetworkEnforcementMode, ProxyAddress, ProxyConfig, - }; + use crate::mxc_common::models::{ContainerPolicy, NetworkEgressPolicy}; use std::io::{Error, ErrorKind}; - fn policy_requesting_mode(mode: NetworkEnforcementMode) -> ContainerPolicy { + fn filtered_policy() -> ContainerPolicy { ContainerPolicy { - network_enforcement_mode: mode, - allowed_hosts: vec!["203.0.113.7".to_string()], + network_egress: Some(NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }), ..Default::default() } } @@ -1889,24 +1333,7 @@ mod tests { fn an_unhooked_caller_is_not_refused_in_firewall_mode() { let _fake = super::test_firewall::install(); let mut manager = NetworkIptablesManager::new("bwrap-nonetns", EgressHookPoint::Unhooked); - let policy = policy_requesting_mode(NetworkEnforcementMode::Firewall); - let mut logger = Logger::new(Mode::Buffer); - - let result = manager.apply_firewall_rules(&policy, &mut logger); - - assert!( - result.is_ok(), - "a caller with no namespace to enforce in must not be failed closed, got {:?}", - result - ); - } - - #[test] - fn an_unhooked_caller_is_not_refused_in_both_mode() { - let _fake = super::test_firewall::install(); - let mut manager = - NetworkIptablesManager::new("bwrap-nonetns-both", EgressHookPoint::Unhooked); - let policy = policy_requesting_mode(NetworkEnforcementMode::Both); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); let result = manager.apply_firewall_rules(&policy, &mut logger); @@ -1923,7 +1350,7 @@ mod tests { let fake = super::test_firewall::install(); let mut manager = NetworkIptablesManager::new("lxc-forget", EgressHookPoint::ContainerNetns(4242)); - let policy = policy_requesting_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); manager @@ -1956,7 +1383,7 @@ mod tests { let fake = super::test_firewall::install(); let mut manager = NetworkIptablesManager::new("lxc-hooked", EgressHookPoint::ContainerNetns(4242)); - let policy = policy_requesting_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); manager @@ -1988,7 +1415,7 @@ mod tests { fn an_unhooked_manager_installs_no_output_hook() { let fake = super::test_firewall::install(); let mut manager = NetworkIptablesManager::new("bwrap-nohook", EgressHookPoint::Unhooked); - let policy = policy_requesting_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); manager @@ -2319,7 +1746,7 @@ mod tests { NetworkIptablesManager::new("already-owned", EgressHookPoint::ContainerNetns(4242)); manager.retain_residual_ownership(CreatedResources::for_test(true, false, false, false)); - let policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); let result = manager.apply_firewall_rules(&policy, &mut logger); @@ -2339,7 +1766,7 @@ mod tests { let fake = test_firewall::install(); let mut manager = NetworkIptablesManager::new("fresh", EgressHookPoint::ContainerNetns(4242)); - let policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); let result = manager.apply_firewall_rules(&policy, &mut logger); @@ -2390,65 +1817,25 @@ mod tests { assert!(mgr.chain_name.len() <= CHAIN_NAME_MAX_LEN); } - #[test] - fn resolve_ip_address() { - let ips = NetworkIptablesManager::resolve_host("127.0.0.1"); - assert_eq!(ips.ipv4, vec!["127.0.0.1"]); - assert!(ips.ipv6.is_empty()); - } - - #[test] - fn resolve_host_retains_ipv6_literal() { - let ips = NetworkIptablesManager::resolve_host("::1"); - assert!(ips.ipv4.is_empty()); - assert_eq!(ips.ipv6, vec!["::1"]); - } - - #[test] - fn resolve_host_rewrites_ipv4_mapped_ipv6_literal_to_ipv4() { - let ips = NetworkIptablesManager::resolve_host("::ffff:127.0.0.1"); - assert_eq!(ips.ipv4, vec!["127.0.0.1"]); - assert!(ips.ipv6.is_empty()); - } - - #[test] - fn resolve_host_keeps_ipv4_literal_unchanged() { - let ips = NetworkIptablesManager::resolve_host("10.0.0.1"); - assert_eq!(ips.ipv4, vec!["10.0.0.1"]); - assert!(ips.ipv6.is_empty()); - } - - #[test] - fn resolve_host_retains_valid_cidr_by_family() { - let v4 = NetworkIptablesManager::resolve_host("140.82.112.0/20"); - assert_eq!(v4.ipv4, vec!["140.82.112.0/20"]); - assert!(v4.ipv6.is_empty()); - - let v6 = NetworkIptablesManager::resolve_host("2606:50c0::/32"); - assert!(v6.ipv4.is_empty()); - assert_eq!(v6.ipv6, vec!["2606:50c0::/32"]); - } - - #[test] - fn resolve_host_rejects_invalid_cidr_prefix() { - assert!(NetworkIptablesManager::resolve_host("140.82.112.0/33").is_empty()); - assert!(NetworkIptablesManager::resolve_host("2606:50c0::/129").is_empty()); - assert!(NetworkIptablesManager::resolve_host("140.82.112.0/not-a-prefix").is_empty()); - } - - #[test] - fn resolve_host_rejects_malformed_cidr_syntax() { - assert!(NetworkIptablesManager::resolve_host("/20").is_empty()); - assert!(NetworkIptablesManager::resolve_host("140.82.112.0/").is_empty()); - assert!(NetworkIptablesManager::resolve_host("140.82.112.0/20/8").is_empty()); + fn cidr(value: &str) -> NetworkCidr { + let (address, prefix) = value + .split_once('/') + .expect("fixture must include a prefix"); + NetworkCidr { + address: address.parse().expect("fixture must include an IP address"), + prefix_length: prefix + .parse() + .expect("fixture must include a numeric prefix"), + } } #[test] fn host_rule_args_route_ipv4_to_iptables_args() { - let args = NetworkIptablesManager::build_host_rule_args( + let args = NetworkIptablesManager::build_destination_rule_args( "MXC-test", - "140.82.112.4", + &cidr("140.82.112.4/32"), &RuleAction::Allow, + RuleMatch::AnyTraffic, ); assert_eq!( @@ -2457,7 +1844,7 @@ mod tests { "-A", "MXC-test", "-d", - "140.82.112.4", + "140.82.112.4/32", "-j", "ACCEPT", ])] @@ -2467,10 +1854,11 @@ mod tests { #[test] fn host_rule_args_route_ipv6_to_ip6tables_args() { - let args = NetworkIptablesManager::build_host_rule_args( + let args = NetworkIptablesManager::build_destination_rule_args( "MXC-test", - "2606:50c0:8000::64", + &cidr("2606:50c0:8000::64/128"), &RuleAction::Deny, + RuleMatch::AnyTraffic, ); assert!(args.ipv4.is_empty()); @@ -2480,7 +1868,7 @@ mod tests { "-A", "MXC-test", "-d", - "2606:50c0:8000::64", + "2606:50c0:8000::64/128", "-j", "DROP", ])] @@ -2489,10 +1877,11 @@ mod tests { #[test] fn host_rule_args_pass_cidr_through_unchanged() { - let v4 = NetworkIptablesManager::build_host_rule_args( + let v4 = NetworkIptablesManager::build_destination_rule_args( "MXC-test", - "140.82.112.0/20", + &cidr("140.82.112.0/20"), &RuleAction::Allow, + RuleMatch::AnyTraffic, ); assert_eq!( v4.ipv4, @@ -2507,10 +1896,11 @@ mod tests { ); assert!(v4.ipv6.is_empty()); - let v6 = NetworkIptablesManager::build_host_rule_args( + let v6 = NetworkIptablesManager::build_destination_rule_args( "MXC-test", - "2606:50c0::/32", + &cidr("2606:50c0::/32"), &RuleAction::Allow, + RuleMatch::AnyTraffic, ); assert!(v6.ipv4.is_empty()); assert_eq!( @@ -2527,366 +1917,84 @@ mod tests { } #[test] - fn host_rule_args_drop_unresolvable_destination() { - let args = NetworkIptablesManager::build_host_rule_args( + fn direct_mapped_ipv6_cidr_uses_ipv4_destination_in_ipv4_chain() { + let args = NetworkIptablesManager::build_destination_rule_args( "MXC-test", - "140.82.112.0/33", + &cidr("::ffff:192.0.2.0/120"), &RuleAction::Allow, + RuleMatch::AnyTraffic, ); - assert!(args.ipv4.is_empty()); + assert_eq!( + args.ipv4, + vec![strings(&[ + "-A", + "MXC-test", + "-d", + "192.0.2.0/24", + "-j", + "ACCEPT", + ])] + ); assert!(args.ipv6.is_empty()); } #[test] - fn build_policy_rule_args_splits_allow_and_block_lists_by_family() { - let policy = ContainerPolicy { - allowed_hosts: vec!["140.82.112.0/20".to_string(), "2606:50c0::/32".to_string()], - blocked_hosts: vec!["10.0.0.0/8".to_string(), "2001:db8::/32".to_string()], - ..Default::default() - }; - - let args = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, false); - - let expected_v4 = vec![ - strings(&["-A", "MXC-test", "-d", "140.82.112.0/20", "-j", "ACCEPT"]), - strings(&["-A", "MXC-test", "-d", "10.0.0.0/8", "-j", "DROP"]), - ]; - let expected_v6 = vec![ - strings(&["-A", "MXC-test", "-d", "2606:50c0::/32", "-j", "ACCEPT"]), - strings(&["-A", "MXC-test", "-d", "2001:db8::/32", "-j", "DROP"]), - ]; + fn invalid_typed_cidr_does_not_program_a_rule() { + let args = NetworkIptablesManager::build_destination_rule_args( + "MXC-test", + &NetworkCidr { + address: IpAddr::from([140, 82, 112, 0]), + prefix_length: 33, + }, + &RuleAction::Allow, + RuleMatch::AnyTraffic, + ); - assert_eq!(args.ipv4.len(), expected_v4.len()); - for rule in &expected_v4 { - assert!( - args.ipv4.contains(rule), - "IPv4 rules should contain {rule:?}; actual: {:?}", - args.ipv4 - ); - } - assert_eq!(args.ipv6.len(), expected_v6.len()); - for rule in &expected_v6 { - assert!( - args.ipv6.contains(rule), - "IPv6 rules should contain {rule:?}; actual: {:?}", - args.ipv6 - ); - } + assert!(args.ipv4.is_empty()); + assert!(args.ipv6.is_empty()); } #[test] fn base_chain_rule_args_are_family_agnostic() { let base = NetworkIptablesManager::build_base_chain_rule_args("MXC-test"); - let dns = NetworkIptablesManager::build_dns_resolution_rule_args("MXC-test"); assert_eq!(base.len(), 2); - assert_eq!(dns.len(), 2); - for rule in base.iter().chain(dns.iter()) { + for rule in &base { assert!(!rule.iter().any(|arg| arg == "icmp")); assert!(!rule.iter().any(|arg| arg == "icmpv6")); } } - fn assert_resolved_exact(input: &str, expected_ipv4: &[&str], expected_ipv6: &[&str]) { - let resolved = NetworkIptablesManager::resolve_host(input); - let expected_ipv4: Vec = expected_ipv4 - .iter() - .map(|value| value.to_string()) - .collect(); - let expected_ipv6: Vec = expected_ipv6 - .iter() - .map(|value| value.to_string()) - .collect(); - - assert_eq!( - resolved.ipv4, expected_ipv4, - "unexpected IPv4 destinations for {input:?}" - ); - assert_eq!( - resolved.ipv6, expected_ipv6, - "unexpected IPv6 destinations for {input:?}" - ); - } - - fn assert_destination_family(input: &str, expected: Option) { - assert_eq!( - NetworkIptablesManager::destination_family(input), - expected, - "unexpected destination family for {input:?}" - ); - } - - #[test] - fn bare_ip_literals_are_routed_only_to_their_matching_family() { - let cases = [ - ("192.0.2.1", &["192.0.2.1"][..], &[][..]), - ("127.0.0.1", &["127.0.0.1"][..], &[][..]), - ("2606:50c0::153", &[][..], &["2606:50c0::153"][..]), - ( - "2606:50c0:0000:0000:0000:0000:0000:0153", - &[][..], - &["2606:50c0:0000:0000:0000:0000:0000:0153"][..], - ), - ("::1", &[][..], &["::1"][..]), - ]; - - for (input, expected_ipv4, expected_ipv6) in cases { - assert_resolved_exact(input, expected_ipv4, expected_ipv6); - } - } - - #[test] - fn ipv4_mapped_ipv6_literal_is_filed_as_ipv4() { - assert_resolved_exact("::ffff:127.0.0.1", &["127.0.0.1"], &[]); - } - - #[test] - fn ipv4_mapped_cidr_is_translated_to_its_ipv4_prefix() { - assert_resolved_exact("::ffff:192.0.2.0/120", &["192.0.2.0/24"], &[]); - assert_resolved_exact("::ffff:198.51.100.42/128", &["198.51.100.42/32"], &[]); - } - - #[test] - fn an_ipv6_prefix_shorter_than_the_mapped_range_stays_ipv6() { - assert_resolved_exact("::ffff:0:0/95", &[], &["::ffff:0:0/95"]); - } - - #[test] - fn valid_cidrs_are_passed_through_unchanged_in_their_matching_family() { - let cases = [ - ("140.82.112.0/20", &["140.82.112.0/20"][..], &[][..]), - ("2606:50c0::/32", &[][..], &["2606:50c0::/32"][..]), - ]; - - for (input, expected_ipv4, expected_ipv6) in cases { - assert_resolved_exact(input, expected_ipv4, expected_ipv6); - } - } - - #[test] - fn v4_cidr_with_host_bits_set_is_passed_through_unchanged() { - assert_resolved_exact("140.82.112.5/20", &["140.82.112.5/20"], &[]); - } - - #[test] - fn cidr_prefix_lengths_accept_only_family_specific_bounds() { - let cases = [ - ("0.0.0.0/0", Some(IpFamily::V4), &["0.0.0.0/0"][..], &[][..]), - ( - "192.0.2.1/32", - Some(IpFamily::V4), - &["192.0.2.1/32"][..], - &[][..], - ), - ("192.0.2.1/33", None, &[][..], &[][..]), - ("192.0.2.1/129", None, &[][..], &[][..]), - ("::/0", Some(IpFamily::V6), &[][..], &["::/0"][..]), - ( - "2001:db8::1/128", - Some(IpFamily::V6), - &[][..], - &["2001:db8::1/128"][..], - ), - ("2001:db8::1/129", None, &[][..], &[][..]), - ]; - - for (input, expected_family, expected_ipv4, expected_ipv6) in cases { - assert_resolved_exact(input, expected_ipv4, expected_ipv6); - assert_destination_family(input, expected_family); - } - } - - #[test] - fn v6_prefix_length_on_v4_address_is_rejected() { - assert_resolved_exact("10.0.0.0/64", &[], &[]); - assert_destination_family("10.0.0.0/64", None); - } - - #[test] - fn malformed_cidr_syntax_and_garbage_resolve_to_nothing() { - let cases = [ - "/24", - "10.0.0.0/", - "10.0.0.0//24", - "10.0.0.0/abc", - "10.0.0.0/-1", - "10.0.0.0/ 24", - "not-a-valid-firewall-destination", - ]; - - for input in cases { - let resolved = NetworkIptablesManager::resolve_host(input); - assert!( - resolved.is_empty(), - "malformed destination {input:?} should resolve to nothing, got {resolved:?}" - ); - assert_destination_family(input, None); - } - } - - #[test] - fn cidr_prefix_with_plus_sign_resolves_to_nothing() { - let input = "10.0.0.0/+24"; - let resolved = NetworkIptablesManager::resolve_host(input); - assert!( - resolved.is_empty(), - "malformed destination {input:?} should resolve to nothing, got {resolved:?}" - ); - assert_destination_family(input, None); - } - - #[test] - fn leading_plus_does_not_smuggle_an_out_of_range_prefix_past_validation() { - let input = "10.0.0.0/+33"; - let resolved = NetworkIptablesManager::resolve_host(input); - assert!( - resolved.is_empty(), - "a leading `+` must not smuggle an out-of-range prefix past validation, got {resolved:?}" - ); - assert_destination_family(input, None); - } - #[test] - fn empty_input_resolves_to_nothing() { - let resolved = NetworkIptablesManager::resolve_host(""); - assert!( - resolved.is_empty(), - "empty input should resolve to nothing, got {resolved:?}" - ); - assert_destination_family("", None); - } - - fn assert_buckets_are_family_pure(input: &str, resolved: &ResolvedDestinations) { - for destination in &resolved.ipv4 { - assert_eq!( - NetworkIptablesManager::destination_family(destination), - Some(IpFamily::V4), - "{input:?}: {destination:?} is in the ipv4 bucket but is not an IPv4 destination" - ); - } - for destination in &resolved.ipv6 { - assert_eq!( - NetworkIptablesManager::destination_family(destination), - Some(IpFamily::V6), - "{input:?}: {destination:?} is in the ipv6 bucket but is not an IPv6 destination" + fn typed_cidrs_route_to_their_firewall_family() { + for (input, expected, is_v4) in [ + ("192.0.2.0/24", "192.0.2.0/24", true), + ("2001:db8::/32", "2001:db8::/32", false), + ("::ffff:192.0.2.0/120", "192.0.2.0/24", true), + ("::ffff:0:0/95", "::fffe:0:0/95", false), + ] { + let peer = NetworkPeer { + cidr: cidr(input), + except: Vec::new(), + }; + let lowered = + NetworkIptablesManager::peer_destinations(&peer).expect("valid CIDR should lower"); + assert_eq!(lowered.len(), 1); + let args = NetworkIptablesManager::build_destination_rule_args( + "MXC-test", + &lowered[0], + &RuleAction::Allow, + RuleMatch::AnyTraffic, ); - } - } - - #[test] - fn aaaa_records_land_in_the_v6_bucket_and_never_in_the_v4_bucket() { - let injected: Vec = [ - "93.184.216.34", - "2606:2800:220:1:248:1893:25c8:1946", - "8.8.8.8", - "2001:4860:4860::8888", - ] - .iter() - .map(|value| { - value - .parse::() - .expect("injected test address must parse") - }) - .collect(); - - let resolved = NetworkIptablesManager::bucket_resolved_addrs(injected); - - assert_eq!( - resolved.ipv4.len(), - 2, - "both injected A records must land in the v4 bucket, got {:?}", - resolved.ipv4 - ); - assert_eq!( - resolved.ipv6.len(), - 2, - "both injected AAAA records must land in the v6 bucket, got {:?}", - resolved.ipv6 - ); - assert!( - !resolved.ipv6.is_empty(), - "AAAA records must produce at least one v6 destination; an empty v6 \ - bucket means the IPv6 arm was dropped or misrouted into the v4 bucket" - ); - assert_buckets_are_family_pure("injected A/AAAA mix", &resolved); - } - - #[test] - fn live_dual_stack_resolution_keeps_buckets_family_pure() { - for host in ["dns.google", "one.one.one.one", "localhost"] { - let resolved = NetworkIptablesManager::resolve_host(host); - assert_buckets_are_family_pure(host, &resolved); - } - } - - #[test] - fn localhost_resolution_populates_available_loopback_families() { - let resolved = NetworkIptablesManager::resolve_host("localhost"); - - // A minimal host can have a degenerate /etc/hosts. Accept whichever - // localhost family is configured while checking that no other address leaks in. - assert!( - !resolved.is_empty(), - "localhost should resolve to at least one loopback family" - ); - assert!( - resolved - .ipv4 - .iter() - .all(|destination| destination == "127.0.0.1"), - "localhost IPv4 results should all be 127.0.0.1, got {:?}", - resolved.ipv4 - ); - assert!( - resolved.ipv6.iter().all(|destination| destination == "::1"), - "localhost IPv6 results should all be ::1, got {:?}", - resolved.ipv6 - ); - assert_buckets_are_family_pure("localhost", &resolved); - } - - #[test] - fn unresolvable_invalid_tld_hostname_resolves_to_nothing() { - let input = "mxc-resolution-spec-7f3b2d9c4a1e6f80.invalid"; - let resolved = NetworkIptablesManager::resolve_host(input); - - assert!( - resolved.is_empty(), - "reserved .invalid hostname {input:?} should resolve to nothing, got {resolved:?}" - ); - assert_destination_family(input, None); - } - - #[test] - fn destination_family_agrees_with_every_resolved_destination() { - let inputs = [ - "192.0.2.44", - "2606:50c0::153", - "140.82.112.5/20", - "2606:50c0::/32", - "::ffff:127.0.0.1", - "localhost", - ]; - - for input in inputs { - let resolved = NetworkIptablesManager::resolve_host(input); - - for destination in &resolved.ipv4 { - assert_eq!( - NetworkIptablesManager::destination_family(destination), - Some(IpFamily::V4), - "destination_family disagreed with IPv4 filing for input {input:?}, destination {destination:?}" - ); - } - - for destination in &resolved.ipv6 { - assert_eq!( - NetworkIptablesManager::destination_family(destination), - Some(IpFamily::V6), - "destination_family disagreed with IPv6 filing for input {input:?}, destination {destination:?}" - ); - } + let (present, absent) = if is_v4 { + (&args.ipv4, &args.ipv6) + } else { + (&args.ipv6, &args.ipv4) + }; + assert!(absent.is_empty(), "wrong family for {input}: {args:?}"); + assert_eq!(present.len(), 1, "missing {input}: {args:?}"); + assert!(present[0].contains(&expected.to_string())); } } @@ -2908,88 +2016,6 @@ mod tests { ); } - fn policy_with_hosts(allowed_hosts: &[&str], blocked_hosts: &[&str]) -> ContainerPolicy { - ContainerPolicy { - allowed_hosts: strings(allowed_hosts), - blocked_hosts: strings(blocked_hosts), - ..Default::default() - } - } - - // `.invalid` is reserved by RFC 2606 and never resolves. - const UNRESOLVABLE_HOST: &str = "blocked.invalid"; - - #[test] - fn an_unresolvable_deny_under_a_blocking_default_is_fatal_beside_a_catch_all_allow() { - let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - ..policy_with_hosts(&["0.0.0.0/0"], &[UNRESOLVABLE_HOST]) - }; - let mut logger = - crate::mxc_common::logger::Logger::new(crate::mxc_common::logger::Mode::Buffer); - - let err = - NetworkIptablesManager::build_policy_rules_logged("MXC-x", &policy, false, &mut logger) - .expect_err("a catch-all allow must not be able to accept an unresolvable deny"); - - assert!( - err.contains(UNRESOLVABLE_HOST) && err.contains("deny precedence"), - "error should name the host and the invariant, got: {err}" - ); - } - - #[test] - fn an_ipv6_catch_all_allow_also_arms_the_deny_precedence_failure() { - let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - ..policy_with_hosts(&["::/0"], &[UNRESOLVABLE_HOST]) - }; - let mut logger = - crate::mxc_common::logger::Logger::new(crate::mxc_common::logger::Mode::Buffer); - - NetworkIptablesManager::build_policy_rules_logged("MXC-x", &policy, false, &mut logger) - .expect_err("a v6 catch-all allow accepts the unresolved deny just as a v4 one does"); - } - - #[test] - fn an_unresolvable_deny_beside_a_bounded_allow_stays_a_warning() { - let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - ..policy_with_hosts(&["192.0.2.10"], &[UNRESOLVABLE_HOST]) - }; - let mut logger = - crate::mxc_common::logger::Logger::new(crate::mxc_common::logger::Mode::Buffer); - - NetworkIptablesManager::build_policy_rules_logged("MXC-x", &policy, false, &mut logger) - .expect("a bounded allow leaves the closing DROP covering the unresolved deny"); - } - - #[test] - fn a_bounded_cidr_allow_is_not_mistaken_for_a_catch_all() { - let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - ..policy_with_hosts(&["192.0.2.0/24"], &[UNRESOLVABLE_HOST]) - }; - let mut logger = - crate::mxc_common::logger::Logger::new(crate::mxc_common::logger::Mode::Buffer); - - NetworkIptablesManager::build_policy_rules_logged("MXC-x", &policy, false, &mut logger) - .expect("a /24 allow covers a bounded set, so it proves nothing about the deny"); - } - - #[test] - fn an_unresolvable_allow_does_not_arm_the_deny_precedence_failure() { - let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - ..policy_with_hosts(&["allowed.invalid"], &[UNRESOLVABLE_HOST]) - }; - let mut logger = - crate::mxc_common::logger::Logger::new(crate::mxc_common::logger::Mode::Buffer); - - NetworkIptablesManager::build_policy_rules_logged("MXC-x", &policy, false, &mut logger) - .expect("an allow that programs no rule cannot accept the unresolved deny"); - } - #[test] fn allow_and_deny_actions_map_to_exact_iptables_jump_targets() { assert_eq!( @@ -3005,19 +2031,25 @@ mod tests { } #[test] - fn destination_literals_and_cidrs_land_only_in_their_address_family_bucket() { + fn cidrs_land_only_in_their_address_family_bucket() { let cases = [ - ("192.0.2.10", "ipv4 bare literal", true), - ("192.0.2.10/24", "ipv4 CIDR", true), - ("2001:db8::10", "ipv6 bare literal", false), - ("2001:db8::10/64", "ipv6 CIDR", false), + ("192.0.2.10/32", "192.0.2.10/32", "ipv4 host CIDR", true), + ("192.0.2.0/24", "192.0.2.0/24", "ipv4 CIDR", true), + ( + "2001:db8::10/128", + "2001:db8::10/128", + "ipv6 host CIDR", + false, + ), + ("2001:db8::10/64", "2001:db8::/64", "ipv6 CIDR", false), ]; - for (destination, label, is_ipv4) in cases { - let rules = NetworkIptablesManager::build_host_rule_args( + for (destination, expected, label, is_ipv4) in cases { + let rules = NetworkIptablesManager::build_destination_rule_args( "MXC-family-split", - destination, + &cidr(destination), &RuleAction::Allow, + RuleMatch::AnyTraffic, ); if is_ipv4 { @@ -3030,7 +2062,7 @@ mod tests { rules.ipv6.is_empty(), "{label} {destination} should leave IPv6 rules empty; actual: {rules:?}" ); - assert_rule_contains(&rules.ipv4[0], destination, destination); + assert_rule_contains(&rules.ipv4[0], expected, destination); } else { assert!( rules.ipv4.is_empty(), @@ -3041,36 +2073,11 @@ mod tests { 1, "{label} {destination} should produce one IPv6 rule; actual: {rules:?}" ); - assert_rule_contains(&rules.ipv6[0], destination, destination); + assert_rule_contains(&rules.ipv6[0], expected, destination); } } } - #[test] - fn mixed_family_host_list_produces_matching_rule_count_in_each_bucket() { - let policy = policy_with_hosts( - &[ - "192.0.2.10", - "198.51.100.0/24", - "2001:db8::10", - "2001:db8:abcd::/48", - ], - &[], - ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-mixed", &policy, false); - - assert_eq!( - rules.ipv4.len(), - 2, - "mixed host list should produce two IPv4 rules; actual: {rules:?}" - ); - assert_eq!( - rules.ipv6.len(), - 2, - "mixed host list should produce two IPv6 rules; actual: {rules:?}" - ); - } - #[test] fn generated_destination_rules_append_to_chain_match_destination_and_jump_target() { let chain_name = "MXC-shape"; @@ -3109,68 +2116,10 @@ mod tests { ); } - #[test] - fn resolved_destinations_are_split_into_ipv4_and_ipv6_rule_args() { - let destinations = ResolvedDestinations { - ipv4: strings(&["192.0.2.10", "198.51.100.0/24"]), - ipv6: strings(&["2001:db8::10", "2001:db8:abcd::/48"]), - }; - let rules = NetworkIptablesManager::build_resolved_destination_rule_args( - "MXC-resolved", - &destinations, - &RuleAction::Allow, - RuleMatch::AnyTraffic, - ); - - assert_eq!( - rules.ipv4.len(), - 2, - "resolved destinations should keep both IPv4 rules in IPv4 bucket; actual: {rules:?}" - ); - assert_eq!( - rules.ipv6.len(), - 2, - "resolved destinations should keep both IPv6 rules in IPv6 bucket; actual: {rules:?}" - ); - for destination in &destinations.ipv4 { - assert!( - rules.ipv4.iter().any(|rule| rule.contains(destination)), - "IPv4 destination {destination} should appear in IPv4 rules; actual: {rules:?}" - ); - assert!( - !rules.ipv6.iter().any(|rule| rule.contains(destination)), - "IPv4 destination {destination} should not appear in IPv6 rules; actual: {rules:?}" - ); - } - for destination in &destinations.ipv6 { - assert!( - rules.ipv6.iter().any(|rule| rule.contains(destination)), - "IPv6 destination {destination} should appear in IPv6 rules; actual: {rules:?}" - ); - assert!( - !rules.ipv4.iter().any(|rule| rule.contains(destination)), - "IPv6 destination {destination} must not appear in IPv4 rules; actual: {rules:?}" - ); - } - } - #[test] fn no_egress_rule_selects_an_incoming_interface() { let chain_name = "MXC-sel"; - let endpoints = vec![ProxyEndpoint { - ip: "10.0.3.1".to_string(), - port: 3128, - }]; - let mut rules = NetworkIptablesManager::build_base_chain_rule_args(chain_name); - rules.extend(NetworkIptablesManager::build_dns_resolution_rule_args( - chain_name, - )); - rules.extend(NetworkIptablesManager::build_proxy_chain_rule_args( - chain_name, &endpoints, - )); - rules.push(NetworkIptablesManager::build_loopback_accept_rule_args( - chain_name, - )); + let rules = NetworkIptablesManager::build_base_chain_rule_args(chain_name); for rule in &rules { assert!( @@ -3229,48 +2178,18 @@ mod tests { } #[test] - fn dns_resolution_is_the_documented_udp_then_tcp_pair() { - let chain_name = "MXC-base"; - let rules = NetworkIptablesManager::build_dns_resolution_rule_args(chain_name); - let expected = vec![ - strings(&[ - "-A", chain_name, "-p", "udp", "--dport", "53", "-j", "ACCEPT", - ]), - strings(&[ - "-A", chain_name, "-p", "tcp", "--dport", "53", "-j", "ACCEPT", - ]), - ]; - - assert_eq!( - rules, expected, - "the DNS resolution grant should be the documented udp/tcp pair" - ); - for (index, rule) in rules.iter().enumerate() { - assert_rule_omits(rule, "-d", &format!("dns rule {index}")); - } - } - - #[test] - fn default_network_policy_maps_to_exact_terminal_rule_vector() { + fn directional_default_maps_to_exact_terminal_rule_vector() { let chain_name = "MXC-default"; assert_eq!( - NetworkIptablesManager::build_default_policy_rule_arg( - chain_name, - NetworkPolicy::Block, - false - ), + NetworkIptablesManager::build_default_policy_rule_arg(chain_name, NetworkAction::Deny), strings(&["-A", chain_name, "-j", "DROP"]), - "NetworkPolicy::Block should produce the exact DROP terminal rule" + "egress.default=deny should produce the exact DROP terminal rule" ); assert_eq!( - NetworkIptablesManager::build_default_policy_rule_arg( - chain_name, - NetworkPolicy::Allow, - false - ), + NetworkIptablesManager::build_default_policy_rule_arg(chain_name, NetworkAction::Allow), strings(&["-A", chain_name, "-j", "ACCEPT"]), - "NetworkPolicy::Allow should produce the exact ACCEPT terminal rule" + "egress.default=allow should produce the exact ACCEPT terminal rule" ); } @@ -3301,8 +2220,8 @@ mod tests { #[test] fn empty_policy_produces_no_destination_rules_in_either_bucket() { - let policy = policy_with_hosts(&[], &[]); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-empty", &policy, false); + let policy = ContainerPolicy::default(); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-empty", &policy); assert!( rules.ipv4.is_empty(), @@ -3314,22 +2233,6 @@ mod tests { ); } - #[test] - fn unresolvable_invalid_hostname_contributes_no_destination_rules() { - let host = "definitely-unresolvable-mxc-rulegen-spec.invalid"; - let rules = - NetworkIptablesManager::build_host_rule_args("MXC-invalid", host, &RuleAction::Allow); - - assert!( - rules.ipv4.is_empty(), - "unresolvable host {host} should produce no IPv4 rules; actual: {rules:?}" - ); - assert!( - rules.ipv6.is_empty(), - "unresolvable host {host} should produce no IPv6 rules; actual: {rules:?}" - ); - } - #[test] fn a_new_manager_reports_no_rules_applied() { let manager = NetworkIptablesManager::new("fresh", EgressHookPoint::ContainerNetns(4242)); @@ -3345,8 +2248,7 @@ mod tests { let mut manager = NetworkIptablesManager::new("skip-noop", EgressHookPoint::ContainerNetns(4242)); let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - default_network_policy: NetworkPolicy::Block, + network_egress: Some(NetworkEgressPolicy::default()), ..Default::default() }; let mut logger = Logger::new(Mode::Buffer); @@ -3365,74 +2267,8 @@ mod tests { } #[test] - fn a_legacy_policy_naming_reachable_hosts_installs_the_firewall() { - for mode in [ - NetworkEnforcementMode::Firewall, - NetworkEnforcementMode::Both, - ] { - let label = format!("{mode:?}"); - let policy = ContainerPolicy { - network_enforcement_mode: mode, - allowed_hosts: vec!["example.com".to_string()], - ..Default::default() - }; - - assert!( - plan_network(&policy).installs_firewall(), - "{label}: a 0.7 policy naming hosts it may reach must install the chain" - ); - } - } - - #[test] - fn a_named_host_list_always_installs_the_chain() { - for (allowed, blocked) in [ - (&["140.82.112.0/20"][..], &[][..]), - (&[][..], &["140.82.112.0/20"][..]), - ] { - for default_policy in [NetworkPolicy::Block, NetworkPolicy::Allow] { - let policy = ContainerPolicy { - default_network_policy: default_policy.clone(), - ..policy_with_hosts(allowed, blocked) - }; - - let plan = plan_network(&policy); - - assert!( - plan.installs_firewall(), - "a policy naming hosts to allow or block states a restriction, and \ - the default enforcement mode must not discard it; \ - allowed={allowed:?} blocked={blocked:?} default={default_policy:?} \ - gave {plan:?}" - ); - } - } - } - - #[test] - fn a_legacy_policy_that_permits_nothing_is_given_no_interface() { - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - network_mode_specified: true, - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - - assert_eq!( - plan_network(&policy), - NetworkPlan::Isolated, - "a policy that blocks outbound, allows no local network, and names no \ - proxy permits nothing, so the container is given no interface rather \ - than an unfiltered one" - ); - } - - #[test] - fn a_directional_policy_installs_the_firewall_under_the_capabilities_default() { + fn a_directional_policy_installs_the_firewall() { let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Capabilities, - network_mode_specified: true, - default_network_policy: NetworkPolicy::Allow, network_egress: Some(NetworkEgressPolicy { default: NetworkAction::Allow, ..Default::default() @@ -3442,13 +2278,12 @@ mod tests { assert!( plan_network(&policy).installs_firewall(), - "a stated 0.8 posture must install the firewall even though enforcementMode \ - is absent from the 0.8 schema and defaults to capabilities" + "a stated directional posture permitting traffic must install the firewall" ); } #[test] - fn a_v08_request_naming_no_network_fields_is_given_no_interface() { + fn a_directional_request_naming_no_network_fields_is_given_no_interface() { let policy = ContainerPolicy { network_egress: Some(NetworkEgressPolicy::default()), network_ingress: Some(crate::mxc_common::models::NetworkIngressPolicy::default()), @@ -3488,24 +2323,10 @@ mod tests { assert!(!plan_network(&policy).omits_interface()); } - #[test] - fn a_legacy_proxy_policy_installs_the_chain_and_needs_the_network() { - let mut policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); - policy.allowed_hosts.clear(); - policy.network_proxy = ProxyConfig { - address: Some(ProxyAddress::new("10.0.0.5".to_string(), 3128)), - builtin_test_server: false, - }; - - assert!(plan_network(&policy).installs_firewall()); - assert!(needs_network(&policy)); - } - // Alpine's DHCP lease arrives around ten seconds after LXC marks the container running. #[test] fn a_plan_that_starts_an_interface_demands_an_address() { - let mut policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); - policy.default_network_policy = NetworkPolicy::Allow; + let policy = filtered_policy(); assert!( !plan_network(&policy).omits_interface(), @@ -3530,53 +2351,35 @@ mod tests { ]; let mut omitted = 0; - for directional in [false, true] { - for egress in egress_options { - for ingress in ingress_options { - for bits in 0u8..32 { - let policy = ContainerPolicy { - network_proxy: ProxyConfig { - builtin_test_server: bits & 1 != 0, - ..Default::default() - }, - allowed_hosts: if bits & 2 != 0 { - vec!["allowed.example".to_string()] + for egress in egress_options { + for ingress in ingress_options { + for allow_entry in [false, true] { + let policy = ContainerPolicy { + network_egress: egress.map(|default| NetworkEgressPolicy { + default, + allow: if allow_entry { + vec![NetworkRule::default()] } else { Vec::new() }, - blocked_hosts: if bits & 4 != 0 { - vec!["blocked.example".to_string()] - } else { - Vec::new() - }, - default_network_policy: if bits & 8 != 0 { - NetworkPolicy::Block - } else { - NetworkPolicy::Allow - }, - allow_local_network: bits & 16 != 0, - network_egress: egress.map(|default| NetworkEgressPolicy { - default, - ..Default::default() - }), - network_ingress: ingress.map(|(default, host_loopback)| { - crate::mxc_common::models::NetworkIngressPolicy { - default, - host_loopback, - } - }), ..Default::default() - }; - - if plan_network(&policy).omits_interface() { - omitted += 1; - assert!( - !needs_network(&policy), - "no interface means no address, yet this policy would treat a \ - missing address as fatal: directional={directional}, \ - egress={egress:?}, ingress={ingress:?}, bits={bits:05b}" - ); - } + }), + network_ingress: ingress.map(|(default, host_loopback)| { + crate::mxc_common::models::NetworkIngressPolicy { + default, + host_loopback, + } + }), + ..Default::default() + }; + + if plan_network(&policy).omits_interface() { + omitted += 1; + assert!( + !needs_network(&policy), + "no interface means no address: egress={egress:?}, \ + ingress={ingress:?}, allow_entry={allow_entry}" + ); } } } @@ -3588,43 +2391,6 @@ mod tests { ); } - #[test] - fn a_proxied_policy_installs_the_chain() { - let mut policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); - policy.allowed_hosts.clear(); - policy.network_proxy = ProxyConfig { - address: Some(ProxyAddress::new("10.0.0.5".to_string(), 3128)), - builtin_test_server: false, - }; - - assert!(plan_network(&policy).installs_firewall()); - } - - #[test] - fn the_builtin_test_server_proxy_installs_the_chain_the_same_way() { - let mut policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); - policy.allowed_hosts.clear(); - policy.network_proxy = ProxyConfig { - address: None, - builtin_test_server: true, - }; - - assert!( - plan_network(&policy).installs_firewall(), - "an address-free proxy is still a proxy and must not go unenforced" - ); - } - - fn policy_with_enforcement_mode( - network_enforcement_mode: NetworkEnforcementMode, - ) -> ContainerPolicy { - ContainerPolicy { - network_enforcement_mode, - allowed_hosts: vec!["203.0.113.7".to_string()], - ..Default::default() - } - } - #[test] fn working_probe_with_active_ipv6_reports_available() { let result = NetworkIptablesManager::classify_ip6tables_status(true, true); @@ -3934,7 +2700,7 @@ mod tests { let mut manager = NetworkIptablesManager::new("stranded", EgressHookPoint::ContainerNetns(4242)); - let policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); let outcome = manager.apply_firewall_rules(&policy, &mut logger); @@ -3962,7 +2728,7 @@ mod tests { let mut manager = NetworkIptablesManager::new("maybe-applied", EgressHookPoint::ContainerNetns(4242)); - let policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut logger = Logger::new(Mode::Buffer); let outcome = manager.apply_firewall_rules(&policy, &mut logger); @@ -3991,7 +2757,7 @@ mod tests { let mut manager = NetworkIptablesManager::new("still-hooked", EgressHookPoint::ContainerNetns(4242)); - let policy = policy_with_enforcement_mode(NetworkEnforcementMode::Firewall); + let policy = filtered_policy(); let mut apply_logger = Logger::new(Mode::Buffer); manager .apply_firewall_rules(&policy, &mut apply_logger) diff --git a/src/mxc-sdk/src/backends/lxc/common/network_iptables_deny_precedence_spec.rs b/src/mxc-sdk/src/backends/lxc/common/network_iptables_deny_precedence_spec.rs deleted file mode 100644 index 399991144..000000000 --- a/src/mxc-sdk/src/backends/lxc/common/network_iptables_deny_precedence_spec.rs +++ /dev/null @@ -1,563 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -use super::*; -use crate::mxc_common::logger::Mode; - -const CHAIN: &str = "mxc_test_chain"; - -fn as_str_slice(rule: &[String]) -> Vec<&str> { - rule.iter().map(String::as_str).collect() -} - -fn destination_of(rule: &[String]) -> &str { - let index = rule - .iter() - .position(|arg| arg == "-d") - .unwrap_or_else(|| panic!("rule has no '-d' flag; actual: {rule:?}")); - &rule[index + 1] -} - -fn action_of(rule: &[String]) -> &str { - let index = rule - .iter() - .position(|arg| arg == "-j") - .unwrap_or_else(|| panic!("rule has no '-j' flag; actual: {rule:?}")); - &rule[index + 1] -} - -fn last_drop_index(rules: &[Vec]) -> Option { - rules.iter().rposition(|rule| action_of(rule) == "DROP") -} - -fn first_accept_index(rules: &[Vec]) -> Option { - rules.iter().position(|rule| action_of(rule) == "ACCEPT") -} - -fn sorted<'a>(items: &[&'a str]) -> Vec<&'a str> { - let mut items = items.to_vec(); - items.sort_unstable(); - items -} - -fn assert_deny_precedence( - rules: &[Vec], - expected_drop_destinations: &[&str], - expected_accept_destinations: &[&str], -) { - let mut drop_destinations: Vec<&str> = Vec::new(); - let mut accept_destinations: Vec<&str> = Vec::new(); - for rule in rules { - match action_of(rule) { - "DROP" => drop_destinations.push(destination_of(rule)), - "ACCEPT" => accept_destinations.push(destination_of(rule)), - other => panic!("unexpected -j target '{other}'; actual rule: {rule:?}"), - } - } - - assert_eq!( - sorted(&drop_destinations), - sorted(expected_drop_destinations), - "DROP destinations did not match expected set; actual rules: {rules:?}" - ); - assert_eq!( - sorted(&accept_destinations), - sorted(expected_accept_destinations), - "ACCEPT destinations did not match expected set; actual rules: {rules:?}" - ); - - if let (Some(last_drop), Some(first_accept)) = - (last_drop_index(rules), first_accept_index(rules)) - { - assert!( - last_drop < first_accept, - "every DROP rule must precede every ACCEPT rule (B1); \ - last DROP at index {last_drop}, first ACCEPT at index {first_accept}; \ - actual rules: {rules:?}" - ); - } -} - -fn expect_ok(result: Result, context: &str) -> FirewallRuleArgs { - match result { - Ok(args) => args, - Err(err) => panic!("{context}; actual Err: {err:?}"), - } -} - -fn parses_as_ipv4(destination: &str) -> bool { - let address = destination.split('/').next().unwrap_or(destination); - address.parse::().is_ok() -} - -fn parses_as_ipv6(destination: &str) -> bool { - let address = destination.split('/').next().unwrap_or(destination); - address.parse::().is_ok() -} - -#[test] -fn a_destination_in_both_lists_is_dropped_because_deny_rules_are_emitted_first() { - let destination = "203.0.113.44"; - let policy = ContainerPolicy { - blocked_hosts: vec![destination.to_string()], - allowed_hosts: vec![destination.to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - - let args = NetworkIptablesManager::build_policy_rule_args(CHAIN, &policy, false); - - assert_eq!( - args.ipv4.len(), - 2, - "expected one DROP rule and one ACCEPT rule for a doubly-listed \ - destination; actual: {:?}", - args.ipv4 - ); - assert_eq!( - as_str_slice(&args.ipv4[0]), - vec!["-A", CHAIN, "-d", destination, "-j", "DROP"], - "the deny rule must be emitted first; actual first rule: {:?}", - args.ipv4[0] - ); - assert_eq!( - as_str_slice(&args.ipv4[1]), - vec!["-A", CHAIN, "-d", destination, "-j", "ACCEPT"], - "the allow rule must follow the deny rule; actual second rule: {:?}", - args.ipv4[1] - ); - assert!( - args.ipv6.is_empty(), - "an IPv4-only policy must not produce IPv6 rules; actual: {:?}", - args.ipv6 - ); -} - -#[test] -fn an_ipv6_destination_in_both_lists_is_dropped_because_deny_rules_are_emitted_first() { - let destination = "2001:db8::44"; - let policy = ContainerPolicy { - blocked_hosts: vec![destination.to_string()], - allowed_hosts: vec![destination.to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - - let args = NetworkIptablesManager::build_policy_rule_args(CHAIN, &policy, false); - - assert_eq!( - args.ipv6.len(), - 2, - "expected one DROP rule and one ACCEPT rule for a doubly-listed \ - IPv6 destination; actual: {:?}", - args.ipv6 - ); - assert_eq!( - as_str_slice(&args.ipv6[0]), - vec!["-A", CHAIN, "-d", destination, "-j", "DROP"], - "the deny rule must be emitted first; actual first rule: {:?}", - args.ipv6[0] - ); - assert_eq!( - as_str_slice(&args.ipv6[1]), - vec!["-A", CHAIN, "-d", destination, "-j", "ACCEPT"], - "the allow rule must follow the deny rule; actual second rule: {:?}", - args.ipv6[1] - ); - assert!( - args.ipv4.is_empty(), - "an IPv6-only policy must not produce IPv4 rules; actual: {:?}", - args.ipv4 - ); -} - -#[test] -fn deny_precedence_holds_across_both_families_with_several_entries_in_each_list() { - let policy = ContainerPolicy { - blocked_hosts: vec![ - "10.0.0.0/8".to_string(), - "198.51.100.42/32".to_string(), - "2606:50c0::/32".to_string(), - ], - allowed_hosts: vec![ - "140.82.112.0/20".to_string(), - "203.0.113.44".to_string(), - "2001:db8::/32".to_string(), - "2001:db8::44".to_string(), - ], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - - let args = NetworkIptablesManager::build_policy_rule_args(CHAIN, &policy, false); - - assert_eq!( - args.ipv4.len(), - 4, - "2 blocked + 2 allowed IPv4 destinations must produce 4 IPv4 rules; \ - actual: {:?}", - args.ipv4 - ); - assert_deny_precedence( - &args.ipv4, - &["10.0.0.0/8", "198.51.100.42/32"], - &["140.82.112.0/20", "203.0.113.44"], - ); - - assert_eq!( - args.ipv6.len(), - 3, - "1 blocked + 2 allowed IPv6 destinations must produce 3 IPv6 rules; \ - actual: {:?}", - args.ipv6 - ); - assert_deny_precedence( - &args.ipv6, - &["2606:50c0::/32"], - &["2001:db8::/32", "2001:db8::44"], - ); -} - -#[test] -fn an_unresolvable_blocked_host_errors_under_an_allow_default_and_names_the_host() { - let host = "140.82.112.0/not-a-prefix"; - let policy = ContainerPolicy { - blocked_hosts: vec![host.to_string()], - default_network_policy: NetworkPolicy::Allow, - ..Default::default() - }; - let mut logger = Logger::new(Mode::Buffer); - - let result = - NetworkIptablesManager::build_policy_rules_logged(CHAIN, &policy, false, &mut logger); - - let err = match result { - Err(err) => err, - Ok(args) => panic!( - "expected Err: a blocked, unresolvable host under an Allow \ - default leaves nothing to stop traffic (B4); actual ipv4: {:?}, \ - ipv6: {:?}", - args.ipv4, args.ipv6 - ), - }; - assert!( - err.contains(host), - "the error message must name the offending host '{host}'; actual \ - message: {err:?}" - ); -} - -#[test] -fn the_same_unresolvable_blocked_host_does_not_error_under_a_block_default() { - let host = "140.82.112.0/not-a-prefix"; - let policy = ContainerPolicy { - blocked_hosts: vec![host.to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - let mut logger = Logger::new(Mode::Buffer); - - let result = - NetworkIptablesManager::build_policy_rules_logged(CHAIN, &policy, false, &mut logger); - let args = expect_ok( - result, - "a Block default already denies everything the allow list did not \ - name, so an unresolvable block entry is redundant, not fatal (B4)", - ); - - assert!( - args.ipv4.is_empty() && args.ipv6.is_empty(), - "an unresolvable entry contributes no rules; actual ipv4: {:?}, \ - ipv6: {:?}", - args.ipv4, - args.ipv6 - ); - - let expected_warning = format!("Warning: could not resolve host '{host}'"); - assert!( - logger - .get_buffer() - .lines() - .any(|line| line == expected_warning), - "expected the exact warning line {expected_warning:?}; actual \ - buffer: {:?}", - logger.get_buffer() - ); -} - -#[test] -fn an_unresolvable_allowed_host_never_errors_under_an_allow_default() { - let host = "/20"; - let policy = ContainerPolicy { - allowed_hosts: vec![host.to_string()], - default_network_policy: NetworkPolicy::Allow, - ..Default::default() - }; - let mut logger = Logger::new(Mode::Buffer); - - let result = - NetworkIptablesManager::build_policy_rules_logged(CHAIN, &policy, false, &mut logger); - let args = expect_ok( - result, - "B4 reserves Err for an unresolvable BLOCK entry under an Allow \ - default; an unresolvable ALLOW entry must never error", - ); - - assert!( - args.ipv4.is_empty() && args.ipv6.is_empty(), - "an unresolvable entry contributes no rules; actual ipv4: {:?}, \ - ipv6: {:?}", - args.ipv4, - args.ipv6 - ); - - let expected_warning = format!("Warning: could not resolve host '{host}'"); - assert!( - logger - .get_buffer() - .lines() - .any(|line| line == expected_warning), - "expected the exact warning line {expected_warning:?}; actual \ - buffer: {:?}", - logger.get_buffer() - ); -} - -#[test] -fn an_unresolvable_allowed_host_never_errors_under_a_block_default() { - let host = "140.82.112.0/20/8"; - let policy = ContainerPolicy { - allowed_hosts: vec![host.to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - let mut logger = Logger::new(Mode::Buffer); - - let result = - NetworkIptablesManager::build_policy_rules_logged(CHAIN, &policy, false, &mut logger); - let args = expect_ok( - result, - "an unresolvable ALLOW entry must never error, regardless of the \ - default network policy (B4)", - ); - - assert!( - args.ipv4.is_empty() && args.ipv6.is_empty(), - "an unresolvable entry contributes no rules; actual ipv4: {:?}, \ - ipv6: {:?}", - args.ipv4, - args.ipv6 - ); - - let expected_warning = format!("Warning: could not resolve host '{host}'"); - assert!( - logger - .get_buffer() - .lines() - .any(|line| line == expected_warning), - "expected the exact warning line {expected_warning:?}; actual \ - buffer: {:?}", - logger.get_buffer() - ); -} - -#[test] -fn an_unresolvable_entry_does_not_suppress_a_sibling_entrys_rule_or_log_line() { - let good_destination = "198.51.100.42/32"; - let bad_host = "2606:50c0::/129"; - let policy = ContainerPolicy { - blocked_hosts: vec![good_destination.to_string(), bad_host.to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - let mut logger = Logger::new(Mode::Buffer); - - let result = - NetworkIptablesManager::build_policy_rules_logged(CHAIN, &policy, false, &mut logger); - let args = expect_ok( - result, - "an unresolvable block entry under a Block default must not error, \ - and must not stop a sibling entry in the same call from producing \ - a rule (B4)", - ); - - assert_eq!( - args.ipv4.len(), - 1, - "the resolvable sibling must still produce exactly one rule; \ - actual: {:?}", - args.ipv4 - ); - assert_eq!( - as_str_slice(&args.ipv4[0]), - vec!["-A", CHAIN, "-d", good_destination, "-j", "DROP"], - "actual rule: {:?}", - args.ipv4[0] - ); - - let buffer = logger.get_buffer(); - let expected_warning = format!("Warning: could not resolve host '{bad_host}'"); - assert!( - buffer.lines().any(|line| line == expected_warning), - "expected the warning line for the unresolvable sibling; actual \ - buffer: {buffer:?}" - ); - let expected_programmed_line = - format!("Programmed iptables rule: -A {CHAIN} -d {good_destination} -j DROP"); - assert!( - buffer.lines().any(|line| line == expected_programmed_line), - "expected the programmed-rule line for the resolvable sibling; \ - actual buffer: {buffer:?}" - ); -} - -#[test] -fn emitted_rules_have_the_exact_iptables_shape_for_both_allow_and_block_actions() { - let allowed = "203.0.113.44"; - let blocked = "10.0.0.0/8"; - let chain = "mxc_shape_chain"; - let policy = ContainerPolicy { - allowed_hosts: vec![allowed.to_string()], - blocked_hosts: vec![blocked.to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - - let args = NetworkIptablesManager::build_policy_rule_args(chain, &policy, false); - - assert_eq!( - args.ipv4.len(), - 2, - "one block entry and one allow entry must produce exactly 2 rules; \ - actual: {:?}", - args.ipv4 - ); - assert_eq!( - as_str_slice(&args.ipv4[0]), - vec!["-A", chain, "-d", blocked, "-j", "DROP"], - "actual rule: {:?}", - args.ipv4[0] - ); - assert_eq!( - as_str_slice(&args.ipv4[1]), - vec!["-A", chain, "-d", allowed, "-j", "ACCEPT"], - "actual rule: {:?}", - args.ipv4[1] - ); - for rule in &args.ipv4 { - assert_eq!( - rule.len(), - 6, - "a rule must have exactly 6 arguments; actual: {rule:?}" - ); - } -} - -#[test] -fn ipv4_and_ipv6_destinations_are_split_into_the_correct_bucket_and_never_cross_over() { - let policy = ContainerPolicy { - allowed_hosts: vec!["140.82.112.0/20".to_string(), "2001:db8::/32".to_string()], - blocked_hosts: vec!["198.51.100.42/32".to_string(), "fe80::1".to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - - let args = NetworkIptablesManager::build_policy_rule_args(CHAIN, &policy, false); - - for rule in &args.ipv4 { - let destination = destination_of(rule); - assert!( - parses_as_ipv4(destination), - "a destination in the ipv4 bucket must parse as IPv4; actual \ - destination: {destination:?}" - ); - } - for rule in &args.ipv6 { - let destination = destination_of(rule); - assert!( - parses_as_ipv6(destination), - "a destination in the ipv6 bucket must parse as IPv6; actual \ - destination: {destination:?}" - ); - } - - assert_eq!( - args.ipv4.len(), - 2, - "2 of the 4 destinations are IPv4; actual: {:?}", - args.ipv4 - ); - assert_eq!( - args.ipv6.len(), - 2, - "2 of the 4 destinations are IPv6; actual: {:?}", - args.ipv6 - ); -} - -#[test] -fn programmed_rules_are_logged_with_the_exact_iptables_and_ip6tables_prefixes() { - let allowed_v4 = "203.0.113.44"; - let blocked_v6 = "2606:50c0::/32"; - let chain = "mxc_log_chain"; - let policy = ContainerPolicy { - allowed_hosts: vec![allowed_v4.to_string()], - blocked_hosts: vec![blocked_v6.to_string()], - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - let mut logger = Logger::new(Mode::Buffer); - - let result = - NetworkIptablesManager::build_policy_rules_logged(chain, &policy, false, &mut logger); - let args = expect_ok(result, "both entries resolve, so no error is expected here"); - - assert_eq!(args.ipv4.len(), 1, "actual: {:?}", args.ipv4); - assert_eq!(args.ipv6.len(), 1, "actual: {:?}", args.ipv6); - - let buffer = logger.get_buffer(); - let expected_ipv4_line = - format!("Programmed iptables rule: -A {chain} -d {allowed_v4} -j ACCEPT"); - let expected_ipv6_line = - format!("Programmed ip6tables rule: -A {chain} -d {blocked_v6} -j DROP"); - assert!( - buffer.lines().any(|line| line == expected_ipv4_line), - "expected the IPv4 programmed-rule line {expected_ipv4_line:?}; \ - actual buffer: {buffer:?}" - ); - assert!( - buffer.lines().any(|line| line == expected_ipv6_line), - "expected the IPv6 programmed-rule line {expected_ipv6_line:?}; \ - actual buffer: {buffer:?}" - ); -} - -#[test] -fn an_empty_policy_produces_an_empty_ok_result_with_no_log_output() { - let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - let mut logger = Logger::new(Mode::Buffer); - - let result = - NetworkIptablesManager::build_policy_rules_logged(CHAIN, &policy, false, &mut logger); - let args = expect_ok(result, "B6: empty host lists must still return Ok"); - - assert!( - args.ipv4.is_empty(), - "an empty policy must produce no IPv4 rules; actual: {:?}", - args.ipv4 - ); - assert!( - args.ipv6.is_empty(), - "an empty policy must produce no IPv6 rules; actual: {:?}", - args.ipv6 - ); - assert!( - logger.get_buffer().is_empty(), - "with nothing to program and nothing unresolvable, nothing should \ - be logged; actual buffer: {:?}", - logger.get_buffer() - ); -} diff --git a/src/mxc-sdk/src/backends/lxc/common/network_iptables_ga_egress_spec.rs b/src/mxc-sdk/src/backends/lxc/common/network_iptables_ga_egress_spec.rs index 457be38e3..2de176c85 100644 --- a/src/mxc-sdk/src/backends/lxc/common/network_iptables_ga_egress_spec.rs +++ b/src/mxc-sdk/src/backends/lxc/common/network_iptables_ga_egress_spec.rs @@ -2,7 +2,6 @@ // Licensed under the MIT License. use super::*; -use crate::mxc_common::models::NetworkEnforcementMode; fn directional_policy( default: NetworkAction, @@ -182,7 +181,7 @@ fn explicit_deny_precedes_an_overlapping_allow_in_both_families() { vec![matching_rule()], vec![matching_rule()], ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( destination_actions(&rules.ipv4, ipv4), @@ -207,7 +206,7 @@ fn an_allow_peer_exclusion_is_not_accepted_alongside_its_parent() { vec![rule(vec![peer(parent, &[exclusion])], Vec::new())], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -244,7 +243,7 @@ fn a_deny_peer_exclusion_remains_outside_the_deny() { Vec::new(), vec![rule(vec![peer(parent, &[exclusion])], Vec::new())], ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -281,7 +280,7 @@ fn an_exclusion_inside_an_allow_rule_under_an_allow_default_stays_reachable() { vec![rule(vec![peer(parent, &[exclusion])], Vec::new())], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -312,7 +311,7 @@ fn an_exclusion_inside_a_deny_rule_under_a_deny_default_stays_blocked() { Vec::new(), vec![rule(vec![peer(parent, &[exclusion])], Vec::new())], ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -343,7 +342,7 @@ fn each_cidr_is_emitted_only_in_its_matching_address_family() { vec![rule(vec![peer(ipv4, &[]), peer(ipv6, &[])], Vec::new())], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( destination_actions(&rules.ipv4, ipv4), @@ -379,7 +378,7 @@ fn an_omitted_to_matches_destinations_in_both_address_families() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( new_connection_action( @@ -413,7 +412,7 @@ fn omitted_ports_match_every_protocol_and_port() { vec![rule(vec![peer(destination, &[])], Vec::new())], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); let address = packet_address("192.0.2.25"); for (protocol, port) in [("tcp", Some(443)), ("udp", Some(53)), ("icmp", None)] { @@ -437,7 +436,7 @@ fn explicit_any_without_a_port_matches_every_protocol_and_port() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); let address = packet_address("192.0.2.25"); for (protocol, port) in [ @@ -470,7 +469,7 @@ fn any_with_a_port_expands_to_tcp_and_udp_rules() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); let address = packet_address("192.0.2.25"); let mut selectors = rules .ipv4 @@ -528,7 +527,7 @@ fn icmp_ignores_a_written_port() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); let emitted_rule = rules.ipv4.iter().find(|rule| { argument_after(rule, "-d") == Some(destination) && argument_after(rule, "-j") == Some("ACCEPT") @@ -561,7 +560,7 @@ fn a_single_port_and_an_inclusive_range_restrict_their_protocols() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); let address = packet_address("192.0.2.25"); for (protocol, packet_port, expected_action) in [ @@ -599,7 +598,7 @@ fn each_peer_combines_with_each_port_selector() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); for (destination, address) in [ (first_destination, "192.0.2.25"), @@ -633,7 +632,7 @@ fn icmp_uses_the_family_specific_protocol_without_a_port() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); let ipv4_rule = rules.ipv4.iter().find(|rule| { argument_after(rule, "-d") == Some(ipv4) && argument_after(rule, "-j") == Some("ACCEPT") }); @@ -792,21 +791,6 @@ fn a_directional_deny_naming_a_resolver_is_not_preceded_by_a_dns_accept() { ); } -#[test] -fn a_legacy_policy_still_opens_dns() { - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - allowed_hosts: vec!["example.com".to_string()], - ..Default::default() - }; - let rules = appended_ipv4_chain_rules("legacy-dns", &policy); - - assert!( - opens_base_dns_exemption(&rules), - "input=legacy defaultPolicy=block; expected the documented port 53 accept; output={rules:?}" - ); -} - #[test] fn a_parsed_directional_request_drops_the_dns_exemption() { let policy = policy_from_json( @@ -819,12 +803,12 @@ fn a_parsed_directional_request_drops_the_dns_exemption() { assert!( !opens_base_dns_exemption(&rules), - "input=0.8 egress.default=deny; expected no port 53 accept; output={rules:?}" + "input=v0.9 egress.default=deny; expected no port 53 accept; output={rules:?}" ); } #[test] -fn a_parsed_v08_request_without_a_network_section_drops_the_dns_exemption() { +fn a_parsed_request_without_a_network_section_drops_the_dns_exemption() { let policy = policy_from_json( r#"{"version": "0.9.0-alpha", "process": {"commandLine": "echo hi"}}"#, @@ -832,7 +816,7 @@ fn a_parsed_v08_request_without_a_network_section_drops_the_dns_exemption() { assert!( !plan_network(&policy).installs_firewall(), - "input=0.8 with no network section; expected no chain at all, so no rule can open DNS" + "input=v0.9 with no network section; expected no chain at all, so no rule can open DNS" ); } #[test] @@ -845,7 +829,7 @@ fn an_exclusion_does_not_shadow_a_later_rule_that_names_it() { rule(vec![peer("10.10.1.0/24", &[])], Vec::new()), ], ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -883,7 +867,7 @@ fn the_emitted_blocks_cover_the_peer_without_its_exclusion() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); for reachable in ["10.0.0.1", "10.9.255.255", "10.11.0.0", "10.255.255.255"] { assert_eq!( @@ -926,7 +910,7 @@ fn an_ipv6_exclusion_is_subtracted_within_its_own_family() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -972,11 +956,10 @@ fn a_peer_that_expands_past_the_block_ceiling_is_refused() { ); let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let error = - NetworkIptablesManager::build_policy_rules_logged("MXC-test", &policy, true, &mut logger) - .expect_err( - "a peer expanding past the ceiling must be refused rather than partly installed", - ); + let error = NetworkIptablesManager::build_policy_rules_logged("MXC-test", &policy, &mut logger) + .expect_err( + "a peer expanding past the ceiling must be refused rather than partly installed", + ); assert!( error.contains("except"), @@ -994,7 +977,7 @@ fn an_ipv4_mapped_peer_still_reaches_the_ipv4_chain_after_subtraction() { )], Vec::new(), ); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -1042,7 +1025,6 @@ fn unresolvable_peer() -> NetworkPeer { #[test] fn an_unresolvable_directional_deny_is_fatal_under_an_allow_egress_default() { let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Block, ..directional_policy( NetworkAction::Allow, Vec::new(), @@ -1051,15 +1033,10 @@ fn an_unresolvable_directional_deny_is_fatal_under_an_allow_egress_default() { }; let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let error = NetworkIptablesManager::build_policy_rules_logged( - "MXC-test", - &policy, - uses_directional_keys(&policy), - &mut logger, - ) - .expect_err( - "input=egress.default=allow with an unresolvable deny, legacy default_network_policy=block; expected a refusal, since reading the legacy field instead would close the chain with ACCEPT and leave the denied destination reachable", - ); + let error = NetworkIptablesManager::build_policy_rules_logged("MXC-test", &policy, &mut logger) + .expect_err( + "egress.default=allow must refuse an unresolvable deny rather than leave it reachable", + ); assert!( error.contains("deny"), @@ -1070,7 +1047,6 @@ fn an_unresolvable_directional_deny_is_fatal_under_an_allow_egress_default() { #[test] fn an_unresolvable_directional_deny_is_tolerated_under_a_deny_egress_default() { let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, ..directional_policy( NetworkAction::Deny, Vec::new(), @@ -1082,17 +1058,16 @@ fn an_unresolvable_directional_deny_is_tolerated_under_a_deny_egress_default() { NetworkIptablesManager::build_policy_rules_logged( "MXC-test", &policy, - uses_directional_keys(&policy), &mut logger, ) .expect( - "input=egress.default=deny with an unresolvable deny, legacy default_network_policy=allow; expected no refusal, since the closing DROP already covers what the deny could not program", + "input=egress.default=deny with an unresolvable deny, expected no refusal, since the closing DROP already covers what the deny could not program", ); } fn lowering_error(policy: &ContainerPolicy) -> String { let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - NetworkIptablesManager::build_policy_rules_logged("MXC-test", policy, true, &mut logger) + NetworkIptablesManager::build_policy_rules_logged("MXC-test", policy, &mut logger) .expect_err("expected the policy to be refused") } @@ -1118,7 +1093,7 @@ fn an_ipv6_peer_whose_exclusion_names_the_mapped_range_is_refused() { #[test] fn an_ipv4_peer_subtracts_an_exclusion_written_in_mapped_notation() { let policy = allow_peer_policy(vec![peer("10.0.0.0/8", &["::ffff:10.10.0.0/112"])]); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -1149,7 +1124,7 @@ fn an_ipv4_peer_subtracts_an_exclusion_written_in_mapped_notation() { #[test] fn a_mapped_peer_subtracts_an_exclusion_written_in_plain_ipv4() { let policy = allow_peer_policy(vec![peer("::ffff:10.0.0.0/104", &["10.10.0.0/16"])]); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert_eq!( chain_verdict( @@ -1215,7 +1190,7 @@ fn an_ipv6_peer_whose_exclusion_neighbours_the_mapped_range_is_refused() { #[test] fn an_ipv6_peer_far_from_the_mapped_range_still_subtracts() { let policy = allow_peer_policy(vec![peer("2001:db8::/32", &["2001:db8:1::/48"])]); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert!( rules.ipv4.is_empty(), @@ -1239,7 +1214,7 @@ fn an_ipv6_peer_far_from_the_mapped_range_still_subtracts() { #[test] fn an_ipv6_catch_all_without_an_exclusion_is_untouched_by_the_guard() { let policy = allow_peer_policy(vec![peer("::/0", &[])]); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy); assert!( rules.ipv4.is_empty(), diff --git a/src/mxc-sdk/src/backends/lxc/common/network_iptables_proxy_spec.rs b/src/mxc-sdk/src/backends/lxc/common/network_iptables_proxy_spec.rs deleted file mode 100644 index f45da9d4a..000000000 --- a/src/mxc-sdk/src/backends/lxc/common/network_iptables_proxy_spec.rs +++ /dev/null @@ -1,535 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -use super::*; -use crate::mxc_common::logger::{Logger, Mode}; -use crate::mxc_common::models::{ - ContainerPolicy, NetworkEnforcementMode, NetworkPolicy, ProxyAddress, ProxyConfig, -}; - -fn policy_with_proxy(host: &str, port: u16) -> ContainerPolicy { - ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - network_proxy: ProxyConfig { - address: Some(ProxyAddress::new(host.to_string(), port)), - builtin_test_server: false, - }, - ..Default::default() - } -} - -fn apply_and_collect( - container: &str, - policy: &ContainerPolicy, -) -> ( - NetworkIptablesManager, - Vec>, - Result, -) { - let fake = super::test_firewall::install(); - let mut manager = NetworkIptablesManager::new(container, EgressHookPoint::ContainerNetns(4242)); - let mut logger = Logger::new(Mode::Buffer); - let _ = fake.forget_issued(); - - let result = manager.apply_firewall_rules(policy, &mut logger); - let issued = fake.issued(); - (manager, issued, result) -} - -fn appended_rules<'a>( - issued: &'a [Vec], - binary: &str, - chain: &str, -) -> Vec<&'a Vec> { - issued - .iter() - .filter(|argv| { - argv.first().map(String::as_str) == Some(binary) - && argv.get(1).map(String::as_str) == Some("-A") - && argv.get(2).map(String::as_str) == Some(chain) - }) - .collect() -} - -fn action_of(rule: &[String]) -> Option<&str> { - let index = rule.iter().position(|arg| arg == "-j")?; - rule.get(index + 1).map(String::as_str) -} - -fn has_pair(rule: &[String], flag: &str, value: &str) -> bool { - rule.windows(2) - .any(|pair| pair[0] == flag && pair[1] == value) -} - -#[test] -fn proxy_mode_forces_a_drop_default_even_when_the_policy_says_allow() { - assert_eq!( - NetworkIptablesManager::default_policy_action(NetworkPolicy::Allow, true), - "DROP" - ); - assert_eq!( - NetworkIptablesManager::default_policy_action(NetworkPolicy::Block, true), - "DROP" - ); -} - -#[test] -fn without_a_proxy_the_configured_default_policy_still_decides_the_catch_all() { - assert_eq!( - NetworkIptablesManager::default_policy_action(NetworkPolicy::Allow, false), - "ACCEPT" - ); - assert_eq!( - NetworkIptablesManager::default_policy_action(NetworkPolicy::Block, false), - "DROP" - ); -} - -#[test] -fn the_terminal_rule_built_in_proxy_mode_drops() { - let rule = NetworkIptablesManager::build_default_policy_rule_arg( - "MXC-proxy-terminal", - NetworkPolicy::Allow, - true, - ); - - assert_eq!(action_of(&rule), Some("DROP"), "actual rule: {rule:?}"); -} - -#[test] -fn an_applied_proxy_chain_ends_in_drop_under_an_allow_default() { - let mut policy = policy_with_proxy("10.9.8.7", 3128); - policy.default_network_policy = NetworkPolicy::Allow; - - let (manager, issued, result) = apply_and_collect("proxy-allow-default", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "iptables", manager.chain_name()); - let last = rules.last().expect("the chain must have at least one rule"); - assert_eq!( - action_of(last), - Some("DROP"), - "the last rule appended to a proxied chain must be the closing DROP; actual: {last:?}" - ); -} - -#[test] -fn the_proxy_accept_names_the_proxy_address_port_and_protocol() { - let policy = policy_with_proxy("10.9.8.7", 3128); - - let (manager, issued, result) = apply_and_collect("proxy-shape", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "iptables", manager.chain_name()); - let accepts: Vec<&&Vec> = rules - .iter() - .filter(|rule| action_of(rule) == Some("ACCEPT") && !has_pair(rule, "-o", "lo")) - .collect(); - - assert_eq!( - accepts.len(), - 1, - "a proxied chain must carry exactly one ACCEPT reaching off-box, for the proxy; \ - actual: {rules:?}" - ); - let accept = accepts[0]; - assert!( - has_pair(accept, "-d", "10.9.8.7"), - "the proxy ACCEPT must name the proxy address; actual: {accept:?}" - ); - assert!( - has_pair(accept, "--dport", "3128"), - "the proxy ACCEPT must name the proxy port; actual: {accept:?}" - ); - assert!( - has_pair(accept, "-p", "tcp"), - "the proxy ACCEPT must be scoped to TCP; actual: {accept:?}" - ); -} - -#[test] -fn the_proxy_accept_is_appended_before_the_closing_drop() { - let policy = policy_with_proxy("10.9.8.7", 3128); - - let (manager, issued, result) = apply_and_collect("proxy-order", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "iptables", manager.chain_name()); - let actions: Vec> = rules.iter().map(|rule| action_of(rule)).collect(); - - assert_eq!( - actions, - vec![Some("ACCEPT"), Some("ACCEPT"), Some("DROP")], - "a proxied chain must read exactly 'keep loopback, accept the proxy, drop the rest'; \ - actual: {rules:?}" - ); - assert!( - has_pair(rules[0], "-o", "lo"), - "loopback comes first; actual: {rules:?}" - ); - assert!( - has_pair(rules[1], "-d", "10.9.8.7"), - "the proxy ACCEPT comes second; actual: {rules:?}" - ); -} - -#[test] -fn every_resolved_proxy_address_is_opened() { - let mut logger = Logger::new(Mode::Buffer); - let policy = policy_with_proxy("localhost", 8888); - - let (endpoints, _pin) = NetworkIptablesManager::resolve_proxy_endpoints(&policy, &mut logger) - .expect("localhost must resolve"); - - assert!( - !endpoints.is_empty(), - "localhost must yield at least one endpoint" - ); - assert!( - endpoints.iter().all(|endpoint| endpoint.port == 8888), - "every endpoint must carry the configured proxy port; actual: {endpoints:?}" - ); -} - -#[test] -fn proxy_mode_opens_no_dns_port() { - let policy = policy_with_proxy("10.9.8.7", 3128); - - let (manager, issued, result) = apply_and_collect("proxy-nodns", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "iptables", manager.chain_name()); - assert!( - !rules.is_empty(), - "the proxied chain must have been programmed at all; issued: {issued:?}" - ); - - for rule in rules { - assert!( - !has_pair(rule, "--dport", "53"), - "a proxied chain must not open DNS; actual: {rule:?}" - ); - } -} - -#[test] -fn proxy_mode_keeps_loopback_and_drops_the_conntrack_exemption() { - let policy = policy_with_proxy("10.9.8.7", 3128); - - let (manager, issued, result) = apply_and_collect("proxy-nobase", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "iptables", manager.chain_name()); - assert!( - !rules.is_empty(), - "the proxied chain must have been programmed at all; issued: {issued:?}" - ); - - assert!( - rules.iter().any(|rule| has_pair(rule, "-o", "lo")), - "a proxied container must still reach its own loopback; actual: {rules:?}" - ); - - for rule in rules { - assert!( - !has_pair(rule, "-i", "lo"), - "this chain hangs off OUTPUT, where -i never matches; actual: {rule:?}" - ); - assert!( - !has_pair(rule, "--state", "ESTABLISHED,RELATED"), - "a proxied chain must not carry the conntrack exemption; actual: {rule:?}" - ); - } -} - -#[test] -fn proxy_mode_does_not_program_the_allow_list() { - let mut policy = policy_with_proxy("10.9.8.7", 3128); - policy.allowed_hosts = vec!["10.1.1.1".to_string()]; - - let (manager, issued, result) = apply_and_collect("proxy-nolists", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "iptables", manager.chain_name()); - assert!( - !rules.is_empty(), - "the proxied chain must have been programmed at all; issued: {issued:?}" - ); - - for rule in rules { - assert!( - !has_pair(rule, "-d", "10.1.1.1"), - "a proxied chain must ignore the allow list; actual: {rule:?}" - ); - } -} - -#[test] -fn a_proxy_combined_with_a_block_list_is_refused() { - let mut policy = policy_with_proxy("10.9.8.7", 3128); - policy.blocked_hosts = vec!["10.2.2.2".to_string()]; - - let (_manager, _issued, result) = apply_and_collect("proxy-blocked", &policy); - - let message = result.expect_err("a policy whose block list cannot be enforced must be refused"); - assert!( - message.contains("blockedHosts"), - "the refusal must name the setting that cannot be enforced, got: {message}" - ); -} - -#[test] -fn the_ipv6_chain_allows_only_loopback_before_its_closing_drop_in_proxy_mode() { - let policy = policy_with_proxy("10.9.8.7", 3128); - - let (manager, issued, result) = apply_and_collect("proxy-v6", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "ip6tables", manager.chain_name()); - let actions: Vec> = rules.iter().map(|rule| action_of(rule)).collect(); - - assert_eq!( - actions, - vec![Some("ACCEPT"), Some("DROP")], - "the IPv6 chain of a proxied container must deny everything it can route; \ - actual: {rules:?}" - ); - assert!( - has_pair(rules[0], "-o", "lo"), - "the only IPv6 ACCEPT must be the loopback one; actual: {rules:?}" - ); -} - -#[test] -fn without_a_proxy_the_base_exemptions_and_host_lists_are_still_programmed() { - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - allowed_hosts: vec!["10.1.1.1".to_string()], - ..Default::default() - }; - - let (manager, issued, result) = apply_and_collect("proxy-control", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let rules = appended_rules(&issued, "iptables", manager.chain_name()); - assert!( - rules.iter().any(|rule| has_pair(rule, "-o", "lo")), - "a non-proxied chain must still carry the loopback exemption; actual: {rules:?}" - ); - assert!( - rules.iter().any(|rule| has_pair(rule, "--dport", "53")), - "a non-proxied chain must still open DNS; actual: {rules:?}" - ); - assert!( - rules.iter().any(|rule| has_pair(rule, "-d", "10.1.1.1")), - "a non-proxied chain must still program its allow list; actual: {rules:?}" - ); -} - -#[test] -fn no_chain_opens_a_dhcp_port() { - let filtered = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - default_network_policy: NetworkPolicy::Block, - allowed_hosts: vec!["10.1.1.1".to_string()], - ..Default::default() - }; - - for (name, policy) in [ - ("dhcp-filtered", filtered), - ("dhcp-proxied", policy_with_proxy("10.9.8.7", 3128)), - ] { - let (manager, issued, result) = apply_and_collect(name, &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let mut programmed = false; - for binary in ["iptables", "ip6tables"] { - for rule in appended_rules(&issued, binary, manager.chain_name()) { - programmed = true; - assert!( - !has_pair(rule, "--dport", "67") && !has_pair(rule, "--dport", "547"), - "{name} must not open a DHCP port; actual: {rule:?}" - ); - } - } - assert!( - programmed, - "{name} never reached the rule builder, so this assertion would hold \ - vacuously; issued: {issued:?}" - ); - } -} - -#[test] -fn an_ipv6_proxy_literal_is_refused_rather_than_silently_dropped() { - let mut logger = Logger::new(Mode::Buffer); - - for host in ["2001:db8::1", "[2001:db8::1]"] { - let policy = policy_with_proxy(host, 3128); - let err = NetworkIptablesManager::resolve_proxy_endpoints(&policy, &mut logger) - .expect_err("an IPv6 proxy endpoint must be refused"); - - assert!( - err.to_lowercase().contains("ipv6"), - "the refusal must say IPv6 is the reason, got: {err}" - ); - } -} - -#[test] -fn ipv6_literals_are_recognized_bracketed_or_bare() { - assert!( - NetworkIptablesManager::host_is_ipv6_literal("::1"), - "a bare IPv6 literal must be recognized" - ); - assert!( - NetworkIptablesManager::host_is_ipv6_literal("[::1]"), - "a bracketed IPv6 literal must be recognized" - ); - assert!( - NetworkIptablesManager::host_is_ipv6_literal("2001:db8::1"), - "a full IPv6 literal must be recognized" - ); - assert!( - !NetworkIptablesManager::host_is_ipv6_literal("10.9.8.7"), - "an IPv4 literal must not be classified as IPv6" - ); - assert!( - !NetworkIptablesManager::host_is_ipv6_literal("proxy.example.com"), - "a hostname must not be classified as IPv6" - ); -} - -#[test] -fn a_hostname_proxy_records_a_pin_naming_an_authorized_address() { - let policy = policy_with_proxy("localhost", 8888); - - let (manager, _issued, result) = apply_and_collect("proxy-pin", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - let pin = manager - .proxy_host_pin() - .expect("a hostname proxy must record a pin"); - assert_eq!(pin.hostname(), "localhost"); - assert_eq!(pin.ip().to_string(), "127.0.0.1"); -} - -#[test] -fn an_ip_literal_proxy_records_no_pin() { - let policy = policy_with_proxy("10.9.8.7", 3128); - - let (manager, _issued, result) = apply_and_collect("proxy-nopin", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - assert!( - manager.proxy_host_pin().is_none(), - "an IP-literal proxy needs no hosts entry" - ); -} - -#[test] -fn a_policy_without_a_proxy_records_no_pin() { - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - ..Default::default() - }; - - let (manager, _issued, result) = apply_and_collect("proxy-absent", &policy); - assert!(result.is_ok(), "apply must succeed, got {result:?}"); - - assert!( - manager.proxy_host_pin().is_none(), - "a policy without a proxy must not record a hosts-file pin" - ); -} - -#[test] -fn a_zero_proxy_port_is_refused() { - let mut logger = Logger::new(Mode::Buffer); - let policy = policy_with_proxy("10.9.8.7", 0); - - let err = NetworkIptablesManager::resolve_proxy_endpoints(&policy, &mut logger) - .expect_err("port 0 must be refused"); - - assert!( - err.to_lowercase().contains("port"), - "the refusal must name the port as the reason, got: {err}" - ); -} - -#[test] -fn an_unresolvable_proxy_host_is_refused() { - let mut logger = Logger::new(Mode::Buffer); - let policy = policy_with_proxy("proxy.invalid", 3128); - - let err = NetworkIptablesManager::resolve_proxy_endpoints(&policy, &mut logger) - .expect_err("an unresolvable proxy host must be refused"); - - assert!( - err.contains("proxy.invalid"), - "the refusal must name the host that failed, got: {err}" - ); -} - -#[test] -fn a_policy_without_a_proxy_resolves_to_no_endpoints() { - let mut logger = Logger::new(Mode::Buffer); - let policy = ContainerPolicy::default(); - - let (endpoints, pin) = NetworkIptablesManager::resolve_proxy_endpoints(&policy, &mut logger) - .expect("a policy with no proxy must not be an error"); - - assert!(endpoints.is_empty()); - assert!(pin.is_none()); -} - -#[test] -fn a_proxy_answer_larger_than_the_cap_contributes_no_more_than_the_cap() { - let addresses: Vec = (0..40).map(|n| format!("203.0.113.{n}")).collect(); - let mut logger = Logger::new(Mode::Buffer); - - let accepted = - NetworkIptablesManager::bound_proxy_addresses("proxy.invalid", &addresses, &mut logger); - - assert_eq!( - accepted.len(), - NetworkIptablesManager::MAX_PROXY_ENDPOINTS, - "a 40-address answer must not become 40 ACCEPT rules and 40 iptables \ - processes on the container-start path; actual count: {}", - accepted.len() - ); - - assert_eq!( - accepted.first().map(String::as_str), - Some("203.0.113.0"), - "the pinned address must survive the bound; actual: {:?}", - accepted.first() - ); - - let buffer = logger.get_buffer(); - assert!( - buffer.contains("proxy.invalid") && buffer.contains("40"), - "a trimmed answer must say which host was trimmed and from what size; \ - actual buffer: {buffer:?}" - ); -} - -#[test] -fn a_proxy_answer_within_the_cap_is_left_alone() { - let addresses: Vec = (0..3).map(|n| format!("203.0.113.{n}")).collect(); - let mut logger = Logger::new(Mode::Buffer); - - let accepted = - NetworkIptablesManager::bound_proxy_addresses("proxy.invalid", &addresses, &mut logger); - - assert_eq!( - accepted, - addresses.as_slice(), - "an answer within the cap must reach the chain intact" - ); - assert!( - logger.get_buffer().is_empty(), - "an untrimmed answer must not warn; actual buffer: {:?}", - logger.get_buffer() - ); -} diff --git a/src/mxc-sdk/src/backends/lxc/common/signal_cleanup.rs b/src/mxc-sdk/src/backends/lxc/common/signal_cleanup.rs index a04268768..e72364e2b 100644 --- a/src/mxc-sdk/src/backends/lxc/common/signal_cleanup.rs +++ b/src/mxc-sdk/src/backends/lxc/common/signal_cleanup.rs @@ -329,7 +329,7 @@ mod tests { fn a_backend_that_registers_no_container_publishes_no_ownership() { use crate::lxc_common::network_iptables::{EgressHookPoint, NetworkIptablesManager}; use crate::mxc_common::logger::{Logger, Mode}; - use crate::mxc_common::models::{ContainerPolicy, NetworkEnforcementMode}; + use crate::mxc_common::models::{ContainerPolicy, NetworkAction, NetworkEgressPolicy}; let _watchdog = Watchdog::installed(); let _fake = crate::lxc_common::network_iptables::test_firewall::install(); @@ -337,8 +337,10 @@ mod tests { let mut manager = NetworkIptablesManager::new("mxc-bwrap-publish-test", EgressHookPoint::Unhooked); let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - allowed_hosts: vec!["192.0.2.10".to_string()], + network_egress: Some(NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }), ..Default::default() }; let mut logger = Logger::new(Mode::Buffer);