From 0531492451b4f99fe469380ba926f22addbfc492 Mon Sep 17 00:00:00 2001 From: Tatsuya Mori Date: Mon, 24 Jul 2023 21:59:42 +0900 Subject: [PATCH] fix(sns-subscriptions): SQS queue encrypted by AWS managed KMS key is allowed to be specified as subscription and dead-letter queue (#26110) To send message to SQS queues encrypted by KMS from SNS, we need to grant SNS service-principal access to the key by key policy. From this reason, we need to use customer managed key because we can't edit key policy for AWS managed key. However, CDK makes it easy to create such a non-functional subscription. To prevent CDK from making such a subscription, I added the validation which throw an error when SQS queue encrypted by AWS managed KMS key is specified as subscription or dead-letter queue. Closes #19796 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-sns-subscriptions/lib/sqs.ts | 12 ++++++ .../aws-sns-subscriptions/test/subs.test.ts | 40 ++++++++++++++++++ .../aws-cdk-lib/aws-sqs/lib/queue-base.ts | 40 ++++++++++++++++++ packages/aws-cdk-lib/aws-sqs/lib/queue.ts | 41 +++++-------------- packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts | 23 +++++++++-- 5 files changed, 121 insertions(+), 35 deletions(-) diff --git a/packages/aws-cdk-lib/aws-sns-subscriptions/lib/sqs.ts b/packages/aws-cdk-lib/aws-sns-subscriptions/lib/sqs.ts index a27bbbb9c810b..c98808c063a48 100644 --- a/packages/aws-cdk-lib/aws-sns-subscriptions/lib/sqs.ts +++ b/packages/aws-cdk-lib/aws-sns-subscriptions/lib/sqs.ts @@ -38,6 +38,18 @@ export class SqsSubscription implements sns.ITopicSubscription { } const snsServicePrincipal = new iam.ServicePrincipal('sns.amazonaws.com'); + // if the queue is encrypted by AWS managed KMS key (alias/aws/sqs), + // throw error message + if (this.queue.encryptionType === sqs.QueueEncryption.KMS_MANAGED) { + throw new Error('SQS queue encrypted by AWS managed KMS key cannot be used as SNS subscription'); + } + + // if the dead-letter queue is encrypted by AWS managed KMS key (alias/aws/sqs), + // throw error message + if (this.props.deadLetterQueue && this.props.deadLetterQueue.encryptionType === sqs.QueueEncryption.KMS_MANAGED) { + throw new Error('SQS queue encrypted by AWS managed KMS key cannot be used as dead-letter queue'); + } + // add a statement to the queue resource policy which allows this topic // to send messages to the queue. const queuePolicyDependable = this.queue.addToResourcePolicy(new iam.PolicyStatement({ diff --git a/packages/aws-cdk-lib/aws-sns-subscriptions/test/subs.test.ts b/packages/aws-cdk-lib/aws-sns-subscriptions/test/subs.test.ts index 09d099a40bbba..c72bd8f601093 100644 --- a/packages/aws-cdk-lib/aws-sns-subscriptions/test/subs.test.ts +++ b/packages/aws-cdk-lib/aws-sns-subscriptions/test/subs.test.ts @@ -1193,6 +1193,46 @@ describe('Restrict sqs decryption feature flag', () => { }); }); +test('throws an error when a queue is encrypted by AWS managed KMS kye for queue subscription', () => { + // WHEN + const queue = new sqs.Queue(stack, 'MyQueue', { + encryption: sqs.QueueEncryption.KMS_MANAGED, + }); + + // THEN + expect(() => topic.addSubscription(new subs.SqsSubscription(queue))) + .toThrowError(/SQS queue encrypted by AWS managed KMS key cannot be used as SNS subscription/); +}); + +test('throws an error when a dead-letter queue is encrypted by AWS managed KMS kye for queue subscription', () => { + // WHEN + const queue = new sqs.Queue(stack, 'MyQueue'); + const dlq = new sqs.Queue(stack, 'MyDLQ', { + encryption: sqs.QueueEncryption.KMS_MANAGED, + }); + + // THEN + expect(() => topic.addSubscription(new subs.SqsSubscription(queue, { + deadLetterQueue: dlq, + }))) + .toThrowError(/SQS queue encrypted by AWS managed KMS key cannot be used as dead-letter queue/); +}); + +test('importing SQS queue and specify this as subscription', () => { + // WHEN + const queue = sqs.Queue.fromQueueArn(stack, 'Queue', 'arn:aws:sqs:us-east-1:123456789012:queue1'); + topic.addSubscription(new subs.SqsSubscription(queue)); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::SNS::Subscription', { + 'Endpoint': 'arn:aws:sqs:us-east-1:123456789012:queue1', + 'Protocol': 'sqs', + 'TopicArn': { + 'Ref': 'MyTopic86869434', + }, + }); +}); + test('lambda subscription', () => { const func = new lambda.Function(stack, 'MyFunc', { runtime: lambda.Runtime.NODEJS_14_X, diff --git a/packages/aws-cdk-lib/aws-sqs/lib/queue-base.ts b/packages/aws-cdk-lib/aws-sqs/lib/queue-base.ts index aded50ce32481..ca445daef9a5f 100644 --- a/packages/aws-cdk-lib/aws-sqs/lib/queue-base.ts +++ b/packages/aws-cdk-lib/aws-sqs/lib/queue-base.ts @@ -36,6 +36,11 @@ export interface IQueue extends IResource { */ readonly fifo: boolean; + /** + * Whether the contents of the queue are encrypted, and by what type of key. + */ + readonly encryptionType?: QueueEncryption; + /** * Adds a statement to the IAM resource policy associated with this queue. * @@ -126,6 +131,11 @@ export abstract class QueueBase extends Resource implements IQueue { */ public abstract readonly fifo: boolean; + /** + * Whether the contents of the queue are encrypted, and by what type of key. + */ + public abstract readonly encryptionType?: QueueEncryption; + /** * Controls automatic creation of policy objects. * @@ -301,3 +311,33 @@ export interface QueueAttributes { */ readonly fifo?: boolean; } + +/** + * What kind of encryption to apply to this queue + */ +export enum QueueEncryption { + /** + * Messages in the queue are not encrypted + */ + UNENCRYPTED = 'NONE', + + /** + * Server-side KMS encryption with a KMS key managed by SQS. + */ + KMS_MANAGED = 'KMS_MANAGED', + + /** + * Server-side encryption with a KMS key managed by the user. + * + * If `encryptionKey` is specified, this key will be used, otherwise, one will be defined. + */ + KMS = 'KMS', + + /** + * Server-side encryption key managed by SQS (SSE-SQS). + * + * To learn more about SSE-SQS on Amazon SQS, please visit the + * [Amazon SQS documentation](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-server-side-encryption.html). + */ + SQS_MANAGED = 'SQS_MANAGED' +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/aws-sqs/lib/queue.ts b/packages/aws-cdk-lib/aws-sqs/lib/queue.ts index 719b5246b3e89..c812cc099d470 100644 --- a/packages/aws-cdk-lib/aws-sqs/lib/queue.ts +++ b/packages/aws-cdk-lib/aws-sqs/lib/queue.ts @@ -1,5 +1,5 @@ import { Construct } from 'constructs'; -import { IQueue, QueueAttributes, QueueBase } from './queue-base'; +import { IQueue, QueueAttributes, QueueBase, QueueEncryption } from './queue-base'; import { CfnQueue } from './sqs.generated'; import { validateProps } from './validate-props'; import * as iam from '../../aws-iam'; @@ -195,36 +195,6 @@ export interface DeadLetterQueue { readonly maxReceiveCount: number; } -/** - * What kind of encryption to apply to this queue - */ -export enum QueueEncryption { - /** - * Messages in the queue are not encrypted - */ - UNENCRYPTED = 'NONE', - - /** - * Server-side KMS encryption with a KMS key managed by SQS. - */ - KMS_MANAGED = 'KMS_MANAGED', - - /** - * Server-side encryption with a KMS key managed by the user. - * - * If `encryptionKey` is specified, this key will be used, otherwise, one will be defined. - */ - KMS = 'KMS', - - /** - * Server-side encryption key managed by SQS (SSE-SQS). - * - * To learn more about SSE-SQS on Amazon SQS, please visit the - * [Amazon SQS documentation](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-server-side-encryption.html). - */ - SQS_MANAGED = 'SQS_MANAGED' -} - /** * What kind of deduplication scope to apply */ @@ -286,6 +256,9 @@ export class Queue extends QueueBase { ? kms.Key.fromKeyArn(this, 'Key', attrs.keyArn) : undefined; public readonly fifo: boolean = this.determineFifo(); + public readonly encryptionType = attrs.keyArn + ? QueueEncryption.KMS + : undefined; protected readonly autoCreatePolicy = false; @@ -337,6 +310,11 @@ export class Queue extends QueueBase { */ public readonly fifo: boolean; + /** + * Whether the contents of the queue are encrypted, and by what type of key. + */ + public readonly encryptionType?: QueueEncryption; + /** * If this queue is configured with a dead-letter queue, this is the dead-letter queue settings. */ @@ -384,6 +362,7 @@ export class Queue extends QueueBase { this.encryptionMasterKey = encryptionMasterKey; this.queueUrl = queue.ref; this.deadLetterQueue = props.deadLetterQueue; + this.encryptionType = props.encryption; function _determineEncryptionProps(this: Queue): { encryptionProps: EncryptionProps, encryptionMasterKey?: kms.IKey } { let encryption = props.encryption; diff --git a/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts b/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts index 4e226a39ef49e..2db4c58550ef8 100644 --- a/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts +++ b/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts @@ -182,6 +182,15 @@ describe('export and import', () => { expect(fifoQueue.fifo).toEqual(true); }); + test('importing with keyArn and encryptionType is set correctly', () => { + const stack = new Stack(); + const queue = sqs.Queue.fromQueueAttributes(stack, 'Queue', { + queueArn: 'arn:aws:sqs:us-east-1:123456789012:queue1', + keyArn: 'arn:aws:kms:us-east-1:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab', + }); + expect(queue.encryptionType).toEqual(sqs.QueueEncryption.KMS); + }); + test('import queueArn from token, fifo and standard queues can be defined', () => { // GIVEN const stack = new Stack(); @@ -357,7 +366,7 @@ describe('queue encryption', () => { test('a kms key will be allocated if encryption = kms but a master key is not specified', () => { const stack = new Stack(); - new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.KMS }); + const queue = new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.KMS }); Template.fromStack(stack).hasResourceProperties('AWS::KMS::Key', Match.anyValue()); Template.fromStack(stack).hasResourceProperties('AWS::SQS::Queue', { @@ -368,12 +377,14 @@ describe('queue encryption', () => { ], }, }); + expect(queue.encryptionType).toEqual(sqs.QueueEncryption.KMS); }); test('it is possible to use a managed kms key', () => { const stack = new Stack(); - new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.KMS_MANAGED }); + const queue = new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.KMS_MANAGED }); + Template.fromStack(stack).templateMatches({ 'Resources': { 'Queue4A7E3555': { @@ -386,6 +397,7 @@ describe('queue encryption', () => { }, }, }); + expect(queue.encryptionType).toEqual(sqs.QueueEncryption.KMS_MANAGED); }); test('grant also affects key on encrypted queue', () => { @@ -433,7 +445,8 @@ describe('queue encryption', () => { test('it is possible to use sqs managed server side encryption', () => { const stack = new Stack(); - new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.SQS_MANAGED }); + const queue = new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.SQS_MANAGED }); + Template.fromStack(stack).templateMatches({ 'Resources': { 'Queue4A7E3555': { @@ -446,12 +459,13 @@ describe('queue encryption', () => { }, }, }); + expect(queue.encryptionType).toEqual(sqs.QueueEncryption.SQS_MANAGED); }); test('it is possible to disable encryption (unencrypted)', () => { const stack = new Stack(); - new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.UNENCRYPTED }); + const queue = new sqs.Queue(stack, 'Queue', { encryption: sqs.QueueEncryption.UNENCRYPTED }); Template.fromStack(stack).templateMatches({ 'Resources': { 'Queue4A7E3555': { @@ -464,6 +478,7 @@ describe('queue encryption', () => { }, }, }); + expect(queue.encryptionType).toEqual(sqs.QueueEncryption.UNENCRYPTED); }); test('encryptionMasterKey is not supported if encryption type SQS_MANAGED is used', () => {