From ff695daee41b22bfaeef148dd0faa8e451bfd9af Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Wed, 9 Dec 2020 19:21:02 +0000 Subject: [PATCH] feat(kms): change default key policy to align with KMS best practices (under feature flag) (#11918) In #5575, a new flag (`trustAccountIdentities`) was introduced which -- when set -- changes the default key policy from a custom key admin policy to one that grants all access to the key to the root account user. This key policy matches the default policy when a key is created via the KMS APIs or console. For backwards-compatibility reasons, the default for `trustAccountIdentities` had to be set to `false`. Without the flag explicitly set, the default key policy is one that (a) doesn't match the KMS-recommended admin policy and (b) doesn't explicitly enable IAM principal policies to acccess the key. This means that all usage operations (e.g., Encrypt, GenerateDataKey) must be added to both the key policy and to the principal policy. This change introduces a new feature flag to flip the default behavior of the `trustAccountIdentities` flag, so new keys created will have the sane defaults matching the KMS recommended best practices. As a related change, this feature flag also changes the behavior when a user passes in `policy` when creating a Key. Without the feature flag set, the policy is always appended to the default key policy. With the feature flag set, the policy will *override* the default key policy, enabling users to opt-out of the default key policy to introduce a more restrictive policy if desired. This also matches the KMS API behavior, where a policy provided by the user will override the defaults. Marking this PR as `requires-two-approvers` to ensure this PR gets an appropriately-critical review. BREAKING CHANGE: change the default value of trustAccountIdentities to true, which will result in the key getting the KMS-recommended default key policy. This is enabled through the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag. fixes #8977 fixes #10575 fixes #11309 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-kms/README.md | 128 +-- packages/@aws-cdk/aws-kms/lib/key.ts | 115 ++- .../@aws-cdk/aws-kms/lib/private/perms.ts | 28 + packages/@aws-cdk/aws-kms/package.json | 2 + packages/@aws-cdk/aws-kms/test/key.test.ts | 783 ++++++++++++------ packages/@aws-cdk/cx-api/lib/features.ts | 17 + 6 files changed, 728 insertions(+), 345 deletions(-) create mode 100644 packages/@aws-cdk/aws-kms/lib/private/perms.ts diff --git a/packages/@aws-cdk/aws-kms/README.md b/packages/@aws-cdk/aws-kms/README.md index 0f9c4c6df67cc..ac7c9c2571a1e 100644 --- a/packages/@aws-cdk/aws-kms/README.md +++ b/packages/@aws-cdk/aws-kms/README.md @@ -31,8 +31,6 @@ key.addAlias('alias/bar'); ## Sharing keys between stacks -> see Trust Account Identities for additional details - To use a KMS key in a different stack in the same CDK application, pass the construct to the other stack: @@ -41,8 +39,6 @@ pass the construct to the other stack: ## Importing existing keys -> see Trust Account Identities for additional details - To use a KMS key that is not defined in this CDK app, but is created through other means, use `Key.fromKeyArn(parent, name, ref)`: @@ -72,61 +68,29 @@ Note that calls to `addToResourcePolicy` and `grant*` methods on `myKeyAlias` wi no-ops, and `addAlias` and `aliasTargetKey` will fail, as the imported alias does not have a reference to the underlying KMS Key. -## Trust Account Identities - -KMS keys can be created to trust IAM policies. This is the default behavior in -the console and is described -[here](https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html). -This same behavior can be enabled by: +## Key Policies -```ts -new Key(stack, 'MyKey', { trustAccountIdentities: true }); -``` +Controlling access and usage of KMS Keys requires the use of key policies (resource-based policies attached to the key); +this is in contrast to most other AWS resources where access can be entirely controlled with IAM policies, +and optionally complemented with resource policies. For more in-depth understanding of KMS key access and policies, see -Using `trustAccountIdentities` solves many issues around cyclic dependencies -between stacks. The most common use case is creating an S3 Bucket with CMK -default encryption which is later accessed by IAM roles in other stacks. +* https://docs.aws.amazon.com/kms/latest/developerguide/control-access-overview.html +* https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html -stack-1 (bucket and key created) +KMS keys can be created to trust IAM policies. This is the default behavior for both the KMS APIs and in +the console. This behavior is enabled by the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag, +which is set for all new projects; for existing projects, this same behavior can be enabled by +passing the `trustAccountIdentities` property as `true` when creating the key: ```ts -// ... snip -const myKmsKey = new kms.Key(this, 'MyKey', { trustAccountIdentities: true }); - -const bucket = new Bucket(this, 'MyEncryptedBucket', { - bucketName: 'myEncryptedBucket', - encryption: BucketEncryption.KMS, - encryptionKey: myKmsKey -}); +new kms.Key(stack, 'MyKey', { trustAccountIdentities: true }); ``` -stack-2 (lambda that operates on bucket and key) - -```ts -// ... snip - -const fn = new lambda.Function(this, 'MyFunction', { - runtime: lambda.Runtime.NODEJS_10_X, - handler: 'index.handler', - code: lambda.Code.fromAsset(path.join(__dirname, 'lambda-handler')), -}); - -const bucket = s3.Bucket.fromBucketName(this, 'BucketId', 'myEncryptedBucket'); - -const key = kms.Key.fromKeyArn(this, 'KeyId', 'arn:aws:...'); // key ARN passed via stack props - -bucket.grantReadWrite(fn); -key.grantEncryptDecrypt(fn); -``` - -The challenge in this scenario is the KMS key policy behavior. The simple way to understand -this, is IAM policies for account entities can only grant the permissions granted to the -account root principle in the key policy. When `trustAccountIdentities` is true, -the following policy statement is added: +With either the `@aws-cdk/aws-kms:defaultKeyPolicies` feature flag set, +or the `trustAccountIdentities` prop set, the Key will be given the following default key policy: ```json { - "Sid": "Enable IAM User Permissions", "Effect": "Allow", "Principal": {"AWS": "arn:aws:iam::111122223333:root"}, "Action": "kms:*", @@ -134,9 +98,67 @@ the following policy statement is added: } ``` -As the name suggests this trusts IAM policies to control access to the key. -If account root does not have permissions to the specific actions, then the key -policy and the IAM policy for the entity (e.g. Lambda) both need to grant -permission. +This policy grants full access to the key to the root account user. +This enables the root account user -- via IAM policies -- to grant access to other IAM principals. +With the above default policy, future permissions can be added to either the key policy or IAM principal policy. +```ts +const key = new kms.Key(stack, 'MyKey'); +const user = new iam.User(stack, 'MyUser'); +key.grantEncrypt(user); // Adds encrypt permissions to user policy; key policy is unmodified. +``` + +Adopting the default KMS key policy (and so trusting account identities) +solves many issues around cyclic dependencies between stacks. +Without this default key policy, future permissions must be added to both the key policy and IAM principal policy, +which can cause cyclic dependencies if the permissions cross stack boundaries. +(For example, an encrypted bucket in one stack, and Lambda function that accesses it in another.) + +### Appending to or replacing the default key policy + +The default key policy can be amended or replaced entirely, depending on your use case and requirements. +A common addition to the key policy would be to add other key admins that are allowed to administer the key +(e.g., change permissions, revoke, delete). Additional key admins can be specified at key creation or after +via the `grantAdmin` method. + +```ts +const myTrustedAdminRole = iam.Role.fromRoleArn(stack, 'TrustedRole', 'arn:aws:iam:....'); +const key = new kms.Key(stack, 'MyKey', { + admins: [myTrustedAdminRole], +}); + +const secondKey = new kms.Key(stack, 'MyKey2'); +secondKey.grantAdmin(myTrustedAdminRole); +``` + +Alternatively, a custom key policy can be specified, which will replace the default key policy. + +> **Note**: In applications without the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag set +and with `trustedAccountIdentities` set to false (the default), specifying a policy at key creation _appends_ the +provided policy to the default key policy, rather than _replacing_ the default policy. + +```ts +const myTrustedAdminRole = iam.Role.fromRoleArn(stack, 'TrustedRole', 'arn:aws:iam:....'); +// Creates a limited admin policy and assigns to the account root. +const myCustomPolicy = new iam.PolicyDocument({ + statements: [new iam.PolicyStatement({ + actions: [ + 'kms:Create*', + 'kms:Describe*', + 'kms:Enable*', + 'kms:List*', + 'kms:Put*', + ], + principals: [new iam.AccountRootPrincipal()], + resources: ['*'], + })], +}); +const key = new kms.Key(stack, 'MyKey', { + policy: myCustomPolicy, +}); +``` +> **Warning:** Replacing the default key policy with one that only grants access to a specific user or role +runs the risk of the key becoming unmanageable if that user or role is deleted. +It is highly recommended that the key policy grants access to the account root, rather than specific principals. +See https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html for more information. diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index db13ee4d761ea..82098129e27a2 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -1,8 +1,10 @@ import * as iam from '@aws-cdk/aws-iam'; -import { IResource, RemovalPolicy, Resource, Stack } from '@aws-cdk/core'; +import { FeatureFlags, IResource, RemovalPolicy, Resource, Stack } from '@aws-cdk/core'; +import * as cxapi from '@aws-cdk/cx-api'; import { IConstruct, Construct } from 'constructs'; import { Alias } from './alias'; import { CfnKey } from './kms.generated'; +import * as perms from './private/perms'; /** * A KMS Key, either managed by this CDK app, or imported. @@ -77,8 +79,9 @@ abstract class KeyBase extends Resource implements IKey { /** * Optional property to control trusting account identities. * - * If specified grants will default identity policies instead of to both - * resource and identity policies. + * If specified, grants will default identity policies instead of to both + * resource and identity policies. This matches the default behavior when creating + * KMS keys via the API or console. */ protected abstract readonly trustAccountIdentities: boolean; @@ -168,35 +171,24 @@ abstract class KeyBase extends Resource implements IKey { } /** - * Grant decryption permisisons using this key to the given principal + * Grant decryption permissions using this key to the given principal */ public grantDecrypt(grantee: iam.IGrantable): iam.Grant { - return this.grant(grantee, - 'kms:Decrypt', - ); + return this.grant(grantee, ...perms.DECRYPT_ACTIONS); } /** - * Grant encryption permisisons using this key to the given principal + * Grant encryption permissions using this key to the given principal */ public grantEncrypt(grantee: iam.IGrantable): iam.Grant { - return this.grant(grantee, - 'kms:Encrypt', - 'kms:ReEncrypt*', - 'kms:GenerateDataKey*', - ); + return this.grant(grantee, ...perms.ENCRYPT_ACTIONS); } /** - * Grant encryption and decryption permisisons using this key to the given principal + * Grant encryption and decryption permissions using this key to the given principal */ public grantEncryptDecrypt(grantee: iam.IGrantable): iam.Grant { - return this.grant(grantee, - 'kms:Decrypt', - 'kms:Encrypt', - 'kms:ReEncrypt*', - 'kms:GenerateDataKey*', - ); + return this.grant(grantee, ...[...perms.DECRYPT_ACTIONS, ...perms.ENCRYPT_ACTIONS]); } /** @@ -293,11 +285,27 @@ export interface KeyProps { /** * Custom policy document to attach to the KMS key. * + * NOTE - If the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag is set (the default for new projects), + * this policy will *override* the default key policy and become the only key policy for the key. If the + * feature flag is not set, this policy will be appended to the default key policy. + * * @default - A policy document with permissions for the account root to * administer the key will be created. */ readonly policy?: iam.PolicyDocument; + /** + * A list of principals to add as key administrators to the key policy. + * + * Key administrators have permissions to manage the key (e.g., change permissions, revoke), but do not have permissions + * to use the key in cryptographic operations (e.g., encrypt, decrypt). + * + * These principals will be added to the default key policy (if none specified), or to the specified policy (if provided). + * + * @default [] + */ + readonly admins?: iam.IPrincipal[]; + /** * Whether the encryption key should be retained when it is removed from the Stack. This is useful when one wants to * retain access to data that was encrypted with a key that is being retired. @@ -311,10 +319,15 @@ export interface KeyProps { * * Setting this to true adds a default statement which delegates key * access control completely to the identity's IAM policy (similar - * to how it works for other AWS resources). + * to how it works for other AWS resources). This matches the default behavior + * when creating KMS keys via the API or console. * - * @default false + * If the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag is set (the default for new projects), + * this flag will always be treated as 'true' and does not need to be explicitly set. + * + * @default - false, unless the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag is set. * @see https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default-allow-root-enable-iam + * @deprecated redundant with the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag */ readonly trustAccountIdentities?: boolean; } @@ -365,12 +378,26 @@ export class Key extends KeyBase { constructor(scope: Construct, id: string, props: KeyProps = {}) { super(scope, id); - this.policy = props.policy || new iam.PolicyDocument(); - this.trustAccountIdentities = props.trustAccountIdentities || false; - if (this.trustAccountIdentities) { - this.allowAccountIdentitiesToControl(); + const defaultKeyPoliciesFeatureEnabled = FeatureFlags.of(this).isEnabled(cxapi.KMS_DEFAULT_KEY_POLICIES); + + this.policy = props.policy ?? new iam.PolicyDocument(); + if (defaultKeyPoliciesFeatureEnabled) { + if (props.trustAccountIdentities === false) { + throw new Error('`trustAccountIdentities` cannot be false if the @aws-cdk/aws-kms:defaultKeyPolicies feature flag is set'); + } + + this.trustAccountIdentities = true; + // Set the default key policy if one hasn't been provided by the user. + if (!props.policy) { + this.addDefaultAdminPolicy(); + } } else { - this.allowAccountToAdmin(); + this.trustAccountIdentities = props.trustAccountIdentities ?? false; + if (this.trustAccountIdentities) { + this.addDefaultAdminPolicy(); + } else { + this.addLegacyAdminPolicy(); + } } const resource = new CfnKey(this, 'Resource', { @@ -384,25 +411,49 @@ export class Key extends KeyBase { this.keyId = resource.ref; resource.applyRemovalPolicy(props.removalPolicy); + (props.admins ?? []).forEach((p) => this.grantAdmin(p)); + if (props.alias !== undefined) { this.addAlias(props.alias); } } - private allowAccountIdentitiesToControl() { + /** + * Grant admins permissions using this key to the given principal + * + * Key administrators have permissions to manage the key (e.g., change permissions, revoke), but do not have permissions + * to use the key in cryptographic operations (e.g., encrypt, decrypt). + */ + public grantAdmin(grantee: iam.IGrantable): iam.Grant { + return this.grant(grantee, ...perms.ADMIN_ACTIONS); + } + + /** + * Adds the default key policy to the key. This policy gives the AWS account (root user) full access to the CMK, + * which reduces the risk of the CMK becoming unmanageable and enables IAM policies to allow access to the CMK. + * This is the same policy that is default when creating a Key via the KMS API or Console. + * @see https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default + */ + private addDefaultAdminPolicy() { this.addToResourcePolicy(new iam.PolicyStatement({ resources: ['*'], actions: ['kms:*'], principals: [new iam.AccountRootPrincipal()], })); - } + /** - * Let users or IAM policies from this account admin this key. + * Grants the account admin privileges -- not full account access -- plus the GenerateDataKey action. + * The GenerateDataKey action was added for interop with S3 in https://github.com/aws/aws-cdk/issues/3458. + * + * This policy is discouraged and deprecated by the '@aws-cdk/aws-kms:defaultKeyPolicies' feature flag. + * * @link https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default - * @link https://aws.amazon.com/premiumsupport/knowledge-center/update-key-policy-future/ + * @deprecated */ - private allowAccountToAdmin() { + private addLegacyAdminPolicy() { + // This is equivalent to `[...perms.ADMIN_ACTIONS, 'kms:GenerateDataKey']`, + // but keeping this explicit ordering for backwards-compatibility (changing the ordering causes resource updates) const actions = [ 'kms:Create*', 'kms:Describe*', diff --git a/packages/@aws-cdk/aws-kms/lib/private/perms.ts b/packages/@aws-cdk/aws-kms/lib/private/perms.ts new file mode 100644 index 0000000000000..fe510359d7e89 --- /dev/null +++ b/packages/@aws-cdk/aws-kms/lib/private/perms.ts @@ -0,0 +1,28 @@ +// https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html + +export const ADMIN_ACTIONS = [ + 'kms:Create*', + 'kms:Describe*', + 'kms:Enable*', + 'kms:List*', + 'kms:Put*', + 'kms:Update*', + 'kms:Revoke*', + 'kms:Disable*', + 'kms:Get*', + 'kms:Delete*', + 'kms:TagResource', + 'kms:UntagResource', + 'kms:ScheduleKeyDeletion', + 'kms:CancelKeyDeletion', +]; + +export const ENCRYPT_ACTIONS = [ + 'kms:Encrypt', + 'kms:ReEncrypt*', + 'kms:GenerateDataKey*', +]; + +export const DECRYPT_ACTIONS = [ + 'kms:Decrypt', +]; diff --git a/packages/@aws-cdk/aws-kms/package.json b/packages/@aws-cdk/aws-kms/package.json index 65c05ca946fbf..04ed686b059de 100644 --- a/packages/@aws-cdk/aws-kms/package.json +++ b/packages/@aws-cdk/aws-kms/package.json @@ -82,12 +82,14 @@ "dependencies": { "@aws-cdk/aws-iam": "0.0.0", "@aws-cdk/core": "0.0.0", + "@aws-cdk/cx-api": "0.0.0", "constructs": "^3.2.0" }, "homepage": "https://github.com/aws/aws-cdk", "peerDependencies": { "@aws-cdk/aws-iam": "0.0.0", "@aws-cdk/core": "0.0.0", + "@aws-cdk/cx-api": "0.0.0", "constructs": "^3.2.0" }, "engines": { diff --git a/packages/@aws-cdk/aws-kms/test/key.test.ts b/packages/@aws-cdk/aws-kms/test/key.test.ts index 93b5d1bc67e31..6560a31381691 100644 --- a/packages/@aws-cdk/aws-kms/test/key.test.ts +++ b/packages/@aws-cdk/aws-kms/test/key.test.ts @@ -1,10 +1,27 @@ -import { ResourcePart } from '@aws-cdk/assert'; +import { arrayWith, ResourcePart } from '@aws-cdk/assert'; import '@aws-cdk/assert/jest'; import * as iam from '@aws-cdk/aws-iam'; -import { App, CfnOutput, RemovalPolicy, Stack, Tags } from '@aws-cdk/core'; -import { Key } from '../lib'; +import * as cdk from '@aws-cdk/core'; +import * as kms from '../lib'; -const ACTIONS: string[] = [ +const ADMIN_ACTIONS: string[] = [ + 'kms:Create*', + 'kms:Describe*', + 'kms:Enable*', + 'kms:List*', + 'kms:Put*', + 'kms:Update*', + 'kms:Revoke*', + 'kms:Disable*', + 'kms:Get*', + 'kms:Delete*', + 'kms:TagResource', + 'kms:UntagResource', + 'kms:ScheduleKeyDeletion', + 'kms:CancelKeyDeletion', +]; + +const LEGACY_ADMIN_ACTIONS: string[] = [ 'kms:Create*', 'kms:Describe*', 'kms:Enable*', @@ -22,160 +39,283 @@ const ACTIONS: string[] = [ 'kms:UntagResource', ]; +let app: cdk.App; +let stack: cdk.Stack; +beforeEach(() => { + app = new cdk.App({ + context: { + // By default, enable the correct key policy behavior. Specific tests will test the disabled behavior. + '@aws-cdk/aws-kms:defaultKeyPolicies': true, + }, + }); + stack = new cdk.Stack(app); +}); + test('default key', () => { - const stack = new Stack(); - - new Key(stack, 'MyKey'); - - expect(stack).toMatchTemplate({ - Resources: { - MyKey6AB29FA6: { - Type: 'AWS::KMS::Key', - Properties: { - KeyPolicy: { - Statement: [ - { - Action: ACTIONS, - Effect: 'Allow', - Principal: { - AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, - }, - Resource: '*', - }, - ], - Version: '2012-10-17', + new kms.Key(stack, 'MyKey'); + + expect(stack).toHaveResource('AWS::KMS::Key', { + Properties: { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', }, - }, - DeletionPolicy: 'Retain', - UpdateReplacePolicy: 'Retain', + ], + Version: '2012-10-17', }, }, - }); + DeletionPolicy: 'Retain', + UpdateReplacePolicy: 'Retain', + }, ResourcePart.CompleteDefinition); }); test('default with no retention', () => { - const app = new App(); - const stack = new Stack(app, 'TestStack'); - - new Key(stack, 'MyKey', { removalPolicy: RemovalPolicy.DESTROY }); + new kms.Key(stack, 'MyKey', { removalPolicy: cdk.RemovalPolicy.DESTROY }); expect(stack).toHaveResource('AWS::KMS::Key', { DeletionPolicy: 'Delete', UpdateReplacePolicy: 'Delete' }, ResourcePart.CompleteDefinition); }); -test('default with some permission', () => { - const app = new App(); - const stack = new Stack(app, 'Test'); - - const key = new Key(stack, 'MyKey'); - const p = new iam.PolicyStatement({ resources: ['*'], actions: ['kms:encrypt'] }); - p.addArnPrincipal('arn'); - key.addToResourcePolicy(p); - - expect(stack).toMatchTemplate({ - Resources: { - MyKey6AB29FA6: { - Type: 'AWS::KMS::Key', - Properties: { - KeyPolicy: { - Statement: [ - { - Action: ACTIONS, - Effect: 'Allow', - Principal: { - AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, - }, - Resource: '*', - }, - { - Action: 'kms:encrypt', - Effect: 'Allow', - Principal: { - AWS: 'arn', - }, - Resource: '*', - }, - ], - Version: '2012-10-17', +describe('key policies', () => { + test('can specify a default key policy', () => { + const policy = new iam.PolicyDocument(); + const statement = new iam.PolicyStatement({ resources: ['*'], actions: ['kms:Put*'] }); + statement.addArnPrincipal('arn:aws:iam::111122223333:root'); + policy.addStatements(statement); + + new kms.Key(stack, 'MyKey', { policy }); + + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:Put*', + Effect: 'Allow', + Principal: { + AWS: 'arn:aws:iam::111122223333:root', + }, + Resource: '*', }, - }, - DeletionPolicy: 'Retain', - UpdateReplacePolicy: 'Retain', + ], + Version: '2012-10-17', }, - }, + }); }); -}); + test('can append to the default key policy', () => { + const statement = new iam.PolicyStatement({ resources: ['*'], actions: ['kms:Put*'] }); + statement.addArnPrincipal('arn:aws:iam::111122223333:root'); + + const key = new kms.Key(stack, 'MyKey'); + key.addToResourcePolicy(statement); + + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', + }, + { + Action: 'kms:Put*', + Effect: 'Allow', + Principal: { + AWS: 'arn:aws:iam::111122223333:root', + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + }); -test('key with some options', () => { - const stack = new Stack(); + test.each([ + ['decrypt', (key: kms.Key, user: iam.IGrantable) => key.grantDecrypt(user), 'kms:Decrypt'], + ['encrypt', (key: kms.Key, user: iam.IGrantable) => key.grantEncrypt(user), ['kms:Encrypt', 'kms:ReEncrypt*', 'kms:GenerateDataKey*']], + ])('grant %s', (_, grantFn, actions) => { + // GIVEN + const key = new kms.Key(stack, 'Key'); + const user = new iam.User(stack, 'User'); + + // WHEN + grantFn(key, user); + + // THEN + // Key policy should be unmodified by the grant. + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] } }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); - const key = new Key(stack, 'MyKey', { - enableKeyRotation: true, - enabled: false, + expect(stack).toHaveResource('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: actions, + Effect: 'Allow', + Resource: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] }, + }, + ], + Version: '2012-10-17', + }, + }); }); - const p = new iam.PolicyStatement({ resources: ['*'], actions: ['kms:encrypt'] }); - p.addArnPrincipal('arn'); - key.addToResourcePolicy(p); - - Tags.of(key).add('tag1', 'value1'); - Tags.of(key).add('tag2', 'value2'); - Tags.of(key).add('tag3', ''); - - expect(stack).toMatchTemplate({ - Resources: { - MyKey6AB29FA6: { - Type: 'AWS::KMS::Key', - Properties: { - KeyPolicy: { - Statement: [ - { - Action: ACTIONS, - Effect: 'Allow', - Principal: { - AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, - }, - Resource: '*', - }, - { - Action: 'kms:encrypt', - Effect: 'Allow', - Principal: { - AWS: 'arn', - }, - Resource: '*', - }, + + test('grant for a principal in a dependent stack works correctly', () => { + const principalStack = new cdk.Stack(app, 'PrincipalStack'); + const principal = new iam.Role(principalStack, 'Role', { + assumedBy: new iam.AnyPrincipal(), + }); + + const keyStack = new cdk.Stack(app, 'KeyStack'); + const key = new kms.Key(keyStack, 'Key'); + + principalStack.addDependency(keyStack); + + key.grantEncrypt(principal); + + expect(principalStack).toHaveResourceLike('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: [ + 'kms:Encrypt', + 'kms:ReEncrypt*', + 'kms:GenerateDataKey*', ], - Version: '2012-10-17', + Effect: 'Allow', + Resource: { + 'Fn::ImportValue': 'KeyStack:ExportsOutputFnGetAttKey961B73FDArn5A860C43', + }, }, - Enabled: false, - EnableKeyRotation: true, - Tags: [ - { - Key: 'tag1', - Value: 'value1', + ], + Version: '2012-10-17', + }, + }); + }); + + test('additional key admins can be specified (with imported/immutable principal)', () => { + const adminRole = iam.Role.fromRoleArn(stack, 'Admin', 'arn:aws:iam::123456789012:role/TrustedAdmin'); + new kms.Key(stack, 'MyKey', { admins: [adminRole] }); + + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, }, - { - Key: 'tag2', - Value: 'value2', + Resource: '*', + }, + { + Action: ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { + AWS: 'arn:aws:iam::123456789012:role/TrustedAdmin', }, - { - Key: 'tag3', - Value: '', + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + }); + + test('additional key admins can be specified (with owned/mutable principal)', () => { + const adminRole = new iam.Role(stack, 'AdminRole', { + assumedBy: new iam.AccountRootPrincipal(), + }); + new kms.Key(stack, 'MyKey', { admins: [adminRole] }); + + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + // Unmodified - default key policy + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, }, - ], - }, - DeletionPolicy: 'Retain', - UpdateReplacePolicy: 'Retain', + Resource: '*', + }, + ], + Version: '2012-10-17', }, - }, + }); + expect(stack).toHaveResource('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: ADMIN_ACTIONS, + Effect: 'Allow', + Resource: { 'Fn::GetAtt': ['MyKey6AB29FA6', 'Arn'] }, + }, + ], + Version: '2012-10-17', + }, + }); }); }); -test('addAlias creates an alias', () => { - const app = new App(); - const stack = new Stack(app, 'Test'); +test('key with some options', () => { + const key = new kms.Key(stack, 'MyKey', { + enableKeyRotation: true, + enabled: false, + }); + + cdk.Tags.of(key).add('tag1', 'value1'); + cdk.Tags.of(key).add('tag2', 'value2'); + cdk.Tags.of(key).add('tag3', ''); + + expect(stack).toHaveResourceLike('AWS::KMS::Key', { + Enabled: false, + EnableKeyRotation: true, + Tags: [ + { + Key: 'tag1', + Value: 'value1', + }, + { + Key: 'tag2', + Value: 'value2', + }, + { + Key: 'tag3', + Value: '', + }, + ], + }); +}); + +test('setting trustAccountIdentities to false will throw (when the defaultKeyPolicies feature flag is enabled)', () => { + expect(() => new kms.Key(stack, 'MyKey', { trustAccountIdentities: false })) + .toThrow('`trustAccountIdentities` cannot be false if the @aws-cdk/aws-kms:defaultKeyPolicies feature flag is set'); +}); - const key = new Key(stack, 'MyKey', { +test('addAlias creates an alias', () => { + const key = new kms.Key(stack, 'MyKey', { enableKeyRotation: true, enabled: false, }); @@ -196,10 +336,7 @@ test('addAlias creates an alias', () => { }); test('can run multiple addAlias', () => { - const app = new App(); - const stack = new Stack(app, 'Test'); - - const key = new Key(stack, 'MyKey', { + const key = new kms.Key(stack, 'MyKey', { enableKeyRotation: true, enabled: false, }); @@ -230,97 +367,10 @@ test('can run multiple addAlias', () => { }); }); -test('grant decrypt on a key', () => { - // GIVEN - const stack = new Stack(); - const key = new Key(stack, 'Key'); - const user = new iam.User(stack, 'User'); - - // WHEN - key.grantDecrypt(user); - - // THEN - expect(stack).toHaveResource('AWS::KMS::Key', { - KeyPolicy: { - Statement: [ - // This one is there by default - { - Action: ACTIONS, - Effect: 'Allow', - Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] } }, - Resource: '*', - }, - // This is the interesting one - { - Action: 'kms:Decrypt', - Effect: 'Allow', - Principal: { AWS: { 'Fn::GetAtt': ['User00B015A1', 'Arn'] } }, - Resource: '*', - }, - ], - Version: '2012-10-17', - }, - }); - - expect(stack).toHaveResource('AWS::IAM::Policy', { - PolicyDocument: { - Statement: [ - { - Action: 'kms:Decrypt', - Effect: 'Allow', - Resource: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] }, - }, - ], - Version: '2012-10-17', - }, - }); - -}); - -test('grant for a principal in a dependent stack works correctly', () => { - const app = new App(); - - const principalStack = new Stack(app, 'PrincipalStack'); - const principal = new iam.Role(principalStack, 'Role', { - assumedBy: new iam.AnyPrincipal(), - }); - - const keyStack = new Stack(app, 'KeyStack'); - const key = new Key(keyStack, 'Key'); - - principalStack.addDependency(keyStack); - - key.grantEncrypt(principal); - - expect(keyStack).toHaveResourceLike('AWS::KMS::Key', { - KeyPolicy: { - Statement: [ - { - // owning account management permissions - we don't care about them in this test - }, - { - Action: [ - 'kms:Encrypt', - 'kms:ReEncrypt*', - 'kms:GenerateDataKey*', - ], - Effect: 'Allow', - Principal: { - AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, - }, - Resource: '*', - }, - ], - }, - }); - -}); - test('keyId resolves to a Ref', () => { - const stack = new Stack(); - const key = new Key(stack, 'MyKey'); + const key = new kms.Key(stack, 'MyKey'); - new CfnOutput(stack, 'Out', { + new cdk.CfnOutput(stack, 'Out', { value: key.keyId, }); @@ -330,29 +380,8 @@ test('keyId resolves to a Ref', () => { }); }); -test('enablePolicyControl changes key policy to allow IAM control', () => { - const stack = new Stack(); - new Key(stack, 'MyKey', { trustAccountIdentities: true }); - expect(stack).toHaveResourceLike('AWS::KMS::Key', { - KeyPolicy: { - Statement: [ - { - Action: 'kms:*', - Effect: 'Allow', - Principal: { - AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, - }, - Resource: '*', - }, - ], - }, - }); -}); - test('fails if key policy has no actions', () => { - const app = new App(); - const stack = new Stack(app, 'my-stack'); - const key = new Key(stack, 'MyKey'); + const key = new kms.Key(stack, 'MyKey'); key.addToResourcePolicy(new iam.PolicyStatement({ resources: ['*'], @@ -363,9 +392,7 @@ test('fails if key policy has no actions', () => { }); test('fails if key policy has no IAM principals', () => { - const app = new App(); - const stack = new Stack(app, 'my-stack'); - const key = new Key(stack, 'MyKey'); + const key = new kms.Key(stack, 'MyKey'); key.addToResourcePolicy(new iam.PolicyStatement({ resources: ['*'], @@ -377,17 +404,15 @@ test('fails if key policy has no IAM principals', () => { describe('imported keys', () => { test('throw an error when providing something that is not a valid key ARN', () => { - const stack = new Stack(); - expect(() => { - Key.fromKeyArn(stack, 'Imported', 'arn:aws:kms:us-east-1:123456789012:key'); + kms.Key.fromKeyArn(stack, 'Imported', 'arn:aws:kms:us-east-1:123456789012:key'); }).toThrow(/KMS key ARN must be in the format 'arn:aws:kms:::key\/', got: 'arn:aws:kms:us-east-1:123456789012:key'/); }); test('can have aliases added to them', () => { - const stack2 = new Stack(); - const myKeyImported = Key.fromKeyArn(stack2, 'MyKeyImported', + const stack2 = new cdk.Stack(app, 'Stack2'); + const myKeyImported = kms.Key.fromKeyArn(stack2, 'MyKeyImported', 'arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012'); // addAlias can be called on imported keys. @@ -407,14 +432,11 @@ describe('imported keys', () => { }, }); }); - }); describe('addToResourcePolicy allowNoOp and there is no policy', () => { test('succeed if set to true (default)', () => { - const stack = new Stack(); - - const key = Key.fromKeyArn(stack, 'Imported', + const key = kms.Key.fromKeyArn(stack, 'Imported', 'arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012'); key.addToResourcePolicy(new iam.PolicyStatement({ resources: ['*'], actions: ['*'] })); @@ -422,9 +444,7 @@ describe('addToResourcePolicy allowNoOp and there is no policy', () => { }); test('fails if set to false', () => { - const stack = new Stack(); - - const key = Key.fromKeyArn(stack, 'Imported', + const key = kms.Key.fromKeyArn(stack, 'Imported', 'arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012'); expect(() => { @@ -433,3 +453,246 @@ describe('addToResourcePolicy allowNoOp and there is no policy', () => { }); }); + +describe('when the defaultKeyPolicies feature flag is disabled', () => { + beforeEach(() => { + app = new cdk.App({ + context: { + '@aws-cdk/aws-kms:defaultKeyPolicies': false, + }, + }); + stack = new cdk.Stack(app); + }); + + test('default key policy', () => { + new kms.Key(stack, 'MyKey'); + + expect(stack).toHaveResource('AWS::KMS::Key', { + Properties: { + KeyPolicy: { + Statement: [ + { + Action: LEGACY_ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }, + DeletionPolicy: 'Retain', + UpdateReplacePolicy: 'Retain', + }, ResourcePart.CompleteDefinition); + }); + + test('policy if specified appends to the default key policy', () => { + const key = new kms.Key(stack, 'MyKey'); + const p = new iam.PolicyStatement({ resources: ['*'], actions: ['kms:Encrypt'] }); + p.addArnPrincipal('arn:aws:iam::111122223333:root'); + key.addToResourcePolicy(p); + + expect(stack).toMatchTemplate({ + Resources: { + MyKey6AB29FA6: { + Type: 'AWS::KMS::Key', + Properties: { + KeyPolicy: { + Statement: [ + { + Action: LEGACY_ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', + }, + { + Action: 'kms:Encrypt', + Effect: 'Allow', + Principal: { + AWS: 'arn:aws:iam::111122223333:root', + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }, + DeletionPolicy: 'Retain', + UpdateReplacePolicy: 'Retain', + }, + }, + }); + }); + + test('trustAccountIdentities changes key policy to allow IAM control', () => { + new kms.Key(stack, 'MyKey', { trustAccountIdentities: true }); + expect(stack).toHaveResourceLike('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: 'kms:*', + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', + }, + ], + }, + }); + }); + + test('additional key admins can be specified (with imported/immutable principal)', () => { + const adminRole = iam.Role.fromRoleArn(stack, 'Admin', 'arn:aws:iam::123456789012:role/TrustedAdmin'); + new kms.Key(stack, 'MyKey', { admins: [adminRole] }); + + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: LEGACY_ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', + }, + { + Action: ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { + AWS: 'arn:aws:iam::123456789012:role/TrustedAdmin', + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + }); + + test('additional key admins can be specified (with owned/mutable principal)', () => { + const adminRole = new iam.Role(stack, 'AdminRole', { + assumedBy: new iam.AccountRootPrincipal(), + }); + new kms.Key(stack, 'MyKey', { admins: [adminRole] }); + + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + { + Action: LEGACY_ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', + }, + { + Action: ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { + AWS: { 'Fn::GetAtt': ['AdminRole38563C57', 'Arn'] }, + }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + expect(stack).toHaveResource('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: ADMIN_ACTIONS, + Effect: 'Allow', + Resource: { 'Fn::GetAtt': ['MyKey6AB29FA6', 'Arn'] }, + }, + ], + Version: '2012-10-17', + }, + }); + }); + + describe('grants', () => { + test('grant decrypt on a key', () => { + // GIVEN + const key = new kms.Key(stack, 'Key'); + const user = new iam.User(stack, 'User'); + + // WHEN + key.grantDecrypt(user); + + // THEN + expect(stack).toHaveResource('AWS::KMS::Key', { + KeyPolicy: { + Statement: [ + // This one is there by default + { + Action: LEGACY_ADMIN_ACTIONS, + Effect: 'Allow', + Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] } }, + Resource: '*', + }, + // This is the interesting one + { + Action: 'kms:Decrypt', + Effect: 'Allow', + Principal: { AWS: { 'Fn::GetAtt': ['User00B015A1', 'Arn'] } }, + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + + expect(stack).toHaveResource('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 'kms:Decrypt', + Effect: 'Allow', + Resource: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] }, + }, + ], + Version: '2012-10-17', + }, + }); + }); + + test('grant for a principal in a dependent stack works correctly', () => { + const principalStack = new cdk.Stack(app, 'PrincipalStack'); + const principal = new iam.Role(principalStack, 'Role', { + assumedBy: new iam.AnyPrincipal(), + }); + + const keyStack = new cdk.Stack(app, 'KeyStack'); + const key = new kms.Key(keyStack, 'Key'); + + principalStack.addDependency(keyStack); + + key.grantEncrypt(principal); + + expect(keyStack).toHaveResourceLike('AWS::KMS::Key', { + KeyPolicy: { + Statement: arrayWith({ + Action: [ + 'kms:Encrypt', + 'kms:ReEncrypt*', + 'kms:GenerateDataKey*', + ], + Effect: 'Allow', + Principal: { + AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':root']] }, + }, + Resource: '*', + }), + }, + }); + }); + }); +}); diff --git a/packages/@aws-cdk/cx-api/lib/features.ts b/packages/@aws-cdk/cx-api/lib/features.ts index 84485ddf3c416..499c8e3438fe8 100644 --- a/packages/@aws-cdk/cx-api/lib/features.ts +++ b/packages/@aws-cdk/cx-api/lib/features.ts @@ -65,6 +65,21 @@ export const DOCKER_IGNORE_SUPPORT = '@aws-cdk/aws-ecr-assets:dockerIgnoreSuppor */ export const SECRETS_MANAGER_PARSE_OWNED_SECRET_NAME = '@aws-cdk/aws-secretsmanager:parseOwnedSecretName'; +/** + * KMS Keys start with a default key policy that grants the account access to administer the key, + * mirroring the behavior of the KMS SDK/CLI/Console experience. Users may override the default key + * policy by specifying their own. + * + * If this flag is not set, the default key policy depends on the setting of the `trustAccountIdentities` + * flag. If false (the default, for backwards-compatibility reasons), the default key policy somewhat + * resemebles the default admin key policy, but with the addition of 'GenerateDataKey' permissions. If + * true, the policy matches what happens when this feature flag is set. + * + * Additionally, if this flag is not set and the user supplies a custom key policy, this will be appended + * to the key's default policy (rather than replacing it). + */ +export const KMS_DEFAULT_KEY_POLICIES = '@aws-cdk/aws-kms:defaultKeyPolicies'; + /** * This map includes context keys and values for feature flags that enable * capabilities "from the future", which we could not introduce as the default @@ -84,6 +99,7 @@ export const FUTURE_FLAGS = { [STACK_RELATIVE_EXPORTS_CONTEXT]: 'true', [DOCKER_IGNORE_SUPPORT]: true, [SECRETS_MANAGER_PARSE_OWNED_SECRET_NAME]: true, + [KMS_DEFAULT_KEY_POLICIES]: true, // We will advertise this flag when the feature is complete // [NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: 'true', @@ -100,6 +116,7 @@ const FUTURE_FLAGS_DEFAULTS: { [key: string]: boolean } = { [NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false, [DOCKER_IGNORE_SUPPORT]: false, [SECRETS_MANAGER_PARSE_OWNED_SECRET_NAME]: false, + [KMS_DEFAULT_KEY_POLICIES]: false, }; export function futureFlagDefault(flag: string): boolean {