From 0de6347b110854ec084bffe1d7190cc967f6ca16 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 20 Oct 2026 12:52:58 -0700 Subject: [PATCH] s3: mask anonymous misses behind AccessDenied unless the bucket is listable (#11857) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * s3: mask anonymous misses behind AccessDenied unless the bucket is listable Deferred anonymous GET/HEAD authorization only runs once an entry is selected, so misses returned 404 while existing keys returned 403 — an unauthenticated existence oracle for bucket names and object keys. Route not-found responses in Get/HeadObjectHandler through a check that only discloses 404 when anonymous listing is allowed (policy, identity or a public-read bucket); otherwise the miss is indistinguishable from a denied read, matching AWS and 4.48 behavior. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: mask missing-directory misses for anonymous reads too Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: check a fixed request shape when masking anonymous misses The miss check passed the caller's request through, so query parameters could retarget the resolved action (?policy) or satisfy an s3:prefix condition on an unrelated subtree. Each check now runs on a request with a fixed shape: a ListBucket scoped to the probed key and a plain read of the key, so the miss only shows when the caller could learn the same fact directly. * s3: mask conditional-header misses for anonymous reads An If-Match conditional on a missing key wrote NoSuchKey directly from processConditionalHeaders, before the handlers reached the masked miss path, so anonymous callers could still distinguish a missing key from a denied read. * s3: keep the versionId when masking anonymous misses The read leg must evaluate the action the caller's request resolves to: a versioned miss is s3:GetObjectVersion, and dropping versionId would let a plain GetObject grant disclose versions the caller may not read. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../s3api_object_anonymous_cache_test.go | 10 ++- weed/s3api/s3api_object_anonymous_read.go | 69 +++++++++++++++++++ weed/s3api/s3api_object_handlers.go | 42 ++++++----- .../s3api_object_lock_actions_authz_test.go | 17 +++++ 4 files changed, 116 insertions(+), 22 deletions(-) diff --git a/weed/s3api/s3api_object_anonymous_cache_test.go b/weed/s3api/s3api_object_anonymous_cache_test.go index 34b03cacf3..d5a0f2f593 100644 --- a/weed/s3api/s3api_object_anonymous_cache_test.go +++ b/weed/s3api/s3api_object_anonymous_cache_test.go @@ -174,7 +174,7 @@ func TestAnonymousObjectACLVersionedEmptyDirectoryMarker(t *testing.T) { require.Equal(t, "null", rr.Header().Get("x-amz-version-id")) } rr := serveAnonymousRead(s3a, method, "folder", "", nil) - require.Equal(t, 404, rr.Code, "the bare prefix is still not an object") + require.Equal(t, 403, rr.Code, "the bare prefix is not an object, and the miss stays masked for a non-listable bucket") marker.Extended[s3_constants.ExtAmzAclKey] = []byte(`[]`) rr = serveAnonymousRead(s3a, method, "folder/", "", nil) require.Equal(t, 403, rr.Code, rr.Body.String()) @@ -238,8 +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. - for _, object := range []string{"folder/", "folder//", "folder\\"} { - assertMissing(serveAnonymousRead(s3a, method, object, route.query, headers)) + assertMissing(serveAnonymousRead(s3a, method, "folder/", 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\\"} { + rr := serveAnonymousRead(s3a, method, object, route.query, headers) + require.Equal(t, http.StatusForbidden, rr.Code, rr.Body.String()) } if kind == "regular file" || kind == "prefix object" || kind == "empty prefix object" { // A slash-only policy must not authorize data belonging to the bare key. diff --git a/weed/s3api/s3api_object_anonymous_read.go b/weed/s3api/s3api_object_anonymous_read.go index 932d2e383c..8146896453 100644 --- a/weed/s3api/s3api_object_anonymous_read.go +++ b/weed/s3api/s3api_object_anonymous_read.go @@ -4,10 +4,12 @@ import ( "context" "encoding/json" "net/http" + "net/url" "path" "strings" "github.com/aws/aws-sdk-go/service/s3" + "github.com/gorilla/mux" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) @@ -110,3 +112,70 @@ func (s3a *S3ApiServer) authorizeAnonymousObjectRead(r *http.Request, bucket, ob } return s3err.ErrAccessDenied } + +// authorizeAnonymousBucketList reports whether an anonymous caller may learn +// which keys exist in the bucket: bucket listing via policy or identity, or a +// 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"] + // 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 + // ListBucket scoped to the probed key, and a plain read of the key itself. + // A caller that could learn the key's existence either way — by listing or + // by getting it — gets the real miss. + listReq := r.Clone(r.Context()) + listReq.URL = &url.URL{Path: "/" + bucket, RawQuery: url.Values{"prefix": {object}}.Encode()} + // versionId must survive: a read of a specific version resolves to + // s3:GetObjectVersion, and dropping it would let a GetObject grant answer + // for versions the caller may not read. + readQuery := url.Values{} + if v := r.URL.Query().Get("versionId"); v != "" { + readQuery.Set("versionId", v) + } + readReq := r.Clone(r.Context()) + readReq.URL = &url.URL{Path: "/" + bucket + "/" + object, RawQuery: readQuery.Encode()} + + principal := buildPrincipalARN(identity, r) + policyAllowed := false + code, allowed := s3a.checkPolicyWithEntry(listReq, bucket, "", string(s3_constants.ACTION_LIST), principal, nil) + if code != s3err.ErrNone { + return code + } + policyAllowed = allowed + if !policyAllowed { + code, allowed = s3a.checkPolicyWithEntry(readReq, bucket, object, string(s3_constants.ACTION_READ), principal, nil) + if code != s3err.ErrNone { + return code + } + policyAllowed = allowed + } + if s3a.iam.isActionExplicitlyDeniedByApplicablePolicies(listReq, identity, s3_constants.ACTION_LIST, bucket, "") || + s3a.iam.isActionExplicitlyDeniedByApplicablePolicies(readReq, identity, s3_constants.ACTION_READ, bucket, object) { + return s3err.ErrAccessDenied + } + if policyAllowed { + return s3err.ErrNone + } + if identity != nil && + (s3a.iam.VerifyActionPermission(listReq, identity, s3_constants.ACTION_LIST, bucket, "") == s3err.ErrNone || + s3a.iam.VerifyActionPermission(readReq, identity, s3_constants.ACTION_READ, bucket, object) == s3err.ErrNone) { + return s3err.ErrNone + } + if config, code := s3a.getBucketConfig(bucket); code == s3err.ErrNone && config.IsPublicRead { + return s3err.ErrNone + } + return s3err.ErrAccessDenied +} + +// writeAnonymousAwareNotFound keeps missing-bucket and missing-key misses +// indistinguishable from denied reads for anonymous callers: a 404 only +// reaches the wire when the bucket is anonymously listable. +func (s3a *S3ApiServer) writeAnonymousAwareNotFound(w http.ResponseWriter, r *http.Request, bucket string, code s3err.ErrorCode) { + if isAnonymousObjectRead(r) && s3a.authorizeAnonymousBucketList(r, bucket) != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) + return + } + s3err.WriteErrorResponse(w, r, code) +} diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index 877268dfb4..a29994b9e8 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -663,7 +663,7 @@ func (s3a *S3ApiServer) handleDirectoryObjectRequest(w http.ResponseWriter, r *h } else if isDirectoryObject { // Directory object but doesn't exist glog.V(2).Infof("%s: directory object %s/%s not found", handlerName, bucket, object) - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return true // Request was handled (with not found) } @@ -770,7 +770,11 @@ func (s3a *S3ApiServer) processConditionalHeaders(w http.ResponseWriter, r *http w.Header().Set("ETag", result.ETag) } - s3err.WriteErrorResponse(w, r, result.ErrorCode) + if result.ErrorCode == s3err.ErrNoSuchKey || result.ErrorCode == s3err.ErrNoSuchBucket { + s3a.writeAnonymousAwareNotFound(w, r, bucket, result.ErrorCode) + } else { + s3err.WriteErrorResponse(w, r, result.ErrorCode) + } return result, true // request handled } return result, false // request not handled @@ -841,7 +845,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) versioningConfigured, err = s3a.isVersioningConfigured(bucket) if err != nil { if err == filer_pb.ErrNotFound { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchBucket) return } glog.Errorf("Error checking versioning status for bucket %s: %v", bucket, err) @@ -860,7 +864,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) if err != nil { glog.Errorf("Failed to get specific version %s: %v", versionId, err) if errors.Is(err, filer_pb.ErrNotFound) { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchVersion) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchVersion) } else { s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) } @@ -886,7 +890,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) entry, err = s3a.getLatestObjectVersion(bucket, object) if err != nil { glog.Errorf("GetObject: Failed to get latest version for %s/%s: %v", bucket, object, err) - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } else if errors.Is(versionsErr, filer_pb.ErrNotFound) { @@ -900,7 +904,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) } else { // No object at regular path either - object doesn't exist glog.V(3).Infof("GetObject: object not found at regular path or .versions for %s/%s", bucket, object) - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } else { @@ -909,7 +913,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) entry, err = s3a.getLatestObjectVersion(bucket, object) if err != nil { glog.Errorf("GetObject: Failed to get latest version for %s/%s: %v", bucket, object, err) - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } @@ -930,7 +934,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) // Check if this is a delete marker if entry.Extended != nil { if deleteMarker, exists := entry.Extended[s3_constants.ExtDeleteMarkerKey]; exists && string(deleteMarker) == "true" { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } @@ -975,7 +979,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) return } if objectEntryForSSE == nil { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } @@ -992,7 +996,7 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) // A bare directory path is not an object: 404 like AWS instead of an empty 200, // so Hadoop-style clients fall back to LIST discovery. HEAD and ranged GET already 404. if !strings.HasSuffix(object, "/") && isBareDirectory(objectEntryForSSE) { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } @@ -2518,7 +2522,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request versioningConfigured, err = s3a.isVersioningConfigured(bucket) if err != nil { if err == filer_pb.ErrNotFound { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchBucket) return } glog.Errorf("Error checking versioning status for bucket %s: %v", bucket, err) @@ -2536,7 +2540,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request if err != nil { glog.Errorf("Failed to get specific version %s for %s/%s: %v", versionId, bucket, object, err) if errors.Is(err, filer_pb.ErrNotFound) { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchVersion) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchVersion) } else { s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) } @@ -2562,7 +2566,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request entry, err = s3a.getLatestObjectVersion(bucket, object) if err != nil { glog.Errorf("HeadObject: Failed to get latest version for %s/%s: %v", bucket, object, err) - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } else if errors.Is(versionsErr, filer_pb.ErrNotFound) { @@ -2576,7 +2580,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request } else { // No object at regular path either - object doesn't exist glog.V(3).Infof("HeadObject: object not found at regular path or .versions for %s/%s", bucket, object) - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } else { @@ -2585,7 +2589,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request entry, err = s3a.getLatestObjectVersion(bucket, object) if err != nil { glog.Errorf("HeadObject: Failed to get latest version for %s/%s: %v", bucket, object, err) - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } @@ -2606,7 +2610,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request // Check if this is a delete marker if entry.Extended != nil { if deleteMarker, exists := entry.Extended[s3_constants.ExtDeleteMarkerKey]; exists && string(deleteMarker) == "true" { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } @@ -2644,7 +2648,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request return } if objectEntryForSSE == nil { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } @@ -2710,7 +2714,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request isZeroByteFile := objectEntryForSSE.Attributes.FileSize == 0 && !objectEntryForSSE.IsDirectory // A directory with data (a promoted file) is retrievable; empty directories 404 for LIST fallback. if isBareDirectory(objectEntryForSSE) { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } if isZeroByteFile { @@ -2718,7 +2722,7 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request if s3a.hasChildren(r.Context(), bucket, object) { // This is an implicit directory with children // Return 404 to force clients (like s3fs) to use LIST-based discovery - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + s3a.writeAnonymousAwareNotFound(w, r, bucket, s3err.ErrNoSuchKey) return } } diff --git a/weed/s3api/s3api_object_lock_actions_authz_test.go b/weed/s3api/s3api_object_lock_actions_authz_test.go index f3d2b8b70b..a90f7d93ae 100644 --- a/weed/s3api/s3api_object_lock_actions_authz_test.go +++ b/weed/s3api/s3api_object_lock_actions_authz_test.go @@ -11,6 +11,7 @@ import ( "github.com/gorilla/mux" iamlib "github.com/seaweedfs/seaweedfs/weed/iam" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/stretchr/testify/require" ) @@ -228,6 +229,22 @@ func TestObjectLockActionsRoundTrip(t *testing.T) { } } +// Anonymous misses must not disclose bucket or key existence: 404 is only +// served when anonymous listing is allowed. +func TestAnonymousBucketListAuthorization(t *testing.T) { + s3a := &S3ApiServer{iam: &IdentityAccessManagement{}, bucketConfigCache: NewBucketConfigCache(time.Minute)} + s3a.bucketConfigCache.Set("open", &BucketConfig{Name: "open", IsPublicRead: true}) + s3a.bucketConfigCache.Set("private", &BucketConfig{Name: "private"}) + + req := httptest.NewRequest(http.MethodGet, "http://localhost:8333/open/key", nil) + if got := s3a.authorizeAnonymousBucketList(req, "open"); got != s3err.ErrNone { + t.Errorf("listable bucket: %v, want ErrNone", got) + } + if got := s3a.authorizeAnonymousBucketList(req, "private"); got != s3err.ErrAccessDenied { + t.Errorf("private bucket: %v, want AccessDenied", got) + } +} + // Internal Seaweed-X-Amz-* headers carry server-validated object lock values // into entry metadata; client-supplied ones must be dropped on entry or the // dedicated-permission check on x-amz-object-lock-* is bypassed.