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

feat(kms): trustAccountIdentities avoids cyclic stack dependencies #5575

Merged
merged 23 commits into from
Feb 20, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b8f3066
feat(kms): add property to enable IAM policy key administration
moofish32 Dec 29, 2019
b438544
Merge branch 'master' into feature/kms-iam-control
moofish32 Jan 20, 2020
0232fc1
fix: update key policies to include admin functions, rename to trustA…
moofish32 Jan 20, 2020
0a145c3
fix(s3): updating bucket tests for new default kms key policy
moofish32 Jan 20, 2020
da6c461
Merge branch 'master' into feature/kms-iam-control
moofish32 Jan 20, 2020
540b266
Merge branch 'master' into feature/kms-iam-control
moofish32 Feb 2, 2020
8350874
fix: revert kms key policy changes
moofish32 Feb 2, 2020
6e8dd2f
refactor: update comment
moofish32 Feb 2, 2020
177eafc
Update key.ts
rix0rrr Feb 4, 2020
93d6666
Merge branch 'master' into feature/kms-iam-control
moofish32 Feb 14, 2020
2835826
test: fix from permission changes
moofish32 Feb 14, 2020
50073b3
chore: update kms readme
moofish32 Feb 14, 2020
d74427b
Merge branch 'feature/kms-iam-control' of https://github.com/moofish3…
moofish32 Feb 14, 2020
bd199db
Merge branch 'master' into feature/kms-iam-control
moofish32 Feb 14, 2020
2fac128
Merge branch 'master' into feature/kms-iam-control
mergify[bot] Feb 17, 2020
7474c68
Merge branch 'master' into feature/kms-iam-control
moofish32 Feb 18, 2020
55a28be
Merge branch 'feature/kms-iam-control' of https://github.com/moofish3…
moofish32 Feb 18, 2020
86a38b1
Merge branch 'master' into feature/kms-iam-control
mergify[bot] Feb 19, 2020
965c402
Merge branch 'master' into feature/kms-iam-control
mergify[bot] Feb 19, 2020
74d379e
Merge branch 'master' into feature/kms-iam-control
rix0rrr Feb 19, 2020
99aad54
Merge branch 'master' into feature/kms-iam-control
rix0rrr Feb 20, 2020
e26d6dd
Merge branch 'master' into feature/kms-iam-control
mergify[bot] Feb 20, 2020
ad75758
Merge branch 'master' into feature/kms-iam-control
mergify[bot] Feb 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions packages/@aws-cdk/aws-kms/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ 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:

Expand All @@ -37,6 +39,8 @@ 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)`:

Expand All @@ -50,3 +54,72 @@ myKeyImported.addAlias('alias/foo');
Note that a call to `.addToPolicy(statement)` on `myKeyImported` will not have
an affect on the key's policy because it is not owned by your stack. The call
will be a no-op.

### 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:

```ts
new Key(stack, 'MyKey', { trustAccountIdentities: true });
```

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.

stack-1 (bucket and key created)

```ts
// ... snip
const myKmsKey = new kms.Key(this, 'MyKey', { trustAccountIdentities: true });

const bucket = new Bucket(this, 'MyEncryptedBucket', {
bucketName: 'myEncryptedBucket',
encryption: BucketEncryption.KMS,
encryptionKey: myKmsKey
});
```

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:

```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.


67 changes: 54 additions & 13 deletions packages/@aws-cdk/aws-kms/lib/key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ abstract class KeyBase extends Resource implements IKey {
*/
protected abstract readonly policy?: iam.PolicyDocument;

/**
* Optional property to control trusting account identities.
*
* If specified grants will default identity policies instead of to both
* resource and identity policies.
*/
protected abstract readonly trustAccountIdentities: boolean;

/**
* Collection of aliases added to the key
*
Expand Down Expand Up @@ -130,19 +138,25 @@ abstract class KeyBase extends Resource implements IKey {
const crossAccountAccess = this.isGranteeFromAnotherAccount(grantee);
const crossRegionAccess = this.isGranteeFromAnotherRegion(grantee);
const crossEnvironment = crossAccountAccess || crossRegionAccess;
return iam.Grant.addToPrincipalAndResource({
const grantOptions: iam.GrantWithResourceOptions = {
grantee,
actions,
resource: this,
resourcePolicyPrincipal: principal,

// if the key is used in a cross-environment matter,
// we can't access the Key ARN (they don't have physical names),
// so fall back to using '*'. ToDo we need to make this better... somehow
resourceArns: crossEnvironment ? ['*'] : [this.keyArn],

resourceArns: [this.keyArn],
resourceSelfArns: crossEnvironment ? undefined : ['*'],
});
};
if (this.trustAccountIdentities) {
return iam.Grant.addToPrincipalOrResource(grantOptions);
} else {
return iam.Grant.addToPrincipalAndResource({
...grantOptions,
// if the key is used in a cross-environment matter,
// we can't access the Key ARN (they don't have physical names),
// so fall back to using '*'. ToDo we need to make this better... somehow
resourceArns: crossEnvironment ? ['*'] : [this.keyArn],
resourcePolicyPrincipal: principal,
});
}
}

/**
Expand Down Expand Up @@ -268,6 +282,18 @@ export interface KeyProps {
* @default RemovalPolicy.Retain
*/
readonly removalPolicy?: RemovalPolicy;

/**
* Whether the key usage can be granted by IAM policies
*
* 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).
*
* @default false
* @see https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default-allow-root-enable-iam
*/
readonly trustAccountIdentities?: boolean;
}

/**
Expand All @@ -288,6 +314,10 @@ export class Key extends KeyBase {
public readonly keyArn = keyArn;
public readonly keyId: string;
protected readonly policy?: iam.PolicyDocument | undefined = undefined;
// defaulting true: if we are importing the key the key policy is
// undefined and impossible to change here; this means updating identity
// policies is really the only option
protected readonly trustAccountIdentities: boolean = true;

constructor(keyId: string) {
super(scope, id);
Expand All @@ -307,14 +337,16 @@ export class Key extends KeyBase {
public readonly keyArn: string;
public readonly keyId: string;
protected readonly policy?: iam.PolicyDocument;
protected readonly trustAccountIdentities: boolean;

constructor(scope: Construct, id: string, props: KeyProps = {}) {
super(scope, id);

if (props.policy) {
this.policy = props.policy;
this.policy = props.policy || new iam.PolicyDocument();
this.trustAccountIdentities = props.trustAccountIdentities || false;
if (this.trustAccountIdentities) {
this.allowAccountIdentitiesToControl();
} else {
this.policy = new iam.PolicyDocument();
this.allowAccountToAdmin();
}

Expand All @@ -334,8 +366,17 @@ export class Key extends KeyBase {
}
}

private allowAccountIdentitiesToControl() {
this.addToResourcePolicy(new iam.PolicyStatement({
resources: ['*'],
actions: ['kms:*'],
principals: [new iam.AccountRootPrincipal()]
}));

}
/**
* Let users from this account admin this key.
* Let users or IAM policies from this account admin this key.
* @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/
*/
private allowAccountToAdmin() {
Expand Down
Loading