diff --git a/README.md b/README.md index f14aa0efe..d80f33e4a 100644 --- a/README.md +++ b/README.md @@ -225,7 +225,7 @@ See [docs/diagnostics.md](docs/diagnostics.md) for full diagnostics reference. wxc-exec.exe --audit policy.json ``` -> **Warning:** `--audit` injects `permissiveLearningMode` — AppContainer restrictions are **not** enforced for the duration of the run. Use only for policy authoring. `learningModeLogging` and `permissiveLearningMode` are reserved internal capability names and are rejected in `processContainer.capabilities`; use `processContainer.learningMode: true` for deny-and-record mode or `--audit` for permissive mode. See [docs/learning-mode/capabilities.md](docs/learning-mode/capabilities.md) for the three learning-mode flows. +> **Warning:** `--audit` injects `permissiveLearningMode` — AppContainer restrictions are **not** enforced for the duration of the run. Use only for policy authoring. It cannot be combined with `processContainer.captureDenials`; use `captureDenials.mode: "allow"` for permissive application-driven capture. `learningModeLogging` and `permissiveLearningMode` are reserved internal capability names and are rejected in `processContainer.capabilities`. See [docs/learning-mode/capabilities.md](docs/learning-mode/capabilities.md) for the three learning-mode flows. ## Telemetry (Experimental) diff --git a/docs/learning-mode/capabilities.md b/docs/learning-mode/capabilities.md index c2f104e6a..f496113bf 100644 --- a/docs/learning-mode/capabilities.md +++ b/docs/learning-mode/capabilities.md @@ -87,6 +87,8 @@ stays enforced: 1. **Developer inner-loop (`--audit`).** A developer runs `wxc-exec --audit` with ProcessContainer containment to discover the capabilities and paths their process needs. `--audit` is rejected for every other Windows backend. + It is also mutually exclusive with `captureDenials`; use + `captureDenials.mode: "allow"` for permissive application-driven capture. It triggers UAC, injects `permissiveLearningMode`, and drives a WPR/ETW permissive-learning-mode trace for the run. This is typically a static config the developer iterates on locally. @@ -115,6 +117,11 @@ Windows-only `captureDenials` config switch drives collecting those events and surfacing the resulting denials to the caller. Its `mode` selects how each ungranted access is handled while it is recorded: +> **Host requirement.** `captureDenials` requires a feature-enabled Windows +> build exposing the BaseContainer security-environment and Learning Mode APIs. +> It is not supported by the AppContainer fallback tiers; unsupported hosts +> return `backend_unavailable`. + - `mode: "block"` (default) maps onto `learningModeLogging` (deny-and-record) — the app / user-configurable flow. - `mode: "allow"` maps onto `permissiveLearningMode` (allow-and-record) diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs b/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs index e31d8e44d..464bde7ad 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs @@ -228,7 +228,11 @@ private SandboxWaitResult WaitCore(CancellationToken cancellationToken) } if (running == 0) { - return new SandboxWaitResult { ExitCode = exit, TimedOut = false }; + // try_wait is intentionally a non-blocking root-process poll. + // Complete the native wait so descendant termination and any + // backend finalization (including captureDenials) finish before + // the managed Wait/WaitAsync contract returns. + return WaitBlocking(); } // try_wait only reports exited / still-running, never a timeout, and diff --git a/src/Cargo.lock b/src/Cargo.lock index 831cbfbac..82b872c1a 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -88,6 +88,7 @@ version = "0.7.0" dependencies = [ "flatbuffers", "getrandom 0.2.17", + "learning_mode_windows", "sandbox_spec", "serde", "serde_json", diff --git a/src/backends/appcontainer/common/Cargo.toml b/src/backends/appcontainer/common/Cargo.toml index f79e9fec4..7e3d3ccf6 100644 --- a/src/backends/appcontainer/common/Cargo.toml +++ b/src/backends/appcontainer/common/Cargo.toml @@ -26,6 +26,7 @@ flatbuffers = { workspace = true } sandbox_spec = { workspace = true } widestring = { workspace = true } winreg = { workspace = true } +learning_mode_windows = { workspace = true } [dev-dependencies] tempfile.workspace = true diff --git a/src/backends/appcontainer/common/src/appcontainer_runner.rs b/src/backends/appcontainer/common/src/appcontainer_runner.rs index 6149b301a..1539c830e 100644 --- a/src/backends/appcontainer/common/src/appcontainer_runner.rs +++ b/src/backends/appcontainer/common/src/appcontainer_runner.rs @@ -34,7 +34,9 @@ use crate::job_object::UiJobObject; use crate::process_mitigation; use wxc_common::error::WxcError; use wxc_common::logger::Logger; -use wxc_common::models::{ExecutionRequest, NetworkEnforcementMode, NetworkPolicy, ScriptResponse}; +use wxc_common::models::{ + ExecutionRequest, FailurePhase, NetworkEnforcementMode, NetworkPolicy, ScriptResponse, +}; use wxc_common::process_util::{ create_std_pipes, InterruptiblePipeReader, OwnedHandle, PipeReadCanceller, PipeWriter, SendOwnedHandle, SidAndAttributes, @@ -46,6 +48,9 @@ use wxc_common::sandbox_process::{ use wxc_common::script_runner::get_timeout_milliseconds; use wxc_common::{string_util, ui_policy}; +pub(crate) const CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG: &str = + "captureDenials requires the BaseContainer learning-mode APIs and is not supported by the AppContainer fallback tier"; + /// `UpdateProcThreadAttribute` value for /// `PROC_THREAD_ATTRIBUTE_ALL_APPLICATION_PACKAGES_POLICY` that opts the /// process out of inheriting `ALL APPLICATION PACKAGES` grants. This @@ -59,7 +64,7 @@ const PROXY_VAR_NAMES: &[&str] = &["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "ALL /// Serialize `KEY=VALUE` pairs into a double-null-terminated UTF-16 environment block. /// /// Entries are sorted case-insensitively by key as required by `CreateProcessW`. -fn encode_env_block(entries: &[(String, String)]) -> Vec { +pub(crate) fn encode_env_block(entries: &[(String, String)]) -> Vec { let mut sorted: Vec<&(String, String)> = entries.iter().collect(); sorted.sort_by(|(a, _), (b, _)| a.to_ascii_uppercase().cmp(&b.to_ascii_uppercase())); @@ -80,7 +85,7 @@ fn encode_env_block(entries: &[(String, String)]) -> Vec { /// Calls `CreateEnvironmentBlock` with `bInherit = FALSE` so that only the /// system/user profile variables are included (no process-level vars leak in). /// Returns the entries as `(key, value)` pairs. -fn create_default_env_entries() -> Result, WxcError> { +pub(crate) fn create_default_env_entries() -> Result, WxcError> { unsafe { let mut token = HANDLE::default(); OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) @@ -1303,6 +1308,12 @@ impl AppContainerScriptRunner { impl SandboxBackend for AppContainerScriptRunner { fn validate(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { + if request.policy.capture_denials.is_some() { + return Err(ScriptResponse { + failure_phase: FailurePhase::BackendUnavailable, + ..ScriptResponse::error(CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG) + }); + } if !request.policy.denied_paths.is_empty() && self.filesystem_mode != FilesystemMode::Dacl { return Err(ScriptResponse::error( wxc_common::error::DENIED_PATHS_NOT_SUPPORTED_MSG, @@ -1823,8 +1834,10 @@ mod tests { // ---- validate_runner: unsupported policy fields surface as errors. ---- - use super::{AppContainerScriptRunner, FilesystemMode}; - use wxc_common::models::ExecutionRequest; + use super::{ + AppContainerScriptRunner, FilesystemMode, CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG, + }; + use wxc_common::models::{ExecutionRequest, FailurePhase}; use wxc_common::sandbox_process::SandboxBackend; #[test] @@ -1885,4 +1898,20 @@ mod tests { let request = ExecutionRequest::default(); assert!(runner.validate(&request).is_ok()); } + + #[test] + fn validate_runner_rejects_capture_denials_on_appcontainer_fallback() { + let runner = AppContainerScriptRunner::new(); + let mut request = ExecutionRequest::default(); + request.policy.capture_denials = Some(Default::default()); + + let error = runner + .validate(&request) + .expect_err("AppContainer fallback must not silently ignore captureDenials"); + assert_eq!(error.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!( + error.error_message, + CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG + ); + } } diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index e80c5fe52..4300574db 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -12,11 +12,18 @@ use std::ffi::c_void; use std::fmt::Write; use std::io::IsTerminal; +use std::path::PathBuf; use std::ptr; +use std::sync::Arc; + +use learning_mode_windows::{ + CaptureSession, LearningModeApi, SecurityEnvironmentApi, SecurityEnvironmentStartupInfo, + PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, +}; use windows::Win32::Foundation::{ - CloseHandle, GetLastError, SetHandleInformation, ERROR_CALL_NOT_IMPLEMENTED, E_NOTIMPL, HANDLE, - HANDLE_FLAG_INHERIT, WAIT_OBJECT_0, WAIT_TIMEOUT, + CloseHandle, GetLastError, SetHandleInformation, ERROR_CALL_NOT_IMPLEMENTED, + ERROR_NOT_SUPPORTED, E_NOTIMPL, HANDLE, HANDLE_FLAG_INHERIT, WAIT_OBJECT_0, WAIT_TIMEOUT, }; use windows::Win32::System::Console::{ GetStdHandle, STD_ERROR_HANDLE, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, @@ -25,10 +32,11 @@ use windows::Win32::System::LibraryLoader::{ GetProcAddress, LoadLibraryExW, LOAD_LIBRARY_SEARCH_SYSTEM32, }; use windows::Win32::System::Threading::{ - GetExitCodeProcess, TerminateProcess, WaitForSingleObject, PROCESS_INFORMATION, + CreateProcessW, GetExitCodeProcess, TerminateProcess, WaitForSingleObject, + EXTENDED_STARTUPINFO_PRESENT, PROCESS_CREATION_FLAGS, PROCESS_INFORMATION, STARTF_USESTDHANDLES, STARTUPINFOW, }; -use windows_core::PCWSTR; +use windows_core::{PCWSTR, PWSTR}; use crate::job_object::UiJobObject; use crate::launch_diagnostics::{ @@ -107,6 +115,83 @@ type PfnCreateProcessInSandbox = unsafe extern "system" fn( /// ambiguous and must not be read as "sandbox unsupported". type PfnQuerySandboxSupport = unsafe extern "system" fn(capabilities: *mut u64) -> i32; +struct SandboxLaunchArgs<'a> { + api: PfnCreateProcessInSandbox, + command_line: &'a mut [u16], + current_directory: *const u16, + startup_info: &'a STARTUPINFOW, + identity: &'a [u16], + sandbox_specification: &'a [u8], + no_window_flag: u32, +} + +impl SandboxLaunchArgs<'_> { + /// Launches through the one-shot API, retrying once without the + /// environment block on downlevel systems that reject that parameter. + fn launch_with_environment_fallback( + &mut self, + creation_flags: u32, + environment: *const c_void, + process_information: &mut PROCESS_INFORMATION, + logger: &mut Logger, + ) -> (i32, Option) { + let mut current_creation_flags = creation_flags; + let mut current_environment = environment; + let mut retries_remaining = 1; + + loop { + *process_information = unsafe { std::mem::zeroed() }; + let result = unsafe { + (self.api)( + ptr::null(), + self.command_line.as_mut_ptr(), + ptr::null(), + ptr::null(), + i32::from(false), + current_creation_flags, + current_environment, + self.current_directory, + self.startup_info, + self.identity.as_ptr(), + self.sandbox_specification.as_ptr(), + self.sandbox_specification.len() as u32, + process_information, + ) + }; + if result != 0 { + return (result, None); + } + + let error = unsafe { GetLastError() }; + if retries_remaining == 0 + || !is_environment_not_supported(error.0, !current_environment.is_null()) + { + return (result, Some(error)); + } + retries_remaining -= 1; + + unsafe { + if !process_information.hProcess.is_invalid() { + let _ = CloseHandle(process_information.hProcess); + } + if !process_information.hThread.is_invalid() { + let _ = CloseHandle(process_information.hThread); + } + } + + let diagnostic = diagnose_environment_not_supported(); + let _ = writeln!( + logger, + "{EMOJI_WARNING} Launch diagnostic [{}]: {}", + diagnostic.kind, diagnostic.message + ); + + current_environment = ptr::null(); + current_creation_flags = CREATE_SUSPENDED.0 | self.no_window_flag; + } + } +} + /// `SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX`: when clear, Tier 1 is unusable. const SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX: u64 = 0x0000_0000_0000_0001; @@ -116,6 +201,14 @@ const SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX: u64 = 0x0000_0000_0000_0001; /// assumed bit 1). When clear, `deniedPaths` is rejected at launch and callers /// must rely on default-deny plus explicit `readwrite`/`readonly` grants. const SANDBOX_CAP_FS_DENY: u64 = 0x0000_0000_0000_0002; +const CAPTURE_API_AVAILABLE_LOG: &str = + "captureDenials: learning-mode trace API available (processmodel.dll)"; +const CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON: &str = + "capture teardown failed and the process security environment may still be live"; +const CLOSE_PROCESS_SECURITY_ENVIRONMENT_API: &str = "CloseProcessSecurityEnvironment"; +const CREATE_PROCESS_IN_SANDBOX_API: &str = "Experimental_CreateProcessInSandbox"; +const CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API: &str = + "CreateProcessW(PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT)"; /// True when a Win32 error code signals the BaseContainer feature is not /// enabled on this build (symbol present, capability gated off). @@ -123,11 +216,108 @@ fn is_api_not_implemented(err: u32) -> bool { err == ERROR_CALL_NOT_IMPLEMENTED.0 || err == E_NOTIMPL.0 as u32 } +fn learning_mode_api_not_implemented(error: &learning_mode_windows::LearningModeError) -> bool { + match error { + learning_mode_windows::LearningModeError::DllLoad(_) + | learning_mode_windows::LearningModeError::ExportMissing { .. } => true, + learning_mode_windows::LearningModeError::ApiCall { code, .. } => { + is_api_not_implemented(*code) || *code == ERROR_NOT_SUPPORTED.0 + } + learning_mode_windows::LearningModeError::CleanupFailed { primary, .. } => { + learning_mode_api_not_implemented(primary) + } + _ => false, + } +} + +fn learning_mode_cleanup_failed(error: &learning_mode_windows::LearningModeError) -> bool { + match error { + learning_mode_windows::LearningModeError::ApiCall { function, .. } => { + *function == CLOSE_PROCESS_SECURITY_ENVIRONMENT_API + } + learning_mode_windows::LearningModeError::CleanupFailed { primary, cleanup } => { + learning_mode_cleanup_failed(primary) || learning_mode_cleanup_failed(cleanup) + } + _ => false, + } +} + +fn combine_capture_operation_and_cleanup_errors( + primary: learning_mode_windows::LearningModeError, + cleanup: Result<(), learning_mode_windows::LearningModeError>, +) -> learning_mode_windows::LearningModeError { + match cleanup { + Ok(()) => primary, + Err(cleanup) => learning_mode_windows::LearningModeError::CleanupFailed { + primary: Box::new(primary), + cleanup: Box::new(cleanup), + }, + } +} + +trait CaptureSessionOps { + fn environment(&self) -> HANDLE; + fn finish( + self: Box, + output_path: Option<&std::path::Path>, + ) -> Result<(), learning_mode_windows::LearningModeError>; +} + +impl CaptureSessionOps for CaptureSession { + fn environment(&self) -> HANDLE { + self.environment() + } + + fn finish( + self: Box, + output_path: Option<&std::path::Path>, + ) -> Result<(), learning_mode_windows::LearningModeError> { + (*self).finish(output_path) + } +} + +trait CaptureSessionFactory: Send + Sync { + fn begin( + &self, + sandbox_specification: &[u8], + flags: u32, + ) -> Result, learning_mode_windows::LearningModeError>; +} + +struct RealCaptureSessionFactory; + +impl CaptureSessionFactory for RealCaptureSessionFactory { + fn begin( + &self, + sandbox_specification: &[u8], + flags: u32, + ) -> Result, learning_mode_windows::LearningModeError> { + let security_environment_api = SecurityEnvironmentApi::load()?; + let learning_mode_api = LearningModeApi::load()?; + CaptureSession::begin( + security_environment_api, + learning_mode_api, + sandbox_specification, + flags, + ) + .map(|session| Box::new(session) as Box) + } +} + /// Script runner that uses `Experimental_CreateProcessInSandbox` API /// to launch a sandboxed process. -#[derive(Default)] pub struct BaseContainerRunner { proxy_coordinator: ProxyCoordinator, + capture_factory: Arc, +} + +impl Default for BaseContainerRunner { + fn default() -> Self { + Self { + proxy_coordinator: ProxyCoordinator::default(), + capture_factory: Arc::new(RealCaptureSessionFactory), + } + } } /// SandboxSpec FlatBuffer schema version embedded in every spec payload. @@ -155,6 +345,42 @@ impl BaseContainerRunner { Self::default() } + #[cfg(test)] + fn with_capture_factory(capture_factory: Arc) -> Self { + Self { + proxy_coordinator: ProxyCoordinator::default(), + capture_factory, + } + } + + fn cleanup_capture_prelaunch_failure( + &mut self, + cleanup_error: Option<&learning_mode_windows::LearningModeError>, + request: &ExecutionRequest, + sid_string: &str, + logger: &mut Logger, + ) { + // This cannot be deferred to BaseContainerRunner::drop: the runner may + // outlive a failed spawn, and the exact error determines whether + // profile deletion is safe or recovery tracking must be retained. + if request.lifecycle.destroy_on_exit { + if cleanup_error.is_some_and(learning_mode_cleanup_failed) { + sandbox_tracking::mark_cleanup_deferred( + sid_string, + CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON, + logger, + ); + } else { + // CaptureSession::begin receives the sandbox specification, + // not the later process identity. Once its environment is + // closed, only the pre-launch tracking entry needs removal. + sandbox_tracking::remove_tracking_entry(sid_string, logger); + } + sandbox_tracking::unregister_ctrl_c_cleanup(); + } + self.proxy_coordinator.stop(logger); + } + /// Pre-flight probe: check whether the current OS build exports the /// `Experimental_CreateProcessInSandbox` symbol from `processmodel.dll`. /// @@ -661,6 +887,25 @@ impl BaseContainerRunner { Self::log_sandbox_spec(&spec_bytes, logger); + let capture_denials = request.policy.capture_denials.clone(); + if capture_denials.is_some() { + let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: captureDenials"); + } + + // Resolve the ETL output path: caller-specified when provided, else a + // managed per-run temp file the runner names (parsed / cleaned up by the + // consume + analyze stages). + let capture_output_path: Option = capture_denials + .as_ref() + .map(|cd| { + cd.output_path + .clone() + .map(PathBuf::from) + .map(Ok) + .unwrap_or_else(managed_capture_output_path) + }) + .transpose()?; + let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: Load API"); // 2. Dynamically load the API from processmodel.dll. @@ -893,13 +1138,22 @@ impl BaseContainerRunner { let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; // Environment block for the sandboxed child. - // If the caller specified explicit env vars, use only those. - // Otherwise, pass NULL to let the OS provide the default environment - // for the sandbox (CreateProcessInSandbox handles this internally). + // Explicit variables are always isolated from the parent environment. + // The one-shot API supplies its own default when this is NULL, but the + // attribute-based CreateProcessW capture path must receive an explicit + // clean block or it would inherit all wxc-exec process variables. let env_block: Option> = if request.env.is_empty() { - // TODO: consider calling CreateEnvironmentBlock(NULL, FALSE) here - // for a cleansed default env if the OS API doesn't do it for us. - None + if capture_denials.is_some() { + let entries = + crate::appcontainer_runner::create_default_env_entries().map_err(|error| { + ScriptResponse::error(&format!( + "captureDenials failed to create a clean child environment: {error}" + )) + })?; + Some(crate::appcontainer_runner::encode_env_block(&entries)) + } else { + None + } } else { Some(encode_env_block(&request.env)) }; @@ -957,78 +1211,143 @@ impl BaseContainerRunner { // If the OS returns ERROR_NOT_SUPPORTED (0x32) and we passed a non-null // environment block, this is a downlevel build that doesn't support the // `environment` parameter. Retry once without it. - let mut current_env_ptr = env_ptr; - let mut current_creation_flags = creation_flags; - let mut retries_remaining: u32 = 1; + let current_env_ptr = env_ptr; + let current_creation_flags = creation_flags; + + // When captureDenials is active, launch inside a process security + // environment that already has a learning-mode trace started against it, + // instead of the one-shot CreateProcessInSandbox. `begin` creates the + // environment and starts the trace *before* the child launches (so no + // early denials are missed); the environment handle is attached to a + // normal CreateProcessW call via PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT. + // On any early return below, `capture_session` drops and its Drop + // discards the trace and closes the environment (no broker leak). + let mut capture_session: Option> = None; + if capture_denials.is_some() { + match self + .capture_factory + .begin(&spec_bytes, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE) + { + Ok(session) => { + let _ = writeln!( + logger, + "{CAPTURE_API_AVAILABLE_LOG}; security environment and trace started" + ); + capture_session = Some(session); + } + Err(e) => { + let msg = format!("captureDenials: failed to start learning-mode capture: {e}"); + let _ = writeln!(logger, "Error: {msg}"); + let failure_phase = if learning_mode_api_not_implemented(&e) { + FailurePhase::BackendUnavailable + } else { + FailurePhase::LaunchFailed + }; + self.cleanup_capture_prelaunch_failure(Some(&e), &request, &sid_string, logger); + return Err(ScriptResponse { + exit_code: -1, + error_message: msg.clone(), + standard_err: msg, + failure_phase, + ..Default::default() + }); + } + } + } - // The loop yields (api_return_code, last_win32_error_on_failure). - let (success, last_error) = loop { + // The launch yields (api_return_code, last_win32_error_on_failure). + let (success, last_error, launch_api_name) = if let Some(session) = capture_session.as_ref() + { + // Single-attempt in-environment launch. The learning-mode security + // environment is attached as a process-thread attribute; the + // CreateProcessInSandbox environment fallback does not apply here. pi = unsafe { std::mem::zeroed() }; - + let inherited_handles = if pipe_mode { + vec![h_stdin, h_stdout, h_stderr] + } else { + Vec::new() + }; + let extended_startup = match SecurityEnvironmentStartupInfo::new( + si, + session.environment(), + &inherited_handles, + ) { + Ok(startup) => startup, + Err(primary) => { + let cleanup = capture_session + .take() + .map(|session| session.finish(None)) + .unwrap_or(Ok(())); + let error = combine_capture_operation_and_cleanup_errors(primary, cleanup); + let msg = format!( + "captureDenials: failed to attach the process security environment: {error}" + ); + let _ = writeln!(logger, "Error: {msg}"); + let failure_phase = if learning_mode_api_not_implemented(&error) { + FailurePhase::BackendUnavailable + } else { + FailurePhase::LaunchFailed + }; + self.cleanup_capture_prelaunch_failure( + Some(&error), + &request, + &sid_string, + logger, + ); + return Err(ScriptResponse { + exit_code: -1, + error_message: msg.clone(), + standard_err: msg, + failure_phase, + ..Default::default() + }); + } + }; + let environment = (!current_env_ptr.is_null()).then_some(current_env_ptr); let result = unsafe { - create_process_in_sandbox( - ptr::null(), // applicationName (resolved from commandLine) - cmd_wide.as_mut_ptr(), // commandLine - ptr::null(), // processAttributes (must be NULL) - ptr::null(), // threadAttributes (must be NULL) - // inheritHandles: must be FALSE per the OS sandbox API contract. - // Unlike regular CreateProcess, CreateProcessInSandbox treats the - // explicit STDIO handles in STARTUPINFO (hStdInput/hStdOutput/hStdError) - // as inheritable when STARTF_USESTDHANDLES is set, but does not support - // general handle inheritance. - i32::from(false), // inheritHandles - current_creation_flags, // creationFlags - current_env_ptr, // environment - cwd_ptr, // currentDirectory - &si, // startupInfo - identity_wide.as_ptr(), // identity - spec_bytes.as_ptr(), // sandboxSpecification - spec_bytes.len() as u32, // sandboxSpecificationSize - &mut pi, // processInformation + CreateProcessW( + PCWSTR::null(), + Some(PWSTR(cmd_wide.as_mut_ptr())), + None, + None, + !inherited_handles.is_empty(), + PROCESS_CREATION_FLAGS(current_creation_flags | EXTENDED_STARTUPINFO_PRESENT.0), + environment, + PCWSTR(cwd_ptr), + &extended_startup.startup_info().StartupInfo, + &mut pi, ) }; - - if result != 0 { - break (result, None); + if result.is_ok() { + (1, None, CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API) + } else { + ( + 0, + Some(unsafe { GetLastError() }), + CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API, + ) } - - // Call failed -- capture the error before any handle cleanup. - let err = unsafe { GetLastError() }; - - if retries_remaining > 0 - && is_environment_not_supported(err.0, !current_env_ptr.is_null()) - { - retries_remaining -= 1; - - // Clean up handles from the failed attempt. - unsafe { - if !pi.hProcess.is_invalid() { - let _ = CloseHandle(pi.hProcess); - } - if !pi.hThread.is_invalid() { - let _ = CloseHandle(pi.hThread); - } - } - - let diag = diagnose_environment_not_supported(); - let _ = writeln!( - logger, - "{EMOJI_WARNING} Launch diagnostic [{}]: {}", - diag.kind, diag.message - ); - - // Retry without the environment block, but keep the child - // suspended (resumed after job assignment). - current_env_ptr = ptr::null(); - current_creation_flags = CREATE_SUSPENDED.0 | no_window_flag; - continue; + } else { + let (success, error) = SandboxLaunchArgs { + api: create_process_in_sandbox, + command_line: &mut cmd_wide, + current_directory: cwd_ptr, + startup_info: &si, + identity: &identity_wide, + sandbox_specification: &spec_bytes, + no_window_flag, } - - // Non-retryable failure. - break (result, Some(err)); + .launch_with_environment_fallback( + current_creation_flags, + current_env_ptr, + &mut pi, + logger, + ); + (success, error, CREATE_PROCESS_IN_SANDBOX_API) }; if success == 0 { + let err = last_error.unwrap_or_else(|| unsafe { GetLastError() }); // Clean up any partially-populated handles from the failed API call. unsafe { if !pi.hProcess.is_invalid() { @@ -1038,9 +1357,19 @@ impl BaseContainerRunner { let _ = CloseHandle(pi.hThread); } } - // The OS may have created the AppContainer profile before failing, - // so run the same cleanup logic used on normal exit. - if request.lifecycle.destroy_on_exit { + let capture_cleanup_error = capture_session + .take() + .and_then(|session| session.finish(None).err()); + if capture_denials.is_some() { + self.cleanup_capture_prelaunch_failure( + capture_cleanup_error.as_ref(), + &request, + &sid_string, + logger, + ); + } else if request.lifecycle.destroy_on_exit { + // The OS may have created the AppContainer profile before + // failing, so run the same cleanup logic used on normal exit. run_sandbox_cleanup( &identity, &sid_string, @@ -1052,14 +1381,19 @@ impl BaseContainerRunner { // // Diagnose the launch failure (FailurePhase::LaunchFailed). // - let err = last_error.unwrap_or_else(|| unsafe { GetLastError() }); let diag = diagnose_create_process_failure( err.0, &request.script_code, &request.policy.readonly_paths, ); - let extended_error = format!("Experimental_CreateProcessInSandbox failed: {err:?}"); + let mut extended_error = format!("{launch_api_name} failed: {err:?}"); + if let Some(cleanup_error) = capture_cleanup_error { + let _ = write!( + extended_error, + "; capture teardown also failed: {cleanup_error}" + ); + } let _ = writeln!(logger, "Error: {extended_error}"); let _ = writeln!( @@ -1142,7 +1476,17 @@ impl BaseContainerRunner { let _ = CloseHandle(pi.hProcess); let _ = CloseHandle(pi.hThread); } - if request.lifecycle.destroy_on_exit { + let capture_cleanup_error = capture_session + .take() + .and_then(|session| session.finish(None).err()); + if capture_denials.is_some() { + self.cleanup_capture_prelaunch_failure( + capture_cleanup_error.as_ref(), + &request, + &sid_string, + logger, + ); + } else if request.lifecycle.destroy_on_exit { run_sandbox_cleanup( &identity, &sid_string, @@ -1151,17 +1495,26 @@ impl BaseContainerRunner { ); sandbox_tracking::unregister_ctrl_c_cleanup(); } - self.proxy_coordinator.stop(logger); + if capture_denials.is_none() { + self.proxy_coordinator.stop(logger); + } const JOB_SETUP_FAILED_MSG: &str = "BaseContainer sandbox could not be placed in a job object, so it \ could not be reliably terminated; the launch was rejected to \ avoid running an uncontainable sandbox."; + let mut extended_error = format!("BaseContainer job-object setup failed: {e}"); + if let Some(cleanup_error) = capture_cleanup_error { + let _ = write!( + extended_error, + "; capture teardown also failed: {cleanup_error}" + ); + } return Err(ScriptResponse { exit_code: -1, error_message: JOB_SETUP_FAILED_MSG.to_string(), standard_err: JOB_SETUP_FAILED_MSG.to_string(), - extended_error: format!("BaseContainer job-object setup failed: {e}"), + extended_error, failure_phase: FailurePhase::LaunchFailed, ..Default::default() }); @@ -1196,6 +1549,8 @@ impl BaseContainerRunner { identity, sid_string, proxy_coordinator: std::mem::take(&mut self.proxy_coordinator), + capture_session, + capture_output_path, }) } } @@ -1222,6 +1577,13 @@ struct BaseChild { identity: String, sid_string: String, proxy_coordinator: ProxyCoordinator, + /// Live learning-mode capture session (`Some` only when `captureDenials` + /// is configured and the OS API is available). Sealed in `run_teardown` + /// after the child exits. + capture_session: Option>, + /// Resolved ETL output path for the capture session (caller-specified or a + /// managed per-run temp file). `Some` iff `capture_session` is `Some`. + capture_output_path: Option, } impl SandboxBackend for BaseContainerRunner { @@ -1307,7 +1669,14 @@ struct BaseContainerSandboxProcess { identity: String, sid_string: String, proxy_coordinator: ProxyCoordinator, - teardown_done: bool, + /// Cached teardown outcome so repeated terminal waits cannot hide a + /// capture failure after the session has been consumed. + teardown_result: Option>, + /// Live learning-mode capture session, moved from the `BaseChild`. Sealed + /// in `run_teardown` once the child has exited and been reaped. + capture_session: Option>, + /// Resolved ETL output path for the capture session. + capture_output_path: Option, } // SAFETY: the fields are Windows HANDLEs / handle-owning managers and owned @@ -1341,26 +1710,84 @@ impl BaseContainerSandboxProcess { identity: std::mem::take(&mut child.identity), sid_string: std::mem::take(&mut child.sid_string), proxy_coordinator: std::mem::take(&mut child.proxy_coordinator), - teardown_done: false, + teardown_result: None, + capture_session: child.capture_session.take(), + capture_output_path: child.capture_output_path.take(), } } - fn run_teardown(&mut self) { - if self.teardown_done { - return; + fn run_teardown(&mut self) -> std::io::Result<()> { + if let Some(result) = &self.teardown_result { + return result.clone().map_err(std::io::Error::other); } - self.teardown_done = true; let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); + + // Seal the learning-mode ETL trace now that the child has exited and + // been reaped (both `wait` and `Drop` kill + reap before calling this). + // `finish` stops the trace to the resolved output path, then closes the + // security environment. The output-file path is surfaced on stderr so a + // caller can locate the denials (the full NDJSON contract is finalized + // in the output/consume stage). + let mut defer_capture_cleanup = false; + let capture_result = if let Some(session) = self.capture_session.take() { + let output_path = self.capture_output_path.take(); + match session.finish(output_path.as_deref()) { + Ok(()) => { + if let Some(path) = &output_path { + eprintln!("captureDenials: denials ETL written to {}", path.display()); + } + Ok(()) + } + Err(error) => { + defer_capture_cleanup = learning_mode_cleanup_failed(&error); + Err(std::io::Error::other(format!( + "captureDenials failed to finalize the denial capture: {error}" + ))) + } + } + } else { + Ok(()) + }; + if self.destroy_on_exit { - run_sandbox_cleanup( - &self.identity, - &self.sid_string, - self.proxy_enabled, - &mut logger, - ); + if defer_capture_cleanup { + sandbox_tracking::mark_cleanup_deferred( + &self.sid_string, + CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON, + &mut logger, + ); + } else { + run_sandbox_cleanup( + &self.identity, + &self.sid_string, + self.proxy_enabled, + &mut logger, + ); + } sandbox_tracking::unregister_ctrl_c_cleanup(); } self.proxy_coordinator.stop(&mut logger); + let result = capture_result.map_err(|error| error.to_string()); + self.teardown_result = Some(result.clone()); + result.map_err(std::io::Error::other) + } + + fn kill_process_tree(&mut self) -> std::io::Result<()> { + if let Some(job) = &self.job { + job.terminate(u32::MAX); + } else { + unsafe { + let _ = TerminateProcess(self.process.get(), u32::MAX); + } + } + Ok(()) + } + + fn terminate_and_reap(&mut self) { + let _ = self.kill_process_tree(); + unsafe { + let _ = WaitForSingleObject(self.process.get(), u32::MAX); + } } } @@ -1392,6 +1819,9 @@ impl SandboxProcess for BaseContainerSandboxProcess { if unsafe { GetExitCodeProcess(self.process.get(), &mut code) }.is_err() { return Err(std::io::Error::other("GetExitCodeProcess failed")); } + // Keep polling non-blocking and independent of captureDenials. + // `wait()` or `Drop` owns descendant termination and capture + // finalization after the root exit becomes observable. Ok(Some(code as i32)) } WAIT_TIMEOUT => Ok(None), @@ -1406,14 +1836,7 @@ impl SandboxProcess for BaseContainerSandboxProcess { fn kill(&mut self) -> std::io::Result<()> { // Tree-kill via the job object when the child was successfully assigned // to one; otherwise fall back to terminating the root process. - if let Some(job) = &self.job { - job.terminate(u32::MAX); - } else { - unsafe { - let _ = TerminateProcess(self.process.get(), u32::MAX); - } - } - Ok(()) + self.kill_process_tree() } fn wait(&mut self) -> std::io::Result { @@ -1450,14 +1873,11 @@ impl SandboxProcess for BaseContainerSandboxProcess { // failure this also terminates it. Then reap the root before releasing // the pipe drains — and killing the tree closes the descendant's pipe // write-ends, so the drains can finish. - let _ = self.kill(); - unsafe { - let _ = WaitForSingleObject(self.process.get(), u32::MAX); - } + self.terminate_and_reap(); cancel_and_join_discard(stdout_thread, &self.stdout_canceller); cancel_and_join_discard(stderr_thread, &self.stderr_canceller); - self.run_teardown(); - result + let teardown_result = self.run_teardown(); + combine_process_and_teardown_results(result, teardown_result) } } @@ -1466,11 +1886,44 @@ impl Drop for BaseContainerSandboxProcess { // Kill and reap before tearing down proxy / sandbox state, so an // abandoned-but-running sandbox cannot outlive its enforcement (or // leak as an orphan). - let _ = self.kill(); - unsafe { - let _ = WaitForSingleObject(self.process.get(), u32::MAX); + self.terminate_and_reap(); + if let Err(error) = self.run_teardown() { + eprintln!("captureDenials teardown failed during drop: {error}"); + } + } +} + +fn managed_capture_output_path() -> Result { + let mut nonce = [0u8; 16]; + getrandom::getrandom(&mut nonce).map_err(|error| { + ScriptResponse::error(&format!( + "captureDenials could not generate a unique temporary output path: {error}" + )) + })?; + let suffix = nonce + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + Ok(std::env::temp_dir().join(format!( + "mxc_capture_denials_{}_{suffix}.etl", + std::process::id() + ))) +} + +fn combine_process_and_teardown_results( + process_result: std::io::Result, + teardown_result: std::io::Result<()>, +) -> std::io::Result { + match (process_result, teardown_result) { + (Ok(exit_code), Ok(())) => Ok(exit_code), + (Ok(_), Err(teardown_error)) => Err(teardown_error), + (Err(wait_error), Ok(())) => Err(wait_error), + (Err(wait_error), Err(teardown_error)) => { + eprintln!( + "captureDenials teardown also failed after process wait failure: {teardown_error}" + ); + Err(wait_error) } - self.run_teardown(); } } @@ -1503,13 +1956,82 @@ mod tests { use super::*; use crate::job_object::to_job_object_uilimit_mask; use sandbox_spec::base_container_layout; + use std::sync::atomic::{AtomicUsize, Ordering}; use wxc_common::models::{ClipboardPolicy, ProxyConfig, UiPolicy}; use wxc_common::ui_policy::EffectiveUiRestrictions; + struct FakeCaptureSession { + finish_error: Option<(&'static str, u32)>, + finish_calls: Arc, + } + + impl CaptureSessionOps for FakeCaptureSession { + fn environment(&self) -> HANDLE { + HANDLE(std::ptr::dangling_mut()) + } + + fn finish( + self: Box, + _output_path: Option<&std::path::Path>, + ) -> Result<(), learning_mode_windows::LearningModeError> { + self.finish_calls.fetch_add(1, Ordering::SeqCst); + match self.finish_error { + Some((function, code)) => { + Err(learning_mode_windows::LearningModeError::ApiCall { function, code }) + } + None => Ok(()), + } + } + } + + struct FakeCaptureFactory { + begin_error: Option<(&'static str, u32)>, + finish_error: Option<(&'static str, u32)>, + begin_calls: AtomicUsize, + finish_calls: Arc, + } + + impl CaptureSessionFactory for FakeCaptureFactory { + fn begin( + &self, + _sandbox_specification: &[u8], + _flags: u32, + ) -> Result, learning_mode_windows::LearningModeError> { + self.begin_calls.fetch_add(1, Ordering::SeqCst); + if let Some((function, code)) = self.begin_error { + return Err(learning_mode_windows::LearningModeError::ApiCall { function, code }); + } + Ok(Box::new(FakeCaptureSession { + finish_error: self.finish_error, + finish_calls: Arc::clone(&self.finish_calls), + })) + } + } + fn expected_mask(r: EffectiveUiRestrictions) -> u64 { to_job_object_uilimit_mask(&r) as u64 } + #[test] + fn managed_capture_paths_are_unique_per_run() { + let first = managed_capture_output_path().expect("first path"); + let second = managed_capture_output_path().expect("second path"); + + assert_ne!(first, second); + assert_eq!(first.parent(), Some(std::env::temp_dir().as_path())); + assert_eq!(second.parent(), Some(std::env::temp_dir().as_path())); + assert_eq!(first.extension().and_then(|ext| ext.to_str()), Some("etl")); + } + + #[test] + fn successful_process_reports_capture_teardown_failure() { + let error = + combine_process_and_teardown_results(Ok(0), Err(std::io::Error::other("seal failed"))) + .expect_err("capture failure must override successful process exit"); + + assert!(error.to_string().contains("seal failed")); + } + #[test] fn is_api_not_implemented_classifies_disabled_feature() { assert!(is_api_not_implemented(ERROR_CALL_NOT_IMPLEMENTED.0)); @@ -1519,6 +2041,136 @@ mod tests { assert!(!is_api_not_implemented(0)); } + #[test] + fn learning_mode_api_not_implemented_checks_primary_failure() { + use learning_mode_windows::LearningModeError; + + let disabled = LearningModeError::CleanupFailed { + primary: Box::new(LearningModeError::ApiCall { + function: "CreateProcessSecurityEnvironment", + code: ERROR_CALL_NOT_IMPLEMENTED.0, + }), + cleanup: Box::new(LearningModeError::ApiCall { + function: "CloseProcessSecurityEnvironment", + code: 87, + }), + }; + assert!(learning_mode_api_not_implemented(&disabled)); + + let ordinary = LearningModeError::ApiCall { + function: "StartLearningModeTrace", + code: 87, + }; + assert!(!learning_mode_api_not_implemented(&ordinary)); + + assert!(learning_mode_api_not_implemented( + &LearningModeError::ExportMissing { + api: "Learning Mode trace", + export: "StartLearningModeTrace", + detail: "not found".to_string(), + } + )); + assert!(learning_mode_api_not_implemented( + &LearningModeError::DllLoad("missing processmodel.dll".to_string()) + )); + } + + #[test] + fn learning_mode_cleanup_failure_is_detected() { + use learning_mode_windows::LearningModeError; + + let error = LearningModeError::CleanupFailed { + primary: Box::new(LearningModeError::ApiCall { + function: "StartLearningModeTrace", + code: 87, + }), + cleanup: Box::new(LearningModeError::ApiCall { + function: "CloseProcessSecurityEnvironment", + code: 5, + }), + }; + assert!(learning_mode_cleanup_failed(&error)); + + let close_only = LearningModeError::ApiCall { + function: CLOSE_PROCESS_SECURITY_ENVIRONMENT_API, + code: 5, + }; + assert!(learning_mode_cleanup_failed(&close_only)); + + let ordinary = LearningModeError::ApiCall { + function: "StartLearningModeTrace", + code: 87, + }; + assert!(!learning_mode_cleanup_failed(&ordinary)); + } + + #[test] + fn prelaunch_failure_preserves_capture_cleanup_error() { + use learning_mode_windows::LearningModeError; + + let error = combine_capture_operation_and_cleanup_errors( + LearningModeError::ApiCall { + function: "UpdateProcThreadAttribute(SecurityEnvironment)", + code: 87, + }, + Err(LearningModeError::ApiCall { + function: CLOSE_PROCESS_SECURITY_ENVIRONMENT_API, + code: 5, + }), + ); + + assert!(matches!(error, LearningModeError::CleanupFailed { .. })); + assert!(learning_mode_cleanup_failed(&error)); + assert!(error.to_string().contains("UpdateProcThreadAttribute")); + assert!(error + .to_string() + .contains(CLOSE_PROCESS_SECURITY_ENVIRONMENT_API)); + } + + #[test] + fn capture_factory_injects_begin_failure() { + let factory = Arc::new(FakeCaptureFactory { + begin_error: Some(("StartLearningModeTrace", 5)), + finish_error: None, + begin_calls: AtomicUsize::new(0), + finish_calls: Arc::new(AtomicUsize::new(0)), + }); + let runner = BaseContainerRunner::with_capture_factory(factory.clone()); + + let error = match runner + .capture_factory + .begin(&[], PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE) + { + Ok(_) => panic!("fake begin must fail"), + Err(error) => error, + }; + + assert!(error.to_string().contains("StartLearningModeTrace")); + assert_eq!(factory.begin_calls.load(Ordering::SeqCst), 1); + assert_eq!(factory.finish_calls.load(Ordering::SeqCst), 0); + } + + #[test] + fn capture_factory_injects_finish_failure_once() { + let factory = Arc::new(FakeCaptureFactory { + begin_error: None, + finish_error: Some((CLOSE_PROCESS_SECURITY_ENVIRONMENT_API, 5)), + begin_calls: AtomicUsize::new(0), + finish_calls: Arc::new(AtomicUsize::new(0)), + }); + let runner = BaseContainerRunner::with_capture_factory(factory.clone()); + let session = runner + .capture_factory + .begin(&[], PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE) + .expect("fake begin"); + + let error = session.finish(None).expect_err("fake finish must fail"); + + assert!(learning_mode_cleanup_failed(&error)); + assert_eq!(factory.begin_calls.load(Ordering::SeqCst), 1); + assert_eq!(factory.finish_calls.load(Ordering::SeqCst), 1); + } + #[test] fn decode_create_capability_table() { let cap = SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX; diff --git a/src/backends/appcontainer/common/src/dispatcher.rs b/src/backends/appcontainer/common/src/dispatcher.rs index 2db648327..24b9fda76 100644 --- a/src/backends/appcontainer/common/src/dispatcher.rs +++ b/src/backends/appcontainer/common/src/dispatcher.rs @@ -145,6 +145,9 @@ pub enum DispatchError { }, /// AppContainer SID derivation failed. Sid(WxcError), + /// captureDenials requires the native BaseContainer tier and cannot + /// proceed through an AppContainer fallback. + CaptureDenialsUnsupported { tier: IsolationTier }, } impl std::fmt::Display for DispatchError { @@ -171,6 +174,12 @@ impl std::fmt::Display for DispatchError { ), DispatchError::Dacl { error, .. } => write!(f, "Failed to apply DACL ACEs: {error}"), DispatchError::Sid(e) => write!(f, "Failed to derive AppContainer SID: {e}"), + DispatchError::CaptureDenialsUnsupported { tier } => write!( + f, + "captureDenials requires the native BaseContainer backend; \ + the selected fallback tier '{}' does not support denial capture.", + tier.as_str() + ), } } } @@ -330,6 +339,11 @@ fn select_backend_with_fallback( DispatchError, > { let decision = fallback_detector::detect(&request.policy, /*prefer_bc=*/ true)?; + if request.policy.capture_denials.is_some() && decision.tier != IsolationTier::BaseContainer { + return Err(DispatchError::CaptureDenialsUnsupported { + tier: decision.tier, + }); + } let (backend, dacl_manager): (SelectedBackend, Option) = match decision.tier { IsolationTier::BaseContainer => { @@ -673,6 +687,25 @@ mod tests { "T3 always requires DaclManager (grants applied)" ); } + + #[test] + fn capture_denials_rejects_fallback_before_backend_or_dacl_setup() { + let _g = ForceTierGuard::set("appcontainer-dacl"); + let (mut policy, _tmp) = policy_with_rw_temp(); + policy.capture_denials = Some(Default::default()); + let req = test_request(policy); + + let error = match dispatch_with_fallback(&req) { + Ok(_) => panic!("capture fallback must be rejected"), + Err(error) => error, + }; + assert!(matches!( + error, + DispatchError::CaptureDenialsUnsupported { + tier: IsolationTier::AppContainerDacl + } + )); + } #[test] fn dispatch_fallback_disabled_errors() { let _g = ForceTierGuard::set("appcontainer-dacl"); diff --git a/src/backends/appcontainer/common/src/sandbox_tracking.rs b/src/backends/appcontainer/common/src/sandbox_tracking.rs index ade2b3da3..6012dd02e 100644 --- a/src/backends/appcontainer/common/src/sandbox_tracking.rs +++ b/src/backends/appcontainer/common/src/sandbox_tracking.rs @@ -137,6 +137,13 @@ pub fn write_tracking_entry(entry: &TrackingEntry, logger: &mut Logger) -> Resul /// /// Adds a `CleanupDeferred` REG_SZ value to the existing tracking key. pub fn mark_cleanup_deferred(sid_string: &str, reason: &str, logger: &mut Logger) { + if sid_string.is_empty() { + let _ = writeln!( + logger, + "warning: cleanup could not be tracked because no AppContainer SID was derived" + ); + return; + } let key_path = format!("{}\\{}", TRACKING_BASE, sid_string); let hkcu = RegKey::predef(HKEY_CURRENT_USER); @@ -193,8 +200,20 @@ pub fn cleanup_sandbox(identity: &str, sid_string: &str, logger: &mut Logger) { delete_tracking_key(sid_string, logger); } +/// Removes the tracking entry for a sandbox that never launched. +pub fn remove_tracking_entry(sid_string: &str, logger: &mut Logger) { + delete_tracking_key(sid_string, logger); +} + /// Remove the registry tracking key and all its subkeys (including Active). fn delete_tracking_key(sid_string: &str, logger: &mut Logger) { + if sid_string.is_empty() { + let _ = writeln!( + logger, + "warning: refusing to delete sandbox tracking without an AppContainer SID" + ); + return; + } let key_path = format!("{}\\{}", TRACKING_BASE, sid_string); let hkcu = RegKey::predef(HKEY_CURRENT_USER); match hkcu.delete_subkey_all(&key_path) { @@ -351,4 +370,16 @@ mod tests { "unexpected SID: {sid_str}" ); } + + #[test] + fn empty_sid_never_targets_shared_tracking_root() { + let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); + + mark_cleanup_deferred("", "test", &mut logger); + remove_tracking_entry("", &mut logger); + + let output = logger.get_buffer(); + assert!(output.contains("no AppContainer SID was derived")); + assert!(output.contains("refusing to delete sandbox tracking")); + } } diff --git a/src/backends/learning_mode/windows/examples/lm_capture.rs b/src/backends/learning_mode/windows/examples/lm_capture.rs index c38a0b5b3..1a925481d 100644 --- a/src/backends/learning_mode/windows/examples/lm_capture.rs +++ b/src/backends/learning_mode/windows/examples/lm_capture.rs @@ -9,8 +9,9 @@ //! 1. build a minimal FlatBuffer sandbox spec with the `permissiveLearningMode` //! capability (the token the OS learning-mode path recognises), //! 2. [`CaptureSession::begin`] — create the security environment + start the trace, -//! 3. launch `cmd.exe` inside the environment via -//! `CreateProcessAsUserInsideSecurityEnvironment`, +//! 3. attach the environment handle through +//! `PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT` and launch `cmd.exe` with +//! `CreateProcessW`, //! 4. wait for it to exit, //! 5. [`CaptureSession::finish`] — seal the ETL to a temp path + close the environment, //! 6. assert the ETL file was produced (non-empty). @@ -41,7 +42,7 @@ mod windows_impl { use flatbuffers::FlatBufferBuilder; use learning_mode_windows::{ - CaptureSession, LearningModeApi, SecurityEnvironmentApi, + CaptureSession, LearningModeApi, SecurityEnvironmentApi, SecurityEnvironmentStartupInfo, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, }; use sandbox_spec::base_container_layout::{ @@ -49,8 +50,10 @@ mod windows_impl { }; use windows::Win32::Foundation::{CloseHandle, HANDLE, WAIT_FAILED, WAIT_OBJECT_0}; use windows::Win32::System::Threading::{ - GetExitCodeProcess, WaitForSingleObject, INFINITE, PROCESS_INFORMATION, STARTUPINFOW, + CreateProcessW, GetExitCodeProcess, WaitForSingleObject, EXTENDED_STARTUPINFO_PRESENT, + INFINITE, PROCESS_INFORMATION, STARTUPINFOW, }; + use windows_core::{PCWSTR, PWSTR}; /// Matches the schema version BaseContainer embeds in every spec payload. const SANDBOX_SPEC_VERSION: &str = "0.1.0"; @@ -118,7 +121,7 @@ mod windows_impl { }; println!("CaptureSession::begin OK — environment + trace live"); - let exit_code = match launch_and_wait(&secenv_api, session.environment()) { + let exit_code = match launch_and_wait(session.environment()) { Ok(code) => { println!("child exited with code {code}"); code @@ -161,11 +164,7 @@ mod windows_impl { /// Launch the child inside `environment` and wait for it to exit, returning its exit /// code. - fn launch_and_wait( - secenv_api: &SecurityEnvironmentApi, - environment: HANDLE, - ) -> Result { - let launch = secenv_api.launch_fn(); + fn launch_and_wait(environment: HANDLE) -> Result { let mut cmd = wide_command_line(); // SAFETY: a zeroed STARTUPINFOW with only `cb` set is valid; the child inherits @@ -173,33 +172,28 @@ mod windows_impl { let mut startup_info: STARTUPINFOW = unsafe { std::mem::zeroed() }; startup_info.cb = u32::try_from(std::mem::size_of::()) .map_err(|_| "STARTUPINFOW size overflow".to_string())?; + let extended_startup = SecurityEnvironmentStartupInfo::new(startup_info, environment, &[]) + .map_err(|error| error.to_string())?; let mut process_information: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; - // SAFETY: `launch` was resolved from processmodel.dll and matches the declared C - // signature. `cmd` is a mutable, null-terminated UTF-16 buffer; `startup_info` - // and `process_information` are valid; `environment` is the live handle from the - // session. `lpEnvironment` is null, so CREATE_UNICODE_ENVIRONMENT is not needed. - let ok = unsafe { - launch( - HANDLE(std::ptr::null_mut()), // userToken: caller context - std::ptr::null(), // applicationName (from command line) - cmd.as_mut_ptr(), // commandLine - 0, // creationFlags - std::ptr::null(), // environment - std::ptr::null(), // currentDirectory - &startup_info, - environment, + // SAFETY: `cmd` is mutable and null-terminated; `extended_startup` + // owns a valid attribute list containing the live environment handle; + // `process_information` is a valid output buffer. + unsafe { + CreateProcessW( + PCWSTR::null(), + Some(PWSTR(cmd.as_mut_ptr())), + None, + None, + false, + EXTENDED_STARTUPINFO_PRESENT, + None, + PCWSTR::null(), + &extended_startup.startup_info().StartupInfo, &mut process_information, ) - }; - if ok == 0 { - // SAFETY: reads the calling thread's last-error slot. - let err = unsafe { windows::Win32::Foundation::GetLastError() }; - return Err(format!( - "CreateProcessAsUserInsideSecurityEnvironment failed (GetLastError = {})", - err.0 - )); } + .map_err(|error| format!("CreateProcessW failed: {error}"))?; // SAFETY: `hProcess` is a valid process handle returned by the launch. let wait = unsafe { WaitForSingleObject(process_information.hProcess, INFINITE) }; diff --git a/src/backends/learning_mode/windows/examples/lm_probe.rs b/src/backends/learning_mode/windows/examples/lm_probe.rs index 2eadf5b0b..1dc3b8e93 100644 --- a/src/backends/learning_mode/windows/examples/lm_probe.rs +++ b/src/backends/learning_mode/windows/examples/lm_probe.rs @@ -6,7 +6,7 @@ //! Prints whether `processmodel.dll` on this machine exposes the Learning Mode trace //! exports (`StartLearningModeTrace` / `StopLearningModeTrace`) and the 2-phase //! security-environment exports (`CreateProcessSecurityEnvironment` / -//! `CreateProcessAsUserInsideSecurityEnvironment` / `CloseProcessSecurityEnvironment`), +//! `CloseProcessSecurityEnvironment`), //! reporting the exact resolved name for each (plain vs `Experimental_`). Intended to //! be run on a feature-enabled Windows build to confirm the runtime FFI resolves //! against the real API. @@ -34,7 +34,6 @@ fn run_probe() -> i32 { let report = learning_mode_windows::probe_security_environment_exports(); println!(" create export = {:?}", report.create); - println!(" launch export = {:?}", report.launch); println!(" close export = {:?}", report.close); match learning_mode_windows::SecurityEnvironmentApi::load() { diff --git a/src/backends/learning_mode/windows/src/lib.rs b/src/backends/learning_mode/windows/src/lib.rs index 8bf6c86d7..14f697b52 100644 --- a/src/backends/learning_mode/windows/src/lib.rs +++ b/src/backends/learning_mode/windows/src/lib.rs @@ -43,7 +43,7 @@ pub use lifecycle::CaptureSession; pub use secenv::{ is_security_environment_api_available, probe_security_environment_exports, ProcessSecurityEnvironment, SecurityEnvironmentApi, SecurityEnvironmentExportReport, - PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, + SecurityEnvironmentStartupInfo, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, }; /// Errors surfaced while loading or invoking the Learning Mode trace API. diff --git a/src/backends/learning_mode/windows/src/lifecycle.rs b/src/backends/learning_mode/windows/src/lifecycle.rs index fd0e7f9bb..b76e03369 100644 --- a/src/backends/learning_mode/windows/src/lifecycle.rs +++ b/src/backends/learning_mode/windows/src/lifecycle.rs @@ -10,8 +10,9 @@ //! 1. `CreateProcessSecurityEnvironment(spec)` → env handle //! 2. `StartLearningModeTrace(env)` → trace handle (**before** the child launches, so no //! early denials are missed) -//! 3. `CreateProcessAsUserInsideSecurityEnvironment(env, …)` → child (**runner's job**; -//! the session exposes the env handle for it via [`CaptureSession::environment`]) +//! 3. attach `env` as `PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT`, then call +//! `CreateProcessW` (**runner's job**; the session exposes the handle via +//! [`CaptureSession::environment`]) //! 4. wait for the child to exit //! 5. `StopLearningModeTrace(trace, outputPath)` → sealed ETL (NULL path discards) //! 6. `CloseProcessSecurityEnvironment(env)` → teardown @@ -91,7 +92,7 @@ impl CaptureSession { } /// The `HPROCESS_SECURITY_ENVIRONMENT` handle to pass to - /// `CreateProcessAsUserInsideSecurityEnvironment`. + /// [`crate::SecurityEnvironmentStartupInfo`]. /// /// # Panics /// Panics only on an internal invariant violation — the environment is present for diff --git a/src/backends/learning_mode/windows/src/secenv.rs b/src/backends/learning_mode/windows/src/secenv.rs index bf4533f2b..610a7ba8d 100644 --- a/src/backends/learning_mode/windows/src/secenv.rs +++ b/src/backends/learning_mode/windows/src/secenv.rs @@ -18,21 +18,15 @@ //! LPCVOID sandboxSpecification, DWORD sandboxSpecificationSize, //! PROCESS_SECURITY_ENVIRONMENT_FLAGS flags, //! HPROCESS_SECURITY_ENVIRONMENT* processSecurityEnvironment); -//! BOOL CreateProcessAsUserInsideSecurityEnvironment( -//! HANDLE userToken, LPCWSTR lpApplicationName, LPWSTR lpCommandLine, -//! DWORD dwCreationFlags, LPCVOID lpEnvironment, LPCWSTR lpCurrentDirectory, -//! LPSTARTUPINFOW lpStartupInfo, HANDLE processSecurityEnvironment, -//! LPPROCESS_INFORMATION lpProcessInformation); //! BOOL CloseProcessSecurityEnvironment(HPROCESS_SECURITY_ENVIRONMENT* processSecurityEnvironment); //! ``` //! //! `sandboxSpecification`/`...Size` is a compiled FlatBuffer sandbox-spec blob (the //! same `"SBOX"` format the BaseContainer runner already builds via `sandbox_spec`); -//! the spec must encode the learning-mode capability. Unlike the RPC one-shot, the -//! launch export returns a real `PROCESS_INFORMATION`, so wxc-exec owns the child -//! handle directly (stdio via `STARTUPINFOW`, wait, and job-object handling behave like -//! the classic path). `Close` tears the environment down; `Detach` (declared by the DLL -//! but not needed here) leaves the child running independently. +//! the spec must encode the learning-mode capability. The environment handle is +//! attached to a normal `CreateProcessW` launch through +//! `PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT`; KernelBase routes that launch +//! through the security environment internally. `Close` tears the environment down. //! //! As with the trace exports, each function is resolved at runtime and tolerates the //! `Experimental_`-prefixed name as a fallback for OS builds that predate the @@ -41,11 +35,14 @@ use std::ffi::c_void; use std::ptr; -use windows::Win32::Foundation::{GetLastError, HANDLE, HMODULE}; +use windows::Win32::Foundation::{GetLastError, ERROR_INSUFFICIENT_BUFFER, HANDLE, HMODULE}; use windows::Win32::System::LibraryLoader::{ GetProcAddress, LoadLibraryExW, LOAD_LIBRARY_SEARCH_SYSTEM32, }; -use windows::Win32::System::Threading::{PROCESS_INFORMATION, STARTUPINFOW}; +use windows::Win32::System::Threading::{ + DeleteProcThreadAttributeList, InitializeProcThreadAttributeList, UpdateProcThreadAttribute, + LPPROC_THREAD_ATTRIBUTE_LIST, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, STARTUPINFOEXW, STARTUPINFOW, +}; use windows_core::{PCSTR, PCWSTR}; use wxc_common::string_util; @@ -75,27 +72,6 @@ type PfnCreateProcessSecurityEnvironment = unsafe extern "system" fn( process_security_environment: *mut HANDLE, ) -> i32; -/// `BOOL CreateProcessAsUserInsideSecurityEnvironment(HANDLE userToken, -/// LPCWSTR lpApplicationName, LPWSTR lpCommandLine, DWORD dwCreationFlags, -/// LPCVOID lpEnvironment, LPCWSTR lpCurrentDirectory, LPSTARTUPINFOW lpStartupInfo, -/// HANDLE processSecurityEnvironment, LPPROCESS_INFORMATION lpProcessInformation)`. -/// -/// `userToken`/`lpApplicationName`/`lpCommandLine`/`lpEnvironment`/`lpCurrentDirectory` -/// are optional; `lpStartupInfo`/`processSecurityEnvironment`/`lpProcessInformation` are -/// required. When `lpEnvironment` is non-null, `dwCreationFlags` must include -/// `CREATE_UNICODE_ENVIRONMENT`. -pub type PfnCreateProcessAsUserInsideSecurityEnvironment = unsafe extern "system" fn( - user_token: HANDLE, - application_name: *const u16, - command_line: *mut u16, - creation_flags: u32, - environment: *const c_void, - current_directory: *const u16, - startup_info: *const STARTUPINFOW, - process_security_environment: HANDLE, - process_information: *mut PROCESS_INFORMATION, -) -> i32; - /// `BOOL CloseProcessSecurityEnvironment(HPROCESS_SECURITY_ENVIRONMENT* processSecurityEnvironment)`. /// /// The export nulls `*processSecurityEnvironment` on success. @@ -160,6 +136,161 @@ impl Drop for ProcessSecurityEnvironment { } } +/// `ProcThreadAttributeSecurityEnvironment` (enum value 35), encoded with the +/// standard `PROC_THREAD_ATTRIBUTE_INPUT` flag. +const PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT: usize = 35 | 0x0002_0000; + +/// Owns the extended startup information required to insert a process into an +/// existing process security environment through `CreateProcessW`. +pub struct SecurityEnvironmentStartupInfo { + storage: Vec, + attribute_list: LPPROC_THREAD_ATTRIBUTE_LIST, + environment_value: Box, + inherited_handles: Vec, + startup_info: STARTUPINFOEXW, +} + +impl SecurityEnvironmentStartupInfo { + /// Add `environment` to an extended copy of `startup_info`. + pub fn new( + mut startup_info: STARTUPINFOW, + environment: HANDLE, + inherited_handles: &[HANDLE], + ) -> Result { + let attribute_count = 1 + u32::from(!inherited_handles.is_empty()); + let mut byte_count = 0usize; + // SAFETY: the documented sizing call uses a null list and writes only + // the required byte count. + let sizing_result = unsafe { + InitializeProcThreadAttributeList(None, attribute_count, None, &mut byte_count) + }; + let sizing_error = last_error(); + if sizing_result.is_ok() || sizing_error != ERROR_INSUFFICIENT_BUFFER.0 || byte_count == 0 { + return Err(LearningModeError::ApiCall { + function: "InitializeProcThreadAttributeList(size)", + code: sizing_error, + }); + } + + let word_count = byte_count.div_ceil(std::mem::size_of::()); + let mut storage = vec![0usize; word_count]; + let attribute_list = LPPROC_THREAD_ATTRIBUTE_LIST(storage.as_mut_ptr().cast()); + + // SAFETY: `storage` is aligned for pointers, has at least + // `byte_count` writable bytes, and remains owned by the returned value. + unsafe { + InitializeProcThreadAttributeList( + Some(attribute_list), + attribute_count, + None, + &mut byte_count, + ) + } + .map_err(|_| LearningModeError::ApiCall { + function: "InitializeProcThreadAttributeList", + code: last_error(), + })?; + + let environment_value = Box::new(environment); + // SAFETY: the attribute list is initialized, and the boxed HANDLE has + // a stable address that outlives every use of the list. + if unsafe { + UpdateProcThreadAttribute( + attribute_list, + 0, + PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT, + Some((&raw const *environment_value).cast()), + std::mem::size_of::(), + None, + None, + ) + } + .is_err() + { + let code = last_error(); + // SAFETY: balances the successful initialization above. + unsafe { DeleteProcThreadAttributeList(attribute_list) }; + return Err(LearningModeError::ApiCall { + function: "UpdateProcThreadAttribute(SecurityEnvironment)", + code, + }); + } + + let inherited_handles = inherited_handles.to_vec(); + if !inherited_handles.is_empty() + && unsafe { + UpdateProcThreadAttribute( + attribute_list, + 0, + PROC_THREAD_ATTRIBUTE_HANDLE_LIST as usize, + Some(inherited_handles.as_ptr().cast()), + std::mem::size_of_val(inherited_handles.as_slice()), + None, + None, + ) + } + .is_err() + { + let code = last_error(); + // SAFETY: balances the successful initialization above. + unsafe { DeleteProcThreadAttributeList(attribute_list) }; + return Err(LearningModeError::ApiCall { + function: "UpdateProcThreadAttribute(HANDLE_LIST)", + code, + }); + } + + startup_info.cb = u32::try_from(std::mem::size_of::()).map_err(|_| { + LearningModeError::ApiCall { + function: "STARTUPINFOEXW size", + code: windows::Win32::Foundation::ERROR_INVALID_PARAMETER.0, + } + })?; + let startup_info = STARTUPINFOEXW { + StartupInfo: startup_info, + lpAttributeList: attribute_list, + }; + + Ok(Self { + storage, + attribute_list, + environment_value, + inherited_handles, + startup_info, + }) + } + + /// Extended startup information to pass to `CreateProcessW`. + #[must_use] + pub fn startup_info(&self) -> &STARTUPINFOEXW { + &self.startup_info + } +} + +impl std::fmt::Debug for SecurityEnvironmentStartupInfo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SecurityEnvironmentStartupInfo") + .field( + "attribute_bytes", + &(self.storage.len() * std::mem::size_of::()), + ) + .field("environment", &self.environment_value) + .field("inherited_handles", &self.inherited_handles) + .finish() + } +} + +impl Drop for SecurityEnvironmentStartupInfo { + fn drop(&mut self) { + if !self.attribute_list.is_invalid() { + // SAFETY: the list was successfully initialized and has not been + // deleted yet. + unsafe { DeleteProcThreadAttributeList(self.attribute_list) }; + self.attribute_list = LPPROC_THREAD_ATTRIBUTE_LIST::default(); + } + } +} + /// Which candidate export name resolved for each function on this machine — a /// diagnostic used by the capability probe to report the exact live surface (plain vs /// `Experimental_`). @@ -167,8 +298,6 @@ impl Drop for ProcessSecurityEnvironment { pub struct SecurityEnvironmentExportReport { /// Resolved name of the create export, if present. pub create: Option<&'static str>, - /// Resolved name of the in-environment launch export, if present. - pub launch: Option<&'static str>, /// Resolved name of the close export, if present. pub close: Option<&'static str>, } @@ -177,7 +306,7 @@ impl SecurityEnvironmentExportReport { /// `true` only when every export required for the 2-phase launch resolved. #[must_use] pub fn is_complete(&self) -> bool { - self.create.is_some() && self.launch.is_some() && self.close.is_some() + self.create.is_some() && self.close.is_some() } } @@ -187,10 +316,6 @@ const CREATE_NAMES: &[&core::ffi::CStr] = &[ c"CreateProcessSecurityEnvironment", c"Experimental_CreateProcessSecurityEnvironment", ]; -const LAUNCH_NAMES: &[&core::ffi::CStr] = &[ - c"CreateProcessAsUserInsideSecurityEnvironment", - c"Experimental_CreateProcessAsUserInsideSecurityEnvironment", -]; const CLOSE_NAMES: &[&core::ffi::CStr] = &[ c"CloseProcessSecurityEnvironment", c"Experimental_CloseProcessSecurityEnvironment", @@ -200,7 +325,6 @@ const CLOSE_NAMES: &[&core::ffi::CStr] = &[ #[derive(Clone, Copy)] pub struct SecurityEnvironmentApi { create: PfnCreateProcessSecurityEnvironment, - launch: PfnCreateProcessAsUserInsideSecurityEnvironment, close: PfnCloseProcessSecurityEnvironment, } @@ -208,7 +332,6 @@ impl std::fmt::Debug for SecurityEnvironmentApi { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("SecurityEnvironmentApi") .field("create", &(self.create as *const ())) - .field("launch", &(self.launch as *const ())) .field("close", &(self.close as *const ())) .finish() } @@ -234,7 +357,6 @@ impl SecurityEnvironmentApi { .map_err(|e| LearningModeError::DllLoad(e.to_string()))?; let create_proc = resolve_any(hmodule, CREATE_NAMES)?; - let launch_proc = resolve_any(hmodule, LAUNCH_NAMES)?; let close_proc = resolve_any(hmodule, CLOSE_NAMES)?; Ok(Self { @@ -242,10 +364,6 @@ impl SecurityEnvironmentApi { unsafe extern "system" fn() -> isize, PfnCreateProcessSecurityEnvironment, >(create_proc), - launch: std::mem::transmute::< - unsafe extern "system" fn() -> isize, - PfnCreateProcessAsUserInsideSecurityEnvironment, - >(launch_proc), close: std::mem::transmute::< unsafe extern "system" fn() -> isize, PfnCloseProcessSecurityEnvironment, @@ -308,20 +426,6 @@ impl SecurityEnvironmentApi { pub fn close(&self, env: &mut ProcessSecurityEnvironment) -> Result<(), LearningModeError> { env.close_with(self.close) } - - /// The resolved in-environment launch export. - /// - /// The stdio/wait/job-object orchestration around - /// `CreateProcessAsUserInsideSecurityEnvironment` belongs to the runner, so the raw - /// function pointer is exposed rather than a fully-wrapped launch here. The runner - /// supplies the `STARTUPINFOW`, receives the real `PROCESS_INFORMATION`, and owns - /// the returned handles. Callers must pass the environment handle from - /// [`ProcessSecurityEnvironment::raw`], and — when supplying an environment block — - /// include `CREATE_UNICODE_ENVIRONMENT` in the creation flags. - #[must_use] - pub fn launch_fn(&self) -> PfnCreateProcessAsUserInsideSecurityEnvironment { - self.launch - } } /// Resolve the first name in `names` that is present in `hmodule`. @@ -379,7 +483,6 @@ pub fn probe_security_environment_exports() -> SecurityEnvironmentExportReport { unsafe { SecurityEnvironmentExportReport { create: first_present(hmodule, CREATE_NAMES), - launch: first_present(hmodule, LAUNCH_NAMES), close: first_present(hmodule, CLOSE_NAMES), } } @@ -459,6 +562,33 @@ mod tests { assert_eq!(PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, 0); } + #[test] + fn startup_info_attaches_security_environment_attribute() { + let startup_info = STARTUPINFOW { + cb: std::mem::size_of::() as u32, + ..Default::default() + }; + let environment = HANDLE(std::ptr::dangling_mut::()); + + match SecurityEnvironmentStartupInfo::new(startup_info, environment, &[]) { + Ok(extended) => { + assert_eq!( + extended.startup_info().StartupInfo.cb as usize, + std::mem::size_of::() + ); + assert!(!extended.startup_info().lpAttributeList.is_invalid()); + } + Err(LearningModeError::ApiCall { code, .. }) + if code == windows::Win32::Foundation::ERROR_NOT_SUPPORTED.0 + || code == windows::Win32::Foundation::ERROR_CALL_NOT_IMPLEMENTED.0 => + { + // Off-feature Windows builds reject the private attribute at + // UpdateProcThreadAttribute time. + } + Err(error) => panic!("unexpected attribute-list error: {error}"), + } + } + #[test] fn failed_close_retains_ownership_and_drop_retries() { CLOSE_CALLS.store(0, Ordering::SeqCst); diff --git a/src/core/wxc/src/main.rs b/src/core/wxc/src/main.rs index aa9529df8..90e8afb06 100644 --- a/src/core/wxc/src/main.rs +++ b/src/core/wxc/src/main.rs @@ -200,16 +200,21 @@ fn apply_permissive_learning_mode(capabilities: &mut Vec) -> bool { } } -fn validate_audit_containment(containment: &ContainmentBackend) -> Result<(), String> { - if *containment == ContainmentBackend::ProcessContainer { - Ok(()) - } else { - Err(format!( +const AUDIT_CAPTURE_DENIALS_CONFLICT_MSG: &str = + "--audit cannot be combined with processContainer.captureDenials; use captureDenials.mode: \"allow\" for permissive capture"; + +fn validate_audit_request(request: &ExecutionRequest) -> Result<(), String> { + if request.containment != ContainmentBackend::ProcessContainer { + return Err(format!( "--audit is only supported with the Windows ProcessContainer backend; \ resolved containment is '{}'", - containment.wire_name() - )) + request.containment.wire_name() + )); + } + if request.policy.capture_denials.is_some() { + return Err(AUDIT_CAPTURE_DENIALS_CONFLICT_MSG.to_string()); } + Ok(()) } fn command_override_from_cli( @@ -1074,7 +1079,7 @@ fn main() { // because Windows derives capability SIDs case-insensitively. #[cfg(target_os = "windows")] if cli.audit { - if let Err(message) = validate_audit_containment(&request.containment) { + if let Err(message) = validate_audit_request(&request) { eprintln!("Error: {message}"); telemetry::emit_early_exit( telemetry_active, @@ -1434,7 +1439,11 @@ mod tests { #[test] fn audit_mode_only_accepts_process_container() { - assert!(validate_audit_containment(&ContainmentBackend::ProcessContainer).is_ok()); + let mut request = ExecutionRequest { + containment: ContainmentBackend::ProcessContainer, + ..Default::default() + }; + assert!(validate_audit_request(&request).is_ok()); for containment in [ ContainmentBackend::WindowsSandbox, @@ -1442,9 +1451,32 @@ mod tests { ContainmentBackend::IsolationSession, ContainmentBackend::MicroVm, ] { - let error = validate_audit_containment(&containment) + let containment_name = containment.wire_name(); + request.containment = containment; + let error = validate_audit_request(&request) .expect_err("non-ProcessContainer backend must reject --audit"); - assert!(error.contains(containment.wire_name())); + assert!(error.contains(containment_name)); + } + } + + #[test] + fn audit_mode_rejects_both_capture_denials_modes() { + use wxc_common::models::{CaptureDenialsConfig, CaptureDenialsMode}; + + for mode in [CaptureDenialsMode::Block, CaptureDenialsMode::Allow] { + let mut request = ExecutionRequest { + containment: ContainmentBackend::ProcessContainer, + ..Default::default() + }; + request.policy.capture_denials = Some(CaptureDenialsConfig { + mode, + output_path: None, + }); + + let error = validate_audit_request(&request) + .expect_err("--audit and captureDenials must be mutually exclusive"); + assert_eq!(error, AUDIT_CAPTURE_DENIALS_CONFLICT_MSG); + assert!(error.contains(r#"mode: "allow""#)); } } diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 0cbefd500..8f26bbef8 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -815,10 +815,6 @@ fn convert_wire_config( // available in every build. if ac.learning_mode.unwrap_or(false) { policy.capabilities.push("learningModeLogging".to_string()); - logger.log( - "NOTE: 'learningModeLogging' enabled - AppContainer restrictions remain \ -enforced; access denials are recorded for diagnostics.\n", - ); } // Learning-mode capability names are reserved for the dedicated entry @@ -863,6 +859,39 @@ enforced; access denials are recorded for diagnostics.\n", Some(wire::CaptureDenialsMode::Allow) => CaptureDenialsMode::Allow, Some(wire::CaptureDenialsMode::Block) | None => CaptureDenialsMode::Block, }; + + // captureDenials drives the learning-mode ETL capture in the runner, + // which requires the corresponding learning-mode capability on the + // child token so the OS emits the access-check records the capture + // path collects. Inject it additively (preserving the workload's real + // capabilities). `block` keeps deny-by-default via + // `learningModeLogging`; `allow` replaces deny-and-record with + // `permissiveLearningMode` (the runner surfaces the security warning). + let capture_capability = match mode { + CaptureDenialsMode::Block => { + // Capability entries are exact names. Comma-packed entries + // were rejected above, so substring matching here would + // incorrectly remove unrelated custom capabilities. + policy.capabilities.retain(|capability| { + !capability.eq_ignore_ascii_case("permissiveLearningMode") + }); + "learningModeLogging" + } + CaptureDenialsMode::Allow => { + policy.capabilities.retain(|capability| { + !capability.eq_ignore_ascii_case("learningModeLogging") + }); + "permissiveLearningMode" + } + }; + if !policy + .capabilities + .iter() + .any(|capability| capability.eq_ignore_ascii_case(capture_capability)) + { + policy.capabilities.push(capture_capability.to_string()); + } + policy.capture_denials = Some(CaptureDenialsConfig { mode, output_path: cd.output_path, @@ -2433,6 +2462,107 @@ mod tests { assert_eq!(cd.mode, CaptureDenialsMode::Allow); } + #[test] + fn capture_denials_block_injects_learning_mode_logging_capability() { + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": {"captureDenials": {"mode": "block"}} + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"learningModeLogging".to_string()), + "block capture must additively inject learningModeLogging: {:?}", + req.policy.capabilities + ); + assert!( + !req.policy + .capabilities + .contains(&"permissiveLearningMode".to_string()), + "block must not inject permissiveLearningMode" + ); + } + + #[test] + fn capture_denials_allow_injects_permissive_learning_mode_capability() { + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": {"captureDenials": {"mode": "allow"}} + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"permissiveLearningMode".to_string()), + "allow capture must inject permissiveLearningMode: {:?}", + req.policy.capabilities + ); + } + + #[test] + fn capture_denials_default_injects_learning_mode_logging_capability() { + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": {"captureDenials": {}} + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"learningModeLogging".to_string()), + "default (block) capture must inject learningModeLogging: {:?}", + req.policy.capabilities + ); + } + + #[test] + fn capture_denials_allow_overrides_learning_mode_boolean() { + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": { + "learningMode": true, + "capabilities": ["internetClient"], + "captureDenials": {"mode": "allow"} + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"internetClient".to_string()), + "the workload's own capabilities must be preserved" + ); + assert!( + req.policy + .capabilities + .contains(&"permissiveLearningMode".to_string()), + "allow capture must inject permissiveLearningMode" + ); + assert!( + !req.policy + .capabilities + .contains(&"learningModeLogging".to_string()), + "allow capture must remove deny-and-record mode" + ); + assert!( + !logger.get_buffer().contains("restrictions remain enforced"), + "parser must not log the superseded deny-and-record mode" + ); + } + #[test] fn capture_denials_unknown_mode_rejected() { let json = r#"{