Allow x-amz-content-sha256 to be optional for PutObject() (#5340)

x-amz-content-sha256 can be optional for any AWS signature v4
requests, make sure to skip sha256 calculation when payload
checksum is not set.

Here is the overall expected behavior

** Signed request **
- X-Amz-Content-Sha256 is set to 'empty' or some 'value'  or its
  not 'UNSIGNED-PAYLOAD'- use it to validate the incoming payload.
- X-Amz-Content-Sha256 is set to 'UNSIGNED-PAYLOAD' - skip checksum verification
- X-Amz-Content-Sha256 is not set we use emptySHA256

** Presigned request **
- X-Amz-Content-Sha256 is set to 'empty' or some 'value'  or its
  not 'UNSIGNED-PAYLOAD'- use it to validate the incoming payload
- X-Amz-Content-Sha256 is set to 'UNSIGNED-PAYLOAD' - skip checksum verification
- X-Amz-Content-Sha256 is not set we use 'UNSIGNED-PAYLOAD'

Fixes #5339

cmd/auth-handler.go

@@ -23,11 +23,6 @@ import (
 	"strings"
 )
 
-// Verify if the request http Header "x-amz-content-sha256" == "UNSIGNED-PAYLOAD"
-func isRequestUnsignedPayload(r *http.Request) bool {
-	return r.Header.Get("x-amz-content-sha256") == unsignedPayload
-}
-
 // Verify if request has JWT.
 func isRequestJWT(r *http.Request) bool {
 	return strings.HasPrefix(r.Header.Get("Authorization"), jwtAlgorithm)

cmd/auth-handler_test.go

@@ -190,40 +190,6 @@ func TestS3SupportedAuthType(t *testing.T) {
 	}
 }
 
-// TestIsRequestUnsignedPayload - Test validates the Unsigned payload detection logic.
-func TestIsRequestUnsignedPayload(t *testing.T) {
-	testCases := []struct {
-		inputAmzContentHeader string
-		expectedResult        bool
-	}{
-		// Test case - 1.
-		// Test case with "X-Amz-Content-Sha256" header set to empty value.
-		{"", false},
-		// Test case - 2.
-		// Test case with "X-Amz-Content-Sha256" header set to  "UNSIGNED-PAYLOAD"
-		// The payload is flagged as unsigned When "X-Amz-Content-Sha256" header is set to  "UNSIGNED-PAYLOAD".
-		{unsignedPayload, true},
-		// Test case - 3.
-		// set to a random value.
-		{"abcd", false},
-	}
-
-	// creating an input HTTP request.
-	// Only the headers are relevant for this particular test.
-	inputReq, err := http.NewRequest("GET", "http://example.com", nil)
-	if err != nil {
-		t.Fatalf("Error initializing input HTTP request: %v", err)
-	}
-
-	for i, testCase := range testCases {
-		inputReq.Header.Set("X-Amz-Content-Sha256", testCase.inputAmzContentHeader)
-		actualResult := isRequestUnsignedPayload(inputReq)
-		if testCase.expectedResult != actualResult {
-			t.Errorf("Test %d: Expected the result to `%v`, but instead got `%v`", i+1, testCase.expectedResult, actualResult)
-		}
-	}
-}
-
 func TestIsRequestPresignedSignatureV2(t *testing.T) {
 	testCases := []struct {
 		inputQueryKey   string

cmd/object-handlers.go

@@ -585,6 +585,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 			writeErrorResponse(w, s3Err, r.URL)
 			return
 		}
+
 	case authTypePresigned, authTypeSigned:
 		if s3Err = reqSignatureV4Verify(r, globalServerConfig.GetRegion()); s3Err != ErrNone {
 			errorIf(errSignatureMismatch, "%s", dumpRequest(r))

cmd/signature-v4-utils.go

@@ -32,38 +32,53 @@ import (
 // client did not calculate sha256 of the payload.
 const unsignedPayload = "UNSIGNED-PAYLOAD"
 
-// http Header "x-amz-content-sha256" == "UNSIGNED-PAYLOAD" indicates that the
-// client did not calculate sha256 of the payload. Hence we skip calculating sha256.
-// We also skip calculating sha256 for presigned requests without "x-amz-content-sha256"
-// query header.
+// skipContentSha256Cksum returns true if caller needs to skip
+// payload checksum, false if not.
 func skipContentSha256Cksum(r *http.Request) bool {
-	queryContentSha256 := r.URL.Query().Get("X-Amz-Content-Sha256")
-	isRequestPresignedUnsignedPayload := func(r *http.Request) bool {
+	var (
+		v  []string
+		ok bool
+	)
+
 	if isRequestPresignedSignatureV4(r) {
-			return queryContentSha256 == "" || queryContentSha256 == unsignedPayload
+		v, ok = r.URL.Query()["X-Amz-Content-Sha256"]
+	} else {
+		v, ok = r.Header["X-Amz-Content-Sha256"]
 	}
-		return false
-	}
-	return isRequestUnsignedPayload(r) || isRequestPresignedUnsignedPayload(r)
+
+	// If x-amz-content-sha256 is set and the value is not
+	// 'UNSIGNED-PAYLOAD' we should validate the content sha256.
+	return !(ok && v[0] != unsignedPayload)
 }
 
 // Returns SHA256 for calculating canonical-request.
 func getContentSha256Cksum(r *http.Request) string {
+	var (
+		defaultSha256Cksum string
+		v                  []string
+		ok                 bool
+	)
+
 	// For a presigned request we look at the query param for sha256.
 	if isRequestPresignedSignatureV4(r) {
-		presignedCkSum := r.URL.Query().Get("X-Amz-Content-Sha256")
-		if presignedCkSum == "" {
-			// If not set presigned is defaulted to UNSIGNED-PAYLOAD.
-			return unsignedPayload
-		}
-		return presignedCkSum
+		// X-Amz-Content-Sha256, if not set in presigned requests, checksum
+		// will default to 'UNSIGNED-PAYLOAD'.
+		defaultSha256Cksum = unsignedPayload
+		v, ok = r.URL.Query()["X-Amz-Content-Sha256"]
+	} else {
+		// X-Amz-Content-Sha256, if not set in signed requests, checksum
+		// will default to sha256([]byte("")).
+		defaultSha256Cksum = emptySHA256
+		v, ok = r.Header["X-Amz-Content-Sha256"]
 	}
-	contentCkSum := r.Header.Get("X-Amz-Content-Sha256")
-	if contentCkSum == "" {
-		// If not set content checksum is defaulted to sha256([]byte("")).
-		contentCkSum = emptySHA256
+
+	// We found 'X-Amz-Content-Sha256' return the captured value.
+	if ok {
+		return v[0]
 	}
-	return contentCkSum
+
+	// We couldn't find 'X-Amz-Content-Sha256'.
+	return defaultSha256Cksum
 }
 
 // isValidRegion - verify if incoming region value is valid with configured Region.

cmd/signature-v4-utils_test.go

@@ -34,18 +34,37 @@ func TestSkipContentSha256Cksum(t *testing.T) {
 		expectedResult bool
 	}{
 		// Test case - 1.
-		// Test case with "X-Amz-Content-Sha256" header set to empty value.
+		// Test case with "X-Amz-Content-Sha256" header set, but to empty value but we can't skip.
 		{"X-Amz-Content-Sha256", "", "", "", false},
+
 		// Test case - 2.
+		// Test case with "X-Amz-Content-Sha256" not set so we can skip.
+		{"", "", "", "", true},
+
+		// Test case - 3.
 		// Test case with "X-Amz-Content-Sha256" header set to  "UNSIGNED-PAYLOAD"
 		// When "X-Amz-Content-Sha256" header is set to  "UNSIGNED-PAYLOAD", validation of content sha256 has to be skipped.
-		{"X-Amz-Content-Sha256", unsignedPayload, "", "", true},
-		// Test case - 3.
-		// Enabling PreSigned Signature v4.
-		{"", "", "X-Amz-Credential", "", true},
+		{"X-Amz-Content-Sha256", unsignedPayload, "X-Amz-Credential", "", true},
+
 		// Test case - 4.
+		// Enabling PreSigned Signature v4, but X-Amz-Content-Sha256 not set has to be skipped.
+		{"", "", "X-Amz-Credential", "", true},
+
+		// Test case - 5.
+		// Enabling PreSigned Signature v4, but X-Amz-Content-Sha256 set and its not UNSIGNED-PAYLOAD, we shouldn't skip.
+		{"X-Amz-Content-Sha256", "somevalue", "X-Amz-Credential", "", false},
+
+		// Test case - 6.
+		// Test case with "X-Amz-Content-Sha256" header set to  "UNSIGNED-PAYLOAD" and its not presigned, we should skip.
+		{"X-Amz-Content-Sha256", unsignedPayload, "", "", true},
+
+		// Test case - 7.
 		// "X-Amz-Content-Sha256" not set and  PreSigned Signature v4 not enabled, sha256 checksum calculation is not skipped.
 		{"", "", "X-Amz-Credential", "", true},
+
+		// Test case - 8.
+		// "X-Amz-Content-Sha256" has a proper value cannot skip.
+		{"X-Amz-Content-Sha256", "somevalue", "", "", false},
 	}
 
 	for i, testCase := range testCases {
@@ -55,13 +74,17 @@ func TestSkipContentSha256Cksum(t *testing.T) {
 		if err != nil {
 			t.Fatalf("Error initializing input HTTP request: %v", err)
 		}
-		if testCase.inputHeaderKey != "" {
-			inputReq.Header.Set(testCase.inputHeaderKey, testCase.inputHeaderValue)
-		}
 		if testCase.inputQueryKey != "" {
 			q := inputReq.URL.Query()
 			q.Add(testCase.inputQueryKey, testCase.inputQueryValue)
+			if testCase.inputHeaderKey != "" {
+				q.Add(testCase.inputHeaderKey, testCase.inputHeaderValue)
+			}
 			inputReq.URL.RawQuery = q.Encode()
+		} else {
+			if testCase.inputHeaderKey != "" {
+				inputReq.Header.Set(testCase.inputHeaderKey, testCase.inputHeaderValue)
+			}
 		}
 
 		actualResult := skipContentSha256Cksum(inputReq)
@@ -243,8 +266,10 @@ func TestGetContentSha256Cksum(t *testing.T) {
 		expected string // expected SHA256
 	}{
 		{"shastring", "", "shastring"},
+		{emptySHA256, "", emptySHA256},
 		{"", "", emptySHA256},
 		{"", "X-Amz-Credential=random", unsignedPayload},
+		{"", "X-Amz-Credential=random&X-Amz-Content-Sha256=" + unsignedPayload, unsignedPayload},
 		{"", "X-Amz-Credential=random&X-Amz-Content-Sha256=shastring", "shastring"},
 	}