From 8569b158f08d1a61780505cdbbd06009d0da1fb9 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Fri, 23 Jan 2026 17:03:03 +0400 Subject: [PATCH] fix: return not implemented in object actions, if acl header is present Fixes #1767 Fixes #1773 As object ACLs are not supported in the gateway, any attempt to set an ACL during object creation must return a NotImplemented error. A check has now been added to `PutObject`, `CopyObject`, and `CreateMultipartUpload` to detect any ACL-related headers and return a NotImplemented error accordingly. --- s3api/controllers/object-post.go | 11 +++++- s3api/controllers/object-put.go | 22 ++++++++++- s3api/utils/utils.go | 22 +++++++++++ tests/integration/CopyObject.go | 44 ++++++++++++++++++++++ tests/integration/CreateMultipartUpload.go | 35 +++++++++++++++++ tests/integration/PutObject.go | 33 ++++++++++++++++ tests/integration/group-tests.go | 6 +++ tests/integration/utils.go | 2 +- 8 files changed, 171 insertions(+), 4 deletions(-) diff --git a/s3api/controllers/object-post.go b/s3api/controllers/object-post.go index aa17b73f..844d4479 100644 --- a/s3api/controllers/object-post.go +++ b/s3api/controllers/object-post.go @@ -158,7 +158,16 @@ func (c S3ApiController) CreateMultipartUpload(ctx *fiber.Ctx) (*Response, error isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool) parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL) - err := auth.VerifyAccess(ctx.Context(), c.be, + err := utils.ValidateNoACLHeaders(ctx) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ Readonly: c.readonly, Acl: parsedAcl, diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index 8d3a67ae..ce46580c 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -510,7 +510,16 @@ func (c S3ApiController) CopyObject(ctx *fiber.Ctx) (*Response, error) { isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool) parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL) - err := utils.ValidateCopySource(copySource) + err := utils.ValidateNoACLHeaders(ctx) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + + err = utils.ValidateCopySource(copySource) if err != nil { return &Response{ MetaOpts: &MetaOptions{ @@ -659,6 +668,15 @@ func (c S3ApiController) PutObject(ctx *fiber.Ctx) (*Response, error) { parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL) IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx) + err := utils.ValidateNoACLHeaders(ctx) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + // Content Length contentLengthStr := ctx.Get("Content-Length") if contentLengthStr == "" { @@ -674,7 +692,7 @@ func (c S3ApiController) PutObject(ctx *fiber.Ctx) (*Response, error) { // load the meta headers metadata := utils.GetUserMetaData(&ctx.Request().Header) - err := auth.VerifyAccess(ctx.Context(), c.be, + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ Readonly: c.readonly, Acl: parsedAcl, diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index dd95abcb..cbb97196 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -948,3 +948,25 @@ func NewTLSListener(network string, address string, getCertificateFunc func(*tls } return tls.NewListener(ln, config), nil } + +// ValidateNoACLHeaders checks whether any ACL-related request headers are set. +// since ACL operations are not supported on objects, the presence of any ACL headers +// results in a NotImplemented error. It returns nil only when all ACL headers +// are absent. +func ValidateNoACLHeaders(ctx *fiber.Ctx) error { + for _, header := range []string{ + "x-amz-acl", + "x-amz-grant-full-control", + "x-amz-grant-read", + "x-amz-grant-read-acp", + "x-amz-grant-write-acp", + } { + value := ctx.Request().Header.Peek(header) + if len(value) != 0 { + debuglogger.Logf("an unsupported object acl header present: %s:%s", header, value) + return s3err.GetAPIError(s3err.ErrNotImplemented) + } + } + + return nil +} diff --git a/tests/integration/CopyObject.go b/tests/integration/CopyObject.go index 07e70a71..ca764a4d 100644 --- a/tests/integration/CopyObject.go +++ b/tests/integration/CopyObject.go @@ -1497,3 +1497,47 @@ func CopyObject_success(s *S3Conf) error { return nil }) } + +func CopyObject_object_acl_not_supported(s *S3Conf) error { + testName := "CopyObject_object_acl_not_supported" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj, srcObj := "my-object", "source-object" + testuser := getUser("user") + err := createUsers(s, []user{testuser}) + if err != nil { + return err + } + + _, err = putObjectWithData(0, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &srcObj, + }, s3client) + if err != nil { + return err + } + + for i, modifyInput := range []func(*s3.CopyObjectInput){ + func(poi *s3.CopyObjectInput) { poi.ACL = types.ObjectCannedACLPublicRead }, + func(poi *s3.CopyObjectInput) { poi.GrantFullControl = &testuser.access }, + func(poi *s3.CopyObjectInput) { poi.GrantRead = &testuser.access }, + func(poi *s3.CopyObjectInput) { poi.GrantReadACP = &testuser.access }, + func(poi *s3.CopyObjectInput) { poi.GrantWriteACP = &testuser.access }, + } { + input := &s3.CopyObjectInput{ + Bucket: &bucket, + Key: &obj, + CopySource: getPtr(fmt.Sprintf("%s/%s", bucket, srcObj)), + } + + modifyInput(input) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CopyObject(ctx, input) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)); err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + return nil + }) +} diff --git a/tests/integration/CreateMultipartUpload.go b/tests/integration/CreateMultipartUpload.go index a30dd1bd..c5aab224 100644 --- a/tests/integration/CreateMultipartUpload.go +++ b/tests/integration/CreateMultipartUpload.go @@ -590,3 +590,38 @@ func CreateMultipartUpload_success(s *S3Conf) error { return nil }) } + +func CreateMultipartUpload_object_acl_not_supported(s *S3Conf) error { + testName := "CreateMultipartUpload_object_acl_not_supported" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-object" + testuser := getUser("user") + err := createUsers(s, []user{testuser}) + if err != nil { + return err + } + + for i, modifyInput := range []func(*s3.CreateMultipartUploadInput){ + func(poi *s3.CreateMultipartUploadInput) { poi.ACL = types.ObjectCannedACLPublicRead }, + func(poi *s3.CreateMultipartUploadInput) { poi.GrantFullControl = &testuser.access }, + func(poi *s3.CreateMultipartUploadInput) { poi.GrantRead = &testuser.access }, + func(poi *s3.CreateMultipartUploadInput) { poi.GrantReadACP = &testuser.access }, + func(poi *s3.CreateMultipartUploadInput) { poi.GrantWriteACP = &testuser.access }, + } { + input := &s3.CreateMultipartUploadInput{ + Bucket: &bucket, + Key: &obj, + } + + modifyInput(input) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateMultipartUpload(ctx, input) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)); err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + return nil + }) +} diff --git a/tests/integration/PutObject.go b/tests/integration/PutObject.go index 02b1671f..6298fd89 100644 --- a/tests/integration/PutObject.go +++ b/tests/integration/PutObject.go @@ -954,6 +954,39 @@ func PutObject_invalid_object_names(s *S3Conf) error { }) } +func PutObject_object_acl_not_supported(s *S3Conf) error { + testName := "PutObject_object_acl_not_supported" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-object" + testuser := getUser("user") + err := createUsers(s, []user{testuser}) + if err != nil { + return err + } + + for i, modifyInput := range []func(*s3.PutObjectInput){ + func(poi *s3.PutObjectInput) { poi.ACL = types.ObjectCannedACLPublicRead }, + func(poi *s3.PutObjectInput) { poi.GrantFullControl = &testuser.access }, + func(poi *s3.PutObjectInput) { poi.GrantRead = &testuser.access }, + func(poi *s3.PutObjectInput) { poi.GrantReadACP = &testuser.access }, + func(poi *s3.PutObjectInput) { poi.GrantWriteACP = &testuser.access }, + } { + input := &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + } + + modifyInput(input) + _, err := putObjectWithData(0, input, s3client) + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented)); err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + return nil + }) +} + func PutObject_false_negative_object_names(s *S3Conf) error { testName := "PutObject_false_negative_object_names" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 3d6c0bab..b7d5db56 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -188,6 +188,7 @@ func TestPutObject(ts *TestState) { } ts.Run(PutObject_invalid_credentials) ts.Run(PutObject_invalid_object_names) + ts.Run(PutObject_object_acl_not_supported) } func TestHeadObject(ts *TestState) { @@ -335,6 +336,7 @@ func TestCopyObject(ts *TestState) { ts.Run(CopyObject_with_legal_hold) ts.Run(CopyObject_with_retention_lock) ts.Run(CopyObject_conditional_reads) + ts.Run(CopyObject_object_acl_not_supported) //TODO: remove the condition after implementing checksums in azure if !ts.conf.azureTests { ts.Run(CopyObject_invalid_checksum_algorithm) @@ -381,6 +383,7 @@ func TestCreateMultipartUpload(ts *TestState) { ts.Run(CreateMultipartUpload_past_retain_until_date) ts.Run(CreateMultipartUpload_invalid_legal_hold) ts.Run(CreateMultipartUpload_invalid_object_lock_mode) + ts.Run(CreateMultipartUpload_object_acl_not_supported) //TODO: remove the condition after implementing checksums in azure if !ts.conf.azureTests { ts.Run(CreateMultipartUpload_invalid_checksum_algorithm) @@ -1283,6 +1286,7 @@ func GetIntTests() IntTests { "PutObject_tagging": PutObject_tagging, "PutObject_success": PutObject_success, "PutObject_invalid_object_names": PutObject_invalid_object_names, + "PutObject_object_acl_not_supported": PutObject_object_acl_not_supported, "PutObject_false_negative_object_names": PutObject_false_negative_object_names, "PutObject_racey_success": PutObject_racey_success, "HeadObject_non_existing_object": HeadObject_non_existing_object, @@ -1387,6 +1391,7 @@ func GetIntTests() IntTests { "CopyObject_with_legal_hold": CopyObject_with_legal_hold, "CopyObject_with_retention_lock": CopyObject_with_retention_lock, "CopyObject_conditional_reads": CopyObject_conditional_reads, + "CopyObject_object_acl_not_supported": CopyObject_object_acl_not_supported, "CopyObject_with_metadata": CopyObject_with_metadata, "CopyObject_invalid_checksum_algorithm": CopyObject_invalid_checksum_algorithm, "CopyObject_create_checksum_on_copy": CopyObject_create_checksum_on_copy, @@ -1417,6 +1422,7 @@ func GetIntTests() IntTests { "CreateMultipartUpload_past_retain_until_date": CreateMultipartUpload_past_retain_until_date, "CreateMultipartUpload_invalid_legal_hold": CreateMultipartUpload_invalid_legal_hold, "CreateMultipartUpload_invalid_object_lock_mode": CreateMultipartUpload_invalid_object_lock_mode, + "CreateMultipartUpload_object_acl_not_supported": CreateMultipartUpload_object_acl_not_supported, "CreateMultipartUpload_invalid_checksum_algorithm": CreateMultipartUpload_invalid_checksum_algorithm, "CreateMultipartUpload_empty_checksum_algorithm_with_checksum_type": CreateMultipartUpload_empty_checksum_algorithm_with_checksum_type, "CreateMultipartUpload_type_algo_mismatch": CreateMultipartUpload_type_algo_mismatch, diff --git a/tests/integration/utils.go b/tests/integration/utils.go index a3d27968..db06121a 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -551,7 +551,7 @@ type putObjectOutput struct { func putObjectWithData(lgth int64, input *s3.PutObjectInput, client *s3.Client) (*putObjectOutput, error) { var csum [32]byte var data []byte - if input.Body == nil { + if input.Body == nil && lgth != 0 { data = make([]byte, lgth) rand.Read(data) csum = sha256.Sum256(data)