From 934fc4b0f59119cd9931657305121cdd1f8a0423 Mon Sep 17 00:00:00 2001 From: Jeffrey Quesnelle Date: Wed, 28 Oct 2026 22:08:43 -0400 Subject: [PATCH 1/2] feat(mxc): pass driver_config denied_paths to MXC deniedPaths A read-write grant covers everything below it, so a file the workload must not see inside a granted folder, such as an agent profile's .env or auth.json, is readable, and an absent one can be created there. The MXC driver already builds MXC's filesystem.deniedPaths but always sends an empty list. Add an optional denied_paths list to the driver-owned driver_config.mxc block and pass it to MXC. Preflight normalizes each entry (backslash separators, no repeated or trailing separator, case-insensitive duplicates removed) and rejects, before any sandbox state is created: - entries that are not absolute paths starting with a drive letter, or that have a '.' or '..' component, a component ending in a dot or space, or a character Windows does not allow in file names; - entries that equal or contain a filesystem grant (a policy grant, the generation staging directory under cwd, or the sandbox executable): MXC drops a grant equal to a denied path and keeps access to a grant nested inside a denied directory. MXC matches paths after resolving 8.3 short names and junctions and ignoring letter case, so both sides are resolved with std::fs::canonicalize (through the deepest existing ancestor for absent paths) and compared ignoring Unicode letter case; a path that cannot be resolved is compared as written; - entries whose parent directory does not exist, which MXC refuses at launch (CreateProcessSecurityEnvironment, HRESULT 0x80070003). The error names the missing directory to deny instead. When the list is not empty, also send fallback.allowDaclMutation=false. MXC 1.0 defaults it to true, and on a host whose base container cannot enforce deniedPaths it would otherwise add deny entries to host file ACLs. The list is empty by default, so existing requests are unchanged. Document the field in the driver README and the gateway configuration page. Signed-off-by: Jeffrey Quesnelle --- crates/openshell-driver-mxc/README.md | 35 + crates/openshell-driver-mxc/src/driver.rs | 645 ++++++++++++++++++- crates/openshell-driver-mxc/src/mxc.rs | 9 +- crates/openshell-driver-mxc/src/policy.rs | 2 +- docs/how-it-works/gateways/configuration.mdx | 17 + 5 files changed, 691 insertions(+), 17 deletions(-) diff --git a/crates/openshell-driver-mxc/README.md b/crates/openshell-driver-mxc/README.md index 9da3c201ef..e4d5cc598d 100644 --- a/crates/openshell-driver-mxc/README.md +++ b/crates/openshell-driver-mxc/README.md @@ -148,6 +148,41 @@ command is supplied, the driver passes `mxc.command` to the supervisor rather than starting a scratch workload. An explicit generic canonical command retains precedence. Environment belongs in `--env` or `--env-from`, not gateway configuration. +The optional `mxc.denied_paths` lists paths the workload cannot use, including +inside the policy's filesystem grants: it cannot read or write a denied file, +list a denied directory, or create a denied path. The driver passes the list to +MXC as `filesystem.deniedPaths`. For example, to keep a profile directory +writable while hiding its credential files: + +```powershell +$config = '{"mxc":{"command":["C:\\Windows\\System32\\cmd.exe","/d","/c","agent.cmd"],"cwd":"C:\\work","denied_paths":["C:\\work\\profile\\.env","C:\\work\\profile\\auth.json"]}}' +``` + +Each entry must be an absolute path that starts with a drive letter, such as +`C:\work\profile\.env`; forward slashes are converted to backslashes. A denied +path may be absent, in which case the workload cannot create it, but its parent +directory must exist because MXC refuses to start the ProcessContainer +otherwise; to cover a missing directory, deny the directory itself. An entry +must not equal or contain a filesystem grant, including the generation staging +directory under `cwd` and the sandbox executable: MXC drops a grant that equals +a denied path and keeps access to a grant nested inside a denied directory. +MXC matches paths after resolving 8.3 short names and junctions and ignoring +letter case, so preflight compares them the same way (following symbolic links +too) and rejects these cases before provisioning. The parts of a path that +do not exist, or that cannot be resolved, are compared as written. + +A denied file's name, size, timestamps and attributes remain visible to the +workload, for example when it lists the parent directory; only the contents +are protected. A deny covers the listed path: a hard link to the same file, +created on the host before launch at a path the workload can read, still +exposes the contents. + +Denied paths only remove access and are fixed at sandbox creation. They require +a host whose `wxc-exec.exe --probe` reports +`probes.baseContainerSupportsDenyPaths: true`. When the list is not empty, the +driver sends MXC `fallback.allowDaclMutation: false`, so on a host without that +support the launch fails and MXC does not change the ACLs of host files. + When gateway TLS is enabled, configure the gateway-owned `guest_tls_ca`. The host supervisor verifies the gateway certificate with that CA and uses main's launch authentication bundle. Gateway client certificates and keys are diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 3d2bef6cef..6fdfdc7a3e 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -44,6 +44,9 @@ const HOST_RUNTIME_DESCRIPTOR_FILE: &str = "runtime-descriptor.json"; const BOUNDARY_CONFIG_FILE: &str = "boundary.json"; const BOUNDARY_TLS_CERT_FILE: &str = "sandbox.crt"; const BOUNDARY_TLS_KEY_FILE: &str = "sandbox.key"; +/// Directory under the workload `cwd` that holds each generation's boundary +/// staging directory. +const BOUNDARY_STAGING_DIR: &str = ".openshell-runtime"; const DIRECT_PROXY_USERNAME: &str = "openshell"; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] @@ -157,6 +160,11 @@ struct MxcSandboxConfig { command: Vec, #[serde(default)] cwd: String, + /// Files and directories the workload cannot read, write, list, or + /// create, including inside filesystem grants. Sent to MXC as + /// `filesystem.deniedPaths`. + #[serde(default)] + denied_paths: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -356,11 +364,26 @@ impl MxcComputeBackend { )); } let policy = sandbox.spec.as_ref().and_then(|spec| spec.policy.as_ref()); - self.map_sandbox_policy( + let mapped = self.map_sandbox_policy( &sandbox.id, policy, "127.0.0.1:3128".parse().expect("fixed proxy address"), )?; + // Denied paths are checked against every grant MXC receives: the + // policy grants, the generation staging directory under `cwd`, and the + // sandbox executable. + let staging_root = Path::new(&config.cwd) + .join(BOUNDARY_STAGING_DIR) + .display() + .to_string(); + let grants: Vec<&str> = mapped + .readwrite_paths + .iter() + .chain(&mapped.readonly_paths) + .chain([&staging_root, &self.config.sandbox_binary_path]) + .map(String::as_str) + .collect(); + check_denied_paths(&config.denied_paths, &grants)?; Ok(()) } @@ -420,7 +443,7 @@ impl MxcComputeBackend { .join(safe_component(&sandbox_id)?) .join(&generation); let boundary_state_dir = PathBuf::from(&sandbox_config.cwd) - .join(".openshell-runtime") + .join(BOUNDARY_STAGING_DIR) .join(&generation); let gate = Arc::new(Mutex::new(())); let startup_guard = gate.clone().lock_owned().await; @@ -734,18 +757,11 @@ async fn run_lifecycle_inner( "--log-level".to_string(), openshell_core::driver_utils::sandbox_log_level(&context.sandbox, "info"), ]); - let mut filesystem = MxcFilesystem { - readwrite_paths: mapped.readwrite_paths, - readonly_paths: mapped.readonly_paths, - denied_paths: Vec::new(), - }; - push_unique_path( - &mut filesystem.readwrite_paths, - context.boundary_state_dir.display().to_string(), - ); - push_unique_path( - &mut filesystem.readonly_paths, - context.config.sandbox_binary_path.clone(), + let filesystem = boundary_filesystem( + &mapped, + &context.sandbox_config, + &context.boundary_state_dir, + &context.config.sandbox_binary_path, ); let process = MxcProcess { command_line: sandbox_command, @@ -984,7 +1000,164 @@ fn sandbox_config(sandbox: &DriverSandbox) -> Result Result { + let normalized = crate::policy::normalize_path(path); + if !has_drive_root(&normalized) { + return Err("is not an absolute path on a local drive, such as C:\\work\\.env"); + } + let components: Vec<&str> = normalized[3..] + .split('\\') + .filter(|component| !component.is_empty()) + .collect(); + if components + .iter() + .any(|component| component.ends_with(['.', ' '])) + { + // Covers `.` and `..`; Windows also drops a trailing dot or space. + return Err("has a '.' or '..' component or one ending in a dot or space"); + } + if components.iter().any(|component| { + component + .chars() + .any(|character| character.is_control() || "<>:\"|?*".contains(character)) + }) { + return Err("contains a character Windows does not allow in file names"); + } + Ok(format!("{}{}", &normalized[..3], components.join("\\"))) +} + +/// Whether a backslash-separated path starts with a drive letter and a root, +/// as in `C:\`. +fn has_drive_root(path: &str) -> bool { + let bytes = path.as_bytes(); + bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'\\' +} + +/// Check denied paths against the grants MXC receives and the host file +/// system before any sandbox state is created. +/// +/// MXC resolves overlapping rules by the file system object each path names: +/// a denied path equal to a grant removes the whole grant, while a grant +/// nested in a denied directory keeps its access. Neither matches a request +/// to hide part of a grant, so a denied path must not equal or contain a +/// grant, compared after resolving 8.3 short names, junctions and symbolic +/// links, and ignoring letter case. MXC also refuses to start a +/// `ProcessContainer` that denies a path whose parent directory does not +/// exist; the denied path itself may be absent, and MXC then prevents its +/// creation. +fn check_denied_paths(denied_paths: &[String], grants: &[&str]) -> Result<(), tonic::Status> { + if denied_paths.is_empty() { + return Ok(()); + } + let grants: Vec<(&str, Vec)> = grants + .iter() + .map(|grant| (*grant, resolved_path_key(grant))) + .collect(); + for denied in denied_paths { + let denied_key = resolved_path_key(denied); + if let Some((grant, _)) = grants + .iter() + .find(|(_, grant_key)| grant_key.starts_with(&denied_key)) + { + return Err(tonic::Status::invalid_argument(format!( + "mxc driver_config.denied_paths entry '{denied}' equals or contains the filesystem grant '{grant}'; deny paths below the grant instead" + ))); + } + let path = Path::new(denied); + if !path.parent().is_some_and(Path::is_dir) { + // Denying the outermost missing directory covers the entry, and + // MXC accepts it because that directory's own parent exists. + let remedy = path + .ancestors() + .skip(1) + .take_while(|ancestor| !ancestor.exists()) + .last() + .filter(|missing| missing.parent().is_some_and(Path::is_dir)) + .map_or_else(String::new, |missing| { + format!( + "; deny the missing directory '{}' instead", + missing.display() + ) + }); + return Err(tonic::Status::failed_precondition(format!( + "mxc driver_config.denied_paths entry '{denied}' has no existing parent directory, which MXC requires{remedy}" + ))); + } + } + Ok(()) +} + +/// Prefix of the verbatim paths `std::fs::canonicalize` returns on Windows. +const VERBATIM_PREFIX: &str = r"\\?\"; + +/// Path components that identify the file system object a Windows path +/// names, for comparing denied paths with grants. A path that starts with a +/// drive letter is resolved with `std::fs::canonicalize`, which follows +/// junctions and symbolic links and returns the long name and on-disk letter +/// case of every component. When the path does not exist, its deepest +/// existing ancestor is resolved and the remaining components are appended +/// as written. Components that cannot be resolved this way, and paths without +/// a drive letter, are compared as written. +fn resolved_path_key(path: &str) -> Vec { + let path = crate::policy::normalize_path(path); + let path = path.strip_prefix(VERBATIM_PREFIX).unwrap_or(&path); + if has_drive_root(path) + && let Some(resolved) = resolve_existing_prefix(Path::new(path)) + { + let resolved = resolved.display().to_string(); + return path_key(resolved.strip_prefix(VERBATIM_PREFIX).unwrap_or(&resolved)); + } + path_key(path) +} + +/// Canonicalize the deepest existing ancestor of `path` and append the +/// remaining components as written. +fn resolve_existing_prefix(path: &Path) -> Option { + let mut missing = Vec::new(); + for ancestor in path.ancestors() { + if let Ok(resolved) = std::fs::canonicalize(ancestor) { + return Some( + missing + .iter() + .rev() + .fold(resolved, |resolved, component| resolved.join(component)), + ); + } + missing.push(ancestor.file_name()?); + } + None +} + +/// Case-insensitive path components for comparing Windows paths. Letter case +/// is ignored for all Unicode letters, not only ASCII. +fn path_key(path: &str) -> Vec { + crate::policy::normalize_path(path) + .split('\\') + .filter(|component| !component.is_empty() && *component != ".") + .map(str::to_lowercase) + .collect() } fn launch_authentication( @@ -1082,6 +1255,31 @@ fn push_unique_path(paths: &mut Vec, path: String) { } } +/// MXC filesystem rules for the boundary `ProcessContainer`: the policy +/// grants, the generation staging directory, the sandbox executable, and the +/// caller's denied paths. +fn boundary_filesystem( + mapped: &MappedConfig, + sandbox_config: &MxcSandboxConfig, + boundary_state_dir: &Path, + sandbox_binary_path: &str, +) -> MxcFilesystem { + let mut filesystem = MxcFilesystem { + readwrite_paths: mapped.readwrite_paths.clone(), + readonly_paths: mapped.readonly_paths.clone(), + denied_paths: sandbox_config.denied_paths.clone(), + }; + push_unique_path( + &mut filesystem.readwrite_paths, + boundary_state_dir.display().to_string(), + ); + push_unique_path( + &mut filesystem.readonly_paths, + sandbox_binary_path.to_string(), + ); + filesystem +} + fn cleanup_runtime_directory(path: &Path) { if let Err(error) = std::fs::remove_dir_all(path) && error.kind() != io::ErrorKind::NotFound @@ -1554,6 +1752,423 @@ mod tests { } } + fn with_denied_paths(mut sandbox: DriverSandbox, denied_paths: &[String]) -> DriverSandbox { + let template = sandbox.spec.as_mut().unwrap().template.as_mut().unwrap(); + let mut config = struct_to_json_value(template.driver_config.as_ref().unwrap()); + config["denied_paths"] = serde_json::json!(denied_paths); + let serde_json::Value::Object(config) = config else { + unreachable!() + }; + template.driver_config = + Some(openshell_core::proto_struct::json_object_to_struct(config).expect("config")); + sandbox + } + + fn with_filesystem_grants( + mut sandbox: DriverSandbox, + read_write: &[&Path], + read_only: &[&Path], + ) -> DriverSandbox { + let strings = |paths: &[&Path]| -> Vec { + paths + .iter() + .map(|path| path.display().to_string()) + .collect() + }; + sandbox + .spec + .as_mut() + .unwrap() + .policy + .as_mut() + .unwrap() + .filesystem = Some(openshell_core::proto::FilesystemPolicy { + read_write: strings(read_write), + read_only: strings(read_only), + ..Default::default() + }); + sandbox + } + + fn display(path: &Path) -> String { + path.display().to_string() + } + + #[test] + fn driver_config_denied_paths_inside_a_grant_pass_preflight() { + let (backend, sandbox, dir) = preflight_fixture(); + let profile = dir.path().join("profile"); + std::fs::create_dir(&profile).unwrap(); + std::fs::write(profile.join(".env"), "fixture").unwrap(); + let sandbox = with_filesystem_grants(sandbox, &[&profile], &[]); + // A denied path may be absent while its parent directory exists. + let sandbox = with_denied_paths( + sandbox, + &[ + display(&profile.join(".env")), + display(&profile.join("auth.json")), + ], + ); + backend + .validate_sandbox_create(&sandbox) + .expect("denied paths inside a grant pass preflight"); + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_paths_reach_the_mxc_request() { + let (backend, sandbox, dir) = preflight_fixture(); + let profile = dir.path().join("profile"); + std::fs::create_dir(&profile).unwrap(); + let env = display(&profile.join(".env")); + let auth = display(&profile.join("auth.json")); + let sandbox = with_filesystem_grants(sandbox, &[&profile], &[]); + let process = MxcProcess { + command_line: "cmd /c exit 0".into(), + cwd: display(dir.path()), + env: Vec::new(), + timeout: 0, + }; + let staging = dir.path().join(BOUNDARY_STAGING_DIR).join("generation"); + let mxc_request = |sandbox: &DriverSandbox| { + backend + .validate_sandbox_create(sandbox) + .expect("valid preflight"); + let config = sandbox_config(sandbox).unwrap(); + let mapped = backend + .map_sandbox_policy( + &sandbox.id, + sandbox.spec.as_ref().unwrap().policy.as_ref(), + "127.0.0.1:3128".parse().unwrap(), + ) + .unwrap(); + let filesystem = boundary_filesystem( + &mapped, + &config, + &staging, + &backend.config.sandbox_binary_path, + ); + crate::mxc::oneshot_config_json( + &sandbox.id, + &filesystem, + &MxcProcessContainer::default(), + &process, + None, + None, + ) + }; + + // Without denied paths the request carries no deniedPaths or fallback + // field. + let request = mxc_request(&sandbox); + assert!(request["filesystem"].get("deniedPaths").is_none()); + assert!(request.get("fallback").is_none()); + + // Forward slashes, a trailing separator, and a case-only duplicate + // normalize to one backslash path per entry. + let sandbox = with_denied_paths( + sandbox, + &[ + env.replace('\\', "/"), + format!("{env}\\"), + env.to_ascii_uppercase(), + auth.clone(), + ], + ); + let request = mxc_request(&sandbox); + assert_eq!( + request["filesystem"]["deniedPaths"], + serde_json::json!([env, auth]) + ); + assert_eq!( + request["filesystem"]["readwritePaths"], + serde_json::json!([display(&profile), display(&staging)]) + ); + // MXC must not fall back to changing host ACLs to enforce the denies. + assert_eq!( + request["fallback"], + serde_json::json!({ "allowDaclMutation": false }) + ); + } + + #[test] + fn driver_config_denied_paths_must_be_absolute_local_paths() { + let (backend, sandbox, dir) = preflight_fixture(); + let base = display(dir.path()); + for path in [ + String::new(), + "relative\\.env".into(), + "\\work\\.env".into(), + "C:work\\.env".into(), + "\\\\server\\share\\.env".into(), + "\\\\?\\C:\\work\\.env".into(), + format!(" {base}\\.env"), + format!("{base}\\.env "), + format!("{base}\\.env."), + format!("{base}\\.\\.env"), + format!("{base}\\profile\\..\\.env"), + format!("{base}\\.env:stream"), + format!("{base}\\*.env"), + ] { + let error = backend + .validate_sandbox_create(&with_denied_paths( + sandbox.clone(), + std::slice::from_ref(&path), + )) + .expect_err("ambiguous denied path must be rejected"); + assert_eq!(error.code(), tonic::Code::InvalidArgument, "{path:?}"); + assert!( + error + .message() + .starts_with("mxc driver_config.denied_paths entry"), + "{path:?}: {}", + error.message() + ); + } + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_paths_must_not_cover_a_grant() { + let (backend, sandbox, dir) = preflight_fixture(); + let work = dir.path().join("work"); + let tools = dir.path().join("tools"); + std::fs::create_dir(&work).unwrap(); + std::fs::create_dir(&tools).unwrap(); + let sandbox = with_filesystem_grants(sandbox, &[&work], &[&tools]); + let staging = display(&dir.path().join(BOUNDARY_STAGING_DIR)); + let binary = backend.config.sandbox_binary_path.clone(); + for (denied, grant) in [ + (display(&work), display(&work)), + ( + display(&work).to_ascii_uppercase().replace('\\', "/"), + display(&work), + ), + (display(dir.path()), display(&work)), + (display(&tools), display(&tools)), + (staging.clone(), staging), + (binary.clone(), binary), + ] { + let error = backend + .validate_sandbox_create(&with_denied_paths( + sandbox.clone(), + std::slice::from_ref(&denied), + )) + .expect_err("a denied path must not cover a grant"); + assert_eq!(error.code(), tonic::Code::InvalidArgument, "{denied}"); + assert!( + error.message().contains(&format!( + "equals or contains the filesystem grant '{grant}'" + )), + "{denied}: {}", + error.message() + ); + } + // A grant written as a verbatim path names the same directory. + let verbatim = PathBuf::from(format!(r"\\?\{}", display(&work))); + let sandbox = with_filesystem_grants(sandbox, &[&verbatim], &[]); + assert_denied_path_covers_grant(&backend, &sandbox, &display(&work), &verbatim); + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_path_parent_must_exist() { + let (backend, sandbox, dir) = preflight_fixture(); + std::fs::write(dir.path().join("file.txt"), "fixture").unwrap(); + let missing = dir.path().join("missing"); + // No directory to suggest when the parent is a file or the drive is + // missing. + let no_volume = PathBuf::from(format!("{}:\\missing\\.env", unused_drive_letter())); + for (denied, remedy) in [ + (missing.join(".env"), Some(&missing)), + (missing.join("nested").join(".env"), Some(&missing)), + (dir.path().join("file.txt").join(".env"), None), + (no_volume, None), + ] { + let sandbox = with_denied_paths(sandbox.clone(), &[display(&denied)]); + let error = backend.validate_sandbox_create(&sandbox).unwrap_err(); + assert_eq!(error.code(), tonic::Code::FailedPrecondition); + assert!( + error.message().contains("has no existing parent directory"), + "{}", + error.message() + ); + // The error names the missing directory to deny instead. + let suggestion = remedy.map(|directory| { + format!( + "; deny the missing directory '{}' instead", + display(directory) + ) + }); + assert_eq!( + error.message().split_once("which MXC requires").unwrap().1, + suggestion.as_deref().unwrap_or_default(), + "{}", + error.message() + ); + } + // Denying the missing directory itself passes preflight. + let sandbox = with_denied_paths(sandbox, &[display(&missing)]); + backend + .validate_sandbox_create(&sandbox) + .expect("a missing directory under an existing one can be denied"); + assert!(!backend.config.state_dir.exists()); + } + + /// A drive letter with no volume behind it. + fn unused_drive_letter() -> char { + (b'D'..=b'Z') + .rev() + .map(char::from) + .find(|letter| !Path::new(&format!("{letter}:\\")).exists()) + .expect("an unused drive letter") + } + + /// Run `cmd.exe /d /c ` and return its trimmed output. + fn run_cmd(command: &str) -> (bool, String) { + use std::os::windows::process::CommandExt as _; + let output = std::process::Command::new("cmd.exe") + .args(["/d", "/c"]) + .raw_arg(command) + .output() + .expect("run cmd.exe"); + ( + output.status.success(), + String::from_utf8_lossy(&output.stdout).trim().to_string(), + ) + } + + /// The 8.3 short spelling of an existing path, or `None` when the volume + /// has no short name for it (8.3 name creation disabled). + fn short_path(path: &Path) -> Option { + let (_, short) = run_cmd(&format!("for %I in (\"{}\") do @echo %~sI", path.display())); + let same_object = + std::fs::canonicalize(&short).ok()? == std::fs::canonicalize(path).ok()?; + (same_object && !short.eq_ignore_ascii_case(&display(path))).then_some(short) + } + + fn assert_denied_path_covers_grant( + backend: &MxcComputeBackend, + sandbox: &DriverSandbox, + denied: &str, + grant: &Path, + ) { + let error = backend + .validate_sandbox_create(&with_denied_paths(sandbox.clone(), &[denied.to_string()])) + .expect_err("a denied path must not cover a grant"); + assert_eq!(error.code(), tonic::Code::InvalidArgument, "{denied}"); + assert!( + error.message().contains(&format!( + "equals or contains the filesystem grant '{}'", + display(grant) + )), + "{denied}: {}", + error.message() + ); + } + + #[test] + fn driver_config_denied_paths_resolve_short_names() { + let (backend, sandbox, dir) = preflight_fixture(); + let long_dir = dir.path().join("denied paths long name"); + let work = long_dir.join("work"); + let pending = long_dir.join("pending").join("nested"); + std::fs::create_dir_all(work.join("profile")).unwrap(); + let Some(short_dir) = short_path(&long_dir) else { + eprintln!( + "SKIP: no 8.3 short name for {}; enable 8.3 name creation on its volume to run this test", + long_dir.display() + ); + return; + }; + // `pending\nested` is a grant that does not exist yet, nor does its + // parent. + let sandbox = with_filesystem_grants(sandbox, &[&work, &pending], &[]); + // The same directories through the 8.3 name of their parent: equal to + // a grant, containing a grant, and equal to the grant that does not + // exist. + for (denied, grant) in [ + (format!("{short_dir}\\work"), &work), + (short_dir.clone(), &work), + (format!("{short_dir}\\pending\\nested"), &pending), + ] { + assert_denied_path_covers_grant(&backend, &sandbox, &denied, grant); + } + // The reverse: a grant spelled with the 8.3 name, denied by its long + // name. + let short_grant = PathBuf::from(format!("{short_dir}\\work")); + let reversed = with_filesystem_grants(sandbox.clone(), &[&short_grant], &[]); + assert_denied_path_covers_grant(&backend, &reversed, &display(&work), &short_grant); + // A file inside the grant can be denied through the 8.3 name. + backend + .validate_sandbox_create(&with_denied_paths( + sandbox, + &[format!("{short_dir}\\work\\profile\\.env")], + )) + .expect("a short spelling of a path inside a grant passes preflight"); + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_paths_resolve_junctions() { + let (backend, sandbox, dir) = preflight_fixture(); + let work = dir.path().join("work"); + std::fs::create_dir_all(work.join("profile")).unwrap(); + let link = dir.path().join("link"); + let (created, output) = run_cmd(&format!( + "mklink /J \"{}\" \"{}\"", + link.display(), + work.display() + )); + assert!(created, "mklink /J failed: {output}"); + let sandbox = with_filesystem_grants(sandbox, &[&work], &[]); + // A junction whose target is the grant names the grant. + assert_denied_path_covers_grant(&backend, &sandbox, &display(&link), &work); + // A grant spelled through the junction is the same directory. + let through_link = with_filesystem_grants(sandbox.clone(), &[&link], &[]); + assert_denied_path_covers_grant(&backend, &through_link, &display(&work), &link); + // A file inside the grant can be denied through the junction. + backend + .validate_sandbox_create(&with_denied_paths( + sandbox, + &[display(&link.join("profile").join(".env"))], + )) + .expect("a path below the grant through a junction passes preflight"); + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_paths_ignore_unicode_case() { + let (backend, sandbox, dir) = preflight_fixture(); + let existing = dir.path().join("\u{e4}rger"); + std::fs::create_dir(&existing).unwrap(); + // A grant that does not exist yet keeps the letter case as written. + let pending = dir.path().join("\u{f6}l"); + let sandbox = with_filesystem_grants(sandbox, &[&existing, &pending], &[]); + for (denied, grant) in [ + (dir.path().join("\u{c4}RGER"), &existing), + (dir.path().join("\u{d6}L"), &pending), + ] { + assert_denied_path_covers_grant(&backend, &sandbox, &display(&denied), grant); + } + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_paths_compare_unresolvable_paths_as_written() { + let (backend, sandbox, _dir) = preflight_fixture(); + // Nothing on a drive letter without a volume can be resolved. + let drive = unused_drive_letter(); + let grant = PathBuf::from(format!("{drive}:\\Work")); + let sandbox = with_filesystem_grants(sandbox, &[&grant], &[]); + let denied = format!("{}:/WORK/", drive.to_ascii_lowercase()); + assert_denied_path_covers_grant(&backend, &sandbox, &denied, &grant); + // A resolved path drops the `\\?\` prefix `canonicalize` adds, so it + // compares in the same form as a path compared as written. + assert_eq!(resolved_path_key(r"C:\"), path_key(r"C:\")); + assert!(!backend.config.state_dir.exists()); + } + #[test] fn isolation_session_does_not_advertise_ui_policy() { let (mut backend, _sandbox, _dir) = preflight_fixture(); diff --git a/crates/openshell-driver-mxc/src/mxc.rs b/crates/openshell-driver-mxc/src/mxc.rs index f39f20f2ce..f252493e2d 100644 --- a/crates/openshell-driver-mxc/src/mxc.rs +++ b/crates/openshell-driver-mxc/src/mxc.rs @@ -165,7 +165,7 @@ fn network_json(network: &MxcNetwork) -> serde_json::Value { value } -fn oneshot_config_json( +pub fn oneshot_config_json( container_id: &str, filesystem: &MxcFilesystem, pc: &MxcProcessContainer, @@ -209,6 +209,13 @@ fn oneshot_config_json( "processContainer": serde_json::Value::Object(pc_json), "filesystem": serde_json::Value::Object(filesystem_json), }); + // MXC 1.0 defaults `fallback.allowDaclMutation` to true, so on a host whose + // base container cannot enforce `deniedPaths` it would add deny entries to + // the ACLs of host files instead. Withhold that consent whenever paths are + // denied; such a host then fails the launch. + if !filesystem.denied_paths.is_empty() { + config["fallback"] = serde_json::json!({ "allowDaclMutation": false }); + } if let Some(network) = network { config["network"] = network_json(network); } diff --git a/crates/openshell-driver-mxc/src/policy.rs b/crates/openshell-driver-mxc/src/policy.rs index 8e4d0b9271..d7dbcfa0c8 100644 --- a/crates/openshell-driver-mxc/src/policy.rs +++ b/crates/openshell-driver-mxc/src/policy.rs @@ -94,7 +94,7 @@ pub trait PolicyMapper: Send + Sync { /// Normalize forward-slash paths to Windows backslash form. Path normalization /// lives here, in one place — the embedded mapper copies path strings through /// unchanged. -fn normalize_path(p: &str) -> String { +pub fn normalize_path(p: &str) -> String { p.replace('/', "\\") } diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index b4c396bc23..487f05b8a7 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -1330,6 +1330,23 @@ openshell sandbox create --name mxc-demo --detach --policy policy.yaml ` --driver-config-json $config ``` +To hide files or directories inside the policy's filesystem grants, such as +credential files in a writable working directory, add `denied_paths`: + +```shell +$config = '{"mxc":{"command":["C:\\Windows\\System32\\cmd.exe","/d","/c","agent.cmd"],"cwd":"C:\\work","denied_paths":["C:\\work\\profile\\.env","C:\\work\\profile\\auth.json"]}}' +``` + +The workload cannot read or write a denied file, list a denied directory, or +create a denied path. A denied file's name, size, timestamps and attributes +remain visible to the workload, and a hard link to the file at another path +the workload can read still exposes its contents. Each entry must be an +absolute path that starts with a drive letter and whose parent directory +exists, and must not equal or contain a filesystem grant, compared after +resolving 8.3 short names and junctions and ignoring letter case. The driver +rejects other entries before provisioning. Denied paths are fixed at sandbox +creation. + Supply workload environment through `sandbox create --env` or `--env-from`; it is not part of gateway configuration. The command runs from host files, not an OCI workload image. From e01a7cf5900f4d333cb6a55aebc41f1ea08f1296 Mon Sep 17 00:00:00 2001 From: Jeffrey Quesnelle Date: Thu, 29 Oct 2026 18:40:53 -0400 Subject: [PATCH 2/2] fix(mxc): reject denied paths that a directory rename exposes MXC applies deniedPaths and filesystem grants to paths. The workload can rename a directory when the directory and its parent are both writable to it, and everything below that directory then sits at a path that no denied path or read-only grant names. Measured with MXC 1.0.0 and 1.0.1: with C:\work granted read-write and C:\work\profile\.env denied, the workload renames profile and reads the file under the new name. Preflight now rejects a denied path below such a directory: two or more levels below a read-write grant root, inside a read-write grant nested in another one, or below a read-only grant whose root is two or more levels inside a read-write grant. The most specific grant or denied path decides a directory's access. The error names the directory and says to deny it or to move the entry directly into the read-write grant root. Entries directly inside a read-write grant root whose parent the workload cannot write, and entries below a read-only grant root that cannot be renamed, are still accepted. Grants written as \\.\C:\, \\?\C:\, \??\C:\, \\?\GLOBALROOT\Device\... or \\?\Volume{...}\, which MXC resolves, are now resolved like drive paths before the overlap and rename checks, so a device-path grant through a junction no longer hides an overlap with a denied directory. While denied_paths is not empty, a UNC grant (MXC grants no access through UNC paths) or a grant that does not resolve to a local drive is rejected. wxc_exec_real gains a probe-gated test that passes seven layouts through preflight and, for each one it accepts, renames each denied path and every directory above it in a real ProcessContainer and checks that the denied contents stay unreadable. The docs state that MXC does not check the target of a rename against deniedPaths: in a writable directory the workload can rename its own file onto a denied name, which replaces a denied file or creates an absent denied path without exposing the original contents. Signed-off-by: Jeffrey Quesnelle --- crates/openshell-driver-mxc/README.md | 63 +- crates/openshell-driver-mxc/src/driver.rs | 580 ++++++++++++++++-- .../tests/wxc_exec_real.rs | 363 +++++++++++ docs/how-it-works/gateways/configuration.mdx | 34 +- 4 files changed, 965 insertions(+), 75 deletions(-) diff --git a/crates/openshell-driver-mxc/README.md b/crates/openshell-driver-mxc/README.md index e4d5cc598d..64ef960b42 100644 --- a/crates/openshell-driver-mxc/README.md +++ b/crates/openshell-driver-mxc/README.md @@ -148,11 +148,11 @@ command is supplied, the driver passes `mxc.command` to the supervisor rather than starting a scratch workload. An explicit generic canonical command retains precedence. Environment belongs in `--env` or `--env-from`, not gateway configuration. -The optional `mxc.denied_paths` lists paths the workload cannot use, including -inside the policy's filesystem grants: it cannot read or write a denied file, -list a denied directory, or create a denied path. The driver passes the list to -MXC as `filesystem.deniedPaths`. For example, to keep a profile directory -writable while hiding its credential files: +The optional `mxc.denied_paths` lists paths the workload cannot open, including +inside the policy's filesystem grants: it cannot read, write or delete a denied +file, list a denied directory, or create a denied path by name. The driver +passes the list to MXC as `filesystem.deniedPaths`. For example, with a policy +that grants `C:\work\profile` read-write, to hide the credential files in it: ```powershell $config = '{"mxc":{"command":["C:\\Windows\\System32\\cmd.exe","/d","/c","agent.cmd"],"cwd":"C:\\work","denied_paths":["C:\\work\\profile\\.env","C:\\work\\profile\\auth.json"]}}' @@ -160,22 +160,43 @@ $config = '{"mxc":{"command":["C:\\Windows\\System32\\cmd.exe","/d","/c","agent. Each entry must be an absolute path that starts with a drive letter, such as `C:\work\profile\.env`; forward slashes are converted to backslashes. A denied -path may be absent, in which case the workload cannot create it, but its parent -directory must exist because MXC refuses to start the ProcessContainer -otherwise; to cover a missing directory, deny the directory itself. An entry -must not equal or contain a filesystem grant, including the generation staging -directory under `cwd` and the sandbox executable: MXC drops a grant that equals -a denied path and keeps access to a grant nested inside a denied directory. -MXC matches paths after resolving 8.3 short names and junctions and ignoring -letter case, so preflight compares them the same way (following symbolic links -too) and rejects these cases before provisioning. The parts of a path that -do not exist, or that cannot be resolved, are compared as written. - -A denied file's name, size, timestamps and attributes remain visible to the -workload, for example when it lists the parent directory; only the contents -are protected. A deny covers the listed path: a hard link to the same file, -created on the host before launch at a path the workload can read, still -exposes the contents. +path may be absent, but its parent directory must exist because MXC refuses to +start the ProcessContainer otherwise; to cover a missing directory, deny the +directory itself. An entry must not equal or contain a filesystem grant, +including the generation staging directory under `cwd` and the sandbox +executable: MXC drops a grant that equals a denied path and keeps access to a +grant nested inside a denied directory. + +MXC applies denied paths and grants to paths. The workload can rename a +directory when the directory and its parent are both writable to it, and +everything below that directory then sits at a path no denied path or read-only +grant names. Preflight therefore rejects an entry below such a directory: two +or more levels below a read-write grant root, inside a read-write grant nested +in another read-write grant, or below a read-only grant whose root is two or +more levels inside a read-write grant. The error names the directory; deny it +instead, or move the entry directly into the read-write grant root. Entries +directly inside a read-write grant root whose parent the workload cannot write, +and entries below a read-only grant root that cannot be renamed, are accepted. + +MXC resolves 8.3 short names, junctions and letter case, and accepts grants in +the `\\.\C:\`, `\\?\C:\`, `\??\C:\`, `\\?\GLOBALROOT\Device\...` and +`\\?\Volume{...}\` spellings, so preflight compares resolved drive paths +(following symbolic links too). The parts of a drive path that do not exist, or +that cannot be resolved, are compared as written. While `denied_paths` is not +empty, preflight rejects a grant it cannot resolve to a drive path: a UNC path, +which MXC grants no access through, a path without a drive letter, or a device +path that does not resolve. + +A denied path protects contents. The file's name, size, timestamps and +attributes remain visible to the workload, for example when it lists the parent +directory, and a hard link to the same file, created on the host before launch +at a path the workload can read, still exposes the contents. MXC does not check +the target of a rename against `deniedPaths`: when the parent directory is +writable, the workload can rename one of its own files onto a denied name, +which replaces a denied file on the host, or creates the file or directory when +it is absent. The original contents stay unreadable, and an existing denied +directory cannot be replaced. Denying the directory that holds such files +prevents this, because nothing can be renamed into a denied directory. Denied paths only remove access and are fixed at sandbox creation. They require a host whose `wxc-exec.exe --probe` reports diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 6fdfdc7a3e..03bdb88d9a 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -160,9 +160,8 @@ struct MxcSandboxConfig { command: Vec, #[serde(default)] cwd: String, - /// Files and directories the workload cannot read, write, list, or - /// create, including inside filesystem grants. Sent to MXC as - /// `filesystem.deniedPaths`. + /// Files and directories the workload cannot open, including inside + /// filesystem grants. Sent to MXC as `filesystem.deniedPaths`. #[serde(default)] denied_paths: Vec, } @@ -370,20 +369,27 @@ impl MxcComputeBackend { "127.0.0.1:3128".parse().expect("fixed proxy address"), )?; // Denied paths are checked against every grant MXC receives: the - // policy grants, the generation staging directory under `cwd`, and the - // sandbox executable. + // policy grants, the generation staging directory under `cwd` + // (read-write, checked as `cwd\.openshell-runtime` because the + // generation is not known yet), and the sandbox executable + // (read-only). let staging_root = Path::new(&config.cwd) .join(BOUNDARY_STAGING_DIR) .display() .to_string(); - let grants: Vec<&str> = mapped + let read_write: Vec<&str> = mapped .readwrite_paths .iter() - .chain(&mapped.readonly_paths) - .chain([&staging_root, &self.config.sandbox_binary_path]) + .chain([&staging_root]) .map(String::as_str) .collect(); - check_denied_paths(&config.denied_paths, &grants)?; + let read_only: Vec<&str> = mapped + .readonly_paths + .iter() + .chain([&self.config.sandbox_binary_path]) + .map(String::as_str) + .collect(); + check_denied_paths(&config.denied_paths, &read_write, &read_only)?; Ok(()) } @@ -1055,6 +1061,24 @@ fn has_drive_root(path: &str) -> bool { bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'\\' } +/// Access a grant or a denied path gives the workload. For rules that name +/// the same path, a denied path removes the grant, and a read-write grant is +/// taken to win over a read-only one, which can only make the rename check +/// below stricter. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +enum Access { + ReadOnly, + ReadWrite, + Denied, +} + +/// A filesystem grant or denied path as written, with its resolved key. +struct FilesystemRule<'a> { + path: &'a str, + key: Vec, + access: Access, +} + /// Check denied paths against the grants MXC receives and the host file /// system before any sandbox state is created. /// @@ -1065,22 +1089,50 @@ fn has_drive_root(path: &str) -> bool { /// grant, compared after resolving 8.3 short names, junctions and symbolic /// links, and ignoring letter case. MXC also refuses to start a /// `ProcessContainer` that denies a path whose parent directory does not -/// exist; the denied path itself may be absent, and MXC then prevents its -/// creation. -fn check_denied_paths(denied_paths: &[String], grants: &[&str]) -> Result<(), tonic::Status> { +/// exist; the denied path itself may be absent. +/// +/// MXC enforces denied paths and grants by path. The workload can rename a +/// directory when the directory and its parent are both writable to it, and +/// everything below the directory then sits at a path that no denied path or +/// read-only grant names. A denied path below such a directory is rejected. +fn check_denied_paths( + denied_paths: &[String], + read_write: &[&str], + read_only: &[&str], +) -> Result<(), tonic::Status> { if denied_paths.is_empty() { return Ok(()); } - let grants: Vec<(&str, Vec)> = grants + let mut rules = Vec::new(); + for (grants, access) in [ + (read_write, Access::ReadWrite), + (read_only, Access::ReadOnly), + ] { + for grant in grants { + let resolved = resolve_grant(grant).map_err(|reason| { + tonic::Status::invalid_argument(format!( + "mxc driver_config.denied_paths cannot be checked against the filesystem grant '{grant}': {reason}" + )) + })?; + rules.push(FilesystemRule { + path: grant, + key: path_key(&resolved), + access, + }); + } + } + let denied: Vec<(&str, String)> = denied_paths .iter() - .map(|grant| (*grant, resolved_path_key(grant))) + .map(|path| (path.as_str(), resolve_path(path))) .collect(); - for denied in denied_paths { - let denied_key = resolved_path_key(denied); - if let Some((grant, _)) = grants - .iter() - .find(|(_, grant_key)| grant_key.starts_with(&denied_key)) - { + rules.extend(denied.iter().map(|(path, resolved)| FilesystemRule { + path, + key: path_key(resolved), + access: Access::Denied, + })); + for (denied, resolved) in &denied { + let denied_key = path_key(resolved); + if let Some(grant) = grant_within(&rules, &denied_key) { return Err(tonic::Status::invalid_argument(format!( "mxc driver_config.denied_paths entry '{denied}' equals or contains the filesystem grant '{grant}'; deny paths below the grant instead" ))); @@ -1105,31 +1157,137 @@ fn check_denied_paths(denied_paths: &[String], grants: &[&str]) -> Result<(), to "mxc driver_config.denied_paths entry '{denied}' has no existing parent directory, which MXC requires{remedy}" ))); } + if let Some(depth) = renamable_ancestor(&rules, &denied_key) { + let components: Vec<&str> = path_components(resolved).collect(); + let directory = join_components(&components[..depth]); + let parent = join_components(&components[..depth - 1]); + // Denying the directory is only possible when no grant lies + // within it. + let remedy = if grant_within(&rules, &denied_key[..depth]).is_some() { + format!("Move the entry directly into '{parent}'") + } else { + format!("Deny '{directory}' instead, or move the entry directly into '{parent}'") + }; + return Err(tonic::Status::invalid_argument(format!( + "mxc driver_config.denied_paths entry '{denied}' is inside '{directory}', which the workload can rename because both it and its parent are writable; the entry is then readable under a new path. {remedy}" + ))); + } } Ok(()) } +/// The first grant, as written, that equals `key` or lies within it. +fn grant_within<'a>(rules: &[FilesystemRule<'a>], key: &[String]) -> Option<&'a str> { + rules + .iter() + .find(|rule| rule.access != Access::Denied && rule.key.starts_with(key)) + .map(|rule| rule.path) +} + +/// Access the most specific rule that names `key` or a directory above it +/// gives the workload; `None` outside every rule. +fn access_at(rules: &[FilesystemRule<'_>], key: &[String]) -> Option { + rules + .iter() + .filter(|rule| key.starts_with(&rule.key)) + .max_by_key(|rule| (rule.key.len(), rule.access)) + .map(|rule| rule.access) +} + +/// The outermost directory above `denied` that the workload can rename, as +/// its number of components: one that is writable and whose parent is +/// writable. A drive root has no parent and cannot be renamed. +fn renamable_ancestor(rules: &[FilesystemRule<'_>], denied: &[String]) -> Option { + let writable = |key: &[String]| access_at(rules, key) == Some(Access::ReadWrite); + (2..denied.len()).find(|&depth| writable(&denied[..depth]) && writable(&denied[..depth - 1])) +} + /// Prefix of the verbatim paths `std::fs::canonicalize` returns on Windows. const VERBATIM_PREFIX: &str = r"\\?\"; -/// Path components that identify the file system object a Windows path -/// names, for comparing denied paths with grants. A path that starts with a -/// drive letter is resolved with `std::fs::canonicalize`, which follows -/// junctions and symbolic links and returns the long name and on-disk letter -/// case of every component. When the path does not exist, its deepest -/// existing ancestor is resolved and the remaining components are appended -/// as written. Components that cannot be resolved this way, and paths without -/// a drive letter, are compared as written. -fn resolved_path_key(path: &str) -> Vec { - let path = crate::policy::normalize_path(path); - let path = path.strip_prefix(VERBATIM_PREFIX).unwrap_or(&path); +/// Prefixes of the Win32 file, Win32 device, and NT object namespaces. +const NAMESPACE_PREFIXES: [&str; 3] = [VERBATIM_PREFIX, r"\\.\", r"\??\"]; + +/// Resolve a filesystem grant to the path that starts with a drive letter +/// and names the same file system object, for comparison with denied paths. +/// MXC also accepts grants in the device and object namespaces, such as +/// `\\.\C:\work`, `\??\C:\work`, `\\?\GLOBALROOT\Device\HarddiskVolume3\work` +/// and `\\?\Volume{...}\work`, and follows junctions in them, so those are +/// resolved through the file system as well. MXC grants nothing through a UNC +/// path, and a path without a drive depends on the current drive; both are +/// rejected, as is a spelling the file system does not resolve to a drive. +fn resolve_grant(grant: &str) -> Result { + let normalized = crate::policy::normalize_path(grant); + let path = strip_drive_namespace(&normalized); + if has_drive_root(path) { + return Ok(resolve_path(path)); + } + if is_unc_path(path) { + return Err("MXC grants no access through a UNC path; grant the local path instead"); + } + if NAMESPACE_PREFIXES + .iter() + .any(|prefix| path.starts_with(prefix)) + && let Some(resolved) = resolve_existing_prefix(Path::new(path)) + { + let resolved = resolved.display().to_string(); + let resolved = resolved.strip_prefix(VERBATIM_PREFIX).unwrap_or(&resolved); + if has_drive_root(resolved) { + return Ok(resolved.to_string()); + } + } + Err( + "it does not resolve to a path on a local drive; write the grant as a path such as C:\\work", + ) +} + +/// The drive path inside a `\\?\C:\`, `\\.\C:\` or `\??\C:\` spelling, or +/// the path unchanged. +fn strip_drive_namespace(path: &str) -> &str { + NAMESPACE_PREFIXES + .iter() + .find_map(|prefix| { + path.strip_prefix(prefix) + .filter(|rest| has_drive_root(rest)) + }) + .unwrap_or(path) +} + +/// Whether a backslash-separated path is a UNC path, in any namespace. +fn is_unc_path(path: &str) -> bool { + NAMESPACE_PREFIXES + .iter() + .find_map(|prefix| path.strip_prefix(prefix)) + .map_or_else( + || path.starts_with(r"\\"), + |rest| { + rest.get(..4) + .is_some_and(|unc| unc.eq_ignore_ascii_case(r"UNC\")) + }, + ) +} + +/// The path that names the same file system object as `path`, for comparing +/// denied paths with grants. A path that starts with a drive letter, also +/// after a `\\?\`, `\\.\` or `\??\` prefix, is resolved with +/// `std::fs::canonicalize`, which follows junctions and symbolic links and +/// returns the long name and on-disk letter case of every component. When the +/// path does not exist, its deepest existing ancestor is resolved and the +/// remaining components are appended as written. Components that cannot be +/// resolved this way, and other paths, are returned as written. +fn resolve_path(path: &str) -> String { + let normalized = crate::policy::normalize_path(path); + let path = strip_drive_namespace(&normalized); if has_drive_root(path) && let Some(resolved) = resolve_existing_prefix(Path::new(path)) { let resolved = resolved.display().to_string(); - return path_key(resolved.strip_prefix(VERBATIM_PREFIX).unwrap_or(&resolved)); + return resolved + .strip_prefix(VERBATIM_PREFIX) + .unwrap_or(&resolved) + .to_string(); } - path_key(path) + path.to_string() } /// Canonicalize the deepest existing ancestor of `path` and append the @@ -1150,12 +1308,27 @@ fn resolve_existing_prefix(path: &Path) -> Option { None } +/// The components of a backslash-separated path, without empty and `.` +/// components. +fn path_components(path: &str) -> impl Iterator { + path.split('\\') + .filter(|component| !component.is_empty() && *component != ".") +} + +/// Join path components with backslashes; a drive alone becomes its root. +fn join_components(components: &[&str]) -> String { + let joined = components.join("\\"); + if components.len() == 1 { + joined + "\\" + } else { + joined + } +} + /// Case-insensitive path components for comparing Windows paths. Letter case /// is ignored for all Unicode letters, not only ASCII. fn path_key(path: &str) -> Vec { - crate::policy::normalize_path(path) - .split('\\') - .filter(|component| !component.is_empty() && *component != ".") + path_components(&crate::policy::normalize_path(path)) .map(str::to_lowercase) .collect() } @@ -1902,6 +2075,10 @@ mod tests { "C:work\\.env".into(), "\\\\server\\share\\.env".into(), "\\\\?\\C:\\work\\.env".into(), + "\\\\.\\C:\\work\\.env".into(), + "\\??\\C:\\work\\.env".into(), + "\\\\?\\GLOBALROOT\\Device\\HarddiskVolume1\\work\\.env".into(), + "\\\\?\\Volume{00000000-0000-0000-0000-000000000000}\\work\\.env".into(), format!(" {base}\\.env"), format!("{base}\\.env "), format!("{base}\\.env."), @@ -2099,11 +2276,11 @@ mod tests { let short_grant = PathBuf::from(format!("{short_dir}\\work")); let reversed = with_filesystem_grants(sandbox.clone(), &[&short_grant], &[]); assert_denied_path_covers_grant(&backend, &reversed, &display(&work), &short_grant); - // A file inside the grant can be denied through the 8.3 name. + // A file directly inside the grant can be denied through the 8.3 name. backend .validate_sandbox_create(&with_denied_paths( sandbox, - &[format!("{short_dir}\\work\\profile\\.env")], + &[format!("{short_dir}\\work\\.env")], )) .expect("a short spelling of a path inside a grant passes preflight"); assert!(!backend.config.state_dir.exists()); @@ -2127,12 +2304,9 @@ mod tests { // A grant spelled through the junction is the same directory. let through_link = with_filesystem_grants(sandbox.clone(), &[&link], &[]); assert_denied_path_covers_grant(&backend, &through_link, &display(&work), &link); - // A file inside the grant can be denied through the junction. + // A file directly inside the grant can be denied through the junction. backend - .validate_sandbox_create(&with_denied_paths( - sandbox, - &[display(&link.join("profile").join(".env"))], - )) + .validate_sandbox_create(&with_denied_paths(sandbox, &[display(&link.join(".env"))])) .expect("a path below the grant through a junction passes preflight"); assert!(!backend.config.state_dir.exists()); } @@ -2163,9 +2337,327 @@ mod tests { let sandbox = with_filesystem_grants(sandbox, &[&grant], &[]); let denied = format!("{}:/WORK/", drive.to_ascii_lowercase()); assert_denied_path_covers_grant(&backend, &sandbox, &denied, &grant); + // The same grant in the device and object namespaces. + for grant in [format!(r"\\.\{drive}:\Work"), format!(r"\??\{drive}:\Work")] { + let grant = PathBuf::from(grant); + let sandbox = with_filesystem_grants(sandbox.clone(), &[&grant], &[]); + assert_denied_path_covers_grant(&backend, &sandbox, &denied, &grant); + } // A resolved path drops the `\\?\` prefix `canonicalize` adds, so it // compares in the same form as a path compared as written. - assert_eq!(resolved_path_key(r"C:\"), path_key(r"C:\")); + assert_eq!(resolve_path(r"C:\"), r"C:\"); + assert!(!backend.config.state_dir.exists()); + } + + /// The resolved drive form of an existing path, as `canonicalize` returns + /// it without the `\\?\` prefix. + fn canonical(path: &Path) -> String { + let resolved = display(&std::fs::canonicalize(path).expect("canonicalize")); + resolved + .strip_prefix(r"\\?\") + .unwrap_or(&resolved) + .to_string() + } + + /// Create a host junction at `link` that points to `target`. + fn junction(link: &Path, target: &Path) { + let (created, output) = run_cmd(&format!( + "mklink /J \"{}\" \"{}\"", + link.display(), + target.display() + )); + assert!(created, "mklink /J failed: {output}"); + } + + /// Assert that preflight rejects `denied` because the workload can rename + /// `directory`, and that the error says what to deny or move instead. + fn assert_renamable_directory( + backend: &MxcComputeBackend, + sandbox: &DriverSandbox, + denied: &str, + directory: &Path, + can_deny_directory: bool, + ) { + let error = backend + .validate_sandbox_create(&with_denied_paths(sandbox.clone(), &[denied.to_string()])) + .expect_err("a denied path below a renamable directory must be rejected"); + assert_eq!(error.code(), tonic::Code::InvalidArgument, "{denied}"); + let message = error.message(); + let parent = canonical(directory.parent().unwrap()); + let directory = canonical(directory); + assert!( + message.contains(&format!( + "entry '{denied}' is inside '{directory}', which the workload can rename" + )), + "{message}" + ); + let remedy = if can_deny_directory { + format!("Deny '{directory}' instead, or move the entry directly into '{parent}'") + } else { + format!("Move the entry directly into '{parent}'") + }; + assert!(message.ends_with(&remedy), "{message}"); + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_paths_below_a_renamable_directory_are_rejected() { + let (backend, sandbox, dir) = preflight_fixture(); + let work = dir.path().join("work"); + let profile = work.join("profile"); + let tools = work.join("tools"); + let bin = tools.join("bin"); + std::fs::create_dir_all(profile.join("sub")).unwrap(); + std::fs::create_dir_all(&bin).unwrap(); + std::fs::write(profile.join(".env"), "fixture").unwrap(); + std::fs::write(profile.join("sub").join(".env"), "fixture").unwrap(); + let read_write = with_filesystem_grants(sandbox.clone(), &[&work], &[]); + // Two and three levels below a read-write grant root, present or + // absent: the outermost directory below the root can be renamed. + for denied in [ + profile.join(".env"), + profile.join("sub").join(".env"), + profile.join("auth.json"), + ] { + assert_renamable_directory(&backend, &read_write, &display(&denied), &profile, true); + } + // The root of a read-write grant nested in another one can be + // renamed; denying it would remove that grant. + let nested = with_filesystem_grants(sandbox.clone(), &[&work, &profile], &[]); + let denied = display(&profile.join(".env")); + assert_renamable_directory(&backend, &nested, &denied, &profile, false); + // A read-only grant two levels inside a read-write grant: the + // read-write directory above the read-only grant root can be renamed. + let read_only = with_filesystem_grants(sandbox.clone(), &[&work], &[&bin]); + let denied = display(&bin.join("key.pem")); + assert_renamable_directory(&backend, &read_only, &denied, &tools, false); + // A path granted both read-write and read-only counts as read-write. + let both = with_filesystem_grants(sandbox.clone(), &[&work, &profile], &[&profile]); + let denied = display(&profile.join(".env")); + assert_renamable_directory(&backend, &both, &denied, &profile, false); + // The generation staging directory under `cwd` is read-write, checked + // from `cwd\.openshell-runtime` because the generation is not known yet. + let generation = dir.path().join(BOUNDARY_STAGING_DIR).join("generation"); + std::fs::create_dir_all(&generation).unwrap(); + let denied = display(&generation.join("x.env")); + assert_renamable_directory(&backend, &sandbox, &denied, &generation, true); + // A read-write grant on the drive root: the outermost directory + // below the root can be renamed. + let resolved = canonical(dir.path()); + let root = PathBuf::from(&resolved[..3]); + let outermost = root.join(resolved[3..].split('\\').next().unwrap()); + let drive = with_filesystem_grants(sandbox, &[&root], &[]); + let denied = display(&work.join(".env")); + assert_renamable_directory(&backend, &drive, &denied, &outermost, false); + } + + #[test] + fn driver_config_denied_paths_out_of_rename_reach_pass_preflight() { + let (backend, sandbox, dir) = preflight_fixture(); + let work = dir.path().join("work"); + let profile = work.join("profile"); + let secrets = work.join("secrets"); + let inner = secrets.join("inner"); + std::fs::create_dir_all(profile.join("sub")).unwrap(); + std::fs::create_dir_all(&inner).unwrap(); + std::fs::write(work.join(".env"), "fixture").unwrap(); + std::fs::write(profile.join(".env"), "fixture").unwrap(); + let env = work.join(".env"); + let auth = work.join("auth.json"); + let profile_env = profile.join(".env"); + let nested_env = profile.join("sub").join(".env"); + let cases: [(&[&Path], &[&Path], &[&Path]); 4] = [ + // Directly inside a read-write grant root whose parent the + // workload cannot write: a file, an absent file, directories, + // and a path inside a denied directory. + (&[&work], &[], &[&env, &auth, &profile, &secrets, &inner]), + // Below a read-only grant root. + (&[], &[&work], &[&nested_env]), + // Below a read-only grant directly inside a read-write grant. + (&[&work], &[&profile], &[&profile_env, &nested_env]), + // Directly inside a read-write grant within a read-only grant. + (&[&profile], &[&work], &[&profile_env]), + ]; + for (read_write, read_only, denied) in cases { + let denied: Vec = denied.iter().map(|path| display(path)).collect(); + let sandbox = with_denied_paths( + with_filesystem_grants(sandbox.clone(), read_write, read_only), + &denied, + ); + backend + .validate_sandbox_create(&sandbox) + .unwrap_or_else(|error| panic!("{denied:?}: {}", error.message())); + } + assert!(!backend.config.state_dir.exists()); + } + + #[test] + fn driver_config_denied_paths_rename_check_resolves_aliases() { + let (backend, sandbox, dir) = preflight_fixture(); + let work = dir.path().join("work area"); + let profile = work.join("profile directory"); + std::fs::create_dir_all(&profile).unwrap(); + std::fs::write(profile.join(".env"), "fixture").unwrap(); + let env = display(&profile.join(".env")); + let read_write = with_filesystem_grants(sandbox.clone(), &[&work], &[]); + // The denied path through a host junction to its directory, and in + // other letter case. + let profile_link = dir.path().join("profile link"); + junction(&profile_link, &profile); + for denied in [display(&profile_link.join(".env")), env.to_uppercase()] { + assert_renamable_directory(&backend, &read_write, &denied, &profile, true); + } + // The read-write grant through a host junction. + let work_link = dir.path().join("work link"); + junction(&work_link, &work); + let through_link = with_filesystem_grants(sandbox.clone(), &[&work_link], &[]); + assert_renamable_directory(&backend, &through_link, &env, &profile, true); + // The denied path's directory and the grant through 8.3 short names. + let (Some(short_profile), Some(short_work)) = (short_path(&profile), short_path(&work)) + else { + eprintln!( + "SKIP: no 8.3 short names under {}; enable 8.3 name creation on its volume to run this part", + dir.path().display() + ); + return; + }; + let denied = format!("{short_profile}\\.env"); + assert_renamable_directory(&backend, &read_write, &denied, &profile, true); + let short_work = PathBuf::from(short_work); + let short_grant = with_filesystem_grants(sandbox, &[&short_work], &[]); + assert_renamable_directory(&backend, &short_grant, &env, &profile, true); + } + + /// `\\?\Volume{...}\` for the volume that holds `path`, from `mountvol`. + fn volume_guid_path(path: &str) -> Option { + let (_, output) = run_cmd(&format!("mountvol {}\\ /L", &path[..2])); + let guid = output.trim(); + guid.starts_with(r"\\?\Volume{").then(|| guid.to_string()) + } + + /// `\Device\HarddiskVolumeN` for the volume that holds `path`. + fn volume_device(path: &str) -> Option { + let root = std::fs::canonicalize(format!("{}\\", &path[..2])).ok()?; + (1..=64) + .map(|number| format!(r"\Device\HarddiskVolume{number}")) + .find(|device| { + std::fs::canonicalize(format!(r"\\?\GLOBALROOT{device}\")) + .is_ok_and(|resolved| resolved == root) + }) + } + + /// Spellings of an existing path in the Win32 file and device namespaces + /// and the NT object namespace, all of which MXC accepts as grants. + fn namespace_spellings(path: &Path) -> Vec { + let path = canonical(path); + let rest = &path[2..]; + let mut spellings = vec![ + format!(r"\\.\{path}"), + format!(r"\\?\{path}"), + format!(r"\??\{path}"), + ]; + match volume_device(&path) { + Some(device) => spellings.extend([ + format!(r"\\?\GLOBALROOT{device}{rest}"), + format!(r"\\.\GLOBALROOT{device}{rest}"), + ]), + None => eprintln!("SKIP: no \\Device\\HarddiskVolumeN name found for {path}"), + } + match volume_guid_path(&path) { + Some(guid) => spellings.extend([ + format!("{guid}{}", &rest[1..]), + format!(r"\\.\{}{}", &guid[4..], &rest[1..]), + ]), + None => eprintln!("SKIP: no volume GUID path found for {path}"), + } + spellings + } + + #[test] + fn driver_config_denied_paths_resolve_namespace_grants() { + let (backend, sandbox, dir) = preflight_fixture(); + let secret = dir.path().join("secret"); + let nested = secret.join("nested"); + let work = dir.path().join("work"); + let profile = work.join("profile"); + std::fs::create_dir_all(&nested).unwrap(); + std::fs::create_dir_all(&profile).unwrap(); + let link = dir.path().join("link"); + junction(&link, &nested); + // A read-only grant through a host junction, written in another + // namespace, names a directory inside the denied directory. + for grant in namespace_spellings(&link) { + let grant = PathBuf::from(grant); + let sandbox = with_filesystem_grants(sandbox.clone(), &[], &[&grant]); + assert_denied_path_covers_grant(&backend, &sandbox, &display(&secret), &grant); + } + // A read-write grant written in another namespace counts for the + // rename check, and a path directly inside it can be denied. + for grant in namespace_spellings(&work) { + let grant = PathBuf::from(grant); + let sandbox = with_filesystem_grants(sandbox.clone(), &[&grant], &[]); + let denied = display(&profile.join(".env")); + assert_renamable_directory(&backend, &sandbox, &denied, &profile, true); + backend + .validate_sandbox_create(&with_denied_paths( + sandbox, + &[display(&work.join(".env"))], + )) + .unwrap_or_else(|error| panic!("{}: {}", grant.display(), error.message())); + } + } + + #[test] + fn driver_config_denied_paths_reject_grants_that_do_not_resolve() { + let (backend, sandbox, dir) = preflight_fixture(); + let work = dir.path().join("work"); + std::fs::create_dir(&work).unwrap(); + let path = canonical(&work); + let (drive, rest) = path.split_at(2); + let share = format!("{}$", &drive[..1]); + let denied = display(&work.join(".env")); + let unc = "MXC grants no access through a UNC path; grant the local path instead"; + let unresolved = "it does not resolve to a path on a local drive"; + for (grant, reason) in [ + (format!(r"\\localhost\{share}{rest}"), unc), + (format!(r"\\?\UNC\localhost\{share}{rest}"), unc), + (format!(r"\\.\UNC\localhost\{share}{rest}"), unc), + // No drive: the path depends on the current drive. + (rest.to_string(), unresolved), + ( + r"\\?\Volume{00000000-0000-0000-0000-000000000000}\work".to_string(), + unresolved, + ), + ( + r"\\?\GLOBALROOT\Device\NoSuchVolume\work".to_string(), + unresolved, + ), + // A UNC path through the multiple UNC provider device. + ( + format!(r"\\?\GLOBALROOT\Device\Mup\localhost\{share}{rest}"), + unresolved, + ), + (r"\\.\pipe\openshell-no-such-pipe".to_string(), unresolved), + ] { + let grant = PathBuf::from(grant); + let sandbox = with_filesystem_grants(sandbox.clone(), &[&grant], &[]); + // Without denied paths the grant reaches MXC unchanged. + backend + .validate_sandbox_create(&sandbox) + .expect("nothing to check without denied paths"); + let error = backend + .validate_sandbox_create(&with_denied_paths(sandbox, std::slice::from_ref(&denied))) + .expect_err("a grant that does not resolve must be rejected"); + assert_eq!(error.code(), tonic::Code::InvalidArgument); + assert!( + error.message().starts_with(&format!( + "mxc driver_config.denied_paths cannot be checked against the filesystem grant '{}': {reason}", + grant.display() + )), + "{}", + error.message() + ); + } assert!(!backend.config.state_dir.exists()); } diff --git a/crates/openshell-driver-mxc/tests/wxc_exec_real.rs b/crates/openshell-driver-mxc/tests/wxc_exec_real.rs index 73d17f6ea6..a216beaa8f 100644 --- a/crates/openshell-driver-mxc/tests/wxc_exec_real.rs +++ b/crates/openshell-driver-mxc/tests/wxc_exec_real.rs @@ -1005,6 +1005,369 @@ fn pc_oneshot_out_of_policy_write_denied() { ); } +// ── Denied paths and directory renames ─────────────────────────────────────── + +/// Set in the environment of a `ProcessContainer` that runs this test binary +/// as the workload of `pc_accepted_denied_paths_survive_directory_renames`. +/// The value is the JSON plan `denied_paths_rename_workload` follows. +const RENAME_WORKLOAD_PLAN: &str = "OPENSHELL_MXC_RENAME_WORKLOAD_PLAN"; + +/// Contents of every denied file in the rename fixtures. +const RENAME_SECRET: &str = "openshell-mxc-rename-fixture-secret"; + +/// Workload half of `pc_accepted_denied_paths_survive_directory_renames`. It +/// does nothing unless `RENAME_WORKLOAD_PLAN` is set. With a plan it reads +/// each protected file, then renames each listed path through the Win32 +/// rename call, reads every protected file below it at its new path, renames +/// it back, and writes what happened to the report file. Command-shell `ren` and `move` cannot stand in for this: in a +/// `ProcessContainer` they fail for every path in a read-write grant, with or +/// without denied paths. +#[test] +#[ignore = "requires real wxc-exec"] +fn denied_paths_rename_workload() { + let Ok(plan) = std::env::var(RENAME_WORKLOAD_PLAN) else { + return; + }; + let plan: serde_json::Value = serde_json::from_str(&plan).expect("workload plan"); + let strings = |key: &str| -> Vec { + plan[key] + .as_array() + .expect("plan list") + .iter() + .map(|value| PathBuf::from(value.as_str().expect("plan path"))) + .collect() + }; + let (protected, renamed) = (strings("protected"), strings("renamed")); + let read = |path: &Path| match std::fs::read_to_string(path) { + Ok(contents) => format!("read {}: {contents}", path.display()), + Err(error) => format!("read {}: error {error}", path.display()), + }; + let mut report = Vec::new(); + for path in &protected { + report.push(read(path)); + } + for target in &renamed { + let mut moved = target.clone().into_os_string(); + moved.push(".moved"); + let moved = PathBuf::from(moved); + if let Err(error) = std::fs::rename(target, &moved) { + report.push(format!("rename {}: error {error}", target.display())); + continue; + } + report.push(format!("rename {}: renamed", target.display())); + for path in &protected { + if let Ok(rest) = path.strip_prefix(target) { + report.push(read(&moved.join(rest))); + } + } + if let Err(error) = std::fs::rename(&moved, target) { + report.push(format!("rename back {}: error {error}", moved.display())); + } + } + report.push("done".to_string()); + std::fs::write( + plan["report"].as_str().expect("report path"), + report.join("\n"), + ) + .expect("write report"); +} + +/// One filesystem layout for `pc_accepted_denied_paths_survive_directory_renames`, +/// relative to its fixture directory. +struct RenameShape { + name: &'static str, + read_write: &'static [&'static str], + read_only: &'static [&'static str], + denied: &'static [&'static str], +} + +/// The driver's preflight result for a layout under `root`. +fn rename_shape_preflight( + wxc: &Path, + root: &Path, + shape: &RenameShape, +) -> Result<(), tonic::Status> { + use openshell_core::proto::compute::v1::{ + DriverSandbox, DriverSandboxSpec, DriverSandboxTemplate, + }; + let absolute = |paths: &[&str]| -> Vec { + paths + .iter() + .map(|path| root.join(path).to_string_lossy().into_owned()) + .collect() + }; + let test_binary = std::env::current_exe() + .expect("test binary") + .to_string_lossy() + .into_owned(); + let backend = openshell_driver_mxc::MxcComputeBackend::new( + "rename-shapes", + openshell_driver_mxc::MxcComputeConfig { + allow_driver_config: true, + wxc_exec_path: wxc.to_string_lossy().into_owned(), + grpc_endpoint: "http://127.0.0.1:1".into(), + supervisor_binary_path: test_binary.clone(), + sandbox_binary_path: test_binary, + state_dir: root.join("not-created"), + ..Default::default() + }, + ); + let serde_json::Value::Object(config) = serde_json::json!({ + "command": ["C:\\Windows\\System32\\cmd.exe", "/c", "exit 0"], + "cwd": root.join("run"), + "denied_paths": absolute(shape.denied), + }) else { + unreachable!() + }; + let sandbox = DriverSandbox { + id: "rename-shapes".into(), + name: "rename-shapes".into(), + spec: Some(DriverSandboxSpec { + policy: Some(SandboxPolicy { + version: 1, + filesystem: Some(FilesystemPolicy { + read_write: absolute(shape.read_write), + read_only: absolute(shape.read_only), + ..Default::default() + }), + ..Default::default() + }), + template: Some(DriverSandboxTemplate { + driver_config: Some( + openshell_core::proto_struct::json_object_to_struct(config).expect("config"), + ), + ..Default::default() + }), + ..Default::default() + }), + ..Default::default() + }; + backend.validate_sandbox_create(&sandbox) +} + +/// Whether `wxc-exec --probe` reports a base container that enforces +/// `deniedPaths` without changing host ACLs. +fn probe_denied_paths(wxc: &Path) -> Result<(), String> { + let out = Command::new(wxc) + .arg("--probe") + .output() + .map_err(|e| format!("wxc-exec --probe spawn failed: {e}"))?; + let probe: serde_json::Value = serde_json::from_slice(&out.stdout) + .map_err(|e| format!("could not parse wxc-exec --probe JSON: {e}"))?; + let supported = probe + .pointer("/probes/baseContainerSupportsDenyPaths") + .and_then(serde_json::Value::as_bool); + let augmentation = probe["needsDaclAugmentation"].as_bool(); + if supported == Some(true) && augmentation == Some(false) { + Ok(()) + } else { + Err(format!( + "baseContainerSupportsDenyPaths={supported:?}, needsDaclAugmentation={augmentation:?}" + )) + } +} + +/// Every denied-path layout the driver accepts keeps the denied files +/// unreadable when the workload renames the directories above them (or the +/// denied paths themselves). The layouts include the ones the driver rejects +/// because a rename exposes them; on a driver that accepted those, this test +/// fails with the denied contents in the workload's report. +#[test] +#[ignore = "requires real wxc-exec"] +fn pc_accepted_denied_paths_survive_directory_renames() { + let Some(wxc) = wxc_path() else { + eprintln!("SKIP: wxc-exec not found"); + return; + }; + if let Err(reason) = probe_processcontainer(&wxc) { + eprintln!("SKIP: processcontainer not live: {reason}"); + return; + } + if let Err(reason) = probe_denied_paths(&wxc) { + eprintln!("SKIP: denied paths not enforced by the base container: {reason}"); + return; + } + let shapes = [ + RenameShape { + name: "two levels below a read-write grant", + read_write: &["work"], + read_only: &[], + denied: &["work\\profile\\.env"], + }, + RenameShape { + name: "directly inside a read-write grant nested in another", + read_write: &["work", "work\\profile"], + read_only: &[], + denied: &["work\\profile\\.env"], + }, + RenameShape { + name: "below a read-only grant two levels inside a read-write grant", + read_write: &["work"], + read_only: &["work\\profile\\sub"], + denied: &["work\\profile\\sub\\.env"], + }, + RenameShape { + name: "directly inside a read-write grant", + read_write: &["work"], + read_only: &[], + denied: &["work\\.env", "work\\secrets"], + }, + RenameShape { + name: "below a read-only grant", + read_write: &[], + read_only: &["work"], + denied: &["work\\profile\\.env"], + }, + RenameShape { + name: "below a read-only grant inside a read-write grant", + read_write: &["work"], + read_only: &["work\\profile"], + denied: &["work\\profile\\.env", "work\\profile\\sub\\.env"], + }, + RenameShape { + name: "directly inside a read-write grant within a read-only grant", + read_write: &["work\\profile"], + read_only: &["work"], + denied: &["work\\profile\\.env"], + }, + ]; + let must_accept = [ + "directly inside a read-write grant", + "below a read-only grant", + "below a read-only grant inside a read-write grant", + "directly inside a read-write grant within a read-only grant", + ]; + let test_binary = std::env::current_exe().expect("test binary"); + let mut failures = Vec::new(); + for (index, shape) in shapes.iter().enumerate() { + let fixture = tempfile::tempdir().expect("rename fixture"); + let root = fixture.path(); + let run = root.join("run"); + std::fs::create_dir_all(&run).unwrap(); + for directory in ["work\\profile\\sub", "work\\secrets"] { + std::fs::create_dir_all(root.join(directory)).unwrap(); + } + let secret_files = [ + "work\\.env", + "work\\secrets\\key.txt", + "work\\profile\\.env", + "work\\profile\\sub\\.env", + ]; + for file in secret_files { + std::fs::write(root.join(file), RENAME_SECRET).unwrap(); + } + match rename_shape_preflight(&wxc, root, shape) { + Err(status) => { + assert_eq!( + status.code(), + tonic::Code::InvalidArgument, + "{}: {}", + shape.name, + status.message() + ); + eprintln!("{}: refused at preflight: {}", shape.name, status.message()); + if must_accept.contains(&shape.name) { + failures.push(format!("{}: refused: {}", shape.name, status.message())); + } + continue; + } + Ok(()) => eprintln!("{}: accepted at preflight", shape.name), + } + // The workload renames each denied path and every directory above it + // inside the fixture, and reads the fixture files they protect. + let mut renamed: Vec = Vec::new(); + for denied in shape.denied { + for relative in Path::new(denied).ancestors() { + let path = root.join(relative); + if !relative.as_os_str().is_empty() && !renamed.contains(&path) { + renamed.push(path); + } + } + } + let protected: Vec = secret_files + .iter() + .map(|file| root.join(file)) + .filter(|file| { + shape + .denied + .iter() + .any(|denied| file.starts_with(root.join(denied))) + }) + .collect(); + let report = run.join("report.txt"); + let absolute = |paths: &[&str]| -> Vec { + paths + .iter() + .map(|path| root.join(path).to_string_lossy().into_owned()) + .collect() + }; + let plan = serde_json::json!({ + "report": report, + "protected": protected, + "renamed": renamed, + }); + let mut read_write = vec![run.to_string_lossy().into_owned()]; + read_write.extend(absolute(shape.read_write)); + let mut read_only = absolute(shape.read_only); + read_only.push(test_binary.to_string_lossy().into_owned()); + let config = serde_json::json!({ + "version": "1.0.0", + "containerId": format!("pc-denied-rename-{index}"), + "containment": "processcontainer", + "process": { + "commandLine": format!( + "\"{}\" denied_paths_rename_workload --exact --ignored --test-threads=1", + test_binary.display() + ), + "cwd": run, + "env": [format!("{RENAME_WORKLOAD_PLAN}={plan}")], + "inheritDefaultEnv": true, + "timeout": 60_000, + }, + "filesystem": { + "readwritePaths": read_write, + "readonlyPaths": read_only, + "deniedPaths": absolute(shape.denied), + }, + "processContainer": { "leastPrivilege": false }, + "fallback": { "allowDaclMutation": false }, + "ui": { "disable": true, "clipboard": "none", "injection": false }, + }); + let b64 = base64::engine::general_purpose::STANDARD + .encode(serde_json::to_string(&config).unwrap().as_bytes()); + let out = Command::new(&wxc) + .arg("--config-base64") + .arg(&b64) + .output() + .expect("wxc-exec spawn"); + let report = std::fs::read_to_string(&report).unwrap_or_else(|error| { + panic!( + "{}: no workload report ({error})\nstdout={}\nstderr={}", + shape.name, + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ) + }); + eprintln!("{}:\n{report}", shape.name); + assert!(report.ends_with("done"), "{}: {report}", shape.name); + if report.contains(RENAME_SECRET) { + failures.push(format!( + "{}: denied contents read after a rename", + shape.name + )); + } + for file in secret_files { + assert_eq!( + std::fs::read_to_string(root.join(file)).ok().as_deref(), + Some(RENAME_SECRET), + "{}: {file} changed on the host", + shape.name + ); + } + } + assert!(failures.is_empty(), "{failures:#?}"); +} + // ── Isolation session enforcement tests ────────────────────────────────────── /// Full `isolation_session` round trip: provision → start → exec → stop → diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 487f05b8a7..f3b330a42d 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -1331,21 +1331,35 @@ openshell sandbox create --name mxc-demo --detach --policy policy.yaml ` ``` To hide files or directories inside the policy's filesystem grants, such as -credential files in a writable working directory, add `denied_paths`: +credential files in a writable working directory, add `denied_paths`. With a +policy that grants `C:\work\profile` read-write: ```shell $config = '{"mxc":{"command":["C:\\Windows\\System32\\cmd.exe","/d","/c","agent.cmd"],"cwd":"C:\\work","denied_paths":["C:\\work\\profile\\.env","C:\\work\\profile\\auth.json"]}}' ``` -The workload cannot read or write a denied file, list a denied directory, or -create a denied path. A denied file's name, size, timestamps and attributes -remain visible to the workload, and a hard link to the file at another path -the workload can read still exposes its contents. Each entry must be an -absolute path that starts with a drive letter and whose parent directory -exists, and must not equal or contain a filesystem grant, compared after -resolving 8.3 short names and junctions and ignoring letter case. The driver -rejects other entries before provisioning. Denied paths are fixed at sandbox -creation. +The workload cannot open a denied path: it cannot read, write or delete a +denied file, list a denied directory, or create a denied path by name. A denied +file's name, size, timestamps and attributes remain visible, and a hard link to +the file at another path the workload can read still exposes its contents. MXC +does not check the target of a rename against denied paths, so in a writable +directory the workload can rename its own file onto a denied name: this +replaces the denied file on the host, or creates it when it is absent, without +exposing the original contents. An existing denied directory cannot be +replaced, and denying the directory that holds such files prevents this. + +Each entry must be an absolute path that starts with a drive letter and whose +parent directory exists, and must not equal or contain a filesystem grant. +MXC enforces paths: the workload can rename a directory that is writable to it +when the directory's parent is writable too, and an entry below that directory +is then readable under a new path. The driver rejects such entries, including +entries two or more levels below a read-write grant root and entries inside a +read-write grant nested in another one; the error names the directory to deny +instead. Paths are compared after resolving 8.3 short names, junctions, letter +case, and the `\\.\`, `\\?\`, `\??\`, `GLOBALROOT` and volume GUID spellings +MXC accepts for grants; with denied paths, a UNC grant or one that does not +resolve to a local drive is rejected. These checks run before provisioning. +Denied paths are fixed at sandbox creation. Supply workload environment through `sandbox create --env` or `--env-from`; it is not part of gateway configuration. The command runs from host files,