gatsby-uc / gatsby-plugin-s3

Deploy your gatsby site to a S3 bucket.
https://gatsby-plugin-s3.jari.io/
MIT License
210 stars 110 forks source link

Any changes required to prepare for upcoming S3 security changes? #427

Closed dan-lind closed 1 year ago

dan-lind commented 1 year ago

I'm wondering if the mentioned upcoming changes here will break the plugin? https://aws.amazon.com/blogs/aws/heads-up-amazon-s3-security-changes-are-coming-in-april-of-2023/

Specifically this part

If you need public access for a new bucket you can create it as usual and then delete the public access block by calling DeletePublicAccessBlock (you will need s3:PutBucketPublicAccessBlock permission in order to call this function

From my understanding, if the plugin creates a new bucket, public access will be blocked by default even if we configure it to use static website hosting?

YoshiWalsh commented 1 year ago

Looks like it.

If anyone wants to submit a PR to implement DeletePublicAccessBlock after bucket creation I'll review it. You're probably better off creating the bucket yourself in Terraform/CloudFormation anyway though.

dan-lind commented 1 year ago

@JoshuaWalsh I'm wondering if there are any other use cases besides static website hosting that would be impacted? If not I'm thinking it might be enough to call DeletePublicAccessBlock in this code block https://github.com/JoshuaWalsh/gatsby-plugin-s3/blob/master/gatsby-plugin-s3/src/bin.ts#L207

YoshiWalsh commented 1 year ago

I can't think of any others. IMO SWH is the only valid reason to not be using CloudFront OAI / OAC.

dan-lind commented 1 year ago

Created a PR yesterday, let me know what you think @JoshuaWalsh , I'll be happy to make changes if needed. Sounds like we should expect AWS to start rolling out the changes already from April 1st.

microbit-matt-hillsdon commented 1 year ago

I think I hit an issue related to the AWS rollout of these changes today (in eu-west-1).

I tried to upgrade to 0.4.1 to fix, but hit further problems because of the public-read ACL which is the default.

ACLs aren't compatible with new default for ObjectOwnership.

Furthermore, the public-read ACL cannot be set until after the public access block is removed.

To attempt to match the old behaviour I've done this:

diff --git a/gatsby-plugin-s3/src/bin.ts b/gatsby-plugin-s3/src/bin.ts
index 00e8371..b38e027 100644
--- a/gatsby-plugin-s3/src/bin.ts
+++ b/gatsby-plugin-s3/src/bin.ts
@@ -192,9 +192,12 @@ export const deploy = async ({ yes, bucket, userAgent }: DeployArguments = {}) =
         spinner.color = 'yellow';

         if (!exists) {
+            // Can't set ACL this until we've deleted public access
+            const acl = config.acl === null ? undefined : config.acl ?? 'public-read';
             const createParams: S3.Types.CreateBucketRequest = {
                 Bucket: config.bucketName,
-                ACL: config.acl === null ? undefined : config.acl ?? 'public-read',
+                // Match legacy behaviour so we can use ACLs
+                ObjectOwnership: acl ? 'ObjectWriter' : undefined,
             };
             if (config.region) {
                 createParams.CreateBucketConfiguration = {
@@ -208,6 +211,13 @@ export const deploy = async ({ yes, bucket, userAgent }: DeployArguments = {}) =
                 };
                 await s3.deletePublicAccessBlock(publicBlockConfig).promise();
             }
+            if (acl) {
+                const putBucketAclRequest: S3.Types.PutBucketAclRequest = {
+                    Bucket: config.bucketName,
+                    ACL: acl,
+                };
+                await s3.putBucketAcl(putBucketAclRequest).promise();
+            }
         }

         if (config.enableS3StaticWebsiteHosting) {

(Apologies, line numbers might be a little off as I have some other changes that aren't generally useful)

Not sure if there's a way to simplify.

YoshiWalsh commented 1 year ago

@microbit-matt-hillsdon Having ACL enabled by default is a legacy issue, we've kept it that way for backwards compat. It's better to remove the ACL and use bucket policies instead. I recommend setting acl to null in the options. Would that fix the problem you encountered?

dan-lind commented 1 year ago

We got this, I guess it's the same issue as @microbit-matt-hillsdon mentioned [Gatsby]: InvalidBucketAclWithObjectOwnership: Bucket cannot have ACLs set with ObjectOwnership's BucketOwnerEnforced setting

Setting acl: null resolved the issue for us

EDIT: It just resolved the build issue, but now the bucket was still inaccessible.

You also have to set public access bucket policy like so

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": "*",
            "Action": "s3:GetObject",
            "Resource": "arn:aws:s3:::<your-bucker-name>/*"
        }
    ]
}
advissor commented 1 year ago

@dan-lind comment helped to fix the InvalidBucketAclWithObjectOwnership error

You have to rerun the "build" command before the deploy to have the acl:null be taken in account Was not very straight forward

  1. Edit gatsby-config.js to include acl:null

    plugins: [
    {
      resolve: `gatsby-theme-blog`,
      options: {},
    },
    {
      resolve: `gatsby-plugin-s3`,
      options: {
        bucketName: "bucket-name" ,
        acl: null
      }
    },
    ],
    }
  2. Re-run build + deploy npm run build && npm run deploy

You will see 403 is you visit the page in browser

  1. In AWS console > Go to S3 > Open your S3 bucket "bucket-name" > Permissions > Bucket policy Copy ARN and use the template higher
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": "*",
            "Action": "s3:GetObject",
            "Resource": "arn:aws:s3:::<your-bucker-name>/*"
        }
    ]
}