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.72k stars 3.94k forks source link

CloudFront config option to opt-out of updating origin bucket policy when passing OAI #6958

Open vineethsoma opened 4 years ago

vineethsoma commented 4 years ago

In the AWS console you can choose option for CloudFront to not update origin s3 bucket policy with read permissions.

The CDK Cloudfront tries to update bucket policy without giving an option to opt-out. Here is the related code. https://github.com/aws/aws-cdk/blob/b3abc681b97a385be83c6efd2fe3e6eb57933e77/packages/%40aws-cdk/aws-cloudfront/lib/web_distribution.ts#L702

if (originConfig.s3OriginSource) {
        // first case for backwards compatibility
        if (originConfig.s3OriginSource.originAccessIdentity) {
          // grant CloudFront OriginAccessIdentity read access to S3 bucket
          originConfig.s3OriginSource.s3BucketSource.grantRead(originConfig.s3OriginSource.originAccessIdentity);

          s3OriginConfig = {
            originAccessIdentity:
              `origin-access-identity/cloudfront/${
              originConfig.s3OriginSource.originAccessIdentity.originAccessIdentityName
            }`
          };
        } else {
          s3OriginConfig = {};
        }
      }

Use Case

We have a use case were we need manage the bucket policy specifically for our automation to work properly.

Proposed Solution

Is it possible to pass a flag on S3OriginConfig interface, something like -

const distibutionConfig: CloudFrontWebDistributionProps = {
            webACLId,
            originConfigs: [
                {

                    s3OriginSource: {
                        s3BucketSource: bucket,
                        originAccessIdentity,
                        updateBucketPolicy: false // true by default
                    },
                    behaviors: [{ isDefaultBehavior: true }]
                }
            ],
            aliasConfiguration
        };

.......
......
# web_distribution.ts

if (originConfig.s3OriginSource) {
        // first case for backwards compatibility
        if (originConfig.s3OriginSource.originAccessIdentity ) {
                if(originConfig.s3OriginSource.updateBucketPolicy) {
                               // grant CloudFront OriginAccessIdentity read access to S3 bucket  
 originConfig.s3OriginSource.s3BucketSource.grantRead(originConfig.s3OriginSource.originAccessIdentity);
          }

......
......

This is a :rocket: Feature Request

vineethsoma commented 4 years ago

@iliapolo Hey, please let me know if I can do anything to get this feature implemented.

iliapolo commented 4 years ago

Hi @vineethsoma

You mentioned:

We have a use case were we need manage the bucket policy specifically for our automation to work properly.

Can you elaborate on this? Is there ever a case where the bucket does not necessarily need to grandRead to the OAI?

vineethsoma commented 4 years ago

Can you elaborate on this? Is there ever a case where the bucket does not necessarily need to grandRead to the OAI?

Hey @iliapolo We have some automation in place that monitors our AWS accounts for specific bucket policy statements. We have to make sure our bucket policy matches those exact permissions so we need to have control over the bucket policy. The read grant permission statements differs from those permission statements throwing off our automation. The automation processes are not in our domain.

The use case is for more control of bucket policy statements.

iliapolo commented 4 years ago

Hi @vineethsoma

Im adding this feature request to our docket. 👍

Would you like to take a stab at implementing this?

vineethsoma commented 4 years ago

Thanks @iliapolo

Yep, let me take a stab at it. I need to setup my local dev first, I will keep you posted if I get stuck anywhere.

Anything I need to look out for? I will be just following the Contributing guide.

iliapolo commented 4 years ago

@vineethsoma Awesome, here to help 👍

maoosi commented 4 years ago

@vineethsoma Just checking if there is still work happening on this?

I'm facing the same issue and would like to opt-out from the default read permissions applied to the S3 bucket policy.

vineethsoma commented 4 years ago

@vineethsoma Just checking if there is still work happening on this?

I'm facing the same issue and would like to opt-out from the default read permissions applied to the S3 bucket policy.

@maoosi hey, we ended up not needing this implementation with a change in business process.

maoosi commented 4 years ago

@vineethsoma thanks for your response. I'll explain my use case in more details so the maintainers can decide what to do about this issue.

When deploying larger feature changes on my web app (single-page application hosted in S3), I sometimes need to serve a temporary maintenance page. Originally inspired by this article, I have set up x2 CloudFront Origin Access Identities:

  1. Default OAI with reading permissions into the bucket and serving my single-page app as normal.
  2. Maintenance OAI with custom reading permissions (/maintenance.html object only). Once enabled, the maintenance OAI takes precedence on the Default OAI and returns a 403, turned into a 503 and serving /maintenance.html via a custom error behaviour. The 503 code also allows my SPA to understand that maintenance mode is turned "on" and redirect users already on the platform accordingly.

In that context, it is critical for the Maintenance OAI to have a custom resource policy applied, instead of the default reading permissions currently applied by the CDK.

So far, what I've just described is the best I could find to easily manage maintenance modes with CloudFront and Single-Page Applications. I have now adopted this pattern across multiple projects and until this issue is resolved, I will have to continue manually update Bucket policies after every single CDK deploy.

@njlynch I am keen to understand if this is something the maintainers' team might consider to implement? Or if there is any sort of workaround I could use to prevent the default reading permissions to apply? Thanks!

maoosi commented 3 years ago

@njlynch any chance you could feedback on this? Thanks.

njlynch commented 3 years ago

@maoosi - A flag, as originally described, could work. I would be willing to accept PRs to that effect.

The other approach I see would be to alter the input into the (CloudFrontWeb)Distribution to prevent the policy from changing. Something like this:

class ImmutablePermissionsBucket extends s3.Bucket {
  public grantRead(identity: iam.IGrantable, _objectsKeyPattern?: any): iam.Grant {
    return iam.Grant.drop(identity, ''); // Silently does nothing.
  }
}
///
          s3OriginSource: {
            s3BucketSource: new ImmutablePermissionsBucket(this, 'Bucket', { /* */ }),
            originAccessIdentity,
          },
maoosi commented 3 years ago

Thanks a lot @njlynch, the suggested approach works perfectly.

lxop commented 3 years ago

Note that the newer Distribution construct does not automatically set the bucket policy, so if switching to that is possible then that might solve the problem.

SupaFuture commented 2 years ago

Note that the newer Distribution construct does not automatically set the bucket policy, so if switching to that is possible then that might solve the problem.

It does in CDK version 2.9.0 in Python.

Alternatively, you can override the Bucket PolicyDocument, with the add_property_override method of the isolated CfnResource

For example, I wanted my OAI to access only 2 specific folders of my S3Origin :


    bucket=s3.Bucket(self, "s3bucket", 
        bucket_name='my-bucket'
    )

    origin_access_identity = cloudfront.OriginAccessIdentity(self, "OAIS3Cloudfront",
        comment="Access S3 from Cloudfront"
    )

    origin=origins.S3Origin(bucket,
        origin_access_identity=origin_access_identity
    )

    distribution=cloudfront.Distribution(self,
        "cloudfront_distribution",
        default_behavior=cloudfront.BehaviorOptions(
            origin=origin,
            allowed_methods=cloudfront.AllowedMethods.ALLOW_GET_HEAD_OPTIONS,
            cached_methods=cloudfront.CachedMethods.CACHE_GET_HEAD_OPTIONS
        )
    )

    new_bucket_policy = iam.PolicyDocument(
        statements=[iam.PolicyStatement(
            actions=["s3:GetObject"],
            resources=[bucket.arn_for_objects("static/*"), bucket.arn_for_objects("media/*")],
            principals=[origin_access_identity.grant_principal]
        )]
    )

    cfn_bucket_policy = bucket.policy.node.find_child("Resource")
    cfn_bucket_policy.add_property_override('PolicyDocument', value=new_bucket_policy)
jhart0 commented 1 year ago

If anyone else stumbles across this, you can use the addOverride method to change any attribute in the default policy e.g. changing the GetObject permission to PutObject:

const policyOverride = bucket.node.findChild("Policy").node
  .defaultChild as CfnBucket;
policyOverride.addOverride(
  "Properties.PolicyDocument.Statement.0.Action",
  "s3:PutObject"
);
philmcmahon commented 1 year ago

Thanks @jhart0 - I took inspiration from this, ended up making it a bit more restrictive:

    const cfnBucketPolicy = uploadBucket.policy?.node
      .defaultChild as CfnBucketPolicy;
    cfnBucketPolicy.addPropertyOverride(
      "PolicyDocument.Statement.0.Action",
      `s3:ReplicateTags`
    );
    cfnBucketPolicy.addPropertyOverride(
      "PolicyDocument.Statement.0.Effect",
      `Deny`
    );