refactor(podman): generalize keep-id group handling

Signed-off-by: Evan Lezar <elezar@nvidia.com>

Author: elezar

crates/openshell-driver-podman/src/client.rs

@@ -21,7 +21,7 @@ use tracing::debug;
 const API_VERSION: &str = "v5.0.0";
 
 /// Timeout for individual Podman API calls.
-const API_TIMEOUT: Duration = Duration::from_secs(120);
+const API_TIMEOUT: Duration = Duration::from_secs(30);
 
 /// Maximum allowed size for the event stream line buffer (1 MB).
 const MAX_EVENT_BUFFER: usize = 1_048_576;

crates/openshell-driver-podman/src/driver.rs

@@ -972,7 +972,9 @@ impl PodmanComputeDriver {
                         &workload_id,
                         &uuid::Uuid::new_v4().to_string(),
                         &identity,
-                        self.config.userns.as_deref(),
+                        crate::isolation::userns_preserves_host_groups(
+                            self.config.userns.as_deref(),
+                        ),
                         child_env,
                         &launch_authentication,
                     )?;
@@ -1314,7 +1316,7 @@ impl PodmanComputeDriver {
                 &container_id,
                 generation.as_str(),
                 &restart_metadata.workload_identity,
-                self.config.userns.as_deref(),
+                crate::isolation::userns_preserves_host_groups(self.config.userns.as_deref()),
                 restart_metadata.child_env,
                 &launch_authentication,
             )?;

crates/openshell-driver-podman/src/isolation.rs

@@ -11,6 +11,7 @@ use std::path::PathBuf;
 use openshell_core::ComputeDriverError;
 use openshell_core::proto::compute::v1::DriverSandbox;
 use openshell_isolation_interface::contract::{DriverFenceEvidence, ResolvedWorkloadIdentity};
+use openshell_sandbox_backend::ALLOW_EXTRA_SUPPLEMENTARY_GROUPS_RESOURCE_CLAIM;
 use openshell_sandbox_backend::boundary_protocol::{
     BoundaryConfig, BoundaryListener, GatewayVerificationKey, SandboxRuntimeDescriptor,
     SandboxTlsClientConfig, SandboxTlsServerConfig, SandboxTransport,
@@ -25,7 +26,6 @@ pub const BOOTSTRAP_PATH: &str = "/.openshell/channel/sandbox/bootstrap.json";
 pub const RUNTIME_DESCRIPTOR_PATH: &str = "/.openshell/supervisor/runtime-descriptor.json";
 pub const AUTH_BUNDLE_PATH: &str = "/.openshell/supervisor/auth.json";
 pub const RESTART_METADATA_PATH: &str = "/.openshell/supervisor/restart-metadata.json";
-pub const USERNS_RESOURCE_CLAIM: &str = "podman.userns";
 const SOCKET_PATH: &str = "/.openshell/channel/sandbox/control.sock";
 
 pub fn supervisor_name(id: &str) -> String {
@@ -35,6 +35,12 @@ pub fn channel_volume_name(id: &str) -> String {
     format!("openshell-channel-{id}")
 }
 
+/// `keep-id` may retain the gateway user's supplementary groups in the
+/// container. Other user-namespace modes, including `auto`, do not.
+pub fn userns_preserves_host_groups(userns: Option<&str>) -> bool {
+    userns.is_some_and(|mode| mode.split(':').next() == Some("keep-id"))
+}
+
 fn invalid(error: impl std::fmt::Display) -> ComputeDriverError {
     ComputeDriverError::Precondition(error.to_string())
 }
@@ -160,7 +166,7 @@ pub fn bootstrap_archives(
     container_id: &str,
     generation: &str,
     identity: &ResolvedWorkloadIdentity,
-    userns: Option<&str>,
+    allow_extra_supplementary_groups: bool,
     child_env: HashMap<String, String>,
     launch_authentication: &openshell_core::jwt::SandboxLaunchAuthentication,
 ) -> Result<BootstrapArchives, ComputeDriverError> {
@@ -174,8 +180,11 @@ pub fn bootstrap_archives(
             identity.resource_digest.clone(),
         ),
     ]);
-    if userns.is_some_and(|mode| mode.split(':').next() == Some("keep-id")) {
-        resource_claims.insert(USERNS_RESOURCE_CLAIM.into(), "keep-id".into());
+    if allow_extra_supplementary_groups {
+        resource_claims.insert(
+            ALLOW_EXTRA_SUPPLEMENTARY_GROUPS_RESOURCE_CLAIM.into(),
+            "true".into(),
+        );
     }
     let driver_fence = DriverFenceEvidence::Podman {
         container_id: container_id.into(),
@@ -439,7 +448,7 @@ mod tests {
             "container",
             "generation-1",
             &identity,
-            None,
+            false,
             child_env.clone(),
             &authentication,
         )
@@ -481,6 +490,11 @@ mod tests {
         assert_eq!(config.session_id, runtime_descriptor.session_id);
         assert_eq!(config.driver_fence, runtime_descriptor.driver_fence);
         assert_eq!(config.workload_identity, identity);
+        assert!(
+            !config
+                .resource_claims
+                .contains_key(ALLOW_EXTRA_SUPPLEMENTARY_GROUPS_RESOURCE_CLAIM)
+        );
         runtime_descriptor.driver_fence.validate().unwrap();
         let restart_metadata: RestartMetadata = serde_json::from_slice(
             supervisor
@@ -499,4 +513,15 @@ mod tests {
                 .any(|window| window == b"PRIVATE KEY")
         );
     }
+
+    #[test]
+    fn keep_id_is_the_only_userns_mode_that_preserves_host_groups() {
+        assert!(userns_preserves_host_groups(Some("keep-id")));
+        assert!(userns_preserves_host_groups(Some(
+            "keep-id:uid=1000,gid=1000"
+        )));
+        assert!(!userns_preserves_host_groups(Some("auto")));
+        assert!(!userns_preserves_host_groups(Some("private")));
+        assert!(!userns_preserves_host_groups(None));
+    }
 }

crates/openshell-sandbox-backend/src/lib.rs

@@ -21,6 +21,11 @@ pub const BACKEND_NAME: &str = "openshell-sandbox";
 /// Resource claim set by compute drivers when the workload requests GPU access.
 pub const GPU_RESOURCE_CLAIM: &str = "openshell.gpu";
 
+/// Resource claim set when the runtime may retain supplementary groups in
+/// addition to the image-derived workload identity.
+pub const ALLOW_EXTRA_SUPPLEMENTARY_GROUPS_RESOURCE_CLAIM: &str =
+    "openshell.identity.allow_extra_supplementary_groups";
+
 /// Memory-backed parent used for supervisor CA material.
 pub const SUPERVISOR_CA_RUNTIME_ROOT: &str = "/run/openshell-supervisor-ca";
 

crates/openshell-sandbox/src/boundary_server.rs

@@ -40,7 +40,6 @@ mod linux {
         CapabilityEvidence, ExecSession, LoopbackTarget, ResolvedWorkloadIdentity,
         SandboxConfirmEvidence,
     };
-    use openshell_sandbox_backend::GPU_RESOURCE_CLAIM;
     use openshell_sandbox_backend::mediation::{
         self, DnsQueryWire, MediationFrame, MediationFrameKind,
     };
@@ -54,6 +53,9 @@ mod linux {
         SandboxConnectionId, SandboxConnectionRegistry, SandboxProtocolAuthenticator,
         SandboxProtocolPrincipal,
     };
+    use openshell_sandbox_backend::{
+        ALLOW_EXTRA_SUPPLEMENTARY_GROUPS_RESOURCE_CLAIM, GPU_RESOURCE_CLAIM,
+    };
     use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _};
     use tokio_stream::wrappers::ReceiverStream;
 
@@ -392,8 +394,8 @@ mod linux {
             .is_some_and(|value| value == "true")
             || config
                 .resource_claims
-                .get("podman.userns")
-                .is_some_and(|value| value == "keep-id")
+                .get(ALLOW_EXTRA_SUPPLEMENTARY_GROUPS_RESOURCE_CLAIM)
+                .is_some_and(|value| value == "true")
     }
 
     fn supplementary_groups_match(actual: &[u32], expected: &[u32], allow_extra: bool) -> bool {
@@ -3829,7 +3831,7 @@ mod linux {
         }
 
         #[test]
-        fn podman_keep_id_allows_runtime_supplementary_groups() {
+        fn generic_identity_claim_allows_runtime_supplementary_groups() {
             let config = BoundaryConfig {
                 boundary_id: "sandbox-1".to_string(),
                 generation: "generation-1".to_string(),
@@ -3844,8 +3846,8 @@ mod linux {
                     tls: placeholder_server_tls(),
                 },
                 resource_claims: std::collections::BTreeMap::from([(
-                    "podman.userns".to_string(),
-                    "keep-id".to_string(),
+                    ALLOW_EXTRA_SUPPLEMENTARY_GROUPS_RESOURCE_CLAIM.to_string(),
+                    "true".to_string(),
                 )]),
                 resource_claim_files: std::collections::BTreeMap::new(),
                 workload_identity: test_workload_identity(),

tests/ansible/playbooks/conformance/cli.yaml

@@ -84,7 +84,7 @@
           - openshell-gateway.service
           - --no-pager
           - --lines
-          - "20"
+          - "500"
       register: openshell_gateway_logs
       changed_when: false
       failed_when: false

tests/suites/drivers/podman/tests/default_userns.rs

@@ -19,8 +19,8 @@ const PODMAN_TEST_IMAGE_ENV: &str = "OPENSHELL_PODMAN_TEST_IMAGE";
 /// Verify that the gateway's user-namespace configuration matches Podman's
 /// direct behavior for the same profile.
 ///
-/// The test creates a sandbox and compares its user-namespace mapping with the
-/// direct-Podman reference stored at
+/// The test runs a short-lived sandbox command and compares its user-namespace
+/// mapping with the direct-Podman reference stored at
 /// `OPENSHELL_TEST_INPUT_DIR/reference-uid-map`. The tmachine pre-test
 /// playbook creates that reference in the same gateway-user context. This deliberately
 /// avoids baking a particular Podman mapping into OpenShell's test contract.
@@ -59,38 +59,21 @@ async fn configured_userns_matches_podman_reference() {
         if let Some(image) = workload_image.as_deref() {
             create_args.extend(["--from", image]);
         }
-        create_args.extend(["--detach", "--", "sleep", "infinity"]);
-        let create = runner
-            .step("userns/create")
-            .description("sandbox from the configured test image is created")
-            .with_timeout(SANDBOX_TIMEOUT)
-            .run(&create_args)
-            .await
-            .map_err(|error| error.to_string())?;
-        create.require_success()?;
-        let exec = runner
+        create_args.extend(["--no-tty", "--", "cat", "/proc/self/uid_map"]);
+        let run = runner
             .step("userns/uid-map")
             .description("sandbox exposes its UID map")
             .with_timeout(SANDBOX_TIMEOUT)
-            .run(&[
-                "sandbox",
-                "exec",
-                "--name",
-                &sandbox_name,
-                "--no-tty",
-                "--",
-                "cat",
-                "/proc/self/uid_map",
-            ])
+            .run(&create_args)
             .await
             .map_err(|error| error.to_string())?;
-        exec.require_success()?;
-        let sandbox_uid_map = normalize_uid_map(exec.stdout()).ok_or_else(|| {
-            exec.failure_diagnostic("sandbox returns a non-empty UID map")
+        run.require_success()?;
+        let sandbox_uid_map = normalize_uid_map(run.stdout()).ok_or_else(|| {
+            run.failure_diagnostic("sandbox returns a non-empty UID map")
         })?;
         if sandbox_uid_map != expected_uid_map {
             return Err(format!(
-                "sandbox UID map differs from direct Podman default:\nexpected:\n{expected_uid_map}\nactual:\n{sandbox_uid_map}"
+                "sandbox UID map differs from the direct Podman reference:\nexpected:\n{expected_uid_map}\nactual:\n{sandbox_uid_map}"
             ));
         }
         Ok(())