signature: use region from Auth header if server's region not configured (#4329)

cmd/bucket-notification-utils.go

@@ -17,6 +17,7 @@
 package cmd
 
 import (
+	"errors"
 	"strings"
 
 	"github.com/minio/minio-go/pkg/set"
@@ -111,19 +112,20 @@ func checkARN(arn, arnType string) APIErrorCode {
 	if !strings.HasPrefix(arn, arnType) {
 		return ErrARNNotification
 	}
-	if !strings.HasPrefix(arn, arnType+serverConfig.GetRegion()+":") {
+	strs := strings.SplitN(arn, ":", -1)
+	if len(strs) != 6 {
+		return ErrARNNotification
+	}
+	if serverConfig.GetRegion() != "" {
+		region := strs[3]
+		if region != serverConfig.GetRegion() {
 			return ErrRegionNotification
 		}
-	account := strings.SplitN(strings.TrimPrefix(arn, arnType+serverConfig.GetRegion()+":"), ":", 2)
-	switch len(account) {
-	case 1:
-		// This means ARN is malformed, account should have min of 2elements.
-		return ErrARNNotification
-	case 2:
-		// Account topic id or topic name cannot be empty.
-		if account[0] == "" || account[1] == "" {
-			return ErrARNNotification
 	}
+	accountID := strs[4]
+	resource := strs[5]
+	if accountID == "" || resource == "" {
+		return ErrARNNotification
 	}
 	return ErrNone
 }
@@ -258,28 +260,39 @@ func validateNotificationConfig(nConfig notificationConfig) APIErrorCode {
 // - webhook
 func unmarshalSqsARN(queueARN string) (mSqs arnSQS) {
 	mSqs = arnSQS{}
-	if !strings.HasPrefix(queueARN, minioSqs+serverConfig.GetRegion()+":") {
+	strs := strings.SplitN(queueARN, ":", -1)
+	if len(strs) != 6 {
 		return mSqs
 	}
-	sqsType := strings.TrimPrefix(queueARN, minioSqs+serverConfig.GetRegion()+":")
-	switch {
-	case hasSuffix(sqsType, queueTypeAMQP):
+	if serverConfig.GetRegion() != "" {
+		region := strs[3]
+		if region != serverConfig.GetRegion() {
+			return mSqs
+		}
+	}
+	sqsType := strs[5]
+	switch sqsType {
+	case queueTypeAMQP:
 		mSqs.Type = queueTypeAMQP
-	case hasSuffix(sqsType, queueTypeNATS):
+	case queueTypeNATS:
 		mSqs.Type = queueTypeNATS
-	case hasSuffix(sqsType, queueTypeElastic):
+	case queueTypeElastic:
 		mSqs.Type = queueTypeElastic
-	case hasSuffix(sqsType, queueTypeRedis):
+	case queueTypeRedis:
 		mSqs.Type = queueTypeRedis
-	case hasSuffix(sqsType, queueTypePostgreSQL):
+	case queueTypePostgreSQL:
 		mSqs.Type = queueTypePostgreSQL
-	case hasSuffix(sqsType, queueTypeMySQL):
+	case queueTypeMySQL:
 		mSqs.Type = queueTypeMySQL
-	case hasSuffix(sqsType, queueTypeKafka):
+	case queueTypeKafka:
 		mSqs.Type = queueTypeKafka
-	case hasSuffix(sqsType, queueTypeWebhook):
+	case queueTypeWebhook:
 		mSqs.Type = queueTypeWebhook
+	default:
+		errorIf(errors.New("invalid SQS type"), "SQS type: %s", sqsType)
 	} // Add more queues here.
-	mSqs.AccountID = strings.TrimSuffix(sqsType, ":"+mSqs.Type)
+
+	mSqs.AccountID = strs[4]
+
 	return mSqs
 }

cmd/bucket-notification-utils_test.go

@@ -259,11 +259,6 @@ func TestQueueARN(t *testing.T) {
 			queueARN: "arn:minio:sns:us-east-1:1:listen",
 			errCode:  ErrARNNotification,
 		},
-		// Invalid region 'us-west-1' in queue arn.
-		{
-			queueARN: "arn:minio:sqs:us-west-1:1:redis",
-			errCode:  ErrRegionNotification,
-		},
 		// Invalid queue name empty in queue arn.
 		{
 			queueARN: "arn:minio:sqs:us-east-1:1:",
@@ -298,6 +293,37 @@ func TestQueueARN(t *testing.T) {
 			t.Errorf("Test %d: Expected \"%d\", got \"%d\"", i+1, testCase.errCode, errCode)
 		}
 	}
+
+	// Test when server region is set.
+	rootPath, err = newTestConfig("us-east-1")
+	if err != nil {
+		t.Fatalf("unable initialize config file, %s", err)
+	}
+	defer removeAll(rootPath)
+
+	testCases = []struct {
+		queueARN string
+		errCode  APIErrorCode
+	}{
+		// Incorrect region should produce error.
+		{
+			queueARN: "arn:minio:sqs:us-west-1:1:webhook",
+			errCode:  ErrRegionNotification,
+		},
+		// Correct region should not produce error.
+		{
+			queueARN: "arn:minio:sqs:us-east-1:1:webhook",
+			errCode:  ErrNone,
+		},
+	}
+
+	// Validate all tests for queue arn.
+	for i, testCase := range testCases {
+		errCode := checkQueueARN(testCase.queueARN)
+		if testCase.errCode != errCode {
+			t.Errorf("Test %d: Expected \"%d\", got \"%d\"", i+1, testCase.errCode, errCode)
+		}
+	}
 }
 
 // Test unmarshal queue arn.
@@ -337,11 +363,6 @@ func TestUnmarshalSQSARN(t *testing.T) {
 			queueARN: "",
 			Type:     "",
 		},
-		// Invalid region 'us-west-1' in queue arn.
-		{
-			queueARN: "arn:minio:sqs:us-west-1:1:redis",
-			Type:     "",
-		},
 		// Partial queue arn.
 		{
 			queueARN: "arn:minio:sqs:",
@@ -361,4 +382,33 @@ func TestUnmarshalSQSARN(t *testing.T) {
 		}
 	}
 
+	// Test when the server region is set.
+	rootPath, err = newTestConfig("us-east-1")
+	if err != nil {
+		t.Fatalf("unable initialize config file, %s", err)
+	}
+	defer removeAll(rootPath)
+
+	testCases = []struct {
+		queueARN string
+		Type     string
+	}{
+		// Incorrect region in ARN returns empty mSqs.Type
+		{
+			queueARN: "arn:minio:sqs:us-west-1:1:webhook",
+			Type:     "",
+		},
+		// Correct regionin ARN returns valid mSqs.Type
+		{
+			queueARN: "arn:minio:sqs:us-east-1:1:webhook",
+			Type:     "webhook",
+		},
+	}
+
+	for i, testCase := range testCases {
+		mSqs := unmarshalSqsARN(testCase.queueARN)
+		if testCase.Type != mSqs.Type {
+			t.Errorf("Test %d: Expected \"%s\", got \"%s\"", i+1, testCase.Type, mSqs.Type)
+		}
+	}
 }

cmd/config-v18.go

@@ -261,11 +261,6 @@ func getValidConfig() (*serverConfigV18, error) {
 		return nil, err
 	}
 
-	// Validate region field
-	if srvCfg.Region == "" {
-		return nil, errors.New("Region config value cannot be empty")
-	}
-
 	// Validate credential fields only when
 	// they are not set via the environment
 

cmd/globals.go

@@ -29,7 +29,7 @@ import (
 const (
 	globalMinioCertExpireWarnDays = time.Hour * 24 * 30 // 30 days.
 
-	globalMinioDefaultRegion       = "us-east-1"
+	globalMinioDefaultRegion       = ""
 	globalMinioDefaultOwnerID      = "minio"
 	globalMinioDefaultStorageClass = "STANDARD"
 	globalWindowsOSName            = "windows"

cmd/handler-utils.go

@@ -38,7 +38,7 @@ func parseLocationConstraint(r *http.Request) (location string, s3Error APIError
 	} // else for both err as nil or io.EOF
 	location = locationConstraint.Location
 	if location == "" {
-		location = globalMinioDefaultRegion
+		location = serverConfig.GetRegion()
 	}
 	return location, ErrNone
 }
@@ -46,7 +46,7 @@ func parseLocationConstraint(r *http.Request) (location string, s3Error APIError
 // Validates input location is same as configured region
 // of Minio server.
 func isValidLocation(location string) bool {
-	return serverConfig.GetRegion() == location
+	return serverConfig.GetRegion() == "" || serverConfig.GetRegion() == location
 }
 
 // Supported headers that needs to be extracted.

cmd/post-policy_test.go

@@ -246,6 +246,7 @@ func testPostPolicyBucketHandler(obj ObjectLayer, instanceType string, t TestErr
 		}
 	}
 
+	region := "us-east-1"
 	// Test cases for signature-V4.
 	testCasesV4BadData := []struct {
 		objectName         string
@@ -330,7 +331,7 @@ func testPostPolicyBucketHandler(obj ObjectLayer, instanceType string, t TestErr
 		testCase.policy = fmt.Sprintf(testCase.policy, testCase.dates...)
 
 		req, perr := newPostRequestV4Generic("", bucketName, testCase.objectName, testCase.data, testCase.accessKey,
-			testCase.secretKey, curTime, []byte(testCase.policy), nil, testCase.corruptedBase64, testCase.corruptedMultipart)
+			testCase.secretKey, region, curTime, []byte(testCase.policy), nil, testCase.corruptedBase64, testCase.corruptedMultipart)
 		if perr != nil {
 			t.Fatalf("Test %d: %s: Failed to create HTTP request for PostPolicyHandler: <ERROR> %v", i+1, instanceType, perr)
 		}
@@ -473,9 +474,10 @@ func testPostPolicyBucketHandlerRedirect(obj ObjectLayer, instanceType string, t
 	// Generate the final policy document
 	policy = fmt.Sprintf(policy, dates...)
 
+	region := "us-east-1"
 	// Create a new POST request with success_action_redirect field specified
 	req, perr := newPostRequestV4Generic("", bucketName, keyName, []byte("objData"),
-		credentials.AccessKey, credentials.SecretKey, curTime,
+		credentials.AccessKey, credentials.SecretKey, region, curTime,
 		[]byte(policy), map[string]string{"success_action_redirect": redirectURL.String()}, false, false)
 
 	if perr != nil {
@@ -565,11 +567,11 @@ func newPostRequestV2(endPoint, bucketName, objectName string, accessKey, secret
 	return req, nil
 }
 
-func buildGenericPolicy(t time.Time, accessKey, bucketName, objectName string, contentLengthRange bool) []byte {
+func buildGenericPolicy(t time.Time, accessKey, region, bucketName, objectName string, contentLengthRange bool) []byte {
 	// Expire the request five minutes from now.
 	expirationTime := t.Add(time.Minute * 5)
 
-	credStr := getCredentialString(accessKey, serverConfig.GetRegion(), t)
+	credStr := getCredentialString(accessKey, region, t)
 	// Create a new post policy.
 	policy := newPostPolicyBytesV4(credStr, bucketName, objectName, expirationTime)
 	if contentLengthRange {
@@ -578,10 +580,10 @@ func buildGenericPolicy(t time.Time, accessKey, bucketName, objectName string, c
 	return policy
 }
 
-func newPostRequestV4Generic(endPoint, bucketName, objectName string, objData []byte, accessKey, secretKey string,
+func newPostRequestV4Generic(endPoint, bucketName, objectName string, objData []byte, accessKey, secretKey string, region string,
 	t time.Time, policy []byte, addFormData map[string]string, corruptedB64 bool, corruptedMultipart bool) (*http.Request, error) {
 	// Get the user credential.
-	credStr := getCredentialString(accessKey, serverConfig.GetRegion(), t)
+	credStr := getCredentialString(accessKey, region, t)
 
 	// Only need the encoding.
 	encodedPolicy := base64.StdEncoding.EncodeToString(policy)
@@ -591,7 +593,7 @@ func newPostRequestV4Generic(endPoint, bucketName, objectName string, objData []
 	}
 
 	// Presign with V4 signature based on the policy.
-	signature := postPresignSignatureV4(encodedPolicy, t, secretKey, serverConfig.GetRegion())
+	signature := postPresignSignatureV4(encodedPolicy, t, secretKey, region)
 
 	formData := map[string]string{
 		"bucket":           bucketName,
@@ -645,12 +647,14 @@ func newPostRequestV4Generic(endPoint, bucketName, objectName string, objData []
 
 func newPostRequestV4WithContentLength(endPoint, bucketName, objectName string, objData []byte, accessKey, secretKey string) (*http.Request, error) {
 	t := UTCNow()
-	policy := buildGenericPolicy(t, accessKey, bucketName, objectName, true)
-	return newPostRequestV4Generic(endPoint, bucketName, objectName, objData, accessKey, secretKey, t, policy, nil, false, false)
+	region := "us-east-1"
+	policy := buildGenericPolicy(t, accessKey, region, bucketName, objectName, true)
+	return newPostRequestV4Generic(endPoint, bucketName, objectName, objData, accessKey, secretKey, region, t, policy, nil, false, false)
 }
 
 func newPostRequestV4(endPoint, bucketName, objectName string, objData []byte, accessKey, secretKey string) (*http.Request, error) {
 	t := UTCNow()
-	policy := buildGenericPolicy(t, accessKey, bucketName, objectName, false)
-	return newPostRequestV4Generic(endPoint, bucketName, objectName, objData, accessKey, secretKey, t, policy, nil, false, false)
+	region := "us-east-1"
+	policy := buildGenericPolicy(t, accessKey, region, bucketName, objectName, false)
+	return newPostRequestV4Generic(endPoint, bucketName, objectName, objData, accessKey, secretKey, region, t, policy, nil, false, false)
 }

cmd/server-startup-msg.go

@@ -79,7 +79,9 @@ func printServerCommonMsg(apiEndpoints []string) {
 	log.Println(colorBlue("\nEndpoint: ") + colorBold(fmt.Sprintf(getFormatStr(len(apiEndpointStr), 1), apiEndpointStr)))
 	log.Println(colorBlue("AccessKey: ") + colorBold(fmt.Sprintf("%s ", cred.AccessKey)))
 	log.Println(colorBlue("SecretKey: ") + colorBold(fmt.Sprintf("%s ", cred.SecretKey)))
+	if region != "" {
 		log.Println(colorBlue("Region: ") + colorBold(fmt.Sprintf(getFormatStr(len(region), 3), region)))
+	}
 	printEventNotifiers()
 
 	log.Println(colorBlue("\nBrowser Access:"))
@@ -92,12 +94,12 @@ func printEventNotifiers() {
 		// In case initEventNotifier() was not done or failed.
 		return
 	}
-	arnMsg := colorBlue("SQS ARNs: ")
 	// Get all configured external notification targets
 	externalTargets := globalEventNotifier.GetAllExternalTargets()
 	if len(externalTargets) == 0 {
-		arnMsg += colorBold(fmt.Sprintf(getFormatStr(len("<none>"), 1), "<none>"))
+		return
 	}
+	arnMsg := colorBlue("SQS ARNs: ")
 	for queueArn := range externalTargets {
 		arnMsg += colorBold(fmt.Sprintf(getFormatStr(len(queueArn), 1), queueArn))
 	}

cmd/signature-v4-parser.go

@@ -69,9 +69,6 @@ func parseCredentialHeader(credElement string) (credentialHeader, APIErrorCode)
 	if e != nil {
 		return credentialHeader{}, ErrMalformedCredentialDate
 	}
-	if credElements[2] == "" {
-		return credentialHeader{}, ErrMalformedCredentialRegion
-	}
 	cred.scope.region = credElements[2]
 	if credElements[3] != "s3" {
 		return credentialHeader{}, ErrInvalidService

cmd/signature-v4-parser_test.go

@@ -141,19 +141,6 @@ func TestParseCredentialHeader(t *testing.T) {
 			expectedErrCode:     ErrMalformedCredentialDate,
 		},
 		// Test Case - 6.
-		// Test case with invalid region.
-		// region should a non empty string.
-		{
-			inputCredentialStr: generateCredentialStr(
-				"Z7IXGOO6BZ0REAN1Q26I",
-				UTCNow().Format(yyyymmdd),
-				"",
-				"ABCD",
-				"ABCD"),
-			expectedCredentials: credentialHeader{},
-			expectedErrCode:     ErrMalformedCredentialRegion,
-		},
-		// Test Case - 7.
 		// Test case with invalid service.
 		// "s3" is the valid service string.
 		{
@@ -166,7 +153,7 @@ func TestParseCredentialHeader(t *testing.T) {
 			expectedCredentials: credentialHeader{},
 			expectedErrCode:     ErrInvalidService,
 		},
-		// Test Case - 8.
+		// Test Case - 7.
 		// Test case with invalid request version.
 		// "aws4_request" is the valid request version.
 		{
@@ -179,7 +166,7 @@ func TestParseCredentialHeader(t *testing.T) {
 			expectedCredentials: credentialHeader{},
 			expectedErrCode:     ErrInvalidRequestVersion,
 		},
-		// Test Case - 9.
+		// Test Case - 8.
 		// Test case with right inputs. Expected to return a valid CredentialHeader.
 		// "aws4_request" is the valid request version.
 		{
@@ -204,7 +191,7 @@ func TestParseCredentialHeader(t *testing.T) {
 		actualCredential, actualErrCode := parseCredentialHeader(testCase.inputCredentialStr)
 		// validating the credential fields.
 		if testCase.expectedErrCode != actualErrCode {
-			t.Fatalf("Test %d: Expected the APIErrCode to be %d, got %d", i+1, testCase.expectedErrCode, actualErrCode)
+			t.Fatalf("Test %d: Expected the APIErrCode to be %s, got %s", i+1, errorCodeResponse[testCase.expectedErrCode].Code, errorCodeResponse[actualErrCode].Code)
 		}
 		if actualErrCode == ErrNone {
 			validateCredentialfields(t, i+1, testCase.expectedCredentials, actualCredential)

cmd/signature-v4-utils.go

@@ -68,7 +68,10 @@ func getContentSha256Cksum(r *http.Request) string {
 
 // isValidRegion - verify if incoming region value is valid with configured Region.
 func isValidRegion(reqRegion string, confRegion string) bool {
-	if confRegion == "" || confRegion == "US" {
+	if confRegion == "" {
+		return true
+	}
+	if confRegion == "US" {
 		confRegion = globalMinioDefaultRegion
 	}
 	// Some older s3 clients set region as "US" instead of

cmd/signature-v4-utils_test.go

@@ -80,7 +80,7 @@ func TestIsValidRegion(t *testing.T) {
 		expectedResult bool
 	}{
 
-		{"", "", false},
+		{"", "", true},
 		{globalMinioDefaultRegion, "", true},
 		{globalMinioDefaultRegion, "US", true},
 		{"us-west-1", "US", false},

cmd/signature-v4.go

@@ -175,7 +175,7 @@ func doesPolicySignatureV4Match(formValues http.Header) APIErrorCode {
 	}
 
 	// Get signing key.
-	signingKey := getSigningKey(cred.SecretKey, credHeader.scope.date, region)
+	signingKey := getSigningKey(cred.SecretKey, credHeader.scope.date, sRegion)
 
 	// Get signature.
 	newSignature := getSignature(signingKey, formValues.Get("Policy"))

cmd/signature-v4_test.go

@@ -54,14 +54,7 @@ func TestDoesPolicySignatureMatch(t *testing.T) {
 			},
 			expected: ErrInvalidAccessKeyID,
 		},
-		// (2) It should fail if the region is invalid.
-		{
-			form: http.Header{
-				"X-Amz-Credential": []string{fmt.Sprintf(credentialTemplate, accessKey, now.Format(yyyymmdd), "invalidregion")},
-			},
-			expected: ErrInvalidRegion,
-		},
-		// (3) It should fail with a bad signature.
+		// (2) It should fail with a bad signature.
 		{
 			form: http.Header{
 				"X-Amz-Credential": []string{fmt.Sprintf(credentialTemplate, accessKey, now.Format(yyyymmdd), globalMinioDefaultRegion)},
@@ -71,7 +64,7 @@ func TestDoesPolicySignatureMatch(t *testing.T) {
 			},
 			expected: ErrSignatureDoesNotMatch,
 		},
-		// (4) It should succeed if everything is correct.
+		// (3) It should succeed if everything is correct.
 		{
 			form: http.Header{
 				"X-Amz-Credential": []string{
@@ -135,21 +128,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "us-west-1",
 			expected: ErrInvalidAccessKeyID,
 		},
-		// (2) Should fail with an invalid region.
-		{
-			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": payloadSHA256,
-			},
-			region:   globalMinioDefaultRegion,
-			expected: ErrInvalidRegion,
-		},
-		// (3) Should NOT fail with an invalid region if it doesn't verify it.
+		// (2) Should NOT fail with an invalid region if it doesn't verify it.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -163,7 +142,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "us-west-1",
 			expected: ErrUnsignedHeaders,
 		},
-		// (4) Should fail to extract headers if the host header is not signed.
+		// (3) Should fail to extract headers if the host header is not signed.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -177,7 +156,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrUnsignedHeaders,
 		},
-		// (5) Should give an expired request if it has expired.
+		// (4) Should give an expired request if it has expired.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -195,7 +174,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrExpiredPresignRequest,
 		},
-		// (6) Should error if the signature is incorrect.
+		// (5) Should error if the signature is incorrect.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":      signV4Algorithm,
@@ -213,7 +192,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrSignatureDoesNotMatch,
 		},
-		// (7) Should error if the request is not ready yet, ie X-Amz-Date is in the future.
+		// (6) 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,
@@ -231,7 +210,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   region,
 			expected: ErrRequestNotReadyYet,
 		},
-		// (8) Should not error with invalid region instead, call should proceed
+		// (7) Should not error with invalid region instead, call should proceed
 		// with sigature does not match.
 		{
 			queryParams: map[string]string{
@@ -250,7 +229,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "",
 			expected: ErrSignatureDoesNotMatch,
 		},
-		// (9) Should error with signature does not match. But handles
+		// (8) Should error with signature does not match. But handles
 		// query params which do not precede with "x-amz-" header.
 		{
 			queryParams: map[string]string{
@@ -270,7 +249,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "",
 			expected: ErrSignatureDoesNotMatch,
 		},
-		// (10) Should error with unsigned headers.
+		// (9) Should error with unsigned headers.
 		{
 			queryParams: map[string]string{
 				"X-Amz-Algorithm":       signV4Algorithm,