mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2025-12-23 05:05:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: adds validation for bucket canned ACL

Browse Source 
Fixes #1379

Adds validation for bucket canned ACLs in `CreateBucket` and `PutBucketAcl`. The gateway supports three values: `private`, `public-read`, and `public-read-write`. All other values (including `authenticated-read`, which is not supported) are considered invalid and result in an `InvalidArgument` error with an empty error message.
This commit is contained in:
niksis02
2025-11-03 22:23:28 +04:00
parent 4345420e12
commit 7744dacced
7 changed files with 100 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
auth/acl.go
View File

@@ -25,6 +25,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/versity/versitygw/backend"
"github.com/versity/versitygw/debuglogger"
"github.com/versity/versitygw/s3err"
)
@@ -493,3 +494,14 @@ func UpdateBucketACLOwner(ctx context.Context, be backend.Backend, bucket, newOw
return be.DeleteBucketPolicy(ctx, bucket)
}
// ValidateCannedACL validates bucket canned acl value
func ValidateCannedACL(acl string) error {
switch types.BucketCannedACL(acl) {
case types.BucketCannedACLPrivate, types.BucketCannedACLPublicRead, types.BucketCannedACLPublicReadWrite, "":
return nil
default:
debuglogger.Logf("invalid bucket canned acl: %v", acl)
return s3err.GetAPIError(s3err.ErrInvalidArgument)
}
}

35
s3api/controllers/bucket-put.go
View File

@@ -352,6 +352,15 @@ func (c S3ApiController) PutBucketAcl(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = auth.ValidateCannedACL(acl)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
ownership, err := c.be.GetBucketOwnershipControls(ctx.Context(), bucket)
if err != nil && !errors.Is(err, s3err.GetAPIError(s3err.ErrOwnershipControlsNotFound)) {
return &Response{
@@ -419,14 +428,6 @@ func (c S3ApiController) PutBucketAcl(ctx *fiber.Ctx) (*Response, error) {
AccessControlPolicy: &accessControlPolicy,
}
} else if acl != "" {
if acl != "private" && acl != "public-read" && acl != "public-read-write" {
debuglogger.Logf("invalid acl: %q", acl)
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetAPIError(s3err.ErrInvalidRequest)
}
if grants != "" {
debuglogger.Logf("invalid request: %q (grants) %q (acl)",
grants, acl)
@@ -500,14 +501,28 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) {
// validate the bucket name
if ok := utils.IsValidBucketName(bucket); !ok {
return &Response{
MetaOpts: &MetaOptions{},
MetaOpts: &MetaOptions{
BucketOwner: acct.Access,
},
}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
// validate bucket canned acl
err := auth.ValidateCannedACL(acl)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: acct.Access,
},
}, err
}
// validate the object ownership value
if ok := utils.IsValidOwnership(objectOwnership); !ok {
return &Response{
MetaOpts: &MetaOptions{},
MetaOpts: &MetaOptions{
BucketOwner: acct.Access,
},
}, s3err.APIError{
Code: "InvalidArgument",
Description: fmt.Sprintf("Invalid x-amz-object-ownership header: %v", objectOwnership),

28
s3api/controllers/bucket-put_test.go
View File

@@ -729,7 +729,9 @@ func TestS3ApiController_CreateBucket(t *testing.T) {
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{},
MetaOpts: &MetaOptions{
BucketOwner: adminAcc.Access,
},
},
err: s3err.GetAPIError(s3err.ErrInvalidBucketName),
},
@@ -749,6 +751,24 @@ func TestS3ApiController_CreateBucket(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrMalformedXML),
},
},
{
name: "invalid canned acl",
input: testInput{
locals: map[utils.ContextKey]any{
utils.ContextKeyAccount: adminAcc,
},
headers: map[string]string{
"x-amz-acl": "invalid_acl",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{BucketOwner: adminAcc.Access},
},
err: s3err.GetAPIError(s3err.ErrInvalidArgument),
},
},
{
name: "invalid location constraint",
input: testInput{
@@ -777,7 +797,9 @@ func TestS3ApiController_CreateBucket(t *testing.T) {
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{},
MetaOpts: &MetaOptions{
BucketOwner: adminAcc.Access,
},
},
err: s3err.APIError{
Code: "InvalidArgument",
@@ -1079,7 +1101,7 @@ func TestS3ApiController_PutBucketAcl(t *testing.T) {
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidRequest),
err: s3err.GetAPIError(s3err.ErrInvalidArgument),
},
},
{

6
s3err/s3err.go
View File

@@ -174,6 +174,7 @@ const (
ErrCORSIsNotEnabled
ErrNotModified
ErrInvalidLocationConstraint
ErrInvalidArgument
// Non-AWS errors
ErrExistingObjectIsDirectory
@@ -768,6 +769,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
Description: "The specified location-constraint is not valid",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidArgument: {
Code: "InvalidArgument",
Description: "",
HTTPStatusCode: http.StatusBadRequest,
},
// non aws errors
ErrExistingObjectIsDirectory: {

13
tests/integration/CreateBucket.go
View File

@@ -490,3 +490,16 @@ func CreateBucket_tag_count_limit(s *S3Conf) error {
return checkApiErr(err, s3err.GetAPIError(s3err.ErrBucketTaggingLimited))
})
}
func CreateBucket_invalid_canned_acl(s *S3Conf) error {
testName := "CreateBucket_invalid_canned_acl"
return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{
Bucket: &bucket,
ACL: types.BucketCannedACL("invalid_acl"),
})
cancel()
return checkSdkApiErr(err, "InvalidArgument")
})
}

15
tests/integration/PutBucketAcl.go
View File

@@ -71,6 +71,21 @@ func PutBucketAcl_none_of_the_options_specified(s *S3Conf) error {
}, withOwnership(types.ObjectOwnershipBucketOwnerPreferred))
}
func PutBucketAcl_invalid_canned_acl(s *S3Conf) error {
testName := "PutBucketAcl_invalid_canned_acl"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutBucketAcl(ctx, &s3.PutBucketAclInput{
Bucket: &bucket,
ACL: types.BucketCannedACL("invalid_acl"),
})
cancel()
// the expected error message is empty for this case
// only check the error Code
return checkSdkApiErr(err, "InvalidArgument")
}, withOwnership(types.ObjectOwnershipBucketOwnerPreferred))
}
func PutBucketAcl_invalid_acl_canned_and_acp(s *S3Conf) error {
testName := "PutBucketAcl_invalid_acl_canned_and_acp"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {

4
tests/integration/group-tests.go
View File

@@ -85,6 +85,7 @@ func TestCreateBucket(ts *TestState) {
ts.Run(CreateBucket_invalid_tags)
ts.Run(CreateBucket_duplicate_keys)
ts.Run(CreateBucket_tag_count_limit)
ts.Run(CreateBucket_invalid_canned_acl)
}
func TestHeadBucket(ts *TestState) {
@@ -509,6 +510,7 @@ func TestPutBucketAcl(ts *TestState) {
ts.Run(PutBucketAcl_non_existing_bucket)
ts.Run(PutBucketAcl_disabled)
ts.Run(PutBucketAcl_none_of_the_options_specified)
ts.Run(PutBucketAcl_invalid_canned_acl)
ts.Run(PutBucketAcl_invalid_acl_canned_and_acp)
ts.Run(PutBucketAcl_invalid_acl_canned_and_grants)
ts.Run(PutBucketAcl_invalid_acl_acp_and_grants)
@@ -1145,6 +1147,7 @@ func GetIntTests() IntTests {
"CreateBucket_invalid_tags": CreateBucket_invalid_tags,
"CreateBucket_duplicate_keys": CreateBucket_duplicate_keys,
"CreateBucket_tag_count_limit": CreateBucket_tag_count_limit,
"CreateBucket_invalid_canned_acl": CreateBucket_invalid_canned_acl,
"HeadBucket_non_existing_bucket": HeadBucket_non_existing_bucket,
"HeadBucket_success": HeadBucket_success,
"ListBuckets_as_user": ListBuckets_as_user,
@@ -1410,6 +1413,7 @@ func GetIntTests() IntTests {
"PutBucketAcl_non_existing_bucket": PutBucketAcl_non_existing_bucket,
"PutBucketAcl_disabled": PutBucketAcl_disabled,
"PutBucketAcl_none_of_the_options_specified": PutBucketAcl_none_of_the_options_specified,
"PutBucketAcl_invalid_canned_acl": PutBucketAcl_invalid_canned_acl,
"PutBucketAcl_invalid_acl_canned_and_acp": PutBucketAcl_invalid_acl_canned_and_acp,
"PutBucketAcl_invalid_acl_canned_and_grants": PutBucketAcl_invalid_acl_canned_and_grants,
"PutBucketAcl_invalid_acl_acp_and_grants": PutBucketAcl_invalid_acl_acp_and_grants,