From 118dbfb6d757bf0908d9d43b1c31e94e6456488f Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Mon, 7 Sep 2020 15:59:22 +0200
Subject: [PATCH] fix(pipelines): cross-region/cross-account key permissions
 are wrong

In aws/aws-cdk#8280 we made a resource's account/region distinct
from the stack in which the construct was defined, to account for
accounts and regions from imported resources.

The pipelines module used to define imported roles in a separate
in-memory Stack so that the old, broken "cross-environment" logic
would do the right thing. That crutch was removed as part of #8280.

The new logic hasn't been carried through everywhere though. For
example, the logic in the grants of KMS keys had not been updated
to match, leading to cross-account/cross-region deployments
being broken (as reported in #10166) because the cross-region support
stack's KMS key had the wrong permissions.

In fact, it switched from:

```
{
    "Action": [ "kms:Decrypt", "kms:DescribeKey" ],
    "Principal": {
        "AWS": {
            "Fn::Sub": "arn:${AWS::Partition}:iam::561462023695:role/cdk-hnb659fds-deploy-role-561462023695-us-east-2"
        }
    },
    "Resource": "*"
}
```

to

```
{
    "Action": [ "kms:Decrypt", "kms:DescribeKey" ],
    "Principal": {
        "AWS": {
            "Fn::Join": [ "", [
                "arn:", { "Ref": "AWS::Partition" }, ":iam::355421412380:root"
            ] ]
        }
    },
    "Resource": "*"
}
```

Ignoring the switch from `Fn::Sub` to `Fn::Join`, it switched from the
`deploy-role` in a DIFFERENT account to the root principal of the SAME
account.
---
 packages/@aws-cdk/aws-kms/lib/key.ts           |  9 ++-------
 packages/@aws-cdk/core/lib/private/refs.ts     |  2 +-
 .../test/cross-environment-infra.test.ts       | 18 ++++++++++++++++++
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts
index 94c65b5c1ee59..37ff1ff66c85e 100644
--- a/packages/@aws-cdk/aws-kms/lib/key.ts
+++ b/packages/@aws-cdk/aws-kms/lib/key.ts
@@ -1,5 +1,5 @@
 import * as iam from '@aws-cdk/aws-iam';
-import { Construct, IResource, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
+import { Construct, IResource, RemovalPolicy, Resource, Stack, Token, TokenComparison } from '@aws-cdk/core';
 import { Alias } from './alias';
 import { CfnKey } from './kms.generated';
 
@@ -230,12 +230,7 @@ abstract class KeyBase extends Resource implements IKey {
   }
 
   private isGranteeFromAnotherAccount(grantee: iam.IGrantable): boolean {
-    if (!(Construct.isConstruct(grantee))) {
-      return false;
-    }
-    const bucketStack = Stack.of(this);
-    const identityStack = Stack.of(grantee);
-    return bucketStack.account !== identityStack.account;
+    return [TokenComparison.DIFFERENT, TokenComparison.ONE_UNRESOLVED].includes(Token.compareStrings(this.env.account, grantee.grantPrincipal.principalAccount ?? ''));
   }
 }
 
diff --git a/packages/@aws-cdk/core/lib/private/refs.ts b/packages/@aws-cdk/core/lib/private/refs.ts
index 0fdc5e1bed40f..c231b072b4ead 100644
--- a/packages/@aws-cdk/core/lib/private/refs.ts
+++ b/packages/@aws-cdk/core/lib/private/refs.ts
@@ -54,7 +54,7 @@ function resolveValue(consumer: Stack, reference: CfnReference): IResolvable {
   // unsupported: stacks are not in the same environment
   if (producer.environment !== consumer.environment) {
     throw new Error(
-      `Stack "${consumer.node.path}" cannot consume a cross reference from stack "${producer.node.path}". ` +
+      `Stack "${consumer.node.path}" (environment '${consumer.environment}') cannot consume a cross reference from stack "${producer.node.path} (environment '${producer.environment}')". ` +
       'Cross stack references are only supported for stacks deployed to the same environment or between nested stacks and their parent stack');
   }
 
diff --git a/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts b/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts
index 15ee878022574..dac6ebb595f0b 100644
--- a/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts
+++ b/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts
@@ -45,6 +45,24 @@ test('in a cross-account/cross-region setup, artifact bucket can be read by depl
       })),
     },
   });
+
+  // And the key to go along with it
+  expect(supportStack).toHaveResourceLike('AWS::KMS::Key', {
+    KeyPolicy: {
+      Statement: arrayWith(objectLike({
+        Action: arrayWith('kms:Decrypt', 'kms:DescribeKey'),
+        Principal: {
+          AWS: {
+            'Fn::Join': ['', [
+              'arn:',
+              { Ref: 'AWS::Partition' },
+              stringLike('*-deploy-role-*'),
+            ]],
+          },
+        },
+      })),
+    },
+  });
 });
 
 test('in a cross-account/same-region setup, artifact bucket can be read by deploy role', () => {