make SSE request header check comprehensive (#8276)

This commit refactors the SSE header check
by moving it into the `crypto` package, adds
a unit test for it and makes the check comprehensive.

cmd/bucket-handlers.go

@@ -557,8 +557,7 @@ 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 !api.EncryptionEnabled() && hasServerSideEncryptionHeader(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -715,7 +714,11 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
 		return
 	}
 	if objectAPI.IsEncryptionSupported() {
-		if hasServerSideEncryptionHeader(formValues) && !hasSuffix(object, SlashSeparator) { // handle SSE-C and SSE-S3 requests
+		if crypto.IsRequested(formValues) && !hasSuffix(object, SlashSeparator) { // handle SSE requests
+			if crypto.SSECopy.IsRequested(r.Header) {
+				writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidEncryptionParameters), r.URL, guessIsBrowserReq(r))
+				return
+			}
 			var reader io.Reader
 			var key []byte
 			if crypto.SSEC.IsRequested(formValues) {

cmd/crypto/header.go

@@ -83,6 +83,13 @@ func RemoveSensitiveHeaders(h http.Header) {
 	h.Del(SSECopyKey)
 }
 
+// IsRequested returns true if the HTTP headers indicates
+// that any form server-side encryption (SSE-C, SSE-S3 or SSE-KMS)
+// is requested.
+func IsRequested(h http.Header) bool {
+	return S3.IsRequested(h) || SSEC.IsRequested(h) || SSECopy.IsRequested(h) || S3KMS.IsRequested(h)
+}
+
 // S3 represents AWS SSE-S3. It provides functionality to handle
 // SSE-S3 requests.
 var S3 = s3{}

cmd/crypto/header_test.go

@@ -20,6 +20,29 @@ import (
 	"testing"
 )
 
+func TestIsRequested(t *testing.T) {
+	for i, test := range kmsIsRequestedTests {
+		if got := IsRequested(test.Header) && S3KMS.IsRequested(test.Header); got != test.Expected {
+			t.Errorf("SSE-KMS: Test %d: Wanted %v but got %v", i, test.Expected, got)
+		}
+	}
+	for i, test := range s3IsRequestedTests {
+		if got := IsRequested(test.Header) && S3.IsRequested(test.Header); got != test.Expected {
+			t.Errorf("SSE-S3: Test %d: Wanted %v but got %v", i, test.Expected, got)
+		}
+	}
+	for i, test := range ssecIsRequestedTests {
+		if got := IsRequested(test.Header) && SSEC.IsRequested(test.Header); got != test.Expected {
+			t.Errorf("SSE-C: Test %d: Wanted %v but got %v", i, test.Expected, got)
+		}
+	}
+	for i, test := range ssecCopyIsRequestedTests {
+		if got := IsRequested(test.Header) && SSECopy.IsRequested(test.Header); got != test.Expected {
+			t.Errorf("SSE-C: Test %d: Wanted %v but got %v", i, test.Expected, got)
+		}
+	}
+}
+
 var kmsIsRequestedTests = []struct {
 	Header   http.Header
 	Expected bool

cmd/encryption-v1.go

@@ -63,12 +63,6 @@ const (
 
 )
 
-// hasServerSideEncryptionHeader returns true if the given HTTP header
-// contains server-side-encryption.
-func hasServerSideEncryptionHeader(header http.Header) bool {
-	return crypto.S3.IsRequested(header) || crypto.SSEC.IsRequested(header)
-}
-
 // isEncryptedMultipart returns true if the current object is
 // uploaded by the user using multipart mechanism:
 // initiate new multipart, upload part, complete upload

cmd/encryption-v1_test.go

@@ -28,86 +28,6 @@ import (
 	"github.com/minio/sio"
 )
 
-var hasServerSideEncryptionHeaderTests = []struct {
-	headers    map[string]string
-	sseRequest bool
-}{
-	{headers: map[string]string{crypto.SSECAlgorithm: "AES256", crypto.SSECKey: "key", crypto.SSECKeyMD5: "md5"}, sseRequest: true},                             // 0
-	{headers: map[string]string{crypto.SSECAlgorithm: "AES256"}, sseRequest: true},                                                                              // 1
-	{headers: map[string]string{crypto.SSECKey: "key"}, sseRequest: true},                                                                                       // 2
-	{headers: map[string]string{crypto.SSECKeyMD5: "md5"}, sseRequest: true},                                                                                    // 3
-	{headers: map[string]string{}, sseRequest: false},                                                                                                           // 4
-	{headers: map[string]string{crypto.SSECopyAlgorithm + " ": "AES256", " " + crypto.SSECopyKey: "key", crypto.SSECopyKeyMD5 + " ": "md5"}, sseRequest: false}, // 5
-	{headers: map[string]string{crypto.SSECopyAlgorithm: "", crypto.SSECopyKey: "", crypto.SSECopyKeyMD5: ""}, sseRequest: false},                               // 6
-	{headers: map[string]string{crypto.SSEHeader: ""}, sseRequest: true},                                                                                        // 7
-}
-
-func TestHasServerSideEncryptionHeader(t *testing.T) {
-	for i, test := range hasServerSideEncryptionHeaderTests {
-		headers := http.Header{}
-		for k, v := range test.headers {
-			headers.Set(k, v)
-		}
-		if hasServerSideEncryptionHeader(headers) != test.sseRequest {
-			t.Errorf("Test %d: Expected hasServerSideEncryptionHeader to return %v", i, test.sseRequest)
-		}
-	}
-}
-
-var hasSSECopyCustomerHeaderTests = []struct {
-	headers    map[string]string
-	sseRequest bool
-}{
-	{headers: map[string]string{crypto.SSECopyAlgorithm: "AES256", crypto.SSECopyKey: "key", crypto.SSECopyKeyMD5: "md5"}, sseRequest: true},                    // 0
-	{headers: map[string]string{crypto.SSECopyAlgorithm: "AES256"}, sseRequest: true},                                                                           // 1
-	{headers: map[string]string{crypto.SSECopyKey: "key"}, sseRequest: true},                                                                                    // 2
-	{headers: map[string]string{crypto.SSECopyKeyMD5: "md5"}, sseRequest: true},                                                                                 // 3
-	{headers: map[string]string{}, sseRequest: false},                                                                                                           // 4
-	{headers: map[string]string{crypto.SSECopyAlgorithm + " ": "AES256", " " + crypto.SSECopyKey: "key", crypto.SSECopyKeyMD5 + " ": "md5"}, sseRequest: false}, // 5
-	{headers: map[string]string{crypto.SSECopyAlgorithm: "", crypto.SSECopyKey: "", crypto.SSECopyKeyMD5: ""}, sseRequest: true},                                // 6
-	{headers: map[string]string{crypto.SSEHeader: ""}, sseRequest: false},                                                                                       // 7
-
-}
-
-func TestIsSSECopyCustomerRequest(t *testing.T) {
-	for i, test := range hasSSECopyCustomerHeaderTests {
-		headers := http.Header{}
-		for k, v := range test.headers {
-			headers.Set(k, v)
-		}
-		if crypto.SSECopy.IsRequested(headers) != test.sseRequest {
-			t.Errorf("Test %d: Expected crypto.SSECopy.IsRequested to return %v", i, test.sseRequest)
-		}
-	}
-}
-
-var hasSSECustomerHeaderTests = []struct {
-	headers    map[string]string
-	sseRequest bool
-}{
-	{headers: map[string]string{crypto.SSECAlgorithm: "AES256", crypto.SSECKey: "key", crypto.SSECKeyMD5: "md5"}, sseRequest: true},                    // 0
-	{headers: map[string]string{crypto.SSECAlgorithm: "AES256"}, sseRequest: true},                                                                     // 1
-	{headers: map[string]string{crypto.SSECKey: "key"}, sseRequest: true},                                                                              // 2
-	{headers: map[string]string{crypto.SSECKeyMD5: "md5"}, sseRequest: true},                                                                           // 3
-	{headers: map[string]string{}, sseRequest: false},                                                                                                  // 4
-	{headers: map[string]string{crypto.SSECAlgorithm + " ": "AES256", " " + crypto.SSECKey: "key", crypto.SSECKeyMD5 + " ": "md5"}, sseRequest: false}, // 5
-	{headers: map[string]string{crypto.SSECAlgorithm: "", crypto.SSECKey: "", crypto.SSECKeyMD5: ""}, sseRequest: true},                                // 6
-	{headers: map[string]string{crypto.SSEHeader: ""}, sseRequest: false},                                                                              // 7
-
-}
-
-func TestHasSSECustomerHeader(t *testing.T) {
-	for i, test := range hasSSECustomerHeaderTests {
-		headers := http.Header{}
-		for k, v := range test.headers {
-			headers.Set(k, v)
-		}
-		if crypto.SSEC.IsRequested(headers) != test.sseRequest {
-			t.Errorf("Test %d: Expected hasSSECustomerHeader to return %v", i, test.sseRequest)
-		}
-	}
-}
-
 var encryptRequestTests = []struct {
 	header   map[string]string
 	metadata map[string]string

cmd/object-api-utils.go

@@ -354,7 +354,7 @@ func (o ObjectInfo) GetActualSize() int64 {
 // Using compression and encryption together enables room for side channel attacks.
 // Eliminate non-compressible objects by extensions/content-types.
 func isCompressible(header http.Header, object string) bool {
-	if hasServerSideEncryptionHeader(header) || excludeForCompression(header, object) {
+	if crypto.IsRequested(header) || excludeForCompression(header, object) {
 		return false
 	}
 	return true

cmd/object-handlers.go

@@ -92,7 +92,7 @@ func (api objectAPIHandlers) SelectObjectContentHandler(w http.ResponseWriter, r
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r))
 		return
 	}
-	if !api.EncryptionEnabled() && hasServerSideEncryptionHeader(r.Header) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -251,7 +251,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() && hasServerSideEncryptionHeader(r.Header) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -422,7 +422,7 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re
 		writeErrorResponseHeadersOnly(w, errorCodes.ToAPIErr(ErrBadRequest))
 		return
 	}
-	if !api.EncryptionEnabled() && hasServerSideEncryptionHeader(r.Header) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrBadRequest), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -646,7 +646,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported
 		return
 	}
-	if !api.EncryptionEnabled() && (hasServerSideEncryptionHeader(r.Header) || crypto.SSECopy.IsRequested(r.Header)) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -1051,7 +1051,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported
 		return
 	}
-	if !api.EncryptionEnabled() && hasServerSideEncryptionHeader(r.Header) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -1232,7 +1232,11 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 
 	var objectEncryptionKey []byte
 	if objectAPI.IsEncryptionSupported() {
-		if hasServerSideEncryptionHeader(r.Header) && !hasSuffix(object, SlashSeparator) { // handle SSE requests
+		if crypto.IsRequested(r.Header) && !hasSuffix(object, SlashSeparator) { // handle SSE requests
+			if crypto.SSECopy.IsRequested(r.Header) {
+				writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidEncryptionParameters), r.URL, guessIsBrowserReq(r))
+				return
+			}
 			reader, objectEncryptionKey, err = EncryptRequest(hashReader, r, bucket, object, metadata)
 			if err != nil {
 				writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
@@ -1264,7 +1268,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 		if !strings.HasSuffix(objInfo.ETag, "-1") {
 			etag = objInfo.ETag + "-1"
 		}
-	} else if hasServerSideEncryptionHeader(r.Header) {
+	} else if crypto.IsRequested(r.Header) {
 		etag = getDecryptedETag(r.Header, objInfo, false)
 	}
 	w.Header()[xhttp.ETag] = []string{"\"" + etag + "\""}
@@ -1318,7 +1322,7 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported
 		return
 	}
-	if !api.EncryptionEnabled() && hasServerSideEncryptionHeader(r.Header) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -1365,7 +1369,7 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r
 	var encMetadata = map[string]string{}
 
 	if objectAPI.IsEncryptionSupported() {
-		if hasServerSideEncryptionHeader(r.Header) {
+		if crypto.IsRequested(r.Header) {
 			if err = setEncryptionMetadata(r, bucket, object, encMetadata); err != nil {
 				writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
 				return
@@ -1432,7 +1436,7 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported
 		return
 	}
-	if !api.EncryptionEnabled() && (hasServerSideEncryptionHeader(r.Header) || crypto.SSECopy.IsRequested(r.Header)) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r))
 		return
 	}
@@ -1747,7 +1751,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r)) // SSE-KMS is not supported
 		return
 	}
-	if !api.EncryptionEnabled() && hasServerSideEncryptionHeader(r.Header) {
+	if !api.EncryptionEnabled() && crypto.IsRequested(r.Header) {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNotImplemented), r.URL, guessIsBrowserReq(r))
 		return
 	}

cmd/web-handlers.go

@@ -1022,7 +1022,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if objectAPI.IsEncryptionSupported() {
-		if hasServerSideEncryptionHeader(r.Header) && !hasSuffix(object, SlashSeparator) { // handle SSE requests
+		if crypto.IsRequested(r.Header) && !hasSuffix(object, SlashSeparator) { // handle SSE requests
 			rawReader := hashReader
 			var objectEncryptionKey []byte
 			reader, objectEncryptionKey, err = EncryptRequest(hashReader, r, bucket, object, metadata)