diff --git a/weed/command/filer.go b/weed/command/filer.go index 90eb04bee9..d1385f918a 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -51,6 +51,11 @@ var ( // S3 gateway, whose remote-mount read paths dial the mounted endpoint directly. const allowUntrustedRemoteEndpointsUsage = "if true, a read of a remote-only entry accepts arbitrary remote S3 endpoints including loopback / link-local hosts. Default rejects internal / metadata endpoints." +// allowBackslashInKeyUsage documents the S3 flag keeping '\' a literal +// object-key character instead of normalizing it to '/'. Repeated '//' in a +// key still collapses: a filer entry name cannot contain an empty segment. +const allowBackslashInKeyUsage = "treat backslash as a regular object-key character instead of converting it to '/' (aws-compatible; '//' still collapses)" + type FilerOptions struct { masters *pb.ServerDiscovery mastersString *string @@ -173,6 +178,7 @@ func init() { filerS3Options.cacheSizeMB = cmdFiler.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") filerS3Options.readerCacheSizeMB = cmdFiler.Flag.Int64("s3.readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") filerS3Options.allowUntrustedRemoteEndpoints = cmdFiler.Flag.Bool("s3.allowUntrustedRemoteEndpoints", false, allowUntrustedRemoteEndpointsUsage) + filerS3Options.allowBackslashInKey = cmdFiler.Flag.Bool("s3.allowBackslashInKey", false, allowBackslashInKeyUsage) // start webdav on filer filerStartWebDav = cmdFiler.Flag.Bool("webdav", false, "whether to start webdav gateway") diff --git a/weed/command/mini.go b/weed/command/mini.go index 1ded2f936c..9d4eb630cb 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -556,6 +556,7 @@ func initMiniS3Flags() { miniS3Options.cacheSizeMB = cmdMini.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") miniS3Options.readerCacheSizeMB = cmdMini.Flag.Int64("s3.readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") miniS3Options.allowUntrustedRemoteEndpoints = cmdMini.Flag.Bool("s3.allowUntrustedRemoteEndpoints", false, allowUntrustedRemoteEndpointsUsage) + miniS3Options.allowBackslashInKey = cmdMini.Flag.Bool("s3.allowBackslashInKey", false, allowBackslashInKeyUsage) // In mini mode, S3 uses the shared debug server started at line 681, not its own separate debug server miniS3Options.debug = new(bool) // explicitly false miniS3Options.debugPort = cmdMini.Flag.Int("s3.debug.port", 6060, "http port for debugging (unused in mini mode)") diff --git a/weed/command/s3.go b/weed/command/s3.go index 036e3813eb..98b0bbcc3c 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -84,6 +84,7 @@ type S3Options struct { readerCacheSizeMB *int64 allowUntrustedRemoteEndpoints *bool + allowBackslashInKey *bool // shutdownCtx, when non-nil, tells startS3Server to gracefully shut down // its HTTP/gRPC servers once the ctx is cancelled, in addition to on // interrupt. Used by weed mini to orchestrate an ordered shutdown. @@ -140,6 +141,7 @@ func init() { s3StandaloneOptions.cacheSizeMB = cmdS3.Flag.Int64("cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") s3StandaloneOptions.readerCacheSizeMB = cmdS3.Flag.Int64("readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") s3StandaloneOptions.allowUntrustedRemoteEndpoints = cmdS3.Flag.Bool("allowUntrustedRemoteEndpoints", false, allowUntrustedRemoteEndpointsUsage) + s3StandaloneOptions.allowBackslashInKey = cmdS3.Flag.Bool("allowBackslashInKey", false, allowBackslashInKeyUsage) } var cmdS3 = &Command{ @@ -399,6 +401,7 @@ func (s3opt *S3Options) startS3Server() bool { MaxMB: filerMaxMB, AllowUntrustedRemoteEndpoints: *s3opt.allowUntrustedRemoteEndpoints, + AllowBackslashInKey: *s3opt.allowBackslashInKey, }) if s3ApiServer_err != nil { glog.Fatalf("S3 API Server startup error: %v", s3ApiServer_err) diff --git a/weed/command/server.go b/weed/command/server.go index 0e4ac169a3..3fd367fd7f 100644 --- a/weed/command/server.go +++ b/weed/command/server.go @@ -195,6 +195,7 @@ func init() { s3Options.iamReadOnly = cmdServer.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") s3Options.cipher = cmdServer.Flag.Bool("s3.encryptVolumeData", false, "encrypt data on volume servers for S3 uploads") s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Advertised to Iceberg and Lance clients, and tried first when verifying S3 signatures behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3Options.allowBackslashInKey = cmdServer.Flag.Bool("s3.allowBackslashInKey", false, allowBackslashInKeyUsage) s3Options.defaultFileMode = cmdServer.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") s3Options.cacheSizeMB = cmdServer.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") s3Options.readerCacheSizeMB = cmdServer.Flag.Int64("s3.readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") diff --git a/weed/s3api/s3_constants/header.go b/weed/s3api/s3_constants/header.go index 28a2d1baf0..6fcedfdedc 100644 --- a/weed/s3api/s3_constants/header.go +++ b/weed/s3api/s3_constants/header.go @@ -216,10 +216,21 @@ func GetBucketAndObject(r *http.Request) (bucket, object string) { return } -// IsValidObjectKey rejects S3 object keys that — after normalization -// (backslash→slash, slash collapse) — contain a `.` or `..` path segment, or -// embed a NUL byte. Such keys are collapsed by filepath.Join inside the filer -// and would escape the bucket directory, so they must not reach the gRPC layer. +// allowBackslashInObjectKey keeps '\' a literal object-key character instead +// of normalizing it to '/'. It is a process-wide mode set once at S3 server +// startup; the filer only treats '/' as a path separator, so a verbatim '\' +// stays inside one filer entry name. +var allowBackslashInObjectKey atomic.Bool + +func SetAllowBackslashInObjectKey(allow bool) { + allowBackslashInObjectKey.Store(allow) +} + +// IsValidObjectKey rejects S3 object keys that — after the mode-dependent +// normalization NormalizeObjectKey applies — contain a `.` or `..` path +// segment, or embed a NUL byte. Such keys are collapsed by filepath.Join +// inside the filer and would escape the bucket directory, so they must not +// reach the gRPC layer. // Gorilla mux URL-decodes captured vars before this runs, so `%2e%2e` is // already `..` here. func IsValidObjectKey(object string) bool { @@ -229,7 +240,9 @@ func IsValidObjectKey(object string) bool { if strings.ContainsRune(object, '\x00') { return false } - object = strings.ReplaceAll(object, "\\", "/") + if !allowBackslashInObjectKey.Load() { + object = strings.ReplaceAll(object, "\\", "/") + } for _, seg := range strings.Split(object, "/") { if seg == "." || seg == ".." { return false @@ -263,7 +276,8 @@ func IsValidPathSegment(value string) bool { // NormalizeObjectKey normalizes object keys by removing duplicate slashes and converting backslashes. // This normalizes keys from various sources (URL path, form values, etc.) to a consistent format. -// It also converts Windows-style backslashes to forward slashes for cross-platform compatibility. +// It also converts Windows-style backslashes to forward slashes for cross-platform compatibility, +// unless allowBackslashInObjectKey is set. // Returns keys WITHOUT leading slash to match S3 API format (e.g., "foo/bar" not "/foo/bar"). // Preserves trailing slash if present (e.g., "foo/" stays "foo/"). func NormalizeObjectKey(object string) string { @@ -271,7 +285,9 @@ func NormalizeObjectKey(object string) string { hasTrailingSlash := strings.HasSuffix(object, "/") // Convert Windows-style backslashes to forward slashes - object = strings.ReplaceAll(object, "\\", "/") + if !allowBackslashInObjectKey.Load() { + object = strings.ReplaceAll(object, "\\", "/") + } object = removeDuplicateSlashes(object) // Remove leading slash to match S3 API format object = strings.TrimPrefix(object, "/") diff --git a/weed/s3api/s3_constants/header_test.go b/weed/s3api/s3_constants/header_test.go index 7e45882710..de780d0d51 100644 --- a/weed/s3api/s3_constants/header_test.go +++ b/weed/s3api/s3_constants/header_test.go @@ -123,6 +123,54 @@ func TestIsValidObjectKey(t *testing.T) { } } +func TestNormalizeObjectKeyAllowBackslash(t *testing.T) { + SetAllowBackslashInObjectKey(true) + defer SetAllowBackslashInObjectKey(false) + + normalizeTests := []struct { + name string + input string + expected string + }{ + {"backslash kept verbatim", "folder\\file.txt", "folder\\file.txt"}, + {"nested backslash kept", "folder\\sub\\file.txt", "folder\\sub\\file.txt"}, + {"trailing backslash kept", "folder\\", "folder\\"}, + {"lone backslash kept", "\\", "\\"}, + {"double backslash kept", "a\\\\b", "a\\\\b"}, + {"mixed separators", "folder\\sub/another\\file.txt", "folder\\sub/another\\file.txt"}, + {"duplicate slashes still collapse", "folder//sub///file.txt", "folder/sub/file.txt"}, + {"leading slash still trimmed", "/folder\\file.txt", "folder\\file.txt"}, + {"trailing slash preserved", "folder/", "folder/"}, + {"windows path style", "C:\\Users\\test\\file.txt", "C:\\Users\\test\\file.txt"}, + } + for _, tt := range normalizeTests { + t.Run(tt.name, func(t *testing.T) { + if got := NormalizeObjectKey(tt.input); got != tt.expected { + t.Errorf("NormalizeObjectKey(%q) = %q, want %q", tt.input, got, tt.expected) + } + }) + } + + validTests := []struct { + name string + input string + want bool + }{ + {"backslash dotdot is one segment", "..\\evil\\test.txt", true}, + {"backslash dotdot inside", "good\\..\\evil/test.txt", true}, + {"nested dotdot still rejected", "good/../evil/test.txt", false}, + {"leading dotdot still rejected", "../evil/test.txt", false}, + {"plain backslash key", "folder\\file.txt", true}, + } + for _, tt := range validTests { + t.Run(tt.name, func(t *testing.T) { + if got := IsValidObjectKey(tt.input); got != tt.want { + t.Errorf("IsValidObjectKey(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + func TestIsValidBucketName(t *testing.T) { tests := []struct { name string diff --git a/weed/s3api/s3api_object_anonymous_cache_test.go b/weed/s3api/s3api_object_anonymous_cache_test.go index d5a0f2f593..a349585727 100644 --- a/weed/s3api/s3api_object_anonymous_cache_test.go +++ b/weed/s3api/s3api_object_anonymous_cache_test.go @@ -238,10 +238,12 @@ func TestAnonymousObjectACLVersionedDirectoryIdentity(t *testing.T) { f.entries["/buckets/b/folder/.versions"] = &filer_pb.Entry{IsDirectory: true, Extended: map[string][]byte{s3_constants.ExtLatestVersionIdKey: []byte("")}} } // Equivalent filer-path spellings must not recover a different object's data. - assertMissing(serveAnonymousRead(s3a, method, "folder/", route.query, headers)) + for _, object := range []string{"folder/", "folder//", "folder\\"} { + assertMissing(serveAnonymousRead(s3a, method, object, route.query, headers)) + } // Spellings the policy's folder/ grant does not cover are // masked: a miss must be indistinguishable from a denied read. - for _, object := range []string{"folder//", "folder\\"} { + for _, object := range []string{"folder//x", "folder\\x"} { rr := serveAnonymousRead(s3a, method, object, route.query, headers) require.Equal(t, http.StatusForbidden, rr.Code, rr.Body.String()) } @@ -281,3 +283,40 @@ func TestAnonymousObjectACLVersionedDirectoryIdentity(t *testing.T) { } } } + +// A literal-backslash key is a different object than its slash spelling only +// when allowBackslashInObjectKey is set. A public grant on b/folder/ must not +// authorize an anonymous read of the separate folder\ object under that mode. +func TestAnonymousObjectACLBackslashKeyIdentity(t *testing.T) { + for _, allowBackslash := range []bool{false, true} { + t.Run(fmt.Sprintf("allowBackslashInKey=%v", allowBackslash), func(t *testing.T) { + s3a, f := newAnonymousReadTestServer(t) + s3_constants.SetAllowBackslashInObjectKey(allowBackslash) + t.Cleanup(func() { s3_constants.SetAllowBackslashInObjectKey(false) }) + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("b", `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3:GetObject*","Resource":"arn:aws:s3:::b/folder/"}]}`)) + slash := anonymousReadEntry([]byte(`[]`)) + slash.Name = "folder" + slash.MarkPrefixObject() + f.entries["/buckets/b/folder"] = slash + backslash := anonymousReadEntry([]byte(`[]`)) + backslash.Name = `folder\` + f.entries[`/buckets/b/folder\`] = backslash + + rr := serveAnonymousRead(s3a, http.MethodGet, "folder/", "", nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, "hello world", rr.Body.String()) + + rr = serveAnonymousRead(s3a, http.MethodGet, `folder\`, "", nil) + if !allowBackslash { + // Default mode folds the spelling onto the slash key the policy covers. + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + } else { + require.Equal(t, http.StatusForbidden, rr.Code, rr.Body.String()) + backslash.Extended[s3_constants.ExtAmzAclKey] = anonymousReadGrant("READ") + rr = serveAnonymousRead(s3a, http.MethodGet, `folder\`, "", nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, "hello world", rr.Body.String()) + } + }) + } +} diff --git a/weed/s3api/s3api_object_anonymous_read.go b/weed/s3api/s3api_object_anonymous_read.go index e5d1c21c28..6fd86001aa 100644 --- a/weed/s3api/s3api_object_anonymous_read.go +++ b/weed/s3api/s3api_object_anonymous_read.go @@ -118,7 +118,7 @@ func (s3a *S3ApiServer) authorizeAnonymousObjectRead(r *http.Request, bucket, ob // public-read bucket. Anything else masks the miss as AccessDenied. func (s3a *S3ApiServer) authorizeAnonymousBucketList(r *http.Request, bucket string) s3err.ErrorCode { identity, _ := s3_constants.GetIdentityFromContext(r).(*Identity) - object := mux.Vars(r)["object"] + object := s3_constants.NormalizeObjectKey(mux.Vars(r)["object"]) // The caller's query parameters would otherwise retarget these checks // (?policy resolves to GetBucketPolicy) or satisfy a prefix condition on an // unrelated subtree, so each runs on a request with a fixed shape: a diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index fbbf2b2fd5..2473ed77cc 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -900,13 +900,13 @@ func pathToBucketObjectAndVersion(rawPath, decodedPath string) (bucket, object, } bucket, object = pathToBucketAndObject(pathForBucket) - return bucket, object, versionId + return bucket, s3_constants.NormalizeObjectKey(object), versionId } } } bucket, object = pathToBucketAndObject(pathForBucket) - return bucket, object, versionId + return bucket, s3_constants.NormalizeObjectKey(object), versionId } type CopyPartResult struct { diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index 3817a28adc..caa0069624 100644 --- a/weed/s3api/s3api_object_handlers_delete.go +++ b/weed/s3api/s3api_object_handlers_delete.go @@ -479,29 +479,44 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h deleteErrors = append(deleteErrors, deleteErrorFromCode(validationCode, object.Key, object.VersionId)) continue } - if _, err := s3a.validateTableBucketObjectPathOrVerify(bucket, object.Key); err != nil { + key := s3_constants.NormalizeObjectKey(object.Key) + if key == "" { + // A key of only slashes can never name a stored entry; like a + // missing key the delete is an idempotent no-op, and a + // conditional delete has no target to match. + if object.ETag != "" { + deleteErrors = append(deleteErrors, deleteErrorFromCode(s3err.ErrNoSuchKey, object.Key, object.VersionId)) + continue + } + deletedCount++ + if !deleteObjects.Quiet { + deletedObjects = append(deletedObjects, ObjectIdentifier{Key: object.Key, VersionId: object.VersionId}) + } + continue + } + if _, err := s3a.validateTableBucketObjectPathOrVerify(bucket, key); err != nil { deleteErrors = append(deleteErrors, deleteErrorFromCode(s3err.ErrAccessDenied, object.Key, object.VersionId)) continue } - if authErr := s3a.iam.AuthorizeObjectDelete(r, identity, bucket, object.Key, object.VersionId); authErr != s3err.ErrNone { + if authErr := s3a.iam.AuthorizeObjectDelete(r, identity, bucket, key, object.VersionId); authErr != s3err.ErrNone { // Defer to the recheck, mirroring the copy-source fallback: a // tag-conditioned Allow may still grant what phase one denied. - if authErr != s3err.ErrAccessDenied || s3a.recheckPolicyForKeyWithLookup(r, http.MethodDelete, bucket, object.Key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler") != s3err.ErrNone { + if authErr != s3err.ErrAccessDenied || s3a.recheckPolicyForKeyWithLookup(r, http.MethodDelete, bucket, key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler") != s3err.ErrNone { deleteErrors = append(deleteErrors, deleteErrorFromCode(authErr, object.Key, object.VersionId)) continue } } - if errCode := s3a.recheckTagConditionedPolicyWithLookup(objectKeyPolicyRequest(r, http.MethodDelete, bucket, object.Key, object.VersionId), bucket, object.Key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler"); errCode != s3err.ErrNone { + if errCode := s3a.recheckTagConditionedPolicyWithLookup(objectKeyPolicyRequest(r, http.MethodDelete, bucket, key, object.VersionId), bucket, key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler"); errCode != s3err.ErrNone { deleteErrors = append(deleteErrors, deleteErrorFromCode(errCode, object.Key, object.VersionId)) continue } var deleteResult deleteMutationResult - deleteCode := s3a.withObjectWriteLock(bucket, object.Key, func() s3err.ErrorCode { - return s3a.checkDeleteIfMatch(bucket, object.Key, object.VersionId, versioningState, object.ETag, s3err.ErrNoSuchKey) + deleteCode := s3a.withObjectWriteLock(bucket, key, func() s3err.ErrorCode { + return s3a.checkDeleteIfMatch(bucket, key, object.VersionId, versioningState, object.ETag, s3err.ErrNoSuchKey) }, func() s3err.ErrorCode { if versioningConfigured { - result, errCode := s3a.deleteVersionedObject(r, bucket, object.Key, object.VersionId, versioningState) + result, errCode := s3a.deleteVersionedObject(r, bucket, key, object.VersionId, versioningState) if errCode != s3err.ErrNone { return errCode } @@ -509,18 +524,18 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h return s3err.ErrNone } - governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key) - if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, "", governanceBypassAllowed); err != nil { - glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, object.Key, err) + governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, key) + if err := s3a.enforceObjectLockProtections(r, bucket, key, "", governanceBypassAllowed); err != nil { + glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, key, err) return s3err.ErrAccessDenied } - if strings.HasSuffix(object.Key, "/") { - return s3a.deleteDirectoryMarker(r, bucket, object.Key) + if strings.HasSuffix(key, "/") { + return s3a.deleteDirectoryMarker(r, bucket, key) } - if err := s3a.deleteUnversionedObjectWithClient(r.Context(), client, bucket, object.Key, false); err != nil { - glog.Errorf("DeleteMultipleObjectsHandler: failed to delete %s/%s: %v", bucket, object.Key, err) + if err := s3a.deleteUnversionedObjectWithClient(r.Context(), client, bucket, key, false); err != nil { + glog.Errorf("DeleteMultipleObjectsHandler: failed to delete %s/%s: %v", bucket, key, err) return s3err.ErrInternalError } diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index ad88378916..2ccd42d6e4 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -78,6 +78,9 @@ type S3ApiServerOption struct { // AllowUntrustedRemoteEndpoints lets a read of a remote-only object dial a // mounted endpoint that resolves to a loopback / private / metadata host. AllowUntrustedRemoteEndpoints bool + // AllowBackslashInKey keeps '\' a literal object-key character instead of + // normalizing it to '/', matching AWS S3 where the two are distinct keys. + AllowBackslashInKey bool } // s3ChunkCacheChunkSizeMB is the assumed chunk size (in MiB) used to convert @@ -532,6 +535,10 @@ func NewS3ApiServerWithStore(router *mux.Router, option *S3ApiServerOption, expl // pointer-to-missing-file states without waiting for a client GET. s3ApiServer.versionsReconcilerStop = s3ApiServer.startVersioningReconciler() + // Publish the key mode only once the server is fully constructed so a + // failed construction cannot flip the mode of a running server. + SetAllowBackslashInObjectKey(option.AllowBackslashInKey) + return s3ApiServer, nil }