[aws-kms] Limit default KMS policy (allowAccountToAdmin) to ROOT only

[OlivierPT]

[Not a Contribution]

Default KMS policy created with allowAccountToAdmin() function (trustAccountIdentities=false) is too large and allow all IAM entities to manage the Key.
At least, there should be an additionnal option to restrict it to ROOT only. Or restriction to ROOT only should be the default option.

Proposed solution is to add a condition in the statement as below:

{
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::AccountID:root"
            },
            "Action": [
                "kms:Create*",
                "kms:Describe*",
                "kms:Enable*",
                "kms:List*",
                "kms:Put*",
                "kms:Update*",
                "kms:Revoke*",
                "kms:Disable*",
                "kms:Get*",
                "kms:Delete*",
                "kms:ScheduleKeyDeletion",
                "kms:CancelKeyDeletion",
                "kms:GenerateDataKey",
                "kms:TagResource",
                "kms:UntagResource"
            ],
            "Resource": "*",
            "Condition": {
                "ArnLike": {
                    "aws:PrincipalArn": "arn:aws:iam::AccountID:root"
                }
            }
        }

Reproduction Steps

Create a KMS key with default policy:

new kms.Key(this, 'KmsKey', {
      alias: 'kms-key',
      enableKeyRotation: true
    });

Error Log

No error produced by CDK

Environment

• CDK CLI Version: 1.49.1
• Module Version: aws-kms
• Node.js Version: v12.13.1
• OS: All
• Language: all

[njlynch]

Thanks for filing this request.

We are consulting internally with the KMS team about the implications of this request to ensure we're modeling this properly, and understand the impacts, as we generally discourage root-user access where possible. We will update this issue once we have some more guidance.

[njlynch]

In the meantime, as a work-around, you can use escape hatches to alter the policy assigned to the default key. I quickly verified this generated the policy you referenced above:

const key = new Key(stack, 'MyKey');
const keyPolicyAsJson = ((key.node.defaultChild as CfnKey).keyPolicy as iam.PolicyDocument).toJSON();
keyPolicyAsJson.Statement[0].Condition = { ArnLike: { AWS: new iam.AccountRootPrincipal().arn } };
(key.node.defaultChild as CfnKey).keyPolicy = iam.PolicyDocument.fromJson(keyPolicyAsJson);

[OlivierPT]

Hi,

Thank you for this tip. I will test it.
Regarding avoid root-user access, I fully agree. So maybe a possibility would be to be able to provide "admin role(s)" to the construct so we avoid to have the policy using :

Principal:` { "AWS": "arn:aws:iam::AccountID:root" }

[njlynch]

@OlivierPT,

How has the work-around worked out for you?

I think there are potentially two follow-ups here:

1. Make it easier for customers to specify key admins to be added to the key admin policy. In line with best practices, this would be in addition to, not instead of, the account root as key admin. Restricting access to the key in this case is done via IAM policies -- same as most other services. This would be the recommended way to add new key admins to the policy.

2. Provide users who have a custom key policy with the option of "opting out" of the default key policy. This would allow restricting key admin to only specific users or roles, with the acknowledgement that if those users or roles are removed, the key is effectively locked and cannot be administered without contacting support.

It sounds like for your use case, #2 (or the workaround already provided) is what you need. Can you confirm something like that would suit your needs? I am picturing a new boolean for KeyProps (e.g., KeyProps.excludeDefaultKeyPolicy) that can be specified along with a policy (KeyProps.policy) that would just take the provided policy as-is without adding any of the "default" admin policies.

[OlivierPT]

Hello @njlynch,

Your proposition for the workaround worked fine for us. We didn't implement it exactly as you proposed but we were able to use the escape hatches solution to get what we wanted. That was a great proposition since we were not aware of this capacity with CDK.

Regarding the follow-up you proposed. They look good indeed and I think they would allow us to implement our need in a more standard way. To be sure, that would be interesting to see exactly what would be the generated policies... mainly for the proposal (1). But I think that with the combination of the 2 proposals, it will match.

Thank you,