fix(sns-subscriptions): encrypted SQS subscribed to SNS don't receive data #12146
Conversation
|
|
| @@ -46,6 +46,11 @@ export class SqsSubscription implements sns.ITopicSubscription { | |||
| }, | |||
| })); | |||
|
|
|||
| // 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')); | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
bot
dismissed
MrArnoldPalmer’s stale review
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
RomainMuller
force-pushed
the
master
branch
from
924c117 to
ebfd5f2
Compare
rix0rrr
added
bug
|
This change was merged as part of PR #14960. Closing this. |
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