signature-v4: Use sha256("") for calculating canonical request (#4064)

cmd/auth-handler.go

@@ -142,19 +142,16 @@ func isReqAuthenticatedV2(r *http.Request) (s3Error APIErrorCode) {
 	return doesPresignV2SignatureMatch(r)
 }
 
-func reqSignatureV4Verify(r *http.Request) (s3Error APIErrorCode) {
-	sha256sum := r.Header.Get("X-Amz-Content-Sha256")
-	// Skips calculating sha256 on the payload on server,
-	// if client requested for it.
-	if skipContentSha256Cksum(r) {
-		sha256sum = unsignedPayload
-	}
-	if isRequestSignatureV4(r) {
-		return doesSignatureMatch(sha256sum, r, serverConfig.GetRegion())
-	} else if isRequestPresignedSignatureV4(r) {
-		return doesPresignedSignatureMatch(sha256sum, r, serverConfig.GetRegion())
-	}
+func reqSignatureV4Verify(r *http.Request, region string) (s3Error APIErrorCode) {
+	sha256sum := getContentSha256Cksum(r)
+	switch {
+	case isRequestSignatureV4(r):
+		return doesSignatureMatch(sha256sum, r, region)
+	case isRequestPresignedSignatureV4(r):
+		return doesPresignedSignatureMatch(sha256sum, r, region)
+	default:
 		return ErrAccessDenied
+	}
 }
 
 // Verify if request has valid AWS Signature Version '4'.
@@ -162,32 +159,39 @@ func isReqAuthenticated(r *http.Request, region string) (s3Error APIErrorCode) {
 	if r == nil {
 		return ErrInternalError
 	}
+	if errCode := reqSignatureV4Verify(r, region); errCode != ErrNone {
+		return errCode
+	}
 	payload, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		errorIf(err, "Unable to read request body for signature verification")
 		return ErrInternalError
 	}
+
+	// Populate back the payload.
+	r.Body = ioutil.NopCloser(bytes.NewReader(payload))
+
 	// Verify Content-Md5, if payload is set.
 	if r.Header.Get("Content-Md5") != "" {
 		if r.Header.Get("Content-Md5") != getMD5HashBase64(payload) {
 			return ErrBadDigest
 		}
 	}
-	// Populate back the payload.
-	r.Body = ioutil.NopCloser(bytes.NewReader(payload))
-	// Skips calculating sha256 on the payload on server, if client requested for it.
-	var sha256sum string
+
 	if skipContentSha256Cksum(r) {
-		sha256sum = unsignedPayload
-	} else {
-		sha256sum = getSHA256Hash(payload)
+		return ErrNone
 	}
-	if isRequestSignatureV4(r) {
-		return doesSignatureMatch(sha256sum, r, region)
-	} else if isRequestPresignedSignatureV4(r) {
-		return doesPresignedSignatureMatch(sha256sum, r, region)
+
+	// Verify that X-Amz-Content-Sha256 Header == sha256(payload)
+	// If X-Amz-Content-Sha256 header is not sent then we don't calculate/verify sha256(payload)
+	sum := r.Header.Get("X-Amz-Content-Sha256")
+	if isRequestPresignedSignatureV4(r) {
+		sum = r.URL.Query().Get("X-Amz-Content-Sha256")
 	}
-	return ErrAccessDenied
+	if sum != "" && sum != getSHA256Hash(payload) {
+		return ErrContentSHA256Mismatch
+	}
+	return ErrNone
 }
 
 // authHandler - handles all the incoming authorization headers and validates them if possible.

cmd/auth-handler_test.go

@@ -308,6 +308,16 @@ func mustNewSignedRequest(method string, urlStr string, contentLength int64, bod
 	return req
 }
 
+func mustNewSignedBadMD5Request(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request {
+	req := mustNewRequest(method, urlStr, contentLength, body, t)
+	req.Header.Set("Content-Md5", "YWFhYWFhYWFhYWFhYWFhCg==")
+	cred := serverConfig.GetCredential()
+	if err := signRequestV4(req, cred.AccessKey, cred.SecretKey); err != nil {
+		t.Fatalf("Unable to initialized new signed http request %s", err)
+	}
+	return req
+}
+
 // Tests is requested authenticated function, tests replies for s3 errors.
 func TestIsReqAuthenticated(t *testing.T) {
 	path, err := newTestConfig(globalMinioDefaultRegion)
@@ -333,16 +343,13 @@ func TestIsReqAuthenticated(t *testing.T) {
 		// When request is unsigned, access denied is returned.
 		{mustNewRequest("GET", "http://127.0.0.1:9000", 0, nil, t), ErrAccessDenied},
 		// When request is properly signed, but has bad Content-MD5 header.
-		{mustNewSignedRequest("PUT", "http://127.0.0.1:9000", 5, bytes.NewReader([]byte("hello")), t), ErrBadDigest},
+		{mustNewSignedBadMD5Request("PUT", "http://127.0.0.1:9000/", 5, bytes.NewReader([]byte("hello")), t), ErrBadDigest},
 		// When request is properly signed, error is none.
 		{mustNewSignedRequest("GET", "http://127.0.0.1:9000", 0, nil, t), ErrNone},
 	}
 
 	// Validates all testcases.
 	for _, testCase := range testCases {
-		if testCase.s3Error == ErrBadDigest {
-			testCase.req.Header.Set("Content-Md5", "garbage")
-		}
 		if s3Error := isReqAuthenticated(testCase.req, serverConfig.GetRegion()); s3Error != testCase.s3Error {
 			t.Fatalf("Unexpected s3error returned wanted %d, got %d", testCase.s3Error, s3Error)
 		}

cmd/object-handlers.go

@@ -524,7 +524,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 		}
 		objInfo, err = objectAPI.PutObject(bucket, object, size, r.Body, metadata, sha256sum)
 	case authTypePresigned, authTypeSigned:
-		if s3Error := reqSignatureV4Verify(r); s3Error != ErrNone {
+		if s3Error := reqSignatureV4Verify(r, serverConfig.GetRegion()); s3Error != ErrNone {
 			errorIf(errSignatureMismatch, dumpRequest(r))
 			writeErrorResponse(w, s3Error, r.URL)
 			return
@@ -805,7 +805,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
 		}
 		partInfo, err = objectAPI.PutObjectPart(bucket, object, uploadID, partID, size, r.Body, incomingMD5, sha256sum)
 	case authTypePresigned, authTypeSigned:
-		if s3Error := reqSignatureV4Verify(r); s3Error != ErrNone {
+		if s3Error := reqSignatureV4Verify(r, serverConfig.GetRegion()); s3Error != ErrNone {
 			errorIf(errSignatureMismatch, dumpRequest(r))
 			writeErrorResponse(w, s3Error, r.URL)
 			return

cmd/server_test.go

@@ -398,12 +398,11 @@ func (s *TestSuiteCommon) TestListenBucketNotificationHandler(c *C) {
 	c.Assert(err, IsNil)
 	verifyError(c, response, "InvalidArgument", "Size of filter rule value cannot exceed 1024 bytes in UTF-8 representation", http.StatusBadRequest)
 
-	req, err = newTestSignedRequest("GET",
+	req, err = newTestSignedBadSHARequest("GET",
 		getListenBucketNotificationURL(s.endPoint, bucketName, []string{}, []string{}, validEvents),
 		0, nil, s.accessKey, s.secretKey, s.signer)
 	c.Assert(err, IsNil)
 
-	req.Header.Set("x-amz-content-sha256", "somethingElse")
 	client = http.Client{Transport: s.transport}
 	// execute the request.
 	response, err = client.Do(req)

cmd/signature-v4-utils.go

@@ -47,6 +47,25 @@ func skipContentSha256Cksum(r *http.Request) bool {
 	return isRequestUnsignedPayload(r) || isRequestPresignedUnsignedPayload(r)
 }
 
+// Returns SHA256 for calculating canonical-request.
+func getContentSha256Cksum(r *http.Request) string {
+	// 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
+	}
+	contentCkSum := r.Header.Get("X-Amz-Content-Sha256")
+	if contentCkSum == "" {
+		// If not set content checksum is defaulted to sha256([]byte("")).
+		contentCkSum = emptySHA256
+	}
+	return contentCkSum
+}
+
 // isValidRegion - verify if incoming region value is valid with configured Region.
 func isValidRegion(reqRegion string, confRegion string) bool {
 	if confRegion == "" || confRegion == "US" {

cmd/signature-v4-utils_test.go

@@ -234,3 +234,31 @@ func TestSignV4TrimAll(t *testing.T) {
 		}
 	}
 }
+
+// Test getContentSha256Cksum
+func TestGetContentSha256Cksum(t *testing.T) {
+	testCases := []struct {
+		h        string // header SHA256
+		q        string // query SHA256
+		expected string // expected SHA256
+	}{
+		{"shastring", "", "shastring"},
+		{"", "", emptySHA256},
+		{"", "X-Amz-Credential=random", unsignedPayload},
+		{"", "X-Amz-Credential=random&X-Amz-Content-Sha256=shastring", "shastring"},
+	}
+
+	for i, testCase := range testCases {
+		r, err := http.NewRequest("GET", "http://localhost/?"+testCase.q, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if testCase.h != "" {
+			r.Header.Set("x-amz-content-sha256", testCase.h)
+		}
+		got := getContentSha256Cksum(r)
+		if got != testCase.expected {
+			t.Errorf("Test %d: got:%s expected:%s", i+1, got, testCase.expected)
+		}
+	}
+}

cmd/signature-v4.go

@@ -210,12 +210,6 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 		return ErrInvalidAccessKeyID
 	}
 
-	// Hashed payload mismatch, return content sha256 mismatch.
-	contentSha256 := req.URL.Query().Get("X-Amz-Content-Sha256")
-	if contentSha256 != "" && hashedPayload != contentSha256 {
-		return ErrContentSHA256Mismatch
-	}
-
 	// Verify if region is valid.
 	sRegion := pSignValues.Credential.scope.region
 	// Should validate region, only if region is set.
@@ -233,7 +227,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 	}
 	// Construct new query.
 	query := make(url.Values)
-	if contentSha256 != "" {
+	if req.URL.Query().Get("X-Amz-Content-Sha256") != "" {
 		query.Set("X-Amz-Content-Sha256", hashedPayload)
 	}
 
@@ -333,11 +327,6 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP
 		return err
 	}
 
-	// Hashed payload mismatch, return content sha256 mismatch.
-	if hashedPayload != req.Header.Get("X-Amz-Content-Sha256") {
-		return ErrContentSHA256Mismatch
-	}
-
 	// Extract all the signed headers along with its values.
 	extractedSignedHeaders, errCode := extractSignedHeaders(signV4Values.SignedHeaders, r)
 	if errCode != ErrNone {
@@ -381,6 +370,7 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP
 
 	// Get canonical request.
 	canonicalRequest := getCanonicalRequest(extractedSignedHeaders, hashedPayload, queryStr, req.URL.Path, req.Method)
+
 	// Get string to sign from canonical request.
 	stringToSign := getStringToSign(canonicalRequest, t, signV4Values.Credential.getScope())
 

cmd/signature-v4_test.go

@@ -135,21 +135,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "us-west-1",
 			expected: ErrInvalidAccessKeyID,
 		},
-		// (2) Should error when the payload sha256 doesn't match.
-		{
-			queryParams: map[string]string{
-				"X-Amz-Algorithm":      signV4Algorithm,
-				"X-Amz-Date":           now.Format(iso8601Format),
-				"X-Amz-Expires":        "60",
-				"X-Amz-Signature":      "badsignature",
-				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), "us-west-1"),
-				"X-Amz-Content-Sha256": "ThisIsNotThePayloadHash",
-			},
-			region:   "us-west-1",
-			expected: ErrContentSHA256Mismatch,
-		},
-		// (3) Should fail with an invalid region.
+		// (2) Should fail with an invalid region.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -163,7 +149,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   globalMinioDefaultRegion,
 			expected: ErrInvalidRegion,
 		},
-		// (4) Should NOT fail with an invalid region if it doesn't verify it.
+		// (3) Should NOT fail with an invalid region if it doesn't verify it.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -177,7 +163,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "us-west-1",
 			expected: ErrUnsignedHeaders,
 		},
-		// (5) Should fail to extract headers if the host header is not signed.
+		// (4) Should fail to extract headers if the host header is not signed.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -191,7 +177,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrUnsignedHeaders,
 		},
-		// (6) Should give an expired request if it has expired.
+		// (5) Should give an expired request if it has expired.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -209,7 +195,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrExpiredPresignRequest,
 		},
-		// (7) Should error if the signature is incorrect.
+		// (6) Should error if the signature is incorrect.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -227,7 +213,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrSignatureDoesNotMatch,
 		},
-		// (8) Should error if the request is not ready yet, ie X-Amz-Date is in the future.
+		// (7) Should error if the request is not ready yet, ie X-Amz-Date is in the future.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -245,7 +231,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrRequestNotReadyYet,
 		},
-		// (9) Should not error with invalid region instead, call should proceed
+		// (8) Should not error with invalid region instead, call should proceed
 		// with sigature does not match.
 		{
 			queryParams: map[string]string{
@@ -264,7 +250,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "",
 			expected: ErrSignatureDoesNotMatch,
 		},
-		// (10) Should error with signature does not match. But handles
+		// (9) Should error with signature does not match. But handles
 		// query params which do not precede with "x-amz-" header.
 		{
 			queryParams: map[string]string{
@@ -284,7 +270,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "",
 			expected: ErrSignatureDoesNotMatch,
 		},
-		// (11) Should error with unsigned headers.
+		// (10) Should error with unsigned headers.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":       signV4Algorithm,

cmd/test-utils_test.go

@@ -1177,6 +1177,29 @@ func newTestSignedRequest(method, urlStr string, contentLength int64, body io.Re
 	return newTestSignedRequestV4(method, urlStr, contentLength, body, accessKey, secretKey)
 }
 
+// Returns request with correct signature but with incorrect SHA256.
+func newTestSignedBadSHARequest(method, urlStr string, contentLength int64, body io.ReadSeeker, accessKey, secretKey string, signer signerType) (*http.Request, error) {
+	req, err := newTestRequest(method, urlStr, contentLength, body)
+	if err != nil {
+		return nil, err
+	}
+
+	// Anonymous request return early.
+	if accessKey == "" || secretKey == "" {
+		return req, nil
+	}
+
+	if signer == signerV2 {
+		err = signRequestV2(req, accessKey, secretKey)
+		req.Header.Del("x-amz-content-sha256")
+	} else {
+		req.Header.Set("x-amz-content-sha256", "92b165232fbd011da355eca0b033db22b934ba9af0145a437a832d27310b89f9")
+		err = signRequestV4(req, accessKey, secretKey)
+	}
+
+	return req, err
+}
+
 // Returns new HTTP request object signed with signature v2.
 func newTestSignedRequestV2(method, urlStr string, contentLength int64, body io.ReadSeeker, accessKey, secretKey string) (*http.Request, error) {
 	req, err := newTestRequest(method, urlStr, contentLength, body)