diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 27b6a1408..cdfbfe5a8 100644 --- a/cmd/auth-handler.go +++ b/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 +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 } - if isRequestSignatureV4(r) { - return doesSignatureMatch(sha256sum, r, serverConfig.GetRegion()) - } else if isRequestPresignedSignatureV4(r) { - return doesPresignedSignatureMatch(sha256sum, r, serverConfig.GetRegion()) - } - 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. diff --git a/cmd/auth-handler_test.go b/cmd/auth-handler_test.go index bd6cafd8e..8deda7b85 100644 --- a/cmd/auth-handler_test.go +++ b/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) } diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 0a3f27816..850556ffb 100644 --- a/cmd/object-handlers.go +++ b/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 diff --git a/cmd/server_test.go b/cmd/server_test.go index 44a4a4a13..241a3ad1f 100644 --- a/cmd/server_test.go +++ b/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) diff --git a/cmd/signature-v4-utils.go b/cmd/signature-v4-utils.go index e35a97d02..6ba30212d 100644 --- a/cmd/signature-v4-utils.go +++ b/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" { diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go index 27e05b499..772f10312 100644 --- a/cmd/signature-v4-utils_test.go +++ b/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) + } + } +} diff --git a/cmd/signature-v4.go b/cmd/signature-v4.go index 2d5fc79ac..157f2aef6 100644 --- a/cmd/signature-v4.go +++ b/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()) diff --git a/cmd/signature-v4_test.go b/cmd/signature-v4_test.go index 7d5d794b2..9c891dc87 100644 --- a/cmd/signature-v4_test.go +++ b/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, diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 0bfea9bc2..d357630c5 100644 --- a/cmd/test-utils_test.go +++ b/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)