Return authHeaderMalformed for an incorrect region in signature (#5618)

cmd/signature-v4-parser.go

@@ -47,7 +47,7 @@ func (c credentialHeader) getScope() string {
 }
 
 // parse credentialHeader string into its structured form.
-func parseCredentialHeader(credElement string) (ch credentialHeader, aec APIErrorCode) {
+func parseCredentialHeader(credElement string, region string) (ch credentialHeader, aec APIErrorCode) {
 	creds := strings.Split(strings.TrimSpace(credElement), "=")
 	if len(creds) != 2 {
 		return ch, ErrMissingFields
@@ -71,7 +71,22 @@ func parseCredentialHeader(credElement string) (ch credentialHeader, aec APIErro
 	if e != nil {
 		return ch, ErrMalformedCredentialDate
 	}
+
 	cred.scope.region = credElements[2]
+	// Verify if region is valid.
+	sRegion := cred.scope.region
+	// Region is set to be empty, we use whatever was sent by the
+	// request and proceed further. This is a work-around to address
+	// an important problem for ListBuckets() getting signed with
+	// different regions.
+	if region == "" {
+		region = sRegion
+	}
+	// Should validate region, only if region is set.
+	if !isValidRegion(sRegion, region) {
+		return ch, ErrAuthorizationHeaderMalformed
+
+	}
 	if credElements[3] != "s3" {
 		return ch, ErrInvalidService
 	}
@@ -150,7 +165,7 @@ func doesV4PresignParamsExist(query url.Values) APIErrorCode {
 }
 
 // Parses all the presigned signature values into separate elements.
-func parsePreSignV4(query url.Values) (psv preSignValues, aec APIErrorCode) {
+func parsePreSignV4(query url.Values, region string) (psv preSignValues, aec APIErrorCode) {
 	var err APIErrorCode
 	// verify whether the required query params exist.
 	err = doesV4PresignParamsExist(query)
@@ -167,7 +182,7 @@ func parsePreSignV4(query url.Values) (psv preSignValues, aec APIErrorCode) {
 	preSignV4Values := preSignValues{}
 
 	// Save credential.
-	preSignV4Values.Credential, err = parseCredentialHeader("Credential=" + query.Get("X-Amz-Credential"))
+	preSignV4Values.Credential, err = parseCredentialHeader("Credential="+query.Get("X-Amz-Credential"), region)
 	if err != ErrNone {
 		return psv, err
 	}
@@ -193,6 +208,7 @@ func parsePreSignV4(query url.Values) (psv preSignValues, aec APIErrorCode) {
 	if preSignV4Values.Expires.Seconds() > 604800 {
 		return psv, ErrMaximumExpires
 	}
+
 	// Save signed headers.
 	preSignV4Values.SignedHeaders, err = parseSignedHeader("SignedHeaders=" + query.Get("X-Amz-SignedHeaders"))
 	if err != ErrNone {
@@ -214,7 +230,7 @@ func parsePreSignV4(query url.Values) (psv preSignValues, aec APIErrorCode) {
 //    Authorization: algorithm Credential=accessKeyID/credScope, \
 //            SignedHeaders=signedHeaders, Signature=signature
 //
-func parseSignV4(v4Auth string) (sv signValues, aec APIErrorCode) {
+func parseSignV4(v4Auth string, region string) (sv signValues, aec APIErrorCode) {
 	// Replace all spaced strings, some clients can send spaced
 	// parameters and some won't. So we pro-actively remove any spaces
 	// to make parsing easier.
@@ -240,7 +256,7 @@ func parseSignV4(v4Auth string) (sv signValues, aec APIErrorCode) {
 
 	var err APIErrorCode
 	// Save credentail values.
-	signV4Values.Credential, err = parseCredentialHeader(authFields[0])
+	signV4Values.Credential, err = parseCredentialHeader(authFields[0], region)
 	if err != ErrNone {
 		return sv, err
 	}

cmd/signature-v4-parser_test.go

@@ -154,6 +154,18 @@ func TestParseCredentialHeader(t *testing.T) {
 			expectedErrCode:     ErrInvalidService,
 		},
 		// Test Case - 7.
+		// Test case with invalid region.
+		{
+			inputCredentialStr: generateCredentialStr(
+				"Z7IXGOO6BZ0REAN1Q26I",
+				UTCNow().Format(yyyymmdd),
+				"us-west-2",
+				"s3",
+				"aws4_request"),
+			expectedCredentials: credentialHeader{},
+			expectedErrCode:     ErrAuthorizationHeaderMalformed,
+		},
+		// Test Case - 8.
 		// Test case with invalid request version.
 		// "aws4_request" is the valid request version.
 		{
@@ -166,7 +178,7 @@ func TestParseCredentialHeader(t *testing.T) {
 			expectedCredentials: credentialHeader{},
 			expectedErrCode:     ErrInvalidRequestVersion,
 		},
-		// Test Case - 8.
+		// Test Case - 9.
 		// Test case with right inputs. Expected to return a valid CredentialHeader.
 		// "aws4_request" is the valid request version.
 		{
@@ -188,7 +200,7 @@ func TestParseCredentialHeader(t *testing.T) {
 	}
 
 	for i, testCase := range testCases {
-		actualCredential, actualErrCode := parseCredentialHeader(testCase.inputCredentialStr)
+		actualCredential, actualErrCode := parseCredentialHeader(testCase.inputCredentialStr, "us-west-1")
 		// validating the credential fields.
 		if testCase.expectedErrCode != actualErrCode {
 			t.Fatalf("Test %d: Expected the APIErrCode to be %s, got %s", i+1, errorCodeResponse[testCase.expectedErrCode].Code, errorCodeResponse[actualErrCode].Code)
@@ -399,7 +411,6 @@ func TestParseSignV4(t *testing.T) {
 					// a valid signature is of form "Signature="
 					"Signature=abcd",
 				}, ","),
-
 			expectedAuthField: signValues{
 				Credential: generateCredentials(
 					t,
@@ -416,7 +427,7 @@ func TestParseSignV4(t *testing.T) {
 	}
 
 	for i, testCase := range testCases {
-		parsedAuthField, actualErrCode := parseSignV4(testCase.inputV4AuthStr)
+		parsedAuthField, actualErrCode := parseSignV4(testCase.inputV4AuthStr, "")
 
 		if testCase.expectedErrCode != actualErrCode {
 			t.Fatalf("Test %d: Expected the APIErrCode to be %d, got %d", i+1, testCase.expectedErrCode, actualErrCode)
@@ -783,7 +794,7 @@ func TestParsePreSignV4(t *testing.T) {
 			inputQuery.Set(testCase.inputQueryKeyVals[j], testCase.inputQueryKeyVals[j+1])
 		}
 		// call the function under test.
-		parsedPreSign, actualErrCode := parsePreSignV4(inputQuery)
+		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.go

@@ -167,7 +167,7 @@ func doesPolicySignatureV4Match(formValues http.Header) APIErrorCode {
 	region := globalServerConfig.GetRegion()
 
 	// Parse credential tag.
-	credHeader, err := parseCredentialHeader("Credential=" + formValues.Get("X-Amz-Credential"))
+	credHeader, err := parseCredentialHeader("Credential="+formValues.Get("X-Amz-Credential"), region)
 	if err != ErrNone {
 		return ErrMissingFields
 	}
@@ -177,14 +177,8 @@ func doesPolicySignatureV4Match(formValues http.Header) APIErrorCode {
 		return ErrInvalidAccessKeyID
 	}
 
-	// Verify if the region is valid.
-	sRegion := credHeader.scope.region
-	if !isValidRegion(sRegion, region) {
-		return ErrInvalidRegion
-	}
-
 	// Get signing key.
-	signingKey := getSigningKey(cred.SecretKey, credHeader.scope.date, sRegion)
+	signingKey := getSigningKey(cred.SecretKey, credHeader.scope.date, credHeader.scope.region)
 
 	// Get signature.
 	newSignature := getSignature(signingKey, formValues.Get("Policy"))
@@ -209,7 +203,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 	req := *r
 
 	// Parse request query string.
-	pSignValues, err := parsePreSignV4(req.URL.Query())
+	pSignValues, err := parsePreSignV4(req.URL.Query(), region)
 	if err != ErrNone {
 		return err
 	}
@@ -219,16 +213,6 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 		return ErrInvalidAccessKeyID
 	}
 
-	// Verify if region is valid.
-	sRegion := pSignValues.Credential.scope.region
-	// Should validate region, only if region is set.
-	if region == "" {
-		region = sRegion
-	}
-	if !isValidRegion(sRegion, region) {
-		return ErrInvalidRegion
-	}
-
 	// Extract all the signed headers along with its values.
 	extractedSignedHeaders, errCode := extractSignedHeaders(pSignValues.SignedHeaders, r)
 	if errCode != ErrNone {
@@ -260,7 +244,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 	query.Set("X-Amz-Date", t.Format(iso8601Format))
 	query.Set("X-Amz-Expires", strconv.Itoa(expireSeconds))
 	query.Set("X-Amz-SignedHeaders", getSignedHeaders(extractedSignedHeaders))
-	query.Set("X-Amz-Credential", cred.AccessKey+"/"+getScope(t, sRegion))
+	query.Set("X-Amz-Credential", cred.AccessKey+"/"+getScope(t, pSignValues.Credential.scope.region))
 
 	// Save other headers available in the request parameters.
 	for k, v := range req.URL.Query() {
@@ -292,7 +276,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 	// Verify if sha256 payload query is same.
 	if req.URL.Query().Get("X-Amz-Content-Sha256") != "" {
 		if req.URL.Query().Get("X-Amz-Content-Sha256") != query.Get("X-Amz-Content-Sha256") {
-			return ErrSignatureDoesNotMatch
+			return ErrContentSHA256Mismatch
 		}
 	}
 
@@ -305,7 +289,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s
 	presignedStringToSign := getStringToSign(presignedCanonicalReq, t, pSignValues.Credential.getScope())
 
 	// Get hmac presigned signing key.
-	presignedSigningKey := getSigningKey(cred.SecretKey, pSignValues.Credential.scope.date, region)
+	presignedSigningKey := getSigningKey(cred.SecretKey, pSignValues.Credential.scope.date, pSignValues.Credential.scope.region)
 
 	// Get new signature.
 	newSignature := getSignature(presignedSigningKey, presignedStringToSign)
@@ -331,7 +315,7 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP
 	v4Auth := req.Header.Get("Authorization")
 
 	// Parse signature version '4' header.
-	signV4Values, err := parseSignV4(v4Auth)
+	signV4Values, err := parseSignV4(v4Auth, region)
 	if err != ErrNone {
 		return err
 	}
@@ -347,20 +331,6 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP
 		return ErrInvalidAccessKeyID
 	}
 
-	// Verify if region is valid.
-	sRegion := signV4Values.Credential.scope.region
-	// Region is set to be empty, we use whatever was sent by the
-	// request and proceed further. This is a work-around to address
-	// an important problem for ListBuckets() getting signed with
-	// different regions.
-	if region == "" {
-		region = sRegion
-	}
-	// Should validate region, only if region is set.
-	if !isValidRegion(sRegion, region) {
-		return ErrInvalidRegion
-	}
-
 	// Extract date, if not present throw error.
 	var date string
 	if date = req.Header.Get(http.CanonicalHeaderKey("x-amz-date")); date == "" {
@@ -384,7 +354,7 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP
 	stringToSign := getStringToSign(canonicalRequest, t, signV4Values.Credential.getScope())
 
 	// Get hmac signing key.
-	signingKey := getSigningKey(cred.SecretKey, signV4Values.Credential.scope.date, region)
+	signingKey := getSigningKey(cred.SecretKey, signV4Values.Credential.scope.date, signV4Values.Credential.scope.region)
 
 	// Calculate signature.
 	newSignature := getSignature(signingKey, stringToSign)

cmd/streaming-signature-v4.go

@@ -70,9 +70,6 @@ func calculateSeedSignature(r *http.Request) (signature string, region string, d
 	// Access credentials.
 	cred := globalServerConfig.GetCredential()
 
-	// Configured region.
-	confRegion := globalServerConfig.GetRegion()
-
 	// Copy request.
 	req := *r
 
@@ -80,7 +77,7 @@ func calculateSeedSignature(r *http.Request) (signature string, region string, d
 	v4Auth := req.Header.Get("Authorization")
 
 	// Parse signature version '4' header.
-	signV4Values, errCode := parseSignV4(v4Auth)
+	signV4Values, errCode := parseSignV4(v4Auth, globalServerConfig.GetRegion())
 	if errCode != ErrNone {
 		return "", "", time.Time{}, errCode
 	}
@@ -105,11 +102,6 @@ func calculateSeedSignature(r *http.Request) (signature string, region string, d
 
 	// Verify if region is valid.
 	region = signV4Values.Credential.scope.region
-	// Should validate region, only if region is set. Some operations
-	// do not need region validated for example GetBucketLocation.
-	if !isValidRegion(region, confRegion) {
-		return "", "", time.Time{}, ErrInvalidRegion
-	}
 
 	// Extract date, if not present throw error.
 	var dateStr string