diff --git a/weed/s3api/policy_engine/bucket_policy.go b/weed/s3api/policy_engine/bucket_policy.go index d909489fd..aac1f4bd5 100644 --- a/weed/s3api/policy_engine/bucket_policy.go +++ b/weed/s3api/policy_engine/bucket_policy.go @@ -49,6 +49,12 @@ func ValidateBucketPolicy(policyDoc *PolicyDocument, bucket string) error { return fmt.Errorf("statement %d: bucket policies only support S3 actions, got %s", i, action) } } + + for operator := range statement.Condition { + if _, err := GetConditionEvaluator(operator); err != nil { + return fmt.Errorf("statement %d: unsupported condition operator %q: %v", i, operator, err) + } + } } return nil diff --git a/weed/s3api/policy_engine/conditions.go b/weed/s3api/policy_engine/conditions.go index af55b06c2..c5ac36fd6 100644 --- a/weed/s3api/policy_engine/conditions.go +++ b/weed/s3api/policy_engine/conditions.go @@ -218,6 +218,36 @@ func (e *StringNotLikeEvaluator) Evaluate(conditionValue interface{}, contextVal return true } +// StringEqualsIgnoreCaseEvaluator evaluates StringEqualsIgnoreCase conditions +type StringEqualsIgnoreCaseEvaluator struct{} + +func (e *StringEqualsIgnoreCaseEvaluator) Evaluate(conditionValue interface{}, contextValues []string) bool { + expectedValues := getCachedNormalizedValues(conditionValue) + for _, expected := range expectedValues { + for _, contextValue := range contextValues { + if strings.EqualFold(expected, contextValue) { + return true + } + } + } + return false +} + +// StringNotEqualsIgnoreCaseEvaluator evaluates StringNotEqualsIgnoreCase conditions +type StringNotEqualsIgnoreCaseEvaluator struct{} + +func (e *StringNotEqualsIgnoreCaseEvaluator) Evaluate(conditionValue interface{}, contextValues []string) bool { + expectedValues := getCachedNormalizedValues(conditionValue) + for _, expected := range expectedValues { + for _, contextValue := range contextValues { + if strings.EqualFold(expected, contextValue) { + return false + } + } + } + return true +} + // NumericEqualsEvaluator evaluates NumericEquals conditions type NumericEqualsEvaluator struct{} @@ -650,6 +680,10 @@ func GetConditionEvaluator(operator string) (ConditionEvaluator, error) { return &StringLikeEvaluator{}, nil case "StringNotLike": return &StringNotLikeEvaluator{}, nil + case "StringEqualsIgnoreCase": + return &StringEqualsIgnoreCaseEvaluator{}, nil + case "StringNotEqualsIgnoreCase": + return &StringNotEqualsIgnoreCaseEvaluator{}, nil case "NumericEquals": return &NumericEqualsEvaluator{}, nil case "NumericNotEquals": @@ -730,7 +764,7 @@ func EvaluateConditions(conditions PolicyConditions, contextValues map[string][] conditionEvaluator, err := GetConditionEvaluator(operator) if err != nil { glog.Warningf("Unsupported condition operator: %s", operator) - continue + return false } for key, value := range conditionMap { diff --git a/weed/s3api/policy_engine/conditions_failclosed_test.go b/weed/s3api/policy_engine/conditions_failclosed_test.go new file mode 100644 index 000000000..f4b0a531c --- /dev/null +++ b/weed/s3api/policy_engine/conditions_failclosed_test.go @@ -0,0 +1,18 @@ +package policy_engine + +import ( + "testing" +) + +func TestEvaluateConditionsFailsClosedOnUnsupportedOperator(t *testing.T) { + conditions := PolicyConditions{ + "StringEqualsBogus": { + "aws:username": NewStringOrStringSlice("alice"), + }, + } + contextValues := map[string][]string{} + got := EvaluateConditions(conditions, contextValues, nil, nil) + if got { + t.Fatalf("EvaluateConditions returned true for unsupported operator; expected false (fail closed)") + } +} diff --git a/weed/s3api/policy_engine/conditions_ignorecase_test.go b/weed/s3api/policy_engine/conditions_ignorecase_test.go new file mode 100644 index 000000000..50ce78a48 --- /dev/null +++ b/weed/s3api/policy_engine/conditions_ignorecase_test.go @@ -0,0 +1,41 @@ +package policy_engine + +import ( + "testing" +) + +func TestConditionEvaluatorsIgnoreCase(t *testing.T) { + tests := []struct { + name string + operator string + conditionValue interface{} + contextValues []string + expected bool + }{ + {"StringEqualsIgnoreCase - match", "StringEqualsIgnoreCase", "ALICE", []string{"alice"}, true}, + {"StringEqualsIgnoreCase - no match", "StringEqualsIgnoreCase", "alice", []string{"bob"}, false}, + {"StringNotEqualsIgnoreCase - match", "StringNotEqualsIgnoreCase", "alice", []string{"bob"}, true}, + {"StringNotEqualsIgnoreCase - no match", "StringNotEqualsIgnoreCase", "ALICE", []string{"alice"}, false}, + } + + 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) + } + result := evaluator.Evaluate(tt.conditionValue, tt.contextValues) + if result != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, result) + } + }) + } +} + +func TestGetConditionEvaluatorRejectsNonAWSIgnoreCaseOperators(t *testing.T) { + for _, op := range []string{"StringLikeIgnoreCase", "StringNotLikeIgnoreCase"} { + if _, err := GetConditionEvaluator(op); err == nil { + t.Fatalf("GetConditionEvaluator accepted non-AWS operator %q; expected error", op) + } + } +} diff --git a/weed/s3api/policy_engine/conditions_validation_test.go b/weed/s3api/policy_engine/conditions_validation_test.go new file mode 100644 index 000000000..ec61feb76 --- /dev/null +++ b/weed/s3api/policy_engine/conditions_validation_test.go @@ -0,0 +1,74 @@ +package policy_engine + +import ( + "testing" +) + +func TestValidateBucketPolicyRejectsUnsupportedConditionOperator(t *testing.T) { + unsupported := `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": {"StringEqualsBogus": {"aws:username": "alice"}} + }] + }` + supportedIgnoreCase := `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": {"StringEqualsIgnoreCase": {"aws:username": "alice"}} + }] + }` + supportedStringEquals := `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::test-bucket/*", + "Condition": {"StringEquals": {"aws:username": "alice"}} + }] + }` + + t.Run("upload rejects unsupported operator", func(t *testing.T) { + policy, err := ParsePolicy(unsupported) + if err != nil { + t.Fatalf("ParsePolicy failed: %v", err) + } + if err := ValidateBucketPolicy(policy, "test-bucket"); err == nil { + t.Fatalf("ValidateBucketPolicy expected error for unsupported operator, got nil") + } + }) + + t.Run("upload accepts supported IgnoreCase operator", func(t *testing.T) { + policy, err := ParsePolicy(supportedIgnoreCase) + if err != nil { + t.Fatalf("ParsePolicy failed: %v", err) + } + if err := ValidateBucketPolicy(policy, "test-bucket"); err != nil { + t.Fatalf("ValidateBucketPolicy unexpected error: %v", err) + } + }) + + t.Run("upload accepts supported StringEquals operator", func(t *testing.T) { + policy, err := ParsePolicy(supportedStringEquals) + if err != nil { + t.Fatalf("ParsePolicy failed: %v", err) + } + if err := ValidateBucketPolicy(policy, "test-bucket"); err != nil { + t.Fatalf("ValidateBucketPolicy unexpected error: %v", err) + } + }) + + t.Run("load tolerates legacy unsupported operator", func(t *testing.T) { + if _, err := ParsePolicy(unsupported); err != nil { + t.Fatalf("ParsePolicy must not reject legacy unsupported operator at load time: %v", err) + } + }) +}