Skip to content

Commit 8191f1f

Browse files
authoredNov 26, 2021
fix(cli): S3 asset uploads are rejected by commonly referenced encryption SCP (introduces bootstrap stack v9) (#17668)
Many organizations around the world have started to use a specific Service Control Policy (SCP) from this blog post: https://aws.amazon.com/blogs/security/how-to-prevent-uploads-of-unencrypted-objects-to-amazon-s3/ in order to make sure all S3 bucket uploads are encrypted. CDK configures the `DefaultEncryptionConfiguration` on the bucket so that objects are always encrypted by default, but this specific SCP can only check that individual upload actions include the "enable encryption" option. That means that even though the end result would still be satisfied (objects are encrypted in S3), the SCP would nevertheless reject the CDK upload. We would rather people use AWS Config to make sure all buckets have `DefaultEncryptionConfiguration` set, so that this SCP is not necessary... but there is no arguing with deployed reality. Change the CDK upload code path to first read out the default encryption configuration from the bucket, only to then mirror those exact same settings in the `PutObject` call so that the SCP can see that they are present. This requires adding a new permission to the `cdk-assets` role, namely `s3:GetEncryptionConfiguration`, so requires a new bootstrap stack version: version 9. If you want this new behavior because your organization applies this specific SCP, you must upgrade to bootstrap stack version 9. If you do not care about this new behavior you don't have to do anything: if the call to `getEncryptionConfiguration` fails, the CDK will fall back to the old behavior (not specifying any header). Fixes #11265. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7e0c9a3 commit 8191f1f

File tree

5 files changed

+223
-21
lines changed

5 files changed

+223
-21
lines changed
 

‎packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml

+2-1
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ Resources:
304304
- Action:
305305
- s3:GetObject*
306306
- s3:GetBucket*
307+
- s3:GetEncryptionConfiguration
307308
- s3:List*
308309
- s3:DeleteObject*
309310
- s3:PutObject*
@@ -490,7 +491,7 @@ Resources:
490491
Type: String
491492
Name:
492493
Fn::Sub: '/cdk-bootstrap/${Qualifier}/version'
493-
Value: '8'
494+
Value: '9'
494495
Outputs:
495496
BucketName:
496497
Description: The name of the S3 bucket owned by the CDK toolkit stack

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

+117-13
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,15 @@ export class FileAssetHandler implements IAssetHandler {
2323
public async publish(): Promise<void> {
2424
const destination = await replaceAwsPlaceholders(this.asset.destination, this.host.aws);
2525
const s3Url = `s3://${destination.bucketName}/${destination.objectKey}`;
26-
2726
const s3 = await this.host.aws.s3Client(destination);
2827
this.host.emitMessage(EventType.CHECK, `Check ${s3Url}`);
2928

29+
const bucketInfo = BucketInformation.for(this.host);
30+
3031
// A thunk for describing the current account. Used when we need to format an error
3132
// message, not in the success case.
3233
const account = async () => (await this.host.aws.discoverCurrentAccount())?.accountId;
33-
switch (await bucketOwnership(s3, destination.bucketName)) {
34+
switch (await bucketInfo.bucketOwnership(s3, destination.bucketName)) {
3435
case BucketOwnership.MINE:
3536
break;
3637
case BucketOwnership.DOES_NOT_EXIST:
@@ -44,17 +45,42 @@ export class FileAssetHandler implements IAssetHandler {
4445
return;
4546
}
4647

48+
// Identify the the bucket encryption type to set the header on upload
49+
// required for SCP rules denying uploads without encryption header
50+
let paramsEncryption: {[index: string]:any}= {};
51+
const encryption2 = await bucketInfo.bucketEncryption(s3, destination.bucketName);
52+
switch (encryption2) {
53+
case BucketEncryption.NO_ENCRYPTION:
54+
break;
55+
case BucketEncryption.SSEAlgorithm_AES256:
56+
paramsEncryption = { ServerSideEncryption: 'AES256' };
57+
break;
58+
case BucketEncryption.SSEAlgorithm_aws_kms:
59+
paramsEncryption = { ServerSideEncryption: 'aws:kms' };
60+
break;
61+
case BucketEncryption.DOES_NOT_EXIST:
62+
this.host.emitMessage(EventType.DEBUG, `No bucket named '${destination.bucketName}'. Is account ${await account()} bootstrapped?`);
63+
break;
64+
case BucketEncryption.ACCES_DENIED:
65+
this.host.emitMessage(EventType.DEBUG, `ACCES_DENIED for getting encryption of bucket '${destination.bucketName}'. Either wrong account ${await account()} or s3:GetEncryptionConfiguration not set for cdk role. Try "cdk bootstrap" again.`);
66+
break;
67+
}
68+
4769
if (this.host.aborted) { return; }
4870
const publishFile = this.asset.source.executable ?
4971
await this.externalPackageFile(this.asset.source.executable) : await this.packageFile(this.asset.source);
5072

5173
this.host.emitMessage(EventType.UPLOAD, `Upload ${s3Url}`);
52-
await s3.upload({
74+
75+
const params = Object.assign({}, {
5376
Bucket: destination.bucketName,
5477
Key: destination.objectKey,
5578
Body: createReadStream(publishFile.packagedPath),
5679
ContentType: publishFile.contentType,
57-
}).promise();
80+
},
81+
paramsEncryption);
82+
83+
await s3.upload(params).promise();
5884
}
5985

6086
private async packageFile(source: FileSource): Promise<PackagedFileAsset> {
@@ -100,15 +126,12 @@ enum BucketOwnership {
100126
SOMEONE_ELSES_OR_NO_ACCESS
101127
}
102128

103-
async function bucketOwnership(s3: AWS.S3, bucket: string): Promise<BucketOwnership> {
104-
try {
105-
await s3.getBucketLocation({ Bucket: bucket }).promise();
106-
return BucketOwnership.MINE;
107-
} catch (e) {
108-
if (e.code === 'NoSuchBucket') { return BucketOwnership.DOES_NOT_EXIST; }
109-
if (['AccessDenied', 'AllAccessDisabled'].includes(e.code)) { return BucketOwnership.SOMEONE_ELSES_OR_NO_ACCESS; }
110-
throw e;
111-
}
129+
enum BucketEncryption {
130+
NO_ENCRYPTION,
131+
SSEAlgorithm_AES256,
132+
SSEAlgorithm_aws_kms,
133+
ACCES_DENIED,
134+
DOES_NOT_EXIST
112135
}
113136

114137
async function objectExists(s3: AWS.S3, bucket: string, key: string) {
@@ -144,3 +167,84 @@ interface PackagedFileAsset {
144167
*/
145168
readonly contentType?: string;
146169
}
170+
171+
172+
/**
173+
* Cache for bucket information, so we don't have to keep doing the same calls again and again
174+
*
175+
* We scope the lifetime of the cache to the lifetime of the host, so that we don't have to do
176+
* anything special for tests and yet the cache will live for the entire lifetime of the asset
177+
* upload session when used by the CLI.
178+
*/
179+
class BucketInformation {
180+
public static for(host: IHandlerHost) {
181+
const existing = BucketInformation.caches.get(host);
182+
if (existing) { return existing; }
183+
184+
const fresh = new BucketInformation();
185+
BucketInformation.caches.set(host, fresh);
186+
return fresh;
187+
}
188+
189+
private static readonly caches = new WeakMap<IHandlerHost, BucketInformation>();
190+
191+
private readonly ownerships = new Map<string, BucketOwnership>();
192+
private readonly encryptions = new Map<string, BucketEncryption>();
193+
194+
private constructor() {
195+
}
196+
197+
public async bucketOwnership(s3: AWS.S3, bucket: string): Promise<BucketOwnership> {
198+
return cached(this.ownerships, bucket, () => this._bucketOwnership(s3, bucket));
199+
}
200+
201+
public async bucketEncryption(s3: AWS.S3, bucket: string): Promise<BucketEncryption> {
202+
return cached(this.encryptions, bucket, () => this._bucketEncryption(s3, bucket));
203+
}
204+
205+
private async _bucketOwnership(s3: AWS.S3, bucket: string): Promise<BucketOwnership> {
206+
try {
207+
await s3.getBucketLocation({ Bucket: bucket }).promise();
208+
return BucketOwnership.MINE;
209+
} catch (e) {
210+
if (e.code === 'NoSuchBucket') { return BucketOwnership.DOES_NOT_EXIST; }
211+
if (['AccessDenied', 'AllAccessDisabled'].includes(e.code)) { return BucketOwnership.SOMEONE_ELSES_OR_NO_ACCESS; }
212+
throw e;
213+
}
214+
}
215+
216+
private async _bucketEncryption(s3: AWS.S3, bucket: string): Promise<BucketEncryption> {
217+
try {
218+
const encryption = await s3.getBucketEncryption({ Bucket: bucket }).promise();
219+
const l = encryption?.ServerSideEncryptionConfiguration?.Rules?.length ?? 0;
220+
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;
224+
}
225+
return BucketEncryption.NO_ENCRYPTION;
226+
} catch (e) {
227+
if (e.code === 'NoSuchBucket') {
228+
return BucketEncryption.DOES_NOT_EXIST;
229+
}
230+
if (e.code === 'ServerSideEncryptionConfigurationNotFoundError') {
231+
return BucketEncryption.NO_ENCRYPTION;
232+
}
233+
234+
if (['AccessDenied', 'AllAccessDisabled'].includes(e.code)) {
235+
return BucketEncryption.ACCES_DENIED;
236+
}
237+
return BucketEncryption.NO_ENCRYPTION;
238+
}
239+
}
240+
}
241+
242+
async function cached<A, B>(cache: Map<A, B>, key: A, factory: (x: A) => Promise<B>): Promise<B> {
243+
if (cache.has(key)) {
244+
return cache.get(key)!;
245+
}
246+
247+
const fresh = await factory(key);
248+
cache.set(key, fresh);
249+
return fresh;
250+
}

‎packages/cdk-assets/lib/publishing.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { AssetManifest, IManifestEntry } from './asset-manifest';
22
import { IAws } from './aws';
3+
import { IHandlerHost } from './private/asset-handler';
34
import { makeAssetHandler } from './private/handlers';
45
import { EventType, IPublishProgress, IPublishProgressListener } from './progress';
56

@@ -67,18 +68,20 @@ export class AssetPublishing implements IPublishProgress {
6768
public async publish(): Promise<void> {
6869
const self = this;
6970

71+
const handlerHost: IHandlerHost = {
72+
aws: this.options.aws,
73+
get aborted() { return self.aborted; },
74+
emitMessage(t, m) { self.progressEvent(t, m); },
75+
};
76+
7077
for (const asset of this.assets) {
7178
if (this.aborted) { break; }
7279
this.currentAsset = asset;
7380

7481
try {
7582
if (this.progressEvent(EventType.START, `Publishing ${asset.id}`)) { break; }
7683

77-
const handler = makeAssetHandler(this.manifest, asset, {
78-
aws: this.options.aws,
79-
get aborted() { return self.aborted; },
80-
emitMessage(t, m) { self.progressEvent(t, m); },
81-
});
84+
const handler = makeAssetHandler(this.manifest, asset, handlerHost);
8285
await handler.publish();
8386

8487
if (this.aborted) {

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

+95-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ jest.mock('child_process');
33
import { Manifest } from '@aws-cdk/cloud-assembly-schema';
44
import * as mockfs from 'mock-fs';
55
import { AssetManifest, AssetPublishing } from '../lib';
6-
import { mockAws, mockedApiResult, mockUpload } from './mock-aws';
6+
import { mockAws, mockedApiFailure, mockedApiResult, mockUpload } from './mock-aws';
77
import { mockSpawn } from './mock-child_process';
88

99
const ABS_PATH = '/simple/cdk.out/some_external_file';
@@ -126,7 +126,6 @@ test('Do nothing if file already exists', async () => {
126126
const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
127127

128128
aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key' }] });
129-
130129
await pub.publish();
131130

132131
expect(aws.mockS3.listObjectsV2).toHaveBeenCalledWith(expect.objectContaining({
@@ -153,6 +152,100 @@ test('upload file if new (list returns other key)', async () => {
153152
// We'll just have to assume the contents are correct
154153
});
155154

155+
test('upload with server side encryption AES256 header', async () => {
156+
const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
157+
158+
aws.mockS3.getBucketEncryption = mockedApiResult({
159+
ServerSideEncryptionConfiguration: {
160+
Rules: [
161+
{
162+
ApplyServerSideEncryptionByDefault: {
163+
SSEAlgorithm: 'AES256',
164+
},
165+
BucketKeyEnabled: false,
166+
},
167+
],
168+
},
169+
});
170+
aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
171+
aws.mockS3.upload = mockUpload('FILE_CONTENTS');
172+
173+
await pub.publish();
174+
175+
expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.objectContaining({
176+
Bucket: 'some_bucket',
177+
Key: 'some_key',
178+
ContentType: 'application/octet-stream',
179+
ServerSideEncryption: 'AES256',
180+
}));
181+
182+
// We'll just have to assume the contents are correct
183+
});
184+
185+
test('upload with server side encryption aws:kms header', async () => {
186+
const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
187+
188+
aws.mockS3.getBucketEncryption = mockedApiResult({
189+
ServerSideEncryptionConfiguration: {
190+
Rules: [
191+
{
192+
ApplyServerSideEncryptionByDefault: {
193+
SSEAlgorithm: 'aws:kms',
194+
},
195+
BucketKeyEnabled: false,
196+
},
197+
],
198+
},
199+
});
200+
201+
aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
202+
aws.mockS3.upload = mockUpload('FILE_CONTENTS');
203+
204+
await pub.publish();
205+
206+
expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.objectContaining({
207+
Bucket: 'some_bucket',
208+
Key: 'some_key',
209+
ContentType: 'application/octet-stream',
210+
ServerSideEncryption: 'aws:kms',
211+
}));
212+
213+
// We'll just have to assume the contents are correct
214+
});
215+
216+
test('will only read bucketEncryption once even for multiple assets', async () => {
217+
const pub = new AssetPublishing(AssetManifest.fromPath('/types/cdk.out'), { aws });
218+
219+
aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
220+
aws.mockS3.upload = mockUpload('FILE_CONTENTS');
221+
222+
await pub.publish();
223+
224+
expect(aws.mockS3.upload).toHaveBeenCalledTimes(2);
225+
expect(aws.mockS3.getBucketEncryption).toHaveBeenCalledTimes(1);
226+
});
227+
228+
test('no server side encryption header if access denied for bucket encryption', async () => {
229+
const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
230+
231+
aws.mockS3.getBucketEncryption = mockedApiFailure('AccessDenied', 'Access Denied');
232+
233+
aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
234+
aws.mockS3.upload = mockUpload('FILE_CONTENTS');
235+
236+
await pub.publish();
237+
238+
expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.not.objectContaining({
239+
ServerSideEncryption: 'aws:kms',
240+
}));
241+
242+
expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.not.objectContaining({
243+
ServerSideEncryption: 'AES256',
244+
}));
245+
246+
// We'll just have to assume the contents are correct
247+
});
248+
156249
test('correctly looks up content type', async () => {
157250
const pub = new AssetPublishing(AssetManifest.fromPath('/types/cdk.out'), { aws });
158251

‎packages/cdk-assets/test/mock-aws.ts

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export function mockAws() {
88

99
// Sane defaults which can be overridden
1010
mockS3.getBucketLocation = mockedApiResult({});
11+
mockS3.getBucketEncryption = mockedApiResult({});
1112
mockEcr.describeRepositories = mockedApiResult({
1213
repositories: [
1314
{

0 commit comments

Comments
 (0)
Please sign in to comment.