Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
426 changes: 393 additions & 33 deletions crates/openshell-driver-vm/src/driver.rs

Large diffs are not rendered by default.

232 changes: 232 additions & 0 deletions crates/openshell-sandbox/src/boundary_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,34 @@ mod linux {
Ok(())
}

/// VM selectors assert the protected overlay owner; they cannot choose a
/// replacement identity. Guest init maps `sandbox` to that owner before
/// launching this boundary, so no account lookup or privilege change belongs here.
fn validate_vm_policy_identity(
config: &BoundaryConfig,
policy: &SandboxPolicyWire,
) -> Result<(), String> {
if !config.resource_claims.contains_key("vm.generation") {
return Ok(());
}
let identity = &config.workload_identity;
for (field, selector, expected) in [
("run_as_user", policy.run_as_user.as_deref(), identity.uid),
("run_as_group", policy.run_as_group.as_deref(), identity.gid),
] {
let Some(selector) = selector.filter(|value| !value.is_empty()) else {
continue;
};
if selector != "sandbox" && selector.parse::<u32>() != Ok(expected) {
return Err(format!(
"VM {field} '{selector}' conflicts with the resolved workload identity {}:{}; omit the selector or request the driver-owned identity",
identity.uid, identity.gid
));
}
}
Ok(())
}

fn tls_paths_are_absolute(
tls: &openshell_sandbox_backend::boundary_protocol::SandboxTlsServerConfig,
) -> bool {
Expand Down Expand Up @@ -2247,6 +2275,11 @@ mod linux {
}

fn attach(&self, policy: SandboxPolicyWire) -> Response {
// Reject a conflicting request before establishing the boundary
// or retaining the caller's policy for later replay.
if let Err(error) = validate_vm_policy_identity(&self.config, &policy) {
return guest_error(BoundaryErrorKind::Denied, error);
}
let mut state = lock(&self.state);
let accepted = match &*state {
RuntimeState::AwaitingAttach => {
Expand Down Expand Up @@ -2439,6 +2472,11 @@ mod linux {
provider_env: std::collections::HashMap<String, String>,
provider_files: std::collections::HashMap<String, String>,
) -> Response {
// Check the supplied launch policy before installing materials or
// replacing its selectors with the measured driver's numeric pair.
if let Err(error) = validate_vm_policy_identity(&self.config, &policy) {
return guest_error(BoundaryErrorKind::Denied, error);
}
let spec = match resolve_agent_spec(spec) {
Ok(spec) => spec,
Err(error) => return guest_error(BoundaryErrorKind::Process, error),
Expand Down Expand Up @@ -4643,6 +4681,200 @@ mod linux {

validate_config(&config).unwrap();
validate_running_identity(&config.workload_identity, false).unwrap();
let mut wrong_user = config.workload_identity.clone();
wrong_user.uid = if wrong_user.uid == 10000 {
10001
} else {
10000
};
assert!(validate_running_identity(&wrong_user, false).is_err());
let mut wrong_group = config.workload_identity;
wrong_group.gid = if wrong_group.gid == 10000 {
10001
} else {
10000
};
assert!(validate_running_identity(&wrong_group, false).is_err());
}

fn vm_identity_test_runtime() -> (tokio::runtime::Runtime, Arc<BoundaryRuntime>) {
let process_runtime = tokio::runtime::Builder::new_multi_thread()
.worker_threads(2)
.enable_all()
.build()
.expect("test process runtime");
let mut boundary = {
let _entered = process_runtime.enter();
availability_test_runtime().0
};
let config = &mut Arc::get_mut(&mut boundary)
.expect("test boundary has one owner")
.config;
config
.resource_claims
.insert("vm.generation".to_string(), config.generation.clone());
(process_runtime, boundary)
}

fn vm_identity_test_policy(user: Option<&str>, group: Option<&str>) -> SandboxPolicyWire {
SandboxPolicyWire::from(openshell_core::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 {
run_as_user: user.map(str::to_string),
run_as_group: group.map(str::to_string),
},
})
}

#[test]
fn vm_policy_identity_checks_independent_selectors() {
let (_runtime, mut boundary) = vm_identity_test_runtime();
let uid = boundary.config.workload_identity.uid.to_string();
let gid = boundary.config.workload_identity.gid.to_string();
for (user, group) in [
(None, None),
(Some(""), Some("")),
(Some(uid.as_str()), None),
(None, Some(gid.as_str())),
(Some(uid.as_str()), Some(gid.as_str())),
(Some("sandbox"), Some(gid.as_str())),
(Some(uid.as_str()), Some("sandbox")),
(Some("sandbox"), Some("sandbox")),
] {
validate_vm_policy_identity(
&boundary.config,
&vm_identity_test_policy(user, group),
)
.expect("matching or omitted VM selectors");
}
let wrong_user = if uid == "10000" { "10001" } else { "10000" };
let wrong_group = if gid == "10000" { "10001" } else { "10000" };
for (user, group, field) in [
(Some(wrong_user), None, "run_as_user"),
(None, Some(wrong_group), "run_as_group"),
(Some(uid.as_str()), Some(wrong_group), "run_as_group"),
(Some(wrong_user), Some(gid.as_str()), "run_as_user"),
(Some(wrong_user), Some(wrong_group), "run_as_user"),
] {
let error = validate_vm_policy_identity(
&boundary.config,
&vm_identity_test_policy(user, group),
)
.expect_err("either mismatched selector must fail");
assert!(error.contains(field), "{error}");
assert!(error.contains(&format!("{uid}:{gid}")), "{error}");
}
for malformed in [
"root",
"-1",
"4294967296",
"1000:1000",
" sandbox",
"sandbox\n",
] {
for (user, group) in [(Some(malformed), None), (None, Some(malformed))] {
assert!(
validate_vm_policy_identity(
&boundary.config,
&vm_identity_test_policy(user, group),
)
.is_err(),
"invalid selector {malformed:?} must fail"
);
}
}
Arc::get_mut(&mut boundary)
.expect("test boundary has one owner")
.config
.resource_claims
.remove("vm.generation");
validate_vm_policy_identity(
&boundary.config,
&vm_identity_test_policy(Some(wrong_user), Some("image-user")),
)
.expect("non-VM identity behavior is unchanged");
}

#[test]
fn vm_attach_rejects_conflicting_identity_without_binding() {
let (_runtime, boundary) = vm_identity_test_runtime();
let identity = &boundary.config.workload_identity;
let uid = identity.uid.to_string();
let gid = identity.gid.to_string();
let wrong_user = if uid == "10000" { "10001" } else { "10000" };
let wrong_group = if gid == "10000" { "10001" } else { "10000" };
for (user, group, field, requested) in [
(Some(wrong_user), None, "run_as_user", wrong_user),
(None, Some(wrong_group), "run_as_group", wrong_group),
] {
let response = boundary.attach(vm_identity_test_policy(user, group));
let Response::Error { kind, message } = response else {
panic!("conflicting identity was attached: {response:?}");
};
assert_eq!(kind, BoundaryErrorKind::Denied);
assert!(
message.contains(&format!("{field} '{requested}'")),
"{message}"
);
assert!(message.contains(&format!("{uid}:{gid}")), "{message}");
assert!(matches!(
*lock(&boundary.state),
RuntimeState::AwaitingAttach
));
assert!(lock(&boundary.attached_policy).is_none());
}
assert!(matches!(
boundary.attach(vm_identity_test_policy(Some("sandbox"), Some("sandbox"))),
Response::Attached { .. }
));
}

#[test]
fn vm_start_rejects_conflicting_identity_without_launch() {
let (_runtime, boundary) = vm_identity_test_runtime();
let identity = &boundary.config.workload_identity;
let uid = identity.uid.to_string();
let gid = identity.gid.to_string();
let wrong_user = if uid == "10000" { "10001" } else { "10000" };
let wrong_group = if gid == "10000" { "10001" } else { "10000" };
*lock(&boundary.state) = RuntimeState::Ready(PreparedBoundary {
network_broker: boundary.network_broker.clone(),
});
for (user, group, field, requested) in [
(Some(wrong_user), None, "run_as_user", wrong_user),
(None, Some(wrong_group), "run_as_group", wrong_group),
] {
let response = boundary.start_agent(
boundary.config.boundary_id.clone(),
AgentSpecWire {
program: "/bin/true".to_string(),
args: Vec::new(),
workdir: None,
timeout_secs: 5,
interactive: false,
},
vm_identity_test_policy(user, group),
None,
None,
0,
std::collections::HashMap::new(),
std::collections::HashMap::new(),
);
let Response::Error { kind, message } = response else {
panic!("conflicting identity reached process start: {response:?}");
};
assert_eq!(kind, BoundaryErrorKind::Denied);
assert!(
message.contains(&format!("{field} '{requested}'")),
"{message}"
);
assert!(message.contains(&format!("{uid}:{gid}")), "{message}");
assert!(matches!(*lock(&boundary.state), RuntimeState::Ready(_)));
assert!(lock(&boundary.started_agent).is_none());
}
}

#[test]
Expand Down
59 changes: 54 additions & 5 deletions crates/openshell-server/src/compute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5992,15 +5992,31 @@ fn public_status_from_driver(
phase: SandboxPhase,
current_policy_version: u32,
) -> SandboxStatus {
let mut conditions = status
.conditions
.iter()
.map(public_condition_from_driver)
.collect::<Vec<_>>();
if let Some(identity) = &status.resolved_identity {
// Keep the requested policy intact. This condition reports the
// immutable runtime identity through existing CLI/API status views.
conditions.retain(|condition| condition.r#type != "WorkloadIdentity");
conditions.push(SandboxCondition {
r#type: "WorkloadIdentity".to_string(),
status: "True".to_string(),
reason: "DriverResolved".to_string(),
message: format!(
"Resolved workload UID:GID is {}:{}",
identity.uid, identity.gid
),
transition_time: None,
});
}
SandboxStatus {
agent_pod: status.instance_id.clone(),
agent_fd: status.agent_fd.clone(),
sandbox_fd: status.sandbox_fd.clone(),
conditions: status
.conditions
.iter()
.map(public_condition_from_driver)
.collect(),
conditions,
phase: phase as i32,
current_policy_version,
main_process_instance_id: String::new(),
Expand Down Expand Up @@ -9764,6 +9780,39 @@ mod tests {
}
}

#[test]
fn public_status_reports_driver_identity_without_rewriting_policy_or_readiness() {
let mut driver_status =
make_driver_status(make_driver_condition("Starting", "VM is starting"));
let unresolved = public_status_from_driver(&driver_status, SandboxPhase::Provisioning, 0);
assert!(
!unresolved
.conditions
.iter()
.any(|condition| condition.r#type == "WorkloadIdentity")
);
driver_status.resolved_identity = Some(
openshell_core::proto::compute::v1::ResolvedWorkloadIdentity {
uid: 1000,
gid: 1001,
source: "vm-config".into(),
resource_digest: "sha256:image".into(),
..Default::default()
},
);
let status = public_status_from_driver(&driver_status, SandboxPhase::Provisioning, 0);
assert_eq!(status.phase, SandboxPhase::Provisioning as i32);
assert_eq!(status.current_policy_version, 0);
assert_eq!(status.conditions[0], unresolved.conditions[0]);
let identity = status
.conditions
.iter()
.find(|condition| condition.r#type == "WorkloadIdentity")
.unwrap();
assert_eq!(identity.reason, "DriverResolved");
assert_eq!(identity.message, "Resolved workload UID:GID is 1000:1001");
}

fn ready_driver_sandbox(id: &str, name: &str) -> DriverSandbox {
DriverSandbox {
id: id.to_string(),
Expand Down
25 changes: 25 additions & 0 deletions crates/openshell-server/src/grpc/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2403,6 +2403,31 @@ mod tests {

// ---- Exec validation ----

#[test]
fn validate_static_fields_rejects_independent_process_identity_changes() {
let baseline = ProtoSandboxPolicy {
process: Some(openshell_core::proto::ProcessPolicy {
run_as_user: "sandbox".into(),
run_as_group: "1001".into(),
}),
..Default::default()
};
for (user, group) in [
("10000", "1001"),
("sandbox", "10001"),
("", "1001"),
("sandbox", ""),
] {
let mut changed = baseline.clone();
changed.process = Some(openshell_core::proto::ProcessPolicy {
run_as_user: user.into(),
run_as_group: group.into(),
});
let error = validate_static_fields_unchanged(&baseline, &changed).unwrap_err();
assert!(error.message().contains("process policy cannot be changed"));
}
}

#[test]
fn reject_control_chars_allows_normal_values() {
assert!(reject_control_chars("hello world", "test").is_ok());
Expand Down
Loading
Loading