Conversation
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 faiscadev#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).
`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`.
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.
Sorttech
force-pushed
the
fix/s3-sse-kms-internal-readers
branch
from
October 2, 2026 14:41
e8f308b to
c8cee60
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
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:
Code.S3Bucket/Code.S3Key;S3Deliveryhook Lambda pulls S3-sourced code through onUpdateFunctionCode;TemplateURLfetch, both for a top-level stack and for a nestedAWS::CloudFormation::Stack;DefinitionS3Location.Version).cdk bootstrapgives its assets bucket defaultaws:kmsencryption, so this breaks CDK deploys in three ways:ZIP extraction failed: invalid Zip archive: Could not find EOCD. On the same 859-byte asset,CodeSizewas 1252 (the envelope) instead of 859.TemplateURLresolved to the envelope. It is base64 ASCII, so it survivedfrom_utf8, passed the non-empty guard and parsed as a template with no resources: the stack reportedCREATE_COMPLETEhaving provisioned nothing.CREATE_FAILED.Fix
The unwrap moves into
fakecloud_s3::sse::decrypt_body. The provisioner's three S3 readers (current object, pinned version, nested-stack template; all via oneread_object_body), the top-levelTemplateURLfetch, the S3 delivery hook andS3Service::decrypt_object_bodyall go 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
Errrather than passing the envelope on, which is how the corruption reached Lambda in the first place.The provisioner and the CloudFormation service reuse the
kms_hookthey already carry (added in faiscadev#2598); the hook is also wired intoS3DeliveryImpl.Tests
cfn_lambda::cfn_creates_lambda_function_from_sse_kms_s3_codeassertsCodeSizeandCodeSha256against the plaintext and fails without the fix (172 vs 48). The existingcfn_creates_lambda_function_from_s3_codeuses an unencrypted bucket, so it never covered this.cloudformation::cfn_reads_a_template_url_from_an_sse_kms_bucketasserts the stack actually provisions its resources.cloudformation::cfn_reads_a_nested_stack_template_from_an_sse_kms_bucketwentCREATE_FAILEDwith the fix reverted and passes with it.sse::decrypt_body: unencrypted pass-through, decrypt, no hook wired, a non-UTF-8 pre-hook body, and a failed decrypt returning an error.Verified locally: build,
cargo fmt --all --check, clippy-D warningson s3/cloudformation/server, unit suites (cloudformation 528, s3 500, server 124), and all three E2E tests.