diff --git a/crates/openshell-sandbox-backend/src/runtime.rs b/crates/openshell-sandbox-backend/src/runtime.rs index bc687e1b80..2675a19291 100644 --- a/crates/openshell-sandbox-backend/src/runtime.rs +++ b/crates/openshell-sandbox-backend/src/runtime.rs @@ -272,10 +272,6 @@ impl OpenShellRuntimeBackend { }; validate_runtime_descriptor(&runtime_descriptor, &sandbox)?; let host_gateway_ip = runtime_descriptor.host_gateway_ip; - let resource_claims = runtime_descriptor.resource_claims.clone(); - let generation = runtime_descriptor.generation.clone(); - let session_id = runtime_descriptor.session_id; - let provisioning_evidence = runtime_descriptor.provisioning_evidence.clone(); let mut client = BoundaryClient::with_backend( runtime_descriptor, self.sandbox_bearer.clone(), @@ -290,14 +286,7 @@ impl OpenShellRuntimeBackend { policy: Box::new(SandboxPolicyWire::from(sandbox.policy.clone())), }) .await?; - let Response::Attached { snapshot } = response else { - return Err(unexpected_response("attached", &response)); - }; - if snapshot.generation != generation { - return Err(BackendError::Confirm( - "sandbox session snapshot generation does not match runtime descriptor".to_string(), - )); - } + client.check_attached(response)?; Ok(Box::new(BoundBoundary { client: client.clone(), agent: sandbox.agent, @@ -307,10 +296,6 @@ impl OpenShellRuntimeBackend { ca_file_paths: self.ca_file_paths.clone(), provider_credentials: self.provider_credentials.clone(), identity: sandbox.identity, - generation, - session_id, - resource_claims, - provisioning_evidence, })) } } @@ -417,10 +402,6 @@ pub struct BoundBoundary { ca_file_paths: Arc>>, provider_credentials: openshell_core::provider_credentials::ProviderCredentialState, identity: ResolvedWorkloadIdentity, - generation: String, - session_id: openshell_core::SandboxSessionId, - resource_claims: std::collections::BTreeMap, - provisioning_evidence: openshell_isolation_interface::contract::ProvisioningEvidence, } impl BoundBoundary { @@ -434,20 +415,7 @@ impl BoundBoundary { pub async fn confirm(self: Box) -> Result { let response = self.client.call_idempotent(Request::Confirm).await?; - let Response::Confirmed { confirmation } = response else { - return Err(unexpected_response("confirmed", &response)); - }; - if confirmation.generation != self.generation - || confirmation.session_id != self.session_id - || confirmation.resource_claims != self.resource_claims - || confirmation.provisioning_evidence_digest - != self.provisioning_evidence.evidence_digest - { - return Err(BackendError::Confirm( - "sandbox confirmation generation, session, resource claims, or provisioning evidence do not match runtime descriptor" - .to_string(), - )); - } + let confirmation = self.client.launch_confirmation(response)?; if self.client.backend_name == crate::BACKEND_NAME { crate::audit::validate_native_linux_audit(&confirmation.backend_audit)?; } @@ -1349,6 +1317,51 @@ impl BoundaryClient { } } + /// Check an Attach reply, including replays on reconnect, against the launch. + fn check_attached(&self, response: Response) -> Result<(), BackendError> { + let Response::Attached { snapshot } = response else { + return Err(unexpected_response("attached", &response)); + }; + if snapshot.generation != self.runtime_descriptor.generation { + return Err(BackendError::Confirm( + "sandbox session snapshot generation does not match runtime descriptor".to_string(), + )); + } + Ok(()) + } + + /// Check a Confirm reply against the launch. Reconnect, credential renewal + /// and liveness probes replay Confirm, so every reply gets the same check. + fn launch_confirmation( + &self, + response: Response, + ) -> Result, BackendError> { + let Response::Confirmed { confirmation } = response else { + return Err(unexpected_response("confirmed", &response)); + }; + let launch = &self.runtime_descriptor; + let mismatch = if confirmation.identity != launch.workload_identity { + "identity" + } else if confirmation.generation.is_empty() { + "generation (empty)" + } else if confirmation.generation != launch.generation { + "generation" + } else if confirmation.session_id != launch.session_id { + "session" + } else if confirmation.resource_claims != launch.resource_claims { + "resource claims" + } else if confirmation.provisioning_evidence_digest + != launch.provisioning_evidence.evidence_digest + { + "provisioning evidence" + } else { + return Ok(confirmation); + }; + Err(BackendError::Confirm(format!( + "sandbox confirmation {mismatch} does not match runtime descriptor" + ))) + } + async fn acquire_network_stream(&self) -> tokio::sync::OwnedSemaphorePermit { let acquire = self.network_streams.clone().acquire_owned(); tokio::pin!(acquire); @@ -1641,7 +1654,12 @@ impl BoundaryClient { .map_err(|_| { BackendError::Unavailable("boundary credential renewal timed out".to_string()) })??; - expect_response(response, "confirmed")?; + if let Err(error) = self.launch_confirmation(response) { + // A backend answering for another launch keeps no live streams. + cached.transport.abort(); + *self.mediation.lock().await = None; + return Err(error); + } if let Some(current) = self.grpc_channel.lock().await.as_mut() && current.generation == cached.generation { @@ -1763,14 +1781,15 @@ impl BoundaryClient { let Ok(credential) = BoundaryCredential::capture(&self.sandbox_bearer) else { return false; }; - matches!( - tokio::time::timeout( - CONNECTION_PROBE_TIMEOUT, - self.exchange_on_channel(cached.channel.clone(), confirm, &credential), - ) - .await, - Ok(Ok(Response::Confirmed { .. })) + let Ok(Ok(response)) = tokio::time::timeout( + CONNECTION_PROBE_TIMEOUT, + self.exchange_on_channel(cached.channel.clone(), confirm, &credential), ) + .await + else { + return false; + }; + self.launch_confirmation(response).is_ok() } /// Open a new physical connection and replay the authenticated lifecycle, @@ -1783,11 +1802,15 @@ impl BoundaryClient { ) -> Result<(tonic::transport::Channel, Arc), BackendError> { let (channel, transport) = self.build_grpc_channel(credential).await?; let result = async { - self.exchange_on_channel(channel.clone(), attach, credential) + let attached = self + .exchange_on_channel(channel.clone(), attach, credential) .await?; + self.check_attached(attached)?; if let Some(confirm) = confirm { - self.exchange_on_channel(channel.clone(), confirm, credential) + let confirmed = self + .exchange_on_channel(channel.clone(), confirm, credential) .await?; + self.launch_confirmation(confirmed)?; } Ok(()) } @@ -2354,9 +2377,7 @@ fn connect_host_vsock( fn expect_response(response: Response, expected: &str) -> Result<(), BackendError> { let matches = matches!( (&response, expected), - (Response::Attached { .. }, "attached") - | (Response::Confirmed { .. }, "confirmed") - | (Response::Signaled, "signaled") + (Response::Signaled, "signaled") | (Response::Terminated, "terminated") | (Response::BoundaryTerminated, "boundary_terminated") ); @@ -2689,6 +2710,7 @@ mod tests { #[derive(Clone)] struct TestGrpcBoundary { + attach_generation: String, wait_for_half_close: bool, expected_token: String, requests: Arc, @@ -2722,6 +2744,7 @@ mod tests { let mediation_ready = self.mediation_ready; let provider_environment_generation = self.provider_environment_generation; let confirmation = self.confirmation.clone(); + let attach_generation = self.attach_generation.clone(); let (outbound, outbound_rx) = tokio::sync::mpsc::channel(1); tokio::spawn(async move { let mut frame = Vec::new(); @@ -2764,7 +2787,7 @@ mod tests { }, Request::Attach { .. } => Response::Attached { snapshot: crate::boundary_protocol::SessionSnapshotWire { - generation: "test-generation".to_string(), + generation: attach_generation, processes: Vec::new(), }, }, @@ -2891,6 +2914,7 @@ mod tests { let address = listener.local_addr().unwrap(); let requests = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token: "a".repeat(32), requests: requests.clone(), @@ -2965,6 +2989,7 @@ mod tests { .unwrap(); server_accepted.fetch_add(1, Ordering::AcqRel); let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token: "a".repeat(32), requests: server_requests.clone(), @@ -3076,6 +3101,7 @@ mod tests { .await .unwrap(); let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token: "a".repeat(32), requests: server_requests.clone(), @@ -3131,6 +3157,7 @@ mod tests { let address = listener.local_addr().unwrap(); let requests = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: true, expected_token: "a".repeat(32), requests, @@ -3330,6 +3357,7 @@ mod tests { confirmation: BoundaryConfirmation, ) { let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token, requests: Arc::new(std::sync::atomic::AtomicUsize::new(0)), @@ -3438,7 +3466,6 @@ mod tests { }); let descriptor = tls_runtime_descriptor(address, certificate.client_tls); - let provisioning_evidence = descriptor.provisioning_evidence.clone(); let context = sandbox(); let client = Arc::new(BoundaryClient::new( descriptor, @@ -3461,10 +3488,6 @@ mod tests { HashMap::new(), ), identity: context.identity, - generation: "test-generation".to_string(), - session_id: test_session_id(), - resource_claims: std::collections::BTreeMap::new(), - provisioning_evidence, }; assert!(matches!( @@ -3848,6 +3871,7 @@ mod tests { let accepted_by_server = accepted.clone(); let handled = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token: "a".repeat(32), requests: handled.clone(), @@ -3921,6 +3945,7 @@ mod tests { let address = listener.local_addr().unwrap(); let requests = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token: "a".repeat(32), requests: requests.clone(), diff --git a/crates/openshell-sandbox-backend/src/runtime/tests/credential_renewal.rs b/crates/openshell-sandbox-backend/src/runtime/tests/credential_renewal.rs index 0c5320a46b..8b49e99f31 100644 --- a/crates/openshell-sandbox-backend/src/runtime/tests/credential_renewal.rs +++ b/crates/openshell-sandbox-backend/src/runtime/tests/credential_renewal.rs @@ -10,6 +10,8 @@ struct RenewingGrpcBoundary { expected_token: Arc>, failures: Arc, failure_code: tonic::Code, + confirmation: Arc>, + attach_generation: Arc>, } #[tonic::async_trait] @@ -35,6 +37,8 @@ impl SandboxProtocol for RenewingGrpcBoundary { } let mut inner = self.inner.clone(); inner.expected_token = self.expected_token.read().unwrap().clone(); + inner.confirmation = self.confirmation.read().unwrap().clone(); + inner.attach_generation = self.attach_generation.read().unwrap().clone(); inner.exchange(request).await } @@ -62,6 +66,7 @@ async fn renewing_boundary_client( let accepted = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let service = RenewingGrpcBoundary { inner: TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token: "a".repeat(32), requests: Arc::new(std::sync::atomic::AtomicUsize::new(0)), @@ -73,6 +78,8 @@ async fn renewing_boundary_client( expected_token: Arc::new(std::sync::RwLock::new("a".repeat(32))), failures: Arc::new(std::sync::atomic::AtomicUsize::new(0)), failure_code, + confirmation: Arc::new(std::sync::RwLock::new(test_confirmation())), + attach_generation: Arc::new(std::sync::RwLock::new("test-generation".to_string())), }; let server_service = service.clone(); let server_accepted = accepted.clone(); @@ -249,3 +256,139 @@ async fn confirmation_authenticates_captured_bearer_after_slot_changes() { assert_eq!(accepted.load(Ordering::Acquire), 1); server.abort(); } + +type Mismatch = (&'static str, &'static str, fn(&mut BoundaryConfirmation)); + +/// A backend that loses its launch state answers with a confirmation that no +/// longer matches the launch. Each case names the field the error must report. +const CONFIRMATION_MISMATCHES: &[Mismatch] = &[ + ("identity", "identity", |c| { + c.identity = + ResolvedWorkloadIdentity::new(10_002, 10_001, Vec::new(), "test".into(), "x".into()) + .unwrap(); + }), + ("empty generation", "generation (empty)", |c| { + c.generation.clear(); + }), + ("generation", "generation", |c| { + c.generation = "other-generation".into(); + }), + ("session", "session", |c| { + c.session_id = "550e8400-e29b-41d4-a716-446655440001".parse().unwrap(); + }), + ("resource claims", "resource claims", |c| { + c.resource_claims.insert("claim".into(), "other".into()); + }), + ("provisioning evidence", "provisioning evidence", |c| { + c.provisioning_evidence_digest = openshell_isolation_interface::contract::ProvisioningEvidence::from_enforcement_evidence( + "test-generation", + b"other-evidence", + ) + .unwrap() + .evidence_digest; + }), +]; + +#[tokio::test] +async fn renewal_rejects_mismatched_confirmation() { + for (case, field, mutate) in CONFIRMATION_MISMATCHES { + let (client, service, accepted, server) = + renewing_boundary_client(tonic::Code::Unavailable).await; + let original = client.grpc_channel.lock().await.clone().unwrap(); + mutate(&mut service.confirmation.write().unwrap()); + let renewed_token = "b".repeat(32); + *service.expected_token.write().unwrap() = renewed_token.clone(); + client + .sandbox_bearer + .update( + SecretJwt::parse(renewed_token).unwrap(), + i64::MAX, + openshell_core::jwt::CredentialEpoch::new(1).unwrap(), + ) + .unwrap(); + + let result = client.ensure_current_credential_connection().await; + assert!( + matches!(&result, Err(BackendError::Confirm(message)) if message.contains(&format!("confirmation {field} does not match"))), + "{case}: renewal must reject the confirmation: {result:?}" + ); + let failed = client.grpc_channel.lock().await.clone().unwrap(); + assert_eq!(failed.bearer_fingerprint, original.bearer_fingerprint); + assert!( + failed.transport.is_aborted(), + "{case}: a mismatched renewal must close the connection's live streams" + ); + assert_eq!(accepted.load(Ordering::Acquire), 1); + server.abort(); + } +} + +#[tokio::test] +async fn reconnect_rejects_mismatched_confirmation() { + for (case, field, mutate) in CONFIRMATION_MISMATCHES { + let (client, service, accepted, server) = + renewing_boundary_client(tonic::Code::Unavailable).await; + mutate(&mut service.confirmation.write().unwrap()); + + // The liveness probe must not keep the connection, and the replacement + // connection's replayed Confirm must not be accepted either. + let result = client + .recover_after_unavailable(client.connection_generation().await) + .await; + assert!( + matches!(&result, Err(BackendError::Confirm(message)) if message.contains(&format!("confirmation {field} does not match"))), + "{case}: reconnect must reject the confirmation: {result:?}" + ); + assert_eq!(accepted.load(Ordering::Acquire), 2); + server.abort(); + } +} + +#[tokio::test] +async fn launch_checks_reject_wrong_response_variants() { + let (client, _service, _accepted, server) = + renewing_boundary_client(tonic::Code::Unavailable).await; + let confirmed = || Response::Confirmed { + confirmation: Box::new(test_confirmation()), + }; + let attached = || Response::Attached { + snapshot: crate::boundary_protocol::SessionSnapshotWire { + generation: "test-generation".to_string(), + processes: Vec::new(), + }, + }; + assert!(client.check_attached(attached()).is_ok()); + assert!(client.launch_confirmation(confirmed()).is_ok()); + for wrong in [confirmed(), Response::Signaled, Response::Terminated] { + assert!(matches!( + client.check_attached(wrong), + Err(BackendError::Process(message)) if message.contains("\"attached\"") + )); + } + for wrong in [attached(), Response::Signaled, Response::Terminated] { + assert!(matches!( + client.launch_confirmation(wrong), + Err(BackendError::Process(message)) if message.contains("\"confirmed\"") + )); + } + server.abort(); +} + +#[tokio::test] +async fn reconnect_rejects_attach_for_another_generation() { + let (client, service, accepted, server) = + renewing_boundary_client(tonic::Code::Unavailable).await; + let cached = client.grpc_channel.lock().await.clone().unwrap(); + cached.transport.abort(); + *service.attach_generation.write().unwrap() = "other-generation".to_string(); + + let result = client + .recover_after_unavailable(client.connection_generation().await) + .await; + assert!( + matches!(&result, Err(BackendError::Confirm(message)) if message.contains("snapshot generation")), + "reconnect must reject an Attach for another generation: {result:?}" + ); + assert_eq!(accepted.load(Ordering::Acquire), 2); + server.abort(); +} diff --git a/crates/openshell-sandbox-backend/src/runtime/tests/delegated.rs b/crates/openshell-sandbox-backend/src/runtime/tests/delegated.rs index 86af9ab75e..c55090d387 100644 --- a/crates/openshell-sandbox-backend/src/runtime/tests/delegated.rs +++ b/crates/openshell-sandbox-backend/src/runtime/tests/delegated.rs @@ -27,6 +27,7 @@ async fn external_backend( .await .unwrap(); let service = TestGrpcBoundary { + attach_generation: "test-generation".to_string(), wait_for_half_close: false, expected_token: "a".repeat(32), requests: Arc::default(), diff --git a/proto/sandbox_protocol.proto b/proto/sandbox_protocol.proto index 40a35cf096..971bc0208f 100644 --- a/proto/sandbox_protocol.proto +++ b/proto/sandbox_protocol.proto @@ -66,7 +66,11 @@ message StdinClosed {} // negotiating the receiver's supporting capability. No client digest required. // Replayable mutations: Attach, Confirm, StartAgent, UpdateProviderEnvironment, // Exec, Signal, Terminate, TerminateBoundary, and Resize. Exec outcomes remain -// retained through the deadline even after process exit. +// retained through the deadline even after process exit. Attach and Confirm +// replays must return the original result for the whole generation: after +// confirmation, the supervisor replays both on every reconnect, and Confirm +// alone to renew its credential and probe liveness. It checks every reply +// against the launch. message RequestEnvelope { string request_id = 1; oneof request {