Skip to content

fix(sns-subscriptions): encrypted SQS subscribed to SNS don't receive data#12146

Closed
jumi-dev wants to merge 3 commits intoaws:masterfrom
jumi-dev:sns2encryptedSqs
Closed

fix(sns-subscriptions): encrypted SQS subscribed to SNS don't receive data#12146
jumi-dev wants to merge 3 commits intoaws:masterfrom
jumi-dev:sns2encryptedSqs

Conversation

@jumi-dev
Copy link
Copy Markdown
Contributor

Grant KMS permission to SNS service principal if the queue is encrypted.

fixes #11122


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Dec 17, 2020

Copy link
Copy Markdown
Contributor

@MrArnoldPalmer MrArnoldPalmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just dependency change for now. Looking into scoping the permissions principal as well.


// grant KMS permission to SNS service principal if the queue is encrypted
if (this.queue.encryptionMasterKey) {
this.queue.encryptionMasterKey.grantEncryptDecrypt(new iam.ServicePrincipal('sns.amazonaws.com'));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure there is a way to limit this using the arn of the sns topic in a conditional so we can limit the scope of resources being granted permissions to the key. I'm still looking around for this but let me know if this is something you've tried already.

Additionally my instinct is that we would only need "encrypt" here, though all the documentation seems to point to needing the decrypt permissions as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure there is a way to limit this using the arn of the sns topic in a conditional so we can limit the scope of resources being granted permissions to the key. I'm still looking around for this but let me know if this is something you've tried already.

I didn't find a solution for it yet. The SQS Queue policy supports property aws:SourceArn to specify a SNS ARN (see
https://docs.aws.amazon.com/sns/latest/dg/sns-access-policy-use-cases.html).

The same attribute isn't available in KMS:
AWS KMS supports all AWS global condition keys except for the following ones: aws:SourceArn
(see https://docs.aws.amazon.com/kms/latest/developerguide/policy-conditions.html)

I will also try to find another way to restrict the permission.

Additionally my instinct is that we would only need "encrypt" here, though all the documentation seems to point to needing the decrypt permissions as well.

Yes, I tried to use only "encrypt" which wasn't sufficient. Therefore, the documentation seems to be correct.
Is it fine to use grantEncryptDecrypt? Or is it better to grant only the actions kms:Decrypt and kms:GenerateDataKey* from the documentation?

Copy link
Copy Markdown
Contributor

@MrArnoldPalmer MrArnoldPalmer Jan 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grantEncrypDecrypt is fine if that is what we need which it seems like we do. I'll ask around a bit more about scoping down further and get back to you in the next couple days.. Let me know if you find a solution in the meantime.

Copy link
Copy Markdown

@HansFalkenberg-Visma HansFalkenberg-Visma May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SourceArn can now be used to restrict KMS access the same way as SQS access.

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

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.

I added the following to my KMS policy

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

When it matches the source topic it works, if it doesn't match the source topic, the SNS fails with this error:

"providerResponse": "{\"ErrorCode\":\"KMS.AccessDeniedException\",\"ErrorMessage\":\"null (Service: AWSKMS; Status Code: 400; Error Code: AccessDeniedException; Request ID: 40edd241-7474-4b44-98bf-de5ba09013fb; Proxy: null)\",\"sqsRequestId\":\"Unrecoverable\"}"

@mergify mergify bot dismissed MrArnoldPalmer’s stale review January 13, 2021 21:07

Pull request has been modified.

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 6d615f8
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@TheRealAmazonKendra
Copy link
Copy Markdown
Contributor

This change was merged as part of PR #14960. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue is a bug. p1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cloudwatch] SQS connected to SNS don't receive data when using KMS

6 participants