diff --git a/cmd/signature-v4-parser.go b/cmd/signature-v4-parser.go index 8f984b900..d1cc3610e 100644 --- a/cmd/signature-v4-parser.go +++ b/cmd/signature-v4-parser.go @@ -74,7 +74,7 @@ func parseCredentialHeader(credElement string) (credentialHeader, APIErrorCode) return cred, ErrNone } -// Parse signature string. +// Parse signature from signature tag. func parseSignature(signElement string) (string, APIErrorCode) { signFields := strings.Split(strings.TrimSpace(signElement), "=") if len(signFields) != 2 { @@ -83,12 +83,15 @@ func parseSignature(signElement string) (string, APIErrorCode) { if signFields[0] != "Signature" { return "", ErrMissingSignTag } + if signFields[1] == "" { + return "", ErrMissingFields + } signature := signFields[1] return signature, ErrNone } -// Parse signed headers string. -func parseSignedHeaders(signedHdrElement string) ([]string, APIErrorCode) { +// Parse slice of signed headers from signed headers tag. +func parseSignedHeader(signedHdrElement string) ([]string, APIErrorCode) { signedHdrFields := strings.Split(strings.TrimSpace(signedHdrElement), "=") if len(signedHdrFields) != 2 { return nil, ErrMissingFields @@ -96,6 +99,9 @@ func parseSignedHeaders(signedHdrElement string) ([]string, APIErrorCode) { if signedHdrFields[0] != "SignedHeaders" { return nil, ErrMissingSignHeadersTag } + if signedHdrFields[1] == "" { + return nil, ErrMissingFields + } signedHeaders := strings.Split(signedHdrFields[1], ";") return signedHeaders, ErrNone } @@ -122,8 +128,7 @@ 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"} @@ -135,6 +140,7 @@ func doesV4PresignParamsExist(query url.Values) APIErrorCode { return ErrNone } +// Parses all the presigned signature values into separate elements. func parsePreSignV4(query url.Values) (preSignValues, APIErrorCode) { var err APIErrorCode // verify whether the required query params exist. @@ -174,7 +180,7 @@ func parsePreSignV4(query url.Values) (preSignValues, APIErrorCode) { return preSignValues{}, ErrNegativeExpires } // Save signed headers. - preSignV4Values.SignedHeaders, err = parseSignedHeaders("SignedHeaders=" + query.Get("X-Amz-SignedHeaders")) + preSignV4Values.SignedHeaders, err = parseSignedHeader("SignedHeaders=" + query.Get("X-Amz-SignedHeaders")) if err != ErrNone { return preSignValues{}, err } @@ -231,7 +237,7 @@ func parseSignV4(v4Auth string) (signValues, APIErrorCode) { } // Save signed headers. - signV4Values.SignedHeaders, err = parseSignedHeaders(authFields[1]) + signV4Values.SignedHeaders, err = parseSignedHeader(authFields[1]) if err != ErrNone { return signValues{}, err } diff --git a/cmd/signature-v4-parser_test.go b/cmd/signature-v4-parser_test.go index 975a5b6cf..16d960686 100644 --- a/cmd/signature-v4-parser_test.go +++ b/cmd/signature-v4-parser_test.go @@ -228,13 +228,21 @@ func TestParseSignature(t *testing.T) { expectedErrCode: ErrMissingFields, }, // Test case - 2. + // SignElement does have 2 parts but doesn't have valid signature value. + // ErrMissingFields expected. + { + inputSignElement: "Signature=", + expectedSignStr: "", + expectedErrCode: ErrMissingFields, + }, + // Test case - 3. // SignElemenet with missing "SignatureTag",ErrMissingSignTag expected. { inputSignElement: "Sign=", expectedSignStr: "", expectedErrCode: ErrMissingSignTag, }, - // Test case - 3. + // Test case - 4. // Test case with valid inputs. { inputSignElement: "Signature=abcd", @@ -289,8 +297,7 @@ func TestParseSignedHeaders(t *testing.T) { } for i, testCase := range testCases { - actualSignedHeaders, actualErrCode := parseSignedHeaders(testCase.inputSignElement) - + actualSignedHeaders, actualErrCode := parseSignedHeader(testCase.inputSignElement) if testCase.expectedErrCode != actualErrCode { t.Errorf("Test %d: Expected the APIErrCode to be %d, got %d", i+1, testCase.expectedErrCode, actualErrCode) } @@ -670,6 +677,50 @@ func TestParsePreSignV4(t *testing.T) { expectedErrCode: ErrMalformedExpires, }, // Test case - 6. + // Test case with negative X-Amz-Expires header. + { + 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(-1), + "X-Amz-Signature", "abcd", + "X-Amz-SignedHeaders", "host;x-amz-content-sha256;x-amz-date", + }, + expectedPreSignValues: preSignValues{}, + expectedErrCode: ErrNegativeExpires, + }, + // Test case - 7. + // Test case with empty X-Amz-SignedHeaders. + { + 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", "", + }, + expectedPreSignValues: preSignValues{}, + expectedErrCode: ErrMissingFields, + }, + // Test case - 8. // 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". { @@ -722,7 +773,6 @@ func TestParsePreSignV4(t *testing.T) { } // 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) } diff --git a/cmd/signature-v4-utils.go b/cmd/signature-v4-utils.go index 4fd4371f9..7dbd32f05 100644 --- a/cmd/signature-v4-utils.go +++ b/cmd/signature-v4-utils.go @@ -86,14 +86,13 @@ func getURLEncodedName(name string) string { continue default: len := utf8.RuneLen(s) - if len < 0 { - return name - } - u := make([]byte, len) - utf8.EncodeRune(u, s) - for _, r := range u { - hex := hex.EncodeToString([]byte{r}) - encodedName = encodedName + "%" + strings.ToUpper(hex) + if len > 0 { + u := make([]byte, len) + utf8.EncodeRune(u, s) + for _, r := range u { + hex := hex.EncodeToString([]byte{r}) + encodedName = encodedName + "%" + strings.ToUpper(hex) + } } } } diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go index 3dff1db1a..9b36ee9ca 100644 --- a/cmd/signature-v4-utils_test.go +++ b/cmd/signature-v4-utils_test.go @@ -86,10 +86,11 @@ func TestIsValidRegion(t *testing.T) { {"us-east-1", "US", true}, {"us-west-1", "US", false}, {"us-west-1", "us-west-1", true}, + // "US" was old naming convention for 'us-east-1'. + {"US", "US", true}, } for i, testCase := range testCases { - actualResult := isValidRegion(testCase.inputReqRegion, testCase.inputConfRegion) if testCase.expectedResult != actualResult { t.Errorf("Test %d: Expected the result to `%v`, but instead got `%v`", i+1, testCase.expectedResult, actualResult) diff --git a/cmd/signature-v4_test.go b/cmd/signature-v4_test.go index 8f6d91f85..48763a991 100644 --- a/cmd/signature-v4_test.go +++ b/cmd/signature-v4_test.go @@ -100,8 +100,14 @@ func TestDoesPolicySignatureMatch(t *testing.T) { } func TestDoesPresignedSignatureMatch(t *testing.T) { + rootPath, err := newTestConfig("us-east-1") + if err != nil { + t.Fatal(err) + } + defer removeAll(rootPath) + // sha256 hash of "payload" - payload := "239f59ed55e737c77147cf55ad0c1b030b6d7ee748a7426952f9b852d5a935e5" + payloadSHA256 := "239f59ed55e737c77147cf55ad0c1b030b6d7ee748a7426952f9b852d5a935e5" now := time.Now().UTC() credentialTemplate := "%s/%s/%s/s3/aws4_request" @@ -152,7 +158,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) { "X-Amz-Signature": "badsignature", "X-Amz-SignedHeaders": "host;x-amz-content-sha256;x-amz-date", "X-Amz-Credential": fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), "us-west-1"), - "X-Amz-Content-Sha256": payload, + "X-Amz-Content-Sha256": payloadSHA256, }, region: "us-east-1", expected: ErrInvalidRegion, @@ -166,7 +172,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) { "X-Amz-Signature": "badsignature", "X-Amz-SignedHeaders": "host;x-amz-content-sha256;x-amz-date", "X-Amz-Credential": fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), "us-west-1"), - "X-Amz-Content-Sha256": payload, + "X-Amz-Content-Sha256": payloadSHA256, }, region: "us-west-1", expected: ErrUnsignedHeaders, @@ -180,7 +186,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) { "X-Amz-Signature": "badsignature", "X-Amz-SignedHeaders": "x-amz-content-sha256;x-amz-date", "X-Amz-Credential": fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()), - "X-Amz-Content-Sha256": payload, + "X-Amz-Content-Sha256": payloadSHA256, }, region: serverConfig.GetRegion(), expected: ErrUnsignedHeaders, @@ -194,11 +200,11 @@ func TestDoesPresignedSignatureMatch(t *testing.T) { "X-Amz-Signature": "badsignature", "X-Amz-SignedHeaders": "host;x-amz-content-sha256;x-amz-date", "X-Amz-Credential": fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()), - "X-Amz-Content-Sha256": payload, + "X-Amz-Content-Sha256": payloadSHA256, }, headers: map[string]string{ "X-Amz-Date": now.AddDate(0, 0, -2).Format(iso8601Format), - "X-Amz-Content-Sha256": payload, + "X-Amz-Content-Sha256": payloadSHA256, }, region: serverConfig.GetRegion(), expected: ErrExpiredPresignRequest, @@ -212,15 +218,72 @@ func TestDoesPresignedSignatureMatch(t *testing.T) { "X-Amz-Signature": "badsignature", "X-Amz-SignedHeaders": "host;x-amz-content-sha256;x-amz-date", "X-Amz-Credential": fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()), - "X-Amz-Content-Sha256": payload, + "X-Amz-Content-Sha256": payloadSHA256, }, headers: map[string]string{ "X-Amz-Date": now.Format(iso8601Format), - "X-Amz-Content-Sha256": payload, + "X-Amz-Content-Sha256": payloadSHA256, }, region: serverConfig.GetRegion(), expected: ErrSignatureDoesNotMatch, }, + // (8) 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, + "X-Amz-Date": now.Add(1 * time.Hour).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, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()), + "X-Amz-Content-Sha256": payloadSHA256, + }, + headers: map[string]string{ + "X-Amz-Date": now.Format(iso8601Format), + "X-Amz-Content-Sha256": payloadSHA256, + }, + region: serverConfig.GetRegion(), + expected: ErrRequestNotReadyYet, + }, + // (9) Should not error with invalid region instead, call should proceed + // with sigature does not 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, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()), + "X-Amz-Content-Sha256": payloadSHA256, + }, + headers: map[string]string{ + "X-Amz-Date": now.Format(iso8601Format), + "X-Amz-Content-Sha256": payloadSHA256, + }, + region: "", + expected: ErrSignatureDoesNotMatch, + }, + // (10) Should error with signature does not match. But handles + // query params which do not precede with "x-amz-" header. + { + 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, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()), + "X-Amz-Content-Sha256": payloadSHA256, + "response-content-type": "application/json", + }, + headers: map[string]string{ + "X-Amz-Date": now.Format(iso8601Format), + "X-Amz-Content-Sha256": payloadSHA256, + }, + region: "", + expected: ErrSignatureDoesNotMatch, + }, } // Run each test case individually. @@ -243,7 +306,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) { } // Check if it matches! - err := doesPresignedSignatureMatch(payload, req, testCase.region) + err := doesPresignedSignatureMatch(payloadSHA256, req, testCase.region) if err != testCase.expected { t.Errorf("(%d) expected to get %s, instead got %s", i, niceError(testCase.expected), niceError(err)) }