alluxio icon indicating copy to clipboard operation
alluxio copied to clipboard

Fix S3 API file mode bits to prevent unauthorized reads

Open ZhuTopher opened this issue 2 years ago • 3 comments

This PR explicitly sets the mode bits for all S3 API gRPC writes instead of relying on implicit parent directory perms.

  • Theoretically writes were supposed to inherit the parent folder's (bucket's) mode bits of 770 but in reality they were committed as 755 for directories and 644 for files.

ZhuTopher avatar Sep 14 '22 15:09 ZhuTopher

  • Theoretically writes were supposed to inherit the parent folder's (bucket's) mode bits of 770 but in reality they were committed as 755 for directories and 644 for files.

This note here means that there is a bug in the way permissions are created in Alluxio master? Or this was only an issue with the S3 proxy?

I am not entirely sure how Alluxio is mapping S3 access policies to file permissions, but looking at this https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-control-overview.html

It seems that each bucket is assigned an access policy, so in Alluxio this translates to having each bucket being private to a given user group? And this would not be modifiable?

I guess then Alluxio does not support changing this access policy, or adding object ACLs as described in the above link?

tcrain avatar Sep 14 '22 22:09 tcrain

Thanks @tcrain :

Theoretically writes were supposed to inherit the parent folder's (bucket's) mode bits of 770 but in reality they were committed as 755 for directories and 644 for files.

I was relying on the Alluxio master to propagate the permissions of the top-level parent folder (i.e: the 770 on the bucket) for all paths underneath it. Instead it set 755 and 644 as I mentioned. I haven't checked if that happens for requests made outside of the S3 API yet.

It seems that each bucket is assigned an access policy, so in Alluxio this translates to having each bucket being private to a given user group? And this would not be modifiable?

I guess then Alluxio does not support changing this access policy, or adding object ACLs as described in the above link?

Yes that's right. At the moment the ownership of a bucket is set on creation and cannot be modified afterwards because we do not support bucket nor object ACLs in the S3 API.

However it appears that the Alluxio Master assigns the "default" group permissions on directories created through the S3 API, so other group members still have access to those paths. I still haven't entirely figured out what implications that has for S3 clients.

ZhuTopher avatar Sep 14 '22 23:09 ZhuTopher

Thanks @tcrain :

Theoretically writes were supposed to inherit the parent folder's (bucket's) mode bits of 770 but in reality they were committed as 755 for directories and 644 for files.

I was relying on the Alluxio master to propagate the permissions of the top-level parent folder (i.e: the 770 on the bucket) for all paths underneath it. Instead it set 755 and 644 as I mentioned. I haven't checked if that happens for requests made outside of the S3 API yet.

Ok this sounds like it could be a bug with master or just maybe with how the default permissions are set on the client requests. Maybe its worth creating an issue to track this just in case it is a bug elsewhere.

It seems that each bucket is assigned an access policy, so in Alluxio this translates to having each bucket being private to a given user group? And this would not be modifiable? I guess then Alluxio does not support changing this access policy, or adding object ACLs as described in the above link?

Yes that's right. At the moment the ownership of a bucket is set on creation and cannot be modified afterwards because we do not support bucket nor object ACLs in the S3 API.

However it appears that the Alluxio Master assigns the "default" group permissions on directories created through the S3 API, so other group members still have access to those paths. I still haven't entirely figured out what implications that has for S3 clients.

Ok, so I guess you are tracking that elsewhere, so its fine with me.

tcrain avatar Sep 15 '22 00:09 tcrain

alluxio-bot, merge this please

yuzhu avatar Sep 28 '22 20:09 yuzhu