diff options
| author | chrislu <chris.lu@gmail.com> | 2025-12-09 00:17:29 -0800 |
|---|---|---|
| committer | chrislu <chris.lu@gmail.com> | 2025-12-09 00:17:29 -0800 |
| commit | 4e6e7b6ac5eaf340a8755882c147fb2d7fac2714 (patch) | |
| tree | e133355e2651d0af1292cae57f264e642534db64 | |
| parent | 50eba1ecf8fc7ec46fb5f4e410cee4ee835828f5 (diff) | |
| download | seaweedfs-4e6e7b6ac5eaf340a8755882c147fb2d7fac2714.tar.xz seaweedfs-4e6e7b6ac5eaf340a8755882c147fb2d7fac2714.zip | |
Implement tag-based policy re-check in handlers
- Add checkPolicyWithEntry helper to S3ApiServer for handlers to re-check
policy after fetching object entry (for s3:ExistingObjectTag conditions)
- Add HasPolicyForBucket method to policy engine for efficient check
- Integrate policy re-check in GetObjectHandler after entry is fetched
- Integrate policy re-check in HeadObjectHandler after entry is fetched
- Update auth_credentials.go comments to explain two-phase evaluation
- Update documentation with supported operations for tag-based conditions
This implements 'Approach 1' where handlers re-check the policy with
the object entry after fetching it, allowing tag-based conditions to
be properly evaluated.
| -rw-r--r-- | weed/s3api/auth_credentials.go | 4 | ||||
| -rw-r--r-- | weed/s3api/policy_engine/README_POLICY_ENGINE.md | 9 | ||||
| -rw-r--r-- | weed/s3api/policy_engine/engine.go | 17 | ||||
| -rw-r--r-- | weed/s3api/s3api_bucket_policy_engine.go | 8 | ||||
| -rw-r--r-- | weed/s3api/s3api_object_handlers.go | 18 | ||||
| -rw-r--r-- | weed/s3api/s3api_server.go | 35 |
6 files changed, 80 insertions, 11 deletions
diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 378788084..3f4670a7e 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -582,7 +582,9 @@ 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) - // Evaluate bucket policy (objectEntry nil - not yet fetched at auth time) + // Phase 1: Evaluate bucket policy without object entry. + // Tag-based conditions (s3:ExistingObjectTag) are re-checked by handlers + // after fetching the entry, which is the Phase 2 check. allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, object, string(action), principal, r, nil) if err != nil { diff --git a/weed/s3api/policy_engine/README_POLICY_ENGINE.md b/weed/s3api/policy_engine/README_POLICY_ENGINE.md index efb19f68d..13d2e0029 100644 --- a/weed/s3api/policy_engine/README_POLICY_ENGINE.md +++ b/weed/s3api/policy_engine/README_POLICY_ENGINE.md @@ -163,6 +163,15 @@ You can control access based on object tags using `s3:ExistingObjectTag/<tag-key This allows anonymous access only to objects that have a tag `status=public`. +**Supported Operations for Tag-Based Conditions:** + +Tag-based conditions (`s3:ExistingObjectTag/<key>`) are evaluated for the following operations: +- `s3:GetObject` (GET object) +- `s3:GetObjectVersion` (GET object with versionId) +- `HeadObject` (HEAD object) + +Note: For these conditions to be evaluated, the object must exist and the policy engine re-checks access after fetching the object metadata. + ## Policy Evaluation ### Evaluation Order (AWS-Compatible) diff --git a/weed/s3api/policy_engine/engine.go b/weed/s3api/policy_engine/engine.go index 57a13881c..db4317b62 100644 --- a/weed/s3api/policy_engine/engine.go +++ b/weed/s3api/policy_engine/engine.go @@ -91,6 +91,14 @@ func (engine *PolicyEngine) DeleteBucketPolicy(bucketName string) error { return nil } +// HasPolicyForBucket checks if a bucket has a policy configured +func (engine *PolicyEngine) HasPolicyForBucket(bucketName string) bool { + engine.mutex.RLock() + defer engine.mutex.RUnlock() + _, exists := engine.contexts[bucketName] + return exists +} + // EvaluatePolicy evaluates a policy for the given arguments func (engine *PolicyEngine) EvaluatePolicy(bucketName string, args *PolicyEvaluationArgs) PolicyEvaluationResult { engine.mutex.RLock() @@ -352,15 +360,6 @@ func GetObjectNameFromArn(arn string) string { return "" } -// HasPolicyForBucket checks if a bucket has a policy -func (engine *PolicyEngine) HasPolicyForBucket(bucketName string) bool { - engine.mutex.RLock() - defer engine.mutex.RUnlock() - - _, exists := engine.contexts[bucketName] - return exists -} - // GetPolicyStatements returns all policy statements for a bucket func (engine *PolicyEngine) GetPolicyStatements(bucketName string) []PolicyStatement { engine.mutex.RLock() diff --git a/weed/s3api/s3api_bucket_policy_engine.go b/weed/s3api/s3api_bucket_policy_engine.go index 8515afd8d..422cf7d26 100644 --- a/weed/s3api/s3api_bucket_policy_engine.go +++ b/weed/s3api/s3api_bucket_policy_engine.go @@ -87,6 +87,11 @@ func (bpe *BucketPolicyEngine) DeleteBucketPolicy(bucket string) error { return bpe.engine.DeleteBucketPolicy(bucket) } +// HasPolicyForBucket checks if a bucket has a policy configured +func (bpe *BucketPolicyEngine) HasPolicyForBucket(bucket string) bool { + return bpe.engine.HasPolicyForBucket(bucket) +} + // EvaluatePolicy evaluates whether an action is allowed by bucket policy // // Parameters: @@ -95,7 +100,8 @@ func (bpe *BucketPolicyEngine) DeleteBucketPolicy(bucket string) error { // - 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) +// - objectEntry: the object's metadata from entry.Extended (can be nil at auth time, +// should be passed when available for tag-based conditions like s3:ExistingObjectTag) // // Returns: // - allowed: whether the policy allows the action diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index 43cc4e5fc..034710c3c 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -634,6 +634,16 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) } entryFetchTime = time.Since(tEntryFetch) + // Re-check bucket policy with object entry for tag-based conditions (e.g., s3:ExistingObjectTag) + if objectEntryForSSE != nil { + identity, _ := s3_constants.GetIdentityFromContext(r).(*Identity) + principal := buildPrincipalARN(identity) + if errCode, _ := s3a.checkPolicyWithEntry(r, bucket, object, string(s3_constants.ACTION_READ), principal, objectEntryForSSE.Extended); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + } + // Check if PartNumber query parameter is present (for multipart GET requests) partNumberStr := r.URL.Query().Get("partNumber") if partNumberStr == "" { @@ -2187,6 +2197,14 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request return } + // Re-check bucket policy with object entry for tag-based conditions (e.g., s3:ExistingObjectTag) + identity, _ := s3_constants.GetIdentityFromContext(r).(*Identity) + principal := buildPrincipalARN(identity) + if errCode, _ := s3a.checkPolicyWithEntry(r, bucket, object, string(s3_constants.ACTION_READ), principal, objectEntryForSSE.Extended); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // Implicit Directory Handling for s3fs Compatibility // ==================================================== // diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 939c891c3..ac277f778 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -252,6 +252,41 @@ func (s3a *S3ApiServer) syncBucketPolicyToEngine(bucket string, policyDoc *polic } } +// checkPolicyWithEntry re-evaluates bucket policy with the object entry metadata. +// This is used by handlers after fetching the entry to enforce tag-based conditions +// like s3:ExistingObjectTag/<key>. +// +// Returns: +// - s3err.ErrCode: ErrNone if allowed, ErrAccessDenied if denied +// - bool: true if policy was evaluated (has policy for bucket), false if no policy +func (s3a *S3ApiServer) checkPolicyWithEntry(r *http.Request, bucket, object, action, principal string, objectEntry map[string][]byte) (s3err.ErrorCode, bool) { + if s3a.policyEngine == nil { + return s3err.ErrNone, false + } + + // Skip if no policy for this bucket + if !s3a.policyEngine.HasPolicyForBucket(bucket) { + return s3err.ErrNone, false + } + + allowed, evaluated, err := s3a.policyEngine.EvaluatePolicy(bucket, object, action, principal, r, objectEntry) + if err != nil { + glog.Errorf("checkPolicyWithEntry: error evaluating policy for %s/%s: %v", bucket, object, err) + return s3err.ErrInternalError, true + } + + if !evaluated { + return s3err.ErrNone, false + } + + if !allowed { + glog.V(3).Infof("checkPolicyWithEntry: policy denied access to %s/%s for principal %s", bucket, object, principal) + return s3err.ErrAccessDenied, true + } + + return s3err.ErrNone, true +} + // classifyDomainNames classifies domains into path-style and virtual-host style domains. // A domain is considered path-style if: // 1. It contains a dot (has subdomains) |
