From 4c3965d87e83cf2d0d8d7c0f3639626e2307ffce Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Wed, 8 Oct 2025 10:23:02 -0700 Subject: [PATCH] feat: add option to disable strict bucket name checks Some systems may choose to allow non-aws compliant bucket names and/or handle the bucket naem validation in the backend instead. This adds the option to turn off the strict bucket name validation checks in the frontend API handlers. When frontend bucket name validation is disabled, we need to do sanity checks for posix compliant names in the posix/scoutfs backends. This is automatically enabled when strict bucket name validation is disabled. Fixes #1564 --- backend/common.go | 16 +++ backend/posix/posix.go | 195 ++++++++++++++++++++++++++++-- backend/scoutfs/scoutfs.go | 24 ++++ backend/scoutfs/scoutfs_compat.go | 11 +- cmd/versitygw/main.go | 10 ++ cmd/versitygw/posix.go | 13 +- cmd/versitygw/scoutfs.go | 1 + extra/example.conf | 6 + s3api/utils/utils.go | 15 +++ s3api/utils/utils_test.go | 22 ++++ 10 files changed, 291 insertions(+), 22 deletions(-) diff --git a/backend/common.go b/backend/common.go index f687c1e..f3d55f9 100644 --- a/backend/common.go +++ b/backend/common.go @@ -570,3 +570,19 @@ func EvaluateObjectDeletePreconditions(etag string, modTime time.Time, size int6 return nil } + +// IsValidDirectoryName returns true if the string is a valid name +// for a directory +func IsValidDirectoryName(name string) bool { + // directories may not contain a path separator + if strings.ContainsRune(name, '/') { + return false + } + + // directories may not contain null character + if strings.ContainsRune(name, 0) { + return false + } + + return true +} diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 18e435e..3d4765e 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -78,6 +78,10 @@ type Posix struct { // if the filesystem supports it. This is needed for cases where // there are different filesystems mounted below the bucket level. forceNoTmpFile bool + + // enable posix level bucket name validations, not needed if the + // frontend handlers are already validating bucket names + validateBucketName bool } var _ backend.Backend = &Posix{} @@ -131,6 +135,10 @@ type PosixOpts struct { // ForceNoTmpFile disables the use of O_TMPFILE even if the filesystem // supports it ForceNoTmpFile bool + // ValidateBucketNames enables minimal bucket name validation to prevent + // incorrect access to the filesystem. This is only needed if the + // frontend is not already validating bucket names. + ValidateBucketNames bool } func New(rootdir string, meta meta.MetadataStorer, opts PosixOpts) (*Posix, error) { @@ -180,17 +188,18 @@ func New(rootdir string, meta meta.MetadataStorer, opts PosixOpts) (*Posix, erro } return &Posix{ - meta: meta, - rootfd: f, - rootdir: rootdir, - euid: os.Geteuid(), - egid: os.Getegid(), - chownuid: opts.ChownUID, - chowngid: opts.ChownGID, - bucketlinks: opts.BucketLinks, - versioningDir: verioningdirAbs, - newDirPerm: opts.NewDirPerm, - forceNoTmpFile: opts.ForceNoTmpFile, + meta: meta, + rootfd: f, + rootdir: rootdir, + euid: os.Geteuid(), + egid: os.Getegid(), + chownuid: opts.ChownUID, + chowngid: opts.ChownGID, + bucketlinks: opts.BucketLinks, + versioningDir: verioningdirAbs, + newDirPerm: opts.NewDirPerm, + forceNoTmpFile: opts.ForceNoTmpFile, + validateBucketName: opts.ValidateBucketNames, }, nil } @@ -335,7 +344,18 @@ func (p *Posix) ListBuckets(_ context.Context, input s3response.ListBucketsInput }, nil } +func (p *Posix) isBucketValid(bucket string) bool { + if !p.validateBucketName { + return true + } + + return backend.IsValidDirectoryName(bucket) +} + func (p *Posix) HeadBucket(_ context.Context, input *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) { + if !p.isBucketValid(*input.Bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Lstat(*input.Bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -357,6 +377,10 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a bucket := *input.Bucket + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + err := os.Mkdir(bucket, p.newDirPerm) if err != nil && os.IsExist(err) { aclJSON, err := p.meta.RetrieveAttribute(nil, bucket, "", aclkey) @@ -459,6 +483,9 @@ func (p *Posix) isBucketEmpty(bucket string) error { } func (p *Posix) DeleteBucket(_ context.Context, bucket string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } // Check if the bucket is empty err := p.isBucketEmpty(bucket) if err != nil { @@ -482,6 +509,9 @@ func (p *Posix) DeleteBucket(_ context.Context, bucket string) error { } func (p *Posix) PutBucketOwnershipControls(_ context.Context, bucket string, ownership types.ObjectOwnership) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -499,6 +529,9 @@ func (p *Posix) PutBucketOwnershipControls(_ context.Context, bucket string, own } func (p *Posix) GetBucketOwnershipControls(_ context.Context, bucket string) (types.ObjectOwnership, error) { var ownship types.ObjectOwnership + if !p.isBucketValid(bucket) { + return ownship, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return ownship, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -518,6 +551,9 @@ func (p *Posix) GetBucketOwnershipControls(_ context.Context, bucket string) (ty return types.ObjectOwnership(ownership), nil } func (p *Posix) DeleteBucketOwnershipControls(_ context.Context, bucket string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -539,6 +575,9 @@ func (p *Posix) DeleteBucketOwnershipControls(_ context.Context, bucket string) } func (p *Posix) PutBucketVersioning(ctx context.Context, bucket string, status types.BucketVersioningStatus) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } if !p.versioningEnabled() { return s3err.GetAPIError(s3err.ErrVersioningNotConfigured) } @@ -588,6 +627,10 @@ func (p *Posix) GetBucketVersioning(_ context.Context, bucket string) (s3respons return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrVersioningNotConfigured) } + if !p.isBucketValid(bucket) { + return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -740,6 +783,10 @@ func (p *Posix) ListObjectVersions(ctx context.Context, input *s3.ListObjectVers var prefix, delim, keyMarker, versionIdMarker string var max int + if !p.isBucketValid(bucket) { + return s3response.ListVersionsResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + if input.Prefix != nil { prefix = *input.Prefix } @@ -1186,6 +1233,10 @@ func (p *Posix) CreateMultipartUpload(ctx context.Context, mpu s3response.Create bucket := *mpu.Bucket object := *mpu.Key + if !p.isBucketValid(bucket) { + return s3response.InitiateMultipartUploadResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3response.InitiateMultipartUploadResult{}, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -1388,6 +1439,10 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C uploadID := *input.UploadId parts := input.MultipartUpload.Parts + if !p.isBucketValid(bucket) { + return res, "", s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return res, "", s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -1994,6 +2049,10 @@ func (p *Posix) AbortMultipartUpload(_ context.Context, mpu *s3.AbortMultipartUp object := *mpu.Key uploadID := *mpu.UploadId + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -2030,6 +2089,10 @@ func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUpl bucket := *mpu.Bucket var delimiter string + + if !p.isBucketValid(bucket) { + return lmu, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } if mpu.Delimiter != nil { delimiter = *mpu.Delimiter } @@ -2190,6 +2253,10 @@ func (p *Posix) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3resp bucket := *input.Bucket object := *input.Key uploadID := *input.UploadId + + if !p.isBucketValid(bucket) { + return lpr, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } stringMarker := "" if input.PartNumberMarker != nil { stringMarker = *input.PartNumberMarker @@ -2340,6 +2407,10 @@ func (p *Posix) UploadPart(ctx context.Context, input *s3.UploadPartInput) (*s3. bucket := *input.Bucket object := *input.Key uploadID := *input.UploadId + + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } part := input.PartNumber length := int64(0) if input.ContentLength != nil { @@ -2521,6 +2592,10 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrNoSuchKey) } + if !p.isBucketValid(*upi.Bucket) { + return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(*upi.Bucket) if errors.Is(err, fs.ErrNotExist) { return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -2752,6 +2827,9 @@ func (p *Posix) PutObject(ctx context.Context, po s3response.PutObjectInput) (s3 if po.ChecksumAlgorithm == "" { po.ChecksumAlgorithm = types.ChecksumAlgorithmCrc64nvme } + if !p.isBucketValid(*po.Bucket) { + return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(*po.Bucket) if errors.Is(err, fs.ErrNotExist) { return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -3086,6 +3164,10 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( object := *input.Key isDir := strings.HasSuffix(object, "/") + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -3496,6 +3578,9 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO } bucket := *input.Bucket + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -3748,6 +3833,10 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. bucket := *input.Bucket object := *input.Key + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -3972,9 +4061,16 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput if err != nil { return s3response.CopyObjectOutput{}, err } + if !p.isBucketValid(srcBucket) { + return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } dstBucket := *input.Bucket dstObject := *input.Key + if !p.isBucketValid(dstBucket) { + return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err = os.Stat(srcBucket) if errors.Is(err, fs.ErrNotExist) { return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4308,6 +4404,10 @@ func (p *Posix) ListObjectsParametrized(ctx context.Context, input *s3.ListObjec maxkeys = *input.MaxKeys } + if !p.isBucketValid(bucket) { + return s3response.ListObjectsResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3response.ListObjectsResult{}, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4337,6 +4437,12 @@ func (p *Posix) ListObjectsParametrized(ctx context.Context, input *s3.ListObjec } func (p *Posix) FileToObj(bucket string, fetchOwner bool) backend.GetObjFunc { + if !p.isBucketValid(bucket) { + return func(string, fs.DirEntry) (s3response.Object, error) { + return s3response.Object{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + } + return func(path string, d fs.DirEntry) (s3response.Object, error) { var owner *types.Owner // Retreive the object owner data from bucket ACL, if fetchOwner is true @@ -4469,6 +4575,10 @@ func (p *Posix) ListObjectsV2Parametrized(ctx context.Context, input *s3.ListObj fetchOwner = *input.FetchOwner } + if !p.isBucketValid(bucket) { + return s3response.ListObjectsV2Result{}, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3response.ListObjectsV2Result{}, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4502,6 +4612,9 @@ func (p *Posix) ListObjectsV2Parametrized(ctx context.Context, input *s3.ListObj } func (p *Posix) PutBucketAcl(_ context.Context, bucket string, data []byte) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4519,6 +4632,9 @@ func (p *Posix) PutBucketAcl(_ context.Context, bucket string, data []byte) erro } func (p *Posix) GetBucketAcl(_ context.Context, input *s3.GetBucketAclInput) ([]byte, error) { + if !p.isBucketValid(*input.Bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(*input.Bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4538,6 +4654,9 @@ func (p *Posix) GetBucketAcl(_ context.Context, input *s3.GetBucketAclInput) ([] } func (p *Posix) PutBucketTagging(_ context.Context, bucket string, tags map[string]string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4569,6 +4688,9 @@ func (p *Posix) PutBucketTagging(_ context.Context, bucket string, tags map[stri } func (p *Posix) GetBucketTagging(_ context.Context, bucket string) (map[string]string, error) { + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4586,10 +4708,16 @@ func (p *Posix) GetBucketTagging(_ context.Context, bucket string) (map[string]s } func (p *Posix) DeleteBucketTagging(ctx context.Context, bucket string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } return p.PutBucketTagging(ctx, bucket, nil) } func (p *Posix) GetObjectTagging(_ context.Context, bucket, object string) (map[string]string, error) { + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4623,6 +4751,9 @@ func (p *Posix) getAttrTags(bucket, object string) (map[string]string, error) { } func (p *Posix) PutObjectTagging(_ context.Context, bucket, object string, tags map[string]string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4662,10 +4793,16 @@ func (p *Posix) PutObjectTagging(_ context.Context, bucket, object string, tags } func (p *Posix) DeleteObjectTagging(ctx context.Context, bucket, object string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } return p.PutObjectTagging(ctx, bucket, object, nil) } func (p *Posix) PutBucketPolicy(ctx context.Context, bucket string, policy []byte) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4696,6 +4833,9 @@ func (p *Posix) PutBucketPolicy(ctx context.Context, bucket string, policy []byt } func (p *Posix) GetBucketPolicy(ctx context.Context, bucket string) ([]byte, error) { + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4719,10 +4859,16 @@ func (p *Posix) GetBucketPolicy(ctx context.Context, bucket string) ([]byte, err } func (p *Posix) DeleteBucketPolicy(ctx context.Context, bucket string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } return p.PutBucketPolicy(ctx, bucket, nil) } func (p *Posix) PutBucketCors(_ context.Context, bucket string, cors []byte) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4749,6 +4895,9 @@ func (p *Posix) PutBucketCors(_ context.Context, bucket string, cors []byte) err } func (p *Posix) GetBucketCors(_ context.Context, bucket string) ([]byte, error) { + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4769,6 +4918,9 @@ func (p *Posix) GetBucketCors(_ context.Context, bucket string) ([]byte, error) } func (p *Posix) DeleteBucketCors(ctx context.Context, bucket string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } return p.PutBucketCors(ctx, bucket, nil) } @@ -4797,6 +4949,9 @@ func (p *Posix) isBucketObjectLockEnabled(bucket string) error { } func (p *Posix) PutObjectLockConfiguration(ctx context.Context, bucket string, config []byte) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4831,6 +4986,9 @@ func (p *Posix) PutObjectLockConfiguration(ctx context.Context, bucket string, c } func (p *Posix) GetObjectLockConfiguration(_ context.Context, bucket string) ([]byte, error) { + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -4851,6 +5009,9 @@ func (p *Posix) GetObjectLockConfiguration(_ context.Context, bucket string) ([] } func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId string, status bool) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } err := p.doesBucketAndObjectExist(bucket, object) if err != nil { return err @@ -4901,6 +5062,9 @@ func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId } func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId string) (*bool, error) { + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } err := p.doesBucketAndObjectExist(bucket, object) if err != nil { return nil, err @@ -4949,6 +5113,9 @@ func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId } func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId string, retention []byte) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } err := p.doesBucketAndObjectExist(bucket, object) if err != nil { return err @@ -4986,6 +5153,9 @@ func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId } func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId string) ([]byte, error) { + if !p.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } err := p.doesBucketAndObjectExist(bucket, object) if err != nil { return nil, err @@ -5032,6 +5202,9 @@ func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId } func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket, owner string) error { + if !p.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } return auth.UpdateBucketACLOwner(ctx, p, bucket, owner) } diff --git a/backend/scoutfs/scoutfs.go b/backend/scoutfs/scoutfs.go index 28eab79..b6d98cb 100644 --- a/backend/scoutfs/scoutfs.go +++ b/backend/scoutfs/scoutfs.go @@ -51,6 +51,10 @@ type ScoutfsOpts struct { GlacierMode bool // DisableNoArchive prevents setting noarchive on temporary files DisableNoArchive bool + // ValidateBucketNames enables minimal bucket name validation to prevent + // incorrect access to the filesystem. This is only needed if the + // frontend is not already validating bucket names. + ValidateBucketNames bool } type ScoutFS struct { @@ -73,6 +77,10 @@ type ScoutFS struct { // on mutlipart parts. This is enabled by default to prevent archive // copies of temporary multipart parts. disableNoArchive bool + + // enable posix level bucket name validations, not needed if the + // frontend handlers are already validating bucket names + validateBucketName bool } var _ backend.Backend = &ScoutFS{} @@ -195,10 +203,22 @@ func (s *ScoutFS) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s return res, nil } +func (s *ScoutFS) isBucketValid(bucket string) bool { + if !s.validateBucketName { + return true + } + + return backend.IsValidDirectoryName(bucket) +} + func (s *ScoutFS) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.GetObjectOutput, error) { bucket := *input.Bucket object := *input.Key + if !s.isBucketValid(bucket) { + return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -290,6 +310,10 @@ func (s *ScoutFS) RestoreObject(_ context.Context, input *s3.RestoreObjectInput) bucket := *input.Bucket object := *input.Key + if !s.isBucketValid(bucket) { + return s3err.GetAPIError(s3err.ErrInvalidBucketName) + } + _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return s3err.GetAPIError(s3err.ErrNoSuchBucket) diff --git a/backend/scoutfs/scoutfs_compat.go b/backend/scoutfs/scoutfs_compat.go index 5f4ad8c..5fd2a30 100644 --- a/backend/scoutfs/scoutfs_compat.go +++ b/backend/scoutfs/scoutfs_compat.go @@ -30,11 +30,12 @@ func New(rootdir string, opts ScoutfsOpts) (*ScoutFS, error) { metastore := meta.XattrMeta{} p, err := posix.New(rootdir, metastore, posix.PosixOpts{ - ChownUID: opts.ChownUID, - ChownGID: opts.ChownGID, - BucketLinks: opts.BucketLinks, - NewDirPerm: opts.NewDirPerm, - VersioningDir: opts.VersioningDir, + ChownUID: opts.ChownUID, + ChownGID: opts.ChownGID, + BucketLinks: opts.BucketLinks, + NewDirPerm: opts.NewDirPerm, + VersioningDir: opts.VersioningDir, + ValidateBucketNames: opts.ValidateBucketNames, }) if err != nil { return nil, err diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index 815c499..710b7a1 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -32,6 +32,7 @@ import ( "github.com/versity/versitygw/metrics" "github.com/versity/versitygw/s3api" "github.com/versity/versitygw/s3api/middlewares" + "github.com/versity/versitygw/s3api/utils" "github.com/versity/versitygw/s3event" "github.com/versity/versitygw/s3log" ) @@ -58,6 +59,7 @@ var ( pprof string quiet bool readonly bool + disableStrictBucketNames bool iamDir string ldapURL, ldapBindDN, ldapPassword string ldapQueryBase, ldapObjClasses string @@ -557,6 +559,12 @@ func initFlags() []cli.Flag { EnvVars: []string{"VGW_READ_ONLY"}, Destination: &readonly, }, + &cli.BoolFlag{ + Name: "disable-strict-bucket-names", + Usage: "allow relaxed bucket naming (disables strict validation checks)", + EnvVars: []string{"VGW_DISABLE_STRICT_BUCKET_NAMES"}, + Destination: &disableStrictBucketNames, + }, &cli.StringFlag{ Name: "metrics-service-name", Usage: "service name tag for metrics, hostname if blank", @@ -616,6 +624,8 @@ func runGateway(ctx context.Context, be backend.Backend) error { return fmt.Errorf("root user access and secret key must be provided") } + utils.SetBucketNameValidationStrict(!disableStrictBucketNames) + if pprof != "" { // listen on specified port for pprof debug // point browser to http:///debug/pprof/ diff --git a/cmd/versitygw/posix.go b/cmd/versitygw/posix.go index b557ea3..17aa704 100644 --- a/cmd/versitygw/posix.go +++ b/cmd/versitygw/posix.go @@ -120,12 +120,13 @@ func runPosix(ctx *cli.Context) error { } opts := posix.PosixOpts{ - ChownUID: chownuid, - ChownGID: chowngid, - BucketLinks: bucketlinks, - VersioningDir: versioningDir, - NewDirPerm: fs.FileMode(dirPerms), - ForceNoTmpFile: forceNoTmpFile, + ChownUID: chownuid, + ChownGID: chowngid, + BucketLinks: bucketlinks, + VersioningDir: versioningDir, + NewDirPerm: fs.FileMode(dirPerms), + ForceNoTmpFile: forceNoTmpFile, + ValidateBucketNames: disableStrictBucketNames, } var ms meta.MetadataStorer diff --git a/cmd/versitygw/scoutfs.go b/cmd/versitygw/scoutfs.go index 6550ed6..59e63ee 100644 --- a/cmd/versitygw/scoutfs.go +++ b/cmd/versitygw/scoutfs.go @@ -113,6 +113,7 @@ func runScoutfs(ctx *cli.Context) error { opts.NewDirPerm = fs.FileMode(dirPerms) opts.DisableNoArchive = disableNoArchive opts.VersioningDir = versioningDir + opts.ValidateBucketNames = disableStrictBucketNames be, err := scoutfs.New(ctx.Args().Get(0), opts) if err != nil { diff --git a/extra/example.conf b/extra/example.conf index 50f8e0e..95bc444 100644 --- a/extra/example.conf +++ b/extra/example.conf @@ -120,6 +120,12 @@ ROOT_SECRET_ACCESS_KEY= # https:/// #VGW_VIRTUAL_DOMAIN= +# By default, versitygw will enforce similar bucket naming rules as described +# in https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html +# Set to true to allow legacy or non-DNS-compliant bucket names by skipping +# strict validation checks. +#VGW_DISABLE_STRICT_BUCKET_NAMES=false + ############### # Access Logs # ############### diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index 8d28de9..589e253 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -26,6 +26,7 @@ import ( "regexp" "strconv" "strings" + "sync/atomic" "time" "github.com/aws/aws-sdk-go-v2/service/s3/types" @@ -41,6 +42,16 @@ var ( bucketNameIpRegexp = regexp.MustCompile(`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$`) ) +var strictBucketNameValidation atomic.Bool + +func init() { + strictBucketNameValidation.Store(true) +} + +func SetBucketNameValidationStrict(strict bool) { + strictBucketNameValidation.Store(strict) +} + func GetUserMetaData(headers *fasthttp.RequestHeader) (metadata map[string]string) { metadata = make(map[string]string) headers.DisableNormalizing() @@ -209,6 +220,10 @@ func StreamResponseBody(ctx *fiber.Ctx, rdr io.ReadCloser, bodysize int) { } func IsValidBucketName(bucket string) bool { + if !strictBucketNameValidation.Load() { + return true + } + if len(bucket) < 3 || len(bucket) > 63 { debuglogger.Logf("bucket name length should be in 3-63 range, got: %v\n", len(bucket)) return false diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 2bf32e4..bf5d4bd 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -231,6 +231,28 @@ func TestIsValidBucketName(t *testing.T) { } } +func TestSetBucketNameValidationStrict(t *testing.T) { + SetBucketNameValidationStrict(true) + t.Cleanup(func() { + SetBucketNameValidationStrict(true) + }) + + invalidBucket := "Invalid_Bucket" + if IsValidBucketName(invalidBucket) { + t.Fatalf("expected %q to be invalid with strict validation", invalidBucket) + } + + SetBucketNameValidationStrict(false) + if !IsValidBucketName(invalidBucket) { + t.Fatalf("expected %q to be accepted when strict validation disabled", invalidBucket) + } + + SetBucketNameValidationStrict(true) + if IsValidBucketName(invalidBucket) { + t.Fatalf("expected %q to be invalid after re-enabling strict validation", invalidBucket) + } +} + func TestParseUint(t *testing.T) { type args struct { str string