diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index 07de687103..5446cf4007 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -148,6 +148,7 @@ Common findings: - Docker daemon unavailable: start Docker Desktop or Docker Engine. - Gateway process stopped: inspect exit status and logs. - Sandbox image missing or pull denied: verify image reference and registry credentials. +- Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected. - Docker driver cannot initialize because it cannot find `openshell-sandbox`: verify `OPENSHELL_DOCKER_SUPERVISOR_BIN`, the sibling binary next to `openshell-gateway`, or the configured supervisor image contains `/openshell-sandbox`. - Sandbox never registers: check gateway logs and supervisor callback endpoint. - Supervisor image exits before printing `openshell-sandbox --version`: the image should be the scratch supervisor image from `deploy/docker/Dockerfile.supervisor` and must contain a static executable at `/openshell-sandbox`. @@ -173,6 +174,7 @@ Common findings: - Podman socket unavailable: start or expose the user socket. - Rootless networking unavailable: inspect Podman network configuration. - Sandbox image missing or pull denied: verify image reference and registry credentials. +- Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected. - Supervisor cannot call back: check callback endpoint and gateway logs. ### Step 6: Check Kubernetes Helm Gateways diff --git a/.agents/skills/generate-sandbox-policy/SKILL.md b/.agents/skills/generate-sandbox-policy/SKILL.md index 8da14420c9..3b261b5ded 100644 --- a/.agents/skills/generate-sandbox-policy/SKILL.md +++ b/.agents/skills/generate-sandbox-policy/SKILL.md @@ -473,15 +473,17 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - network_policies: # ``` -The `filesystem_policy`, `landlock`, and `process` sections above are sensible defaults. Tell the user these are defaults and may need adjustment for their environment. Gateway inference is configured separately through `openshell inference set/get`. The generated `network_policies` block is the primary output. +The `filesystem_policy` and `landlock` sections above are sensible defaults. +Process identity is omitted so the selected compute driver can choose it. For +Docker and Podman, each omitted identity field falls back to the image's OCI +`USER`. Tell the user these are defaults and may need adjustment for their +environment. Gateway inference is configured separately through `openshell +inference set/get`. The generated `network_policies` block is the primary +output. If the user provides a file path, write to it. Otherwise, ask where to place it. A common convention is a project-local policy file (e.g., `sandbox-policy.yaml`) passed to `openshell sandbox create --policy ` or set via the `OPENSHELL_SANDBOX_POLICY` env var. diff --git a/.agents/skills/generate-sandbox-policy/examples.md b/.agents/skills/generate-sandbox-policy/examples.md index b6acbee8bf..179e46c00f 100644 --- a/.agents/skills/generate-sandbox-policy/examples.md +++ b/.agents/skills/generate-sandbox-policy/examples.md @@ -830,10 +830,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - network_policies: github_readonly: name: github_readonly @@ -858,7 +854,10 @@ network_policies: - { path: /usr/local/bin/claude } ``` -The agent notes that `filesystem_policy`, `landlock`, and `process` are sensible defaults that may need adjustment, and that gateway inference is configured separately via `openshell inference set/get` rather than an `inference` policy block. +The agent notes that `filesystem_policy` and `landlock` are sensible defaults +that may need adjustment. Process identity is omitted so the compute driver can +select it. Gateway inference is configured separately via `openshell inference +set/get` rather than an `inference` policy block. --- diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index a36a372df7..06b06e4470 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -422,6 +422,12 @@ The `--from` flag accepts a Dockerfile path, a directory containing a Dockerfile Local Dockerfile and directory builds require a local gateway because the CLI builds through the local Docker daemon. Use a registry image reference for remote gateways. Bare community names resolve under `ghcr.io/nvidia/openshell-community/sandboxes` unless `OPENSHELL_COMMUNITY_REGISTRY` overrides the prefix. +For Docker and Podman gateways, custom images should declare a non-root OCI +`USER`. Each explicit `process.run_as_user` or `process.run_as_group` policy +field wins independently; omitted fields fall back to the image declaration. +An image with no `USER` fails before readiness unless policy supplies both +fields. + ### Forward ports ```bash diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index f211f906d6..1629c50de4 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -125,6 +125,30 @@ Driver-controlled environment variables must override sandbox image or template values for sandbox ID, sandbox name, gateway endpoint, relay socket path, TLS paths, and command metadata. +## Process Identity + +The gateway preserves whether each policy process field was omitted. The active +driver then supplies one authoritative identity input to the supervisor: + +- Docker and Podman inspect the final sandbox image, pin container creation to + its immutable image ID, and pass its raw OCI `Config.User`. +- Kubernetes passes its platform-resolved numeric UID/GID, including OpenShift + SCC-derived values. +- VM keeps its existing guest identity behavior. + +For Docker and Podman, policy values take precedence independently. An omitted +`run_as_user` or `run_as_group` falls back to the corresponding identity from +the image. The supervisor resolves names from the image's `/etc/passwd` and +`/etc/group` before readiness, preserves declared name or numeric components, +and uses the same privilege-drop path for direct and SSH children. When a +declaration omits the group, the supervisor fills it with the user's numeric +primary GID. It does not rewrite the account files. + +Sandbox creation fails before the workload becomes ready when a required image +identity is absent, malformed, unknown, ambiguous, or resolves to UID/GID 0. +The supervisor itself remains root so it can establish isolation before +starting unprivileged children. + Kubernetes can run the supervisor in the default combined topology or in a sidecar topology. Combined mode keeps network and process supervision in the agent container. Sidecar mode runs network enforcement, the proxy, and gateway diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 4f95e1ef69..4f7dfe4272 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -32,7 +32,7 @@ only when the set is already empty; any other outcome fails the spawn. 4. It starts the policy proxy and local SSH server. 5. It opens a supervisor session back to the gateway for connect, exec, file sync, config polling, and log push. -6. It launches the agent command as the restricted sandbox user. +6. It launches the agent command as the resolved restricted identity. ## Isolation Layers diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index f15ce34bb1..1549258fa3 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -115,6 +115,17 @@ pub const SANDBOX_UID: &str = "OPENSHELL_SANDBOX_UID"; /// supervisor drops privileges to a group other than the UID's primary group. pub const SANDBOX_GID: &str = "OPENSHELL_SANDBOX_GID"; +/// Raw OCI `Config.User` declaration from the immutable image selected by a +/// local container driver. +/// +/// Docker and Podman overwrite this value with the image declaration, +/// including an empty string when the image has no `USER`, and clear +/// [`SANDBOX_UID`] and [`SANDBOX_GID`]. Drivers with an authoritative numeric +/// identity overwrite this value with an empty string while supplying both +/// numeric fields. The supervisor resolves omitted policy identity fields from +/// OCI only for the former contract. +pub const OCI_IMAGE_USER: &str = "OPENSHELL_OCI_IMAGE_USER"; + // The corporate upstream-proxy configuration deliberately has no reserved // environment variables: it travels on the supervisor's argv // (`--upstream-proxy` and friends), which a sandbox image cannot forge the diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index e17791e747..b2e74231ba 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -18,6 +18,15 @@ The gateway runs as a host process. The Docker driver creates one container per sandbox and starts the `openshell-sandbox` supervisor inside that container. The supervisor then creates the nested sandbox namespace for the agent process. +Before creating the container, the driver inspects the final sandbox image and +captures its immutable image ID and raw OCI `Config.User`. Container creation +uses that image ID, preventing a mutable tag from changing between inspection +and launch. The supervisor runs as root, resolves omitted policy identity fields +from the image declaration, and drops only agent children to the resulting +identity. Named OCI components remain names after validation; a missing group +is filled with the user's numeric primary GID. Explicit `process.run_as_user` +and `process.run_as_group` values take precedence independently. + Docker containers join an OpenShell-managed bridge network. The driver injects `host.openshell.internal` and `host.docker.internal` so supervisors have stable names for reaching the gateway host. On Docker Desktop, Colima, Rancher diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 2f89c2229e..1b3fd25ef1 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -227,6 +227,12 @@ struct DockerProvisioningFailure { message: String, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct DockerImageMetadata { + id: String, + user: String, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] struct DockerResourceLimits { nano_cpus: Option, @@ -710,7 +716,8 @@ impl DockerComputeDriver { DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; let template = validated.template; - self.ensure_image_available(&sandbox.id, &template.image) + let image = self + .ensure_image_available(&sandbox.id, &template.image) .await .map_err(|status| { DockerProvisioningFailure::new("ImagePullFailed", status.message()) @@ -735,11 +742,12 @@ impl DockerComputeDriver { } DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; - let create_body = build_container_create_body_with_gpu_devices( + let create_body = build_container_create_body_for_image( sandbox, &self.config, &validated.driver_config, gpu_devices.as_deref(), + &image, ) .map_err(|status| { if token_file_created { @@ -1280,41 +1288,71 @@ impl DockerComputeDriver { })) } - async fn ensure_image_available(&self, sandbox_id: &str, image: &str) -> Result<(), Status> { + async fn ensure_image_available( + &self, + sandbox_id: &str, + image: &str, + ) -> Result { let policy = self.config.image_pull_policy.trim().to_ascii_lowercase(); - match policy.as_str() { + let inspect = match policy.as_str() { "" | "ifnotpresent" => { - if self.docker.inspect_image(image).await.is_ok() { + if let Ok(inspect) = self.docker.inspect_image(image).await { self.publish_docker_progress( sandbox_id, "ImagePresent", format!("Docker image \"{image}\" is already present"), HashMap::from([("image_ref".to_string(), image.to_string())]), ); - return Ok(()); + inspect + } else { + self.pull_image(sandbox_id, image).await?; + self.docker + .inspect_image(image) + .await + .map_err(|err| internal_status("inspect Docker image after pull", err))? } - self.pull_image(sandbox_id, image).await } - "always" => self.pull_image(sandbox_id, image).await, + "always" => { + self.pull_image(sandbox_id, image).await?; + self.docker + .inspect_image(image) + .await + .map_err(|err| internal_status("inspect Docker image after pull", err))? + } "never" => match self.docker.inspect_image(image).await { - Ok(_) => { + Ok(inspect) => { self.publish_docker_progress( sandbox_id, "ImagePresent", format!("Docker image \"{image}\" is already present"), HashMap::from([("image_ref".to_string(), image.to_string())]), ); - Ok(()) + inspect + } + Err(err) if is_not_found_error(&err) => { + return Err(Status::failed_precondition(format!( + "docker image '{image}' is not present locally and image_pull_policy=Never" + ))); } - Err(err) if is_not_found_error(&err) => Err(Status::failed_precondition(format!( - "docker image '{image}' is not present locally and image_pull_policy=Never" - ))), - Err(err) => Err(internal_status("inspect Docker image", err)), + Err(err) => return Err(internal_status("inspect Docker image", err)), }, - other => Err(Status::failed_precondition(format!( - "unsupported docker image_pull_policy '{other}'; expected Always, IfNotPresent, or Never", - ))), - } + other => { + return Err(Status::failed_precondition(format!( + "unsupported docker image_pull_policy '{other}'; expected Always, IfNotPresent, or Never", + ))); + } + }; + + let id = inspect.id.ok_or_else(|| { + Status::failed_precondition(format!( + "docker image '{image}' inspection did not return an immutable image ID" + )) + })?; + let user = inspect + .config + .and_then(|config| config.user) + .unwrap_or_default(); + Ok(DockerImageMetadata { id, user }) } async fn pull_image(&self, sandbox_id: &str, image: &str) -> Result<(), Status> { @@ -2132,7 +2170,16 @@ fn cleanup_sandbox_token_file_by_id(sandbox_id: &str, config: &DockerDriverRunti } } +#[cfg(test)] fn build_environment(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) -> Vec { + build_environment_for_oci_user(sandbox, config, "") +} + +fn build_environment_for_oci_user( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, + oci_user: &str, +) -> Vec { let mut environment = HashMap::from([ ("HOME".to_string(), "/root".to_string()), ("PATH".to_string(), SUPERVISOR_PATH.to_string()), @@ -2204,6 +2251,18 @@ fn build_environment(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN); environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE); + environment.insert( + openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(), + oci_user.to_string(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_UID.to_string(), + String::new(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_GID.to_string(), + String::new(), + ); // Gateway-minted sandbox JWT. Keep the raw bearer out of container // metadata; the supervisor reads it from this driver-owned bind mount. @@ -2288,6 +2347,30 @@ fn build_container_create_body_with_gpu_devices( config: &DockerDriverRuntimeConfig, driver_config: &DockerSandboxDriverConfig, gpu_device_ids: Option<&[String]>, +) -> Result { + let template = sandbox + .spec + .as_ref() + .and_then(|spec| spec.template.as_ref()) + .ok_or_else(|| Status::invalid_argument("sandbox.spec.template is required"))?; + build_container_create_body_for_image( + sandbox, + config, + driver_config, + gpu_device_ids, + &DockerImageMetadata { + id: template.image.clone(), + user: String::new(), + }, + ) +} + +fn build_container_create_body_for_image( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, + driver_config: &DockerSandboxDriverConfig, + gpu_device_ids: Option<&[String]>, + image: &DockerImageMetadata, ) -> Result { let spec = sandbox .spec @@ -2328,9 +2411,9 @@ fn build_container_create_body_with_gpu_devices( ); Ok(ContainerCreateBody { - image: Some(template.image.clone()), + image: Some(image.id.clone()), user: Some("0".to_string()), - env: Some(build_environment(sandbox, config)), + env: Some(build_environment_for_oci_user(sandbox, config, &image.user)), entrypoint: Some(vec![SUPERVISOR_MOUNT_PATH.to_string()]), // Clear the image CMD so Docker does not append inherited args to the // supervisor entrypoint. diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 67036b2192..7562e4a8fb 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -541,6 +541,54 @@ fn build_environment_sets_docker_tls_paths() { assert!(env.contains(&"OPENSHELL_SANDBOX_COMMAND=sleep infinity".to_string())); } +#[test] +fn build_environment_protects_oci_identity_metadata() { + let mut sandbox = test_sandbox(); + let spec = sandbox.spec.as_mut().unwrap(); + for (key, value) in [ + (openshell_core::sandbox_env::OCI_IMAGE_USER, "spoofed"), + (openshell_core::sandbox_env::SANDBOX_UID, "9999"), + (openshell_core::sandbox_env::SANDBOX_GID, "9999"), + ] { + spec.environment.insert(key.to_string(), value.to_string()); + } + + let env = build_environment_for_oci_user(&sandbox, &runtime_config(), "app:staff"); + + assert!(env.contains(&format!( + "{}=app:staff", + openshell_core::sandbox_env::OCI_IMAGE_USER + ))); + assert!(env.contains(&format!("{}=", openshell_core::sandbox_env::SANDBOX_UID))); + assert!(env.contains(&format!("{}=", openshell_core::sandbox_env::SANDBOX_GID))); + assert!(!env.iter().any(|entry| entry.ends_with("=spoofed"))); + assert!(!env.iter().any(|entry| entry.ends_with("=9999"))); +} + +#[test] +fn container_creation_uses_inspected_immutable_image() { + let sandbox = test_sandbox(); + let metadata = DockerImageMetadata { + id: "sha256:immutable".to_string(), + user: "1234:1235".to_string(), + }; + let body = build_container_create_body_for_image( + &sandbox, + &runtime_config(), + &DockerSandboxDriverConfig::default(), + None, + &metadata, + ) + .unwrap(); + + assert_eq!(body.image.as_deref(), Some("sha256:immutable")); + assert_eq!(body.user.as_deref(), Some("0")); + assert!(body.env.unwrap().contains(&format!( + "{}=1234:1235", + openshell_core::sandbox_env::OCI_IMAGE_USER + ))); +} + #[test] fn build_environment_keeps_path_driver_controlled() { let mut sandbox = test_sandbox(); diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index c784f10db9..513aaafe0d 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -1624,21 +1624,15 @@ fn apply_supervisor_sideload( volume_mounts.push(supervisor_volume_mount()); } - // Inject resolved sandbox UID/GID as environment variables so the - // supervisor can use them directly without /etc/passwd lookups. + // Inject the protected resolved identity contract. Clearing the OCI + // input prevents image or user environment from selecting a + // conflicting identity path. let env = container .entry("env") .or_insert_with(|| serde_json::json!([])) .as_array_mut(); if let Some(env) = env { - env.push(serde_json::json!({ - "name": openshell_core::sandbox_env::SANDBOX_UID.to_string(), - "value": sandbox_uid.to_string(), - })); - env.push(serde_json::json!({ - "name": openshell_core::sandbox_env::SANDBOX_GID.to_string(), - "value": sandbox_gid.to_string(), - })); + apply_resolved_identity_env(env, sandbox_uid, sandbox_gid); } } } @@ -1728,16 +1722,7 @@ fn supervisor_sidecar_env( openshell_core::sandbox_env::PROXY_TLS_DIR, SIDECAR_TLS_MOUNT_PATH, ); - upsert_env( - &mut env, - openshell_core::sandbox_env::SANDBOX_UID, - ¶ms.sandbox_uid.to_string(), - ); - upsert_env( - &mut env, - openshell_core::sandbox_env::SANDBOX_GID, - ¶ms.sandbox_gid.to_string(), - ); + apply_resolved_identity_env(&mut env, params.sandbox_uid, params.sandbox_gid); if !params.process_binary_aware_network_policy { upsert_env( &mut env, @@ -2015,16 +2000,7 @@ fn apply_supervisor_sidecar_topology( openshell_core::sandbox_env::PROXY_TLS_DIR, SIDECAR_TLS_MOUNT_PATH, ); - upsert_env( - env, - openshell_core::sandbox_env::SANDBOX_UID, - ¶ms.sandbox_uid.to_string(), - ); - upsert_env( - env, - openshell_core::sandbox_env::SANDBOX_GID, - ¶ms.sandbox_gid.to_string(), - ); + apply_resolved_identity_env(env, params.sandbox_uid, params.sandbox_gid); } } @@ -3017,6 +2993,23 @@ fn upsert_env(env: &mut Vec, name: &str, value: &str) { env.push(serde_json::json!({"name": name, "value": value})); } +fn apply_resolved_identity_env(env: &mut Vec, uid: u32, gid: u32) { + remove_env(env, openshell_core::sandbox_env::OCI_IMAGE_USER); + remove_env(env, openshell_core::sandbox_env::SANDBOX_UID); + remove_env(env, openshell_core::sandbox_env::SANDBOX_GID); + upsert_env(env, openshell_core::sandbox_env::OCI_IMAGE_USER, ""); + upsert_env( + env, + openshell_core::sandbox_env::SANDBOX_UID, + &uid.to_string(), + ); + upsert_env( + env, + openshell_core::sandbox_env::SANDBOX_GID, + &gid.to_string(), + ); +} + fn remove_env(env: &mut Vec, name: &str) { env.retain(|item| item.get("name").and_then(|value| value.as_str()) != Some(name)); } @@ -3908,6 +3901,59 @@ mod tests { ); } + #[test] + fn supervisor_sideload_replaces_spoofed_identity_environment() { + let mut pod_template = serde_json::json!({ + "spec": { + "containers": [{ + "name": "agent", + "image": "custom-image:latest", + "env": [ + {"name": openshell_core::sandbox_env::OCI_IMAGE_USER, "value": "spoofed"}, + {"name": openshell_core::sandbox_env::SANDBOX_UID, "value": "9999"}, + {"name": openshell_core::sandbox_env::SANDBOX_GID, "value": "9999"}, + {"name": openshell_core::sandbox_env::OCI_IMAGE_USER, "value": "duplicate"} + ] + }] + } + }); + + apply_supervisor_sideload( + &mut pod_template, + "supervisor-image:latest", + "IfNotPresent", + SupervisorSideloadMethod::InitContainer, + 1500, + 1600, + ); + + let agent = &pod_template["spec"]["containers"][0]; + let env = agent["env"].as_array().unwrap(); + for name in [ + openshell_core::sandbox_env::OCI_IMAGE_USER, + openshell_core::sandbox_env::SANDBOX_UID, + openshell_core::sandbox_env::SANDBOX_GID, + ] { + assert_eq!( + env.iter().filter(|item| item["name"] == name).count(), + 1, + "{name} must have one driver-owned value" + ); + } + assert_eq!( + rendered_env(agent, openshell_core::sandbox_env::OCI_IMAGE_USER), + Some("") + ); + assert_eq!( + rendered_env(agent, openshell_core::sandbox_env::SANDBOX_UID), + Some("1500") + ); + assert_eq!( + rendered_env(agent, openshell_core::sandbox_env::SANDBOX_GID), + Some("1600") + ); + } + #[test] fn supervisor_sideload_adds_security_context_when_missing() { let mut pod_template = serde_json::json!({ @@ -4112,6 +4158,20 @@ mod tests { let pod_template = sandbox_template_to_k8s( &SandboxTemplate { image: "agent-image:latest".to_string(), + environment: std::collections::HashMap::from([ + ( + openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(), + "spoofed".to_string(), + ), + ( + openshell_core::sandbox_env::SANDBOX_UID.to_string(), + "9999".to_string(), + ), + ( + openshell_core::sandbox_env::SANDBOX_GID.to_string(), + "9999".to_string(), + ), + ]), ..SandboxTemplate::default() }, false, @@ -4188,6 +4248,10 @@ mod tests { rendered_env(agent, openshell_core::sandbox_env::SANDBOX_UID), Some("1500") ); + assert_eq!( + rendered_env(agent, openshell_core::sandbox_env::OCI_IMAGE_USER), + Some("") + ); let sidecar = containers .iter() @@ -4233,6 +4297,10 @@ mod tests { rendered_env(sidecar, openshell_core::sandbox_env::SANDBOX_GID), Some("1500") ); + assert_eq!( + rendered_env(sidecar, openshell_core::sandbox_env::OCI_IMAGE_USER), + Some("") + ); assert_eq!( rendered_env(sidecar, openshell_core::sandbox_env::SIDECAR_CONTROL_SOCKET), Some(SIDECAR_CONTROL_SOCKET) diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 90cbac6169..965a295d19 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -7,6 +7,16 @@ driver runs in-process within the gateway server and delegates all sandbox isolation enforcement to the `openshell-sandbox` supervisor binary, which is sideloaded into each container via an OCI image volume mount. +Before creating the container, the driver inspects the final sandbox image and +captures its immutable image ID and raw OCI `Config.User`. Container creation +uses that image ID with pulling disabled, preventing a mutable tag from changing +between inspection and launch. The supervisor runs as root, resolves omitted +policy identity fields from the image declaration, and drops only agent +children to the completed identity. Named OCI components remain names after +validation; a missing group is filled with the user's numeric primary GID. Explicit +`process.run_as_user` and `process.run_as_group` values take precedence +independently. + For a rootless networking deep dive, see [NETWORKING.md](NETWORKING.md). ## Architecture @@ -186,7 +196,7 @@ graph TB subgraph Container["Sandbox Container"] SV["Supervisor
(root in user ns)"] subgraph NestedNS["Nested Network Namespace"] - SP["Sandbox Process
(sandbox user)"] + SP["Sandbox Process
(resolved non-root identity)"] VE2["veth1: 10.200.0.2"] end VE1["veth0: 10.200.0.1
(CONNECT proxy)"] diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 59cbda545d..952938d47a 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -162,6 +162,23 @@ pub struct ContainerConfig { pub labels: HashMap, } +/// Immutable image metadata needed to bind OCI identity inspection to launch. +#[derive(Debug, Clone, serde::Deserialize)] +#[serde(rename_all = "PascalCase")] +pub struct ImageInspect { + #[serde(alias = "ID")] + pub id: String, + #[serde(default)] + pub config: Option, +} + +#[derive(Debug, Clone, Default, serde::Deserialize)] +#[serde(rename_all = "PascalCase")] +pub struct ImageConfig { + #[serde(default)] + pub user: String, +} + /// A container summary returned by the list API. #[derive(Debug, Clone, serde::Deserialize)] #[serde(rename_all = "PascalCase")] @@ -675,6 +692,16 @@ impl PodmanClient { Ok(()) } + /// Inspect a locally selected image for immutable ID and OCI config. + pub async fn inspect_image(&self, reference: &str) -> Result { + self.request_json( + hyper::Method::GET, + &format!("/libpod/images/{}/json", url_encode(reference)), + None, + ) + .await + } + // ── System operations ──────────────────────────────────────────────── /// Ping the Podman API to verify connectivity. @@ -903,4 +930,36 @@ mod tests { ); let _ = std::fs::remove_file(socket_path); } + + #[tokio::test] + async fn inspect_image_reads_immutable_id_and_oci_user() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "inspect-image", + vec![StubResponse::new( + StatusCode::OK, + r#"{"Id":"sha256:immutable","Config":{"User":"app:staff"}}"#, + )], + ); + let client = PodmanClient::new(socket_path.clone()); + + let image = client + .inspect_image("example/image:latest") + .await + .expect("image inspect should parse"); + + assert_eq!(image.id, "sha256:immutable"); + assert_eq!( + image.config.as_ref().map(|config| config.user.as_str()), + Some("app:staff") + ); + handle.await.expect("stub task should finish"); + assert_eq!( + request_log + .lock() + .expect("request log lock should not be poisoned") + .as_slice(), + ["GET /v5.0.0/libpod/images/example%2Fimage%3Alatest/json"] + ); + let _ = std::fs::remove_file(socket_path); + } } diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index e417358c6e..90ef0fec21 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -397,6 +397,7 @@ fn build_env( sandbox: &DriverSandbox, config: &PodmanComputeConfig, image: &str, + oci_user: &str, ) -> BTreeMap { let spec = sandbox.spec.as_ref(); let template = spec.and_then(|s| s.template.as_ref()); @@ -482,6 +483,18 @@ fn build_env( env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN); env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE); + env.insert( + openshell_core::sandbox_env::OCI_IMAGE_USER.into(), + oci_user.to_string(), + ); + env.insert( + openshell_core::sandbox_env::SANDBOX_UID.into(), + String::new(), + ); + env.insert( + openshell_core::sandbox_env::SANDBOX_GID.into(), + String::new(), + ); // 4. Gateway-minted sandbox JWT. Keep the raw bearer out of container // metadata; the supervisor reads it from a driver-owned bind mount. @@ -876,6 +889,7 @@ pub fn try_build_container_spec_with_token( build_container_spec_with_token_and_gpu_devices(sandbox, config, token_secret_name, cdi_devices) } +#[cfg(test)] pub fn build_container_spec_with_token_and_gpu_devices( sandbox: &DriverSandbox, config: &PodmanComputeConfig, @@ -883,10 +897,30 @@ pub fn build_container_spec_with_token_and_gpu_devices( gpu_device_ids: Option<&[String]>, ) -> Result { let image = resolve_image(sandbox, config); + build_container_spec_for_image( + sandbox, + config, + token_secret_name, + gpu_device_ids, + image, + image, + "", + ) +} + +pub fn build_container_spec_for_image( + sandbox: &DriverSandbox, + config: &PodmanComputeConfig, + token_secret_name: Option<&str>, + gpu_device_ids: Option<&[String]>, + requested_image: &str, + image_id: &str, + oci_user: &str, +) -> Result { let name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); let vol = volume_name(&sandbox.id); - let env = build_env(sandbox, config, image); + let env = build_env(sandbox, config, requested_image, oci_user); let labels = build_labels(sandbox); let resource_limits = build_resource_limits(sandbox, config); let user_mounts = podman_user_mounts(sandbox, config.enable_bind_mounts) @@ -932,7 +966,7 @@ pub fn build_container_spec_with_token_and_gpu_devices( let container_spec = ContainerSpec { name, - image: image.to_string(), + image: image_id.to_string(), labels, env, volumes, @@ -1030,7 +1064,7 @@ pub fn build_container_spec_with_token_and_gpu_devices( // locks itself down. no_new_privileges: true, seccomp_profile_path: "unconfined".into(), - image_pull_policy: config.image_pull_policy.as_str().to_string(), + image_pull_policy: "never".to_string(), healthconfig: HealthConfig { test: vec![ "CMD-SHELL".into(), @@ -1325,6 +1359,50 @@ mod tests { ); } + #[test] + fn container_spec_pins_inspected_image_and_protects_oci_identity() { + let mut sandbox = test_sandbox("test-id", "test-name"); + let spec = sandbox.spec.get_or_insert_default(); + for (key, value) in [ + (openshell_core::sandbox_env::OCI_IMAGE_USER, "spoofed"), + (openshell_core::sandbox_env::SANDBOX_UID, "9999"), + (openshell_core::sandbox_env::SANDBOX_GID, "9999"), + ] { + spec.environment.insert(key.to_string(), value.to_string()); + } + + let container = build_container_spec_for_image( + &sandbox, + &test_config(), + None, + None, + "registry.example/app:latest", + "sha256:immutable", + "app:staff", + ) + .unwrap(); + + assert_eq!(container["image"].as_str(), Some("sha256:immutable")); + assert_eq!( + container["env"]["OPENSHELL_CONTAINER_IMAGE"].as_str(), + Some("registry.example/app:latest") + ); + assert_eq!(container["user"].as_str(), Some("0:0")); + assert_eq!(container["image_pull_policy"].as_str(), Some("never")); + assert_eq!( + container["env"][openshell_core::sandbox_env::OCI_IMAGE_USER].as_str(), + Some("app:staff") + ); + assert_eq!( + container["env"][openshell_core::sandbox_env::SANDBOX_UID].as_str(), + Some("") + ); + assert_eq!( + container["env"][openshell_core::sandbox_env::SANDBOX_GID].as_str(), + Some("") + ); + } + #[test] fn volume_name_uses_id() { assert_eq!( diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 3878f59836..aa3df9cbd7 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -571,6 +571,20 @@ impl PodmanComputeDriver { .pull_image(image, pull_policy) .await .map_err(ComputeDriverError::from)?; + let inspected_image = self + .client + .inspect_image(image) + .await + .map_err(ComputeDriverError::from)?; + if inspected_image.id.is_empty() { + return Err(ComputeDriverError::Precondition(format!( + "podman image '{image}' inspection did not return an immutable image ID" + ))); + } + let image_user = inspected_image + .config + .as_ref() + .map_or("", |config| config.user.as_str()); for image in container::podman_driver_image_mount_sources(sandbox, self.config.enable_bind_mounts) @@ -630,11 +644,14 @@ impl PodmanComputeDriver { return Err(e); } }; - let spec = match container::build_container_spec_with_token_and_gpu_devices( + let spec = match container::build_container_spec_for_image( sandbox, &self.config, token_secret_name.as_deref(), gpu_devices.as_deref(), + image, + &inspected_image.id, + image_user, ) { Ok(spec) => spec, Err(e) => { @@ -1653,6 +1670,10 @@ mod tests { vec![ StubResponse::new(StatusCode::OK, "{}"), // pull supervisor image StubResponse::new(StatusCode::OK, "{}"), // pull sandbox image + StubResponse::new( + StatusCode::OK, + r#"{"Id":"sha256:sandbox","Config":{"User":"1234:1235"}}"#, + ), // inspect sandbox image StubResponse::new(StatusCode::CREATED, "{}"), // create volume StubResponse::new(StatusCode::CREATED, "{}"), // create proxy-auth secret StubResponse::new(StatusCode::INTERNAL_SERVER_ERROR, r#"{"message":"boom"}"#), // create container @@ -1691,6 +1712,10 @@ mod tests { vec![ StubResponse::new(StatusCode::OK, "{}"), // pull supervisor image StubResponse::new(StatusCode::OK, "{}"), // pull sandbox image + StubResponse::new( + StatusCode::OK, + r#"{"Id":"sha256:sandbox","Config":{"User":"1234:1235"}}"#, + ), // inspect sandbox image StubResponse::new(StatusCode::CREATED, "{}"), // create volume StubResponse::new(StatusCode::CREATED, "{}"), // create proxy-auth secret StubResponse::new(StatusCode::CREATED, "{}"), // create container diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 6c92b1b567..7937a3757e 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -954,8 +954,7 @@ const SANDBOX_NAME: &str = "sandbox"; /// `u32` within the range `[MIN_SANDBOX_UID, MAX_SANDBOX_UID]`. /// /// Rejects: -/// - The empty string (callers should use `ensure_sandbox_process_identity` -/// to fill defaults before validation) +/// - The empty string (represents an omitted policy field) /// - UID 0 or values below `MIN_SANDBOX_UID` /// - Values above `MAX_SANDBOX_UID` /// - Non-numeric strings other than `"sandbox"` (e.g. `"root"`, `"nobody"`) @@ -1053,9 +1052,10 @@ pub const LEGACY_CONTAINER_POLICY_PATH: &str = "/etc/navigator/policy.yaml"; /// Return a restrictive default policy suitable for sandboxes that have no /// explicit policy configured. /// -/// This policy grants filesystem access to standard system paths, runs as the -/// `sandbox` user, enables Landlock in best-effort mode, and **blocks all -/// network access** (no network policies, no inference routing). +/// This policy grants filesystem access to standard system paths, leaves +/// process identity selection to the compute runtime, enables Landlock in +/// best-effort mode, and **blocks all network access** (no network policies, +/// no inference routing). pub fn restrictive_default_policy() -> SandboxPolicy { SandboxPolicy { version: 1, @@ -1075,20 +1075,17 @@ pub fn restrictive_default_policy() -> SandboxPolicy { landlock: Some(LandlockPolicy { compatibility: "best_effort".into(), }), - process: Some(ProcessPolicy { - run_as_user: "sandbox".into(), - run_as_group: "sandbox".into(), - }), + process: None, network_policies: HashMap::new(), network_middlewares: HashMap::default(), } } -/// Ensure the policy has `run_as_user: sandbox` and `run_as_group: sandbox`. +/// Fill omitted process identity fields with the legacy `sandbox` defaults. /// -/// If the process section is missing, or either field is empty, this fills in -/// the required `"sandbox"` value. Call this before validation so that -/// policies without an explicit process section get the correct default. +/// Docker and Podman preserve omission so their supervisors can fall back to +/// OCI `Config.User`. Other drivers call this before validation and +/// persistence to retain the existing public policy representation. pub fn ensure_sandbox_process_identity(policy: &mut SandboxPolicy) { let process = policy.process.get_or_insert_with(ProcessPolicy::default); if process.run_as_user.is_empty() { @@ -1112,7 +1109,7 @@ const MAX_PATH_LENGTH: usize = 4096; /// A safety violation found in a sandbox policy. #[derive(Debug, Clone, PartialEq, Eq)] pub enum PolicyViolation { - /// `run_as_user` or `run_as_group` is not "sandbox". + /// An explicit `run_as_user` or `run_as_group` is unsafe. InvalidProcessIdentity { field: &'static str, value: String }, /// A filesystem path contains `..` components. PathTraversal { path: String }, @@ -1277,7 +1274,7 @@ impl fmt::Display for PolicyViolation { /// error vs. logged warning). /// /// Checks performed: -/// - `run_as_user` / `run_as_group` must be "sandbox" +/// - Explicit `run_as_user` / `run_as_group` fields must be safe identities /// - Filesystem paths must be absolute (start with `/`) /// - Filesystem paths must not contain `..` components /// - Read-write paths must not be overly broad (just `/`) @@ -1292,18 +1289,17 @@ pub fn validate_sandbox_policy( ) -> std::result::Result<(), Vec> { let mut violations = Vec::new(); - // Check process identity — must be "sandbox" or a numeric UID/GID - // within the acceptable sandbox range. - // `ensure_sandbox_process_identity` should be called before this to - // fill in defaults; any invalid value is rejected. + // Omitted process identity fields are resolved by the compute runtime. + // Explicit fields must be "sandbox" or a numeric UID/GID within the + // acceptable sandbox range. if let Some(ref process) = policy.process { - if !is_valid_sandbox_identity(&process.run_as_user) { + if !process.run_as_user.is_empty() && !is_valid_sandbox_identity(&process.run_as_user) { violations.push(PolicyViolation::InvalidProcessIdentity { field: "run_as_user", value: process.run_as_user.clone(), }); } - if !is_valid_sandbox_identity(&process.run_as_group) { + if !process.run_as_group.is_empty() && !is_valid_sandbox_identity(&process.run_as_group) { violations.push(PolicyViolation::InvalidProcessIdentity { field: "run_as_group", value: process.run_as_group.clone(), @@ -1664,11 +1660,9 @@ network_policies: } #[test] - fn restrictive_default_has_process_identity() { + fn restrictive_default_omits_process_identity() { let policy = restrictive_default_policy(); - let proc = policy.process.expect("must have process policy"); - assert_eq!(proc.run_as_user, "sandbox"); - assert_eq!(proc.run_as_group, "sandbox"); + assert!(policy.process.is_none()); } #[test] @@ -1693,6 +1687,46 @@ network_policies: assert!(policy.filesystem.is_none()); } + #[test] + fn process_identity_omission_survives_yaml_round_trip() { + let policy = parse_sandbox_policy("version: 1\nprocess:\n run_as_user: \"1234\"\n") + .expect("partial process identity should parse"); + let process = policy.process.as_ref().expect("process section"); + assert_eq!(process.run_as_user, "1234"); + assert!(process.run_as_group.is_empty()); + assert!(validate_sandbox_policy(&policy).is_ok()); + + let yaml = serialize_sandbox_policy(&policy).expect("partial identity should serialize"); + assert!(yaml.contains("run_as_user")); + assert!(!yaml.contains("run_as_group")); + let reparsed = parse_sandbox_policy(&yaml).expect("round trip should parse"); + assert!(reparsed.process.unwrap().run_as_group.is_empty()); + } + + #[test] + fn ensure_sandbox_process_identity_fills_each_omitted_field() { + let cases = [ + (None, None, "sandbox", "sandbox"), + (Some("1234"), None, "1234", "sandbox"), + (None, Some("1235"), "sandbox", "1235"), + (Some("1234"), Some("1235"), "1234", "1235"), + ]; + + for (user, group, expected_user, expected_group) in cases { + let mut policy = restrictive_default_policy(); + policy.process = Some(ProcessPolicy { + run_as_user: user.unwrap_or_default().to_string(), + run_as_group: group.unwrap_or_default().to_string(), + }); + + ensure_sandbox_process_identity(&mut policy); + + let process = policy.process.expect("normalized process policy"); + assert_eq!(process.run_as_user, expected_user); + assert_eq!(process.run_as_group, expected_group); + } + } + #[test] fn parse_policy_with_network_rules() { let yaml = r" @@ -1819,38 +1853,6 @@ network_policies: assert!(err.to_string().contains("on_parse_error")); } - #[test] - fn ensure_sandbox_process_identity_fills_defaults() { - let mut policy = restrictive_default_policy(); - policy.process = None; - ensure_sandbox_process_identity(&mut policy); - let proc = policy.process.unwrap(); - assert_eq!(proc.run_as_user, "sandbox"); - assert_eq!(proc.run_as_group, "sandbox"); - } - - #[test] - fn ensure_sandbox_process_identity_fills_empty_strings() { - let mut policy = restrictive_default_policy(); - policy.process = Some(ProcessPolicy { - run_as_user: String::new(), - run_as_group: String::new(), - }); - ensure_sandbox_process_identity(&mut policy); - let proc = policy.process.unwrap(); - assert_eq!(proc.run_as_user, "sandbox"); - assert_eq!(proc.run_as_group, "sandbox"); - } - - #[test] - fn ensure_sandbox_process_identity_preserves_sandbox() { - let mut policy = restrictive_default_policy(); - ensure_sandbox_process_identity(&mut policy); - let proc = policy.process.unwrap(); - assert_eq!(proc.run_as_user, "sandbox"); - assert_eq!(proc.run_as_group, "sandbox"); - } - #[test] fn container_policy_path_is_expected() { assert_eq!(CONTAINER_POLICY_PATH, "/etc/openshell/policy.yaml"); @@ -2363,14 +2365,25 @@ network_policies: } #[test] - fn validate_rejects_empty_run_as_user() { + fn validate_accepts_omitted_process_fields() { let mut policy = restrictive_default_policy(); policy.process = Some(ProcessPolicy { run_as_user: String::new(), run_as_group: String::new(), }); - let violations = validate_sandbox_policy(&policy).unwrap_err(); - assert_eq!(violations.len(), 2); + assert!(validate_sandbox_policy(&policy).is_ok()); + + policy.process = Some(ProcessPolicy { + run_as_user: "sandbox".into(), + run_as_group: String::new(), + }); + assert!(validate_sandbox_policy(&policy).is_ok()); + + policy.process = Some(ProcessPolicy { + run_as_user: String::new(), + run_as_group: "1234".into(), + }); + assert!(validate_sandbox_policy(&policy).is_ok()); } #[test] diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index e696403f39..5ba41b9d7e 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -184,38 +184,21 @@ pub async fn run_sandbox( .await? }; - // Override the policy's process identity with the driver-resolved UID/GID - // from the pod environment. The policy defaults to the name "sandbox" which - // resolves via /etc/passwd, but the driver may have chosen a different - // numeric UID (e.g. from OpenShift SCC annotations). - // Validate overrides against the same rules as the policy layer to prevent - // env-injected values (e.g. GID 0) from bypassing policy restrictions. - if let Ok(uid) = std::env::var(openshell_core::sandbox_env::SANDBOX_UID) - && !uid.is_empty() - { - if !openshell_policy::is_valid_sandbox_identity(&uid) { - return Err(miette::miette!( - "OPENSHELL_SANDBOX_UID contains invalid sandbox identity '{uid}'; \ - expected 'sandbox' or a numeric UID in range [{}, {}]", - openshell_policy::MIN_SANDBOX_UID, - openshell_policy::MAX_SANDBOX_UID, - )); - } - policy.process.run_as_user = Some(uid); - } - if let Ok(gid) = std::env::var(openshell_core::sandbox_env::SANDBOX_GID) - && !gid.is_empty() - { - if !openshell_policy::is_valid_sandbox_identity(&gid) { - return Err(miette::miette!( - "OPENSHELL_SANDBOX_GID contains invalid sandbox identity '{gid}'; \ - expected 'sandbox' or a numeric GID in range [{}, {}]", - openshell_policy::MIN_SANDBOX_UID, - openshell_policy::MAX_SANDBOX_UID, - )); - } - policy.process.run_as_group = Some(gid); - } + // Normalize the active driver's identity contract once, while both the + // policy and launched image filesystem are available. Kubernetes and + // OpenShift retain their authoritative numeric pair; Docker and Podman + // fill only omitted policy fields from OCI Config.User. + #[cfg(unix)] + let resolved_process_identity = { + let driver_identity = openshell_supervisor_process::identity::DriverIdentity::from_env()?; + openshell_supervisor_process::identity::resolve_process_identity( + &mut policy, + &driver_identity, + )? + }; + #[cfg(not(unix))] + let resolved_process_identity = + openshell_supervisor_process::process::ResolvedProcessIdentity::default(); #[cfg_attr(not(target_os = "linux"), allow(unused_mut))] let (provider_credentials, mut provider_env) = @@ -708,6 +691,7 @@ pub async fn run_sandbox( ssh_socket_path, sidecar_network_enforcement, &process_policy, + resolved_process_identity, process_enforcement_mode, entrypoint_pid, entrypoint_started_tx, @@ -3367,9 +3351,10 @@ mod tests { let policy = discover_policy_from_path(path); // Restrictive default has no network policies. assert!(policy.network_policies.is_empty()); - // But does have filesystem and process policies. + // It keeps filesystem restrictions while leaving identity to the + // active compute driver. assert!(policy.filesystem.is_some()); - assert!(policy.process.is_some()); + assert!(policy.process.is_none()); } #[test] @@ -3439,9 +3424,7 @@ filesystem_policy: let policy = discover_policy_from_path(&path); // Falls back to restrictive default because of root user. - let proc = policy.process.unwrap(); - assert_eq!(proc.run_as_user, "sandbox"); - assert_eq!(proc.run_as_group, "sandbox"); + assert!(policy.process.is_none()); } #[test] diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index cf6b17c7e9..60e97126c7 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -2743,11 +2743,16 @@ impl ComputeDriver for NoopTestDriver { #[cfg(test)] pub async fn new_test_runtime(store: Arc) -> ComputeRuntime { + new_test_runtime_for_driver(store, "test").await +} + +#[cfg(test)] +pub async fn new_test_runtime_for_driver(store: Arc, driver_name: &str) -> ComputeRuntime { ComputeRuntime { driver: Arc::new(NoopTestDriver), driver_info: ComputeDriverInfoSnapshot { - name: "test".to_string(), - driver_name: "test".to_string(), + name: driver_name.to_string(), + driver_name: driver_name.to_string(), driver_version: "test".to_string(), }, shutdown_cleanup: None, diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 8618fd8013..eed96c3598 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -788,7 +788,7 @@ pub mod test_support { use std::sync::Arc; use crate::ServerState; - use crate::compute::new_test_runtime; + use crate::compute::{new_test_runtime, new_test_runtime_for_driver}; use crate::persistence::Store; use crate::sandbox_index::SandboxIndex; use crate::sandbox_watch::SandboxWatchBus; @@ -798,13 +798,22 @@ pub mod test_support { /// Build an in-memory `ServerState` for unit tests. pub async fn test_server_state() -> Arc { + test_server_state_with_driver("test").await + } + + /// Build an in-memory `ServerState` with a selected built-in driver name. + pub async fn test_server_state_with_driver(driver_name: &str) -> Arc { let store = Arc::new( Store::connect("sqlite::memory:?cache=shared") .await .unwrap(), ); crate::ensure_default_workspace(&store).await.unwrap(); - let compute = new_test_runtime(store.clone()).await; + let compute = if driver_name == "test" { + new_test_runtime(store.clone()).await + } else { + new_test_runtime_for_driver(store.clone(), driver_name).await + }; Arc::new(ServerState::new( Config::new(None).with_database_url("sqlite::memory:?cache=shared"), store, diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 53124261e6..e99ea1a30d 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -76,8 +76,9 @@ use tonic::{Request, Response, Status}; use tracing::{debug, info, warn}; use super::validation::{ - level_matches, source_matches, validate_annotations, validate_no_reserved_provider_policy_keys, - validate_policy_safety, validate_static_fields_unchanged, + level_matches, normalize_process_identity_for_driver, source_matches, validate_annotations, + validate_no_reserved_provider_policy_keys, validate_policy_safety, + validate_static_fields_unchanged, }; use super::{MAX_PAGE_SIZE, StoredSettingValue, StoredSettings, clamp_limit}; use crate::persistence::current_time_ms; @@ -1738,7 +1739,7 @@ async fn handle_update_config_inner( let mut new_policy = req.policy.ok_or_else(|| { Status::invalid_argument("policy is required for global policy update") })?; - openshell_policy::ensure_sandbox_process_identity(&mut new_policy); + normalize_process_identity_for_driver(&mut new_policy, state.compute.driver_kind()); validate_no_reserved_provider_policy_keys(&new_policy)?; validate_policy_safety(&new_policy)?; crate::middleware::validate_policy(state.middleware_registry.as_ref(), &new_policy) @@ -2014,11 +2015,15 @@ async fn handle_update_config_inner( provenance: &req.annotations, annotations: &req.annotations, }; + let mut baseline_policy = spec.policy.clone(); + if let Some(policy) = baseline_policy.as_mut() { + normalize_process_identity_for_driver(policy, state.compute.driver_kind()); + } let (version, hash, updated_sandbox) = apply_merge_operations_with_retry( state.store.as_ref(), &sandbox_id, &workspace, - spec.policy.as_ref(), + baseline_policy.as_ref(), &merge_ops, Some(&atomic_context), ) @@ -2084,6 +2089,7 @@ async fn handle_update_config_inner( let mut new_policy = req .policy .ok_or_else(|| Status::invalid_argument("policy is required"))?; + normalize_process_identity_for_driver(&mut new_policy, state.compute.driver_kind()); let global_settings = load_global_settings(state.store.as_ref()).await?; if global_settings.settings.contains_key(POLICY_SETTING_KEY) { @@ -2097,7 +2103,6 @@ async fn handle_update_config_inner( .as_ref() .ok_or_else(|| Status::internal("sandbox has no spec"))?; - openshell_policy::ensure_sandbox_process_identity(&mut new_policy); if sandbox_caller { if openshell_policy::strip_provider_rule_names(&mut new_policy) { debug!( @@ -2110,7 +2115,12 @@ async fn handle_update_config_inner( } let backfill_policy = if let Some(baseline_policy) = spec.policy.as_ref() { - validate_static_fields_unchanged(baseline_policy, &new_policy)?; + let mut comparable_baseline = baseline_policy.clone(); + normalize_process_identity_for_driver( + &mut comparable_baseline, + state.compute.driver_kind(), + ); + validate_static_fields_unchanged(&comparable_baseline, &new_policy)?; None } else { Some(new_policy.clone()) @@ -12078,7 +12088,13 @@ mod tests { let current_version = current.metadata.as_ref().unwrap().resource_version; // Backfill the policy with correct expected_resource_version - let new_policy = ProtoSandboxPolicy::default(); + let new_policy = ProtoSandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: "1234".to_string(), + run_as_group: String::new(), + }), + ..Default::default() + }; let response = handle_update_config( &state, @@ -12109,6 +12125,14 @@ mod tests { .await .unwrap() .unwrap(); + let process = updated_sandbox + .spec + .as_ref() + .and_then(|spec| spec.policy.as_ref()) + .and_then(|policy| policy.process.as_ref()) + .expect("legacy process identity should be persisted"); + assert_eq!(process.run_as_user, "1234"); + assert_eq!(process.run_as_group, "sandbox"); assert_eq!( updated_sandbox.metadata.as_ref().unwrap().resource_version, current_version + 1, @@ -12225,8 +12249,7 @@ mod tests { #[tokio::test] async fn update_config_same_policy_hash_with_new_provenance_creates_revision() { let state = test_server_state().await; - let mut policy = test_policy_with_rule("sandbox_only", "sandbox.example.com"); - openshell_policy::ensure_sandbox_process_identity(&mut policy); + let policy = test_policy_with_rule("sandbox_only", "sandbox.example.com"); let hash = deterministic_policy_hash(&policy); let sandbox = test_sandbox("sb-same-hash", "same-hash", policy.clone(), Vec::new()); state.store.put_message(&sandbox).await.unwrap(); @@ -12303,8 +12326,7 @@ mod tests { #[tokio::test] async fn update_config_same_policy_and_provenance_is_idempotent() { let state = test_server_state().await; - let mut policy = test_policy_with_rule("sandbox_only", "sandbox.example.com"); - openshell_policy::ensure_sandbox_process_identity(&mut policy); + let policy = test_policy_with_rule("sandbox_only", "sandbox.example.com"); state .store .put_message(&test_sandbox( @@ -12360,8 +12382,7 @@ mod tests { #[tokio::test] async fn update_config_full_policy_empty_annotations_preserves_existing_annotations() { let state = test_server_state().await; - let mut baseline = test_policy_with_rule("sandbox_only", "old.example.com"); - openshell_policy::ensure_sandbox_process_identity(&mut baseline); + let baseline = test_policy_with_rule("sandbox_only", "old.example.com"); let mut sandbox = test_sandbox( "sb-preserve-full", "preserve-full", @@ -12386,8 +12407,7 @@ mod tests { .await .unwrap(); - let mut updated = test_policy_with_rule("sandbox_only", "new.example.com"); - openshell_policy::ensure_sandbox_process_identity(&mut updated); + let updated = test_policy_with_rule("sandbox_only", "new.example.com"); let response = handle_update_config( &state, with_user(Request::new(UpdateConfigRequest { @@ -12428,8 +12448,7 @@ mod tests { #[tokio::test] async fn update_config_merge_empty_annotations_preserves_existing_annotations() { let state = test_server_state().await; - let mut baseline = test_policy_with_rule("sandbox_only", "sandbox.example.com"); - openshell_policy::ensure_sandbox_process_identity(&mut baseline); + let baseline = test_policy_with_rule("sandbox_only", "sandbox.example.com"); let mut sandbox = test_sandbox("sb-preserve-merge", "preserve-merge", baseline, Vec::new()); sandbox.metadata.as_mut().unwrap().annotations.insert( "openshell.nvidia.com/policy-provenance".to_string(), @@ -12492,8 +12511,7 @@ mod tests { #[tokio::test] async fn update_config_merge_stores_revision_provenance_atomically() { let state = test_server_state().await; - let mut baseline = test_policy_with_rule("sandbox_only", "sandbox.example.com"); - openshell_policy::ensure_sandbox_process_identity(&mut baseline); + let baseline = test_policy_with_rule("sandbox_only", "sandbox.example.com"); state .store .put_message(&test_sandbox( diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index c87fc01436..0d6a4e277f 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -47,8 +47,9 @@ use super::provider::{ get_provider_record, is_valid_env_key, validate_provider_environment_keys_unique, }; use super::validation::{ - level_matches, source_matches, validate_exec_request_fields, - validate_no_reserved_provider_policy_keys, validate_policy_safety, validate_sandbox_spec, + level_matches, normalize_process_identity_for_driver, source_matches, + validate_exec_request_fields, validate_no_reserved_provider_policy_keys, + validate_policy_safety, validate_sandbox_spec, }; use super::{MAX_PAGE_SIZE, MAX_PROVIDERS, MAX_ROUTABLE_NAME_LEN, clamp_limit}; use crate::persistence::current_time_ms; @@ -172,10 +173,10 @@ async fn handle_create_sandbox_inner( template.image = state.compute.default_image().to_string(); } - // Ensure process identity defaults to "sandbox" when missing or - // empty, then validate policy safety before persisting. + // Docker and Podman preserve omitted identity fields for OCI USER + // fallback. Other drivers retain the legacy persisted sandbox defaults. if let Some(ref mut policy) = spec.policy { - openshell_policy::ensure_sandbox_process_identity(policy); + normalize_process_identity_for_driver(policy, state.compute.driver_kind()); validate_no_reserved_provider_policy_keys(policy)?; validate_policy_safety(policy)?; crate::middleware::validate_policy(state.middleware_registry.as_ref(), policy).await?; @@ -2106,7 +2107,7 @@ async fn run_exec_with_russh( #[cfg(test)] mod tests { use super::*; - use crate::grpc::test_support::test_server_state; + use crate::grpc::test_support::{test_server_state, test_server_state_with_driver}; use openshell_core::proto::datamodel::v1::ObjectMeta; // ---- shell_escape ---- @@ -2969,6 +2970,114 @@ mod tests { ); } + #[tokio::test] + async fn create_and_get_preserve_partial_process_identity() { + let state = + test_server_state_with_driver(openshell_core::ComputeDriverKind::Docker.as_str()).await; + let policy = openshell_core::proto::SandboxPolicy { + version: 1, + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: String::new(), + run_as_group: "1234".to_string(), + }), + ..Default::default() + }; + + let response = handle_create_sandbox( + &state, + Request::new(CreateSandboxRequest { + name: "partial-id".to_string(), + spec: Some(openshell_core::proto::SandboxSpec { + policy: Some(policy), + ..Default::default() + }), + labels: HashMap::new(), + annotations: HashMap::new(), + workspace: String::new(), + }), + ) + .await + .expect("partial process identity should be accepted") + .into_inner(); + + let created_process = response + .sandbox + .unwrap() + .spec + .unwrap() + .policy + .unwrap() + .process + .unwrap(); + assert!(created_process.run_as_user.is_empty()); + assert_eq!(created_process.run_as_group, "1234"); + + let fetched_process = handle_get_sandbox( + &state, + Request::new(GetSandboxRequest { + name: "partial-id".to_string(), + workspace: String::new(), + }), + ) + .await + .unwrap() + .into_inner() + .sandbox + .unwrap() + .spec + .unwrap() + .policy + .unwrap() + .process + .unwrap(); + assert!(fetched_process.run_as_user.is_empty()); + assert_eq!(fetched_process.run_as_group, "1234"); + } + + #[tokio::test] + async fn create_and_get_restore_legacy_identity_defaults_for_non_local_driver() { + let state = + test_server_state_with_driver(openshell_core::ComputeDriverKind::Kubernetes.as_str()) + .await; + let policy = openshell_core::proto::SandboxPolicy { + version: 1, + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: String::new(), + run_as_group: "1234".to_string(), + }), + ..Default::default() + }; + + let response = handle_create_sandbox( + &state, + Request::new(CreateSandboxRequest { + name: "kube-partial-id".to_string(), + spec: Some(openshell_core::proto::SandboxSpec { + policy: Some(policy), + ..Default::default() + }), + labels: HashMap::new(), + annotations: HashMap::new(), + workspace: String::new(), + }), + ) + .await + .expect("Kubernetes identity defaults should be accepted") + .into_inner(); + + let process = response + .sandbox + .unwrap() + .spec + .unwrap() + .policy + .unwrap() + .process + .unwrap(); + assert_eq!(process.run_as_user, "sandbox"); + assert_eq!(process.run_as_group, "1234"); + } + #[tokio::test] async fn create_sandbox_still_rejects_long_label_values() { let state = test_server_state().await; diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 2f0ad8d139..c5d10c2b3a 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -8,6 +8,7 @@ #![allow(clippy::result_large_err)] // Validation returns Result<_, Status> +use openshell_core::ComputeDriverKind; use openshell_core::proto::{ ExecSandboxRequest, Provider, SandboxPolicy as ProtoSandboxPolicy, SandboxTemplate, }; @@ -25,6 +26,23 @@ use super::{ // Exec request validation // --------------------------------------------------------------------------- +/// Preserve process-identity omission only for the local OCI-aware drivers. +/// +/// Kubernetes, VM, and unknown/remote drivers retain the legacy persisted +/// `sandbox:sandbox` defaults so existing policy hashes and live-update +/// workflows do not change. +pub(super) fn normalize_process_identity_for_driver( + policy: &mut ProtoSandboxPolicy, + driver_kind: Option, +) { + if !matches!( + driver_kind, + Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman) + ) { + openshell_policy::ensure_sandbox_process_identity(policy); + } +} + /// Maximum number of arguments in the command array. pub(super) const MAX_EXEC_COMMAND_ARGS: usize = 1024; /// Maximum length of a single command argument or environment value (bytes). @@ -1636,6 +1654,44 @@ mod tests { // ---- Policy safety ---- + #[test] + fn process_identity_omission_is_driver_scoped() { + use openshell_core::proto::ProcessPolicy; + + for driver in [ComputeDriverKind::Docker, ComputeDriverKind::Podman] { + let mut policy = ProtoSandboxPolicy { + process: Some(ProcessPolicy { + run_as_user: "1234".into(), + run_as_group: String::new(), + }), + ..Default::default() + }; + normalize_process_identity_for_driver(&mut policy, Some(driver)); + assert!( + policy.process.unwrap().run_as_group.is_empty(), + "{driver:?} must preserve omission" + ); + } + + for driver in [ + Some(ComputeDriverKind::Kubernetes), + Some(ComputeDriverKind::Vm), + None, + ] { + let mut policy = ProtoSandboxPolicy { + process: Some(ProcessPolicy { + run_as_user: "1234".into(), + run_as_group: String::new(), + }), + ..Default::default() + }; + normalize_process_identity_for_driver(&mut policy, driver); + let process = policy.process.unwrap(); + assert_eq!(process.run_as_user, "1234"); + assert_eq!(process.run_as_group, "sandbox"); + } + } + #[test] fn validate_policy_safety_rejects_root_user() { use openshell_core::proto::{FilesystemPolicy, ProcessPolicy}; diff --git a/crates/openshell-supervisor-process/src/identity.rs b/crates/openshell-supervisor-process/src/identity.rs new file mode 100644 index 0000000000..6a9a785542 --- /dev/null +++ b/crates/openshell-supervisor-process/src/identity.rs @@ -0,0 +1,738 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Driver identity normalization and OCI `USER` resolution. + +use crate::process::ResolvedProcessIdentity; +use miette::{IntoDiagnostic, Result}; +use openshell_core::policy::SandboxPolicy; +use std::fs::{File, OpenOptions}; +use std::io::Read; +use std::os::unix::fs::OpenOptionsExt; +use std::path::Path; + +const PASSWD_PATH: &str = "/etc/passwd"; +const GROUP_PATH: &str = "/etc/group"; +const MAX_ACCOUNT_FILE_SIZE: u64 = 1024 * 1024; +const MAX_ACCOUNT_LINE_SIZE: usize = 8 * 1024; +const MAX_ACCOUNT_FIELD_SIZE: usize = 1024; + +/// Identity input selected by the active compute driver. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DriverIdentity { + /// Platform-selected identity used by Kubernetes and `OpenShift`. + Resolved { uid: u32, gid: u32 }, + /// Raw OCI `Config.User` selected by Docker and Podman. + OciUser { declaration: String }, + /// Drivers with no authoritative identity metadata. + None, +} + +impl DriverIdentity { + /// Normalize the protected driver environment into one identity variant. + pub fn from_env() -> Result { + let oci_user = optional_utf8_env(openshell_core::sandbox_env::OCI_IMAGE_USER)?; + let uid = optional_nonempty_utf8_env(openshell_core::sandbox_env::SANDBOX_UID)?; + let gid = optional_nonempty_utf8_env(openshell_core::sandbox_env::SANDBOX_GID)?; + Self::from_values(oci_user, uid, gid) + } + + fn from_values( + oci_user: Option, + uid: Option, + gid: Option, + ) -> Result { + // Resolved-identity drivers explicitly clear the OCI declaration so + // an image-baked or user-supplied value cannot select the OCI path. + // Preserve an empty declaration when no resolved pair is present: + // Docker and Podman use that state to reject images without USER. + let oci_user = if oci_user.as_deref() == Some("") && (uid.is_some() || gid.is_some()) { + None + } else { + oci_user + }; + + match (oci_user, uid, gid) { + (Some(declaration), None, None) => Ok(Self::OciUser { declaration }), + (None, Some(uid), Some(gid)) => { + let uid = uid.parse::().ok().filter(|uid| { + (openshell_policy::MIN_SANDBOX_UID..=openshell_policy::MAX_SANDBOX_UID) + .contains(uid) + }); + let gid = gid.parse::().ok().filter(|gid| { + (openshell_policy::MIN_SANDBOX_UID..=openshell_policy::MAX_SANDBOX_UID) + .contains(gid) + }); + let (Some(uid), Some(gid)) = (uid, gid) else { + return Err(miette::miette!( + "driver UID/GID must be numeric identities in range [{}, {}]", + openshell_policy::MIN_SANDBOX_UID, + openshell_policy::MAX_SANDBOX_UID + )); + }; + Ok(Self::Resolved { uid, gid }) + } + (None, None, None) => Ok(Self::None), + (Some(_), _, _) => Err(miette::miette!( + "{} conflicts with non-empty {}/{} driver identity", + openshell_core::sandbox_env::OCI_IMAGE_USER, + openshell_core::sandbox_env::SANDBOX_UID, + openshell_core::sandbox_env::SANDBOX_GID + )), + (None, _, _) => Err(miette::miette!( + "{} and {} must be supplied together", + openshell_core::sandbox_env::SANDBOX_UID, + openshell_core::sandbox_env::SANDBOX_GID + )), + } + } +} + +/// Apply a driver identity before any workload child becomes reachable. +pub fn resolve_process_identity( + policy: &mut SandboxPolicy, + driver_identity: &DriverIdentity, +) -> Result { + match driver_identity { + DriverIdentity::Resolved { uid, gid } => { + policy.process.run_as_user = Some(uid.to_string()); + policy.process.run_as_group = Some(gid.to_string()); + // Kubernetes/OpenShift already supply numeric policy values and + // retain their existing privilege-drop path. + Ok(ResolvedProcessIdentity::default()) + } + DriverIdentity::OciUser { declaration } => resolve_oci_process_identity_at( + policy, + declaration, + Path::new(PASSWD_PATH), + Path::new(GROUP_PATH), + ), + DriverIdentity::None => { + // VM/offline drivers retain the pre-OCI per-field fallback. A + // partial policy must never leave the omitted component at the + // root supervisor identity. + if policy + .process + .run_as_user + .as_deref() + .is_none_or(str::is_empty) + { + policy.process.run_as_user = Some("sandbox".into()); + } + if policy + .process + .run_as_group + .as_deref() + .is_none_or(str::is_empty) + { + policy.process.run_as_group = Some("sandbox".into()); + } + Ok(ResolvedProcessIdentity::default()) + } + } +} + +#[allow(clippy::similar_names)] +fn resolve_oci_process_identity_at( + policy: &mut SandboxPolicy, + declaration: &str, + passwd_path: &Path, + group_path: &Path, +) -> Result { + let explicit_user = policy + .process + .run_as_user + .as_deref() + .is_some_and(|value| !value.is_empty()); + let explicit_group = policy + .process + .run_as_group + .as_deref() + .is_some_and(|value| !value.is_empty()); + + if explicit_user && explicit_group { + return Ok(ResolvedProcessIdentity::default()); + } + + let (oci_user, oci_group) = split_oci_declaration(declaration); + let needs_primary_gid = !explicit_group && oci_group.is_none(); + let resolved_user = if !explicit_user || needs_primary_gid { + Some(resolve_required_oci_user( + oci_user, + passwd_path, + declaration, + needs_primary_gid, + )?) + } else { + None + }; + + let oci_uid = if explicit_user { + None + } else { + Some( + resolved_user + .as_ref() + .expect("omitted OCI user must have been resolved") + .0, + ) + }; + + if !explicit_user { + policy.process.run_as_user = Some(oci_user.to_string()); + } + + let oci_gid = if explicit_group { + None + } else { + let (group_value, gid) = match oci_group { + Some(group) if !group.is_empty() => { + let gid = validate_oci_group(group, group_path, declaration)?; + (group.to_string(), gid) + } + Some(_) => { + return Err(miette::miette!( + "OCI USER '{declaration}' has an empty group component" + )); + } + None => { + let gid = resolved_user + .and_then(|(_, primary_gid)| primary_gid) + .ok_or_else(|| { + miette::miette!( + "OCI USER '{declaration}' uses a numeric UID without an explicit group, \ + but /etc/passwd has no matching primary GID" + ) + })?; + (gid.to_string(), gid) + } + }; + policy.process.run_as_group = Some(group_value); + Some(gid) + }; + + Ok(ResolvedProcessIdentity::new(oci_uid, oci_gid)) +} + +fn split_oci_declaration(declaration: &str) -> (&str, Option<&str>) { + declaration + .split_once(':') + .map_or((declaration, None), |(user, group)| (user, Some(group))) +} + +fn resolve_required_oci_user( + user: &str, + passwd_path: &Path, + declaration: &str, + require_primary_gid: bool, +) -> Result<(u32, Option)> { + if user.is_empty() { + return Err(miette::miette!( + "OCI USER is required because run_as_user is omitted" + )); + } + validate_component(user, "OCI user")?; + if user == "root" { + return Err(miette::miette!("OCI USER '{declaration}' selects root")); + } + if let Ok(uid) = user.parse::() { + if uid == 0 { + return Err(miette::miette!("OCI USER '{declaration}' selects UID 0")); + } + let primary_gid = if require_primary_gid { + find_passwd_by_uid(passwd_path, uid)?.map(|entry| entry.gid) + } else { + None + }; + if primary_gid == Some(0) { + return Err(miette::miette!( + "OCI USER '{declaration}' resolves to prohibited primary GID 0" + )); + } + return Ok((uid, primary_gid)); + } + let entry = find_passwd_by_name(passwd_path, user)? + .ok_or_else(|| miette::miette!("OCI USER name '{user}' was not found in /etc/passwd"))?; + if entry.uid == 0 { + return Err(miette::miette!( + "OCI USER '{declaration}' resolves to prohibited UID 0" + )); + } + if require_primary_gid && entry.gid == 0 { + return Err(miette::miette!( + "OCI USER '{declaration}' resolves to prohibited primary GID 0" + )); + } + Ok((entry.uid, require_primary_gid.then_some(entry.gid))) +} + +fn validate_oci_group(value: &str, group_path: &Path, declaration: &str) -> Result { + validate_component(value, "OCI group")?; + if value == "root" { + return Err(miette::miette!( + "OCI USER '{declaration}' selects root group" + )); + } + let gid = if let Ok(gid) = value.parse::() { + gid + } else { + find_group_by_name(group_path, value)? + .ok_or_else(|| miette::miette!("OCI group '{value}' was not found in /etc/group"))? + .gid + }; + if gid == 0 { + return Err(miette::miette!( + "OCI USER '{declaration}' resolves to prohibited GID 0" + )); + } + Ok(gid) +} + +fn validate_component(value: &str, kind: &str) -> Result<()> { + if value.is_empty() + || value.len() > MAX_ACCOUNT_FIELD_SIZE + || value.trim() != value + || value.chars().any(|ch| ch.is_control() || ch == ':') + { + return Err(miette::miette!("{kind} component '{value}' is malformed")); + } + Ok(()) +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PasswdEntry { + uid: u32, + gid: u32, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct GroupEntry { + gid: u32, +} + +fn find_passwd_by_name(path: &Path, name: &str) -> Result> { + find_unique(path, |fields| { + (fields.first().copied() == Some(name)).then(|| parse_passwd(fields)) + }) +} + +fn find_passwd_by_uid(path: &Path, uid: u32) -> Result> { + find_unique(path, |fields| { + fields + .get(2) + .and_then(|value| value.parse::().ok()) + .filter(|candidate| *candidate == uid) + .map(|_| parse_passwd(fields)) + }) +} + +fn find_group_by_name(path: &Path, name: &str) -> Result> { + find_unique(path, |fields| { + (fields.first().copied() == Some(name)).then(|| parse_group(fields)) + }) +} + +fn find_unique( + path: &Path, + mut select: impl FnMut(&[&str]) -> Option>, +) -> Result> { + let content = read_account_file(path)?; + let mut found = None; + for line in content.lines() { + if line.is_empty() || line.starts_with('#') { + continue; + } + if line.len() > MAX_ACCOUNT_LINE_SIZE { + return Err(miette::miette!( + "account file '{}' contains an oversized line", + path.display() + )); + } + let fields = line.split(':').collect::>(); + if fields + .iter() + .any(|field| field.len() > MAX_ACCOUNT_FIELD_SIZE) + { + return Err(miette::miette!( + "account file '{}' contains an oversized field", + path.display() + )); + } + let Some(candidate) = select(&fields) else { + continue; + }; + let candidate = candidate?; + if found.replace(candidate).is_some() { + return Err(miette::miette!( + "account identity is ambiguous in '{}'", + path.display() + )); + } + } + Ok(found) +} + +fn parse_passwd(fields: &[&str]) -> Result { + if fields.len() != 7 { + return Err(miette::miette!("matching /etc/passwd entry is malformed")); + } + Ok(PasswdEntry { + uid: fields[2] + .parse() + .map_err(|_| miette::miette!("matching /etc/passwd UID is malformed"))?, + gid: fields[3] + .parse() + .map_err(|_| miette::miette!("matching /etc/passwd GID is malformed"))?, + }) +} + +fn parse_group(fields: &[&str]) -> Result { + if fields.len() != 4 { + return Err(miette::miette!("matching /etc/group entry is malformed")); + } + Ok(GroupEntry { + gid: fields[2] + .parse() + .map_err(|_| miette::miette!("matching /etc/group GID is malformed"))?, + }) +} + +fn read_account_file(path: &Path) -> Result { + let mut options = OpenOptions::new(); + options + .read(true) + .custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW); + let mut file = options + .open(path) + .into_diagnostic() + .map_err(|error| miette::miette!("failed to open '{}': {error}", path.display()))?; + validate_account_file(&file, path)?; + + let mut bytes = Vec::new(); + file.by_ref() + .take(MAX_ACCOUNT_FILE_SIZE + 1) + .read_to_end(&mut bytes) + .into_diagnostic()?; + if bytes.len() as u64 > MAX_ACCOUNT_FILE_SIZE { + return Err(miette::miette!( + "account file '{}' exceeds {MAX_ACCOUNT_FILE_SIZE} bytes", + path.display() + )); + } + String::from_utf8(bytes) + .map_err(|_| miette::miette!("account file '{}' is not valid UTF-8", path.display())) +} + +fn validate_account_file(file: &File, path: &Path) -> Result<()> { + let metadata = file.metadata().into_diagnostic()?; + if !metadata.is_file() { + return Err(miette::miette!( + "account path '{}' is not a regular file", + path.display() + )); + } + if metadata.len() > MAX_ACCOUNT_FILE_SIZE { + return Err(miette::miette!( + "account file '{}' exceeds {MAX_ACCOUNT_FILE_SIZE} bytes", + path.display() + )); + } + Ok(()) +} + +fn optional_utf8_env(name: &str) -> Result> { + std::env::var_os(name) + .map(|value| { + value + .into_string() + .map_err(|_| miette::miette!("{name} is not valid UTF-8")) + }) + .transpose() +} + +fn optional_nonempty_utf8_env(name: &str) -> Result> { + Ok(optional_utf8_env(name)?.filter(|value| !value.is_empty())) +} + +#[cfg(test)] +mod tests { + use super::*; + use openshell_core::policy::SandboxPolicy; + use std::fs; + use tempfile::tempdir; + + fn account_files( + passwd: &str, + group: &str, + ) -> (tempfile::TempDir, std::path::PathBuf, std::path::PathBuf) { + let dir = tempdir().unwrap(); + let passwd_path = dir.path().join("passwd"); + let group_path = dir.path().join("group"); + fs::write(&passwd_path, passwd).unwrap(); + fs::write(&group_path, group).unwrap(); + (dir, passwd_path, group_path) + } + + fn policy(user: Option<&str>, group: Option<&str>) -> SandboxPolicy { + let mut policy = SandboxPolicy { + version: 1, + filesystem: openshell_core::policy::FilesystemPolicy::default(), + network: openshell_core::policy::NetworkPolicy::default(), + landlock: openshell_core::policy::LandlockPolicy::default(), + process: openshell_core::policy::ProcessPolicy::default(), + }; + policy.process.run_as_user = user.map(str::to_string); + policy.process.run_as_group = group.map(str::to_string); + policy + } + + #[test] + fn per_field_policy_precedence_resolves_complete_pair() { + let (_dir, passwd, group) = account_files( + "app:x:1234:1235::/home/app:/bin/sh\nsandbox:x:2000:2001::/sandbox:/bin/sh\n", + "staff:x:1235:\nsandbox:x:2001:\n", + ); + let cases = [ + ( + Some("2000"), + Some("2001"), + "root", + "2000", + "2001", + None, + None, + ), + ( + Some("2000"), + None, + "app:staff", + "2000", + "staff", + None, + Some(1235), + ), + ( + None, + Some("2001"), + "app:root", + "app", + "2001", + Some(1234), + None, + ), + ( + None, + None, + "app:staff", + "app", + "staff", + Some(1234), + Some(1235), + ), + (None, None, "app", "app", "1235", Some(1234), Some(1235)), + ]; + for ( + user, + group_name, + declaration, + expected_user, + expected_group, + resolved_uid, + resolved_gid, + ) in cases + { + let mut policy = policy(user, group_name); + let resolved = + resolve_oci_process_identity_at(&mut policy, declaration, &passwd, &group).unwrap(); + assert_eq!(policy.process.run_as_user.as_deref(), Some(expected_user)); + assert_eq!(policy.process.run_as_group.as_deref(), Some(expected_group)); + assert_eq!(resolved.uid(), resolved_uid); + assert_eq!(resolved.gid(), resolved_gid); + } + } + + #[test] + fn numeric_pair_does_not_require_account_entries() { + let dir = tempdir().unwrap(); + let passwd = dir.path().join("missing-passwd"); + let group = dir.path().join("missing-group"); + let mut policy = policy(None, None); + let resolved = + resolve_oci_process_identity_at(&mut policy, "1234:1235", &passwd, &group).unwrap(); + assert_eq!(policy.process.run_as_user.as_deref(), Some("1234")); + assert_eq!(policy.process.run_as_group.as_deref(), Some("1235")); + assert_eq!( + resolved, + ResolvedProcessIdentity::new(Some(1234), Some(1235)) + ); + } + + #[test] + fn explicit_identity_is_preserved_without_inspecting_oci_or_accounts() { + let dir = tempdir().unwrap(); + let mut policy = policy(Some("sandbox"), Some("sandbox")); + + let resolved = resolve_oci_process_identity_at( + &mut policy, + "root:root", + &dir.path().join("missing-passwd"), + &dir.path().join("missing-group"), + ) + .unwrap(); + + assert_eq!(policy.process.run_as_user.as_deref(), Some("sandbox")); + assert_eq!(policy.process.run_as_group.as_deref(), Some("sandbox")); + assert_eq!(resolved, ResolvedProcessIdentity::default()); + } + + #[test] + fn driver_identity_inputs_are_mutually_exclusive_and_complete() { + assert_eq!( + DriverIdentity::from_values(Some("app".into()), None, None).unwrap(), + DriverIdentity::OciUser { + declaration: "app".into() + } + ); + assert_eq!( + DriverIdentity::from_values(None, Some("1234".into()), Some("1235".into())).unwrap(), + DriverIdentity::Resolved { + uid: 1234, + gid: 1235 + } + ); + assert_eq!( + DriverIdentity::from_values( + Some(String::new()), + Some("1234".into()), + Some("1235".into()) + ) + .unwrap(), + DriverIdentity::Resolved { + uid: 1234, + gid: 1235 + } + ); + assert_eq!( + DriverIdentity::from_values(Some(String::new()), None, None).unwrap(), + DriverIdentity::OciUser { + declaration: String::new() + } + ); + assert_eq!( + DriverIdentity::from_values(None, None, None).unwrap(), + DriverIdentity::None + ); + assert!( + DriverIdentity::from_values( + Some("app".into()), + Some("1234".into()), + Some("1235".into()) + ) + .is_err() + ); + assert!(DriverIdentity::from_values(None, Some("1234".into()), None).is_err()); + } + + #[test] + fn no_driver_identity_completes_partial_policy_with_sandbox() { + let cases = [ + (None, Some("staff"), "sandbox", "staff"), + (Some("app"), None, "app", "sandbox"), + (None, None, "sandbox", "sandbox"), + (Some("app"), Some("staff"), "app", "staff"), + ]; + + for (user, group, expected_user, expected_group) in cases { + let mut policy = policy(user, group); + let resolved = resolve_process_identity(&mut policy, &DriverIdentity::None).unwrap(); + + assert_eq!(policy.process.run_as_user.as_deref(), Some(expected_user)); + assert_eq!(policy.process.run_as_group.as_deref(), Some(expected_group)); + assert_eq!(resolved, ResolvedProcessIdentity::default()); + } + } + + #[test] + fn numeric_uid_uses_passwd_primary_gid() { + let (_dir, passwd, group) = account_files("app:x:1234:4321::/home/app:/bin/sh\n", ""); + let mut policy = policy(None, None); + let resolved = + resolve_oci_process_identity_at(&mut policy, "1234", &passwd, &group).unwrap(); + assert_eq!(policy.process.run_as_group.as_deref(), Some("4321")); + assert_eq!( + resolved, + ResolvedProcessIdentity::new(Some(1234), Some(4321)) + ); + } + + #[test] + fn missing_unknown_ambiguous_and_root_identities_fail() { + let (_dir, passwd, group) = account_files( + "app:x:1234:1235::/home/app:/bin/sh\napp:x:2234:2235::/home/app2:/bin/sh\n", + "staff:x:1235:\nstaff:x:2235:\n", + ); + for declaration in ["", "unknown", "app", "9999", "0:1235", "1234:0"] { + let mut policy = policy(None, None); + assert!( + resolve_oci_process_identity_at(&mut policy, declaration, &passwd, &group).is_err(), + "{declaration:?} unexpectedly resolved" + ); + } + } + + #[test] + fn selected_component_is_validated_independently() { + let (_dir, passwd, group) = + account_files("app:x:1234:1235::/home/app:/bin/sh\n", "staff:x:1235:\n"); + + let mut explicit_user = policy(Some("1234"), None); + let resolved = + resolve_oci_process_identity_at(&mut explicit_user, "root:staff", &passwd, &group) + .unwrap(); + assert_eq!(explicit_user.process.run_as_user.as_deref(), Some("1234")); + assert_eq!(explicit_user.process.run_as_group.as_deref(), Some("staff")); + assert_eq!(resolved, ResolvedProcessIdentity::new(None, Some(1235))); + + let mut explicit_group = policy(None, Some("1235")); + let resolved = + resolve_oci_process_identity_at(&mut explicit_group, "app:root", &passwd, &group) + .unwrap(); + assert_eq!(explicit_group.process.run_as_user.as_deref(), Some("app")); + assert_eq!(explicit_group.process.run_as_group.as_deref(), Some("1235")); + assert_eq!(resolved, ResolvedProcessIdentity::new(Some(1234), None)); + } + + #[test] + fn named_oci_components_mapping_to_root_are_rejected() { + let (_dir, passwd, group) = account_files( + "root_alias:x:0:1235::/root:/bin/sh\napp:x:1234:1235::/home/app:/bin/sh\n", + "root_alias:x:0:\nstaff:x:1235:\n", + ); + + let mut root_user = policy(None, None); + assert!( + resolve_oci_process_identity_at(&mut root_user, "root_alias:staff", &passwd, &group) + .is_err() + ); + + let mut root_group = policy(None, None); + assert!( + resolve_oci_process_identity_at(&mut root_group, "app:root_alias", &passwd, &group) + .is_err() + ); + } + + #[cfg(unix)] + #[test] + fn account_file_symlinks_are_rejected() { + use std::os::unix::fs::symlink; + + let (_dir, passwd, group) = + account_files("app:x:1234:1235::/home/app:/bin/sh\n", "staff:x:1235:\n"); + let link = passwd.with_file_name("passwd-link"); + symlink(&passwd, &link).unwrap(); + + let mut policy = policy(None, None); + assert!(resolve_oci_process_identity_at(&mut policy, "app:staff", &link, &group).is_err()); + } +} diff --git a/crates/openshell-supervisor-process/src/lib.rs b/crates/openshell-supervisor-process/src/lib.rs index 842b62f9df..ca93230929 100644 --- a/crates/openshell-supervisor-process/src/lib.rs +++ b/crates/openshell-supervisor-process/src/lib.rs @@ -10,6 +10,8 @@ pub mod child_env; pub mod debug_rpc; +#[cfg(unix)] +pub mod identity; pub mod log_push; pub mod managed_children; pub mod process; diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 3733c7c7e3..93ab4787b3 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -40,6 +40,45 @@ pub enum ProcessEnforcementMode { NetworkOnly, } +/// Numeric identity components resolved once from driver-owned metadata. +/// +/// A component is `None` when the corresponding policy field was explicit and +/// must continue through the existing policy identity path. OCI-derived +/// components are carried numerically so later filesystem setup and direct/SSH +/// privilege drops cannot resolve them differently through NSS. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct ResolvedProcessIdentity { + uid: Option, + gid: Option, +} + +impl ResolvedProcessIdentity { + #[must_use] + pub const fn new(uid: Option, gid: Option) -> Self { + Self { uid, gid } + } + + #[must_use] + pub const fn uid(self) -> Option { + self.uid + } + + #[must_use] + pub const fn gid(self) -> Option { + self.gid + } + + /// Whether at least one process identity component came from OCI `USER`. + /// + /// Platform-resolved identities are written directly into the policy and + /// return the default value, so this is specific to Docker/Podman OCI + /// fallback without adding another driver contract. + #[must_use] + pub const fn uses_oci_user_fallback(self) -> bool { + self.uid.is_some() || self.gid.is_some() + } +} + impl ProcessEnforcementMode { #[must_use] pub const fn uses_privileged_process_setup(self) -> bool { @@ -71,6 +110,9 @@ pub(crate) fn prepare_child_sandbox( } const SUPERVISOR_ONLY_ENV_VARS: &[&str] = &[ + openshell_core::sandbox_env::OCI_IMAGE_USER, + openshell_core::sandbox_env::SANDBOX_UID, + openshell_core::sandbox_env::SANDBOX_GID, openshell_core::sandbox_env::SANDBOX_TOKEN, openshell_core::sandbox_env::SANDBOX_TOKEN_FILE, openshell_core::sandbox_env::K8S_SA_TOKEN_FILE, @@ -488,6 +530,7 @@ impl ProcessHandle { workdir: Option<&str>, interactive: bool, policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, netns: Option<&NetworkNamespace>, ca_paths: Option<&(PathBuf, PathBuf)>, @@ -499,6 +542,7 @@ impl ProcessHandle { workdir, interactive, policy, + resolved_identity, enforcement_mode, netns.and_then(NetworkNamespace::ns_fd), ca_paths, @@ -519,6 +563,7 @@ impl ProcessHandle { workdir: Option<&str>, interactive: bool, policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ca_paths: Option<&(PathBuf, PathBuf)>, provider_env: &HashMap, @@ -529,6 +574,7 @@ impl ProcessHandle { workdir, interactive, policy, + resolved_identity, enforcement_mode, ca_paths, provider_env, @@ -543,6 +589,7 @@ impl ProcessHandle { workdir: Option<&str>, interactive: bool, policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, netns_fd: Option, ca_paths: Option<&(PathBuf, PathBuf)>, @@ -660,7 +707,7 @@ impl ProcessHandle { // /etc/group and /etc/passwd which would be blocked if // Landlock were already enforced. if enforcement_mode.uses_privileged_process_setup() { - drop_privileges(&policy) + drop_privileges_with_identity(&policy, resolved_identity) .map_err(|err| std::io::Error::other(err.to_string()))?; } @@ -697,6 +744,7 @@ impl ProcessHandle { workdir: Option<&str>, interactive: bool, policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ca_paths: Option<&(PathBuf, PathBuf)>, provider_env: &HashMap, @@ -761,7 +809,7 @@ impl ProcessHandle { // initgroups/setgid/setuid need access to /etc/group and /etc/passwd // which may be blocked by Landlock. if enforcement_mode.uses_privileged_process_setup() { - drop_privileges(&policy) + drop_privileges_with_identity(&policy, resolved_identity) .map_err(|err| std::io::Error::other(err.to_string()))?; } @@ -855,21 +903,20 @@ impl Drop for ProcessHandle { } } -/// Validate that the configured sandbox identity exists in this image. +/// Validate the configured process user. /// -/// When the identity is the literal `"sandbox"`, verifies the user exists -/// in `/etc/passwd` (all sandbox images ship with one). -/// -/// When the identity is a numeric UID, skips the passwd lookup entirely — -/// the kernel will use the resolved UID regardless of whether an entry -/// exists in `/etc/passwd`. Logs an OCSF event confirming numeric UID usage. -/// Non-numeric, non-"sandbox" values are rejected. +/// Numeric identities do not require a passwd entry. The legacy explicit +/// `"sandbox"` identity and other names must resolve in `/etc/passwd`. #[cfg(unix)] pub fn validate_sandbox_user(policy: &SandboxPolicy) -> Result<()> { let identity = policy.process.run_as_user.as_deref().unwrap_or("sandbox"); - // Numeric UID — no passwd entry required; kernel resolves directly. - if openshell_policy::is_valid_sandbox_identity(identity) && identity.parse::().is_ok() { + if let Ok(uid) = identity.parse::() { + if !(MIN_SANDBOX_UID..=MAX_SANDBOX_UID).contains(&uid) { + return Err(miette::miette!( + "process user UID must be in range [{MIN_SANDBOX_UID}, {MAX_SANDBOX_UID}]" + )); + } openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) .severity(openshell_ocsf::SeverityId::Informational) @@ -883,7 +930,7 @@ pub fn validate_sandbox_user(policy: &SandboxPolicy) -> Result<()> { return Ok(()); } - // "sandbox" name — must exist in /etc/passwd. + // Legacy explicit "sandbox" name — must exist in /etc/passwd. if identity == "sandbox" { match User::from_name("sandbox") { Ok(Some(_)) => { @@ -898,8 +945,7 @@ pub fn validate_sandbox_user(policy: &SandboxPolicy) -> Result<()> { } Ok(None) => { return Err(miette::miette!( - "sandbox user 'sandbox' not found in image; \ - all sandbox images must include a 'sandbox' user and group" + "explicit process user 'sandbox' was not found in the image" )); } Err(e) => { @@ -907,15 +953,11 @@ pub fn validate_sandbox_user(policy: &SandboxPolicy) -> Result<()> { } } } else if !identity.is_empty() { - // Non-numeric, non-sandbox string — attempt passwd lookup. - // This catches cases where someone accidentally put "root" or similar. + // Other names are supported by local/offline policy paths and must + // resolve before privilege dropping. match User::from_name(identity) { Ok(Some(_)) => { - tracing::warn!( - identity, - "non-sandbox user accepted via passwd entry; \ - consider using a numeric UID for UID-injected images" - ); + tracing::warn!(identity, "named process user accepted via passwd entry"); } Ok(None) => { return Err(miette::miette!( @@ -936,14 +978,17 @@ pub fn validate_sandbox_user(policy: &SandboxPolicy) -> Result<()> { /// Validate that the configured sandbox group identity is acceptable. /// -/// Mirrors [`validate_sandbox_user`] for the group dimension: numeric GIDs -/// must fall within the allowed sandbox range, the literal `"sandbox"` must -/// resolve via `/etc/group`, and unrecognised strings are rejected. +/// Mirrors [`validate_sandbox_user`] for the group dimension. #[cfg(unix)] pub fn validate_sandbox_group(policy: &SandboxPolicy) -> Result<()> { let identity = policy.process.run_as_group.as_deref().unwrap_or("sandbox"); - if openshell_policy::is_valid_sandbox_identity(identity) && identity.parse::().is_ok() { + if let Ok(gid) = identity.parse::() { + if !(MIN_SANDBOX_UID..=MAX_SANDBOX_UID).contains(&gid) { + return Err(miette::miette!( + "process group GID must be in range [{MIN_SANDBOX_UID}, {MAX_SANDBOX_UID}]" + )); + } openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) .severity(openshell_ocsf::SeverityId::Informational) @@ -971,8 +1016,7 @@ pub fn validate_sandbox_group(policy: &SandboxPolicy) -> Result<()> { } Ok(None) => { return Err(miette::miette!( - "sandbox group 'sandbox' not found in image; \ - all sandbox images must include a 'sandbox' user and group" + "explicit process group 'sandbox' was not found in the image" )); } Err(e) => { @@ -982,11 +1026,7 @@ pub fn validate_sandbox_group(policy: &SandboxPolicy) -> Result<()> { } else if !identity.is_empty() { match Group::from_name(identity) { Ok(Some(_)) => { - tracing::warn!( - identity, - "non-sandbox group accepted via group entry; \ - consider using a numeric GID for GID-injected images" - ); + tracing::warn!(identity, "named process group accepted via group entry"); } Ok(None) => { return Err(miette::miette!( @@ -1005,6 +1045,34 @@ pub fn validate_sandbox_group(policy: &SandboxPolicy) -> Result<()> { Ok(()) } +#[cfg(unix)] +pub fn validate_sandbox_user_with_identity( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, +) -> Result<()> { + let Some(uid) = resolved_identity.uid() else { + return validate_sandbox_user(policy); + }; + if uid == 0 { + return Err(miette::miette!("process user must not select UID 0")); + } + Ok(()) +} + +#[cfg(unix)] +pub fn validate_sandbox_group_with_identity( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, +) -> Result<()> { + let Some(gid) = resolved_identity.gid() else { + return validate_sandbox_group(policy); + }; + if gid == 0 { + return Err(miette::miette!("process group must not select GID 0")); + } + Ok(()) +} + pub use openshell_policy::{MAX_SANDBOX_UID, MIN_SANDBOX_UID}; /// Prepare a `read_write` path for the sandboxed process. @@ -1162,15 +1230,9 @@ fn rewrite_group_at(path: &Path, gid: &str) -> Result<()> { /// Recursively chown a directory tree to the given UID/GID. /// -/// Symlinks are skipped (not followed) to prevent privilege escalation via -/// malicious container images. The TOCTOU window is not exploitable because -/// no untrusted process is running yet. -/// -/// The root path is chowned unconditionally — EROFS there is a hard error -/// (a read-only `/sandbox` is a misconfiguration). For children, `EROFS` -/// causes the walker to skip that path and its entire subtree — descending -/// into a read-only mount we do not control would be a TOCTOU risk -/// (CWE-367/CWE-59). Siblings of the read-only path are still visited. +/// This retains the Kubernetes/OpenShift workspace reconciliation from before +/// OCI image identity fallback. Symlinks are skipped, and read-only nested +/// mounts are not traversed. #[cfg(unix)] fn chown_sandbox_home(root: &Path, uid: Option, gid: Option) -> Result<()> { let meta = std::fs::symlink_metadata(root).into_diagnostic()?; @@ -1190,8 +1252,6 @@ fn chown_sandbox_home(root: &Path, uid: Option, gid: Option) -> Result Ok(()) } -/// Walk directory children and chown each entry, skipping symlinks and -/// EROFS subtrees. Called after the parent has already been chowned. #[cfg(unix)] fn chown_children( dir: &Path, @@ -1203,12 +1263,15 @@ fn chown_children( Ok(entries) => { for entry in entries { let entry = entry.into_diagnostic()?; - let child = entry.path(); - chown_recursive(&child, uid, gid, do_chown)?; + chown_recursive(&entry.path(), uid, gid, do_chown)?; } } - Err(e) => { - debug!(path = %dir.display(), error = %e, "Cannot list directory during sandbox home chown"); + Err(error) => { + debug!( + path = %dir.display(), + %error, + "Cannot list directory during sandbox home chown" + ); } } Ok(()) @@ -1222,18 +1285,17 @@ fn chown_recursive( do_chown: &impl Fn(&Path, Option, Option) -> nix::Result<()>, ) -> Result<()> { let meta = std::fs::symlink_metadata(path).into_diagnostic()?; - if meta.file_type().is_symlink() { debug!(path = %path.display(), "Skipping symlink during sandbox home chown"); return Ok(()); } - if let Err(e) = do_chown(path, uid, gid) { - if e == nix::errno::Errno::EROFS { + if let Err(error) = do_chown(path, uid, gid) { + if error == nix::errno::Errno::EROFS { debug!(path = %path.display(), "Skipping read-only path during sandbox home chown"); return Ok(()); } - return Err(e).into_diagnostic(); + return Err(error).into_diagnostic(); } if meta.is_dir() { @@ -1253,6 +1315,14 @@ fn chown_recursive( /// UIDs/GIDs (passed directly to `chown` without a passwd lookup). #[cfg(unix)] pub fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { + prepare_filesystem_with_identity(policy, ResolvedProcessIdentity::default()) +} + +#[cfg(unix)] +pub fn prepare_filesystem_with_identity( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, +) -> Result<()> { use nix::unistd::chown; use nix::unistd::{Gid, Uid}; @@ -1271,21 +1341,27 @@ pub fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { } // Resolve UID: numeric values are passed directly; names resolve via passwd. - let uid = match user_name { - Some(name) if name.parse::().is_ok() => { - Some(Uid::from_raw(name.parse().into_diagnostic()?)) - } - Some(name) => User::from_name(name).into_diagnostic()?.map(|u| u.uid), - _ => None, + let uid = match resolved_identity.uid() { + Some(uid) => Some(Uid::from_raw(uid)), + None => match user_name { + Some(name) if name.parse::().is_ok() => { + Some(Uid::from_raw(name.parse().into_diagnostic()?)) + } + Some(name) => User::from_name(name).into_diagnostic()?.map(|u| u.uid), + _ => None, + }, }; // Resolve GID: numeric values are passed directly; names resolve via group. - let gid = match group_name { - Some(name) if name.parse::().is_ok() => { - Some(Gid::from_raw(name.parse().into_diagnostic()?)) - } - Some(name) => Group::from_name(name).into_diagnostic()?.map(|g| g.gid), - _ => None, + let gid = match resolved_identity.gid() { + Some(gid) => Some(Gid::from_raw(gid)), + None => match group_name { + Some(name) if name.parse::().is_ok() => { + Some(Gid::from_raw(name.parse().into_diagnostic()?)) + } + Some(name) => Group::from_name(name).into_diagnostic()?.map(|g| g.gid), + _ => None, + }, }; // Create missing read_write paths and only chown the ones we created. @@ -1301,12 +1377,10 @@ pub fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { } } - // When a driver injects a custom UID/GID via environment variables, the - // /sandbox home directory may already exist with image-default ownership - // (e.g. UID 1000) that differs from the driver-assigned identity. - // Recursively chown /sandbox so the sandbox process can use its home - // directory. - if std::env::var(openshell_core::sandbox_env::SANDBOX_UID).is_ok() { + // Retain the existing Kubernetes/OpenShift behavior for driver-injected + // numeric identities. Docker and Podman clear this variable and do not + // receive identity-specific workspace preparation. + if std::env::var(openshell_core::sandbox_env::SANDBOX_UID).is_ok_and(|uid| !uid.is_empty()) { let sandbox_home = Path::new("/sandbox"); if sandbox_home.exists() { info!(?uid, ?gid, "Chowning /sandbox for driver-injected UID/GID"); @@ -1327,6 +1401,28 @@ pub fn prepare_filesystem(_policy: &SandboxPolicy) -> Result<()> { #[cfg(unix)] #[allow(clippy::similar_names)] pub fn drop_privileges(policy: &SandboxPolicy) -> Result<()> { + drop_privileges_with_identity(policy, ResolvedProcessIdentity::default()) +} + +#[cfg(unix)] +fn should_clear_supplementary_groups( + current_uid: Uid, + target_uid: Uid, + user_name: Option<&str>, + resolved_identity: ResolvedProcessIdentity, +) -> bool { + resolved_identity.uses_oci_user_fallback() + && target_uid != current_uid + && !(user_name.is_some_and(|name| name.parse::().is_err()) + && resolved_identity.uid().is_none()) +} + +#[cfg(unix)] +#[allow(clippy::similar_names)] +pub fn drop_privileges_with_identity( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, +) -> Result<()> { let user_name = match policy.process.run_as_user.as_deref() { Some(name) if !name.is_empty() => Some(name), _ => None, @@ -1338,94 +1434,122 @@ pub fn drop_privileges(policy: &SandboxPolicy) -> Result<()> { // If no user/group is configured and we are running as root, fall back to // "sandbox:sandbox" instead of silently keeping root. This covers the - // local/dev-mode path where policies are loaded from disk and never pass - // through the server-side `ensure_sandbox_process_identity` normalization. + // local/dev-mode path for drivers that provide no identity metadata. // For non-root runtimes, the no-op is safe -- we are already unprivileged. if user_name.is_none() && group_name.is_none() { if nix::unistd::geteuid().is_root() { let mut fallback = policy.clone(); fallback.process.run_as_user = Some("sandbox".into()); fallback.process.run_as_group = Some("sandbox".into()); - return drop_privileges(&fallback); + return drop_privileges_with_identity(&fallback, resolved_identity); } return Ok(()); } // Resolve UID: numeric values are used directly; names resolve via passwd. - let target_uid = match user_name { - Some(name) if name.parse::().is_ok() => Uid::from_raw(name.parse().into_diagnostic()?), - Some(name) => { - User::from_name(name) - .into_diagnostic()? - .ok_or_else(|| miette::miette!("Sandbox user not found: {name}"))? - .uid - } - None => nix::unistd::geteuid(), + let target_uid = match resolved_identity.uid() { + Some(uid) => Uid::from_raw(uid), + None => match user_name { + Some(name) if name.parse::().is_ok() => { + Uid::from_raw(name.parse().into_diagnostic()?) + } + Some(name) => { + User::from_name(name) + .into_diagnostic()? + .ok_or_else(|| miette::miette!("Sandbox user not found: {name}"))? + .uid + } + None => nix::unistd::geteuid(), + }, }; // Resolve group: if a numeric GID is configured use it directly. // Otherwise try name resolution, then fall back to current user's primary group. - let target_gid = match group_name { - Some(name) if name.parse::().is_ok() => Gid::from_raw(name.parse().into_diagnostic()?), - Some(name) => { - Group::from_name(name) - .into_diagnostic()? - .ok_or_else(|| miette::miette!("Sandbox group not found: {name}"))? - .gid - } - None => match target_uid.as_raw() { - 0 => nix::unistd::getegid(), - _ => Group::from_gid( - User::from_uid(target_uid) + let target_gid = match resolved_identity.gid() { + Some(gid) => Gid::from_raw(gid), + None => match group_name { + Some(name) if name.parse::().is_ok() => { + Gid::from_raw(name.parse().into_diagnostic()?) + } + Some(name) => { + Group::from_name(name) .into_diagnostic()? - .ok_or_else(|| miette::miette!("Failed to resolve user from UID {target_uid}"))? - .gid, - ) - .into_diagnostic()? - .map_or_else(nix::unistd::getegid, |g| g.gid), + .ok_or_else(|| miette::miette!("Sandbox group not found: {name}"))? + .gid + } + None => match target_uid.as_raw() { + 0 => nix::unistd::getegid(), + _ => Group::from_gid( + User::from_uid(target_uid) + .into_diagnostic()? + .ok_or_else(|| { + miette::miette!("Failed to resolve user from UID {target_uid}") + })? + .gid, + ) + .into_diagnostic()? + .map_or_else(nix::unistd::getegid, |g| g.gid), + }, }, }; - // Resolve the user record for initgroups only when identity is name-based. - // Numeric UIDs may not have a /etc/passwd entry; skip the lookup rather than - // failing with a spurious "user record not found" error. + // Resolve the name for initgroups only for the existing explicit-policy + // path. OCI-derived users carry a numeric UID from the bounded parser and + // must not be looked up again through NSS. let user_name_is_numeric = user_name.is_some_and(|n| n.parse::().is_ok()); - let user = if user_name.is_some() && !user_name_is_numeric { - Some( - User::from_uid(target_uid) - .into_diagnostic()? - .ok_or_else(|| { - miette::miette!("Failed to resolve user record for UID {target_uid}") - })?, - ) - } else { - None - }; + let initgroups_name = + if user_name.is_some() && !user_name_is_numeric && resolved_identity.uid().is_none() { + Some( + User::from_uid(target_uid) + .into_diagnostic()? + .ok_or_else(|| { + miette::miette!("Failed to resolve user record for UID {target_uid}") + })? + .name, + ) + } else { + None + }; - // Set supplementary groups only when we have a name-based identity. - // Numeric UIDs may not have a passwd entry, so initgroups would fail. - if let Some(ref user) = user - && target_uid != nix::unistd::geteuid() - { - let user_cstr = - CString::new(user.name.clone()).map_err(|_| miette::miette!("Invalid user name"))?; - #[cfg(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - ))] - { - let _ = user_cstr; - } - #[cfg(not(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - )))] - { - nix::unistd::initgroups(user_cstr.as_c_str(), target_gid).into_diagnostic()?; + if target_uid != nix::unistd::geteuid() { + if should_clear_supplementary_groups( + nix::unistd::geteuid(), + target_uid, + user_name, + resolved_identity, + ) { + // OCI-derived users do not have a trustworthy NSS + // supplementary-group source. Clear the root supervisor's + // inherited groups before changing UID/GID. Platform-resolved and + // explicit numeric identities retain their pre-OCI behavior. + #[cfg(not(any( + target_os = "macos", + target_os = "ios", + target_os = "haiku", + target_os = "redox" + )))] + nix::unistd::setgroups(&[]).into_diagnostic()?; + } else if let Some(ref user_name) = initgroups_name { + let user_cstr = CString::new(user_name.as_str()) + .map_err(|_| miette::miette!("Invalid user name"))?; + #[cfg(any( + target_os = "macos", + target_os = "ios", + target_os = "haiku", + target_os = "redox" + ))] + { + let _ = user_cstr; + } + #[cfg(not(any( + target_os = "macos", + target_os = "ios", + target_os = "haiku", + target_os = "redox" + )))] + { + nix::unistd::initgroups(user_cstr.as_c_str(), target_gid).into_diagnostic()?; + } } } @@ -1564,6 +1688,110 @@ mod tests { ); } + #[test] + #[cfg(unix)] + fn explicit_identity_rejects_non_root_system_ids() { + let policy = policy_with_process(ProcessPolicy { + run_as_user: Some("101".into()), + run_as_group: Some("102".into()), + }); + + assert!(validate_sandbox_user(&policy).is_err()); + assert!(validate_sandbox_group(&policy).is_err()); + } + + #[test] + #[cfg(unix)] + fn resolved_oci_identity_accepts_non_root_system_ids() { + let policy = policy_with_process(ProcessPolicy { + run_as_user: Some("app".into()), + run_as_group: Some("staff".into()), + }); + let resolved = ResolvedProcessIdentity::new(Some(101), Some(102)); + + assert!(validate_sandbox_user_with_identity(&policy, resolved).is_ok()); + assert!(validate_sandbox_group_with_identity(&policy, resolved).is_ok()); + } + + #[test] + #[cfg(unix)] + fn completed_runtime_identity_rejects_numeric_root() { + let root_user = policy_with_process(ProcessPolicy { + run_as_user: Some("0".into()), + run_as_group: Some("102".into()), + }); + let root_group = policy_with_process(ProcessPolicy { + run_as_user: Some("101".into()), + run_as_group: Some("0".into()), + }); + + assert!(validate_sandbox_user(&root_user).is_err()); + assert!(validate_sandbox_group(&root_group).is_err()); + } + + #[test] + #[cfg(unix)] + fn resolved_oci_components_do_not_repeat_nss_validation() { + let policy = policy_with_process(ProcessPolicy { + run_as_user: Some("__oci_name_not_in_host_nss__".into()), + run_as_group: Some("__oci_group_not_in_host_nss__".into()), + }); + let resolved = ResolvedProcessIdentity::new(Some(1234), Some(1235)); + + assert!(validate_sandbox_user_with_identity(&policy, resolved).is_ok()); + assert!(validate_sandbox_group_with_identity(&policy, resolved).is_ok()); + } + + #[test] + #[cfg(unix)] + fn explicit_policy_components_keep_existing_validation_path() { + let policy = policy_with_process(ProcessPolicy { + run_as_user: Some("__explicit_name_not_in_host_nss__".into()), + run_as_group: Some("__oci_group_not_in_host_nss__".into()), + }); + let resolved = ResolvedProcessIdentity::new(None, Some(1235)); + + assert!(validate_sandbox_user_with_identity(&policy, resolved).is_err()); + assert!(validate_sandbox_group_with_identity(&policy, resolved).is_ok()); + } + + #[test] + #[cfg(unix)] + fn only_oci_numeric_user_paths_clear_supplementary_groups_before_uid_drop() { + let current_uid = Uid::from_raw(0); + let target_uid = Uid::from_raw(1234); + + assert!(!should_clear_supplementary_groups( + current_uid, + target_uid, + Some("1234"), + ResolvedProcessIdentity::default(), + )); + assert!(should_clear_supplementary_groups( + current_uid, + target_uid, + Some("1234"), + ResolvedProcessIdentity::new(None, Some(1235)), + )); + } + + #[test] + #[cfg(unix)] + fn supplementary_group_clearing_preserves_explicit_named_user_behavior() { + assert!(!should_clear_supplementary_groups( + Uid::from_raw(0), + Uid::from_raw(1234), + Some("app"), + ResolvedProcessIdentity::default(), + )); + assert!(!should_clear_supplementary_groups( + Uid::from_raw(1234), + Uid::from_raw(1234), + Some("1234"), + ResolvedProcessIdentity::new(None, Some(1235)), + )); + } + #[test] fn full_enforcement_uses_privileged_setup_and_child_sandbox() { assert!(ProcessEnforcementMode::Full.uses_privileged_process_setup()); @@ -2088,7 +2316,6 @@ mod tests { let expected_uid = nix::unistd::geteuid(); let expected_gid = nix::unistd::getegid(); - chown_sandbox_home(&root, Some(expected_uid), Some(expected_gid)).unwrap(); for path in &[ @@ -2098,18 +2325,8 @@ mod tests { root.join("subdir").join("nested.txt"), ] { let meta = std::fs::metadata(path).unwrap(); - assert_eq!( - meta.uid(), - expected_uid.as_raw(), - "uid mismatch for {}", - path.display() - ); - assert_eq!( - meta.gid(), - expected_gid.as_raw(), - "gid mismatch for {}", - path.display() - ); + assert_eq!(meta.uid(), expected_uid.as_raw()); + assert_eq!(meta.gid(), expected_gid.as_raw()); } } @@ -2153,7 +2370,7 @@ mod tests { Some(nix::unistd::geteuid()), Some(nix::unistd::getegid()), ) - .expect("should skip symlink children without error"); + .expect("symlink children should be skipped"); } #[cfg(unix)] @@ -2168,31 +2385,32 @@ mod tests { let readonly_dir = root.join("ro-mount"); std::fs::create_dir(&readonly_dir).unwrap(); std::fs::write(readonly_dir.join("child-under-ro.txt"), "data").unwrap(); - std::fs::write(root.join("writable-sibling.txt"), "data").unwrap(); - let uid = Some(nix::unistd::geteuid()); - let gid = Some(nix::unistd::getegid()); - - let chowned: Arc>> = Arc::new(Mutex::new(Vec::new())); - let chowned_ref = Arc::clone(&chowned); - - let readonly_dir_clone = readonly_dir.clone(); + let chowned = Arc::new(Mutex::new(Vec::new())); + let observed = Arc::clone(&chowned); + let readonly_dir_for_chown = readonly_dir.clone(); let fake_chown = move |path: &Path, _uid: Option, _gid: Option| -> nix::Result<()> { - if path == readonly_dir_clone { + if path == readonly_dir_for_chown { return Err(nix::errno::Errno::EROFS); } - chowned_ref.lock().unwrap().push(path.to_path_buf()); + observed.lock().unwrap().push(path.to_path_buf()); Ok(()) }; - chown_children(&root, uid, gid, &fake_chown).expect("EROFS should be handled gracefully"); + chown_children( + &root, + Some(nix::unistd::geteuid()), + Some(nix::unistd::getegid()), + &fake_chown, + ) + .expect("read-only subtree should be skipped"); let chowned = chowned.lock().unwrap(); assert!( !chowned.contains(&readonly_dir.join("child-under-ro.txt")), - "children under EROFS directory must NOT be descended into" + "children under EROFS directory must not be traversed" ); assert!( chowned.contains(&root.join("writable-sibling.txt")), @@ -2206,39 +2424,19 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let root = dir.path().join("sandbox"); std::fs::create_dir(&root).unwrap(); - - let uid = Some(nix::unistd::geteuid()); - let gid = Some(nix::unistd::getegid()); - let fake_chown = |_path: &Path, _uid: Option, _gid: Option| -> nix::Result<()> { Err(nix::errno::Errno::EPERM) }; - let result = chown_recursive(&root, uid, gid, &fake_chown); + let result = chown_recursive( + &root, + Some(nix::unistd::geteuid()), + Some(nix::unistd::getegid()), + &fake_chown, + ); assert!(result.is_err(), "non-EROFS errors should propagate"); } - #[cfg(unix)] - #[test] - fn chown_children_skips_all_erofs_children_gracefully() { - let dir = tempfile::tempdir().unwrap(); - let root = dir.path().join("sandbox"); - std::fs::create_dir(&root).unwrap(); - std::fs::create_dir(root.join("a")).unwrap(); - std::fs::write(root.join("b.txt"), "data").unwrap(); - - let uid = Some(nix::unistd::geteuid()); - let gid = Some(nix::unistd::getegid()); - - let always_erofs = |_path: &Path, - _uid: Option, - _gid: Option| - -> nix::Result<()> { Err(nix::errno::Errno::EROFS) }; - - chown_children(&root, uid, gid, &always_erofs) - .expect("EROFS on all children should be skipped gracefully"); - } - #[cfg(unix)] #[test] fn rewrite_passwd_modifies_existing_sandbox_entry() { diff --git a/crates/openshell-supervisor-process/src/run.rs b/crates/openshell-supervisor-process/src/run.rs index ba6d446dea..a5ff0456c9 100644 --- a/crates/openshell-supervisor-process/src/run.rs +++ b/crates/openshell-supervisor-process/src/run.rs @@ -34,7 +34,9 @@ use openshell_core::denial::DenialEvent; #[cfg(target_os = "linux")] use crate::managed_children; -use crate::process::{ProcessEnforcementMode, ProcessHandle, ProcessStatus}; +use crate::process::{ + ProcessEnforcementMode, ProcessHandle, ProcessStatus, ResolvedProcessIdentity, +}; fn ocsf_ctx() -> &'static openshell_ocsf::SandboxContext { openshell_ocsf::ctx::ctx() @@ -59,6 +61,7 @@ pub async fn run_process( ssh_socket_path: Option, shared_ssh_socket: bool, policy: &SandboxPolicy, + resolved_process_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, entrypoint_pid: Arc, entrypoint_started_tx: Option>, @@ -72,21 +75,19 @@ pub async fn run_process( >, #[cfg(target_os = "linux")] bypass_activity_tx: Option, ) -> Result { - // When a driver injects a custom UID/GID, update /etc/passwd and - // /etc/group so the "sandbox" entry matches. Must run before - // validate_sandbox_user so passwd lookups see the correct identity. + // Platform drivers with a resolved numeric UID/GID retain the legacy + // account-file update. OCI-image identity leaves those environment values + // empty, so the image's account files remain unchanged. #[cfg(unix)] if enforcement_mode.uses_privileged_process_setup() { crate::process::update_sandbox_passwd_entries()?; } - // Validate that the sandbox user exists in the image. All sandbox images - // must include a "sandbox" user for privilege dropping; failing fast here - // beats silently running children as root. + // Validate the completed process identity before exposing a child. #[cfg(unix)] if enforcement_mode.uses_privileged_process_setup() { - crate::process::validate_sandbox_user(policy)?; - crate::process::validate_sandbox_group(policy)?; + crate::process::validate_sandbox_user_with_identity(policy, resolved_process_identity)?; + crate::process::validate_sandbox_group_with_identity(policy, resolved_process_identity)?; } // Create read_write directories and chown newly-created ones to the @@ -94,7 +95,7 @@ pub async fn run_process( // is forked so the workload sees writable paths it owns. #[cfg(unix)] if enforcement_mode.uses_privileged_process_setup() { - crate::process::prepare_filesystem(policy)?; + crate::process::prepare_filesystem_with_identity(policy, resolved_process_identity)?; } // Eagerly fetch initial settings and install the agent skill if the @@ -248,6 +249,7 @@ pub async fn run_process( ca_paths, provider_credentials_clone, user_env_clone, + resolved_process_identity, enforcement_mode, shared_ssh_socket, ) @@ -320,6 +322,7 @@ pub async fn run_process( workdir, interactive, policy, + resolved_process_identity, enforcement_mode, netns, ca_file_paths.as_ref(), @@ -333,6 +336,7 @@ pub async fn run_process( workdir, interactive, policy, + resolved_process_identity, enforcement_mode, ca_file_paths.as_ref(), &provider_env, diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index f5a3ee0793..7fa6482a99 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -6,7 +6,10 @@ use crate::child_env; #[cfg(target_os = "linux")] use crate::managed_children; -use crate::process::{ProcessEnforcementMode, drop_privileges, is_supervisor_only_env_var}; +use crate::process::{ + ProcessEnforcementMode, ResolvedProcessIdentity, drop_privileges_with_identity, + is_supervisor_only_env_var, +}; use crate::sandbox; use miette::{IntoDiagnostic, Result}; use nix::pty::{Winsize, openpty}; @@ -114,6 +117,7 @@ pub async fn run_ssh_server( ca_file_paths: Option<(PathBuf, PathBuf)>, provider_credentials: ProviderCredentialState, user_environment: HashMap, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, shared_socket: bool, ) -> Result<()> { @@ -158,6 +162,7 @@ pub async fn run_ssh_server( ca_paths, provider_credentials, user_environment, + resolved_identity, enforcement_mode, ) .await @@ -186,6 +191,7 @@ async fn handle_connection( ca_file_paths: Option>, provider_credentials: ProviderCredentialState, user_environment: HashMap, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ) -> Result<()> { // Access is gated by the Unix-socket filesystem permissions (root-only), @@ -210,6 +216,7 @@ async fn handle_connection( ca_file_paths, provider_credentials, user_environment, + resolved_identity, enforcement_mode, ); russh::server::run_stream(config, stream, handler) @@ -239,6 +246,7 @@ struct SshHandler { ca_file_paths: Option>, provider_credentials: ProviderCredentialState, user_environment: HashMap, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, channels: HashMap, } @@ -253,6 +261,7 @@ impl SshHandler { ca_file_paths: Option>, provider_credentials: ProviderCredentialState, user_environment: HashMap, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ) -> Self { Self { @@ -263,6 +272,7 @@ impl SshHandler { ca_file_paths, provider_credentials, user_environment, + resolved_identity, enforcement_mode, channels: HashMap::new(), } @@ -487,6 +497,7 @@ impl russh::server::Handler for SshHandler { self.ca_file_paths.clone(), &self.provider_credentials.child_env_with_gcp_resolved(), &self.user_environment, + self.resolved_identity, self.enforcement_mode, )?; let state = self.channels.get_mut(&channel).ok_or_else(|| { @@ -584,6 +595,7 @@ impl SshHandler { self.ca_file_paths.clone(), &provider_env, &self.user_environment, + self.resolved_identity, self.enforcement_mode, )?; state.pty_master = Some(pty_master); @@ -603,6 +615,7 @@ impl SshHandler { self.ca_file_paths.clone(), &provider_env, &self.user_environment, + self.resolved_identity, self.enforcement_mode, )?; state.input_sender = Some(input_sender); @@ -770,6 +783,7 @@ fn spawn_pty_shell( ca_file_paths: Option>, provider_env: &HashMap, user_environment: &HashMap, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ) -> anyhow::Result<(std::fs::File, mpsc::Sender>)> { let winsize = Winsize { @@ -847,6 +861,7 @@ fn spawn_pty_shell( workdir.clone(), slave_fd, netns_fd, + resolved_identity, enforcement_mode, #[cfg(target_os = "linux")] prepared_sandbox, @@ -940,6 +955,7 @@ fn spawn_pipe_exec( ca_file_paths: Option>, provider_env: &HashMap, user_environment: &HashMap, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ) -> anyhow::Result>> { let mut cmd = command.map_or_else( @@ -1000,6 +1016,7 @@ fn spawn_pipe_exec( policy.clone(), workdir.clone(), netns_fd, + resolved_identity, enforcement_mode, #[cfg(target_os = "linux")] prepared_sandbox, @@ -1101,7 +1118,8 @@ mod unsafe_pty { #[cfg(not(target_os = "linux"))] use super::sandbox; use super::{ - Command, ProcessEnforcementMode, RawFd, SandboxPolicy, Winsize, drop_privileges, setsid, + Command, ProcessEnforcementMode, RawFd, ResolvedProcessIdentity, SandboxPolicy, Winsize, + drop_privileges_with_identity, setsid, }; #[cfg(unix)] use std::os::unix::process::CommandExt; @@ -1128,6 +1146,7 @@ mod unsafe_pty { } #[allow(unsafe_code)] + #[allow(clippy::too_many_arguments)] #[cfg_attr( not(target_os = "linux"), allow( @@ -1141,6 +1160,7 @@ mod unsafe_pty { _workdir: Option, slave_fd: RawFd, netns_fd: Option, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, #[cfg(target_os = "linux")] prepared: Option, ) -> anyhow::Result<()> { @@ -1164,6 +1184,7 @@ mod unsafe_pty { enter_netns_and_sandbox( netns_fd, &policy, + resolved_identity, enforcement_mode, #[cfg(target_os = "linux")] supervisor_identity_mount, @@ -1191,6 +1212,7 @@ mod unsafe_pty { policy: SandboxPolicy, _workdir: Option, netns_fd: Option, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, #[cfg(target_os = "linux")] prepared: Option, ) -> anyhow::Result<()> { @@ -1209,6 +1231,7 @@ mod unsafe_pty { enter_netns_and_sandbox( netns_fd, &policy, + resolved_identity, enforcement_mode, #[cfg(target_os = "linux")] supervisor_identity_mount, @@ -1223,6 +1246,7 @@ mod unsafe_pty { fn enter_netns_and_sandbox( netns_fd: Option, policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, #[cfg(target_os = "linux")] supervisor_identity_mount: Option< &crate::process::SupervisorIdentityMountNamespace, @@ -1253,7 +1277,8 @@ mod unsafe_pty { // Drop privileges. initgroups/setgid/setuid need /etc/group and // /etc/passwd which would be blocked if Landlock were already enforced. if enforcement_mode.uses_privileged_process_setup() { - drop_privileges(policy).map_err(|err| std::io::Error::other(err.to_string()))?; + drop_privileges_with_identity(policy, resolved_identity) + .map_err(|err| std::io::Error::other(err.to_string()))?; } crate::process::harden_child_process() .map_err(|err| std::io::Error::other(err.to_string()))?; @@ -1795,6 +1820,7 @@ mod tests { policy, None, None, // no netns fd + ResolvedProcessIdentity::default(), ProcessEnforcementMode::Full, #[cfg(target_os = "linux")] Some( @@ -1827,4 +1853,60 @@ mod tests { "echo output should contain 'drop-privileges-ok'" ); } + + /// SSH pre-exec uses the numeric identity resolved from OCI metadata rather + /// than looking the preserved declaration up through host NSS. + #[cfg(unix)] + #[test] + fn pre_exec_uses_resolved_oci_identity() { + use openshell_core::policy::{ + FilesystemPolicy, LandlockPolicy, NetworkPolicy, ProcessPolicy, SandboxPolicy, + }; + + if rustix::process::geteuid().is_root() { + return; + } + + let policy = SandboxPolicy { + version: 0, + filesystem: FilesystemPolicy::default(), + network: NetworkPolicy::default(), + landlock: LandlockPolicy::default(), + process: ProcessPolicy { + run_as_user: Some("__oci_user_not_in_host_nss__".into()), + run_as_group: Some("__oci_group_not_in_host_nss__".into()), + }, + }; + let resolved = ResolvedProcessIdentity::new( + Some(rustix::process::geteuid().as_raw()), + Some(rustix::process::getegid().as_raw()), + ); + + let mut cmd = Command::new("echo"); + cmd.arg("resolved-identity-ok"); + cmd.stdout(Stdio::piped()); + + unsafe_pty::install_pre_exec_no_pty( + &mut cmd, + policy, + None, + None, + resolved, + ProcessEnforcementMode::Full, + #[cfg(target_os = "linux")] + None, + ) + .expect("install pre_exec should succeed"); + + let output = cmd + .spawn() + .expect("spawn should use resolved numeric identity") + .wait_with_output() + .expect("wait should succeed"); + assert!(output.status.success()); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + "resolved-identity-ok" + ); + } } diff --git a/docs/get-started/tutorials/first-network-policy.mdx b/docs/get-started/tutorials/first-network-policy.mdx index 5071f3e2d4..178bd2f095 100644 --- a/docs/get-started/tutorials/first-network-policy.mdx +++ b/docs/get-started/tutorials/first-network-policy.mdx @@ -100,9 +100,6 @@ filesystem_policy: read_write: [/sandbox, /tmp, /dev/null] landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox network_policies: github_api: @@ -117,7 +114,7 @@ network_policies: - { path: /usr/bin/curl } ``` -The `filesystem_policy`, `landlock`, and `process` sections preserve the default sandbox settings. This is required because `policy set` replaces the entire policy. The `network_policies` section is the key part: `curl` can make GET, HEAD, and OPTIONS requests to `api.github.com` over HTTPS. Everything else is denied. The proxy auto-detects TLS on HTTPS endpoints and terminates it to inspect each HTTP request and enforce the `read-only` access preset at the method level. +The `filesystem_policy` and `landlock` sections preserve the default sandbox settings, while process identity is omitted so the active compute driver can select it. These sections are required because `policy set` replaces the entire policy. The `network_policies` section is the key part: `curl` can make GET, HEAD, and OPTIONS requests to `api.github.com` over HTTPS. Everything else is denied. The proxy auto-detects TLS on HTTPS endpoints and terminates it to inspect each HTTP request and enforce the `read-only` access preset at the method level. Apply it: diff --git a/docs/get-started/tutorials/github-sandbox.mdx b/docs/get-started/tutorials/github-sandbox.mdx index 7c76d4e411..0b11b39345 100644 --- a/docs/get-started/tutorials/github-sandbox.mdx +++ b/docs/get-started/tutorials/github-sandbox.mdx @@ -145,7 +145,7 @@ In terminal 2, paste the deny reason from the previous step into your coding age ```md title="Prompt" wordWrap showLineNumbers={false} Based on the following deny reasons, recommend a sandbox policy update that allows GitHub pushes to `https://github.com//`, and save to `/tmp/sandbox-policy-update.yaml`: -The `filesystem_policy`, `landlock`, and `process` sections are static. They are read once at sandbox creation and cannot be changed by a hot-reload. They are included here for completeness so the file is self-contained, but only the `network_policies` section takes effect when you apply this to a running sandbox. +The `filesystem_policy` and `landlock` sections are static. They are read once at sandbox creation and cannot be changed by a hot reload. They are included here for completeness so the file is self-contained. Process identity is omitted so the active compute driver can select it, and only the `network_policies` section takes effect when you apply this to a running sandbox. ``` The following steps outline the expected process done by the agent: @@ -162,7 +162,7 @@ Refer to the following policy example to compare with the generated policy befor The following YAML shows a complete policy that extends the [default policy](/reference/default-policy) with GitHub access for a single repository. Replace `` with your GitHub organization or username and `` with your repository name. -The `filesystem_policy`, `landlock`, and `process` sections are static. OpenShell reads them at sandbox creation, and a hot reload cannot change them. They are included here for completeness so the file is self-contained, but only the `network_policies` section takes effect when you apply this to a running sandbox. +The `filesystem_policy` and `landlock` sections are static. OpenShell reads them at sandbox creation, and a hot reload cannot change them. They are included here for completeness so the file is self-contained. Process identity is omitted so the active compute driver can select it, and only the `network_policies` section takes effect when you apply this to a running sandbox. ```yaml version: 1 @@ -187,10 +187,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - # ── Dynamic (hot-reloadable) ───────────────────────────────────── network_policies: diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index d585517d13..9c4e877eaf 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -119,17 +119,24 @@ Sets the OS-level identity for the agent process inside the sandbox. | Field | Type | Required | Description | |---|---|---|---| -| `run_as_user` | string | No | The user name or UID the agent process runs as. Default: `sandbox`. | -| `run_as_group` | string | No | The group name or GID the agent process runs as. Default: `sandbox`. | +| `run_as_user` | string | No | Overrides the user name or UID selected by the compute driver. Docker and Podman fall back to the image's OCI `USER`. | +| `run_as_group` | string | No | Overrides the group name or GID selected by the compute driver. Docker and Podman fall back to the image's OCI `USER`. | -**Validation constraint:** Neither `run_as_user` nor `run_as_group` can be set to `root` or `0`. Policies that request root process identity are rejected at creation or update time. +**Validation constraint:** An explicit policy value must be `sandbox` or a +numeric UID/GID in the allowed sandbox range. Docker and Podman may select +other named identities or non-root system IDs only through OCI `USER` +fallback. Root identities are always rejected. + +Omission is preserved independently for each field. For example, setting only +`run_as_user` keeps that explicit user while allowing the active driver to +select the group. Example: ```yaml showLineNumbers={false} process: - run_as_user: sandbox - run_as_group: sandbox + run_as_user: "1500" + run_as_group: "1500" ``` ## Network Policies diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index a616823eea..d260cb5257 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -415,7 +415,27 @@ image. ## Sandbox User Identity -OpenShell accepts both the hardcoded username `"sandbox"` and numeric UIDs in `[1000, 2_000_000_000]` for the supervisor's process identity (the policy's `run_as_user` field). The driver resolves the UID at sandbox creation time and passes it to the supervisor via environment variables. +The policy can set `process.run_as_user` and `process.run_as_group` +independently. Each explicit field wins. The active compute driver supplies the +identity for omitted fields. + +### Docker / Podman + +Docker and Podman inspect the final image and use its OCI `USER` declaration as +a per-field fallback. Supported forms include `app`, `app:staff`, a numeric UID +whose passwd entry supplies its primary GID, and an accountless numeric pair +such as `1234:1235`. + +The driver pins container creation to the immutable image ID it inspected. The +supervisor validates any required names inside that image and preserves the +declared name or numeric components for both direct and SSH children. When +`USER` omits the group, the supervisor uses the user's numeric primary GID. It +does not modify `/etc/passwd` or `/etc/group`. + +Sandbox creation fails before readiness if a required `USER` component is +missing, malformed, unknown, ambiguous, or resolves to UID/GID 0. An image +without `USER` therefore works only when policy explicitly provides both +identity fields. ### Kubernetes / OpenShift @@ -438,4 +458,10 @@ The VM driver injects the sandbox UID into the rootfs guest's `/etc/passwd`, `/e ### Custom Images -Custom sandbox images no longer need a baked-in `"sandbox"` user. If your image requires a passwd entry for tools like `sudo` or `ssh`, add one manually (e.g. `RUN useradd -m -u 1500 deploy`). The supervisor resolves the numeric UID directly via `setuid()` without needing `/etc/passwd`. +Docker and Podman custom images do not need a baked-in `"sandbox"` user. Declare +a non-root OCI `USER`, or set both process identity fields explicitly in policy. +Named image users require matching account entries; a numeric `UID:GID` pair +does not. OpenShell continues to use `/sandbox` as the workspace; it does not +adopt the image's OCI working directory. Until OCI working-directory support is +added, custom images must create `/sandbox` and make it writable by the selected +identity. diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 990f260cea..8eee13bee5 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -26,10 +26,10 @@ filesystem_policy: landlock: compatibility: best_effort -# Static: Unprivileged user/group the agent process runs as. -process: - run_as_user: sandbox - run_as_group: sandbox +# Static, optional: override the identity selected by the compute driver. +# process: +# run_as_user: "1500" +# run_as_group: "1500" # Dynamic: hot-reloadable. Named blocks of endpoints + binaries allowed to reach them. network_policies: @@ -68,7 +68,7 @@ Raw streams are connection-scoped and outside L7 live-reload guarantees. This in |---|---|---| | `filesystem_policy` | Static | Controls which directories the agent can access on disk. Paths are split into `read_only` and `read_write` lists. Any path not listed in either list is inaccessible. Set `include_workdir: true` to automatically add the agent's working directory to `read_write`. [Landlock LSM](https://docs.kernel.org/security/landlock.html) enforces these restrictions at the kernel level. | | `landlock` | Static | Configures Landlock LSM enforcement behavior. Set `compatibility` to `best_effort` (skip individual inaccessible paths while applying remaining rules) or `hard_requirement` (fail if any path is inaccessible or the required kernel ABI is unavailable). Refer to the [Policy Schema Reference](/reference/policy-schema#landlock) for the full behavior table. | -| `process` | Static | Sets the OS-level identity for the agent process. `run_as_user` and `run_as_group` default to `sandbox`. Root (`root` or `0`) is rejected. The agent also runs with seccomp filters that block dangerous system calls. | +| `process` | Static | Optionally overrides the OS-level identity for the agent process. Explicit values must be `sandbox` or numeric UID/GID values in the allowed sandbox range. Docker and Podman may use named identities or non-root system IDs only through per-field OCI `USER` fallback; Kubernetes uses its platform-selected numeric identity. Root identities are always rejected. The agent also runs with seccomp filters that block dangerous system calls. | | `network_policies` | Dynamic | Controls network access for ordinary outbound traffic from the sandbox. Each block has a name, a list of endpoints (host, port, protocol, and optional rules), and a list of binaries allowed to use those endpoints.
Every outbound connection except `https://inference.local` goes through the proxy, which queries the [policy engine](/about/how-it-works#core-components) with the destination and calling binary. A connection is allowed only when both match an entry in the same policy block.
For endpoints with `protocol: rest`, the proxy auto-detects TLS and terminates it so each HTTP request can be checked against that endpoint's `rules` (method and path). For endpoints with `protocol: websocket`, the proxy validates the RFC 6455 upgrade and evaluates `GET` rules for the handshake plus either `WEBSOCKET_TEXT` rules for raw client text messages or GraphQL operation rules for GraphQL-over-WebSocket messages. Set `websocket_credential_rewrite: true` only when a WebSocket or REST compatibility endpoint must keep placeholder credentials in sandbox-owned text frames and resolve them at the OpenShell relay boundary.
Endpoints without `protocol` allow the TCP stream through without inspecting payloads.
If no endpoint matches, the connection is denied. Configure managed inference separately through [Inference Routing](/sandboxes/inference-routing). | | `network_middlewares` | Dynamic | Declares keyed HTTP request middleware configs. After network and L7 policy admit a request, OpenShell matches each config's host selectors independently and runs matching entries by their unique ascending `order` before credential injection. | diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index b63883c2b8..0ac0d5528f 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -201,10 +201,10 @@ The sandbox process runs as a non-root user after explicit privilege dropping. | Aspect | Detail | |---|---| -| Default | `run_as_user: sandbox`, `run_as_group: sandbox`. The supervisor calls `setuid()`/`setgid()` with post-condition verification, disables core dumps with `RLIMIT_CORE=0`, and on Linux sets `PR_SET_DUMPABLE=0`. | -| What you can change | Set `run_as_user` and `run_as_group` in the `process` section. Validation rejects root (`root` or `0`). | +| Default | The compute driver selects a non-root identity. Docker and Podman use the image's OCI `USER` as a per-field fallback. The supervisor calls `setuid()`/`setgid()` with post-condition verification, disables core dumps with `RLIMIT_CORE=0`, and on Linux sets `PR_SET_DUMPABLE=0`. | +| What you can change | Set either or both `run_as_user` and `run_as_group` fields in the `process` section. Each explicit field takes precedence and must be `sandbox` or a numeric UID/GID value in the allowed sandbox range. Docker and Podman may use named identities or non-root system IDs only through OCI `USER` fallback. Root identities are always rejected. | | Risk if relaxed | Running as a higher-privilege user increases the impact of container escape vulnerabilities. | -| Recommendation | Keep the `sandbox` user. Do not attempt to set root. | +| Recommendation | Use a dedicated non-root image identity or explicit numeric policy identity. Do not attempt to set root. | ### Seccomp Filters diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index b36a32203f..466a97c573 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -67,6 +67,11 @@ name = "podman_corporate_proxy" path = "tests/podman_corporate_proxy.rs" required-features = ["e2e-podman"] +[[test]] +name = "podman_oci_identity" +path = "tests/podman_oci_identity.rs" +required-features = ["e2e-podman"] + [[test]] name = "vm_gateway_resume" path = "tests/vm_gateway_resume.rs" diff --git a/e2e/rust/tests/custom_image.rs b/e2e/rust/tests/custom_image.rs index fa905bbf19..4cda100dfe 100644 --- a/e2e/rust/tests/custom_image.rs +++ b/e2e/rust/tests/custom_image.rs @@ -1,13 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -#![cfg(feature = "e2e")] +#![cfg(feature = "e2e-local-container-driver")] -//! E2E test: build a custom container image and run a sandbox with it. +//! E2E test: build custom container images and run sandboxes with them. //! //! Prerequisites: -//! - A running Docker-backed openshell gateway (`mise run gateway:docker`) -//! - Docker daemon running (for image build) +//! - A running Docker- or Podman-backed openshell gateway +//! - The matching container runtime running (for image builds) //! - The `openshell` binary (built automatically from the workspace) use std::io::Write; @@ -21,24 +21,30 @@ const DOCKERFILE_CONTENT: &str = r#"FROM public.ecr.aws/docker/library/python:3. RUN apt-get update && apt-get install -y --no-install-recommends iproute2 \ && rm -rf /var/lib/apt/lists/* -# Create the sandbox user/group so the supervisor can switch to it. -# Use a high UID range to avoid conflicts with host users when running without -# user namespace remapping (UID in container = UID on host). -RUN groupadd -g 1000660000 sandbox && \ - useradd -m -u 1000660000 -g sandbox sandbox +RUN groupadd -g 1235 appstaff && \ + useradd -m -u 1234 -g appstaff app # Write a marker file so we can verify this is our custom image. # Place under /etc (Landlock baseline read-only path) so the sandbox # can read it when filesystem restrictions are properly enforced. RUN echo "custom-image-e2e-marker" > /etc/marker.txt +USER app +CMD ["sleep", "infinity"] +"#; + +const NUMERIC_DOCKERFILE_CONTENT: &str = r#"FROM public.ecr.aws/docker/library/python:3.13-slim + +RUN apt-get update && apt-get install -y --no-install-recommends iproute2 \ + && rm -rf /var/lib/apt/lists/* + +USER 2345:2346 CMD ["sleep", "infinity"] "#; const MARKER: &str = "custom-image-e2e-marker"; -/// Build a custom Docker image from a Dockerfile and verify that a sandbox -/// created from it contains the expected marker file. +/// Direct and SSH children use the same named OCI identity. #[tokio::test] async fn sandbox_from_custom_dockerfile() { // Step 1: Write a temporary Dockerfile. @@ -52,10 +58,17 @@ async fn sandbox_from_custom_dockerfile() { // Step 2: Create a sandbox from the Dockerfile. let dockerfile_str = dockerfile_path.to_str().expect("Dockerfile path is UTF-8"); - let mut guard = - SandboxGuard::create(&["--from", dockerfile_str, "--", "cat", "/etc/marker.txt"]) - .await - .expect("sandbox create from Dockerfile"); + let mut guard = SandboxGuard::create_keep_with_args( + &["--from", dockerfile_str, "--no-tty"], + &[ + "sh", + "-c", + "set -eu; id -u; id -g; cat /etc/marker.txt; echo Ready; sleep infinity", + ], + "Ready", + ) + .await + .expect("sandbox create from Dockerfile"); // Step 3: Verify the marker file content appears in the output. let clean_output = strip_ansi(&guard.create_output); @@ -63,7 +76,50 @@ async fn sandbox_from_custom_dockerfile() { clean_output.contains(MARKER), "expected marker '{MARKER}' in sandbox output:\n{clean_output}" ); + assert!( + clean_output.contains("1234") && clean_output.contains("1235"), + "expected named OCI identity 1234:1235 in sandbox output:\n{clean_output}" + ); + + let ssh_output = guard + .exec(&[ + "sh", + "-c", + "set -eu; test \"$(id -u):$(id -g)\" = 1234:1235; echo ssh-identity-ok", + ]) + .await + .expect("SSH child should use OCI identity"); + assert!( + ssh_output.contains("ssh-identity-ok"), + "expected SSH identity marker:\n{ssh_output}" + ); // Explicit cleanup (also happens in Drop, but explicit is clearer in tests). guard.cleanup().await; } + +/// A numeric OCI user/group pair works without passwd or group entries. +#[tokio::test] +async fn sandbox_from_passwd_less_numeric_oci_user() { + let tmpdir = tempfile::tempdir().expect("create tmpdir"); + let dockerfile_path = tmpdir.path().join("Dockerfile"); + { + let mut f = std::fs::File::create(&dockerfile_path).expect("create Dockerfile"); + f.write_all(NUMERIC_DOCKERFILE_CONTENT.as_bytes()) + .expect("write Dockerfile"); + } + + let dockerfile_str = dockerfile_path.to_str().expect("Dockerfile path is UTF-8"); + let mut guard = + SandboxGuard::create(&["--from", dockerfile_str, "--", "sh", "-c", "id -u; id -g"]) + .await + .expect("sandbox create from numeric OCI Dockerfile"); + + let clean_output = strip_ansi(&guard.create_output); + assert!( + clean_output.contains("2345") && clean_output.contains("2346"), + "expected numeric OCI identity 2345:2346 in sandbox output:\n{clean_output}" + ); + + guard.cleanup().await; +} diff --git a/e2e/rust/tests/live_policy_update.rs b/e2e/rust/tests/live_policy_update.rs index 423b260946..7a1e12923a 100644 --- a/e2e/rust/tests/live_policy_update.rs +++ b/e2e/rust/tests/live_policy_update.rs @@ -103,10 +103,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - network_policies: {network_rules}" ); @@ -141,10 +137,6 @@ filesystem_policy: landlock: compatibility: best_effort - -process: - run_as_user: sandbox - run_as_group: sandbox "; file.write_all(policy.as_bytes()) @@ -253,8 +245,8 @@ fn list_output_contains_version(output: &str, version: u32) -> bool { /// Test the full live policy update lifecycle: /// -/// 1. Create sandbox with `--keep` -/// 2. Set policy A, verify initial version >= 1 +/// 1. Create sandbox with policy A and `--keep` +/// 2. Verify initial version >= 1 /// 3. Push same policy A -> version unchanged (idempotent) /// 4. Push policy B (adds example.com) with `--wait` -> new version /// 5. Push policy B again -> idempotent @@ -277,29 +269,14 @@ async fn live_policy_update_round_trip() { .expect("policy B path should be utf-8") .to_string(); - // --- Create a long-running sandbox --- - let mut guard = - SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready") - .await - .expect("create keep sandbox"); - - // --- Set initial policy A --- - let r = run_cli(&[ - "policy", - "set", - &guard.name, - "--policy", - &policy_a_path, - "--wait", - "--timeout", - "120", - ]) - .await; - assert!( - r.success, - "policy set A should succeed (exit {:?}):\n{}", - r.exit_code, r.output - ); + // --- Create a long-running sandbox with its startup-only policy fields --- + let mut guard = SandboxGuard::create_keep_with_args( + &["--policy", &policy_a_path, "--no-tty"], + &["sh", "-c", "echo Ready && sleep infinity"], + "Ready", + ) + .await + .expect("create keep sandbox with policy A"); // --- Verify initial policy version --- let r = run_cli(&["policy", "get", &guard.name]).await; @@ -451,10 +428,9 @@ async fn live_policy_update_round_trip() { /// Test live policy update from an initially empty network policy: /// -/// 1. Create sandbox with `--keep` -/// 2. Set policy with no network rules -/// 3. Push policy with a network rule using `--wait` -/// 4. Verify the version bumped +/// 1. Create sandbox with no network rules and `--keep` +/// 2. Push policy with a network rule using `--wait` +/// 3. Verify the version bumped #[tokio::test] async fn live_policy_update_from_empty_network_policies() { let empty_policy = write_empty_network_policy().expect("write empty network policy"); @@ -471,29 +447,16 @@ async fn live_policy_update_from_empty_network_policies() { .expect("full policy path should be utf-8") .to_string(); - // Create sandbox with empty network policy. - let mut guard = - SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready") - .await - .expect("create keep sandbox"); - - // Set initial empty policy. - let r = run_cli(&[ - "policy", - "set", - &guard.name, - "--policy", - &empty_path, - "--wait", - "--timeout", - "120", - ]) - .await; - assert!( - r.success, - "policy set (empty) should succeed (exit {:?}):\n{}", - r.exit_code, r.output - ); + // Create the sandbox with the empty network policy so subsequent live + // updates retain the same startup-only filesystem, landlock, and process + // fields. + let mut guard = SandboxGuard::create_keep_with_args( + &["--policy", &empty_path, "--no-tty"], + &["sh", "-c", "echo Ready && sleep infinity"], + "Ready", + ) + .await + .expect("create keep sandbox with empty network policy"); let r = run_cli(&["policy", "get", &guard.name]).await; assert!( diff --git a/e2e/rust/tests/podman_oci_identity.rs b/e2e/rust/tests/podman_oci_identity.rs new file mode 100644 index 0000000000..e30516bf09 --- /dev/null +++ b/e2e/rust/tests/podman_oci_identity.rs @@ -0,0 +1,237 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e-podman")] + +//! Podman-specific E2E coverage for OCI identity inspection and immutable-image +//! launch. +//! +//! The test builds an image through the selected Podman engine, creates a +//! sandbox from its mutable tag, and verifies both the child identity and the +//! image ID recorded on the real sandbox container. This exercises the Podman +//! API inspect → protected metadata → create path rather than only its unit +//! serialization boundaries. + +use std::process::Stdio; + +use openshell_e2e::harness::container::{ContainerEngine, is_e2e_driver}; +use openshell_e2e::harness::output::strip_ansi; +use openshell_e2e::harness::sandbox::SandboxGuard; + +const BASE_IMAGE: &str = "ghcr.io/nvidia/openshell-community/sandboxes/base:latest"; +const READY_MARKER: &str = "podman-oci-identity-ready"; +const OCI_UID: &str = "2345"; +const OCI_GID: &str = "2346"; +const OCI_FALLBACK_POLICY: &str = r#"version: 1 + +filesystem_policy: + include_workdir: true + read_only: [/usr, /lib, /lib64, /proc, /dev/urandom, /etc] + read_write: [/sandbox, /tmp, /dev/null] +landlock: + compatibility: best_effort + +network_policies: {} +"#; + +struct ImageGuard { + engine: ContainerEngine, + tag: String, + id: String, +} + +impl ImageGuard { + fn build() -> Result { + let engine = ContainerEngine::from_env()?; + if engine.name() != "podman" { + return Err(format!( + "Podman OCI identity E2E requires podman, got {}", + engine.name() + )); + } + + let context = tempfile::tempdir().map_err(|err| format!("create build context: {err}"))?; + let containerfile = context.path().join("Containerfile"); + std::fs::write( + &containerfile, + format!("FROM {BASE_IMAGE}\nUSER {OCI_UID}:{OCI_GID}\n"), + ) + .map_err(|err| format!("write Containerfile: {err}"))?; + + let tag = format!( + "localhost/openshell-e2e-podman-oci-identity:{}", + std::process::id() + ); + run_engine( + &engine, + &[ + "build", + "--pull=never", + "--file", + containerfile + .to_str() + .ok_or_else(|| "Containerfile path is not UTF-8".to_string())?, + "--tag", + &tag, + context + .path() + .to_str() + .ok_or_else(|| "build context path is not UTF-8".to_string())?, + ], + )?; + let id = run_engine(&engine, &["image", "inspect", "--format", "{{.Id}}", &tag])?; + let user = run_engine( + &engine, + &["image", "inspect", "--format", "{{.Config.User}}", &tag], + )?; + if user != format!("{OCI_UID}:{OCI_GID}") { + return Err(format!( + "Podman-built image has OCI user '{user}', expected {OCI_UID}:{OCI_GID}" + )); + } + + Ok(Self { engine, tag, id }) + } +} + +impl Drop for ImageGuard { + fn drop(&mut self) { + let _ = self + .engine + .command() + .args(["image", "rm", "--force", &self.tag]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + } +} + +fn run_engine(engine: &ContainerEngine, args: &[&str]) -> Result { + let output = engine + .command() + .args(args) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .map_err(|err| format!("failed to run {} {}: {err}", engine.name(), args.join(" ")))?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if !output.status.success() { + return Err(format!( + "{} {} failed (exit {:?}):\n{stdout}{stderr}", + engine.name(), + args.join(" "), + output.status.code() + )); + } + Ok(stdout.trim().to_string()) +} + +fn sandbox_container_id(engine: &ContainerEngine, sandbox_name: &str) -> Result { + let name_filter = format!("label=openshell.ai/sandbox-name={sandbox_name}"); + let stdout = run_engine( + engine, + &[ + "ps", + "-aq", + "--filter", + "label=openshell.managed=true", + "--filter", + &name_filter, + ], + )?; + let ids = stdout + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .collect::>(); + match ids.as_slice() { + [id] => Ok((*id).to_string()), + [] => Err(format!( + "no Podman container found for sandbox '{sandbox_name}'" + )), + _ => Err(format!( + "multiple Podman containers found for sandbox '{sandbox_name}': {ids:?}" + )), + } +} + +fn normalized_image_id(image_id: &str) -> &str { + image_id + .trim() + .strip_prefix("sha256:") + .unwrap_or(image_id.trim()) +} + +#[tokio::test] +async fn podman_uses_oci_identity_and_inspected_image_id() { + if !is_e2e_driver("podman") { + eprintln!("Skipping Podman OCI identity test: e2e driver is not podman"); + return; + } + + let image = ImageGuard::build().expect("build Podman OCI identity image"); + // The community base image contains a baked default policy with an + // explicit `sandbox` process identity. Supply a complete policy that + // intentionally omits `process` so this test exercises OCI fallback. + let policy = tempfile::NamedTempFile::new().expect("create OCI fallback policy"); + std::fs::write(policy.path(), OCI_FALLBACK_POLICY).expect("write OCI fallback policy"); + let policy_path = policy.path().to_str().expect("policy path is UTF-8"); + let mut sandbox = SandboxGuard::create_keep_with_args( + &[ + "--from", + &image.tag, + "--policy", + policy_path, + "--no-tty", + ], + &[ + "sh", + "-c", + "set -eu; printf 'direct-identity=%s:%s\n' \"$(id -u)\" \"$(id -g)\"; echo podman-oci-identity-ready; sleep infinity", + ], + READY_MARKER, + ) + .await + .expect("create sandbox from Podman-built OCI identity image"); + + let direct_output = strip_ansi(&sandbox.create_output); + assert!( + direct_output.contains("direct-identity=2345:2346"), + "expected direct child identity {OCI_UID}:{OCI_GID}:\n{direct_output}" + ); + + let ssh_output = sandbox + .exec(&[ + "sh", + "-c", + "test \"$(id -u):$(id -g)\" = 2345:2346; echo podman-ssh-identity-ok", + ]) + .await + .expect("SSH child should use Podman OCI identity"); + assert!( + ssh_output.contains("podman-ssh-identity-ok"), + "expected SSH identity marker:\n{ssh_output}" + ); + + let container_id = + sandbox_container_id(&image.engine, &sandbox.name).expect("find Podman sandbox container"); + let launched_image_id = run_engine( + &image.engine, + &[ + "container", + "inspect", + "--format", + "{{.Image}}", + &container_id, + ], + ) + .expect("inspect Podman sandbox container image"); + assert_eq!( + normalized_image_id(&launched_image_id), + normalized_image_id(&image.id), + "Podman sandbox must launch the immutable image ID inspected before creation" + ); + + sandbox.cleanup().await; +} diff --git a/examples/agent-driven-policy-management/policy.template.yaml b/examples/agent-driven-policy-management/policy.template.yaml index 0498ecfcc8..01f2d6e3b0 100644 --- a/examples/agent-driven-policy-management/policy.template.yaml +++ b/examples/agent-driven-policy-management/policy.template.yaml @@ -29,10 +29,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - network_policies: codex: name: codex diff --git a/examples/bring-your-own-container/Dockerfile b/examples/bring-your-own-container/Dockerfile index fc65bd6956..4b8ccf8abe 100644 --- a/examples/bring-your-own-container/Dockerfile +++ b/examples/bring-your-own-container/Dockerfile @@ -14,22 +14,20 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ curl iproute2 nftables \ && rm -rf /var/lib/apt/lists/* -# The sandbox user is injected at runtime by the compute driver. -# Kubernetes: resolved from OpenShift SCC namespace annotations or explicit -# sandbox_uid config. VM: resolves to 10001 by default, configurable in -# gateway TOML. -# -# Images no longer need a baked-in "sandbox" user — numeric UIDs are accepted -# and the driver passes them directly to setuid()/chown() at sandbox start. -# If your image requires a passwd entry for tools like ssh or sudo, add one -# manually (e.g. RUN useradd -m -u 1500 deploy). - -RUN install -d /sandbox +RUN groupadd --gid 1500 app \ + && useradd --uid 1500 --gid app --create-home app + +RUN install -d -o app -g app /sandbox WORKDIR /sandbox -COPY app.py . +COPY --chown=app:app app.py . EXPOSE 8080 +# Docker and Podman use this non-root identity when policy omits either process +# identity field. OpenShell starts the supervisor as root and drops only agent +# children to this account. +USER app + # NOTE: The sandbox supervisor replaces CMD at runtime. Pass the start # command explicitly: openshell sandbox create ... -- python /sandbox/app.py CMD ["python", "app.py"] diff --git a/examples/bring-your-own-container/README.md b/examples/bring-your-own-container/README.md index ea4f1cb9e6..c79e571f51 100644 --- a/examples/bring-your-own-container/README.md +++ b/examples/bring-your-own-container/README.md @@ -59,17 +59,17 @@ key requirements are: - **Pass your start command explicitly** — use `-- ` on the CLI. The image's `CMD` / `ENTRYPOINT` is replaced by the sandbox supervisor at runtime. -- **Create a `sandbox` user** (uid/gid 1000660000) for non-root execution. - Use a high UID (1000000000+) to avoid conflicts with host users when running - without user namespace remapping. -- **Make your application workdir writable by `sandbox`**. This example creates - `/sandbox` with `sandbox:sandbox` ownership before copying `app.py`. +- **Declare a non-root OCI `USER`** for Docker and Podman. Use a named account + such as `app`, a numeric UID with a passwd entry that supplies its primary + GID, or a numeric pair such as `1500:1500`. You can instead set both + `process.run_as_user` and `process.run_as_group` explicitly in policy. +- **Prepare `/sandbox` as the workspace.** Until OCI working-directory support + is added, create `/sandbox` and make it writable by the selected identity. + The example does this with `install -d -o app -g app /sandbox`. - **Install `iproute2`** for full network namespace isolation. - **Use a standard Linux base image** — distroless and `FROM scratch` images are not supported. -TODO(#70): Remove the sandbox user note once custom images are secure by default without requiring manual setup. - ## How it works OpenShell handles all the wiring automatically. You build a standard diff --git a/examples/governance-interceptor/policy.yaml b/examples/governance-interceptor/policy.yaml index 021e635db2..1ffe34a9f1 100644 --- a/examples/governance-interceptor/policy.yaml +++ b/examples/governance-interceptor/policy.yaml @@ -11,10 +11,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - network_policies: my_api: name: my-api diff --git a/examples/governance-interceptor/smoke.sh b/examples/governance-interceptor/smoke.sh index 34f93fa2c6..88610cf1ee 100755 --- a/examples/governance-interceptor/smoke.sh +++ b/examples/governance-interceptor/smoke.sh @@ -512,10 +512,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - network_policies: example_api: name: example-api diff --git a/examples/local-inference/sandbox-policy.yaml b/examples/local-inference/sandbox-policy.yaml index 79fde8ea29..a0d7ba1f41 100644 --- a/examples/local-inference/sandbox-policy.yaml +++ b/examples/local-inference/sandbox-policy.yaml @@ -21,10 +21,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - # Allow PyPI access so pip can install dependencies inside the sandbox. network_policies: pypi: diff --git a/examples/multi-agent-notepad/policy.template.yaml b/examples/multi-agent-notepad/policy.template.yaml index bb12863676..30be728754 100644 --- a/examples/multi-agent-notepad/policy.template.yaml +++ b/examples/multi-agent-notepad/policy.template.yaml @@ -11,10 +11,6 @@ filesystem_policy: landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox - network_policies: codex: name: codex diff --git a/examples/sandbox-policy-quickstart/README.md b/examples/sandbox-policy-quickstart/README.md index 34ecfbc9d6..ce6b16bfb3 100644 --- a/examples/sandbox-policy-quickstart/README.md +++ b/examples/sandbox-policy-quickstart/README.md @@ -81,8 +81,8 @@ cat examples/sandbox-policy-quickstart/policy.yaml ```yaml version: 1 -# Default sandbox filesystem and process settings. -# These static fields are required when using `openshell policy set` +# Default sandbox filesystem settings. +# These filesystem fields are required when using `openshell policy set` # because it replaces the entire policy. filesystem_policy: include_workdir: true @@ -90,9 +90,6 @@ filesystem_policy: read_write: [/sandbox, /tmp, /dev/null] landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox network_policies: github_api: @@ -108,8 +105,10 @@ network_policies: - { path: /usr/bin/curl } ``` -The top section preserves the default sandbox filesystem and process -settings (required because `policy set` replaces the entire policy). +The top section preserves the default sandbox filesystem and Landlock +settings while omitting process identity so the active compute driver can +select it. These settings are required because `policy set` replaces the +entire policy. The `network_policies` section is the interesting part: **curl may make GET, HEAD, and OPTIONS requests to `api.github.com` over HTTPS. Everything else is denied.** The proxy terminates TLS (`tls: terminate`) diff --git a/examples/sandbox-policy-quickstart/policy.yaml b/examples/sandbox-policy-quickstart/policy.yaml index 6bb0cb7d02..a17b359ebc 100644 --- a/examples/sandbox-policy-quickstart/policy.yaml +++ b/examples/sandbox-policy-quickstart/policy.yaml @@ -6,18 +6,15 @@ version: 1 -# Default sandbox filesystem and process settings. -# These static fields are required when using `openshell policy set` -# because it replaces the entire policy. +# Default sandbox filesystem and Landlock settings. Process identity is omitted +# so the active compute driver can select it. These fields are required when +# using `openshell policy set` because it replaces the entire policy. filesystem_policy: include_workdir: true read_only: [/usr, /lib, /proc, /dev/urandom, /app, /etc, /var/log] read_write: [/sandbox, /tmp, /dev/null] landlock: compatibility: best_effort -process: - run_as_user: sandbox - run_as_group: sandbox network_policies: github_api: