Skip to content

Commit

Permalink
fix(sns-subscriptions): SQS queue encrypted by AWS managed KMS key is…
Browse files Browse the repository at this point in the history
… 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*
  • Loading branch information
tam0ri committed Jul 24, 2023
1 parent 6033c9a commit 0531492
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 35 deletions.
12 changes: 12 additions & 0 deletions packages/aws-cdk-lib/aws-sns-subscriptions/lib/sqs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
40 changes: 40 additions & 0 deletions packages/aws-cdk-lib/aws-sns-subscriptions/test/subs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
40 changes: 40 additions & 0 deletions packages/aws-cdk-lib/aws-sqs/lib/queue-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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'
}
41 changes: 10 additions & 31 deletions packages/aws-cdk-lib/aws-sqs/lib/queue.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 19 additions & 4 deletions packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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', {
Expand All @@ -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': {
Expand All @@ -386,6 +397,7 @@ describe('queue encryption', () => {
},
},
});
expect(queue.encryptionType).toEqual(sqs.QueueEncryption.KMS_MANAGED);
});

test('grant also affects key on encrypted queue', () => {
Expand Down Expand Up @@ -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': {
Expand All @@ -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': {
Expand All @@ -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', () => {
Expand Down

0 comments on commit 0531492

Please sign in to comment.