diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index 59888e255..354be0826 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -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/ 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) } diff --git a/weed/s3api/s3api_bucket_handlers_misc_test.go b/weed/s3api/s3api_bucket_handlers_misc_test.go index 2e704d860..b8d98fe26 100644 --- a/weed/s3api/s3api_bucket_handlers_misc_test.go +++ b/weed/s3api/s3api_bucket_handlers_misc_test.go @@ -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") +} diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index fb5f39660..acc1ddcb2 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -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