search

dandi / dandi-infrastructure

A repository to collect docs/issues on DANDI project infrastructure
Apache License 2.0
0 stars 6 forks source link

Allow Get, List, and Delete without ACL #110

Closed dchiquito closed 2 years ago

dchiquito commented 2 years ago

The header setting X-Amz-ACL=bucket-owner-full-control is only meaningful on "Put" operations. We were requiring it for all operations, which meant that read operations were not allowed, as they did not include it.

Fixes https://github.com/dandi/dandi-archive/issues/809

mvandenburgh commented 2 years ago

It seems like AWS's example configuration for this explicitly enforces bucket-owner-full-control for s3:PutObject actions, instead of enforcing it for all actions by default and then making an exception for get, list, and delete. Is there a reason to not do it that way?

dchiquito commented 2 years ago

@mvandenburgh there is a laundry list of possible S3 operations. A smattering of those operations involve creating objects in the bucket (like s3:CompleteMultipartUpload), and we want all of those to require the bucket-owner-full-control header. There is another smattering of operations that do not create objects in the bucket that do not match s3:Get, s3:List*, or s3:Delete*, but empirically we don't seem to use those (like s3:AbortMultipartUpload for one).

This policy will implicitly allow any object creation operation with the ACL header, and will also allow Get, List, and Delete without the ACL header, which seems like all the operations we actually use in practice. We could explicitly only require the ACL header on the creation operations we use, but I think we would still need the other policy section to grant no-header access.

I'm sure there are alternative ways to write this that will match all S3 actions more precisely (perhaps Effect = Disallow?), but I don't know how they work and I would have to appeal to a higher authority to review that. In the interests of time I think this is good enough.