mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-09-29 19:25:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

s3: evaluate bucket policy before ACL public-read for anonymous requests (#11471)

Browse Source 
* s3: evaluate bucket policy before ACL public-read for anonymous requests

AuthWithPublicRead granted anonymous access on a public-read ACL before
consulting the bucket policy, so an explicit Deny (e.g. s3:ListBucket)
was skipped for anonymous callers while still enforced for authenticated
ones. Run the policy engine first: a matching Deny or Allow is honored,
otherwise fall through to the ACL grant as before.

* s3: defer object-level anonymous requests to the handler's policy recheck

Evaluating the bucket policy with a nil entry at middleware time makes
tag conditions like s3:ExistingObjectTag/<key> resolve against missing
values, so a conditional Deny could wrongly block anonymous Get/Head on
a public bucket whose handler recheck would permit it. Object requests
now take the ACL grant and let Get/HeadObjectHandler re-evaluate with
the fetched entry; only bucket-level requests (List, HeadBucket), which
have no such recheck, are decided by the middleware policy verdict.

Reading the bucket config first also refreshes the compiled policy on a
cache miss, so a remotely deleted policy cannot leave a stale verdict
in the engine for nonresident buckets.

* s3: recheck bucket policy before serving directory objects

handleDirectoryObjectRequest runs before the object handlers' policy
recheck, so directory content on a public-read bucket was served to
anonymous callers without any policy evaluation. Evaluate the policy
with the directory entry, matching the recheck the file path performs.
This commit is contained in:
Chris Lu
2026-09-26 17:59:57 +08:00
committed by GitHub
parent 3c1e8ca7a8
commit 7129e1178e
3 changed files with 76 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/s3api/s3api_bucket_handlers.go
+17 -5
View File
@@ -868,18 +868,24 @@ func (s3a *S3ApiServer) AuthWithPublicRead(handler http.HandlerFunc, action Acti
glog.V(4).Infof("AuthWithPublicRead: bucket=%s, object=%s, authType=%v, isAnonymous=%v", bucket, object, authType, isAnonymous)
// For anonymous requests, check if bucket allows public read via ACLs or bucket policies
// For anonymous requests, check if bucket allows public read via bucket policies or ACLs
if isAnonymous {
// First check ACL-based public access
// Loading the bucket config on a cache miss also refreshes the
// compiled policy, so a remotely deleted policy cannot leave a
// stale verdict in the engine.
isPublic := s3a.isBucketPublicRead(bucket)
glog.V(4).Infof("AuthWithPublicRead: bucket=%s, isPublicACL=%v", bucket, isPublic)
if isPublic {
// Object requests are re-evaluated inside Get/HeadObjectHandler
// once the entry is fetched, where tag conditions like
// s3:ExistingObjectTag/<key> can resolve correctly. Only
// bucket-level requests (List, HeadBucket) rely on this check alone.
if isPublic && object != "" {
glog.V(3).Infof("AuthWithPublicRead: allowing anonymous access to public-read bucket %s (ACL)", bucket)
handler(w, r)
return
}
// Check bucket policy for anonymous access using the policy engine
principal := "*" // Anonymous principal
// Evaluate bucket policy (objectEntry nil - not yet fetched)
allowed, evaluated, err := s3a.policyEngine.EvaluatePolicy(bucket, object, string(action), principal, r, nil, nil)
@@ -903,7 +909,13 @@ func (s3a *S3ApiServer) AuthWithPublicRead(handler http.HandlerFunc, action Acti
return
}
}
// No matching policy statement - fall through to check ACLs and then IAM auth
// No matching policy statement - fall back to the ACL grant
if isPublic {
glog.V(3).Infof("AuthWithPublicRead: allowing anonymous access to public-read bucket %s (ACL)", bucket)
handler(w, r)
return
}
glog.V(3).Infof("AuthWithPublicRead: no bucket policy match for %s, checking ACLs", bucket)
}
weed/s3api/s3api_bucket_handlers_misc_test.go
+55
View File
@@ -2,6 +2,7 @@ package s3api
import (
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
@@ -13,6 +14,7 @@ import (
"github.com/gorilla/mux"
"github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
"github.com/stretchr/testify/require"
)
func newMiscTestServer(t *testing.T, bucket string) *S3ApiServer {
@@ -308,3 +310,56 @@ func TestUploadMissingBucketAutoCreateDisabled(t *testing.T) {
})
}
}
func TestAuthWithPublicReadHonorsPolicyDeny(t *testing.T) {
const bucket = "public-deny"
s3a := newMiscTestServer(t, bucket)
s3a.bucketConfigCache.Set(bucket, &BucketConfig{Name: bucket, IsPublicRead: true})
s3a.policyEngine = NewBucketPolicyEngine()
called := false
handler := s3a.AuthWithPublicRead(func(w http.ResponseWriter, r *http.Request) { called = true }, s3_constants.ACTION_LIST)
serve := func() *httptest.ResponseRecorder {
called = false
rr := httptest.NewRecorder()
handler(rr, newBucketRequest(http.MethodGet, bucket, "list-type=2", ""))
return rr
}
setPolicy := func(statements ...string) {
doc := `{"Version":"2012-10-17","Statement":[` + strings.Join(statements, ",") + `]}`
require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy(bucket, doc))
}
arn := fmt.Sprintf("arn:aws:s3:::%s", bucket)
denyList := fmt.Sprintf(`{"Effect":"Deny","Principal":"*","Action":"s3:ListBucket","Resource":"%s"}`, arn)
denyGet := fmt.Sprintf(`{"Effect":"Deny","Principal":"*","Action":"s3:GetObject","Resource":"%s/*"}`, arn)
allowList := fmt.Sprintf(`{"Effect":"Allow","Principal":"*","Action":"s3:ListBucket","Resource":"%s"}`, arn)
setPolicy(denyList)
rr := serve()
require.False(t, called, "anonymous ListObjectsV2 reached the handler despite an explicit ListBucket Deny")
require.Equal(t, http.StatusForbidden, rr.Code)
setPolicy(denyGet)
rr = serve()
require.True(t, called, "an unrelated Deny must not block the ACL public-read grant")
setPolicy(denyList, allowList)
rr = serve()
require.False(t, called, "explicit Deny must beat a matching Allow")
setPolicy(allowList)
rr = serve()
require.True(t, called, "explicit Allow must still permit anonymous listing")
// Object requests defer to the handler's phase-2 recheck, which evaluates
// tag conditions against the fetched entry — a tag-conditional Deny must
// not terminate them here on a missing value.
tagDeny := fmt.Sprintf(`{"Effect":"Deny","Principal":"*","Action":"s3:GetObject","Resource":"%s/*","Condition":{"StringNotEquals":{"s3:ExistingObjectTag/classification":"public"}}}`, arn)
setPolicy(tagDeny)
called = false
rr = httptest.NewRecorder()
objectReq := newBucketRequest(http.MethodGet, bucket, "", "")
objectReq = mux.SetURLVars(objectReq, map[string]string{"bucket": bucket, "object": "secret/x"})
s3a.AuthWithPublicRead(func(w http.ResponseWriter, r *http.Request) { called = true }, s3_constants.ACTION_READ)(rr, objectReq)
require.True(t, called, "anonymous object request on a public bucket must reach the handler's tag-aware recheck")
}
weed/s3api/s3api_object_handlers.go
+4
View File
@@ -508,6 +508,10 @@ func (s3a *S3ApiServer) handleDirectoryObjectRequest(w http.ResponseWriter, r *h
s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
return true // Request was handled (with error)
} else if dirEntry != nil {
if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, string(s3_constants.ACTION_READ), dirEntry.Extended, handlerName); errCode != s3err.ErrNone {
s3err.WriteErrorResponse(w, r, errCode)
return true // Request was handled (denied)
}
glog.V(2).Infof("%s: directory object %s/%s found, serving content", handlerName, bucket, object)
s3a.serveDirectoryContent(w, r, dirEntry)
return true // Request was handled successfully