fix: move versionId validation to backend

Closes #1813

We use a specific `versionId` format(`ulid` package) to generate versionIds in posix, which is not compatible to S3. The versionId validation was performed in frontend which is a potential source of failure for s3 proxy configured on an s3 service which doesn't use ulid for versionId generation(e.g. aws S3). These changes move the specific `ulid` versionId validation to posix to not force any specific versionId format in the gateway.
This commit is contained in:
niksis02
2026-04-07 01:56:51 +04:00
parent 09413c5f60
commit b473aa0545
12 changed files with 59 additions and 352 deletions
+54
View File
@@ -347,6 +347,18 @@ func (p *Posix) versioningEnabled() bool {
return p.versioningDir != ""
}
// validateVersionId checks if the input versionId is 'ulid' compatible
func (p *Posix) validateVersionId(versionId string) error {
if versionId == "" || versionId == "null" {
return nil
}
_, err := ulid.Parse(versionId)
if err != nil {
return s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
return nil
}
func (p *Posix) doesBucketAndObjectExist(bucket, object string) error {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
@@ -3044,6 +3056,9 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput)
if err != nil {
return s3response.CopyPartResult{}, err
}
if err := p.validateVersionId(srcVersionId); err != nil {
return s3response.CopyPartResult{}, err
}
_, err = os.Stat(srcBucket)
if errors.Is(err, fs.ErrNotExist) {
@@ -3727,6 +3742,10 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
if err := p.validateVersionId(backend.GetStringFromPtr(input.VersionId)); err != nil {
return nil, err
}
_, err = os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)
@@ -4146,6 +4165,10 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge
versionId = *input.VersionId
}
if err := p.validateVersionId(versionId); err != nil {
return nil, err
}
if input.PartNumber != nil {
// querying an object by part number is not supported
return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
@@ -4411,6 +4434,10 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
}
versionId := backend.GetStringFromPtr(input.VersionId)
if err := p.validateVersionId(versionId); err != nil {
return nil, err
}
if input.PartNumber != nil {
// querying an object by part number is not supported
return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
@@ -4670,6 +4697,9 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput
if err != nil {
return s3response.CopyObjectOutput{}, err
}
if err := p.validateVersionId(srcVersionId); err != nil {
return s3response.CopyObjectOutput{}, err
}
if !p.isBucketValid(srcBucket) {
return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
@@ -5381,6 +5411,10 @@ func (p *Posix) GetObjectTagging(ctx context.Context, bucket, object, versionId
return nil, fmt.Errorf("stat bucket: %w", err)
}
if err := p.validateVersionId(versionId); err != nil {
return nil, err
}
if versionId != "" {
if !p.versioningEnabled() {
//TODO: Maybe we need to return our custom error here?
@@ -5454,6 +5488,10 @@ func (p *Posix) PutObjectTagging(ctx context.Context, bucket, object, versionId
return fmt.Errorf("stat bucket: %w", err)
}
if err := p.validateVersionId(versionId); err != nil {
return err
}
if versionId != "" {
if !p.versioningEnabled() {
//TODO: Maybe we need to return our custom error here?
@@ -5785,6 +5823,10 @@ func (p *Posix) PutObjectLegalHold(ctx context.Context, bucket, object, versionI
return err
}
if err := p.validateVersionId(versionId); err != nil {
return err
}
var statusData []byte
if status {
statusData = []byte{1}
@@ -5849,6 +5891,10 @@ func (p *Posix) GetObjectLegalHold(ctx context.Context, bucket, object, versionI
return nil, err
}
if err := p.validateVersionId(versionId); err != nil {
return nil, err
}
if versionId != "" {
if !p.versioningEnabled() {
//TODO: Maybe we need to return our custom error here?
@@ -5911,6 +5957,10 @@ func (p *Posix) PutObjectRetention(ctx context.Context, bucket, object, versionI
return err
}
if err := p.validateVersionId(versionId); err != nil {
return err
}
if versionId != "" {
if !p.versioningEnabled() {
//TODO: Maybe we need to return our custom error here?
@@ -5962,6 +6012,10 @@ func (p *Posix) GetObjectRetention(ctx context.Context, bucket, object, versionI
return nil, err
}
if err := p.validateVersionId(versionId); err != nil {
return nil, err
}
if versionId != "" {
if !p.versioningEnabled() {
//TODO: Maybe we need to return our custom error here?
-18
View File
@@ -62,15 +62,6 @@ func (c S3ApiController) DeleteObjectTagging(ctx *fiber.Ctx) (*Response, error)
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
err = c.be.DeleteObjectTagging(ctx.Context(), bucket, key, versionId)
return &Response{
Headers: map[string]*string{
@@ -170,15 +161,6 @@ func (c S3ApiController) DeleteObject(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
err = auth.CheckObjectAccess(
ctx.Context(),
bucket,
-34
View File
@@ -47,23 +47,6 @@ func TestS3ApiController_DeleteObjectTagging(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "backend returns error",
input: testInput{
@@ -238,23 +221,6 @@ func TestS3ApiController_DeleteObject(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "object locked",
input: testInput{
-45
View File
@@ -65,15 +65,6 @@ func (c S3ApiController) GetObjectTagging(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
data, err := c.be.GetObjectTagging(ctx.Context(), bucket, key, versionId)
if err != nil {
return &Response{
@@ -132,15 +123,6 @@ func (c S3ApiController) GetObjectRetention(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
data, err := c.be.GetObjectRetention(ctx.Context(), bucket, key, versionId)
if err != nil {
return &Response{
@@ -189,15 +171,6 @@ func (c S3ApiController) GetObjectLegalHold(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
data, err := c.be.GetObjectLegalHold(ctx.Context(), bucket, key, versionId)
return &Response{
Data: auth.ParseObjectLegalHoldOutput(data),
@@ -351,15 +324,6 @@ func (c S3ApiController) GetObjectAttributes(ctx *fiber.Ctx) (*Response, error)
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
var maxParts *int32
// parse max parts
parsed, err := utils.ParseMaxLimiter(maxPartsStr, utils.LimiterTypeMaxParts)
@@ -499,15 +463,6 @@ func (c S3ApiController) GetObject(ctx *fiber.Ctx) (*Response, error) {
partNumber = &partNumberQuery
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
// validate the checksum mode
if checksumMode != "" && checksumMode != types.ChecksumModeEnabled {
debuglogger.Logf("invalid x-amz-checksum-mode header value: %v", checksumMode)
-85
View File
@@ -54,23 +54,6 @@ func TestS3ApiController_GetObjectTagging(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "backend returns error",
input: testInput{
@@ -173,23 +156,6 @@ func TestS3ApiController_GetObjectRetention(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "backend returns error",
input: testInput{
@@ -293,23 +259,6 @@ func TestS3ApiController_GetObjectLegalHold(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "backend returns error",
input: testInput{
@@ -617,23 +566,6 @@ func TestS3ApiController_GetObjectAttributes(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "invalid object attributes",
input: testInput{
@@ -756,23 +688,6 @@ func TestS3ApiController_GetObject(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "invalid checksum mode",
input: testInput{
-9
View File
@@ -110,15 +110,6 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) (*Response, error) {
partNumber = &partNumberQuery
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
checksumMode := types.ChecksumMode(strings.ToUpper(ctx.Get("x-amz-checksum-mode")))
if checksumMode != "" && checksumMode != types.ChecksumModeEnabled {
debuglogger.Logf("invalid x-amz-checksum-mode header value: %v", checksumMode)
-17
View File
@@ -80,23 +80,6 @@ func TestS3ApiController_HeadObject(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAnonymousResponseHeaders),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "invalid part number",
input: testInput{
-27
View File
@@ -67,15 +67,6 @@ func (c S3ApiController) PutObjectTagging(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
tagging, err := utils.ParseTagging(ctx.Body(), utils.TagLimitObject)
if err != nil {
return &Response{
@@ -127,15 +118,6 @@ func (c S3ApiController) PutObjectRetention(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
// parse the request body bytes into a go struct and validate
retention, err := auth.ParseObjectLockRetentionInput(ctx.Body())
if err != nil {
@@ -203,15 +185,6 @@ func (c S3ApiController) PutObjectLegalHold(ctx *fiber.Ctx) (*Response, error) {
}, err
}
err = utils.ValidateVersionId(versionId)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
var legalHold types.ObjectLockLegalHold
if err := xml.Unmarshal(ctx.Body(), &legalHold); err != nil {
debuglogger.Logf("failed to parse request body: %v", err)
-88
View File
@@ -67,23 +67,6 @@ func TestS3ApiController_PutObjectTagging(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "invalid request body",
input: testInput{
@@ -204,23 +187,6 @@ func TestS3ApiController_PutObjectRetention(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid versionId",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "invalid request body",
input: testInput{
@@ -349,23 +315,6 @@ func TestS3ApiController_PutObjectLegalHold(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid request body",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"versionId": "invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "invalid request body",
input: testInput{
@@ -649,26 +598,6 @@ func TestS3ApiController_UploadPartCopy(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrAccessDenied),
},
},
{
name: "invalid copy source: invalid versionId",
input: testInput{
locals: defaultLocals,
headers: map[string]string{
"X-Amz-Copy-Source": "bucket/object?versionId=invalid_versionId",
},
queries: map[string]string{
"partNumber": "2",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "invalid copy source",
input: testInput{
@@ -929,23 +858,6 @@ func TestS3ApiController_CopyObject(t *testing.T) {
err: s3err.GetAPIError(s3err.ErrInvalidCopySourceBucket),
},
},
{
name: "invalid copy source: versionId",
input: testInput{
locals: defaultLocals,
headers: map[string]string{
"X-Amz-Copy-Source": "bucket/object?versionId=invalid_versionId",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetAPIError(s3err.ErrInvalidVersionId),
},
},
{
name: "non empty request body",
input: testInput{
+1 -22
View File
@@ -33,7 +33,6 @@ import (
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/gofiber/fiber/v2"
"github.com/oklog/ulid/v2"
"github.com/valyala/fasthttp"
"github.com/versity/versitygw/debuglogger"
"github.com/versity/versitygw/s3err"
@@ -1015,7 +1014,7 @@ func ValidateCopySource(copysource string) error {
// cut till the versionId as it's the only query param
// that is recognized in copy source
object, versionId, _ := strings.Cut(rest, "?versionId=")
object, _, _ := strings.Cut(rest, "?versionId=")
// objects containing '../', '...../' ... are considered valid in AWS
// but for the security purposes these should be considered as invalid
@@ -1025,12 +1024,6 @@ func ValidateCopySource(copysource string) error {
return s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)
}
// validate the versionId
err = ValidateVersionId(versionId)
if err != nil {
return err
}
return nil
}
@@ -1051,20 +1044,6 @@ func ApplyOverride(original, override *string) *string {
return original
}
// ValidateVersionId check if the input versionId is 'ulid' compatible
func ValidateVersionId(versionId string) error {
if versionId == "" || versionId == "null" {
return nil
}
_, err := ulid.Parse(versionId)
if err != nil {
debuglogger.Logf("invalid versionId: %s", versionId)
return s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
return nil
}
// GenerateObjectLocation generates the object location path-styled or host-styled
// depending on the gateway configuration
func GenerateObjectLocation(ctx *fiber.Ctx, virtualDomain, bucket, object string) string {
-3
View File
@@ -1389,9 +1389,6 @@ func TestValidateCopySource(t *testing.T) {
{"invalid object name 3", "bucket", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
{"invalid object name 4", "bucket/../foo/dir/../../../", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
{"invalid object name 5", "bucket/.?versionId=smth", s3err.GetAPIError(s3err.ErrInvalidCopySourceObject)},
// invalid versionId
{"invalid versionId 1", "bucket/object?versionId=invalid", s3err.GetAPIError(s3err.ErrInvalidVersionId)},
{"invalid versionId 2", "bucket/object?versionId=01BX5ZZKBKACTAV9WEVGEMMV", s3err.GetAPIError(s3err.ErrInvalidVersionId)},
// success
{"no error 1", "bucket/object", nil},
{"no error 2", "bucket/object/key", nil},
+4 -4
View File
@@ -2100,7 +2100,7 @@ func Versioning_PutObjectRetention_invalid_versionId(s *S3Conf) error {
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidVersionId))
})
}, withLock())
}
func Versioning_PutObjectRetention_non_existing_object_version(s *S3Conf) error {
@@ -2149,7 +2149,7 @@ func Versioning_GetObjectRetention_invalid_versionId(s *S3Conf) error {
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidVersionId))
})
}, withLock())
}
func Versioning_GetObjectRetention_non_existing_object_version(s *S3Conf) error {
@@ -2293,7 +2293,7 @@ func Versioning_PutObjectLegalHold_invalid_versionId(s *S3Conf) error {
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidVersionId))
})
}, withLock())
}
func Versioning_PutObjectLegalHold_non_existing_object_version(s *S3Conf) error {
@@ -2340,7 +2340,7 @@ func Versioning_GetObjectLegalHold_invalid_versionId(s *S3Conf) error {
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidVersionId))
})
}, withLock())
}
func Versioning_GetObjectLegalHold_non_existing_object_version(s *S3Conf) error {