Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grantSendMessages of encrypted SQS to Lambda function not generating kms:Decrypt permission #6609

Closed
aherzog-trx opened this issue Mar 6, 2020 · 7 comments · Fixed by #7522
Assignees
Labels
@aws-cdk/aws-sqs Related to Amazon Simple Queue Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. guidance Question that needs advice or information. in-progress This issue is being actively worked on. p1

Comments

@aherzog-trx
Copy link

aherzog-trx commented Mar 6, 2020

General Issue

We deployed a SQS queue to Account-A that is encrypted with a CMK that resides in Account-B. Permissions of CMK are set to grant allow kms:* to AccountPrincipal('Account-A').
Then we instantiate ourLambda function that is granted write permissions to ourQueue using ourQueue.grantSendMessages(ourLambda).

const sqsKey = Key.fromKeyArn('arn:aws:kms:eu-central-1:0987654321:key/ourKeyId');

const ourQueue = new Queue(this, 'OurQueue', {
    encryption: QueueEncryption.KMS,
    encryptionMasterKey: sqsKey
});

const ourLambda = new Function(this, 'OurLambda', {...});

ourQueue.grantSendMessages(ourLambda);

This results in

"Action": [
    kms:Encrypt",
    kms:ReEncrypt*",
    kms:GenerateDataKey*"
],
"Resource": "arn:aws:kms:eu-central-1:0987654321:key/ourKeyId",
"Effect": "Allow"

being added to ourLambdaExecutionRoleDefaultPolicy.

Lambda function execution failed with KMS AccessDeniedException when sending a message to ourQueue.

After adding

sqsKey.grantDecrypt(ourLambda);

the lambda function executes successfully.

The Question

Is kms:Decrypt strictly necessary to execute sendMessage to an SSE enabled SQS queue? Is this due to the CMK residing in a seperate AWS account?
If so, should kms:Decrypt be added to the Lambda execution role policy when Queue.grantSendMessage(Lambda) is used?

Environment

  • CDK CLI Version: 1.27.0 (build a98c0b3)
  • Module Version: 1.27.0
  • OS: Windows 10
  • Language: TypeScript

Other information

Thanks for the good work!

@SomayaB SomayaB added @aws-cdk/aws-sqs Related to Amazon Simple Queue Service guidance Question that needs advice or information. labels Mar 6, 2020
@MrArnoldPalmer MrArnoldPalmer added feature-request A feature should be added or improved. p1 labels Mar 6, 2020
@MrArnoldPalmer
Copy link
Contributor

Good catch!
After some reading it looks like that accounts in other accounts require the kms:GenerateDataKey* to write to an encrypted queue.

My instinct is to automatically add this permission in the role passed to grantSendMessage when the queue is encrypted and in another account (I think we can detect this reliably).

Other option is to make sure this is documented somewhere that users who need it are going to find it. Probably both.

@rix0rrr knows some about this and may have some opinions.

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 9, 2020

I think this is about the Lambda reading from the queue, right, rather than having to do with grantSendMessages() ?

@aherzog-trx
Copy link
Author

The Lambda writing to the queue failed until we added

"Action": [
    kms:Decrypt"
],
"Resource": "arn:aws:kms:eu-central-1:0987654321:key/ourKeyId",
"Effect": "Allow"

to the Lambda's ExecutionRoleDefaultPolicy.

@ishassan
Copy link

I have the same issue.

Apparently grantSendMessage() isn't enough to send encrypted messages. You'd need to allow kms:Decrypt as well, which doesn't make a lot of sense to me.

@MrArnoldPalmer MrArnoldPalmer added the effort/small Small work item – less than a day of effort label Mar 14, 2020
@shreyasdamle
Copy link
Contributor

Does kms:Decrypt action is required for every instance of grantSendMessage() to send encrypted messages or is it just required when CMK is in another account?

If it only requires for the second case, can we check if key is from the different account before granting kms:Decrypt permission for grantSendMessage() with something like this:

private isKeyFromAnotherAccount(key: kms.IKey): boolean {
  if (!(Construct.isConstruct(key))) {
    return false;
  }
  const bucketStack = Stack.of(this);
  const identityStack = Stack.of(key);
  return bucketStack.account !== identityStack.account;
}

@ishassan
Copy link

In my case, everything is in the same account, yet a decrypt permission is still required to send messages.

@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Apr 23, 2020
@mergify mergify bot closed this as completed in #7522 May 4, 2020
mergify bot pushed a commit that referenced this issue May 4, 2020
Added 'kms:Decrypt' action to `grantSendMessages`

Fixes: #6609
@csbszb
Copy link

csbszb commented Sep 3, 2020

I have the same issue when I create a FIFO queue with an EventBus target and set the following encryption value:
encryption: sqs.QueueEncryption.KMS_MANAGED

I followed the instruction in this article but this also didn't fix the problem.

When I change the encryption back to unencrypted, the messages are delivered to the queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-sqs Related to Amazon Simple Queue Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. guidance Question that needs advice or information. in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants