diff --git a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts index 518e3e728ec1a..a0ac4037d9741 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts @@ -1,4 +1,5 @@ import * as cloudfront from '@aws-cdk/aws-cloudfront'; +import * as iam from '@aws-cdk/aws-iam'; import * as s3 from '@aws-cdk/aws-s3'; import * as cdk from '@aws-cdk/core'; import { HttpOrigin } from './http-origin'; @@ -68,7 +69,7 @@ class S3BucketOrigin extends cloudfront.OriginBase { public bind(scope: Construct, options: cloudfront.OriginBindOptions): cloudfront.OriginBindConfig { if (!this.originAccessIdentity) { // Using a bucket from another stack creates a cyclic reference with - // the bucket taking a dependency on the generated S3CanonicalUserId when `grantRead` is called, + // the bucket taking a dependency on the generated S3CanonicalUserId for the grant principal, // and the distribution having a dependency on the bucket's domain name. // Fix this by parenting the OAI in the bucket's stack when cross-stack usage is detected. const bucketStack = cdk.Stack.of(this.bucket); @@ -79,7 +80,16 @@ class S3BucketOrigin extends cloudfront.OriginBase { this.originAccessIdentity = new cloudfront.OriginAccessIdentity(oaiScope, oaiId, { comment: `Identity for ${options.originId}`, }); - this.bucket.grantRead(this.originAccessIdentity); + + // Used rather than `grantRead` because `grantRead` will grant overly-permissive policies. + // Only GetObject is needed to retrieve objects for the distribution. + // This also excludes KMS permissions; currently, OAI only supports SSE-S3 for buckets. + // Source: https://aws.amazon.com/blogs/networking-and-content-delivery/serving-sse-kms-encrypted-content-from-s3-using-cloudfront/ + this.bucket.addToResourcePolicy(new iam.PolicyStatement({ + resources: [this.bucket.arnForObjects('*')], + actions: ['s3:GetObject'], + principals: [this.originAccessIdentity.grantPrincipal], + })); } return super.bind(scope, options); } diff --git a/packages/@aws-cdk/aws-cloudfront-origins/package.json b/packages/@aws-cdk/aws-cloudfront-origins/package.json index 8e2a945533280..02e293de5f675 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/package.json +++ b/packages/@aws-cdk/aws-cloudfront-origins/package.json @@ -79,6 +79,7 @@ "dependencies": { "@aws-cdk/aws-cloudfront": "0.0.0", "@aws-cdk/aws-elasticloadbalancingv2": "0.0.0", + "@aws-cdk/aws-iam": "0.0.0", "@aws-cdk/aws-s3": "0.0.0", "@aws-cdk/core": "0.0.0", "constructs": "^3.2.0" @@ -88,8 +89,9 @@ "@aws-cdk/core": "0.0.0", "constructs": "^3.2.0", "@aws-cdk/aws-cloudfront": "0.0.0", - "@aws-cdk/aws-s3": "0.0.0", - "@aws-cdk/aws-elasticloadbalancingv2": "0.0.0" + "@aws-cdk/aws-elasticloadbalancingv2": "0.0.0", + "@aws-cdk/aws-iam": "0.0.0", + "@aws-cdk/aws-s3": "0.0.0" }, "engines": { "node": ">= 10.13.0 <13 || >=13.7.0" diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.expected.json index 8e123f87338d4..ecd2096329404 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.expected.json +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.expected.json @@ -14,11 +14,7 @@ "PolicyDocument": { "Statement": [ { - "Action": [ - "s3:GetObject*", - "s3:GetBucket*", - "s3:List*" - ], + "Action": "s3:GetObject", "Effect": "Allow", "Principal": { "CanonicalUser": { @@ -28,28 +24,20 @@ ] } }, - "Resource": [ - { - "Fn::GetAtt": [ - "Bucket83908E77", - "Arn" - ] - }, - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "Bucket83908E77", - "Arn" - ] - }, - "/*" - ] + "Resource": { + "Fn::Join": [ + "", + [ + { + "Fn::GetAtt": [ + "Bucket83908E77", + "Arn" + ] + }, + "/*" ] - } - ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json index 5eb310b5b92d7..25461aa9cf7de 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json @@ -14,11 +14,7 @@ "PolicyDocument": { "Statement": [ { - "Action": [ - "s3:GetObject*", - "s3:GetBucket*", - "s3:List*" - ], + "Action": "s3:GetObject", "Effect": "Allow", "Principal": { "CanonicalUser": { @@ -28,28 +24,20 @@ ] } }, - "Resource": [ - { - "Fn::GetAtt": [ - "Bucket83908E77", - "Arn" - ] - }, - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "Bucket83908E77", - "Arn" - ] - }, - "/*" - ] + "Resource": { + "Fn::Join": [ + "", + [ + { + "Fn::GetAtt": [ + "Bucket83908E77", + "Arn" + ] + }, + "/*" ] - } - ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts index 54f9861685945..a502dd5cf4f46 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts @@ -87,9 +87,13 @@ describe('With bucket', () => { expect(stack).toHaveResourceLike('AWS::S3::BucketPolicy', { PolicyDocument: { Statement: [{ + Action: 's3:GetObject', Principal: { CanonicalUser: { 'Fn::GetAtt': ['DistOrigin1S3Origin87D64058', 'S3CanonicalUserId'] }, }, + Resource: { + 'Fn::Join': ['', [{ 'Fn::GetAtt': ['Bucket83908E77', 'Arn'] }, '/*']], + }, }], }, }); diff --git a/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts index f191813dada2b..a678c82ba6087 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts @@ -1027,9 +1027,15 @@ export class CloudFrontWebDistribution extends cdk.Resource implements IDistribu // first case for backwards compatibility if (originConfig.s3OriginSource.originAccessIdentity) { // grant CloudFront OriginAccessIdentity read access to S3 bucket - originConfig.s3OriginSource.s3BucketSource.grantRead( - originConfig.s3OriginSource.originAccessIdentity, - ); + // Used rather than `grantRead` because `grantRead` will grant overly-permissive policies. + // Only GetObject is needed to retrieve objects for the distribution. + // This also excludes KMS permissions; currently, OAI only supports SSE-S3 for buckets. + // Source: https://aws.amazon.com/blogs/networking-and-content-delivery/serving-sse-kms-encrypted-content-from-s3-using-cloudfront/ + originConfig.s3OriginSource.s3BucketSource.addToResourcePolicy(new iam.PolicyStatement({ + resources: [originConfig.s3OriginSource.s3BucketSource.arnForObjects('*')], + actions: ['s3:GetObject'], + principals: [originConfig.s3OriginSource.originAccessIdentity.grantPrincipal], + })); s3OriginConfig = { originAccessIdentity: `origin-access-identity/cloudfront/${originConfig.s3OriginSource.originAccessIdentity.originAccessIdentityName}`, diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json index 8f6357e72272e..72ed767994cc0 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json @@ -14,11 +14,7 @@ "PolicyDocument": { "Statement": [ { - "Action": [ - "s3:GetObject*", - "s3:GetBucket*", - "s3:List*" - ], + "Action": "s3:GetObject", "Effect": "Allow", "Principal": { "AWS": { @@ -37,28 +33,20 @@ ] } }, - "Resource": [ - { - "Fn::GetAtt": [ - "Bucket83908E77", - "Arn" + "Resource": { + "Fn::Join": [ + "", + [ + { + "Fn::GetAtt": [ + "Bucket83908E77", + "Arn" + ] + }, + "/*" ] - }, - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "Bucket83908E77", - "Arn" - ] - }, - "/*" - ] - ] - } - ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts b/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts index 8eea2cad5bc92..d24c7aa1220dc 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts @@ -10,6 +10,7 @@ import { GeoRestriction, KeyGroup, LambdaEdgeEventType, + OriginAccessIdentity, PublicKey, SecurityPolicyProtocol, SSLMethod, @@ -197,6 +198,59 @@ nodeunitShim({ test.done(); }, + 'distribution with bucket and OAI'(test: Test) { + const stack = new cdk.Stack(); + const s3BucketSource = new s3.Bucket(stack, 'Bucket'); + const originAccessIdentity = new OriginAccessIdentity(stack, 'OAI'); + + new CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', { + originConfigs: [{ + s3OriginSource: { s3BucketSource, originAccessIdentity }, + behaviors: [{ isDefaultBehavior: true }], + }], + }); + + expect(stack).to(haveResourceLike('AWS::CloudFront::Distribution', { + DistributionConfig: { + Origins: [ + { + ConnectionAttempts: 3, + ConnectionTimeout: 10, + DomainName: { + 'Fn::GetAtt': [ + 'Bucket83908E77', + 'RegionalDomainName', + ], + }, + Id: 'origin1', + S3OriginConfig: { + OriginAccessIdentity: { + 'Fn::Join': ['', ['origin-access-identity/cloudfront/', { Ref: 'OAIE1EFC67F' }]], + }, + }, + }, + ], + }, + })); + + expect(stack).to(haveResourceLike('AWS::S3::BucketPolicy', { + PolicyDocument: { + Statement: [{ + Action: 's3:GetObject', + Principal: { + CanonicalUser: { 'Fn::GetAtt': ['OAIE1EFC67F', 'S3CanonicalUserId'] }, + }, + Resource: { + 'Fn::Join': ['', [{ 'Fn::GetAtt': ['Bucket83908E77', 'Arn'] }, '/*']], + }, + }], + }, + })); + + test.done(); + }, + + 'distribution with trusted signers on default distribution'(test: Test) { const stack = new cdk.Stack(); const sourceBucket = new s3.Bucket(stack, 'Bucket');