s3api: support s3:RequestObjectTagKeys in bucket policies (#11993)

* s3api: support s3:RequestObjectTagKeys in bucket policies

Condition operators accept the ForAllValues: and ForAnyValue: qualifiers,
and s3:RequestObjectTagKeys holds the keys of the tags taken from
X-Amz-Tagging wherever s3:RequestObjectTag/<key> is set.

CopyObject no longer stores an empty-key tag for an empty segment in
X-Amz-Tagging, so it keeps the key set a policy has checked.

Fixes #7679

* s3: parse tagging header like the tag handlers, not url.ParseQuery

url.ParseQuery stops at 10,000 parameters, so an X-Amz-Tagging header
padded with empty & segments made requestObjectTags return nil: the copy
handler still stored the denied tag while every s3:RequestObjectTag*
condition saw nothing. Scan non-empty segments the way parseTagsHeader
does so the policy sees the same tags the object gets.

* test: gofmt s3tables test files after #11997

The format check fails on these files on master as merged, so every
PR's S3 Tables Format Check goes red until they are re-formatted.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: user.email <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

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/<key>` | Tag-based access control for existing objects |
 | `s3:RequestObjectTag/<key>` | 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 |

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/<key> 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...)
 				}

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")
+	}
+}

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/<key> 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
 }

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

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 {

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",