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

(secretsmanager ): grant_read/grant_write on secret throws KMS key error on role from same account #15450

Closed
mooiweer opened this issue Jul 7, 2021 · 7 comments
Labels
@aws-cdk/aws-kms Related to AWS Key Management @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. p2

Comments

@mooiweer
Copy link

mooiweer commented Jul 7, 2021

Giving a preexisting role read/write access to a secret using the grant_read/grant_write method on a secret throws the error:
"KMS Key must be provided for cross account access to Secret"
Even though both secret and role are from the same AWS account.

Reproduction Steps

Use an existing role and create a new secret
Give it read/write access

from aws_cdk import (
    aws_iam as iam,
    aws_secretsmanager as secretsmanager,
    core
)

class Stack(core.Stack):
  role = iam.Role.from_role_arn(self, "role", <existing iam role>)
  
  secret = secretsmanager.Secret(
      scope=self,
      id="secret-id",
      description='secret desc'
  )
  secret.grant_read(role)
  secret.grant_write(role)


app = core.App()
Stack(app, "SecretKMSError")
app.synth()

What did you expect to happen?

I expect the created secret to be created with read and write rights added to the existing role.

What actually happened?

CDK deploy throws an error:
KMS Key must be provided for cross account access to Secret
Even though both the (pre-existing) and the secret are on the same AWS account

Environment

  • CDK CLI Version : 1.111.0
  • Framework Version: 1.111.0
  • Node.js Version: v12.20.1
  • OS : Windows 10
  • Language (Version): Python (3.8)

Other

This worked before version 1.111.0

Workaround:

role.add_to_policy(iam.PolicyStatement(effect=iam.Effect.ALLOW, actions=['secretsmanager:PutSecretValue', 'secretsmanager:UpdateSecret', 'secretsmanager:GetSecretValue', 'secretsmanager:DescribeSecret'], resources=[secret._arn_for_policies]))

I think it was introduced by this commit:
ea40cfe


This is 🐛 Bug Report

@mooiweer mooiweer added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 7, 2021
@github-actions github-actions bot added @aws-cdk/aws-kms Related to AWS Key Management @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager labels Jul 7, 2021
@madeline-k
Copy link
Contributor

Hi @mooiweer , I am not able to reproduce this error if I use a Role.fromRoleArn role that is inside the same account as my stack environment.

However, I can reproduce the error if I supply an ARN such as: "arn:aws:iam::123456789012:role/MyRole-AJJHDSKSDF" where the account id inside the ARN does not match the account id of my stack's environment.

Can you please verify that the ARN of the role is in fact in the same account that your stack environment is using?

@madeline-k madeline-k added the needs-reproduction This issue needs reproduction. label Jul 15, 2021
@nwouda
Copy link

nwouda commented Jul 16, 2021

I'm experiencing the same issue. In my case the ARN of the role is stored in the parameter store. This is indeed introduced in version 1.111.0. The ARN of the role is the same as the account deployed to, but my best bet is that CDK can't tell because it doesn't actively pull it from SSM.

@madeline-k
Copy link
Contributor

Thanks for the extra detail, @nwouda. This seems to be a case where the CDK is performing validation in some cases where we can't actually know the value, like referencing an ARN in SSM. And agreed this is probably a regression introduced in 1.111.0.

Despite it being a regression, I am going to triage as a p2 since customers have a workaround which is to explicitly add to the role's policy instead of using the grant functions. p2 means that the CDK team won't be able to prioritize this right now, but we always welcome contributions!

@madeline-k
Copy link
Contributor

@nwouda and @mooiweer if either of you are interesting in fixing this yourself, please take a look at the contributing guide.

@madeline-k madeline-k added effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 16, 2021
@njlynch njlynch removed their assignment Jul 23, 2021
@jrruethe
Copy link

I am affected by this, and I believe the issue was introduced with this commit:
#14834

mergify bot pushed a commit that referenced this issue Jan 18, 2022
…cess (#17812)

Fix for #15450 Previous code did not check if the account IDs were the different. This checks if CDK is able to resolve the account ids and they are different then fail otherwise let the user create a secret.

FYI first PR. Let me know if there is something that I missed.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@kaiz-io
Copy link
Contributor

kaiz-io commented Jan 25, 2022

This can be closed was fixed in https://github.com/aws/aws-cdk/releases/tag/v1.140.0

@github-actions
Copy link

⚠️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.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…cess (aws#17812)

Fix for aws#15450 Previous code did not check if the account IDs were the different. This checks if CDK is able to resolve the account ids and they are different then fail otherwise let the user create a secret.

FYI first PR. Let me know if there is something that I missed.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-kms Related to AWS Key Management @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. p2
Projects
None yet
Development

No branches or pull requests

6 participants