Skip to content

Commit 2b917ec

Browse files
authored
fix(kms): cross-environment usage fails when trustAccountIdentities is set (#12925)
When the `trustAccountIdentities` flag is set (either directly, or set by default via the `@aws-cdk/aws-kms:defaultKeyPolicies` feature flag), any grant statements are always added to the principal OR resource, defaulting to the principal if possible. In cross-environment usage, this means that the principal IAM policy is updated, but the key policy is never updated. However, cross-environment usage always requires the key policy to be updated. Attempting to use the key in a cross-environment usage with `trustAccountIdentities` enabled initially presents itself as a cross-environment reference issue, but even if the stack was successfully deployed, the delegated account would not have access to use/admin the key. This change updates the logic to make use of the existing cross-environment logic whenever cross-environment usage is detected, regardless of the value of `trustAccountIdentities`. This has the impact of fixing the cross-env references and ensuring the key policy is properly updated. fixes #12921, fixes #12741 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 9028269 commit 2b917ec

File tree

2 files changed

+98
-1
lines changed

2 files changed

+98
-1
lines changed

packages/@aws-cdk/aws-kms/lib/key.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ abstract class KeyBase extends Resource implements IKey {
156156
resourceArns: [this.keyArn],
157157
resourceSelfArns: crossEnvironment ? undefined : ['*'],
158158
};
159-
if (this.trustAccountIdentities) {
159+
if (this.trustAccountIdentities && !crossEnvironment) {
160160
return iam.Grant.addToPrincipalOrResource(grantOptions);
161161
} else {
162162
return iam.Grant.addToPrincipalAndResource({

packages/@aws-cdk/aws-kms/test/key.test.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,103 @@ describe('key policies', () => {
245245
});
246246
});
247247

248+
testFutureBehavior('grant for a principal in a different region', flags, cdk.App, (app) => {
249+
const principalStack = new cdk.Stack(app, 'PrincipalStack', { env: { region: 'testregion1' } });
250+
const principal = new iam.Role(principalStack, 'Role', {
251+
assumedBy: new iam.AnyPrincipal(),
252+
roleName: 'MyRolePhysicalName',
253+
});
254+
255+
const keyStack = new cdk.Stack(app, 'KeyStack', { env: { region: 'testregion2' } });
256+
const key = new kms.Key(keyStack, 'Key');
257+
258+
key.grantEncrypt(principal);
259+
260+
expect(keyStack).toHaveResourceLike('AWS::KMS::Key', {
261+
KeyPolicy: {
262+
Statement: arrayWith(
263+
{
264+
Action: [
265+
'kms:Encrypt',
266+
'kms:ReEncrypt*',
267+
'kms:GenerateDataKey*',
268+
],
269+
Effect: 'Allow',
270+
Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::', { Ref: 'AWS::AccountId' }, ':role/MyRolePhysicalName']] } },
271+
Resource: '*',
272+
},
273+
),
274+
Version: '2012-10-17',
275+
},
276+
});
277+
expect(principalStack).toHaveResourceLike('AWS::IAM::Policy', {
278+
PolicyDocument: {
279+
Statement: [
280+
{
281+
Action: [
282+
'kms:Encrypt',
283+
'kms:ReEncrypt*',
284+
'kms:GenerateDataKey*',
285+
],
286+
Effect: 'Allow',
287+
Resource: '*',
288+
},
289+
],
290+
Version: '2012-10-17',
291+
},
292+
});
293+
});
294+
295+
testFutureBehavior('grant for a principal in a different account', flags, cdk.App, (app) => {
296+
const principalStack = new cdk.Stack(app, 'PrincipalStack', { env: { account: '0123456789012' } });
297+
const principal = new iam.Role(principalStack, 'Role', {
298+
assumedBy: new iam.AnyPrincipal(),
299+
roleName: 'MyRolePhysicalName',
300+
});
301+
302+
const keyStack = new cdk.Stack(app, 'KeyStack', { env: { account: '111111111111' } });
303+
const key = new kms.Key(keyStack, 'Key');
304+
305+
key.grantEncrypt(principal);
306+
307+
expect(keyStack).toHaveResourceLike('AWS::KMS::Key', {
308+
KeyPolicy: {
309+
Statement: [
310+
{
311+
// Default policy, unmodified
312+
},
313+
{
314+
Action: [
315+
'kms:Encrypt',
316+
'kms:ReEncrypt*',
317+
'kms:GenerateDataKey*',
318+
],
319+
Effect: 'Allow',
320+
Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::0123456789012:role/MyRolePhysicalName']] } },
321+
Resource: '*',
322+
},
323+
],
324+
Version: '2012-10-17',
325+
},
326+
});
327+
expect(principalStack).toHaveResourceLike('AWS::IAM::Policy', {
328+
PolicyDocument: {
329+
Statement: [
330+
{
331+
Action: [
332+
'kms:Encrypt',
333+
'kms:ReEncrypt*',
334+
'kms:GenerateDataKey*',
335+
],
336+
Effect: 'Allow',
337+
Resource: '*',
338+
},
339+
],
340+
Version: '2012-10-17',
341+
},
342+
});
343+
});
344+
248345
testFutureBehavior('additional key admins can be specified (with imported/immutable principal)', flags, cdk.App, (app) => {
249346
const stack = new cdk.Stack(app);
250347
const adminRole = iam.Role.fromRoleArn(stack, 'Admin', 'arn:aws:iam::123456789012:role/TrustedAdmin');

0 commit comments

Comments
 (0)