fix: reject a ListBuckets bucket-region other than the gateway region

S3 never lists an empty result for another region: a valid region
other than the endpoint's own gets 400 InvalidArgument, "Requests with
bucket-region specified must be made to the corresponding regional
endpoint", with no ArgumentValue. The gateway is a single regional
endpoint, so it now does the same instead of clearing the listing.

Match the region case-insensitively like S3, and check an empty
bucket-region= too by looking for the query argument itself, since an
empty value is not a valid AWS region either.

s3api/controllers/bucket-list.go

@@ -19,7 +19,6 @@ import (
 	"github.com/versity/versitygw/auth"
 	"github.com/versity/versitygw/debuglogger"
 	"github.com/versity/versitygw/s3api/utils"
-	"github.com/versity/versitygw/s3err"
 	"github.com/versity/versitygw/s3response"
 )
 
@@ -58,11 +57,14 @@ func (c S3ApiController) ListBuckets(ctx fiber.Ctx) (*Response, error) {
 		}, err
 	}
 
-	if bucketRegion != "" && !utils.IsValidBucketRegion(bucketRegion, region) {
+	// an empty bucket-region= is checked too, so look for the argument itself
-		debuglogger.Logf("invalid bucket region: %s", bucketRegion)
+	if ctx.Request().URI().QueryArgs().Has("bucket-region") {
+		if err := utils.ValidateBucketRegion(bucketRegion, region); err != nil {
+			debuglogger.Logf("invalid bucket region: %q", bucketRegion)
 			return &Response{
 				MetaOpts: &MetaOptions{},
-		}, s3err.GetInvalidArgBucketRegion(bucketRegion)
+			}, err
+		}
 	}
 
 	// IsAdmin is the backends' "return every bucket, unfiltered" flag.
@@ -78,13 +80,6 @@ func (c S3ApiController) ListBuckets(ctx fiber.Ctx) (*Response, error) {
 		return &Response{}, err
 	}
 
-	// every bucket lives in the gateway region, so there is nothing
-	// to list for any other region
-	if bucketRegion != "" && bucketRegion != region {
-		res.Buckets.Bucket = nil
-		res.ContinuationToken = ""
-	}
-
 	for i := range res.Buckets.Bucket {
 		res.Buckets.Bucket[i].BucketRegion = region
 	}

s3api/controllers/bucket-list_test.go

@@ -119,25 +119,46 @@ func TestS3ApiController_ListBuckets(t *testing.T) {
 			name: "bucket region of another region",
 			input: testInput{
 				locals: defaultLocals,
-				beRes: s3response.ListAllMyBucketsResult{
+				beRes:  validRes,
-					Owner:   validRes.Owner,
-					Buckets: validRes.Buckets,
-					Prefix:  "te",
-					// a continuation token from the backend must not
-					// leak into the empty listing
-					ContinuationToken: "test",
-				},
 				queries: map[string]string{
 					"bucket-region": "us-west-2",
-					"prefix":        "te",
 				},
 			},
 			output: testOutput{
 				response: &Response{
-					Data: s3response.ListAllMyBucketsResult{
+					MetaOpts: &MetaOptions{},
-						Owner:  validRes.Owner,
-						Prefix: "te",
 				},
+				err: s3err.GetInvalidArgumentErr(s3err.InvalidArgBucketRegionEndpoint, ""),
+			},
+		},
+		{
+			name: "empty bucket region",
+			input: testInput{
+				locals: defaultLocals,
+				beRes:  validRes,
+				queries: map[string]string{
+					"bucket-region": "",
+				},
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{},
+				},
+				err: s3err.GetInvalidArgBucketRegion(""),
+			},
+		},
+		{
+			name: "bucket region of the gateway region in uppercase",
+			input: testInput{
+				locals: defaultLocals,
+				beRes:  validRes,
+				queries: map[string]string{
+					"bucket-region": "US-EAST-1",
+				},
+			},
+			output: testOutput{
+				response: &Response{
+					Data: validRes,
 				},
 			},
 		},

s3api/utils/utils.go

@@ -1155,11 +1155,17 @@ func ValidateLocationConstraint(constraint *string, region string) error {
 	return nil
 }
 
-// IsValidBucketRegion checks a ListBuckets bucket-region value. The gateway
+// ValidateBucketRegion checks a ListBuckets bucket-region value. Every bucket
-// region is valid whatever its name, any other value has to be an AWS region
+// lives in the gateway region, so like an S3 regional endpoint the gateway
-// code.
+// rejects any other region, matching it case-insensitively.
-func IsValidBucketRegion(bucketRegion, region string) bool {
+func ValidateBucketRegion(bucketRegion, region string) error {
-	return bucketRegion == region || awsRegionRegexp.MatchString(bucketRegion)
+	if strings.EqualFold(bucketRegion, region) {
+		return nil
+	}
+	if !awsRegionRegexp.MatchString(strings.ToLower(bucketRegion)) {
+		return s3err.GetInvalidArgBucketRegion(bucketRegion)
+	}
+	return s3err.GetInvalidArgumentErr(s3err.InvalidArgBucketRegionEndpoint, "")
 }
 
 // AwsChunkedEncoding is the coding a client announces when it frames a body in

s3api/utils/utils_test.go

@@ -1539,27 +1539,30 @@ func TestHasAwsChunkedEncoding(t *testing.T) {
 	}
 }
 
-func TestIsValidBucketRegion(t *testing.T) {
+func TestValidateBucketRegion(t *testing.T) {
+	endpointErr := s3err.GetInvalidArgumentErr(s3err.InvalidArgBucketRegionEndpoint, "")
 	tests := []struct {
 		name         string
 		bucketRegion string
 		region       string
-		want         bool
+		want         error
 	}{
-		{"gateway region", "us-east-1", "us-east-1", true},
+		{"gateway region", "us-east-1", "us-east-1", nil},
-		{"custom gateway region", "my-region", "my-region", true},
+		{"gateway region in uppercase", "EU-CENTRAL-1", "eu-central-1", nil},
-		{"another aws region", "eu-central-1", "us-east-1", true},
+		{"custom gateway region", "my-region", "my-region", nil},
-		{"gov cloud region", "us-gov-west-1", "us-east-1", true},
+		{"another aws region", "eu-central-1", "us-east-1", endpointErr},
-		{"iso partition region", "us-isob-east-1", "us-east-1", true},
+		{"another aws region in uppercase", "US-EAST-1", "eu-central-1", endpointErr},
-		{"sovereign cloud region", "eusc-de-east-1", "us-east-1", true},
+		{"gov cloud region", "us-gov-west-1", "us-east-1", endpointErr},
-		{"gibberish", "abc", "us-east-1", false},
+		{"iso partition region", "us-isob-east-1", "us-east-1", endpointErr},
-		{"missing number", "us-east", "us-east-1", false},
+		{"sovereign cloud region", "eusc-de-east-1", "us-east-1", endpointErr},
-		{"uppercase", "US-EAST-1", "us-east-1", false},
+		{"gibberish", "abc", "us-east-1", s3err.GetInvalidArgBucketRegion("abc")},
-		{"custom region of another gateway", "my-region", "us-east-1", false},
+		{"empty", "", "us-east-1", s3err.GetInvalidArgBucketRegion("")},
+		{"missing number", "us-east", "us-east-1", s3err.GetInvalidArgBucketRegion("us-east")},
+		{"custom region of another gateway", "my-region", "us-east-1", s3err.GetInvalidArgBucketRegion("my-region")},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equal(t, tt.want, IsValidBucketRegion(tt.bucketRegion, tt.region))
+			assert.Equal(t, tt.want, ValidateBucketRegion(tt.bucketRegion, tt.region))
 		})
 	}
 }

s3err/invalid-argument.go

@@ -74,9 +74,14 @@ const (
 	InvalidArgSSEInvalidEncryptionMethod
 	InvalidArgSSECRequiresTLS
 	InvalidArgUploadIdWithUploads
+	InvalidArgBucketRegionEndpoint
 )
 
 var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
+	InvalidArgBucketRegionEndpoint: {
+		Description:  "Requests with bucket-region specified must be made to the corresponding regional endpoint",
+		ArgumentName: "bucket-region",
+	},
 	InvalidArgMaxBuckets: {
 		Description:  "Argument max-buckets must be an integer between 1 and 10000.",
 		ArgumentName: "max-buckets",
@@ -386,9 +391,13 @@ func GetInvalidArgObjectOwnership(value string) InvalidArgumentError {
 }
 
 func GetInvalidArgBucketRegion(value string) InvalidArgumentError {
+	description := fmt.Sprintf("Argument value %s is not a valid AWS Region", value)
+	if value == "" {
+		description = "Argument value is not a valid AWS Region"
+	}
 	return InvalidArgumentError{
 		ArgumentName: "bucket-region",
 		// no ArgumentValue is returned for this error
-		Description: fmt.Sprintf("Argument value %s is not a valid AWS Region", value),
+		Description: description,
 	}
 }

tests/integration/ListBuckets.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"slices"
 	"sort"
+	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
@@ -308,21 +309,30 @@ func ListBuckets_bucket_region(s *S3Conf) error {
 				bucket, s.awsRegion, buckets)
 		}
 
-		// every bucket of the gateway lives in its own region
+		// the region is matched case-insensitively
+		buckets, err = listBuckets(strings.ToUpper(s.awsRegion))
+		if err != nil {
+			return err
+		}
+		if !slices.ContainsFunc(buckets, func(b types.Bucket) bool { return getString(b.Name) == bucket }) {
+			return fmt.Errorf("expected bucket %v in the %v listing, instead got %v",
+				bucket, strings.ToUpper(s.awsRegion), buckets)
+		}
+
+		// every bucket of the gateway lives in its own region, so like an S3
+		// regional endpoint it rejects any other region
 		otherRegion := "us-west-2"
 		if s.awsRegion == otherRegion {
 			otherRegion = "us-east-1"
 		}
-		buckets, err = listBuckets(otherRegion)
+		_, err = listBuckets(otherRegion)
-		if err != nil {
+		if err := checkApiErr(err, s3err.GetInvalidArgumentErr(s3err.InvalidArgBucketRegionEndpoint, "")); err != nil {
 			return err
 		}
-		if len(buckets) != 0 {
-			return fmt.Errorf("expected an empty %v listing, instead got %v",
-				otherRegion, buckets)
-		}
 
-		return nil
+		// an empty bucket-region is not a valid region either
+		_, err = listBuckets("")
+		return checkApiErr(err, s3err.GetInvalidArgBucketRegion(""))
 	})
 }
 

tests/test_rest_list_buckets.sh

@@ -285,12 +285,16 @@ export RUN_USERS=true
   run setup_bucket "$bucket_name"
   assert_success
 
-  local params=()
   if [ "$DIRECT" == "true" ]; then
-    params+=("-url" "https://s3.$test_region.amazonaws.com" "-awsRegion" "$test_region")
+    # the regional endpoint of that region lists only its own buckets
+    run list_buckets_bucket_not_in_list "$bucket_name" "$test_region" "-url" "https://s3.$test_region.amazonaws.com" "-awsRegion" "$test_region"
+    assert_success
+    return
   fi
 
-  run list_buckets_bucket_not_in_list "$bucket_name" "$test_region" "${params[@]}"
+  # the gateway is a single regional endpoint, so any other region is rejected
+  run send_rest_go_command_expect_error_with_specific_arg_name_value "400" "InvalidArgument" "Requests with bucket-region specified must be made to the corresponding regional endpoint" \
+   "ArgumentName" "bucket-region" "-query" "bucket-region=$test_region"
   assert_success
 }