feat(s3-tables): add KMS support for TableBucket L2 construct #34281

xuxey · 2025-04-28T18:54:09Z

Reason for this change

This adds support for encryption settings for TableBucket including providing KMS keys for server side encryption.

Description of changes

L1 reference: CfnTableBucket#encryptionConfiguration

Backwards compatible changes were made to the TableBucket construct in the following places:

TableBucketProps now include optional fields encryption and encryptionKey
grant methods now provide permissions to the bucket encryptionKey, if applicable
A new KMS key is created if the user provides KMS encryptionType but no key
Updated README with rosetta support

Usage

// Provide a user defined KMS Key:
const key = new kms.Key(scope, 'UserKey', {});
const encryptedBucket = new TableBucket(scope, 'EncryptedTableBucket', {
    tableBucketName: 'table-bucket-1',
    encryption: TableBucketEncryption.KMS,
    encryptionKey: key,
});
// This account principal will also receive kms:Decrypt access to the KMS key
encryptedBucket.grantRead(new iam.AccountPrincipal('123456789012'), '*');

// If no key is provided, one will be created automatically
const encryptedBucketAuto = new TableBucket(scope, 'EncryptedTableBucketAuto', {
    tableBucketName: 'table-bucket-2',
    encryption: TableBucketEncryption.KMS,
});

// Use S3 managed server side encryption (default)
const encryptedBucketDefault = new TableBucket(scope, 'EncryptedTableBucketDefault', {
    tableBucketName: 'table-bucket-3',
    encryption: TableBucketEncryption.S3_MANAGED,
});

Describe any new or updated permissions being added

These permissions were added for KMS support:

export const KEY_READ_ACCESS = [
  'kms:Decrypt',
];

export const KEY_WRITE_ACCESS = [
  'kms:Decrypt',
  'kms:GenerateDataKey*',
];

When grant methods are used, these policies are applied to the principal for the TableBucket's encryption key. For example, giving read access to an encrypted bucket without giving decrypt permissions to the bucket key will not be sufficient permissions for the principal to read the bucket data.

Description of how you validated changes

Added unit test coverage for all possible scenarios of bucket encryption config, as well as all grant methods for each valid encryption config.
Added integration tests with snapshot and assertions. The assertions are currently disabled due to the aws-sdk version not supporting GetTableBucketEncryptionCommand but will be included once resolved.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

godwingrs22

Thanks @xuxey for adding KMS encryption support to the S3 TableBucket L2 construct. I have left some thoughts for your considerations. Also since we are adding the encryption support, this would required a security review. I'll initiate the security review in parallel.

godwingrs22 · 2025-05-02T19:00:58Z

packages/@aws-cdk/aws-s3tables-alpha/README.md

+// If no key is provided, one will be created automatically
+const encryptedBucketAuto = new TableBucket(scope, 'EncryptedTableBucketAuto', {
+    tableBucketName: 'table-bucket-2',
+    encryption: TableBucketEncryption.KMS,
+});


Since S3 TableBuckets already have S3-managed SSE enabled by default (which I assume is free and secure for most use cases), having an auto-generated KMS key option might lead to unexpected costs for users ($1/month per key plus API call costs - here). Organizations that require KMS encryption typically either have existing KMS keys or want explicit control over key management to align with their security policies.

I suggest we simplify the API to just support customer-provided KMS keys while keeping S3-managed SSE as the default. This would eliminate the need for the TableBucketEncryption enum since the only encryption option a user would need to specify is their KMS key. This approach would reduce complexity, avoid unexpected costs, and better align with real-world usage patterns.

What are your thoughts on this?

having an auto-generated KMS key option might lead to unexpected costs for users

This could be true for some users, however there are advantages for customers with auto-generated keys

This is consistent with the general purpose S3 Bucket construct, which auto-generates KMS keys

Customers can leverage autogenerated keys by not having to explicitly create keys, and these keys will also contain all of the necessary access policies (for example allowlisting S3 Tables Maintenance)

I suggest we simplify the API to just support customer-provided KMS keys while keeping S3-managed SSE as the default

Since both encryptionKey and encryption are optional parameters, this behavior already exists. We would like to keep the interface with TableBucketEncryption since it is more in-sync with the S3 Tables API and CloudFormation resource definitions, and can easily extend to other encryption formats that may be supported in the future.

godwingrs22 · 2025-05-02T19:15:51Z

packages/@aws-cdk/aws-s3tables-alpha/lib/table-bucket.ts

+  /**
+   * Use S3 managed encryption keys with AES256 encryption
+   */
+  S3_MANAGED = 'AES256',


Do we need to explicity pass AES256 algorithm to enable the default encryption (S3 managed keys - SSE-S3)? Based on this doc, seems by default SSE encryption with S3 managed keys will be already with AES256 algorithm to encrypt the data. So I assume CDK users not required to pass this explicitly.

If we don't need to explicitly set this in CloudFormation (as it's already the default behavior), then perhaps we don't need the TableBucketEncryption enum at all? The API could be simpler - if a user wants to use KMS encryption, they just provide a KMS key, otherwise it defaults to S3-managed encryption (AES-256).

The construct could look something like:

interface TableBucketProps { tableBucketName: string; encryptionKey?: kms.IKey; // If provided, uses KMS encryption // ... other props }

by default SSE encryption with S3 managed keys will be already with AES256 algorithm to encrypt the data. So I assume CDK users not required to pass this explicitly.

That's correct, this is not explicitly needed since AES256 is the default.

The API could be simpler - if a user wants to use KMS encryption, they just provide a KMS key, otherwise it defaults to S3-managed encryption (AES-256).

Addressed this in the other comment -- we would like to keep our interface extensible for the future if we add more encryption support, and keep the interface in-line with the Tables API & resource defs.

That said, the behavior you described (use kms key if provided) is still supported by the current interface

xuxey

Thanks for the feedback, Godwin! I spoke with the team about the interface decisions and have left some follow-ups in the replies. Let me know what you think :)

xuxey · 2025-05-05T17:30:00Z

packages/@aws-cdk/aws-s3tables-alpha/README.md

+// If no key is provided, one will be created automatically
+const encryptedBucketAuto = new TableBucket(scope, 'EncryptedTableBucketAuto', {
+    tableBucketName: 'table-bucket-2',
+    encryption: TableBucketEncryption.KMS,
+});


having an auto-generated KMS key option might lead to unexpected costs for users

This could be true for some users, however there are advantages for customers with auto-generated keys

This is consistent with the general purpose S3 Bucket construct, which auto-generates KMS keys

Customers can leverage autogenerated keys by not having to explicitly create keys, and these keys will also contain all of the necessary access policies (for example allowlisting S3 Tables Maintenance)

I suggest we simplify the API to just support customer-provided KMS keys while keeping S3-managed SSE as the default

Since both encryptionKey and encryption are optional parameters, this behavior already exists. We would like to keep the interface with TableBucketEncryption since it is more in-sync with the S3 Tables API and CloudFormation resource definitions, and can easily extend to other encryption formats that may be supported in the future.

xuxey · 2025-05-05T17:34:22Z

packages/@aws-cdk/aws-s3tables-alpha/lib/table-bucket.ts

+  /**
+   * Use S3 managed encryption keys with AES256 encryption
+   */
+  S3_MANAGED = 'AES256',


by default SSE encryption with S3 managed keys will be already with AES256 algorithm to encrypt the data. So I assume CDK users not required to pass this explicitly.

That's correct, this is not explicitly needed since AES256 is the default.

The API could be simpler - if a user wants to use KMS encryption, they just provide a KMS key, otherwise it defaults to S3-managed encryption (AES-256).

Addressed this in the other comment -- we would like to keep our interface extensible for the future if we add more encryption support, and keep the interface in-line with the Tables API & resource defs.

That said, the behavior you described (use kms key if provided) is still supported by the current interface

godwingrs22 · 2025-05-07T20:44:08Z

packages/@aws-cdk/aws-s3tables-alpha/README.md

+// If no key is provided, one will be created automatically
+const encryptedBucketAuto = new TableBucket(scope, 'EncryptedTableBucketAuto', {
+    tableBucketName: 'table-bucket-2',
+    encryption: TableBucketEncryption.KMS,
+});


Would be good to elaborate this in the readme about CDK creating the new KMS key if it is not provided when KMS encryption type is selected.

Can you clarify what additional information we can include in the README?

For eg:

When using KMS encryption (`TableBucketEncryption.KMS`), if no encryption key is provided, CDK will automatically create a new KMS key for the table bucket with necessary permissions.

Elaborated in the latest commit. Thanks for the suggestion!

godwingrs22 · 2025-05-07T21:14:16Z

packages/@aws-cdk/aws-s3tables-alpha/lib/table-bucket.ts

+    encryptionKey.addToResourcePolicy(new iam.PolicyStatement({
+      sid: 'AllowS3TablesMaintenanceAccess',
+      effect: iam.Effect.ALLOW,
+      principals: [
+        new iam.ServicePrincipal('maintenance.s3tables.amazonaws.com'),
+      ],
+      actions: [
+        'kms:GenerateDataKey',
+        'kms:Decrypt',
+      ],
+      resources: ['*'],
+    }));


Can we scope down the resource to specific table arn instead of using wildcard?

Can we able to use grant method to provide access to the encryption key?

Unfortunately this is not possible because of circular dependencies. The TableBucket construct depends on the creation of the KMS key to be passed in as the encryptionKey parameter. If we include a policy to scope down to the table bucket ARN, the KMS key will take a dependency on the table bucket which has not been created at this point.

The KMS key grant methods don't have an option to provide this combination of policies (See here), so it made more sense to create the IAM policy here directly for the specific actions that are needed.

godwingrs22

Thanks @xuxey for the contribution. LGTM.

mergify · 2025-05-14T20:46:54Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2025-05-14T20:46:59Z

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated.

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

Pull request has been modified.

aws-cdk-automation · 2025-05-14T21:17:53Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: eb1fddf
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2025-05-14T21:18:14Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2025-05-14T21:18:31Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

Soham Kulkarni and others added 5 commits April 24, 2025 16:01

Add KMS support for TableBucket

5b8574e

Add unit test coverage for KMS support

a2fc86a

Add integration tests for TableBucket encryption

a0501d8

Update documentation for KMS support

999f037

Merge branch 'aws:main' into s3tables-kms

461c8df

github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Apr 28, 2025

aws-cdk-automation requested a review from a team April 28, 2025 18:54

Fix key perms and parseEncryption error message

5077264

xuxey marked this pull request as ready for review April 28, 2025 20:10

godwingrs22 self-assigned this Apr 28, 2025

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Apr 30, 2025

github-actions bot mentioned this pull request May 1, 2025

Monthly PR metrics report #34324

Open

godwingrs22 reviewed May 2, 2025

View reviewed changes

godwingrs22 added p1 and removed p2 labels May 2, 2025

aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels May 2, 2025

xuxey commented May 5, 2025

View reviewed changes

godwingrs22 reviewed May 7, 2025

View reviewed changes

godwingrs22 added the needs-security-review Related to feature or issues that needs security review label May 7, 2025

Soham Kulkarni and others added 3 commits May 7, 2025 15:41

Add additional information about auto generated KMS keys

21733db

Add encryption context condition to maintenance allowlist

e050d01

Merge branch 'main' into s3tables-kms

5b1ecb5

godwingrs22 mentioned this pull request May 14, 2025

feat(s3tables): server-side encryption by customer managed KMS key #34229

Merged

1 task

godwingrs22 previously approved these changes May 14, 2025

View reviewed changes

Merge branch 'main' into s3tables-kms

eb1fddf

godwingrs22 approved these changes May 14, 2025

View reviewed changes

mergify bot merged commit ac0e4ca into aws:main May 14, 2025
17 of 19 checks passed

github-actions bot locked as resolved and limited conversation to collaborators May 14, 2025

feat(s3-tables): add KMS support for TableBucket L2 construct #34281

feat(s3-tables): add KMS support for TableBucket L2 construct #34281

Uh oh!

Conversation

xuxey commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for this change

Description of changes

Usage

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

godwingrs22 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuxey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

godwingrs22 left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented May 14, 2025

Uh oh!

mergify bot commented May 14, 2025

Uh oh!

aws-cdk-automation commented May 14, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented May 14, 2025

Uh oh!

Uh oh!

github-actions bot commented May 14, 2025

Uh oh!

Uh oh!

xuxey commented Apr 28, 2025 •

edited

Loading