mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-06-13 23:36:45 +03:00
fix(s3): populate s3:prefix from query param for ListObjects policy conditions (#8971)
* fix(s3): populate s3:prefix from query param for ListObjects policy conditions (#8969) ListObjectsV2/V1 requests with prefix-restricted STS session policies were denied because: 1. s3:prefix was derived from objectKey, which the auth middleware set to the prefix value, but the resource ARN then included the prefix (e.g. arn:aws:s3:::bucket/prefix) instead of staying at bucket level (arn:aws:s3:::bucket) as AWS requires for ListBucket. 2. When objectKey was empty (no middleware propagation), s3:prefix was never populated from the query parameter at all. Now AuthorizeAction extracts the prefix query parameter directly, sets it as s3:prefix in the request context, and uses a bucket-level resource ARN when the objectKey matches the propagated prefix. * fix(s3): use AWS-style wildcard matching for StringLike policy conditions filepath.Match treats * as not matching /, which breaks IAM StringLike conditions on paths (e.g. arn:aws:s3:::bucket/* won't match nested keys). Replace with a case-sensitive variant of AwsWildcardMatch that correctly treats * as matching any character including /. * refactor(s3): replace regex wildcard matching with string-based matcher Use the existing wildcard.MatchesWildcard utility instead of compiling and caching regexes for IAM wildcard matching. Removes the regexCache, its mutex, and the sync import. * refactor(s3): inline and remove AwsWildcardMatch wrapper functions Replace all call sites with direct wildcard.MatchesWildcard calls. * fix(s3): scope s3:prefix condition key to list operations only The s3:prefix logic was running for all actions, so a GetObject on "foo/bar" would wrongly populate s3:prefix. Restrict it to action "List" and always reset resourceObjectKey to "" so the resource ARN stays at bucket level. Also set s3:prefix to "" when no prefix is provided, so policies with StringEquals {"s3:prefix": ""} evaluate correctly.
This commit is contained in:
@@ -1,8 +1,10 @@
|
||||
package policy
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/util/wildcard"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
@@ -286,7 +288,7 @@ func TestAWSWildcardMatch(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := AwsWildcardMatch(tt.pattern, tt.value)
|
||||
result := wildcard.MatchesWildcard(strings.ToLower(tt.pattern), strings.ToLower(tt.value))
|
||||
assert.Equal(t, tt.expected, result, "AWS wildcard match should match expected")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -5,12 +5,12 @@ import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/util/wildcard"
|
||||
)
|
||||
|
||||
// Effect represents the policy evaluation result
|
||||
@@ -21,10 +21,7 @@ const (
|
||||
EffectDeny Effect = "Deny"
|
||||
)
|
||||
|
||||
// Package-level regex cache for performance optimization
|
||||
var (
|
||||
regexCache = make(map[string]*regexp.Regexp)
|
||||
regexCacheMu sync.RWMutex
|
||||
policyVariablePattern = regexp.MustCompile(`\$\{([^}]+)\}`)
|
||||
safePolicyVariables = map[string]bool{
|
||||
// AWS standard identity variables
|
||||
@@ -1064,8 +1061,7 @@ func (e *PolicyEngine) EvaluateStringCondition(block map[string]interface{}, eva
|
||||
for _, expected := range expectedStrings {
|
||||
expandedExpected := expandPolicyVariables(expected, evalCtx)
|
||||
if useWildcard {
|
||||
// Use filepath.Match for case-sensitive wildcard matching, as required by StringLike
|
||||
if matched, _ := filepath.Match(expandedExpected, contextValue); matched {
|
||||
if wildcard.MatchesWildcard(expandedExpected, contextValue) {
|
||||
contextValueMatchedSet = true
|
||||
break
|
||||
}
|
||||
@@ -1106,13 +1102,11 @@ func (e *PolicyEngine) EvaluateStringCondition(block map[string]interface{}, eva
|
||||
for _, expected := range expectedStrings {
|
||||
expandedExpected := expandPolicyVariables(expected, evalCtx)
|
||||
if useWildcard {
|
||||
// Use filepath.Match for case-sensitive wildcard matching, as required by StringLike
|
||||
if matched, _ := filepath.Match(expandedExpected, contextValue); matched {
|
||||
if wildcard.MatchesWildcard(expandedExpected, contextValue) {
|
||||
contextValueMatchedSet = true
|
||||
break
|
||||
}
|
||||
} else {
|
||||
// For StringEquals/StringNotEquals, also support policy variables but be case-sensitive
|
||||
if expandedExpected == contextValue {
|
||||
contextValueMatchedSet = true
|
||||
break
|
||||
@@ -1229,7 +1223,7 @@ func awsIAMMatch(pattern, value string, evalCtx *EvaluationContext) bool {
|
||||
|
||||
// Step 4: Handle AWS-style wildcards (case-insensitive)
|
||||
if strings.Contains(expandedPattern, "*") || strings.Contains(expandedPattern, "?") {
|
||||
return AwsWildcardMatch(expandedPattern, value)
|
||||
return wildcard.MatchesWildcard(strings.ToLower(expandedPattern), strings.ToLower(value))
|
||||
}
|
||||
|
||||
return false
|
||||
@@ -1265,44 +1259,6 @@ func expandPolicyVariables(pattern string, evalCtx *EvaluationContext) string {
|
||||
return result
|
||||
}
|
||||
|
||||
// AwsWildcardMatch performs case-insensitive wildcard matching like AWS IAM
|
||||
func AwsWildcardMatch(pattern, value string) bool {
|
||||
// Create regex pattern key for caching
|
||||
// First escape all regex metacharacters, then replace wildcards
|
||||
regexPattern := regexp.QuoteMeta(pattern)
|
||||
regexPattern = strings.ReplaceAll(regexPattern, "\\*", ".*")
|
||||
regexPattern = strings.ReplaceAll(regexPattern, "\\?", ".")
|
||||
regexPattern = "^" + regexPattern + "$"
|
||||
regexKey := "(?i)" + regexPattern
|
||||
|
||||
// Try to get compiled regex from cache
|
||||
regexCacheMu.RLock()
|
||||
regex, found := regexCache[regexKey]
|
||||
regexCacheMu.RUnlock()
|
||||
|
||||
if !found {
|
||||
// Compile and cache the regex
|
||||
compiledRegex, err := regexp.Compile(regexKey)
|
||||
if err != nil {
|
||||
// Fallback to simple case-insensitive comparison if regex fails
|
||||
return strings.EqualFold(pattern, value)
|
||||
}
|
||||
|
||||
// Store in cache with write lock
|
||||
regexCacheMu.Lock()
|
||||
// Double-check in case another goroutine added it
|
||||
if existingRegex, exists := regexCache[regexKey]; exists {
|
||||
regex = existingRegex
|
||||
} else {
|
||||
regexCache[regexKey] = compiledRegex
|
||||
regex = compiledRegex
|
||||
}
|
||||
regexCacheMu.Unlock()
|
||||
}
|
||||
|
||||
return regex.MatchString(value)
|
||||
}
|
||||
|
||||
// evaluateStringConditionIgnoreCase evaluates string conditions with case insensitivity
|
||||
func (e *PolicyEngine) evaluateStringConditionIgnoreCase(block map[string]interface{}, evalCtx *EvaluationContext, shouldMatch bool, useWildcard bool, forAllValues bool) bool {
|
||||
for key, expectedValues := range block {
|
||||
@@ -1347,7 +1303,7 @@ func (e *PolicyEngine) evaluateStringConditionIgnoreCase(block map[string]interf
|
||||
case string:
|
||||
expandedPattern := expandPolicyVariables(v, evalCtx)
|
||||
if useWildcard {
|
||||
if AwsWildcardMatch(expandedPattern, ctxStr) {
|
||||
if wildcard.MatchesWildcard(strings.ToLower(expandedPattern), strings.ToLower(ctxStr)) {
|
||||
itemMatchedSet = true
|
||||
}
|
||||
} else {
|
||||
@@ -1369,7 +1325,7 @@ func (e *PolicyEngine) evaluateStringConditionIgnoreCase(block map[string]interf
|
||||
for _, valStr := range slice {
|
||||
expandedPattern := expandPolicyVariables(valStr, evalCtx)
|
||||
if useWildcard {
|
||||
if AwsWildcardMatch(expandedPattern, ctxStr) {
|
||||
if wildcard.MatchesWildcard(strings.ToLower(expandedPattern), strings.ToLower(ctxStr)) {
|
||||
itemMatchedSet = true
|
||||
break
|
||||
}
|
||||
@@ -1409,7 +1365,7 @@ func (e *PolicyEngine) evaluateStringConditionIgnoreCase(block map[string]interf
|
||||
case string:
|
||||
expandedPattern := expandPolicyVariables(v, evalCtx)
|
||||
if useWildcard {
|
||||
if AwsWildcardMatch(expandedPattern, ctxStr) {
|
||||
if wildcard.MatchesWildcard(strings.ToLower(expandedPattern), strings.ToLower(ctxStr)) {
|
||||
itemMatchedSet = true
|
||||
}
|
||||
} else {
|
||||
@@ -1431,7 +1387,7 @@ func (e *PolicyEngine) evaluateStringConditionIgnoreCase(block map[string]interf
|
||||
for _, valStr := range slice {
|
||||
expandedPattern := expandPolicyVariables(valStr, evalCtx)
|
||||
if useWildcard {
|
||||
if AwsWildcardMatch(expandedPattern, ctxStr) {
|
||||
if wildcard.MatchesWildcard(strings.ToLower(expandedPattern), strings.ToLower(ctxStr)) {
|
||||
itemMatchedSet = true
|
||||
break
|
||||
}
|
||||
|
||||
@@ -4,10 +4,11 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/mail"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/glog"
|
||||
"github.com/seaweedfs/seaweedfs/weed/iam/policy"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util/wildcard"
|
||||
)
|
||||
|
||||
// IdentityProvider defines the interface for external identity providers
|
||||
@@ -225,7 +226,7 @@ func (r *MappingRule) Matches(claims *TokenClaims) bool {
|
||||
// matchValue checks if a value matches the rule value (with wildcard support)
|
||||
// Uses AWS IAM-compliant case-insensitive wildcard matching for consistency with policy engine
|
||||
func (r *MappingRule) matchValue(value string) bool {
|
||||
matched := policy.AwsWildcardMatch(r.Value, value)
|
||||
matched := wildcard.MatchesWildcard(strings.ToLower(r.Value), strings.ToLower(value))
|
||||
glog.V(3).Infof("AWS IAM pattern match result: '%s' matches '%s' = %t", value, r.Value, matched)
|
||||
return matched
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user