mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-21 21:44:22 +02:00
s3: mask anonymous misses behind AccessDenied unless the bucket is listable (#11857)
* 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>
This commit is contained in:
1 parent
6936bbdc3e
commit
0de6347b11
4 files changed
+116
-22
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in new issue
Block a user