leo-project / leofs

The LeoFS Storage System
https://leo-project.net/leofs/
Apache License 2.0
1.55k stars 155 forks source link

One user can list bucket belonging to other user #269

Closed trociny closed 9 years ago

trociny commented 9 years ago

A request to get the list of files for a bucket belonging to other user succeeds.

It looks like unexpected behavior. I would expect the request to fail with Access Denied error.

The suspicious code is leo_s3_auth:authenticate/3 function from leo_s3_libs. It has two clauses:

authenticate(Authorization, #sign_params{raw_uri = <<"/">>} = SignParams, _IsCreateBucketOp)

and

authenticate(Authorization, #sign_params{bucket = Bucket} = SignParams, IsCreateBucketOp)

The first one calls directly authenticate_1/1, while the second one checks the bucket access before calling authenticate_1/1.

When getting a bucket using uri like http://bucket.endpoind/, the first clause is matched due to raw_uri = <<"/">>, as a result the bucket access is not checked.

Changing the pattern match from raw_uri = <<"/">> to bucket = <<>>, like in the diff below gives me the expected behavior.

diff --git a/src/leo_s3_auth.erl b/src/leo_s3_auth.erl
index 71d21d8..8882cd2 100644
--- a/src/leo_s3_auth.erl
+++ b/src/leo_s3_auth.erl
@@ -240,7 +240,7 @@ has_credential(MasterNodes, AccessKey) ->
              {ok, binary()} | {error, any()} when Authorization::binary(),
                                                   SignParams::#sign_params{},
                                                   IsCreateBucketOp::boolean()).
-authenticate(Authorization, #sign_params{raw_uri = <<"/">>} = SignParams, _IsCreateBucketOp) ->
+authenticate(Authorization, #sign_params{bucket = <<>>} = SignParams, _IsCreateBucketOp) ->
     [AccWithAWS,Signature|_] = binary:split(Authorization, <<":">>),
     <<"AWS ", AccessKeyId/binary>> = AccWithAWS,
     authenticate_1(#auth_params{access_key_id = AccessKeyId,
yosukehara commented 9 years ago

Thank you for your report. We'll check this issue.

mocchira commented 9 years ago

@trociny you are right. your code with tiny modifications for unit test have fixed this issue (https://github.com/leo-project/leo_s3_libs/commit/ba144a34a75b749fc58153909741996783de0d36). This fix will be included in the next release. Thank you for your contribution.