diff --git a/backend/azure/azure.go b/backend/azure/azure.go index bd205c5..cb591f8 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -177,7 +177,21 @@ func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, meta[string(keyBucketLock)] = backend.GetPtrFromString(encodeBytes(defaultLockParsed)) } - _, err := az.client.CreateContainer(ctx, *input.Bucket, &container.CreateOptions{Metadata: meta}) + tagging, err := backend.ParseCreateBucketTags(input.CreateBucketConfiguration.Tags) + if err != nil { + return err + } + + if tagging != nil { + tags, err := json.Marshal(tagging) + if err != nil { + return fmt.Errorf("marshal tags: %w", err) + } + + meta[string(keyTags)] = backend.GetPtrFromString(encodeBytes(tags)) + } + + _, err = az.client.CreateContainer(ctx, *input.Bucket, &container.CreateOptions{Metadata: meta}) if errors.Is(s3err.GetAPIError(s3err.ErrBucketAlreadyExists), azureErrToS3Err(err)) { aclBytes, err := az.getContainerMetaData(ctx, *input.Bucket, string(keyAclCapital)) if err != nil { diff --git a/backend/common.go b/backend/common.go index 9092a17..ccd1146 100644 --- a/backend/common.go +++ b/backend/common.go @@ -317,6 +317,54 @@ func ParseObjectTags(tagging string) (map[string]string, error) { return tagSet, nil } +// ParseCreateBucketTags parses and validates the bucket +// tagging from CreateBucket input +func ParseCreateBucketTags(tagging []types.Tag) (map[string]string, error) { + if len(tagging) == 0 { + return nil, nil + } + + tagset := make(map[string]string, len(tagging)) + + if len(tagging) > 50 { + return nil, s3err.GetAPIError(s3err.ErrBucketTaggingLimited) + } + + for _, tag := range tagging { + // validate tag key length + key := GetStringFromPtr(tag.Key) + if len(key) == 0 || len(key) > 128 { + return nil, s3err.GetAPIError(s3err.ErrInvalidTagKey) + } + + // validate tag key string chars + if !isValidTagComponent(key) { + return nil, s3err.GetAPIError(s3err.ErrInvalidTagKey) + } + + // validate tag value length + value := GetStringFromPtr(tag.Value) + if len(value) > 256 { + return nil, s3err.GetAPIError(s3err.ErrInvalidTagValue) + } + + // validate tag value string chars + if !isValidTagComponent(value) { + return nil, s3err.GetAPIError(s3err.ErrInvalidTagValue) + } + + // make sure there are no duplicate keys + _, ok := tagset[key] + if ok { + return nil, s3err.GetAPIError(s3err.ErrDuplicateTagKey) + } + + tagset[key] = value + } + + return tagset, nil +} + // tag component (key/value) name rule regexp // https://docs.aws.amazon.com/AmazonS3/latest/API/API_control_Tag.html var validTagComponent = regexp.MustCompile(`^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$`) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 115ec67..d77f8d4 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -381,7 +381,12 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a return s3err.GetAPIError(s3err.ErrInvalidBucketName) } - err := os.Mkdir(bucket, p.newDirPerm) + tagging, err := backend.ParseCreateBucketTags(input.CreateBucketConfiguration.Tags) + if err != nil { + return err + } + + err = os.Mkdir(bucket, p.newDirPerm) if err != nil && os.IsExist(err) { aclJSON, err := p.meta.RetrieveAttribute(nil, bucket, "", aclkey) if err != nil { @@ -420,6 +425,16 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a if err != nil { return fmt.Errorf("set ownership: %w", err) } + if tagging != nil { + tags, err := json.Marshal(tagging) + if err != nil { + return fmt.Errorf("marshal tags: %w", err) + } + err = p.meta.StoreAttribute(nil, bucket, "", tagHdr, tags) + if err != nil { + return fmt.Errorf("set tags: %w", err) + } + } if input.ObjectLockEnabledForBucket != nil && *input.ObjectLockEnabledForBucket { // First enable bucket versioning diff --git a/s3api/controllers/bucket-put.go b/s3api/controllers/bucket-put.go index 6c4d30e..af21dc4 100644 --- a/s3api/controllers/bucket-put.go +++ b/s3api/controllers/bucket-put.go @@ -533,6 +533,32 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { }, s3err.GetAPIError(s3err.ErrBothCannedAndHeaderGrants) } + var body s3response.CreateBucketConfiguration + if len(ctx.Body()) != 0 { + // request body is optional for CreateBucket + err := xml.Unmarshal(ctx.Body(), &body) + if err != nil { + debuglogger.Logf("failed to parse the request body: %v", err) + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: acct.Access, + }, + }, s3err.GetAPIError(s3err.ErrMalformedXML) + } + + if body.LocationConstraint != "" { + region := utils.ContextKeyRegion.Get(ctx).(string) + if body.LocationConstraint != region { + debuglogger.Logf("invalid location constraint: %s", body.LocationConstraint) + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: acct.Access, + }, + }, s3err.GetAPIError(s3err.ErrInvalidLocationConstraint) + } + } + } + defACL := auth.ACL{ Owner: acct.Access, } @@ -562,6 +588,9 @@ func (c S3ApiController) CreateBucket(ctx *fiber.Ctx) (*Response, error) { Bucket: &bucket, ObjectOwnership: objectOwnership, ObjectLockEnabledForBucket: &lockEnabled, + CreateBucketConfiguration: &types.CreateBucketConfiguration{ + Tags: body.TagSet, + }, }, updAcl) return &Response{ MetaOpts: &MetaOptions{ diff --git a/s3api/controllers/bucket-put_test.go b/s3api/controllers/bucket-put_test.go index 1d37ecb..3dadc3f 100644 --- a/s3api/controllers/bucket-put_test.go +++ b/s3api/controllers/bucket-put_test.go @@ -695,6 +695,11 @@ func TestS3ApiController_CreateBucket(t *testing.T) { Role: auth.RoleUser, } + invLocConstBody, err := xml.Marshal(s3response.CreateBucketConfiguration{ + LocationConstraint: "us-west-1", + }) + assert.NoError(t, err) + tests := []struct { name string input testInput @@ -729,6 +734,37 @@ func TestS3ApiController_CreateBucket(t *testing.T) { err: s3err.GetAPIError(s3err.ErrInvalidBucketName), }, }, + { + name: "malformed body", + input: testInput{ + locals: map[utils.ContextKey]any{ + utils.ContextKeyAccount: adminAcc, + }, + body: []byte("invalid_body"), + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{BucketOwner: adminAcc.Access}, + }, + err: s3err.GetAPIError(s3err.ErrMalformedXML), + }, + }, + { + name: "invalid location constraint", + input: testInput{ + locals: map[utils.ContextKey]any{ + utils.ContextKeyAccount: adminAcc, + utils.ContextKeyRegion: "us-east-1", + }, + body: invLocConstBody, + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{BucketOwner: adminAcc.Access}, + }, + err: s3err.GetAPIError(s3err.ErrInvalidLocationConstraint), + }, + }, { name: "invalid ownership", input: testInput{ diff --git a/s3err/s3err.go b/s3err/s3err.go index bc5f999..44a54dd 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -173,6 +173,7 @@ const ( ErrMissingCORSOrigin ErrCORSIsNotEnabled ErrNotModified + ErrInvalidLocationConstraint // Non-AWS errors ErrExistingObjectIsDirectory @@ -762,6 +763,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "Not Modified", HTTPStatusCode: http.StatusNotModified, }, + ErrInvalidLocationConstraint: { + Code: "InvalidLocationConstraint", + Description: "The specified location-constraint is not valid", + HTTPStatusCode: http.StatusBadRequest, + }, // non aws errors ErrExistingObjectIsDirectory: { diff --git a/s3response/s3response.go b/s3response/s3response.go index 506996b..1ead5ab 100644 --- a/s3response/s3response.go +++ b/s3response/s3response.go @@ -728,3 +728,8 @@ type LocationConstraint struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ LocationConstraint"` Value string `xml:",chardata"` } + +type CreateBucketConfiguration struct { + LocationConstraint string + TagSet []types.Tag `xml:"Tags>Tag"` +} diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 710f907..dfc888c 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -80,6 +80,11 @@ func TestCreateBucket(ts *TestState) { ts.Run(CreateBucket_non_default_acl) ts.Run(CreateDeleteBucket_success) ts.Run(CreateBucket_default_object_lock) + ts.Run(CreateBucket_invalid_location_constraint) + ts.Run(CreateBucket_long_tags) + ts.Run(CreateBucket_invalid_tags) + ts.Run(CreateBucket_duplicate_keys) + ts.Run(CreateBucket_tag_count_limit) } func TestHeadBucket(ts *TestState) { @@ -1135,6 +1140,11 @@ func GetIntTests() IntTests { "CreateBucket_default_acl": CreateBucket_default_acl, "CreateBucket_non_default_acl": CreateBucket_non_default_acl, "CreateBucket_default_object_lock": CreateBucket_default_object_lock, + "CreateBucket_invalid_location_constraint": CreateBucket_invalid_location_constraint, + "CreateBucket_long_tags": CreateBucket_long_tags, + "CreateBucket_invalid_tags": CreateBucket_invalid_tags, + "CreateBucket_duplicate_keys": CreateBucket_duplicate_keys, + "CreateBucket_tag_count_limit": CreateBucket_tag_count_limit, "HeadBucket_non_existing_bucket": HeadBucket_non_existing_bucket, "HeadBucket_success": HeadBucket_success, "ListBuckets_as_user": ListBuckets_as_user, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 2af6472..dcfce04 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -1741,6 +1741,165 @@ func CreateBucket_default_object_lock(s *S3Conf) error { return nil } +func CreateBucket_invalid_location_constraint(s *S3Conf) error { + testName := "CreateBucket_invalid_location_constraint" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + var region types.BucketLocationConstraint + if types.BucketLocationConstraint(s.awsID) == types.BucketLocationConstraintUsWest1 { + region = types.BucketLocationConstraintUsWest2 + } else { + region = types.BucketLocationConstraintUsWest1 + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + CreateBucketConfiguration: &types.CreateBucketConfiguration{ + LocationConstraint: region, + }, + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidLocationConstraint)) + }) +} + +func CreateBucket_long_tags(s *S3Conf) error { + testName := "CreateBucket_long_tags" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + tagging := []types.Tag{{Key: getPtr(genRandString(200)), Value: getPtr("val")}} + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + CreateBucketConfiguration: &types.CreateBucketConfiguration{ + Tags: tagging, + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTagKey)); err != nil { + return err + } + + tagging = []types.Tag{{Key: getPtr("key"), Value: getPtr(genRandString(300))}} + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + CreateBucketConfiguration: &types.CreateBucketConfiguration{ + Tags: tagging, + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTagValue)); err != nil { + return err + } + + return nil + }) +} + +func CreateBucket_invalid_tags(s *S3Conf) error { + testName := "CreateBucket_invalid_tags" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + for i, test := range []struct { + tags []types.Tag + err s3err.APIError + }{ + // invalid tag key tests + {[]types.Tag{{Key: getPtr("user!name"), Value: getPtr("value")}}, s3err.GetAPIError(s3err.ErrInvalidTagKey)}, + {[]types.Tag{{Key: getPtr("foo#bar"), Value: getPtr("value")}}, s3err.GetAPIError(s3err.ErrInvalidTagKey)}, + {[]types.Tag{ + {Key: getPtr("validkey"), Value: getPtr("validvalue")}, + {Key: getPtr("data%20"), Value: getPtr("value")}, + }, s3err.GetAPIError(s3err.ErrInvalidTagKey)}, + {[]types.Tag{ + {Key: getPtr("abcd"), Value: getPtr("xyz123")}, + {Key: getPtr("a*b"), Value: getPtr("value")}, + }, s3err.GetAPIError(s3err.ErrInvalidTagKey)}, + // invalid tag value tests + {[]types.Tag{ + {Key: getPtr("hello"), Value: getPtr("world")}, + {Key: getPtr("key"), Value: getPtr("name?test")}, + }, s3err.GetAPIError(s3err.ErrInvalidTagValue)}, + {[]types.Tag{ + {Key: getPtr("foo"), Value: getPtr("bar")}, + {Key: getPtr("key"), Value: getPtr("`path")}, + }, s3err.GetAPIError(s3err.ErrInvalidTagValue)}, + {[]types.Tag{{Key: getPtr("valid"), Value: getPtr("comma,separated")}}, s3err.GetAPIError(s3err.ErrInvalidTagValue)}, + {[]types.Tag{{Key: getPtr("valid"), Value: getPtr("semicolon;test")}}, s3err.GetAPIError(s3err.ErrInvalidTagValue)}, + {[]types.Tag{{Key: getPtr("valid"), Value: getPtr("(parentheses)")}}, s3err.GetAPIError(s3err.ErrInvalidTagValue)}, + } { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + CreateBucketConfiguration: &types.CreateBucketConfiguration{ + Tags: test.tags, + }, + }) + cancel() + if err == nil { + return fmt.Errorf("test %v failed: expected err %w, instead got nil", i+1, test.err) + } + + if err := checkApiErr(err, test.err); err != nil { + return fmt.Errorf("test %v failed: %w", i+1, err) + } + } + + return nil + }) +} + +func CreateBucket_duplicate_keys(s *S3Conf) error { + testName := "CreateBucket_duplicate_keys" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + tagging := []types.Tag{ + {Key: getPtr("key"), Value: getPtr("value")}, + {Key: getPtr("key"), Value: getPtr("value-1")}, + {Key: getPtr("key-1"), Value: getPtr("value-2")}, + {Key: getPtr("key-2"), Value: getPtr("value-3")}, + } + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + CreateBucketConfiguration: &types.CreateBucketConfiguration{ + Tags: tagging, + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrDuplicateTagKey)); err != nil { + return err + } + + return nil + }) +} + +func CreateBucket_tag_count_limit(s *S3Conf) error { + testName := "CreateBucket_tag_count_limit" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + tagSet := []types.Tag{} + + for i := range 51 { + tagSet = append(tagSet, types.Tag{ + Key: getPtr(fmt.Sprintf("key-%v", i)), + Value: getPtr(genRandString(10)), + }) + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &bucket, + CreateBucketConfiguration: &types.CreateBucketConfiguration{ + Tags: tagSet, + }, + }) + cancel() + return checkApiErr(err, s3err.GetAPIError(s3err.ErrBucketTaggingLimited)) + }) +} + func HeadBucket_non_existing_bucket(s *S3Conf) error { testName := "HeadBucket_non_existing_bucket" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index ba8b8ca..debd7b1 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -265,7 +265,8 @@ func actionHandler(s *S3Conf, testName string, handler func(s3client *s3.Client, func actionHandlerNoSetup(s *S3Conf, testName string, handler func(s3client *s3.Client, bucket string) error, _ ...setupOpt) error { runF(testName) client := s.GetClient() - handlerErr := handler(client, "") + bucket := getBucketName() + handlerErr := handler(client, bucket) if handlerErr != nil { failF("%v: %v", testName, handlerErr) }