fix(cli): S3 asset uploads are rejected by commonly referenced encryp…

…tion SCP (introduces bootstrap stack v9) (aws#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 aws#11265. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
pedrosola · Dec 1, 2021 · f3d8265 · f3d8265
1 parent 9c7a5a4
commit f3d8265
Show file tree

Hide file tree

Showing 5 changed files with 223 additions and 21 deletions.
diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml
@@ -304,6 +304,7 @@ Resources:
           - Action:
               - s3:GetObject*
               - s3:GetBucket*
+              - s3:GetEncryptionConfiguration
               - s3:List*
               - s3:DeleteObject*
               - s3:PutObject*
@@ -490,7 +491,7 @@ Resources:
       Type: String
       Name:
         Fn::Sub: '/cdk-bootstrap/${Qualifier}/version'
-      Value: '8'
+      Value: '9'
 Outputs:
   BucketName:
     Description: The name of the S3 bucket owned by the CDK toolkit stack

diff --git a/packages/cdk-assets/lib/private/handlers/files.ts b/packages/cdk-assets/lib/private/handlers/files.ts
@@ -23,14 +23,15 @@ export class FileAssetHandler implements IAssetHandler {
   public async publish(): Promise<void> {
     const destination = await replaceAwsPlaceholders(this.asset.destination, this.host.aws);
     const s3Url = `s3://${destination.bucketName}/${destination.objectKey}`;
-
     const s3 = await this.host.aws.s3Client(destination);
     this.host.emitMessage(EventType.CHECK, `Check ${s3Url}`);
 
+    const bucketInfo = BucketInformation.for(this.host);
+
     // A thunk for describing the current account. Used when we need to format an error
     // message, not in the success case.
     const account = async () => (await this.host.aws.discoverCurrentAccount())?.accountId;
-    switch (await bucketOwnership(s3, destination.bucketName)) {
+    switch (await bucketInfo.bucketOwnership(s3, destination.bucketName)) {
       case BucketOwnership.MINE:
         break;
       case BucketOwnership.DOES_NOT_EXIST:
@@ -44,17 +45,42 @@ export class FileAssetHandler implements IAssetHandler {
       return;
     }
 
+    // Identify the the bucket encryption type to set the header on upload
+    // required for SCP rules denying uploads without encryption header
+    let paramsEncryption: {[index: string]:any}= {};
+    const encryption2 = await bucketInfo.bucketEncryption(s3, destination.bucketName);
+    switch (encryption2) {
+      case BucketEncryption.NO_ENCRYPTION:
+        break;
+      case BucketEncryption.SSEAlgorithm_AES256:
+        paramsEncryption = { ServerSideEncryption: 'AES256' };
+        break;
+      case BucketEncryption.SSEAlgorithm_aws_kms:
+        paramsEncryption = { ServerSideEncryption: 'aws:kms' };
+        break;
+      case BucketEncryption.DOES_NOT_EXIST:
+        this.host.emitMessage(EventType.DEBUG, `No bucket named '${destination.bucketName}'. Is account ${await account()} bootstrapped?`);
+        break;
+      case BucketEncryption.ACCES_DENIED:
+        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.`);
+        break;
+    }
+
     if (this.host.aborted) { return; }
     const publishFile = this.asset.source.executable ?
       await this.externalPackageFile(this.asset.source.executable) : await this.packageFile(this.asset.source);
 
     this.host.emitMessage(EventType.UPLOAD, `Upload ${s3Url}`);
-    await s3.upload({
+
+    const params = Object.assign({}, {
       Bucket: destination.bucketName,
       Key: destination.objectKey,
       Body: createReadStream(publishFile.packagedPath),
       ContentType: publishFile.contentType,
-    }).promise();
+    },
+    paramsEncryption);
+
+    await s3.upload(params).promise();
   }
 
   private async packageFile(source: FileSource): Promise<PackagedFileAsset> {
@@ -100,15 +126,12 @@ enum BucketOwnership {
   SOMEONE_ELSES_OR_NO_ACCESS
 }
 
-async function bucketOwnership(s3: AWS.S3, bucket: string): Promise<BucketOwnership> {
-  try {
-    await s3.getBucketLocation({ Bucket: bucket }).promise();
-    return BucketOwnership.MINE;
-  } catch (e) {
-    if (e.code === 'NoSuchBucket') { return BucketOwnership.DOES_NOT_EXIST; }
-    if (['AccessDenied', 'AllAccessDisabled'].includes(e.code)) { return BucketOwnership.SOMEONE_ELSES_OR_NO_ACCESS; }
-    throw e;
-  }
+enum BucketEncryption {
+  NO_ENCRYPTION,
+  SSEAlgorithm_AES256,
+  SSEAlgorithm_aws_kms,
+  ACCES_DENIED,
+  DOES_NOT_EXIST
 }
 
 async function objectExists(s3: AWS.S3, bucket: string, key: string) {
@@ -144,3 +167,84 @@ interface PackagedFileAsset {
    */
   readonly contentType?: string;
 }
+
+
+/**
+ * Cache for bucket information, so we don't have to keep doing the same calls again and again
+ *
+ * We scope the lifetime of the cache to the lifetime of the host, so that we don't have to do
+ * anything special for tests and yet the cache will live for the entire lifetime of the asset
+ * upload session when used by the CLI.
+ */
+class BucketInformation {
+  public static for(host: IHandlerHost) {
+    const existing = BucketInformation.caches.get(host);
+    if (existing) { return existing; }
+
+    const fresh = new BucketInformation();
+    BucketInformation.caches.set(host, fresh);
+    return fresh;
+  }
+
+  private static readonly caches = new WeakMap<IHandlerHost, BucketInformation>();
+
+  private readonly ownerships = new Map<string, BucketOwnership>();
+  private readonly encryptions = new Map<string, BucketEncryption>();
+
+  private constructor() {
+  }
+
+  public async bucketOwnership(s3: AWS.S3, bucket: string): Promise<BucketOwnership> {
+    return cached(this.ownerships, bucket, () => this._bucketOwnership(s3, bucket));
+  }
+
+  public async bucketEncryption(s3: AWS.S3, bucket: string): Promise<BucketEncryption> {
+    return cached(this.encryptions, bucket, () => this._bucketEncryption(s3, bucket));
+  }
+
+  private async _bucketOwnership(s3: AWS.S3, bucket: string): Promise<BucketOwnership> {
+    try {
+      await s3.getBucketLocation({ Bucket: bucket }).promise();
+      return BucketOwnership.MINE;
+    } catch (e) {
+      if (e.code === 'NoSuchBucket') { return BucketOwnership.DOES_NOT_EXIST; }
+      if (['AccessDenied', 'AllAccessDisabled'].includes(e.code)) { return BucketOwnership.SOMEONE_ELSES_OR_NO_ACCESS; }
+      throw e;
+    }
+  }
+
+  private async _bucketEncryption(s3: AWS.S3, bucket: string): Promise<BucketEncryption> {
+    try {
+      const encryption = await s3.getBucketEncryption({ Bucket: bucket }).promise();
+      const l = encryption?.ServerSideEncryptionConfiguration?.Rules?.length ?? 0;
+      if (l > 0) {
+        let ssealgo = encryption?.ServerSideEncryptionConfiguration?.Rules[0]?.ApplyServerSideEncryptionByDefault?.SSEAlgorithm;
+        if (ssealgo === 'AES256') return BucketEncryption.SSEAlgorithm_AES256;
+        if (ssealgo === 'aws:kms') return BucketEncryption.SSEAlgorithm_aws_kms;
+      }
+      return BucketEncryption.NO_ENCRYPTION;
+    } catch (e) {
+      if (e.code === 'NoSuchBucket') {
+        return BucketEncryption.DOES_NOT_EXIST;
+      }
+      if (e.code === 'ServerSideEncryptionConfigurationNotFoundError') {
+        return BucketEncryption.NO_ENCRYPTION;
+      }
+
+      if (['AccessDenied', 'AllAccessDisabled'].includes(e.code)) {
+        return BucketEncryption.ACCES_DENIED;
+      }
+      return BucketEncryption.NO_ENCRYPTION;
+    }
+  }
+}
+
+async function cached<A, B>(cache: Map<A, B>, key: A, factory: (x: A) => Promise<B>): Promise<B> {
+  if (cache.has(key)) {
+    return cache.get(key)!;
+  }
+
+  const fresh = await factory(key);
+  cache.set(key, fresh);
+  return fresh;
+}
diff --git a/packages/cdk-assets/lib/publishing.ts b/packages/cdk-assets/lib/publishing.ts
@@ -1,5 +1,6 @@
 import { AssetManifest, IManifestEntry } from './asset-manifest';
 import { IAws } from './aws';
+import { IHandlerHost } from './private/asset-handler';
 import { makeAssetHandler } from './private/handlers';
 import { EventType, IPublishProgress, IPublishProgressListener } from './progress';
 
@@ -67,18 +68,20 @@ export class AssetPublishing implements IPublishProgress {
   public async publish(): Promise<void> {
     const self = this;
 
+    const handlerHost: IHandlerHost = {
+      aws: this.options.aws,
+      get aborted() { return self.aborted; },
+      emitMessage(t, m) { self.progressEvent(t, m); },
+    };
+
     for (const asset of this.assets) {
       if (this.aborted) { break; }
       this.currentAsset = asset;
 
       try {
         if (this.progressEvent(EventType.START, `Publishing ${asset.id}`)) { break; }
 
-        const handler = makeAssetHandler(this.manifest, asset, {
-          aws: this.options.aws,
-          get aborted() { return self.aborted; },
-          emitMessage(t, m) { self.progressEvent(t, m); },
-        });
+        const handler = makeAssetHandler(this.manifest, asset, handlerHost);
         await handler.publish();
 
         if (this.aborted) {

diff --git a/packages/cdk-assets/test/files.test.ts b/packages/cdk-assets/test/files.test.ts
@@ -3,7 +3,7 @@ jest.mock('child_process');
 import { Manifest } from '@aws-cdk/cloud-assembly-schema';
 import * as mockfs from 'mock-fs';
 import { AssetManifest, AssetPublishing } from '../lib';
-import { mockAws, mockedApiResult, mockUpload } from './mock-aws';
+import { mockAws, mockedApiFailure, mockedApiResult, mockUpload } from './mock-aws';
 import { mockSpawn } from './mock-child_process';
 
 const ABS_PATH = '/simple/cdk.out/some_external_file';
@@ -126,7 +126,6 @@ test('Do nothing if file already exists', async () => {
   const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
 
   aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key' }] });
-
   await pub.publish();
 
   expect(aws.mockS3.listObjectsV2).toHaveBeenCalledWith(expect.objectContaining({
@@ -153,6 +152,100 @@ test('upload file if new (list returns other key)', async () => {
   // We'll just have to assume the contents are correct
 });
 
+test('upload with server side encryption AES256 header', async () => {
+  const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
+
+  aws.mockS3.getBucketEncryption = mockedApiResult({
+    ServerSideEncryptionConfiguration: {
+      Rules: [
+        {
+          ApplyServerSideEncryptionByDefault: {
+            SSEAlgorithm: 'AES256',
+          },
+          BucketKeyEnabled: false,
+        },
+      ],
+    },
+  });
+  aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
+  aws.mockS3.upload = mockUpload('FILE_CONTENTS');
+
+  await pub.publish();
+
+  expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.objectContaining({
+    Bucket: 'some_bucket',
+    Key: 'some_key',
+    ContentType: 'application/octet-stream',
+    ServerSideEncryption: 'AES256',
+  }));
+
+  // We'll just have to assume the contents are correct
+});
+
+test('upload with server side encryption aws:kms header', async () => {
+  const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
+
+  aws.mockS3.getBucketEncryption = mockedApiResult({
+    ServerSideEncryptionConfiguration: {
+      Rules: [
+        {
+          ApplyServerSideEncryptionByDefault: {
+            SSEAlgorithm: 'aws:kms',
+          },
+          BucketKeyEnabled: false,
+        },
+      ],
+    },
+  });
+
+  aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
+  aws.mockS3.upload = mockUpload('FILE_CONTENTS');
+
+  await pub.publish();
+
+  expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.objectContaining({
+    Bucket: 'some_bucket',
+    Key: 'some_key',
+    ContentType: 'application/octet-stream',
+    ServerSideEncryption: 'aws:kms',
+  }));
+
+  // We'll just have to assume the contents are correct
+});
+
+test('will only read bucketEncryption once even for multiple assets', async () => {
+  const pub = new AssetPublishing(AssetManifest.fromPath('/types/cdk.out'), { aws });
+
+  aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
+  aws.mockS3.upload = mockUpload('FILE_CONTENTS');
+
+  await pub.publish();
+
+  expect(aws.mockS3.upload).toHaveBeenCalledTimes(2);
+  expect(aws.mockS3.getBucketEncryption).toHaveBeenCalledTimes(1);
+});
+
+test('no server side encryption header if access denied for bucket encryption', async () => {
+  const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
+
+  aws.mockS3.getBucketEncryption = mockedApiFailure('AccessDenied', 'Access Denied');
+
+  aws.mockS3.listObjectsV2 = mockedApiResult({ Contents: [{ Key: 'some_key.but_not_the_one' }] });
+  aws.mockS3.upload = mockUpload('FILE_CONTENTS');
+
+  await pub.publish();
+
+  expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.not.objectContaining({
+    ServerSideEncryption: 'aws:kms',
+  }));
+
+  expect(aws.mockS3.upload).toHaveBeenCalledWith(expect.not.objectContaining({
+    ServerSideEncryption: 'AES256',
+  }));
+
+  // We'll just have to assume the contents are correct
+});
+
 test('correctly looks up content type', async () => {
   const pub = new AssetPublishing(AssetManifest.fromPath('/types/cdk.out'), { aws });
 

diff --git a/packages/cdk-assets/test/mock-aws.ts b/packages/cdk-assets/test/mock-aws.ts
@@ -8,6 +8,7 @@ export function mockAws() {
 
   // Sane defaults which can be overridden
   mockS3.getBucketLocation = mockedApiResult({});
+  mockS3.getBucketEncryption = mockedApiResult({});
   mockEcr.describeRepositories = mockedApiResult({
     repositories: [
       {