Merge pull request #2523 from versity/sis/posix-reserved-sgwtmp-keys

fix: reject posix object keys that resolve into the `.sgwtmp` staging directory
This commit is contained in:
Ben McClelland
2026-10-16 14:38:19 -07:00
committed by GitHub
7 changed files with 434 additions and 7 deletions
+87 -4
View File
@@ -616,6 +616,32 @@ func (p *Posix) validateVersionId(versionId string) error {
return nil
}
// ValidateObjectKey returns ReservedObjectKey if key can't name an object of
// bucket. Backends built on Posix that add their own object entry points must
// check keys with it before resolving them to paths.
func (p *Posix) ValidateObjectKey(bucket, key string) error {
return p.validateObjectName(bucket, p.encodeKey(key))
}
// validateObjectName returns ReservedObjectKey if object, an object path
// relative to the bucket directory, resolves to MetaTmpDir or a path in it.
// The gateway stages multipart uploads and temporary files there, so a key
// resolving there would read and replace the files of other clients' uploads.
// The path is resolved like ObjectPath resolves it, so that "./", "x/../" and
// repeated slashes don't hide the directory, and its name is matched ignoring
// case, like case-insensitive file systems match it.
func (p *Posix) validateObjectName(bucket, object string) error {
rel, err := filepath.Rel(p.BucketPath(bucket), p.ObjectPath(bucket, object))
if err != nil {
return fmt.Errorf("resolve object path: %w", err)
}
dir, _, _ := strings.Cut(filepath.ToSlash(rel), "/")
if strings.EqualFold(dir, MetaTmpDir) {
return s3err.GetAPIError(s3err.ErrReservedObjectKey)
}
return nil
}
// DoesBucketExist returns NoSuchBucket unless bucket names a bucket. Backends
// built on Posix that add their own entry points must check buckets with it
// rather than stat the bucket path themselves.
@@ -649,14 +675,18 @@ func (p *Posix) doesBucketExist(bucket string) error {
return nil
}
// doesBucketAndObjectExist checks that bucket exists and, when versionId is
// empty, that object has a current version. A specific version is looked up
// once versionId is validated.
// doesBucketAndObjectExist checks that bucket exists, that object isn't
// reserved and, when versionId is empty, that object has a current version.
// A specific version is looked up once versionId is validated.
func (p *Posix) doesBucketAndObjectExist(bucket, object, versionId string) error {
err := p.doesBucketExist(bucket)
if err != nil {
return err
}
err = p.validateObjectName(bucket, object)
if err != nil {
return err
}
if versionId != "" {
return nil
}
@@ -2056,6 +2086,11 @@ func (p *Posix) CreateMultipartUpload(ctx context.Context, mpu s3response.Create
return s3response.InitiateMultipartUploadResult{}, err
}
err = p.ValidateObjectKey(bucket, object)
if err != nil {
return s3response.InitiateMultipartUploadResult{}, err
}
if strings.HasSuffix(*mpu.Key, "/") {
// directory objects can't be uploaded with multipart uploads
// because posix directories can't contain data
@@ -2364,6 +2399,11 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C
return res, "", err
}
err = p.validateObjectName(bucket, object)
if err != nil {
return res, "", err
}
if err := validateUploadID(uploadID); err != nil {
return res, "", err
}
@@ -4170,6 +4210,11 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput)
return s3response.CopyPartResult{}, err
}
err = p.validateObjectName(srcBucket, srcObject)
if err != nil {
return s3response.CopyPartResult{}, err
}
if upi.ExpectedSourceBucketOwner != nil && *upi.ExpectedSourceBucketOwner != "" {
aclData, err := p.meta.RetrieveAttribute(nil, srcBucket, "", aclkey)
if err != nil {
@@ -4602,12 +4647,17 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje
return s3response.PutObjectOutput{}, err
}
object := p.encodeKey(*po.Key)
err = p.validateObjectName(*po.Bucket, object)
if err != nil {
return s3response.PutObjectOutput{}, err
}
tags, err := backend.ParseObjectTags(getString(po.Tagging))
if err != nil {
return s3response.PutObjectOutput{}, err
}
object := p.encodeKey(*po.Key)
name := p.ObjectPath(*po.Bucket, object)
// Fast-fail precondition check before the request body is staged. This
@@ -5205,6 +5255,11 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return nil, err
}
err = p.validateObjectName(bucket, object)
if err != nil {
return nil, err
}
objpath := p.ObjectPath(bucket, object)
vStatus, err := p.getBucketVersioningStatus(ctx, bucket)
@@ -5890,6 +5945,11 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge
}
object := p.encodeKey(*input.Key)
err = p.validateObjectName(bucket, object)
if err != nil {
return nil, err
}
if versionId != "" {
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) {
@@ -6239,6 +6299,11 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
return nil, err
}
err = p.validateObjectName(bucket, object)
if err != nil {
return nil, err
}
if versionId != "" {
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) {
@@ -6567,6 +6632,11 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput
return s3response.CopyObjectOutput{}, err
}
err = p.validateObjectName(dstBucket, dstObject)
if err != nil {
return s3response.CopyObjectOutput{}, err
}
// The destination's conditional write headers are evaluated before the
// source is looked up. Like PutObject's own fast-fail check, this one is
// only advisory: the authoritative check is repeated under the object
@@ -6581,6 +6651,11 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput
return s3response.CopyObjectOutput{}, err
}
err = p.ValidateObjectKey(srcBucket, srcObject)
if err != nil {
return s3response.CopyObjectOutput{}, err
}
if input.ExpectedSourceBucketOwner != nil && *input.ExpectedSourceBucketOwner != "" {
aclData, err := p.meta.RetrieveAttribute(nil, srcBucket, "", aclkey)
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -7378,6 +7453,10 @@ func (p *Posix) GetObjectTagging(ctx context.Context, bucket, object, versionId
}
object = p.encodeKey(object)
err = p.validateObjectName(bucket, object)
if err != nil {
return nil, err
}
bucket, object, err = p.objVersionAttrPath(bucket, object, versionId)
if err != nil {
return nil, err
@@ -7446,6 +7525,10 @@ func (p *Posix) PutObjectTagging(ctx context.Context, bucket, object, versionId
}
object = p.encodeKey(object)
err = p.validateObjectName(bucket, object)
if err != nil {
return err
}
bucket, object, err = p.objVersionAttrPath(bucket, object, versionId)
if err != nil {
return err
+10
View File
@@ -362,6 +362,11 @@ func (s *ScoutFS) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.
return nil, err
}
err = s.ValidateObjectKey(bucket, object)
if err != nil {
return nil, err
}
objPath := s.ObjectPath(bucket, object)
fi, err := os.Stat(objPath)
@@ -456,6 +461,11 @@ func (s *ScoutFS) RestoreObject(_ context.Context, input *s3.RestoreObjectInput)
return err
}
err = s.ValidateObjectKey(bucket, object)
if err != nil {
return err
}
err = setStaging(s.ObjectPath(bucket, object))
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
+18 -3
View File
@@ -19,7 +19,6 @@ import (
"errors"
"fmt"
"io/fs"
"path"
"slices"
"strings"
"syscall"
@@ -563,6 +562,21 @@ func shouldSkip(name string, walkstate *walkState) bool {
return slices.Contains(walkstate.skipdirs, name)
}
// inSkipDir reports whether dir, a directory named by the listing prefix, is
// a skipped directory or inside one. The prefix spells the directory names
// as the client sent them, which a case-insensitive file system resolves to
// a skipped directory in any case, so they are matched ignoring case.
func inSkipDir(dir string, walkstate *walkState) bool {
for name := range strings.SplitSeq(dir, pathSeparator) {
if slices.ContainsFunc(walkstate.skipdirs, func(skip string) bool {
return strings.EqualFold(name, skip)
}) {
return true
}
}
return false
}
func addContentEntry(objectName string, dirEntry fs.DirEntry, walkstate *walkState) bool {
obj, err := walkstate.getObj(objectName, dirEntry)
if err == ErrSkipObj {
@@ -622,8 +636,9 @@ func quickWalk(walkstate *walkState) error {
}
if useRootEntries {
// keep fast-path semantics but still honor skipdirs for the prefixed root
if !shouldSkip(path.Base(rootDir), walkstate) {
// keep fast-path semantics but still honor skipdirs for the prefixed
// root and every directory above it
if !inSkipDir(rootDir, walkstate) {
// Mirror processDir: offer the root directory itself to getObj whenever
// its key sorts after the marker, using checkAndMaybeAddCommonPrefix to
// handle the delimiter case. This replaces the previous guards that
+51
View File
@@ -589,6 +589,57 @@ func TestWalkStop(t *testing.T) {
}
}
// TestWalkPrefixInSkipDir checks that a prefix naming a skipped directory, or
// a directory inside one, lists nothing, also when it spells the skipped
// directory in another case on a case-insensitive file system.
func TestWalkPrefixInSkipDir(t *testing.T) {
fsys := caseFoldFS{fstest.MapFS{
".sgwtmp/multipart/hash/upload/1": {},
"obj": {},
}}
for _, prefix := range []string{
".sgwtmp/",
".sgwtmp/multipart/",
".sgwtmp/multipart/hash/",
".sgwtmp/multipart/hash/upload/",
".sgwtmp/multipart/hash/upload/1",
".SGWTMP/multipart/hash/upload/",
".SgwTmp/multipart/",
} {
for _, delimiter := range []string{"", "/"} {
res, err := backend.Walk(context.Background(), fsys, prefix, delimiter, "", 1000,
getObj, []string{".sgwtmp"})
if err != nil {
t.Errorf("prefix %q delimiter %q: walk: %v", prefix, delimiter, err)
continue
}
if len(res.Objects) != 0 || len(res.CommonPrefixes) != 0 {
t.Errorf("prefix %q delimiter %q: listed objects %v and common prefixes %v",
prefix, delimiter, printObjects(res.Objects), printCommonPrefixes(res.CommonPrefixes))
}
}
}
}
// caseFoldFS opens the files of a MapFS with lowercase names by names in any
// case, like a case-insensitive file system.
type caseFoldFS struct {
fstest.MapFS
}
func (c caseFoldFS) Open(name string) (fs.File, error) {
return c.MapFS.Open(strings.ToLower(name))
}
func (c caseFoldFS) Stat(name string) (fs.FileInfo, error) {
return c.MapFS.Stat(strings.ToLower(name))
}
func (c caseFoldFS) ReadDir(name string) ([]fs.DirEntry, error) {
return c.MapFS.ReadDir(strings.ToLower(name))
}
func TestWalkDirectoryErrorPropagates(t *testing.T) {
fSys := fstest.MapFS{
"dir/file.txt": {},
+6
View File
@@ -211,6 +211,7 @@ const (
ErrVersioningNotConfigured
ErrACLsDisabled
ErrNoSpaceLeftOnDevice
ErrReservedObjectKey
// Admin api errors
ErrAdminAccessDenied
@@ -810,6 +811,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
Description: "No space left on device.",
HTTPStatusCode: http.StatusInsufficientStorage,
},
ErrReservedObjectKey: {
Code: "ReservedObjectKey",
Description: "Object keys in the .sgwtmp directory are reserved for the gateway's internal use.",
HTTPStatusCode: http.StatusBadRequest,
},
// Admin api errors
ErrAdminAccessDenied: {
+18
View File
@@ -1093,6 +1093,12 @@ func TestPosix(ts *TestState) {
ts.Run(ObjectTagging_trailing_slash_counterpart)
ts.Run(ObjectLock_trailing_slash_counterpart)
ts.Run(DeleteObject_directory_not_empty)
ts.Run(PutObject_reserved_key)
ts.Run(CreateMultipartUpload_reserved_key)
ts.Run(GetObject_reserved_key)
ts.Run(CopyObject_reserved_key)
ts.Run(DeleteObject_reserved_key)
ts.Run(ListObjectsV2_reserved_prefix)
if !ts.conf.windowsTests {
ts.Run(PutObject_race_with_delete)
}
@@ -1203,6 +1209,12 @@ func TestScoutfs(ts *TestState) {
ts.Run(ObjectTagging_trailing_slash_counterpart)
ts.Run(ObjectLock_trailing_slash_counterpart)
ts.Run(DeleteObject_directory_not_empty)
ts.Run(PutObject_reserved_key)
ts.Run(CreateMultipartUpload_reserved_key)
ts.Run(GetObject_reserved_key)
ts.Run(CopyObject_reserved_key)
ts.Run(DeleteObject_reserved_key)
ts.Run(ListObjectsV2_reserved_prefix)
TestSSECTransport(ts)
}
@@ -3661,6 +3673,12 @@ func GetIntTests() IntTests {
"PutObject_with_slashes": PutObject_with_slashes,
"PutObject_windows_reserved_names": PutObject_windows_reserved_names,
"PutObject_race_with_delete": PutObject_race_with_delete,
"PutObject_reserved_key": PutObject_reserved_key,
"CreateMultipartUpload_reserved_key": CreateMultipartUpload_reserved_key,
"GetObject_reserved_key": GetObject_reserved_key,
"CopyObject_reserved_key": CopyObject_reserved_key,
"DeleteObject_reserved_key": DeleteObject_reserved_key,
"ListObjectsV2_reserved_prefix": ListObjectsV2_reserved_prefix,
"CreateMultipartUpload_dir_obj": CreateMultipartUpload_dir_obj,
"PutObject_sse_c_not_implemented": PutObject_sse_c_not_implemented,
"PutObject_unsupported_sse_not_implemented": PutObject_unsupported_sse_not_implemented,
+244
View File
@@ -16,6 +16,7 @@ package integration
import (
"context"
"crypto/sha256"
"fmt"
"io"
"slices"
@@ -647,3 +648,246 @@ func PutObject_race_with_delete(s *S3Conf) error {
return eg.Wait()
})
}
// PutObject_reserved_key checks that no key reaches the .sgwtmp directory of
// a bucket, where its multipart uploads are staged: the parts of an upload at
// .sgwtmp/multipart/<sha256 of the key>/<upload id>/. A client can't replace
// the parts of another client's upload, however the key spells their path.
func PutObject_reserved_key(s *S3Conf) error {
testName := "PutObject_reserved_key"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
mp, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
parts, _, err := uploadParts(s3client, 100, 1, bucket, obj, *mp.UploadId)
if err != nil {
return err
}
partKey := fmt.Sprintf(".sgwtmp/multipart/%x/%v/1", sha256.Sum256([]byte(obj)), *mp.UploadId)
for _, key := range []string{
partKey,
"./" + partKey,
"my-dir/../" + partKey,
".SGWTMP" + strings.TrimPrefix(partKey, ".sgwtmp"),
".sgwtmp/",
".sgwtmp/my-obj",
} {
_, err := putObjectWithData(objDataLen(key, 10), &s3.PutObjectInput{
Bucket: &bucket,
Key: &key,
}, s3client)
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrReservedObjectKey)); err != nil {
return fmt.Errorf("%v: %w", key, err)
}
}
// the staged part is the one the client uploaded
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{
Bucket: &bucket,
Key: &obj,
UploadId: mp.UploadId,
MultipartUpload: &types.CompletedMultipartUpload{
Parts: []types.CompletedPart{
{ETag: parts[0].ETag, PartNumber: parts[0].PartNumber},
},
},
})
cancel()
return err
})
}
func CreateMultipartUpload_reserved_key(s *S3Conf) error {
testName := "CreateMultipartUpload_reserved_key"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
_, err := createMp(s3client, bucket, ".sgwtmp/my-obj")
return checkApiErr(err, s3err.GetAPIError(s3err.ErrReservedObjectKey))
})
}
func GetObject_reserved_key(s *S3Conf) error {
testName := "GetObject_reserved_key"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
mp, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
_, _, err = uploadParts(s3client, 100, 1, bucket, obj, *mp.UploadId)
if err != nil {
return err
}
partKey := fmt.Sprintf(".sgwtmp/multipart/%x/%v/1", sha256.Sum256([]byte(obj)), *mp.UploadId)
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.GetObject(ctx, &s3.GetObjectInput{
Bucket: &bucket,
Key: &partKey,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrReservedObjectKey)); err != nil {
return err
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &partKey,
})
cancel()
if err == nil {
return fmt.Errorf("expected HeadObject to fail")
}
if err := checkSdkApiErr(err, "BadRequest"); err != nil {
return err
}
return checkAndAbortUpload(s3client, bucket, obj, *mp.UploadId)
})
}
func CopyObject_reserved_key(s *S3Conf) error {
testName := "CopyObject_reserved_key"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
mp, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
_, _, err = uploadParts(s3client, 100, 1, bucket, obj, *mp.UploadId)
if err != nil {
return err
}
_, err = putObjects(s3client, []string{"src-obj"}, bucket)
if err != nil {
return err
}
partKey := fmt.Sprintf(".sgwtmp/multipart/%x/%v/1", sha256.Sum256([]byte(obj)), *mp.UploadId)
for _, keys := range [][2]string{
{partKey, "dst-obj"},
{"src-obj", partKey},
} {
src, dst := keys[0], keys[1]
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: &bucket,
Key: &dst,
CopySource: getPtr(bucket + "/" + src),
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrReservedObjectKey)); err != nil {
return fmt.Errorf("%v to %v: %w", src, dst, err)
}
}
return checkAndAbortUpload(s3client, bucket, obj, *mp.UploadId)
})
}
func DeleteObject_reserved_key(s *S3Conf) error {
testName := "DeleteObject_reserved_key"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
mp, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
parts, _, err := uploadParts(s3client, 100, 1, bucket, obj, *mp.UploadId)
if err != nil {
return err
}
partKey := fmt.Sprintf(".sgwtmp/multipart/%x/%v/1", sha256.Sum256([]byte(obj)), *mp.UploadId)
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &partKey,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrReservedObjectKey)); err != nil {
return err
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
Bucket: &bucket,
Delete: &types.Delete{
Objects: []types.ObjectIdentifier{{Key: &partKey}},
},
})
cancel()
if err != nil {
return err
}
expectedErr := s3err.GetAPIError(s3err.ErrReservedObjectKey)
if len(out.Deleted) != 0 || len(out.Errors) != 1 ||
getString(out.Errors[0].Key) != partKey ||
getString(out.Errors[0].Code) != expectedErr.Code ||
getString(out.Errors[0].Message) != expectedErr.Description {
return fmt.Errorf("expected the deletion of %v to fail with %v, instead got deleted %v and errors %v",
partKey, expectedErr.Code, len(out.Deleted), out.Errors)
}
// the staged part is still there
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{
Bucket: &bucket,
Key: &obj,
UploadId: mp.UploadId,
MultipartUpload: &types.CompletedMultipartUpload{
Parts: []types.CompletedPart{
{ETag: parts[0].ETag, PartNumber: parts[0].PartNumber},
},
},
})
cancel()
return err
})
}
func ListObjectsV2_reserved_prefix(s *S3Conf) error {
testName := "ListObjectsV2_reserved_prefix"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
mp, err := createMp(s3client, bucket, obj)
if err != nil {
return err
}
_, _, err = uploadParts(s3client, 100, 1, bucket, obj, *mp.UploadId)
if err != nil {
return err
}
uploadDir := fmt.Sprintf(".sgwtmp/multipart/%x/%v/", sha256.Sum256([]byte(obj)), *mp.UploadId)
for _, prefix := range []string{
".sgwtmp/",
".sgwtmp/multipart/",
uploadDir,
".SGWTMP" + strings.TrimPrefix(uploadDir, ".sgwtmp"),
} {
for _, delimiter := range []string{"", "/"} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
res, err := s3client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{
Bucket: &bucket,
Prefix: &prefix,
Delimiter: &delimiter,
})
cancel()
if err != nil {
return err
}
if len(res.Contents) != 0 || len(res.CommonPrefixes) != 0 {
return fmt.Errorf("prefix %q delimiter %q: expected nothing to be listed, instead got objects %v and common prefixes %v",
prefix, delimiter, objStrings(res.Contents), res.CommonPrefixes)
}
}
}
return checkAndAbortUpload(s3client, bucket, obj, *mp.UploadId)
})
}