From 07f017bdd7f01127061b7d0a7fbd233e4b952840 Mon Sep 17 00:00:00 2001 From: Romain Marcadier-Muller Date: Mon, 5 Aug 2019 10:00:51 -0700 Subject: [PATCH] fix(sqs): do not emit grants to the AWS-managed encryption key (#3169) Grants on the `alias/aws/sqs` KMS key alias are not necessary since the key will implicitly allow for it's intended usage to be fulfilled (as opposed to how you have to manage grants yourself when using a user-managed key instead). This removes the statement that was generated using an invalid resource entry. Fixes #2794 --- .../aws-s3-notifications/test/queue.test.ts | 9 - packages/@aws-cdk/aws-sqs/lib/queue.ts | 3 - packages/@aws-cdk/aws-sqs/package.json | 3 +- .../aws-sqs/test/integ.sqs.expected.json | 177 +++++++++++++++++- packages/@aws-cdk/aws-sqs/test/integ.sqs.ts | 21 ++- 5 files changed, 194 insertions(+), 19 deletions(-) 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 3c6f33c4ca7a0..482859513a219 100644 --- a/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts +++ b/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts @@ -140,12 +140,3 @@ test('if the queue is encrypted with a custom kms key, the key resource policy i Description: "Created by Queue" }); }); - -test('fails if trying to subscribe to a queue with managed kms encryption', () => { - const stack = new Stack(); - const queue = new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.KMS_MANAGED }); - const bucket = new s3.Bucket(stack, 'Bucket'); - expect(() => { - bucket.addObjectRemovedNotification(new notif.SqsDestination(queue)); - }).toThrow('Unable to add statement to IAM resource policy for KMS key: "alias/aws/sqs"'); -}); diff --git a/packages/@aws-cdk/aws-sqs/lib/queue.ts b/packages/@aws-cdk/aws-sqs/lib/queue.ts index 9247c156ef188..c1718eb1eaa45 100644 --- a/packages/@aws-cdk/aws-sqs/lib/queue.ts +++ b/packages/@aws-cdk/aws-sqs/lib/queue.ts @@ -277,10 +277,7 @@ export class Queue extends QueueBase { } if (encryption === QueueEncryption.KMS_MANAGED) { - const masterKey = kms.Key.fromKeyArn(this, 'Key', 'alias/aws/sqs'); - return { - encryptionMasterKey: masterKey, encryptionProps: { kmsMasterKeyId: 'alias/aws/sqs', kmsDataKeyReusePeriodSeconds: props.dataKeyReuse && props.dataKeyReuse.toSeconds() diff --git a/packages/@aws-cdk/aws-sqs/package.json b/packages/@aws-cdk/aws-sqs/package.json index 8129917c57fe7..ef4e782986e82 100644 --- a/packages/@aws-cdk/aws-sqs/package.json +++ b/packages/@aws-cdk/aws-sqs/package.json @@ -62,6 +62,7 @@ }, "license": "Apache-2.0", "devDependencies": { + "@aws-cdk/aws-kms": "^1.3.0", "@aws-cdk/assert": "^1.3.0", "@aws-cdk/aws-s3": "^1.3.0", "aws-sdk": "^2.438.0", @@ -93,4 +94,4 @@ ] }, "stability": "stable" -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-sqs/test/integ.sqs.expected.json b/packages/@aws-cdk/aws-sqs/test/integ.sqs.expected.json index 8f53d9d476299..ae5da0f4d0525 100644 --- a/packages/@aws-cdk/aws-sqs/test/integ.sqs.expected.json +++ b/packages/@aws-cdk/aws-sqs/test/integ.sqs.expected.json @@ -6,6 +6,7 @@ "Queue4A7E3555": { "Type": "AWS::SQS::Queue", "Properties": { + "KmsMasterKeyId": "alias/aws/sqs", "RedrivePolicy": { "deadLetterTargetArn": { "Fn::GetAtt": [ @@ -17,10 +18,184 @@ } } }, + "EncryptionKey1B843E66": { + "Type": "AWS::KMS::Key", + "Properties": { + "KeyPolicy": { + "Statement": [ + { + "Action": [ + "kms:Create*", + "kms:Describe*", + "kms:Enable*", + "kms:List*", + "kms:Put*", + "kms:Update*", + "kms:Revoke*", + "kms:Disable*", + "kms:Get*", + "kms:Delete*", + "kms:ScheduleKeyDeletion", + "kms:CancelKeyDeletion", + "kms:GenerateDataKey" + ], + "Effect": "Allow", + "Principal": { + "AWS": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::", + { + "Ref": "AWS::AccountId" + }, + ":root" + ] + ] + } + }, + "Resource": "*" + }, + { + "Action": "kms:Decrypt", + "Effect": "Allow", + "Principal": { + "AWS": { + "Fn::GetAtt": [ + "Role1ABCC5F0", + "Arn" + ] + } + }, + "Resource": "*" + } + ], + "Version": "2012-10-17" + } + }, + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" + }, "FifoQueueE5FF7273": { "Type": "AWS::SQS::Queue", "Properties": { - "FifoQueue": true + "FifoQueue": true, + "KmsMasterKeyId": { + "Fn::GetAtt": [ + "EncryptionKey1B843E66", + "Arn" + ] + } + } + }, + "Role1ABCC5F0": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "AWS": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::", + { + "Ref": "AWS::AccountId" + }, + ":root" + ] + ] + } + } + } + ], + "Version": "2012-10-17" + } + } + }, + "RoleDefaultPolicy5FFB7DAB": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "sqs:ReceiveMessage", + "sqs:ChangeMessageVisibility", + "sqs:GetQueueUrl", + "sqs:DeleteMessage", + "sqs:GetQueueAttributes" + ], + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "DeadLetterQueue9F481546", + "Arn" + ] + } + }, + { + "Action": [ + "sqs:ReceiveMessage", + "sqs:ChangeMessageVisibility", + "sqs:GetQueueUrl", + "sqs:DeleteMessage", + "sqs:GetQueueAttributes" + ], + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "Queue4A7E3555", + "Arn" + ] + } + }, + { + "Action": [ + "sqs:ReceiveMessage", + "sqs:ChangeMessageVisibility", + "sqs:GetQueueUrl", + "sqs:DeleteMessage", + "sqs:GetQueueAttributes" + ], + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "FifoQueueE5FF7273", + "Arn" + ] + } + }, + { + "Action": "kms:Decrypt", + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "EncryptionKey1B843E66", + "Arn" + ] + } + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "RoleDefaultPolicy5FFB7DAB", + "Roles": [ + { + "Ref": "Role1ABCC5F0" + } + ] } } }, diff --git a/packages/@aws-cdk/aws-sqs/test/integ.sqs.ts b/packages/@aws-cdk/aws-sqs/test/integ.sqs.ts index c17b3f423dbea..5e0136e750ede 100644 --- a/packages/@aws-cdk/aws-sqs/test/integ.sqs.ts +++ b/packages/@aws-cdk/aws-sqs/test/integ.sqs.ts @@ -1,5 +1,7 @@ -import { App, CfnOutput, Stack } from '@aws-cdk/core'; -import { Queue } from '../lib'; +import { AccountRootPrincipal, Role } from '@aws-cdk/aws-iam'; +import { Key } from '@aws-cdk/aws-kms'; +import { App, CfnOutput, RemovalPolicy, Stack } from '@aws-cdk/core'; +import { Queue, QueueEncryption } from '../lib'; const app = new App(); @@ -7,13 +9,22 @@ const stack = new Stack(app, 'aws-cdk-sqs'); const dlq = new Queue(stack, 'DeadLetterQueue'); const queue = new Queue(stack, 'Queue', { - deadLetterQueue: { queue: dlq, maxReceiveCount: 5 } + deadLetterQueue: { queue: dlq, maxReceiveCount: 5 }, + encryption: QueueEncryption.KMS_MANAGED, +}); +const fifo = new Queue(stack, 'FifoQueue', { + fifo: true, + encryptionMasterKey: new Key(stack, 'EncryptionKey', { removalPolicy: RemovalPolicy.DESTROY }) }); -new Queue(stack, 'FifoQueue', { - fifo: true +const role = new Role(stack, 'Role', { + assumedBy: new AccountRootPrincipal(), }); +dlq.grantConsumeMessages(role); +queue.grantConsumeMessages(role); +fifo.grantConsumeMessages(role); + new CfnOutput(stack, 'QueueUrl', { value: queue.queueUrl }); app.synth();