Skip to content

Commit e10d66f

Browse files
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>
1 parent ef2a235 commit e10d66f

5 files changed

Lines changed: 226 additions & 93 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: 71 additions & 7 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.
@@ -187,6 +189,25 @@ pub struct ImageConfig {
187189
pub env: Vec<String>,
188190
}
189191

192+
/// Whether a driver-owned volume has exactly the options `OpenShell` creates it
193+
/// with: none, or `uid`/`gid` for `owner`. Podman records the parsed `UID` and
194+
/// `GID` next to the raw `o` option.
195+
pub fn volume_options_match_owner(
196+
options: &HashMap<String, String>,
197+
owner: Option<(u32, u32)>,
198+
) -> bool {
199+
let Some((uid, gid)) = owner else {
200+
return options.is_empty();
201+
};
202+
options.get("o").map(String::as_str) == Some(format!("uid={uid},gid={gid}").as_str())
203+
&& options.iter().all(|(key, value)| match key.as_str() {
204+
"o" => true,
205+
"UID" => *value == uid.to_string(),
206+
"GID" => *value == gid.to_string(),
207+
_ => false,
208+
})
209+
}
210+
190211
/// A container summary returned by the list API.
191212
#[derive(Debug, Clone, serde::Deserialize)]
192213
#[serde(rename_all = "PascalCase")]
@@ -701,12 +722,18 @@ impl PodmanClient {
701722
// ── Volume operations ────────────────────────────────────────────────
702723

703724
/// Never adopt an unrelated existing volume on a private provisioning path.
725+
///
726+
/// With `owner`, Podman creates the volume root owned by that UID and GID,
727+
/// so a non-root workload can use it without a privileged chown.
704728
pub(crate) async fn create_owned_volume(
705729
&self,
706730
name: &str,
707731
sandbox_id: &str,
708732
workspace: &str,
733+
owner: Option<(u32, u32)>,
709734
) -> Result<(), PodmanApiError> {
735+
let owned_as_requested =
736+
|options: &HashMap<String, String>| volume_options_match_owner(options, owner);
710737
let labels = HashMap::from([
711738
(
712739
openshell_core::driver_utils::LABEL_SANDBOX_ID.to_string(),
@@ -720,7 +747,7 @@ impl PodmanClient {
720747
match self.inspect_volume(name).await {
721748
Ok(existing) => {
722749
if existing.driver != "local"
723-
|| !existing.options.is_empty()
750+
|| !owned_as_requested(&existing.options)
724751
|| existing.labels.as_ref() != Some(&labels)
725752
{
726753
return Err(PodmanApiError::InvalidInput(
@@ -732,14 +759,15 @@ impl PodmanClient {
732759
Err(PodmanApiError::NotFound(_)) => {}
733760
Err(error) => return Err(error),
734761
}
735-
self.create_ignore_conflict(
736-
"/libpod/volumes/create",
737-
&serde_json::json!({"Name":name,"Driver":"local","Labels":labels}),
738-
)
739-
.await?;
762+
let mut body = serde_json::json!({"Name":name,"Driver":"local","Labels":labels});
763+
if let Some((uid, gid)) = owner {
764+
body["Options"] = serde_json::json!({ "o": format!("uid={uid},gid={gid}") });
765+
}
766+
self.create_ignore_conflict("/libpod/volumes/create", &body)
767+
.await?;
740768
let created = self.inspect_volume(name).await?;
741769
if created.driver != "local"
742-
|| !created.options.is_empty()
770+
|| !owned_as_requested(&created.options)
743771
|| created.labels.as_ref() != Some(&labels)
744772
{
745773
return Err(PodmanApiError::InvalidInput(
@@ -1157,6 +1185,42 @@ mod tests {
11571185
let _ = std::fs::remove_file(socket_path);
11581186
}
11591187

1188+
#[tokio::test]
1189+
async fn create_owned_volume_verifies_requested_owner() {
1190+
let labels =
1191+
r#"{"openshell.ai/sandbox-id":"sandbox-1","openshell.ai/sandbox-workspace":"team-a"}"#;
1192+
for (options, accepted) in [
1193+
(
1194+
r#"{"o":"uid=1234,gid=1235","UID":"1234","GID":"1235"}"#,
1195+
true,
1196+
),
1197+
(r#"{"o":"uid=1234,gid=1235"}"#, true),
1198+
(r#"{"o":"uid=1234,gid=1235","UID":"0","GID":"1235"}"#, false),
1199+
(r#"{"o":"uid=1234,gid=1235","device":"/srv/work"}"#, false),
1200+
("{}", false),
1201+
] {
1202+
let (socket_path, _, handle) = spawn_podman_stub(
1203+
"owned-volume",
1204+
vec![
1205+
StubResponse::new(StatusCode::NOT_FOUND, ""),
1206+
StubResponse::new(StatusCode::CREATED, "{}"),
1207+
StubResponse::new(
1208+
StatusCode::OK,
1209+
format!(
1210+
r#"{{"Name":"work","Driver":"local","Options":{options},"Labels":{labels}}}"#
1211+
),
1212+
),
1213+
],
1214+
);
1215+
let result = PodmanClient::new(socket_path.clone())
1216+
.create_owned_volume("work", "sandbox-1", "team-a", Some((1234, 1235)))
1217+
.await;
1218+
assert_eq!(result.is_ok(), accepted, "options {options}: {result:?}");
1219+
handle.await.expect("stub task should finish");
1220+
let _ = std::fs::remove_file(socket_path);
1221+
}
1222+
}
1223+
11601224
#[tokio::test]
11611225
async fn inspect_image_reads_immutable_id_and_oci_user() {
11621226
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
@@ -1454,8 +1454,6 @@ pub struct IsolationSpecInput<'a> {
14541454
pub supervisor_bin: Option<&'a Path>,
14551455
pub tls_secrets: Option<&'a [String; 3]>,
14561456
pub identity: &'a openshell_isolation_interface::contract::ResolvedWorkloadIdentity,
1457-
/// Whether this workload is created by a rootless Podman service.
1458-
pub rootless: bool,
14591457
}
14601458

14611459
pub struct IsolationSpecs {
@@ -1507,44 +1505,19 @@ pub fn build_isolation_specs(
15071505
.iter()
15081506
.filter_map(|entry| entry.split_once('=').map(|(key, _)| key.to_string()))
15091507
.collect();
1510-
if input.rootless || input.identity.source == "default" {
1511-
// Podman's archive endpoint leaves named-volume contents owned by
1512-
// container root for rootless services and for a rootful USER-less
1513-
// image's newly-created workspace. Start the trusted runtime as root
1514-
// only long enough to chown the workspace, then irreversibly drop to
1515-
// the resolved workload identity before reading bootstrap material or
1516-
// accepting a control connection.
1517-
workload.command = vec![
1518-
"launch-capability-free".into(),
1519-
input.identity.uid.to_string(),
1520-
input.identity.gid.to_string(),
1521-
crate::isolation::BOOTSTRAP_PATH.into(),
1522-
driver_mounts::DEFAULT_WORKSPACE_ROOT.into(),
1523-
];
1524-
workload.user = "0:0".into();
1525-
workload.groups.clear();
1526-
workload.cap_drop = vec!["ALL".into()];
1527-
workload.cap_add = vec![
1528-
"CHOWN".into(),
1529-
"SETGID".into(),
1530-
"SETUID".into(),
1531-
"SETPCAP".into(),
1532-
];
1533-
} else {
1534-
workload.command = vec![
1535-
"--bootstrap".into(),
1536-
crate::isolation::BOOTSTRAP_PATH.into(),
1537-
];
1538-
workload.user.clone_from(&user);
1539-
workload.groups = input
1540-
.identity
1541-
.supplementary_gids
1542-
.iter()
1543-
.map(ToString::to_string)
1544-
.collect();
1545-
workload.cap_drop = vec!["ALL".into()];
1546-
workload.cap_add.clear();
1547-
}
1508+
workload.command = vec![
1509+
"--bootstrap".into(),
1510+
crate::isolation::BOOTSTRAP_PATH.into(),
1511+
];
1512+
workload.user.clone_from(&user);
1513+
workload.groups = input
1514+
.identity
1515+
.supplementary_gids
1516+
.iter()
1517+
.map(ToString::to_string)
1518+
.collect();
1519+
workload.cap_drop = vec!["ALL".into()];
1520+
workload.cap_add.clear();
15481521
workload.apparmor_profile = input
15491522
.config
15501523
.app_armor_profile
@@ -1842,29 +1815,21 @@ mod tests {
18421815
supervisor_bin: None,
18431816
tls_secrets: None,
18441817
identity: &identity,
1845-
rootless: true,
18461818
})
18471819
.unwrap();
18481820
for spec in [&specs.workload, &specs.supervisor] {
18491821
assert_eq!(spec.cap_drop, vec!["ALL"]);
18501822
assert!(spec.seccomp_profile_path.is_empty());
18511823
assert!(spec.no_new_privileges);
18521824
}
1853-
assert_eq!(specs.workload.user, "0:0");
1854-
assert!(specs.workload.groups.is_empty());
1855-
assert_eq!(
1856-
specs.workload.cap_add,
1857-
vec!["CHOWN", "SETGID", "SETUID", "SETPCAP"]
1858-
);
1825+
// The driver creates the managed workspace volume owned by the
1826+
// workload identity, so the workload never starts as root.
1827+
assert_eq!(specs.workload.user, "1000:1001");
1828+
assert_eq!(specs.workload.groups, vec!["2000"]);
1829+
assert!(specs.workload.cap_add.is_empty());
18591830
assert_eq!(
18601831
specs.workload.command,
1861-
vec![
1862-
"launch-capability-free",
1863-
"1000",
1864-
"1001",
1865-
crate::isolation::BOOTSTRAP_PATH,
1866-
driver_mounts::DEFAULT_WORKSPACE_ROOT,
1867-
]
1832+
vec!["--bootstrap", crate::isolation::BOOTSTRAP_PATH]
18681833
);
18691834
assert_eq!(specs.supervisor.user, "1000:1001");
18701835
assert_eq!(specs.supervisor.groups, vec!["2000"]);
@@ -1893,7 +1858,7 @@ mod tests {
18931858
"sha256:image".into(),
18941859
)
18951860
.unwrap();
1896-
let rootful_specs = build_isolation_specs(IsolationSpecInput {
1861+
let default_specs = build_isolation_specs(IsolationSpecInput {
18971862
sandbox: &sandbox,
18981863
config: &config,
18991864
token_secret: Some("jwt"),
@@ -1906,19 +1871,13 @@ mod tests {
19061871
supervisor_bin: None,
19071872
tls_secrets: None,
19081873
identity: &default_identity,
1909-
rootless: false,
19101874
})
19111875
.unwrap();
1912-
assert_eq!(rootful_specs.workload.user, "0:0");
1876+
assert_eq!(default_specs.workload.user, "1000:1000");
1877+
assert!(default_specs.workload.cap_add.is_empty());
19131878
assert_eq!(
1914-
rootful_specs.workload.command,
1915-
vec![
1916-
"launch-capability-free",
1917-
"1000",
1918-
"1000",
1919-
crate::isolation::BOOTSTRAP_PATH,
1920-
driver_mounts::DEFAULT_WORKSPACE_ROOT,
1921-
]
1879+
default_specs.workload.command,
1880+
vec!["--bootstrap", crate::isolation::BOOTSTRAP_PATH]
19221881
);
19231882
let workload_json = serde_json::to_string(&specs.workload).unwrap();
19241883
assert!(workload_json.contains("\"apparmor_profile\":\"openshell-sandbox\""));

0 commit comments

Comments
 (0)