diff --git a/s3api/middlewares/acl-parser.go b/s3api/middlewares/acl-parser.go index 53f3ec8..e138a11 100644 --- a/s3api/middlewares/acl-parser.go +++ b/s3api/middlewares/acl-parser.go @@ -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 } diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 2e81bc4..dbb0698 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -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, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index a7c442a..6416e6e 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -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 {