Browse Source

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.
copilot/sub-pr-7677
chrislu 1 month ago
parent
commit
4e6e7b6ac5
  1. 4
      weed/s3api/auth_credentials.go
  2. 9
      weed/s3api/policy_engine/README_POLICY_ENGINE.md
  3. 17
      weed/s3api/policy_engine/engine.go
  4. 8
      weed/s3api/s3api_bucket_policy_engine.go
  5. 18
      weed/s3api/s3api_object_handlers.go
  6. 35
      weed/s3api/s3api_server.go

4
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 {

9
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)

17
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()

8
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

18
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
// ====================================================
//

35
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)

Loading…
Cancel
Save