From e26a706dff4fd26c14ca6d4e6f5cc1393647d04d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 1 Sep 2017 12:16:54 -0700 Subject: [PATCH] Ignore reservedBucket checks for net/rpc requests (#4884) All `net/rpc` requests go to `/minio`, so the existing generic handler for reserved bucket check would essentially erroneously send errors leading to distributed setups to wait infinitely. For `net/rpc` requests alone we should skip this check and allow resource bucket names to be from `/minio` . --- cmd/gateway-main.go | 2 +- cmd/generic-handlers.go | 28 +++++++++++++++++++--------- cmd/generic-handlers_test.go | 21 +++++++++++++++++++++ cmd/routers.go | 2 +- cmd/typed-errors.go | 6 +++--- cmd/web-handlers.go | 12 ++++++------ 6 files changed, 51 insertions(+), 20 deletions(-) diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go index 43bdd71e9..cafbaa113 100644 --- a/cmd/gateway-main.go +++ b/cmd/gateway-main.go @@ -351,7 +351,7 @@ func gatewayMain(ctx *cli.Context, backendType gatewayBackend) { // Redirect some pre-defined browser request paths to a static location prefix. setBrowserRedirectHandler, // Validates if incoming request is for restricted buckets. - setPrivateBucketHandler, + setReservedBucketHandler, // Adds cache control for all browser requests. setBrowserCacheControlHandler, // Validates all incoming requests to have a valid date header. diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go index b93892b99..a32872c7b 100644 --- a/cmd/generic-handlers.go +++ b/cmd/generic-handlers.go @@ -156,6 +156,14 @@ func guessIsBrowserReq(req *http.Request) bool { return strings.Contains(req.Header.Get("User-Agent"), "Mozilla") } +// guessIsRPCReq - returns true if the request is for an RPC endpoint. +func guessIsRPCReq(req *http.Request) bool { + if req == nil { + return false + } + return req.Method == http.MethodConnect && req.Proto == "HTTP/1.0" +} + func (h redirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { aType := getRequestAuthType(r) // Re-direct only for JWT and anonymous requests from browser. @@ -202,20 +210,22 @@ func (h cacheControlHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Adds verification for incoming paths. -type minioPrivateBucketHandler struct { +type minioReservedBucketHandler struct { handler http.Handler } -func setPrivateBucketHandler(h http.Handler) http.Handler { - return minioPrivateBucketHandler{h} +func setReservedBucketHandler(h http.Handler) http.Handler { + return minioReservedBucketHandler{h} } -func (h minioPrivateBucketHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // For all non browser requests, reject access to 'minioReservedBucketPath'. - bucketName, _ := urlPath2BucketObjectName(r.URL) - if !guessIsBrowserReq(r) && (isMinioReservedBucket(bucketName) || isMinioMetaBucket(bucketName)) { - writeErrorResponse(w, ErrAllAccessDisabled, r.URL) - return +func (h minioReservedBucketHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if !guessIsRPCReq(r) && !guessIsBrowserReq(r) { + // For all non browser, non RPC requests, reject access to 'minioReservedBucketPath'. + bucketName, _ := urlPath2BucketObjectName(r.URL) + if isMinioReservedBucket(bucketName) || isMinioMetaBucket(bucketName) { + writeErrorResponse(w, ErrAllAccessDisabled, r.URL) + return + } } h.handler.ServeHTTP(w, r) } diff --git a/cmd/generic-handlers_test.go b/cmd/generic-handlers_test.go index f8b98649a..bda87767f 100644 --- a/cmd/generic-handlers_test.go +++ b/cmd/generic-handlers_test.go @@ -69,6 +69,27 @@ func TestRedirectLocation(t *testing.T) { } } +// Tests request guess function for net/rpc requests. +func TestGuessIsRPC(t *testing.T) { + if guessIsRPCReq(nil) { + t.Fatal("Unexpected return for nil request") + } + r := &http.Request{ + Proto: "HTTP/1.0", + Method: http.MethodConnect, + } + if !guessIsRPCReq(r) { + t.Fatal("Test shouldn't fail for a possible net/rpc request.") + } + r = &http.Request{ + Proto: "HTTP/1.1", + Method: http.MethodGet, + } + if guessIsRPCReq(r) { + t.Fatal("Test shouldn't report as net/rpc for a non net/rpc request.") + } +} + // Tests browser request guess function. func TestGuessIsBrowser(t *testing.T) { if guessIsBrowserReq(nil) { diff --git a/cmd/routers.go b/cmd/routers.go index b87ce3241..3326d8591 100644 --- a/cmd/routers.go +++ b/cmd/routers.go @@ -98,7 +98,7 @@ func configureServerHandler(endpoints EndpointList) (http.Handler, error) { // Redirect some pre-defined browser request paths to a static location prefix. setBrowserRedirectHandler, // Validates if incoming request is for restricted buckets. - setPrivateBucketHandler, + setReservedBucketHandler, // Adds cache control for all browser requests. setBrowserCacheControlHandler, // Validates all incoming requests to have a valid date header. diff --git a/cmd/typed-errors.go b/cmd/typed-errors.go index 1c5a65dc7..f34b5cf00 100644 --- a/cmd/typed-errors.go +++ b/cmd/typed-errors.go @@ -51,9 +51,9 @@ var errServerVersionMismatch = errors.New("Server versions do not match") // errServerTimeMismatch - server times are too far apart. var errServerTimeMismatch = errors.New("Server times are too far apart") -// errReservedBucket - bucket name is reserved for Minio, usually -// returned for 'minio', '.minio.sys' -var errReservedBucket = errors.New("All access to this bucket is disabled") +// errInvalidBucketName - bucket name is reserved for Minio, usually +// returned for 'minio', '.minio.sys', buckets with capital letters. +var errInvalidBucketName = errors.New("The specified bucket is not valid") // errInvalidRange - returned when given range value is not valid. var errInvalidRange = errors.New("Invalid range") diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index c107274d8..9ad32b1a2 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -124,9 +124,9 @@ func (web *webAPIHandlers) MakeBucket(r *http.Request, args *MakeBucketArgs, rep return toJSONError(errAuthentication) } - // Check if bucket is a reserved bucket name. - if isMinioMetaBucket(args.BucketName) || isMinioReservedBucket(args.BucketName) { - return toJSONError(errReservedBucket) + // Check if bucket is a reserved bucket name or invalid. + if isReservedOrInvalidBucket(args.BucketName) { + return toJSONError(errInvalidBucketName) } bucketLock := globalNSMutex.NewNSLock(args.BucketName, "") @@ -1048,10 +1048,10 @@ func toWebAPIError(err error) APIError { HTTPStatusCode: http.StatusMethodNotAllowed, Description: err.Error(), } - } else if err == errReservedBucket { + } else if err == errInvalidBucketName { return APIError{ - Code: "AllAccessDisabled", - HTTPStatusCode: http.StatusForbidden, + Code: "InvalidBucketName", + HTTPStatusCode: http.StatusBadRequest, Description: err.Error(), } } else if err == errInvalidArgument {