Files
seaweedfs/weed/s3api/policy_engine/request_grant_conditions_test.go
T
zhao-ycandChris Lu 483dd4b12e s3api: persist ACLs on PutObject uploads (#11592)
* s3api: persist ACLs on PutObject uploads

Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com>

* s3api: fix PutObject ACL edge cases found in review

- Only enforce BucketOwnerEnforced when explicitly configured; buckets
  without a stored ownership control keep accepting upload ACLs
- Ignore ACL query parameters on SigV2 requests, which do not sign them
- Mirror signed-query ACL values into headers after authentication so
  grant parsing and resolveFileMode agree on presigned uploads
- Validate only caller-supplied grantees against the account registry;
  default grants now work for accounts outside the local registry
- Reject unknown grantee keys and accept comma-separated grantee lists
  without spaces in ParseCustomAclHeader
- Guard against identities without an account

* s3api: harden upload ACL parsing and authorization

Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com>

* s3api: evaluate upload ACL grantees individually in policies

A comma-joined grant header or a signed query parameter reached policy
conditions as one value, so a deny on a later grantee did not fire. Split
grant headers into per-grantee values for policy evaluation and share the
grantee pair parser with ParseCustomAclHeader.

* s3api: keep raw grant header values visible to policy conditions

Exact-match conditions written against the signed header value stopped
matching once grantees were split for evaluation. Preserve the original
wire values alongside the per-grantee values so deny policies fire on
either granularity.

* s3api: evaluate upload ACL grants as one canonical list in policies

Conditions on s3:x-amz-grant-* now see a single comma-separated canonical
grant list identical for a single line, repeated header lines, or a signed
query parameter. This keeps StringEquals allows and exact-list or
allowlist (StringNotEquals) denies accurate regardless of wire encoding.

* s3api: preserve upload ACL denies and align policy checks

Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com>

* s3api: retain upload owner grants and literal policy values

Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com>

---------

Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
2026-10-05 09:13:19 +08:00

64 lines
3.7 KiB
Go

package policy_engine
import (
"fmt"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/require"
)
// TestOriginalGrantConditionsDeny verifies original positive denies without expanding allows or negative conditions.
func TestOriginalGrantConditionsDeny(t *testing.T) {
const key = "s3:x-amz-grant-read"
const canonical, original = `id="bucket-owner"`, `id = "bucket-\u006fwner"`
tests := []struct {
name, effect, conditions string
result PolicyEvaluationResult
}{
{"original exact deny", "Deny", fmt.Sprintf(`{"StringEquals":{%q:%q}}`, key, original), PolicyResultDeny},
{"original wildcard deny", "Deny", fmt.Sprintf(`{"StringLike":{%q:%q}}`, key, `id = *`), PolicyResultDeny},
{"canonical deny unchanged", "Deny", fmt.Sprintf(`{"StringEquals":{%q:%q}}`, key, canonical), PolicyResultDeny},
{"approved negative condition not denied", "Deny", fmt.Sprintf(`{"StringNotEquals":{%q:%q}}`, key, canonical), PolicyResultIndeterminate},
{"negative deny unchanged", "Deny", fmt.Sprintf(`{"StringNotEquals":{%q:%q}}`, key, `id="other"`), PolicyResultDeny},
{"original value cannot allow", "Allow", fmt.Sprintf(`{"StringEquals":{%q:%q}}`, key, original), PolicyResultIndeterminate},
{"negative allow not expanded", "Allow", fmt.Sprintf(`{"StringNotEquals":{%q:%q}}`, key, canonical), PolicyResultIndeterminate},
{"complete list allow unchanged", "Allow", fmt.Sprintf(`{"StringEquals":{%q:%q}}`, key, canonical), PolicyResultAllow},
{"positive and negative conditions evaluated separately", "Deny", fmt.Sprintf(`{"StringEquals":{%q:%q},"StringNotEquals":{%q:%q}}`, key, original, key, canonical), PolicyResultIndeterminate},
{"other conditions must still match", "Deny", fmt.Sprintf(`{"StringEquals":{%q:%q,"aws:username":"other"}}`, key, original), PolicyResultIndeterminate},
{"variables remain canonical", "Deny", fmt.Sprintf(`{"StringEquals":{%q:"${s3:x-amz-grant-read}"}}`, key), PolicyResultDeny},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
engine := NewPolicyEngine()
policy := fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":%q,"Principal":"*","Action":"s3:PutObjectAcl","Resource":"arn:aws:s3:::bucket/object","Condition":%s}]}`, tt.effect, tt.conditions)
require.NoError(t, engine.SetBucketPolicy("bucket", policy))
req := WithOriginalGrantConditions(httptest.NewRequest("PUT", "/bucket/object", nil), map[string][]string{key: {original}})
args := &PolicyEvaluationArgs{
Action: "s3:PutObjectAcl", Resource: "arn:aws:s3:::bucket/object", Principal: "upload-writer",
Conditions: map[string][]string{key: {canonical}, "aws:username": {"upload-writer"}},
OriginalGrantConditions: OriginalGrantConditionsFromRequest(req),
}
require.Equal(t, tt.result, engine.EvaluatePolicy("bucket", args))
})
}
}
// TestOriginalGrantConditionsSnapshot verifies original values cannot be forged through headers or later mutations.
func TestOriginalGrantConditionsSnapshot(t *testing.T) {
const key = "s3:x-amz-grant-read"
values := map[string][]string{key: {`id = "bucket-owner"`}, "aws:username": {"forged"}}
original := httptest.NewRequest("PUT", "/bucket/object", nil)
original.Header.Set("X-Amz-Original-Grant-Read", `id="attacker"`)
require.Nil(t, OriginalGrantConditionsFromRequest(original))
require.Nil(t, OriginalGrantConditionsFromRequest(nil))
req := WithOriginalGrantConditions(original, values)
values[key][0] = `id="attacker"`
delete(values, key)
first := OriginalGrantConditionsFromRequest(req)
require.Equal(t, `id = "bucket-owner"`, first[key][0])
require.NotContains(t, first, "aws:username")
first[key][0] = `id="attacker"`
require.Equal(t, `id = "bucket-owner"`, OriginalGrantConditionsFromRequest(req)[key][0])
}