sns_subscriptions: Disallow SQS with AWS managed KMS key subscribing to SNS

[huonw]

Describe the bug

According to https://aws.amazon.com/premiumsupport/knowledge-center/sns-topic-sqs-queue-sse-kms-key-policy/ , it's not possible to have SNS -> SQS when the SQS queue uses an AWS managed KMS key for encryption, but CDK makes it easy to create such a subscription.

Expected Behavior

Either:

• subscriptions created via CDK to Just Work
• subscriptions that won't work (for obvious, known-at-synth-time reasons) to be flagged as invalid during synth, so I can fix it immediately

Current Behavior

SNS fails to send any messages to SQS, with logged error messages like:

{
  "notification": {...}
  "delivery": {
    "providerResponse": "{\"ErrorCode\":\"KMS.AccessDeniedException\",\"ErrorMessage\":\"null (Service: AWSKMS; Status Code: 400; Error Code: AccessDeniedException; Request ID: bb4fbbce-7269-4a16-956a-51927600b6a0; Proxy: null)\",\"sqsRequestId\":\"Unrecoverable\"}"
    ...
  }
}

Reproduction Steps

import {
  aws_sqs as sqs,
  aws_sns as sns,
  aws_sns_subscriptions as snsSubscriptions,
} from 'aws-cdk-lib';
import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';

class Stack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const t = new sns.Topic(this, 'T', {
      topicName: 't',
    });
    const q = new sqs.Queue(this, 'Q', {
      queueName: 'q',
      encryption: sqs.QueueEncryption.KMS_MANAGED,
    });
    const dlc = new sqs.Queue(this, 'DLQ', {
      queueName: 'dlq',
      encryption: sqs.QueueEncryption.KMS_MANAGED,
    });
    // this subscription will never do anything, nor will the DLQ
    t.addSubscription(
      new snsSubscriptions.SqsSubscription(q, {
        deadLetterQueue: dlq,
      })
    );
  }
}

const app = new cdk.App();
new Stack(app, 's', {});

(I haven't tested this reduced form, sorry)

Possible Solution

It would be nice for CDK to emit an error during synth, since this subscription is never going to work, and thus it's better to tell the user immediately than force them to debug in a live environment.

Additional Information/Context

No response

CDK CLI Version

2.19.0 (build e0d3e62)

Framework Version

2.19.0

Node.js Version

v14.19.1

OS

macOS

Language

Typescript

Language Version

4.6.3

Other information

No response

[NGL321]

Thanks for reporting this @huonw. I have verified the behavior and I think your suggestion of emitting and exception during synth is logical considering the context. Someone from the dev team will address this as time permits. If you want to increase the speed, feel free to submit a PR or collect upvotes (which will increase its urgency)

[github-actions]

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

[huonw]

This still seems to be an issue.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[MurraySpeight]

@NGL321 @bmoffatt it looks like this change is preventing my originally working CDK on v2.88 to no longer synth on v2.89.

I am currently using an SSE enabled queue subscribed to a topic and with the necessary IAM privileges mentioned here https://docs.aws.amazon.com/sns/latest/dg/sns-key-management.html#sns-what-permissions-for-sse the CDK was synthing and deploying in CloudFormation without issue. Now that I have upgraded CDK, the code is refusing to synth with this new exception!

Here is what the code I am using looks like:

import { RemovalPolicy } from "aws-cdk-lib";
import { Queue, QueueEncryption } from "aws-cdk-lib/aws-sqs";
import { Construct } from "constructs";
import { Topic } from "aws-cdk-lib/aws-sns";
import { SqsSubscription } from "aws-cdk-lib/aws-sns-subscriptions";
import { Key } from "aws-cdk-lib/aws-kms";
import { Effect, PolicyStatement, ServicePrincipal } from "aws-cdk-lib/aws-iam";
import { StageProps } from "@ros-aws-coop/shared-constructs";

interface SubscriptionQueueProps {
  stageName: string;
}

export class SubscriptionQueue extends Construct {
  public readonly dlq;
  public readonly subDlq;
  public readonly queue;

  constructor(
    scope: Construct,
    id: string,
    props: StageProps<SubscriptionQueueProps>
  ) {
    super(scope, id);

    const key = new Key(this, "Key");
    const keyAlias = key.addAlias(`my-sqs-${props.stageName}`);
    keyAlias.applyRemovalPolicy(RemovalPolicy.DESTROY);

    this.dlq = new Queue(scope, "SqsDlq", {
      encryption: QueueEncryption.KMS_MANAGED,
      encryptionMasterKey: keyAlias,
      enforceSSL: true
    });

    this.subDlq = new Queue(scope, "SubDlq", {
      encryption: QueueEncryption.KMS_MANAGED,
      encryptionMasterKey: keyAlias,
      enforceSSL: true
    });

    this.queue = new Queue(scope, "Queue", {
      encryption: QueueEncryption.KMS_MANAGED,
      encryptionMasterKey: keyAlias,
      enforceSSL: true,
      deadLetterQueue: {
        queue: this.dlq,
        maxReceiveCount: 1
      }
    });

    const someTopic = Topic.fromTopicArn(
      this,
      "TopicInAnotherAccount",
      "arn:aws:sns:us-east-1:12345678910:topicinanotheraccount"
    );
    someTopic.addSubscription(
      new SqsSubscription(this.queue, {
        rawMessageDelivery: true,
        deadLetterQueue: this.subDlq
      })
    );

    // Allow SNS topics to write into the queue
    keyAlias.addToResourcePolicy(
      new PolicyStatement({
        sid: "sns-allow",
        effect: Effect.ALLOW,
        resources: [someTopic.topicArn],
        principals: [new ServicePrincipal("sns")],
        actions: ["kms:Decrypt", "kms:GenerateDataKey"]
      })
    );
  }
}

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.