Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(cloudfront-origins): grant key permissions to distribution #31188

Merged
merged 2 commits into from
Aug 22, 2024
Merged

chore(cloudfront-origins): grant key permissions to distribution #31188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b776ea
address feedback
gracelu0 Aug 22, 2024
202c4b4
fix unit tests
gracelu0 Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 65 additions & 14 deletions packages/aws-cdk-lib/aws-cloudfront-origins/lib/s3-bucket-origin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@ import { Construct } from 'constructs';
import * as cloudfront from '../../aws-cloudfront';
import { AccessLevel } from '../../aws-cloudfront';
import * as iam from '../../aws-iam';
import { IKey } from '../../aws-kms';
import { IBucket } from '../../aws-s3';
import { Annotations, Aws, Names, Stack } from '../../core';
import { Annotations, Aws, DefaultTokenResolver, Names, Stack, StringConcat, Token, Tokenization } from '../../core';

const BUCKET_ACTIONS = {
const BUCKET_ACTIONS: Record<string, string[]> = {
READ: ['s3:GetObject'],
WRITE: ['s3:PutObject'],
DELETE: ['s3:DeleteObject'],
};

const KEY_ACTIONS: Record<string, string[]> = {
READ: ['kms:Decrypt'],
WRITE: ['kms:Encrypt', 'kms:GenerateDataKey*'],
};

/**
* Properties for configuring a origin using a standard S3 bucket
*/
Expand Down Expand Up @@ -67,19 +73,37 @@ export abstract class S3BucketOrigin extends cloudfront.OriginBase {
}

const distributionId = options.distributionId;
const actions = this.getActions(props?.originAccessLevels ?? [cloudfront.AccessLevel.READ]);
const result = this.grantDistributionAccessToBucket(distributionId, actions);
const bucketPolicyActions = this.getBucketPolicyActions(props?.originAccessLevels ?? [cloudfront.AccessLevel.READ]);
const bucketPolicyResult = this.grantDistributionAccessToBucket(distributionId!, bucketPolicyActions);

// Failed to update bucket policy, assume using imported bucket
if (!result.statementAdded) {
Annotations.of(scope).addWarningV2('@aws-cdk/aws-cloudfront-origins:updateBucketPolicy',
'Cannot update bucket policy of an imported bucket. Set overrideImportedBucketPolicy to true or update the policy manually instead.');
if (!bucketPolicyResult.statementAdded) {
Annotations.of(scope).addWarningV2('@aws-cdk/aws-cloudfront-origins:updateImportedBucketPolicy',
'Cannot update bucket policy of an imported bucket. You may need to update the policy manually instead.\n' +
'See the "Using pre-existing S3 buckets" section of module\'s README for more info.');
}

if (bucket.encryptionKey) {
// TODO: update this warning
Annotations.of(scope).addWarningV2('@aws-cdk/aws-cloudfront-origins:updateKeyPolicy',
'If you run into a circular dependency issue during deployment, please refer to README for instructions.');
let bucketName = bucket.bucketName;
if (Token.isUnresolved(bucket.bucketName)) {
bucketName = JSON.stringify(Tokenization.resolve(bucket.bucketName, {
scope,
resolver: new DefaultTokenResolver(new StringConcat()),
}));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have links to examples of this trick when the code is doing JSON.stringify(Tokenization.resolve ... ? Trying to understand what it does. Does it resolve to the logical ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example of usage in iam module:

aws-cdk/packages/aws-cdk-lib/aws-iam/lib/private/util.ts

Lines 24 to 31 in af50620

export function generatePolicyName(scope: IConstruct, logicalId: string): string {
// as logicalId is itself a Token, resolve it first
const resolvedLogicalId = Tokenization.resolve(logicalId, {
scope,
resolver: new DefaultTokenResolver(new StringConcat()),
});
return lastNCharacters(resolvedLogicalId, MAX_POLICY_NAME_LEN);
}

Output from when I tested it locally is like this:

[Info at /MigrationOacStack/MyDistribution/Origin2] Granting OAC permissions to access KMS key for S3 bucket origin {"Ref":"MyBucketF68F3FF0"} may cause a circular dependency error when this stack deploys.
The key policy references the distribution's id, the distribution references the bucket, and the bucket references the key.
See the 'Using OAC for a SSE-KMS encrypted S3 origin' section in the module README for more details.

So yes it resolves to the logical ID. I thought it would be helpful to include the specific resource involved in the info message, but there might be a cleaner way to do this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thank you. Looks good to me.

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);
// Failed to update key policy, assume using imported key
if (!keyPolicyResult.statementAdded) {
Annotations.of(scope).addWarningV2('@aws-cdk/aws-cloudfront-origins:updateImportedKeyPolicy',
'Cannot update key policy of an imported key. You may need to update the policy manually instead.');
}
}

const originBindConfig = this._bind(scope, options);
Expand All @@ -94,18 +118,27 @@ export abstract class S3BucketOrigin extends cloudfront.OriginBase {
};
}

getActions(accessLevels: cloudfront.AccessLevel[]) {
getBucketPolicyActions(accessLevels: cloudfront.AccessLevel[]) {
let actions: string[] = [];
for (const accessLevel of new Set(accessLevels)) {
actions = actions.concat(BUCKET_ACTIONS[accessLevel]);
}
return actions;
}

getKeyPolicyActions(accessLevels: cloudfront.AccessLevel[]) {
let actions: string[] = [];
// Remove duplicates and filters out DELETE since delete permissions are not applicable to KMS key actions
const keyAccessLevels = [...new Set(accessLevels.filter(level => level !== AccessLevel.DELETE))];
for (const accessLevel of new Set(keyAccessLevels)) {
actions = actions.concat(KEY_ACTIONS[accessLevel]);
}
return actions;
}

grantDistributionAccessToBucket(distributionId: string, actions: string[]): iam.AddToResourcePolicyResult {
const oacReadOnlyBucketPolicyStatement = new iam.PolicyStatement(
const oacBucketPolicyStatement = new iam.PolicyStatement(
{
sid: 'GrantCloudFrontOACAccessToS3Origin',
effect: iam.Effect.ALLOW,
principals: [new iam.ServicePrincipal('cloudfront.amazonaws.com')],
actions,
Expand All @@ -117,7 +150,25 @@ export abstract class S3BucketOrigin extends cloudfront.OriginBase {
},
},
);
const result = bucket.addToResourcePolicy(oacReadOnlyBucketPolicyStatement);
const result = bucket.addToResourcePolicy(oacBucketPolicyStatement);
return result;
}

grantDistributionAccessToKey(distributionId: string, 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}`,
},
},
},
);
const result = key.addToResourcePolicy(oacKeyPolicyStatement);
return result;
}
}();
Expand Down
6 changes: 2 additions & 4 deletions packages/aws-cdk-lib/aws-cloudfront-origins/test/s3-bucket-origin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ describe('S3BucketOrigin', () => {
],
],
},
Sid: 'GrantCloudFrontOACAccessToS3Origin',
},
],
Version: '2012-10-17',
Expand Down Expand Up @@ -245,7 +244,6 @@ describe('S3BucketOrigin', () => {
],
],
},
Sid: 'GrantCloudFrontOACAccessToS3Origin',
},
{
Action: 's3:GetObject',
Expand Down Expand Up @@ -290,7 +288,6 @@ describe('S3BucketOrigin', () => {
],
],
},
Sid: 'GrantCloudFrontOACAccessToS3Origin',
},
],
Version: '2012-10-17',
Expand Down Expand Up @@ -404,7 +401,8 @@ describe('S3BucketOrigin', () => {

it('should warn user bucket policy is not updated', () => {
Annotations.fromStack(stack).hasWarning('/Default/MyDistributionA/Origin1',
'Cannot update bucket policy of an imported bucket. Set overrideImportedBucketPolicy to true or update the policy manually instead. [ack: @aws-cdk/aws-cloudfront-origins:updateBucketPolicy]');
'Cannot update bucket policy of an imported bucket. You may need to update the policy manually instead.\n' +
'See the "Using pre-existing S3 buckets" section of module\'s README for more info. [ack: @aws-cdk/aws-cloudfront-origins:updateImportedBucketPolicy]');
});

it('should match expected template resources', () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/aws-cdk-lib/aws-cloudfront/lib/origin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ export interface OriginBindOptions {

/**
* The identifier of the Distribution this Origin is used for.
* This is used to grant origin access permissions to the distribution for origin access control.
* @default - no distribution id
*/
readonly distributionId: string;
readonly distributionId?: string;
}

/**
Expand Down
Loading