diff --git a/crates/openshell-core/src/driver_mounts.rs b/crates/openshell-core/src/driver_mounts.rs index 138fbdf1d..fa43edf53 100644 --- a/crates/openshell-core/src/driver_mounts.rs +++ b/crates/openshell-core/src/driver_mounts.rs @@ -5,6 +5,26 @@ use std::path::Path; +/// `SELinux` relabelling mode for bind mounts. +/// +/// On hosts with `SELinux` enabled (e.g. Fedora, RHEL) a bind-mounted path +/// must be relabelled so the container process can access it. +/// +/// * `shared` (`:z`) — the label is shared across all containers that mount +/// the same path. Safe when multiple sandboxes read the same data set. +/// * `private` (`:Z`) — the label is private to *this* container. The host +/// directory becomes inaccessible to other containers (and potentially to +/// the host) until the container is removed. Use only when exclusive +/// ownership is acceptable. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum SelinuxLabel { + /// Shared `SELinux` label (`:z`). + Shared, + /// Private `SELinux` label (`:Z`). + Private, +} + const RESERVED_MOUNT_TARGETS: &[&str] = &[ "/opt/openshell", "/etc/openshell", diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index a59352018..d9bea99e7 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -303,6 +303,8 @@ impl DockerSandboxDriverConfig { } } +use openshell_core::driver_mounts::SelinuxLabel; + #[derive(Debug, Clone, serde::Deserialize)] #[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)] enum DockerDriverMountConfig { @@ -311,6 +313,8 @@ enum DockerDriverMountConfig { target: String, #[serde(default = "default_true")] read_only: bool, + #[serde(default)] + selinux_label: Option, }, Volume { source: String, @@ -1761,29 +1765,90 @@ fn docker_driver_config( Ok(config) } +/// Collect user-supplied bind mounts as string-format binds. +/// +/// Bind mounts use the legacy `Binds` field (`-v` syntax) rather than the +/// structured `Mount` API because the Docker Engine Mount object does not +/// support `SELinux` relabelling (`:z` / `:Z`). The string format does. +fn docker_driver_bind_strings(config: &DockerSandboxDriverConfig) -> Result, Status> { + config + .mounts + .iter() + .filter_map(|m| match m { + DockerDriverMountConfig::Bind { + source, + target, + read_only, + selinux_label, + } => Some(docker_bind_string( + source, + target, + *read_only, + *selinux_label, + )), + _ => None, + }) + .collect() +} + +fn docker_bind_string( + source: &str, + target: &str, + read_only: bool, + selinux_label: Option, +) -> Result { + driver_mounts::validate_absolute_mount_source(source, "bind source") + .map_err(Status::failed_precondition)?; + // Legacy `-v` binds silently create missing source directories as empty, + // root-owned paths. The structured `--mount` API that was used before this + // change rejected missing sources at container-create time. Preserve that + // fail-fast behaviour with an explicit existence check. + if !Path::new(source).exists() { + return Err(Status::failed_precondition(format!( + "bind source path does not exist: {source}" + ))); + } + driver_mounts::validate_container_mount_target(target).map_err(Status::failed_precondition)?; + let normalized_target = driver_mounts::normalize_mount_target(target); + + let mut opts = Vec::new(); + if read_only { + opts.push("ro"); + } + match selinux_label { + Some(SelinuxLabel::Shared) => opts.push("z"), + Some(SelinuxLabel::Private) => opts.push("Z"), + None => {} + } + + if opts.is_empty() { + Ok(format!("{source}:{normalized_target}")) + } else { + Ok(format!("{source}:{normalized_target}:{}", opts.join(","))) + } +} + +/// Collect user-supplied non-bind mounts as structured `Mount` objects. fn docker_driver_mounts(config: &DockerSandboxDriverConfig) -> Result, Status> { - config.mounts.iter().map(docker_mount_from_config).collect() + config + .mounts + .iter() + .filter_map(|m| docker_mount_from_config(m).transpose()) + .collect() } -fn docker_mount_from_config(config: &DockerDriverMountConfig) -> Result { +fn docker_mount_from_config(config: &DockerDriverMountConfig) -> Result, Status> { match config { - DockerDriverMountConfig::Bind { - source, - target, - read_only, - } => Ok(Mount { - typ: Some(MountTypeEnum::BIND), - source: Some(source.clone()), - target: Some(target.clone()), - read_only: Some(*read_only), - ..Default::default() - }), + DockerDriverMountConfig::Bind { .. } => { + // Bind mounts are handled via docker_driver_bind_strings. + Ok(None) + } DockerDriverMountConfig::Volume { source, target, read_only, subpath, - } => Ok(Mount { + } => Ok(Some(Mount { typ: Some(MountTypeEnum::VOLUME), source: Some(source.clone()), target: Some(target.clone()), @@ -1793,13 +1858,13 @@ fn docker_mount_from_config(config: &DockerDriverMountConfig) -> Result Ok(Mount { + } => Ok(Some(Mount { typ: Some(MountTypeEnum::TMPFS), target: Some(target.clone()), tmpfs_options: Some(MountTmpfsOptions { @@ -1818,7 +1883,7 @@ fn docker_mount_from_config(config: &DockerDriverMountConfig) -> Result Err(Status::failed_precondition( "invalid docker driver_config: docker image mounts are not supported", )), @@ -2261,6 +2326,7 @@ fn build_container_create_body_with_gpu_devices( .ok_or_else(|| Status::invalid_argument("sandbox.spec.template is required"))?; let resource_limits = docker_resource_limits(template)?; let user_mounts = docker_driver_mounts(driver_config)?; + let user_bind_strings = docker_driver_bind_strings(driver_config)?; let device_requests = gpu_device_ids.map(|device_ids| { vec![DeviceRequest { driver: Some("cdi".to_string()), @@ -2298,7 +2364,11 @@ fn build_container_create_body_with_gpu_devices( memory: resource_limits.memory_bytes, pids_limit: docker_pids_limit(config.sandbox_pids_limit)?, device_requests, - binds: Some(build_binds(sandbox, config)?), + binds: { + let mut binds = build_binds(sandbox, config)?; + binds.extend(user_bind_strings); + Some(binds) + }, mounts: Some(user_mounts), restart_policy: Some(RestartPolicy { name: Some(RestartPolicyNameEnum::UNLESS_STOPPED), diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 923c6d618..d42d41099 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -781,6 +781,8 @@ fn driver_config_rejects_bind_mounts_unless_enabled() { #[test] fn build_container_create_body_includes_bind_mounts_when_enabled() { + let bind_src = TempDir::new().unwrap(); + let src_path = bind_src.path().to_str().unwrap(); let mut sandbox = test_sandbox(); sandbox .spec @@ -792,7 +794,7 @@ fn build_container_create_body_includes_bind_mounts_when_enabled() { .driver_config = Some(json_struct(serde_json::json!({ "mounts": [{ "type": "bind", - "source": "/host/path", + "source": src_path, "target": "/sandbox/host", "read_only": true }] @@ -801,21 +803,32 @@ fn build_container_create_body_includes_bind_mounts_when_enabled() { config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); - let mounts = body + let binds = body .host_config + .as_ref() .unwrap() - .mounts - .expect("driver config mounts should be set"); + .binds + .as_ref() + .expect("binds should be set"); - assert_eq!(mounts.len(), 1); - assert_eq!(mounts[0].typ, Some(MountTypeEnum::BIND)); - assert_eq!(mounts[0].source.as_deref(), Some("/host/path")); - assert_eq!(mounts[0].target.as_deref(), Some("/sandbox/host")); - assert_eq!(mounts[0].read_only, Some(true)); + // User bind mount appears after the system binds. + let expected = format!("{src_path}:/sandbox/host:ro"); + assert!( + binds.iter().any(|b| b == &expected), + "expected bind entry '{expected}', got {binds:?}" + ); + // Bind mounts must not appear in the structured mounts vec. + let mounts = body.host_config.unwrap().mounts.unwrap_or_default(); + assert!( + mounts.iter().all(|m| m.typ != Some(MountTypeEnum::BIND)), + "bind mounts should not appear in structured mounts" + ); } #[test] fn driver_config_defaults_enabled_bind_mounts_to_read_only() { + let bind_src = TempDir::new().unwrap(); + let src_path = bind_src.path().to_str().unwrap(); let mut sandbox = test_sandbox(); sandbox .spec @@ -827,7 +840,7 @@ fn driver_config_defaults_enabled_bind_mounts_to_read_only() { .driver_config = Some(json_struct(serde_json::json!({ "mounts": [{ "type": "bind", - "source": "/host/path", + "source": src_path, "target": "/sandbox/host" }] }))); @@ -835,13 +848,160 @@ fn driver_config_defaults_enabled_bind_mounts_to_read_only() { config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); - let mounts = body + let binds = body .host_config .unwrap() - .mounts - .expect("driver config mounts should be set"); + .binds + .expect("binds should be set"); - assert_eq!(mounts[0].read_only, Some(true)); + let expected = format!("{src_path}:/sandbox/host:ro"); + assert!( + binds.iter().any(|b| b == &expected), + "default bind mount should be read-only, got {binds:?}" + ); +} + +#[test] +fn bind_mount_selinux_shared_label() { + let bind_src = TempDir::new().unwrap(); + let src_path = bind_src.path().to_str().unwrap(); + let mut sandbox = test_sandbox(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": src_path, + "target": "/sandbox/data", + "read_only": true, + "selinux_label": "shared" + }] + }))); + let mut config = runtime_config(); + config.enable_bind_mounts = true; + + let body = build_container_create_body(&sandbox, &config).unwrap(); + let binds = body + .host_config + .unwrap() + .binds + .expect("binds should be set"); + + let expected = format!("{src_path}:/sandbox/data:ro,z"); + assert!( + binds.iter().any(|b| b == &expected), + "expected ':ro,z' label, got {binds:?}" + ); +} + +#[test] +fn bind_mount_selinux_private_label() { + let bind_src = TempDir::new().unwrap(); + let src_path = bind_src.path().to_str().unwrap(); + let mut sandbox = test_sandbox(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": src_path, + "target": "/sandbox/data", + "read_only": false, + "selinux_label": "private" + }] + }))); + let mut config = runtime_config(); + config.enable_bind_mounts = true; + + let body = build_container_create_body(&sandbox, &config).unwrap(); + let binds = body + .host_config + .unwrap() + .binds + .expect("binds should be set"); + + let expected = format!("{src_path}:/sandbox/data:Z"); + assert!( + binds.iter().any(|b| b == &expected), + "expected ':Z' label, got {binds:?}" + ); +} + +#[test] +fn bind_mount_without_selinux_label() { + let bind_src = TempDir::new().unwrap(); + let src_path = bind_src.path().to_str().unwrap(); + let mut sandbox = test_sandbox(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": src_path, + "target": "/sandbox/host", + "read_only": false + }] + }))); + let mut config = runtime_config(); + config.enable_bind_mounts = true; + + let body = build_container_create_body(&sandbox, &config).unwrap(); + let binds = body + .host_config + .unwrap() + .binds + .expect("binds should be set"); + + let expected = format!("{src_path}:/sandbox/host"); + assert!( + binds.iter().any(|b| b == &expected), + "expected no options suffix, got {binds:?}" + ); +} + +#[test] +fn driver_config_rejects_missing_bind_source() { + let mut sandbox = test_sandbox(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": "/no/such/path", + "target": "/sandbox/data" + }] + }))); + let mut config = runtime_config(); + config.enable_bind_mounts = true; + + let err = build_container_create_body(&sandbox, &config).unwrap_err(); + + assert_eq!(err.code(), tonic::Code::FailedPrecondition); + assert!( + err.message().contains("bind source path does not exist"), + "expected missing-source error, got: {}", + err.message() + ); } #[test] diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index ba47e4eaa..7c9ab269f 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -5,6 +5,7 @@ use crate::config::PodmanComputeConfig; use openshell_core::ComputeDriverError; +use openshell_core::driver_mounts::SelinuxLabel; #[cfg(test)] use openshell_core::gpu::{driver_gpu_requirements, validate_specific_gpu_device_request}; use openshell_core::proto::compute::v1::{DriverSandbox, DriverSandboxTemplate}; @@ -105,6 +106,8 @@ enum PodmanDriverMountConfig { target: String, #[serde(default = "default_true")] read_only: bool, + #[serde(default)] + selinux_label: Option, }, Volume { source: String, @@ -540,17 +543,24 @@ fn podman_user_mounts( source, target, read_only, + selinux_label, } => { + let mut options = vec![ + if read_only { "ro" } else { "rw" }.to_string(), + "rbind".to_string(), + ]; + match selinux_label { + Some(SelinuxLabel::Shared) => options.push("z".to_string()), + Some(SelinuxLabel::Private) => options.push("Z".to_string()), + None => {} + } driver_mounts::validate_absolute_mount_source(&source, "bind source")?; driver_mounts::validate_container_mount_target(&target)?; result.mounts.push(Mount { kind: "bind".into(), source, - destination: target, - options: vec![ - if read_only { "ro" } else { "rw" }.to_string(), - "rbind".to_string(), - ], + destination: driver_mounts::normalize_mount_target(&target), + options, }); } PodmanDriverMountConfig::Volume { @@ -2097,6 +2107,84 @@ mod tests { ); } + #[test] + fn container_spec_bind_mount_selinux_shared_label() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + template: Some(DriverSandboxTemplate { + driver_config: Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": "/data/shared", + "target": "/sandbox/data", + "read_only": true, + "selinux_label": "shared" + }] + }))), + ..Default::default() + }), + ..Default::default() + }); + let mut config = test_config(); + config.enable_bind_mounts = true; + + let spec = build_container_spec(&sandbox, &config); + let mounts = spec["mounts"] + .as_array() + .expect("mounts should be an array"); + + assert!(mounts.iter().any(|mount| { + mount["type"].as_str() == Some("bind") + && mount["source"].as_str() == Some("/data/shared") + && mount["destination"].as_str() == Some("/sandbox/data") + && mount["options"].as_array().is_some_and(|options| { + options.iter().any(|o| o.as_str() == Some("ro")) + && options.iter().any(|o| o.as_str() == Some("z")) + }) + })); + } + + #[test] + fn container_spec_bind_mount_selinux_private_label() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + template: Some(DriverSandboxTemplate { + driver_config: Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": "/data/exclusive", + "target": "/sandbox/data", + "read_only": false, + "selinux_label": "private" + }] + }))), + ..Default::default() + }), + ..Default::default() + }); + let mut config = test_config(); + config.enable_bind_mounts = true; + + let spec = build_container_spec(&sandbox, &config); + let mounts = spec["mounts"] + .as_array() + .expect("mounts should be an array"); + + assert!(mounts.iter().any(|mount| { + mount["type"].as_str() == Some("bind") + && mount["source"].as_str() == Some("/data/exclusive") + && mount["destination"].as_str() == Some("/sandbox/data") + && mount["options"].as_array().is_some_and(|options| { + options.iter().any(|o| o.as_str() == Some("rw")) + && options.iter().any(|o| o.as_str() == Some("Z")) + }) + })); + } + #[test] fn driver_config_rejects_reserved_mount_targets() { use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 341d9e9f4..c764ac923 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -162,7 +162,7 @@ Docker mount schema: | Type | Fields | |---|---| -| `bind` | `source`, `target`, optional `read_only` (`true` by default). `source` must be an absolute host path. Requires `enable_bind_mounts = true`. | +| `bind` | `source`, `target`, optional `read_only` (`true` by default), optional `selinux_label` (`shared` for `:z` or `private` for `:Z`). `source` must be an absolute host path. Requires `enable_bind_mounts = true`. | | `volume` | `source`, `target`, optional `read_only` (`true` by default), optional `subpath`. The named volume must already exist. Docker local-driver bind-backed volumes require `enable_bind_mounts = true`. | | `tmpfs` | `target`, optional `options`, optional `size_bytes`, optional `mode`. | @@ -227,7 +227,7 @@ Podman mount schema: | Type | Fields | |---|---| -| `bind` | `source`, `target`, optional `read_only` (`true` by default). `source` must be an absolute host path. Requires `enable_bind_mounts = true`. | +| `bind` | `source`, `target`, optional `read_only` (`true` by default), optional `selinux_label` (`shared` for `:z` or `private` for `:Z`). `source` must be an absolute host path. Requires `enable_bind_mounts = true`. | | `volume` | `source`, `target`, optional `read_only` (`true` by default). The named volume must already exist. Podman local-driver bind-backed volumes require `enable_bind_mounts = true`. | | `tmpfs` | `target`, optional `options`, optional `size_bytes`, optional `mode`. | | `image` | `source`, `target`, optional `read_only` (`true` by default). |