diff --git a/weed/s3api/policy_engine/README_POLICY_ENGINE.md b/weed/s3api/policy_engine/README_POLICY_ENGINE.md index 97c4344e4a..78e1df5545 100644 --- a/weed/s3api/policy_engine/README_POLICY_ENGINE.md +++ b/weed/s3api/policy_engine/README_POLICY_ENGINE.md @@ -125,6 +125,7 @@ Support for all AWS condition operators: - **Boolean**: `Bool` - **ARN**: `ArnEquals`, `ArnLike` - **Null**: `Null` +- **Set qualifiers**: `ForAllValues:` and `ForAnyValue:` before any of the above, for multi-valued keys such as `s3:RequestObjectTagKeys` ### 4. Condition Keys @@ -357,6 +358,7 @@ The policy engine provides a seamless upgrade path from SeaweedFS's existing sim | `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 | +| `s3:RequestObjectTagKeys` | The set of tag keys a PUT request sets | | Wildcard Patterns | Support for `*` and `?` in actions and resources | | Principal Matching | `*`, account IDs, and user ARNs | @@ -364,7 +366,6 @@ The policy engine provides a seamless upgrade path from SeaweedFS's existing sim | Feature | GitHub Issue | |---------|--------------| -| `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 | | `s3:x-amz-storage-class` | Storage class condition | diff --git a/weed/s3api/policy_engine/conditions.go b/weed/s3api/policy_engine/conditions.go index a995c5ddb9..bfb7bf0b94 100644 --- a/weed/s3api/policy_engine/conditions.go +++ b/weed/s3api/policy_engine/conditions.go @@ -669,8 +669,58 @@ func (e *NullEvaluator) Evaluate(conditionValue interface{}, contextValues []str return false } -// GetConditionEvaluator returns the appropriate evaluator for a condition operator +// forAllValuesEvaluator matches when every request value satisfies the base +// operator on its own, and so also when the request carries no value. +type forAllValuesEvaluator struct { + base ConditionEvaluator +} + +func (e *forAllValuesEvaluator) Evaluate(conditionValue interface{}, contextValues []string) bool { + for _, v := range contextValues { + if !e.base.Evaluate(conditionValue, []string{v}) { + return false + } + } + return true +} + +// forAnyValueEvaluator matches when at least one request value satisfies the +// base operator on its own. Checking values one at a time keeps a negated +// base such as StringNotEquals from meaning "none of them". +type forAnyValueEvaluator struct { + base ConditionEvaluator +} + +func (e *forAnyValueEvaluator) Evaluate(conditionValue interface{}, contextValues []string) bool { + for _, v := range contextValues { + if e.base.Evaluate(conditionValue, []string{v}) { + return true + } + } + return false +} + +// GetConditionEvaluator returns the appropriate evaluator for a condition +// operator, including the ForAllValues: and ForAnyValue: set qualifiers. func GetConditionEvaluator(operator string) (ConditionEvaluator, error) { + if base, ok := strings.CutPrefix(operator, "ForAllValues:"); ok { + evaluator, err := baseConditionEvaluator(base) + if err != nil { + return nil, err + } + return &forAllValuesEvaluator{base: evaluator}, nil + } + if base, ok := strings.CutPrefix(operator, "ForAnyValue:"); ok { + evaluator, err := baseConditionEvaluator(base) + if err != nil { + return nil, err + } + return &forAnyValueEvaluator{base: evaluator}, nil + } + return baseConditionEvaluator(operator) +} + +func baseConditionEvaluator(operator string) (ConditionEvaluator, error) { switch operator { case "StringEquals": return &StringEqualsEvaluator{}, nil @@ -732,6 +782,10 @@ const ExistingObjectTagPrefix = "s3:ExistingObjectTag/" // request sets through the X-Amz-Tagging header. const RequestObjectTagPrefix = "s3:RequestObjectTag/" +// RequestObjectTagKeys is the multi-valued condition key listing the keys of +// the tags a request sets through the X-Amz-Tagging header. +const RequestObjectTagKeys = "s3:RequestObjectTagKeys" + // 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. @@ -780,7 +834,8 @@ func evaluateConditions(conditions PolicyConditions, contextValues map[string][] for key, value := range conditionMap { contextVals := getConditionContextValue(key, contextValues, objectEntry) if original := originalGrants[key]; len(original) != 0 && isGrantConditionKey(key) { - switch operator { + // Extra values can only make ForAllValues fail, so it keeps the canonical list. + switch strings.TrimPrefix(operator, "ForAnyValue:") { case "StringEquals", "StringEqualsIgnoreCase", "StringLike", "ArnEquals", "ArnLike": contextVals = append(append([]string(nil), contextVals...), original...) } diff --git a/weed/s3api/policy_engine/conditions_set_test.go b/weed/s3api/policy_engine/conditions_set_test.go new file mode 100644 index 0000000000..80aac29a0f --- /dev/null +++ b/weed/s3api/policy_engine/conditions_set_test.go @@ -0,0 +1,102 @@ +package policy_engine + +import ( + "testing" +) + +func TestConditionSetOperators(t *testing.T) { + allowed := []string{"department", "project"} + tests := []struct { + name string + operator string + conditionValue interface{} + contextValues []string + expected bool + }{ + {"ForAllValues all match", "ForAllValues:StringEquals", allowed, []string{"department", "project"}, true}, + {"ForAllValues subset matches", "ForAllValues:StringEquals", allowed, []string{"project"}, true}, + {"ForAllValues one outsider fails", "ForAllValues:StringEquals", allowed, []string{"department", "owner"}, false}, + {"ForAllValues empty set matches", "ForAllValues:StringEquals", allowed, []string{}, true}, + {"ForAllValues nil set matches", "ForAllValues:StringEquals", allowed, nil, true}, + {"ForAllValues is case sensitive", "ForAllValues:StringEquals", allowed, []string{"Department"}, false}, + {"ForAllValues StringLike all match", "ForAllValues:StringLike", "proj-*", []string{"proj-a", "proj-b"}, true}, + {"ForAllValues StringLike one outsider fails", "ForAllValues:StringLike", "proj-*", []string{"proj-a", "team"}, false}, + {"ForAllValues StringNotEquals none listed", "ForAllValues:StringNotEquals", allowed, []string{"owner", "team"}, true}, + {"ForAllValues StringNotEquals one listed fails", "ForAllValues:StringNotEquals", allowed, []string{"owner", "project"}, false}, + + {"ForAnyValue one match", "ForAnyValue:StringEquals", allowed, []string{"owner", "project"}, true}, + {"ForAnyValue no match", "ForAnyValue:StringEquals", allowed, []string{"owner", "team"}, false}, + {"ForAnyValue empty set fails", "ForAnyValue:StringEquals", allowed, []string{}, false}, + {"ForAnyValue nil set fails", "ForAnyValue:StringEquals", allowed, nil, false}, + {"ForAnyValue StringLike one match", "ForAnyValue:StringLike", []string{"secret*"}, []string{"owner", "secret-level"}, true}, + {"ForAnyValue StringLike no match", "ForAnyValue:StringLike", []string{"secret*"}, []string{"owner"}, false}, + {"ForAnyValue StringNotEquals one unlisted", "ForAnyValue:StringNotEquals", allowed, []string{"department", "owner"}, true}, + {"ForAnyValue StringNotEquals all listed fails", "ForAnyValue:StringNotEquals", allowed, []string{"department", "project"}, false}, + {"ForAnyValue StringEqualsIgnoreCase", "ForAnyValue:StringEqualsIgnoreCase", allowed, []string{"PROJECT"}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + evaluator, err := GetConditionEvaluator(tt.operator) + if err != nil { + t.Fatalf("Failed to get condition evaluator: %v", err) + } + if got := evaluator.Evaluate(tt.conditionValue, tt.contextValues); got != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, got) + } + }) + } +} + +func TestGetConditionEvaluatorRejectsUnknownSetOperators(t *testing.T) { + for _, op := range []string{ + "ForAllValues:StringBogus", + "ForAnyValue:", + "ForEachValue:StringEquals", + "ForAllValues:ForAnyValue:StringEquals", + } { + if _, err := GetConditionEvaluator(op); err == nil { + t.Errorf("GetConditionEvaluator accepted %q; expected error", op) + } + } +} + +func TestValidateBucketPolicyAcceptsSetOperators(t *testing.T) { + policy := `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Deny", + "Principal": "*", + "Action": "s3:PutObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": { + "ForAllValues:StringNotEquals": {"s3:RequestObjectTagKeys": ["department", "project"]}, + "ForAnyValue:StringLike": {"s3:RequestObjectTagKeys": ["secret*"]} + } + }] + }` + engine := NewPolicyEngine() + if err := engine.SetBucketPolicy("test-bucket", policy); err != nil { + t.Fatalf("SetBucketPolicy rejected set operators: %v", err) + } +} + +func TestOriginalGrantsSupplementOnlyForAnyValue(t *testing.T) { + const key = "s3:x-amz-grant-read" + contextValues := map[string][]string{key: {"id=canonical"}} + originalGrants := map[string][]string{key: {"uri=original"}} + + anyValue := PolicyConditions{ + "ForAnyValue:StringEquals": {key: NewStringOrStringSlice("uri=original")}, + } + if !evaluateConditions(anyValue, contextValues, nil, nil, originalGrants) { + t.Error("ForAnyValue should see the original grant values") + } + + allValues := PolicyConditions{ + "ForAllValues:StringEquals": {key: NewStringOrStringSlice("id=canonical")}, + } + if !evaluateConditions(allValues, contextValues, nil, nil, originalGrants) { + t.Error("ForAllValues should be evaluated on the canonical values only") + } +} diff --git a/weed/s3api/policy_engine/engine.go b/weed/s3api/policy_engine/engine.go index 0275f33627..e544b227bd 100644 --- a/weed/s3api/policy_engine/engine.go +++ b/weed/s3api/policy_engine/engine.go @@ -2,9 +2,11 @@ package policy_engine import ( "fmt" + "maps" "net/http" "net/url" "regexp" + "slices" "strings" "sync" "sync/atomic" @@ -441,8 +443,11 @@ 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} + if tags := requestObjectTags(r); len(tags) > 0 { + for key, value := range tags { + values[RequestObjectTagPrefix+key] = []string{value} + } + values[RequestObjectTagKeys] = slices.Sorted(maps.Keys(tags)) } // S3 bucket-level conditions @@ -514,16 +519,24 @@ func requestObjectTags(r *http.Request) map[string]string { 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] + // Parse the same non-empty segments the tag handlers store: url.ParseQuery + // caps at 10,000 parameters, so a header padded with "&" segments would + // hide stored tag keys from every s3:RequestObjectTag* condition. A + // malformed pair is rejected by the handler with InvalidTag. + tags := make(map[string]string) + for _, segment := range strings.Split(tagging, "&") { + if segment == "" { + continue + } + rawKey, rawValue, _ := strings.Cut(segment, "=") + key, errKey := url.QueryUnescape(rawKey) + value, errValue := url.QueryUnescape(rawValue) + if errKey != nil || errValue != 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[key] = value } return tags } diff --git a/weed/s3api/policy_engine/engine_test.go b/weed/s3api/policy_engine/engine_test.go index 587ffdc115..6f4cccc687 100644 --- a/weed/s3api/policy_engine/engine_test.go +++ b/weed/s3api/policy_engine/engine_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" @@ -1051,6 +1052,158 @@ func TestRequestObjectTagConditionDeny(t *testing.T) { } } +func evaluateTaggedRequest(engine *PolicyEngine, method, target string, headers map[string]string, tagging string) PolicyEvaluationResult { + if method == "" { + method = http.MethodPut + } + req := httptest.NewRequest(method, "/test-bucket/test-object"+target, nil) + for k, v := range headers { + req.Header.Set(k, v) + } + if tagging != "" { + req.Header.Set(s3_constants.AmzObjectTagging, tagging) + } + return engine.EvaluatePolicy("test-bucket", &PolicyEvaluationArgs{ + Action: "s3:PutObject", + Resource: "arn:aws:s3:::test-bucket/test-object", + Principal: "*", + Conditions: engine.ExtractConditionValuesFromRequest(req), + }) +} + +func TestRequestObjectTagKeysCondition(t *testing.T) { + engine := NewPolicyEngine() + + policyJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:PutObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": { + "ForAllValues:StringEquals": { + "s3:RequestObjectTagKeys": ["department", "project"] + }, + "Null": { + "s3:RequestObjectTagKeys": "false" + } + } + }, + { + "Effect": "Deny", + "Principal": "*", + "Action": "s3:PutObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": { + "ForAnyValue:StringLike": { + "s3:RequestObjectTagKeys": ["secret*"] + } + } + } + ] + }` + + 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 + }{ + {"listed key allows", "", "", nil, "department=engineering", PolicyResultAllow}, + {"all listed keys allow", "", "", nil, "department=engineering&project=alpha", PolicyResultAllow}, + {"unlisted key is not allowed", "", "", nil, "department=engineering&owner=bob", PolicyResultIndeterminate}, + {"no tagging header is not allowed", "", "", nil, "", PolicyResultIndeterminate}, + {"repeated key counts once", "", "", nil, "department=a&department=b", PolicyResultAllow}, + {"url-encoded key is decoded", "", "", nil, "depart%6Dent=engineering", PolicyResultAllow}, + {"empty segments add no key", "", "", nil, "&department=engineering&", PolicyResultAllow}, + {"empty key is not allowed", "", "", nil, "department=engineering&=x", PolicyResultIndeterminate}, + {"key case is preserved", "", "", nil, "Department=engineering", PolicyResultIndeterminate}, + {"malformed header is not allowed", "", "", nil, "department=engineering&%zz", PolicyResultIndeterminate}, + {"secret key is denied", "", "", nil, "secret=1", PolicyResultDeny}, + {"secret key behind a padded header is denied", "", "", nil, "secret=1" + strings.Repeat("&", 10001), PolicyResultDeny}, + {"secret key among listed keys is denied", "", "", nil, "department=engineering&secret-level=2", PolicyResultDeny}, + {"url-encoded secret key is denied", "", "", nil, "secr%65t=1", PolicyResultDeny}, + {"secret key on a copy that replaces tags is denied", "", "", replaceHeaders, "secret=1", PolicyResultDeny}, + {"secret key on a multipart create is denied", "POST", "?uploads", nil, "secret=1", PolicyResultDeny}, + {"header is ignored on a copy that keeps source tags", "", "", copyHeaders, "secret=1", PolicyResultIndeterminate}, + {"header is ignored on PutObjectTagging", "", "?tagging", nil, "department=engineering", PolicyResultIndeterminate}, + {"header is ignored on a part upload", "", "?partNumber=1&uploadId=x", nil, "secret=1", PolicyResultIndeterminate}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if result := evaluateTaggedRequest(engine, tt.method, tt.target, tt.headers, tt.tagging); result != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, result) + } + }) + } +} + +func TestRequestObjectTagKeysRequiredTagDeny(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": { + "ForAllValues:StringNotEquals": { + "s3:RequestObjectTagKeys": ["department", "project"] + } + } + } + ] + }` + + if err := engine.SetBucketPolicy("test-bucket", policyJSON); err != nil { + t.Fatalf("Failed to set bucket policy: %v", err) + } + + tests := []struct { + name string + tagging string + expected PolicyEvaluationResult + }{ + {"required key passes", "department=engineering", PolicyResultAllow}, + {"required key with another key passes", "project=alpha&owner=bob", PolicyResultAllow}, + {"only other keys are denied", "owner=bob", PolicyResultDeny}, + {"no tagging header is denied", "", PolicyResultDeny}, + {"malformed header is denied", "department=engineering&%zz", PolicyResultDeny}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if result := evaluateTaggedRequest(engine, "", "", nil, tt.tagging); 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() diff --git a/weed/s3api/tags.go b/weed/s3api/tags.go index 7b1654529d..76830ccf14 100644 --- a/weed/s3api/tags.go +++ b/weed/s3api/tags.go @@ -53,6 +53,11 @@ func FromTags(tags map[string]string) (t *Tagging) { func parseTagsHeader(tags string) (map[string]string, error) { parsedTags := make(map[string]string) for _, v := range util.StringSplit(tags, "&") { + // url.ParseQuery skips empty segments too, so a copy stores the + // same tag keys that PutObject and bucket policies see. + if v == "" { + continue + } key, value, hasValue := strings.Cut(v, "=") decodedKey, err := url.QueryUnescape(key) if err != nil { diff --git a/weed/s3api/tags_test.go b/weed/s3api/tags_test.go index 2002d65f28..a7dd1b806e 100644 --- a/weed/s3api/tags_test.go +++ b/weed/s3api/tags_test.go @@ -71,6 +71,15 @@ func TestParseTagsHeader(t *testing.T) { }, expectError: false, }, + { + name: "empty segments add no tag", + input: "&key1=value1&&key2=value2&", + expected: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectError: false, + }, { name: "unescaped equals in value", input: "token=part1=part2&normal=test",