fix(core): Access Denied using legacy synthesizer with new bootstrap (#…

…9831)

We always intended the FileAsset KMS Key to be transparently
usable by any IAM identity allowed to read from and write to the
FileAsset Bucket. We incorrectly implemented this, however.

We used to use the following key policy:

```
     - Action: [...]
       Principal: { AWS: "123456789012" }
       Condition:
         StringEquals:
           kms:ViaService: Fn::Sub: s3.${AWS::Region}.amazonaws.com
```

And this was intended to mean "any identity from the given account". That is *not* how KMS interprets it, though.

`Principal: { AWS: "123456789012" }` is equivalent to `Principal: { AWS: "arn:aws:iam::123456789012:root" }`,
and `arn:aws:iam::123456789012:root` is a principal which is treated
in a special way by KMS, and it means "use the caller's IAM Identity Policy instead".

So while I was under the impression that it was strictly necessary for
KMS usage permissions to exist both on the key and on the identity, this
is only true if you use the `arn:aws:iam::123456789012:root` principal.

The correct way to express the condition we had intended to express was
instead to use a condition called `kms:CallerAccount` in combination
with the principal `*`:

```
     - Action: [...]
       Principal: { AWS: "*" }
       Condition:
         StringEquals:
           kms:CallerAccount: "123456789012"
           kms:ViaService: Fn::Sub: s3.${AWS::Region}.amazonaws.com
```

This PR changes the key policy in the bootstrap resources to use the
policy that we always had intended. This now gets rid of the requirement
for IAM identities to list `kms:Decrypt` in their role policy, and so
gets rid of the requirement for them to know the KMS key ARN.

This makes the stack synthesized by the legacy stack synthesizer work
with the new bootstrap stack, and also removes the need for the new
synthesizer to import the KMS key ARN using `Fn::ImportValue`.

---

However, the new stack synthesizer *does* now require that you have
the newest bootstrap stack installed, and since templates are likely
deployed using a pipeline, the CLI is not involved to do the
`MINIMUM_BOOTSTRAP_STACK` version check.

Originally I had intended to use the version `Export` to add version
checking to the template, but that doesn't actually work for 2 reasons:

- `Fn::ImportValue` can only occur in a limited set of positions in
  the CloudFormation template.
- If an `Export` is used by a Stack, it cannot be changed anymore. That
  means that even if we had done the check using `Fn::ImportValue`,
  users wouldn't have been allowed to update the bootstrap stack
  anymore.

What we should have done from the start, and what this PR
introduces, is storing the bootstrap stack version in an SSM Parameter
Store Parameter. This value can be inspected in a CloudFormation
**Rules** section, which will produce a readable error message
about why the template cannot be deployed.

Any assertion failure reasons will be reported on a
`ROLLBACK_IN_PROGRESS` event, so classify those appropriately
in the stack monitor so the error message gets displayed.

Fixes #9607.

BREAKING CHANGE: (cdk-pipelines) users of CDK Pipelines (and other users
of the new stack synthesizer) will need to update their bootstrap stack
by running `cdk bootstrap` with the new CLI. Until they do, deployments
will fail with the error: `Unable to fetch parameters
[/aws-cdk-bootstrap/hnb659fds/version]`


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/@aws-cdk/aws-s3-assets/test/asset.test.ts

@@ -1,4 +1,4 @@
-import { arrayWith, ResourcePart, SynthUtils } from '@aws-cdk/assert';
+import { ResourcePart, SynthUtils } from '@aws-cdk/assert';
 import '@aws-cdk/assert/jest';
 import * as iam from '@aws-cdk/aws-iam';
 import * as cxschema from '@aws-cdk/cloud-assembly-schema';
@@ -124,31 +124,6 @@ test('"readers" or "grantRead" can be used to grant read permissions on the asse
   });
 });
 
-test('"grantRead" also gives KMS permissions when using the new bootstrap stack', () => {
-  const stack = new cdk.Stack(undefined, undefined, {
-    synthesizer: new cdk.DefaultStackSynthesizer(),
-  });
-  const group = new iam.Group(stack, 'MyGroup');
-
-  const asset = new Asset(stack, 'MyAsset', {
-    path: path.join(__dirname, 'sample-asset-directory'),
-    readers: [group],
-  });
-
-  asset.grantRead(group);
-
-  expect(stack).toHaveResource('AWS::IAM::Policy', {
-    PolicyDocument: {
-      Version: '2012-10-17',
-      Statement: arrayWith({
-        Action: ['kms:Decrypt', 'kms:DescribeKey'],
-        Effect: 'Allow',
-        Resource: { 'Fn::ImportValue': 'CdkBootstrap-hnb659fds-FileAssetKeyArn' },
-      }),
-    },
-  });
-});
-
 test('fails if directory not found', () => {
   const stack = new cdk.Stack();
   expect(() => new Asset(stack, 'MyDirectory', {

packages/@aws-cdk/aws-s3-deployment/test/bucket-deployment.test.ts

@@ -1,4 +1,3 @@
-import { arrayWith } from '@aws-cdk/assert';
 import '@aws-cdk/assert/jest';
 import * as cloudfront from '@aws-cdk/aws-cloudfront';
 import * as iam from '@aws-cdk/aws-iam';
@@ -621,30 +620,4 @@ test('deploy without deleting missing files from destination', () => {
   expect(stack).toHaveResourceLike('Custom::CDKBucketDeployment', {
     Prune: false,
   });
-});
-
-test('Deployment role gets KMS permissions when using assets from new style synthesizer', () => {
-  const stack = new cdk.Stack(undefined, undefined, {
-    synthesizer: new cdk.DefaultStackSynthesizer(),
-  });
-  const bucket = new s3.Bucket(stack, 'Dest');
-
-  // WHEN
-  new s3deploy.BucketDeployment(stack, 'Deploy', {
-    sources: [s3deploy.Source.asset(path.join(__dirname, 'my-website'))],
-    destinationBucket: bucket,
-  });
-
-  // THEN
-  expect(stack).toHaveResource('AWS::IAM::Policy', {
-    PolicyDocument: {
-      Version: '2012-10-17',
-      Statement: arrayWith({
-        Action: ['kms:Decrypt', 'kms:DescribeKey'],
-        Effect: 'Allow',
-        Resource: { 'Fn::ImportValue': 'CdkBootstrap-hnb659fds-FileAssetKeyArn' },
-      }),
-    },
-  });
-
-});
+});

packages/@aws-cdk/core/lib/assets.ts

@@ -223,6 +223,8 @@ export interface FileAssetLocation {
    * can be used as an example for how to configure the key properly.
    *
    * @default - Asset bucket is not encrypted
+   * @deprecated Since bootstrap bucket v4, the key policy properly allows use of the
+   * key via the bucket and no additional parameters have to be granted anymore.
    */
   readonly kmsKeyArn?: string;
 }

packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts

@@ -4,6 +4,8 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema';
 import * as cxapi from '@aws-cdk/cx-api';
 import { DockerImageAssetLocation, DockerImageAssetSource, FileAssetLocation, FileAssetPackaging, FileAssetSource } from '../assets';
 import { Fn } from '../cfn-fn';
+import { CfnParameter } from '../cfn-parameter';
+import { CfnRule } from '../cfn-rule';
 import { ISynthesisSession } from '../construct-compat';
 import { Stack } from '../stack';
 import { Token } from '../token';
@@ -17,7 +19,7 @@ export const BOOTSTRAP_QUALIFIER_CONTEXT = '@aws-cdk/core:bootstrapQualifier';
 /**
  * The minimum bootstrap stack version required by this app.
  */
-const MIN_BOOTSTRAP_STACK_VERSION = 3;
+const MIN_BOOTSTRAP_STACK_VERSION = 4;
 
 /**
  * Configuration properties for DefaultStackSynthesizer
@@ -233,6 +235,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
     this.imageAssetPublishingRoleArn = specialize(this.props.imageAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN);
     this._kmsKeyArnExportName = specialize(this.props.fileAssetKeyArnExportName ?? DefaultStackSynthesizer.DEFAULT_FILE_ASSET_KEY_ARN_EXPORT_NAME);
     /* eslint-enable max-len */
+
+    addBootstrapVersionRule(stack, MIN_BOOTSTRAP_STACK_VERSION, qualifier);
   }
 
   public addFileAsset(asset: FileAssetSource): FileAssetLocation {
@@ -270,7 +274,6 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
       httpUrl,
       s3ObjectUrl,
       s3Url: httpUrl,
-      kmsKeyArn: Fn.importValue(cfnify(this._kmsKeyArnExportName)),
     };
   }
 
@@ -462,3 +465,38 @@ function stackLocationOrInstrinsics(stack: Stack) {
     urlSuffix: resolvedOr(stack.urlSuffix, '${AWS::URLSuffix}'),
   };
 }
+
+/**
+ * Add a CfnRule to the Stack which checks the current version of the bootstrap stack this template is targeting
+ *
+ * The CLI normally checks this, but in a pipeline the CLI is not involved
+ * so we encode this rule into the template in a way that CloudFormation will check it.
+ */
+function addBootstrapVersionRule(stack: Stack, requiredVersion: number, qualifier: string) {
+  const param = new CfnParameter(stack, 'BootstrapVersion', {
+    type: 'AWS::SSM::Parameter::Value<String>',
+    description: 'Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store.',
+    default: `/aws-cdk-bootstrap/${qualifier}/version`,
+  });
+
+  // There is no >= check in CloudFormation, so we have to check the number
+  // is NOT in [1, 2, 3, ... <required> - 1]
+  const oldVersions = range(1, requiredVersion).map(n => `${n}`);
+
+  new CfnRule(stack, 'CheckBootstrapVersion', {
+    assertions: [
+      {
+        assert: Fn.conditionNot(Fn.conditionContains(oldVersions, param.valueAsString)),
+        assertDescription: `CDK bootstrap stack version ${requiredVersion} required. Please run 'cdk bootstrap' with a recent version of the CDK CLI.`,
+      },
+    ],
+  });
+}
+
+function range(startIncl: number, endExcl: number) {
+  const ret = new Array<number>();
+  for (let i = startIncl; i < endExcl; i++) {
+    ret.push(i);
+  }
+  return ret;
+}

packages/@aws-cdk/core/test/stack-synthesis/test.new-style-synthesis.ts

@@ -35,7 +35,10 @@ export = {
 
     // THEN -- the S3 url is advertised on the stack artifact
     const stackArtifact = asm.getStackArtifact('Stack');
-    test.equals(stackArtifact.stackTemplateAssetObjectUrl, 's3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/4bdae6e3b1b15f08c889d6c9133f24731ee14827a9a9ab9b6b6a9b42b6d34910');
+
+    const templateHash = '19e1e8612660f79362e091714ab7b3583961936d762c75be8b8083c3af40850a';
+
+    test.equals(stackArtifact.stackTemplateAssetObjectUrl, `s3://cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}/${templateHash}`);
 
     // THEN - the template is in the asset manifest
     const manifestArtifact = asm.artifacts.filter(isAssetManifest)[0];
@@ -49,7 +52,7 @@ export = {
       destinations: {
         'current_account-current_region': {
           bucketName: 'cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}',
-          objectKey: '4bdae6e3b1b15f08c889d6c9133f24731ee14827a9a9ab9b6b6a9b42b6d34910',
+          objectKey: templateHash,
           assumeRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}',
         },
       },
@@ -58,6 +61,28 @@ export = {
     test.done();
   },
 
+  'version check is added to template'(test: Test) {
+    // GIVEN
+    new CfnResource(stack, 'Resource', {
+      type: 'Some::Resource',
+    });
+
+    // THEN
+    const template = app.synth().getStackByName('Stack').template;
+    test.deepEqual(template?.Parameters?.BootstrapVersion?.Type, 'AWS::SSM::Parameter::Value<String>');
+    test.deepEqual(template?.Parameters?.BootstrapVersion?.Default, '/aws-cdk-bootstrap/hnb659fds/version');
+
+    const assertions = template?.Rules?.CheckBootstrapVersion?.Assertions ?? [];
+    test.deepEqual(assertions.length, 1);
+    test.deepEqual(assertions[0].Assert, {
+      'Fn::Not': [
+        { 'Fn::Contains': [['1', '2', '3'], { Ref: 'BootstrapVersion' }] },
+      ],
+    });
+
+    test.done();
+  },
+
   'add file asset'(test: Test) {
     // WHEN
     const location = stack.synthesizer.addFileAsset({

packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json

@@ -1,4 +1,36 @@
 {
+  "Parameters": {
+    "BootstrapVersion": {
+      "Type": "AWS::SSM::Parameter::Value<String>",
+      "Default": "/aws-cdk-bootstrap/hnb659fds/version",
+      "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store."
+    }
+  },
+  "Rules": {
+    "CheckBootstrapVersion": {
+      "Assertions": [
+        {
+          "Assert": {
+            "Fn::Not": [
+              {
+                "Fn::Contains": [
+                  [
+                    "1",
+                    "2",
+                    "3"
+                  ],
+                  {
+                    "Ref": "BootstrapVersion"
+                  }
+                ]
+              }
+            ]
+          },
+          "AssertDescription": "CDK bootstrap stack version 4 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
+        }
+      ]
+    }
+  },
   "Resources": {
     "PipelineUpdatePipelineSelfMutationRole57E559E8": {
       "Type": "AWS::IAM::Role",

packages/@aws-cdk/pipelines/test/integ.pipeline.expected.json

@@ -1,4 +1,36 @@
 {
+  "Parameters": {
+    "BootstrapVersion": {
+      "Type": "AWS::SSM::Parameter::Value<String>",
+      "Default": "/aws-cdk-bootstrap/hnb659fds/version",
+      "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store."
+    }
+  },
+  "Rules": {
+    "CheckBootstrapVersion": {
+      "Assertions": [
+        {
+          "Assert": {
+            "Fn::Not": [
+              {
+                "Fn::Contains": [
+                  [
+                    "1",
+                    "2",
+                    "3"
+                  ],
+                  {
+                    "Ref": "BootstrapVersion"
+                  }
+                ]
+              }
+            ]
+          },
+          "AssertDescription": "CDK bootstrap stack version 4 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
+        }
+      ]
+    }
+  },
   "Resources": {
     "PipelineUpdatePipelineSelfMutationRole57E559E8": {
       "Type": "AWS::IAM::Role",

packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts

@@ -8,6 +8,8 @@ export const REPOSITORY_NAME_OUTPUT = 'RepositoryName';
 export const BUCKET_DOMAIN_NAME_OUTPUT = 'BucketDomainName';
 /** @experimental */
 export const BOOTSTRAP_VERSION_OUTPUT = 'BootstrapVersion';
+/** @experimental */
+export const BOOTSTRAP_VERSION_RESOURCE = 'CdkBootstrapVersion';
 
 /**
  * Options for the bootstrapEnvironment operation(s)

packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml

@@ -65,7 +65,7 @@ Conditions:
   UsePublicAccessBlockConfiguration:
     Fn::Equals:
       - 'true'
-      - Ref: PublicAccessBlockConfiguration  
+      - Ref: PublicAccessBlockConfiguration
 Resources:
   FileAssetsBucketEncryptionKey:
     Type: AWS::KMS::Key
@@ -99,11 +99,14 @@ Resources:
               - kms:GenerateDataKey*
             Effect: Allow
             Principal:
-              AWS:
-                Ref: AWS::AccountId
+              # Not actually everyone -- see below for Conditions
+              AWS: "*"
             Resource: "*"
             Condition:
               StringEquals:
+                # See https://docs.aws.amazon.com/kms/latest/developerguide/policy-conditions.html#conditions-kms-caller-account
+                kms:CallerAccount:
+                  Ref: AWS::AccountId
                 kms:ViaService:
                   - Fn::Sub: s3.${AWS::Region}.amazonaws.com
           - Action:
@@ -143,7 +146,7 @@ Resources:
             BlockPublicPolicy: true
             IgnorePublicAcls: true
             RestrictPublicBuckets: true
-          - Ref: AWS::NoValue       
+          - Ref: AWS::NoValue
     UpdateReplacePolicy: Retain
   StagingBucketPolicy:
     Type: 'AWS::S3::BucketPolicy'
@@ -350,6 +353,15 @@ Resources:
           - Ref: AWS::NoValue
       RoleName:
         Fn::Sub: cdk-${Qualifier}-cfn-exec-role-${AWS::AccountId}-${AWS::Region}
+  # The SSM parameter is used in pipeline-deployed templates to verify the version
+  # of the bootstrap resources.
+  CdkBootstrapVersion:
+    Type: AWS::SSM::Parameter
+    Properties:
+      Type: String
+      Name:
+        Fn::Sub: '/aws-cdk-bootstrap/${Qualifier}/version'
+      Value: 4
 Outputs:
   BucketName:
     Description: The name of the S3 bucket owned by the CDK toolkit stack
@@ -359,8 +371,11 @@ Outputs:
     Description: The domain name of the S3 bucket owned by the CDK toolkit stack
     Value:
       Fn::Sub: "${StagingBucket.RegionalDomainName}"
+  # @deprecated - This Export can be removed at some future point in time.
+  # We can't do it today because if there are stacks that use it, the bootstrap
+  # stack cannot be updated. Not used anymore by apps >= 1.60.0
   FileAssetKeyArn:
-    Description: The ARN of the KMS key used to encrypt the asset bucket
+    Description: The ARN of the KMS key used to encrypt the asset bucket (deprecated)
     Value:
       Fn::If:
         - CreateNewKey
@@ -373,10 +388,9 @@ Outputs:
     Description: The name of the ECR repository which hosts docker image assets
     Value:
       Fn::Sub: "${ContainerAssetsRepository}"
+  # The Output is used by the CLI to verify the version of the bootstrap resources.
   BootstrapVersion:
     Description: The version of the bootstrap resources that are currently mastered
       in this stack
-    Value: '3'
-    Export:
-      Name:
-        Fn::Sub: CdkBootstrap-${Qualifier}-Version
+    Value:
+      Fn::GetAtt: [CdkBootstrapVersion, Value]

packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts

@@ -59,5 +59,16 @@ export async function deployBootstrapStack(
 }
 
 function bootstrapVersionFromTemplate(template: any): number {
-  return parseInt(template.Outputs?.[BOOTSTRAP_VERSION_OUTPUT]?.Value ?? '0', 10);
+  const versionSources = [
+    template.Outputs?.[BOOTSTRAP_VERSION_OUTPUT]?.Value,
+    template.Resources?.[BOOTSTRAP_VERSION_OUTPUT]?.Properties?.Value,
+  ];
+
+  for (const vs of versionSources) {
+    if (typeof vs === 'number') { return vs; }
+    if (typeof vs === 'string' && !isNaN(parseInt(vs, 10))) {
+      return parseInt(vs, 10);
+    }
+  }
+  return 0;
 }