diff --git a/docs/backends/nanvix/nanvix.md b/docs/backends/nanvix/nanvix.md index 956ab738c..0ea9ed61d 100644 --- a/docs/backends/nanvix/nanvix.md +++ b/docs/backends/nanvix/nanvix.md @@ -181,70 +181,15 @@ including egress allow with omitted ingress defaults. Disabled networking prevents guest socket creation (`OSError: [Errno 134]`). Unrestricted networking includes host-backed bind/listen capabilities; NanVix cannot independently enforce ingress or host-loopback restrictions. -Directional egress rules are explicitly rejected because the legacy IPv4 -filter does not implement their full semantics, including default-deny DNS. +Directional egress rules are explicitly rejected rather than translated to +the guest's IPv4 host filter, which cannot implement their full semantics, +including default-deny DNS. Runtime proxy configuration is also unsupported. -### Legacy per-host filter implementation (compatibility/reference only) - -The following describes the retained legacy runtime filter, not accepted v1.1 -JSON vocabulary. The exact cutover does not silently translate directional -rules into this weaker contract. - -Legacy `defaultPolicy` and host-list interactions follow the -[backend-agnostic network policy semantics](../../schema.md#legacy-network-host-list-semantics). -Invalid legacy combinations are rejected by shared policy validation: -`blockedHosts` requires an `allowedHosts` exception set under a block default, -and `allowedHosts` cannot be used under an allow default. - -NanVix forwards the validated host list to the guest's host-side socket proxy, -which enforces egress at `connect()`. The guest filter is **allow-XOR-block**, -so NanVix rejects requests that supply both lists instead of dropping either -one. - -Entries may be IPv4 literals (`93.184.216.34`), IPv4 CIDR blocks -(`10.0.0.0/8`), or hostnames. Hostnames are resolved to their IPv4 (A-record) -addresses at preflight; IPv6 (AAAA) results are dropped because the guest filter -is IPv4-only. Resolution failures are handled per direction so neither list ever -fails open: - -- **allowlist** (deny-by-default): each dropped entry is logged as a warning and - the run continues, since dropping an entry only *narrows* access. If the list - resolves to **no** IPv4 address at all, the run is rejected at preflight rather - than silently allowing all traffic. -- **blocklist** (allow-by-default): **any** entry that resolves to no IPv4 - address rejects the run at preflight. Silently dropping a blocked host would - let traffic the policy explicitly blocks flow freely, and the static preflight - filter cannot enforce a name that does not resolve — so the blocklist - fails closed. - -**DNS:** in allowlist mode the guest daemon automatically exempts the DNS port -(53), so name resolution works without adding the resolver to `allowedHosts`. - -Network proxies (`network.proxy`) are not supported and are rejected at -preflight. - -```jsonc -{ - "containment": "microvm", - "process": { "commandLine": "import urllib.request; ..." }, - // Historical legacy shape, not accepted by the exact v1.1 contract: - "network": { "defaultPolicy": "allow" } -} -``` - -```jsonc -{ - "containment": "microvm", - "process": { "commandLine": "import urllib.request; ..." }, - // Historical legacy shape, not accepted by the exact v1.1 contract: - "network": { "allowedHosts": ["example.com", "10.0.0.0/8"] } -} -``` - ## Not Supported -| Workload | Error | -| ------------------------------- | ----------------------------------- | -| Both `allowedHosts` + `blockedHosts` | Rejected at preflight (mutually exclusive) | -| File writing outside `/mnt/rw/` | `OSError: Read-only file system` | +| Workload | Error | +| -------------------------------------------- | -------------------------------- | +| Mixed directional networking or egress rules | Rejected before VM creation | +| Runtime proxy | Rejected before VM creation | +| File writing outside `/mnt/rw/` | `OSError: Read-only file system` | diff --git a/docs/backends/process-container/networking.md b/docs/backends/process-container/networking.md index 134f6efc2..d9bcdd647 100644 --- a/docs/backends/process-container/networking.md +++ b/docs/backends/process-container/networking.md @@ -205,7 +205,7 @@ middle rows provide different protections and are not ordered relative to each o For identity-scoped proxies, the scoped peer rule and `privateNetworkClientServer` do not bypass Windows Firewall's block-inbound-to-non-allowed-apps policy. A packaged AppContainer proxy uses the package-owned firewall -declaration shown in the [historical schema 0.8 manifest example](examples/0.8.0-schema.md); its application entry uses +declaration shown in the [proxy package manifest example](examples/0.8.0-schema.md); its application entry uses `uap10:RuntimeBehavior="packagedClassicApp"` with `uap10:TrustLevel="appContainer"`. An unpackaged AppContainer proxy requires its installer or administrator to own an equivalent rule scoped to the AppContainer profile SID, proxy executable, and configured port. @@ -279,7 +279,10 @@ proxy peer identity, and host-loopback allow fail with a typed unsupported-polic ## 3. WFP enforcement PSEC applies outbound WFP filters in the OS's elevated context and owns their lifetime. AppContainer fallback does not -install directional WFP filters. +install directional WFP filters. It uses capabilities for supported direction +defaults and keeps the external runtime proxy setup. Its network audit records retain the firewall fields with +`firewall_rules_created` and `firewall_rules_removed` at `0`, `firewall_applied` at `false`, and +`firewall_removal_ok` at `true`. WFP implements `egress` rules for public and private destinations. `internetClient` enables public-network access. `privateNetworkClientServer`, selected through `ingress.default`, is the prerequisite for private-network access and diff --git a/docs/backends/windows-sandbox/windows-sandbox.md b/docs/backends/windows-sandbox/windows-sandbox.md index 9c9504961..ce2c0d562 100644 --- a/docs/backends/windows-sandbox/windows-sandbox.md +++ b/docs/backends/windows-sandbox/windows-sandbox.md @@ -151,9 +151,9 @@ non-default values. | `filesystem.readwritePaths` | Existing directories mapped read-write at the same path | | `filesystem.readonlyPaths` | Existing directories mapped read-only at the same path | | `filesystem.deniedPaths` | Accepted outside shares; rejected when overlapping a mapped share | -| Default network policy `block` | Enforced by the guest firewall | -| Default network policy `allow` | Rejected | -| `allowedHosts` / `blockedHosts` | Rejected | +| Omitted network policy | Guest firewall blocks external networking | +| `network.egress` or `network.ingress` supplied (even empty or deny-only) | Rejected; omit both sections to use guest isolation | +| Retired `defaultPolicy` and host lists | Rejected by the exact contract | | Network proxy | Rejected | Mapped paths must be absolute existing directories. Files, nested mapped roots, diff --git a/docs/backends/wslc/wsl-container-getting-started.md b/docs/backends/wslc/wsl-container-getting-started.md index bcf4442e1..8a2fe4bb4 100644 --- a/docs/backends/wslc/wsl-container-getting-started.md +++ b/docs/backends/wslc/wsl-container-getting-started.md @@ -422,11 +422,10 @@ rules, but a WSLC container runs **without** `CAP_NET_ADMIN` (the SDK's `Privileged` flag does not grant it), so those rules cannot be applied — and MXC has no VM-level enforcement hook either (WSLC cannot expose one without breaking other security promises such as MDE). Rather than fail the run at exec time, -such configs are **rejected at config-parse time**: +such configs are **rejected before provisioning**: ``` -WSLc: per-host egress filtering (allowedHosts with defaultPolicy='block', or -blockedHosts with defaultPolicy='allow') is not supported. ... +WSLc does not support network.egress allow/deny rules; networking is all-or-nothing ``` Use a runtime proxy with unrestricted bridged networking for cooperative host diff --git a/docs/development/architecture/telemetry.md b/docs/development/architecture/telemetry.md index 3e8f62616..7786ae378 100644 --- a/docs/development/architecture/telemetry.md +++ b/docs/development/architecture/telemetry.md @@ -547,6 +547,11 @@ rejection records from the same invocation. A successful launch emits no | `mxc.SandboxTornDown` | Per-run resources released, once per handle | ProcessContainer: `backend`, `identity`, `tier`, `pid`, `status`, `firewall_rules_removed`, `firewall_removal_ok`, `bfs_removed`, `proxy_stopped`, `preserve_policy`, `container_released`, `skip_reason`. IsolationSession: `backend`, `identity`, `phase`, `status`, `session_stopped`, `agent_user_deprovisioned`, `client_unregistered` | | `mxc.ConfigRejected` | A request was refused before it could run | `correlation_id`, `backend`, `reason`, `offending_field`, `phase` | +For the ProcessContainer AppContainer fallback, the firewall fields remain in +these records for compatibility: no local firewall rules are created or +removed, `firewall_applied` is `false`, and `firewall_removal_ok` is `true`. +Proxy setup failures still set `mxc.NetworkPolicyApplied.status` to failure. + ### Error semantics: `FallbackError` vs `ActivityError` Tier selection can *degrade* (proceed with weaker enforcement) or *fail* @@ -710,7 +715,7 @@ that it was skipped. | Process outcome (M-ETW-1) | ✅ | ✅ | ✅ | ✅ (shared `create_process`) | | Enforcement degradation (M-ETW-2) | ✅ (shared dispatcher; records the tier actually selected) | ✅ | n/a — no tier/fallback ladder exists for this backend | n/a | | Policy hash (M-ETW-3) | ✅ | ✅ | ✅ | ✅ | -| Network policy (M-ETW-4) | ✅ (`enforcement_mode: capabilities` — policy travels in the sandbox spec and the OS enforces it, so `firewall_rules_created` is honestly `0`) | ✅ (`firewall` / `both`) | n/a — MXC rejects network and proxy policy for this backend before provisioning | n/a | +| Network policy (M-ETW-4) | ✅ (`enforcement_mode: capabilities` — policy travels in the sandbox spec and the OS enforces it, so `firewall_rules_created` is honestly `0`) | ✅ (`enforcement_mode: capabilities` for supported directional requests; egress default is reported as `allow` or `block`) | n/a — MXC rejects network and proxy policy for this backend before provisioning | n/a | | Sandbox teardown (M-ETW-5) | ✅ | ✅ | ✅ | ✅ (`stop` and `deprovision` phases) | | IsolationSession telemetry (M-ETW-6) | n/a | n/a | ✅ Applicable lifecycle events use `Microsoft.MXC`; no separate OS provider is assumed | ✅ Same provider path | | Configuration rejection (M-ETW-7) | ✅ | ✅ | ✅ | ✅ (`phase` names the rejecting phase) | diff --git a/docs/development/plans/linux-wsl-roadmap-june-2026.md b/docs/development/plans/linux-wsl-roadmap-june-2026.md index cec96adce..44425cb33 100644 --- a/docs/development/plans/linux-wsl-roadmap-june-2026.md +++ b/docs/development/plans/linux-wsl-roadmap-june-2026.md @@ -503,7 +503,7 @@ These items depend on the WSLC SDK team and are not unilaterally schedulable. > **Why network enforcement must be container-scoped (host vs. VM vs. container).** Network policy can be enforced at three layers: the Windows **host** (Windows Firewall), the WSL2 **VM**, or the **container** network namespace inside the VM. GA decision **D6 (per-sandbox scoping)** requires every sandbox's policy to be independent — concurrent WSLC containers must not affect each other's access — and names the container network namespace as WSLC's scoping identity. A machine-wide **host** firewall can't attribute traffic to one container vs. another, so it violates D6 (and per **D8**, host firewalls apply *on top of* enforcement, never *as* it). A **VM-wide** rule fails the same way when one utility VM hosts multiple containers — sandbox A's rules would bleed into sandbox B. Only the **container namespace** is inherently per-sandbox, which is why it's the required enforcement point. The catch: MXC can't install rules into that namespace today (`Privileged` doesn't grant `CAP_NET_ADMIN`, and the VM may lack iptables tooling). Hence SDK dep #1 — a VM-level API that applies rules **scoped to a specific container's namespace**: physically enforced at the VM boundary, logically attributed to one container. > -> **Contrast with Hyperlight/Nanvix, and the state-aware wrinkle.** Hyperlight (host-proxied sockets, per-instance) and Nanvix (per-guest egress filter) get D6 scoping for free because each sandbox *is* its own VM instance/process — no shared surface to bleed across. WSLC today is also effectively 1 sandbox : 1 VM (the one-shot flow creates a session, one container, then tears it down), but the highest-value WSLC optimization — **state-aware session reuse** (Misc #29), keeping a warm VM to amortize startup cost — makes one VM host **multiple** containers, at which point a host- or VM-wide rule genuinely bleeds across co-resident sandboxes. That is exactly when namespace-scoped enforcement (SDK dep #1) stops being merely cleaner and becomes mandatory. +> **Contrast with Hyperlight/Nanvix, and the state-aware wrinkle.** Hyperlight (network disabled for supported requests, per-instance) and Nanvix (all-deny or unrestricted networking, per-guest) keep their network posture scoped to each VM instance/process — no shared surface to bleed across. WSLC today is also effectively 1 sandbox : 1 VM (the one-shot flow creates a session, one container, then tears it down), but the highest-value WSLC optimization — **state-aware session reuse** (Misc #29), keeping a warm VM to amortize startup cost — makes one VM host **multiple** containers, at which point a host- or VM-wide rule genuinely bleeds across co-resident sandboxes. That is exactly when namespace-scoped enforcement (SDK dep #1) stops being merely cleaner and becomes mandatory. --- diff --git a/src/mxc-sdk/src/backends/hyperlight/common/mod.rs b/src/mxc-sdk/src/backends/hyperlight/common/mod.rs index 512beaff4..d6f4d3bb3 100644 --- a/src/mxc-sdk/src/backends/hyperlight/common/mod.rs +++ b/src/mxc-sdk/src/backends/hyperlight/common/mod.rs @@ -22,7 +22,7 @@ //! | Script delivery | `AppSandbox::run`, or `submit` + `step` under a deadline | //! | Cold start | Snapshot restore (~50–60 ms) | //! | Filesystem | Host dir mounts via `Mount` | -//! | Networking | Host-proxied sockets via `NetworkPolicy` | +//! | Networking | Disabled for supported requests | //! | Script I/O | Host's stdout/stderr (HostPrint) | //! | stdlib coverage | Full CPython + preloaded ML stack (numpy, pandas, etc.) | //! @@ -100,15 +100,15 @@ use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ - ExecutionRequest, HyperlightRuntime, NetworkPolicy, ScriptResponse, + ExecutionRequest, HyperlightRuntime, NetworkEnforcementMode, NetworkPolicy, ScriptResponse, }; use crate::mxc_common::script_runner::ScriptRunner; -use crate::mxc_common::validator::{validate_network_policy_support, NetworkPolicySupport}; +use crate::mxc_common::validator::{ + validate_common, validate_network_policy_support, NetworkPolicySupport, +}; use hyperlight_unikraft::hyperlight_host::HyperlightError; -use hyperlight_unikraft::{ - AllowList, AppSandbox, BlockList, Mount, SandboxBuilder, Snapshot, Yield, -}; +use hyperlight_unikraft::{AppSandbox, Mount, SandboxBuilder, Snapshot, Yield}; // -- Availability probe ------------------------------------------------------- @@ -259,34 +259,6 @@ pub struct HyperlightScriptRunner { rewind: Option>, active_home: Option, active_mounts: Vec, - active_policy: Option, - active_network: NetworkKey, -} - -/// The request's network policy as the runner keys a booted guest on it. -/// Both host lists are kept, so an allow list and a block list of the -/// same hosts key differently. -#[derive(Clone, Debug, PartialEq, Default)] -struct NetworkKey { - allowed: Vec, - blocked: Vec, - default: NetworkPolicy, -} - -impl NetworkKey { - fn from_request(request: &ExecutionRequest) -> Self { - let sorted = |hosts: &[String]| { - let mut hosts = hosts.to_vec(); - hosts.sort(); - hosts.dedup(); - hosts - }; - Self { - allowed: sorted(&request.policy.allowed_hosts), - blocked: sorted(&request.policy.blocked_hosts), - default: request.policy.default_network_policy.clone(), - } - } } /// A booted guest, parked at a boundary between calls. @@ -405,8 +377,6 @@ impl HyperlightScriptRunner { rewind: None, active_home: None, active_mounts: Vec::new(), - active_policy: None, - active_network: NetworkKey::default(), } } @@ -473,8 +443,7 @@ impl HyperlightScriptRunner { os_data_home().join(DEFAULT_HOME_LEAF) } - /// Reject only policies that the hyperlight backend genuinely cannot honor. - /// Filesystem mounts and network policies ARE supported. + /// Reject policies the guest cannot enforce before any sandbox is booted. fn validate_policies(request: &ExecutionRequest) -> Result<(), RunnerError> { if request.policy.network_proxy.is_enabled() { return Err(RunnerError::Preflight(ERR_PROXY_POLICY.to_string())); @@ -482,12 +451,15 @@ impl HyperlightScriptRunner { if !request.working_directory.is_empty() { return Err(RunnerError::Preflight(ERR_WORKDIR.to_string())); } - if request.policy.default_network_policy == NetworkPolicy::Block - && !request.policy.allowed_hosts.is_empty() - && !request.policy.blocked_hosts.is_empty() + if request.policy.default_network_policy != NetworkPolicy::Block + || request.policy.network_enforcement_mode != NetworkEnforcementMode::Capabilities + || request.policy.allow_local_network + || !request.policy.allowed_hosts.is_empty() + || !request.policy.blocked_hosts.is_empty() { return Err(RunnerError::Preflight( - "allowedHosts and blockedHosts are mutually exclusive".to_string(), + "retired network fields are not supported by Hyperlight; the guest runs without networking" + .to_string(), )); } @@ -524,35 +496,6 @@ impl HyperlightScriptRunner { Ok(()) } - /// Translate MXC's network policy fields into a guest `NetworkPolicy`. - /// - /// - `allowed_hosts` non-empty → `AllowList` (only listed hosts reachable) - /// - `blocked_hosts` non-empty → `BlockList` (listed hosts denied, rest allowed) - /// - `default_network_policy == Block`, no host lists → `None` (networking disabled) - /// - `default_network_policy == Allow`, no host lists → `AllowAll` - fn network_policy_from_key( - key: &NetworkKey, - ) -> Result, RunnerError> { - if !key.allowed.is_empty() { - let allow_list = AllowList::from_hosts(&key.allowed) - .map_err(|e| RunnerError::Preflight(format!("resolve allowed_hosts: {e}")))?; - return Ok(Some(hyperlight_unikraft::NetworkPolicy::AllowList( - allow_list, - ))); - } - if !key.blocked.is_empty() { - let block_list = BlockList::from_hosts(&key.blocked) - .map_err(|e| RunnerError::Preflight(format!("resolve blocked_hosts: {e}")))?; - return Ok(Some(hyperlight_unikraft::NetworkPolicy::BlockList( - block_list, - ))); - } - if key.default == NetworkPolicy::Block { - return Ok(None); - } - Ok(Some(hyperlight_unikraft::NetworkPolicy::AllowAll)) - } - /// Translate `ContainerPolicy.{readwrite,readonly}Paths` into /// [`Mount`] entries. Each host path is exposed inside the guest at /// `/host/` so scripts can find mounts predictably. @@ -636,28 +579,22 @@ impl HyperlightScriptRunner { /// /// The guest restores the persisted snapshot (warming and persisting /// one first if only the rootfs is present, a cold boot once per - /// image) with the request's mounts and network policy; the kernel - /// builds its mount table from them on resume. Later calls on the + /// image) with the request's mounts; the kernel builds its mount table + /// from them on resume. Later calls on the /// same runner rewind rather than boot. /// - /// The mount set and network policy are fixed at boot; a change in - /// either boots another guest from the same image. + /// The mount set is fixed at boot; changing it boots another guest from + /// the same image. fn ensure_runtime( &mut self, home: &Path, runtime: HyperlightRuntime, mounts: Vec, - network: NetworkKey, logger: &mut Logger, ) -> Result<(&mut Guest, Arc), RunnerError> { - let same_config = self.active_home.as_deref() == Some(home) - && mounts_equal(&self.active_mounts, &mounts) - && self.active_network == network; + let same_config = + self.active_home.as_deref() == Some(home) && mounts_equal(&self.active_mounts, &mounts); if !same_config { - // Host lists resolve names here, once per configuration, so a - // guest that is already up is not held to the resolver on - // every call. - let policy = Self::network_policy_from_key(&network)?; // Nothing booted so far applies to the new configuration; the // image still does, unless the home changed. self.guest = None; @@ -666,8 +603,6 @@ impl HyperlightScriptRunner { } self.active_home = Some(home.to_path_buf()); self.active_mounts = mounts; - self.active_policy = policy; - self.active_network = network; } let rewind = match self.rewind.clone() { Some(rewind) => rewind, @@ -682,7 +617,7 @@ impl HyperlightScriptRunner { Some(guest) => guest, None => { configure_surrogates(); - Self::boot_from_snapshot(rewind.clone(), &self.active_mounts, &self.active_policy)? + Self::boot_from_snapshot(rewind.clone(), &self.active_mounts)? } }; Ok((self.guest.insert(guest), rewind)) @@ -691,13 +626,9 @@ impl HyperlightScriptRunner { /// Restore the warm image into a new sandbox with `mounts` and /// `policy`: the kernel builds its mount table from them on resume, /// and the host serves them. - fn boot_from_snapshot( - rewind: Arc, - mounts: &[Mount], - policy: &Option, - ) -> Result { + fn boot_from_snapshot(rewind: Arc, mounts: &[Mount]) -> Result { let builder = SandboxBuilder::from_snapshot(rewind).mounts(mounts.iter().cloned()); - let sandbox = with_network(builder, policy) + let sandbox = builder .boot() .map_err(|e| RunnerError::Runtime(format!("restore hyperlight snapshot: {e}")))?; Ok(Guest { @@ -750,16 +681,12 @@ impl HyperlightScriptRunner { } Ok(timing) } -} - -impl ScriptRunner for HyperlightScriptRunner { - fn validate_runner(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { - Self::validate_policies(request).map_err(|e| e.to_response())?; - validate_network_policy_support(request, NetworkPolicySupport::default())?; - Ok(()) - } - fn execute(&mut self, request: &ExecutionRequest, logger: &mut Logger) -> ScriptResponse { + fn execute_validated( + &mut self, + request: &ExecutionRequest, + logger: &mut Logger, + ) -> ScriptResponse { let runtime = request .hyperlight .as_ref() @@ -779,13 +706,7 @@ impl ScriptRunner for HyperlightScriptRunner { return e.to_response(); } }; - let (guest, rewind) = match self.ensure_runtime( - &home, - runtime, - mounts, - NetworkKey::from_request(request), - logger, - ) { + let (guest, rewind) = match self.ensure_runtime(&home, runtime, mounts, logger) { Ok(pair) => pair, Err(e) => { logger.log_line(&e.to_string()); @@ -830,6 +751,37 @@ impl ScriptRunner for HyperlightScriptRunner { } } +impl ScriptRunner for HyperlightScriptRunner { + fn validate_runner(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { + Self::validate_policies(request).map_err(|e| e.to_response())?; + validate_network_policy_support(request, NetworkPolicySupport::default())?; + Ok(()) + } + + fn run(&mut self, request: &ExecutionRequest, logger: &mut Logger) -> ScriptResponse { + if let Err(response) = validate_common(request) { + return response; + } + if let Err(response) = self.validate_runner(request) { + return response; + } + if request.dry_run { + return ScriptResponse { + exit_code: 0, + ..Default::default() + }; + } + self.execute_validated(request, logger) + } + + fn execute(&mut self, request: &ExecutionRequest, logger: &mut Logger) -> ScriptResponse { + if let Err(response) = self.validate_runner(request) { + return response; + } + self.execute_validated(request, logger) + } +} + // -- Guest driving ----------------------------------------------------------- /// Run `code` and wait for it, giving up at `timeout`. @@ -1064,16 +1016,6 @@ fn rootfs_builder(home: &Path, runtime: HyperlightRuntime) -> SandboxBuilder { .scratch_mb(runtime_image(runtime).scratch_mb) } -fn with_network( - builder: SandboxBuilder, - policy: &Option, -) -> SandboxBuilder { - match policy { - Some(policy) => builder.network(policy.clone()), - None => builder, - } -} - /// Pull the rootfs CPIO out of the published image into `dst`, straight /// from the registry's distribution API: no container runtime needed. /// Staged beside `dst` and renamed into place so a failed pull leaves no @@ -2177,152 +2119,105 @@ mod tests { } #[test] - fn network_key_tells_an_allow_list_from_a_block_list() { - let allow = ExecutionRequest { - policy: ContainerPolicy { - allowed_hosts: vec!["a.example".to_string()], - ..Default::default() - }, - ..Default::default() - }; - let block = ExecutionRequest { - policy: ContainerPolicy { - blocked_hosts: vec!["a.example".to_string()], - ..Default::default() - }, - ..Default::default() - }; - assert_ne!( - NetworkKey::from_request(&allow), - NetworkKey::from_request(&block) - ); - assert_eq!( - NetworkKey::from_request(&allow), - NetworkKey::from_request(&allow) - ); + fn omitted_network_policy_is_valid_and_keeps_guest_disconnected() { + runner() + .validate_runner(&ExecutionRequest::default()) + .unwrap(); } #[test] - fn network_policy_allow_all_when_default_allow() { - let request = ExecutionRequest { - policy: ContainerPolicy { + fn retired_network_fields_are_rejected_before_boot() { + for policy in [ + ContainerPolicy { default_network_policy: NetworkPolicy::Allow, ..Default::default() }, - ..Default::default() - }; - let policy = - HyperlightScriptRunner::network_policy_from_key(&NetworkKey::from_request(&request)) - .unwrap(); - assert!(matches!( - policy, - Some(hyperlight_unikraft::NetworkPolicy::AllowAll) - )); - } - - #[test] - fn network_policy_allowlist_from_allowed_hosts() { - let request = ExecutionRequest { - policy: ContainerPolicy { - allowed_hosts: vec!["127.0.0.1".to_string()], + ContainerPolicy { + allowed_hosts: vec!["a.example".to_string()], ..Default::default() }, - ..Default::default() - }; - let policy = - HyperlightScriptRunner::network_policy_from_key(&NetworkKey::from_request(&request)) - .unwrap(); - assert!(matches!( - policy, - Some(hyperlight_unikraft::NetworkPolicy::AllowList(_)) - )); - } - - #[test] - fn network_policy_none_when_blocked() { - let request = ExecutionRequest { - policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Block, + ContainerPolicy { + blocked_hosts: vec!["b.example".to_string()], ..Default::default() }, - ..Default::default() - }; - let policy = - HyperlightScriptRunner::network_policy_from_key(&NetworkKey::from_request(&request)) - .unwrap(); - assert!(policy.is_none()); + ContainerPolicy { + network_enforcement_mode: NetworkEnforcementMode::Firewall, + ..Default::default() + }, + ContainerPolicy { + allow_local_network: true, + ..Default::default() + }, + ] { + let request = ExecutionRequest { + policy, + ..Default::default() + }; + let error = runner().validate_runner(&request).unwrap_err(); + assert!(error.error_message.contains("retired network fields")); + } } #[test] - fn network_policy_blocklist_from_blocked_hosts() { - let request = ExecutionRequest { - policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, - blocked_hosts: vec!["127.0.0.1".to_string()], - ..Default::default() - }, + fn explicit_directional_allow_is_rejected_before_boot() { + let mut request = ExecutionRequest::default(); + request.policy.network_egress = Some(crate::mxc_common::models::NetworkEgressPolicy { + default: crate::mxc_common::models::NetworkAction::Allow, ..Default::default() - }; - let policy = - HyperlightScriptRunner::network_policy_from_key(&NetworkKey::from_request(&request)) - .unwrap(); - assert!(matches!( - policy, - Some(hyperlight_unikraft::NetworkPolicy::BlockList(_)) - )); + }); + let error = runner().validate_runner(&request).unwrap_err(); + assert!(error.error_message.contains("network.egress.default")); } #[test] - fn policy_rejects_blocklist_without_allowlist_under_block_default() { + fn direct_execution_rejects_retired_host_lists_before_boot() { + let mut r = runner(); let request = ExecutionRequest { policy: ContainerPolicy { - blocked_hosts: vec!["127.0.0.1".to_string()], + allowed_hosts: vec!["a.example".to_string()], ..Default::default() }, ..Default::default() }; - - let error = runner().validate_runner(&request).unwrap_err(); - assert_eq!( - error.error_message, - "blockedHosts requires allowedHosts when network.defaultPolicy='block'" - ); + let mut logger = Logger::new(Mode::Buffer); + let resp = r.execute(&request, &mut logger); + assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + assert!(resp.error_message.contains("retired network fields")); } #[test] - fn policy_rejects_allowlist_under_allow_default() { - let request = ExecutionRequest { - policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, - allowed_hosts: vec!["127.0.0.1".to_string()], - ..Default::default() - }, + fn direct_execute_rejects_network_policy_before_boot() { + let mut request = ExecutionRequest { + script_code: "print('x')".to_string(), ..Default::default() }; - - let error = runner().validate_runner(&request).unwrap_err(); - assert_eq!( - error.error_message, - "allowedHosts requires network.defaultPolicy='block'" - ); + request.policy.network_egress = Some(crate::mxc_common::models::NetworkEgressPolicy { + default: crate::mxc_common::models::NetworkAction::Allow, + ..Default::default() + }); + let mut r = runner(); + let mut logger = Logger::new(Mode::Buffer); + let response = r.execute(&request, &mut logger); + assert!(response.error_message.contains("network.egress.default")); } #[test] - fn policy_rejects_allowed_and_blocked_hosts() { - let mut r = runner(); - let request = ExecutionRequest { + fn dry_run_validates_without_booting() { + let mut request = ExecutionRequest { script_code: "print('x')".to_string(), - policy: ContainerPolicy { - allowed_hosts: vec!["a.com".to_string()], - blocked_hosts: vec!["b.com".to_string()], - ..Default::default() - }, + dry_run: true, ..Default::default() }; + let mut r = runner(); let mut logger = Logger::new(Mode::Buffer); - let resp = r.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); - assert!(resp.error_message.contains("mutually exclusive")); + assert_eq!(r.run(&request, &mut logger).exit_code, 0); + + request.policy.network_egress = Some(crate::mxc_common::models::NetworkEgressPolicy { + default: crate::mxc_common::models::NetworkAction::Allow, + ..Default::default() + }); + let response = r.run(&request, &mut logger); + assert!(response.error_message.contains("network.egress.default")); } #[test] diff --git a/src/mxc-sdk/src/backends/nanvix/runner/mod.rs b/src/mxc-sdk/src/backends/nanvix/runner/mod.rs index a578d9f97..fb84b0b71 100644 --- a/src/mxc-sdk/src/backends/nanvix/runner/mod.rs +++ b/src/mxc-sdk/src/backends/nanvix/runner/mod.rs @@ -36,10 +36,10 @@ //! ## Networking //! //! Host networking is **off by default** and is enabled per-run by passing -//! `-allow-host-networking` to `nanvixd`. The runner adds that flag when the -//! request sets `network.defaultPolicy = "allow"`, or when `allowedHosts` / -//! `blockedHosts` is present (forwarded as `-allow-host` / `-block-host`). -//! Network proxies are not supported and are rejected at validation time. +//! `-allow-host-networking` to `nanvixd` only when egress.default, +//! ingress.default, and ingress.hostLoopback all allow. All-deny (including +//! omitted defaults) stays disconnected; mixed postures, egress rules, proxies, +//! and retired legacy network fields are rejected before execution. //! //! Auto-discovery //! @@ -47,7 +47,6 @@ //! are discovered next to the running executable. No configuration is needed. use std::fmt::Write; -use std::net::ToSocketAddrs; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; use std::sync::atomic::{AtomicBool, Ordering}; @@ -57,7 +56,9 @@ use std::thread::JoinHandle; use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ExecutionRequest, NetworkAction, NetworkPolicy, ScriptResponse}; +use crate::mxc_common::models::{ + ExecutionRequest, NetworkAction, NetworkEnforcementMode, NetworkPolicy, ScriptResponse, +}; use crate::mxc_common::script_runner::ScriptRunner; use crate::mxc_common::validator::{validate_network_policy_support, NetworkPolicySupport}; @@ -99,44 +100,17 @@ const ERR_DENIED_PATHS: &str = concat!( "-- the guest has no host filesystem visibility. ", "Only readwrite_paths and readonly_paths are supported", ); -const ERR_NETWORK_HOSTS: &str = concat!( - "allowedHosts and blockedHosts are mutually exclusive for the NanVix backend -- ", - "the guest egress filter is allow-XOR-block. Specify an allowlist (allowedHosts) ", - "or a blocklist (blockedHosts), not both", -); -const ERR_HOSTS_UNRESOLVED: &str = concat!( - "none of the specified allowedHosts/blockedHosts resolved to an IPv4 address -- ", - "the NanVix guest filter is IPv4-only; use IPv4 literals/CIDR or hosts with A records", -); -const ERR_BLOCKED_HOST_UNRESOLVED: &str = concat!( - "a blockedHosts entry did not resolve to any IPv4 address -- ", - "the NanVix guest egress filter is static (resolved once at preflight) and IPv4-only, ", - "so an unresolvable blocked host cannot be enforced. Silently dropping it would let ", - "traffic the policy explicitly blocks flow freely, so the run is rejected (fail-closed). ", - "Use an IPv4 literal/CIDR or a host with A records", -); +const ERR_LEGACY_NETWORK: &str = "NanVix does not support retired legacy network fields \ + (defaultPolicy, enforcementMode, allowLocalNetwork, allowedHosts, blockedHosts); \ + use network.egress and network.ingress"; const ERR_PROXY_POLICY: &str = "network proxy is not supported by the NanVix backend"; const ERR_DIRECTIONAL_NETWORK: &str = "NanVix supports only fully isolated or explicitly \ unrestricted directional networking: egress.default, ingress.default and ingress.hostLoopback \ must all be deny or all be allow; independent ingress or host-loopback restrictions are not supported"; -const ERR_DIRECTIONAL_FILTERS: &str = "NanVix cannot enforce directional egress rules: its legacy \ - IPv4 filter has implicit exceptions and does not implement the directional rule contract; \ +const ERR_DIRECTIONAL_FILTERS: &str = "NanVix cannot enforce directional egress rules; \ use fully isolated networking or explicitly unrestricted networking without rules"; const ERR_WORKDIR: &str = "workingDirectory is not supported by the NanVix backend -- guest has its own filesystem namespace"; -/// Outcome of resolving a request's egress host lists. -/// -/// `allow`/`block` are the IPv4/CIDR literals handed to nanvixd; at most one is -/// non-empty. `warnings` carries human-readable notices for allowlist entries -/// that were dropped during resolution (blocklist drops are a hard error and -/// never reach here). -#[derive(Debug)] -struct ResolvedHostLists { - allow: Vec, - block: Vec, - warnings: Vec, -} - /// Maps a finished child's [`ExitStatus`] to a host-visible exit code. /// /// On Unix, processes terminated by a signal have no exit code (`status.code()` @@ -527,16 +501,18 @@ impl NanVixScriptRunner { fn resolve_networking_mode(request: &ExecutionRequest) -> Result { let policy = &request.policy; - if policy.network_egress.is_none() && policy.network_ingress.is_none() { - return Ok(policy.default_network_policy == NetworkPolicy::Allow - || !policy.allowed_hosts.is_empty()); - } - if !policy.allowed_hosts.is_empty() + if policy.default_network_policy != NetworkPolicy::Block + || policy.network_enforcement_mode != NetworkEnforcementMode::Capabilities + || policy.allow_local_network + || !policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty() - || policy - .network_egress - .as_ref() - .is_some_and(|egress| !egress.allow.is_empty() || !egress.deny.is_empty()) + { + return Err(NanVixError::Preflight(ERR_LEGACY_NETWORK.to_string())); + } + if policy + .network_egress + .as_ref() + .is_some_and(|egress| !egress.allow.is_empty() || !egress.deny.is_empty()) { return Err(NanVixError::Preflight(ERR_DIRECTIONAL_FILTERS.to_string())); } @@ -561,122 +537,11 @@ impl NanVixScriptRunner { Ok(egress == NetworkAction::Allow) } - /// Resolves a host entry list into IPv4/CIDR literals for nanvixd's - /// `-allow-host`/`-block-host` flags, alongside the entries that resolved - /// to nothing. - /// - /// - `a.b.c.d` and `a.b.c.d/n` literals pass through unchanged. - /// - Hostnames resolve to their IPv4 (A-record) addresses; AAAA results are - /// dropped because the guest filter is IPv4-only. - /// - Entries that fail to parse or resolve to any IPv4 address contribute - /// nothing to the resolved list and are collected into the second return - /// value so callers can warn (allowlist) or reject (blocklist). - /// - /// Mirrors `lxc::network_iptables::resolve_host` for the hostname-to-IPv4 - /// mapping; unlike that helper this also preserves IPv4/CIDR literals. - /// - /// Returns `(resolved_ips, unresolved_entries)`. Empty/whitespace entries - /// are ignored entirely and appear in neither list. - fn resolve_hosts_detailed(hosts: &[String]) -> (Vec, Vec) { - let mut out: Vec = Vec::new(); - let mut unresolved: Vec = Vec::new(); - for host in hosts { - let entry = host.trim(); - if entry.is_empty() { - continue; - } - let before = out.len(); - // CIDR literal: pass through only when the address is IPv4 and the - // prefix is in range. nanvixd parses CIDR directly. - if let Some((addr, prefix)) = entry.split_once('/') { - let addr_ok = addr.trim().parse::().is_ok(); - let prefix_ok = prefix - .trim() - .parse::() - .map(|p| p <= 32) - .unwrap_or(false); - if addr_ok && prefix_ok { - out.push(entry.to_string()); - } - } else if let Ok(addr) = entry.parse::() { - // Bare IP literal: keep IPv4, drop IPv6. - if addr.is_ipv4() { - out.push(entry.to_string()); - } - } else if let Ok(addrs) = format!("{}:0", entry).to_socket_addrs() { - // Hostname: resolve to IPv4 A records. - for ip in addrs.map(|a| a.ip()).filter(|ip| ip.is_ipv4()) { - out.push(ip.to_string()); - } - } - if out.len() == before { - unresolved.push(entry.to_string()); - } - } - (out, unresolved) - } - - /// Resolves the request's allow/block host lists, failing closed. - /// - /// Returns the resolved allow/block IPv4 lists plus human-readable warnings - /// for any allowlist entries that were dropped. Shared validation rejects - /// invalid default/list combinations before execution, and NanVix rejects - /// simultaneous lists because its guest filter is allow-XOR-block. - /// - /// Fail-closed semantics differ by list direction: - /// - **Allowlist** (deny-by-default): a fully unresolvable allowlist is an - /// error, because emitting `-allow-host-networking` with no filter would - /// fail open (nanvixd treats no list as allow-all). Partially dropped - /// entries only narrow access, so they are reported as warnings and the - /// run continues. - /// - **Blocklist** (allow-by-default): *any* unresolvable entry is an - /// error. Silently dropping a blocked host would let traffic the policy - /// explicitly blocks flow freely (fail-open), and the static preflight - /// filter cannot enforce a name that does not resolve. - fn resolve_host_lists(request: &ExecutionRequest) -> Result { - let (allow, allow_unresolved) = Self::resolve_hosts_detailed(&request.policy.allowed_hosts); - if !request.policy.allowed_hosts.is_empty() && allow.is_empty() { - return Err(NanVixError::Preflight(ERR_HOSTS_UNRESOLVED.to_string())); - } - - let (block, block_unresolved) = Self::resolve_hosts_detailed(&request.policy.blocked_hosts); - if let Some(first) = block_unresolved.first() { - return Err(NanVixError::Preflight(format!( - "{} (entry: '{}')", - ERR_BLOCKED_HOST_UNRESOLVED, first - ))); - } - - let warnings = allow_unresolved - .iter() - .map(|h| { - format!( - "Warning: could not resolve allowedHosts entry '{}' to an IPv4 address; skipping", - h - ) - }) - .collect(); - - Ok(ResolvedHostLists { - allow, - block, - warnings, - }) - } - fn validate_policies(request: &ExecutionRequest) -> Result<(), NanVixError> { // denied_paths is explicitly rejected — microvm has no host visibility. if !request.policy.denied_paths.is_empty() { return Err(NanVixError::Preflight(ERR_DENIED_PATHS.to_string())); } - // NanVix's guest egress filter is allow-XOR-block and cannot represent - // simultaneous allow and block lists, even though the shared policy model can. - if request.policy.default_network_policy == NetworkPolicy::Block - && !request.policy.allowed_hosts.is_empty() - && !request.policy.blocked_hosts.is_empty() - { - return Err(NanVixError::Preflight(ERR_NETWORK_HOSTS.to_string())); - } if request.policy.network_proxy.is_enabled() { return Err(NanVixError::Preflight(ERR_PROXY_POLICY.to_string())); } @@ -692,8 +557,6 @@ impl NanVixScriptRunner { paths: &ResolvedPaths, staging_dir: &Path, request: &ExecutionRequest, - allow_hosts: &[String], - block_hosts: &[String], ) -> Result { let host_networking = Self::resolve_networking_mode(request)?; let trace = nanvix_trace_enabled(); @@ -718,18 +581,6 @@ impl NanVixScriptRunner { cmd.arg("-allow-host-networking"); } - // Per-host egress filtering. The supplied lists have already been - // reduced to the one that refines the shared legacy default. NanVix - // rejects simultaneous lists during validation, so at most one loop - // emits flags. The guest daemon auto-exempts the DNS port in allowlist - // mode, so no resolver IPs are added here. - for host in allow_hosts { - cmd.arg("-allow-host").arg(host); - } - for host in block_hosts { - cmd.arg("-block-host").arg(host); - } - #[cfg(target_os = "windows")] { // nanvixd loads kernel.vmem from /snapshots/ so cwd must be @@ -777,10 +628,8 @@ impl NanVixScriptRunner { paths: &ResolvedPaths, staging_dir: &Path, request: &ExecutionRequest, - allow_hosts: &[String], - block_hosts: &[String], ) -> Result { - Self::nanvixd_command(paths, staging_dir, request, allow_hosts, block_hosts)? + Self::nanvixd_command(paths, staging_dir, request)? .spawn() .map_err(|e| { NanVixError::Platform(format!("failed to spawn {}: {}", NANVIXD_BINARY, e)) @@ -990,6 +839,9 @@ impl ScriptRunner for NanVixScriptRunner { } fn execute(&mut self, request: &ExecutionRequest, logger: &mut Logger) -> ScriptResponse { + if let Err(response) = self.validate_runner(request) { + return response; + } let host_networking = match Self::resolve_networking_mode(request) { Ok(enabled) => enabled, Err(error) => return error.to_response(), @@ -1026,31 +878,7 @@ impl ScriptRunner for NanVixScriptRunner { if host_networking { let _ = writeln!(logger, "NanVix: host networking enabled"); } - let (allow_hosts, block_hosts) = match Self::resolve_host_lists(request) { - Ok(resolved) => { - for warning in &resolved.warnings { - let _ = writeln!(logger, "NanVix: {}", warning); - } - (resolved.allow, resolved.block) - } - Err(e) => { - let _ = writeln!(logger, "{}", e); - return e.to_response(); - } - }; - if !allow_hosts.is_empty() { - let _ = writeln!(logger, "NanVix: egress allowlist={:?}", allow_hosts); - } - if !block_hosts.is_empty() { - let _ = writeln!(logger, "NanVix: egress blocklist={:?}", block_hosts); - } - let mut child = match Self::spawn_nanvixd( - &paths, - staging.path(), - request, - &allow_hosts, - &block_hosts, - ) { + let mut child = match Self::spawn_nanvixd(&paths, staging.path(), request) { Ok(c) => c, Err(e) => { let _ = writeln!(logger, "{}", e); @@ -1102,7 +930,7 @@ impl ScriptRunner for NanVixScriptRunner { mod tests { use super::*; use crate::mxc_common::logger::{Logger, Mode}; - use crate::mxc_common::models::{ContainerPolicy, NetworkPolicy}; + use crate::mxc_common::models::ContainerPolicy; #[test] fn total_timeout_adds_boot_staging_and_script() { @@ -1147,11 +975,7 @@ mod tests { } } - fn command_arguments( - request: &ExecutionRequest, - allow_hosts: &[String], - block_hosts: &[String], - ) -> Result, NanVixError> { + fn command_arguments(request: &ExecutionRequest) -> Result, NanVixError> { let root = PathBuf::from("nanvix-command-test"); let paths = ResolvedPaths { nanvixd: root.join(NANVIXD_BINARY), @@ -1160,13 +984,7 @@ mod tests { exe_dir: root.clone(), snapshot_home: root, }; - let command = NanVixScriptRunner::nanvixd_command( - &paths, - Path::new("staging"), - request, - allow_hosts, - block_hosts, - )?; + let command = NanVixScriptRunner::nanvixd_command(&paths, Path::new("staging"), request)?; Ok(command .get_args() .map(|argument| argument.to_string_lossy().into_owned()) @@ -1186,7 +1004,7 @@ mod tests { NanVixScriptRunner::resolve_networking_mode(&request).unwrap(), egress == NetworkAction::Allow ); - let arguments = command_arguments(&request, &[], &[]).unwrap(); + let arguments = command_arguments(&request).unwrap(); assert_eq!( arguments .iter() @@ -1197,7 +1015,7 @@ mod tests { } else { let error = runner.validate_runner(&request).unwrap_err(); assert!(error.error_message.contains(ERR_DIRECTIONAL_NETWORK)); - assert!(command_arguments(&request, &[], &[]).is_err()); + assert!(command_arguments(&request).is_err()); } } } @@ -1213,6 +1031,8 @@ mod tests { ); request.policy.network_ingress = None; assert!(NanVixScriptRunner::resolve_networking_mode(&request).is_err()); + assert!(NanVixScriptRunner::new().validate_runner(&request).is_err()); + assert!(command_arguments(&request).is_err()); } #[test] @@ -1240,7 +1060,10 @@ mod tests { "unexpected validation error: {}", error.error_message ); - assert!(command_arguments(&request, &[], &[]).is_err()); + assert!(command_arguments(&request).is_err()); + let mut runner = NanVixScriptRunner::new(); + let response = runner.run(&request, &mut Logger::new(Mode::Buffer)); + assert!(response.error_message.contains(ERR_DIRECTIONAL_FILTERS)); } } @@ -1260,7 +1083,7 @@ mod tests { }; NanVixScriptRunner::new().validate_runner(&request).unwrap(); assert!(NanVixScriptRunner::resolve_networking_mode(&request).unwrap()); - let arguments = command_arguments(&request, &[], &[]).unwrap(); + let arguments = command_arguments(&request).unwrap(); assert_eq!( arguments.first().map(String::as_str), Some("-allow-host-networking") @@ -1272,46 +1095,35 @@ mod tests { } #[test] - fn command_arguments_preserve_network_defaults_and_host_filters() { - for (default, allow, block, expected_prefix) in [ - (NetworkPolicy::Block, vec![], vec![], vec![]), + fn command_arguments_use_only_directional_host_networking() { + for (request, enabled) in [ + (ExecutionRequest::default(), false), ( - NetworkPolicy::Allow, - vec![], - vec![], - vec!["-allow-host-networking"], + directional_request( + NetworkAction::Deny, + NetworkAction::Deny, + NetworkAction::Deny, + ), + false, ), ( - NetworkPolicy::Block, - vec!["192.0.2.1"], - vec![], - vec!["-allow-host-networking", "-allow-host", "192.0.2.1"], - ), - ( - NetworkPolicy::Allow, - vec![], - vec!["192.0.2.0/24"], - vec!["-allow-host-networking", "-block-host", "192.0.2.0/24"], + directional_request( + NetworkAction::Allow, + NetworkAction::Allow, + NetworkAction::Allow, + ), + true, ), ] { - let request = ExecutionRequest { - policy: ContainerPolicy { - default_network_policy: default, - allowed_hosts: allow.into_iter().map(str::to_owned).collect(), - blocked_hosts: block.into_iter().map(str::to_owned).collect(), - ..Default::default() - }, - ..Default::default() - }; - let resolved = NanVixScriptRunner::resolve_host_lists(&request).unwrap(); - let arguments = command_arguments(&request, &resolved.allow, &resolved.block).unwrap(); - let expected: Vec = expected_prefix.into_iter().map(str::to_owned).collect(); - assert!(arguments.starts_with(&expected), "{arguments:?}"); + let arguments = command_arguments(&request).unwrap(); assert_eq!( - arguments.iter().any(|arg| arg == "-allow-host-networking"), - !expected.is_empty(), + arguments.first().map(String::as_str) == Some("-allow-host-networking"), + enabled, "{arguments:?}" ); + assert!(!arguments + .iter() + .any(|arg| arg == "-allow-host" || arg == "-block-host")); } } @@ -1364,248 +1176,88 @@ mod tests { } #[test] - fn policy_accepts_allowlist_only() { - // A bare allowlist is now supported (forwarded as -allow-host). - let request = ExecutionRequest { - script_code: "echo test".to_string(), - policy: ContainerPolicy { - allowed_hosts: vec!["93.184.216.34".to_string()], - ..Default::default() - }, - ..Default::default() - }; - assert!( - NanVixScriptRunner::validate_policies(&request).is_ok(), - "a bare allowlist should pass validation" - ); - } - - #[test] - fn policy_rejects_blocklist_without_allowlist_under_block_default() { - let request = ExecutionRequest { - script_code: "echo test".to_string(), - policy: ContainerPolicy { - blocked_hosts: vec!["93.184.216.34".to_string()], - ..Default::default() - }, - ..Default::default() - }; - let error = NanVixScriptRunner::new() - .validate_runner(&request) - .unwrap_err(); - assert_eq!( - error.error_message, - "blockedHosts requires allowedHosts when network.defaultPolicy='block'" - ); - } - - #[test] - fn policy_rejects_both_host_lists() { - // allow + block are mutually exclusive (the guest filter is allow-XOR-block). - let request = ExecutionRequest { - script_code: "echo test".to_string(), - policy: ContainerPolicy { - allowed_hosts: vec!["10.0.0.1".to_string()], - blocked_hosts: vec!["10.0.0.2".to_string()], - ..Default::default() - }, - ..Default::default() - }; - let err = NanVixScriptRunner::validate_policies(&request).unwrap_err(); - assert!( - err.to_string().contains(ERR_NETWORK_HOSTS), - "both lists should be rejected, got: {}", - err - ); - } - - // -- Host resolution / decision-matrix tests -------------------------------- - - #[test] - fn resolve_hosts_passes_ipv4_and_cidr_literals() { - let hosts = vec![ - "1.2.3.4".to_string(), - "10.0.0.0/8".to_string(), - "192.168.1.1/32".to_string(), - ]; - let (resolved, unresolved) = NanVixScriptRunner::resolve_hosts_detailed(&hosts); - assert_eq!(resolved, vec!["1.2.3.4", "10.0.0.0/8", "192.168.1.1/32"]); - assert!(unresolved.is_empty()); - } - - #[test] - fn resolve_hosts_drops_ipv6_and_bad_entries() { - let hosts = vec![ - "::1".to_string(), // IPv6 literal -> dropped - "2001:db8::/32".to_string(), // IPv6 CIDR -> dropped (addr not IPv4) - "1.2.3.4/33".to_string(), // out-of-range prefix -> dropped - " ".to_string(), // blank -> skipped - "5.6.7.8".to_string(), // valid -> kept - ]; - let (resolved, _) = NanVixScriptRunner::resolve_hosts_detailed(&hosts); - assert_eq!(resolved, vec!["5.6.7.8"]); - } - - #[test] - fn resolve_host_lists_returns_resolved_allow() { - let request = ExecutionRequest { - policy: ContainerPolicy { - allowed_hosts: vec!["1.1.1.1".to_string(), "8.8.8.8/32".to_string()], - ..Default::default() - }, - ..Default::default() - }; - let resolved = NanVixScriptRunner::resolve_host_lists(&request).unwrap(); - assert_eq!(resolved.allow, vec!["1.1.1.1", "8.8.8.8/32"]); - assert!(resolved.block.is_empty()); - assert!(resolved.warnings.is_empty()); - } - - #[test] - fn resolve_host_lists_fails_closed_when_allowlist_unresolvable() { - // A non-empty allowlist that resolves to nothing must error rather than - // silently fall through to allow-all. - let request = ExecutionRequest { - policy: ContainerPolicy { - // IPv6-only literal resolves to no IPv4 entry. - allowed_hosts: vec!["::1".to_string()], - ..Default::default() - }, - ..Default::default() - }; - let err = NanVixScriptRunner::resolve_host_lists(&request).unwrap_err(); - assert!( - err.to_string().contains(ERR_HOSTS_UNRESOLVED), - "unresolvable allowlist should fail closed, got: {}", - err - ); - } - - #[test] - fn resolve_hosts_detailed_reports_unresolved_entries() { - // IPv4/CIDR pass through; IPv6 + malformed entries are reported as - // unresolved; blanks are ignored entirely. - let hosts = vec![ - "5.6.7.8".to_string(), // valid -> kept - "::1".to_string(), // IPv6 literal -> unresolved - "2001:db8::/32".to_string(), // IPv6 CIDR -> unresolved - "1.2.3.4/33".to_string(), // out-of-range prefix -> unresolved - "not_a_host.invalid".to_string(), // no A record -> unresolved - " ".to_string(), // blank -> ignored (neither list) + fn directly_constructed_legacy_network_fields_fail_closed() { + let legacy_policies = [ + ( + "defaultPolicy", + ContainerPolicy { + default_network_policy: NetworkPolicy::Allow, + ..Default::default() + }, + ), + ( + "enforcementMode", + ContainerPolicy { + network_enforcement_mode: NetworkEnforcementMode::Firewall, + ..Default::default() + }, + ), + ( + "allowLocalNetwork", + ContainerPolicy { + allow_local_network: true, + ..Default::default() + }, + ), + ( + "allowedHosts", + ContainerPolicy { + allowed_hosts: vec!["192.0.2.1".to_string()], + ..Default::default() + }, + ), + ( + "blockedHosts", + ContainerPolicy { + blocked_hosts: vec!["192.0.2.1".to_string()], + ..Default::default() + }, + ), ]; - let (resolved, unresolved) = NanVixScriptRunner::resolve_hosts_detailed(&hosts); - assert_eq!(resolved, vec!["5.6.7.8"]); - assert_eq!( - unresolved, - vec!["::1", "2001:db8::/32", "1.2.3.4/33", "not_a_host.invalid"] - ); - } + for (field, legacy_policy) in legacy_policies { + let mut request = directional_request( + NetworkAction::Allow, + NetworkAction::Allow, + NetworkAction::Allow, + ); + request.script_code = "print(1)".to_string(); + request.policy.default_network_policy = legacy_policy.default_network_policy; + request.policy.network_enforcement_mode = legacy_policy.network_enforcement_mode; + request.policy.allow_local_network = legacy_policy.allow_local_network; + request.policy.allowed_hosts = legacy_policy.allowed_hosts; + request.policy.blocked_hosts = legacy_policy.blocked_hosts; - #[test] - fn resolve_host_lists_warns_on_dropped_allowlist_entries() { - // A partially-resolvable allowlist narrows access (fail-safe): the run - // continues and each dropped entry produces a warning. - let request = ExecutionRequest { - policy: ContainerPolicy { - allowed_hosts: vec![ - "9.9.9.9".to_string(), - "::1".to_string(), - "dropme.invalid".to_string(), - ], - ..Default::default() - }, - ..Default::default() - }; - let resolved = NanVixScriptRunner::resolve_host_lists(&request).unwrap(); - assert_eq!(resolved.allow, vec!["9.9.9.9"]); - assert!(resolved.block.is_empty()); - assert_eq!(resolved.warnings.len(), 2, "two entries were dropped"); - assert!(resolved.warnings.iter().any(|w| w.contains("::1"))); - assert!(resolved - .warnings - .iter() - .any(|w| w.contains("dropme.invalid"))); - } + let error = NanVixScriptRunner::new() + .validate_runner(&request) + .unwrap_err(); + assert!(error.error_message.contains(ERR_LEGACY_NETWORK), "{field}"); + assert!(command_arguments(&request).is_err(), "{field}"); - #[test] - fn resolve_host_lists_fails_closed_on_unresolvable_blocklist_entry() { - // A blocklist (allow-by-default) must fail closed if ANY entry cannot - // be resolved -- silently dropping it would let blocked traffic flow. - let request = ExecutionRequest { - policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, - blocked_hosts: vec!["10.0.0.1".to_string(), "::1".to_string()], - ..Default::default() - }, - ..Default::default() - }; - let err = NanVixScriptRunner::resolve_host_lists(&request).unwrap_err(); - assert!( - err.to_string().contains(ERR_BLOCKED_HOST_UNRESOLVED), - "unresolvable blocklist entry should fail closed, got: {}", - err - ); - assert!( - err.to_string().contains("::1"), - "error should name the offending entry, got: {}", - err - ); + let mut runner = NanVixScriptRunner::new(); + let mut logger = Logger::new(Mode::Buffer); + let response = runner.run(&request, &mut logger); + assert!( + response.error_message.contains(ERR_LEGACY_NETWORK), + "{field}" + ); + let response = runner.execute(&request, &mut logger); + assert!( + response.error_message.contains(ERR_LEGACY_NETWORK), + "{field}" + ); + } } - #[test] - fn resolve_host_lists_accepts_fully_resolved_blocklist() { - let request = ExecutionRequest { - policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, - blocked_hosts: vec!["10.0.0.1".to_string(), "192.168.0.0/16".to_string()], - ..Default::default() - }, - ..Default::default() - }; - let resolved = NanVixScriptRunner::resolve_host_lists(&request).unwrap(); - assert!(resolved.allow.is_empty()); - assert_eq!(resolved.block, vec!["10.0.0.1", "192.168.0.0/16"]); - assert!(resolved.warnings.is_empty()); - } + // -- Directional network decision tests ------------------------------------ #[test] - fn default_block_no_lists_disables_host_networking() { - // The default posture (block, no lists) keeps networking off. + fn omitted_network_policy_disables_host_networking() { let request = ExecutionRequest::default(); assert!(!NanVixScriptRunner::resolve_networking_mode(&request).unwrap()); - let resolved = NanVixScriptRunner::resolve_host_lists(&request).unwrap(); - assert!(resolved.allow.is_empty() && resolved.block.is_empty()); - assert!(resolved.warnings.is_empty()); - } - - #[test] - fn allow_policy_enables_host_networking() { - // `network.defaultPolicy = "allow"` maps to host networking and must - // pass validation (the run later fails on missing nanvixd binaries, - // not on policy). Per-host filtering is absent, so it is accepted. - let request = ExecutionRequest { - script_code: "echo test".to_string(), - policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, - ..Default::default() - }, - ..Default::default() - }; - assert!(NanVixScriptRunner::resolve_networking_mode(&request).unwrap()); - assert!( - NanVixScriptRunner::validate_policies(&request).is_ok(), - "allow posture without per-host filtering should pass validation" - ); - - let mut runner = NanVixScriptRunner::new(); - let mut logger = Logger::new(Mode::Buffer); - let resp = runner.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); - assert!( - !resp.error_message.contains(ERR_NETWORK_HOSTS), - "allow posture must not trigger a network policy rejection, got: {}", - resp.error_message - ); + assert!(!command_arguments(&request) + .unwrap() + .iter() + .any(|argument| argument == "-allow-host-networking")); } #[test] @@ -1624,9 +1276,8 @@ mod tests { #[test] fn policy_allows_defaults() { - // NanVix accepts a default (deny-by-default) policy. With no host - // networking requested, the run later fails on missing nanvixd - // binaries, not on policy. + // NanVix accepts a default isolated policy. The run later fails on + // missing nanvixd binaries, not on policy. let mut runner = NanVixScriptRunner::new(); let request = ExecutionRequest { script_code: "echo test".to_string(), @@ -1637,7 +1288,7 @@ mod tests { let resp = runner.run(&request, &mut logger); assert_eq!(resp.exit_code, ERROR_EXIT_CODE); assert!( - !resp.error_message.contains(ERR_NETWORK_HOSTS), + !resp.error_message.contains(ERR_LEGACY_NETWORK), "default request should not trigger network policy rejection" ); assert!( diff --git a/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs b/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs index a2caf66de..5b3606029 100644 --- a/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs +++ b/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs @@ -477,8 +477,7 @@ pub(crate) fn derive_sid_string(profile_name: &str) -> Result /// instead. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub enum FilesystemMode { - /// Configure the AppContainer's BFS policy via `bfscfg.exe` (default - /// historical behavior). + /// Configure the AppContainer's BFS policy via `bfscfg.exe` (default). #[default] Bfs, /// Skip BFS setup; the caller has handled filesystem policy via host @@ -627,9 +626,9 @@ pub struct AppContainerScriptRunner { denied_paths_enforced_externally: bool, /// Optional pre-derived SID string supplied by the dispatcher. /// - /// When `Some`, the runner uses this value for the firewall - /// principal-id and any other capability-string lookups instead of - /// re-running `ConvertSidToStringSidW` on its owned `PSID`. The + /// When `Some`, the runner uses this value for proxy setup and + /// diagnostics instead of re-running `ConvertSidToStringSidW` on its + /// owned `PSID`. The /// `PSID` itself is still derived by [`create_app_container_sid`] /// at run time because `windows-rs` does not expose a safe /// "string → PSID" conversion with the same ownership semantics as @@ -1328,7 +1327,7 @@ impl AppContainerScriptRunner { Ok(()) } - /// Return the SID string for firewall rule association. + /// Return the SID string for proxy setup and diagnostics. fn get_principal_id(&self) -> String { // Prefer the dispatcher-supplied string when present — saves a // `ConvertSidToStringSidW` round-trip (the dispatcher has @@ -1503,7 +1502,7 @@ impl Drop for AppContainerScriptRunner { // Shared setup/teardown + streaming (handle-based) execution // ─────────────────────────────────────────────────────────────────────────── -/// Per-run resources (firewall + filesystem policy) whose lifetime is tied to +/// Per-run resources (proxy + filesystem policy) whose lifetime is tied to /// the sandboxed child. Created by [`AppContainerScriptRunner::prepare`] and /// torn down by [`AppContainerScriptRunner::teardown`] after the child exits. struct Prepared { @@ -1584,11 +1583,7 @@ impl AppContainerScriptRunner { logger, ); if logger.has_diagnostic_sink() { - let firewall_applied = network_manager.firewall_applied(); - let plan = NetworkManager::describe_policy(&request.policy); - let firewall_ok = !plan.rules_will_be_installed - || matches!(network_manager.firewall_apply_ok(), Some(true)); - let status = if network_result.is_ok() && firewall_ok { + let status = if network_result.is_ok() { OperationStatus::Success } else { OperationStatus::Failure @@ -1599,11 +1594,11 @@ impl AppContainerScriptRunner { .str("tier", self.tier_str()) .str( "enforcement_mode", - request.policy.network_enforcement_mode.as_str(), + crate::process_container_common::network_policy_helpers::CAPABILITIES_ENFORCEMENT_MODE, ) .str( "default_policy", - request.policy.default_network_policy.as_str(), + crate::process_container_common::network_policy_helpers::audit_egress_default(&request.policy), ) .u64( "proxy_port", @@ -1612,19 +1607,16 @@ impl AppContainerScriptRunner { .map(|address| address.port as u64) .unwrap_or(0), ) - .u64( - "firewall_rules_created", - network_manager.rule_count() as u64, - ) - .bool("firewall_applied", firewall_applied) + .u64("firewall_rules_created", 0) + .bool("firewall_applied", false) .str("status", status.as_str()); logger.log_audit_event(&record); } if crate::mxc_common::telemetry::is_active() { crate::mxc_common::telemetry::log_network_policy_applied( sanitize_identity(&self.app_container_name), - request.policy.network_enforcement_mode.as_str(), - request.policy.default_network_policy.as_str(), + crate::process_container_common::network_policy_helpers::CAPABILITIES_ENFORCEMENT_MODE, + crate::process_container_common::network_policy_helpers::audit_egress_default(&request.policy), network_manager .proxy_address() .map(|address| address.port as u64) @@ -1647,10 +1639,9 @@ impl AppContainerScriptRunner { }) } - /// Tear down the per-run firewall and filesystem policy. Idempotent at the - /// manager level; called once after the child exits. + /// Tear down the per-run proxy and filesystem policy after the child exits. fn teardown(&self, prepared: &mut Prepared, preserve_policy: bool, logger: &mut Logger) { - let network = prepared.network_manager.stop_all(!preserve_policy, logger); + let proxy_stopped = prepared.network_manager.stop_all(logger); let bfs_requested = self.filesystem_mode == FilesystemMode::Bfs && prepared.bfs_manager.configured() && !preserve_policy; @@ -1659,22 +1650,18 @@ impl AppContainerScriptRunner { } else { false }; - let (status, skip_reason) = appcontainer_teardown_status_with_bfs( - preserve_policy, - network.firewall_removal_ok, - bfs_requested, - bfs_removed, - ); + let (status, skip_reason) = + appcontainer_teardown_status_with_bfs(preserve_policy, bfs_requested, bfs_removed); if logger.has_diagnostic_sink() { let mut record = AuditEvent::new(AuditEventName::SandboxTornDown) .str("backend", ContainmentBackend::ProcessContainer.wire_name()) .str("identity", sanitize_identity(&self.app_container_name)) .str("tier", self.tier_str()) .str("status", status.as_str()) - .u64("firewall_rules_removed", network.rules_removed as u64) - .bool("firewall_removal_ok", network.firewall_removal_ok) + .u64("firewall_rules_removed", 0) + .bool("firewall_removal_ok", true) .bool("bfs_removed", bfs_removed) - .bool("proxy_stopped", network.proxy_stopped) + .bool("proxy_stopped", proxy_stopped) .bool("preserve_policy", preserve_policy) .bool("container_released", false); if let Some(reason) = skip_reason { @@ -1686,11 +1673,7 @@ impl AppContainerScriptRunner { crate::mxc_common::telemetry::log_sandbox_torn_down( sanitize_identity(&self.app_container_name), status.as_str(), - &format_released_resources( - network.rules_removed, - bfs_removed, - network.proxy_stopped, - ), + &format_released_resources(bfs_removed, proxy_stopped), ); } } @@ -1700,20 +1683,15 @@ impl AppContainerScriptRunner { } } -fn format_released_resources( - firewall_rules_removed: usize, - bfs_removed: bool, - proxy_stopped: bool, -) -> String { +fn format_released_resources(bfs_removed: bool, proxy_stopped: bool) -> String { format!( - "firewall_rules_removed={firewall_rules_removed},bfs_removed={bfs_removed},\ + "firewall_rules_removed=0,bfs_removed={bfs_removed},\ proxy_stopped={proxy_stopped},container_released=false" ) } fn appcontainer_teardown_status_with_bfs( preserve_policy: bool, - firewall_removal_ok: bool, bfs_requested: bool, bfs_removed: bool, ) -> (TeardownStatus, Option) { @@ -1723,7 +1701,7 @@ fn appcontainer_teardown_status_with_bfs( Some(TeardownSkipReason::PreservePolicy), ); } - if firewall_removal_ok && (!bfs_requested || bfs_removed) { + if !bfs_requested || bfs_removed { (TeardownStatus::Success, None) } else { (TeardownStatus::Failure, None) @@ -1808,6 +1786,9 @@ impl SandboxBackend for AppContainerScriptRunner { crate::mxc_common::error::HOST_LISTS_NOT_SUPPORTED_MSG, )); } + crate::process_container_common::network_policy_helpers::reject_retired_network_policy( + &request.policy, + )?; Ok(()) } @@ -1863,7 +1844,7 @@ impl SandboxBackend for AppContainerScriptRunner { /// A running AppContainer-sandboxed process exposed as a [`SandboxProcess`]. /// Owns the process/job handles, the parent-side pipes, and the per-run -/// firewall/filesystem policy, which it tears down once the child exits. +/// proxy/filesystem policy, which it tears down once the child exits. struct AppContainerSandboxProcess { process: SendOwnedHandle, _thread: SendOwnedHandle, @@ -1908,17 +1889,9 @@ struct AppContainerSandboxProcess { // and this handle is owned exclusively by the caller (not shared), so it is // only ever touched from one thread at a time. // -// The one historically thread-affine field was the `NetworkManager` inside -// `prepared`: it used to cache an STA `INetFwPolicy2` interface plus its -// `CoInitializeEx` state and reuse them at teardown, which is unsound when -// `wait()`/`kill()`/`Drop` run on a different thread (e.g. a tokio -// `spawn_blocking` worker) than `spawn`. That no longer happens: each firewall -// apply/remove is apartment-self-contained (it opens its own COM apartment, -// creates a fresh interface, and uninitializes — all on whichever thread runs -// it), so no COM interface or apartment state is moved across threads. The only -// remaining OS state the manager keeps is the process-global Winsock refcount, -// which is thread-agnostic. Moving this handle across threads is therefore -// sound. +// `NetworkManager` owns only the proxy coordinator's process-global handles +// and paths; no thread-affine COM interface or apartment state is retained. +// Moving this handle across threads is therefore sound. unsafe impl Send for AppContainerSandboxProcess {} impl AppContainerSandboxProcess { @@ -2016,10 +1989,10 @@ impl AppContainerSandboxProcess { if let Some(result) = &self.teardown_result { return result.clone().map_err(std::io::Error::other); } - let network = self + let proxy_stopped = self .prepared .network_manager - .stop_all(!self.preserve_policy, &mut self.audit_logger); + .stop_all(&mut self.audit_logger); let bfs_requested = self.filesystem_mode == FilesystemMode::Bfs && self.prepared.bfs_manager.configured() && !self.preserve_policy; @@ -2056,12 +2029,8 @@ impl AppContainerSandboxProcess { Ok(()) }; let result = result.map_err(|error| error.to_string()); - let (mut status, skip_reason) = appcontainer_teardown_status_with_bfs( - self.preserve_policy, - network.firewall_removal_ok, - bfs_requested, - bfs_removed, - ); + let (mut status, skip_reason) = + appcontainer_teardown_status_with_bfs(self.preserve_policy, bfs_requested, bfs_removed); if result.is_err() { status = TeardownStatus::Failure; } @@ -2069,10 +2038,10 @@ impl AppContainerSandboxProcess { let mut record = self .audit(AuditEventName::SandboxTornDown) .str("status", status.as_str()) - .u64("firewall_rules_removed", network.rules_removed as u64) - .bool("firewall_removal_ok", network.firewall_removal_ok) + .u64("firewall_rules_removed", 0) + .bool("firewall_removal_ok", true) .bool("bfs_removed", bfs_removed) - .bool("proxy_stopped", network.proxy_stopped) + .bool("proxy_stopped", proxy_stopped) .bool("preserve_policy", self.preserve_policy) .bool("container_released", false); if let Some(reason) = skip_reason { @@ -2084,11 +2053,7 @@ impl AppContainerSandboxProcess { crate::mxc_common::telemetry::log_sandbox_torn_down( &self.identity, status.as_str(), - &format_released_resources( - network.rules_removed, - bfs_removed, - network.proxy_stopped, - ), + &format_released_resources(bfs_removed, proxy_stopped), ); } self.teardown_result = Some(result.clone()); @@ -2255,10 +2220,10 @@ impl SandboxProcess for AppContainerSandboxProcess { }; // Tree-kill the job so any backgrounded descendant dies *before* - // `run_teardown()` removes the firewall / BFS enforcement (keyed to the - // shared AppContainer package SID) — upholding the same invariant as - // `Drop`. The foreground child has already exited on the success path; on - // a timeout or wait failure this also terminates it. Then reap the root + // `run_teardown()` removes BFS enforcement (keyed to the shared + // AppContainer package SID). The foreground child has already exited + // on the success path; on a timeout or wait failure this terminates it. + // Then reap the root // (immediate once it has exited) before releasing the pipe drains — and // killing the tree closes the descendant's pipe write-ends, so the drains // can finish. @@ -2281,7 +2246,7 @@ impl SandboxProcess for AppContainerSandboxProcess { impl Drop for AppContainerSandboxProcess { fn drop(&mut self) { - // Kill the tree and reap before tearing down firewall/filesystem + // Kill the tree and reap before tearing down filesystem // policy, so an abandoned-but-running sandbox cannot outlive its // enforcement (or leak as an orphan). `kill()` terminates the job. if let Err(error) = self.kill() { @@ -2309,43 +2274,36 @@ mod tests { #[test] fn released_resources_format_is_stable() { assert_eq!( - super::format_released_resources(2, true, false), - "firewall_rules_removed=2,bfs_removed=true,proxy_stopped=false,container_released=false" + super::format_released_resources(true, false), + "firewall_rules_removed=0,bfs_removed=true,proxy_stopped=false,container_released=false" ); } #[test] fn teardown_status_reports_preserve_policy_as_skipped() { - for firewall_ok in [true, false] { - for (bfs_requested, bfs_ok) in [(false, false), (true, true), (true, false)] { - assert_eq!( - super::appcontainer_teardown_status_with_bfs( - true, - firewall_ok, - bfs_requested, - bfs_ok, - ), - ( - TeardownStatus::Skipped, - Some(TeardownSkipReason::PreservePolicy) - ) - ); - } + for (bfs_requested, bfs_ok) in [(false, false), (true, true), (true, false)] { + assert_eq!( + super::appcontainer_teardown_status_with_bfs(true, bfs_requested, bfs_ok), + ( + TeardownStatus::Skipped, + Some(TeardownSkipReason::PreservePolicy) + ) + ); } } #[test] fn teardown_status_distinguishes_cleanup_failures() { assert_eq!( - super::appcontainer_teardown_status_with_bfs(false, true, false, false), + super::appcontainer_teardown_status_with_bfs(false, false, false), (TeardownStatus::Success, None) ); assert_eq!( - super::appcontainer_teardown_status_with_bfs(false, false, false, false), - (TeardownStatus::Failure, None) + super::appcontainer_teardown_status_with_bfs(false, true, true), + (TeardownStatus::Success, None) ); assert_eq!( - super::appcontainer_teardown_status_with_bfs(false, true, true, false), + super::appcontainer_teardown_status_with_bfs(false, true, false), (TeardownStatus::Failure, None) ); } diff --git a/src/mxc-sdk/src/backends/process_container/common/base_container_helpers.rs b/src/mxc-sdk/src/backends/process_container/common/base_container_helpers.rs index 4dfa57496..1b60e3950 100644 --- a/src/mxc-sdk/src/backends/process_container/common/base_container_helpers.rs +++ b/src/mxc-sdk/src/backends/process_container/common/base_container_helpers.rs @@ -4,8 +4,8 @@ //! BaseContainer configuration and policy helpers. use crate::mxc_common::models::{ - ContainerPolicy, ExecutionRequest, NetworkAction, NetworkCidr, NetworkPeer, NetworkPolicy, - NetworkPort, NetworkProtocol, NetworkRule, + ContainerPolicy, ExecutionRequest, NetworkAction, NetworkCidr, NetworkPeer, NetworkPort, + NetworkProtocol, NetworkRule, }; use crate::process_security_environment_spec::process_security_environment_layout::{ finish_process_security_environment_buffer, DestinationRuleT as PsecDestinationRuleT, @@ -143,13 +143,10 @@ fn psec_filter_action(action: NetworkAction) -> PsecFilterAction { } fn effective_egress_default(policy: &ContainerPolicy) -> NetworkAction { - policy.network_egress.as_ref().map_or( - match policy.default_network_policy { - NetworkPolicy::Allow => NetworkAction::Allow, - NetworkPolicy::Block => NetworkAction::Deny, - }, - |egress| egress.default, - ) + policy + .network_egress + .as_ref() + .map_or(NetworkAction::Deny, |egress| egress.default) } pub(super) fn unrestricted_host_loopback_allowed(policy: &ContainerPolicy) -> bool { diff --git a/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs b/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs index 0556e58f7..4d08349a3 100644 --- a/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs +++ b/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs @@ -1044,8 +1044,10 @@ impl BaseContainerRunner { crate::mxc_common::telemetry::log_network_policy_applied( sanitize_identity(&identity), - request.policy.network_enforcement_mode.as_str(), - request.policy.default_network_policy.as_str(), + crate::process_container_common::network_policy_helpers::CAPABILITIES_ENFORCEMENT_MODE, + crate::process_container_common::network_policy_helpers::audit_egress_default( + &request.policy, + ), request .policy .network_proxy @@ -1064,11 +1066,13 @@ impl BaseContainerRunner { ) .str( "enforcement_mode", - request.policy.network_enforcement_mode.as_str(), + crate::process_container_common::network_policy_helpers::CAPABILITIES_ENFORCEMENT_MODE, ) .str( "default_policy", - request.policy.default_network_policy.as_str(), + crate::process_container_common::network_policy_helpers::audit_egress_default( + &request.policy, + ), ) .u64( "proxy_port", @@ -1165,6 +1169,9 @@ impl SandboxBackend for BaseContainerRunner { crate::mxc_common::error::HOST_LISTS_NOT_SUPPORTED_MSG, )); } + crate::process_container_common::network_policy_helpers::reject_retired_network_policy( + &request.policy, + )?; if has_conflicting_proxy_identity(&request.policy) { return Err(ScriptResponse::rejected( "processContainer.network.allowedProxyPeer grants loopback access only to the \ @@ -2055,8 +2062,7 @@ mod tests { }; use crate::mxc_common::models::{ BaseProcessUiConfig, ClipboardPolicy, ContainerPolicy, NetworkAction, NetworkCidr, - NetworkPeer, NetworkPolicy, NetworkPort, NetworkProtocol, NetworkRule, ProxyConfig, - UiPolicy, + NetworkPeer, NetworkPort, NetworkProtocol, NetworkRule, ProxyConfig, UiPolicy, }; use crate::mxc_common::ui_policy::EffectiveUiRestrictions; use crate::process_container_common::job_object::to_job_object_uilimit_mask; @@ -2781,7 +2787,10 @@ mod tests { fn build_process_security_environment_spec_ignores_empty_capability() { let mut request = ExecutionRequest::default(); request.policy.capabilities = vec![String::new()]; - request.policy.default_network_policy = NetworkPolicy::Allow; + request.policy.network_egress = Some(crate::mxc_common::models::NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }); let bytes = BaseContainerRunner::build_process_security_environment_spec(&request); let spec = psec_layout::root_as_process_security_environment(&bytes).unwrap(); @@ -2790,9 +2799,8 @@ mod tests { } #[test] - fn build_process_security_environment_spec_preserves_allow_egress() { - let mut request = ExecutionRequest::default(); - request.policy.default_network_policy = NetworkPolicy::Allow; + fn build_process_security_environment_spec_preserves_implicit_deny_egress() { + let request = ExecutionRequest::default(); let bytes = BaseContainerRunner::build_process_security_environment_spec(&request); let spec = psec_layout::root_as_process_security_environment(&bytes).unwrap(); @@ -2801,8 +2809,8 @@ mod tests { .and_then(|policy| policy.egress()) .expect("PSEC must carry an explicit egress default"); - assert_eq!(egress.default_action(), psec_layout::FilterAction::allow); - assert_eq!(spec.capabilities(), Some("internetClient")); + assert_eq!(egress.default_action(), psec_layout::FilterAction::deny); + assert!(spec.capabilities().is_none()); } #[test] diff --git a/src/mxc-sdk/src/backends/process_container/common/network_manager.rs b/src/mxc-sdk/src/backends/process_container/common/network_manager.rs index 6742f3a23..67876c96d 100644 --- a/src/mxc-sdk/src/backends/process_container/common/network_manager.rs +++ b/src/mxc-sdk/src/backends/process_container/common/network_manager.rs @@ -1,435 +1,50 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::net::{IpAddr, ToSocketAddrs}; - -use windows::core::BSTR; -use windows::Win32::Foundation::VARIANT_BOOL; -use windows::Win32::NetworkManagement::WindowsFirewall::{ - INetFwPolicy2, INetFwRule3, NetFwPolicy2, NetFwRule, NET_FW_ACTION, NET_FW_ACTION_ALLOW, - NET_FW_ACTION_BLOCK, NET_FW_RULE_DIR_OUT, -}; -use windows::Win32::Networking::WinSock::{WSACleanup, WSAStartup, WSADATA}; -use windows::Win32::System::Com::{ - CoCreateInstance, CoInitializeEx, CoUninitialize, CLSCTX_INPROC_SERVER, - COINIT_APARTMENTTHREADED, -}; -use windows::Win32::System::Variant::VARIANT; -use windows_core::Interface; +use windows::Win32::Security::PSID; use crate::mxc_common::error::WxcError; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ContainerPolicy, NetworkEnforcementMode, NetworkPolicy}; +use crate::mxc_common::models::{ContainerPolicy, ProxyAddress}; +use crate::process_container_common::network_policy_helpers::reject_retired_network_policy; use crate::process_container_common::proxy_coordinator::ProxyCoordinator; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum DefaultPolicy { - Allow, - Block, -} - -/// `RPC_E_CHANGED_MODE`: `CoInitializeEx` returns this when COM is already -/// initialized on the calling thread with a *different* apartment model. The -/// existing initialization is reused and must **not** be balanced by our own -/// `CoUninitialize`. -const RPC_E_CHANGED_MODE: u32 = 0x8001_0106; - -/// RAII guard for a per-call COM apartment on the **current** thread. -/// -/// Every firewall operation creates one of these, does all of its COM work -/// (`CoCreateInstance`, interface use, release) while it is alive, and lets it -/// drop — running the matching `CoUninitialize` — before returning. Because no -/// COM interface or apartment state is ever cached on [`NetworkManager`] across -/// calls, teardown (`remove_firewall_rules`) can run on a *different* thread -/// than setup (`apply_firewall_rules`) without ever using an interface from -/// another apartment or pairing `CoInitializeEx`/`CoUninitialize` across -/// threads. That self-containment is what makes the `unsafe impl Send` on the -/// owning sandbox handle sound. -struct ComApartment { - /// Whether *this* guard performed the initialization that it must balance - /// with `CoUninitialize`. `false` when COM was already initialized on this - /// thread under a different model (`RPC_E_CHANGED_MODE`). - owns_init: bool, -} - -impl ComApartment { - /// Join (or initialize) an apartment-threaded COM apartment for the current - /// thread. `S_OK`/`S_FALSE` both count as an initialization this guard must - /// balance; `RPC_E_CHANGED_MODE` reuses an existing apartment without - /// taking ownership of its teardown. - fn new() -> Result { - // SAFETY: `CoInitializeEx` is always safe to call; the matching - // `CoUninitialize` runs in `Drop` on this same thread when we own it. - let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) }; - if hr.is_ok() { - Ok(Self { owns_init: true }) - } else if hr.0 as u32 == RPC_E_CHANGED_MODE { - Ok(Self { owns_init: false }) - } else { - Err(WxcError::Firewall(format!( - "CoInitializeEx failed: 0x{:08X}", - hr.0 as u32 - ))) - } - } -} - -impl Drop for ComApartment { - fn drop(&mut self) { - if self.owns_init { - // SAFETY: balances the `CoInitializeEx` in `new` on the same thread. - unsafe { CoUninitialize() }; - } - } -} - +#[derive(Default)] pub struct NetworkManager { - created_rule_names: Vec, - wsa_initialized: bool, proxy_coordinator: ProxyCoordinator, - /// The result of the most recent `apply_firewall_rules` call. `None` when - /// apply has never been attempted (e.g. `start` returned early on a proxy - /// failure). `Some(true)` when apply succeeded — either all requested rules - /// were installed, or the policy required no rules at all. `Some(false)` - /// when apply failed. Consulted by the caller when building the - /// `NetworkPolicyApplied` audit record so `firewall_applied` reflects the - /// actual apply outcome rather than the policy *plan*. - firewall_apply_ok: Option, -} - -/// What [`NetworkManager::stop_all`] actually released. -/// -/// Previously the firewall-removal `Result` was discarded at the call site, so -/// a partially-failed cleanup was indistinguishable from a clean one. These are -/// diagnostic values only — teardown remains best-effort and non-fatal. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct NetworkTeardown { - /// How many firewall rules this manager had created and attempted to - /// remove. Zero when no rules were installed or cleanup was skipped. - pub rules_removed: usize, - /// Whether every rule removal succeeded. `true` when there was nothing to - /// remove. - pub firewall_removal_ok: bool, - /// Whether an active proxy coordinator was stopped. - pub proxy_stopped: bool, -} - -/// What [`NetworkManager::remove_firewall_rules`] actually released. -/// -/// Windows Firewall `Rules.Remove` is per-rule: some can land while others -/// fail. Reporting the entry count as `rules_removed` when only a subset -/// actually came out would hide a partial-cleanup failure — the two are kept -/// distinct here so `stop_all` can carry the truthful count into the audit -/// record and derive `firewall_removal_ok` from the aggregate. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct FirewallRemoval { - /// Number of rules whose `Rules.Remove` call returned success. - pub removed: usize, - /// Whether every removal succeeded. `false` when at least one rule - /// removal failed. - pub all_success: bool, -} - -impl Default for FirewallRemoval { - /// "Nothing to remove, and that is fine." Written by hand for the same - /// reason as [`NetworkTeardown::default`]: the derived default would set - /// `all_success: false`, which reads as a failed removal. - fn default() -> Self { - Self { - removed: 0, - all_success: true, - } - } -} - -impl Default for NetworkTeardown { - /// "Nothing to remove, and that is fine." - /// - /// Written by hand because the derived default would set - /// `firewall_removal_ok: false`, which reads as a *failed* removal — the - /// opposite of what an empty teardown means. - fn default() -> Self { - Self { - rules_removed: 0, - firewall_removal_ok: true, - proxy_stopped: false, - } - } -} - -/// Invariant context for creating firewall rules within a single -/// `apply_firewall_rules` call: the firewall interface (valid only for the -/// current COM apartment / thread) and the AppContainer principal the rules are -/// scoped to. Bundled so the rule helpers stay within the argument-count lint. -struct RuleContext<'a> { - fw_policy: &'a INetFwPolicy2, - principal_id: &'a str, -} - -/// The outcome of evaluating a request's network policy: the effective default -/// policy, whether the caller asked for firewall-rule enforcement, and whether -/// any rules will actually be installed. -/// -/// The last two are **not** the same, and conflating them is a real bug: with -/// `enforcementMode: firewall`, no host lists, and `defaultPolicy: allow`, the -/// caller asked for firewall enforcement but there is nothing to install. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct NetworkPolicyPlan { - /// Effective default policy for the rules that get installed. - pub default_policy: DefaultPolicy, - /// Whether `enforcementMode` selects firewall-rule enforcement at all. - pub firewall_mode_selected: bool, - /// Whether this policy actually produces firewall rules to install. - pub rules_will_be_installed: bool, } impl NetworkManager { pub fn new() -> Self { Self { - created_rule_names: Vec::new(), - wsa_initialized: false, proxy_coordinator: ProxyCoordinator::new(), - firewall_apply_ok: None, } } - /// Decide the effective default policy and whether firewall rules will be - /// installed for `policy` — the pure core of [`Self::initialize_policy`], - /// factored out so callers that only need the *decision* (e.g. audit - /// records) do not have to log an "Applying network firewall rules" line as - /// a side effect of asking. - pub fn describe_policy(policy: &ContainerPolicy) -> NetworkPolicyPlan { - let firewall_mode_selected = matches!( - policy.network_enforcement_mode, - NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both - ); - - let default_policy = - if firewall_mode_selected && policy.default_network_policy == NetworkPolicy::Block { - DefaultPolicy::Block - } else { - DefaultPolicy::Allow - }; - - NetworkPolicyPlan { - default_policy, - firewall_mode_selected, - // `apply_firewall_rules_inner` always falls through to install a - // default Allow/BlockAll rule whenever firewall mode is - // selected, even with no explicit host list and an allow - // default — see - // `firewall_mode_default_allow_installs_default_rule_without_log_line`. - // So this is unconditional on `firewall_mode_selected`, not on - // the narrower "has hosts, or blocks by default" condition - // `initialize_policy` uses below, which governs only the legacy - // startup log line. - rules_will_be_installed: firewall_mode_selected, - } - } - - pub fn initialize_policy( - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> (DefaultPolicy, bool) { - let plan = Self::describe_policy(policy); - // The legacy log line is reserved for the case where the caller gave - // firewall enforcement something explicit to do (a host list) or - // asked to block by default; the coincidental default-rule - // fallthrough for the plain-allow, no-hosts case stays silent, as - // documented on `NetworkPolicyPlan::rules_will_be_installed` above. - if plan.firewall_mode_selected - && (!policy.allowed_hosts.is_empty() - || !policy.blocked_hosts.is_empty() - || policy.default_network_policy == NetworkPolicy::Block) - { - logger.log_line("Applying network firewall rules..."); - } - (plan.default_policy, plan.firewall_mode_selected) - } - - /// Number of firewall rules currently created by this manager. - /// - /// The *count* is deliberately what the audit record carries: rule names - /// embed the AppContainer principal id and a timestamp, which is - /// high-cardinality and semi-identifying. - pub fn rule_count(&self) -> usize { - self.created_rule_names.len() - } - - /// Actual apply outcome of the most recent `apply_firewall_rules` call. - /// - /// * `None` — apply was never attempted (e.g. `start` returned early on a - /// proxy failure). - /// * `Some(true)` — apply succeeded, meaning either all requested rules - /// were installed or the policy required none. - /// * `Some(false)` — apply failed. - /// - /// Used by the caller to derive the `NetworkPolicyApplied` audit record's - /// `firewall_applied` field and aggregate `status` from the *observed* - /// outcome, not from the policy plan. - pub fn firewall_apply_ok(&self) -> Option { - self.firewall_apply_ok - } - - /// Whether firewall rules were actually installed by the most recent apply. - /// `false` when apply was never attempted, failed, or the policy required - /// no rules. This is the truth value the `firewall_applied` audit field - /// should carry — the policy-plan value can be true while the actual - /// installed rule count is zero. - pub fn firewall_applied(&self) -> bool { - matches!(self.firewall_apply_ok, Some(true)) && !self.created_rule_names.is_empty() - } - - pub fn apply_firewall_rules( - &mut self, - principal_id: &str, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result<(), WxcError> { - self.firewall_apply_ok = Some(true); - let outcome = self.apply_firewall_rules_inner(principal_id, policy, logger); - if outcome.is_err() { - self.firewall_apply_ok = Some(false); - } - outcome - } - - fn apply_firewall_rules_inner( - &mut self, - principal_id: &str, - policy: &ContainerPolicy, - logger: &mut Logger, - ) -> Result<(), WxcError> { - let (default_policy, use_firewall_rules) = Self::initialize_policy(policy, logger); - if !use_firewall_rules { - return Ok(()); - } - - // Open a COM apartment and create the firewall interface for the - // duration of *this* call only — nothing is cached on `self`. See - // [`ComApartment`] for why this self-containment matters. - let _com = ComApartment::new()?; - let fw_policy: INetFwPolicy2 = - unsafe { CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER) } - .map_err(|e| WxcError::Firewall(format!("Failed to create NetFwPolicy2: {e}")))?; - self.ensure_wsa_initialized(logger)?; - - let now = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default(); - let millis = now.as_millis(); - let mut sanitized_principal_id: String = principal_id - .chars() - .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) - .collect(); - const MAX_PRINCIPAL_ID_LEN: usize = 64; - if sanitized_principal_id.len() > MAX_PRINCIPAL_ID_LEN { - sanitized_principal_id.truncate(MAX_PRINCIPAL_ID_LEN); - } - let rule_prefix = format!("WXC_{}_{}", sanitized_principal_id, millis); - let ctx = RuleContext { - fw_policy: &fw_policy, - principal_id, - }; - - if default_policy == DefaultPolicy::Block { - let block_all_name = format!("{}_BlockAll", rule_prefix); - if !self.create_rule(&ctx, &block_all_name, NET_FW_ACTION_BLOCK, "", logger)? { - self.firewall_apply_ok = Some(false); - return Ok(()); - } - self.created_rule_names.push(block_all_name); - self.process_host_list( - &ctx, - &policy.allowed_hosts, - &rule_prefix, - NET_FW_ACTION_ALLOW, - "Allow", - logger, - )?; - } else { - let allow_all_name = format!("{}_AllowAll", rule_prefix); - if !self.create_rule(&ctx, &allow_all_name, NET_FW_ACTION_ALLOW, "*", logger)? { - self.firewall_apply_ok = Some(false); - return Ok(()); - } - self.created_rule_names.push(allow_all_name); - self.process_host_list( - &ctx, - &policy.blocked_hosts, - &rule_prefix, - NET_FW_ACTION_BLOCK, - "Block", - logger, - )?; - } - - Ok(()) - } - - fn process_host_list( - &mut self, - ctx: &RuleContext, - hosts: &[String], - rule_prefix: &str, - action: NET_FW_ACTION, - action_name: &str, - logger: &mut Logger, - ) -> Result<(), WxcError> { - for (index, host) in hosts.iter().enumerate() { - let ip_address = if validate_ip_or_cidr(host) { - host.clone() - } else { - match resolve_hostname(host) { - Ok(ip) => ip, - Err(_) => { - logger.log_line(&format!("Warning: Could not resolve {}", host)); - self.firewall_apply_ok = Some(false); - continue; - } - } - }; - - let rule_name = format!("{}_{}_{}", rule_prefix, action_name, index); - match self.create_rule(ctx, &rule_name, action, &ip_address, logger) { - Ok(true) => { - self.created_rule_names.push(rule_name); - } - Ok(false) => { - self.firewall_apply_ok = Some(false); - } - Err(_) => { - self.firewall_apply_ok = Some(false); - } - } - } - Ok(()) - } - - /// Returns `true` if any firewall rules have been created and are currently active. - pub fn rules_applied(&self) -> bool { - !self.created_rule_names.is_empty() - } - - /// Returns the proxy address if a proxy is active. - pub fn proxy_address(&self) -> Option<&crate::mxc_common::models::ProxyAddress> { + pub fn proxy_address(&self) -> Option<&ProxyAddress> { self.proxy_coordinator.address() } - /// Start the proxy (if configured) and apply firewall rules. - /// - /// This is the single entry point for all network setup. It handles: - /// 1. Launching the builtin test proxy or configuring the external proxy - /// 2. Setting WinHTTP proxy policy via the elevated shim - /// 3. Creating Windows Firewall rules for host allow/block lists + /// Configure the supported runtime proxy. Directional network access on + /// this tier is enforced by AppContainer capabilities, not firewall rules. pub fn start( &mut self, principal_id: &str, container_name: &str, policy: &ContainerPolicy, - script_sid: windows::Win32::Security::PSID, + script_sid: PSID, logger: &mut Logger, ) -> Result<(), WxcError> { + if !policy.allowed_hosts.is_empty() || !policy.blocked_hosts.is_empty() { + return Err(WxcError::Validation( + "network.allowedHosts / network.blockedHosts are retired; \ + use network.egress and network.ingress" + .into(), + )); + } + reject_retired_network_policy(policy) + .map_err(|error| WxcError::Validation(error.error_message))?; + if policy.network_proxy.is_enabled() { self.proxy_coordinator.start( &policy.network_proxy, @@ -440,459 +55,87 @@ impl NetworkManager { )?; } - if let Err(err) = self.apply_firewall_rules(principal_id, policy, logger) { - if self.proxy_coordinator.is_active() { - self.proxy_coordinator.stop(logger); - } - return Err(err); - } - - Ok(()) - } - - /// Stop all network resources: firewall rules, proxy policy, test proxy. - /// - /// Returns what was actually released, so a caller can record an honest - /// teardown record instead of assuming success. Failures remain non-fatal — - /// this is a best-effort cleanup path and the return value is diagnostic. - pub fn stop_all(&mut self, cleanup_policy: bool, logger: &mut Logger) -> NetworkTeardown { - let mut outcome = NetworkTeardown { - rules_removed: 0, - firewall_removal_ok: true, - proxy_stopped: false, - }; - - if self.rules_applied() && cleanup_policy { - match self.remove_firewall_rules(logger) { - Ok(removal) => { - // `rules_removed` counts *successful* removals only; a - // partial success no longer inflates the count to the - // full list length. `firewall_removal_ok` is the aggregate. - outcome.rules_removed = removal.removed; - outcome.firewall_removal_ok = removal.all_success; - } - Err(_) => { - // `remove_firewall_rules` failed before it could remove - // anything, so nothing was removed. - outcome.firewall_removal_ok = false; - } - } - } - outcome.proxy_stopped = self.proxy_coordinator.stop(logger); - outcome - } - - pub fn remove_firewall_rules( - &mut self, - logger: &mut Logger, - ) -> Result { - if self.created_rule_names.is_empty() { - return Ok(FirewallRemoval { - removed: 0, - all_success: true, - }); - } - - // Re-acquire a fresh firewall interface in its own apartment on the - // current thread. Windows Firewall rules persist by name independently - // of the COM client that created them, so removal does not need (and - // must not reuse) the interface or apartment `apply_firewall_rules` - // used — which may have run on a different thread. See [`ComApartment`]. - let _com = ComApartment::new()?; - let fw_policy: INetFwPolicy2 = - unsafe { CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER) } - .map_err(|e| WxcError::Firewall(format!("Failed to create NetFwPolicy2: {e}")))?; - - let rules = unsafe { fw_policy.Rules() } - .map_err(|e| WxcError::Firewall(format!("Failed to get firewall rules: {}", e)))?; - - // Count *successful* removals, not the entry count. Windows Firewall - // `Rules.Remove` returns per-rule success/failure; conflating the two - // makes a partially-failed cleanup indistinguishable from a clean one - // in the audit record. - let mut removed = 0usize; - let mut all_success = true; - for rule_name in &self.created_rule_names { - let bstr_name = BSTR::from(rule_name.as_str()); - if unsafe { rules.Remove(&bstr_name) }.is_ok() { - removed += 1; - } else { - all_success = false; - } - } - self.created_rule_names.clear(); - if !all_success { - logger.log_line("Warning: some firewall rules could not be removed"); - } - Ok(FirewallRemoval { - removed, - all_success, - }) - } - - fn ensure_wsa_initialized(&mut self, _logger: &mut Logger) -> Result<(), WxcError> { - if self.wsa_initialized { - return Ok(()); - } - let mut wsa_data = WSADATA::default(); - let result = unsafe { WSAStartup(0x0202, &mut wsa_data) }; - if result != 0 { - return Err(WxcError::Firewall(format!( - "WSAStartup failed with code {}", - result - ))); - } - self.wsa_initialized = true; Ok(()) } - fn cleanup_wsa(&mut self) { - if self.wsa_initialized { - unsafe { WSACleanup() }; - self.wsa_initialized = false; - } - } - - fn create_rule( - &self, - ctx: &RuleContext, - rule_name: &str, - action: NET_FW_ACTION, - remote_addresses: &str, - _logger: &mut Logger, - ) -> Result { - let rules = unsafe { ctx.fw_policy.Rules() } - .map_err(|e| WxcError::Firewall(format!("Failed to get firewall rules: {}", e)))?; - - let rule: windows::Win32::NetworkManagement::WindowsFirewall::INetFwRule = - unsafe { CoCreateInstance(&NetFwRule, None, CLSCTX_INPROC_SERVER) } - .map_err(|e| WxcError::Firewall(format!("Failed to create NetFwRule: {}", e)))?; - - let rule3: INetFwRule3 = rule.cast().map_err(|e| { - WxcError::Firewall(format!("Failed to get INetFwRule3 interface: {}", e)) - })?; - - unsafe { - rule.SetName(&BSTR::from(rule_name)) - .map_err(|e| WxcError::Firewall(format!("put_Name failed: {}", e)))?; - - rule.SetDescription(&BSTR::from("WXC AppContainer network policy")) - .map_err(|e| WxcError::Firewall(format!("put_Description failed: {}", e)))?; - - rule3 - .SetLocalAppPackageId(&BSTR::from(ctx.principal_id)) - .map_err(|e| WxcError::Firewall(format!("put_LocalAppPackageId failed: {}", e)))?; - - rule.SetDirection(NET_FW_RULE_DIR_OUT) - .map_err(|e| WxcError::Firewall(format!("put_Direction failed: {}", e)))?; - - rule.SetAction(action) - .map_err(|e| WxcError::Firewall(format!("put_Action failed: {}", e)))?; - - let empty_variant = VARIANT::default(); - rule.SetInterfaces(&empty_variant) - .map_err(|e| WxcError::Firewall(format!("put_Interfaces failed: {}", e)))?; - - if !remote_addresses.is_empty() { - rule.SetRemoteAddresses(&BSTR::from(remote_addresses)) - .map_err(|e| { - WxcError::Firewall(format!("put_RemoteAddresses failed: {}", e)) - })?; - } - - rule.SetEnabled(VARIANT_BOOL::from(true)) - .map_err(|e| WxcError::Firewall(format!("put_Enabled failed: {}", e)))?; - - match rules.Add(&rule) { - Ok(()) => Ok(true), - Err(e) => { - _logger.log_line(&format!( - "Failed to add firewall rule '{}': {}", - rule_name, e - )); - Ok(false) - } - } - } - } -} - -impl Default for NetworkManager { - fn default() -> Self { - Self::new() - } -} - -impl Drop for NetworkManager { - fn drop(&mut self) { - // No COM state is cached across calls (each firewall op is - // apartment-self-contained), so there is nothing COM-related to undo - // here. Only the process-global Winsock refcount — which is - // thread-agnostic — needs balancing. - self.cleanup_wsa(); - } -} - -/// Resolve a hostname to an IP address string. -pub fn resolve_hostname(hostname: &str) -> Result { - let addr = (hostname, 0) - .to_socket_addrs() - .map_err(|e| WxcError::Firewall(format!("Failed to resolve '{}': {}", hostname, e)))? - .next() - .ok_or_else(|| WxcError::Firewall(format!("No addresses found for '{}'", hostname)))?; - - Ok(addr.ip().to_string()) -} - -/// Validate whether a string is a valid IP address or CIDR notation. -pub fn validate_ip_or_cidr(address: &str) -> bool { - let (ip_part, cidr_part) = match address.find('/') { - Some(pos) => { - let ip = &address[..pos]; - let cidr = &address[pos + 1..]; - if cidr.is_empty() { - return false; - } - let bits: u32 = match cidr.parse() { - Ok(b) => b, - Err(_) => return false, - }; - (ip, Some(bits)) - } - None => (address, None), - }; - - if let Ok(ip) = ip_part.parse::() { - match (ip, cidr_part) { - (IpAddr::V4(_), Some(bits)) => bits <= 32, - (IpAddr::V6(_), Some(bits)) => bits <= 128, - (_, None) => true, - } - } else { - false + /// Stop the proxy, including when the caller preserves filesystem policy. + pub fn stop_all(&mut self, logger: &mut Logger) -> bool { + self.proxy_coordinator.stop(logger) } } #[cfg(test)] mod tests { use super::*; + use crate::mxc_common::logger::Mode; + use crate::mxc_common::models::{ + NetworkAction, NetworkEgressPolicy, NetworkEnforcementMode, NetworkPolicy, + }; - /// `firewall_applied` in the audit record used to come from the policy - /// *plan* ("rules will be installed"). With apply now propagating its - /// actual outcome, a manager that never even attempted apply reports - /// `firewall_applied() == false` — the truth value the audit field should - /// carry. - #[test] - fn firewall_applied_defaults_to_false_when_apply_not_attempted() { - let mgr = NetworkManager::new(); - assert_eq!(mgr.firewall_apply_ok(), None); - assert!(!mgr.firewall_applied()); - } - - /// `stop_all` on a manager that installed nothing reports zero removals - /// as a clean teardown, and — regression for finding #7 — the - /// `FirewallRemoval` empty-list branch matches. #[test] - fn remove_firewall_rules_with_nothing_installed_is_clean() { - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); + fn proxyless_policy_has_no_network_resources_to_stop() { + let mut logger = Logger::new(Mode::Buffer); let mut manager = NetworkManager::new(); - let removal = manager.remove_firewall_rules(&mut logger).unwrap(); - assert_eq!(removal.removed, 0); - assert!(removal.all_success); - } - - /// `FirewallRemoval::default` must read as "nothing to remove, and that - /// is fine". The derived default sets `all_success: false`, which reads - /// as a *failed* removal — the opposite of what an empty removal means. - #[test] - fn firewall_removal_default_is_clean() { - assert_eq!(FirewallRemoval::default().removed, 0); - assert!(FirewallRemoval::default().all_success); - } - - #[test] - fn test_validate_ip_or_cidr_valid_ipv4() { - assert!(validate_ip_or_cidr("192.168.1.1")); - assert!(validate_ip_or_cidr("10.0.0.0/8")); - assert!(validate_ip_or_cidr("172.16.0.0/12")); - assert!(validate_ip_or_cidr("0.0.0.0/0")); - assert!(validate_ip_or_cidr("255.255.255.255/32")); - } - - #[test] - fn test_validate_ip_or_cidr_valid_ipv6() { - assert!(validate_ip_or_cidr("::1")); - assert!(validate_ip_or_cidr("fe80::1/64")); - assert!(validate_ip_or_cidr("::1/128")); - } - - #[test] - fn test_validate_ip_or_cidr_invalid() { - assert!(!validate_ip_or_cidr("not_an_ip")); - assert!(!validate_ip_or_cidr("192.168.1.1/")); - assert!(!validate_ip_or_cidr("192.168.1.1/33")); - assert!(!validate_ip_or_cidr("::1/129")); - assert!(!validate_ip_or_cidr("192.168.1.1/abc")); - assert!(!validate_ip_or_cidr("")); - } - - #[test] - fn test_initialize_policy_firewall_mode_block() { - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }; - let (default_policy, use_fw) = NetworkManager::initialize_policy(&policy, &mut logger); - assert!(use_fw); - assert_eq!(default_policy, DefaultPolicy::Block); - } - - /// `describe_policy` is the pure core of `initialize_policy`; the audit - /// record calls it instead so asking the question does not also emit an - /// "Applying network firewall rules..." line as a side effect. - #[test] - fn describe_policy_matches_initialize_policy_but_is_side_effect_free() { - let cases = [ - ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }, - ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Capabilities, - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }, - ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Both, - default_network_policy: NetworkPolicy::Allow, - ..Default::default() - }, - ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - default_network_policy: NetworkPolicy::Allow, + network_egress: Some(NetworkEgressPolicy { + default: NetworkAction::Allow, ..Default::default() - }, - ]; - for policy in cases { - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let (default_policy, use_fw) = NetworkManager::initialize_policy(&policy, &mut logger); - let plan = NetworkManager::describe_policy(&policy); - assert_eq!(default_policy, plan.default_policy, "policy: {policy:?}"); - assert_eq!(use_fw, plan.firewall_mode_selected, "policy: {policy:?}"); - // The operator-visible line is emitted only when the legacy - // startup path announces rule installation. - assert_eq!( - logger - .get_buffer() - .contains("Applying network firewall rules"), - plan.firewall_mode_selected - && (!policy.allowed_hosts.is_empty() - || !policy.blocked_hosts.is_empty() - || policy.default_network_policy == NetworkPolicy::Block), - "log line must track rule installation; policy: {policy:?}" - ); - } - } - - /// With `enforcementMode: firewall`, no host lists, and - /// `defaultPolicy: allow`, the legacy startup log stays silent even - /// though the apply path still installs its default allow rule. - #[test] - fn firewall_mode_default_allow_installs_default_rule_without_log_line() { - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, - default_network_policy: NetworkPolicy::Allow, + }), ..Default::default() }; - let plan = NetworkManager::describe_policy(&policy); - assert!(plan.firewall_mode_selected); - assert!(plan.rules_will_be_installed); + manager + .start( + "principal", + "container", + &policy, + PSID::default(), + &mut logger, + ) + .unwrap(); - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let (_, use_fw) = NetworkManager::initialize_policy(&policy, &mut logger); - assert!(use_fw, "firewall mode selection must be preserved"); - assert!( - !logger - .get_buffer() - .contains("Applying network firewall rules"), - "must not claim rules are being applied; buffer: {}", - logger.get_buffer() - ); + assert!(manager.proxy_address().is_none()); + assert!(!manager.stop_all(&mut logger)); } - /// `stop_all` on a manager that installed nothing must report a clean, - /// empty teardown — not a failure. The derived `Default` for - /// `NetworkTeardown` would get this backwards, which is why it is - /// hand-written. #[test] - fn stop_all_with_nothing_installed_reports_a_clean_teardown() { - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); + fn direct_retired_policy_is_rejected_before_proxy_setup() { + let mut logger = Logger::new(Mode::Buffer); let mut manager = NetworkManager::new(); - let outcome = manager.stop_all(true, &mut logger); - assert_eq!(outcome, NetworkTeardown::default()); - assert_eq!(outcome.rules_removed, 0); - assert!(outcome.firewall_removal_ok); - assert!(!outcome.proxy_stopped); - } - - /// Skipping cleanup (`preservePolicy`) must not be reported as a failed - /// removal. - #[test] - fn stop_all_without_cleanup_reports_no_removal_failure() { - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let mut manager = NetworkManager::new(); - let outcome = manager.stop_all(false, &mut logger); - assert!(outcome.firewall_removal_ok); - assert_eq!(outcome.rules_removed, 0); - } - - #[test] - fn test_initialize_policy_capabilities_mode() { - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Capabilities, - ..Default::default() - }; - let (default_policy, use_fw) = NetworkManager::initialize_policy(&policy, &mut logger); - assert!(!use_fw); - assert_eq!(default_policy, DefaultPolicy::Allow); - } - - #[test] - fn test_initialize_policy_firewall_with_allowed_hosts() { - let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); - let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Both, - default_network_policy: NetworkPolicy::Allow, - allowed_hosts: vec!["example.com".to_string()], - ..Default::default() - }; - let (default_policy, use_fw) = NetworkManager::initialize_policy(&policy, &mut logger); - assert!(use_fw); - assert_eq!(default_policy, DefaultPolicy::Allow); - } - - #[test] - fn test_default_creates_new_manager() { - let mgr = NetworkManager::default(); - assert!(mgr.created_rule_names.is_empty()); - assert!(!mgr.wsa_initialized); - } - - #[test] - fn test_resolve_hostname_localhost() { - let result = resolve_hostname("localhost"); - assert!(result.is_ok()); - let ip = result.unwrap(); - assert!(ip == "127.0.0.1" || ip == "::1"); - } - - #[test] - fn test_resolve_hostname_invalid() { - let result = resolve_hostname("this.host.definitely.does.not.exist.invalid"); - assert!(result.is_err()); + let mut policy = ContainerPolicy::default(); + policy.network_proxy.address = Some(ProxyAddress::new("127.0.0.1".into(), 8080)); + policy.allowed_hosts.push("example.com".into()); + assert!(matches!( + manager.start("principal", "container", &policy, PSID::default(), &mut logger), + Err(WxcError::Validation(message)) if message.contains("allowedHosts") + )); + + policy.allowed_hosts.clear(); + policy.blocked_hosts.push("example.com".into()); + assert!(matches!( + manager.start("principal", "container", &policy, PSID::default(), &mut logger), + Err(WxcError::Validation(message)) if message.contains("blockedHosts") + )); + + policy.blocked_hosts.clear(); + policy.default_network_policy = NetworkPolicy::Allow; + assert!(matches!( + manager.start("principal", "container", &policy, PSID::default(), &mut logger), + Err(WxcError::Validation(message)) if message.contains("defaultPolicy") + )); + + policy.default_network_policy = NetworkPolicy::Block; + policy.network_enforcement_mode = NetworkEnforcementMode::Firewall; + assert!(matches!( + manager.start("principal", "container", &policy, PSID::default(), &mut logger), + Err(WxcError::Validation(message)) if message.contains("enforcementMode") + )); + + policy.network_enforcement_mode = NetworkEnforcementMode::Capabilities; + policy.allow_local_network = true; + assert!(matches!( + manager.start("principal", "container", &policy, PSID::default(), &mut logger), + Err(WxcError::Validation(message)) if message.contains("allowLocalNetwork") + )); + assert!(manager.proxy_address().is_none()); } } diff --git a/src/mxc-sdk/src/backends/process_container/common/network_policy_helpers.rs b/src/mxc-sdk/src/backends/process_container/common/network_policy_helpers.rs index a66972869..368112459 100644 --- a/src/mxc-sdk/src/backends/process_container/common/network_policy_helpers.rs +++ b/src/mxc-sdk/src/backends/process_container/common/network_policy_helpers.rs @@ -4,11 +4,12 @@ //! Shared ProcessContainer network-policy helpers. use crate::mxc_common::models::{ - ContainerPolicy, NetworkAction, NetworkEnforcementMode, NetworkPolicy, + ContainerPolicy, NetworkAction, NetworkEnforcementMode, NetworkPolicy, ScriptResponse, }; pub(crate) const INTERNET_CLIENT_CAPABILITY: &str = "internetClient"; pub(crate) const PRIVATE_NETWORK_CAPABILITY: &str = "privateNetworkClientServer"; +pub(crate) const CAPABILITIES_ENFORCEMENT_MODE: &str = "capabilities"; const POLICY_OWNED_NETWORK_CAPABILITIES: [&str; 4] = [ INTERNET_CLIENT_CAPABILITY, "internetClientServer", @@ -17,10 +18,41 @@ const POLICY_OWNED_NETWORK_CAPABILITIES: [&str; 4] = [ ]; pub(crate) fn allows_network_egress(policy: &ContainerPolicy) -> bool { - policy.network_egress.as_ref().map_or( - policy.default_network_policy == NetworkPolicy::Allow, - |egress| egress.default == NetworkAction::Allow || !egress.allow.is_empty(), - ) + policy + .network_egress + .as_ref() + .is_some_and(|egress| egress.default == NetworkAction::Allow || !egress.allow.is_empty()) +} + +pub(crate) fn audit_egress_default(policy: &ContainerPolicy) -> &'static str { + match policy + .network_egress + .as_ref() + .map_or(NetworkAction::Deny, |egress| egress.default) + { + NetworkAction::Allow => "allow", + NetworkAction::Deny => "block", + } +} + +pub(crate) fn reject_retired_network_policy( + policy: &ContainerPolicy, +) -> Result<(), ScriptResponse> { + let field = 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 { + None + }; + if let Some(field) = field { + return Err(ScriptResponse::rejected(&format!( + "{field} is retired; use network.egress and network.ingress" + ))); + } + Ok(()) } pub(crate) fn ensure_capability(capabilities: &mut Vec, capability: &str) { @@ -32,17 +64,6 @@ pub(crate) fn ensure_capability(capabilities: &mut Vec, capability: &str } } -pub(crate) fn uses_network_capabilities(policy: &ContainerPolicy) -> bool { - // Directional networking has no enforcementMode field. Its capability - // gates are always required; the legacy mode applies only to legacy fields. - policy.network_egress.is_some() - || policy.network_ingress.is_some() - || matches!( - policy.network_enforcement_mode, - NetworkEnforcementMode::Capabilities | NetworkEnforcementMode::Both - ) -} - pub(crate) fn add_default_network_capabilities( policy: &ContainerPolicy, capabilities: &mut Vec, @@ -55,16 +76,14 @@ pub(crate) fn add_default_network_capabilities( }); } - let uses_capabilities = uses_network_capabilities(policy); - if uses_capabilities && allows_network_egress(policy) { + if allows_network_egress(policy) { ensure_capability(capabilities, INTERNET_CLIENT_CAPABILITY); } - if uses_capabilities - && policy - .network_ingress - .as_ref() - .is_some_and(|ingress| ingress.default == NetworkAction::Allow) + if policy + .network_ingress + .as_ref() + .is_some_and(|ingress| ingress.default == NetworkAction::Allow) { ensure_capability(capabilities, PRIVATE_NETWORK_CAPABILITY); } @@ -76,9 +95,8 @@ mod tests { use crate::mxc_common::models::{NetworkEgressPolicy, NetworkIngressPolicy}; #[test] - fn directional_egress_default_overrides_legacy_default() { + fn directional_egress_defaults_and_rules_select_capability() { let mut policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, network_egress: Some(NetworkEgressPolicy { default: NetworkAction::Deny, ..Default::default() @@ -87,7 +105,6 @@ mod tests { }; assert!(!allows_network_egress(&policy)); - policy.default_network_policy = NetworkPolicy::Block; policy.network_egress = Some(NetworkEgressPolicy { default: NetworkAction::Allow, ..Default::default() @@ -105,7 +122,10 @@ mod tests { #[test] fn default_network_capabilities_are_deduplicated_case_insensitively() { let policy = ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, + network_egress: Some(NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }), network_ingress: Some(NetworkIngressPolicy { default: NetworkAction::Allow, ..Default::default() @@ -123,9 +143,8 @@ mod tests { } #[test] - fn directional_networking_ignores_legacy_enforcement_mode() { + fn directional_networking_uses_capabilities_for_both_directions() { let policy = ContainerPolicy { - network_enforcement_mode: NetworkEnforcementMode::Firewall, network_egress: Some(NetworkEgressPolicy { default: NetworkAction::Allow, ..Default::default() @@ -149,6 +168,51 @@ mod tests { ); } + #[test] + fn directly_built_requests_cannot_activate_retired_network_fields() { + let mut policy = ContainerPolicy { + default_network_policy: NetworkPolicy::Allow, + ..Default::default() + }; + assert!(reject_retired_network_policy(&policy) + .unwrap_err() + .error_message + .contains("defaultPolicy")); + + policy.default_network_policy = NetworkPolicy::Block; + policy.network_enforcement_mode = NetworkEnforcementMode::Firewall; + assert!(reject_retired_network_policy(&policy) + .unwrap_err() + .error_message + .contains("enforcementMode")); + + policy.network_enforcement_mode = NetworkEnforcementMode::Capabilities; + policy.allow_local_network = true; + assert!(reject_retired_network_policy(&policy) + .unwrap_err() + .error_message + .contains("allowLocalNetwork")); + } + + #[test] + fn audit_default_describes_directional_egress_not_allowed_exceptions() { + let mut policy = ContainerPolicy::default(); + assert_eq!(audit_egress_default(&policy), "block"); + + policy.network_egress = Some(NetworkEgressPolicy { + default: NetworkAction::Deny, + allow: vec![Default::default()], + ..Default::default() + }); + assert_eq!(audit_egress_default(&policy), "block"); + + policy.network_egress = Some(NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }); + assert_eq!(audit_egress_default(&policy), "allow"); + } + #[test] fn directional_networking_replaces_caller_owned_network_capabilities() { let policy = ContainerPolicy { @@ -182,7 +246,7 @@ mod tests { } #[test] - fn legacy_networking_preserves_caller_owned_network_capabilities() { + fn omitted_network_sections_preserve_caller_owned_capabilities() { let policy = ContainerPolicy::default(); let mut capabilities = vec![ INTERNET_CLIENT_CAPABILITY.to_string(), diff --git a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/policy.rs b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/policy.rs index 77e65ac33..aa05523c9 100644 --- a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/policy.rs +++ b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/policy.rs @@ -10,7 +10,9 @@ use std::path::Path; -use crate::mxc_common::models::{ExecutionRequest, NetworkPolicy}; +use crate::mxc_common::models::{ + ExecutionRequest, NetworkAction, NetworkEnforcementMode, NetworkPolicy, +}; use crate::windows_sandbox_lifecycle::error::OneShotError; use crate::windows_sandbox_lifecycle::vm::MappedFolder; @@ -30,9 +32,8 @@ pub(crate) fn plan_policy(request: &ExecutionRequest) -> Result Result<(), OneShotError> { let policy = &request.policy; @@ -49,14 +50,33 @@ fn validate_network(request: &ExecutionRequest) -> Result<(), OneShotError> { )); } - match policy.default_network_policy { - NetworkPolicy::Block => Ok(()), - NetworkPolicy::Allow => Err(OneShotError::Policy( - "outbound network access (network policy 'allow') is not supported by the Windows \ - Sandbox backend; the guest agent enforces network isolation" + if policy.default_network_policy != NetworkPolicy::Block + || policy.network_enforcement_mode != NetworkEnforcementMode::Capabilities + || policy.allow_local_network + { + return Err(OneShotError::Policy( + "retired network fields are not supported by the Windows Sandbox backend; use \ + network.egress and network.ingress" + .to_string(), + )); + } + if policy.network_mode_specified + || policy.network_egress.as_ref().is_some_and(|egress| { + egress.default == NetworkAction::Allow + || !egress.allow.is_empty() + || !egress.deny.is_empty() + }) + || policy.network_ingress.as_ref().is_some_and(|ingress| { + ingress.default == NetworkAction::Allow || ingress.host_loopback == NetworkAction::Allow + }) + { + return Err(OneShotError::Policy( + "directional network policy is not supported by the Windows Sandbox backend; \ + the guest agent enforces network isolation" .to_string(), - )), + )); } + Ok(()) } /// A mapped root in normalized form: the cleaned absolute string used for the @@ -295,7 +315,10 @@ fn is_descendant(child: &[String], ancestor: &[String]) -> bool { #[cfg(test)] mod tests { use super::*; - use crate::mxc_common::models::{ContainerPolicy, ProxyAddress, ProxyConfig}; + use crate::mxc_common::models::{ + ContainerPolicy, NetworkEgressPolicy, NetworkIngressPolicy, NetworkPeer, NetworkRule, + ProxyAddress, ProxyConfig, + }; fn request_with(policy: ContainerPolicy) -> ExecutionRequest { ExecutionRequest { @@ -313,23 +336,62 @@ mod tests { } } + fn assert_rejects_direct_network(policy: ContainerPolicy) { + assert!(!policy.network_mode_specified); + let err = plan_policy(&request_with(policy)).unwrap_err(); + assert_policy_err_contains(err, "directional network policy"); + } + + fn sample_rule() -> NetworkRule { + NetworkRule { + to: vec![NetworkPeer { + cidr: "192.0.2.1/32".parse().unwrap(), + except: Vec::new(), + }], + ..Default::default() + } + } + // ===== network ===== #[test] - fn default_policy_blocks_network_and_maps_nothing() { - // Schema default is Block, which is honored natively (guest enforces). + fn omitted_network_defaults_to_guest_firewall_isolation() { let plan = plan_policy(&ExecutionRequest::default()).unwrap(); assert!(plan.mapped_folders.is_empty()); } + #[test] + fn explicitly_supplied_network_posture_is_not_silently_ignored() { + let err = plan_policy(&request_with(ContainerPolicy { + network_mode_specified: true, + network_egress: Some(NetworkEgressPolicy::default()), + ..Default::default() + })) + .unwrap_err(); + assert_policy_err_contains(err, "directional network policy"); + } + #[test] fn allow_network_rejected() { + let err = plan_policy(&request_with(ContainerPolicy { + network_egress: Some(NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }), + ..Default::default() + })) + .unwrap_err(); + assert_policy_err_contains(err, "directional network policy"); + } + + #[test] + fn retired_outbound_default_is_rejected() { let err = plan_policy(&request_with(ContainerPolicy { default_network_policy: NetworkPolicy::Allow, ..Default::default() })) .unwrap_err(); - assert_policy_err_contains(err, "outbound network access"); + assert_policy_err_contains(err, "retired network fields"); } #[test] @@ -352,6 +414,50 @@ mod tests { assert_policy_err_contains(err, "per-host network filtering"); } + #[test] + fn direct_egress_allow_rule_rejected_without_presence_flag() { + assert_rejects_direct_network(ContainerPolicy { + network_egress: Some(NetworkEgressPolicy { + allow: vec![sample_rule()], + ..Default::default() + }), + ..Default::default() + }); + } + + #[test] + fn direct_egress_deny_rule_rejected_without_presence_flag() { + assert_rejects_direct_network(ContainerPolicy { + network_egress: Some(NetworkEgressPolicy { + deny: vec![sample_rule()], + ..Default::default() + }), + ..Default::default() + }); + } + + #[test] + fn direct_ingress_allow_rejected_without_presence_flag() { + assert_rejects_direct_network(ContainerPolicy { + network_ingress: Some(NetworkIngressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }), + ..Default::default() + }); + } + + #[test] + fn direct_host_loopback_allow_rejected_without_presence_flag() { + assert_rejects_direct_network(ContainerPolicy { + network_ingress: Some(NetworkIngressPolicy { + host_loopback: NetworkAction::Allow, + ..Default::default() + }), + ..Default::default() + }); + } + #[test] fn enumerate_paths_rejected() { let err = plan_policy(&request_with(ContainerPolicy { diff --git a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs index 739a8f603..94bdd46d0 100644 --- a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs +++ b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs @@ -226,6 +226,10 @@ fn reject_post_provision_policy(request: &ExecutionRequest) -> Result<(), MxcErr || !p.denied_paths.is_empty() || !p.allowed_hosts.is_empty() || !p.blocked_hosts.is_empty() + || p.default_network_policy != crate::mxc_common::models::NetworkPolicy::Block + || p.network_enforcement_mode + != crate::mxc_common::models::NetworkEnforcementMode::Capabilities + || p.allow_local_network || p.network_proxy.is_enabled() || p.network_mode_specified || p.network_egress.is_some() diff --git a/src/mxc-sdk/src/backends/wslc/common/policy.rs b/src/mxc-sdk/src/backends/wslc/common/policy.rs index 703b16b8d..156a2f5b9 100644 --- a/src/mxc-sdk/src/backends/wslc/common/policy.rs +++ b/src/mxc-sdk/src/backends/wslc/common/policy.rs @@ -13,13 +13,9 @@ //! | `readwrite` / `readonly` | honoured (volume mounts) | rejected (immutable) | rejected (immutable) | //! | `denied_paths` | rejected if overlapping [^1] | rejected | rejected | //! | `ui` | rejected (no UI primitive) | rejected | rejected | -//! | `allowed` / `blocked` hosts | rejected (no host filtering) | rejected | rejected | -//! | `allow_local_network` | rejected if `true` | rejected | rejected | -//! | `network_enforcement_mode` | rejected if not `capabilities` | rejected | rejected | //! | `network.egress.default` | honoured (None / Bridged) | rejected (immutable) | rejected (immutable) | //! | `network.ingress.*` | must match egress (deny all / allow all) | rejected | rejected | //! | `runtimeConfig.networkProxy` | rejected (applies at exec) | rejected | honoured (cooperative env) | -//! | legacy `default_network_policy` / `network.proxy` | compatibility inputs only | rejected | proxy URL only | //! //! [^1]: a standalone `denied_path` is honoured by container isolation (unlisted //! host paths are simply never mounted); only a denied path nested under a @@ -28,9 +24,8 @@ //! //! Checks run filesystem → ui → network, so a request that trips several gets a //! stable message rather than one that depends on field order (asserted by -//! tests). [`reject_ui_policy`] and [`reject_unsupported_enforcement_mode`] -//! describe the backend rather than a phase, so the one-shot `validate_runner` -//! calls them too. +//! tests). [`reject_ui_policy`] and [`reject_retired_network_fields`] apply +//! to both one-shot and state-aware validation. use crate::mxc_common::models::{ ExecutionRequest, NetworkAction, NetworkEnforcementMode, NetworkPolicy, @@ -46,36 +41,23 @@ const ERR_FILESYSTEM_IMMUTABLE: &str = phase and cannot be changed by the WSLc backend after provisioning"; const ERR_ENUMERATE_PATHS: &str = "processContainer.filesystem.enumeratePaths is not supported by the WSLc backend"; -const ERR_HOST_FILTERING: &str = - "per-host network filtering (allowedHosts / blockedHosts) is not supported by the WSLc backend"; const ERR_NETWORK_IMMUTABLE: &str = "network mode is bound to the provision phase and cannot be changed by the WSLc backend after \ provisioning"; const ERR_PROXY_AT_PROVISION: &str = - "runtimeConfig.networkProxy (legacy network.proxy) is applied per-exec by the WSLc backend; set it on the exec phase, not provision"; + "runtimeConfig.networkProxy is applied per-exec by the WSLc backend; set it on the exec phase, not provision"; const ERR_PROXY_AT_PHASE: &str = - "runtimeConfig.networkProxy (legacy network.proxy) is only honoured on the exec phase by the WSLc backend"; + "runtimeConfig.networkProxy is only honoured on the exec phase by the WSLc backend"; const ERR_PROXY_URL_FORM: &str = - "WSLc: network.proxy requires the 'url' form (a routable proxy URL); the localhost and \ - builtinTestServer forms are not supported because a WSL container runs in its own network \ - namespace"; + "WSLc: runtimeConfig.networkProxy requires a proxy url reachable from inside the container; \ + a host-loopback proxy is not reachable from the container's own network namespace"; const ERR_UI_POLICY: &str = "WSLc: the ui section is not supported. The backend has no mechanism to enforce UI \ restrictions on a container, so no ui posture is truthful here. Omitting the ui section is \ accepted but applies no restriction — it is not the lockdown the schema's default implies. \ Use a backend that enforces UI policy if you need one"; -const ERR_ALLOW_LOCAL_NETWORK_STATE_AWARE: &str = - "WSLc: network.allowLocalNetwork=true is not supported by the state-aware WSLc backend. The \ - container's network is all-or-nothing (defaultPolicy 'block' → isolated, 'allow' → bridged \ - NAT), and the state-aware provision phase has no port-mapping primitive to expose an \ - inbound port"; -const ERR_ENFORCEMENT_MODE: &str = - "WSLc: network.enforcementMode 'firewall' and 'both' are not supported. A WSL container has \ - no CAP_NET_ADMIN for in-container firewall rules, and VM-level enforcement is not available \ - without breaking other security guarantees (e.g. MDE). Remove the field or set it to \ - 'capabilities' — WSLc's network is all-or-nothing at the container level"; const ERR_PROXY_CREDENTIALS_IN_ARGV: &str = - "WSLc: runtimeConfig.networkProxy (legacy network.proxy) must not carry credentials when \ + "WSLc: runtimeConfig.networkProxy must not carry credentials when \ process.env is supplied without process.inheritDefaultEnv. That combination replaces the \ container image's environment, which WSLc performs by prefixing the command line with \ 'env -i NAME=VALUE', so the proxy URL becomes a process argument readable through \ @@ -92,12 +74,13 @@ pub(crate) fn network_policy_support() -> NetworkPolicySupport { | NetworkPolicySupport::RUNTIME_PROXY } -/// Read the authoritative directional posture, falling back only for legacy input. +/// Read the directional posture; absent egress defaults to deny. pub(crate) fn network_is_isolated(request: &ExecutionRequest) -> bool { - request.policy.network_egress.as_ref().map_or_else( - || request.policy.default_network_policy == NetworkPolicy::Block, - |egress| egress.default == NetworkAction::Deny, - ) + request + .policy + .network_egress + .as_ref() + .is_none_or(|egress| egress.default == NetworkAction::Deny) } /// No firewall is installed inside or outside a WSLc container. NONE denies all @@ -140,7 +123,7 @@ pub(crate) fn validate_directional_network(request: &ExecutionRequest) -> Result } /// Validate the request for the provision phase. `rw` / `ro` paths become -/// volume mounts and `default_network_policy` selects the container network +/// volume mounts and `network.egress.default` selects the container network /// mode; both are honoured here. Everything else in the module table is /// rejected. pub(crate) fn validate_provision_policy(request: &ExecutionRequest) -> Result<(), MxcError> { @@ -154,9 +137,7 @@ pub(crate) fn validate_provision_policy(request: &ExecutionRequest) -> Result<() ) .map_err(MxcError::policy_validation)?; reject_ui_policy(request)?; - reject_host_filtering(request)?; - reject_provision_allow_local_network(request)?; - reject_unsupported_enforcement_mode(request)?; + reject_retired_network_fields(request)?; validate_directional_network(request)?; if request.policy.network_proxy.is_enabled() { return Err(MxcError::policy_validation(ERR_PROXY_AT_PROVISION)); @@ -170,7 +151,7 @@ pub(crate) fn validate_provision_policy(request: &ExecutionRequest) -> Result<() pub(crate) fn validate_post_provision_policy(request: &ExecutionRequest) -> Result<(), MxcError> { reject_filesystem_policy(request)?; reject_ui_policy(request)?; - reject_host_filtering(request)?; + reject_retired_network_fields(request)?; reject_post_provision_network_mode(request)?; if request.policy.network_proxy.is_enabled() { return Err(MxcError::policy_validation(ERR_PROXY_AT_PHASE)); @@ -184,7 +165,7 @@ pub(crate) fn validate_post_provision_policy(request: &ExecutionRequest) -> Resu pub(crate) fn validate_exec_policy(request: &ExecutionRequest) -> Result<(), MxcError> { reject_filesystem_policy(request)?; reject_ui_policy(request)?; - reject_host_filtering(request)?; + reject_retired_network_fields(request)?; reject_post_provision_network_mode(request)?; if request.policy.network_proxy.is_enabled() && exec_proxy_url(request).is_none() { return Err(MxcError::policy_validation(ERR_PROXY_URL_FORM)); @@ -240,13 +221,6 @@ fn reject_filesystem_policy(request: &ExecutionRequest) -> Result<(), MxcError> Ok(()) } -fn reject_host_filtering(request: &ExecutionRequest) -> Result<(), MxcError> { - if !request.policy.allowed_hosts.is_empty() || !request.policy.blocked_hosts.is_empty() { - return Err(MxcError::policy_validation(ERR_HOST_FILTERING)); - } - Ok(()) -} - /// Reject any supplied UI policy: WSLc cannot enforce UI restrictions. pub(crate) fn reject_ui_policy(request: &ExecutionRequest) -> Result<(), MxcError> { if request.policy.ui_specified { @@ -255,34 +229,33 @@ pub(crate) fn reject_ui_policy(request: &ExecutionRequest) -> Result<(), MxcErro Ok(()) } -/// Reject an enforcement mode WSLc cannot implement. Value-based: an explicit -/// `capabilities` is accepted, since it describes what WSLc does. -pub(crate) fn reject_unsupported_enforcement_mode( - request: &ExecutionRequest, -) -> Result<(), MxcError> { - match request.policy.network_enforcement_mode { - NetworkEnforcementMode::Capabilities => Ok(()), - NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both => { - Err(MxcError::policy_validation(ERR_ENFORCEMENT_MODE)) - } - } -} - -/// Reject inbound local networking at provision. Separate from the one-shot -/// message, which points at `wslc.portMappings` — a primitive the -/// state-aware surface does not have. -fn reject_provision_allow_local_network(request: &ExecutionRequest) -> Result<(), MxcError> { - if request.policy.allow_local_network { - return Err(MxcError::policy_validation( - ERR_ALLOW_LOCAL_NETWORK_STATE_AWARE, - )); +/// Reject retired runtime fields before backend provisioning or execution. +pub(crate) fn reject_retired_network_fields(request: &ExecutionRequest) -> Result<(), MxcError> { + let policy = &request.policy; + let field = 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 { + None + }; + if let Some(field) = field { + return Err(MxcError::policy_validation(format!( + "WSLc: {field} is retired; use network.egress and network.ingress" + ))); } Ok(()) } /// Reject any network *mode* field supplied after provision: the posture is /// bound to the provision phase. Presence, not value — an explicit -/// `defaultPolicy: "block"` is indistinguishable from an omitted one by value. +/// `egress.default: "deny"` must still be rejected. /// The cooperative proxy is a separate exec-time concern handled by the callers. fn reject_post_provision_network_mode(request: &ExecutionRequest) -> Result<(), MxcError> { if request.policy.network_mode_specified @@ -298,7 +271,8 @@ fn reject_post_provision_network_mode(request: &ExecutionRequest) -> Result<(), mod tests { use super::*; use crate::mxc_common::models::{ - ContainerPolicy, NetworkPolicy, ProxyAddress, ProxyConfig, UiPolicy, + ContainerPolicy, NetworkEgressPolicy, NetworkIngressPolicy, ProxyAddress, ProxyConfig, + UiPolicy, }; use crate::mxc_common::mxc_error::MxcErrorCode; @@ -481,7 +455,14 @@ mod tests { let req = request_with_policy(ContainerPolicy { readwrite_paths: vec!["C:\\src".to_string()], readonly_paths: vec!["C:\\data".to_string()], - default_network_policy: NetworkPolicy::Allow, + network_egress: Some(NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }), + network_ingress: Some(NetworkIngressPolicy { + default: NetworkAction::Allow, + host_loopback: NetworkAction::Allow, + }), ..Default::default() }); validate_provision_policy(&req).unwrap(); @@ -689,7 +670,7 @@ mod tests { /// the field sets `network_mode_specified`, which immutability already /// refuses. Pinned so both rejections can't be dropped as "redundant". #[test] - fn post_provision_rejects_allow_local_network_as_a_mode_change() { + fn post_provision_rejects_retired_allow_local_network() { let req = request_with_policy(ContainerPolicy { allow_local_network: true, network_mode_specified: true, @@ -697,9 +678,9 @@ mod tests { }); assert_policy_validation( validate_post_provision_policy(&req).unwrap_err(), - "network mode", + "allowLocalNetwork", ); - assert_policy_validation(validate_exec_policy(&req).unwrap_err(), "network mode"); + assert_policy_validation(validate_exec_policy(&req).unwrap_err(), "allowLocalNetwork"); } // ---- enforcementMode ---- diff --git a/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs b/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs index a39a57dfd..d26035b9e 100644 --- a/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs +++ b/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs @@ -466,121 +466,15 @@ fn validate_denied_path_overlap_with( /// - `None` — no network interface, fully isolated /// - `Bridged` — NAT networking through the WSL2 VM's virtual adapter /// -/// Per-host filtering (`allowedHosts`/`blockedHosts` that need enforcement) is -/// rejected before this runs — see `build_iptables_rules`. This only maps the -/// bare-default posture: -/// -/// - `Block` with no host rules → `None` (fully isolated) -/// - `Allow` → `Bridged` (NAT) -pub fn map_network_policy(is_block: bool, has_host_rules: bool) -> WslcContainerNetworkingMode { - if is_block && !has_host_rules { +/// Directional egress selects the all-or-nothing network mode. +pub fn map_network_policy(is_block: bool) -> WslcContainerNetworkingMode { + if is_block { WslcContainerNetworkingMode::WSLC_CONTAINER_NETWORKING_MODE_NONE } else { WslcContainerNetworkingMode::WSLC_CONTAINER_NETWORKING_MODE_BRIDGED } } -/// Returns true if the policy requests per-host filtering (which WSLc cannot -/// enforce — such configs are rejected before execution). -/// -/// Thin wrapper over [`crate::mxc_common::models::needs_host_filtering`] so the parser -/// and this backend share one definition: -/// - `Block` → only `allowed_hosts` matter (allowlist) -/// - `Allow` → only `blocked_hosts` matter (blocklist) -pub fn needs_host_filtering( - is_default_block: bool, - allowed_hosts: &[String], - blocked_hosts: &[String], -) -> bool { - crate::mxc_common::models::needs_host_filtering(is_default_block, allowed_hosts, blocked_hosts) -} - -/// Validate that a host string is safe for use in an iptables command. -/// Accepts hostnames (a-z, 0-9, dots, hyphens) and IPv4/IPv6 addresses -/// (digits, dots, colons, brackets, slash for CIDR). -/// Rejects empty strings and anything containing shell metacharacters. -fn is_valid_host(host: &str) -> bool { - !host.is_empty() - && host - .bytes() - .all(|b| b.is_ascii_alphanumeric() || b".-:[]/_".contains(&b)) -} - -/// Build iptables commands for per-host network filtering. -/// -/// **NOTE: WSLC per-host filtering is non-functional.** Two independent -/// blockers prevent it from working: -/// 1. WSLC containers lack `CAP_NET_ADMIN` — the SDK's `Privileged` flag does -/// **not** grant it — so the in-container `iptables` exec is rejected. -/// 2. WSLC cannot expose VM-level network enforcement without breaking other -/// security guarantees (e.g. MDE); a host-enforced design is longer-tail. -/// -/// Configs that require per-host filtering are therefore **rejected at -/// config-parse time** (and by `WSLContainerRunner::validate_runner`) before -/// this function is reached. This function is retained for reference but its -/// output is never applied. -/// -/// When `defaultPolicy` is `Block` + `allowedHosts`: -/// - Default DROP all outbound -/// - ACCEPT to each allowed host -/// - ACCEPT established/related (for return traffic) -/// - ACCEPT loopback -/// -/// When `defaultPolicy` is `Allow` + `blockedHosts`: -/// - DROP to each blocked host -/// -/// Returns a shell command string to be exec'd inside the container. -/// -/// Host values are validated to prevent shell command injection. -pub fn build_iptables_rules( - allowed_hosts: &[String], - blocked_hosts: &[String], - is_default_block: bool, -) -> Option { - if allowed_hosts.is_empty() && blocked_hosts.is_empty() { - return None; - } - - let mut rules = Vec::new(); - - if is_default_block && !allowed_hosts.is_empty() { - // Allow loopback and established connections first - rules.push("iptables -A OUTPUT -o lo -j ACCEPT".to_string()); - rules.push("iptables -A OUTPUT -m state --state ESTABLISHED,RELATED -j ACCEPT".to_string()); - - // Allow DNS (needed to resolve hostnames) - rules.push("iptables -A OUTPUT -p udp --dport 53 -j ACCEPT".to_string()); - rules.push("iptables -A OUTPUT -p tcp --dport 53 -j ACCEPT".to_string()); - - // Allow each specified host - for host in allowed_hosts { - if !is_valid_host(host) { - continue; - } - rules.push(format!("iptables -A OUTPUT -d {} -j ACCEPT", host)); - } - - // Default drop everything else - rules.push("iptables -A OUTPUT -j DROP".to_string()); - } else if !is_default_block && !blocked_hosts.is_empty() { - // Block specific hosts - for host in blocked_hosts { - if !is_valid_host(host) { - continue; - } - rules.push(format!("iptables -A OUTPUT -d {} -j DROP", host)); - } - } - - if rules.is_empty() { - None - } else { - // Join with shell && so each rule must succeed before the next runs. - // If any iptables command fails, the chain stops and the error propagates. - Some(rules.join(" && ")) - } -} - // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -1190,59 +1084,21 @@ mod tests { // -- Network policy tests -- #[test] - fn network_block_no_hosts_maps_to_none() { + fn denied_egress_maps_to_none() { assert_eq!( - map_network_policy(true, false), + map_network_policy(true), WslcContainerNetworkingMode::WSLC_CONTAINER_NETWORKING_MODE_NONE ); } #[test] - fn network_block_with_hosts_maps_to_bridged() { + fn allowed_egress_maps_to_bridged() { assert_eq!( - map_network_policy(true, true), + map_network_policy(false), WslcContainerNetworkingMode::WSLC_CONTAINER_NETWORKING_MODE_BRIDGED ); } - #[test] - fn network_allow_maps_to_bridged() { - assert_eq!( - map_network_policy(false, false), - WslcContainerNetworkingMode::WSLC_CONTAINER_NETWORKING_MODE_BRIDGED - ); - } - - // -- Host filtering tests -- - - #[test] - fn needs_host_filtering_empty() { - assert!(!needs_host_filtering(true, &[], &[])); - assert!(!needs_host_filtering(false, &[], &[])); - } - - #[test] - fn needs_host_filtering_block_with_allowed() { - assert!(needs_host_filtering(true, &["1.2.3.4".to_string()], &[])); - } - - #[test] - fn needs_host_filtering_allow_with_blocked() { - assert!(needs_host_filtering(false, &[], &["evil.com".to_string()])); - } - - #[test] - fn needs_host_filtering_block_with_blocked_only_is_false() { - // block + blockedHosts makes no sense — blocking is already the default - assert!(!needs_host_filtering(true, &[], &["evil.com".to_string()])); - } - - #[test] - fn needs_host_filtering_allow_with_allowed_only_is_false() { - // allow + allowedHosts makes no sense — everything is already allowed - assert!(!needs_host_filtering(false, &["1.2.3.4".to_string()], &[])); - } - // -- Path edge case tests -- #[test] @@ -1259,76 +1115,4 @@ mod tests { Some("/mnt/c".to_string()) ); } - - #[test] - fn iptables_none_when_no_hosts() { - assert!(build_iptables_rules(&[], &[], true).is_none()); - assert!(build_iptables_rules(&[], &[], false).is_none()); - } - - #[test] - fn iptables_block_with_allowed_hosts() { - let rules = build_iptables_rules( - &["1.2.3.4".to_string(), "example.com".to_string()], - &[], - true, - ) - .unwrap(); - assert!(rules.contains("iptables -A OUTPUT -o lo -j ACCEPT")); - assert!(rules.contains("iptables -A OUTPUT -d 1.2.3.4 -j ACCEPT")); - assert!(rules.contains("iptables -A OUTPUT -d example.com -j ACCEPT")); - assert!(rules.contains("iptables -A OUTPUT -j DROP")); - } - - #[test] - fn iptables_allow_with_blocked_hosts() { - let rules = build_iptables_rules( - &[], - &["evil.com".to_string(), "10.0.0.1".to_string()], - false, - ) - .unwrap(); - assert!(rules.contains("iptables -A OUTPUT -d evil.com -j DROP")); - assert!(rules.contains("iptables -A OUTPUT -d 10.0.0.1 -j DROP")); - assert!(!rules.contains("-j ACCEPT")); - } - - #[test] - fn is_valid_host_accepts_valid_entries() { - assert!(is_valid_host("example.com")); - assert!(is_valid_host("192.168.1.1")); - assert!(is_valid_host("10.0.0.0/8")); - assert!(is_valid_host("my-host.example.com")); - assert!(is_valid_host("::1")); - assert!(is_valid_host("[::1]")); - assert!(is_valid_host("2001:db8::1")); - } - - #[test] - fn is_valid_host_rejects_injection() { - assert!(!is_valid_host("")); - assert!(!is_valid_host("; rm -rf /")); - assert!(!is_valid_host("host && echo pwned")); - assert!(!is_valid_host("host | cat /etc/passwd")); - assert!(!is_valid_host("$(whoami)")); - assert!(!is_valid_host("host`id`")); - assert!(!is_valid_host("host name with spaces")); - } - - #[test] - fn iptables_skips_invalid_hosts() { - let rules = build_iptables_rules( - &[], - &[ - "good.com".to_string(), - "; rm -rf /".to_string(), - "10.0.0.1".to_string(), - ], - false, - ) - .unwrap(); - assert!(rules.contains("good.com")); - assert!(rules.contains("10.0.0.1")); - assert!(!rules.contains("rm")); - } } diff --git a/src/mxc-sdk/src/backends/wslc/common/state_aware.rs b/src/mxc-sdk/src/backends/wslc/common/state_aware.rs index 761982b69..3a50b42f2 100644 --- a/src/mxc-sdk/src/backends/wslc/common/state_aware.rs +++ b/src/mxc-sdk/src/backends/wslc/common/state_aware.rs @@ -22,8 +22,6 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use crate::mxc_common::logger::{Logger, Mode}; -#[cfg(test)] -use crate::mxc_common::models::NetworkPolicy; use crate::mxc_common::models::{ContainerPolicy, ExecutionRequest, WslcProvisionConfig}; use crate::mxc_common::mxc_error::MxcError; use crate::mxc_common::state_aware_backend::{ @@ -526,6 +524,7 @@ fn build_provision_config( request: &ExecutionRequest, config: Option, ) -> Result { + crate::wslc_common::policy::reject_retired_network_fields(request)?; let image = config .as_ref() .and_then(|c| c.image.clone()) @@ -846,13 +845,7 @@ mod tests { #[test] fn map_network_maps_block_to_none() { - let req = ExecutionRequest { - policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Block, - ..Default::default() - }, - ..Default::default() - }; + let req = ExecutionRequest::default(); assert_eq!(map_network(&req), NetworkMode::None); } @@ -860,7 +853,10 @@ mod tests { fn map_network_maps_allow_to_bridged() { let req = ExecutionRequest { policy: ContainerPolicy { - default_network_policy: NetworkPolicy::Allow, + network_egress: Some(NetworkEgressPolicy { + default: NetworkAction::Allow, + ..Default::default() + }), ..Default::default() }, ..Default::default() diff --git a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs index 12727e6c9..faa567d4d 100644 --- a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs +++ b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs @@ -432,27 +432,7 @@ impl ScriptRunner for WSLContainerRunner { policy_mapping::container_working_directory(&request.working_directory) .map_err(|msg| WslcError::Rejected(msg).into_response())?; policy::reject_ui_policy(request).map_err(as_wslc_rejection)?; - if request.policy.needs_host_filtering() { - return Err(WslcError::Rejected( - "WSLc: per-host egress filtering (allowedHosts with \ - defaultPolicy='block', or blockedHosts with defaultPolicy='allow') \ - is not supported. A WSL container has no CAP_NET_ADMIN for in-container \ - iptables, and VM-level enforcement is not available without breaking other \ - security guarantees (e.g. MDE). Use network.proxy (defaultPolicy='allow') \ - for cooperative host filtering, or remove the host lists." - .to_string(), - ) - .into_response()); - } - if request.policy.allow_local_network { - return Err(WslcError::Rejected( - "WSLc: network.allowLocalNetwork=true is not supported. Expose specific \ - ports with wslc portMappings instead." - .to_string(), - ) - .into_response()); - } - policy::reject_unsupported_enforcement_mode(request).map_err(as_wslc_rejection)?; + policy::reject_retired_network_fields(request).map_err(as_wslc_rejection)?; // The shared validator returns an untagged response; retag it so its // rejections reach SDK callers as `policy_validation` like the checks above. validate_network_policy_support(request, policy::network_policy_support()) @@ -600,105 +580,6 @@ impl WSLContainerRunner { )) } - /// Apply iptables rules inside a running container for host filtering. - /// - /// # Safety - /// `sdk` must contain valid function pointers and `container` must be a - /// live container handle for a started container. - unsafe fn apply_iptables_rules( - sdk: &'static WslcSdk, - container: WslcContainer, - ipt_cmd: &str, - logger: &mut Logger, - ) -> Result<(), ScriptResponse> { - let _ = writeln!(logger, "[WSLC] Applying iptables rules for host filtering"); - let mut ipt_settings = std::mem::zeroed::(); - let hr = sdk.WslcInitProcessSettings(&mut ipt_settings); - if hr != S_OK { - return Err(sdk_error( - "WslcInitProcessSettings (iptables) failed", - hr, - "", - )); - } - - let ipt_sh = b"/bin/sh\0"; - let ipt_c = b"-c\0"; - let ipt_script = format!("{}\0", ipt_cmd); - let ipt_script_bytes = ipt_script.as_bytes(); - let ipt_argv: [PCSTR; 3] = [ - ipt_sh.as_ptr() as PCSTR, - ipt_c.as_ptr() as PCSTR, - ipt_script_bytes.as_ptr() as PCSTR, - ]; - let hr = - sdk.WslcSetProcessSettingsCmdLine(&mut ipt_settings, ipt_argv.as_ptr(), ipt_argv.len()); - if hr != S_OK { - return Err(sdk_error( - "WslcSetProcessSettingsCmdLine (iptables) failed", - hr, - "", - )); - } - - let mut ipt_process: WslcProcess = ptr::null_mut(); - let mut err_msg = CoTaskMemPWSTR::null(); - let hr = sdk.WslcCreateContainerProcess( - container, - &mut ipt_settings, - &mut ipt_process, - err_msg.as_mut_ptr(), - ); - if hr != S_OK { - let msg = err_msg.to_string_lossy(); - return Err(sdk_error("Failed to exec iptables rules", hr, &msg)); - } - let ipt_guard = WslcProcessGuard::from_raw(ipt_process, sdk.release_process_fn()); - - // Wait for iptables to complete - let mut ipt_exit_event: HANDLE = ptr::null_mut(); - let hr = sdk.WslcGetProcessExitEvent(ipt_guard.as_raw(), &mut ipt_exit_event); - if hr != S_OK { - return Err(sdk_error( - "WslcGetProcessExitEvent (iptables) failed", - hr, - "", - )); - } - if !ipt_exit_event.is_null() { - let wait_result = windows::Win32::System::Threading::WaitForSingleObject( - windows::Win32::Foundation::HANDLE(ipt_exit_event), - 30_000, - ); - if wait_result == windows::Win32::Foundation::WAIT_TIMEOUT { - return Err( - WslcError::Runtime("iptables rules timed out after 30s".to_string()) - .into_response(), - ); - } - } - - let mut ipt_exit_code: i32 = -1; - let hr = sdk.WslcGetProcessExitCode(ipt_guard.as_raw(), &mut ipt_exit_code); - if hr != S_OK { - return Err(sdk_error( - "WslcGetProcessExitCode (iptables) failed", - hr, - "", - )); - } - if ipt_exit_code != 0 { - return Err(WslcError::Runtime(format!( - "iptables rules failed with exit code {} \ - (image may not have iptables installed)", - ipt_exit_code - )) - .into_response()); - } - let _ = writeln!(logger, "[WSLC] iptables rules applied successfully"); - Ok(()) - } - /// Wait for process exit with timeout enforcement. /// Returns (exit_code, timed_out). /// @@ -918,6 +799,7 @@ impl WSLContainerRunner { logger: &mut Logger, output: OutputMode, ) -> Result { + policy::reject_retired_network_fields(request).map_err(as_wslc_rejection)?; let _ = writeln!(logger, "{START_CONTAINER_BANNER}"); // WSLc provision-time filesystem-policy gate (D6 normalization → D3 @@ -1192,12 +1074,7 @@ impl WSLContainerRunner { } let is_default_block = policy::network_is_isolated(request); - let has_host_rules = policy_mapping::needs_host_filtering( - is_default_block, - &request.policy.allowed_hosts, - &request.policy.blocked_hosts, - ); - let net_mode = policy_mapping::map_network_policy(is_default_block, has_host_rules); + let net_mode = policy_mapping::map_network_policy(is_default_block); let hr = sdk.WslcSetContainerSettingsNetworkingMode(&mut container_settings, net_mode); if hr != S_OK { return Err(sdk_error( @@ -1208,12 +1085,6 @@ impl WSLContainerRunner { } let _ = writeln!(logger, "[WSLC] Networking mode: {:?}", net_mode); - let iptables_cmd = policy_mapping::build_iptables_rules( - &request.policy.allowed_hosts, - &request.policy.blocked_hosts, - is_default_block, - ); - let mut flags = WslcContainerFlags::WSLC_CONTAINER_FLAG_NONE; if request.lifecycle.destroy_on_exit { flags |= WslcContainerFlags::WSLC_CONTAINER_FLAG_AUTO_REMOVE; @@ -1221,9 +1092,6 @@ impl WSLContainerRunner { if self.config.gpu { flags |= WslcContainerFlags::WSLC_CONTAINER_FLAG_ENABLE_GPU; } - if has_host_rules { - flags |= WslcContainerFlags::WSLC_CONTAINER_FLAG_PRIVILEGED; - } let hr = sdk.WslcSetContainerSettingsFlags(&mut container_settings, flags); if hr != S_OK { return Err(sdk_error("WslcSetContainerSettingsFlags failed", hr, "")); @@ -1272,11 +1140,8 @@ impl WSLContainerRunner { // reverse declaration order — freeing the callback context (`io_ctx` / // `io_ctx_guard`) *before* the session is terminated and the DLL // unloaded — so a late callback could dereference freed memory. Every - // failure past this point therefore quiesces the container first. This - // is not a corner case: `apply_iptables_rules` fails for any host-rule - // policy today, since the container is not granted `CAP_NET_ADMIN`. - let post_start = - Self::attach_init_process(sdk, &container_guard, iptables_cmd.as_deref(), logger); + // failure past this point therefore quiesces the container first. + let post_start = Self::attach_init_process(sdk, &container_guard); let process_guard = match post_start { Ok(guard) => guard, Err(e) => { @@ -1306,22 +1171,16 @@ impl WSLContainerRunner { }) } - /// Apply any host-rule `iptables` chain and take the container's init - /// process handle. Split out so every failure between "container started" - /// and "handle in hand" funnels through one caller-side cleanup path. + /// Take the container's init process handle. Split out so every failure + /// between "container started" and "handle in hand" funnels through one + /// caller-side cleanup path. /// /// # Safety /// `sdk` must hold valid function pointers and `container` a live handle. unsafe fn attach_init_process( sdk: &'static WslcSdk, container: &WslcContainerGuard, - iptables_cmd: Option<&str>, - logger: &mut Logger, ) -> Result { - if let Some(ipt_cmd) = iptables_cmd { - Self::apply_iptables_rules(sdk, container.as_raw(), ipt_cmd, logger)?; - } - let mut process: WslcProcess = ptr::null_mut(); let hr = sdk.WslcGetContainerInitProcess(container.as_raw(), &mut process); if hr != S_OK { @@ -1867,7 +1726,7 @@ mod tests { } #[test] - fn validate_runner_rejects_allowlist_host_filtering() { + fn validate_runner_rejects_retired_allowlist() { // block default + allowlist = per-host filtering WSLc can't enforce. let request = ExecutionRequest { containment: crate::mxc_common::models::ContainmentBackend::Wslc, @@ -1880,7 +1739,7 @@ mod tests { }; let runner = WSLContainerRunner::new(&WslcConfig::default()); let err = runner.validate_runner(&request).unwrap_err(); - assert!(err.error_message.contains("per-host egress filtering")); + assert!(err.error_message.contains("allowedHosts")); } #[test] @@ -1914,12 +1773,9 @@ mod tests { assert!(err.error_message.contains("allowLocalNetwork")); } - /// The two surfaces refuse `allowLocalNetwork` with deliberately different - /// remedies: one-shot has `wslc.portMappings` to point at, - /// state-aware has no port-mapping primitive at all. Unifying the messages - /// would send state-aware users after a dead end. + /// Neither surface accepts a retired network field. #[test] - fn both_surfaces_reject_allow_local_network_with_surface_specific_remedies() { + fn both_surfaces_reject_retired_allow_local_network() { let request = ExecutionRequest { containment: crate::mxc_common::models::ContainmentBackend::Wslc, policy: crate::mxc_common::models::ContainerPolicy { @@ -1936,11 +1792,7 @@ mod tests { one_shot.failure_phase, crate::mxc_common::models::FailurePhase::Rejected ); - assert!( - one_shot.error_message.contains("portMappings"), - "one-shot has a port-mapping primitive and must name it; got: {}", - one_shot.error_message - ); + assert!(one_shot.error_message.contains("allowLocalNetwork")); let state_aware = crate::wslc_common::policy::validate_provision_policy(&request).unwrap_err(); @@ -1948,11 +1800,6 @@ mod tests { state_aware.code, crate::mxc_common::mxc_error::MxcErrorCode::PolicyValidation ); - assert!( - !state_aware.message.contains("portMappings"), - "state-aware has no port-mapping primitive to point at; got: {}", - state_aware.message - ); assert!( state_aware.message.contains("allowLocalNetwork"), "got: {}", @@ -1983,12 +1830,22 @@ mod tests { #[test] fn validate_runner_accepts_bare_defaults() { - // Full cutoff / full NAT (no host lists) is enforceable — must pass. - for policy in [NetworkPolicy::Allow, NetworkPolicy::Block] { + // Both supported all-or-nothing postures are enforceable. + for action in [ + crate::mxc_common::models::NetworkAction::Allow, + crate::mxc_common::models::NetworkAction::Deny, + ] { let request = ExecutionRequest { containment: crate::mxc_common::models::ContainmentBackend::Wslc, policy: crate::mxc_common::models::ContainerPolicy { - default_network_policy: policy, + network_egress: Some(crate::mxc_common::models::NetworkEgressPolicy { + default: action, + ..Default::default() + }), + network_ingress: Some(crate::mxc_common::models::NetworkIngressPolicy { + default: action, + host_loopback: action, + }), ..Default::default() }, ..Default::default()