tests: Improve coverage on signature v4 tests. (#3188)

Fixes #3065

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.
+// Parse slice of signed headers from signed headers tag.
-func parseSignedHeaders(signedHdrElement string) ([]string, APIErrorCode) {
+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
 	}

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 "<int>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)
 		}

cmd/signature-v4-utils.go

@@ -86,14 +86,13 @@ func getURLEncodedName(name string) string {
 			continue
 		default:
 			len := utf8.RuneLen(s)
-			if len < 0 {
+			if len > 0 {
-				return name
+				u := make([]byte, len)
-			}
+				utf8.EncodeRune(u, s)
-			u := make([]byte, len)
+				for _, r := range u {
-			utf8.EncodeRune(u, s)
+					hex := hex.EncodeToString([]byte{r})
-			for _, r := range u {
+					encodedName = encodedName + "%" + strings.ToUpper(hex)
-				hex := hex.EncodeToString([]byte{r})
+				}
-				encodedName = encodedName + "%" + strings.ToUpper(hex)
 			}
 		}
 	}

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)

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))
 		}