From ab66b2319490136e154d76fa5c0565f335880aa9 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 2 Apr 2020 12:35:22 -0700 Subject: [PATCH] fix: allow listBuckets with listBuckets permission (#9253) --- cmd/auth-handler.go | 8 +++--- cmd/bucket-handlers.go | 57 +++++++++++++++++++++++++----------------- cmd/test-utils_test.go | 12 +++++---- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index a4578af5b..9a163cc52 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -272,7 +272,7 @@ func checkRequestAuthTypeToAccessKey(ctx context.Context, r *http.Request, actio var cred auth.Credentials switch getRequestAuthType(r) { case authTypeUnknown, authTypeStreamingSigned: - return accessKey, owner, ErrAccessDenied + return accessKey, owner, ErrSignatureVersionNotSupported case authTypePresignedV2, authTypeSignedV2: if s3Err = isReqAuthenticatedV2(r); s3Err != ErrNone { return accessKey, owner, s3Err @@ -334,7 +334,7 @@ func checkRequestAuthTypeToAccessKey(ctx context.Context, r *http.Request, actio // Request is allowed return the appropriate access key. return cred.AccessKey, owner, ErrNone } - return accessKey, owner, ErrAccessDenied + return cred.AccessKey, owner, ErrAccessDenied } if globalIAMSys.IsAllowed(iampolicy.Args{ AccountName: cred.AccessKey, @@ -348,7 +348,7 @@ func checkRequestAuthTypeToAccessKey(ctx context.Context, r *http.Request, actio // Request is allowed return the appropriate access key. return cred.AccessKey, owner, ErrNone } - return accessKey, owner, ErrAccessDenied + return cred.AccessKey, owner, ErrAccessDenied } // Verify if request has valid AWS Signature Version '2'. @@ -472,7 +472,7 @@ func isPutActionAllowed(atype authType, bucketName, objectName string, r *http.R var owner bool switch atype { case authTypeUnknown: - return ErrAccessDenied + return ErrSignatureVersionNotSupported case authTypeSignedV2, authTypePresignedV2: cred, owner, s3Err = getReqAccessKeyV2(r) case authTypeStreamingSigned, authTypePresigned, authTypeSigned: diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 33917edd2..17865a4c2 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -266,7 +266,7 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R listBuckets := objectAPI.ListBuckets accessKey, owner, s3Error := checkRequestAuthTypeToAccessKey(ctx, r, policy.ListAllMyBucketsAction, "", "") - if s3Error != ErrNone { + if s3Error != ErrNone && s3Error != ErrAccessDenied { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) return } @@ -295,32 +295,43 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R } } - // Set prefix value for "s3:prefix" policy conditionals. - r.Header.Set("prefix", "") - - // Set delimiter value for "s3:delimiter" policy conditionals. - r.Header.Set("delimiter", SlashSeparator) - - // err will be nil here as we already called this function - // earlier in this request. - claims, _ := getClaimsFromToken(r) - var newBucketsInfo []BucketInfo - for _, bucketInfo := range bucketsInfo { - if globalIAMSys.IsAllowed(iampolicy.Args{ - AccountName: accessKey, - Action: iampolicy.ListBucketAction, - BucketName: bucketInfo.Name, - ConditionValues: getConditionValues(r, "", accessKey, claims), - IsOwner: owner, - ObjectName: "", - Claims: claims, - }) { - newBucketsInfo = append(newBucketsInfo, bucketInfo) + if s3Error == ErrAccessDenied { + // Set prefix value for "s3:prefix" policy conditionals. + r.Header.Set("prefix", "") + + // Set delimiter value for "s3:delimiter" policy conditionals. + r.Header.Set("delimiter", SlashSeparator) + + // err will be nil here as we already called this function + // earlier in this request. + claims, _ := getClaimsFromToken(r) + n := 0 + // Use the following trick to filter in place + // https://github.com/golang/go/wiki/SliceTricks#filter-in-place + for _, bucketInfo := range bucketsInfo { + if globalIAMSys.IsAllowed(iampolicy.Args{ + AccountName: accessKey, + Action: iampolicy.ListBucketAction, + BucketName: bucketInfo.Name, + ConditionValues: getConditionValues(r, "", accessKey, claims), + IsOwner: owner, + ObjectName: "", + Claims: claims, + }) { + bucketsInfo[n] = bucketInfo + n++ + } + } + bucketsInfo = bucketsInfo[:n] + // No buckets can be filtered return access denied error. + if len(bucketsInfo) == 0 { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) + return } } // Generate response. - response := generateListBucketsResponse(newBucketsInfo) + response := generateListBucketsResponse(bucketsInfo) encodedSuccessResponse := encodeResponse(response) // Write response. diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 6200540f4..6813dc37d 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -1737,9 +1737,9 @@ func ExecObjectLayerAPIAnonTest(t *testing.T, obj ObjectLayer, testName, bucketN apiRouter.ServeHTTP(rec, anonReq) // expected error response when the unsigned HTTP request is not permitted. - accesDeniedHTTPStatus := getAPIError(ErrAccessDenied).HTTPStatusCode - if rec.Code != accesDeniedHTTPStatus { - t.Fatal(failTestStr(anonTestStr, fmt.Sprintf("Object API Nil Test expected to fail with %d, but failed with %d", accesDeniedHTTPStatus, rec.Code))) + accessDenied := getAPIError(ErrAccessDenied).HTTPStatusCode + if rec.Code != accessDenied { + t.Fatal(failTestStr(anonTestStr, fmt.Sprintf("Object API Nil Test expected to fail with %d, but failed with %d", accessDenied, rec.Code))) } // HEAD HTTTP request doesn't contain response body. @@ -1826,8 +1826,10 @@ func ExecObjectLayerAPIAnonTest(t *testing.T, obj ObjectLayer, testName, bucketN } } - if rec.Code != accesDeniedHTTPStatus { - t.Fatal(failTestStr(unknownSignTestStr, fmt.Sprintf("Object API Unknow auth test for \"%s\", expected to fail with %d, but failed with %d", testName, accesDeniedHTTPStatus, rec.Code))) + // expected error response when the unsigned HTTP request is not permitted. + unsupportedSignature := getAPIError(ErrSignatureVersionNotSupported).HTTPStatusCode + if rec.Code != unsupportedSignature { + t.Fatal(failTestStr(unknownSignTestStr, fmt.Sprintf("Object API Unknow auth test for \"%s\", expected to fail with %d, but failed with %d", testName, unsupportedSignature, rec.Code))) } }