From 826a94f27dec1a952d4860e11c6b53a1b94333ed Mon Sep 17 00:00:00 2001 From: Shiju Date: Mon, 19 Oct 2026 09:11:31 +0530 Subject: [PATCH 1/2] fix(sandbox): check every backend confirmation against the launch The delegated client compared a backend's Confirmed with the launch binding only on the first confirmation. Credential renewal, the liveness probe and reconnect accepted any Confirmed, and reconnect ignored the replayed Attach reply. A backend that lost its launch state could answer for another session and keep serving the sandbox. Check Attach and Confirm replies against the client's runtime descriptor on every path. Close the connection when a same-epoch renewal returns another launch. Document that Attach and Confirm must stay replayable for the whole generation. Closes #4703 Signed-off-by: Shiju --- .../openshell-sandbox-backend/src/runtime.rs | 131 +++++++++------- .../src/runtime/tests/credential_renewal.rs | 143 ++++++++++++++++++ .../src/runtime/tests/delegated.rs | 1 + proto/sandbox_protocol.proto | 5 +- 4 files changed, 226 insertions(+), 54 deletions(-) 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..5944d8d039 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(field)), + "{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(field)), + "{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..7e12e3cce1 100644 --- a/proto/sandbox_protocol.proto +++ b/proto/sandbox_protocol.proto @@ -66,7 +66,10 @@ 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 succeed for the whole generation: after confirmation, the supervisor +// replays both on every reconnect, and Confirm alone to renew its credential +// and probe liveness. The client checks every reply against the launch. message RequestEnvelope { string request_id = 1; oneof request { From ddb9f71641fd4d7232755a983150204a99eb954a Mon Sep 17 00:00:00 2001 From: Shiju Date: Wed, 21 Oct 2026 12:32:26 +0530 Subject: [PATCH 2/2] fix(sandbox): state the replay contract and match exact mismatch fields Attach and Confirm replays must return the original result for the whole generation. The renewal and reconnect tests now match the full mismatch phrase, so the generation case cannot pass on the empty-generation error. Signed-off-by: Shiju --- .../src/runtime/tests/credential_renewal.rs | 4 ++-- proto/sandbox_protocol.proto | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) 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 5944d8d039..8b49e99f31 100644 --- a/crates/openshell-sandbox-backend/src/runtime/tests/credential_renewal.rs +++ b/crates/openshell-sandbox-backend/src/runtime/tests/credential_renewal.rs @@ -309,7 +309,7 @@ async fn renewal_rejects_mismatched_confirmation() { let result = client.ensure_current_credential_connection().await; assert!( - matches!(&result, Err(BackendError::Confirm(message)) if message.contains(field)), + 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(); @@ -336,7 +336,7 @@ async fn reconnect_rejects_mismatched_confirmation() { .recover_after_unavailable(client.connection_generation().await) .await; assert!( - matches!(&result, Err(BackendError::Confirm(message)) if message.contains(field)), + 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); diff --git a/proto/sandbox_protocol.proto b/proto/sandbox_protocol.proto index 7e12e3cce1..971bc0208f 100644 --- a/proto/sandbox_protocol.proto +++ b/proto/sandbox_protocol.proto @@ -67,9 +67,10 @@ message StdinClosed {} // Replayable mutations: Attach, Confirm, StartAgent, UpdateProviderEnvironment, // Exec, Signal, Terminate, TerminateBoundary, and Resize. Exec outcomes remain // retained through the deadline even after process exit. Attach and Confirm -// replays succeed for the whole generation: after confirmation, the supervisor -// replays both on every reconnect, and Confirm alone to renew its credential -// and probe liveness. The client checks every reply against the launch. +// 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 {