ks888 / LambStatus

[Maintenance mode] Serverless Status Page System
https://lambstatus.github.io
Apache License 2.0
1.3k stars 119 forks source link

Close down buckets #119

Closed iconara closed 6 years ago

iconara commented 6 years ago

This fixes #115

Replace the permissive bucket policies with object ACLs.

Instead of having bucket policies that permit reading all objects in the buckets, upload the objects with ACLs that permit reading.

The primary motivation is that most security tools will complain loudly when you have permissive bucket policies. Even S3's console points out buckets with policies that allow anyone to read the contents.

By instead using object ACLs the intention is made more clear that these objects are in fact public – and it adds the possibility of uploading non-public objects, should that need arise in the future.

An alternative to this would be to use an origin access identity to restrict the contents of the buckets to CloudFront, but with the object ACL method the S3 website hosting still works, which is nice.

iconara commented 6 years ago

I have not yet attempted to set up an installation from this version, I will do that as soon as I have figured out how to.

ks888 commented 6 years ago

Thank you for the awesome PR. The object ACLs sounds the better option than the origin access identity, as it's better to hide some S3 objects (like non-published metrics data) from users of status page.

I will try out this PR tomorrow.

ks888 commented 6 years ago

One problem I noticed is that simply removing the bucket policy makes the existing s3 objects inaccessible.

iconara commented 6 years ago

Yes, that is a good point. Have you had some kind of update strategy for previous breaking changes?

I haven't figured out how to test this myself actually, I came as far as realising that to test it I would have to build release zips and get the update to load my zips instead of your zips and I ran out of time, unfortunately.

ks888 commented 6 years ago

I've avoided breaking changes as possible so far. In this issue's case, it's possible to write a migration code to add 'public-read' ACL into every existing S3 objects, but the situation will be very complicated if the migration code fails. Now I think an origin access identity looks simpler and safer option.

This doc may be helpful for testing. Basically you can directly update the lambda functions with the npm run deploy command.

ks888 commented 6 years ago

I've merged this PR which fixes the issue with Origin Access Identity approach. So let me close this PR. Thank you again for your contribution.

iconara commented 6 years ago

As long as it resolves the issue I'm happy. Thank you for taking your time with this.