Skip to content

fix(s3): decrypt SSE-KMS bodies for internal object readers - #6

Open
Sorttech wants to merge 3 commits into
mainfrom
fix/s3-sse-kms-internal-readers
Open

Sorttech wants to merge 3 commits into
mainfrom
fix/s3-sse-kms-internal-readers

Conversation

@Sorttech

@Sorttech Sorttech commented Oct 1, 2026

Copy link
Copy Markdown
Owner

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:

  • the CloudFormation provisioner hydrating Lambda Code.S3Bucket/Code.S3Key;
  • the S3Delivery hook Lambda pulls S3-sourced code through on UpdateFunctionCode;
  • CloudFormation's TemplateURL fetch, both for a top-level stack and for a nested AWS::CloudFormation::Stack;
  • the provisioner's pinned-version read (Step Functions DefinitionS3Location.Version).

cdk bootstrap gives its assets bucket default aws:kms encryption, so this breaks CDK deploys in three ways:

  1. 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. On the same 859-byte asset, CodeSize was 1252 (the envelope) instead of 859.
  2. The CDK CLI always uploads the template, so TemplateURL resolved to the envelope. It is base64 ASCII, so it survived from_utf8, passed the non-empty guard and parsed as a template with no resources: the stack reported CREATE_COMPLETE having provisioned nothing.
  3. Nested-stack templates also live in that bucket; the child template failed to parse and the parent went 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 one read_object_body), the top-level TemplateURL fetch, the S3 delivery hook and S3Service::decrypt_object_body all 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 Err rather 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_hook they already carry (added in faiscadev#2598); the hook is also wired into S3DeliveryImpl.

Tests

  • E2E cfn_lambda::cfn_creates_lambda_function_from_sse_kms_s3_code asserts CodeSize and CodeSha256 against the plaintext and fails without the fix (172 vs 48). The existing cfn_creates_lambda_function_from_s3_code uses an unencrypted bucket, so it never covered this.
  • E2E cloudformation::cfn_reads_a_template_url_from_an_sse_kms_bucket asserts the stack actually provisions its resources.
  • E2E cloudformation::cfn_reads_a_nested_stack_template_from_an_sse_kms_bucket went CREATE_FAILED with the fix reverted and passes with it.
  • Unit tests for each provisioner reader against a body stored as a real KMS envelope (the version and nested-template ones returned the envelope before the fix).
  • Unit tests for 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 warnings on s3/cloudformation/server, unit suites (cloudformation 528, s3 500, server 124), and all three E2E tests.

James Price and others added 3 commits October 2, 2026 13:00
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
Sorttech force-pushed the fix/s3-sse-kms-internal-readers branch from e8f308b to c8cee60 Compare October 2, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant