Merge pull request #1585 from versity/sis/expected-bucket-owner

feat: adds the x-amz-expected-bucket-owner check in the gateway
This commit is contained in:
Ben McClelland
2025-10-15 14:35:07 -07:00
committed by GitHub
3 changed files with 119 additions and 0 deletions

View File

@@ -20,6 +20,7 @@ import (
"github.com/versity/versitygw/auth"
"github.com/versity/versitygw/backend"
"github.com/versity/versitygw/s3api/utils"
"github.com/versity/versitygw/s3err"
)
// ParseAcl retreives the bucket acl and stores in the context locals
@@ -42,6 +43,16 @@ func ParseAcl(be backend.Backend) fiber.Handler {
parsedAcl.Owner = utils.ContextKeyRootAccessKey.Get(ctx).(string)
}
// if expected bucket owner doesn't match the bucket owner
// the gateway should return AccessDenied.
// This header appears in all actions except 'CreateBucket' and 'ListBuckets'.
// 'ParseACL' is also applied to all actions except for 'CreateBucket' and 'ListBuckets',
// so it's a perfect place to check the expected bucket owner
bucketOwner := ctx.Get("X-Amz-Expected-Bucket-Owner")
if bucketOwner != "" && bucketOwner != parsedAcl.Owner {
return s3err.GetAPIError(s3err.ErrAccessDenied)
}
utils.ContextKeyParsedAcl.Set(ctx, parsedAcl)
return nil
}

View File

@@ -101,6 +101,7 @@ func TestDeleteBucket(ts *TestState) {
ts.Run(DeleteBucket_non_existing_bucket)
ts.Run(DeleteBucket_non_empty_bucket)
ts.Run(DeleteBucket_success_status_code)
ts.Run(DeleteBucket_incorrect_expected_bucket_owner)
}
func TestPutBucketOwnershipControls(ts *TestState) {
@@ -291,6 +292,8 @@ func TestDeleteObject(ts *TestState) {
ts.Run(DeleteObject_conditional_writes)
ts.Run(DeleteObject_success)
ts.Run(DeleteObject_success_status_code)
ts.Run(DeleteObject_incorrect_expected_bucket_owner)
ts.Run(DeleteObject_expected_bucket_owner)
}
func TestDeleteObjects(ts *TestState) {
@@ -348,6 +351,7 @@ func TestDeleteObjectTagging(ts *TestState) {
ts.Run(DeleteObjectTagging_non_existing_object)
ts.Run(DeleteObjectTagging_success_status)
ts.Run(DeleteObjectTagging_success)
ts.Run(DeleteObjectTagging_expected_bucket_owner)
}
func TestCreateMultipartUpload(ts *TestState) {
@@ -1128,6 +1132,7 @@ func GetIntTests() IntTests {
"ListBuckets_success": ListBuckets_success,
"DeleteBucket_non_existing_bucket": DeleteBucket_non_existing_bucket,
"DeleteBucket_non_empty_bucket": DeleteBucket_non_empty_bucket,
"DeleteBucket_incorrect_expected_bucket_owner": DeleteBucket_incorrect_expected_bucket_owner,
"DeleteBucket_success_status_code": DeleteBucket_success_status_code,
"PutBucketOwnershipControls_non_existing_bucket": PutBucketOwnershipControls_non_existing_bucket,
"PutBucketOwnershipControls_multiple_rules": PutBucketOwnershipControls_multiple_rules,
@@ -1238,6 +1243,8 @@ func GetIntTests() IntTests {
"DeleteObject_directory_object": DeleteObject_directory_object,
"DeleteObject_success": DeleteObject_success,
"DeleteObject_success_status_code": DeleteObject_success_status_code,
"DeleteObject_incorrect_expected_bucket_owner": DeleteObject_incorrect_expected_bucket_owner,
"DeleteObject_expected_bucket_owner": DeleteObject_expected_bucket_owner,
"DeleteObjects_empty_input": DeleteObjects_empty_input,
"DeleteObjects_non_existing_objects": DeleteObjects_non_existing_objects,
"DeleteObjects_success": DeleteObjects_success,
@@ -1277,6 +1284,7 @@ func GetIntTests() IntTests {
"DeleteObjectTagging_non_existing_object": DeleteObjectTagging_non_existing_object,
"DeleteObjectTagging_success_status": DeleteObjectTagging_success_status,
"DeleteObjectTagging_success": DeleteObjectTagging_success,
"DeleteObjectTagging_expected_bucket_owner": DeleteObjectTagging_expected_bucket_owner,
"CreateMultipartUpload_non_existing_bucket": CreateMultipartUpload_non_existing_bucket,
"CreateMultipartUpload_with_metadata": CreateMultipartUpload_with_metadata,
"CreateMultipartUpload_with_tagging": CreateMultipartUpload_with_tagging,

View File

@@ -2300,6 +2300,19 @@ func DeleteBucket_success_status_code(s *S3Conf) error {
return nil
}
func DeleteBucket_incorrect_expected_bucket_owner(s *S3Conf) error {
testName := "DeleteBucket_incorrect_expected_bucket_owner"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.DeleteBucket(ctx, &s3.DeleteBucketInput{
Bucket: &bucket,
ExpectedBucketOwner: getPtr(s.awsID + "something"),
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied))
})
}
func PutBucketOwnershipControls_non_existing_bucket(s *S3Conf) error {
testName := "PutBucketOwnershipControls_non_existing_bucket"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -7191,6 +7204,38 @@ func DeleteObject_success_status_code(s *S3Conf) error {
})
}
func DeleteObject_incorrect_expected_bucket_owner(s *S3Conf) error {
testName := "DeleteObject_incorrect_expected_bucket_owner"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
// anyways if object doesn't exist, a 200 response should be received
Key: getPtr("my-obj"),
ExpectedBucketOwner: getPtr(s.awsID + "something"),
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied))
})
}
func DeleteObject_expected_bucket_owner(s *S3Conf) error {
testName := "DeleteObject_expected_bucket_owner"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
// anyways if object doesn't exist, a 200 response should be received
Key: getPtr("my-obj"),
ExpectedBucketOwner: &s.awsID,
})
cancel()
return err
})
}
func DeleteObjects_empty_input(s *S3Conf) error {
testName := "DeleteObjects_empty_input"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -8994,6 +9039,61 @@ func DeleteObjectTagging_success(s *S3Conf) error {
})
}
func DeleteObjectTagging_expected_bucket_owner(s *S3Conf) error {
testName := "DeleteObjectTagging_expected_bucket_owner"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
tagging := types.Tagging{TagSet: []types.Tag{
{Key: getPtr("key1"), Value: getPtr("val2")},
{Key: getPtr("key2"), Value: getPtr("val2")},
}}
_, err := putObjects(s3client, []string{obj}, bucket)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.PutObjectTagging(ctx, &s3.PutObjectTaggingInput{
Bucket: &bucket,
Key: &obj,
Tagging: &tagging,
ExpectedBucketOwner: &s.awsID,
})
cancel()
if err != nil {
return err
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.DeleteObjectTagging(ctx, &s3.DeleteObjectTaggingInput{
Bucket: &bucket,
Key: &obj,
ExpectedBucketOwner: &s.awsID,
})
cancel()
if err != nil {
return nil
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.GetObjectTagging(ctx, &s3.GetObjectTaggingInput{
Bucket: &bucket,
Key: &obj,
ExpectedBucketOwner: &s.awsID,
})
cancel()
if err != nil {
return nil
}
if len(out.TagSet) > 0 {
return fmt.Errorf("expected empty tag set, instead got %v", out.TagSet)
}
return nil
})
}
func CreateMultipartUpload_non_existing_bucket(s *S3Conf) error {
testName := "CreateMultipartUpload_non_existing_bucket"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {