diff --git a/backend/posix/posix.go b/backend/posix/posix.go index b160347c..60574f7a 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -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 diff --git a/backend/scoutfs/scoutfs_compat.go b/backend/scoutfs/scoutfs_compat.go index 29708846..d0badfbb 100644 --- a/backend/scoutfs/scoutfs_compat.go +++ b/backend/scoutfs/scoutfs_compat.go @@ -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) diff --git a/backend/walk.go b/backend/walk.go index 6fc4af0f..22f487d8 100644 --- a/backend/walk.go +++ b/backend/walk.go @@ -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 diff --git a/backend/walk_test.go b/backend/walk_test.go index 42a00c2c..27cb394f 100644 --- a/backend/walk_test.go +++ b/backend/walk_test.go @@ -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": {}, diff --git a/s3err/s3err.go b/s3err/s3err.go index 8fe40423..bc5fd1f1 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -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: { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index c7645348..929ffaac 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -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, diff --git a/tests/integration/posix.go b/tests/integration/posix.go index f3eb9e97..1088b46c 100644 --- a/tests/integration/posix.go +++ b/tests/integration/posix.go @@ -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///. 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) + }) +}