Skip to content

Commit 0b429fb

Browse files
kxue-godaddykxue-godaddy
authored
fix(s3-deployment): BucketDeployment fails when bootstrap stack's StagingBucket is encrypted with customer managed KMS key (#29540)
### Issue #25100 Closes #25100. ### Reason for this change When the CDK bootstrap stack's `StagingBucket` is encrypted with a customer managed KMS key whose key policy does not include wildcard KMS permissions similar to those of the S3 managed KMS key, `BucketDeployment` fails because the Lambda's execution role doesn't get the `kms:Decrypt` permission necessary to download from the bucket. In addition, if one of the sources for `BucketDeployment` comes from the `Source.bucket` static method, and the bucket is an "out-of-app imported reference" created from `s3.Bucket.fromBucketName`, the bucket's `encryptionKey` attribute is `null` and the current code won't add the `kms:Decrypt` permission on the bucket's encryption key to the Lambda's execution role. If this bucket is additionally encrypted with a customer managed KMS key without sufficient resource-based policy, the deployment fails as well. ### Description of changes It's not easy to make the code "just work" in every situation, because there's no way to pinpoint the source bucket's encryption key ARN without using another custom resource, which is a heavy-lifting and it's hard to give this new Lambda a reasonable and minimal set of execution role permissions. Therefore, this PR resolves the issue by changing `BucketDeployment.handlerRole` from `private readonly` to `public readonly`, and adding documentations on how to handle errors resulting from "not authorized to perform: kms:Decrypt". The current code allows customizing `handlerRole` by passing in `BucketDeploymentProps.role`. This change makes the customization easier because users don't need to manually add the S3 permissions. The only code change is on the visibility of `BucketDeployment.handlerRole`. All other changes are documentations. I proposed 4 possible changes in my comment to Issue #25100, and only the first one (changing visibility) is pursued in this PR. The second one was abandoned because the CFN export `CdkBootstrap-hnb659fds-FileAssetKeyArn` is deprecated. ### Description of how you validated changes I wrote a CDK app which uses the `BucketDeployment` construct. After manually adding relevant KMS permissions to the Lambda execution role, I verified that the bucket deployment worked in the following two scenarios: - My personal account; bootstrap stack's `StagingBucket` encrypted with a custom KMS key which only has the default key policy. - GoDaddy corporate account; `StagingBucket` encrypted with a KMS key from an AWS Organization management account. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent a7b4e29 commit 0b429fb

File tree

4 files changed

+89
-7
lines changed

4 files changed

+89
-7
lines changed

‎packages/aws-cdk-lib/aws-s3-deployment/README.md

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ This is what happens under the hood:
2424

2525
1. When this stack is deployed (either via `cdk deploy` or via CI/CD), the
2626
contents of the local `website-dist` directory will be archived and uploaded
27-
to an intermediary assets bucket. If there is more than one source, they will
28-
be individually uploaded.
29-
2. The `BucketDeployment` construct synthesizes a custom CloudFormation resource
27+
to an intermediary assets bucket (the `StagingBucket` of the CDK bootstrap stack).
28+
If there is more than one source, they will be individually uploaded.
29+
2. The `BucketDeployment` construct synthesizes a Lambda-backed custom CloudFormation resource
3030
of type `Custom::CDKBucketDeployment` into the template. The source bucket/key
3131
is set to point to the assets bucket.
32-
3. The custom resource downloads the .zip archive, extracts it and issues `aws
33-
s3 sync --delete` against the destination bucket (in this case
32+
3. The custom resource invokes its associated Lambda function, which downloads the .zip archive,
33+
extracts it and issues `aws s3 sync --delete` against the destination bucket (in this case
3434
`websiteBucket`). If there is more than one source, the sources will be
3535
downloaded and merged pre-deployment at this step.
3636

@@ -67,6 +67,58 @@ const deployment = new s3deploy.BucketDeployment(this, 'DeployWebsite', {
6767
deployment.addSource(s3deploy.Source.asset('./another-asset'));
6868
```
6969

70+
For the Lambda function to download object(s) from the source bucket, besides the obvious
71+
`s3:GetObject*` permissions, the Lambda's execution role needs the `kms:Decrypt` and `kms:DescribeKey`
72+
permissions on the KMS key that is used to encrypt the bucket. By default, when the source bucket is
73+
encrypted with the S3 managed key of the account, these permissions are granted by the key's
74+
resource-based policy, so they do not need to be on the Lambda's execution role policy explicitly.
75+
However, if the encryption key is not the s3 managed one, its resource-based policy is quite likely
76+
to NOT grant such KMS permissions. In this situation, the Lambda execution will fail with an error
77+
message like below:
78+
79+
```txt
80+
download failed: ...
81+
An error occurred (AccessDenied) when calling the GetObject operation:
82+
User: *** is not authorized to perform: kms:Decrypt on the resource associated with this ciphertext
83+
because no identity-based policy allows the kms:Decrypt action
84+
```
85+
86+
When this happens, users can use the public `handlerRole` property of `BucketDeployment` to manually
87+
add the KMS permissions:
88+
89+
```ts
90+
declare const destinationBucket: s3.Bucket;
91+
92+
const deployment = new s3deploy.BucketDeployment(this, 'DeployFiles', {
93+
sources: [s3deploy.Source.asset(path.join(__dirname, 'source-files'))],
94+
destinationBucket,
95+
});
96+
97+
deployment.handlerRole.addToPolicy(
98+
new iam.PolicyStatement({
99+
actions: ['kms:Decrypt', 'kms:DescribeKey'],
100+
effect: iam.Effect.ALLOW,
101+
resources: ['<encryption key ARN>'],
102+
}),
103+
);
104+
```
105+
106+
The situation above could arise from the following scenarios:
107+
108+
- User created a customer managed KMS key and passed its ID to the `cdk bootstrap` command via
109+
the `--bootstrap-kms-key-id` CLI option.
110+
The [default key policy](https://docs.aws.amazon.com/kms/latest/developerguide/key-policy-default.html#key-policy-default-allow-root-enable-iam)
111+
alone is not sufficient to grant the Lambda KMS permissions.
112+
113+
- A corporation uses its own custom CDK bootstrap process, which encrypts the CDK `StagingBucket`
114+
by a KMS key from a management account of the corporation's AWS Organization. In this cross-account
115+
access scenario, the KMS permissions must be explicitly present in the Lambda's execution role policy.
116+
117+
- One of the sources for the `BucketDeployment` comes from the `Source.bucket` static method, which
118+
points to a bucket whose encryption key is not the S3 managed one, and the resource-based policy
119+
of the encryption key is not sufficient to grant the Lambda `kms:Decrypt` and `kms:DescribeKey`
120+
permissions.
121+
70122
## Supported sources
71123

72124
The following source types are supported for bucket deployments:
@@ -370,7 +422,6 @@ The syntax for template variables is `{{ variableName }}` in your local file. Th
370422
specify the substitutions in CDK like this:
371423

372424
```ts
373-
import * as iam from 'aws-cdk-lib/aws-iam';
374425
import * as lambda from 'aws-cdk-lib/aws-lambda';
375426

376427
declare const myLambdaFunction: lambda.Function;

‎packages/aws-cdk-lib/aws-s3-deployment/lib/bucket-deployment.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,11 @@ export class BucketDeployment extends Construct {
277277
private requestDestinationArn: boolean = false;
278278
private readonly destinationBucket: s3.IBucket;
279279
private readonly sources: SourceConfig[];
280-
private readonly handlerRole: iam.IRole;
280+
281+
/**
282+
* Execution role of the Lambda function behind the custom CloudFormation resource of type `Custom::CDKBucketDeployment`.
283+
*/
284+
public readonly handlerRole: iam.IRole;
281285

282286
constructor(scope: Construct, id: string, props: BucketDeploymentProps) {
283287
super(scope, id);

‎packages/aws-cdk-lib/aws-s3-deployment/lib/source.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,31 @@ export class Source {
6868
*
6969
* Make sure you trust the producer of the archive.
7070
*
71+
* If the `bucket` parameter is an "out-of-app" reference "imported" via static methods such
72+
* as `s3.Bucket.fromBucketName`, be cautious about the bucket's encryption key. In general,
73+
* CDK does not query for additional properties of imported constructs at synthesis time.
74+
* For example, for a bucket created from `s3.Bucket.fromBucketName`, CDK does not know
75+
* its `IBucket.encryptionKey` property, and therefore will NOT give KMS permissions to the
76+
* Lambda execution role of the `BucketDeployment` construct. If you want the
77+
* `kms:Decrypt` and `kms:DescribeKey` permissions on the bucket's encryption key
78+
* to be added automatically, reference the imported bucket via `s3.Bucket.fromBucketAttributes`
79+
* and pass in the `encryptionKey` attribute explicitly.
80+
*
81+
* @example
82+
* declare const destinationBucket: s3.Bucket;
83+
* const sourceBucket = s3.Bucket.fromBucketAttributes(this, 'SourceBucket', {
84+
* bucketArn: 'arn:aws:s3:::my-source-bucket-name',
85+
* encryptionKey: kms.Key.fromKeyArn(
86+
* this,
87+
* 'SourceBucketEncryptionKey',
88+
* 'arn:aws:kms:us-east-1:123456789012:key/<key-id>'
89+
* ),
90+
* });
91+
* const deployment = new s3deploy.BucketDeployment(this, 'DeployFiles', {
92+
* sources: [s3deploy.Source.bucket(sourceBucket, 'source.zip')],
93+
* destinationBucket,
94+
* });
95+
*
7196
* @param bucket The S3 Bucket
7297
* @param zipObjectKey The S3 object key of the zip file with contents
7398
*/

‎packages/aws-cdk-lib/rosetta/aws_s3_deployment/default.ts-fixture

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { Construct } from 'constructs';
44
import * as s3deploy from 'aws-cdk-lib/aws-s3-deployment';
55
import * as s3 from 'aws-cdk-lib/aws-s3';
66
import * as ec2 from'aws-cdk-lib/aws-ec2';
7+
import * as iam from 'aws-cdk-lib/aws-iam';
8+
import * as kms from 'aws-cdk-lib/aws-kms';
79
import * as path from 'path';
810

911
class Fixture extends Stack {

0 commit comments

Comments
 (0)