From 7441418fbf9ffdf8d85a573e3c81c45c5648fe8a Mon Sep 17 00:00:00 2001 From: markussiebert Date: Tue, 22 Feb 2022 19:35:37 +0100 Subject: [PATCH] fix(s3-notifications): notifications allowed with imported kms keys (#18989) ---- fixes: #18988 If you add an sqs queue as notification target to an s3 bucket, and this sqs queue is encrypted with an imported kms IKey, the stack won't synthesize. Instead of failing, it should warn the user, that it can not ensure the correct kms key policy permissions. This fix will solve this. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-s3-notifications/README.md | 12 +++++++++ .../@aws-cdk/aws-s3-notifications/lib/sqs.ts | 12 +++++++-- .../aws-s3-notifications/package.json | 2 ++ .../aws-s3-notifications/test/queue.test.ts | 27 ++++++++++++++++++- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/README.md b/packages/@aws-cdk/aws-s3-notifications/README.md index 0b57126001cf8..75fba9152e2b8 100644 --- a/packages/@aws-cdk/aws-s3-notifications/README.md +++ b/packages/@aws-cdk/aws-s3-notifications/README.md @@ -26,6 +26,18 @@ const topic = new sns.Topic(this, 'Topic'); bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SnsDestination(topic)); ``` +The following example shows how to send a notification to an SQS queue +when an object is created in an S3 bucket: + +```ts +import * as sqs from '@aws-cdk/aws-sqs'; + +const bucket = new s3.Bucket(this, 'Bucket'); +const queue = new sqs.Queue(this, 'Queue'); + +bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(queue)); +``` + The following example shows how to send a notification to a Lambda function when an object is created in an S3 bucket: ```ts diff --git a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts index 330a941a9ae86..5562a07c5182a 100644 --- a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts +++ b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts @@ -1,6 +1,10 @@ import * as iam from '@aws-cdk/aws-iam'; import * as s3 from '@aws-cdk/aws-s3'; import * as sqs from '@aws-cdk/aws-sqs'; +import { Annotations } from '@aws-cdk/core'; + +// keep this import separate from other imports to reduce chance for merge conflicts with v2-main +// eslint-disable-next-line no-duplicate-imports, import/order import { Construct } from '@aws-cdk/core'; /** @@ -24,11 +28,15 @@ export class SqsDestination implements s3.IBucketNotificationDestination { // if this queue is encrypted, we need to allow S3 to read messages since that's how // it verifies that the notification destination configuration is valid. if (this.queue.encryptionMasterKey) { - this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement({ + const statement = new iam.PolicyStatement({ principals: [new iam.ServicePrincipal('s3.amazonaws.com')], actions: ['kms:GenerateDataKey*', 'kms:Decrypt'], resources: ['*'], - }), /* allowNoOp */ false); + }); + const addResult = this.queue.encryptionMasterKey.addToResourcePolicy(statement, /* allowNoOp */ true); + if (!addResult.statementAdded) { + Annotations.of(this.queue.encryptionMasterKey).addWarning(`Can not change key policy of imported kms key. Ensure that your key policy contains the following permissions: \n${JSON.stringify(statement.toJSON(), null, 2)}`); + } } return { diff --git a/packages/@aws-cdk/aws-s3-notifications/package.json b/packages/@aws-cdk/aws-s3-notifications/package.json index 966572d8350ea..3b004ef015a21 100644 --- a/packages/@aws-cdk/aws-s3-notifications/package.json +++ b/packages/@aws-cdk/aws-s3-notifications/package.json @@ -80,6 +80,7 @@ }, "dependencies": { "@aws-cdk/aws-iam": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", "@aws-cdk/aws-s3": "0.0.0", "@aws-cdk/aws-sns": "0.0.0", @@ -90,6 +91,7 @@ "homepage": "https://github.com/aws/aws-cdk", "peerDependencies": { "@aws-cdk/aws-iam": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", "@aws-cdk/aws-s3": "0.0.0", "@aws-cdk/aws-sns": "0.0.0", diff --git a/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts b/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts index b1ae15e895a7c..44404de1b1d79 100644 --- a/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts +++ b/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts @@ -1,4 +1,5 @@ -import { Match, Template } from '@aws-cdk/assertions'; +import { Match, Template, Annotations } from '@aws-cdk/assertions'; +import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import * as sqs from '@aws-cdk/aws-sqs'; import { Stack } from '@aws-cdk/core'; @@ -96,3 +97,27 @@ test('if the queue is encrypted with a custom kms key, the key resource policy i }, }); }); + +test('if the queue is encrypted with a imported kms key, printout warning', () => { + const stack = new Stack(); + const bucket = new s3.Bucket(stack, 'Bucket'); + const key = kms.Key.fromKeyArn(stack, 'ImportedKey', 'arn:aws:kms:us-west-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab'); + const queue = new sqs.Queue(stack, 'Queue', { + encryption: sqs.QueueEncryption.KMS, + encryptionMasterKey: key, + }); + + bucket.addObjectCreatedNotification(new notif.SqsDestination(queue)); + + Annotations.fromStack(stack).hasWarning('/Default/ImportedKey', `Can not change key policy of imported kms key. Ensure that your key policy contains the following permissions: \n${JSON.stringify({ + Action: [ + 'kms:GenerateDataKey*', + 'kms:Decrypt', + ], + Effect: 'Allow', + Principal: { + Service: 's3.amazonaws.com', + }, + Resource: '*', + }, null, 2)}`); +});