diff options
| author | chrislu <chris.lu@gmail.com> | 2025-12-08 23:58:22 -0800 |
|---|---|---|
| committer | chrislu <chris.lu@gmail.com> | 2025-12-09 00:01:31 -0800 |
| commit | 02a041b28d17b8eac2c466c2c53a75fd3dc1423f (patch) | |
| tree | a0f2127f40b84b3e61035c2569fda40779b1d7fa | |
| parent | c6721bb18d8f70ec9e86b6aa043b488d2d2f0239 (diff) | |
| download | seaweedfs-02a041b28d17b8eac2c466c2c53a75fd3dc1423f.tar.xz seaweedfs-02a041b28d17b8eac2c466c2c53a75fd3dc1423f.zip | |
s3: update EvaluatePolicy to accept object entry for tag conditions
Update BucketPolicyEngine.EvaluatePolicy to accept objectEntry parameter
(entry.Extended metadata) for evaluating tag-based policy conditions.
Changes:
- Add objectEntry parameter to EvaluatePolicy method
- Update callers in auth_credentials.go and s3api_bucket_handlers.go
- Pass nil for objectEntry in auth layer (entry fetched later in handlers)
For tag-based conditions to work, handlers should call EvaluatePolicy
with the object's entry.Extended after fetching the entry from filer.
| -rw-r--r-- | weed/s3api/auth_credentials.go | 6 | ||||
| -rw-r--r-- | weed/s3api/s3api_bucket_handlers.go | 5 | ||||
| -rw-r--r-- | weed/s3api/s3api_bucket_policy_engine.go | 84 |
3 files changed, 30 insertions, 65 deletions
diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index b5824f2d1..ec9edb6a0 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -582,8 +582,10 @@ func (iam *IdentityAccessManagement) authRequest(r *http.Request, action Action) // - No policy or indeterminate → fall through to IAM checks if iam.policyEngine != nil && bucket != "" { principal := buildPrincipalARN(identity) - // Use context-aware policy evaluation to get the correct S3 action - allowed, evaluated, err := iam.policyEngine.EvaluatePolicyWithContext(bucket, object, string(action), principal, r) + // Evaluate bucket policy with request context for accurate action resolution + // Note: objectEntry is nil here as we don't have the entry at auth time + // For tag-based conditions to work, the caller should re-evaluate with entry after fetching it + allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, object, string(action), principal, r, nil) if err != nil { // SECURITY: Fail-close on policy evaluation errors diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index 09bea9aa8..928d500b0 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -765,8 +765,9 @@ func (s3a *S3ApiServer) AuthWithPublicRead(handler http.HandlerFunc, action Acti // Check bucket policy for anonymous access using the policy engine principal := "*" // Anonymous principal - // Use context-aware policy evaluation to get the correct S3 action - allowed, evaluated, err := s3a.policyEngine.EvaluatePolicyWithContext(bucket, object, string(action), principal, r) + // Evaluate bucket policy with request context for accurate action resolution + // Note: objectEntry is nil here - for tag-based conditions, re-evaluate after fetching entry + allowed, evaluated, err := s3a.policyEngine.EvaluatePolicy(bucket, object, string(action), principal, r, nil) if err != nil { // SECURITY: Fail-close on policy evaluation errors // If we can't evaluate the policy, deny access rather than falling through to IAM diff --git a/weed/s3api/s3api_bucket_policy_engine.go b/weed/s3api/s3api_bucket_policy_engine.go index fc674e12f..8515afd8d 100644 --- a/weed/s3api/s3api_bucket_policy_engine.go +++ b/weed/s3api/s3api_bucket_policy_engine.go @@ -88,11 +88,20 @@ func (bpe *BucketPolicyEngine) DeleteBucketPolicy(bucket string) error { } // EvaluatePolicy evaluates whether an action is allowed by bucket policy -// Returns: (allowed bool, evaluated bool, error) -// - allowed: whether the policy allows the action -// - evaluated: whether a policy was found and evaluated (false = no policy exists) -// - error: any error during evaluation -func (bpe *BucketPolicyEngine) EvaluatePolicy(bucket, object, action, principal string) (allowed bool, evaluated bool, err error) { +// +// Parameters: +// - bucket: the bucket name +// - object: the object key (can be empty for bucket-level operations) +// - action: the action being performed (e.g., "Read", "Write") +// - principal: the principal ARN or identifier +// - r: the HTTP request (optional, used for condition evaluation and action resolution) +// - objectEntry: the object's metadata from entry.Extended (can be nil) +// +// Returns: +// - allowed: whether the policy allows the action +// - evaluated: whether a policy was found and evaluated (false = no policy exists) +// - error: any error during evaluation +func (bpe *BucketPolicyEngine) EvaluatePolicy(bucket, object, action, principal string, r *http.Request, objectEntry map[string][]byte) (allowed bool, evaluated bool, err error) { // Validate required parameters if bucket == "" { return false, false, fmt.Errorf("bucket cannot be empty") @@ -101,19 +110,22 @@ func (bpe *BucketPolicyEngine) EvaluatePolicy(bucket, object, action, principal return false, false, fmt.Errorf("action cannot be empty") } - // Convert action to S3 action format using base mapping (no HTTP context available) - s3Action := mapBaseActionToS3Format(action) + // Convert action to S3 action format + // ResolveS3Action handles nil request internally (falls back to mapBaseActionToS3Format) + s3Action := ResolveS3Action(r, action, bucket, object) // Build resource ARN resource := buildResourceARN(bucket, object) - glog.V(4).Infof("EvaluatePolicy: bucket=%s, resource=%s, action=%s, principal=%s", bucket, resource, s3Action, principal) + glog.V(4).Infof("EvaluatePolicy: bucket=%s, resource=%s, action=%s, principal=%s", + bucket, resource, s3Action, principal) // Evaluate using the policy engine args := &policy_engine.PolicyEvaluationArgs{ - Action: s3Action, - Resource: resource, - Principal: principal, + Action: s3Action, + Resource: resource, + Principal: principal, + ObjectEntry: objectEntry, } result := bpe.engine.EvaluatePolicy(bucket, args) @@ -133,53 +145,3 @@ func (bpe *BucketPolicyEngine) EvaluatePolicy(bucket, object, action, principal return false, false, fmt.Errorf("unknown policy result: %v", result) } } - -// EvaluatePolicyWithContext evaluates whether an action is allowed by bucket policy using HTTP request context -// This version uses the HTTP request to determine the actual S3 action more accurately -func (bpe *BucketPolicyEngine) EvaluatePolicyWithContext(bucket, object, action, principal string, r *http.Request) (allowed bool, evaluated bool, err error) { - // Validate required parameters - if bucket == "" { - return false, false, fmt.Errorf("bucket cannot be empty") - } - if action == "" { - return false, false, fmt.Errorf("action cannot be empty") - } - - // Convert action to S3 action format using request context - // ResolveS3Action handles nil request internally (falls back to mapBaseActionToS3Format) - s3Action := ResolveS3Action(r, action, bucket, object) - - // Build resource ARN - resource := buildResourceARN(bucket, object) - - glog.V(4).Infof("EvaluatePolicyWithContext: bucket=%s, resource=%s, action=%s (from %s), principal=%s", - bucket, resource, s3Action, action, principal) - - // Evaluate using the policy engine - args := &policy_engine.PolicyEvaluationArgs{ - Action: s3Action, - Resource: resource, - Principal: principal, - } - - result := bpe.engine.EvaluatePolicy(bucket, args) - - switch result { - case policy_engine.PolicyResultAllow: - glog.V(3).Infof("EvaluatePolicyWithContext: ALLOW - bucket=%s, action=%s, principal=%s", bucket, s3Action, principal) - return true, true, nil - case policy_engine.PolicyResultDeny: - glog.V(3).Infof("EvaluatePolicyWithContext: DENY - bucket=%s, action=%s, principal=%s", bucket, s3Action, principal) - return false, true, nil - case policy_engine.PolicyResultIndeterminate: - // No policy exists for this bucket - glog.V(4).Infof("EvaluatePolicyWithContext: INDETERMINATE (no policy) - bucket=%s", bucket) - return false, false, nil - default: - return false, false, fmt.Errorf("unknown policy result: %v", result) - } -} - -// NOTE: The convertActionToS3Format wrapper has been removed for simplicity. -// EvaluatePolicy and EvaluatePolicyWithContext now call ResolveS3Action or -// mapBaseActionToS3Format directly, making the control flow more explicit. |
