From 4c4cf21dbdd2555f8ba78c1908999ca0ed1b7d78 Mon Sep 17 00:00:00 2001 From: James Price Date: Tue, 8 Sep 2026 13:26:51 +0100 Subject: [PATCH 1/3] fix(s3): decrypt SSE-KMS bodies for internal object readers Objects in an SSE-KMS bucket are stored as a fakecloud-kms envelope, and only the S3 API's own read paths unwrapped it. Every internal reader took the stored bytes straight off S3 state: the CloudFormation provisioner hydrating Lambda `Code.S3Bucket`/`Code.S3Key`, and the `S3Delivery` hook Lambda pulls S3-sourced code through on UpdateFunctionCode. `cdk bootstrap` gives its assets bucket default `aws:kms` encryption, so a Lambda deployed from a CDK asset received the base64 envelope in place of its ZIP and failed at invoke time with "ZIP extraction failed: invalid Zip archive: Could not find EOCD". Measured on the same 859-byte asset, CodeSize was 1252 (the envelope) instead of 859. Move the unwrap into `fakecloud_s3::sse::decrypt_body` and route both internal readers plus `S3Service::decrypt_object_body` through it, so the service no longer carries its own copy and a new reader cannot forget it. It fails closed: a decrypt that errors with a hook wired returns Err rather than passing the envelope on, which is how the corruption reached Lambda in the first place. The provisioner reuses the `kms_hook` it already carries (added upstream in #2598); the hook is also threaded into `S3DeliveryImpl`. The pre-existing `cfn_creates_lambda_function_from_s3_code` E2E uses an unencrypted bucket, so it never covered this; the new SSE-KMS sibling asserts CodeSize and CodeSha256 against the plaintext and fails without the fix (172 vs 48). --- .../src/resource_provisioner/mod.rs | 22 +- crates/fakecloud-e2e/tests/cfn_lambda.rs | 144 +++++++++++ crates/fakecloud-s3/src/delivery.rs | 35 ++- crates/fakecloud-s3/src/lib.rs | 1 + crates/fakecloud-s3/src/service/mod.rs | 5 +- crates/fakecloud-s3/src/sse.rs | 225 ++++++++++++++++++ crates/fakecloud-server/src/main.rs | 4 + 7 files changed, 425 insertions(+), 11 deletions(-) create mode 100644 crates/fakecloud-s3/src/sse.rs diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs index b7566fae3..71ae5d536 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs @@ -3224,14 +3224,30 @@ impl ResourceProvisioner { .objects .get(key) .ok_or_else(|| format!("S3 object s3://{bucket}/{key} does not exist"))?; - object.body.clone() + ( + object.body.clone(), + object.sse_algorithm.clone(), + object.sse_kms_key_id.clone(), + ) }; + let (body_ref, sse_algorithm, kms_key_id) = body_ref; // `read_body` consults the body cache (which is owned by the state), // so re-borrow `state` after dropping the bucket borrow above. - state + let stored = state .read_body(&body_ref) .map(|b| b.to_vec()) - .map_err(|e| format!("S3 read failed: {e}")) + .map_err(|e| format!("S3 read failed: {e}"))?; + // An SSE-KMS bucket stores an envelope, not the object. `cdk bootstrap` + // makes its assets bucket `aws:kms`, so skipping this hands Lambda a + // base64 KMS blob in place of its ZIP. + fakecloud_s3::sse::decrypt_body( + self.kms_hook.as_ref(), + &self.account_id, + bucket, + sse_algorithm.as_deref(), + kms_key_id.as_deref(), + stored, + ) } /// Read a specific object version's bytes. Used when a CFN property diff --git a/crates/fakecloud-e2e/tests/cfn_lambda.rs b/crates/fakecloud-e2e/tests/cfn_lambda.rs index c460e8260..bf2f841ca 100644 --- a/crates/fakecloud-e2e/tests/cfn_lambda.rs +++ b/crates/fakecloud-e2e/tests/cfn_lambda.rs @@ -7,6 +7,10 @@ mod helpers; use aws_sdk_cloudformation::types::{Capability, OnFailure, Parameter}; use aws_sdk_s3::primitives::ByteStream; +use aws_sdk_s3::types::{ + ServerSideEncryption, ServerSideEncryptionByDefault, ServerSideEncryptionConfiguration, + ServerSideEncryptionRule, +}; use helpers::TestServer; const ROLE_TEMPLATE_FRAGMENT: &str = r#" @@ -369,3 +373,143 @@ async fn cfn_update_lambda_function_mutates_handler_and_env() { // RevisionId rotates whenever the configuration mutates. assert_ne!(cfg.revision_id().map(String::from), v1_revision); } + +/// Regression: `cdk bootstrap` gives its assets bucket default `aws:kms` +/// encryption, so every asset is stored as a KMS envelope. The provisioner read +/// the stored bytes straight off S3 state, skipping the unwrap the S3 API does, +/// and handed Lambda the envelope in place of the ZIP — which surfaced at invoke +/// time as `ZIP extraction failed: invalid Zip archive: Could not find EOCD`. +/// The sibling test above passes an unencrypted bucket and so never caught it. +#[tokio::test] +async fn cfn_creates_lambda_function_from_sse_kms_s3_code() { + let server = TestServer::start().await; + let cfn = server.cloudformation_client().await; + let lambda = server.lambda_client().await; + let s3 = server.s3_client().await; + let kms = server.kms_client().await; + + let key_id = kms + .create_key() + .send() + .await + .expect("create_key") + .key_metadata() + .expect("key metadata") + .key_id() + .to_string(); + + let bucket = "cfn-lambda-kms-code-bucket"; + let key = "code/main.zip"; + s3.create_bucket() + .bucket(bucket) + .send() + .await + .expect("create_bucket"); + s3.put_bucket_encryption() + .bucket(bucket) + .server_side_encryption_configuration( + ServerSideEncryptionConfiguration::builder() + .rules( + ServerSideEncryptionRule::builder() + .apply_server_side_encryption_by_default( + ServerSideEncryptionByDefault::builder() + .sse_algorithm(ServerSideEncryption::AwsKms) + .kms_master_key_id(&key_id) + .build() + .unwrap(), + ) + .build(), + ) + .build() + .unwrap(), + ) + .send() + .await + .expect("put_bucket_encryption"); + + let code_bytes = b"def handler(event, context): return {'ok': True}".to_vec(); + s3.put_object() + .bucket(bucket) + .key(key) + .body(ByteStream::from(code_bytes.clone())) + .send() + .await + .expect("put_object"); + + // Guard the premise: if the bucket stopped encrypting, the assertions + // below would pass without exercising the decrypt at all. + let head = s3 + .head_object() + .bucket(bucket) + .key(key) + .send() + .await + .expect("head_object"); + assert_eq!( + head.server_side_encryption(), + Some(&ServerSideEncryption::AwsKms) + ); + + let template = template_s3_code("cfn-lambda-kms-role"); + cfn.create_stack() + .stack_name("cfn-lambda-kms") + .template_body(template) + .parameters( + Parameter::builder() + .parameter_key("CodeBucket") + .parameter_value(bucket) + .build(), + ) + .parameters( + Parameter::builder() + .parameter_key("CodeKey") + .parameter_value(key) + .build(), + ) + .capabilities(Capability::CapabilityNamedIam) + .on_failure(OnFailure::Rollback) + .send() + .await + .expect("create_stack"); + + let described = cfn + .describe_stacks() + .stack_name("cfn-lambda-kms") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().expect("stack"); + assert_eq!(stack.stack_status().unwrap().as_str(), "CREATE_COMPLETE"); + + let func_name = stack + .outputs() + .iter() + .find(|o| o.output_key() == Some("FuncName")) + .and_then(|o| o.output_value()) + .expect("FuncName output"); + + let got = lambda + .get_function() + .function_name(func_name) + .send() + .await + .expect("get_function"); + let cfg = got.configuration().expect("configuration"); + // The plaintext length, not the (longer) envelope's: before the fix this + // was the base64 KMS blob. + assert_eq!(cfg.code_size(), code_bytes.len() as i64); + assert_eq!( + cfg.code_sha256(), + Some(sha256_b64(&code_bytes).as_str()), + "stored code must hash to the plaintext object, not the envelope" + ); +} + +/// Base64 SHA-256, matching the `CodeSha256` Lambda reports. +fn sha256_b64(bytes: &[u8]) -> String { + use base64::Engine as _; + use sha2::{Digest, Sha256}; + let mut hasher = Sha256::new(); + hasher.update(bytes); + base64::engine::general_purpose::STANDARD.encode(hasher.finalize()) +} diff --git a/crates/fakecloud-s3/src/delivery.rs b/crates/fakecloud-s3/src/delivery.rs index ae3639099..e877e3f6d 100644 --- a/crates/fakecloud-s3/src/delivery.rs +++ b/crates/fakecloud-s3/src/delivery.rs @@ -21,6 +21,11 @@ pub struct S3DeliveryImpl { /// so callers built before the S3 store exists during startup can wire it up /// later via [`Self::set_s3_store`]. `None` => memory mode (no store). store: RwLock>>, + /// KMS hook used to unwrap SSE-KMS object bodies on read. Without it + /// `get_object` returns the stored envelope rather than the object (see + /// [`crate::sse::decrypt_body`]). Optional so memory-only callers and + /// tests can skip it; `None` means bodies are returned as stored. + kms_hook: RwLock>>, } impl S3DeliveryImpl { @@ -28,9 +33,16 @@ impl S3DeliveryImpl { Self { state, store: RwLock::new(None), + kms_hook: RwLock::new(None), } } + /// Wire the KMS hook so `get_object` can unwrap SSE-KMS bodies. Set after + /// construction for the same startup-ordering reason as the store. + pub fn set_kms_hook(&self, hook: Arc) { + *self.kms_hook.write() = Some(hook); + } + /// Wire the durable S3 store after construction, so delivered objects are /// written through to disk and survive a restart. Several producers (the /// CloudWatch-Logs delivery bus, ELB access logs) build this impl before the @@ -138,7 +150,7 @@ impl S3Delivery for S3DeliveryImpl { fn get_object(&self, account_id: &str, bucket: &str, key: &str) -> Result, String> { // Resolve and open the body under the guard, read it after the guard // is dropped (the open fd pins the inode against a concurrent PUT). - let handle = { + let (handle, sse_algorithm, kms_key_id) = { let accounts = self.state.read(); let state = accounts .get(account_id) @@ -151,12 +163,27 @@ impl S3Delivery for S3DeliveryImpl { .objects .get(key) .ok_or_else(|| format!("key {key} not found in bucket {bucket}"))?; - crate::body_io::ObjectBodyHandle::open(&object.body) - .map_err(|e| format!("failed to read body: {e}"))? + let handle = crate::body_io::ObjectBodyHandle::open(&object.body) + .map_err(|e| format!("failed to read body: {e}"))?; + ( + handle, + object.sse_algorithm.clone(), + object.sse_kms_key_id.clone(), + ) }; let body = crate::service::run_blocking_io(|| handle.read_all()) .map_err(|e| format!("failed to read body: {e}"))?; - Ok(body.to_vec()) + // An SSE-KMS bucket stores an envelope, not the object. Unwrap it here + // or every consumer (Lambda code pulls especially) gets ciphertext. + let hook = self.kms_hook.read().clone(); + crate::sse::decrypt_body( + hook.as_ref(), + account_id, + bucket, + sse_algorithm.as_deref(), + kms_key_id.as_deref(), + body.to_vec(), + ) } } diff --git a/crates/fakecloud-s3/src/lib.rs b/crates/fakecloud-s3/src/lib.rs index e36c80e02..7ab019791 100644 --- a/crates/fakecloud-s3/src/lib.rs +++ b/crates/fakecloud-s3/src/lib.rs @@ -12,6 +12,7 @@ pub mod resource_policy; pub(crate) mod select; pub(crate) mod service; pub mod simulation; +pub mod sse; pub(crate) mod state; mod xml_util; diff --git a/crates/fakecloud-s3/src/service/mod.rs b/crates/fakecloud-s3/src/service/mod.rs index 789176f5f..13ce05bae 100644 --- a/crates/fakecloud-s3/src/service/mod.rs +++ b/crates/fakecloud-s3/src/service/mod.rs @@ -572,10 +572,7 @@ impl S3Service { .bucket_region(bucket) .unwrap_or_else(|| req.region.clone()); self.authorize_kms_key_use(req, key_account, ®ion, key, "Decrypt")?; - let bucket_arn = Arn::s3(bucket).to_string(); - let mut ctx = std::collections::HashMap::new(); - ctx.insert("aws:s3:arn".to_string(), bucket_arn); - hook.decrypt(key_account, envelope, "s3.amazonaws.com", ctx) + crate::sse::decrypt_envelope(hook.as_ref(), account_id, bucket, Some(key), envelope) .map(bytes::Bytes::from) .map_err(|err| { tracing::warn!(bucket = %bucket, error = %err, "SSE-KMS decrypt failed"); diff --git a/crates/fakecloud-s3/src/sse.rs b/crates/fakecloud-s3/src/sse.rs new file mode 100644 index 000000000..671d19408 --- /dev/null +++ b/crates/fakecloud-s3/src/sse.rs @@ -0,0 +1,225 @@ +//! Shared SSE-KMS body decryption. +//! +//! Objects written to an SSE-KMS bucket are stored as a fakecloud-kms envelope +//! (base64 ASCII), and only the S3 API's read paths used to unwrap it. Every +//! *internal* reader — the CloudFormation resource provisioner hydrating +//! `Code.S3Bucket`/`Code.S3Key`, the `S3Delivery` hook Lambda pulls code +//! through — read the stored bytes straight off the state and handed the +//! envelope on as if it were the object. A `cdk bootstrap` assets bucket +//! defaults to `aws:kms`, so a Lambda deployed from a CDK asset received +//! ciphertext instead of its ZIP. +//! +//! This is that unwrap in one place, so a reader cannot forget it. + +use std::sync::Arc; + +use fakecloud_aws::arn::Arn; +use fakecloud_core::delivery::KmsHook; + +/// Unwrap `stored` when it is an SSE-KMS envelope. +/// +/// `kms_key_id` is the key the object records (normally its ARN). A key ARN +/// names the account whose KMS holds the key, which may not be the bucket +/// owner's; without one the key is `account_id`'s (see +/// [`decrypt_envelope`]). +/// +/// Returns the bytes unchanged when the object was not written under +/// `aws:kms`, when no KMS hook is wired, or when the bytes are not UTF-8 +/// (snapshots taken before the hook existed hold plaintext). A decrypt that +/// fails with a hook present is an error rather than a silent passthrough: +/// handing a caller a raw envelope is how the ZIP corruption above happened. +pub fn decrypt_body( + hook: Option<&Arc>, + account_id: &str, + bucket: &str, + sse_algorithm: Option<&str>, + kms_key_id: Option<&str>, + stored: Vec, +) -> Result, String> { + // Taken by value: the common case is a passthrough, and returning the + // caller's own buffer keeps that free. + if sse_algorithm != Some("aws:kms") { + return Ok(stored); + } + let Some(hook) = hook else { + return Ok(stored); + }; + let Ok(envelope) = std::str::from_utf8(&stored) else { + return Ok(stored); + }; + decrypt_envelope(hook.as_ref(), account_id, bucket, kms_key_id, envelope) + .map_err(|e| format!("SSE-KMS decrypt failed for s3://{bucket}: {e}")) +} + +/// Decrypt one SSE-KMS envelope through the hook, returning the hook's own +/// error string unchanged so the S3 API path can map it to a typed KMS +/// error. Shared by [`decrypt_body`] and `S3Service::decrypt_object_body`. +/// +/// The decrypt runs in the account the key reference names (a key or alias +/// ARN), falling back to `account_id` (the bucket owner) for a bare key id, +/// alias name, or no key at all, matching how the body was encrypted. +pub(crate) fn decrypt_envelope( + hook: &dyn KmsHook, + account_id: &str, + bucket: &str, + kms_key_id: Option<&str>, + envelope: &str, +) -> Result, String> { + let key_account = kms_key_id + .filter(|k| !k.is_empty()) + .and_then(crate::service::kms_reference_account) + .unwrap_or(account_id); + let mut ctx = std::collections::HashMap::new(); + ctx.insert("aws:s3:arn".to_string(), Arn::s3(bucket).to_string()); + hook.decrypt(key_account, envelope, "s3.amazonaws.com", ctx) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + + /// Reverses the trivial "encryption" used below, so a decrypt that actually + /// ran is distinguishable from bytes passed straight through. + struct StubKms { + fail: bool, + decrypted_in: std::sync::Mutex>, + } + + impl KmsHook for StubKms { + fn encrypt( + &self, + _account_id: &str, + _region: &str, + _key_id: &str, + plaintext: &[u8], + _service_principal: &str, + _encryption_context: HashMap, + ) -> Result { + Ok(format!("env:{}", String::from_utf8_lossy(plaintext))) + } + + fn decrypt( + &self, + account_id: &str, + ciphertext_b64: &str, + _service_principal: &str, + _encryption_context: HashMap, + ) -> Result, String> { + *self.decrypted_in.lock().unwrap() = Some(account_id.to_string()); + if self.fail { + return Err("key revoked".into()); + } + Ok(ciphertext_b64 + .strip_prefix("env:") + .unwrap_or(ciphertext_b64) + .as_bytes() + .to_vec()) + } + } + + fn stub(fail: bool) -> Arc { + Arc::new(StubKms { + fail, + decrypted_in: std::sync::Mutex::new(None), + }) + } + + fn hook(fail: bool) -> Arc { + stub(fail) + } + + #[test] + fn unencrypted_object_is_returned_unchanged() { + let h = hook(false); + for sse in [None, Some("AES256")] { + let out = decrypt_body(Some(&h), "1", "b", sse, None, b"env:plain".to_vec()).unwrap(); + assert_eq!(out, b"env:plain", "{sse:?}"); + } + } + + #[test] + fn kms_object_is_decrypted() { + let h = hook(false); + let out = decrypt_body( + Some(&h), + "1", + "b", + Some("aws:kms"), + None, + b"env:PK\x03\x04zip".to_vec(), + ) + .unwrap(); + assert_eq!(out, b"PK\x03\x04zip"); + } + + #[test] + fn kms_object_without_a_hook_is_returned_unchanged() { + // No KMS wired: nothing can unwrap it, so passing it through is all + // that's left. + let out = decrypt_body(None, "1", "b", Some("aws:kms"), None, b"env:x".to_vec()).unwrap(); + assert_eq!(out, b"env:x"); + } + + #[test] + fn non_utf8_body_is_treated_as_a_pre_hook_snapshot() { + let h = hook(false); + let raw = [0x50, 0x4b, 0x03, 0x04, 0xff, 0xfe]; + let out = decrypt_body(Some(&h), "1", "b", Some("aws:kms"), None, raw.to_vec()).unwrap(); + assert_eq!(out, raw); + } + + #[test] + fn a_failed_decrypt_is_an_error_not_a_passthrough() { + // The whole point: handing the caller the raw envelope is what fed a + // KMS blob to Lambda as if it were a ZIP. + let h = hook(true); + let err = + decrypt_body(Some(&h), "1", "b", Some("aws:kms"), None, b"env:x".to_vec()).unwrap_err(); + assert!(err.contains("key revoked"), "{err}"); + } + + #[test] + fn decrypts_in_the_account_the_recorded_key_arn_names() { + // A body written under another account's key was encrypted in that + // account's KMS, so it only decrypts there. + let s = stub(false); + let h: Arc = s.clone(); + let key = "arn:aws:kms:us-east-1:222222222222:key/abc"; + decrypt_body( + Some(&h), + "111111111111", + "b", + Some("aws:kms"), + Some(key), + b"env:x".to_vec(), + ) + .unwrap(); + assert_eq!( + s.decrypted_in.lock().unwrap().as_deref(), + Some("222222222222") + ); + } + + #[test] + fn a_bare_key_id_decrypts_in_the_bucket_owners_account() { + let s = stub(false); + let h: Arc = s.clone(); + for key in [None, Some(""), Some("abc"), Some("alias/mine")] { + decrypt_body( + Some(&h), + "111111111111", + "b", + Some("aws:kms"), + key, + b"env:x".to_vec(), + ) + .unwrap(); + assert_eq!( + s.decrypted_in.lock().unwrap().as_deref(), + Some("111111111111"), + "{key:?}" + ); + } + } +} diff --git a/crates/fakecloud-server/src/main.rs b/crates/fakecloud-server/src/main.rs index ae4421948..b74156e40 100644 --- a/crates/fakecloud-server/src/main.rs +++ b/crates/fakecloud-server/src/main.rs @@ -2681,6 +2681,9 @@ async fn main() { // Export tasks a previous process left PENDING/RUNNING can run now that // their S3 destination is durable. logs_service.resume_interrupted_export_tasks(); + // Lambda pulls S3-sourced code through this delivery, and an SSE-KMS + // bucket stores an envelope: the hook lets it unwrap the object. + s3_delivery_for_logs.set_kms_hook(kms_hook_for_services.clone()); let s3_store_for_inbound = s3_store.clone(); if let Some(ref cache) = shared_body_cache { // Share the cache between the S3Store and S3State so read_body honors @@ -3641,6 +3644,7 @@ async fn main() { // Route ELB access-log deliveries through the durable S3 store so they // survive a restart (S3 is rebuilt from the store on boot). s3_delivery_for_elbv2.set_s3_store(s3_store.clone()); + s3_delivery_for_elbv2.set_kms_hook(kms_hook_for_services.clone()); let elbv2_delivery_bus = Arc::new(DeliveryBus::new().with_s3(s3_delivery_for_elbv2)); let elbv2_snapshot_store: Option> = if persistence_config.mode == fakecloud_persistence::StorageMode::Persistent { From bc71615cad5f1da0cc63e724c8c75ddee27d867a Mon Sep 17 00:00:00 2001 From: James Price Date: Tue, 8 Sep 2026 16:44:40 +0100 Subject: [PATCH 2/3] fix(cloudformation): decrypt a TemplateURL body from an SSE-KMS bucket `fetch_template_from_url` read the stored object body straight off S3 state, so a template in an SSE-KMS bucket came back as the fakecloud-kms envelope. The envelope is base64 ASCII, so it survived `from_utf8` and passed the non-empty guard, and parsed as a template with no resources: the stack reported CREATE_COMPLETE having provisioned nothing. That is what `cdk deploy` hits. `cdk bootstrap` gives its assets bucket default `aws:kms` encryption and the CLI always uploads the template, so every CDK deploy silently produced an empty stack. Route the body through `fakecloud_s3::sse::decrypt_body`, the same unwrap the other internal readers use, with the service's existing `kms_hook`. --- crates/fakecloud-cloudformation/src/extras.rs | 25 ++++- .../fakecloud-cloudformation/src/service.rs | 5 +- crates/fakecloud-e2e/tests/cloudformation.rs | 95 +++++++++++++++++++ 3 files changed, 120 insertions(+), 5 deletions(-) diff --git a/crates/fakecloud-cloudformation/src/extras.rs b/crates/fakecloud-cloudformation/src/extras.rs index bc10aa949..b775d3d38 100644 --- a/crates/fakecloud-cloudformation/src/extras.rs +++ b/crates/fakecloud-cloudformation/src/extras.rs @@ -592,12 +592,31 @@ impl CloudFormationService { let (bucket, key) = parse_s3_url(url)?; let mut accounts = self.deps.s3.write(); let state = accounts.get_or_create(account_id); - let body_ref = { + let (body_ref, sse_algorithm, kms_key_id) = { let b = state.buckets.get(&bucket)?; - b.objects.get(&key)?.body.clone() + let o = b.objects.get(&key)?; + ( + o.body.clone(), + o.sse_algorithm.clone(), + o.sse_kms_key_id.clone(), + ) }; let bytes = state.read_body(&body_ref).ok()?; - String::from_utf8(bytes.to_vec()).ok() + // An SSE-KMS bucket stores an envelope, not the object -- and + // `cdk bootstrap` makes its assets bucket `aws:kms`. The envelope is + // base64 ASCII, so it survives `from_utf8` and passes the non-empty + // check below, producing a "template" with no resources: the stack then + // reports CREATE_COMPLETE having provisioned nothing. + let plaintext = fakecloud_s3::sse::decrypt_body( + self.kms_hook.as_ref(), + account_id, + &bucket, + sse_algorithm.as_deref(), + kms_key_id.as_deref(), + bytes.to_vec(), + ) + .ok()?; + String::from_utf8(plaintext).ok() } /// Whether a stack resource's physical resource still exists in its diff --git a/crates/fakecloud-cloudformation/src/service.rs b/crates/fakecloud-cloudformation/src/service.rs index b87a7400f..77b5dbfa4 100644 --- a/crates/fakecloud-cloudformation/src/service.rs +++ b/crates/fakecloud-cloudformation/src/service.rs @@ -682,8 +682,9 @@ pub struct CloudFormationService { /// real store via `with_s3_store` once it has been built. s3_store: Arc, /// The server's KMS hook (persisting minted keys), used by provisioners - /// that report an AWS-managed key for a default-encrypted resource. - kms_hook: Option>, + /// that report an AWS-managed key for a default-encrypted resource, and to + /// unwrap SSE-KMS bodies read from S3 (a `TemplateURL`, Lambda code). + pub(crate) kms_hook: Option>, /// Service Quotas, so EC2 resources a stack provisions are held to the /// account's applied quotas like direct API calls. quota_provider: Option>, diff --git a/crates/fakecloud-e2e/tests/cloudformation.rs b/crates/fakecloud-e2e/tests/cloudformation.rs index 25d6e4746..4608cab85 100644 --- a/crates/fakecloud-e2e/tests/cloudformation.rs +++ b/crates/fakecloud-e2e/tests/cloudformation.rs @@ -923,3 +923,98 @@ async fn s3_bucket_endpoint_attributes_follow_region() { assert_eq!(outputs.get("Website").copied(), Some(website), "{region}"); } } + +/// Regression: `CreateStack` with a `TemplateURL` pointing into an SSE-KMS +/// bucket read the stored envelope instead of the template. The envelope is +/// base64 ASCII, so it parsed as a template with no resources and the stack +/// reported CREATE_COMPLETE having provisioned nothing — which is what the CDK +/// CLI hits, since `cdk bootstrap` makes its assets bucket `aws:kms` and the +/// CLI always uploads the template. +#[tokio::test] +async fn cfn_reads_a_template_url_from_an_sse_kms_bucket() { + let server = TestServer::start().await; + let cf = server.cloudformation_client().await; + let s3 = server.s3_client().await; + let kms = server.kms_client().await; + + let key_id = kms + .create_key() + .send() + .await + .expect("create_key") + .key_metadata() + .expect("key metadata") + .key_id() + .to_string(); + + let bucket = "cfn-template-kms-bucket"; + s3.create_bucket() + .bucket(bucket) + .send() + .await + .expect("create_bucket"); + s3.put_bucket_encryption() + .bucket(bucket) + .server_side_encryption_configuration( + aws_sdk_s3::types::ServerSideEncryptionConfiguration::builder() + .rules( + aws_sdk_s3::types::ServerSideEncryptionRule::builder() + .apply_server_side_encryption_by_default( + aws_sdk_s3::types::ServerSideEncryptionByDefault::builder() + .sse_algorithm(aws_sdk_s3::types::ServerSideEncryption::AwsKms) + .kms_master_key_id(&key_id) + .build() + .unwrap(), + ) + .build(), + ) + .build() + .unwrap(), + ) + .send() + .await + .expect("put_bucket_encryption"); + + let template = r#"{"Resources":{"P":{"Type":"AWS::SSM::Parameter", + "Properties":{"Name":"/from-kms-template","Type":"String","Value":"hi"}}}}"#; + s3.put_object() + .bucket(bucket) + .key("t.json") + .body(aws_sdk_s3::primitives::ByteStream::from( + template.as_bytes().to_vec(), + )) + .send() + .await + .expect("put_object"); + + cf.create_stack() + .stack_name("cfn-template-kms") + .template_url(format!( + "https://{bucket}.s3.us-east-1.amazonaws.com/t.json" + )) + .send() + .await + .expect("create_stack"); + + let described = cf + .describe_stacks() + .stack_name("cfn-template-kms") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().expect("stack"); + assert_eq!(stack.stack_status().unwrap().as_str(), "CREATE_COMPLETE"); + + // The point: CREATE_COMPLETE with zero resources is the failure mode. + let resources = cf + .describe_stack_resources() + .stack_name("cfn-template-kms") + .send() + .await + .expect("describe_stack_resources"); + assert_eq!( + resources.stack_resources().len(), + 1, + "template from the SSE-KMS bucket must actually provision" + ); +} From 55d17b45519492a5fccd7cbd1b82ecc575b9ae18 Mon Sep 17 00:00:00 2001 From: James Price Date: Thu, 1 Oct 2026 14:09:13 +0100 Subject: [PATCH 3/3] fix(cloudformation): decrypt SSE-KMS bodies on every provisioner S3 read Two provisioner readers still took the stored bytes straight off S3 state, so an object in an SSE-KMS bucket came back as the fakecloud-kms envelope: - `fetch_s3_template`, which loads an `AWS::CloudFormation::Stack` `TemplateURL`. CDK uploads nested-stack templates to its `aws:kms` assets bucket, so the child template failed to parse and the parent stack went CREATE_FAILED. - `read_s3_object_version_bytes`, used when a property pins an S3 object `Version` (Step Functions `DefinitionS3Location.Version`), which handed the envelope on as the definition. All three provisioner readers now share `read_object_body`, which reads the body and runs it through `fakecloud_s3::sse::decrypt_body`, so a reader cannot skip the unwrap. Unit tests cover each reader against a body stored as a real KMS envelope; the version and nested-template ones failed before (they got the envelope back). The new E2E `cfn_reads_a_nested_stack_template_from_an_sse_kms_bucket` went CREATE_FAILED with `fetch_s3_template` reverted and passes with it. --- .../src/resource_provisioner/mod.rs | 158 ++++++++++++++++-- crates/fakecloud-e2e/tests/cloudformation.rs | 77 ++++++++- 2 files changed, 213 insertions(+), 22 deletions(-) diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs index 71ae5d536..72fe09daf 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs @@ -3230,22 +3230,43 @@ impl ResourceProvisioner { object.sse_kms_key_id.clone(), ) }; - let (body_ref, sse_algorithm, kms_key_id) = body_ref; // `read_body` consults the body cache (which is owned by the state), // so re-borrow `state` after dropping the bucket borrow above. + let (body_ref, sse_algorithm, kms_key_id) = body_ref; + self.read_object_body( + state, + bucket, + &body_ref, + sse_algorithm.as_deref(), + kms_key_id.as_deref(), + ) + } + + /// Read a stored object body and unwrap it when it is an SSE-KMS envelope. + /// + /// Every provisioner read of an S3 object goes through here. An SSE-KMS + /// bucket stores an envelope, not the object, and `cdk bootstrap` makes its + /// assets bucket `aws:kms`: a reader that skips this hands Lambda a base64 + /// KMS blob in place of its ZIP, or parses a nested-stack "template" with no + /// resources. + fn read_object_body( + &self, + state: &fakecloud_s3::S3State, + bucket: &str, + body_ref: &fakecloud_persistence::s3::BodyRef, + sse_algorithm: Option<&str>, + kms_key_id: Option<&str>, + ) -> Result, String> { let stored = state - .read_body(&body_ref) + .read_body(body_ref) .map(|b| b.to_vec()) .map_err(|e| format!("S3 read failed: {e}"))?; - // An SSE-KMS bucket stores an envelope, not the object. `cdk bootstrap` - // makes its assets bucket `aws:kms`, so skipping this hands Lambda a - // base64 KMS blob in place of its ZIP. fakecloud_s3::sse::decrypt_body( self.kms_hook.as_ref(), &self.account_id, bucket, - sse_algorithm.as_deref(), - kms_key_id.as_deref(), + sse_algorithm, + kms_key_id, stored, ) } @@ -3272,24 +3293,39 @@ impl ResourceProvisioner { .objects .get(key) .filter(|o| o.version_id.as_deref() == Some(version_id)) - .map(|o| o.body.clone()); + .map(|o| { + ( + o.body.clone(), + o.sse_algorithm.clone(), + o.sse_kms_key_id.clone(), + ) + }); from_current .or_else(|| { b.object_versions.get(key).and_then(|versions| { versions .iter() .find(|o| o.version_id.as_deref() == Some(version_id)) - .map(|o| o.body.clone()) + .map(|o| { + ( + o.body.clone(), + o.sse_algorithm.clone(), + o.sse_kms_key_id.clone(), + ) + }) }) }) .ok_or_else(|| { format!("S3 object s3://{bucket}/{key} version {version_id} does not exist") })? }; - state - .read_body(&body_ref) - .map(|b| b.to_vec()) - .map_err(|e| format!("S3 read failed: {e}")) + self.read_object_body( + state, + bucket, + &body_ref.0, + body_ref.1.as_deref(), + body_ref.2.as_deref(), + ) } /// Build a canonical Lambda resource-policy statement from a CFN @@ -3695,10 +3731,14 @@ impl ResourceProvisioner { .objects .get(key) .ok_or_else(|| format!("S3 object not found: {bucket}/{key}"))?; - let bytes = s3_state - .read_body(&obj.body) - .map_err(|e| format!("Failed to read S3 object body: {e}"))?; - String::from_utf8(bytes.to_vec()).map_err(|e| format!("S3 object is not valid UTF-8: {e}")) + let bytes = self.read_object_body( + s3_state, + bucket, + &obj.body, + obj.sse_algorithm.as_deref(), + obj.sse_kms_key_id.as_deref(), + )?; + String::from_utf8(bytes).map_err(|e| format!("S3 object is not valid UTF-8: {e}")) } } @@ -7968,6 +8008,90 @@ mod tests { assert!(prov.delete_resource(&sr).is_ok()); } + /// Store `plaintext` the way S3 does in an SSE-KMS bucket: as the KMS + /// envelope, with `sse_algorithm = aws:kms`. `version` lands on the + /// current object. + fn put_sse_kms_object( + prov: &ResourceProvisioner, + bucket: &str, + key: &str, + version: &str, + plaintext: &[u8], + ) { + let mut ctx = std::collections::HashMap::new(); + ctx.insert( + "aws:s3:arn".to_string(), + fakecloud_aws::arn::Arn::s3(bucket).to_string(), + ); + let envelope = prov + .kms_hook + .as_ref() + .expect("make_provisioner wires KMS") + .encrypt( + &prov.account_id, + &prov.region, + "aws/s3", + plaintext, + "s3.amazonaws.com", + ctx, + ) + .expect("encrypt"); + assert_ne!( + envelope.as_bytes(), + plaintext, + "the stored body must be the envelope" + ); + let mut accounts = prov.s3_state.write(); + let state = accounts.get_or_create(&prov.account_id); + let mut b = fakecloud_s3::S3Bucket::new(bucket, &prov.region, &prov.account_id); + b.objects.insert( + key.to_string(), + fakecloud_s3::S3Object { + key: key.to_string(), + size: envelope.len() as u64, + body: fakecloud_s3::memory_body(bytes::Bytes::from(envelope.into_bytes())), + sse_algorithm: Some("aws:kms".to_string()), + version_id: Some(version.to_string()), + ..Default::default() + }, + ); + state.buckets.insert(bucket.to_string(), b); + } + + #[test] + fn current_object_read_decrypts_an_sse_kms_body() { + let prov = make_provisioner(); + put_sse_kms_object(&prov, "assets", "code.zip", "v1", b"plaintext body"); + assert_eq!( + prov.read_s3_object_bytes("assets", "code.zip").unwrap(), + b"plaintext body" + ); + } + + #[test] + fn pinned_version_read_decrypts_an_sse_kms_body() { + // A Step Functions `DefinitionS3Location.Version` takes this path. + let prov = make_provisioner(); + put_sse_kms_object(&prov, "assets", "sm.json", "v1", br#"{"StartAt":"A"}"#); + assert_eq!( + prov.read_s3_object_version_bytes("assets", "sm.json", "v1") + .unwrap(), + br#"{"StartAt":"A"}"# + ); + } + + #[test] + fn nested_stack_template_read_decrypts_an_sse_kms_body() { + // CDK uploads nested-stack templates to its `aws:kms` assets bucket. + let prov = make_provisioner(); + let template = r#"{"Resources":{"Q":{"Type":"AWS::SQS::Queue"}}}"#; + put_sse_kms_object(&prov, "assets", "nested.json", "v1", template.as_bytes()); + assert_eq!( + prov.fetch_s3_template("assets", "nested.json").unwrap(), + template + ); + } + fn cn_provisioner() -> ResourceProvisioner { let mut prov = make_provisioner(); prov.region = "cn-north-1".to_string(); diff --git a/crates/fakecloud-e2e/tests/cloudformation.rs b/crates/fakecloud-e2e/tests/cloudformation.rs index 4608cab85..18c18198e 100644 --- a/crates/fakecloud-e2e/tests/cloudformation.rs +++ b/crates/fakecloud-e2e/tests/cloudformation.rs @@ -930,10 +930,9 @@ async fn s3_bucket_endpoint_attributes_follow_region() { /// reported CREATE_COMPLETE having provisioned nothing — which is what the CDK /// CLI hits, since `cdk bootstrap` makes its assets bucket `aws:kms` and the /// CLI always uploads the template. -#[tokio::test] -async fn cfn_reads_a_template_url_from_an_sse_kms_bucket() { - let server = TestServer::start().await; - let cf = server.cloudformation_client().await; +/// Create `bucket` with default SSE-KMS under a fresh customer key, the shape +/// `cdk bootstrap` gives its assets bucket. +async fn create_sse_kms_bucket(server: &TestServer, bucket: &str) { let s3 = server.s3_client().await; let kms = server.kms_client().await; @@ -947,7 +946,6 @@ async fn cfn_reads_a_template_url_from_an_sse_kms_bucket() { .key_id() .to_string(); - let bucket = "cfn-template-kms-bucket"; s3.create_bucket() .bucket(bucket) .send() @@ -974,6 +972,16 @@ async fn cfn_reads_a_template_url_from_an_sse_kms_bucket() { .send() .await .expect("put_bucket_encryption"); +} + +#[tokio::test] +async fn cfn_reads_a_template_url_from_an_sse_kms_bucket() { + let server = TestServer::start().await; + let cf = server.cloudformation_client().await; + let s3 = server.s3_client().await; + + let bucket = "cfn-template-kms-bucket"; + create_sse_kms_bucket(&server, bucket).await; let template = r#"{"Resources":{"P":{"Type":"AWS::SSM::Parameter", "Properties":{"Name":"/from-kms-template","Type":"String","Value":"hi"}}}}"#; @@ -1018,3 +1026,62 @@ async fn cfn_reads_a_template_url_from_an_sse_kms_bucket() { "template from the SSE-KMS bucket must actually provision" ); } +#[tokio::test] +async fn cfn_reads_a_nested_stack_template_from_an_sse_kms_bucket() { + // CDK uploads nested-stack templates to its `aws:kms` assets bucket, and + // the provisioner fetches them through a different reader than the + // top-level TemplateURL. + let server = TestServer::start().await; + let cf = server.cloudformation_client().await; + let s3 = server.s3_client().await; + let ssm = server.ssm_client().await; + + let bucket = "cfn-nested-kms-bucket"; + create_sse_kms_bucket(&server, bucket).await; + + let child = r#"{"Resources":{"P":{"Type":"AWS::SSM::Parameter", + "Properties":{"Name":"/from-kms-nested","Type":"String","Value":"nested"}}}}"#; + s3.put_object() + .bucket(bucket) + .key("child.json") + .body(aws_sdk_s3::primitives::ByteStream::from( + child.as_bytes().to_vec(), + )) + .send() + .await + .expect("put_object"); + + let parent = format!( + r#"{{"Resources":{{"Child":{{"Type":"AWS::CloudFormation::Stack", + "Properties":{{"TemplateURL":"https://{bucket}.s3.us-east-1.amazonaws.com/child.json"}}}}}}}}"# + ); + cf.create_stack() + .stack_name("cfn-nested-kms") + .template_body(parent) + .send() + .await + .expect("create_stack"); + + let described = cf + .describe_stacks() + .stack_name("cfn-nested-kms") + .send() + .await + .expect("describe_stacks"); + let stack = described.stacks().first().expect("stack"); + assert_eq!( + stack.stack_status().unwrap().as_str(), + "CREATE_COMPLETE", + "{:?}", + stack.stack_status_reason() + ); + + // The child's resource exists only if its template was decrypted. + let param = ssm + .get_parameter() + .name("/from-kms-nested") + .send() + .await + .expect("nested stack must provision its parameter"); + assert_eq!(param.parameter().unwrap().value(), Some("nested")); +}