From 712e82344c2fa32647ddbb6be68603e6942f1e6a Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 16 Feb 2020 11:37:52 +0530 Subject: [PATCH] acl: Support PUT calls with success for 'private' ACL's (#9000) Add dummy calls which respond success when ACL's are set to be private and fails, if user tries to change them from their default 'private' Some applications such as nuxeo may have an unnecessary requirement for this operation, we support this anyways such that don't have to fully implement the functionality just that we can respond with success for default ACLs --- cmd/acl-handlers.go | 134 +++++++++++++++++++++++++++++- cmd/api-errors.go | 13 +++ cmd/api-router.go | 4 + cmd/auth-handler.go | 9 +- cmd/bucket-encryption-handlers.go | 13 +-- cmd/bucket-handlers.go | 2 +- cmd/bucket-lifecycle-handler.go | 13 +-- cmd/generic-handlers.go | 30 +++---- cmd/http/headers.go | 3 + cmd/object-handlers.go | 3 + cmd/server_test.go | 10 --- cmd/utils.go | 4 +- 12 files changed, 193 insertions(+), 45 deletions(-) diff --git a/cmd/acl-handlers.go b/cmd/acl-handlers.go index 9c21f1855..93a539c64 100644 --- a/cmd/acl-handlers.go +++ b/cmd/acl-handlers.go @@ -1,5 +1,5 @@ /* - * MinIO Cloud Storage, (C) 2018 MinIO, Inc. + * MinIO Cloud Storage, (C) 2018-2020 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,10 +18,12 @@ package cmd import ( "encoding/xml" + "io" "net/http" "net/url" "github.com/gorilla/mux" + xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/bucket/policy" ) @@ -51,6 +53,71 @@ type accessControlPolicy struct { } `xml:"AccessControlList"` } +// PutBucketACLHandler - PUT Bucket ACL +// ----------------- +// This operation uses the ACL subresource +// to set ACL for a bucket, this is a dummy call +// only responds success if the ACL is private. +func (api objectAPIHandlers) PutBucketACLHandler(w http.ResponseWriter, r *http.Request) { + ctx := newContext(r, w, "PutBucketACL") + + defer logger.AuditLog(w, r, "PutBucketACL", mustGetClaimsFromToken(r)) + + vars := mux.Vars(r) + bucket := vars["bucket"] + + objAPI := api.ObjectAPI() + if objAPI == nil { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) + return + } + + // Allow putBucketACL if policy action is set, since this is a dummy call + // we are simply re-purposing the bucketPolicyAction. + if s3Error := checkRequestAuthType(ctx, r, policy.PutBucketPolicyAction, bucket, ""); s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) + return + } + + // Before proceeding validate if bucket exists. + _, err := objAPI.GetBucketInfo(ctx, bucket) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } + + aclHeader := r.Header.Get(xhttp.AmzACL) + if aclHeader == "" { + acl := &accessControlPolicy{} + if err = xmlDecoder(r.Body, acl, r.ContentLength); err != nil { + if err == io.EOF { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingSecurityHeader), + r.URL, guessIsBrowserReq(r)) + return + } + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } + + if len(acl.AccessControlList.Grants) == 0 { + writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidArgument), r.URL, guessIsBrowserReq(r)) + return + } + + if acl.AccessControlList.Grants[0].Permission != "FULL_CONTROL" { + writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidArgument), r.URL, guessIsBrowserReq(r)) + return + } + } + + if aclHeader != "" && aclHeader != "private" { + writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidArgument), r.URL, guessIsBrowserReq(r)) + return + } + + w.(http.Flusher).Flush() +} + // GetBucketACLHandler - GET Bucket ACL // ----------------- // This operation uses the ACL @@ -100,6 +167,71 @@ func (api objectAPIHandlers) GetBucketACLHandler(w http.ResponseWriter, r *http. w.(http.Flusher).Flush() } +// PutObjectACLHandler - PUT Object ACL +// ----------------- +// This operation uses the ACL subresource +// to set ACL for a bucket, this is a dummy call +// only responds success if the ACL is private. +func (api objectAPIHandlers) PutObjectACLHandler(w http.ResponseWriter, r *http.Request) { + ctx := newContext(r, w, "PutObjectACL") + + defer logger.AuditLog(w, r, "PutObjectACL", mustGetClaimsFromToken(r)) + + vars := mux.Vars(r) + bucket := vars["bucket"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } + + objAPI := api.ObjectAPI() + if objAPI == nil { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) + return + } + + // Allow putObjectACL if policy action is set, since this is a dummy call + // we are simply re-purposing the bucketPolicyAction. + if s3Error := checkRequestAuthType(ctx, r, policy.PutBucketPolicyAction, bucket, ""); s3Error != ErrNone { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) + return + } + + // Before proceeding validate if object exists. + _, err = objAPI.GetObjectInfo(ctx, bucket, object, ObjectOptions{}) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } + + aclHeader := r.Header.Get(xhttp.AmzACL) + if aclHeader == "" { + acl := &accessControlPolicy{} + if err = xmlDecoder(r.Body, acl, r.ContentLength); err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } + + if len(acl.AccessControlList.Grants) == 0 { + writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidArgument), r.URL, guessIsBrowserReq(r)) + return + } + + if acl.AccessControlList.Grants[0].Permission != "FULL_CONTROL" { + writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidArgument), r.URL, guessIsBrowserReq(r)) + return + } + } + + if aclHeader != "" && aclHeader != "private" { + writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidArgument), r.URL, guessIsBrowserReq(r)) + return + } + + w.(http.Flusher).Flush() +} + // GetObjectACLHandler - GET Object ACL // ----------------- // This operation uses the ACL diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 9f6bce267..14f3b2876 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -95,6 +95,7 @@ const ( ErrMissingContentLength ErrMissingContentMD5 ErrMissingRequestBodyError + ErrMissingSecurityHeader ErrNoSuchBucket ErrNoSuchBucketPolicy ErrNoSuchBucketLifecycle @@ -471,6 +472,11 @@ var errorCodes = errorCodeMap{ Description: "Missing required header for this request: Content-Md5.", HTTPStatusCode: http.StatusBadRequest, }, + ErrMissingSecurityHeader: { + Code: "MissingSecurityHeader", + Description: "Your request was missing a required header", + HTTPStatusCode: http.StatusBadRequest, + }, ErrMissingRequestBodyError: { Code: "MissingRequestBodyError", Description: "Request body is empty.", @@ -1803,6 +1809,13 @@ func toAPIError(ctx context.Context, err error) APIError { // their internal error types. This code is only // useful with gateway implementations. switch e := err.(type) { + case *xml.SyntaxError: + apiErr = APIError{ + Code: "MalformedXML", + Description: fmt.Sprintf("%s (%s)", errorCodes[ErrMalformedXML].Description, + e.Error()), + HTTPStatusCode: errorCodes[ErrMalformedXML].HTTPStatusCode, + } case url.EscapeError: apiErr = APIError{ Code: "XMinioInvalidObjectName", diff --git a/cmd/api-router.go b/cmd/api-router.go index a8e6bf703..a74bd6a65 100644 --- a/cmd/api-router.go +++ b/cmd/api-router.go @@ -105,6 +105,8 @@ func registerAPIRouter(router *mux.Router, encryptionEnabled, allowSSEKMS bool) bucket.Methods(http.MethodDelete).Path("/{object:.+}").HandlerFunc(collectAPIStats("abortmultipartupload", httpTraceAll(api.AbortMultipartUploadHandler))).Queries("uploadId", "{uploadId:.*}") // GetObjectACL - this is a dummy call. bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(collectAPIStats("getobjectacl", httpTraceHdrs(api.GetObjectACLHandler))).Queries("acl", "") + // PutObjectACL - this is a dummy call. + bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(collectAPIStats("putobjectacl", httpTraceHdrs(api.PutObjectACLHandler))).Queries("acl", "") // GetObjectTagging bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(collectAPIStats("getobjecttagging", httpTraceHdrs(api.GetObjectTaggingHandler))).Queries("tagging", "") // PutObjectTagging @@ -144,6 +146,8 @@ func registerAPIRouter(router *mux.Router, encryptionEnabled, allowSSEKMS bool) // Dummy Bucket Calls // GetBucketACL -- this is a dummy call. bucket.Methods(http.MethodGet).HandlerFunc(collectAPIStats("getbucketacl", httpTraceAll(api.GetBucketACLHandler))).Queries("acl", "") + // PutBucketACL -- this is a dummy call. + bucket.Methods(http.MethodPut).HandlerFunc(collectAPIStats("putbucketacl", httpTraceAll(api.PutBucketACLHandler))).Queries("acl", "") // GetBucketCors - this is a dummy call. bucket.Methods(http.MethodGet).HandlerFunc(collectAPIStats("getbucketcors", httpTraceAll(api.GetBucketCorsHandler))).Queries("cors", "") // GetBucketWebsiteHandler - this is a dummy call. diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index db9f2d126..ee55d90c6 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -380,12 +380,11 @@ func isReqAuthenticated(ctx context.Context, r *http.Request, region string, sty err error contentMD5, contentSHA256 []byte ) + // Extract 'Content-Md5' if present. - if _, ok := r.Header[xhttp.ContentMD5]; ok { - contentMD5, err = base64.StdEncoding.Strict().DecodeString(r.Header.Get(xhttp.ContentMD5)) - if err != nil || len(contentMD5) == 0 { - return ErrInvalidDigest - } + contentMD5, err = checkValidMD5(r.Header) + if err != nil { + return ErrInvalidDigest } // Extract either 'X-Amz-Content-Sha256' header or 'X-Amz-Content-Sha256' query parameter (if V4 presigned) diff --git a/cmd/bucket-encryption-handlers.go b/cmd/bucket-encryption-handlers.go index eed04bdef..e4f9ae488 100644 --- a/cmd/bucket-encryption-handlers.go +++ b/cmd/bucket-encryption-handlers.go @@ -22,6 +22,7 @@ import ( "net/http" "github.com/gorilla/mux" + xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" bucketsse "github.com/minio/minio/pkg/bucket/encryption" "github.com/minio/minio/pkg/bucket/policy" @@ -48,6 +49,12 @@ func (api objectAPIHandlers) PutBucketEncryptionHandler(w http.ResponseWriter, r vars := mux.Vars(r) bucket := vars["bucket"] + // PutBucketEncyrption API requires Content-Md5 + if _, ok := r.Header[xhttp.ContentMD5]; !ok { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingContentMD5), r.URL, guessIsBrowserReq(r)) + return + } + if s3Error := checkRequestAuthType(ctx, r, policy.PutBucketEncryptionAction, bucket, ""); s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) return @@ -59,12 +66,6 @@ func (api objectAPIHandlers) PutBucketEncryptionHandler(w http.ResponseWriter, r return } - // PutBucketEncyrption API requires Content-Md5 - if _, ok := r.Header["Content-Md5"]; !ok { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingContentMD5), r.URL, guessIsBrowserReq(r)) - return - } - // Parse bucket encryption xml encConfig, err := validateBucketSSEConfig(io.LimitReader(r.Body, maxBucketSSEConfigSize)) if err != nil { diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index f6777aae1..10bb62c54 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -351,7 +351,7 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, // Content-Md5 is requied should be set // http://docs.aws.amazon.com/AmazonS3/latest/API/multiobjectdeleteapi.html - if _, ok := r.Header["Content-Md5"]; !ok { + if _, ok := r.Header[xhttp.ContentMD5]; !ok { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingContentMD5), r.URL, guessIsBrowserReq(r)) return } diff --git a/cmd/bucket-lifecycle-handler.go b/cmd/bucket-lifecycle-handler.go index 7161baf9f..10ea678ae 100644 --- a/cmd/bucket-lifecycle-handler.go +++ b/cmd/bucket-lifecycle-handler.go @@ -22,6 +22,7 @@ import ( "net/http" "github.com/gorilla/mux" + xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/bucket/lifecycle" "github.com/minio/minio/pkg/bucket/policy" @@ -48,6 +49,12 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r vars := mux.Vars(r) bucket := vars["bucket"] + // PutBucketLifecycle always needs a Content-Md5 + if _, ok := r.Header[xhttp.ContentMD5]; !ok { + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingContentMD5), r.URL, guessIsBrowserReq(r)) + return + } + if s3Error := checkRequestAuthType(ctx, r, policy.PutBucketLifecycleAction, bucket, ""); s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) return @@ -59,12 +66,6 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r return } - // PutBucketLifecycle always needs a Content-Md5 - if _, ok := r.Header["Content-Md5"]; !ok { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingContentMD5), r.URL, guessIsBrowserReq(r)) - return - } - bucketLifecycle, err := lifecycle.ParseLifecycleConfig(io.LimitReader(r.Body, r.ContentLength)) if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go index 2d367489b..a0bab8026 100644 --- a/cmd/generic-handlers.go +++ b/cmd/generic-handlers.go @@ -436,12 +436,16 @@ func setIgnoreResourcesHandler(h http.Handler) http.Handler { // Checks requests for not implemented Bucket resources func ignoreNotImplementedBucketResources(req *http.Request) bool { for name := range req.URL.Query() { - // Enable GetBucketACL, GetBucketCors, GetBucketWebsite, - // GetBucketAcccelerate, GetBucketRequestPayment, - // GetBucketLogging, GetBucketLifecycle, - // GetBucketReplication, GetBucketTagging, - // GetBucketVersioning, DeleteBucketTagging, - // and DeleteBucketWebsite dummy calls specifically. + // Enable PutBucketACL, GetBucketACL, GetBucketCors, + // GetBucketWebsite, GetBucketAcccelerate, + // GetBucketRequestPayment, GetBucketLogging, + // GetBucketLifecycle, GetBucketReplication, + // GetBucketTagging, GetBucketVersioning, + // DeleteBucketTagging, and DeleteBucketWebsite + // dummy calls specifically. + if name == "acl" && req.Method == http.MethodPut { + return false + } if ((name == "acl" || name == "cors" || name == "website" || @@ -457,7 +461,7 @@ func ignoreNotImplementedBucketResources(req *http.Request) bool { return false } - if notimplementedBucketResourceNames[name] { + if notImplementedBucketResourceNames[name] { return true } } @@ -467,11 +471,11 @@ func ignoreNotImplementedBucketResources(req *http.Request) bool { // Checks requests for not implemented Object resources func ignoreNotImplementedObjectResources(req *http.Request) bool { for name := range req.URL.Query() { - // Enable GetObjectACL dummy call specifically. - if name == "acl" && req.Method == http.MethodGet { + // Enable Get/PutObjectACL dummy call specifically. + if name == "acl" && (req.Method == http.MethodGet || req.Method == http.MethodPut) { return false } - if notimplementedObjectResourceNames[name] { + if notImplementedObjectResourceNames[name] { return true } } @@ -479,9 +483,8 @@ func ignoreNotImplementedObjectResources(req *http.Request) bool { } // List of not implemented bucket queries -var notimplementedBucketResourceNames = map[string]bool{ +var notImplementedBucketResourceNames = map[string]bool{ "accelerate": true, - "acl": true, "cors": true, "inventory": true, "logging": true, @@ -494,8 +497,7 @@ var notimplementedBucketResourceNames = map[string]bool{ } // List of not implemented object queries -var notimplementedObjectResourceNames = map[string]bool{ - "acl": true, +var notImplementedObjectResourceNames = map[string]bool{ "restore": true, "torrent": true, } diff --git a/cmd/http/headers.go b/cmd/http/headers.go index 704d153ad..ac24feb93 100644 --- a/cmd/http/headers.go +++ b/cmd/http/headers.go @@ -77,6 +77,9 @@ const ( AmzObjectLockLegalHold = "X-Amz-Object-Lock-Legal-Hold" AmzObjectLockBypassGovernance = "X-Amz-Bypass-Governance-Retention" + // Dummy putBucketACL + AmzACL = "x-amz-acl" + // Signature V4 related contants. AmzContentSha256 = "X-Amz-Content-Sha256" AmzDate = "X-Amz-Date" diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 5900a049f..b5652b312 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1181,6 +1181,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidDigest), r.URL, guessIsBrowserReq(r)) return } + /// if Content-Length is unknown/missing, deny the request size := r.ContentLength rAuthType := getRequestAuthType(r) @@ -2610,6 +2611,7 @@ func (api objectAPIHandlers) PutObjectLegalHoldHandler(w http.ResponseWriter, r writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidDigest), r.URL, guessIsBrowserReq(r)) return } + if _, ok := globalBucketObjectLockConfig.Get(bucket); !ok { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidBucketObjectLockConfiguration), r.URL, guessIsBrowserReq(r)) return @@ -2772,6 +2774,7 @@ func (api objectAPIHandlers) PutObjectRetentionHandler(w http.ResponseWriter, r writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidDigest), r.URL, guessIsBrowserReq(r)) return } + if _, ok := globalBucketObjectLockConfig.Get(bucket); !ok { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidBucketObjectLockConfiguration), r.URL, guessIsBrowserReq(r)) return diff --git a/cmd/server_test.go b/cmd/server_test.go index 2f1a861e1..897c7501b 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -1803,16 +1803,6 @@ func (s *TestSuiteCommon) TestPutBucketErrors(c *check) { c.Assert(err, nil) verifyError(c, response, "BucketAlreadyOwnedByYou", "Your previous request to create the named bucket succeeded and you already own it.", http.StatusConflict) - - // request for ACL. - // Since MinIO server doesn't support ACL's the request is expected to fail with "NotImplemented" error message. - request, err = newTestSignedRequest("PUT", s.endPoint+SlashSeparator+bucketName+"?acl", - 0, nil, s.accessKey, s.secretKey, s.signer) - c.Assert(err, nil) - - response, err = client.Do(request) - c.Assert(err, nil) - verifyError(c, response, "NotImplemented", "A header you provided implies functionality that is not implemented", http.StatusNotImplemented) } func (s *TestSuiteCommon) TestGetObjectLarge10MiB(c *check) { diff --git a/cmd/utils.go b/cmd/utils.go index f598466f8..913ab86af 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -117,12 +117,12 @@ func xmlDecoder(body io.Reader, v interface{}, size int64) error { // checkValidMD5 - verify if valid md5, returns md5 in bytes. func checkValidMD5(h http.Header) ([]byte, error) { - md5B64, ok := h["Content-Md5"] + md5B64, ok := h[xhttp.ContentMD5] if ok { if md5B64[0] == "" { return nil, fmt.Errorf("Content-Md5 header set to empty value") } - return base64.StdEncoding.DecodeString(md5B64[0]) + return base64.StdEncoding.Strict().DecodeString(md5B64[0]) } return []byte{}, nil }