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

fix(kms): kms key grant methods misidentify region when enclosing stack is different region #29315

Merged
merged 6 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions packages/aws-cdk-lib/aws-kms/lib/key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ abstract class KeyBase extends Resource implements IKey {
}
const bucketStack = Stack.of(this);
const identityStack = Stack.of(grantee.grantPrincipal);

if (FeatureFlags.of(this).isEnabled(cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE)) {
// if two compared stacks have the same region, this should return 'false' since it's from the
// same region; if two stacks have different region, then compare env.region
return bucketStack.region !== identityStack.region && this.env.region !== identityStack.region;
}
return bucketStack.region !== identityStack.region;
}

Expand All @@ -271,6 +277,12 @@ abstract class KeyBase extends Resource implements IKey {
}
const bucketStack = Stack.of(this);
const identityStack = Stack.of(grantee.grantPrincipal);

if (FeatureFlags.of(this).isEnabled(cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE)) {
// if two compared stacks have the same region, this should return 'false' since it's from the
// same region; if two stacks have different region, then compare env.account
return bucketStack.account !== identityStack.account && this.env.account !== identityStack.account;
}
return bucketStack.account !== identityStack.account;
}
}
Expand Down
61 changes: 61 additions & 0 deletions packages/aws-cdk-lib/aws-kms/test/key.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describeDeprecated } from '@aws-cdk/cdk-build-tools';
import { Match, Template } from '../../assertions';
import * as iam from '../../aws-iam';
import * as cdk from '../../core';
import * as cxapi from '../../cx-api';
import * as kms from '../lib';
import { KeySpec, KeyUsage } from '../lib';

Expand Down Expand Up @@ -81,6 +82,66 @@ describe('key policies', () => {
});
});

test('cross region key with iam role grant', () => {
const app = new cdk.App({ context: { [cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE]: true } });
const stack = new cdk.Stack(app, 'test-stack', { env: { account: '000000000000', region: 'us-west-2' } });
const key = kms.Key.fromKeyArn(
stack,
'Key',
'arn:aws:kms:eu-north-1:000000000000:key/e3ab59e5-3dc3-4bc4-9c3f-c790231d2287',
);

const roleStack = new cdk.Stack(app, 'RoleStack', {
env: { account: '000000000000', region: 'eu-north-1' },
});
const role = new iam.Role(roleStack, 'Role', {
assumedBy: new iam.AccountPrincipal('000000000000'),
});
key.grantEncryptDecrypt(role);

Template.fromStack(roleStack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Effect: 'Allow',
Resource: 'arn:aws:kms:eu-north-1:000000000000:key/e3ab59e5-3dc3-4bc4-9c3f-c790231d2287',
},
],
Version: '2012-10-17',
},
});
});

test('cross region key with iam role grant when feature flag is disabled', () => {
const app = new cdk.App({ context: { [cxapi.KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE]: false } });
const stack = new cdk.Stack(app, 'test-stack', { env: { account: '000000000000', region: 'us-west-2' } });
const key = kms.Key.fromKeyArn(
stack,
'Key',
'arn:aws:kms:eu-north-1:000000000000:key/e3ab59e5-3dc3-4bc4-9c3f-c790231d2287',
);

const roleStack = new cdk.Stack(app, 'RoleStack', {
env: { account: '000000000000', region: 'eu-north-1' },
});
const role = new iam.Role(roleStack, 'Role', {
assumedBy: new iam.AccountPrincipal('000000000000'),
});
key.grantEncryptDecrypt(role);

Template.fromStack(roleStack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Effect: 'Allow',
Resource: '*',
},
],
Version: '2012-10-17',
},
});
});

test('can append to the default key policy', () => {
const stack = new cdk.Stack();
const statement = new iam.PolicyStatement({ resources: ['*'], actions: ['kms:Put*'] });
Expand Down
18 changes: 17 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Flags come in three types:
| [@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction](#aws-cdkaws-cloudwatch-actionschangelambdapermissionlogicalidforlambdaaction) | When enabled, the logical ID of a Lambda permission for a Lambda action includes an alarm ID. | 2.124.0 | (fix) |
| [@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse](#aws-cdkaws-codepipelinecrossaccountkeysdefaultvaluetofalse) | Enables Pipeline to set the default value for crossAccountKeys to false. | 2.127.0 | (default) |
| [@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2](#aws-cdkaws-codepipelinedefaultpipelinetypetov2) | Enables Pipeline to set the default pipeline type to V2. | V2NEXT | (default) |
| [@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope](#aws-cdkaws-kmsreducecrossaccountregionpolicyscope) | When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only. | V2NEXT | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -122,7 +123,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource": true,
"@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction": true,
"@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse": true,
"@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true
"@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true,
"@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope": true
}
}
```
Expand Down Expand Up @@ -1249,4 +1251,18 @@ construct, the construct automatically defaults the value of this property to `P
**Compatibility with old behavior:** Pass `pipelineType: PipelineType.V1` to `Pipeline` construct to restore the previous behavior.


### @aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope

*When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only.* (fix)

When this feature flag is enabled and calling KMS key grant method, the created IAM policy will reduce the resource scope from
'*' to this specific granting KMS key.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |


<!-- END details -->
17 changes: 17 additions & 0 deletions packages/aws-cdk-lib/cx-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,20 @@ _cdk.json_
}
}
```

* `@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope`

Reduce resource scope of the IAM Policy created from KMS key grant to granting key only.

When this feature flag is enabled and calling KMS key grant method, the created IAM policy will reduce the resource scope from
'*' to this specific granting KMS key.

_cdk.json_

```json
{
"context": {
"@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope": true
}
}
```
13 changes: 13 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export const CODECOMMIT_SOURCE_ACTION_DEFAULT_BRANCH_NAME = '@aws-cdk/aws-codepi
export const LAMBDA_PERMISSION_LOGICAL_ID_FOR_LAMBDA_ACTION = '@aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction';
export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse';
export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1021,6 +1022,18 @@ export const FLAGS: Record<string, FlagInfo> = {
recommendedValue: true,
compatibilityWithOldBehaviorMd: 'Pass `pipelineType: PipelineType.V1` to `Pipeline` construct to restore the previous behavior.',
},

//////////////////////////////////////////////////////////////////////
[KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE]: {
type: FlagType.BugFix,
summary: 'When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only.',
detailsMd: `
When this feature flag is enabled and calling KMS key grant method, the created IAM policy will reduce the resource scope from
'*' to this specific granting KMS key.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
},
};

const CURRENT_MV = 'v2';
Expand Down
Loading