Skip to content

Commit

Permalink
Use wildcard in KMS key policy instead of referencing Distribution to…
Browse files Browse the repository at this point in the history 
… avoid circular dependency (#31227)

Making the KMS key policy to use wildcard instead on the distribution id
part instead of referencing the Distribution resource to get the exact
distribution ID. This breaks the circular dependency in the template.
  • Loading branch information
@gracelu0
gracelu0 authored Aug 28, 2024
2 parents f33dffb + dd11f45 commit c8eaa3e
Show file tree
Hide file tree
Showing 3 changed files with 232 additions and 20 deletions.
27 changes: 9 additions & 18 deletions packages/aws-cdk-lib/aws-cloudfront-origins/lib/s3-bucket-origin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { AccessLevel } from '../../aws-cloudfront';
import * as iam from '../../aws-iam';
import { IKey } from '../../aws-kms';
import { IBucket } from '../../aws-s3';
import { Annotations, Aws, DefaultTokenResolver, Names, Stack, StringConcat, Token, Tokenization } from '../../core';
import { Annotations, Aws, Names, Stack } from '../../core';

const BUCKET_ACTIONS: Record<string, string[]> = {
READ: ['s3:GetObject'],
Expand Down Expand Up @@ -84,21 +84,8 @@ export abstract class S3BucketOrigin extends cloudfront.OriginBase {
}

if (bucket.encryptionKey) {
let bucketName = bucket.bucketName;
if (Token.isUnresolved(bucket.bucketName)) {
bucketName = JSON.stringify(Tokenization.resolve(bucket.bucketName, {
scope,
resolver: new DefaultTokenResolver(new StringConcat()),
}));
}
Annotations.of(scope).addInfo(
`Granting OAC permissions to access KMS key for S3 bucket origin ${bucketName} may cause a circular dependency error when this stack deploys.\n` +
'The key policy references the distribution\'s id, the distribution references the bucket, and the bucket references the key.\n'+
'See the "Using OAC for a SSE-KMS encrypted S3 origin" section in the module README for more details.\n',
);

const keyPolicyActions = this.getKeyPolicyActions(props?.originAccessLevels ?? [cloudfront.AccessLevel.READ]);
const keyPolicyResult = this.grantDistributionAccessToKey(distributionId!, keyPolicyActions, bucket.encryptionKey);
const keyPolicyResult = this.grantDistributionAccessToKey(keyPolicyActions, bucket.encryptionKey);
// Failed to update key policy, assume using imported key
if (!keyPolicyResult.statementAdded) {
Annotations.of(scope).addWarningV2('@aws-cdk/aws-cloudfront-origins:updateImportedKeyPolicy',
Expand Down Expand Up @@ -154,20 +141,24 @@ export abstract class S3BucketOrigin extends cloudfront.OriginBase {
return result;
}

grantDistributionAccessToKey(distributionId: string, actions: string[], key: IKey): iam.AddToResourcePolicyResult {
grantDistributionAccessToKey(actions: string[], key: IKey): iam.AddToResourcePolicyResult {
const oacKeyPolicyStatement = new iam.PolicyStatement(
{
effect: iam.Effect.ALLOW,
principals: [new iam.ServicePrincipal('cloudfront.amazonaws.com')],
actions,
resources: ['*'],
conditions: {
StringEquals: {
'AWS:SourceArn': `arn:${Aws.PARTITION}:cloudfront::${Aws.ACCOUNT_ID}:distribution/${distributionId}`,
ArnLike: {
'AWS:SourceArn': `arn:${Aws.PARTITION}:cloudfront::${Aws.ACCOUNT_ID}:distribution/*`,
},
},
},
);
Annotations.of(key.node.scope!).addWarningV2('@aws-cdk/aws-cloudfront-origins:wildcardKeyPolicyForOac',
'To avoid circular dependency between the KMS key, Bucket, and Distribution,' +
'a wildcard is used to match all Distribution IDs in Key policy condition.\n' +
'To further scope down the policy for best security practices, see the "Using OAC for a SSE-KMS encrypted S3 origin" section in the module README.');
const result = key.addToResourcePolicy(oacKeyPolicyStatement);
return result;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk-lib/aws-cloudfront-origins/lib/s3-static-website-origin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ export interface S3StaticWebsiteOriginProps extends HttpOriginProps {
export class S3StaticWebsiteOrigin extends HttpOrigin {
constructor(props: S3StaticWebsiteOriginProps) {
super(props.bucket.bucketWebsiteDomainName, {
protocolPolicy: cloudfront.OriginProtocolPolicy.HTTP_ONLY, // S3 only supports HTTP for website buckets
// S3 only supports HTTP for website buckets. See https://docs.aws.amazon.com/AmazonS3/latest/userguide/WebsiteEndpoints.html
protocolPolicy: cloudfront.OriginProtocolPolicy.HTTP_ONLY,
...props,
});
}
Expand Down
222 changes: 221 additions & 1 deletion packages/aws-cdk-lib/aws-cloudfront-origins/test/s3-bucket-origin.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Annotations, Template } from '../../assertions';
import * as cloudfront from '../../aws-cloudfront/index';
import * as origins from '../../aws-cloudfront-origins';
import * as kms from '../../aws-kms';
import * as s3 from '../../aws-s3/index';
import { App, Duration, Stack } from '../../core';
import { App, Duration, Fn, Stack } from '../../core';

describe('S3BucketOrigin', () => {
describe('withOriginAccessControl', () => {
Expand Down Expand Up @@ -165,6 +166,225 @@ describe('S3BucketOrigin', () => {
});
});

describe('when using bucket with KMS Customer Managed key', () => {
it('should match expected template resource and warn user about wildcard key policy', () => {
const stack = new Stack();
const kmsKey = new kms.Key(stack, 'myKey');
const bucket = new s3.Bucket(stack, 'myEncryptedBucket', {
encryption: s3.BucketEncryption.KMS,
encryptionKey: kmsKey,
objectOwnership: s3.ObjectOwnership.BUCKET_OWNER_ENFORCED,
});
new cloudfront.Distribution(stack, 'MyDistributionA', {
defaultBehavior: { origin: origins.S3BucketOrigin.withOriginAccessControl(bucket) },
});
const template = Template.fromStack(stack);

expect(template.toJSON().Resources).toEqual({
myKey441A1E73: {
Type: 'AWS::KMS::Key',
Properties: {
KeyPolicy: {
Statement: [
{
Action: 'kms:*',
Effect: 'Allow',
Principal: {
AWS: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':iam::',
{
Ref: 'AWS::AccountId',
},
':root',
],
],
},
},
Resource: '*',
},
{
Action: 'kms:Decrypt',
Condition: {
ArnLike: {
'AWS:SourceArn': {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':cloudfront::',
{
Ref: 'AWS::AccountId',
},
':distribution/*',
],
],
},
},
},
Effect: 'Allow',
Principal: {
Service: 'cloudfront.amazonaws.com',
},
Resource: '*',
},
],
Version: '2012-10-17',
},
},
UpdateReplacePolicy: 'Retain',
DeletionPolicy: 'Retain',
},
myEncryptedBucket939A51C0: {
Type: 'AWS::S3::Bucket',
Properties: {
BucketEncryption: {
ServerSideEncryptionConfiguration: [
{
ServerSideEncryptionByDefault: {
KMSMasterKeyID: {
'Fn::GetAtt': [
'myKey441A1E73',
'Arn',
],
},
SSEAlgorithm: 'aws:kms',
},
},
],
},
OwnershipControls: {
Rules: [
{
ObjectOwnership: 'BucketOwnerEnforced',
},
],
},
},
UpdateReplacePolicy: 'Retain',
DeletionPolicy: 'Retain',
},
myEncryptedBucketPolicyF516140B: {
Type: 'AWS::S3::BucketPolicy',
Properties: {
Bucket: {
Ref: 'myEncryptedBucket939A51C0',
},
PolicyDocument: {
Statement: [
{
Action: 's3:GetObject',
Condition: {
StringEquals: {
'AWS:SourceArn': {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':cloudfront::',
{
Ref: 'AWS::AccountId',
},
':distribution/',
{
Ref: 'MyDistributionA2150CE0F',
},
],
],
},
},
},
Effect: 'Allow',
Principal: {
Service: 'cloudfront.amazonaws.com',
},
Resource: {
'Fn::Join': [
'',
[
{
'Fn::GetAtt': [
'myEncryptedBucket939A51C0',
'Arn',
],
},
'/*',
],
],
},
},
],
Version: '2012-10-17',
},
},
},
MyDistributionAOrigin1S3OriginAccessControlE2649D73: {
Type: 'AWS::CloudFront::OriginAccessControl',
Properties: {
OriginAccessControlConfig: {
Name: 'MyDistributionAOrigin1S3OriginAccessControl2859DD54',
OriginAccessControlOriginType: 's3',
SigningBehavior: 'always',
SigningProtocol: 'sigv4',
},
},
},
MyDistributionA2150CE0F: {
Type: 'AWS::CloudFront::Distribution',
Properties: {
DistributionConfig: {
DefaultCacheBehavior: {
CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6',
Compress: true,
TargetOriginId: 'MyDistributionAOrigin11BE8FF8C',
ViewerProtocolPolicy: 'allow-all',
},
Enabled: true,
HttpVersion: 'http2',
IPV6Enabled: true,
Origins: [
{
DomainName: {
'Fn::GetAtt': [
'myEncryptedBucket939A51C0',
'RegionalDomainName',
],
},
Id: 'MyDistributionAOrigin11BE8FF8C',
OriginAccessControlId: {
'Fn::GetAtt': [
'MyDistributionAOrigin1S3OriginAccessControlE2649D73',
'Id',
],
},
S3OriginConfig: {
OriginAccessIdentity: '',
},
},
],
},
},
},
});
Annotations.fromStack(stack).hasWarning('/Default',
'To avoid circular dependency between the KMS key, Bucket, and Distribution,' +
'a wildcard is used to match all Distribution IDs in Key policy condition.\n' +
'To further scope down the policy for best security practices, see the "Using OAC for a SSE-KMS encrypted S3 origin" section in the module README. [ack: @aws-cdk/aws-cloudfront-origins:wildcardKeyPolicyForOac]');
});
});

describe('when attaching to a multiple distribution', () => {
let stack: Stack;
let bucket: s3.Bucket;
Expand Down

0 comments on commit c8eaa3e

Please sign in to comment.