diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 223882a92..d38d1d20b 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -521,7 +521,7 @@ func (iam *IdentityAccessManagement) authRequest(r *http.Request, action Action) glog.Errorf("Error evaluating bucket policy for %s/%s: %v - denying access", bucket, object, err) return identity, s3err.ErrInternalError } else if evaluated { - // A bucket policy exists and was evaluated + // A bucket policy exists and was evaluated with a matching statement if allowed { // Policy explicitly allows this action - grant access immediately // This bypasses IAM checks to support cross-account access and policy-only principals @@ -529,11 +529,12 @@ func (iam *IdentityAccessManagement) authRequest(r *http.Request, action Action) policyAllows = true } else { // Policy explicitly denies this action - deny access immediately - glog.V(3).Infof("Bucket policy denies %s to %s on %s/%s", identity.Name, action, bucket, object) + // Note: Explicit Deny in bucket policy overrides all other permissions + glog.V(3).Infof("Bucket policy explicitly denies %s to %s on %s/%s", identity.Name, action, bucket, object) return identity, s3err.ErrAccessDenied } } - // If not evaluated (no policy), fall through to IAM/identity checks + // If not evaluated (no policy or no matching statements), fall through to IAM/identity checks } // Only check IAM if bucket policy didn't explicitly allow diff --git a/weed/s3api/policy_engine/engine.go b/weed/s3api/policy_engine/engine.go index 709fafda4..01af3c240 100644 --- a/weed/s3api/policy_engine/engine.go +++ b/weed/s3api/policy_engine/engine.go @@ -109,7 +109,7 @@ func (engine *PolicyEngine) evaluateCompiledPolicy(policy *CompiledPolicy, args // AWS Policy evaluation logic: // 1. Check for explicit Deny - if found, return Deny // 2. Check for explicit Allow - if found, return Allow - // 3. If no explicit Allow is found, return Deny (default deny) + // 3. If no matching statements, return Indeterminate (fall through to IAM) hasExplicitAllow := false @@ -128,7 +128,9 @@ func (engine *PolicyEngine) evaluateCompiledPolicy(policy *CompiledPolicy, args return PolicyResultAllow } - return PolicyResultDeny // Default deny + // No matching statements - return Indeterminate to fall through to IAM + // This allows IAM policies to grant access even when bucket policy doesn't mention the action + return PolicyResultIndeterminate } // evaluateStatement evaluates a single policy statement diff --git a/weed/s3api/policy_engine/engine_test.go b/weed/s3api/policy_engine/engine_test.go index 799579ce6..1bb36dc4a 100644 --- a/weed/s3api/policy_engine/engine_test.go +++ b/weed/s3api/policy_engine/engine_test.go @@ -76,8 +76,8 @@ func TestPolicyEngine(t *testing.T) { } result = engine.EvaluatePolicy("test-bucket", args) - if result != PolicyResultDeny { - t.Errorf("Expected Deny for non-matching action, got %v", result) + if result != PolicyResultIndeterminate { + t.Errorf("Expected Indeterminate for non-matching action (should fall through to IAM), got %v", result) } // Test GetBucketPolicy @@ -471,8 +471,8 @@ func TestPolicyEvaluationWithConditions(t *testing.T) { // Test non-matching IP args.Conditions["aws:SourceIp"] = []string{"10.0.0.1"} result = engine.EvaluatePolicy("test-bucket", args) - if result != PolicyResultDeny { - t.Errorf("Expected Deny for non-matching IP, got %v", result) + if result != PolicyResultIndeterminate { + t.Errorf("Expected Indeterminate for non-matching IP (should fall through to IAM), got %v", result) } } diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index cffcfe8cf..28b227ce2 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -617,12 +617,22 @@ func (s3a *S3ApiServer) AuthWithPublicRead(handler http.HandlerFunc, action Acti glog.Errorf("AuthWithPublicRead: error evaluating bucket policy for %s/%s: %v - denying access", bucket, object, err) s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) return - } else if evaluated && allowed { - glog.V(3).Infof("AuthWithPublicRead: allowing anonymous access to bucket %s (bucket policy)", bucket) - handler(w, r) - return + } else if evaluated { + // A bucket policy exists and was evaluated with a matching statement + if allowed { + // Policy explicitly allows anonymous access + glog.V(3).Infof("AuthWithPublicRead: allowing anonymous access to bucket %s (bucket policy)", bucket) + handler(w, r) + return + } else { + // Policy explicitly denies anonymous access + glog.V(3).Infof("AuthWithPublicRead: bucket policy explicitly denies anonymous access to %s/%s", bucket, object) + s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) + return + } } - glog.V(3).Infof("AuthWithPublicRead: bucket %s does not allow public access, falling back to IAM auth", bucket) + // No matching policy statement - fall through to check ACLs and then IAM auth + glog.V(3).Infof("AuthWithPublicRead: no bucket policy match for %s, checking ACLs", bucket) } // For all authenticated requests and anonymous requests to non-public buckets,