feat(kms): change default key policy to align with KMS best practices (under feature flag)

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,71 +68,97 @@ 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:*",
   "Resource": "*"
 }
 ```
 
-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.

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*',

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',
+];