fix: validate and apply the ListBuckets bucket-region query
ListBuckets ignored bucket-region: a value that is not a region was accepted, and a region other than the gateway one still listed every bucket. S3 rejects an invalid value with 400 InvalidArgument, "Argument value abc is not a valid AWS Region", and lists only the buckets of the requested region. Check bucket-region against the AWS region code format, always accepting the gateway region whatever its name, and return an empty list for any other region, since every bucket lives in the gateway region. Enable the REST tests that cover both cases. Fixes #1923 Fixes #1930
This commit is contained in:
1 parent
2bb327ec69
commit
b7ff97181d
8 files changed
+175
-6
No files matched your search
@@ -17,7 +17,9 @@ package controllers
|
||||
import (
|
||||
"github.com/gofiber/fiber/v3"
|
||||
"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"
|
||||
)
|
||||
|
||||
@@ -25,6 +27,7 @@ func (c S3ApiController) ListBuckets(ctx fiber.Ctx) (*Response, error) {
|
||||
cToken := ctx.Query("continuation-token")
|
||||
prefix := ctx.Query("prefix")
|
||||
maxBucketsStr := ctx.Query("max-buckets")
|
||||
bucketRegion := ctx.Query("bucket-region")
|
||||
acct := utils.ContextKeyAccount.Get(ctx).(auth.Account)
|
||||
isRoot, _ := utils.ContextKeyIsRoot.Get(ctx).(bool)
|
||||
|
||||
@@ -55,6 +58,13 @@ func (c S3ApiController) ListBuckets(ctx fiber.Ctx) (*Response, error) {
|
||||
}, err
|
||||
}
|
||||
|
||||
if bucketRegion != "" && !utils.IsValidBucketRegion(bucketRegion, region) {
|
||||
debuglogger.Logf("invalid bucket region: %s", bucketRegion)
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{},
|
||||
}, s3err.GetInvalidArgBucketRegion(bucketRegion)
|
||||
}
|
||||
|
||||
// IsAdmin is the backends' "return every bucket, unfiltered" flag.
|
||||
res, err := c.be.ListBuckets(ctx.RequestCtx(),
|
||||
s3response.ListBucketsInput{
|
||||
@@ -68,6 +78,13 @@ 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
|
||||
}
|
||||
|
||||
@@ -99,6 +99,63 @@ func TestS3ApiController_ListBuckets(t *testing.T) {
|
||||
err: s3err.GetInvalidArgMaxLimiter("max-buckets", "bla"),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "invalid bucket region",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
beRes: validRes,
|
||||
queries: map[string]string{
|
||||
"bucket-region": "abc",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{},
|
||||
},
|
||||
err: s3err.GetInvalidArgBucketRegion("abc"),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "bucket region of another region",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
beRes: s3response.ListAllMyBucketsResult{
|
||||
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{
|
||||
Owner: validRes.Owner,
|
||||
Prefix: "te",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "bucket region of the gateway region",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
beRes: validRes,
|
||||
queries: map[string]string{
|
||||
"bucket-region": "us-east-1",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
Data: validRes,
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "backend returns error",
|
||||
input: testInput{
|
||||
|
||||
@@ -40,6 +40,8 @@ import (
|
||||
var (
|
||||
bucketNameRegexp = regexp.MustCompile(`^[a-z0-9][a-z0-9.-]+[a-z0-9]$`)
|
||||
bucketNameIpRegexp = regexp.MustCompile(`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$`)
|
||||
// AWS region codes, such as us-east-1, us-gov-west-1 or eusc-de-east-1
|
||||
awsRegionRegexp = regexp.MustCompile(`^[a-z]{2,4}(-[a-z]+)+-[0-9]+$`)
|
||||
)
|
||||
|
||||
var strictBucketNameValidation atomic.Bool
|
||||
@@ -1153,6 +1155,13 @@ func ValidateLocationConstraint(constraint *string, region string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// IsValidBucketRegion checks a ListBuckets bucket-region value. The gateway
|
||||
// region is valid whatever its name, any other value has to be an AWS region
|
||||
// code.
|
||||
func IsValidBucketRegion(bucketRegion, region string) bool {
|
||||
return bucketRegion == region || awsRegionRegexp.MatchString(bucketRegion)
|
||||
}
|
||||
|
||||
// AwsChunkedEncoding is the coding a client announces when it frames a body in
|
||||
// aws-chunked, as the SDKs do to carry a trailing checksum.
|
||||
const AwsChunkedEncoding = "aws-chunked"
|
||||
|
||||
@@ -1539,6 +1539,31 @@ func TestHasAwsChunkedEncoding(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsValidBucketRegion(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
bucketRegion string
|
||||
region string
|
||||
want bool
|
||||
}{
|
||||
{"gateway region", "us-east-1", "us-east-1", true},
|
||||
{"custom gateway region", "my-region", "my-region", true},
|
||||
{"another aws region", "eu-central-1", "us-east-1", true},
|
||||
{"gov cloud region", "us-gov-west-1", "us-east-1", true},
|
||||
{"iso partition region", "us-isob-east-1", "us-east-1", true},
|
||||
{"sovereign cloud region", "eusc-de-east-1", "us-east-1", true},
|
||||
{"gibberish", "abc", "us-east-1", false},
|
||||
{"missing number", "us-east", "us-east-1", false},
|
||||
{"uppercase", "US-EAST-1", "us-east-1", false},
|
||||
{"custom region of another gateway", "my-region", "us-east-1", false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equal(t, tt.want, IsValidBucketRegion(tt.bucketRegion, tt.region))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseContentEncoding(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
Reference in new issue
Block a user