Skip to content

Commit 64ae9f3

Browse files
authored
fix(cli): assets are KMS-encrypted using wrong key (#18340)
Even though a custom KMS key is specified as the default encryption key on the file assets bucket, all uploaded assets are encrypted using the default key. The reason is that in #17668 we added an explicit `encryption: kms` parameter to the `putObject` operation, so that an SCP that is commonly in use across large organizations to prevent files from ending up unencrypted, can be used (the SCP can only validate call parameters, such as whether the `putObject` call includes the parameter that reuests encryption, not the effective end result, such as whether a file would end up encrypted). However, we did not include the KMS Key Id into the `putObject` request, which caused S3 to fall back to the default key. Solution: also look up the key id and pass that along as well. Fixes #18262. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 2256680 commit 64ae9f3

File tree

2 files changed

+30
-23
lines changed

2 files changed

+30
-23
lines changed

packages/cdk-assets/lib/private/handlers/files.ts

+27-22
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,23 @@ export class FileAssetHandler implements IAssetHandler {
4949
// required for SCP rules denying uploads without encryption header
5050
let paramsEncryption: {[index: string]:any}= {};
5151
const encryption2 = await bucketInfo.bucketEncryption(s3, destination.bucketName);
52-
switch (encryption2) {
53-
case BucketEncryption.NO_ENCRYPTION:
52+
switch (encryption2.type) {
53+
case 'no_encryption':
5454
break;
55-
case BucketEncryption.SSEAlgorithm_AES256:
55+
case 'aes256':
5656
paramsEncryption = { ServerSideEncryption: 'AES256' };
5757
break;
58-
case BucketEncryption.SSEAlgorithm_aws_kms:
59-
paramsEncryption = { ServerSideEncryption: 'aws:kms' };
58+
case 'kms':
59+
// We must include the key ID otherwise S3 will encrypt with the default key
60+
paramsEncryption = {
61+
ServerSideEncryption: 'aws:kms',
62+
SSEKMSKeyId: encryption2.kmsKeyId,
63+
};
6064
break;
61-
case BucketEncryption.DOES_NOT_EXIST:
65+
case 'does_not_exist':
6266
this.host.emitMessage(EventType.DEBUG, `No bucket named '${destination.bucketName}'. Is account ${await account()} bootstrapped?`);
6367
break;
64-
case BucketEncryption.ACCES_DENIED:
68+
case 'access_denied':
6569
this.host.emitMessage(EventType.DEBUG, `Could not read encryption settings of bucket '${destination.bucketName}': uploading with default settings ("cdk bootstrap" to version 9 if your organization's policies prevent a successful upload or to get rid of this message).`);
6670
break;
6771
}
@@ -126,13 +130,13 @@ enum BucketOwnership {
126130
SOMEONE_ELSES_OR_NO_ACCESS
127131
}
128132

129-
enum BucketEncryption {
130-
NO_ENCRYPTION,
131-
SSEAlgorithm_AES256,
132-
SSEAlgorithm_aws_kms,
133-
ACCES_DENIED,
134-
DOES_NOT_EXIST
135-
}
133+
type BucketEncryption =
134+
| { readonly type: 'no_encryption' }
135+
| { readonly type: 'aes256' }
136+
| { readonly type: 'kms'; readonly kmsKeyId?: string }
137+
| { readonly type: 'access_denied' }
138+
| { readonly type: 'does_not_exist' }
139+
;
136140

137141
async function objectExists(s3: AWS.S3, bucket: string, key: string) {
138142
/*
@@ -218,23 +222,24 @@ class BucketInformation {
218222
const encryption = await s3.getBucketEncryption({ Bucket: bucket }).promise();
219223
const l = encryption?.ServerSideEncryptionConfiguration?.Rules?.length ?? 0;
220224
if (l > 0) {
221-
let ssealgo = encryption?.ServerSideEncryptionConfiguration?.Rules[0]?.ApplyServerSideEncryptionByDefault?.SSEAlgorithm;
222-
if (ssealgo === 'AES256') return BucketEncryption.SSEAlgorithm_AES256;
223-
if (ssealgo === 'aws:kms') return BucketEncryption.SSEAlgorithm_aws_kms;
225+
const apply = encryption?.ServerSideEncryptionConfiguration?.Rules[0]?.ApplyServerSideEncryptionByDefault;
226+
let ssealgo = apply?.SSEAlgorithm;
227+
if (ssealgo === 'AES256') return { type: 'aes256' };
228+
if (ssealgo === 'aws:kms') return { type: 'kms', kmsKeyId: apply?.KMSMasterKeyID };
224229
}
225-
return BucketEncryption.NO_ENCRYPTION;
230+
return { type: 'no_encryption' };
226231
} catch (e) {
227232
if (e.code === 'NoSuchBucket') {
228-
return BucketEncryption.DOES_NOT_EXIST;
233+
return { type: 'does_not_exist' };
229234
}
230235
if (e.code === 'ServerSideEncryptionConfigurationNotFoundError') {
231-
return BucketEncryption.NO_ENCRYPTION;
236+
return { type: 'no_encryption' };
232237
}
233238

234239
if (['AccessDenied', 'AllAccessDisabled'].includes(e.code)) {
235-
return BucketEncryption.ACCES_DENIED;
240+
return { type: 'access_denied' };
236241
}
237-
return BucketEncryption.NO_ENCRYPTION;
242+
return { type: 'no_encryption' };
238243
}
239244
}
240245
}

packages/cdk-assets/test/files.test.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ test('upload with server side encryption AES256 header', async () => {
183183
// We'll just have to assume the contents are correct
184184
});
185185

186-
test('upload with server side encryption aws:kms header', async () => {
186+
test('upload with server side encryption aws:kms header and key id', async () => {
187187
const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
188188

189189
aws.mockS3.getBucketEncryption = mockedApiResult({
@@ -192,6 +192,7 @@ test('upload with server side encryption aws:kms header', async () => {
192192
{
193193
ApplyServerSideEncryptionByDefault: {
194194
SSEAlgorithm: 'aws:kms',
195+
KMSMasterKeyID: 'the-key-id',
195196
},
196197
BucketKeyEnabled: false,
197198
},
@@ -209,6 +210,7 @@ test('upload with server side encryption aws:kms header', async () => {
209210
Key: 'some_key',
210211
ContentType: 'application/octet-stream',
211212
ServerSideEncryption: 'aws:kms',
213+
SSEKMSKeyId: 'the-key-id',
212214
}));
213215

214216
// We'll just have to assume the contents are correct

0 commit comments

Comments
 (0)