Skip to content

(sns-subscriptions): add SourceArn condition to CMK policy when subscribing SQS to SNS #20339

@HansFalkenberg-Visma

Description

@HansFalkenberg-Visma

Describe the feature

When topic.addSubscription is used with an SqsSubscription referencing a queue with SSE encryption enabled, the queue's CMK policy is extended to allow the SNS service principal for all accounts. This was implemented in #14960

The generated statement should include a condition to restrict usage to the specific topic, ie.

            "Condition": {
                "ArnEquals": {
                    "aws:SourceArn": "arn:aws:sns:REGION:ACCOUNT:TOPIC"
                }
            }

Use Case

To help prevent an AWS service from being used as a confused deputy in a policy where the principal is an AWS service principal, you can use the aws:SourceArn or aws:SourceAccount global condition keys.

https://docs.aws.amazon.com/kms/latest/developerguide/policy-conditions.html

This was apparently not possible before. While this is a security improvement, I do not believe it rates reporting as a security vulnerability, because

Proposed Solution

The implementation itself should be trivial because the necessary code is already in https://github.com/aws/aws-cdk/blob/v2-main/packages/%40aws-cdk/aws-sns-subscriptions/lib/sqs.ts for the queue's policy.

Simply add three lines to

    if (this.queue.encryptionMasterKey) {
      this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement({
        resources: ['*'],
        actions: ['kms:Decrypt', 'kms:GenerateDataKey'],
        principals: [snsServicePrincipal],
      }));
    }

like so

    if (this.queue.encryptionMasterKey) {
      this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement({
        resources: ['*'],
        actions: ['kms:Decrypt', 'kms:GenerateDataKey'],
        principals: [snsServicePrincipal],
        conditions: {
          ArnEquals: { 'aws:SourceArn': topic.topicArn },
        },
      }));
    }

I don't know that a workaround is currently possible. I tried adding the more restrictive policy (with the condition) to the CMK first, but CDK will always add the more permissive policy (without the condition) with no (easily available) interfaces for removing it again.

Other Information

This would not be a breaking change to the CDK API, but the generated policy would be more restrictive. Solutions that have -- erroneously, I would say -- depended on the too permissive policy for use by SNS topics not managed by the same CDK application/stack might have issues. They have the simple fix of explicitly adding back the permissive policy statement to the CMK.

The following is the exact statement that was generated before the proposed change. After the change it would be replaced by a more restrictive statement, one per topic (with each topic's ARN as a condition). This can of course be used as is, but it might be just as easy to add statements with ARN conditions for the other topics too:

        {
            "Effect": "Allow",
            "Principal": {
                "Service": "sns.amazonaws.com"
            },
            "Action": [
                "kms:Decrypt",
                "kms:GenerateDataKey"
            ],
            "Resource": "*"
        }

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

v2.24.1

Environment details (OS name and version, etc.)

n/a

Metadata

Metadata

Assignees

No one assigned

    Labels

    @aws-cdk/aws-sns-subscriptionseffort/smallSmall work item – less than a day of effortfeature-requestA feature should be added or improved.p3response-requestedWaiting on additional info and feedback. Will move to "closing-soon" in 7 days.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions