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.

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

feat(cloudfront): s3 origin access control L2 construct #30663

Closed
wants to merge 14 commits into from
Prev Previous commit
Next Next commit
Update feature flag to OAC default only
gracelu0 committed Jun 25, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit f09ee03d4a4689f70bf06bb312e7a577348cbf06
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ interface updateBucketPolicyProps {
distributionId: string;
partition: string;
accountId: string;
actions: string[];
}

export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) {
@@ -21,8 +22,9 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
const accountId = props.AccountId;
const partition = props.Partition;
const bucketName = props.BucketName;
const actions = props.Actions;

await updateBucketPolicy({ bucketName, distributionId, partition, accountId });
await updateBucketPolicy({ bucketName, distributionId, partition, accountId, actions });

return {
IsComplete: true,
@@ -39,13 +41,14 @@ async function updateBucketPolicy(props: updateBucketPolicyProps) {
const prevPolicyJson = (await s3.getBucketPolicy({ Bucket: props.bucketName }))?.Policy ?? S3_POLICY_STUB;
const policy = JSON.parse(prevPolicyJson);
console.log('Previous bucket policy:', JSON.stringify(policy, undefined, 2));

const oacBucketPolicyStatement = {
Sid: 'AllowS3OACAccess',
Sid: 'GrantOACAccessToS3',
Principal: {
Service: ['cloudfront.amazonaws.com'],
},
Effect: 'Allow',
Action: ['s3:GetObject'],
Action: props.actions,
Resource: [`arn:${props.partition}:s3:::${props.bucketName}/*`],
Condition: {
StringEquals: {
Original file line number Diff line number Diff line change
@@ -4,6 +4,11 @@ import { KMS, KeyManagerType } from '@aws-sdk/client-kms';

const kms = new KMS({});

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

export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) {

if (event.RequestType === 'Create' || event.RequestType === 'Update') {
@@ -13,6 +18,7 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
const accountId = props.AccountId;
const partition = props.Partition;
const region = process.env.AWS_REGION;
const accessLevels = props.AccessLevels;

const describeKeyCommandResponse = await kms.describeKey({
KeyId: kmsKeyId,
@@ -38,6 +44,9 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
// Define the updated key policy to allow CloudFront Distribution access
const keyPolicy = JSON.parse(getKeyPolicyCommandResponse?.Policy);
console.log('Retrieved key policy', JSON.stringify(keyPolicy, undefined, 2));

const actions = getActions(accessLevels);

const kmsKeyPolicyStatement = {
Sid: 'AllowCloudFrontServicePrincipalSSE-KMS',
Effect: 'Allow',
@@ -46,18 +55,15 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
'cloudfront.amazonaws.com',
],
},
Action: [
'kms:Decrypt',
'kms:Encrypt',
'kms:GenerateDataKey*',
],
Action: actions,
Resource: `arn:${partition}:kms:${region}:${accountId}:key/${kmsKeyId}`,
Condition: {
StringEquals: {
'AWS:SourceArn': `arn:${partition}:cloudfront::${accountId}:distribution/${distributionId}`,
},
},
};

const updatedKeyPolicy = updatePolicy(keyPolicy, kmsKeyPolicyStatement);
console.log('Updated key policy', JSON.stringify(updatedKeyPolicy, undefined, 2));
await kms.putKeyPolicy({
@@ -74,6 +80,14 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
}
}

function getActions(accessLevels: string[]): string[] {
let actions: string[] = [];
for (const accessLevel of accessLevels) {
actions = actions.concat(KEY_ACTIONS[accessLevel]);
}
return actions;
}

/**
* Updates a provided policy with a provided policy statement. First checks whether the provided policy statement
* already exists. If an existing policy is found with a matching sid, the provided policy will overwrite the existing
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { S3Client, GetBucketPolicyCommand, PutBucketPolicyCommand } from '@aws-sdk/client-s3';
import { mockClient } from 'aws-sdk-client-mock';
import 'aws-sdk-client-mock-jest';
// import { handler } from '../../lib/aws-cloudfront-origins/s3-origin-access-control-key-policy-handler/index';


const s3Mock = mockClient(S3Client);
beforeEach(() => {
s3Mock.reset();
});

79 changes: 66 additions & 13 deletions packages/aws-cdk-lib/aws-cloudfront-origins/lib/s3-origin.ts
Original file line number Diff line number Diff line change
@@ -9,8 +9,15 @@ import { S3OriginAccessControlBucketPolicyProvider } from '../../custom-resource
import { S3OriginAccessControlKeyPolicyProvider } from '../../custom-resource-handlers/dist/aws-cloudfront-origins/s3-origin-access-control-key-policy-provider.generated';
import * as cxapi from '../../cx-api';

const S3_ORIGIN_ACCESS_CONTROL_KEY_RESOURCE_TYPE = 'Custom::S3OriginAccessControlKeyPolicy';
const S3_ORIGIN_ACCESS_CONTROL_BUCKET_RESOURCE_TYPE = 'Custom::S3OriginAccessControlBucketPolicy';
const S3_ORIGIN_ACCESS_CONTROL_KEY_RESOURCE_TYPE = 'Custom::S3OriginAccessControlKeyPolicyUpdater';
const S3_ORIGIN_ACCESS_CONTROL_BUCKET_RESOURCE_TYPE = 'Custom::S3OriginAccessControlBucketPolicyUpdater';

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

/**
* Properties to use to customize an S3 Origin.
*/
@@ -29,11 +36,36 @@ export interface S3OriginProps extends cloudfront.OriginProps {
readonly originAccessControl?: cloudfront.IOriginAccessControl;

/**
* When set to 'true', an attempt will be made to update the bucket policy to allow the
* When set to 'true', a best-effort attempt will be made to update the bucket policy to allow the
* CloudFront distribution access.
* @default false
*/
readonly overrideImportedBucketPolicy?: boolean;

/**
* The level of permissions granted in the bucket policy and key policy (if applicable)
* to the CloudFront distribution.
* @default AccessLevel.READ
*/
readonly originAccessLevels?: AccessLevel[];
}

/**
* The types of permissions to grant OAC access to th S3 origin
*/
export enum AccessLevel {
/**
* Grants 's3:GetObject' permission to OAC
*/
READ = 'READ',
/**
* Grants 's3:PutObject' permission to OAC
*/
WRITE = 'WRITE',
/**
* Grants 's3:DeleteObject' permission to OAC
*/
DELETE = 'DELETE',
}

/**
@@ -56,10 +88,12 @@ export class S3Origin implements cloudfront.IOrigin {
protocolPolicy: cloudfront.OriginProtocolPolicy.HTTP_ONLY, // S3 only supports HTTP for website buckets
...props,
});
} else if (props.originAccessIdentity || !FeatureFlags.of(bucket.stack).isEnabled(cxapi.CLOUDFRONT_USE_ORIGIN_ACCESS_CONTROL)) {
} else if (props.originAccessControl) {
this.origin = S3BucketOrigin.withAccessControl(bucket, props);
} else if (props.originAccessIdentity) {
this.origin = S3BucketOrigin.withAccessIdentity(bucket, props);
} else {
this.origin = S3BucketOrigin.withAccessControl(bucket, props);
this.origin = FeatureFlags.of(bucket.stack).isEnabled(cxapi.CLOUDFRONT_USE_ORIGIN_ACCESS_CONTROL_BY_DEFAULT) ? S3BucketOrigin.withAccessControl(bucket, props) : S3BucketOrigin.withAccessIdentity(bucket, props);
}
}

@@ -139,20 +173,26 @@ abstract class S3BucketOrigin extends cloudfront.OriginBase {
}

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

// Failed to update bucket policy, assume using imported bucket
if (!result.statementAdded) {
if (props.overrideImportedBucketPolicy) {
this.grantDistributionAccessToImportedBucket(scope, distributionId);
this.grantDistributionAccessToImportedBucket(scope, distributionId, actions);
} else {
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 (this.bucket.encryptionKey) {
this.grantDistributionAccessToKey(scope, distributionId, this.bucket.encryptionKey);
this.grantDistributionAccessToKey(
scope,
distributionId,
this.bucket.encryptionKey,
props.originAccessLevels ?? [AccessLevel.READ],
);
}

const originBindConfig = this._bind(scope, options);
@@ -175,13 +215,21 @@ abstract class S3BucketOrigin extends cloudfront.OriginBase {
return { originAccessIdentity: '' };
}

private grantDistributionAccessToBucket(distributionId: string): iam.AddToResourcePolicyResult {
private getActions(accessLevels: AccessLevel[]): string[] {
let actions: string[] = [];
for (const accessLevel of new Set(accessLevels)) {
actions = actions.concat(BUCKET_ACTIONS[accessLevel]);
}
return actions;
}

private grantDistributionAccessToBucket(distributionId: string | undefined, actions: string[]): iam.AddToResourcePolicyResult {
const oacReadOnlyBucketPolicyStatement = new iam.PolicyStatement(
{
sid: 'AllowS3OACAccess',
sid: 'GrantOACAccessToS3',
effect: iam.Effect.ALLOW,
principals: [new iam.ServicePrincipal('cloudfront.amazonaws.com')],
actions: ['s3:GetObject'],
actions,
resources: [this.bucket.arnForObjects('*')],
conditions: {
StringEquals: {
@@ -197,7 +245,7 @@ abstract class S3BucketOrigin extends cloudfront.OriginBase {
/**
* Use custom resource to update bucket policy and remove OAI policy statement if it exists
*/
private grantDistributionAccessToImportedBucket(scope: Construct, distributionId: string) {
private grantDistributionAccessToImportedBucket(scope: Construct, distributionId: string | undefined, actions: string[]) {
const provider = S3OriginAccessControlBucketPolicyProvider.getOrCreateProvider(scope, S3_ORIGIN_ACCESS_CONTROL_BUCKET_RESOURCE_TYPE, {
description: 'Lambda function that updates S3 bucket policy to allow CloudFront distribution access.',
});
@@ -215,14 +263,15 @@ abstract class S3BucketOrigin extends cloudfront.OriginBase {
AccountId: this.bucket.env.account,
Partition: Stack.of(scope).partition,
BucketName: this.bucket.bucketName,
Actions: actions,
},
});
}

/**
* Use custom resource to update KMS key policy
*/
private grantDistributionAccessToKey(scope: Construct, distributionId: string, key: IKey) {
private grantDistributionAccessToKey(scope: Construct, distributionId: string | undefined, key: IKey, accessLevels: AccessLevel[]) {
const provider = S3OriginAccessControlKeyPolicyProvider.getOrCreateProvider(scope, S3_ORIGIN_ACCESS_CONTROL_KEY_RESOURCE_TYPE,
{
description: 'Lambda function that updates SSE-KMS key policy to allow CloudFront distribution access.',
@@ -233,6 +282,9 @@ abstract class S3BucketOrigin extends cloudfront.OriginBase {
Resource: [key.keyArn],
});

// Remove duplicates and DELETE permissions which are not applicable to KMS key actions
const keyAccessLevels = [...new Set(accessLevels.filter(level => level !== AccessLevel.DELETE))];

new CustomResource(scope, 'S3OriginKMSKeyPolicyCustomResource', {
resourceType: S3_ORIGIN_ACCESS_CONTROL_KEY_RESOURCE_TYPE,
serviceToken: provider.serviceToken,
@@ -241,6 +293,7 @@ abstract class S3BucketOrigin extends cloudfront.OriginBase {
KmsKeyId: key.keyId,
AccountId: this.bucket.env.account,
Partition: Stack.of(scope).partition,
AccessLevels: keyAccessLevels
},
});
}
13 changes: 12 additions & 1 deletion packages/aws-cdk-lib/aws-cloudfront/lib/origin-access-control.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Construct } from 'constructs';
import { CfnOriginAccessControl } from './cloudfront.generated';
import { IResource, Resource, Names } from '../../core';
import { IResource, Resource, Names, Token } from '../../core';

/**
* Represents a CloudFront Origin Access Control
@@ -142,6 +142,11 @@ export class OriginAccessControl extends Resource implements IOriginAccessContro

this.originAccessControlOriginType = props.originAccessControlOriginType ?? OriginAccessControlOriginType.S3;

// check if origin access control name is 64 characters or less
if (props.originAccessControlName) {
this.validateOriginAccessControlName(props.originAccessControlName);
}

const resource = new CfnOriginAccessControl(this, 'Resource', {
originAccessControlConfig: {
description: props.description,
@@ -156,4 +161,10 @@ export class OriginAccessControl extends Resource implements IOriginAccessContro

this.originAccessControlId = resource.attrId;
}

private validateOriginAccessControlName(name: string) {
if (!Token.isUnresolved(name) && name.length > 64) {
throw new Error(`Origin access control name must be 64 characters or less, '${name}' has length ${name.length}`);
}
}
}
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-cloudfront/lib/origin.ts
Original file line number Diff line number Diff line change
@@ -130,7 +130,7 @@ export interface OriginBindOptions {
/**
* The identifier of the Distribution this Origin is used for.
*/
readonly distributionId: string;
readonly distributionId?: string | undefined;
}

/**
Loading