From 32a94ab6c93ace4acde4630a3518cb764986a9f3 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 29 Jul 2026 21:32:13 +0000 Subject: [PATCH] feat(driver-podman): add userns config with supervisor bind-mount fallback Add a `userns` option to the Podman compute driver that maps to Podman's `--userns` flag. When set to `auto`, the container spec includes `idmappings.AutoUserNs = true` as required by the API. Podman image volumes use overlay mounts internally and the kernel does not support idmapped mounts on overlay. When userns is configured, the driver extracts the supervisor binary from the image to a host-side cache and bind-mounts it instead of using an image volume. The image volume path is preserved when userns is unset. Signed-off-by: Giuseppe Scrivano --- Cargo.lock | 2 + crates/openshell-driver-podman/Cargo.toml | 2 + crates/openshell-driver-podman/README.md | 1 + crates/openshell-driver-podman/src/client.rs | 26 +++ crates/openshell-driver-podman/src/config.rs | 5 + .../openshell-driver-podman/src/container.rs | 162 +++++++++++++++- crates/openshell-driver-podman/src/driver.rs | 179 ++++++++++++++++++ crates/openshell-driver-podman/src/main.rs | 6 + docs/reference/gateway-config.mdx | 2 + 9 files changed, 380 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31e2104987..0e0d2d45ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3969,7 +3969,9 @@ dependencies = [ "rustix 1.1.4", "serde", "serde_json", + "tar", "temp-env", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-stream", diff --git a/crates/openshell-driver-podman/Cargo.toml b/crates/openshell-driver-podman/Cargo.toml index ed798c0ab2..d65ae50d33 100644 --- a/crates/openshell-driver-podman/Cargo.toml +++ b/crates/openshell-driver-podman/Cargo.toml @@ -34,6 +34,8 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true } thiserror = { workspace = true } miette = { workspace = true } +tar = "0.4" +tempfile = "3" [dev-dependencies] prost-types = { workspace = true } diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 965a295d19..788ba5506f 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -360,6 +360,7 @@ Podman resources after out-of-band container removal or label drift. | `OPENSHELL_SANDBOX_PROXY_AUTH_FILE` | `--sandbox-proxy-auth-file` | unset | Path to a file containing the proxy credentials as `user:pass`. Staged as a root-only Podman secret so credentials never appear in config or container metadata. Requires the insecure-auth acknowledgement below. | | `OPENSHELL_SANDBOX_PROXY_AUTH_ALLOW_INSECURE` | `--sandbox-proxy-auth-allow-insecure` | unset | Explicit acknowledgement (`true`) that the credential is sent as cleartext Basic auth over the plain-TCP connection to the `http://` proxy. Required when the auth file is set; rejected when it is not. | | `OPENSHELL_SANDBOX_PROXY_CONNECT_BY_HOSTNAME` | `--sandbox-proxy-connect-by-hostname` | unset | Send the destination hostname in CONNECT requests instead of a validated IP. Last resort for proxies whose ACLs filter on hostnames: the proxy then resolves the name itself, so sandbox SSRF/`allowed_ips` validation no longer binds the connection. | +| `OPENSHELL_PODMAN_USERNS` | `--userns` | unset | User namespace mode for sandbox containers (e.g. `auto`). When unset, containers use the default user namespace. | Through the gateway, the same settings are the `https_proxy`, `no_proxy`, `proxy_auth_file`, `proxy_auth_allow_insecure`, and diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 952938d47a..deb8b10ee0 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -490,6 +490,32 @@ impl PodmanClient { .await } + /// Download a file from a container as a tar archive. + /// + /// Calls `GET /libpod/containers/{name}/archive?path={path}` and returns + /// the raw tar bytes. The container does not need to be running. + pub async fn copy_from_container( + &self, + name: &str, + path: &str, + ) -> Result { + validate_name(name)?; + let encoded_path = url_encode(path); + let (status, bytes) = self + .request( + hyper::Method::GET, + &format!("/libpod/containers/{name}/archive?path={encoded_path}"), + None, + API_TIMEOUT, + ) + .await?; + if status.is_success() { + Ok(bytes) + } else { + Err(error_from_response(status.as_u16(), &bytes)) + } + } + /// Inspect a container by name or ID. pub async fn inspect_container(&self, name: &str) -> Result { validate_name(name)?; diff --git a/crates/openshell-driver-podman/src/config.rs b/crates/openshell-driver-podman/src/config.rs index 2d226397b4..4fe43f682c 100644 --- a/crates/openshell-driver-podman/src/config.rs +++ b/crates/openshell-driver-podman/src/config.rs @@ -185,6 +185,9 @@ pub struct PodmanComputeConfig { /// pointing the gateway host at the corporate resolver so validated-IP /// CONNECT works in split-horizon networks. pub proxy_connect_by_hostname: Option, + /// User namespace mode for sandbox containers (e.g. `auto`). + /// When unset, containers use the default user namespace. + pub userns: Option, } pub const DEFAULT_HEALTH_CHECK_INTERVAL_SECS: u64 = 10; @@ -380,6 +383,7 @@ impl Default for PodmanComputeConfig { proxy_auth_file: None, proxy_auth_allow_insecure: None, proxy_connect_by_hostname: None, + userns: None, } } } @@ -412,6 +416,7 @@ impl std::fmt::Debug for PodmanComputeConfig { .field("proxy_auth_file", &self.proxy_auth_file.is_some()) .field("proxy_auth_allow_insecure", &self.proxy_auth_allow_insecure) .field("proxy_connect_by_hostname", &self.proxy_connect_by_hostname) + .field("userns", &self.userns) .finish() } } diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 90ef0fec21..734b23643d 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -229,6 +229,13 @@ struct ContainerSpec { /// Port mappings from host to container. Using `host_port=0` requests an /// ephemeral port, readable back from the inspect response. portmappings: Vec, + /// User namespace mode override (e.g. `auto`). + #[serde(skip_serializing_if = "Option::is_none")] + userns: Option, + /// UID/GID mapping options. Required for `userns = "auto"` — the Podman + /// API needs `AutoUserNs: true` alongside the namespace mode. + #[serde(skip_serializing_if = "Option::is_none")] + idmappings: Option, } /// A port mapping entry for the libpod `SpecGenerator`. @@ -328,6 +335,17 @@ struct NetNS { nsmode: String, } +#[derive(Serialize)] +struct UserNS { + nsmode: String, +} + +#[derive(Serialize)] +#[allow(non_snake_case)] +struct IDMappings { + AutoUserNs: bool, +} + #[derive(Serialize)] struct NetworkAttachment {} @@ -905,6 +923,7 @@ pub fn build_container_spec_with_token_and_gpu_devices( image, image, "", + None, ) } @@ -916,6 +935,7 @@ pub fn build_container_spec_for_image( requested_image: &str, image_id: &str, oci_user: &str, + supervisor_bin_path: Option<&Path>, ) -> Result { let name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); let vol = volume_name(&sandbox.id); @@ -957,11 +977,15 @@ pub fn build_container_spec_for_image( }]; volumes.extend(user_mounts.volumes); - let mut image_volumes = vec![ImageVolume { - source: config.supervisor_image.clone(), - destination: SUPERVISOR_MOUNT_DIR.into(), - rw: false, - }]; + let mut image_volumes = if supervisor_bin_path.is_some() { + Vec::new() + } else { + vec![ImageVolume { + source: config.supervisor_image.clone(), + destination: SUPERVISOR_MOUNT_DIR.into(), + rw: false, + }] + }; image_volumes.extend(user_mounts.image_volumes); let container_spec = ContainerSpec { @@ -1166,6 +1190,18 @@ pub fn build_container_spec_for_image( options: ro, }); } + if let Some(bin_path) = supervisor_bin_path { + let mut opts = vec!["ro".into(), "rbind".into()]; + if is_selinux_enabled() { + opts.push("z".into()); + } + m.push(Mount { + kind: "bind".into(), + source: bin_path.display().to_string(), + destination: SUPERVISOR_BINARY_PATH.into(), + options: opts, + }); + } m.extend(user_mounts.mounts); m }, @@ -1177,6 +1213,14 @@ pub fn build_container_spec_for_image( container_port: openshell_core::config::DEFAULT_SSH_PORT, protocol: "tcp".into(), }], + userns: config.userns.as_deref().map(|mode| UserNS { + nsmode: mode.to_string(), + }), + idmappings: config + .userns + .as_deref() + .filter(|mode| mode.eq_ignore_ascii_case("auto")) + .map(|_| IDMappings { AutoUserNs: true }), }; Ok(serde_json::to_value(container_spec).expect("ContainerSpec serialization cannot fail")) @@ -1379,6 +1423,7 @@ mod tests { "registry.example/app:latest", "sha256:immutable", "app:staff", + None, ) .unwrap(); @@ -2777,4 +2822,111 @@ mod tests { .count(); assert_eq!(bind_count, 0, "no bind mounts without TLS config"); } + + #[test] + fn container_spec_includes_userns_when_configured() { + let sandbox = test_sandbox("userns-id", "userns-name"); + let mut config = test_config(); + config.userns = Some("auto".to_string()); + let spec = build_container_spec(&sandbox, &config); + + let userns = &spec["userns"]; + assert_eq!(userns["nsmode"].as_str(), Some("auto")); + + let idmappings = &spec["idmappings"]; + assert_eq!( + idmappings["AutoUserNs"].as_bool(), + Some(true), + "idmappings.AutoUserNs must be true for userns=auto" + ); + } + + #[test] + fn container_spec_omits_userns_when_unset() { + let sandbox = test_sandbox("no-userns-id", "no-userns-name"); + let config = test_config(); + let spec = build_container_spec(&sandbox, &config); + + assert!( + spec.get("userns").is_none(), + "userns should not be set when unconfigured" + ); + assert!( + spec.get("idmappings").is_none(), + "idmappings should not be set when userns is unconfigured" + ); + } + + #[test] + fn container_spec_uses_bind_mount_for_supervisor_when_path_provided() { + let sandbox = test_sandbox("bind-sv-id", "bind-sv-name"); + let config = test_config(); + let image = resolve_image(&sandbox, &config); + let spec = build_container_spec_for_image( + &sandbox, + &config, + None, + None, + image, + image, + "", + Some(Path::new("/host/cache/openshell-sandbox")), + ) + .unwrap(); + + let image_volumes = spec["image_volumes"] + .as_array() + .expect("image_volumes should be an array"); + assert!( + !image_volumes + .iter() + .any(|v| v["destination"].as_str() == Some(SUPERVISOR_MOUNT_DIR)), + "supervisor image volume should not be present when bind path is provided" + ); + + let mounts = spec["mounts"] + .as_array() + .expect("mounts should be an array"); + let sv_bind = mounts + .iter() + .find(|m| m["destination"].as_str() == Some(SUPERVISOR_BINARY_PATH)); + assert!( + sv_bind.is_some(), + "supervisor bind mount should be present at {SUPERVISOR_BINARY_PATH}" + ); + let sv_bind = sv_bind.unwrap(); + assert_eq!( + sv_bind["source"].as_str(), + Some("/host/cache/openshell-sandbox") + ); + assert_eq!(sv_bind["type"].as_str(), Some("bind")); + } + + #[test] + fn container_spec_uses_image_volume_when_no_bind_path() { + let sandbox = test_sandbox("imgvol-id", "imgvol-name"); + let config = test_config(); + let spec = build_container_spec(&sandbox, &config); + + let image_volumes = spec["image_volumes"] + .as_array() + .expect("image_volumes should be an array"); + assert!( + image_volumes + .iter() + .any(|v| v["destination"].as_str() == Some(SUPERVISOR_MOUNT_DIR)), + "supervisor image volume should be present by default" + ); + + let mounts = spec["mounts"] + .as_array() + .expect("mounts should be an array"); + assert!( + !mounts.iter().any( + |m| m["destination"].as_str() == Some(SUPERVISOR_BINARY_PATH) + && m["type"].as_str() == Some("bind") + ), + "supervisor bind mount should not be present by default" + ); + } } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index aa3df9cbd7..9618f015b8 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -644,6 +644,16 @@ impl PodmanComputeDriver { return Err(e); } }; + let supervisor_bin_path = if self.config.userns.is_some() { + Some( + extract_supervisor_bin(&self.client, &self.config) + .await + .map_err(ComputeDriverError::from)?, + ) + } else { + None + }; + let spec = match container::build_container_spec_for_image( sandbox, &self.config, @@ -652,6 +662,7 @@ impl PodmanComputeDriver { image, &inspected_image.id, image_user, + supervisor_bin_path.as_deref(), ) { Ok(spec) => spec, Err(e) => { @@ -950,6 +961,174 @@ fn check_subuid_range() { } } +// ── Supervisor binary extraction (userns fallback) ───────────────────── + +use openshell_core::driver_utils::SUPERVISOR_IMAGE_BINARY_PATH; + +async fn extract_supervisor_bin( + client: &PodmanClient, + config: &PodmanComputeConfig, +) -> Result { + let inspect = client + .inspect_image(&config.supervisor_image) + .await + .map_err(ComputeDriverError::from)?; + let digest = if inspect.id.is_empty() { + return Err(ComputeDriverError::Precondition(format!( + "supervisor image '{}' has no ID", + config.supervisor_image, + ))); + } else { + &inspect.id + }; + + let cache_path = supervisor_cache_path(digest)?; + if cache_path.is_file() { + info!( + cache_path = %cache_path.display(), + "Using cached supervisor binary" + ); + return Ok(cache_path); + } + + info!( + image = %config.supervisor_image, + cache_path = %cache_path.display(), + "Extracting supervisor binary from image" + ); + + let container_name = temp_extract_container_name(); + let spec = serde_json::json!({ + "image": config.supervisor_image, + "name": container_name, + "entrypoint": [SUPERVISOR_IMAGE_BINARY_PATH], + "command": [], + }); + client + .create_container(&spec) + .await + .map_err(ComputeDriverError::from)?; + + let result = extract_binary_from_container(client, &container_name, &cache_path).await; + + if let Err(err) = client.remove_container(&container_name).await { + warn!( + container = container_name, + error = %err, + "Failed to remove supervisor extractor container" + ); + } + + result +} + +async fn extract_binary_from_container( + client: &PodmanClient, + container_name: &str, + cache_path: &Path, +) -> Result { + let tar_bytes = client + .copy_from_container(container_name, SUPERVISOR_IMAGE_BINARY_PATH) + .await + .map_err(ComputeDriverError::from)?; + + let binary_bytes = extract_first_tar_entry(&tar_bytes).map_err(|err| { + ComputeDriverError::Precondition(format!( + "failed to extract supervisor binary from tar: {err}" + )) + })?; + + write_cache_binary_atomic(cache_path, &binary_bytes)?; + Ok(cache_path.to_path_buf()) +} + +fn extract_first_tar_entry(tar_bytes: &[u8]) -> Result, String> { + let mut archive = tar::Archive::new(std::io::Cursor::new(tar_bytes)); + let mut entries = archive + .entries() + .map_err(|err| format!("open tar archive: {err}"))?; + let mut entry = entries + .next() + .ok_or_else(|| "tar archive was empty".to_string())? + .map_err(|err| format!("read tar entry: {err}"))?; + let mut bytes = Vec::new(); + std::io::Read::read_to_end(&mut entry, &mut bytes) + .map_err(|err| format!("read tar entry payload: {err}"))?; + Ok(bytes) +} + +fn write_cache_binary_atomic(final_path: &Path, bytes: &[u8]) -> Result<(), ComputeDriverError> { + let dir = final_path.parent().ok_or_else(|| { + ComputeDriverError::Precondition(format!( + "supervisor cache path '{}' has no parent", + final_path.display(), + )) + })?; + std::fs::create_dir_all(dir).map_err(|err| { + ComputeDriverError::Precondition(format!( + "failed to create supervisor cache dir '{}': {err}", + dir.display(), + )) + })?; + + let mut temp = tempfile::Builder::new() + .prefix(".openshell-sandbox-") + .tempfile_in(dir) + .map_err(|err| { + ComputeDriverError::Precondition(format!( + "failed to create temp file in '{}': {err}", + dir.display(), + )) + })?; + std::io::Write::write_all(&mut temp, bytes).map_err(|err| { + ComputeDriverError::Precondition(format!("failed to write supervisor binary: {err}")) + })?; + temp.as_file().sync_all().map_err(|err| { + ComputeDriverError::Precondition(format!("failed to sync supervisor binary: {err}")) + })?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(temp.path(), std::fs::Permissions::from_mode(0o755)).map_err( + |err| { + ComputeDriverError::Precondition(format!( + "failed to chmod supervisor binary: {err}" + )) + }, + )?; + } + + temp.persist(final_path).map_err(|err| { + ComputeDriverError::Precondition(format!( + "failed to persist supervisor binary to '{}': {}", + final_path.display(), + err.error, + )) + })?; + Ok(()) +} + +fn supervisor_cache_path(digest: &str) -> Result { + let base = openshell_core::paths::xdg_data_dir().map_err(|err| { + ComputeDriverError::Precondition(format!("failed to resolve XDG data dir: {err}")) + })?; + let sanitized = digest.replace(':', "-"); + Ok(base + .join("openshell") + .join("podman-supervisor") + .join(sanitized) + .join("openshell-sandbox")) +} + +fn temp_extract_container_name() -> String { + use std::sync::atomic::{AtomicU64, Ordering}; + static SEQ: AtomicU64 = AtomicU64::new(0); + let pid = std::process::id(); + let seq = SEQ.fetch_add(1, Ordering::Relaxed); + format!("openshell-supervisor-extract-{pid}-{seq}") +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/openshell-driver-podman/src/main.rs b/crates/openshell-driver-podman/src/main.rs index 4a38643f38..cec5765357 100644 --- a/crates/openshell-driver-podman/src/main.rs +++ b/crates/openshell-driver-podman/src/main.rs @@ -133,6 +133,11 @@ struct Args { /// SSRF/`allowed_ips` validation no longer binds the connection. #[arg(long, env = "OPENSHELL_SANDBOX_PROXY_CONNECT_BY_HOSTNAME")] sandbox_proxy_connect_by_hostname: Option, + + /// User namespace mode for sandbox containers (e.g. `auto`). + /// When unset, containers use the default user namespace. + #[arg(long, env = "OPENSHELL_PODMAN_USERNS")] + userns: Option, } #[tokio::main] @@ -168,6 +173,7 @@ async fn main() -> Result<()> { proxy_auth_file: args.sandbox_proxy_auth_file, proxy_auth_allow_insecure: args.sandbox_proxy_auth_allow_insecure, proxy_connect_by_hostname: args.sandbox_proxy_connect_by_hostname, + userns: args.userns, ..PodmanComputeConfig::default() }) .await diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 27a1d3d45a..a7ea6307b2 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -395,6 +395,8 @@ sandbox_pids_limit = 2048 # but increase process churn (each check spawns a conmon subprocess). # Set to 0 to disable health checks entirely. Default: 10. health_check_interval_secs = 10 +# User namespace mode for sandbox containers. Omit to use the default. +# userns = "auto" # Corporate forward proxy for sandbox egress. When set, the in-container # supervisor chains policy-approved TLS tunnels through this proxy with HTTP # CONNECT instead of dialing destinations directly. Plain-HTTP requests are