From 4e0c80f89353731edc6d5f7aba6539a4f340296c Mon Sep 17 00:00:00 2001 From: Joshua Weber <57131123+daschaa@users.noreply.github.com> Date: Fri, 8 Jul 2022 16:23:05 +0200 Subject: [PATCH] fix(sns-subscriptions): restrict encryption of queue to only the respective sns topic (under feature flag) (#20521) Implements #20339 The change adds a conditional that only the corresponding SNS Topic can decrypt the SQS queue with the KMS-Key if one was given. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-sns-subscriptions/lib/sqs.ts | 6 +- .../aws-sns-subscriptions/package.json | 1 + .../test/integ.sns-sqs.lit.ts | 12 +- .../aws-cdk-sns-sqs.template.json | 257 +++++++++++------- .../aws-sns-subscriptions/test/subs.test.ts | 153 ++++++++++- packages/@aws-cdk/cx-api/README.md | 19 ++ packages/@aws-cdk/cx-api/lib/features.ts | 12 + 7 files changed, 358 insertions(+), 102 deletions(-) mode change 100644 => 100755 packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-sqs.lit.ts diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts index 8b8f533ad6788..3d9acff2d2a65 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts @@ -1,7 +1,8 @@ import * as iam from '@aws-cdk/aws-iam'; import * as sns from '@aws-cdk/aws-sns'; import * as sqs from '@aws-cdk/aws-sqs'; -import { ArnFormat, Names, Stack, Token } from '@aws-cdk/core'; +import { ArnFormat, FeatureFlags, Names, Stack, Token } from '@aws-cdk/core'; +import * as cxapi from '@aws-cdk/cx-api'; import { Construct } from 'constructs'; import { SubscriptionProps } from './subscription'; @@ -55,6 +56,9 @@ export class SqsSubscription implements sns.ITopicSubscription { resources: ['*'], actions: ['kms:Decrypt', 'kms:GenerateDataKey'], principals: [snsServicePrincipal], + conditions: FeatureFlags.of(topic).isEnabled(cxapi.SNS_SUBSCRIPTIONS_SQS_DECRYPTION_POLICY) + ? { ArnEquals: { 'aws:SourceArn': topic.topicArn } } + : undefined, })); } diff --git a/packages/@aws-cdk/aws-sns-subscriptions/package.json b/packages/@aws-cdk/aws-sns-subscriptions/package.json index a5477a14400ca..5c8d4c76df917 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/package.json +++ b/packages/@aws-cdk/aws-sns-subscriptions/package.json @@ -87,6 +87,7 @@ "@aws-cdk/aws-sns": "0.0.0", "@aws-cdk/aws-sqs": "0.0.0", "@aws-cdk/core": "0.0.0", + "@aws-cdk/cx-api": "0.0.0", "constructs": "^10.0.0" }, "homepage": "https://github.com/aws/aws-cdk", diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-sqs.lit.ts b/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-sqs.lit.ts old mode 100644 new mode 100755 index 4c65c3ca011c7..cb3f4d8e27151 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-sqs.lit.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-sqs.lit.ts @@ -1,15 +1,21 @@ +import * as kms from '@aws-cdk/aws-kms'; import * as sns from '@aws-cdk/aws-sns'; import * as sqs from '@aws-cdk/aws-sqs'; import * as cdk from '@aws-cdk/core'; +import * as cxapi from '@aws-cdk/cx-api'; import * as subs from '../lib'; +const restrictSqsDescryption = { [cxapi.SNS_SUBSCRIPTIONS_SQS_DECRYPTION_POLICY]: true }; + class SnsToSqs extends cdk.Stack { constructor(scope: cdk.App, id: string, props?: cdk.StackProps) { super(scope, id, props); /// !show const topic = new sns.Topic(this, 'MyTopic'); - const queue = new sqs.Queue(this, 'MyQueue'); + const queue = new sqs.Queue(this, 'MyQueue', { + encryptionMasterKey: new kms.Key(this, 'EncryptionMasterKey'), + }); topic.addSubscription(new subs.SqsSubscription(queue, { deadLetterQueue: new sqs.Queue(this, 'DeadLetterQueue'), @@ -18,7 +24,9 @@ class SnsToSqs extends cdk.Stack { } } -const app = new cdk.App(); +const app = new cdk.App({ + context: restrictSqsDescryption, +}); new SnsToSqs(app, 'aws-cdk-sns-sqs'); diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json b/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json index 4d4d40244ceae..20a1f73ef916f 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json @@ -1,110 +1,171 @@ { - "Resources": { - "MyTopic86869434": { - "Type": "AWS::SNS::Topic" - }, - "MyQueueE6CA6235": { - "Type": "AWS::SQS::Queue", - "UpdateReplacePolicy": "Delete", - "DeletionPolicy": "Delete" - }, - "MyQueuePolicy6BBEDDAC": { - "Type": "AWS::SQS::QueuePolicy", - "Properties": { - "PolicyDocument": { - "Statement": [ - { - "Action": "sqs:SendMessage", - "Condition": { - "ArnEquals": { - "aws:SourceArn": { - "Ref": "MyTopic86869434" - } + "Resources": { + "MyTopic86869434": { + "Type": "AWS::SNS::Topic" + }, + "MyQueueE6CA6235": { + "Type": "AWS::SQS::Queue", + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete", + "Properties": { + "KmsMasterKeyId": { + "Fn::GetAtt": [ + "EncryptionMasterKey5BD393B9", + "Arn" + ] } - }, - "Effect": "Allow", - "Principal": { - "Service": "sns.amazonaws.com" - }, - "Resource": { - "Fn::GetAtt": [ - "MyQueueE6CA6235", - "Arn" - ] - } } - ], - "Version": "2012-10-17" }, - "Queues": [ - { - "Ref": "MyQueueE6CA6235" - } - ] - } - }, - "MyQueueawscdksnssqsMyTopic9361DEA223429051": { - "Type": "AWS::SNS::Subscription", - "Properties": { - "Protocol": "sqs", - "TopicArn": { - "Ref": "MyTopic86869434" - }, - "Endpoint": { - "Fn::GetAtt": [ - "MyQueueE6CA6235", - "Arn" - ] + "MyQueuePolicy6BBEDDAC": { + "Type": "AWS::SQS::QueuePolicy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "sqs:SendMessage", + "Condition": { + "ArnEquals": { + "aws:SourceArn": { + "Ref": "MyTopic86869434" + } + } + }, + "Effect": "Allow", + "Principal": { + "Service": "sns.amazonaws.com" + }, + "Resource": { + "Fn::GetAtt": [ + "MyQueueE6CA6235", + "Arn" + ] + } + } + ], + "Version": "2012-10-17" + }, + "Queues": [ + { + "Ref": "MyQueueE6CA6235" + } + ] + } }, - "RedrivePolicy": { - "deadLetterTargetArn": { - "Fn::GetAtt": [ - "DeadLetterQueue9F481546", - "Arn" - ] - } - } - } - }, - "DeadLetterQueue9F481546": { - "Type": "AWS::SQS::Queue", - "UpdateReplacePolicy": "Delete", - "DeletionPolicy": "Delete" - }, - "DeadLetterQueuePolicyB1FB890C": { - "Type": "AWS::SQS::QueuePolicy", - "Properties": { - "PolicyDocument": { - "Statement": [ - { - "Action": "sqs:SendMessage", - "Condition": { - "ArnEquals": { - "aws:SourceArn": { + "MyQueueawscdksnssqsMyTopic9361DEA223429051": { + "Type": "AWS::SNS::Subscription", + "Properties": { + "Protocol": "sqs", + "TopicArn": { "Ref": "MyTopic86869434" - } + }, + "Endpoint": { + "Fn::GetAtt": [ + "MyQueueE6CA6235", + "Arn" + ] + }, + "RedrivePolicy": { + "deadLetterTargetArn": { + "Fn::GetAtt": [ + "DeadLetterQueue9F481546", + "Arn" + ] + } } - }, - "Effect": "Allow", - "Principal": { - "Service": "sns.amazonaws.com" - }, - "Resource": { - "Fn::GetAtt": [ - "DeadLetterQueue9F481546", - "Arn" + } + }, + "DeadLetterQueue9F481546": { + "Type": "AWS::SQS::Queue", + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" + }, + "DeadLetterQueuePolicyB1FB890C": { + "Type": "AWS::SQS::QueuePolicy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "sqs:SendMessage", + "Condition": { + "ArnEquals": { + "aws:SourceArn": { + "Ref": "MyTopic86869434" + } + } + }, + "Effect": "Allow", + "Principal": { + "Service": "sns.amazonaws.com" + }, + "Resource": { + "Fn::GetAtt": [ + "DeadLetterQueue9F481546", + "Arn" + ] + } + } + ], + "Version": "2012-10-17" + }, + "Queues": [ + { + "Ref": "DeadLetterQueue9F481546" + } ] - } } - ], - "Version": "2012-10-17" }, - "Queues": [ - { - "Ref": "DeadLetterQueue9F481546" - } - ] - } + "EncryptionMasterKey5BD393B9": { + "Type": "AWS::KMS::Key", + "Properties": { + "KeyPolicy": { + "Statement": [ + { + "Action": "kms:*", + "Effect": "Allow", + "Principal": { + "AWS": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::", + { + "Ref": "AWS::AccountId" + }, + ":root" + ] + ] + } + }, + "Resource": "*" + }, + { + "Action": [ + "kms:Decrypt", + "kms:GenerateDataKey" + ], + "Effect": "Allow", + "Principal": { + "Service": "sns.amazonaws.com" + }, + "Condition": { + "ArnEquals": { + "aws:SourceArn": { + "Ref": "MyTopic86869434" + } + } + }, + "Resource": "*" + } + ], + "Version": "2012-10-17" + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + } } - } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts index 69cb80216d0e0..4eed6baf482e1 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts @@ -4,10 +4,11 @@ import * as lambda from '@aws-cdk/aws-lambda'; import * as sns from '@aws-cdk/aws-sns'; import * as sqs from '@aws-cdk/aws-sqs'; import { App, CfnParameter, Duration, RemovalPolicy, Stack, Token } from '@aws-cdk/core'; +import * as cxapi from '@aws-cdk/cx-api'; import * as subs from '../lib'; /* eslint-disable quote-props */ - +const restrictSqsDescryption = { [cxapi.SNS_SUBSCRIPTIONS_SQS_DECRYPTION_POLICY]: true }; let stack: Stack; let topic: sns.Topic; @@ -1042,6 +1043,156 @@ test('encrypted queue subscription', () => { }); }); +describe('Restrict sqs decryption feature flag', () => { + test('Restrict decryption of sqs to sns service principal', () => { + const stackUnderTest = new Stack( + new App(), + ); + const topicUnderTest = new sns.Topic(stackUnderTest, 'MyTopic', { + topicName: 'topicName', + displayName: 'displayName', + }); + const key = new kms.Key(stackUnderTest, 'MyKey', { + removalPolicy: RemovalPolicy.DESTROY, + }); + + const queue = new sqs.Queue(stackUnderTest, 'MyQueue', { + encryptionMasterKey: key, + }); + + topicUnderTest.addSubscription(new subs.SqsSubscription(queue)); + + Template.fromStack(stackUnderTest).templateMatches({ + 'Resources': { + 'MyKey6AB29FA6': { + 'Type': 'AWS::KMS::Key', + 'Properties': { + 'KeyPolicy': { + 'Statement': [ + { + 'Action': 'kms:*', + 'Effect': 'Allow', + 'Principal': { + 'AWS': { + 'Fn::Join': [ + '', + [ + 'arn:', + { + 'Ref': 'AWS::Partition', + }, + ':iam::', + { + 'Ref': 'AWS::AccountId', + }, + ':root', + ], + ], + }, + }, + 'Resource': '*', + }, + { + 'Action': [ + 'kms:Decrypt', + 'kms:GenerateDataKey', + ], + 'Effect': 'Allow', + 'Principal': { + 'Service': 'sns.amazonaws.com', + }, + 'Resource': '*', + }, + ], + 'Version': '2012-10-17', + }, + }, + 'UpdateReplacePolicy': 'Delete', + 'DeletionPolicy': 'Delete', + }, + }, + }); + }); + test('Restrict decryption of sqs to sns topic', () => { + const stackUnderTest = new Stack( + new App({ + context: restrictSqsDescryption, + }), + ); + const topicUnderTest = new sns.Topic(stackUnderTest, 'MyTopic', { + topicName: 'topicName', + displayName: 'displayName', + }); + const key = new kms.Key(stackUnderTest, 'MyKey', { + removalPolicy: RemovalPolicy.DESTROY, + }); + + const queue = new sqs.Queue(stackUnderTest, 'MyQueue', { + encryptionMasterKey: key, + }); + + topicUnderTest.addSubscription(new subs.SqsSubscription(queue)); + + Template.fromStack(stackUnderTest).templateMatches({ + 'Resources': { + 'MyKey6AB29FA6': { + 'Type': 'AWS::KMS::Key', + 'Properties': { + 'KeyPolicy': { + 'Statement': [ + { + 'Action': 'kms:*', + 'Effect': 'Allow', + 'Principal': { + 'AWS': { + 'Fn::Join': [ + '', + [ + 'arn:', + { + 'Ref': 'AWS::Partition', + }, + ':iam::', + { + 'Ref': 'AWS::AccountId', + }, + ':root', + ], + ], + }, + }, + 'Resource': '*', + }, + { + 'Action': [ + 'kms:Decrypt', + 'kms:GenerateDataKey', + ], + 'Effect': 'Allow', + 'Principal': { + 'Service': 'sns.amazonaws.com', + }, + 'Resource': '*', + 'Condition': { + 'ArnEquals': { + 'aws:SourceArn': { + 'Ref': 'MyTopic86869434', + }, + }, + }, + }, + ], + 'Version': '2012-10-17', + }, + }, + 'UpdateReplacePolicy': 'Delete', + 'DeletionPolicy': 'Delete', + }, + }, + }); + }); +}); + test('lambda subscription', () => { const fction = new lambda.Function(stack, 'MyFunc', { runtime: lambda.Runtime.NODEJS_14_X, diff --git a/packages/@aws-cdk/cx-api/README.md b/packages/@aws-cdk/cx-api/README.md index 4e88bdf3776c5..84d45500e0008 100644 --- a/packages/@aws-cdk/cx-api/README.md +++ b/packages/@aws-cdk/cx-api/README.md @@ -39,3 +39,22 @@ _cdk.json_ } ``` +* `@aws-cdk/aws-sns-subscriptions:restrictSqsDescryption` + +Enable this feature flag to restrict the decryption of a SQS queue, which is subscribed to a SNS topic, to +only the topic which it is subscribed to and not the whole SNS service of an account. + +Previously the decryption was only restricted to the SNS service principal. To make the SQS subscription more +secure, it is a good practice to restrict the decryption further and only allow the connected SNS topic to decryption +the subscribed queue. + +_cdk.json_ + +```json +{ + "context": { + "@aws-cdk/aws-sns-subscriptions:restrictSqsDescryption": true + } +} +``` + diff --git a/packages/@aws-cdk/cx-api/lib/features.ts b/packages/@aws-cdk/cx-api/lib/features.ts index fcbb188d57cb5..7790d3e49e679 100644 --- a/packages/@aws-cdk/cx-api/lib/features.ts +++ b/packages/@aws-cdk/cx-api/lib/features.ts @@ -282,6 +282,17 @@ export const CODEPIPELINE_CROSS_ACCOUNT_KEY_ALIAS_STACK_SAFE_RESOURCE_NAME = '@a */ export const S3_CREATE_DEFAULT_LOGGING_POLICY = '@aws-cdk/aws-s3:createDefaultLoggingPolicy'; +/** +* Enable this feature flag to restrict the decryption of a SQS queue, which is subscribed to a SNS topic, to +* only the topic which it is subscribed to and not the whole SNS service of an account. +* +* Previously the decryption was only restricted to the SNS service principal. To make the SQS subscription more +* secure, it is a good practice to restrict the decryption further and only allow the connected SNS topic to decryption +* the subscribed queue. +* +*/ +export const SNS_SUBSCRIPTIONS_SQS_DECRYPTION_POLICY = '@aws-cdk/aws-sns-subscriptions:restrictSqsDescryption'; + /** * Flag values that should apply for new projects * @@ -313,6 +324,7 @@ export const FUTURE_FLAGS: { [key: string]: boolean } = { [VALIDATE_SNAPSHOT_REMOVAL_POLICY]: true, [CODEPIPELINE_CROSS_ACCOUNT_KEY_ALIAS_STACK_SAFE_RESOURCE_NAME]: true, [S3_CREATE_DEFAULT_LOGGING_POLICY]: true, + [SNS_SUBSCRIPTIONS_SQS_DECRYPTION_POLICY]: true, }; /**