kms.Key: When custom policy is provided, key.grant doesn't always add to key policy

[faridnsh]

Describe the bug

In our organization, the default policy of KMS construct that allows everything for the account is not acceptable. Instead we prefer to have something that allows admins and the deployer role to do management actions but not actually encryption related actions, therefore we set the keys similar to the following:

        key = kms.Key(
            self,
            id,
            alias=alias,
            enable_key_rotation=True,
            policy=iam.PolicyDocument(),
        )

        key.add_to_resource_policy(
            iam.PolicyStatement(
                actions=[
                    "kms:CancelKeyDeletion",
                    "kms:Create*",
                    "kms:Delete*",
                    "kms:Describe*",
                    "kms:Disable*",
                    "kms:Enable*",
                    "kms:Get*",
                    "kms:List*",
                    "kms:Put*",
                    "kms:Revoke*",
                    "kms:ScheduleKeyDeletion",
                    "kms:TagResource",
                    "kms:UntagResource",
                    "kms:Update*",
                ],
                principals=[deployer_principal, admin_principal],
                resources=["*"],
            )
        )

However due to @aws-cdk/aws-kms:defaultKeyPolicies feature flag that we can't turn off, KMS assume we have default policy of KMS that allows the account principals for everything. trustAccountIdentities must be also set to true.

This is problematic because now .grant methods rely on that assumption to not grant on the resource policy when it's not cross account.

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

.grant modify the resource policy when the action is not allowed.

Current Behavior

.grant not modifying the resource policy when the action is not allowed.

Reproduction Steps

1. Define a key with a custom policy document that allows nothing.
2. Call key.grant with a same-account principal
3. Check key policy to see that nothing has been added.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

v2.171.0

Framework Version

No response

Node.js Version

Nodejs 22

OS

Mac

Language

Python

Language Version

No response

Other information

No response

[khushail]

Hi @faridnsh , thanks for reaching out. Could you please provide a minimal self contained code for reproduction of the issue?

[faridnsh]

Here's some code that shows the issue:

#!/usr/bin/env python3
import aws_cdk as cdk
from aws_cdk import aws_kms as kms, aws_iam as iam
from aws_cdk.assertions import Template, Match

app = cdk.App()
stack = cdk.Stack(app, "KmsExample")
key = kms.Key(
    stack,
    "Key",
    enable_key_rotation=True,
    policy=iam.PolicyDocument(),
)

key.add_to_resource_policy(
    iam.PolicyStatement(
        actions=[
            "kms:CancelKeyDeletion",
            "kms:Create*",
            "kms:Delete*",
            "kms:Describe*",
            "kms:Disable*",
            "kms:Enable*",
            "kms:Get*",
            "kms:List*",
            "kms:Put*",
            "kms:Revoke*",
            "kms:ScheduleKeyDeletion",
            "kms:TagResource",
            "kms:UntagResource",
            "kms:Update*",
        ],
        principals=[iam.AccountRootPrincipal()],
        resources=["*"],
    )
)
role = iam.Role.from_role_name(stack, "Role", "Role")
key.grant_decrypt(role)
app.synth()

Template.from_stack(stack).has_resource_properties(
    "AWS::KMS::Key",
    {
        "KeyPolicy": {
            "Statement": Match.array_with(
                [
                    Match.object_like(
                        {
                            "Action": "kms:Decrypt",
                        }
                    )
                ]
            )
        }
    },
)

[khushail]

Hi @faridnsh , thanks for sharing the code for repro of the issue.

While running this code -

        key = kms.Key(
            self,
            "Key",
            enable_key_rotation=True,
            policy=iam.PolicyDocument(),
        )

        key.add_to_resource_policy(
            iam.PolicyStatement(
            actions=[
                "kms:CancelKeyDeletion",
                "kms:Create*",
                "kms:Delete*",
                "kms:Describe*",
                "kms:Disable*",
                "kms:Enable*",
                "kms:Get*",
                "kms:List*",
                "kms:Put*",
                "kms:Revoke*",
                "kms:ScheduleKeyDeletion",
                "kms:TagResource",
                "kms:UntagResource",
                "kms:Update*",
            ],
            principals=[iam.AccountRootPrincipal()],
            resources=["*"],
        )
        )
        role = iam.Role.from_role_name(self, "Role", "Role")
        key.grant_decrypt(role)

and here is synthesized template -

    {
    "Resources": {
    "Key961B73FD": {
    "Type": "AWS::KMS::Key",
    "Properties": {
        "EnableKeyRotation": true,
        "KeyPolicy": {
        "Statement": [
        {
        "Action": [
            "kms:CancelKeyDeletion",
            "kms:Create*",
            "kms:Delete*",
            "kms:Describe*",
            "kms:Disable*",
            "kms:Enable*",
            "kms:Get*",
            "kms:List*",
            "kms:Put*",
            "kms:Revoke*",
            "kms:ScheduleKeyDeletion",
            "kms:TagResource",
            "kms:UntagResource",
            "kms:Update*"
        ],
        "Effect": "Allow",
        "Principal": {
            "AWS": {
            "Fn::Join": [
            "",
            [
            "arn:",
            {
                "Ref": "AWS::Partition"
            },
            ":iam::",
            {
                "Ref": "AWS::AccountId"
            },
            ":root"
            ]
            ]
            }
        },
        "Resource": "*"
        }
        ],
        "Version": "2012-10-17"
        }
    },
    "UpdateReplacePolicy": "Retain",
    "DeletionPolicy": "Retain",
    "Metadata": {
        "aws:cdk:path": "KmsIssueStack/Key/Resource"
    }
    },
    "RolePolicyKmsIssueStackRole4AA23D9BFE5761E8": {
    "Type": "AWS::IAM::Policy",
    "Properties": {
        "PolicyDocument": {
        "Statement": [
        {
        "Action": "kms:Decrypt",
        "Effect": "Allow",
        "Resource": {
            "Fn::GetAtt": [
            "Key961B73FD",
            "Arn"
            ]
        }
        }
        ],
        "Version": "2012-10-17"
        },
        "PolicyName": "PolicyKmsIssueStackRole4AA23D9B",
        "Roles": [
        "Role"
        ]
    },

so an IAM role is created with kms:Decrypt with reference to KMS key.

Sharing the insights -

• AWS Docs - https://github.com/aws/aws-cdk/tree/main/packages/aws-cdk-lib/aws-kms state that the @aws-cdk/aws-kms:defaultKeyPolicies flag is set by default which enables KMS keys can be created with IAM Policies. So you are setting the Key initially with blank which is resetting it to null and enabling the behavior of replacing/appending policies to the key.
• Also this flag trustAccountIdentities is used for existing projects for enabling the same behavior as listed in here -

so this example listed in the snapshot above mentions that

key.grantEncrypt(user); // Adds encrypt permissions to user policy; key policy is unmodified.

encrypt permissions is added to user policy. In your case, its the role that is being granted permission, which seems like exepected behavior-

aws-cdk/packages/aws-cdk-lib/aws-kms/lib/key.ts

Line 201 in ce3598b

public grantDecrypt(grantee: iam.IGrantable): iam.Grant {

Hope that makes sense. Please feel free to share your thoughts if something is misunderstood.
Thanks

[github-actions]

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

[faridnsh]

HI @khushail thanks for the in-depth analysis. Yes, the behaviour makes sense. I guess what I'm asking here is more a feature than a bug if you put it that way. What I would like is to enable the use case for organizations that need least priviledged KMS key policies that not every action is allowed by everyone in the account and they can use .grant methods to simplify their code.

Regarding the issue of user or role being deleted, in cases that we'd like to ensure there's no hard dependency on the principal due to the KMS policy we use conditions to specify the principal ARN which allow us to create the role after KMS policy is created.

[khushail]

@faridnsh , thanks for getting back and sharing your point of view. from Feature perspective, I agree with you on the case when not all permissions should be added by default. But this depends on situation as well. Like it works for now as - initially grant null policies and then grant on need basis but thats a bit of a hassle and it would have been much better to provide the minimum required policies first and then upgrade the access as needed.

Since we currently have a workaround (keeping the policies null initially and then grating user access as needed) I am converting this bug to FR and marking it as P3(which means it won't be immediately looked at by the team), leaving the room for contribution by the CDK community members as well as from the team. It might be breaking change and lead to access issues, so would request Upvotes from the community members to have more eyes on it for discussion and implementation.

Also requesting Core team's input on this request as does this change make sense and would be something they would like to get contributions on.