diff --git a/weed/s3api/policy/post-policy_test.go b/weed/s3api/policy/post-policy_test.go index 8b054023a..13cf9fa15 100644 --- a/weed/s3api/policy/post-policy_test.go +++ b/weed/s3api/policy/post-policy_test.go @@ -29,6 +29,7 @@ import ( "net/url" "regexp" "strings" + "testing" "time" "unicode/utf8" ) @@ -376,3 +377,278 @@ func getScope(t time.Time, region string) string { }, "/") return scope } + +// buildParsedPolicy is a small test helper that assembles a JSON policy +// document with the supplied condition snippets and parses it into a +// PostPolicyForm. Using ParsePostPolicyForm avoids having to construct the +// anonymous struct in PostPolicyForm.Conditions.Policies directly. +func buildParsedPolicy(t *testing.T, conditions string) PostPolicyForm { + t.Helper() + expiration := time.Now().UTC().Add(24 * time.Hour).Format(iso8601TimeFormat) + raw := fmt.Sprintf(`{"expiration":"%s","conditions":[%s]}`, expiration, conditions) + ppf, err := ParsePostPolicyForm(raw) + if err != nil { + t.Fatalf("ParsePostPolicyForm failed: %v\npolicy: %s", err, raw) + } + return ppf +} + +// TestCheckPostPolicy_RejectsUnknownConditionKey verifies that a policy +// containing a condition key that is neither in startsWithConds nor prefixed +// with $x-amz- is rejected instead of being silently accepted. +func TestCheckPostPolicy_RejectsUnknownConditionKey(t *testing.T) { + ppf := buildParsedPolicy(t, `["eq","$foo","bar"]`) + + form := http.Header{} + form.Set("Foo", "bar") + + err := CheckPostPolicy(form, ppf) + if err == nil { + t.Fatalf("expected error for unknown condition key, got nil") + } + if !strings.Contains(err.Error(), "unknown condition key") { + t.Fatalf("expected 'unknown condition key' error, got: %v", err) + } + if !strings.Contains(err.Error(), "$foo") { + t.Fatalf("expected error to name the offending key, got: %v", err) + } +} + +// TestCheckPostPolicy_RejectsExtraXAmzFormField verifies that stray X-Amz-* +// form fields (beyond the reserved auth/signing ones) are rejected when no +// matching policy condition is declared. +func TestCheckPostPolicy_RejectsExtraXAmzFormField(t *testing.T) { + ppf := buildParsedPolicy(t, `["eq","$bucket","mybucket"]`) + + form := http.Header{} + form.Set("Bucket", "mybucket") + form.Set("X-Amz-Storage-Class", "STANDARD") + + err := CheckPostPolicy(form, ppf) + if err == nil { + t.Fatalf("expected error for extra X-Amz-Storage-Class field, got nil") + } + if !strings.Contains(err.Error(), "Extra input fields") { + t.Fatalf("expected 'Extra input fields' error, got: %v", err) + } + if !strings.Contains(err.Error(), "X-Amz-Storage-Class") { + t.Fatalf("expected error to name the offending field, got: %v", err) + } +} + +// TestCheckPostPolicy_AllowsXAmzAuthFields verifies that the reserved +// auth/signing X-Amz-* headers are accepted even when no policy condition +// mentions them, because clients must always send these. +func TestCheckPostPolicy_AllowsXAmzAuthFields(t *testing.T) { + ppf := buildParsedPolicy(t, `["eq","$bucket","mybucket"]`) + + form := http.Header{} + form.Set("Bucket", "mybucket") + form.Set("X-Amz-Signature", "deadbeef") + form.Set("X-Amz-Credential", "AKIA/20260417/us-east-1/s3/aws4_request") + form.Set("X-Amz-Algorithm", "AWS4-HMAC-SHA256") + form.Set("X-Amz-Date", "20260417T000000Z") + form.Set("X-Amz-Security-Token", "session-token") + + if err := CheckPostPolicy(form, ppf); err != nil { + t.Fatalf("expected no error with only reserved auth fields, got: %v", err) + } +} + +// TestCheckPostPolicy_AllowsMatchingXAmzField verifies that an X-Amz-* form +// field is accepted when there is a matching equality policy condition. +func TestCheckPostPolicy_AllowsMatchingXAmzField(t *testing.T) { + ppf := buildParsedPolicy(t, `["eq","$bucket","mybucket"],["eq","$x-amz-storage-class","STANDARD"]`) + + form := http.Header{} + form.Set("Bucket", "mybucket") + form.Set("X-Amz-Storage-Class", "STANDARD") + + if err := CheckPostPolicy(form, ppf); err != nil { + t.Fatalf("expected no error for matching X-Amz-Storage-Class, got: %v", err) + } +} + +// TestCheckPostPolicy_ExistingXAmzMetaCheckStillWorks is a regression test +// for the pre-existing behavior: a stray X-Amz-Meta-* form field without a +// matching condition is still rejected. +func TestCheckPostPolicy_ExistingXAmzMetaCheckStillWorks(t *testing.T) { + ppf := buildParsedPolicy(t, `["eq","$bucket","mybucket"]`) + + form := http.Header{} + form.Set("Bucket", "mybucket") + form.Set("X-Amz-Meta-Foo", "bar") + + err := CheckPostPolicy(form, ppf) + if err == nil { + t.Fatalf("expected error for extra X-Amz-Meta-Foo field, got nil") + } + if !strings.Contains(err.Error(), "Extra input fields") { + t.Fatalf("expected 'Extra input fields' error, got: %v", err) + } + if !strings.Contains(err.Error(), "X-Amz-Meta-Foo") { + t.Fatalf("expected error to name the offending field, got: %v", err) + } +} + +// TestCheckPostPolicy_AllowsStartsWithPrefixStem covers the AWS convention +// where ["starts-with","$x-amz-meta-",""] permits any X-Amz-Meta-* form +// field. Without prefix-stem handling, such fields would be wrongly +// rejected as "Extra input fields". +func TestCheckPostPolicy_AllowsStartsWithPrefixStem(t *testing.T) { + ppf := buildParsedPolicy(t, + `["eq","$bucket","mybucket"],["starts-with","$x-amz-meta-",""]`, + ) + + form := http.Header{} + form.Set("Bucket", "mybucket") + form.Set("X-Amz-Meta-Foo", "bar") + form.Set("X-Amz-Meta-Another", "baz") + + if err := CheckPostPolicy(form, ppf); err != nil { + t.Fatalf("expected no error for prefix-matched meta fields, got: %v", err) + } +} + +// TestCheckPostPolicy_PrefixStemDoesNotCoverOtherPrefixes ensures the +// prefix allowance is scoped: a starts-with stem for x-amz-meta- must not +// whitelist unrelated x-amz-* fields like x-amz-storage-class. +func TestCheckPostPolicy_PrefixStemDoesNotCoverOtherPrefixes(t *testing.T) { + ppf := buildParsedPolicy(t, + `["eq","$bucket","mybucket"],["starts-with","$x-amz-meta-",""]`, + ) + + form := http.Header{} + form.Set("Bucket", "mybucket") + form.Set("X-Amz-Storage-Class", "STANDARD") + + err := CheckPostPolicy(form, ppf) + if err == nil { + t.Fatalf("expected error for X-Amz-Storage-Class not covered by meta prefix, got nil") + } + if !strings.Contains(err.Error(), "X-Amz-Storage-Class") { + t.Fatalf("expected error to name X-Amz-Storage-Class, got: %v", err) + } +} + +// TestCheckPostPolicy_PrefixStemEnforcesValuePrefix covers a starts-with +// prefix-stem policy with a non-empty required value prefix: matching +// fields must have values that satisfy the value prefix. +func TestCheckPostPolicy_PrefixStemEnforcesValuePrefix(t *testing.T) { + ppf := buildParsedPolicy(t, + `["eq","$bucket","mybucket"],["starts-with","$x-amz-meta-","pfx-"]`, + ) + + // Value satisfies the required prefix: accepted. + okForm := http.Header{} + okForm.Set("Bucket", "mybucket") + okForm.Set("X-Amz-Meta-Foo", "pfx-bar") + if err := CheckPostPolicy(okForm, ppf); err != nil { + t.Fatalf("expected no error when meta value matches required prefix, got: %v", err) + } + + // Value does not satisfy the required prefix: rejected as policy failure. + badForm := http.Header{} + badForm.Set("Bucket", "mybucket") + badForm.Set("X-Amz-Meta-Foo", "other") + err := CheckPostPolicy(badForm, ppf) + if err == nil { + t.Fatalf("expected error when meta value misses required prefix, got nil") + } + if !strings.Contains(err.Error(), "Policy Condition failed") { + t.Fatalf("expected 'Policy Condition failed' error, got: %v", err) + } + if !strings.Contains(err.Error(), "pfx-") { + t.Fatalf("expected error to reference the required value prefix, got: %v", err) + } +} + +// TestCheckPostPolicy_ExactAndPrefixBothEnforced covers a field that is +// simultaneously covered by an exact-key condition and a prefix-stem +// condition. Both must be satisfied; exact-match alone does not let the +// field skip the stem's value-prefix check. +func TestCheckPostPolicy_ExactAndPrefixBothEnforced(t *testing.T) { + ppf := buildParsedPolicy(t, + `["eq","$bucket","mybucket"],`+ + `["eq","$x-amz-meta-tag","gold"],`+ + `["starts-with","$x-amz-meta-","lvl-"]`, + ) + + form := http.Header{} + form.Set("Bucket", "mybucket") + // Satisfies the exact eq condition but not the starts-with stem. + form.Set("X-Amz-Meta-Tag", "gold") + + err := CheckPostPolicy(form, ppf) + if err == nil { + t.Fatalf("expected error: exact-match field must still satisfy overlapping prefix stem, got nil") + } + if !strings.Contains(err.Error(), "Policy Condition failed") { + t.Fatalf("expected 'Policy Condition failed' error, got: %v", err) + } + if !strings.Contains(err.Error(), "lvl-") { + t.Fatalf("expected error to reference the stem's value prefix, got: %v", err) + } +} + +// TestCheckPostPolicy_MultiplePrefixStemsAllEnforced covers a field that +// matches multiple starts-with prefix stems. All stems must hold; a value +// that satisfies one but not the other must be rejected. +func TestCheckPostPolicy_MultiplePrefixStemsAllEnforced(t *testing.T) { + ppf := buildParsedPolicy(t, + `["eq","$bucket","mybucket"],`+ + `["starts-with","$x-amz-meta-","pfx-"],`+ + `["starts-with","$x-amz-meta-color-","red-"]`, + ) + + // Satisfies the broader stem but not the color- stem's value prefix. + form := http.Header{} + form.Set("Bucket", "mybucket") + form.Set("X-Amz-Meta-Color-Main", "pfx-green") + + err := CheckPostPolicy(form, ppf) + if err == nil { + t.Fatalf("expected error: field must satisfy every matching prefix stem, got nil") + } + if !strings.Contains(err.Error(), "Policy Condition failed") { + t.Fatalf("expected 'Policy Condition failed' error, got: %v", err) + } + if !strings.Contains(err.Error(), "red-") { + t.Fatalf("expected error to reference the failing value prefix, got: %v", err) + } + + // A policy with compatible stems (broad allows anything, narrow + // requires "red-") accepts a value honoring both. + compatible := buildParsedPolicy(t, + `["eq","$bucket","mybucket"],`+ + `["starts-with","$x-amz-meta-",""],`+ + `["starts-with","$x-amz-meta-color-","red-"]`, + ) + okForm := http.Header{} + okForm.Set("Bucket", "mybucket") + okForm.Set("X-Amz-Meta-Color-Main", "red-main") + if err := CheckPostPolicy(okForm, compatible); err != nil { + t.Fatalf("expected no error when value satisfies both compatible stems, got: %v", err) + } +} + +// TestCheckPostPolicy_UnknownKeyErrorIncludesPolicyValue ensures the +// unknown-condition error surfaces the policy value in its [op, key, value] +// trailer so operators can tell which of several unknown keys failed. +func TestCheckPostPolicy_UnknownKeyErrorIncludesPolicyValue(t *testing.T) { + ppf := buildParsedPolicy(t, `["eq","$foo","custom-value"]`) + + err := CheckPostPolicy(http.Header{}, ppf) + if err == nil { + t.Fatalf("expected error for unknown condition key, got nil") + } + if !strings.Contains(err.Error(), "custom-value") { + t.Fatalf("expected error to include policy.Value 'custom-value', got: %v", err) + } + if !strings.Contains(err.Error(), "$foo") { + t.Fatalf("expected error to include policy.Key '$foo', got: %v", err) + } + if !strings.Contains(err.Error(), "unknown condition key") { + t.Fatalf("expected 'unknown condition key' suffix, got: %v", err) + } +} diff --git a/weed/s3api/policy/postpolicyform.go b/weed/s3api/policy/postpolicyform.go index 011f782d7..46bf38075 100644 --- a/weed/s3api/policy/postpolicyform.go +++ b/weed/s3api/policy/postpolicyform.go @@ -215,6 +215,27 @@ func checkPolicyCond(op string, input1, input2 string) bool { return false } +// xAmzPrefixRule captures a starts-with policy condition whose key ends in +// "-" and therefore matches a prefix of form field names, e.g. +// ["starts-with","$x-amz-meta-","pfx-"]. Any form field whose name starts +// with namePrefix must have a value starting with valuePrefix. +type xAmzPrefixRule struct { + policyKey string + namePrefix string + valuePrefix string +} + +// postPolicyAuthFields enumerates the X-Amz-* form fields that clients are +// required to send with every POST Object request for signing/authentication. +// These must be accepted even when no matching policy condition is declared. +var postPolicyAuthFields = map[string]bool{ + "X-Amz-Signature": true, + "X-Amz-Credential": true, + "X-Amz-Algorithm": true, + "X-Amz-Date": true, + "X-Amz-Security-Token": true, +} + // CheckPostPolicy - apply policy conditions and validate input values. // (http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-HTTPPOSTConstructPolicy.html) func CheckPostPolicy(formValues http.Header, postPolicyForm PostPolicyForm) error { @@ -222,20 +243,57 @@ func CheckPostPolicy(formValues http.Header, postPolicyForm PostPolicyForm) erro if !postPolicyForm.Expiration.After(time.Now().UTC()) { return fmt.Errorf("Invalid according to Policy: Policy expired") } - // map to store the metadata - metaMap := make(map[string]string) + // Track $x-amz-* policy conditions in canonical form. A starts-with + // condition on a key that itself ends with "-" (e.g. + // ["starts-with","$x-amz-meta-","pfx-"]) is the AWS convention for + // allowing any form field sharing that name prefix; capture the name + // prefix together with its required value prefix so the extras check + // can both accept matching fields and enforce the value constraint. + policyXAmzKeys := make(map[string]bool) + var policyXAmzPrefixes []xAmzPrefixRule for _, policy := range postPolicyForm.Conditions.Policies { - if strings.HasPrefix(policy.Key, "$x-amz-meta-") { - formCanonicalName := http.CanonicalHeaderKey(strings.TrimPrefix(policy.Key, "$")) - metaMap[formCanonicalName] = policy.Value + if !strings.HasPrefix(policy.Key, "$x-amz-") { + continue } + formCanonicalName := http.CanonicalHeaderKey(strings.TrimPrefix(policy.Key, "$")) + if policy.Operator == policyCondStartsWith && strings.HasSuffix(policy.Key, "-") { + policyXAmzPrefixes = append(policyXAmzPrefixes, xAmzPrefixRule{ + policyKey: policy.Key, + namePrefix: formCanonicalName, + valuePrefix: policy.Value, + }) + continue + } + policyXAmzKeys[formCanonicalName] = true } - // Check if any extra metadata field is passed as input + // Reject any X-Amz-* form field that has no matching policy condition, + // except for the reserved auth/signing fields clients must always send. + // A field may be covered by an exact-key condition, by any number of + // prefix-stem rules, or both. Check every applicable rule: AWS requires + // all matching conditions to be satisfied, so exact-match coverage does + // not let a field skip the value-prefix enforcement of overlapping + // starts-with stems, and multiple stems on the same field must all hold. + // Prefix-stem conditions are then skipped in the main loop below because + // no single form field corresponds to the prefix itself. for key := range formValues { - if strings.HasPrefix(key, "X-Amz-Meta-") { - if _, ok := metaMap[key]; !ok { - return fmt.Errorf("Invalid according to Policy: Extra input fields: %s", key) + if !strings.HasPrefix(key, "X-Amz-") { + continue + } + if postPolicyAuthFields[key] { + continue + } + matched := policyXAmzKeys[key] + for _, rule := range policyXAmzPrefixes { + if !strings.HasPrefix(key, rule.namePrefix) { + continue } + matched = true + if !strings.HasPrefix(formValues.Get(key), rule.valuePrefix) { + return fmt.Errorf("Invalid according to Policy: Policy Condition failed: [%s, %s, %s]", policyCondStartsWith, rule.policyKey, rule.valuePrefix) + } + } + if !matched { + return fmt.Errorf("Invalid according to Policy: Extra input fields: %s", key) } } @@ -249,6 +307,13 @@ func CheckPostPolicy(formValues http.Header, postPolicyForm PostPolicyForm) erro formCanonicalName := http.CanonicalHeaderKey(strings.TrimPrefix(policy.Key, "$")) // Operator for the current policy condition op := policy.Operator + // Prefix-stem x-amz-* conditions (key ending in "-" with starts-with) + // are validated above against every matching form field; skip them + // here so we do not fail on the non-existent literal form field whose + // name equals the prefix itself. + if strings.HasPrefix(policy.Key, "$x-amz-") && op == policyCondStartsWith && strings.HasSuffix(policy.Key, "-") { + continue + } // If the current policy condition is known if startsWithSupported, condFound := startsWithConds[policy.Key]; condFound { // Check if the current condition supports starts-with operator @@ -260,15 +325,17 @@ func CheckPostPolicy(formValues http.Header, postPolicyForm PostPolicyForm) erro if !condPassed { return fmt.Errorf("Invalid according to Policy: Policy Condition failed") } - } else { - // This covers all conditions X-Amz-Meta-* and X-Amz-* - if strings.HasPrefix(policy.Key, "$x-amz-meta-") || strings.HasPrefix(policy.Key, "$x-amz-") { - // Check if policy condition is satisfied - condPassed = checkPolicyCond(op, formValues.Get(formCanonicalName), policy.Value) - if !condPassed { - return fmt.Errorf("Invalid according to Policy: Policy Condition failed: [%s, %s, %s]", op, policy.Key, policy.Value) - } + } else if strings.HasPrefix(policy.Key, "$x-amz-") { + // Check if policy condition is satisfied + condPassed = checkPolicyCond(op, formValues.Get(formCanonicalName), policy.Value) + if !condPassed { + return fmt.Errorf("Invalid according to Policy: Policy Condition failed: [%s, %s, %s]", op, policy.Key, policy.Value) } + } else { + // Unknown condition key: neither in startsWithConds nor a $x-amz-* + // prefixed key. AWS rejects these outright instead of silently + // treating the condition as satisfied. + return fmt.Errorf("Invalid according to Policy: Policy Condition failed: [%s, %s, %s]: unknown condition key", op, policy.Key, policy.Value) } }