Skip to content

Commit d676e03

Browse files
authored
fix(vm): reject conflicting workload identity selectors (#4036)
* fix(vm): enforce the configured workload identity Reject conflicting policy users and groups before VM image preparation and before guest attach or process startup changes state. Validate supervisor policy updates against the protected VM workload identity. Preserve the gateway CA transport and capability-free sandbox launcher. Signed-off-by: Shiju <shiju@nvidia.com> * test(sandbox): clarify VM identity rejection fixtures Name invalid user and group fixtures distinctly and move the final workload identity into its group mismatch test. Signed-off-by: Shiju <shiju@nvidia.com> * fix(supervisor): align VM identity startup with current APIs Pass the optional rejection-log key for VM identity failures and keep generic startup-write regressions free of VM identity constraints. Repair the call sites after the branch rebase so the identity and cleanup proposals compile against the current startup helpers. Signed-off-by: Shiju <shiju@nvidia.com> * fix(vm): restore inactive sandbox workload identity Recover the persisted overlay owner before publishing stopped and terminal sandboxes. Keep resources manageable when identity metadata is invalid. Clarify fixed MicroVM ownership in policy-generation guidance. Signed-off-by: Shiju <shiju@nvidia.com> * test(vm): flush identity fixture before restart Persist the canonical identity file before readiness and report the observed exec, canonical and file-owner identities before comparing them. Signed-off-by: Shiju <shiju@nvidia.com> --------- Signed-off-by: Shiju <shiju@nvidia.com>
1 parent fcd8fe5 commit d676e03

9 files changed

Lines changed: 1097 additions & 45 deletions

File tree

‎crates/openshell-driver-vm/src/driver.rs‎

Lines changed: 393 additions & 33 deletions
Large diffs are not rendered by default.

‎crates/openshell-sandbox/src/boundary_server.rs‎

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,34 @@ mod linux {
358358
Ok(())
359359
}
360360

361+
/// VM selectors assert the protected overlay owner; they cannot choose a
362+
/// replacement identity. Guest init maps `sandbox` to that owner before
363+
/// launching this boundary, so no account lookup or privilege change belongs here.
364+
fn validate_vm_policy_identity(
365+
config: &BoundaryConfig,
366+
policy: &SandboxPolicyWire,
367+
) -> Result<(), String> {
368+
if !config.resource_claims.contains_key("vm.generation") {
369+
return Ok(());
370+
}
371+
let identity = &config.workload_identity;
372+
for (field, selector, expected) in [
373+
("run_as_user", policy.run_as_user.as_deref(), identity.uid),
374+
("run_as_group", policy.run_as_group.as_deref(), identity.gid),
375+
] {
376+
let Some(selector) = selector.filter(|value| !value.is_empty()) else {
377+
continue;
378+
};
379+
if selector != "sandbox" && selector.parse::<u32>() != Ok(expected) {
380+
return Err(format!(
381+
"VM {field} '{selector}' conflicts with the resolved workload identity {}:{}; omit the selector or request the driver-owned identity",
382+
identity.uid, identity.gid
383+
));
384+
}
385+
}
386+
Ok(())
387+
}
388+
361389
fn tls_paths_are_absolute(
362390
tls: &openshell_sandbox_backend::boundary_protocol::SandboxTlsServerConfig,
363391
) -> bool {
@@ -2287,6 +2315,11 @@ mod linux {
22872315
}
22882316

22892317
fn attach(&self, policy: SandboxPolicyWire) -> Response {
2318+
// Reject a conflicting request before establishing the boundary
2319+
// or retaining the caller's policy for later replay.
2320+
if let Err(error) = validate_vm_policy_identity(&self.config, &policy) {
2321+
return guest_error(BoundaryErrorKind::Denied, error);
2322+
}
22902323
let mut state = lock(&self.state);
22912324
let accepted = match &*state {
22922325
RuntimeState::AwaitingAttach => {
@@ -2479,6 +2512,11 @@ mod linux {
24792512
provider_env: std::collections::HashMap<String, String>,
24802513
provider_files: std::collections::HashMap<String, String>,
24812514
) -> Response {
2515+
// Check the supplied launch policy before installing materials or
2516+
// replacing its selectors with the measured driver's numeric pair.
2517+
if let Err(error) = validate_vm_policy_identity(&self.config, &policy) {
2518+
return guest_error(BoundaryErrorKind::Denied, error);
2519+
}
24822520
let spec = match resolve_agent_spec(spec) {
24832521
Ok(spec) => spec,
24842522
Err(error) => return guest_error(BoundaryErrorKind::Process, error),
@@ -4710,6 +4748,200 @@ mod linux {
47104748

47114749
validate_config(&config).unwrap();
47124750
validate_running_identity(&config.workload_identity, false).unwrap();
4751+
let mut wrong_user = config.workload_identity.clone();
4752+
wrong_user.uid = if wrong_user.uid == 10000 {
4753+
10001
4754+
} else {
4755+
10000
4756+
};
4757+
assert!(validate_running_identity(&wrong_user, false).is_err());
4758+
let mut wrong_group = config.workload_identity;
4759+
wrong_group.gid = if wrong_group.gid == 10000 {
4760+
10001
4761+
} else {
4762+
10000
4763+
};
4764+
assert!(validate_running_identity(&wrong_group, false).is_err());
4765+
}
4766+
4767+
fn vm_identity_test_runtime() -> (tokio::runtime::Runtime, Arc<BoundaryRuntime>) {
4768+
let process_runtime = tokio::runtime::Builder::new_multi_thread()
4769+
.worker_threads(2)
4770+
.enable_all()
4771+
.build()
4772+
.expect("test process runtime");
4773+
let mut boundary = {
4774+
let _entered = process_runtime.enter();
4775+
availability_test_runtime().0
4776+
};
4777+
let config = &mut Arc::get_mut(&mut boundary)
4778+
.expect("test boundary has one owner")
4779+
.config;
4780+
config
4781+
.resource_claims
4782+
.insert("vm.generation".to_string(), config.generation.clone());
4783+
(process_runtime, boundary)
4784+
}
4785+
4786+
fn vm_identity_test_policy(user: Option<&str>, group: Option<&str>) -> SandboxPolicyWire {
4787+
SandboxPolicyWire::from(openshell_core::policy::SandboxPolicy {
4788+
version: 1,
4789+
filesystem: openshell_core::policy::FilesystemPolicy::default(),
4790+
network: openshell_core::policy::NetworkPolicy::default(),
4791+
landlock: openshell_core::policy::LandlockPolicy::default(),
4792+
process: openshell_core::policy::ProcessPolicy {
4793+
run_as_user: user.map(str::to_string),
4794+
run_as_group: group.map(str::to_string),
4795+
},
4796+
})
4797+
}
4798+
4799+
#[test]
4800+
fn vm_policy_identity_checks_independent_selectors() {
4801+
let (_runtime, mut boundary) = vm_identity_test_runtime();
4802+
let uid = boundary.config.workload_identity.uid.to_string();
4803+
let gid = boundary.config.workload_identity.gid.to_string();
4804+
for (user, group) in [
4805+
(None, None),
4806+
(Some(""), Some("")),
4807+
(Some(uid.as_str()), None),
4808+
(None, Some(gid.as_str())),
4809+
(Some(uid.as_str()), Some(gid.as_str())),
4810+
(Some("sandbox"), Some(gid.as_str())),
4811+
(Some(uid.as_str()), Some("sandbox")),
4812+
(Some("sandbox"), Some("sandbox")),
4813+
] {
4814+
validate_vm_policy_identity(
4815+
&boundary.config,
4816+
&vm_identity_test_policy(user, group),
4817+
)
4818+
.expect("matching or omitted VM selectors");
4819+
}
4820+
let wrong_user = if uid == "10000" { "10001" } else { "10000" };
4821+
let wrong_group = if gid == "10000" { "10001" } else { "10000" };
4822+
for (user, group, field) in [
4823+
(Some(wrong_user), None, "run_as_user"),
4824+
(None, Some(wrong_group), "run_as_group"),
4825+
(Some(uid.as_str()), Some(wrong_group), "run_as_group"),
4826+
(Some(wrong_user), Some(gid.as_str()), "run_as_user"),
4827+
(Some(wrong_user), Some(wrong_group), "run_as_user"),
4828+
] {
4829+
let error = validate_vm_policy_identity(
4830+
&boundary.config,
4831+
&vm_identity_test_policy(user, group),
4832+
)
4833+
.expect_err("either mismatched selector must fail");
4834+
assert!(error.contains(field), "{error}");
4835+
assert!(error.contains(&format!("{uid}:{gid}")), "{error}");
4836+
}
4837+
for malformed in [
4838+
"root",
4839+
"-1",
4840+
"4294967296",
4841+
"1000:1000",
4842+
" sandbox",
4843+
"sandbox\n",
4844+
] {
4845+
for (user, group) in [(Some(malformed), None), (None, Some(malformed))] {
4846+
assert!(
4847+
validate_vm_policy_identity(
4848+
&boundary.config,
4849+
&vm_identity_test_policy(user, group),
4850+
)
4851+
.is_err(),
4852+
"invalid selector {malformed:?} must fail"
4853+
);
4854+
}
4855+
}
4856+
Arc::get_mut(&mut boundary)
4857+
.expect("test boundary has one owner")
4858+
.config
4859+
.resource_claims
4860+
.remove("vm.generation");
4861+
validate_vm_policy_identity(
4862+
&boundary.config,
4863+
&vm_identity_test_policy(Some(wrong_user), Some("image-user")),
4864+
)
4865+
.expect("non-VM identity behavior is unchanged");
4866+
}
4867+
4868+
#[test]
4869+
fn vm_attach_rejects_conflicting_identity_without_binding() {
4870+
let (_runtime, boundary) = vm_identity_test_runtime();
4871+
let identity = &boundary.config.workload_identity;
4872+
let uid = identity.uid.to_string();
4873+
let gid = identity.gid.to_string();
4874+
let wrong_user = if uid == "10000" { "10001" } else { "10000" };
4875+
let wrong_group = if gid == "10000" { "10001" } else { "10000" };
4876+
for (user, group, field, requested) in [
4877+
(Some(wrong_user), None, "run_as_user", wrong_user),
4878+
(None, Some(wrong_group), "run_as_group", wrong_group),
4879+
] {
4880+
let response = boundary.attach(vm_identity_test_policy(user, group));
4881+
let Response::Error { kind, message } = response else {
4882+
panic!("conflicting identity was attached: {response:?}");
4883+
};
4884+
assert_eq!(kind, BoundaryErrorKind::Denied);
4885+
assert!(
4886+
message.contains(&format!("{field} '{requested}'")),
4887+
"{message}"
4888+
);
4889+
assert!(message.contains(&format!("{uid}:{gid}")), "{message}");
4890+
assert!(matches!(
4891+
*lock(&boundary.state),
4892+
RuntimeState::AwaitingAttach
4893+
));
4894+
assert!(lock(&boundary.attached_policy).is_none());
4895+
}
4896+
assert!(matches!(
4897+
boundary.attach(vm_identity_test_policy(Some("sandbox"), Some("sandbox"))),
4898+
Response::Attached { .. }
4899+
));
4900+
}
4901+
4902+
#[test]
4903+
fn vm_start_rejects_conflicting_identity_without_launch() {
4904+
let (_runtime, boundary) = vm_identity_test_runtime();
4905+
let identity = &boundary.config.workload_identity;
4906+
let uid = identity.uid.to_string();
4907+
let gid = identity.gid.to_string();
4908+
let wrong_user = if uid == "10000" { "10001" } else { "10000" };
4909+
let wrong_group = if gid == "10000" { "10001" } else { "10000" };
4910+
*lock(&boundary.state) = RuntimeState::Ready(PreparedBoundary {
4911+
network_broker: boundary.network_broker.clone(),
4912+
});
4913+
for (user, group, field, requested) in [
4914+
(Some(wrong_user), None, "run_as_user", wrong_user),
4915+
(None, Some(wrong_group), "run_as_group", wrong_group),
4916+
] {
4917+
let response = boundary.start_agent(
4918+
boundary.config.boundary_id.clone(),
4919+
AgentSpecWire {
4920+
program: "/bin/true".to_string(),
4921+
args: Vec::new(),
4922+
workdir: None,
4923+
timeout_secs: 5,
4924+
interactive: false,
4925+
},
4926+
vm_identity_test_policy(user, group),
4927+
None,
4928+
None,
4929+
0,
4930+
std::collections::HashMap::new(),
4931+
std::collections::HashMap::new(),
4932+
);
4933+
let Response::Error { kind, message } = response else {
4934+
panic!("conflicting identity reached process start: {response:?}");
4935+
};
4936+
assert_eq!(kind, BoundaryErrorKind::Denied);
4937+
assert!(
4938+
message.contains(&format!("{field} '{requested}'")),
4939+
"{message}"
4940+
);
4941+
assert!(message.contains(&format!("{uid}:{gid}")), "{message}");
4942+
assert!(matches!(*lock(&boundary.state), RuntimeState::Ready(_)));
4943+
assert!(lock(&boundary.started_agent).is_none());
4944+
}
47134945
}
47144946

47154947
#[test]

‎crates/openshell-server/src/compute/mod.rs‎

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5992,15 +5992,31 @@ fn public_status_from_driver(
59925992
phase: SandboxPhase,
59935993
current_policy_version: u32,
59945994
) -> SandboxStatus {
5995+
let mut conditions = status
5996+
.conditions
5997+
.iter()
5998+
.map(public_condition_from_driver)
5999+
.collect::<Vec<_>>();
6000+
if let Some(identity) = &status.resolved_identity {
6001+
// Keep the requested policy intact. This condition reports the
6002+
// immutable runtime identity through existing CLI/API status views.
6003+
conditions.retain(|condition| condition.r#type != "WorkloadIdentity");
6004+
conditions.push(SandboxCondition {
6005+
r#type: "WorkloadIdentity".to_string(),
6006+
status: "True".to_string(),
6007+
reason: "DriverResolved".to_string(),
6008+
message: format!(
6009+
"Resolved workload UID:GID is {}:{}",
6010+
identity.uid, identity.gid
6011+
),
6012+
transition_time: None,
6013+
});
6014+
}
59956015
SandboxStatus {
59966016
agent_pod: status.instance_id.clone(),
59976017
agent_fd: status.agent_fd.clone(),
59986018
sandbox_fd: status.sandbox_fd.clone(),
5999-
conditions: status
6000-
.conditions
6001-
.iter()
6002-
.map(public_condition_from_driver)
6003-
.collect(),
6019+
conditions,
60046020
phase: phase as i32,
60056021
current_policy_version,
60066022
main_process_instance_id: String::new(),
@@ -9764,6 +9780,39 @@ mod tests {
97649780
}
97659781
}
97669782

9783+
#[test]
9784+
fn public_status_reports_driver_identity_without_rewriting_policy_or_readiness() {
9785+
let mut driver_status =
9786+
make_driver_status(make_driver_condition("Starting", "VM is starting"));
9787+
let unresolved = public_status_from_driver(&driver_status, SandboxPhase::Provisioning, 0);
9788+
assert!(
9789+
!unresolved
9790+
.conditions
9791+
.iter()
9792+
.any(|condition| condition.r#type == "WorkloadIdentity")
9793+
);
9794+
driver_status.resolved_identity = Some(
9795+
openshell_core::proto::compute::v1::ResolvedWorkloadIdentity {
9796+
uid: 1000,
9797+
gid: 1001,
9798+
source: "vm-config".into(),
9799+
resource_digest: "sha256:image".into(),
9800+
..Default::default()
9801+
},
9802+
);
9803+
let status = public_status_from_driver(&driver_status, SandboxPhase::Provisioning, 0);
9804+
assert_eq!(status.phase, SandboxPhase::Provisioning as i32);
9805+
assert_eq!(status.current_policy_version, 0);
9806+
assert_eq!(status.conditions[0], unresolved.conditions[0]);
9807+
let identity = status
9808+
.conditions
9809+
.iter()
9810+
.find(|condition| condition.r#type == "WorkloadIdentity")
9811+
.unwrap();
9812+
assert_eq!(identity.reason, "DriverResolved");
9813+
assert_eq!(identity.message, "Resolved workload UID:GID is 1000:1001");
9814+
}
9815+
97679816
fn ready_driver_sandbox(id: &str, name: &str) -> DriverSandbox {
97689817
DriverSandbox {
97699818
id: id.to_string(),

‎crates/openshell-server/src/grpc/validation.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2403,6 +2403,31 @@ mod tests {
24032403

24042404
// ---- Exec validation ----
24052405

2406+
#[test]
2407+
fn validate_static_fields_rejects_independent_process_identity_changes() {
2408+
let baseline = ProtoSandboxPolicy {
2409+
process: Some(openshell_core::proto::ProcessPolicy {
2410+
run_as_user: "sandbox".into(),
2411+
run_as_group: "1001".into(),
2412+
}),
2413+
..Default::default()
2414+
};
2415+
for (user, group) in [
2416+
("10000", "1001"),
2417+
("sandbox", "10001"),
2418+
("", "1001"),
2419+
("sandbox", ""),
2420+
] {
2421+
let mut changed = baseline.clone();
2422+
changed.process = Some(openshell_core::proto::ProcessPolicy {
2423+
run_as_user: user.into(),
2424+
run_as_group: group.into(),
2425+
});
2426+
let error = validate_static_fields_unchanged(&baseline, &changed).unwrap_err();
2427+
assert!(error.message().contains("process policy cannot be changed"));
2428+
}
2429+
}
2430+
24062431
#[test]
24072432
fn reject_control_chars_allows_normal_values() {
24082433
assert!(reject_control_chars("hello world", "test").is_ok());

0 commit comments

Comments
 (0)