From 80fab03b630943118211fa04a02409da32dd3f4b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 15 Sep 2020 13:57:15 -0700 Subject: [PATCH] fix: S3 gateway doesn't support full passthrough for encryption (#10484) The entire encryption layer is dependent on the fact that KMS should be configured for S3 encryption to work properly and we only support passing the headers as is to the backend for encryption only if KMS is configured. Make sure that this predictability is maintained, currently the code was allowing encryption to go through and fail at later to indicate that KMS was not configured. We should simply reply "NotImplemented" if KMS is not configured, this allows clients to simply proceed with their tests. --- cmd/api-router.go | 12 +------ cmd/bucket-handlers.go | 3 +- cmd/gateway-common.go | 16 ++++++--- cmd/gateway-main.go | 6 +--- cmd/gateway/s3/gateway-s3-sse.go | 2 +- cmd/gateway/s3/gateway-s3.go | 10 +++--- cmd/object-handlers.go | 58 ++++++++++++++++++++------------ cmd/routers.go | 5 ++- cmd/test-utils_test.go | 5 ++- 9 files changed, 61 insertions(+), 56 deletions(-) diff --git a/cmd/api-router.go b/cmd/api-router.go index 00abcf4d0..d8dcd36e6 100644 --- a/cmd/api-router.go +++ b/cmd/api-router.go @@ -61,10 +61,6 @@ func newCachedObjectLayerFn() CacheObjectLayer { type objectAPIHandlers struct { ObjectAPI func() ObjectLayer CacheAPI func() CacheObjectLayer - // Returns true of handlers should interpret encryption. - EncryptionEnabled func() bool - // Returns true if handlers allow SSE-KMS encryption headers. - AllowSSEKMS func() bool } // getHost tries its best to return the request host. @@ -78,17 +74,11 @@ func getHost(r *http.Request) string { } // registerAPIRouter - registers S3 compatible APIs. -func registerAPIRouter(router *mux.Router, encryptionEnabled, allowSSEKMS bool) { +func registerAPIRouter(router *mux.Router) { // Initialize API. api := objectAPIHandlers{ ObjectAPI: newObjectLayerFn, CacheAPI: newCachedObjectLayerFn, - EncryptionEnabled: func() bool { - return encryptionEnabled - }, - AllowSSEKMS: func() bool { - return allowSSEKMS - }, } // API Router diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 5fd77048d..374349c2e 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -659,7 +659,8 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } diff --git a/cmd/gateway-common.go b/cmd/gateway-common.go index 41db53d3e..b8fd38ec8 100644 --- a/cmd/gateway-common.go +++ b/cmd/gateway-common.go @@ -349,16 +349,22 @@ func ComputeCompleteMultipartMD5(parts []CompletePart) string { // parse gateway sse env variable func parseGatewaySSE(s string) (gatewaySSE, error) { l := strings.Split(s, ";") - var gwSlice = make([]string, 0) + var gwSlice gatewaySSE for _, val := range l { v := strings.ToUpper(val) - if v == gatewaySSES3 || v == gatewaySSEC { + switch v { + case "": + continue + case gatewaySSES3: + fallthrough + case gatewaySSEC: gwSlice = append(gwSlice, v) continue + default: + return nil, config.ErrInvalidGWSSEValue(nil).Msg("gateway SSE cannot be (%s) ", v) } - return nil, config.ErrInvalidGWSSEValue(nil).Msg("gateway SSE cannot be (%s) ", v) } - return gatewaySSE(gwSlice), nil + return gwSlice, nil } // handle gateway env vars @@ -372,7 +378,7 @@ func gatewayHandleEnvVars() { } gwsseVal := env.Get("MINIO_GATEWAY_SSE", "") - if len(gwsseVal) != 0 { + if gwsseVal != "" { var err error GlobalGatewaySSE, err = parseGatewaySSE(gwsseVal) if err != nil { diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go index f5863972e..2a98aa5af 100644 --- a/cmd/gateway-main.go +++ b/cmd/gateway-main.go @@ -261,12 +261,8 @@ func StartGateway(ctx *cli.Context, gw Gateway) { logger.FatalIf(registerWebRouter(router), "Unable to configure web browser") } - // Currently only NAS and S3 gateway support encryption headers. - encryptionEnabled := gatewayName == S3BackendGateway || gatewayName == NASBackendGateway - allowSSEKMS := gatewayName == S3BackendGateway // Only S3 can support SSE-KMS (as pass-through) - // Add API router. - registerAPIRouter(router, encryptionEnabled, allowSSEKMS) + registerAPIRouter(router) // Use all the middlewares router.Use(registerMiddlewares) diff --git a/cmd/gateway/s3/gateway-s3-sse.go b/cmd/gateway/s3/gateway-s3-sse.go index 9b373dfe4..a65ab110c 100644 --- a/cmd/gateway/s3/gateway-s3-sse.go +++ b/cmd/gateway/s3/gateway-s3-sse.go @@ -316,7 +316,7 @@ func (l *s3EncObjects) GetObjectNInfo(ctx context.Context, bucket, object string } fn, off, length, err := minio.NewGetObjectReader(rs, objInfo, o) if err != nil { - return nil, minio.ErrorRespToObjectError(err) + return nil, minio.ErrorRespToObjectError(err, bucket, object) } if l.isGWEncrypted(ctx, bucket, object) { object = getGWContentPath(object) diff --git a/cmd/gateway/s3/gateway-s3.go b/cmd/gateway/s3/gateway-s3.go index 86cd1a919..3f3b86b71 100644 --- a/cmd/gateway/s3/gateway-s3.go +++ b/cmd/gateway/s3/gateway-s3.go @@ -391,21 +391,21 @@ func (l *s3Objects) GetObjectNInfo(ctx context.Context, bucket, object string, r return nil, minio.ErrorRespToObjectError(err, bucket, object) } - var startOffset, length int64 - startOffset, length, err = rs.GetOffsetLength(objInfo.Size) + fn, off, length, err := minio.NewGetObjectReader(rs, objInfo, opts) if err != nil { return nil, minio.ErrorRespToObjectError(err, bucket, object) } pr, pw := io.Pipe() go func() { - err := l.GetObject(ctx, bucket, object, startOffset, length, pw, objInfo.ETag, opts) + err := l.GetObject(ctx, bucket, object, off, length, pw, objInfo.ETag, opts) pw.CloseWithError(err) }() + // Setup cleanup function to cause the above go-routine to // exit in case of partial read pipeCloser := func() { pr.Close() } - return minio.NewGetObjectReaderFromReader(pr, objInfo, opts, pipeCloser) + return fn(pr, h, opts.CheckPrecondFn, pipeCloser) } // GetObject reads an object from S3. Supports additional @@ -745,7 +745,7 @@ func (l *s3Objects) IsCompressionSupported() bool { // IsEncryptionSupported returns whether server side encryption is implemented for this layer. func (l *s3Objects) IsEncryptionSupported() bool { - return minio.GlobalKMS != nil || len(minio.GlobalGatewaySSE) > 0 + return minio.GlobalKMS != nil || minio.GlobalGatewaySSE.IsSet() } func (l *s3Objects) IsTaggingSupported() bool { diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 2c33391f2..90137421c 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -99,11 +99,13 @@ func (api objectAPIHandlers) SelectObjectContentHandler(w http.ResponseWriter, r writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) return } - if crypto.S3KMS.IsRequested(r.Header) { // SSE-KMS is not supported - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) + + if crypto.S3.IsRequested(r.Header) || crypto.S3KMS.IsRequested(r.Header) { // If SSE-S3 or SSE-KMS present -> AWS fails with undefined error + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r)) return } @@ -308,7 +310,7 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r)) return } @@ -507,7 +509,7 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re writeErrorResponseHeadersOnly(w, errorCodes.ToAPIErr(ErrBadRequest)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r)) return } @@ -781,11 +783,13 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) return } - if crypto.S3KMS.IsRequested(r.Header) { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported + + if crypto.S3KMS.IsRequested(r.Header) { // SSE-KMS is not supported + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } @@ -1297,14 +1301,17 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) return } - if crypto.S3KMS.IsRequested(r.Header) && !api.AllowSSEKMS() { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported + + if crypto.S3KMS.IsRequested(r.Header) { // SSE-KMS is not supported + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } + vars := mux.Vars(r) bucket := vars["bucket"] object, err := url.PathUnescape(vars["object"]) @@ -1429,7 +1436,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req // Check if bucket encryption is enabled _, err = globalBucketSSEConfigSys.Get(bucket) // This request header needs to be set prior to setting ObjectOptions - if (globalAutoEncryption || err == nil) && !crypto.SSEC.IsRequested(r.Header) && !crypto.S3KMS.IsRequested(r.Header) { + if (globalAutoEncryption || err == nil) && !crypto.SSEC.IsRequested(r.Header) { r.Header.Set(crypto.SSEHeader, crypto.SSEAlgorithmAES256) } @@ -1604,14 +1611,17 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) return } - if crypto.S3KMS.IsRequested(r.Header) && !api.AllowSSEKMS() { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported + + if crypto.S3KMS.IsRequested(r.Header) { // SSE-KMS is not supported + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } + vars := mux.Vars(r) bucket := vars["bucket"] object, err := url.PathUnescape(vars["object"]) @@ -1628,7 +1638,7 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r // Check if bucket encryption is enabled _, err = globalBucketSSEConfigSys.Get(bucket) // This request header needs to be set prior to setting ObjectOptions - if (globalAutoEncryption || err == nil) && !crypto.SSEC.IsRequested(r.Header) && !crypto.S3KMS.IsRequested(r.Header) { + if (globalAutoEncryption || err == nil) && !crypto.SSEC.IsRequested(r.Header) { r.Header.Set(crypto.SSEHeader, crypto.SSEAlgorithmAES256) } @@ -1729,11 +1739,13 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) return } - if crypto.S3KMS.IsRequested(r.Header) { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported + + if crypto.S3KMS.IsRequested(r.Header) { // SSE-KMS is not supported + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } @@ -2043,11 +2055,13 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) return } - if crypto.S3KMS.IsRequested(r.Header) { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported + + if crypto.S3KMS.IsRequested(r.Header) { // SSE-KMS is not supported + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } - if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) { + + if !objectAPI.IsEncryptionSupported() && crypto.IsRequested(r.Header) { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) return } diff --git a/cmd/routers.go b/cmd/routers.go index 64e2977f0..23439bb10 100644 --- a/cmd/routers.go +++ b/cmd/routers.go @@ -108,9 +108,8 @@ func configureServerHandler(endpointZones EndpointZones) (http.Handler, error) { } } - // Add API router, additionally all server mode support encryption - // but don't allow SSE-KMS. - registerAPIRouter(router, true, false) + // Add API router + registerAPIRouter(router) router.Use(registerMiddlewares) diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index b2844fcb3..c44bf2ac4 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -2058,7 +2058,7 @@ func registerBucketLevelFunc(bucket *mux.Router, api objectAPIHandlers, apiFunct func registerAPIFunctions(muxRouter *mux.Router, objLayer ObjectLayer, apiFunctions ...string) { if len(apiFunctions) == 0 { // Register all api endpoints by default. - registerAPIRouter(muxRouter, true, false) + registerAPIRouter(muxRouter) return } // API Router. @@ -2088,7 +2088,6 @@ func registerAPIFunctions(muxRouter *mux.Router, objLayer ObjectLayer, apiFuncti } return nil }, - EncryptionEnabled: func() bool { return true }, } // Register ListBuckets handler. @@ -2109,7 +2108,7 @@ func initTestAPIEndPoints(objLayer ObjectLayer, apiFunctions []string) http.Hand registerAPIFunctions(muxRouter, objLayer, apiFunctions...) return muxRouter } - registerAPIRouter(muxRouter, true, false) + registerAPIRouter(muxRouter) return muxRouter }