(aws-sns): addSubscription() doesn't lead to working subscription

[ahammond]

To subscribe an SNS to an SQS, you can write something like

myTopic.addSubscription(new SqsSubscription(mySqs));

and then your messages will disappear.

Reproduction Steps

mySqs = new Queue(this, 'MyQ');
myTopic = new Topic(this, 'MyTopic');
myTopic.addSubscription(new SqsSubscription(mySqs));

What did you expect to happen?

Everything necessary to get the SNS publishing messages into the SQS should be handled. Or at least tell me what I need to do in addition to get it working.

What actually happened?

Messages disappeared into the void.

Environment

• **CDK CLI Version: ** 1.78.0
• **Framework Version: ** 1.78.0
• Node.js Version: v14.3.0
• OS : MacOS and CircleCI docker runners
• Language (Version): TypeScript (4.1.3)

Other

For it to actually work, you need

• a CMK used by the SNS (and maybe also the SQS, not sure)
• a policy on the CMK (details below)
• a policy on the SQS (details below)

I'm not sure the Right Way for cdk to do this. Ideally,

• calling new SqsSubscription(mySqs) would validate that the SQS has a policy that looks reasonable and has a CMK
• calling mySns.addSubscription() would validate that the SNS involved has a CMK and that it has a reasonable policy

Working code snippet for The Next Person:

key = new kms.Key(this, 'Key);
key.addToResourcePolicy(
  new PolicyStatement({
    sid: 'Allow SNS to use this key',
    effect: iam.Effect.ALLOW,
    principals: [new iam.ServicePrincipal('sns.amazonaws.com')],
    actions: ['kms:Decrypt', 'kms:GenerateDataKey'],
    resources: ['*'],
  }),
);

mySqs = new aws-sqs.Queue(this, 'MyQ', {
  encryption: aws-sqs.QueueEncryption.KMS,
  encryptionMasterKey: key,
});
mySqs.addToResourcePolicy(
  new PolicyStatement({
    sid: 'allow sqs actions',
    effect: Effect.ALLOW,
    principals: [new AccountRootPrincipal()],
    actions: ['sqs:*'],
    resources: ['*'],
  }),
);
const allowSns = new PolicyStatement({
  sid: 'allow publishing from the sns',
  effect: Effect.ALLOW,
  principals: [new ServicePrincipal('sns.amazonaws.com')],
  actions: ['sqs:sendMessage'],
  resources: ['*'],
});
allowSns.addCondition('ArnLike', {
  // Annoyingly, you can't just ref the ARN. Chicken & egg.
  // Or at least I don't know how to get around the issue without resorting to L1 stuff.
  'aws:SourceArn': `arn:aws:sns:${props.env.region}:${props.env.account}:${mySnsName}`,
});
mySqs.addToResourcePolicy(allowSns);

myTopic = new aws-sns.Topic(this, 'MyTopic', {
  topicName: mySnsName,
  masterKey: key,
});
myTopic.addSubscription(new aws-sns-subscriptions.SqsSubscription(mySqs));

---

This is 🐛 Bug Report

[jumi-dev]

I analyzed your example to figure out what is really missing.

const key = new kms.Key(this, 'Key');

const mySqs = new sqs.Queue(this, 'MyQueue', {
  encryption: sqs.QueueEncryption.KMS,
  encryptionMasterKey: key,
});

const myTopic = new sns.Topic(this, 'MyTopic', {
  masterKey: key,
});

myTopic.addSubscription(new subscriptions.SqsSubscription(mySqs));

1. I agree that the following KMS permission is missing. This is already requested in issue [cloudwatch] SQS connected to SNS don't receive data when using KMS #11122.

key.addToResourcePolicy(
  new PolicyStatement({
    sid: 'Allow SNS to use this key',
    effect: iam.Effect.ALLOW,
    principals: [new iam.ServicePrincipal('sns.amazonaws.com')],
    actions: ['kms:Decrypt', 'kms:GenerateDataKey'],
    resources: ['*'],
  }),
);

2. Is this part necessary? Where do you get an error if you remove this policy? I could publish a message to the SNS topic and receive it in the SQS queue without this policy.

mySqs.addToResourcePolicy(
  new PolicyStatement({
    sid: 'allow sqs actions',
    effect: Effect.ALLOW,
    principals: [new AccountRootPrincipal()],
    actions: ['sqs:*'],
    resources: ['*'],
  }),
);

3. SQS Queue policy to allow send messages from SNS:

const allowSns = new PolicyStatement({
  sid: 'allow publishing from the sns',
  effect: Effect.ALLOW,
  principals: [new ServicePrincipal('sns.amazonaws.com')],
  actions: ['sqs:sendMessage'],
  resources: ['*'],
});
allowSns.addCondition('ArnLike', {
  // Annoyingly, you can't just ref the ARN. Chicken & egg.
  // Or at least I don't know how to get around the issue without resorting to L1 stuff.
  'aws:SourceArn': `arn:aws:sns:${props.env.region}:${props.env.account}:${mySnsName}`,
});
mySqs.addToResourcePolicy(allowSns);

In my example, CDK already generates a queue policy for SNS service principal. Can you please double check this part again? Do you really need to add this policy?

"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"
      }
    ]
  }
}

[github-actions]

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

[ahammond]

@jumi-dev thank you for looking!

I think part 2 was necessary for older versions of CDK and could have been implemented using the trustAccountIdentities: true parameter when declaring the key. Newer CDK already does this by default for new KMS keys. That said, I don't know what happens if you pass it a non-default key policy. I think that .addSubscription() should always ensure that at least enough permissions are granted that the subscription works.

Regarding the 3rd stanza, it was required when I initially wrote this. I don't know if it is still required since it's been almost a year. I'd be happy to see that it isn't required anymore.

[ayozemr]

I dont know if its relateed to this, but last week I did this type of sqs subscribed to sns topic and messages didnt arrive the queue... I have been following this to know if may be related... I was able to see subscription in aws console, but because messages where not arriving sqs, I thought if it may be related to policy mentioned here...

This was my code

const SNSTopic = sns.Topic.fromTopicArn(
  this,
  'SNSTopic',
  ssm.StringParameter.fromStringParameterName(
    this,
    'SNSTopicArn',
    'Topic_arn_parameter_name'
  ).stringValue
);

const processingQueue = new sqs.Queue(this, 'Queue', {
  encryption: sqs.QueueEncryption.KMS_MANAGED,
  removalPolicy: cdk.RemovalPolicy.DESTROY,
});

SNSTopic.addSubscription(
  new sns_subscriptions.SqsSubscription(processingQueue, {
    rawMessageDelivery: true,
    filterPolicy: {
      // Some filter policy
    },
  })
);

[ahammond]

@ayozemr you must use a CMK and not sqs.QueueEncryption.KMS_MANAGED for this. Otherwise you can't grant the SNS permissions necessary to publish to your queue. And yes, that's kind of the point of this bug. At the very least, IMHO, cdk should throw an error if you try to .addSubscription() to an encrypted queue without having the CMK, etc set up.

[ayozemr]

@ahammond thanks for the info, will modify code with that in mind

And yes, throwing and error would help a lot to know whats the problem. Because you end in a situation where events don't reach SQS with no info why.

Thanks again

[kornicameister]

What's the status here? Stumbled on this few days ago?

[HansFalkenberg-Visma]

This was fixed by #2504 released in https://github.com/aws/aws-cdk/releases/tag/v1.111.0 (July 2nd 2021).

As others have noted, you have to specify encryptionMasterKey for your SQS because you must use a custom CMK, not an AWS key. @ahammond, there is an open ticket to verify that: #19796

@ayozemr, you can (now?) get information about why your SNS messages don't reach SQS. You can set up delivery status logging to CloudWatch for SNS failures (and successes). Unfortunately it is not supported by CloudFormation, so that will have to be done outside CDK for quite some time. https://docs.aws.amazon.com/sns/latest/dg/sns-topic-attributes.html#topics-attrib

[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.