From 7350543f2422239bf4d6a595accbddf73780cf63 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 8 Jan 2018 23:19:50 -0800 Subject: [PATCH] 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 | 5 --- cmd/auth-handler_test.go | 34 -------------------- cmd/object-handlers.go | 1 + cmd/signature-v4-utils.go | 59 +++++++++++++++++++++------------- cmd/signature-v4-utils_test.go | 41 ++++++++++++++++++----- 5 files changed, 71 insertions(+), 69 deletions(-) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 52db08918..6d2ed5ff7 100644 --- a/cmd/auth-handler.go +++ b/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) diff --git a/cmd/auth-handler_test.go b/cmd/auth-handler_test.go index d9d7babfb..be7118fcb 100644 --- a/cmd/auth-handler_test.go +++ b/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 diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 5b78d7235..ad10fe447 100644 --- a/cmd/object-handlers.go +++ b/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)) diff --git a/cmd/signature-v4-utils.go b/cmd/signature-v4-utils.go index d4c659cdc..b8ca0a5d0 100644 --- a/cmd/signature-v4-utils.go +++ b/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 { - if isRequestPresignedSignatureV4(r) { - return queryContentSha256 == "" || queryContentSha256 == unsignedPayload - } - return false + var ( + v []string + ok bool + ) + + if isRequestPresignedSignatureV4(r) { + v, ok = r.URL.Query()["X-Amz-Content-Sha256"] + } else { + v, ok = r.Header["X-Amz-Content-Sha256"] } - 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. diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go index 07914eedb..e10188b0e 100644 --- a/cmd/signature-v4-utils_test.go +++ b/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"}, }