From e0cf4ee9fcd19c73d3f15b5e0b48d36bf1f92ff3 Mon Sep 17 00:00:00 2001 From: karthic rao Date: Tue, 9 Aug 2016 21:43:15 +0530 Subject: [PATCH] presignV4: fix errors response and tests. (#2375) - Fix error response when one of the query params in the presign URL is missing. - Exhasutive test coverage for presignv4. --- api-errors.go | 65 +++++++- signature-v4-parser.go | 36 +++- signature-v4-parser_test.go | 319 +++++++++++++++++++++++++++++++++++- signature-v4-utils.go | 33 +++- signature-v4-utils_test.go | 62 +++++-- signature-v4.go | 11 +- streaming-signature-v4.go | 6 +- test-utils_test.go | 1 - 8 files changed, 492 insertions(+), 41 deletions(-) diff --git a/api-errors.go b/api-errors.go index 71289be10..06cfe4dd6 100644 --- a/api-errors.go +++ b/api-errors.go @@ -96,9 +96,14 @@ const ( ErrMissingSignHeadersTag ErrPolicyAlreadyExpired ErrMalformedDate + ErrMalformedPresignedDate + ErrMalformedCredentialDate + ErrMalformedCredentialRegion ErrMalformedExpires + ErrNegativeExpires ErrAuthHeaderEmpty ErrExpiredPresignRequest + ErrUnsignedHeaders ErrMissingDateHeader ErrInvalidQuerySignatureAlgo ErrInvalidQueryParams @@ -346,8 +351,8 @@ var errorCodeResponse = map[APIErrorCode]APIError{ HTTPStatusCode: http.StatusBadRequest, }, ErrCredMalformed: { - Code: "CredentialMalformed", - Description: "Credential field malformed does not follow accessKeyID/credScope.", + Code: "AuthorizationQueryParametersError", + Description: "Error parsing the X-Amz-Credential parameter; the Credential is mal-formed; expecting \"/YYYYMMDD/REGION/SERVICE/aws4_request\".", HTTPStatusCode: http.StatusBadRequest, }, ErrMalformedDate: { @@ -355,19 +360,46 @@ var errorCodeResponse = map[APIErrorCode]APIError{ Description: "Invalid date format header, expected to be in ISO8601, RFC1123 or RFC1123Z time format.", HTTPStatusCode: http.StatusBadRequest, }, + ErrMalformedPresignedDate: { + Code: "AuthorizationQueryParametersError", + Description: "X-Amz-Date must be in the ISO8601 Long Format \"yyyyMMdd'T'HHmmss'Z'\"", + HTTPStatusCode: http.StatusBadRequest, + }, + // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. + // right Description: "Error parsing the X-Amz-Credential parameter; incorrect date format \"%s\". This date in the credential must be in the format \"yyyyMMdd\".", + // Need changes to make sure variable messages can be constructed. + ErrMalformedCredentialDate: { + Code: "AuthorizationQueryParametersError", + Description: "Error parsing the X-Amz-Credential parameter; incorrect date format \"%s\". This date in the credential must be in the format \"yyyyMMdd\".", + HTTPStatusCode: http.StatusBadRequest, + }, + // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. + // right Description: "Error parsing the X-Amz-Credential parameter; the region 'us-east-' is wrong; expecting 'us-east-1'". + // Need changes to make sure variable messages can be constructed. + ErrMalformedCredentialRegion: { + Code: "AuthorizationQueryParametersError", + Description: "Error parsing the X-Amz-Credential parameter; the region is wrong;", + HTTPStatusCode: http.StatusBadRequest, + }, ErrInvalidRegion: { Code: "InvalidRegion", Description: "Region does not match.", HTTPStatusCode: http.StatusBadRequest, }, + // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. + // right Description: "Error parsing the X-Amz-Credential parameter; incorrect service \"s4\". This endpoint belongs to \"s3\".". + // Need changes to make sure variable messages can be constructed. ErrInvalidService: { - Code: "AccessDenied", - Description: "Service scope should be of value 's3'.", + Code: "AuthorizationQueryParametersError", + Description: "Error parsing the X-Amz-Credential parameter; incorrect service. This endpoint belongs to \"s3\".", HTTPStatusCode: http.StatusBadRequest, }, + // FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385. + // Description: "Error parsing the X-Amz-Credential parameter; incorrect terminal "aws4_reque". This endpoint uses "aws4_request". + // Need changes to make sure variable messages can be constructed. ErrInvalidRequestVersion: { - Code: "AccessDenied", - Description: "Request scope should be of value 'aws4_request'.", + Code: "AuthorizationQueryParametersError", + Description: "Error parsing the X-Amz-Credential parameter; incorrect terminal. This endpoint uses \"aws4_request\".", HTTPStatusCode: http.StatusBadRequest, }, ErrMissingSignTag: { @@ -386,8 +418,13 @@ var errorCodeResponse = map[APIErrorCode]APIError{ HTTPStatusCode: http.StatusBadRequest, }, ErrMalformedExpires: { - Code: "MalformedExpires", - Description: "Malformed expires header, expected non-zero number.", + Code: "AuthorizationQueryParametersError", + Description: "X-Amz-Expires should be a number", + HTTPStatusCode: http.StatusBadRequest, + }, + ErrNegativeExpires: { + Code: "AuthorizationQueryParametersError", + Description: "X-Amz-Expires must be non-negative", HTTPStatusCode: http.StatusBadRequest, }, ErrAuthHeaderEmpty: { @@ -407,7 +444,13 @@ var errorCodeResponse = map[APIErrorCode]APIError{ }, ErrExpiredPresignRequest: { Code: "AccessDenied", - Description: "Request has expired.", + Description: "Request has expired", + HTTPStatusCode: http.StatusBadRequest, + }, + // FIXME: Actual XML error response also contains the header which missed in lsit of signed header parameters. + ErrUnsignedHeaders: { + Code: "AccessDenied", + Description: "There were headers present in the request which were not signed", HTTPStatusCode: http.StatusBadRequest, }, ErrInvalidQueryParams: { @@ -579,3 +622,7 @@ func getAPIErrorResponse(err APIError, resource string) APIErrorResponse { return data } + +func getErrMalformedCredentialDate(malformedDateStr string) { + +} diff --git a/signature-v4-parser.go b/signature-v4-parser.go index afcb4ce0d..6ac12d453 100644 --- a/signature-v4-parser.go +++ b/signature-v4-parser.go @@ -57,10 +57,10 @@ func parseCredentialHeader(credElement string) (credentialHeader, APIErrorCode) var e error cred.scope.date, e = time.Parse(yyyymmdd, credElements[1]) if e != nil { - return credentialHeader{}, ErrMalformedDate + return credentialHeader{}, ErrMalformedCredentialDate } if credElements[2] == "" { - return credentialHeader{}, ErrInvalidRegion + return credentialHeader{}, ErrMalformedCredentialRegion } cred.scope.region = credElements[2] if credElements[3] != "s3" { @@ -122,8 +122,27 @@ type preSignValues struct { // querystring += &X-Amz-Expires=timeout interval // querystring += &X-Amz-SignedHeaders=signed_headers // querystring += &X-Amz-Signature=signature -// +//{ + +// verifies if any of the necessary query params are missing in the presigned request. +func doesV4PresignParamsExist(query url.Values) APIErrorCode { + v4PresignQueryParams := []string{"X-Amz-Algorithm", "X-Amz-Credential", "X-Amz-Signature", "X-Amz-Date", "X-Amz-SignedHeaders", "X-Amz-Expires"} + for _, v4PresignQueryParam := range v4PresignQueryParams { + if _, ok := query[v4PresignQueryParam]; !ok { + return ErrInvalidQueryParams + } + } + return ErrNone +} + func parsePreSignV4(query url.Values) (preSignValues, APIErrorCode) { + var err APIErrorCode + // verify whether the required query params exist. + err = doesV4PresignParamsExist(query) + if err != ErrNone { + return preSignValues{}, err + } + // Verify if the query algorithm is supported or not. if query.Get("X-Amz-Algorithm") != signV4Algorithm { return preSignValues{}, ErrInvalidQuerySignatureAlgo @@ -132,7 +151,6 @@ func parsePreSignV4(query url.Values) (preSignValues, APIErrorCode) { // Initialize signature version '4' structured header. preSignV4Values := preSignValues{} - var err APIErrorCode // Save credential. preSignV4Values.Credential, err = parseCredentialHeader("Credential=" + query.Get("X-Amz-Credential")) if err != ErrNone { @@ -143,7 +161,7 @@ func parsePreSignV4(query url.Values) (preSignValues, APIErrorCode) { // Save date in native time.Time. preSignV4Values.Date, e = time.Parse(iso8601Format, query.Get("X-Amz-Date")) if e != nil { - return preSignValues{}, ErrMalformedDate + return preSignValues{}, ErrMalformedPresignedDate } // Save expires in native time.Duration. @@ -152,11 +170,19 @@ func parsePreSignV4(query url.Values) (preSignValues, APIErrorCode) { return preSignValues{}, ErrMalformedExpires } + if preSignV4Values.Expires < 0 { + return preSignValues{}, ErrNegativeExpires + } // Save signed headers. preSignV4Values.SignedHeaders, err = parseSignedHeaders("SignedHeaders=" + query.Get("X-Amz-SignedHeaders")) if err != ErrNone { return preSignValues{}, err } + // `host` is the only header used during the presigned request. + // Malformed signed headers has be caught here, otherwise it'll lead to signature mismatch. + if preSignV4Values.SignedHeaders[0] != "host" { + return preSignValues{}, ErrUnsignedHeaders + } // Save signature. preSignV4Values.Signature, err = parseSignature("Signature=" + query.Get("X-Amz-Signature")) diff --git a/signature-v4-parser_test.go b/signature-v4-parser_test.go index 1fefcfa49..18d34123b 100644 --- a/signature-v4-parser_test.go +++ b/signature-v4-parser_test.go @@ -17,6 +17,8 @@ package main import ( + "net/url" + "strconv" "strings" "testing" "time" @@ -24,7 +26,12 @@ import ( // generates credential string from its fields. func generateCredentialStr(accessKey, date, region, service, requestVersion string) string { - return "Credential=" + strings.Join([]string{ + return "Credential=" + joinWithSlash(accessKey, date, region, service, requestVersion) +} + +// joins the argument strings with a '/' and returns it. +func joinWithSlash(accessKey, date, region, service, requestVersion string) string { + return strings.Join([]string{ accessKey, date, region, @@ -131,7 +138,7 @@ func TestParseCredentialHeader(t *testing.T) { "ABCD", "ABCD"), expectedCredentials: credentialHeader{}, - expectedErrCode: ErrMalformedDate, + expectedErrCode: ErrMalformedCredentialDate, }, // Test Case - 6. // Test case with invalid region. @@ -144,7 +151,7 @@ func TestParseCredentialHeader(t *testing.T) { "ABCD", "ABCD"), expectedCredentials: credentialHeader{}, - expectedErrCode: ErrInvalidRegion, + expectedErrCode: ErrMalformedCredentialRegion, }, // Test Case - 7. // Test case with invalid service. @@ -357,7 +364,7 @@ func TestParseSignV4(t *testing.T) { expectedAuthField: signValues{}, expectedErrCode: ErrMissingSignHeadersTag, }, - // Test case - 5. + // Test case - 6. // Auth string with missing "SignatureTag",ErrMissingSignTag expected. // A vaild credential is generated. // Test case with invalid credential field. @@ -381,6 +388,7 @@ func TestParseSignV4(t *testing.T) { expectedAuthField: signValues{}, expectedErrCode: ErrMissingSignTag, }, + // Test case - 7. { inputV4AuthStr: signV4Algorithm + strings.Join([]string{ @@ -438,3 +446,306 @@ func TestParseSignV4(t *testing.T) { } } + +// TestDoesV4PresignParamsExist - tests validate the logic to +func TestDoesV4PresignParamsExist(t *testing.T) { + testCases := []struct { + inputQueryKeyVals []string + expectedErrCode APIErrorCode + }{ + // Test case - 1. + // contains all query param keys which are necessary for v4 presign request. + { + inputQueryKeyVals: []string{ + "X-Amz-Algorithm", "", + "X-Amz-Credential", "", + "X-Amz-Signature", "", + "X-Amz-Date", "", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + }, + expectedErrCode: ErrNone, + }, + // Test case - 2. + // missing "X-Amz-Algorithm" in tdhe query param. + // contains all query param keys which are necessary for v4 presign request. + { + inputQueryKeyVals: []string{ + "X-Amz-Credential", "", + "X-Amz-Signature", "", + "X-Amz-Date", "", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + }, + expectedErrCode: ErrInvalidQueryParams, + }, + // Test case - 3. + // missing "X-Amz-Credential" in the query param. + { + inputQueryKeyVals: []string{ + "X-Amz-Algorithm", "", + "X-Amz-Signature", "", + "X-Amz-Date", "", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + }, + expectedErrCode: ErrInvalidQueryParams, + }, + // Test case - 4. + // missing "X-Amz-Signature" in the query param. + { + inputQueryKeyVals: []string{ + "X-Amz-Algorithm", "", + "X-Amz-Credential", "", + "X-Amz-Date", "", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + }, + expectedErrCode: ErrInvalidQueryParams, + }, + // Test case - 5. + // missing "X-Amz-Date" in the query param. + { + inputQueryKeyVals: []string{ + "X-Amz-Algorithm", "", + "X-Amz-Credential", "", + "X-Amz-Signature", "", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + }, + expectedErrCode: ErrInvalidQueryParams, + }, + // Test case - 6. + // missing "X-Amz-SignedHeaders" in the query param. + { + inputQueryKeyVals: []string{ + "X-Amz-Algorithm", "", + "X-Amz-Credential", "", + "X-Amz-Signature", "", + "X-Amz-Date", "", + "X-Amz-Expires", "", + }, + expectedErrCode: ErrInvalidQueryParams, + }, + // Test case - 7. + // missing "X-Amz-Expires" in the query param. + { + inputQueryKeyVals: []string{ + "X-Amz-Algorithm", "", + "X-Amz-Credential", "", + "X-Amz-Signature", "", + "X-Amz-Date", "", + "X-Amz-SignedHeaders", "", + }, + expectedErrCode: ErrInvalidQueryParams, + }, + } + + for i, testCase := range testCases { + inputQuery := url.Values{} + // iterating through input query key value and setting the inputQuery of type url.Values. + for j := 0; j < len(testCase.inputQueryKeyVals)-1; j += 2 { + + inputQuery.Set(testCase.inputQueryKeyVals[j], testCase.inputQueryKeyVals[j+1]) + } + + actualErrCode := doesV4PresignParamsExist(inputQuery) + + if testCase.expectedErrCode != actualErrCode { + t.Fatalf("Test %d: Expected the APIErrCode to be %d, got %d", i+1, testCase.expectedErrCode, actualErrCode) + } + } + +} + +// TestParsePreSignV4 - Validates the parsing logic of Presignied v4 request from its url query values. +func TestParsePreSignV4(t *testing.T) { + // converts the duration in seconds into string format. + getDurationStr := func(expires int) string { + return strconv.FormatInt(int64(expires), 10) + } + // used in expected preSignValues, preSignValues.Date is of type time.Time . + queryTime := time.Now() + + sampleTimeStr := time.Now().UTC().Format(yyyymmdd) + + testCases := []struct { + inputQueryKeyVals []string + expectedPreSignValues preSignValues + expectedErrCode APIErrorCode + }{ + // Test case - 1. + // A Valid v4 presign URL requires the following params to be in the query. + // "X-Amz-Algorithm", "X-Amz-Credential", "X-Amz-Signature", " X-Amz-Date", "X-Amz-SignedHeaders", "X-Amz-Expires". + // If these params are missing its expected to get ErrInvalidQueryParams . + // In the following test case 2 out of 6 query params are missing. + { + inputQueryKeyVals: []string{ + "X-Amz-Algorithm", "", + "X-Amz-Credential", "", + "X-Amz-Signature", "", + "X-Amz-Expires", "", + }, + expectedPreSignValues: preSignValues{}, + expectedErrCode: ErrInvalidQueryParams, + }, + // Test case - 2. + // Test case with invalid "X-Amz-Algorithm" query value. + // The other query params should exist, other wise ErrInvalidQueryParams will be returned because of missing fields. + { + inputQueryKeyVals: []string{ + + "X-Amz-Algorithm", "InvalidValue", + "X-Amz-Credential", "", + "X-Amz-Signature", "", + "X-Amz-Date", "", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + }, + expectedPreSignValues: preSignValues{}, + expectedErrCode: ErrInvalidQuerySignatureAlgo, + }, + // Test case - 3. + // Test case with valid "X-Amz-Algorithm" query value, but invalid "X-Amz-Credential" header. + // Malformed crenential. + { + inputQueryKeyVals: []string{ + // valid "X-Amz-Algorithm" header. + "X-Amz-Algorithm", signV4Algorithm, + // valid "X-Amz-Credential" header. + "X-Amz-Credential", "invalid-credential", + "X-Amz-Signature", "", + "X-Amz-Date", "", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + }, + expectedPreSignValues: preSignValues{}, + expectedErrCode: ErrCredMalformed, + }, + + // Test case - 4. + // Test case with valid "X-Amz-Algorithm" query value. + // Malformed date. + { + inputQueryKeyVals: []string{ + // valid "X-Amz-Algorithm" header. + "X-Amz-Algorithm", signV4Algorithm, + // valid "X-Amz-Credential" header. + "X-Amz-Credential", joinWithSlash( + "Z7IXGOO6BZ0REAN1Q26I", + sampleTimeStr, + "us-west-1", + "s3", + "aws4_request"), + // invalid "X-Amz-Date" query. + "X-Amz-Date", "invalid-time", + "X-Amz-SignedHeaders", "", + "X-Amz-Expires", "", + "X-Amz-Signature", "", + }, + expectedPreSignValues: preSignValues{}, + expectedErrCode: ErrMalformedPresignedDate, + }, + // Test case - 5. + // Test case with valid "X-Amz-Algorithm", "X-Amz-Credential", "X-Amz-Date" query value. + // Malformed Expiry, a valid expiry should be of format "s". + { + inputQueryKeyVals: []string{ + // valid "X-Amz-Algorithm" header. + "X-Amz-Algorithm", signV4Algorithm, + // valid "X-Amz-Credential" header. + "X-Amz-Credential", joinWithSlash( + "Z7IXGOO6BZ0REAN1Q26I", + sampleTimeStr, + "us-west-1", + "s3", + "aws4_request"), + // valid "X-Amz-Date" query. + "X-Amz-Date", time.Now().UTC().Format(iso8601Format), + "X-Amz-Expires", "MalformedExpiry", + "X-Amz-SignedHeaders", "", + "X-Amz-Signature", "", + }, + expectedPreSignValues: preSignValues{}, + expectedErrCode: ErrMalformedExpires, + }, + // Test case - 6. + // Test case with valid "X-Amz-Algorithm", "X-Amz-Credential", "X-Amz-Date" query value. + // Malformed Expiry, a valid expiry should be of format "s". + { + inputQueryKeyVals: []string{ + // valid "X-Amz-Algorithm" header. + "X-Amz-Algorithm", signV4Algorithm, + // valid "X-Amz-Credential" header. + "X-Amz-Credential", joinWithSlash( + "Z7IXGOO6BZ0REAN1Q26I", + sampleTimeStr, + "us-west-1", + "s3", + "aws4_request"), + // valid "X-Amz-Date" query. + "X-Amz-Date", queryTime.UTC().Format(iso8601Format), + "X-Amz-Expires", getDurationStr(100), + "X-Amz-Signature", "abcd", + "X-Amz-SignedHeaders", "host;x-amz-content-sha256;x-amz-date", + }, + expectedPreSignValues: preSignValues{ + signValues{ + // Credentials. + generateCredentials( + t, + "Z7IXGOO6BZ0REAN1Q26I", + sampleTimeStr, + "us-west-1", + "s3", + "aws4_request", + ), + // SignedHeaders. + []string{"host", "x-amz-content-sha256", "x-amz-date"}, + // Signature. + "abcd", + }, + // Date + queryTime, + // Expires. + 100 * time.Second, + }, + expectedErrCode: ErrNone, + }, + } + + for i, testCase := range testCases { + inputQuery := url.Values{} + // iterating through input query key value and setting the inputQuery of type url.Values. + for j := 0; j < len(testCase.inputQueryKeyVals)-1; j += 2 { + inputQuery.Set(testCase.inputQueryKeyVals[j], testCase.inputQueryKeyVals[j+1]) + } + // call the function under test. + parsedPreSign, actualErrCode := parsePreSignV4(inputQuery) + + if testCase.expectedErrCode != actualErrCode { + t.Fatalf("Test %d: Expected the APIErrCode to be %d, got %d", i+1, testCase.expectedErrCode, actualErrCode) + } + if actualErrCode == ErrNone { + // validating credentials. + validateCredentialfields(t, i+1, testCase.expectedPreSignValues.Credential, parsedPreSign.Credential) + // validating signed headers. + if strings.Join(testCase.expectedPreSignValues.SignedHeaders, ",") != strings.Join(parsedPreSign.SignedHeaders, ",") { + t.Errorf("Test %d: Expected the result to be \"%v\", but got \"%v\". ", i+1, testCase.expectedPreSignValues.SignedHeaders, parsedPreSign.SignedHeaders) + } + // validating signature field. + if testCase.expectedPreSignValues.Signature != parsedPreSign.Signature { + t.Errorf("Test %d: Signature field mismatch: Expected \"%s\", got \"%s\"", i+1, testCase.expectedPreSignValues.Signature, parsedPreSign.Signature) + } + // validating expiry duration. + if testCase.expectedPreSignValues.Expires != parsedPreSign.Expires { + t.Errorf("Test %d: Expected expiry time to be %v, but got %v", i+1, testCase.expectedPreSignValues.Expires, parsedPreSign.Expires) + } + // validating presign date field. + if testCase.expectedPreSignValues.Date.UTC().Format(iso8601Format) != parsedPreSign.Date.UTC().Format(iso8601Format) { + t.Errorf("Test %d: Expected date to be %v, but got %v", i+1, testCase.expectedPreSignValues.Date.UTC().Format(iso8601Format), parsedPreSign.Date.UTC().Format(iso8601Format)) + } + } + + } +} diff --git a/signature-v4-utils.go b/signature-v4-utils.go index 67a4ab867..901774c13 100644 --- a/signature-v4-utils.go +++ b/signature-v4-utils.go @@ -99,10 +99,27 @@ func getURLEncodedName(name string) string { return encodedName } +// find whether "host" is part of list of signed headers. +func findHost(signedHeaders []string) APIErrorCode { + for _, header := range signedHeaders { + if header == "host" { + return ErrNone + } + } + return ErrUnsignedHeaders +} + // extractSignedHeaders extract signed headers from Authorization header -func extractSignedHeaders(signedHeaders []string, reqHeaders http.Header) http.Header { +func extractSignedHeaders(signedHeaders []string, reqHeaders http.Header) (http.Header, APIErrorCode) { + errCode := findHost(signedHeaders) + if errCode != ErrNone { + return nil, errCode + } extractedSignedHeaders := make(http.Header) for _, header := range signedHeaders { + // `host` will not be found in the headers, can be found in r.Host. + // but its alway necessary that the list of signed headers containing host in it. + val, ok := reqHeaders[http.CanonicalHeaderKey(header)] if !ok { // Golang http server strips off 'Expect' header, if the @@ -123,11 +140,19 @@ func extractSignedHeaders(signedHeaders []string, reqHeaders http.Header) http.H // doesn't filter out the 'Expect' header. if header == "expect" { extractedSignedHeaders[header] = []string{"100-continue"} + continue } - // If not found continue, we will fail later. - continue + // the "host" field will not be found in the header map, it can be found in req.Host. + // but its necessary to make sure that the "host" field exists in the list of signed paramaters, + // the check is done above. + if header == "host" { + continue + } + // If not found continue, we will stop here. + return nil, ErrUnsignedHeaders } extractedSignedHeaders[header] = val } - return extractedSignedHeaders + + return extractedSignedHeaders, ErrNone } diff --git a/signature-v4-utils_test.go b/signature-v4-utils_test.go index 9e1f74f37..92ac9627c 100644 --- a/signature-v4-utils_test.go +++ b/signature-v4-utils_test.go @@ -133,10 +133,6 @@ func TestGetURLEncodedName(t *testing.T) { func TestExtractSignedHeaders(t *testing.T) { signedHeaders := []string{"host", "x-amz-content-sha256", "x-amz-date"} - for i, key := range signedHeaders { - signedHeaders[i] = http.CanonicalHeaderKey(key) - } - // If the `expect` key exists in the signed headers then golang server would have stripped out the value, expecting the `expect` header set to `100-continue` in the result. signedHeaders = append(signedHeaders, "expect") // expected header values. @@ -150,27 +146,67 @@ func TestExtractSignedHeaders(t *testing.T) { inputHeader.Set(signedHeaders[1], expectedContentSha256) inputHeader.Set(signedHeaders[2], expectedTime) // calling the function being tested. - extractedSignedHeaders := extractSignedHeaders(signedHeaders, inputHeader) + extractedSignedHeaders, errCode := extractSignedHeaders(signedHeaders, inputHeader) + if errCode != ErrNone { + t.Fatalf("Expected the APIErrorCode to be %d, but got %d", ErrNone, errCode) + } + // "x-amz-content-sha256" header value from the extracted result. - extractedContentSha256 := extractedSignedHeaders.Get(signedHeaders[1]) + extractedContentSha256 := extractedSignedHeaders[signedHeaders[1]] // "host" header value from the extracted result. - extractedHost := extractedSignedHeaders.Get(signedHeaders[0]) + extractedHost := extractedSignedHeaders[signedHeaders[0]] // "x-amz-date" header from the extracted result. - extractedDate := extractedSignedHeaders.Get(signedHeaders[2]) + extractedDate := extractedSignedHeaders[signedHeaders[2]] // extracted `expect` header. extractedExpect := extractedSignedHeaders["expect"][0] + if expectedHost != extractedHost[0] { + t.Errorf("host header mismatch: expected `%s`, got `%s`", expectedHost, extractedHost) + } // assert the result with the expected value. - if expectedContentSha256 != extractedContentSha256 { + if expectedContentSha256 != extractedContentSha256[0] { t.Errorf("x-amz-content-sha256 header mismatch: expected `%s`, got `%s`", expectedContentSha256, extractedContentSha256) } - if expectedTime != extractedDate { + if expectedTime != extractedDate[0] { t.Errorf("x-amz-date header mismatch: expected `%s`, got `%s`", expectedTime, extractedDate) } - if expectedHost != extractedHost { - t.Errorf("host header mismatch: expected `%s`, got `%s`", expectedHost, extractedHost) - } + // Since the list of signed headers value contained `expect`, the default value of `100-continue` will be added to extracted signed headers. if extractedExpect != "100-continue" { t.Errorf("expect header incorrect value: expected `%s`, got `%s`", "100-continue", extractedExpect) } + + // case where the headers doesn't contain the one of the signed header in the signed headers list. + signedHeaders = append(signedHeaders, " X-Amz-Credential") + // expected to fail with `ErrUnsignedHeaders`. + extractedSignedHeaders, errCode = extractSignedHeaders(signedHeaders, inputHeader) + if errCode != ErrUnsignedHeaders { + t.Fatalf("Expected the APIErrorCode to %d, but got %d", ErrUnsignedHeaders, errCode) + } + + // case where the list of signed headers doesn't contain the host field. + signedHeaders = signedHeaders[1:] + // expected to fail with `ErrUnsignedHeaders`. + extractedSignedHeaders, errCode = extractSignedHeaders(signedHeaders, inputHeader) + if errCode != ErrUnsignedHeaders { + t.Fatalf("Expected the APIErrorCode to %d, but got %d", ErrUnsignedHeaders, errCode) + } +} + +// TestFindHost - tests the logic to find whether "host" is part of signed headers. +func TestFindHost(t *testing.T) { + // doesn't contain "host". + signedHeaders := []string{"x-amz-content-sha256", "x-amz-date"} + errCode := findHost(signedHeaders) + // expected to error out with code ErrUnsignedHeaders . + if errCode != ErrUnsignedHeaders { + t.Fatalf("Expected the APIErrorCode to be %d, but got %d", ErrUnsignedHeaders, errCode) + } + + // adding "host". + signedHeaders = append(signedHeaders, "host") + // epxected to pass. + errCode = findHost(signedHeaders) + if errCode != ErrNone { + t.Fatalf("Expected the APIErrorCode to be %d, but got %d", ErrNone, errCode) + } } diff --git a/signature-v4.go b/signature-v4.go index 52224d323..e61201d57 100644 --- a/signature-v4.go +++ b/signature-v4.go @@ -234,8 +234,10 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, validate } // Extract all the signed headers along with its values. - extractedSignedHeaders := extractSignedHeaders(preSignValues.SignedHeaders, req.Header) - + extractedSignedHeaders, errCode := extractSignedHeaders(preSignValues.SignedHeaders, req.Header) + if errCode != ErrNone { + return errCode + } // Construct new query. query := make(url.Values) if contentSha256 != "" { @@ -341,7 +343,10 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, validateRegion bo } // Extract all the signed headers along with its values. - extractedSignedHeaders := extractSignedHeaders(signV4Values.SignedHeaders, req.Header) + extractedSignedHeaders, errCode := extractSignedHeaders(signV4Values.SignedHeaders, req.Header) + if errCode != ErrNone { + return errCode + } // Verify if the access key id matches. if signV4Values.Credential.accessKey != cred.AccessKeyID { diff --git a/streaming-signature-v4.go b/streaming-signature-v4.go index 585b545d2..3adf00bae 100644 --- a/streaming-signature-v4.go +++ b/streaming-signature-v4.go @@ -95,8 +95,10 @@ func calculateSeedSignature(r *http.Request) (signature string, date time.Time, } // Extract all the signed headers along with its values. - extractedSignedHeaders := extractSignedHeaders(signV4Values.SignedHeaders, req.Header) - + extractedSignedHeaders, errCode := extractSignedHeaders(signV4Values.SignedHeaders, req.Header) + if errCode != ErrNone { + return "", time.Time{}, errCode + } // Verify if the access key id matches. if signV4Values.Credential.accessKey != cred.AccessKeyID { return "", time.Time{}, ErrInvalidAccessKeyID diff --git a/test-utils_test.go b/test-utils_test.go index 0012a3ace..82f5a3659 100644 --- a/test-utils_test.go +++ b/test-utils_test.go @@ -312,7 +312,6 @@ func newTestRequest(method, urlStr string, contentLength int64, body io.ReadSeek req.Header.Set("Content-Md5", md5Base64) } req.Header.Set("x-amz-content-sha256", hashedPayload) - // Seek back to beginning. if body != nil { body.Seek(0, 0)