From a39d182582c033507af0432b9c87c2abeeda0574 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 18 Sep 2026 11:36:42 +0200 Subject: [PATCH] feat(sandbox): enforce CDI policy in workload boundary Signed-off-by: Evan Lezar --- architecture/security-policy.md | 30 ++- crates/openshell-core/src/cdi.rs | 2 + crates/openshell-core/src/cdi_linux.rs | 79 +++---- crates/openshell-core/src/policy.rs | 7 + .../openshell-driver-docker/src/isolation.rs | 1 + .../src/isolation.rs | 1 + .../openshell-driver-podman/src/isolation.rs | 1 + .../openshell-driver-vm/src/isolation/mod.rs | 1 + .../src/boundary_protocol.rs | 12 ++ .../openshell-sandbox/src/boundary_server.rs | 203 +++++++++++++++++- crates/openshell-sandbox/src/process.rs | 142 +++++++----- .../openshell-supervisor-network/src/opa.rs | 1 + docs/sandboxes/policies.mdx | 28 ++- 13 files changed, 415 insertions(+), 93 deletions(-) diff --git a/architecture/security-policy.md b/architecture/security-policy.md index 667694a1fc..f1aaf0c841 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -41,12 +41,30 @@ before any consumer-specific projection runs. There is no permissive parsing profile: unsupported policy fields always invalidate the document. Middleware `config`, query and persisted-query names, and recursive MCP parameter names are open user-data maps rather than schema extensions. -Before applying Landlock, the supervisor enriches baseline filesystem paths that -the runtime needs. Missing baseline paths are skipped so one absent runtime path -does not weaken the whole ruleset. When GPU devices are present, GPU baseline -enrichment adds existing GPU device nodes as read-write paths and promotes -`/proc` to read-write because CUDA workloads write thread metadata under -`/proc//task//comm`. +Before applying Landlock, trusted runtime components enrich baseline filesystem +paths that the workload needs. Missing optional baseline paths are skipped so +one absent runtime path does not weaken the whole ruleset. The workload-side +sandbox runtime performs GPU enrichment in the workload mount namespace. GPU +sandboxes without CDI context use the legacy device baseline; CDI sandboxes use +requirements derived from the selected CDI specs. Both paths grant read-only +access to the existing container sysfs view and promote `/proc` to read-write. + +A compute driver can place a protected CDI context and read-only CDI spec +projection in the workload boundary. Before agent exec, `openshell-sandbox` +resolves the selected CDI IDs and validates all derived entities against the +workload namespace. It adds device nodes, exact destinations of non-library +read-only mounts, parent directories of shared-library mounts, and supplemental +GIDs. CDI host paths are ignored for policy. Library filenames must end in +`.so` or a numeric SONAME suffix with any number of dot-separated components, +such as `.so.1`, `.so.1.2`, or `.so.1.2.3`. A derived library directory or +non-library mount is omitted when an existing read-only or read-write ancestor +already covers it. Writable CDI single-file mounts require an exact +`filesystem_policy.read_write` opt-in, and writable CDI directory mounts fail +closed. CDI resolution errors fail agent startup. + +The `/sys` grant is a runtime-owned GPU compatibility baseline, not a +CDI-derived permission. CDI specs remain unable to request broad `/sys` access, +and non-GPU sandboxes do not receive the grant. Landlock rules are tailored to the inode type reported by the already-opened path descriptor. Directories retain the requested directory and file rights; diff --git a/crates/openshell-core/src/cdi.rs b/crates/openshell-core/src/cdi.rs index 2c1d03b9d7..cc89ada422 100644 --- a/crates/openshell-core/src/cdi.rs +++ b/crates/openshell-core/src/cdi.rs @@ -105,6 +105,8 @@ pub enum CdiError { WritableMountNotFile { path: String, kind: String }, #[error("CDI device node '{path}' must target a character or block device, found {kind}")] DeviceNodeNotDevice { path: String, kind: String }, + #[error("CDI read-only path '{path}' does not exist in the workload namespace")] + ReadOnlyPathMissing { path: String }, #[error("CDI additionalGids must not contain root GID 0")] RootAdditionalGid, #[error("CDI mount '{path}' has conflicting ro/rw options")] diff --git a/crates/openshell-core/src/cdi_linux.rs b/crates/openshell-core/src/cdi_linux.rs index b10e98167b..8000467f3c 100644 --- a/crates/openshell-core/src/cdi_linux.rs +++ b/crates/openshell-core/src/cdi_linux.rs @@ -183,6 +183,11 @@ where }); } } + for path in &requirements.read_only_paths { + if path_kind(path).is_none() { + return Err(CdiError::ReadOnlyPathMissing { path: path.clone() }); + } + } for path in &requirements.read_write_mount_paths { if !normalized_allowlist.contains(path) { return Err(CdiError::WritableMountNotAllowed { path: path.clone() }); @@ -445,12 +450,8 @@ mod tests { let dir = tempfile::tempdir().unwrap(); write_spec(dir.path(), "nvidia.yaml", spec); - let requirements = resolve_with_kind( - &context(dir.path(), &["nvidia.com/gpu=all"]), - &[], - fake_device_node, - ) - .unwrap(); + let requirements = + resolve_cdi_context(&context(dir.path(), &["nvidia.com/gpu=all"])).unwrap(); let baseline = CdiRequirementsBaseline { device_node_paths: &requirements.device_node_paths, read_only_paths: &requirements.read_only_paths, @@ -507,12 +508,8 @@ devices: "#, ); - let requirements = resolve_with_kind( - &context(dir.path(), &["nvidia.com/gpu=0"]), - &[], - fake_device_node, - ) - .unwrap(); + let requirements = + resolve_cdi_context(&context(dir.path(), &["nvidia.com/gpu=0"])).unwrap(); assert_eq!( requirements.device_node_paths, @@ -540,12 +537,8 @@ devices: ", ); - let requirements = resolve_with_kind( - &context(dir.path(), &["nvidia.com/gpu=all"]), - &[], - fake_device_node, - ) - .unwrap(); + let requirements = + resolve_cdi_context(&context(dir.path(), &["nvidia.com/gpu=all"])).unwrap(); assert_eq!( requirements.device_node_paths, @@ -573,12 +566,8 @@ devices: ", ); - let requirements = resolve_with_kind( - &context(dir.path(), &["nvidia.com/gpu=all"]), - &[], - fake_device_node, - ) - .unwrap(); + let requirements = + resolve_cdi_context(&context(dir.path(), &["nvidia.com/gpu=all"])).unwrap(); assert_eq!(requirements.device_node_paths, vec!["/dev/dxg"]); assert_eq!(requirements.read_only_paths, vec!["/usr/lib/wsl/lib"]); @@ -618,12 +607,8 @@ devices: "#, ); - let requirements = resolve_with_kind( - &context(dir.path(), &["nvidia.com/gpu=0"]), - &[], - always_missing, - ) - .unwrap(); + let requirements = + resolve_cdi_context(&context(dir.path(), &["nvidia.com/gpu=0"])).unwrap(); assert_eq!( requirements.read_only_paths, @@ -660,12 +645,8 @@ devices: "#, ); - let requirements = resolve_with_kind( - &context(dir.path(), &["nvidia.com/gpu=0"]), - &[], - fake_device_node, - ) - .unwrap(); + let requirements = + resolve_cdi_context(&context(dir.path(), &["nvidia.com/gpu=0"])).unwrap(); assert_eq!(requirements.additional_gids, vec![44]); assert_eq!( @@ -950,6 +931,32 @@ devices: assert!(matches!(err, CdiError::RootAdditionalGid)); } + #[test] + fn validates_read_only_paths_in_the_workload_namespace() { + let requirements = CdiDerivedRequirements { + read_only_paths: vec!["/opt/nvidia/runtime.json".to_string()], + ..CdiDerivedRequirements::default() + }; + let writable_file_allowlist = HashSet::::new(); + + let err = validate_cdi_requirements_with_path_kind( + &requirements, + &writable_file_allowlist, + always_missing, + ) + .unwrap_err(); + assert!(matches!( + err, + CdiError::ReadOnlyPathMissing { path } + if path == "/opt/nvidia/runtime.json" + )); + + validate_cdi_requirements_with_path_kind(&requirements, &writable_file_allowlist, |_| { + Some(CdiPathKind::File) + }) + .unwrap(); + } + #[test] fn rejects_device_node_that_is_not_device() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/openshell-core/src/policy.rs b/crates/openshell-core/src/policy.rs index af474325c5..6e23780988 100644 --- a/crates/openshell-core/src/policy.rs +++ b/crates/openshell-core/src/policy.rs @@ -83,6 +83,12 @@ pub struct ProcessPolicy { /// Group name to run the sandboxed process as. pub run_as_group: Option, + + /// Linux supplemental groups to apply before dropping privileges. + /// + /// Runtime-specific inputs can use different terminology; CDI + /// `additionalGids` are converted into this process-level representation. + pub supplemental_groups: Vec, } #[derive(Debug, Clone, Default)] @@ -185,6 +191,7 @@ impl From for ProcessPolicy { } else { Some(proto.run_as_group) }, + supplemental_groups: Vec::new(), } } } diff --git a/crates/openshell-driver-docker/src/isolation.rs b/crates/openshell-driver-docker/src/isolation.rs index 5e50a37ad7..88aad82aaf 100644 --- a/crates/openshell-driver-docker/src/isolation.rs +++ b/crates/openshell-driver-docker/src/isolation.rs @@ -77,6 +77,7 @@ impl DockerBoundarySpec { }, resource_claims: resource_claims.clone(), resource_claim_files: BTreeMap::new(), + cdi_context: None, workload_identity: self.workload_identity.clone(), driver_fence: driver_fence.clone(), child_env: self.child_env, diff --git a/crates/openshell-driver-kubernetes/src/isolation.rs b/crates/openshell-driver-kubernetes/src/isolation.rs index bb9ae8ff66..15ec209147 100644 --- a/crates/openshell-driver-kubernetes/src/isolation.rs +++ b/crates/openshell-driver-kubernetes/src/isolation.rs @@ -232,6 +232,7 @@ impl KubernetesSandboxRuntimeBoundarySpec { "kubernetes.workload_pod_uid".to_string(), self.workload_pod_uid_path, )]), + cdi_context: None, workload_identity: self.workload_identity.clone(), driver_fence: driver_fence.clone(), child_env: self.child_env, diff --git a/crates/openshell-driver-podman/src/isolation.rs b/crates/openshell-driver-podman/src/isolation.rs index b616fa6474..81a05df2a1 100644 --- a/crates/openshell-driver-podman/src/isolation.rs +++ b/crates/openshell-driver-podman/src/isolation.rs @@ -197,6 +197,7 @@ pub fn bootstrap_archives( }, resource_claims: resource_claims.clone(), resource_claim_files: BTreeMap::new(), + cdi_context: None, workload_identity: identity.clone(), driver_fence: driver_fence.clone(), child_env: child_env.clone(), diff --git a/crates/openshell-driver-vm/src/isolation/mod.rs b/crates/openshell-driver-vm/src/isolation/mod.rs index 49c1135310..af0912f60a 100644 --- a/crates/openshell-driver-vm/src/isolation/mod.rs +++ b/crates/openshell-driver-vm/src/isolation/mod.rs @@ -76,6 +76,7 @@ impl VmBoundarySpec { }, resource_claims: resource_claims.clone(), resource_claim_files: BTreeMap::new(), + cdi_context: None, workload_identity: workload_identity.clone(), driver_fence: driver_fence.clone(), child_env: self.child_env, diff --git a/crates/openshell-sandbox-backend/src/boundary_protocol.rs b/crates/openshell-sandbox-backend/src/boundary_protocol.rs index 05542fb2bc..51e9edb6fa 100644 --- a/crates/openshell-sandbox-backend/src/boundary_protocol.rs +++ b/crates/openshell-sandbox-backend/src/boundary_protocol.rs @@ -352,6 +352,12 @@ pub struct BoundaryConfig { /// Downward API. Other drivers may leave the map empty. #[serde(default)] pub resource_claim_files: std::collections::BTreeMap, + /// Driver-protected CDI selection and workload-local specification projections. + /// + /// The sandbox runtime resolves this context inside the workload mount + /// namespace before applying launch-time filesystem controls. + #[serde(default)] + pub cdi_context: Option, /// Exact identity already applied by the runtime to the sandbox process. pub workload_identity: openshell_isolation_interface::contract::ResolvedWorkloadIdentity, /// Concrete outer-fence evidence validated by the driver. @@ -382,6 +388,7 @@ impl fmt::Debug for BoundaryConfig { .field("listener", &self.listener) .field("resource_claims", &self.resource_claims) .field("resource_claim_files", &self.resource_claim_files) + .field("has_cdi_context", &self.cdi_context.is_some()) .field("workload_identity", &self.workload_identity) .field("driver_fence", &self.driver_fence) .field("child_env_keys", &self.child_env.keys().collect::>()) @@ -926,6 +933,8 @@ pub struct SandboxPolicyWire { pub landlock: LandlockCompatibilityWire, pub run_as_user: Option, pub run_as_group: Option, + #[serde(default)] + pub supplemental_groups: Vec, } impl From for SandboxPolicyWire { @@ -950,6 +959,7 @@ impl From for SandboxPolicyWire { let ProcessPolicy { run_as_user, run_as_group, + supplemental_groups, } = process; Self { version, @@ -961,6 +971,7 @@ impl From for SandboxPolicyWire { landlock: LandlockCompatibilityWire::from(compatibility), run_as_user, run_as_group, + supplemental_groups, } } } @@ -987,6 +998,7 @@ impl From for SandboxPolicy { process: ProcessPolicy { run_as_user: policy.run_as_user, run_as_group: policy.run_as_group, + supplemental_groups: policy.supplemental_groups, }, } } diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index 5452ea6efa..32aba92c26 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -13,6 +13,7 @@ use std::path::Path; #[cfg(target_os = "linux")] mod linux { + use std::collections::BTreeSet; use std::fs::File; use std::io::{self, Read, Write}; use std::mem::size_of; @@ -101,6 +102,118 @@ mod linux { fn enrich_gpu_filesystem_paths( policy: &mut openshell_core::policy::SandboxPolicy, gpu_requested: bool, + cdi_context: Option<&openshell_core::cdi::CdiContext>, + ) -> Result { + if let Some(context) = cdi_context { + if !gpu_requested { + return Err("CDI context was provided without a GPU resource claim".to_string()); + } + return enrich_cdi_filesystem_paths(policy, context); + } + Ok(enrich_legacy_gpu_filesystem_paths(policy, gpu_requested)) + } + + fn enrich_cdi_filesystem_paths( + policy: &mut openshell_core::policy::SandboxPolicy, + context: &openshell_core::cdi::CdiContext, + ) -> Result { + let writable_file_allowlist = policy + .filesystem + .read_write + .iter() + .map(|path| path.to_string_lossy().into_owned()) + .collect::>(); + let requirements = openshell_core::cdi::resolve_cdi_context(context) + .map_err(|error| format!("resolve workload CDI requirements: {error}"))?; + openshell_core::cdi::validate_cdi_requirements(&requirements, &writable_file_allowlist) + .map_err(|error| format!("validate workload CDI requirements: {error}"))?; + + let mut read_only = requirements + .read_only_paths + .iter() + .map(std::path::PathBuf::from) + .collect::>(); + if Path::new("/sys").exists() { + read_only.push("/sys".into()); + } + read_only.sort_by(path_depth_order); + read_only.dedup(); + + let mut modified = false; + for path in read_only { + if path_is_covered_by_policy(policy, &path) { + continue; + } + policy.filesystem.read_only.push(path); + modified = true; + } + + let mut read_write = requirements + .device_node_paths + .iter() + .map(std::path::PathBuf::from) + .collect::>(); + read_write.push("/proc".into()); + read_write.sort_by(path_depth_order); + read_write.dedup(); + for path in read_write { + if path_is_covered(&path, &policy.filesystem.read_write) { + continue; + } + if policy.filesystem.read_only.contains(&path) { + if path != Path::new("/proc") { + continue; + } + policy + .filesystem + .read_only + .retain(|allowed| allowed != &path); + } + policy.filesystem.read_write.push(path); + modified = true; + } + + let mut supplemental_groups = policy + .process + .supplemental_groups + .iter() + .copied() + .collect::>(); + let original_group_count = supplemental_groups.len(); + supplemental_groups.extend(requirements.additional_gids); + if supplemental_groups.len() != original_group_count { + policy.process.supplemental_groups = supplemental_groups.into_iter().collect(); + modified = true; + } + + Ok(modified) + } + + fn path_depth_order( + left: &std::path::PathBuf, + right: &std::path::PathBuf, + ) -> std::cmp::Ordering { + left.components() + .count() + .cmp(&right.components().count()) + .then_with(|| left.cmp(right)) + } + + fn path_is_covered_by_policy( + policy: &openshell_core::policy::SandboxPolicy, + candidate: &Path, + ) -> bool { + path_is_covered(candidate, &policy.filesystem.read_only) + || path_is_covered(candidate, &policy.filesystem.read_write) + } + + fn path_is_covered(candidate: &Path, allowed: &[std::path::PathBuf]) -> bool { + allowed.iter().any(|path| candidate.starts_with(path)) + } + + fn enrich_legacy_gpu_filesystem_paths( + policy: &mut openshell_core::policy::SandboxPolicy, + gpu_requested: bool, ) -> bool { if !gpu_requested { return false; @@ -2403,7 +2516,15 @@ mod linux { .resource_claims .get(GPU_RESOURCE_CLAIM) .is_some_and(|value| value == "true"); - if enrich_gpu_filesystem_paths(&mut policy, gpu_requested) { + let enriched = match enrich_gpu_filesystem_paths( + &mut policy, + gpu_requested, + self.config.cdi_context.as_ref(), + ) { + Ok(enriched) => enriched, + Err(error) => return guest_error(BoundaryErrorKind::Process, error), + }; + if enriched { openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) .severity(openshell_ocsf::SeverityId::Informational) @@ -3527,6 +3648,79 @@ mod linux { }; use rcgen::{KeyPair, PKCS_ED25519}; + fn cdi_test_policy(read_only: &[&str]) -> openshell_core::policy::SandboxPolicy { + openshell_core::policy::SandboxPolicy { + version: 1, + filesystem: openshell_core::policy::FilesystemPolicy { + read_only: read_only.iter().map(std::path::PathBuf::from).collect(), + read_write: Vec::new(), + include_workdir: false, + }, + network: openshell_core::policy::NetworkPolicy::default(), + landlock: openshell_core::policy::LandlockPolicy::default(), + process: openshell_core::policy::ProcessPolicy::default(), + } + } + + fn cdi_test_context() -> (tempfile::TempDir, openshell_core::cdi::CdiContext) { + let directory = tempfile::tempdir().unwrap(); + std::fs::write( + directory.path().join("nvidia.yaml"), + r#" +cdiVersion: 0.7.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /dev/null +containerEdits: + mounts: + - hostPath: /host/libfake.so.1 + containerPath: /usr/lib/libfake.so.1 + options: [ro] + - hostPath: /host/nvidia-info + containerPath: /etc/hosts + options: [ro] + additionalGids: [44] +"#, + ) + .unwrap(); + let context = openshell_core::cdi::CdiContext::new( + vec!["nvidia.com/gpu=0".to_string()], + vec![openshell_core::cdi::CdiSpecDirectory::new( + directory.path().to_string_lossy(), + "/var/run/cdi", + )], + ); + (directory, context) + } + + #[test] + fn workload_cdi_enrichment_respects_authored_ancestors() { + let (_directory, context) = cdi_test_context(); + let mut policy = cdi_test_policy(&["/usr"]); + + assert!(enrich_cdi_filesystem_paths(&mut policy, &context).unwrap()); + + assert!(policy.filesystem.read_only.contains(&"/usr".into())); + assert!(!policy.filesystem.read_only.contains(&"/usr/lib".into())); + assert!(policy.filesystem.read_only.contains(&"/etc/hosts".into())); + assert!(policy.filesystem.read_only.contains(&"/sys".into())); + assert!(policy.filesystem.read_write.contains(&"/dev/null".into())); + assert!(policy.filesystem.read_write.contains(&"/proc".into())); + assert_eq!(policy.process.supplemental_groups, vec![44]); + } + + #[test] + fn workload_cdi_enrichment_is_idempotent() { + let (_directory, context) = cdi_test_context(); + let mut policy = cdi_test_policy(&["/usr"]); + + assert!(enrich_cdi_filesystem_paths(&mut policy, &context).unwrap()); + assert!(!enrich_cdi_filesystem_paths(&mut policy, &context).unwrap()); + } + #[test] fn exec_tombstones_outlive_retained_handles_and_fail_closed_at_capacity() { let mut requests = std::collections::HashSet::new(); @@ -3704,6 +3898,7 @@ mod linux { }, resource_claims: std::collections::BTreeMap::new(), resource_claim_files: std::collections::BTreeMap::new(), + cdi_context: None, workload_identity: test_workload_identity(), driver_fence: test_driver_fence(), child_env: std::collections::HashMap::new(), @@ -3855,6 +4050,7 @@ mod linux { }, resource_claims: std::collections::BTreeMap::new(), resource_claim_files: std::collections::BTreeMap::new(), + cdi_context: None, workload_identity: test_workload_identity(), driver_fence: test_driver_fence(), child_env: std::collections::HashMap::new(), @@ -4409,6 +4605,7 @@ mod linux { }, resource_claims: std::collections::BTreeMap::new(), resource_claim_files: std::collections::BTreeMap::new(), + cdi_context: None, workload_identity: test_workload_identity(), driver_fence: test_driver_fence(), child_env: std::collections::HashMap::new(), @@ -4444,6 +4641,7 @@ mod linux { "kubernetes.pod_uid".to_string(), pod_uid_path, )]), + cdi_context: None, workload_identity: test_workload_identity(), driver_fence: test_driver_fence(), child_env: std::collections::HashMap::new(), @@ -4484,6 +4682,7 @@ mod linux { }, resource_claims: std::collections::BTreeMap::new(), resource_claim_files: std::collections::BTreeMap::new(), + cdi_context: None, workload_identity: test_workload_identity(), driver_fence: test_driver_fence(), child_env: std::collections::HashMap::new(), @@ -4659,6 +4858,7 @@ mod linux { }, resource_claims: std::collections::BTreeMap::new(), resource_claim_files: std::collections::BTreeMap::new(), + cdi_context: None, workload_identity: test_workload_identity(), driver_fence: test_driver_fence(), child_env: std::collections::HashMap::new(), @@ -4977,6 +5177,7 @@ mod linux { }, resource_claims: std::collections::BTreeMap::new(), resource_claim_files: std::collections::BTreeMap::new(), + cdi_context: None, workload_identity: test_workload_identity(), driver_fence: test_driver_fence(), child_env: std::collections::HashMap::new(), diff --git a/crates/openshell-sandbox/src/process.rs b/crates/openshell-sandbox/src/process.rs index 56d4f244ad..f795c96f75 100644 --- a/crates/openshell-sandbox/src/process.rs +++ b/crates/openshell-sandbox/src/process.rs @@ -13,7 +13,10 @@ use miette::{IntoDiagnostic, Result}; use nix::sys::signal::{self, Signal}; use nix::unistd::{Gid, Group, Pid, Uid, User}; use openshell_core::policy::SandboxPolicy; +#[cfg(unix)] +use std::collections::BTreeSet; use std::collections::HashMap; +#[cfg(target_os = "linux")] use std::ffi::CString; #[cfg(unix)] use std::os::fd::AsRawFd; @@ -1923,7 +1926,11 @@ fn resolve_filesystem_identity( _ => Vec::new(), }; - Ok((uid, gid, supplementary_gids)) + Ok(( + uid, + gid, + merged_supplemental_gids(supplementary_gids, &policy.process.supplemental_groups)?, + )) } #[cfg(not(unix))] @@ -1933,6 +1940,24 @@ pub fn prepare_filesystem(_policy: &SandboxPolicy) -> Result<()> { // `effective_gid`/`effective_uid` are intentionally parallel names (same role // for different identifiers) and the noise from renaming would obscure intent. +#[cfg(unix)] +fn merged_supplemental_gids(base: Vec, extra: &[u32]) -> Result> { + let mut groups = base + .into_iter() + .map(Gid::as_raw) + .filter(|gid| *gid != 0) + .collect::>(); + + for raw_gid in extra { + if *raw_gid == 0 { + return Err(miette::miette!("Supplemental group GID 0 is not allowed")); + } + groups.insert(*raw_gid); + } + + Ok(groups.into_iter().map(Gid::from_raw).collect()) +} + #[cfg(unix)] #[allow(clippy::similar_names)] pub fn drop_privileges(policy: &SandboxPolicy) -> Result<()> { @@ -1965,6 +1990,11 @@ pub fn drop_privileges_with_identity( fallback.process.run_as_group = Some("sandbox".into()); return drop_privileges_with_identity(&fallback, resolved_identity); } + if !policy.process.supplemental_groups.is_empty() { + return Err(miette::miette!( + "Supplemental groups require a privileged supervisor process" + )); + } return Ok(()); } @@ -2015,61 +2045,41 @@ pub fn drop_privileges_with_identity( }, }; - // Resolve the name for initgroups only for the existing explicit-policy - // path. OCI-derived users carry a numeric UID from the bounded parser and - // must not be looked up again through NSS. - let user_name_is_numeric = user_name.is_some_and(|n| n.parse::().is_ok()); - let initgroups_name = - if user_name.is_some() && !user_name_is_numeric && resolved_identity.uid().is_none() { - Some( - User::from_uid(target_uid) - .into_diagnostic()? - .ok_or_else(|| { - miette::miette!("Failed to resolve user record for UID {target_uid}") - })? - .name, - ) - } else { - None - }; - + #[cfg(target_os = "linux")] if target_uid != nix::unistd::geteuid() { - if resolved_identity.uses_oci_user_fallback() { + let user_name_is_numeric = user_name.is_some_and(|n| n.parse::().is_ok()); + let initgroups_name = + if user_name.is_some() && !user_name_is_numeric && resolved_identity.uid().is_none() { + Some( + User::from_uid(target_uid) + .into_diagnostic()? + .ok_or_else(|| { + miette::miette!("Failed to resolve user record for UID {target_uid}") + })? + .name, + ) + } else { + None + }; + let supplemental_groups = if resolved_identity.uses_oci_user_fallback() { // OCI named users use the bounded /etc/group parser shared with // workspace validation. Numeric OCI users resolve to an empty // list. Never retain the root supervisor's inherited groups. - #[cfg(not(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - )))] - { - let (_, _, supplementary_gids) = - resolve_filesystem_identity(policy, resolved_identity)?; - nix::unistd::setgroups(&supplementary_gids).into_diagnostic()?; - } + resolve_filesystem_identity(policy, resolved_identity)?.2 } else if let Some(ref user_name) = initgroups_name { let user_cstr = CString::new(user_name.as_str()) .map_err(|_| miette::miette!("Invalid user name"))?; - #[cfg(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - ))] - { - let _ = user_cstr; - } - #[cfg(not(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - )))] - { - nix::unistd::initgroups(user_cstr.as_c_str(), target_gid).into_diagnostic()?; - } + nix::unistd::initgroups(user_cstr.as_c_str(), target_gid).into_diagnostic()?; + merged_supplemental_gids( + nix::unistd::getgroups().into_diagnostic()?, + &policy.process.supplemental_groups, + )? + } else { + merged_supplemental_gids(Vec::new(), &policy.process.supplemental_groups)? + }; + + if !supplemental_groups.is_empty() || resolved_identity.uses_oci_user_fallback() { + nix::unistd::setgroups(&supplemental_groups).into_diagnostic()?; } } @@ -2212,6 +2222,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(current_user.name.clone()), run_as_group: None, + ..Default::default() }); let workspace = ResolvedWorkspace::default(); let mut cmd = Command::new("/usr/bin/env"); @@ -2249,6 +2260,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(current_user.name), run_as_group: None, + ..Default::default() }); for interactive in [false, true] { let mut cmd = Command::new("/usr/bin/env"); @@ -2311,6 +2323,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("101".into()), run_as_group: Some("102".into()), + ..Default::default() }); assert!(validate_sandbox_user(&policy).is_ok()); @@ -2323,6 +2336,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("app".into()), run_as_group: Some("staff".into()), + ..Default::default() }); let resolved = ResolvedProcessIdentity::new(Some(101), Some(102)); @@ -2336,10 +2350,12 @@ mod tests { let root_user = policy_with_process(ProcessPolicy { run_as_user: Some("0".into()), run_as_group: Some("102".into()), + ..Default::default() }); let root_group = policy_with_process(ProcessPolicy { run_as_user: Some("101".into()), run_as_group: Some("0".into()), + ..Default::default() }); assert!(validate_sandbox_user(&root_user).is_err()); @@ -2352,6 +2368,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("__oci_name_not_in_host_nss__".into()), run_as_group: Some("__oci_group_not_in_host_nss__".into()), + ..Default::default() }); let resolved = ResolvedProcessIdentity::new(Some(1234), Some(1235)); @@ -2365,6 +2382,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("__explicit_name_not_in_host_nss__".into()), run_as_group: Some("__oci_group_not_in_host_nss__".into()), + ..Default::default() }); let resolved = ResolvedProcessIdentity::new(None, Some(1235)); @@ -2472,6 +2490,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }); if nix::unistd::geteuid().is_root() { // As root, drop_privileges falls back to "sandbox:sandbox". @@ -2489,6 +2508,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(String::new()), run_as_group: Some(String::new()), + ..Default::default() }); if nix::unistd::geteuid().is_root() { let has_sandbox = User::from_name("sandbox").ok().flatten().is_some(); @@ -2498,6 +2518,21 @@ mod tests { } } + #[test] + fn merged_supplemental_gids_deduplicates_and_rejects_root() { + let merged = merged_supplemental_gids( + vec![Gid::from_raw(44), Gid::from_raw(44), Gid::from_raw(0)], + &[44, 107], + ) + .unwrap(); + + assert_eq!( + merged.iter().map(|gid| gid.as_raw()).collect::>(), + vec![44, 107] + ); + assert!(merged_supplemental_gids(Vec::new(), &[0]).is_err()); + } + #[test] fn drop_privileges_succeeds_for_current_group() { // Set only run_as_group (no run_as_user) so that initgroups() is not @@ -2512,6 +2547,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: Some(current_group.name), + ..Default::default() }); let result = drop_privileges(&policy); @@ -2549,6 +2585,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: Some(current_group.name), + ..Default::default() }); let mut cmd = std::process::Command::new(std::env::current_exe().expect("current exe")); @@ -2590,6 +2627,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(current_user.name), run_as_group: Some(current_group.name), + ..Default::default() }); assert!(drop_privileges(&policy).is_ok()); @@ -2600,6 +2638,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("__nonexistent_test_user_42__".to_string()), run_as_group: None, + ..Default::default() }); let result = drop_privileges(&policy); @@ -2613,6 +2652,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: Some("__nonexistent_test_group_42__".to_string()), + ..Default::default() }); let result = drop_privileges(&policy); @@ -2786,6 +2826,7 @@ mod tests { process: ProcessPolicy { run_as_user, run_as_group, + ..Default::default() }, } } @@ -3832,6 +3873,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(uid_raw.to_string()), run_as_group: Some(gid_raw.to_string()), + ..Default::default() }); assert!( @@ -3863,6 +3905,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(current_uid.to_string()), // numeric UID, no passwd entry needed run_as_group: Some(current_group.name), // name-based group + ..Default::default() }); assert!( @@ -3879,6 +3922,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("999999".into()), run_as_group: Some("999999".into()), + ..Default::default() }); match drop_privileges(&policy) { Ok(()) => {} diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index ba1f8d1a28..e5518d2dc1 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -1389,6 +1389,7 @@ fn parse_process_policy(val: ®orus::Value) -> ProcessPolicy { ProcessPolicy { run_as_user: get_str(val, "run_as_user"), run_as_group: get_str(val, "run_as_group"), + ..ProcessPolicy::default() } } diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index a8b743b5cd..82081d9df5 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -126,7 +126,33 @@ See [Supervisor Middleware](/extensibility/supervisor-middleware) for registrati When a sandbox runs in proxy mode (the default), OpenShell automatically adds baseline filesystem paths required for the sandbox child process to function: `/usr`, `/lib`, `/etc`, and `/var/log` (read-only), plus `/tmp` (read-write). When `filesystem.include_workdir` is `true`, OpenShell also adds the resolved working directory as read-write. Paths like `/app` are included in the baseline set but are only added if they exist in the container image. -For GPU sandboxes, OpenShell also adds existing GPU device nodes as read-write paths. CUDA workloads require write access to procfs for thread metadata, so GPU baseline enrichment moves `/proc` from read-only to read-write when GPU devices are present. +For GPU sandboxes without a CDI context, OpenShell also adds existing GPU +device nodes as read-write paths. GPU baseline enrichment grants read-only +access to the `/sys` tree already exposed by the container runtime because CUDA +reads hardware and driver topology during initialization. CUDA workloads also +require write access to procfs for thread metadata, so GPU enrichment moves +`/proc` from read-only to read-write when GPU devices or CDI specs are present. + +GPU sandboxes from CDI-capable compute drivers receive CDI-derived filesystem +requirements instead of the hard-coded GPU device and library baseline. The +workload-side sandbox runtime resolves the selected CDI device specs and adds +CDI device nodes as read-write paths. For read-only mounts, it adds the parent directories of shared +libraries whose filenames end in `.so` or a numeric SONAME suffix with any +number of dot-separated components, such as `.so.1`, `.so.1.2`, or +`.so.1.2.3`. It retains exact destinations for other read-only mounts. OpenShell +does not add a derived path when an existing read-only or read-write parent +already covers it. CDI `additionalGids` are applied as +supplemental groups before agent processes drop privileges. + +Writable CDI mount destinations are fail-closed. OpenShell accepts a writable +CDI mount only when it targets a single file and that exact path is already +listed in `filesystem_policy.read_write`. Writable CDI directory mounts and +CDI paths such as `/`, `/dev`, `/proc`, `/sys`, `/run`, or `/usr` are rejected +during sandbox startup. + +The GPU `/sys` permission is a runtime-owned compatibility baseline. CDI +specs cannot request broad sysfs access, and non-GPU sandboxes do not receive +this baseline. This filtering prevents a missing baseline path from degrading Landlock enforcement. Without it, a single missing path could cause the entire Landlock ruleset to fail, leaving the sandbox with no filesystem restrictions at all.