mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2026-04-24 14:40:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1371 from versity/sis/bucket-object-name-validation

Browse Source 
feat: adds a middleware to validate bucket/object names
This commit is contained in:
Ben McClelland
2025-07-03 19:57:18 -07:00
committed by GitHub
parent b09efa532c 98a7b7f402
commit 4ce0ba33e9
9 changed files with 151 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
backend/posix/posix.go
View File

@@ -334,10 +334,6 @@ func (p *Posix) ListBuckets(_ context.Context, input s3response.ListBucketsInput
}
func (p *Posix) HeadBucket(_ context.Context, input *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) {
if input.Bucket == nil {
return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
_, err := os.Lstat(*input.Bucket)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)
@@ -350,10 +346,6 @@ func (p *Posix) HeadBucket(_ context.Context, input *s3.HeadBucketInput) (*s3.He
}
func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, acl []byte) error {
if input.Bucket == nil {
return s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
acct, ok := ctx.Value("account").(auth.Account)
if !ok {
acct = auth.Account{}
@@ -465,10 +457,6 @@ func (p *Posix) isBucketEmpty(bucket string) error {
}
func (p *Posix) DeleteBucket(_ context.Context, bucket string) error {
if bucket == "" {
return s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
// Check if the bucket is empty
err := p.isBucketEmpty(bucket)
if err != nil {
@@ -1189,9 +1177,6 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc {
}
func (p *Posix) CreateMultipartUpload(ctx context.Context, mpu s3response.CreateMultipartUploadInput) (s3response.InitiateMultipartUploadResult, error) {
if mpu.Bucket == nil {
return s3response.InitiateMultipartUploadResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if mpu.Key == nil {
return s3response.InitiateMultipartUploadResult{}, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -1380,9 +1365,6 @@ func (p *Posix) CompleteMultipartUpload(ctx context.Context, input *s3.CompleteM
var res s3response.CompleteMultipartUploadResult
if input.Bucket == nil {
return res, "", s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return res, "", s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -1950,9 +1932,6 @@ func isValidMeta(val string) bool {
}
func (p *Posix) AbortMultipartUpload(_ context.Context, mpu *s3.AbortMultipartUploadInput) error {
if mpu.Bucket == nil {
return s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if mpu.Key == nil {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -1992,10 +1971,6 @@ func (p *Posix) AbortMultipartUpload(_ context.Context, mpu *s3.AbortMultipartUp
func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResult, error) {
var lmu s3response.ListMultipartUploadsResult
if mpu.Bucket == nil {
return lmu, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
bucket := *mpu.Bucket
var delimiter string
if mpu.Delimiter != nil {
@@ -2143,9 +2118,6 @@ func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUpl
func (p *Posix) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3response.ListPartsResult, error) {
var lpr s3response.ListPartsResult
if input.Bucket == nil {
return lpr, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return lpr, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -2299,9 +2271,6 @@ func (p *Posix) UploadPart(ctx context.Context, input *s3.UploadPartInput) (*s3.
acct = auth.Account{}
}
if input.Bucket == nil {
return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -2469,9 +2438,6 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput)
acct = auth.Account{}
}
if upi.Bucket == nil {
return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if upi.Key == nil {
return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -2683,9 +2649,6 @@ func (p *Posix) PutObject(ctx context.Context, po s3response.PutObjectInput) (s3
acct = auth.Account{}
}
if po.Bucket == nil {
return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if po.Key == nil {
return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -3003,9 +2966,6 @@ func (p *Posix) PutObject(ctx context.Context, po s3response.PutObjectInput) (s3
}
func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
if input.Bucket == nil {
return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -3361,9 +3321,6 @@ func (p *Posix) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInput)
}
func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetObjectOutput, error) {
if input.Bucket == nil {
return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -3592,9 +3549,6 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
}
func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) {
if input.Bucket == nil {
return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -3862,9 +3816,6 @@ func (p *Posix) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttr
}
func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput) (s3response.CopyObjectOutput, error) {
if input.Bucket == nil {
return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidCopyDest)
}
@@ -4177,9 +4128,6 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput
}
func (p *Posix) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (s3response.ListObjectsResult, error) {
if input.Bucket == nil {
return s3response.ListObjectsResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
bucket := *input.Bucket
prefix := ""
if input.Prefix != nil {
@@ -4329,9 +4277,6 @@ func (p *Posix) fileToObj(bucket string, fetchOwner bool) backend.GetObjFunc {
}
func (p *Posix) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input) (s3response.ListObjectsV2Result, error) {
if input.Bucket == nil {
return s3response.ListObjectsV2Result{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
bucket := *input.Bucket
prefix := ""
if input.Prefix != nil {
@@ -4408,9 +4353,6 @@ func (p *Posix) PutBucketAcl(_ context.Context, bucket string, data []byte) erro
}
func (p *Posix) GetBucketAcl(_ context.Context, input *s3.GetBucketAclInput) ([]byte, error) {
if input.Bucket == nil {
return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
_, err := os.Stat(*input.Bucket)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)

9
backend/scoutfs/scoutfs.go
View File

@@ -201,9 +201,6 @@ func (s *ScoutFS) CompleteMultipartUpload(ctx context.Context, input *s3.Complet
var res s3response.CompleteMultipartUploadResult
if input.Bucket == nil {
return res, "", s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if input.Key == nil {
return res, "", s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -730,9 +727,6 @@ func (s *ScoutFS) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.
}
func (s *ScoutFS) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (s3response.ListObjectsResult, error) {
if input.Bucket == nil {
return s3response.ListObjectsResult{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
bucket := *input.Bucket
prefix := ""
if input.Prefix != nil {
@@ -780,9 +774,6 @@ func (s *ScoutFS) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (
}
func (s *ScoutFS) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input) (s3response.ListObjectsV2Result, error) {
if input.Bucket == nil {
return s3response.ListObjectsV2Result{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
bucket := *input.Bucket
prefix := ""
if input.Prefix != nil {

8
s3api/controllers/base.go
View File

@@ -1720,14 +1720,6 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error {
})
}
if ok := utils.IsValidBucketName(bucket); !ok {
return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidBucketName),
&MetaOpts{
Logger: c.logger,
MetricsMng: c.mm,
Action: metrics.ActionCreateBucket,
})
}
if ok := utils.IsValidOwnership(objectOwnership); !ok {
return SendResponse(ctx, s3err.APIError{
Code: "InvalidArgument",

9
s3api/controllers/base_test.go
View File

@@ -884,15 +884,6 @@ func TestS3ApiController_PutBucketActions(t *testing.T) {
wantErr: false,
statusCode: 400,
},
{
name: "Create-bucket-invalid-bucket-name",
app: app,
args: args{
req: httptest.NewRequest(http.MethodPut, "/aa", nil),
},
wantErr: false,
statusCode: 400,
},
{
name: "Create-bucket-success",
app: app,

58
s3api/middlewares/bucket-object-name-validator.go Normal file
View File

@@ -0,0 +1,58 @@
// Copyright 2023 Versity Software
// This file is licensed under the Apache License, Version 2.0
// (the "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package middlewares
import (
"net/http"
"github.com/gofiber/fiber/v2"
"github.com/versity/versitygw/metrics"
"github.com/versity/versitygw/s3api/utils"
"github.com/versity/versitygw/s3err"
"github.com/versity/versitygw/s3log"
)
// BucketObjectNameValidator extracts and validates
// the bucket and object names from the request URI.
func BucketObjectNameValidator(l s3log.AuditLogger, mm *metrics.Manager) fiber.Handler {
return func(ctx *fiber.Ctx) error {
// skip the check for admin apis
if ctx.Method() == http.MethodPatch {
return ctx.Next()
}
path := ctx.Path()
// skip the check if the operation isn't bucket/object scoped
// e.g ListBuckets
if path == "/" {
return ctx.Next()
}
bucket, object := parsePath(path)
// check if the provided bucket name is valid
if !utils.IsValidBucketName(bucket) {
return sendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidBucketName), l, mm)
}
// check if the provided object name is valid
// skip for empty objects: e.g bucket operations: HeadBucket...
if object != "" && !utils.IsObjectNameValid(object) {
return sendResponse(ctx, s3err.GetAPIError(s3err.ErrBadRequest), l, mm)
}
return ctx.Next()
}
}

3
s3api/server.go
View File

@@ -91,6 +91,9 @@ func New(
app.Use(middlewares.DebugLogger())
}
// initialize the bucket/object name validator
app.Use(middlewares.BucketObjectNameValidator(l, mm))
// Public buckets access checker
app.Use(middlewares.AuthorizePublicBucketAccess(be, l, mm))

6
s3err/s3err.go
View File

@@ -167,6 +167,7 @@ const (
ErrChecksumTypeWithAlgo
ErrInvalidChecksumHeader
ErrTrailerHeaderNotSupported
ErrBadRequest
// Non-AWS errors
ErrExistingObjectIsDirectory
@@ -726,6 +727,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
Description: "The value specified in the x-amz-trailer header is not supported",
HTTPStatusCode: http.StatusBadRequest,
},
ErrBadRequest: {
Code: "400",
Description: "Bad Request",
HTTPStatusCode: http.StatusBadRequest,
},
// non aws errors
ErrExistingObjectIsDirectory: {

5
tests/integration/group-tests.go
View File

@@ -150,12 +150,15 @@ func TestPutObject(s *S3Conf) {
PutObject_incorrect_checksums(s)
PutObject_default_checksum(s)
PutObject_checksums_success(s)
// azure applies some encoding mechanisms.
PutObject_false_negative_object_names(s)
}
PutObject_success(s)
if !s.versioningEnabled {
PutObject_racey_success(s)
}
PutObject_invalid_credentials(s)
PutObject_invalid_object_names(s)
}
func TestHeadObject(s *S3Conf) {
@@ -969,6 +972,8 @@ func GetIntTests() IntTests {
"PutObject_special_chars": PutObject_special_chars,
"PutObject_tagging": PutObject_tagging,
"PutObject_success": PutObject_success,
"PutObject_invalid_object_names": PutObject_invalid_object_names,
"PutObject_false_negative_object_names": PutObject_false_negative_object_names,
"PutObject_racey_success": PutObject_racey_success,
"HeadObject_non_existing_object": HeadObject_non_existing_object,
"HeadObject_invalid_part_number": HeadObject_invalid_part_number,

83
tests/integration/tests.go
View File

@@ -3415,6 +3415,81 @@ func PutObject_invalid_credentials(s *S3Conf) error {
})
}
func PutObject_invalid_object_names(s *S3Conf) error {
testName := "PutObject_invalid_object_names"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
for _, obj := range []string{
".",
"..",
"./",
"/.",
"//",
"../",
"/..",
"/..",
"../.",
"../../../.",
"../../../etc/passwd",
"../../../../tmp/foo",
"for/../../bar/",
"a/a/a/../../../../../etc/passwd",
"/a/../../b/../../c/../../../etc/passwd",
} {
_, err := putObjects(s3client, []string{obj}, bucket)
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrBadRequest)); err != nil {
return 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 {
objs := []string{
"%252e%252e%252fetc/passwd", // double encoding
"%2e%2e/%2e%2e/%2e%2e/.ssh/id_rsa", // double URL-encoded
"%u002e%u002e/%u002e%u002e/etc/passwd", // unicode escape
"..%2f..%2f..%2fsecret/file.txt", // URL-encoded
"..%c0%af..%c0%afetc/passwd", // UTF-8 overlong trick
".../.../.../target.txt",
"..\\u2215..\\u2215etc/passwd", // Unicode division slash
"dir/%20../file.txt", // encoded space
"dir/%c0%ae%c0%ae/%c0%ae%c0%ae/etc/passwd", // overlong UTF-8 encoding
"logs/latest -> /etc/passwd", // symlink attacks
//TODO: add this test case in advanced routing
// "/etc/passwd" // absolute path injection
}
_, err := putObjects(s3client, objs, bucket)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
res, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}
if len(res.Contents) != len(objs) {
return fmt.Errorf("expected %v objects, instead got %v", len(objs), len(res.Contents))
}
for i, obj := range res.Contents {
if *obj.Key != objs[i] {
return fmt.Errorf("expected the %vth object name to be %s, instead got %s", i+1, objs[i], *obj.Key)
}
}
return nil
})
}
func HeadObject_non_existing_object(s *S3Conf) error {
testName := "HeadObject_non_existing_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -9995,7 +10070,7 @@ func AbortMultipartUpload_non_existing_bucket(s *S3Conf) error {
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.AbortMultipartUpload(ctx, &s3.AbortMultipartUploadInput{
Bucket: getPtr("incorrectBucket"),
Bucket: getPtr("incorrect-bucket"),
Key: getPtr("my-obj"),
UploadId: getPtr("uploadId"),
})
@@ -12328,7 +12403,7 @@ func PutBucketPolicy_non_existing_bucket(s *S3Conf) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
doc := genPolicyDoc("Allow", `"*"`, `"s3:*"`, fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket))
_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
Bucket: getPtr("non_existing_bucket"),
Bucket: getPtr("non-existing-bucket"),
Policy: &doc,
})
cancel()
@@ -13003,7 +13078,7 @@ func GetBucketPolicy_non_existing_bucket(s *S3Conf) error {
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.GetBucketPolicy(ctx, &s3.GetBucketPolicyInput{
Bucket: getPtr("non_existing_bucket"),
Bucket: getPtr("non-existing-bucket"),
})
cancel()
@@ -13069,7 +13144,7 @@ func DeleteBucketPolicy_non_existing_bucket(s *S3Conf) error {
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.DeleteBucketPolicy(ctx, &s3.DeleteBucketPolicyInput{
Bucket: getPtr("non_existing_bucket"),
Bucket: getPtr("non-existing-bucket"),
})
cancel()