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 · Aug 5, 2019 · 07f017b · 07f017b
1 parent fac7c61
commit 07f017b
Show file tree

Hide file tree

Showing 5 changed files with 194 additions and 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
@@ -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
@@ -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
@@ -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"
-}
+}
diff --git 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
@@ -1,19 +1,30 @@
-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();
 
 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();