diff --git a/weed/s3api/policy_engine/README_POLICY_ENGINE.md b/weed/s3api/policy_engine/README_POLICY_ENGINE.md index b4846628da..97c4344e4a 100644 --- a/weed/s3api/policy_engine/README_POLICY_ENGINE.md +++ b/weed/s3api/policy_engine/README_POLICY_ENGINE.md @@ -356,6 +356,7 @@ The policy engine provides a seamless upgrade path from SeaweedFS's existing sim | `aws:SecureTransport` | Require HTTPS | | `aws:CurrentTime` | Time-based access control | | `s3:ExistingObjectTag/` | Tag-based access control for existing objects | +| `s3:RequestObjectTag/` | Tag conditions on the tags a PUT request sets | | Wildcard Patterns | Support for `*` and `?` in actions and resources | | Principal Matching | `*`, account IDs, and user ARNs | @@ -363,7 +364,6 @@ The policy engine provides a seamless upgrade path from SeaweedFS's existing sim | Feature | GitHub Issue | |---------|--------------| -| `s3:RequestObjectTag/` | For tag conditions on PUT requests | | `s3:RequestObjectTagKeys` | Check which tag keys are in request | | `s3:x-amz-content-sha256` | Content hash condition | | `s3:x-amz-server-side-encryption` | SSE condition | diff --git a/weed/s3api/policy_engine/conditions.go b/weed/s3api/policy_engine/conditions.go index becf8836da..a995c5ddb9 100644 --- a/weed/s3api/policy_engine/conditions.go +++ b/weed/s3api/policy_engine/conditions.go @@ -728,6 +728,10 @@ func GetConditionEvaluator(operator string) (ConditionEvaluator, error) { // ExistingObjectTagPrefix is the prefix for S3 policy condition keys const ExistingObjectTagPrefix = "s3:ExistingObjectTag/" +// RequestObjectTagPrefix is the prefix for condition keys on the tags a +// request sets through the X-Amz-Tagging header. +const RequestObjectTagPrefix = "s3:RequestObjectTag/" + // getConditionContextValue resolves the value(s) for a condition key. // For s3:ExistingObjectTag/ conditions, it looks up the tag in objectEntry. // For other condition keys, it looks up the value in contextValues. diff --git a/weed/s3api/policy_engine/engine.go b/weed/s3api/policy_engine/engine.go index 933ed1d96c..0275f33627 100644 --- a/weed/s3api/policy_engine/engine.go +++ b/weed/s3api/policy_engine/engine.go @@ -3,6 +3,7 @@ package policy_engine import ( "fmt" "net/http" + "net/url" "regexp" "strings" "sync" @@ -10,6 +11,7 @@ import ( "time" "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" ) // PolicyEvaluationResult represents the result of policy evaluation @@ -439,6 +441,10 @@ func (engine *PolicyEngine) ExtractConditionValuesFromRequest(r *http.Request) m // Note: s3:ExistingObjectTag/ conditions are evaluated using objectEntry // passed to EvaluatePolicy, not extracted from the request. + for key, value := range requestObjectTags(r) { + values[RequestObjectTagPrefix+key] = []string{value} + } + // S3 bucket-level conditions if delimiter := r.URL.Query().Get("delimiter"); delimiter != "" { values["s3:delimiter"] = []string{delimiter} @@ -496,6 +502,86 @@ func (engine *PolicyEngine) ExtractConditionValuesFromRequest(r *http.Request) m return values } +// requestObjectTags returns the tags that X-Amz-Tagging will store on the +// object, or nil when the handler ignores the header — otherwise a tag +// attached to a read or to a write that never stores it would satisfy a +// tag-conditioned policy the object cannot meet. +func requestObjectTags(r *http.Request) map[string]string { + tagging := r.Header.Get(s3_constants.AmzObjectTagging) + if tagging == "" { + return nil + } + if !storesRequestObjectTags(r) { + return nil + } + // A malformed header is rejected by the handler with InvalidTag. + parsed, err := url.ParseQuery(tagging) + if err != nil { + return nil + } + // putToFiler rejects a repeated key, but CopyObject stores its last + // value, so that is the one the policy has to see. + tags := make(map[string]string, len(parsed)) + for key, vals := range parsed { + tags[key] = vals[len(vals)-1] + } + return tags +} + +// storesRequestObjectTags reports whether the request's handler saves the +// X-Amz-Tagging header on the object: PutObject, a copy under +// X-Amz-Tagging-Directive: REPLACE, and CreateMultipartUpload do; every +// other operation — reads, deletes, part uploads, completion, tagging +// subresources — does not. +func storesRequestObjectTags(r *http.Request) bool { + query := r.URL.Query() + if r.Method == http.MethodPost { + return query.Has("uploads") && !query.Has("uploadId") + } + if r.Method != http.MethodPut { + return false + } + // A numeric partNumber together with uploadId routes to the part + // handlers — with or without a copy source — which never read + // X-Amz-Tagging. + if query.Has("partNumber") && query.Has("uploadId") && isDigits(query.Get("partNumber")) { + return false + } + // The PUT subresources route before CopyObject and never read + // X-Amz-Tagging; any other query key — presign parameters included — + // still reaches the object PUT handlers. + for _, subresource := range []string{"tagging", "renameObject", "acl", "retention", "legal-hold"} { + if query.Has(subresource) { + return false + } + } + if r.Header.Get("X-Amz-Copy-Source") != "" { + return r.Header.Get(s3_constants.AmzObjectTaggingDirective) == "REPLACE" + } + if query.Has("partNumber") || query.Has("uploadId") { + return false + } + // An explicit directory-marker PUT stores no tags. + if strings.HasSuffix(r.URL.Path, "/") && r.ContentLength <= 1024 { + return false + } + return true +} + +// isDigits reports whether s is non-empty and all ASCII digits, matching the +// partNumber route constraint. +func isDigits(s string) bool { + if s == "" { + return false + } + for _, c := range s { + if c < '0' || c > '9' { + return false + } + } + return true +} + // IsMultipartContinuationAction returns true for actions that do not carry // their own SSE header because SSE is inherited from CreateMultipartUpload. func IsMultipartContinuationAction(action string) bool { diff --git a/weed/s3api/policy_engine/engine_test.go b/weed/s3api/policy_engine/engine_test.go index d527e54bb3..587ffdc115 100644 --- a/weed/s3api/policy_engine/engine_test.go +++ b/weed/s3api/policy_engine/engine_test.go @@ -896,6 +896,161 @@ func TestExistingObjectTagCondition(t *testing.T) { } } +func TestRequestObjectTagCondition(t *testing.T) { + engine := NewPolicyEngine() + + policyJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:PutObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": { + "StringEquals": { + "s3:RequestObjectTag/department": ["engineering"] + } + } + } + ] + }` + + if err := engine.SetBucketPolicy("test-bucket", policyJSON); err != nil { + t.Fatalf("Failed to set bucket policy: %v", err) + } + + copyHeaders := map[string]string{"X-Amz-Copy-Source": "/src-bucket/src-object"} + replaceHeaders := map[string]string{ + "X-Amz-Copy-Source": "/src-bucket/src-object", + s3_constants.AmzObjectTaggingDirective: "REPLACE", + } + + tests := []struct { + name string + method string + target string + headers map[string]string + tagging string + expected PolicyEvaluationResult + }{ + {"matching tag allows", "", "", nil, "department=engineering", PolicyResultAllow}, + {"matching tag among others allows", "", "", nil, "project=alpha&department=engineering", PolicyResultAllow}, + {"url-encoded value is decoded", "", "", nil, "department=engin%65ering", PolicyResultAllow}, + {"other value is not allowed", "", "", nil, "department=sales", PolicyResultIndeterminate}, + {"missing tag is not allowed", "", "", nil, "project=alpha", PolicyResultIndeterminate}, + {"no tagging header is not allowed", "", "", nil, "", PolicyResultIndeterminate}, + {"malformed header is not allowed", "", "", nil, "department=engineering&%zz", PolicyResultIndeterminate}, + {"last value of a repeated key counts", "", "", nil, "department=sales&department=engineering", PolicyResultAllow}, + {"earlier value of a repeated key is ignored", "", "", nil, "department=engineering&department=sales", PolicyResultIndeterminate}, + {"header is ignored on PutObjectTagging", "", "?tagging", nil, "department=engineering", PolicyResultIndeterminate}, + {"header counts with an unrelated query parameter", "", "?foo=bar", nil, "department=engineering", PolicyResultAllow}, + {"header counts on a Signature V2 presigned PUT", "", "?AWSAccessKeyId=k&Signature=s&Expires=1", nil, "department=engineering", PolicyResultAllow}, + {"header is ignored on a renameObject subresource", "", "?renameObject", nil, "department=engineering", PolicyResultIndeterminate}, + {"header is ignored on a copy that keeps source tags", "", "", copyHeaders, "department=engineering", PolicyResultIndeterminate}, + {"header counts on a copy that replaces tags", "", "", replaceHeaders, "department=engineering", PolicyResultAllow}, + {"header is ignored on a multipart copy that replaces tags", "", "?partNumber=1&uploadId=x", replaceHeaders, "department=engineering", PolicyResultIndeterminate}, + {"header counts on a copy with a lone partNumber", "", "?partNumber=1", replaceHeaders, "department=engineering", PolicyResultAllow}, + {"header counts on a multipart create", "POST", "?uploads", nil, "department=engineering", PolicyResultAllow}, + {"header is ignored on a part upload", "", "?partNumber=1&uploadId=x", nil, "department=engineering", PolicyResultIndeterminate}, + {"header is ignored on multipart completion", "POST", "?uploadId=x", nil, "department=engineering", PolicyResultIndeterminate}, + {"header is ignored on a directory marker", "", "/", nil, "department=engineering", PolicyResultIndeterminate}, + {"header is ignored on an acl subresource", "", "?acl", nil, "department=engineering", PolicyResultIndeterminate}, + {"header is ignored on a GET", "GET", "", nil, "department=engineering", PolicyResultIndeterminate}, + {"header is ignored on a DELETE", "DELETE", "", nil, "department=engineering", PolicyResultIndeterminate}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + method := tt.method + if method == "" { + method = http.MethodPut + } + req := httptest.NewRequest(method, "/test-bucket/test-object"+tt.target, nil) + for k, v := range tt.headers { + req.Header.Set(k, v) + } + if tt.tagging != "" { + req.Header.Set(s3_constants.AmzObjectTagging, tt.tagging) + } + + args := &PolicyEvaluationArgs{ + Action: "s3:PutObject", + Resource: "arn:aws:s3:::test-bucket/test-object", + Principal: "*", + Conditions: engine.ExtractConditionValuesFromRequest(req), + } + + if result := engine.EvaluatePolicy("test-bucket", args); result != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, result) + } + }) + } +} + +func TestRequestObjectTagConditionDeny(t *testing.T) { + engine := NewPolicyEngine() + + policyJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:PutObject", + "Resource": "arn:aws:s3:::test-bucket/*" + }, + { + "Effect": "Deny", + "Principal": "*", + "Action": "s3:PutObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": { + "StringNotEquals": { + "s3:RequestObjectTag/department": ["engineering"] + } + } + } + ] + }` + + if err := engine.SetBucketPolicy("test-bucket", policyJSON); err != nil { + t.Fatalf("Failed to set bucket policy: %v", err) + } + + tests := []struct { + name string + target string + tagging string + expected PolicyEvaluationResult + }{ + {"required tag passes", "", "department=engineering", PolicyResultAllow}, + {"other value is denied", "", "department=sales", PolicyResultDeny}, + {"missing tag is denied", "", "project=alpha", PolicyResultDeny}, + {"repeated key ending in another value is denied", "", "department=engineering&department=sales", PolicyResultDeny}, + {"malformed header is denied", "", "department=engineering&%zz", PolicyResultDeny}, + {"unrelated query parameter does not hide the tag", "?foo=bar", "department=sales", PolicyResultDeny}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPut, "/test-bucket/test-object"+tt.target, nil) + req.Header.Set(s3_constants.AmzObjectTagging, tt.tagging) + + args := &PolicyEvaluationArgs{ + Action: "s3:PutObject", + Resource: "arn:aws:s3:::test-bucket/test-object", + Principal: "*", + Conditions: engine.ExtractConditionValuesFromRequest(req), + } + + if result := engine.EvaluatePolicy("test-bucket", args); result != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, result) + } + }) + } +} + // TestExistingObjectTagConditionMultipleTags tests policies with multiple tag conditions func TestExistingObjectTagConditionMultipleTags(t *testing.T) { engine := NewPolicyEngine()