aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

cloudfront: Support Origin Access Control #21771

Closed laurelmay closed 2 months ago

laurelmay commented 2 years ago

Describe the feature

Amazon CloudFront now supports Origin Access Control, an improved method for accessing S3 Origins over Origin Access Identity.

It seems that the CloudFormation documentation (and resource specification) has not yet been updated but the OAC docs contain an example of deploying using CloudFormation.

I am opening an issue to discuss this because the S3Origin class currently only supports OAI and creates one by default; migrating to do so would likely

Use Case

The release announcement and documentation describe OAC as an improvement over OAI. OAC supports requests other than GET as well as SSE-KMS, and all AWS regions. Documentation already refers to OAI as "legacy".

Proposed Solution

I propose doing all of the following:

Feature flag behavior

Flag value S3Props.originAccessIdentity provided S3Props.originAccessControl provided Behavior
true no no Only an OAC is created and used
true yes no The given OAI is used and an OAC is created and used
true no yes The given OAC is used, no OAI is created
true yes yes The given OAI and given OAC are used
false no no An OAI is created and used
false yes no The given OAI is used, no OAC is created
false no yes The given OAC is used, no OAI is created or used
false yes yes The given OAI and given OAC are used

To migrate, a user would enable the feature flag. If they were already passing an OAI, they'd run with both side-by-side (a supported configuration), if not they'd use only an OAC. To migrate to OAC-only, they can then use a custom-created OAC or the default one.

Other Information

The CreateOriginAccessControl action seems like it's pretty likely to map 1:1 to the CloudFormation resource https://docs.aws.amazon.com/cloudfront/latest/APIReference/API_CreateOriginAccessControl.html. The example in the docs is:

Type: AWS::CloudFront::OriginAccessControl
Properties: 
  OriginAccessControlConfig: 
      Description: An optional description for the origin access control
      DisplayName: ExampleOAC
      OriginType: s3
      SigningBehavior: always
      SigningProtocol: sigv4

Implementation requires a new CloudFormation Resource Specification but since an example is given in the CloudFront docs, hopefully that won't take and since this may require other changes, having a conversation may be helpful.

This felt smaller than an RFC but I'm happy to open one if needed.

Acknowledgements

CDK version used

2.39.0

Environment details (OS name and version, etc.)

-

peterwoodworth commented 2 years ago

We should have L1 support soon, any higher level abstractions for this new feature may take a bit to be implemented as we see how customers use the L1 construct

zwlego commented 2 years ago

Hey, Is the L1 support out? Is it for now we can only use OAI with CDK? Thanks

We should have L1 support soon, any higher level abstractions for this new feature may take a bit to be implemented as we see how customers use the L1 construct

laurelmay commented 2 years ago

Hey, Is the L1 support out? Is it for now we can only use OAI with CDK? Thanks

~It looks like L1 updates may currently be failing due to https://github.com/aws/aws-cdk/issues/21957~

Okay, so while it's true that updates haven't happen, the required type has not made it into the resource specification yet. So this is still basically waiting on CloudFormation Resource Specification to update.

agdimech commented 2 years ago

I just had a look and do actually see the OAC in the resource specification listed as:

"AWS::CloudFront::OriginAccessControl": {
      "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-cloudfront-originaccesscontrol.html",
      "Properties": {
        "OriginAccessControlConfig": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-cloudfront-originaccesscontrol.html#cfn-cloudfront-originaccesscontrol-originaccesscontrolconfig",
          "UpdateType": "Mutable",
          "Required": true,
          "Type": "OriginAccessControlConfig"
        }
      },
      "Attributes": {
        "Id": {
          "PrimitiveType": "String"
        }
      }
    },
laurelmay commented 2 years ago

Oh nice! Yeah, so it looks like now this actually is blocked by #21957 (or the underlying tracked issue) since OAC was added in spec v88.0.0!

agdimech commented 2 years ago

Looks like once this PR is merged and released we should have L1 constructs :)

https://github.com/aws/aws-cdk/pull/22026

laurelmay commented 2 years ago

I've started poking at implementing this (based off the branch created by #22026) and the real pain is going to be from the fact that the principal of the OAC is the CloudFront service principal with a condition of { StringEquals: { "AWS:SourceArn": "DISTRIBUTION_ARN" } }; however, the OAC does not "know" the ARN of the Distribution, so it has to be calculated. Doing so would require passing the Distribution (or a property of it) to the S3Origin construct (because the Bucket Policy will need to be modified) and today, that doesn't really happen anywhere; it'd require a deeper dive into the Origin API and a lot of the binding/resolving that happens there.

This can possibly be solved with some lazy evaluation but it will probably require a bit more than that.

A somewhat obvious (and equally obviously suboptimal) solution would be to not specify the Condition shown in the docs and to instead just allow the raw service principal but that's excessively (and likely, to most, unacceptably) broad access.

Another thing is that while the examples in the docs always show a 1:1 OAC:Origin mapping, the CloudFormation resource structure seems to imply that it could be many:many (whether the underlying service allows this is a different matter).

Even if a Distribution "silently" created an OAC, that would need to be passed to the Origin constructs (for the distribution ARN and the OAC ID). But some good news is that at least if passing this data around is solved, adding the statement(s) the KMS key's (if present) resource policy should be trivial (since it'd be a mirror of the S3 bucket).

So with this situation, it will be a little harder to implement than I had assumed in the opening message before seeing the L1 constructs irl.

I am not going to open a PR based on by branch because I fear it'd generate quite a bit of noise. But the following links should be sufficient to understand where it's at:

github-actions[bot] commented 2 years ago

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

rupe120 commented 2 years ago

I think the CloudFormation team may have pulled the rug out from under the feature flag effort. It looks like the Origin Access Identity is not supported in CloudFormation and now the L2 CloudFront classes won't work at all in terms of S3 access control.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-distribution-legacys3origin.html#cfn-cloudfront-distribution-legacys3origin-originaccessidentity

I have a demonstration here https://github.com/aws/aws-cdk/issues/22451


A note for those working around this via L1 cfn classes. The Route53 CfnRecordSet Alias has a hosted zone Id attribute that is different from the hosted zone Id attribute on the RecordSet itself. You will probably just want to set it to the static CLOUTFRONT_ZONE_ID, unless you are in China. : o)

new CfnRecordSet(this, 'cfn-recordset',
    {
      name: props.environmentDomainName,
      type: "A",
      hostedZoneId: hostedZone.hostedZoneId,
      aliasTarget: 
      {
        dnsName: cfnDistribution.attrDomainName,
        hostedZoneId: CloudFrontTarget.CLOUDFRONT_ZONE_ID
      }
    }).addDependsOn(cfnDistribution);

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_route53.CfnRecordSet.AliasTargetProperty.html#hostedzoneid

kornicameister commented 2 years ago

Did someone manage to make it work for use cases like:

  1. S3 (SSE-S3) + OAC
  2. S3 (SEE-KMS) + OAC

I have a piece of WIP code related to 1st point in the list but no I continue to get forbidden when I try to access the content inside the S3 origin.

        oac = cf.CfnOriginAccessControl(
            self,
            'OAC',
            origin_access_control_config=cf.CfnOriginAccessControl.
            OriginAccessControlConfigProperty(
                name=f'{cdk.Stack.of(self).stack_name}_OAC',
                description=f'[{environment}] OAC to access buckets',
                origin_access_control_origin_type='s3',
                signing_behavior='always',
                signing_protocol='sigv4',
            ),
        )
        cf_distro = t.cast(
            cf.CfnDistribution,
            distro.node.default_child,
        )
        cf_distro.add_property_override(
            'DistributionConfig.Origins.0.OriginAccessControlId',
            oac.get_att('Id'),
        )
        cf_distro.add_property_override(
            'DistributionConfig.Origins.0.S3OriginConfig.OriginAccessIdentity',
            '',
        )

        bucket.add_to_resource_policy(
            iam.PolicyStatement(
                effect=iam.Effect.ALLOW,
                actions=[
                    's3:GetObject',
                ],
                principals=[iam.ServicePrincipal('cloudfront')],
                resources=[bucket.arn_for_objects('*')],
                conditions={
                    'StringEquals': {
                        'AWS:SourceArn': cdk.Stack.of(self).format_arn(
                            service='cloudfront',
                            region='',
                            resource='distribution',
                            resource_name=distro.distribution_id, 
                            arn_format=cdk.ArnFormat.SLASH_RESOURCE_NAME,
                        ),
                    },
                },
            ),
        )
hfgbarrigas commented 2 years ago

@kornicameister

I got it working for the first case, here's the snippet.

    const oac = new cloudfront.CfnOriginAccessControl(this, 'AOC', {
      originAccessControlConfig: {
        name: 'AOC',
        originAccessControlOriginType: 's3',
        signingBehavior: 'always',
        signingProtocol: 'sigv4',
      },
    })

    const cloudFrontWebDistribution = new cloudfront.CloudFrontWebDistribution(this, 'CDN', {
      originConfigs: [
        {
          s3OriginSource: {
            s3BucketSource: this.s3Bucket,
          },
          behaviors: [
            {
              isDefaultBehavior: true,
              allowedMethods: cloudfront.CloudFrontAllowedMethods.GET_HEAD,
              compress: true,
              cachedMethods: cloudfront.CloudFrontAllowedCachedMethods.GET_HEAD,
              viewerProtocolPolicy: cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
              minTtl: cdk.Duration.seconds(0),
              maxTtl: cdk.Duration.seconds(86400),
              defaultTtl: cdk.Duration.seconds(3600),
            },
          ],
        },
      ],
    })

    const cfnDistribution = cloudFrontWebDistribution.node.defaultChild as CfnDistribution

    cfnDistribution.addPropertyOverride('DistributionConfig.Origins.0.OriginAccessControlId', oac.getAtt('Id'))
kornicameister commented 2 years ago

@hfgbarrigas Hasn't that left OAI somewhere around? I think I have been trying something similar but it hasn't worked until I have removed OAI reference.

scottbisker commented 2 years ago

@kornicameister What does the origins section look like for you after you synth the stack? Every time I've used the override to remove OriginAccessIdentity, the entire S3Origin config is removed instead of being S3OriginConfig: {}

kornicameister commented 2 years ago

@scottbisker I was referring to OAI cloudformation resource.

As for the origins, it actually looks fine.

Anyway, what I feel I need to point out as a separate thing is that I wanted to have a single stack that's domain is storage (buckets, efs, whatnot) and another stack just for CDN. Needless to say that design is circular dependency all around. Bucket policy needs to reference distribution and distribution needs bucket.

@kylelaker do you plan to address that somehow or is it just me organizing stacks in wrong way?

kornicameister commented 2 years ago

FYI: I am also observing this issue when trying to create CDN + buckets inside a single stack. The key aspect of the setup is having the bucket encrypted with customer KMS key. Here's a short repro:

import aws_cdk as cdk
from aws_cdk import (
    aws_cloudfront as cf,
    aws_cloudfront_origins as cf_origins,
    aws_iam as iam,
    aws_kms as kms,
    aws_s3 as s3,
    aws_sns as sns,
)

class OACCircularIssueStack(cdk.Stack):

    def __init__(
        self,
        scope: Construct,
        construct_id: str,
        **kwargs: t.Any,
    ) -> None:
        super().__init__(
            scope,
            construct_id,
            description='Demonstrates circular issue when using OAC',
            termination_protection=False,
            analytics_reporting=False,
            **kwargs,
        )

        encryption_key = kms.Key(
            self,
            'Key',
            description='Some key to just have bucket encrypted',
            removal_policy=cdk.RemovalPolicy.DESTROY,
        )
        bucket = s3.Bucket(
            self,
            'Bucket',
            enforce_ssl=True,
            public_read_access=False,
            removal_policy=cdk.RemovalPolicy.DESTROY,
            auto_delete_objects=True,
            encryption_key=encryption_key,
            encryption=s3.BucketEncryption.KMS,
            bucket_key_enabled=True,
            block_public_access=s3.BlockPublicAccess.BLOCK_ALL,
            object_ownership=s3.ObjectOwnership.BUCKET_OWNER_ENFORCED,
        )

        oac = cf.CfnOriginAccessControl(
            self,
            'OAC',
            origin_access_control_config=(
                cf.CfnOriginAccessControl.OriginAccessControlConfigProperty(
                    name=f'{cdk.Stack.of(self).stack_name}_OAC',
                    description='OAC to access buckets',
                    origin_access_control_origin_type='s3',
                    signing_behavior='always',
                    signing_protocol='sigv4',
                )
            ),
        )
        distro = cf.Distribution(
            self,
            'Distribution',
            comment='Distribution using OAC',
            enable_ipv6=False,
            http_version=cf.HttpVersion.HTTP2_AND_3,
            price_class=cf.PriceClass.PRICE_CLASS_100,
            default_behavior=cf.BehaviorOptions(
                origin=cf_origins.S3Origin(
                    bucket,
                    origin_path='/',
                ),
                allowed_methods=cf.AllowedMethods.ALLOW_GET_HEAD,
                cached_methods=cf.CachedMethods.CACHE_GET_HEAD,
                viewer_protocol_policy=cf.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
                origin_request_policy=cf.OriginRequestPolicy.CORS_S3_ORIGIN,
                cache_policy=cf.CachePolicy.CACHING_OPTIMIZED,
            ),
        )

        cf_distro = t.cast(
            cf.CfnDistribution,
            distro.node.default_child,
        )
        cf_distro.add_property_override(
            'DistributionConfig.Origins.0.OriginAccessControlId',
            oac.get_att('Id'),
        )
        cf_distro.add_property_override(
            'DistributionConfig.Origins.0.S3OriginConfig.OriginAccessIdentity',
            '',
        )

        distribution_arn = cdk.Stack.of(self).format_arn(
            service='cloudfront',
            region='',
            resource='distribution',
            resource_name=distro.distribution_id,
            arn_format=cdk.ArnFormat.SLASH_RESOURCE_NAME,
        )
        cloudfront = iam.ServicePrincipal('cloudfront')

        bucket.add_to_resource_policy(
            iam.PolicyStatement(
                effect=iam.Effect.ALLOW,
                actions=[
                    's3:GetObject',
                ],
                principals=[cloudfront],
                resources=[bucket.arn_for_objects('*')],
                conditions={
                    'ArnEquals': {
                        'AWS:SourceArn': distribution_arn,
                    },
                },
            ),
        )
        encryption_key.add_to_resource_policy(
            iam.PolicyStatement(
                effect=iam.Effect.ALLOW,
                actions=[
                    'kms:Decrypt',
                    'kms:Encrypt',
                    'kms:GenerateDataKey*',
                ],
                principals=[cloudfront],
                resources=['*'],
                conditions={
                    'ArnEquals': {
                        'AWS:SourceArn': distribution_arn,
                    },
                },
            ),
            allow_no_op=False,
        )

app = cdk.App()
OACCircularIssueStack(app,   'oac-circular-issue')
app.synth()

cdk synth works just fine but cdk deploy fails with:

Deployment failed: Error: Stack Deployments Failed: ValidationError: Circular dependency between resources: [Bucket83908E77, BucketPolicyE9A3008A, Key961B73FD, CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F, Distribution830FAC52, BucketAutoDeleteObjectsCustomResourceBAFD23C2]

So the problem lies well below CDK. It's CloudFormation issue.

kornicameister commented 2 years ago
@@ -21,9 +21,11 @@ def __init__(
             description='Some key to just have bucket encrypted',
             removal_policy=cdk.RemovalPolicy.DESTROY,
         )
+        bucket_name = f'{self.stack_name}.oac-bucket'
         bucket = s3.Bucket(
             self,
             'Bucket',
+            bucket_name=bucket_name,
             enforce_ssl=True,
             public_read_access=False,
             removal_policy=cdk.RemovalPolicy.DESTROY,
@@ -57,7 +59,11 @@ def __init__(
             price_class=cf.PriceClass.PRICE_CLASS_100,
             default_behavior=cf.BehaviorOptions(
                 origin=cf_origins.S3Origin(
-                    bucket,
+                    s3.Bucket.from_bucket_name(
+                        self,
+                        'BucketOrigin',
+                        bucket_name,
+                    ),
                     origin_path='/',
                 ),
                 allowed_methods=cf.AllowedMethods.ALLOW_GET_HEAD,

Seems like a way to break the circular dependency is to:

  1. created hardcoded name for the bucket
  2. reimport the bucket when creating the origin
kornicameister commented 2 years ago

@kylelaker is there any progress here?

wz2b commented 2 years ago

@kylelaker is there any progress here?

I'm curious where this stands, too. I'm standing up a brand new project, and I am doing it with OriginAccessIdentity but then I see that marked as "Legacy." I tried to follow the workaround above but I don't follow which worked / didn't work. My thought here is to continue using OAI until this is all sorted out.

laurelmay commented 2 years ago

Hey, I am probably not suited to continue moving forward with making this change. I have now unchecked the box in the original message to indicate I'm not planning to work on this. It does look like this issue has also gotten enough 👍🏻s to be put onto the "CDK Ownership" board by the bot; however, it hasn't moved from the "Incoming" state.

I think that because the API around this needs a really healthy amount of consideration, because OAI and OAC can't be used together, and because the OAC to Distribution to Source relationship isn't 1:1, this should probably be created as an RFC and handled over there; that way a bar raiser can be found and there can be more back and forth with the core CDK team around the design.

kornicameister commented 1 year ago

@wz2b you can pick my last two code examples if you want. They key is to set the bucket name and reimport the bucket. This is what made it work for me.

It is a workaround but that compared to deploying us-east-1 stack with Lambda@edge sounded like a reasonable tradeoff.

andreprawira commented 1 year ago

Let me start by saying that this has been a huge pain. We are trying to create a very simple CDN Distribution using S3 Origin that does not use OAI nor OIC, we simply want to use Public. However, by default, CDK keeps creating OAI by default and adding a policy that overwrites any policy we implement in the S3 bucket policy!

When going through this link or this or even this, we could not find any reference how to use Public. However, after talking to a very helpful AWS support engineer, we were able to change the origin to Public by using L1 construct

public_variable_distribution = myWebDistribution.node.default_child
        public_variable_distribution.add_override('Properties.DistributionConfig.Origins.0.S3OriginConfig.OriginAccessIdentity', "")

Keep in mind this solution only solves half the problem as the bucket policy is still overwritten by the default OAI bucket policy created by CDK, and to override that default OAI bucket policy, we would do

com_s3_policy_override = frontend_bucket.node.find_child("Policy").node.default_child
        com_s3_policy_override.add_override('Properties.PolicyDocument.Statement.0.Action', "s3:GetObject");
        com_s3_policy_override.add_override('Properties.PolicyDocument.Statement.0.Effect', "Allow");
        com_s3_policy_override.add_override('Properties.PolicyDocument.Statement.0.Principal', "*");

The way we solved the problem is very difficult, even the AWS engineer agreed that it was difficult to find the reference for it, and he found it here. and here

Notice that its extremely unclear when whoever wrote this doc says

to be able to access objects using either the CloudFront URL or the Amazon S3 URL, specify an empty OriginAccessIdentity element

Nowhere in the documentation that says it will by default create an OAI and overrides any bucket policy already declared, nowhere that says you can override the origin and bucket policy in the relevant documentations! Nowhere in any of the links that i provided above mentioned the keywords "Public" and "Legacy Access Identity" which is what we see in the console. He also informed me that he has raised internal feature request to update the wording. I definitely agree that an L2 construct should be created for an easier implementation and at the very least the relevant CDK docs should mentioned the OAI and default policy created! 5 stars to the AWS engineer who helped us to solve this issue

kornicameister commented 1 year ago

Long story short, a custom L2 resource would be in order to work around current implementation and limitations coming directly from CFN?

rupe120 commented 1 year ago

Long story short, a custom L2 resource would be in order to work around current implementation and limitations coming directly from CFN?

Correct

cornelcroi commented 1 year ago

Any updates on this? I would really need OAC instead of OAI.

AMZN-hgoffin commented 1 year ago

I got bit by this too. AFAICT there are three inter-related problems currently -

1) CloudFront L2 S3Origin construct automatically attaches new policy to provided Bucket construct 2) CloudFront L2 S3Origin construct always creates OAI policy and attaches it to Origin by ID 3) There is no L2 construct for OAC.

The third issue is easy, you can construct a L1 OAC and override the origin properties to add the OAC ID. See comment above (https://github.com/aws/aws-cdk/issues/21771#issuecomment-1281190832)

You can solve the other two issues by reconstructing buckets from hostnames and overriding the S3Origin implementation details to bypass all of the OAI junk. I haven't bothered to work out how to do this in a properly deferred way, I'm using hardcoded buckets, but it should be possible ...

class MySimpleS3Origin extends cf.OriginBase {
  constructor(fullyQualifiedBucketDomain: string, props?: cf_origins.S3OriginProps) {
    super(fullyQualifiedBucketDomain, props);
  }
  // note, intentionally violates the return type to render an object with no OAI properties
  protected renderS3OriginConfig() {
    return {};
  }
}
    const cfdist = new cf.Distribution(this, 'Dist', {
      defaultBehavior: {
        origin: new MySimpleS3Origin(
          `${bucketName}.s3.${bucketRegion}.amazonaws.com`,
          { originPath: `/cdnroot` },
        ),
      },
    });

    // OAC access control can't be specified via high-level constructs at this time,
    // so build it from L1 construct and attach it by id, via raw property override.
    const cfCfnDist = cfdist.node.defaultChild as cf.CfnDistribution;
    const oac = new cf.CfnOriginAccessControl(this, 'OAC', {
      originAccessControlConfig: {
        name: 'ProtocolCdnOAC',
        originAccessControlOriginType: 's3',
        signingBehavior: 'always',
        signingProtocol: 'sigv4',
      },
    });
    cfCfnDist.addPropertyOverride('DistributionConfig.Origins.0.OriginAccessControlId', oac.getAtt('Id'));

Now, none of this solves the problem of putting an accessible policy on the S3 bucket, but that problem is easier to understand than magic policies being automatically added by the old S3Origin construct. In my case, my buckets are configured to grant GetObject access to the cloudfront service principal with any SourceArn that matches my deployment account, using StringLike to check a prefix of the SourceArn including the account id up through ":deployment/*"

AMZN-hgoffin commented 1 year ago

This suggests that the right solution to this problem is to implement an entirely new Origin construct, and deprecate S3Origin as a legacy construct that does a bunch of things that nobody wants any more.

Given that, then the correct way to solve the current "circular permissions dependency" problem is just to leave the bucket overly restrictive at first, configure CloudFront to point at the bucket even though it doesn't have permissions yet, and then open up a new Allow policy statement on the bucket after you know the CloudFront distribution ID to put in the statement. I'm sure there is a way to express this in the dependency system, I just don't have time to find it right now.

cornelcroi commented 1 year ago

The code I use is the following.

const oac = new cloudfront.CfnOriginAccessControl(this, 'AOC', {
      originAccessControlConfig: {
        name: 'AOC',
        originAccessControlOriginType: 's3',
        signingBehavior: 'always',
        signingProtocol: 'sigv4',
      },
    });

const distribution = new cloudfront.Distribution(this, "Distribution", {
      comment: "Test AOC",
      defaultRootObject: "index.html",
      httpVersion: cloudfront.HttpVersion.HTTP2_AND_3,
      minimumProtocolVersion: cloudfront.SecurityPolicyProtocol.TLS_V1_2016,
      defaultBehavior: {
        origin: s3origin,
        allowedMethods: cloudfront.AllowedMethods.ALLOW_GET_HEAD,
        viewerProtocolPolicy: cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,

      }
    });

The I need to set the OAC for CF

const cfnDistribution = distribution.node.defaultChild as CfnDistribution
cfnDistribution.addOverride('Properties.DistributionConfig.Origins.0.S3OriginConfig.OriginAccessIdentity', "")
cfnDistribution.addPropertyOverride('DistributionConfig.Origins.0.OriginAccessControlId', oac.getAtt('Id'))

Then I'm deleting the CanonicalUser for the bucket policy. If you don't delete it, the OAC condition will be append it.

  const comS3PolicyOverride = hostingBucket.node.findChild("Policy").node.defaultChild as CfnBucketPolicy;
  const statement = comS3PolicyOverride.policyDocument.statements[0];
  if(statement['_principal'] && statement['_principal'].CanonicalUser){
    delete statement['_principal'].CanonicalUser
  }
  comS3PolicyOverride.addOverride('Properties.PolicyDocument.Statement.0.Principal', {"Service": "cloudfront.amazonaws.com" });
    comS3PolicyOverride.addOverride('Properties.PolicyDocument.Statement.0.Condition', 
    {
      'StringEquals': {
        'AWS:SourceArn': this.formatArn({
          service: 'cloudfront',
          region: '',
          resource: 'distribution',
          resourceName: distribution.distributionId,
          arnFormat: ArnFormat.SLASH_RESOURCE_NAME
        })
      }
    }
 );

This code works. The only problem is that an OAI is created but not used. I tried to remove it but no luck.

kyleplant commented 1 year ago

Try the following to remove OAI

const s3OriginNode = distribution.node.findAll().filter((child) => child.node.id === 'S3Origin'); s3OriginNode[0].node.tryRemoveChild('Resource');

cornelcroi commented 1 year ago

Awesome @kyleplant , thanks.

s-nt-s commented 1 year ago
const cfnDistribution = distribution.node.defaultChild as CfnDistribution
cfnDistribution.addOverride('Properties.DistributionConfig.Origins.0.S3OriginConfig.OriginAccessIdentity', "")
cfnDistribution.addPropertyOverride('DistributionConfig.Origins.0.OriginAccessControlId', oac.getAtt('Id'))

works, but I need to know the index of my origin (in the example it would be 0)

is there any way to do this for all S3Origin without knowing its index?

sebpaulus commented 1 year ago

@kornicameister Even after splitting stacks and reimporting bucket by name in the distribution stack I'm getting circular dependencies between: bucket, kms key, auto delete handler and the policy. Could I ask you for a small code sample of the setup?

kornicameister commented 1 year ago

The sample I have provided and code used to create that now seen couple deployments and several upgrades hasn't really changed.

I can't really give out the code because it is proprietary, sorry 😔

sebpaulus commented 1 year ago

@kornicameister No problem at all! I think it was on me, I had some resources split in multiple stacks. Trying now with everything in a single stack (with the bucket reimport)

AMZN-hgoffin commented 1 year ago

I am working on a solution (draft linked above - https://github.com/aws/aws-cdk/pull/24861)

There are several parts to the solution: 1) add new L2 constructs for OAC - done 2) support OAC instead of OAI using L2 constructs - done 3) automatically grant cloudfront read permission via resource policies on S3 buckets and KMS keys - ... absolute nightmare

As some people have noticed, the KMS Key resource policy is part of the Key construct and cannot be resolved "later". There is an implicit circular dependency between the KMS key resource policy and the CloudFront distribution ID, which is an output that is only known after deploying the Distribution. The Distribution can't be configured without resolving its origins, and the origin buckets can't be resolved without their KMS keys, but the keys can't be resolved until their resource policy knows the distribution ID from deploying the Distribution.

There are complicated reasons why neither KMS Keys nor CloudFront Distributions can change their behavior and allow the dependency to be broken.

The best idea I have for now is to create and deploy the stack in a broken, permission-less state (where CloudFront can't actually access the origin) and then use a Custom Resource to reconfigure the KMS key policy at the end.

rupe120 commented 1 year ago

The best idea I have for now is to create and deploy the stack in a broken, permission-less state (where CloudFront can't actually access the origin) and then use a Custom Resource to reconfigure the KMS key policy at the end.

I feel like you could just make the key policy overly permissive to any CloudFront distribution instance by excluding the condition, and then just update the policy with the specific ARN condition in the Custom Resource.

It's still awkward but at least there is some level of permissions at all times.

KarthikChandy commented 1 year ago

If anyone here is looking for a Python workaround to get OAC working with the cloudfront.Distribution() construct, then read-on. Thanks to @cornelcroi and @kyleplant for earlier workarounds in Typescript.

...
# create an S3 bucket
    demo_site_bucket = s3.Bucket(
        ...
    )
...

# create an OAC
demo_oac_config = cloudfront.CfnOriginAccessControl.OriginAccessControlConfigProperty(
    name="TTLabDemo-OAC-Config",
    origin_access_control_origin_type="s3",
    signing_behavior="always",
    signing_protocol="sigv4"
)

demo_oac = cloudfront.CfnOriginAccessControl(
    self,
    "TTLabDemo-OAC",
    origin_access_control_config=demo_oac_config
)

# create a distribution
demo_distribution = cloudfront.Distribution(
    self,
    "TTLabDemo-Distribution",
    ...
    default_behavior = cloudfront.BehaviorOptions(
        allowed_methods = cloudfront.AllowedMethods.ALLOW_GET_HEAD_OPTIONS,
        cached_methods = cloudfront.CachedMethods.CACHE_GET_HEAD_OPTIONS,
        cache_policy = cloudfront.CachePolicy.CACHING_OPTIMIZED,
        compress=True,
        edge_lambdas = [
            cloudfront.EdgeLambda(
                event_type=cloudfront.LambdaEdgeEventType.ORIGIN_REQUEST,
                function_version=latest_redirect_lambda_edge_version,
                include_body=True
            )
        ],
        origin_request_policy = cloudfront.OriginRequestPolicy.CORS_S3_ORIGIN,
        response_headers_policy = cloudfront.ResponseHeadersPolicy.CORS_ALLOW_ALL_ORIGINS,
        viewer_protocol_policy = cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
        origin=origins.S3Origin(
            demo_site_bucket
        )
    ),
    default_root_object='index.html',
    price_class = cloudfront.PriceClass.PRICE_CLASS_ALL
)

demo_oac_bucket_statement = iam.PolicyStatement(
    sid='TTAllowS3OACAccess',
    effect=iam.Effect.ALLOW,
    principals=[iam.ServicePrincipal('cloudfront.amazonaws.com')],
    actions=['s3:GetObject', 's3:PutObject'],
    resources=[f"arn:aws:s3:::something23874832473847328/*"],
    conditions={
        "StringEquals": {
            "aws:sourceArn": f"arn:aws:cloudfront::{self.account}:distribution/{demo_distribution.distribution_id}",
        }
    }
)

# add the above S3 bucket policy to the "demo_site_bucket"
demo_site_bucket.add_to_resource_policy(demo_oac_bucket_statement)

## clean-up the OAI reference and associate the OAC with the cloudfront distribution 
# query the site bucket policy as a document
bucket_policy = demo_site_bucket.policy
bucket_policy_document = bucket_policy.document

# remove the CloudFront Origin Access Identity permission from the bucket policy
if isinstance(bucket_policy_document, iam.PolicyDocument):
    bucket_policy_document_json = bucket_policy_document.to_json()
    # create an updated policy without the OAI reference
    bucket_policy_updated_json = {'Version': '2012-10-17', 'Statement': []}
    for statement in bucket_policy_document_json['Statement']:
        if 'CanonicalUser' not in statement['Principal']:
            bucket_policy_updated_json['Statement'].append(statement)

# apply the updated bucket policy to the bucket
bucket_policy_override = demo_site_bucket.node.find_child("Policy").node.default_child
bucket_policy_override.add_override('Properties.PolicyDocument', bucket_policy_updated_json)

# remove the created OAI reference (S3 Origin property) for the distribution
all_distribution_props = demo_distribution.node.find_all()
for child in all_distribution_props:
    if child.node.id == 'S3Origin':
        child.node.try_remove_child('Resource')

# associate the created OAC with the distribution
distribution_props = demo_distribution.node.default_child
distribution_props.add_override('Properties.DistributionConfig.Origins.0.S3OriginConfig.OriginAccessIdentity', '')
distribution_props.add_property_override(
    "DistributionConfig.Origins.0.OriginAccessControlId",
    demo_oac.ref
)
piotrekwitkowski commented 1 year ago

I would like to encourage everyone in this thread to react to the CloudFront Origin Access Control L2 RFC either by giving a thumbs up in the first post or commenting on the RFC

agdimech commented 11 months ago

I have just opened: https://github.com/aws/aws-pdk/pull/660 which is for the aws-pdk, however it is completely generic and my preference would be to contribute it to the aws-cdk directly. Keen to get feedback from interested parties.

biffgaut commented 10 months ago

The AWS Solutions Constructs library has addressed this issue in its aws-cloudfront-s3 construct. This construct will create a CloudFront distribution in front of a new or existing S3 bucket, and implement an OAC instead of the default OAI. As an additional benefit, it includes a custom resource that addresses the circular dependency issue when the bucket is encrypted by a CMK, allowing the policies to specify a the CMK rather than just use an '*'. The Solutions Constructs library is available in npm, PyPi and Maven.

antstanley commented 7 months ago

Hey Folks! AWS announced support for Origin Access Control for Lambda Function URLs and AWS Elemental MediaPackage Origin yesterday.

Please can we get some movement on an OAC L2 construct. This is getting beyond embarrassing and now actively making workloads less secure.

@colifran @scanlonp @evgenyka please escalate to get some movement on this RFC

abaschen commented 7 months ago

Hi everyone, I stumbled upon the same issue too and started using the AWS Solutions Constructs too but ended up discovering something unusual. My goal is to serve a default S3 bucket on '/' and many "subsites" on '/site-a', 'site-b' using behaviors ... I also wanted to use OAC which the construct provided conveniently with the override. But here are my findings:

An example, for what is worth is available here: https://github.com/aws-abaschen/example-cdk-cloudfront-s3-static-sites

aripalo commented 6 months ago

I have a feeling the Lambda Function URL OAC protection is going to be very painful to implement with infra-as-code as there seems to be some challenging state / ordering required:

Before you create an OAC or set it up in a CloudFront distribution, make sure the OAC has permission to access the Lambda function URL. Do this after you create a CloudFront distribution, but before you add the OAC to the Lambda function URL in the distribution configuration.

I've been trying to set it up with some CDK hacks but without success so far.

BowlesCR commented 6 months ago
* it will create a OAI and it's fairly difficult to remove from the template since it requires to remove the resource and the policy

I'm doing it without the construct, using the method described in a previous comment

You can see my code here:

* Subsites HAVE to be with Website S3 Buckets configuration or the forward will not be working (in the access log we see `REST.GET.WEBSITE - "GET /?website HTTP/1.1" 404 NoSuchWebsiteConfiguration`

* Subsite request is entirely forwarded, prefix are kept. A Cloudfront lambda is necessary to remove this and have a clean bucket.

I believe both of these are solved with a Cloudfront Function doing URL rewrites... you probably don't need multiple behaviors if they're all in the same bucket (but you definitely do if the subsites are in their own buckets). I don't have a public-facing example of this code, but I am doing this in production at work. Remember that the behaviors see the request after the rewrite is done, so structure the rewriter output and the behaviors accordingly.

aripalo commented 6 months ago

Well coming back to Lambda Function URLs & OAC regarding my https://github.com/aws/aws-cdk/issues/21771#issuecomment-2073282057: I managed to get it working in the end:

Screenshot 2024-04-23 at 23 48 27

For anyone looking a workaround, here's an example setup with Lambda Function URL and CloudFront Origin Access Control (OAC).

[!CAUTION]
Just note that if you define Lambda function and CloudFront distribution in different stacks, you'll most probably end up with a CloudFormation cyclic reference.

For simplicity, here's a single stack solution:

import * as cdk from "aws-cdk-lib";
import { Construct } from "constructs";
import * as lambda from "aws-cdk-lib/aws-lambda";
import * as cloudfront from "aws-cdk-lib/aws-cloudfront";
import * as origins from "aws-cdk-lib/aws-cloudfront-origins";
import * as iam from "aws-cdk-lib/aws-iam";

export class DemoStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    // Just a basic "Hello world" Lambda function
    const fn = new lambda.Function(this, "MyFunction", {
      code: lambda.Code.fromInline(`
def handler(event, context):
  return {
    "statusCode": 200,
    "headers": {
        "Content-Type": "text/plain"
    },
    "body": "Hello from Lambda Function Url!"
  }`),
      runtime: lambda.Runtime.PYTHON_3_12,
      handler: "index.handler",
    });

    // Define the function URL
    const fnUrl = fn.addFunctionUrl({
      authType: lambda.FunctionUrlAuthType.AWS_IAM, // Note the IAM auth type
    });

    // Output the "raw" FN URL so you can verify that it doesn't work without Sigv4/IAM auth
    new cdk.CfnOutput(this, "FunctionUrl", { value: fnUrl.url });

    // Create an Origin Access Control using Cfn construct
    const oac = new cloudfront.CfnOriginAccessControl(this, "OAC", {
      originAccessControlConfig: {
        name: "MyFunctionOAC",
        originAccessControlOriginType: "lambda",
        signingBehavior: "always",
        signingProtocol: "sigv4",
      },
    });

    // Basic CloudFront setup
    const distribution = new cloudfront.Distribution(this, "MyDistribution", {
      priceClass: cloudfront.PriceClass.PRICE_CLASS_100,
      defaultBehavior: {
        origin: new origins.FunctionUrlOrigin(fnUrl),
        cachePolicy: cloudfront.CachePolicy.CACHING_DISABLED,
        viewerProtocolPolicy: cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
      },
    });

    // Get reference to the underlying CloudFormation construct of the distribution
    const cfnDistribution = distribution.node
      .defaultChild as cloudfront.CfnDistribution;

    // Override the OAC ID into the CloudFormation distribution CFN construct
    cfnDistribution.addPropertyOverride(
      "DistributionConfig.Origins.0.OriginAccessControlId",
      oac.getAtt("Id")
    );

    // Grant the CloudFront service principal permission to invoke the Lambda function URL
    fn.grantInvokeUrl(
      new iam.ServicePrincipal("cloudfront.amazonaws.com", {
        conditions: {
          ArnLike: {
            // Note if you do Lambda Function and CloudFront Distribution in different stacks
            // you'll most-likely end up with a circular dependency.
            // Important, don't specify region as CloudFront needs to access the function from all regions
            "aws:SourceArn": `arn:aws:cloudfront::${
              cdk.Stack.of(this).account
            }:distribution/${distribution.distributionId}`,
          },
        },
      })
    );

    // Output the CloudFront URL, this should now work and respond with "Hello from Lambda Function Url!"
    new cdk.CfnOutput(this, "DistributionUrl", {
      value: `https://${distribution.distributionDomainName}`,
    });
  }
}
Darasimi-Ajewole commented 6 months ago

I got it working with this snippet below:

import { Reference } from 'aws-cdk-lib';
import { S3Origin, S3OriginProps } from 'aws-cdk-lib/aws-cloudfront-origins';
// other imports

type S3OriginWithOACPatchProps = S3OriginProps & {
    oacId: Reference;
};

class S3OriginWithOACPatch extends S3Origin {
    private readonly oacId: Reference;

    constructor(bucket: IBucket, props: S3OriginWithOACPatchProps) {
        super(bucket, props);
        this.oacId = props.oacId;
    }

    public bind(scope: Construct, options: OriginBindOptions): OriginBindConfig {
        const originConfig = super.bind(scope, options);

        if (!originConfig.originProperty) throw new Error('originProperty is required');

        return {
            ...originConfig,
            originProperty: {
                ...originConfig.originProperty,
                originAccessControlId: this.oacId.toString(), // Adds OAC to  S3 origin config
                s3OriginConfig: {
                    ...originConfig.originProperty.s3OriginConfig,
                    originAccessIdentity: '', // removes OAI from S3 origin config
                },
            },
        };
    }
}

Usage:

const s3BucketOAC = new CfnOriginAccessControl(this, 's3-bucket-OAC', {
    originAccessControlConfig: {
        name: "s3-bucket-OAC",
        originAccessControlOriginType: 's3',
        signingBehavior: 'always',
        signingProtocol: 'sigv4',
    },
});
const s3BucketOrigin = new S3OriginWithOACPatch(s3Bucket, { originAccessIdentity: cloudfrontOAI });

// Reuse s3BucketOrigin as cloudfront default behaviour or other behaviours

This snippet also works if your S3 origin is not at index 0 like my use case as well as @s-nt-s .

its-felix commented 6 months ago

In addition to the OAC Patch Construct posted by @Darasimi-Ajewole , you can prevent the OAI from being added to your BucketPolicy by doing a de-tour by reconstructing the Bucket-Construct via fromBucketName like shown here:

new S3OriginWithOAC(
  // prevent CF from adding its OriginAccessIdentity to the BucketPolicy since we're using OriginAccessControl (see below)
  Bucket.fromBucketName(this, 'UIResourcesBucketCopy', props.uiResourcesBucket.bucketName),
  { oacId: uiResourcesOAC.getAtt('Id') },
),
cj-christoph-gysin commented 6 months ago

you can prevent the OAI from being added to your BucketPolicy by doing a de-tour by reconstructing the Bucket-Construct via fromBucketName

You could also inherit from OriginBase instead:


import { OriginBase } from 'aws-cdk-lib/aws-cloudfront';

class S3OriginWithOACPatch extends OriginBase {
    ...
    constructor(bucket: IBucket, props: S3OriginWithOACPatchProps) {
      const { bucketRegionalDomainName } = bucket
      super(bucketRegionalDomainName, props);
      ...
adamjkeller commented 5 months ago

Hey y’all,

As part of our goal to improve service coverage with CDK constructs, we are actively working on this feature! While we can’t commit to specific dates, we’re making progress and will keep you updated along the way. Your feedback and support are helping us shape the roadmap, so thank you!

paunovic5ar commented 2 months ago

If anyone here is looking for a Python workaround to get OAC working with the cloudfront.Distribution() construct, then read-on. Thanks to @cornelcroi and @kyleplant for earlier workarounds in Typescript.

...
# create an S3 bucket
    demo_site_bucket = s3.Bucket(
        ...
    )
...

# create an OAC
demo_oac_config = cloudfront.CfnOriginAccessControl.OriginAccessControlConfigProperty(
    name="TTLabDemo-OAC-Config",
    origin_access_control_origin_type="s3",
    signing_behavior="always",
    signing_protocol="sigv4"
)

demo_oac = cloudfront.CfnOriginAccessControl(
    self,
    "TTLabDemo-OAC",
    origin_access_control_config=demo_oac_config
)

# create a distribution
demo_distribution = cloudfront.Distribution(
    self,
    "TTLabDemo-Distribution",
    ...
    default_behavior = cloudfront.BehaviorOptions(
        allowed_methods = cloudfront.AllowedMethods.ALLOW_GET_HEAD_OPTIONS,
        cached_methods = cloudfront.CachedMethods.CACHE_GET_HEAD_OPTIONS,
        cache_policy = cloudfront.CachePolicy.CACHING_OPTIMIZED,
        compress=True,
        edge_lambdas = [
            cloudfront.EdgeLambda(
                event_type=cloudfront.LambdaEdgeEventType.ORIGIN_REQUEST,
                function_version=latest_redirect_lambda_edge_version,
                include_body=True
            )
        ],
        origin_request_policy = cloudfront.OriginRequestPolicy.CORS_S3_ORIGIN,
        response_headers_policy = cloudfront.ResponseHeadersPolicy.CORS_ALLOW_ALL_ORIGINS,
        viewer_protocol_policy = cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
        origin=origins.S3Origin(
            demo_site_bucket
        )
    ),
    default_root_object='index.html',
    price_class = cloudfront.PriceClass.PRICE_CLASS_ALL
)

demo_oac_bucket_statement = iam.PolicyStatement(
    sid='TTAllowS3OACAccess',
    effect=iam.Effect.ALLOW,
    principals=[iam.ServicePrincipal('cloudfront.amazonaws.com')],
    actions=['s3:GetObject', 's3:PutObject'],
    resources=[f"arn:aws:s3:::something23874832473847328/*"],
    conditions={
        "StringEquals": {
            "aws:sourceArn": f"arn:aws:cloudfront::{self.account}:distribution/{demo_distribution.distribution_id}",
        }
    }
)

# add the above S3 bucket policy to the "demo_site_bucket"
demo_site_bucket.add_to_resource_policy(demo_oac_bucket_statement)

## clean-up the OAI reference and associate the OAC with the cloudfront distribution 
# query the site bucket policy as a document
bucket_policy = demo_site_bucket.policy
bucket_policy_document = bucket_policy.document

# remove the CloudFront Origin Access Identity permission from the bucket policy
if isinstance(bucket_policy_document, iam.PolicyDocument):
    bucket_policy_document_json = bucket_policy_document.to_json()
    # create an updated policy without the OAI reference
    bucket_policy_updated_json = {'Version': '2012-10-17', 'Statement': []}
    for statement in bucket_policy_document_json['Statement']:
        if 'CanonicalUser' not in statement['Principal']:
            bucket_policy_updated_json['Statement'].append(statement)

# apply the updated bucket policy to the bucket
bucket_policy_override = demo_site_bucket.node.find_child("Policy").node.default_child
bucket_policy_override.add_override('Properties.PolicyDocument', bucket_policy_updated_json)

# remove the created OAI reference (S3 Origin property) for the distribution
all_distribution_props = demo_distribution.node.find_all()
for child in all_distribution_props:
    if child.node.id == 'S3Origin':
        child.node.try_remove_child('Resource')

# associate the created OAC with the distribution
distribution_props = demo_distribution.node.default_child
distribution_props.add_override('Properties.DistributionConfig.Origins.0.S3OriginConfig.OriginAccessIdentity', '')
distribution_props.add_property_override(
    "DistributionConfig.Origins.0.OriginAccessControlId",
    demo_oac.ref
)

Worked! Thank you!

github-actions[bot] commented 2 months ago

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.