diff --git a/Cargo.lock b/Cargo.lock index d061c74496..fc02da01cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -61,6 +61,7 @@ dependencies = [ "cfg-if", "getrandom 0.3.4", "once_cell", + "serde", "version_check", "zerocopy", ] @@ -567,6 +568,21 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "1.3.2" @@ -672,6 +688,12 @@ dependencies = [ "serde_repr", ] +[[package]] +name = "borrow-or-share" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc0b364ead1874514c8c2855ab558056ebfeb775653e7ae45ff72f28f8f3166c" + [[package]] name = "bstr" version = "1.12.1" @@ -688,6 +710,12 @@ version = "3.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" +[[package]] +name = "bytecount" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e" + [[package]] name = "byteorder" version = "1.5.0" @@ -996,6 +1024,27 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d52eff69cd5e647efe296129160853a42795992097e8af39800e1060caeea9b" +[[package]] +name = "container-device-interface" +version = "1.1.1" +source = "git+https://github.com/cncf-tags/container-device-interface-rs?tag=v1.1.1#06644c1b167c8dcc247816590901e0a819636b01" +dependencies = [ + "anyhow", + "clap", + "const_format", + "jsonschema", + "lazy_static", + "libc", + "oci-spec 0.10.0", + "once_cell", + "path-clean", + "regex", + "semver", + "serde", + "serde_json", + "serde_yaml", +] + [[package]] name = "core-foundation" version = "0.10.1" @@ -1542,6 +1591,15 @@ dependencies = [ "zeroize", ] +[[package]] +name = "email_address" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e079f19b08ca6239f47f8ba8509c11cf3ea30095831f7fed61441475edd8c449" +dependencies = [ + "serde", +] + [[package]] name = "encode_unicode" version = "1.0.0" @@ -1618,6 +1676,17 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "fancy-regex" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1e1dacd0d2082dfcf1351c4bdd566bbe89a2b263235a2b50058f1e130a47277" +dependencies = [ + "bit-set", + "regex-automata", + "regex-syntax", +] + [[package]] name = "fastrand" version = "2.4.1" @@ -1674,6 +1743,17 @@ dependencies = [ "zlib-rs", ] +[[package]] +name = "fluent-uri" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc74ac4d8359ae70623506d512209619e5cf8f347124910440dbc221714b328e" +dependencies = [ + "borrow-or-share", + "ref-cast", + "serde", +] + [[package]] name = "flume" version = "0.11.1" @@ -1712,6 +1792,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fraction" +version = "0.15.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e076045bb43dac435333ed5f04caf35c7463631d0dae2deb2638d94dd0a5b872" +dependencies = [ + "lazy_static", + "num", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -1993,6 +2083,8 @@ version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" dependencies = [ + "allocator-api2", + "equivalent", "foldhash 0.2.0", ] @@ -2705,6 +2797,44 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "jsonschema" +version = "0.46.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0a699d3e77675e6aa4bfffe3b907c8b5f7ed3241f9965bffb25475ad4b08d05" +dependencies = [ + "ahash", + "bytecount", + "data-encoding", + "email_address", + "fancy-regex", + "fraction", + "getrandom 0.3.4", + "idna", + "itoa", + "jsonschema-regex", + "num-cmp", + "num-traits", + "percent-encoding", + "referencing", + "regex", + "reqwest 0.13.2", + "rustls", + "serde", + "serde_json", + "unicode-general-category", + "uuid-simd", +] + +[[package]] +name = "jsonschema-regex" +version = "0.46.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dbd1086b01b9349fd4ef9a07433965af64c8ce8159abe633a189e4ff817bd13" +dependencies = [ + "regex-syntax", +] + [[package]] name = "jsonwebtoken" version = "9.3.1" @@ -2922,9 +3052,9 @@ checksum = "b3a6a8c165077efc8f3a971534c50ea6a1a18b329ef4a66e897a7e3a1494565f" [[package]] name = "libc" -version = "0.2.185" +version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52ff2c0fe9bc6cb6b14a0592c2ff4fa9ceb83eea9db979b0487cd054946a2b8f" +checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" [[package]] name = "libcrux-intrinsics" @@ -3181,6 +3311,12 @@ dependencies = [ "sketches-ddsketch", ] +[[package]] +name = "micromap" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a86d3146ed3995b5913c414f6664344b9617457320782e64f0bb44afd49d74" + [[package]] name = "miette" version = "7.6.0" @@ -3372,6 +3508,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "num-cmp" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63335b2e2c34fae2fb0aa2cecfd9f0832a1e24b3b32ecec612c3426d46dc8aaa" + [[package]] name = "num-complex" version = "0.4.6" @@ -3486,7 +3628,7 @@ dependencies = [ "http-auth", "jsonwebtoken 10.3.0", "lazy_static", - "oci-spec", + "oci-spec 0.9.0", "olpc-cjson", "regex", "reqwest 0.13.2", @@ -3516,6 +3658,23 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "oci-spec" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3df6f876ad774d6a676f7e968f5c3edacc32f90e65fe680a8b686235396556fb" +dependencies = [ + "const_format", + "derive_builder", + "getset", + "regex", + "serde", + "serde_json", + "strum 0.27.2", + "strum_macros 0.27.2", + "thiserror 2.0.18", +] + [[package]] name = "oid-registry" version = "0.7.1" @@ -3630,6 +3789,7 @@ version = "0.0.0" dependencies = [ "base64 0.22.1", "chrono", + "container-device-interface", "ipnet", "miette", "prost", @@ -4231,6 +4391,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "35fb2e5f958ec131621fdd531e9fc186ed768cbe395337403ae56c17a74c68ec" +[[package]] +name = "path-clean" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17359afc20d7ab31fdb42bb844c8b3bb1dabd7dcf7e68428492da7f16966fcef" + [[package]] name = "pbkdf2" version = "0.12.2" @@ -4889,6 +5055,43 @@ dependencies = [ "bitflags 2.11.1", ] +[[package]] +name = "ref-cast" +version = "1.0.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f354300ae66f76f1c85c5f84693f0ce81d747e2c3f21a45fef496d89c960bf7d" +dependencies = [ + "ref-cast-impl", +] + +[[package]] +name = "ref-cast-impl" +version = "1.0.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7186006dcb21920990093f30e3dea63b7d6e977bf1256be20c3563a5db070da" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + +[[package]] +name = "referencing" +version = "0.46.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fbf332a2f81899f6836f22c03da73dae8a664c32e3016b84692c23cddadc95d" +dependencies = [ + "ahash", + "fluent-uri", + "getrandom 0.3.4", + "hashbrown 0.16.1", + "itoa", + "micromap", + "parking_lot", + "percent-encoding", + "serde_json", +] + [[package]] name = "regex" version = "1.12.3" @@ -4986,8 +5189,10 @@ checksum = "ab3f43e3283ab1488b624b44b0e988d0acea0b3214e694730a055cb6b2efa801" dependencies = [ "base64 0.22.1", "bytes", + "futures-channel", "futures-core", "futures-util", + "h2", "http 1.4.0", "http-body 1.0.1", "http-body-util", @@ -6890,6 +7095,12 @@ version = "0.3.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5c1cb5db39152898a79168971543b1cb5020dff7fe43c8dc468b0885f5e29df5" +[[package]] +name = "unicode-general-category" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b993bddc193ae5bd0d623b49ec06ac3e9312875fdae725a975c51db1cc1677f" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -7022,6 +7233,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "uuid-simd" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b082222b4f6619906941c17eb2297fff4c2fb96cb60164170522942a200bd8" +dependencies = [ + "outref", + "vsimd", +] + [[package]] name = "valuable" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index 184c9aa398..7b29ab6ce9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,6 +69,7 @@ nix = { version = "0.29", features = ["signal", "process", "user", "fs", "term"] rustix = { version = "1.1", features = ["process"] } # Serialization +container-device-interface = { git = "https://github.com/cncf-tags/container-device-interface-rs", tag = "v1.1.1" } serde = { version = "1", features = ["derive"] } serde_json = "1" serde_yml = "0.0.12" diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index d5d0912e23..5574344936 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -60,6 +60,17 @@ Resource requirements enter the driver layer through `SandboxSpec.resource_requi can request a specific number of GPUs or the driver-specific default behaviour. For all in-tree drivers, this is equivalent to selecting a single GPU. +For Docker GPU sandboxes, the driver treats CDI specs as runtime metadata for +both outer injection and inner sandbox policy. It selects opaque CDI device IDs, +passes them to Docker, mounts daemon-reported CDI spec directories into +supervisor-only paths, and uploads a versioned CDI context before starting the +container. The supervisor resolves that context inside the sandbox and derives +Landlock paths and supplemental groups from CDI `containerEdits`. Host-side CDI +spec paths are diagnostic only and are never treated as sandbox policy paths. +Kubernetes must not infer CDI device IDs from the `nvidia.com/gpu` resource +request; it needs a node-local selected-device handoff before using the same +supervisor resolver. + VM runtime state paths are derived only from driver-validated sandbox IDs matching `[A-Za-z0-9._-]{1,128}`. The gateway-owned VM driver socket uses a private `run/` directory plus Unix peer UID/PID checks. Standalone diff --git a/architecture/security-policy.md b/architecture/security-policy.md index c4eaa2328a..3da86221d9 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -23,10 +23,20 @@ dynamic and can be hot-reloaded when the new policy validates successfully. 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`. +does not weaken the whole ruleset. When GPU devices are present without a CDI +context, GPU baseline enrichment adds existing GPU device nodes as read-write +paths. GPU sandboxes with CDI context use CDI-derived paths instead of the +hard-coded GPU baseline. Both paths promote `/proc` to read-write because CUDA +workloads write thread metadata under `/proc//task//comm`. + +GPU/CDI sandboxes can also carry a supervisor-only CDI context from the compute +driver. The supervisor resolves selected CDI IDs from mounted CDI specs and +adds derived device nodes, library mount destinations, and supplemental GIDs +before agent exec. CDI host paths are ignored for policy. Derived mount +destinations default to read-only; writable CDI single-file mounts require an +exact `filesystem_policy.read_write` opt-in, and writable CDI directory mounts +fail closed. CDI resolution errors are security-relevant startup failures and +emit OCSF findings. ## Network Decisions diff --git a/crates/openshell-core/Cargo.toml b/crates/openshell-core/Cargo.toml index 0ff6d06d6c..9b21be1c46 100644 --- a/crates/openshell-core/Cargo.toml +++ b/crates/openshell-core/Cargo.toml @@ -20,6 +20,7 @@ thiserror = { workspace = true } miette = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +container-device-interface = { workspace = true } tracing = { workspace = true } url = { workspace = true } ipnet = "2" diff --git a/crates/openshell-core/src/cdi.rs b/crates/openshell-core/src/cdi.rs new file mode 100644 index 0000000000..88851382f3 --- /dev/null +++ b/crates/openshell-core/src/cdi.rs @@ -0,0 +1,890 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Shared CDI context schema and resolver helpers. + +use crate::paths::normalize_path; +use container_device_interface::{ + cache::{Cache, with_auto_refresh}, + container_edits::ContainerEdits as UpstreamContainerEdits, + spec_dirs::with_spec_dirs, +}; +use serde::{Deserialize, Serialize}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::fmt; +use std::hash::BuildHasher; +use std::path::{Component, Path, PathBuf}; + +pub const CDI_CONTEXT_VERSION: u32 = 1; +pub const SUPERVISOR_CDI_DIR: &str = "/run/openshell/supervisor"; +pub const CDI_CONTEXT_FILE_NAME: &str = "cdi-context.json"; +pub const CDI_CONTEXT_PATH: &str = "/run/openshell/supervisor/cdi-context.json"; +pub const CDI_SPEC_DIR_BASE: &str = "/run/openshell/supervisor/cdi-specs"; + +#[must_use] +pub fn cdi_spec_mount_path(index: usize) -> String { + format!("{CDI_SPEC_DIR_BASE}/{index}") +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CdiContext { + pub version: u32, + pub runtime: String, + pub selected_devices: Vec, + pub spec_dirs: Vec, +} + +impl CdiContext { + #[must_use] + pub fn new( + runtime: impl Into, + selected_devices: Vec, + spec_dirs: Vec, + ) -> Self { + Self { + version: CDI_CONTEXT_VERSION, + runtime: runtime.into(), + selected_devices, + spec_dirs, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CdiSpecDirectory { + pub path: String, + pub source: String, +} + +impl CdiSpecDirectory { + #[must_use] + pub fn new(path: impl Into, source: impl Into) -> Self { + Self { + path: path.into(), + source: source.into(), + } + } +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct CdiDerivedRequirements { + pub device_node_paths: Vec, + pub read_only_mount_paths: Vec, + pub read_write_mount_paths: Vec, + pub additional_gids: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CdiPathKind { + File, + Directory, + Other, +} + +impl fmt::Display for CdiPathKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::File => f.write_str("file"), + Self::Directory => f.write_str("directory"), + Self::Other => f.write_str("other"), + } + } +} + +#[derive(Debug, thiserror::Error)] +pub enum CdiError { + #[error("failed to read CDI context '{}': {source}", path.display())] + ContextRead { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to parse CDI context '{}': {source}", path.display())] + ContextParse { + path: PathBuf, + source: serde_json::Error, + }, + #[error("unsupported CDI context version {0}")] + UnsupportedContextVersion(u32), + #[error("CDI spec dir '{path}' from source '{diagnostic_source}' is unsafe: {reason}")] + UnsafeSpecDir { + path: String, + diagnostic_source: String, + reason: &'static str, + }, + #[error("selected CDI device '{0}' was not found in mounted CDI specs")] + MissingDevice(String), + #[error("failed to merge CDI edits for '{device}': {error}")] + EditMerge { device: String, error: String }, + #[error("failed to encode resolved CDI edits: {source}")] + EditEncode { source: serde_json::Error }, + #[error("failed to decode resolved CDI edits: {source}")] + EditDecode { source: serde_json::Error }, + #[error("CDI-derived path '{path}' is unsafe: {reason}")] + UnsafePolicyPath { path: String, reason: &'static str }, + #[error("CDI path '{path}' requested conflicting access modes")] + ConflictingAccess { path: String }, + #[error( + "CDI writable mount '{path}' is not explicitly listed in the sandbox policy read_write paths" + )] + WritableMountNotAllowed { path: String }, + #[error("CDI writable mount '{path}' must target a single file, found {kind}")] + WritableMountNotFile { path: String, kind: String }, + #[error("CDI mount '{path}' has conflicting ro/rw options")] + ConflictingMountOptions { path: String }, +} + +// Temporary view over upstream-resolved CDI edits. The Rust CDI crate currently +// keeps some spec-model fields crate-private even though they are serialized and +// public in specs-go, so OpenShell serializes the merged upstream model and +// decodes only the policy-relevant fields here. +#[derive(Debug, Default, Deserialize)] +struct CdiContainerEdits { + #[serde(default, rename = "deviceNodes")] + device_nodes: Vec, + #[serde(default)] + mounts: Vec, + #[serde(default, rename = "additionalGids")] + additional_gids: Vec, +} + +#[derive(Debug, Deserialize)] +struct CdiDeviceNode { + path: String, +} + +#[derive(Debug, Deserialize)] +struct CdiMount { + #[serde(rename = "containerPath")] + container_path: String, + #[serde(default)] + options: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CdiAccess { + ReadOnly, + ReadWrite, +} + +#[derive(Debug, Default)] +struct RequirementAccumulator { + device_node_paths: BTreeSet, + mount_paths: BTreeMap, + additional_gids: BTreeSet, +} + +impl RequirementAccumulator { + fn add_device_node(&mut self, path: String) -> Result<(), CdiError> { + match self.mount_paths.get(&path).copied() { + Some(CdiAccess::ReadOnly) => Err(CdiError::ConflictingAccess { path }), + Some(CdiAccess::ReadWrite) | None => { + self.device_node_paths.insert(path); + Ok(()) + } + } + } + + fn add_mount(&mut self, path: String, access: CdiAccess) -> Result<(), CdiError> { + if access == CdiAccess::ReadOnly && self.device_node_paths.contains(&path) { + return Err(CdiError::ConflictingAccess { path }); + } + match self.mount_paths.get(&path).copied() { + Some(existing) if existing != access => Err(CdiError::ConflictingAccess { path }), + Some(_) => Ok(()), + None => { + self.mount_paths.insert(path, access); + Ok(()) + } + } + } + + fn add_gid(&mut self, gid: u32) { + if gid == 0 { + tracing::debug!("Skipping CDI additionalGids entry for root GID 0"); + return; + } + self.additional_gids.insert(gid); + } + + fn validate_writable_mounts( + &self, + normalized_writable_file_allowlist: &HashSet, + path_kind: &F, + ) -> Result<(), CdiError> + where + F: Fn(&str) -> Option, + { + for (path, access) in &self.mount_paths { + if *access != CdiAccess::ReadWrite { + continue; + } + if !normalized_writable_file_allowlist.contains(path) { + return Err(CdiError::WritableMountNotAllowed { path: path.clone() }); + } + let kind = path_kind(path); + if kind != Some(CdiPathKind::File) { + return Err(CdiError::WritableMountNotFile { + path: path.clone(), + kind: kind.map_or_else(|| "missing".to_string(), |kind| kind.to_string()), + }); + } + } + Ok(()) + } + + fn build(self) -> CdiDerivedRequirements { + let mut read_only_mount_paths = Vec::new(); + let mut read_write_mount_paths = Vec::new(); + for (path, access) in self.mount_paths { + match access { + CdiAccess::ReadOnly => read_only_mount_paths.push(path), + CdiAccess::ReadWrite => read_write_mount_paths.push(path), + } + } + CdiDerivedRequirements { + device_node_paths: self.device_node_paths.into_iter().collect(), + read_only_mount_paths, + read_write_mount_paths, + additional_gids: self.additional_gids.into_iter().collect(), + } + } +} + +pub fn read_context(path: impl AsRef) -> Result { + let path = path.as_ref(); + let json = std::fs::read_to_string(path).map_err(|source| CdiError::ContextRead { + path: path.to_path_buf(), + source, + })?; + serde_json::from_str(&json).map_err(|source| CdiError::ContextParse { + path: path.to_path_buf(), + source, + }) +} + +pub fn resolve_cdi_context( + context: &CdiContext, + writable_file_allowlist: &HashSet, +) -> Result { + resolve_cdi_context_with_path_kind(context, writable_file_allowlist, filesystem_path_kind) +} + +pub fn resolve_cdi_context_with_path_kind( + context: &CdiContext, + writable_file_allowlist: &HashSet, + path_kind: F, +) -> Result +where + F: Fn(&str) -> Option, + S: BuildHasher, +{ + validate_context(context)?; + let selected_devices = selected_cdi_devices(&context.selected_devices); + if selected_devices.is_empty() || context.spec_dirs.is_empty() { + return Ok(CdiDerivedRequirements::default()); + } + + let normalized_allowlist = writable_file_allowlist + .iter() + .map(|path| normalize_path(path)) + .collect::>(); + + let edits = resolve_container_edits(context, &selected_devices)?; + let mut accumulator = RequirementAccumulator::default(); + accumulate_requirements(&edits, &normalized_allowlist, &path_kind, &mut accumulator)?; + Ok(accumulator.build()) +} + +fn validate_context(context: &CdiContext) -> Result<(), CdiError> { + if context.version != CDI_CONTEXT_VERSION { + return Err(CdiError::UnsupportedContextVersion(context.version)); + } + for spec_dir in &context.spec_dirs { + validate_absolute_no_parent(&spec_dir.path).map_err(|reason| CdiError::UnsafeSpecDir { + path: spec_dir.path.clone(), + diagnostic_source: spec_dir.source.clone(), + reason, + })?; + } + Ok(()) +} + +fn selected_cdi_devices(device_ids: &[String]) -> Vec { + let mut seen = HashSet::new(); + let mut parsed = Vec::new(); + for raw in device_ids { + let raw = raw.trim(); + if raw.is_empty() { + continue; + } + if seen.insert(raw.to_string()) { + parsed.push(raw.to_string()); + } + } + parsed +} + +fn resolve_container_edits( + context: &CdiContext, + selected_devices: &[String], +) -> Result { + let mut cache = build_cache(&context.spec_dirs); + let mut merged = UpstreamContainerEdits::new(); + let mut applied_specs = BTreeSet::new(); + + for device_id in selected_devices { + let device = cache + .get_device(device_id) + .cloned() + .ok_or_else(|| CdiError::MissingDevice(device_id.clone()))?; + let mut spec = device.get_spec(); + let spec_path = spec.get_path(); + if applied_specs.insert(spec_path) + && let Some(spec_edits) = spec.edits() + { + merged + .append(spec_edits) + .map_err(|err| CdiError::EditMerge { + device: device_id.clone(), + error: err.to_string(), + })?; + } + merged + .append(device.edits()) + .map_err(|err| CdiError::EditMerge { + device: device_id.clone(), + error: err.to_string(), + })?; + } + + let value = serde_json::to_value(&merged.container_edits) + .map_err(|source| CdiError::EditEncode { source })?; + serde_json::from_value(value).map_err(|source| CdiError::EditDecode { source }) +} + +fn build_cache(spec_dirs: &[CdiSpecDirectory]) -> Cache { + let spec_dir_paths = spec_dirs + .iter() + .map(|spec_dir| spec_dir.path.as_str()) + .collect::>(); + let mut cache = Cache::default(); + cache.configure(vec![ + with_spec_dirs(&spec_dir_paths), + with_auto_refresh(false), + ]); + if let Err(err) = cache.refresh() { + tracing::debug!( + error = %err, + "Ignoring CDI cache refresh error; requested device lookup will determine availability" + ); + } + cache +} + +fn accumulate_requirements( + edits: &CdiContainerEdits, + normalized_writable_file_allowlist: &HashSet, + path_kind: &F, + accumulator: &mut RequirementAccumulator, +) -> Result<(), CdiError> +where + F: Fn(&str) -> Option, +{ + for device_node in &edits.device_nodes { + let path = normalize_policy_path(&device_node.path)?; + accumulator.add_device_node(path)?; + } + + for mount in &edits.mounts { + let path = normalize_policy_path(&mount.container_path)?; + let access = mount_access(&path, &mount.options)?; + accumulator.add_mount(path, access)?; + } + + for gid in &edits.additional_gids { + accumulator.add_gid(*gid); + } + + accumulator.validate_writable_mounts(normalized_writable_file_allowlist, path_kind)?; + Ok(()) +} + +fn mount_access(path: &str, options: &[String]) -> Result { + let read_only_requested = options + .iter() + .any(|option| option.eq_ignore_ascii_case("ro")); + let read_write_requested = options + .iter() + .any(|option| option.eq_ignore_ascii_case("rw")); + // CDI mount options are stringly typed and runtime-specific normalization + // can differ. Treat simultaneous ro/rw as malformed instead of guessing + // which option a later mount syscall would effectively apply. + if read_only_requested && read_write_requested { + return Err(CdiError::ConflictingMountOptions { + path: path.to_string(), + }); + } + if read_write_requested { + Ok(CdiAccess::ReadWrite) + } else { + Ok(CdiAccess::ReadOnly) + } +} + +fn normalize_policy_path(path: &str) -> Result { + validate_absolute_no_parent(path).map_err(|reason| CdiError::UnsafePolicyPath { + path: path.to_string(), + reason, + })?; + let normalized = normalize_path(path); + if matches!( + normalized.as_str(), + "/" | "/dev" | "/proc" | "/sys" | "/run" | "/usr" + ) { + return Err(CdiError::UnsafePolicyPath { + path: path.to_string(), + reason: "broad root path is not allowed", + }); + } + Ok(normalized) +} + +fn validate_absolute_no_parent(path: &str) -> Result<(), &'static str> { + let path = Path::new(path); + if !path.is_absolute() { + return Err("path must be absolute"); + } + for component in path.components() { + match component { + Component::ParentDir => return Err("path must not contain '..'"), + Component::Prefix(_) => return Err("path must be a Unix-style absolute path"), + Component::CurDir => return Err("path must be normalized"), + Component::RootDir | Component::Normal(_) => {} + } + } + Ok(()) +} + +pub fn filesystem_path_kind(path: &str) -> Option { + let metadata = std::fs::metadata(path).ok()?; + if metadata.is_file() { + Some(CdiPathKind::File) + } else if metadata.is_dir() { + Some(CdiPathKind::Directory) + } else { + Some(CdiPathKind::Other) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write_spec(dir: &Path, name: &str, yaml: &str) { + std::fs::write(dir.join(name), yaml).unwrap(); + } + + fn context(dir: &Path, selected_devices: &[&str]) -> CdiContext { + CdiContext::new( + "test", + selected_devices + .iter() + .map(|device| (*device).to_string()) + .collect(), + vec![CdiSpecDirectory::new( + dir.to_string_lossy().into_owned(), + "/host/cdi", + )], + ) + } + + fn resolve_with_kind( + context: &CdiContext, + writable: &[&str], + kind: impl Fn(&str) -> Option, + ) -> Result { + let writable_file_allowlist: HashSet = + writable.iter().map(|path| (*path).to_string()).collect(); + resolve_cdi_context_with_path_kind(context, &writable_file_allowlist, kind) + } + + fn always_missing(_: &str) -> Option { + None + } + + #[test] + fn resolves_native_single_device_requirements() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r#" +cdiVersion: 0.6.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /dev/nvidiactl + - path: /dev/nvidia0 + mounts: + - hostPath: /host/libcuda.so.1 + containerPath: /usr/local/cuda/lib64/libcuda.so.1 +"#, + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap(); + + assert_eq!( + requirements.device_node_paths, + vec!["/dev/nvidia0", "/dev/nvidiactl"] + ); + assert_eq!( + requirements.read_only_mount_paths, + vec!["/usr/local/cuda/lib64/libcuda.so.1"] + ); + } + + #[test] + fn resolves_native_all_device_requirements() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: all + containerEdits: + deviceNodes: + - path: /dev/nvidiactl + - path: /dev/nvidia0 + - path: /dev/nvidia1 +", + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=all"]), + &[], + always_missing, + ) + .unwrap(); + + assert_eq!( + requirements.device_node_paths, + vec!["/dev/nvidia0", "/dev/nvidia1", "/dev/nvidiactl"] + ); + } + + #[test] + fn resolves_wsl_shape_requirements() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "wsl.yaml", + r" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: all + containerEdits: + deviceNodes: + - path: /dev/dxg + mounts: + - hostPath: /host/wsl/lib/libcuda.so.1 + containerPath: /usr/lib/wsl/lib/libcuda.so.1 +", + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=all"]), + &[], + always_missing, + ) + .unwrap(); + + assert_eq!(requirements.device_node_paths, vec!["/dev/dxg"]); + assert_eq!( + requirements.read_only_mount_paths, + vec!["/usr/lib/wsl/lib/libcuda.so.1"] + ); + } + + #[test] + fn resolves_tegra_shape_requirements_and_gids() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "tegra.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +containerEdits: + additionalGids: [44] +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /dev/nvmap + - path: /dev/nvhost-gpu +"#, + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap(); + + assert_eq!(requirements.additional_gids, vec![44]); + assert_eq!( + requirements.device_node_paths, + vec!["/dev/nvhost-gpu", "/dev/nvmap"] + ); + } + + #[test] + fn accepts_writable_single_file_mount_with_explicit_policy_opt_in() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "rw-file.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + mounts: + - hostPath: /host/nvidia/cache.db + containerPath: /opt/nvidia/cache.db + options: [rw] +"#, + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &["/opt/nvidia/cache.db"], + |path| (path == "/opt/nvidia/cache.db").then_some(CdiPathKind::File), + ) + .unwrap(); + + assert_eq!( + requirements.read_write_mount_paths, + vec!["/opt/nvidia/cache.db"] + ); + } + + #[test] + fn rejects_writable_directory_mount() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "rw-dir.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + mounts: + - hostPath: /host/nvidia/cache + containerPath: /opt/nvidia/cache + options: [rw] +"#, + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &["/opt/nvidia/cache"], + |path| (path == "/opt/nvidia/cache").then_some(CdiPathKind::Directory), + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::WritableMountNotFile { .. })); + } + + #[test] + fn rejects_missing_selected_device() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + env: + - OPEN_SHELL_TEST=1 +"#, + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=1"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::MissingDevice(device) if device == "nvidia.com/gpu=1")); + } + + #[test] + fn defers_malformed_spec_errors_to_device_resolution() { + let dir = tempfile::tempdir().unwrap(); + write_spec(dir.path(), "broken.yaml", "kind: ["); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::MissingDevice(device) if device == "nvidia.com/gpu=0")); + } + + #[test] + fn empty_selection_is_noop() { + let dir = tempfile::tempdir().unwrap(); + let requirements = resolve_with_kind(&context(dir.path(), &[]), &[], always_missing) + .expect("empty selection should resolve to empty requirements"); + + assert_eq!(requirements, CdiDerivedRequirements::default()); + } + + #[test] + fn empty_spec_dirs_are_noop() { + let context = CdiContext::new("test", vec!["nvidia.com/gpu=0".to_string()], Vec::new()); + + let requirements = resolve_with_kind(&context, &[], always_missing) + .expect("empty spec dirs should resolve to empty requirements"); + + assert_eq!(requirements, CdiDerivedRequirements::default()); + } + + #[test] + fn defers_device_id_shape_to_upstream_resolution() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + env: + - OPEN_SHELL_TEST=1 +"#, + ); + + let err = resolve_with_kind(&context(dir.path(), &["not-a-cdi-id"]), &[], always_missing) + .unwrap_err(); + + assert!(matches!(err, CdiError::MissingDevice(device) if device == "not-a-cdi-id")); + } + + #[test] + fn rejects_unsafe_policy_paths() { + for path in [ + "relative", "/dev", "/proc", "/sys", "/run", "/usr", "/a/../b", + ] { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "unsafe.yaml", + &format!( + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: {path} +"# + ), + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!( + matches!(err, CdiError::UnsafePolicyPath { .. }), + "expected unsafe path error for {path}, got {err:?}" + ); + } + } + + #[test] + fn rejects_duplicate_access_conflicts() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "conflict.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /dev/nvidia0 + mounts: + - hostPath: /host/dev/nvidia0 + containerPath: /dev/nvidia0 + options: [ro] +"#, + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::ConflictingAccess { .. })); + } + + #[test] + fn skips_root_additional_gid() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "root-gid.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + additionalGids: [0, 44] +"#, + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap(); + + assert_eq!(requirements.additional_gids, vec![44]); + } +} diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index bb61e1f56f..90ae93a2f1 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -11,6 +11,7 @@ pub mod activity; pub mod auth; +pub mod cdi; pub mod config; pub mod denial; pub mod driver_mounts; diff --git a/crates/openshell-core/src/policy.rs b/crates/openshell-core/src/policy.rs index 1645b9da44..30c19ca4dc 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)] @@ -162,6 +168,7 @@ impl From for ProcessPolicy { } else { Some(proto.run_as_group) }, + supplemental_groups: Vec::new(), } } } diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index f066580636..b70ab63624 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -85,6 +85,14 @@ pub const SANDBOX_TOKEN_FILE: &str = "OPENSHELL_SANDBOX_TOKEN_FILE"; /// SSH child processes (which use `env_clear()` for security isolation). pub const USER_ENVIRONMENT: &str = "OPENSHELL_USER_ENVIRONMENT"; +/// Path to the supervisor-only CDI context JSON file. +/// +/// Set by CDI-capable compute drivers only when a sandbox requested GPU/CDI +/// devices. The sandbox supervisor reads this file before policy construction +/// and derives inner filesystem and process requirements from mounted CDI +/// specs. +pub const CDI_CONTEXT: &str = "OPENSHELL_CDI_CONTEXT"; + /// Path to the projected `ServiceAccount` JWT (Kubernetes driver). /// /// Used to bootstrap a gateway-minted JWT via `IssueSandboxToken`. Kubelet diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index 82e5eea4b7..ec348faf35 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -33,9 +33,33 @@ contract: | `restart_policy = unless-stopped` | Keeps managed sandboxes resumable across daemon or gateway restarts. | | `PidsLimit` | Enforces the sandbox PID budget at the Docker cgroup layer. Set `[openshell.drivers.docker].sandbox_pids_limit = 0` to inherit the Docker/runtime default. | | CDI GPU request | Uses opaque `driver_config.cdi_devices` values when set; otherwise selects the requested count of NVIDIA CDI GPUs in round-robin order when daemon CDI support is detected. Docker daemon `/info` can permit `nvidia.com/gpu=all` as a WSL2 all-only compatibility fallback, where it counts as one selectable device. Exact CDI device lists must not contain duplicates and must match the effective GPU count. | +| CDI context upload | For GPU/CDI sandboxes only, mounts daemon-reported CDI spec directories read-only under `/run/openshell/supervisor/cdi-specs/` and uploads `/run/openshell/supervisor/cdi-context.json` after container create and before start. | The agent child process does not retain these supervisor privileges. +## CDI GPU Metadata + +Docker remains the source of truth for GPU injection. The driver selects opaque +CDI device IDs from `driver_config.cdi_devices` or the daemon's discovered CDI +inventory, then passes the same IDs to Docker with a CDI `DeviceRequest`. + +When a GPU/CDI request is present, the driver also mounts the Docker +daemon-reported `Info.CDISpecDirs` into supervisor-only paths and uploads a +small versioned CDI context through Docker's container archive API. The context +uses container-side spec paths for resolution and keeps host-side spec sources +diagnostic-only. If the upload fails, the driver removes the created container +and sandbox token file before reporting the failure. + +The sandbox supervisor resolves the selected IDs from those mounted specs +before it launches agent processes. CDI device nodes become read-write +Landlock paths, mount destinations default to read-only paths, and +`additionalGids` become supplemental groups for the entrypoint and SSH child +processes. Writable CDI mount destinations are accepted only for exact +single-file paths already listed in the sandbox policy `read_write` list; +writable CDI directory mounts fail closed. Kubernetes, Podman, WSL2 hardware +validation, and Tegra/Jetson hardware validation are separate follow-up +targets. + ## Driver Config Mounts The gateway forwards the `docker` block from `--driver-config-json` to this diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 06d575a1e1..7ae033ca66 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -5,7 +5,6 @@ #![allow(clippy::result_large_err)] -use bollard::Docker; use bollard::errors::Error as BollardError; use bollard::models::{ ContainerCreateBody, ContainerSummary, ContainerSummaryStateEnum, CreateImageInfo, @@ -16,9 +15,12 @@ use bollard::models::{ use bollard::query_parameters::{ CreateContainerOptionsBuilder, CreateImageOptions, DownloadFromContainerOptionsBuilder, ListContainersOptionsBuilder, RemoveContainerOptionsBuilder, StopContainerOptionsBuilder, + UploadToContainerOptionsBuilder, }; +use bollard::{Docker, body_full}; use bytes::Bytes; use futures::{Stream, StreamExt}; +use openshell_core::cdi::{CdiContext, CdiSpecDirectory, cdi_spec_mount_path}; use openshell_core::config::{ DEFAULT_DOCKER_NETWORK_NAME, DEFAULT_SANDBOX_PIDS_LIMIT, DEFAULT_STOP_TIMEOUT_SECS, }; @@ -50,7 +52,7 @@ use openshell_core::proto_struct::{ }; use openshell_core::{Config, Error, Result as CoreResult}; use std::collections::{HashMap, HashSet}; -use std::io::Read; +use std::io::{Cursor, Read}; use std::net::{IpAddr, SocketAddr}; use std::path::{Path, PathBuf}; use std::pin::Pin; @@ -184,12 +186,65 @@ struct DockerDriverRuntimeConfig { supervisor_bin: PathBuf, guest_tls: Option, daemon_version: String, - supports_gpu: bool, - allow_all_default_gpu: bool, + gpu: DockerGpuRuntimeConfig, sandbox_pids_limit: i64, enable_bind_mounts: bool, } +#[derive(Debug, Clone, Default)] +struct DockerGpuRuntimeConfig { + cdi_spec_dirs: Vec, + allow_all_default: bool, +} + +impl DockerGpuRuntimeConfig { + fn supports_gpu(&self) -> bool { + !self.cdi_spec_dirs.is_empty() + } + + fn cdi_context(&self, gpu_device_ids: Option<&[String]>) -> Result, Status> { + let Some(gpu_device_ids) = gpu_device_ids.filter(|device_ids| !device_ids.is_empty()) + else { + return Ok(None); + }; + self.require_cdi_spec_dirs()?; + Ok(Some(CdiContext::new( + "docker", + gpu_device_ids.to_vec(), + self.cdi_spec_dirs + .iter() + .enumerate() + .map(|(index, source)| CdiSpecDirectory::new(cdi_spec_mount_path(index), source)) + .collect(), + ))) + } + + fn cdi_spec_bind_strings( + &self, + gpu_device_ids: Option<&[String]>, + ) -> Result, Status> { + let Some(_) = gpu_device_ids.filter(|device_ids| !device_ids.is_empty()) else { + return Ok(Vec::new()); + }; + self.require_cdi_spec_dirs()?; + Ok(self + .cdi_spec_dirs + .iter() + .enumerate() + .map(|(index, source)| format!("{source}:{}:ro,z", cdi_spec_mount_path(index))) + .collect()) + } + + fn require_cdi_spec_dirs(&self) -> Result<(), Status> { + if self.cdi_spec_dirs.is_empty() { + return Err(Status::failed_precondition( + "docker GPU sandboxes require Docker CDI spec directories reported by the daemon", + )); + } + Ok(()) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] enum DockerGatewayRoute { Bridge { @@ -315,12 +370,11 @@ impl DockerComputeDriver { let info = docker.info().await.map_err(|err| { Error::execution(format!("failed to query Docker daemon info: {err}")) })?; - let supports_gpu = info - .cdi_spec_dirs - .as_ref() - .is_some_and(|dirs| !dirs.is_empty()); + let gpu = DockerGpuRuntimeConfig { + cdi_spec_dirs: info.cdi_spec_dirs.clone().unwrap_or_default(), + allow_all_default: docker_info_reports_wsl2(&info), + }; let cdi_gpu_inventory = docker_cdi_gpu_inventory(&info); - let allow_all_default_gpu = docker_info_reports_wsl2(&info); validate_sandbox_pids_limit(docker_config.sandbox_pids_limit)?; let gateway_port = config.bind_address.port(); if gateway_port == 0 { @@ -367,8 +421,7 @@ impl DockerComputeDriver { supervisor_bin, guest_tls, daemon_version: version.version.unwrap_or_else(|| "unknown".to_string()), - supports_gpu, - allow_all_default_gpu, + gpu: gpu.clone(), sandbox_pids_limit: docker_config.sandbox_pids_limit, enable_bind_mounts: docker_config.enable_bind_mounts, }, @@ -377,7 +430,7 @@ impl DockerComputeDriver { supervisor_readiness, gpu_selector: Arc::new(CdiGpuDefaultSelector::new( cdi_gpu_inventory, - allow_all_default_gpu, + gpu.allow_all_default, )), }; @@ -433,7 +486,7 @@ impl DockerComputeDriver { DockerSandboxDriverConfig::from_template(template).map_err(Status::invalid_argument)?; validate_docker_driver_mounts(&driver_config.mounts, config.enable_bind_mounts)?; let gpu_requirements = driver_gpu_requirements(spec.resource_requirements.as_ref()); - Self::validate_gpu_request(gpu_requirements, config.supports_gpu, &driver_config)?; + Self::validate_gpu_request(gpu_requirements, config.gpu.supports_gpu(), &driver_config)?; Ok(ValidatedDockerSandbox { template, driver_config, @@ -541,7 +594,7 @@ impl DockerComputeDriver { .map_err(|err| internal_status("query Docker daemon info", err))?; self.gpu_selector.refresh( docker_cdi_gpu_inventory(&info), - self.config.allow_all_default_gpu, + self.config.gpu.allow_all_default, ); Ok(()) } @@ -694,11 +747,6 @@ impl DockerComputeDriver { .map_err(|status| { DockerProvisioningFailure::new("ImagePullFailed", status.message()) })?; - let token_file_created = write_sandbox_token_file(sandbox, &self.config) - .await - .map_err(|status| { - DockerProvisioningFailure::new("SandboxTokenWriteFailed", status.message()) - })?; let container_name = container_name_for_sandbox(sandbox); let gpu_devices = self @@ -709,9 +757,13 @@ impl DockerComputeDriver { ) .await .map_err(|status| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } + DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) + })?; + let cdi_context = self + .config + .gpu + .cdi_context(gpu_devices.as_deref()) + .map_err(|status| { DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; let create_body = build_container_create_body_with_gpu_devices( @@ -721,11 +773,13 @@ impl DockerComputeDriver { gpu_devices.as_deref(), ) .map_err(|status| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; + write_sandbox_token_file(sandbox, &self.config) + .await + .map_err(|status| { + DockerProvisioningFailure::new("SandboxTokenWriteFailed", status.message()) + })?; self.docker .create_container( Some( @@ -737,9 +791,6 @@ impl DockerComputeDriver { ) .await .map_err(|err| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } DockerProvisioningFailure::from_status( "ContainerCreateFailed", create_status_from_docker_error("create docker sandbox container", err), @@ -752,25 +803,28 @@ impl DockerComputeDriver { HashMap::from([("container_name".to_string(), container_name.clone())]), ); + if let Some(cdi_context) = cdi_context + && let Err(err) = self.upload_cdi_context(&container_name, &cdi_context).await + { + self.cleanup_created_container_after_failure( + &sandbox.id, + &container_name, + "CDI context upload failure", + ) + .await; + return Err(DockerProvisioningFailure::from_status( + "CdiContextUploadFailed", + err, + )); + } + if let Err(err) = self.docker.start_container(&container_name, None).await { - let cleanup = self - .docker - .remove_container( - &container_name, - Some(RemoveContainerOptionsBuilder::default().force(true).build()), - ) - .await; - if let Err(cleanup_err) = cleanup { - warn!( - sandbox_id = %sandbox.id, - container_name, - error = %cleanup_err, - "Failed to clean up Docker container after start failure" - ); - } - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } + self.cleanup_created_container_after_failure( + &sandbox.id, + &container_name, + "container start failure", + ) + .await; return Err(DockerProvisioningFailure::from_status( "ContainerStartFailed", create_status_from_docker_error("start docker sandbox container", err), @@ -796,6 +850,51 @@ impl DockerComputeDriver { Ok(()) } + async fn cleanup_created_container_after_failure( + &self, + sandbox_id: &str, + container_name: &str, + phase: &'static str, + ) { + let cleanup = self + .docker + .remove_container( + container_name, + Some(RemoveContainerOptionsBuilder::default().force(true).build()), + ) + .await; + if let Err(cleanup_err) = cleanup { + warn!( + sandbox_id = %sandbox_id, + container_name = %container_name, + phase, + error = %cleanup_err, + "Failed to clean up Docker container after provisioning failure" + ); + } + } + + async fn upload_cdi_context( + &self, + container_name: &str, + context: &CdiContext, + ) -> Result<(), Status> { + let archive = build_cdi_context_archive(context).map_err(Status::internal)?; + self.docker + .upload_to_container( + container_name, + Some( + UploadToContainerOptionsBuilder::default() + .path("/run") + .no_overwrite_dir_non_dir("true") + .build(), + ), + body_full(Bytes::from(archive)), + ) + .await + .map_err(|err| internal_status("upload CDI context to Docker container", err)) + } + async fn delete_sandbox_inner( &self, sandbox_id: &str, @@ -2110,7 +2209,11 @@ fn cleanup_sandbox_token_file_by_id(sandbox_id: &str, config: &DockerDriverRunti } } -fn build_environment(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) -> Vec { +fn build_environment( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, + include_cdi_context: bool, +) -> Vec { let mut environment = HashMap::from([ ("HOME".to_string(), "/root".to_string()), ("PATH".to_string(), SUPERVISOR_PATH.to_string()), @@ -2162,6 +2265,12 @@ fn build_environment(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig openshell_core::sandbox_env::TELEMETRY_ENABLED.to_string(), openshell_core::telemetry::enabled_env_value().to_string(), ); + if include_cdi_context { + environment.insert( + openshell_core::sandbox_env::CDI_CONTEXT.to_string(), + openshell_core::cdi::CDI_CONTEXT_PATH.to_string(), + ); + } // The root supervisor executes namespace helpers during bootstrap; keep // their search path driver-owned even when the template/spec set PATH. environment.insert("PATH".to_string(), SUPERVISOR_PATH.to_string()); @@ -2232,6 +2341,68 @@ fn docker_gpu_selection_status(err: CdiGpuSelectionError) -> Status { Status::failed_precondition(err.to_string()) } +fn cdi_context_requested(gpu_device_ids: Option<&[String]>) -> bool { + gpu_device_ids.is_some_and(|device_ids| !device_ids.is_empty()) +} + +fn build_cdi_context_archive(context: &CdiContext) -> Result, String> { + let json = serde_json::to_vec_pretty(context).map_err(|err| err.to_string())?; + let mut archive = DockerTarArchiveBuilder::new(); + archive.append_dir("openshell", 0o700)?; + archive.append_dir("openshell/supervisor", 0o700)?; + archive.append_file( + &format!( + "openshell/supervisor/{}", + openshell_core::cdi::CDI_CONTEXT_FILE_NAME + ), + 0o600, + json, + )?; + archive.into_inner() +} + +struct DockerTarArchiveBuilder { + inner: tar::Builder>, +} + +impl DockerTarArchiveBuilder { + fn new() -> Self { + Self { + inner: tar::Builder::new(Vec::new()), + } + } + + fn append_dir(&mut self, path: &str, mode: u32) -> Result<(), String> { + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Directory); + header.set_size(0); + header.set_mode(mode); + header.set_uid(0); + header.set_gid(0); + header.set_cksum(); + self.inner + .append_data(&mut header, path, std::io::empty()) + .map_err(|err| err.to_string()) + } + + fn append_file(&mut self, path: &str, mode: u32, contents: Vec) -> Result<(), String> { + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Regular); + header.set_size(u64::try_from(contents.len()).map_err(|err| err.to_string())?); + header.set_mode(mode); + header.set_uid(0); + header.set_gid(0); + header.set_cksum(); + self.inner + .append_data(&mut header, path, Cursor::new(contents)) + .map_err(|err| err.to_string()) + } + + fn into_inner(self) -> Result, String> { + self.inner.into_inner().map_err(|err| err.to_string()) + } +} + #[cfg(test)] fn build_container_create_body( sandbox: &DriverSandbox, @@ -2304,7 +2475,11 @@ fn build_container_create_body_with_gpu_devices( Ok(ContainerCreateBody { image: Some(template.image.clone()), user: Some("0".to_string()), - env: Some(build_environment(sandbox, config)), + env: Some(build_environment( + sandbox, + config, + cdi_context_requested(gpu_device_ids), + )), entrypoint: Some(vec![SUPERVISOR_MOUNT_PATH.to_string()]), // Clear the image CMD so Docker does not append inherited args to the // supervisor entrypoint. @@ -2317,6 +2492,7 @@ fn build_container_create_body_with_gpu_devices( device_requests, binds: { let mut binds = build_binds(sandbox, config)?; + binds.extend(config.gpu.cdi_spec_bind_strings(gpu_device_ids)?); binds.extend(user_bind_strings); Some(binds) }, @@ -3254,7 +3430,7 @@ async fn download_binary_from_container( /// Docker's `/containers//archive` endpoint returns a single-file tar /// when `path` points to a file, so we only need the first entry. fn extract_first_tar_entry(tar_bytes: &[u8]) -> Result, String> { - let mut archive = tar::Archive::new(std::io::Cursor::new(tar_bytes)); + let mut archive = tar::Archive::new(Cursor::new(tar_bytes)); let mut entries = archive .entries() .map_err(|err| format!("open tar archive: {err}"))?; diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 594308bce5..512680fa27 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -86,7 +86,10 @@ fn gpu_resources(count: Option) -> ResourceRequirements { } } -fn runtime_config() -> DockerDriverRuntimeConfig { +const TEST_CDI_SPEC_DIR: &str = "/opt/openshell-test/cdi"; +const TEST_CDI_SPEC_DIR_ALT: &str = "/srv/openshell-test/cdi"; + +fn runtime_config(supports_gpu: bool) -> DockerDriverRuntimeConfig { DockerDriverRuntimeConfig { default_image: "image:latest".to_string(), image_pull_policy: String::new(), @@ -110,13 +113,32 @@ fn runtime_config() -> DockerDriverRuntimeConfig { key: PathBuf::from("/tmp/tls.key"), }), daemon_version: "28.0.0".to_string(), - supports_gpu: false, - allow_all_default_gpu: false, + gpu: gpu_runtime_config(supports_gpu), sandbox_pids_limit: DEFAULT_SANDBOX_PIDS_LIMIT, enable_bind_mounts: false, } } +fn runtime_config_with_cdi_spec_dirs(cdi_spec_dirs: &[&str]) -> DockerDriverRuntimeConfig { + let mut config = runtime_config(false); + config.gpu.cdi_spec_dirs = cdi_spec_dirs + .iter() + .map(|path| (*path).to_string()) + .collect(); + config +} + +fn gpu_runtime_config(supports_gpu: bool) -> DockerGpuRuntimeConfig { + if supports_gpu { + DockerGpuRuntimeConfig { + cdi_spec_dirs: vec![TEST_CDI_SPEC_DIR.to_string()], + ..Default::default() + } + } else { + DockerGpuRuntimeConfig::default() + } +} + fn json_struct(value: serde_json::Value) -> prost_types::Struct { match json_value(value).kind { Some(prost_types::value::Kind::StructValue(value)) => value, @@ -178,7 +200,7 @@ impl SupervisorReadiness for DisconnectedSupervisorReadiness { } fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDriver { - let allow_all_default_gpu = config.allow_all_default_gpu; + let allow_all_default_gpu = config.gpu.allow_all_default; DockerComputeDriver { docker: Arc::new( Docker::connect_with_http("http://127.0.0.1:2375", 1, bollard::API_DEFAULT_VERSION) @@ -553,14 +575,14 @@ fn docker_compute_config_disables_bind_mounts_by_default() { #[test] fn container_create_body_sets_driver_owned_pids_limit() { - let body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); let host_config = body.host_config.expect("host config"); assert_eq!(host_config.pids_limit, Some(DEFAULT_SANDBOX_PIDS_LIMIT)); } #[test] fn build_environment_sets_docker_tls_paths() { - let env = build_environment(&test_sandbox(), &runtime_config()); + let env = build_environment(&test_sandbox(), &runtime_config(false), false); assert!(env.contains(&format!("OPENSHELL_TLS_CA={TLS_CA_MOUNT_PATH}"))); assert!(env.contains(&format!("OPENSHELL_TLS_CERT={TLS_CERT_MOUNT_PATH}"))); assert!(env.contains(&format!("OPENSHELL_TLS_KEY={TLS_KEY_MOUNT_PATH}"))); @@ -581,7 +603,7 @@ fn build_environment_keeps_path_driver_controlled() { .environment .insert("PATH".to_string(), "/malicious/template/bin".to_string()); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); let path_entries = env .iter() .filter(|entry| entry.starts_with("PATH=")) @@ -607,7 +629,7 @@ fn build_environment_keeps_telemetry_toggle_driver_controlled() { "true".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); let telemetry_entries = env .iter() .filter(|entry| { @@ -629,7 +651,7 @@ fn build_environment_keeps_telemetry_toggle_driver_controlled() { #[test] fn build_binds_uses_docker_tls_directory() { - let binds = build_binds(&test_sandbox(), &runtime_config()).unwrap(); + let binds = build_binds(&test_sandbox(), &runtime_config(false)).unwrap(); let targets = binds .iter() .filter_map(|bind| bind.split(':').nth(1).map(String::from)) @@ -668,7 +690,7 @@ fn build_container_create_body_includes_driver_config_mounts() { ] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -716,7 +738,7 @@ fn driver_config_defaults_volume_mounts_to_read_only() { }] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -745,7 +767,7 @@ fn driver_config_allows_explicit_writable_volume_mounts() { }] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -779,7 +801,7 @@ fn driver_config_rejects_duplicate_mount_targets() { ] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!( @@ -806,7 +828,7 @@ fn driver_config_rejects_bind_mounts_unless_enabled() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("enable_bind_mounts = true")); @@ -832,7 +854,7 @@ fn build_container_create_body_includes_bind_mounts_when_enabled() { "read_only": true }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -877,7 +899,7 @@ fn driver_config_defaults_enabled_bind_mounts_to_read_only() { "target": "/sandbox/host" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -915,7 +937,7 @@ fn bind_mount_selinux_shared_label() { "selinux_label": "shared" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -953,7 +975,7 @@ fn bind_mount_selinux_private_label() { "selinux_label": "private" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -990,7 +1012,7 @@ fn bind_mount_without_selinux_label() { "read_only": false }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1024,7 +1046,7 @@ fn driver_config_rejects_missing_bind_source() { "target": "/sandbox/data" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let err = build_container_create_body(&sandbox, &config).unwrap_err(); @@ -1054,7 +1076,7 @@ fn driver_config_rejects_relative_bind_sources_when_enabled() { "target": "/sandbox/host" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let err = build_container_create_body(&sandbox, &config).unwrap_err(); @@ -1084,7 +1106,7 @@ fn driver_config_rejects_image_mounts() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("invalid docker driver_config")); @@ -1108,7 +1130,7 @@ fn driver_config_rejects_reserved_mount_targets() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("reserved OpenShell path")); @@ -1176,7 +1198,7 @@ fn build_environment_uses_token_file_without_raw_token_env() { "user-provided-token".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); assert!(!env.iter().any(|entry| { entry.starts_with(&format!("{}=", openshell_core::sandbox_env::SANDBOX_TOKEN)) @@ -1200,7 +1222,7 @@ fn managed_container_label_filters_include_gateway_namespace() { #[test] fn build_container_create_body_clears_inherited_cmd() { - let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let create_body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); assert_eq!( create_body.entrypoint, @@ -1246,7 +1268,7 @@ fn build_container_create_body_clears_inherited_cmd() { #[test] fn validate_sandbox_rejects_gpu_when_cdi_unavailable() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1258,7 +1280,7 @@ fn validate_sandbox_rejects_gpu_when_cdi_unavailable() { #[test] fn validate_sandbox_rejects_missing_gpu_support_before_request_shape() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1272,7 +1294,7 @@ fn validate_sandbox_rejects_missing_gpu_support_before_request_shape() { #[test] fn validate_sandbox_rejects_invalid_cdi_devices_before_gpu_capability() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1287,7 +1309,7 @@ fn validate_sandbox_rejects_invalid_cdi_devices_before_gpu_capability() { #[test] fn validate_sandbox_rejects_unknown_driver_config_fields() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1302,8 +1324,7 @@ fn validate_sandbox_rejects_unknown_driver_config_fields() { #[test] fn validate_sandbox_accepts_gpu_count_request_shape() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(Some(2))); @@ -1313,8 +1334,7 @@ fn validate_sandbox_accepts_gpu_count_request_shape() { #[test] fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1329,8 +1349,7 @@ fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() { #[test] fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1342,8 +1361,7 @@ fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() { #[test] fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1363,8 +1381,7 @@ fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() { #[test] fn validate_sandbox_rejects_cdi_devices_without_gpu_request() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox .spec @@ -1383,8 +1400,7 @@ fn validate_sandbox_rejects_cdi_devices_without_gpu_request() { #[test] fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1401,7 +1417,7 @@ fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() { #[test] fn validate_sandbox_rejects_template_errors_before_device_config() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1439,8 +1455,7 @@ fn validate_sandbox_auth_accepts_gateway_token() { #[test] fn build_container_create_body_maps_default_gpu_to_selected_cdi_device() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1467,10 +1482,102 @@ fn build_container_create_body_maps_default_gpu_to_selected_cdi_device() { ); } +#[test] +fn build_container_create_body_adds_cdi_context_env_and_spec_mounts_for_gpu() { + let config = runtime_config_with_cdi_spec_dirs(&[TEST_CDI_SPEC_DIR, TEST_CDI_SPEC_DIR_ALT]); + let mut sandbox = test_sandbox(); + sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); + + let driver_config = DockerSandboxDriverConfig::default(); + let gpu_devices = vec!["nvidia.com/gpu=1".to_string()]; + let create_body = build_container_create_body_with_gpu_devices( + &sandbox, + &config, + &driver_config, + Some(&gpu_devices), + ) + .unwrap(); + + let env = create_body.env.expect("env should be set"); + assert!(env.iter().any(|entry| { + entry + == &format!( + "{}={}", + openshell_core::sandbox_env::CDI_CONTEXT, + openshell_core::cdi::CDI_CONTEXT_PATH + ) + })); + + let binds = create_body + .host_config + .expect("host config") + .binds + .expect("binds should be set"); + assert!( + binds.iter().any(|bind| { + bind == &format!("{TEST_CDI_SPEC_DIR}:{}:ro,z", cdi_spec_mount_path(0)) + }) + ); + assert!(binds.iter().any(|bind| { + bind == &format!("{TEST_CDI_SPEC_DIR_ALT}:{}:ro,z", cdi_spec_mount_path(1)) + })); +} + +#[test] +fn build_container_create_body_omits_cdi_context_for_non_gpu() { + let mut config = runtime_config(false); + config.gpu.cdi_spec_dirs = vec![TEST_CDI_SPEC_DIR.to_string()]; + let create_body = build_container_create_body(&test_sandbox(), &config).unwrap(); + + let env = create_body.env.expect("env should be set"); + assert!( + !env.iter() + .any(|entry| entry.starts_with(openshell_core::sandbox_env::CDI_CONTEXT)) + ); + + let binds = create_body + .host_config + .expect("host config") + .binds + .expect("binds should be set"); + assert!( + !binds + .iter() + .any(|bind| bind.contains(openshell_core::cdi::CDI_SPEC_DIR_BASE)) + ); +} + +#[test] +fn build_cdi_context_archive_contains_context_json() { + use std::io::Read as _; + + let context = CdiContext::new( + "docker", + vec!["nvidia.com/gpu=0".to_string()], + vec![CdiSpecDirectory::new( + cdi_spec_mount_path(0), + TEST_CDI_SPEC_DIR, + )], + ); + let bytes = build_cdi_context_archive(&context).unwrap(); + let mut archive = tar::Archive::new(Cursor::new(bytes)); + let mut found = false; + for entry in archive.entries().unwrap() { + let mut entry = entry.unwrap(); + if entry.path().unwrap().as_ref() == Path::new("openshell/supervisor/cdi-context.json") { + let mut payload = String::new(); + entry.read_to_string(&mut payload).unwrap(); + let parsed: CdiContext = serde_json::from_str(&payload).unwrap(); + assert_eq!(parsed, context); + found = true; + } + } + assert!(found, "archive must include cdi-context.json"); +} + #[test] fn build_container_create_body_omits_devices_without_resolved_default_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1487,8 +1594,7 @@ fn build_container_create_body_omits_devices_without_resolved_default_cdi_device #[test] fn build_container_create_body_passes_explicit_cdi_device_id_through() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1511,8 +1617,7 @@ fn build_container_create_body_passes_explicit_cdi_device_id_through() { #[test] fn build_container_create_body_rejects_gpu_count_mismatched_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1539,7 +1644,7 @@ fn build_container_create_body_rejects_cdi_devices_without_gpu_request() { .unwrap() .driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"])); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::InvalidArgument); assert!(err.message().contains("requires a gpu request")); } @@ -1551,15 +1656,14 @@ fn build_container_create_body_rejects_empty_cdi_devices() { spec.resource_requirements = Some(gpu_resources(None)); spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&[])); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::InvalidArgument); assert!(err.message().contains("non-empty list")); } #[test] fn driver_default_gpu_selection_consumes_distinct_devices_for_creates() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let driver = test_driver_with_config(config); driver.gpu_selector.refresh( CdiGpuInventory::new(["nvidia.com/gpu=0", "nvidia.com/gpu=1"]), @@ -1691,7 +1795,7 @@ fn require_sandbox_identifier_rejects_when_id_and_name_are_empty() { #[test] fn build_container_create_body_uses_bridge_network() { - let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let create_body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); let host_config = create_body.host_config.expect("host_config is populated"); assert_eq!( @@ -1717,7 +1821,7 @@ fn build_container_create_body_uses_runtime_namespace_label() { // with that empty value would not match subsequent list/get/find // queries (which filter on `config.sandbox_namespace`), leaking // sandboxes that the driver itself cannot observe. - let mut config = runtime_config(); + let mut config = runtime_config(false); config.sandbox_namespace = "tenant-a".to_string(); let mut sandbox = test_sandbox(); sandbox.namespace = "ignored-by-driver".to_string(); diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 7a1085f1bf..50d9f095e5 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -15,7 +15,9 @@ mod metadata_server; mod sidecar_control; use miette::{IntoDiagnostic, Result, WrapErr}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::future::Future; +use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicU32; use std::time::Duration; @@ -446,7 +448,7 @@ pub async fn run_sandbox( retained_proto.clone(), openshell_endpoint.clone(), sandbox_id.clone(), - std::path::PathBuf::from(trusted_ssh_socket_path), + PathBuf::from(trusted_ssh_socket_path), ); } @@ -783,11 +785,11 @@ fn process_enforcement_mode() -> ProcessEnforcementMode { } } -fn sidecar_control_socket() -> Option { +fn sidecar_control_socket() -> Option { std::env::var(openshell_core::sandbox_env::SIDECAR_CONTROL_SOCKET) .ok() .filter(|path| !path.is_empty()) - .map(std::path::PathBuf::from) + .map(PathBuf::from) } #[cfg_attr(not(target_os = "linux"), allow(dead_code))] @@ -882,7 +884,7 @@ fn spawn_sidecar_entrypoint_handler( retained_proto: Option, openshell_endpoint: Option, sandbox_id: Option, - trusted_ssh_socket_path: std::path::PathBuf, + trusted_ssh_socket_path: PathBuf, ) { tokio::spawn(async move { let mut session_started = false; @@ -943,7 +945,7 @@ fn spawn_sidecar_entrypoint_handler( }); } -fn sidecar_ca_file_paths() -> Option<(std::path::PathBuf, std::path::PathBuf)> { +fn sidecar_ca_file_paths() -> Option<(PathBuf, PathBuf)> { let tls_dir = std::env::var(openshell_core::sandbox_env::PROXY_TLS_DIR) .unwrap_or_else(|_| SIDECAR_TLS_DIR.to_string()); let cert = std::path::Path::new(&tls_dir).join(SIDECAR_CA_CERT); @@ -1107,42 +1109,33 @@ const PROXY_BASELINE_READ_ONLY: &[&str] = &[ /// user working directory and temporary files. const PROXY_BASELINE_READ_WRITE: &[&str] = &["/sandbox", "/tmp"]; -/// GPU read-only paths. +/// GPU read-only paths for the legacy device-scan fallback. /// /// `/run/nvidia-persistenced`: NVML tries to connect to the persistenced /// socket at init time. If the directory exists but Landlock denies traversal /// (EACCES vs ECONNREFUSED), NVML returns `NVML_ERROR_INSUFFICIENT_PERMISSIONS` /// even though the daemon is optional. Only read/traversal access is needed. /// -/// `/usr/lib/wsl`: On WSL2, CDI bind-mounts GPU libraries (libdxcore.so, -/// libcuda.so.1.1, etc.) into paths under `/usr/lib/wsl/`. Although `/usr` -/// is already in `PROXY_BASELINE_READ_ONLY`, individual file bind-mounts may -/// not be covered by the parent-directory Landlock rule when the mount crosses -/// a filesystem boundary. Listing `/usr/lib/wsl` explicitly ensures traversal -/// is permitted regardless of Landlock's cross-mount behaviour. +/// `/usr/lib/wsl`: retained for the legacy device-scan fallback. CDI +/// sandboxes use resolved mount destinations instead of this broad directory +/// baseline. const GPU_BASELINE_READ_ONLY: &[&str] = &[ "/run/nvidia-persistenced", - "/usr/lib/wsl", // WSL2: CDI-injected GPU library directory + "/usr/lib/wsl", // Legacy fallback; CDI uses resolved mount destinations. ]; -/// GPU read-write paths (static). +/// GPU read-write paths for the legacy device-scan fallback. /// /// `/dev/nvidiactl`, `/dev/nvidia-uvm`, `/dev/nvidia-uvm-tools`, -/// `/dev/nvidia-modeset`: control and UVM devices injected by CDI on native -/// Linux. Landlock restricts `open(2)` on device files even when DAC allows -/// it; these need read-write because NVML/CUDA opens them with `O_RDWR`. -/// These devices do not exist on WSL2 and will be skipped by the existence -/// check in `enrich_proto_baseline_paths()`. +/// `/dev/nvidia-modeset`: control and UVM devices. Landlock restricts +/// `open(2)` on device files even when DAC allows it; these need read-write +/// because NVML/CUDA opens them with `O_RDWR`. CDI sandboxes derive device +/// nodes from the selected CDI specs instead of this hard-coded list. /// /// `/dev/dxg`: On WSL2, NVIDIA GPUs are exposed through the DXG kernel driver -/// (DirectX Graphics) rather than the native nvidia* devices. CDI injects -/// `/dev/dxg` as the sole GPU device node; it does not exist on native Linux -/// and will be skipped there by the existence check. -/// -/// `/proc`: CUDA writes to `/proc//task//comm` during `cuInit()` -/// to set thread names. Without write access, `cuInit()` returns error 304. -/// Must use `/proc` (not `/proc/self/task`) because Landlock rules bind to -/// inodes and child processes have different procfs inodes than the parent. +/// (DirectX Graphics) rather than the native nvidia* devices. This is retained +/// for the legacy device-scan fallback; CDI sandboxes derive it from specs when +/// needed. /// /// Per-GPU device files (`/dev/nvidia0`, …) are enumerated at runtime by /// `enumerate_gpu_device_nodes()` since the count varies. @@ -1151,190 +1144,533 @@ const GPU_BASELINE_READ_WRITE: &[&str] = &[ "/dev/nvidia-uvm", "/dev/nvidia-uvm-tools", "/dev/nvidia-modeset", - "/dev/dxg", // WSL2: DXG device (GPU via DirectX kernel driver, injected by CDI) - "/proc", + "/dev/dxg", // WSL2: DXG device exposed through the DirectX kernel driver. ]; -/// Returns true if GPU devices are present in the container. -/// -/// Checks both the native Linux NVIDIA control device (`/dev/nvidiactl`) and -/// the WSL2 DXG device (`/dev/dxg`). CDI injects exactly one of these -/// depending on the host kernel; the other will not exist. -fn has_gpu_devices() -> bool { - std::path::Path::new("/dev/nvidiactl").exists() || std::path::Path::new("/dev/dxg").exists() +/// CUDA writes to `/proc//task//comm` during `cuInit()` to set thread +/// names. Without write access, `cuInit()` returns error 304. Must use `/proc` +/// (not `/proc/self/task`) because Landlock rules bind to inodes and child +/// processes have different procfs inodes than the parent. +const GPU_PROC_READ_WRITE: &str = "/proc"; + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +struct EnrichmentPathSources { + baseline: bool, + runtime: bool, } -/// Enumerate per-GPU device nodes (`/dev/nvidia0`, `/dev/nvidia1`, …). -fn enumerate_gpu_device_nodes() -> Vec { - let mut paths = Vec::new(); - if let Ok(entries) = std::fs::read_dir("/dev") { - for entry in entries.flatten() { - let name = entry.file_name(); - let name = name.to_string_lossy(); - if let Some(suffix) = name.strip_prefix("nvidia") { - if suffix.is_empty() || !suffix.chars().all(|c| c.is_ascii_digit()) { - continue; - } - paths.push(entry.path().to_string_lossy().into_owned()); - } +impl EnrichmentPathSources { + fn baseline() -> Self { + Self { + baseline: true, + runtime: false, } } - paths + + fn runtime() -> Self { + Self { + baseline: false, + runtime: true, + } + } + + fn merge(&mut self, other: Self) { + self.baseline |= other.baseline; + self.runtime |= other.runtime; + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum MissingPathBehavior { + Skip, + Error, } -fn push_unique(paths: &mut Vec, path: String) { - if !paths.iter().any(|p| p == &path) { - paths.push(path); +impl MissingPathBehavior { + fn merge(self, other: Self) -> Self { + match (self, other) { + (Self::Error, _) | (_, Self::Error) => Self::Error, + (Self::Skip, Self::Skip) => Self::Skip, + } } } -fn collect_baseline_enrichment_paths( - include_proxy: bool, - include_gpu: bool, - gpu_device_nodes: Vec, -) -> (Vec, Vec) { - let mut ro = Vec::new(); - let mut rw = Vec::new(); +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReadOnlyConflictBehavior { + KeepReadOnly, + PromoteToReadWrite, + Reject, +} - if include_proxy { - for &path in PROXY_BASELINE_READ_ONLY { - push_unique(&mut ro, path.to_string()); +impl ReadOnlyConflictBehavior { + fn merge(self, other: Self) -> Self { + match (self, other) { + (Self::Reject, _) | (_, Self::Reject) => Self::Reject, + (Self::PromoteToReadWrite, _) | (_, Self::PromoteToReadWrite) => { + Self::PromoteToReadWrite + } + (Self::KeepReadOnly, Self::KeepReadOnly) => Self::KeepReadOnly, } - for &path in PROXY_BASELINE_READ_WRITE { - push_unique(&mut rw, path.to_string()); + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct EnrichmentPathPolicy { + sources: EnrichmentPathSources, + missing_path: MissingPathBehavior, + read_only_conflict: ReadOnlyConflictBehavior, +} + +impl EnrichmentPathPolicy { + fn baseline() -> Self { + Self { + sources: EnrichmentPathSources::baseline(), + missing_path: MissingPathBehavior::Skip, + read_only_conflict: ReadOnlyConflictBehavior::KeepReadOnly, } } - if include_gpu { - for &path in GPU_BASELINE_READ_ONLY { - push_unique(&mut ro, path.to_string()); + fn runtime_device_node() -> Self { + Self { + sources: EnrichmentPathSources::runtime(), + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::KeepReadOnly, } - for &path in GPU_BASELINE_READ_WRITE { - push_unique(&mut rw, path.to_string()); + } + + fn runtime_mount() -> Self { + Self { + sources: EnrichmentPathSources::runtime(), + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::KeepReadOnly, } - for path in gpu_device_nodes { - push_unique(&mut rw, path); + } + + fn runtime_writable_mount() -> Self { + Self { + sources: EnrichmentPathSources::runtime(), + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::Reject, } } - // A path promoted to read_write (e.g. /proc for GPU) should not also - // appear in read_only — Landlock handles the overlap correctly but the - // duplicate is confusing when inspecting the effective policy. - ro.retain(|p| !rw.contains(p)); + fn gpu_proc(sources: EnrichmentPathSources) -> Self { + Self { + sources, + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::PromoteToReadWrite, + } + } - (ro, rw) + fn merge(&mut self, other: Self) { + self.sources.merge(other.sources); + self.missing_path = self.missing_path.merge(other.missing_path); + self.read_only_conflict = self.read_only_conflict.merge(other.read_only_conflict); + } } -fn active_baseline_enrichment_paths(include_proxy: bool) -> (Vec, Vec) { - let include_gpu = has_gpu_devices(); - let gpu_device_nodes = if include_gpu { - enumerate_gpu_device_nodes() - } else { - Vec::new() - }; - collect_baseline_enrichment_paths(include_proxy, include_gpu, gpu_device_nodes) +#[derive(Debug, Clone, PartialEq, Eq)] +struct EnrichmentPath { + path: String, + policy: EnrichmentPathPolicy, } -/// Collect all active baseline paths for tests and diagnostics. -/// Returns `(read_only, read_write)` as owned `String` vecs. -#[cfg(test)] -fn baseline_enrichment_paths() -> (Vec, Vec) { - active_baseline_enrichment_paths(true) +impl EnrichmentPath { + fn should_apply(&self, access: &str, path_exists: &F) -> Result + where + F: Fn(&str) -> bool, + { + if path_exists(&self.path) { + return Ok(true); + } + + match self.policy.missing_path { + MissingPathBehavior::Skip => { + debug!( + path = %self.path, + access, + "Baseline enrichment path does not exist, skipping" + ); + Ok(false) + } + MissingPathBehavior::Error => Err(miette::miette!( + "Runtime-derived enrichment path '{}' does not exist", + self.path + )), + } + } } -fn enrich_proto_baseline_paths_with( - proto: &mut openshell_core::proto::SandboxPolicy, - ro: &[String], - rw: &[String], - path_exists: F, -) -> bool -where - F: Fn(&str) -> bool, -{ - if ro.is_empty() && rw.is_empty() { - return false; +#[derive(Debug, Clone, Default, PartialEq, Eq)] +struct EnrichmentPlan { + read_only: BTreeMap, + read_write: BTreeMap, + additional_gids: BTreeSet, +} + +impl EnrichmentPlan { + fn for_proto_policy( + proto: &openshell_core::proto::SandboxPolicy, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + ) -> Self { + Self::active(!proto.network_policies.is_empty(), cdi_requirements) + } + + fn for_sandbox_policy( + policy: &SandboxPolicy, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + ) -> Self { + Self::active( + matches!(policy.network.mode, NetworkMode::Proxy), + cdi_requirements, + ) } - let fs = proto - .filesystem - .get_or_insert_with(|| openshell_core::proto::FilesystemPolicy { - include_workdir: true, - ..Default::default() - }); + fn active( + include_proxy: bool, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + ) -> Self { + let mut plan = Self::default(); + if include_proxy { + plan = plan.merge(Self::proxy_baseline()); + } + let gpu_plan = cdi_requirements.map_or_else( + || { + if has_gpu_devices() { + Self::legacy_gpu_fallback(enumerate_gpu_device_nodes()) + } else { + Self::default() + } + }, + Self::cdi_gpu, + ); + plan.merge(gpu_plan) + } - let mut modified = false; - for path in ro { - if !fs.read_only.iter().any(|p| p == path) && !fs.read_write.iter().any(|p| p == path) { - if !path_exists(path) { - debug!( - path, - "Baseline read-only path does not exist, skipping enrichment" - ); + fn proxy_baseline() -> Self { + Self::from_baseline_paths(PROXY_BASELINE_READ_ONLY, PROXY_BASELINE_READ_WRITE) + } + + fn legacy_gpu_fallback(gpu_device_nodes: Vec) -> Self { + let mut plan = Self::from_baseline_paths(GPU_BASELINE_READ_ONLY, GPU_BASELINE_READ_WRITE); + for path in gpu_device_nodes { + plan.insert_read_write_path(path, EnrichmentPathPolicy::baseline()); + } + plan.insert_read_write_path( + GPU_PROC_READ_WRITE, + EnrichmentPathPolicy::gpu_proc(EnrichmentPathSources::baseline()), + ); + plan + } + + fn cdi_gpu(requirements: &openshell_core::cdi::CdiDerivedRequirements) -> Self { + let mut plan = Self::default(); + for path in &requirements.read_only_mount_paths { + plan.insert_read_only_path(path, EnrichmentPathPolicy::runtime_mount()); + } + for path in &requirements.device_node_paths { + plan.insert_read_write_path(path, EnrichmentPathPolicy::runtime_device_node()); + } + for path in &requirements.read_write_mount_paths { + plan.insert_read_write_path(path, EnrichmentPathPolicy::runtime_writable_mount()); + } + plan.insert_read_write_path( + GPU_PROC_READ_WRITE, + EnrichmentPathPolicy::gpu_proc(EnrichmentPathSources::runtime()), + ); + for gid in &requirements.additional_gids { + plan.insert_additional_gid(*gid); + } + plan + } + + fn from_baseline_paths(read_only: &[&str], read_write: &[&str]) -> Self { + let mut plan = Self::default(); + for &path in read_only { + plan.insert_read_only_path(path, EnrichmentPathPolicy::baseline()); + } + for &path in read_write { + plan.insert_read_write_path(path, EnrichmentPathPolicy::baseline()); + } + plan + } + + fn merge(mut self, other: Self) -> Self { + for (path, policy) in other.read_only { + self.insert_read_only_path(path, policy); + } + for (path, policy) in other.read_write { + self.insert_read_write_path(path, policy); + } + for gid in other.additional_gids { + self.insert_additional_gid(gid); + } + self + } + + fn insert_read_only_path(&mut self, path: impl Into, policy: EnrichmentPathPolicy) { + insert_enrichment_path(&mut self.read_only, path.into(), policy); + } + + fn insert_read_write_path(&mut self, path: impl Into, policy: EnrichmentPathPolicy) { + insert_enrichment_path(&mut self.read_write, path.into(), policy); + } + + fn insert_additional_gid(&mut self, gid: u32) { + self.additional_gids.insert(gid); + } + + fn entries(&self) -> (Vec, Vec) { + let mut read_write = self.read_write.clone(); + let mut read_only = Vec::new(); + + // A path promoted to read_write (e.g. /proc for GPU) should not also + // appear in read_only — Landlock handles the overlap correctly but the + // duplicate is confusing when inspecting the effective policy. + for (path, policy) in &self.read_only { + if let Some(read_write_policy) = read_write.get_mut(path) { + read_write_policy.merge(*policy); + } else { + read_only.push(EnrichmentPath { + path: path.clone(), + policy: *policy, + }); + } + } + + let read_write = read_write + .into_iter() + .map(|(path, policy)| EnrichmentPath { path, policy }) + .collect(); + (read_only, read_write) + } + + fn has_additional_gids(&self) -> bool { + !self.additional_gids.is_empty() + } + + fn additional_gids(&self) -> Vec { + self.additional_gids.iter().copied().collect() + } + + fn apply_to_proto_policy( + &self, + proto: &mut openshell_core::proto::SandboxPolicy, + ) -> Result { + // Baseline paths are system-injected, not user-specified. Skip paths + // that do not exist in this container image to avoid noisy warnings + // from Landlock and, more critically, to prevent a single missing + // baseline path from abandoning the entire Landlock ruleset under + // best-effort mode (see issue #664). + self.apply_to_proto_policy_with(proto, |path| std::path::Path::new(path).exists()) + } + + fn apply_to_proto_policy_with( + &self, + proto: &mut openshell_core::proto::SandboxPolicy, + path_exists: F, + ) -> Result + where + F: Fn(&str) -> bool, + { + let (read_only, read_write) = self.entries(); + if read_only.is_empty() && read_write.is_empty() { + return Ok(EnrichmentApplication::default()); + } + + let fs = proto + .filesystem + .get_or_insert_with(|| openshell_core::proto::FilesystemPolicy { + include_workdir: true, + ..Default::default() + }); + + let mut application = EnrichmentApplication::default(); + for addition in read_only { + if !addition.should_apply("read_only", &path_exists)? { continue; } - fs.read_only.push(path.clone()); - modified = true; + if !fs.read_only.iter().any(|p| p == &addition.path) + && !fs.read_write.iter().any(|p| p == &addition.path) + { + fs.read_only.push(addition.path); + application.record(addition.policy.sources); + } } + for addition in read_write { + if !addition.should_apply("read_write", &path_exists)? { + continue; + } + if fs.read_write.iter().any(|p| p == &addition.path) { + continue; + } + if fs.read_only.iter().any(|p| p == &addition.path) { + match addition.policy.read_only_conflict { + ReadOnlyConflictBehavior::KeepReadOnly => {} + ReadOnlyConflictBehavior::PromoteToReadWrite => { + info!( + path = %addition.path, + "Promoting /proc from read-only to read-write for GPU runtime compatibility" + ); + fs.read_only.retain(|p| p != &addition.path); + fs.read_write.push(addition.path); + application.record(addition.policy.sources); + } + ReadOnlyConflictBehavior::Reject => { + return Err(miette::miette!( + "Runtime-derived read-write path '{}' conflicts with sandbox policy read_only", + addition.path + )); + } + } + continue; + } + fs.read_write.push(addition.path); + application.record(addition.policy.sources); + } + + Ok(application) } - for path in rw { - if fs.read_write.iter().any(|p| p == path) { - continue; + + fn apply_to_sandbox_policy(&self, policy: &mut SandboxPolicy) -> Result { + let (read_only, read_write) = self.entries(); + let mut application = EnrichmentApplication::default(); + + for addition in read_only { + if !addition.should_apply("read_only", &|path| std::path::Path::new(path).exists())? { + continue; + } + let p = PathBuf::from(&addition.path); + if !policy.filesystem.read_only.contains(&p) + && !policy.filesystem.read_write.contains(&p) + { + policy.filesystem.read_only.push(p); + application.record(addition.policy.sources); + } } - if !path_exists(path) { - debug!( - path, - "Baseline read-write path does not exist, skipping enrichment" - ); - continue; + for addition in read_write { + if !addition.should_apply("read_write", &|path| std::path::Path::new(path).exists())? { + continue; + } + let p = PathBuf::from(&addition.path); + if policy.filesystem.read_write.contains(&p) { + continue; + } + if policy.filesystem.read_only.contains(&p) { + match addition.policy.read_only_conflict { + ReadOnlyConflictBehavior::KeepReadOnly => {} + ReadOnlyConflictBehavior::PromoteToReadWrite => { + info!( + path = %addition.path, + "Promoting /proc from read-only to read-write for GPU runtime compatibility" + ); + policy + .filesystem + .read_only + .retain(|existing| existing != &p); + policy.filesystem.read_write.push(p); + application.record(addition.policy.sources); + } + ReadOnlyConflictBehavior::Reject => { + return Err(miette::miette!( + "Runtime-derived read-write path '{}' conflicts with sandbox policy read_only", + addition.path + )); + } + } + continue; + } + policy.filesystem.read_write.push(p); + application.record(addition.policy.sources); } - if fs.read_only.iter().any(|p| p == path) { - if path == "/proc" { - info!( - path, - "Promoting /proc from read-only to read-write for GPU runtime compatibility" - ); - fs.read_only.retain(|p| p != path); - fs.read_write.push(path.clone()); - modified = true; + + if self.has_additional_gids() { + let additional_gids = self.additional_gids(); + if policy.process.supplemental_groups != additional_gids { + // CDI calls these `additionalGids`; the supervisor applies them + // as Linux supplemental groups before dropping privileges. + policy.process.supplemental_groups = additional_gids; + application.record(EnrichmentPathSources::runtime()); } - continue; } - fs.read_write.push(path.clone()); - modified = true; + + Ok(application) } - modified + #[cfg(test)] + fn paths(&self) -> (Vec, Vec) { + let (read_only, read_write) = self.entries(); + ( + read_only.into_iter().map(|path| path.path).collect(), + read_write.into_iter().map(|path| path.path).collect(), + ) + } } -/// Ensure a proto `SandboxPolicy` includes the baseline filesystem paths -/// required by proxy-mode sandboxes and GPU runtimes. Paths are only added if -/// missing; user-specified paths are never removed. +/// Returns true if GPU devices are present in the container. /// -/// Returns `true` if the policy was modified (caller may want to sync back). -fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) -> bool { - let (ro, rw) = active_baseline_enrichment_paths(!proto.network_policies.is_empty()); - - // Baseline paths are system-injected, not user-specified. Skip paths - // that do not exist in this container image to avoid noisy warnings from - // Landlock and, more critically, to prevent a single missing baseline - // path from abandoning the entire Landlock ruleset under best-effort - // mode (see issue #664). - let modified = enrich_proto_baseline_paths_with(proto, &ro, &rw, |path| { - std::path::Path::new(path).exists() - }); +/// Checks both the native Linux NVIDIA control device (`/dev/nvidiactl`) and +/// the WSL2 DXG device (`/dev/dxg`) for the legacy fallback path. +fn has_gpu_devices() -> bool { + std::path::Path::new("/dev/nvidiactl").exists() || std::path::Path::new("/dev/dxg").exists() +} - if modified { - ocsf_emit!( - ConfigStateChangeBuilder::new(ocsf_ctx()) - .severity(SeverityId::Informational) - .status(StatusId::Success) - .state(StateId::Enabled, "enriched") - .message("Enriched policy with baseline filesystem paths for proxy mode") - .build() - ); +/// Enumerate per-GPU device nodes (`/dev/nvidia0`, `/dev/nvidia1`, …). +fn enumerate_gpu_device_nodes() -> Vec { + let mut paths = Vec::new(); + if let Ok(entries) = std::fs::read_dir("/dev") { + for entry in entries.flatten() { + let name = entry.file_name(); + let name = name.to_string_lossy(); + if let Some(suffix) = name.strip_prefix("nvidia") { + if suffix.is_empty() || !suffix.chars().all(|c| c.is_ascii_digit()) { + continue; + } + paths.push(entry.path().to_string_lossy().into_owned()); + } + } + } + paths +} + +fn insert_enrichment_path( + paths: &mut BTreeMap, + path: String, + policy: EnrichmentPathPolicy, +) { + if let Some(existing) = paths.get_mut(&path) { + existing.merge(policy); + } else { + paths.insert(path, policy); + } +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +struct EnrichmentApplication { + baseline_modified: bool, + runtime_modified: bool, +} + +impl EnrichmentApplication { + fn modified(self) -> bool { + self.baseline_modified || self.runtime_modified + } + + fn record(&mut self, sources: EnrichmentPathSources) { + if sources.baseline { + self.baseline_modified = true; + } + if sources.runtime { + self.runtime_modified = true; + } } +} - modified +/// Collect all active baseline paths for tests and diagnostics. +/// Returns `(read_only, read_write)` as owned `String` vecs. +#[cfg(test)] +fn baseline_enrichment_paths() -> (Vec, Vec) { + EnrichmentPlan::active(true, None).paths() } fn strip_proto_provider_policy_entries(proto: &mut openshell_core::proto::SandboxPolicy) -> bool { @@ -1354,57 +1690,161 @@ fn proto_sync_payload_for_enriched_policy( Some(sync_policy) } -/// Ensure a `SandboxPolicy` (Rust type) includes the baseline filesystem -/// paths required by proxy-mode sandboxes and GPU runtimes. Used for the -/// local-file code path where no proto is available. -fn enrich_sandbox_baseline_paths(policy: &mut SandboxPolicy) { - let (ro, rw) = - active_baseline_enrichment_paths(matches!(policy.network.mode, NetworkMode::Proxy)); - if ro.is_empty() && rw.is_empty() { - return; +fn cdi_writable_file_allowlist_from_proto( + proto: &openshell_core::proto::SandboxPolicy, +) -> HashSet { + proto + .filesystem + .as_ref() + .map(|fs| fs.read_write.iter().cloned().collect()) + .unwrap_or_default() +} + +fn cdi_writable_file_allowlist_from_policy(policy: &SandboxPolicy) -> HashSet { + policy + .filesystem + .read_write + .iter() + .map(|path| path.to_string_lossy().into_owned()) + .collect() +} + +struct CdiPolicyEnrichment { + enriched: bool, + additional_gids: Vec, +} + +fn enrich_sandbox_policy_with_baseline_and_cdi(policy: &mut SandboxPolicy) -> Result<()> { + let cdi_writable_file_allowlist = cdi_writable_file_allowlist_from_policy(policy); + let requirements = resolve_cdi_requirements_from_env(&cdi_writable_file_allowlist)?; + let plan = EnrichmentPlan::for_sandbox_policy(policy, requirements.as_ref()); + let application = plan.apply_to_sandbox_policy(policy)?; + emit_policy_enrichment_events(application, requirements.as_ref(), &plan); + Ok(()) +} + +fn enrich_proto_policy_with_baseline_and_cdi( + proto: &mut openshell_core::proto::SandboxPolicy, +) -> Result { + let cdi_writable_file_allowlist = cdi_writable_file_allowlist_from_proto(proto); + let requirements = resolve_cdi_requirements_from_env(&cdi_writable_file_allowlist)?; + let plan = EnrichmentPlan::for_proto_policy(proto, requirements.as_ref()); + let application = plan.apply_to_proto_policy(proto)?; + emit_policy_enrichment_events(application, requirements.as_ref(), &plan); + + Ok(CdiPolicyEnrichment { + enriched: application.modified(), + additional_gids: plan.additional_gids(), + }) +} + +fn resolve_cdi_requirements_from_env( + writable_file_allowlist: &HashSet, +) -> Result> { + let Ok(context_path) = std::env::var(openshell_core::sandbox_env::CDI_CONTEXT) else { + return Ok(None); + }; + let context_path = context_path.trim(); + if context_path.is_empty() { + return Ok(None); } - let mut modified = false; - for path in &ro { - let p = std::path::PathBuf::from(path); - if !policy.filesystem.read_only.contains(&p) && !policy.filesystem.read_write.contains(&p) { - if !p.exists() { - debug!( - path, - "Baseline read-only path does not exist, skipping enrichment" - ); - continue; - } - policy.filesystem.read_only.push(p); - modified = true; - } + let context = openshell_core::cdi::read_context(context_path).map_err(|err| { + emit_cdi_validation_failure(&err.to_string()); + miette::miette!("Failed to load CDI context from {context_path}: {err}") + })?; + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "loaded") + .unmapped( + "cdi_selected_device_count", + serde_json::json!(context.selected_devices.len()) + ) + .unmapped("cdi_runtime", serde_json::json!(&context.runtime)) + .message(format!("Loaded CDI context [path:{context_path}]")) + .build() + ); + + let requirements = openshell_core::cdi::resolve_cdi_context(&context, writable_file_allowlist) + .map_err(|err| { + emit_cdi_validation_failure(&err.to_string()); + miette::miette!("Failed to resolve CDI requirements: {err}") + })?; + Ok(Some(requirements)) +} + +fn emit_cdi_validation_failure(message: &str) { + ocsf_emit!( + DetectionFindingBuilder::new(ocsf_ctx()) + .activity(ActivityId::Open) + .severity(SeverityId::High) + .action(ActionId::Denied) + .disposition(DispositionId::Blocked) + .finding_info( + FindingInfo::new( + "cdi-policy-validation-failed", + "CDI Policy Validation Failed", + ) + .with_desc(message), + ) + .message(format!("CDI validation failed: {message}")) + .build() + ); +} + +fn emit_baseline_enrichment_event() { + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "enriched") + .message("Enriched policy with baseline filesystem paths") + .build() + ); +} + +fn emit_policy_enrichment_events( + application: EnrichmentApplication, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + plan: &EnrichmentPlan, +) { + if application.baseline_modified { + emit_baseline_enrichment_event(); } - for path in &rw { - let p = std::path::PathBuf::from(path); - if policy.filesystem.read_only.contains(&p) || policy.filesystem.read_write.contains(&p) { - continue; - } - if !p.exists() { - debug!( - path, - "Baseline read-write path does not exist, skipping enrichment" - ); - continue; - } - policy.filesystem.read_write.push(p); - modified = true; + if let Some(requirements) = cdi_requirements + && (application.runtime_modified || plan.has_additional_gids()) + { + emit_cdi_enrichment_event(requirements); } +} - if modified { - ocsf_emit!( - ConfigStateChangeBuilder::new(ocsf_ctx()) - .severity(SeverityId::Informational) - .status(StatusId::Success) - .state(StateId::Enabled, "enriched") - .message("Enriched policy with baseline filesystem paths for proxy mode") - .build() - ); - } +fn emit_cdi_enrichment_event(requirements: &openshell_core::cdi::CdiDerivedRequirements) { + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "enriched") + .unmapped( + "cdi_device_node_count", + serde_json::json!(requirements.device_node_paths.len()) + ) + .unmapped( + "cdi_read_only_mount_count", + serde_json::json!(requirements.read_only_mount_paths.len()) + ) + .unmapped( + "cdi_read_write_mount_count", + serde_json::json!(requirements.read_write_mount_paths.len()) + ) + .unmapped( + "cdi_additional_gid_count", + serde_json::json!(requirements.additional_gids.len()) + ) + .message("Enriched policy with CDI-derived filesystem and process requirements") + .build() + ); } #[cfg(test)] @@ -1505,7 +1945,8 @@ mod baseline_tests { }, ); - enrich_proto_baseline_paths(&mut policy); + let plan = EnrichmentPlan::for_proto_policy(&policy, None); + plan.apply_to_proto_policy(&mut policy).unwrap(); let filesystem = policy.filesystem.expect("filesystem policy"); assert!( @@ -1608,16 +2049,17 @@ mod baseline_tests { policy.network_policies.is_empty(), "regression setup must exercise the no-network default path" ); - let (ro, rw) = - collect_baseline_enrichment_paths(false, true, vec!["/dev/nvidia0".to_string()]); + let plan = EnrichmentPlan::legacy_gpu_fallback(vec!["/dev/nvidia0".to_string()]); - let enriched = enrich_proto_baseline_paths_with(&mut policy, &ro, &rw, |path| { - matches!(path, "/proc" | "/dev/nvidia0") - }); + let application = plan + .apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/proc" | "/dev/nvidia0") + }) + .unwrap(); let filesystem = policy.filesystem.expect("filesystem policy"); assert!( - enriched, + application.modified(), "GPU enrichment should not require network policies" ); assert!( @@ -1635,13 +2077,235 @@ mod baseline_tests { } #[test] - fn gpu_baseline_read_write_contains_dxg() { - // /dev/dxg must be present so WSL2 sandboxes get the Landlock - // read-write rule for the CDI-injected DXG device. The existence - // check in enrich_proto_baseline_paths() skips it on native Linux. + fn cdi_gpu_baseline_uses_proc_without_legacy_gpu_paths() { + let requirements = openshell_core::cdi::CdiDerivedRequirements::default(); + let (ro, rw) = EnrichmentPlan::cdi_gpu(&requirements).paths(); + + assert!( + rw.contains(&GPU_PROC_READ_WRITE.to_string()), + "CDI GPU baseline should keep the CUDA procfs write exception" + ); + for path in [ + "/dev/nvidiactl", + "/dev/nvidia-uvm", + "/dev/dxg", + "/dev/nvidia0", + ] { + assert!( + !rw.contains(&path.to_string()), + "CDI GPU baseline should not include legacy read-write path {path}" + ); + } + for path in ["/run/nvidia-persistenced", "/usr/lib/wsl"] { + assert!( + !ro.contains(&path.to_string()), + "CDI GPU baseline should not include legacy read-only path {path}" + ); + } + } + + #[test] + fn active_cdi_gpu_baseline_ignores_detected_gpu_devices() { + let requirements = openshell_core::cdi::CdiDerivedRequirements::default(); + let (ro, rw) = EnrichmentPlan::active(false, Some(&requirements)).paths(); + + assert!( + ro.is_empty(), + "CDI-only GPU baseline should not add read-only legacy paths" + ); + assert_eq!(rw, vec![GPU_PROC_READ_WRITE.to_string()]); + } + + #[test] + fn enrichment_plan_composes_proxy_baseline_and_cdi_gpu_paths() { + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/dxg".to_string()], + read_only_mount_paths: vec!["/usr/lib/wsl/lib/libcuda.so.1".to_string()], + read_write_mount_paths: Vec::new(), + additional_gids: vec![44], + }; + let plan = EnrichmentPlan::proxy_baseline().merge(EnrichmentPlan::cdi_gpu(&requirements)); + let (ro, rw) = plan.paths(); + + assert!(ro.contains(&"/usr".to_string())); + assert!(ro.contains(&"/usr/lib/wsl/lib/libcuda.so.1".to_string())); + assert!(rw.contains(&"/tmp".to_string())); + assert!(rw.contains(&"/dev/dxg".to_string())); + assert!(rw.contains(&GPU_PROC_READ_WRITE.to_string())); + assert!( + !ro.contains(&GPU_PROC_READ_WRITE.to_string()), + "read_write /proc should normalize away proxy read_only /proc" + ); + assert_eq!(plan.additional_gids(), vec![44]); + } + + #[test] + fn enrichment_plan_merges_baseline_and_runtime_path_sources() { + let mut plan = EnrichmentPlan::default(); + plan.insert_read_write_path("/shared", EnrichmentPathPolicy::baseline()); + plan.insert_read_write_path("/shared", EnrichmentPathPolicy::runtime_device_node()); + + let (_ro, rw) = plan.entries(); + assert_eq!(rw.len(), 1); + assert!(rw[0].policy.sources.baseline); + assert!(rw[0].policy.sources.runtime); + } + + #[test] + fn proto_cdi_enrichment_adds_derived_paths() { + let mut policy = openshell_policy::restrictive_default_policy(); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/dxg".to_string()], + read_only_mount_paths: vec!["/usr/lib/wsl/lib/libcuda.so.1".to_string()], + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + assert!( + plan.apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/usr/lib/wsl/lib/libcuda.so.1" | "/dev/dxg" | "/proc") + }) + .unwrap() + .modified() + ); + let filesystem = policy.filesystem.expect("filesystem policy"); + assert!( + filesystem + .read_only + .contains(&"/usr/lib/wsl/lib/libcuda.so.1".to_string()) + ); + assert!(filesystem.read_write.contains(&"/dev/dxg".to_string())); + } + + #[test] + fn proto_cdi_enrichment_errors_for_missing_runtime_path() { + let mut policy = openshell_policy::restrictive_default_policy(); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/dxg".to_string()], + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + let err = plan + .apply_to_proto_policy_with(&mut policy, |path| path == "/proc") + .unwrap_err(); + assert!( + err.to_string().contains("does not exist"), + "unexpected error: {err}" + ); + } + + #[test] + fn proto_cdi_device_node_conflict_keeps_explicit_read_only() { + let mut policy = openshell_policy::restrictive_default_policy(); + policy.filesystem = Some(openshell_core::proto::FilesystemPolicy { + read_only: vec!["/dev/nvidia0".to_string()], + read_write: Vec::new(), + include_workdir: false, + }); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/nvidia0".to_string()], + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + plan.apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/dev/nvidia0" | "/proc") + }) + .unwrap(); + let filesystem = policy.filesystem.expect("filesystem policy"); + assert!( + filesystem.read_only.contains(&"/dev/nvidia0".to_string()), + "CDI device nodes should not promote existing read_only policy entries" + ); + assert!( + !filesystem.read_write.contains(&"/dev/nvidia0".to_string()), + "existing read_only device node should remain read_only" + ); + } + + #[test] + fn proto_cdi_writable_mount_rejects_read_write_conflict() { + let mut policy = openshell_policy::restrictive_default_policy(); + policy.filesystem = Some(openshell_core::proto::FilesystemPolicy { + read_only: vec!["/opt/nvidia/cache.db".to_string()], + read_write: Vec::new(), + include_workdir: false, + }); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: Vec::new(), + read_only_mount_paths: Vec::new(), + read_write_mount_paths: vec!["/opt/nvidia/cache.db".to_string()], + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + let err = plan + .apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/opt/nvidia/cache.db" | "/proc") + }) + .unwrap_err(); + assert!( + err.to_string().contains("conflicts"), + "unexpected error: {err}" + ); + } + + #[test] + fn proto_cdi_enrichment_promotes_proc_read_write() { + let mut policy = openshell_policy::restrictive_default_policy(); + policy.filesystem = Some(openshell_core::proto::FilesystemPolicy { + read_only: vec!["/proc".to_string()], + read_write: Vec::new(), + include_workdir: false, + }); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/proc".to_string()], + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + assert!(plan.apply_to_proto_policy(&mut policy).unwrap().modified()); + let filesystem = policy.filesystem.expect("filesystem policy"); + assert!(!filesystem.read_only.contains(&"/proc".to_string())); + assert!(filesystem.read_write.contains(&"/proc".to_string())); + } + + #[test] + fn local_cdi_enrichment_applies_additional_gids_as_supplemental_groups() { + let mut policy = SandboxPolicy { + version: 1, + filesystem: FilesystemPolicy::default(), + network: NetworkPolicy::default(), + landlock: LandlockPolicy::default(), + process: ProcessPolicy::default(), + }; + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: Vec::new(), + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: vec![44, 107], + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + plan.apply_to_sandbox_policy(&mut policy).unwrap(); + assert_eq!(policy.process.supplemental_groups, vec![44, 107]); + } + + #[test] + fn legacy_gpu_baseline_read_write_contains_dxg() { + // /dev/dxg stays in the legacy device-scan fallback. CDI mode derives + // device nodes from the selected specs instead of this list. assert!( GPU_BASELINE_READ_WRITE.contains(&"/dev/dxg"), - "/dev/dxg must be in GPU_BASELINE_READ_WRITE for WSL2 support" + "/dev/dxg should remain in the legacy GPU read-write fallback" ); } @@ -1650,7 +2314,7 @@ mod baseline_tests { let mut policy = SandboxPolicy { version: 1, filesystem: FilesystemPolicy { - read_only: vec![std::path::PathBuf::from("/tmp")], + read_only: vec![PathBuf::from("/tmp")], read_write: vec![], include_workdir: false, }, @@ -1662,31 +2326,61 @@ mod baseline_tests { process: ProcessPolicy::default(), }; - enrich_sandbox_baseline_paths(&mut policy); + let plan = EnrichmentPlan::for_sandbox_policy(&policy, None); + plan.apply_to_sandbox_policy(&mut policy).unwrap(); assert!( - policy - .filesystem - .read_only - .contains(&std::path::PathBuf::from("/tmp")), + policy.filesystem.read_only.contains(&PathBuf::from("/tmp")), "explicit read_only baseline path should be preserved" ); assert!( !policy .filesystem .read_write - .contains(&std::path::PathBuf::from("/tmp")), + .contains(&PathBuf::from("/tmp")), "baseline enrichment must not promote explicit read_only /tmp to read_write" ); } #[test] - fn gpu_baseline_read_only_contains_usr_lib_wsl() { - // /usr/lib/wsl must be present so CDI-injected WSL2 GPU library - // bind-mounts are accessible under Landlock. Skipped on native Linux. + fn legacy_gpu_baseline_read_only_contains_usr_lib_wsl() { + // /usr/lib/wsl stays in the legacy device-scan fallback. CDI mode uses + // resolved mount destinations instead of this broad directory. assert!( GPU_BASELINE_READ_ONLY.contains(&"/usr/lib/wsl"), - "/usr/lib/wsl must be in GPU_BASELINE_READ_ONLY for WSL2 CDI library paths" + "/usr/lib/wsl should remain in the legacy GPU read-only fallback" + ); + } + + #[test] + fn local_cdi_baseline_promotes_proc_read_write() { + let mut policy = SandboxPolicy { + version: 1, + filesystem: FilesystemPolicy { + read_only: vec![PathBuf::from(GPU_PROC_READ_WRITE)], + read_write: Vec::new(), + include_workdir: false, + }, + network: NetworkPolicy::default(), + landlock: LandlockPolicy::default(), + process: ProcessPolicy::default(), + }; + + let requirements = openshell_core::cdi::CdiDerivedRequirements::default(); + let plan = EnrichmentPlan::for_sandbox_policy(&policy, Some(&requirements)); + plan.apply_to_sandbox_policy(&mut policy).unwrap(); + + assert!( + !policy + .filesystem + .read_only + .contains(&PathBuf::from(GPU_PROC_READ_WRITE)) + ); + assert!( + policy + .filesystem + .read_write + .contains(&PathBuf::from(GPU_PROC_READ_WRITE)) ); } @@ -1816,7 +2510,7 @@ async fn load_policy( landlock: config.landlock, process: config.process, }; - enrich_sandbox_baseline_paths(&mut policy); + enrich_sandbox_policy_with_baseline_and_cdi(&mut policy)?; return Ok(( policy, Some(Arc::new(engine)), @@ -1854,7 +2548,7 @@ async fn load_policy( let mut discovered = discover_policy_from_disk_or_default(); // Enrich before syncing so the gateway baseline includes // baseline paths from the start. - enrich_proto_baseline_paths(&mut discovered); + enrich_proto_policy_with_baseline_and_cdi(&mut discovered)?; strip_proto_provider_policy_entries(&mut discovered); let sandbox = sandbox.as_deref().ok_or_else(|| { miette::miette!( @@ -1888,7 +2582,8 @@ async fn load_policy( // Ensure baseline filesystem paths are present for proxy-mode // sandboxes. If the policy was enriched, sync the updated version // back to the gateway so users can see the effective policy. - let enriched = enrich_proto_baseline_paths(&mut proto_policy); + let cdi_enrichment = enrich_proto_policy_with_baseline_and_cdi(&mut proto_policy)?; + let enriched = cdi_enrichment.enriched; let sync_policy = proto_sync_payload_for_enriched_policy(&proto_policy, enriched); if let Some(sync_policy) = sync_policy { if let Some(sandbox_name) = sandbox.as_deref() { @@ -1943,7 +2638,7 @@ async fn load_policy( } }; - let policy = match SandboxPolicy::try_from(proto_policy.clone()) { + let mut policy = match SandboxPolicy::try_from(proto_policy.clone()) { Ok(policy) => policy, Err(e) => { report_initial_policy_failure(endpoint, id, loaded_policy_revision.as_ref(), &e) @@ -1951,6 +2646,9 @@ async fn load_policy( return Err(e); } }; + if !cdi_enrichment.additional_gids.is_empty() { + policy.process.supplemental_groups = cdi_enrichment.additional_gids; + } return Ok(( policy, opa_engine, diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 829c63caa4..5fa21a1cce 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -789,6 +789,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"), + supplemental_groups: Vec::new(), } } diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 72effa98f8..a990590ba6 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -13,7 +13,7 @@ use miette::{IntoDiagnostic, Result}; use nix::sys::signal::{self, Signal}; use nix::unistd::{Gid, Group, Pid, Uid, User}; use openshell_core::policy::{NetworkMode, SandboxPolicy}; -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; use std::ffi::CString; #[cfg(target_os = "linux")] use std::os::fd::{AsRawFd, OwnedFd, RawFd}; @@ -1283,6 +1283,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<()> { @@ -1307,6 +1325,11 @@ pub fn drop_privileges(policy: &SandboxPolicy) -> Result<()> { fallback.process.run_as_group = Some("sandbox".into()); return drop_privileges(&fallback); } + if !policy.process.supplemental_groups.is_empty() { + return Err(miette::miette!( + "Supplemental groups require a privileged supervisor process" + )); + } return Ok(()); } @@ -1363,6 +1386,8 @@ pub fn drop_privileges(policy: &SandboxPolicy) -> Result<()> { // Set supplementary groups only when we have a name-based identity. // Numeric UIDs may not have a passwd entry, so initgroups would fail. + let extra_supplemental_groups = &policy.process.supplemental_groups; + let mut supplemental_groups = Vec::new(); if let Some(ref user) = user && target_uid != nix::unistd::geteuid() { @@ -1385,6 +1410,19 @@ pub fn drop_privileges(policy: &SandboxPolicy) -> Result<()> { )))] { nix::unistd::initgroups(user_cstr.as_c_str(), target_gid).into_diagnostic()?; + if !extra_supplemental_groups.is_empty() { + supplemental_groups = nix::unistd::getgroups().into_diagnostic()?; + } + } + } + if !extra_supplemental_groups.is_empty() { + // CDI `additionalGids` are carried in the policy as Linux supplemental + // groups. They must be set while the supervisor still has privilege, + // before setgid/setuid drop the ability to change group membership. + let supplemental_groups = + merged_supplemental_gids(supplemental_groups, extra_supplemental_groups)?; + if !supplemental_groups.is_empty() { + nix::unistd::setgroups(&supplemental_groups).into_diagnostic()?; } } @@ -1635,6 +1673,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". @@ -1652,6 +1691,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(); @@ -1661,6 +1701,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 @@ -1675,6 +1730,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); @@ -1712,6 +1768,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")); @@ -1753,6 +1810,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()); @@ -1763,6 +1821,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); @@ -1776,6 +1835,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); @@ -1949,6 +2009,7 @@ mod tests { process: ProcessPolicy { run_as_user, run_as_group, + ..Default::default() }, } } @@ -2322,6 +2383,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!( @@ -2348,6 +2410,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!( @@ -2364,6 +2427,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-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index f5a3ee0793..e4725ada33 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -1666,6 +1666,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("1000".into()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy); @@ -1687,6 +1688,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("sandbox".into()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy); @@ -1708,6 +1710,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some(String::new()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy); @@ -1728,6 +1731,7 @@ mod tests { process: ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy); @@ -1748,6 +1752,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("1000660000".into()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy); @@ -1777,6 +1782,7 @@ mod tests { process: ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }, }; @@ -1807,6 +1813,7 @@ mod tests { process: ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }, }, None, diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 03a9b564f0..83203b4f3e 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -119,6 +119,15 @@ Select Docker with `compute_drivers = ["docker"]` in `[openshell.gateway]`. Conf For GPU-backed Docker sandboxes, configure Docker CDI before starting the gateway so OpenShell can detect the daemon capability. +For Docker GPU/CDI sandboxes, OpenShell uses Docker's selected CDI device IDs +and daemon-reported CDI spec directories to build a supervisor-only CDI +context. The driver mounts the spec directories read-only into the sandbox +container, uploads `cdi-context.json` before starting the container, and removes +the created container if that upload fails. The supervisor resolves the context +inside the sandbox and derives the inner filesystem and supplemental group +requirements from CDI specs. Non-GPU Docker sandboxes do not receive the CDI +context, spec mounts, or CDI-derived policy changes. + ### Docker Driver Config Mounts Docker driver config accepts user-supplied `volume` and `tmpfs` mounts. It also diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 212ba76fce..f90f35af97 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -62,7 +62,23 @@ Raw streams are connection-scoped and outside L7 live-reload guarantees. This in 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`, `/var/log` (read-only) and `/sandbox`, `/tmp` (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. 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 or CDI specs are present. + +Docker GPU/CDI sandboxes receive CDI-derived filesystem requirements instead +of the hard-coded GPU device and library baseline. The supervisor resolves the +selected CDI device specs inside the sandbox and adds CDI device nodes as +read-write paths and CDI mount destinations as read-only paths. 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. 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.