Skip to content

Commit 13c248f

Browse files
fix(podman): create managed workspace volumes owned by the workload identity (#3981)
* fix(podman): create managed workspace volumes owned by the workload identity Podman now creates the managed /sandbox volume with uid/gid options for the resolved workload identity, so the workload starts directly as that identity. This fixes rootful sandboxes whose image USER or policy run_as_user could not write to a root-owned /sandbox, and removes the root-then-drop workspace chown start path. Resource admission accepts the managed workspace volume when its options match the workload container's final identity, or are empty for volumes created by older gateways. The channel volume still requires empty options. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * test(podman): cover managed volume reuse and workspace access Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(podman): clarify managed volume creation and validation Signed-off-by: Evan Lezar <elezar@nvidia.com> * test(podman): verify workspace access across user namespaces Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> Signed-off-by: Evan Lezar <elezar@nvidia.com> Co-authored-by: Evan Lezar <elezar@nvidia.com>
1 parent 3592a48 commit 13c248f

6 files changed

Lines changed: 334 additions & 107 deletions

File tree

‎crates/openshell-driver-podman/README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ stopped Podman container does not populate nested named volumes. Restart restore
4343
only the channel bootstrap into the existing channel volume, preserving the
4444
workspace. The workload starts before the supervisor so its user namespace exists
4545
when the supervisor joins it; a stopped supervisor resolves that namespace again
46-
on its next start.
46+
on its next start. The driver creates the managed workspace volume owned by
47+
the workload's final UID and GID, so the workload never starts as root.
4748

4849
The runtime must pass the sandbox's unprivileged enforcement probe, including
4950
nested seccomp notification and Landlock. Unsupported runtime defaults fail

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

Lines changed: 141 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ pub struct PortBinding {
166166
pub struct ContainerConfig {
167167
#[serde(default)]
168168
pub labels: HashMap<String, String>,
169+
#[serde(default)]
170+
pub user: String,
169171
}
170172

171173
/// Immutable image metadata needed to bind OCI identity inspection to launch.
@@ -238,6 +240,38 @@ pub struct VolumeInspect {
238240
}
239241

240242
impl VolumeInspect {
243+
/// Whether metadata matches a managed local volume with the exact labels
244+
/// and requested ownership options. This does not inspect filesystem ownership.
245+
pub(crate) fn matches_managed_volume(
246+
&self,
247+
labels: &HashMap<String, String>,
248+
requested_owner: Option<(u32, u32)>,
249+
) -> bool {
250+
self.driver == "local"
251+
&& self.labels.as_ref() == Some(labels)
252+
&& self.options_match_requested_owner(requested_owner)
253+
}
254+
255+
/// Whether option metadata matches the requested owner. `None` means no
256+
/// ownership options were requested and requires an empty options map.
257+
/// This does not inspect filesystem ownership. Podman records the parsed
258+
/// `UID` and `GID` next to the raw `o` option.
259+
pub(crate) fn options_match_requested_owner(
260+
&self,
261+
requested_owner: Option<(u32, u32)>,
262+
) -> bool {
263+
let Some((uid, gid)) = requested_owner else {
264+
return self.options.is_empty();
265+
};
266+
self.options.get("o").map(String::as_str) == Some(format!("uid={uid},gid={gid}").as_str())
267+
&& self.options.iter().all(|(key, value)| match key.as_str() {
268+
"o" => true,
269+
"UID" => *value == uid.to_string(),
270+
"GID" => *value == gid.to_string(),
271+
_ => false,
272+
})
273+
}
274+
241275
pub(crate) fn admission_identity(&self) -> Value {
242276
serde_json::json!({"name": self.name, "driver": self.driver, "options": self.options, "created_at": self.created_at})
243277
}
@@ -700,12 +734,38 @@ impl PodmanClient {
700734

701735
// ── Volume operations ────────────────────────────────────────────────
702736

737+
/// Create and inspect a local volume. HTTP 409 conflicts also proceed to
738+
/// inspection; callers must verify the returned labels and options.
739+
async fn create_volume(
740+
&self,
741+
name: &str,
742+
labels: &HashMap<String, String>,
743+
options: &HashMap<String, String>,
744+
) -> Result<VolumeInspect, PodmanApiError> {
745+
validate_name(name)?;
746+
let mut body = serde_json::json!({
747+
"Name": name,
748+
"Driver": "local",
749+
"Labels": labels,
750+
});
751+
if !options.is_empty() {
752+
body["Options"] = serde_json::json!(options);
753+
}
754+
self.create_ignore_conflict("/libpod/volumes/create", &body)
755+
.await?;
756+
self.inspect_volume(name).await
757+
}
758+
703759
/// Never adopt an unrelated existing volume on a private provisioning path.
760+
///
761+
/// With `owner`, Podman creates the volume root owned by that UID and GID,
762+
/// so a non-root workload can use it without a privileged chown.
704763
pub(crate) async fn create_owned_volume(
705764
&self,
706765
name: &str,
707766
sandbox_id: &str,
708767
workspace: &str,
768+
owner: Option<(u32, u32)>,
709769
) -> Result<(), PodmanApiError> {
710770
let labels = HashMap::from([
711771
(
@@ -719,10 +779,7 @@ impl PodmanClient {
719779
]);
720780
match self.inspect_volume(name).await {
721781
Ok(existing) => {
722-
if existing.driver != "local"
723-
|| !existing.options.is_empty()
724-
|| existing.labels.as_ref() != Some(&labels)
725-
{
782+
if !existing.matches_managed_volume(&labels, owner) {
726783
return Err(PodmanApiError::InvalidInput(
727784
"private volume name collides with an unrelated resource".into(),
728785
));
@@ -732,16 +789,11 @@ impl PodmanClient {
732789
Err(PodmanApiError::NotFound(_)) => {}
733790
Err(error) => return Err(error),
734791
}
735-
self.create_ignore_conflict(
736-
"/libpod/volumes/create",
737-
&serde_json::json!({"Name":name,"Driver":"local","Labels":labels}),
738-
)
739-
.await?;
740-
let created = self.inspect_volume(name).await?;
741-
if created.driver != "local"
742-
|| !created.options.is_empty()
743-
|| created.labels.as_ref() != Some(&labels)
744-
{
792+
let options = owner.map_or_else(HashMap::new, |(uid, gid)| {
793+
HashMap::from([("o".to_string(), format!("uid={uid},gid={gid}"))])
794+
});
795+
let created = self.create_volume(name, &labels, &options).await?;
796+
if !created.matches_managed_volume(&labels, owner) {
745797
return Err(PodmanApiError::InvalidInput(
746798
"private volume ownership verification failed".into(),
747799
));
@@ -1157,6 +1209,81 @@ mod tests {
11571209
let _ = std::fs::remove_file(socket_path);
11581210
}
11591211

1212+
#[tokio::test]
1213+
async fn create_owned_volume_verifies_requested_options() {
1214+
let labels =
1215+
r#"{"openshell.ai/sandbox-id":"sandbox-1","openshell.ai/sandbox-workspace":"team-a"}"#;
1216+
for (owner, options, accepted) in [
1217+
(
1218+
Some((1234, 1235)),
1219+
r#"{"o":"uid=1234,gid=1235","UID":"1234","GID":"1235"}"#,
1220+
true,
1221+
),
1222+
(Some((1234, 1235)), r#"{"o":"uid=1234,gid=1235"}"#, true),
1223+
// Podman accepts either order, but OpenShell always requests uid first.
1224+
(Some((1234, 1235)), r#"{"o":"gid=1235,uid=1234"}"#, false),
1225+
(
1226+
Some((1234, 1235)),
1227+
r#"{"o":"uid=1234,gid=1235","UID":"0","GID":"1235"}"#,
1228+
false,
1229+
),
1230+
(
1231+
Some((1234, 1235)),
1232+
r#"{"o":"uid=1234,gid=1235","device":"/srv/work"}"#,
1233+
false,
1234+
),
1235+
(Some((1234, 1235)), "{}", false),
1236+
(None, "{}", true),
1237+
(None, r#"{"o":"uid=1234,gid=1235"}"#, false),
1238+
(None, r#"{"o":"bind","device":"/srv/work"}"#, false),
1239+
] {
1240+
for existing in [false, true] {
1241+
let inspected = || {
1242+
StubResponse::new(
1243+
StatusCode::OK,
1244+
format!(
1245+
r#"{{"Name":"work","Driver":"local","Options":{options},"Labels":{labels}}}"#
1246+
),
1247+
)
1248+
};
1249+
let responses = if existing {
1250+
vec![inspected()]
1251+
} else {
1252+
vec![
1253+
StubResponse::new(StatusCode::NOT_FOUND, ""),
1254+
StubResponse::new(StatusCode::CREATED, "{}"),
1255+
inspected(),
1256+
]
1257+
};
1258+
let (socket_path, request_log, handle) =
1259+
spawn_podman_stub("owned-volume", responses);
1260+
let result = PodmanClient::new(socket_path.clone())
1261+
.create_owned_volume("work", "sandbox-1", "team-a", owner)
1262+
.await;
1263+
assert_eq!(
1264+
result.is_ok(),
1265+
accepted,
1266+
"owner {owner:?}, options {options}, existing {existing}: {result:?}"
1267+
);
1268+
handle.await.expect("stub task should finish");
1269+
let expected_requests = if existing {
1270+
vec!["GET /v5.0.0/libpod/volumes/work/json"]
1271+
} else {
1272+
vec![
1273+
"GET /v5.0.0/libpod/volumes/work/json",
1274+
"POST /v5.0.0/libpod/volumes/create",
1275+
"GET /v5.0.0/libpod/volumes/work/json",
1276+
]
1277+
};
1278+
assert_eq!(
1279+
request_log.lock().expect("request log lock").as_slice(),
1280+
expected_requests,
1281+
);
1282+
let _ = std::fs::remove_file(socket_path);
1283+
}
1284+
}
1285+
}
1286+
11601287
#[tokio::test]
11611288
async fn inspect_image_reads_immutable_id_and_oci_user() {
11621289
let (socket_path, request_log, handle) = spawn_podman_stub(

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

Lines changed: 24 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,8 +1406,6 @@ pub struct IsolationSpecInput<'a> {
14061406
pub supervisor_bin: Option<&'a Path>,
14071407
pub tls_secrets: Option<&'a [String; 1]>,
14081408
pub identity: &'a openshell_isolation_interface::contract::ResolvedWorkloadIdentity,
1409-
/// Whether this workload is created by a rootless Podman service.
1410-
pub rootless: bool,
14111409
}
14121410

14131411
pub struct IsolationSpecs {
@@ -1459,44 +1457,19 @@ pub fn build_isolation_specs(
14591457
.iter()
14601458
.filter_map(|entry| entry.split_once('=').map(|(key, _)| key.to_string()))
14611459
.collect();
1462-
if input.rootless || input.identity.source == "default" {
1463-
// Podman's archive endpoint leaves named-volume contents owned by
1464-
// container root for rootless services and for a rootful USER-less
1465-
// image's newly-created workspace. Start the trusted runtime as root
1466-
// only long enough to chown the workspace, then irreversibly drop to
1467-
// the resolved workload identity before reading bootstrap material or
1468-
// accepting a control connection.
1469-
workload.command = vec![
1470-
"launch-capability-free".into(),
1471-
input.identity.uid.to_string(),
1472-
input.identity.gid.to_string(),
1473-
crate::isolation::BOOTSTRAP_PATH.into(),
1474-
driver_mounts::DEFAULT_WORKSPACE_ROOT.into(),
1475-
];
1476-
workload.user = "0:0".into();
1477-
workload.groups.clear();
1478-
workload.cap_drop = vec!["ALL".into()];
1479-
workload.cap_add = vec![
1480-
"CHOWN".into(),
1481-
"SETGID".into(),
1482-
"SETUID".into(),
1483-
"SETPCAP".into(),
1484-
];
1485-
} else {
1486-
workload.command = vec![
1487-
"--bootstrap".into(),
1488-
crate::isolation::BOOTSTRAP_PATH.into(),
1489-
];
1490-
workload.user.clone_from(&user);
1491-
workload.groups = input
1492-
.identity
1493-
.supplementary_gids
1494-
.iter()
1495-
.map(ToString::to_string)
1496-
.collect();
1497-
workload.cap_drop = vec!["ALL".into()];
1498-
workload.cap_add.clear();
1499-
}
1460+
workload.command = vec![
1461+
"--bootstrap".into(),
1462+
crate::isolation::BOOTSTRAP_PATH.into(),
1463+
];
1464+
workload.user.clone_from(&user);
1465+
workload.groups = input
1466+
.identity
1467+
.supplementary_gids
1468+
.iter()
1469+
.map(ToString::to_string)
1470+
.collect();
1471+
workload.cap_drop = vec!["ALL".into()];
1472+
workload.cap_add.clear();
15001473
workload.apparmor_profile = input
15011474
.config
15021475
.app_armor_profile
@@ -1790,29 +1763,21 @@ mod tests {
17901763
supervisor_bin: None,
17911764
tls_secrets: None,
17921765
identity: &identity,
1793-
rootless: true,
17941766
})
17951767
.unwrap();
17961768
for spec in [&specs.workload, &specs.supervisor] {
17971769
assert_eq!(spec.cap_drop, vec!["ALL"]);
17981770
assert!(spec.seccomp_profile_path.is_empty());
17991771
assert!(spec.no_new_privileges);
18001772
}
1801-
assert_eq!(specs.workload.user, "0:0");
1802-
assert!(specs.workload.groups.is_empty());
1803-
assert_eq!(
1804-
specs.workload.cap_add,
1805-
vec!["CHOWN", "SETGID", "SETUID", "SETPCAP"]
1806-
);
1773+
// The driver creates the managed workspace volume owned by the
1774+
// workload identity, so the workload never starts as root.
1775+
assert_eq!(specs.workload.user, "1000:1001");
1776+
assert_eq!(specs.workload.groups, vec!["2000"]);
1777+
assert!(specs.workload.cap_add.is_empty());
18071778
assert_eq!(
18081779
specs.workload.command,
1809-
vec![
1810-
"launch-capability-free",
1811-
"1000",
1812-
"1001",
1813-
crate::isolation::BOOTSTRAP_PATH,
1814-
driver_mounts::DEFAULT_WORKSPACE_ROOT,
1815-
]
1780+
vec!["--bootstrap", crate::isolation::BOOTSTRAP_PATH]
18161781
);
18171782
assert_eq!(specs.supervisor.user, "1000:1001");
18181783
assert_eq!(specs.supervisor.groups, vec!["2000"]);
@@ -1841,7 +1806,7 @@ mod tests {
18411806
"sha256:image".into(),
18421807
)
18431808
.unwrap();
1844-
let rootful_specs = build_isolation_specs(IsolationSpecInput {
1809+
let default_specs = build_isolation_specs(IsolationSpecInput {
18451810
sandbox: &sandbox,
18461811
config: &config,
18471812
token_secret: Some("jwt"),
@@ -1854,19 +1819,13 @@ mod tests {
18541819
supervisor_bin: None,
18551820
tls_secrets: None,
18561821
identity: &default_identity,
1857-
rootless: false,
18581822
})
18591823
.unwrap();
1860-
assert_eq!(rootful_specs.workload.user, "0:0");
1824+
assert_eq!(default_specs.workload.user, "1000:1000");
1825+
assert!(default_specs.workload.cap_add.is_empty());
18611826
assert_eq!(
1862-
rootful_specs.workload.command,
1863-
vec![
1864-
"launch-capability-free",
1865-
"1000",
1866-
"1000",
1867-
crate::isolation::BOOTSTRAP_PATH,
1868-
driver_mounts::DEFAULT_WORKSPACE_ROOT,
1869-
]
1827+
default_specs.workload.command,
1828+
vec!["--bootstrap", crate::isolation::BOOTSTRAP_PATH]
18701829
);
18711830
let workload_json = serde_json::to_string(&specs.workload).unwrap();
18721831
assert!(workload_json.contains("\"apparmor_profile\":\"openshell-sandbox\""));

0 commit comments

Comments
 (0)