diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index da165b798..9db792809 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -18,6 +18,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" @@ -1175,14 +1176,16 @@ func (identity *Identity) canDo(action Action, bucket string, objectKey string) for _, a := range identity.Actions { act := string(a) - if strings.HasSuffix(act, "*") { - if strings.HasPrefix(target, act[:len(act)-1]) { + if strings.ContainsAny(act, "*?") { + // Pattern has wildcards - use smart matching + if policy_engine.MatchesWildcard(act, target) { return true } - if strings.HasPrefix(adminTarget, act[:len(act)-1]) { + if policy_engine.MatchesWildcard(act, adminTarget) { return true } } else { + // No wildcards - exact match only if act == limitedByBucket { return true } diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index 5bdf27256..b01d8e3ce 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/seaweedfs/seaweedfs/weed/credential" + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" . "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/stretchr/testify/assert" @@ -159,6 +160,102 @@ func TestCanDo(t *testing.T) { assert.Equal(t, true, ident7.canDo(ACTION_DELETE_BUCKET, "bucket1", "")) } +func TestMatchWildcardPattern(t *testing.T) { + tests := []struct { + pattern string + target string + match bool + }{ + // Basic * wildcard tests + {"Bucket/*", "Bucket/a/b", true}, + {"Bucket/*", "x/Bucket/a", false}, + {"Bucket/*/admin", "Bucket/x/admin", true}, + {"Bucket/*/admin", "Bucket/x/y/admin", true}, + {"Bucket/*/admin", "Bucket////x////uwu////y////admin", true}, + {"abc*def", "abcXYZdef", true}, + {"abc*def", "abcXYZdefZZ", false}, + {"syr/*", "syr/a/b", true}, + + // ? wildcard tests (matches exactly one character) + {"ab?d", "abcd", true}, + {"ab?d", "abXd", true}, + {"ab?d", "abd", false}, // ? must match exactly one character + {"ab?d", "abcXd", false}, // ? matches only one character + {"a?c", "abc", true}, + {"a?c", "aXc", true}, + {"a?c", "ac", false}, + {"???", "abc", true}, + {"???", "ab", false}, + {"???", "abcd", false}, + + // Combined * and ? wildcards + {"a*?", "ab", true}, // * matches empty, ? matches 'b' + {"a*?", "abc", true}, // * matches 'b', ? matches 'c' + {"a*?", "a", false}, // ? must match something + {"a?*", "ab", true}, // ? matches 'b', * matches empty + {"a?*", "abc", true}, // ? matches 'b', * matches 'c' + {"a?*b", "aXb", true}, // ? matches 'X', * matches empty + {"a?*b", "aXYZb", true}, + {"*?*", "a", true}, + {"*?*", "", false}, // ? requires at least one character + + // Edge cases: * matches empty string + {"a*b", "ab", true}, // * matches empty string + {"a**b", "ab", true}, // multiple stars match empty + {"a**b", "axb", true}, // multiple stars match 'x' + {"a**b", "axyb", true}, + {"*", "", true}, + {"*", "anything", true}, + {"**", "", true}, + {"**", "anything", true}, + + // Edge cases: empty strings + {"", "", true}, + {"a", "", false}, + {"", "a", false}, + + // Trailing * matches empty + {"a*", "a", true}, + {"a*", "abc", true}, + {"abc*", "abc", true}, + {"abc*", "abcdef", true}, + + // Leading * matches empty + {"*a", "a", true}, + {"*a", "XXXa", true}, + {"*abc", "abc", true}, + {"*abc", "XXXabc", true}, + + // Multiple wildcards + {"*a*", "a", true}, + {"*a*", "Xa", true}, + {"*a*", "aX", true}, + {"*a*", "XaX", true}, + {"*a*b*", "ab", true}, + {"*a*b*", "XaYbZ", true}, + + // Exact match (no wildcards) + {"exact", "exact", true}, + {"exact", "notexact", false}, + {"exact", "exactnot", false}, + + // S3-style action patterns + {"Read:bucket*", "Read:bucket-test", true}, + {"Read:bucket*", "Read:bucket", true}, + {"Write:bucket/path/*", "Write:bucket/path/file.txt", true}, + {"Admin:*", "Admin:anything", true}, + } + + for _, tt := range tests { + t.Run(tt.pattern+"_"+tt.target, func(t *testing.T) { + result := policy_engine.MatchesWildcard(tt.pattern, tt.target) + if result != tt.match { + t.Errorf("policy_engine.MatchesWildcard(%q, %q) = %v, want %v", tt.pattern, tt.target, result, tt.match) + } + }) + } +} + type LoadS3ApiConfigurationTestCase struct { pbAccount *iam_pb.Account pbIdent *iam_pb.Identity diff --git a/weed/s3api/policy_engine/wildcard_matcher.go b/weed/s3api/policy_engine/wildcard_matcher.go index 7fd36abf9..7ba01c52e 100644 --- a/weed/s3api/policy_engine/wildcard_matcher.go +++ b/weed/s3api/policy_engine/wildcard_matcher.go @@ -125,21 +125,12 @@ func (c *WildcardMatcherCache) GetCacheStats() (size int, maxSize int) { } // NewWildcardMatcher creates a new wildcard matcher for the given pattern +// The matcher uses an efficient string-based algorithm that handles both * and ? wildcards +// without requiring regex compilation. func NewWildcardMatcher(pattern string) (*WildcardMatcher, error) { matcher := &WildcardMatcher{ - pattern: pattern, - } - - // Determine if we need regex (contains ? wildcards) - if strings.Contains(pattern, "?") { - matcher.useRegex = true - regex, err := compileWildcardPattern(pattern) - if err != nil { - return nil, err - } - matcher.regex = regex - } else { - matcher.useRegex = false + pattern: pattern, + useRegex: false, // String-based matching now handles both * and ? } return matcher, nil @@ -155,19 +146,12 @@ func (m *WildcardMatcher) Match(str string) bool { // MatchesWildcard provides a simple function interface for wildcard matching // This function consolidates the logic from the previous separate implementations +// +// Rules: +// - '*' matches any sequence of characters (including empty string) +// - '?' matches exactly one character (any character) func MatchesWildcard(pattern, str string) bool { - // Handle simple cases first - if pattern == "*" { - return true - } - if pattern == str { - return true - } - - // Use regex for patterns with ? wildcards, string manipulation for * only - if strings.Contains(pattern, "?") { - return matchWildcardRegex(pattern, str) - } + // matchWildcardString now handles both * and ? efficiently without regex return matchWildcardString(pattern, str) } @@ -177,7 +161,13 @@ func CompileWildcardPattern(pattern string) (*regexp.Regexp, error) { return compileWildcardPattern(pattern) } -// matchWildcardString uses string manipulation for * wildcards only (more efficient) +// matchWildcardString uses efficient string manipulation for * and ? wildcards +// This implementation uses a backtracking algorithm that handles both wildcard types +// without requiring regex compilation. +// +// Rules: +// - '*' matches any sequence of characters (including empty string) +// - '?' matches exactly one character (any character) func matchWildcardString(pattern, str string) bool { // Handle simple cases if pattern == "*" { @@ -187,43 +177,52 @@ func matchWildcardString(pattern, str string) bool { return true } - // Split pattern by wildcards - parts := strings.Split(pattern, "*") - if len(parts) == 1 { - // No wildcards, exact match - return pattern == str - } + targetIndex := 0 + patternIndex := 0 - // Check if string starts with first part - if len(parts[0]) > 0 && !strings.HasPrefix(str, parts[0]) { - return false - } + // Index of the most recent '*' in the pattern (-1 if none) + lastStarIndex := -1 - // Check if string ends with last part - if len(parts[len(parts)-1]) > 0 && !strings.HasSuffix(str, parts[len(parts)-1]) { - return false - } + // Index in target where the last '*' started matching + lastStarMatchIndex := 0 - // Check middle parts - searchStr := str - if len(parts[0]) > 0 { - searchStr = searchStr[len(parts[0]):] - } - if len(parts[len(parts)-1]) > 0 { - searchStr = searchStr[:len(searchStr)-len(parts[len(parts)-1])] - } + for targetIndex < len(str) { + switch { + // Case 1: Current characters match directly or '?' matches any single character + case patternIndex < len(pattern) && + (pattern[patternIndex] == '?' || pattern[patternIndex] == str[targetIndex]): + + targetIndex++ + patternIndex++ + + // Case 2: Wildcard '*' found in pattern + case patternIndex < len(pattern) && + pattern[patternIndex] == '*': - for i := 1; i < len(parts)-1; i++ { - if len(parts[i]) > 0 { - index := strings.Index(searchStr, parts[i]) - if index == -1 { - return false - } - searchStr = searchStr[index+len(parts[i]):] + lastStarIndex = patternIndex + lastStarMatchIndex = targetIndex + patternIndex++ + + // Case 3: Previous '*' can absorb one more character + case lastStarIndex != -1: + + patternIndex = lastStarIndex + 1 + lastStarMatchIndex++ + targetIndex = lastStarMatchIndex + + // Case 4: No match possible + default: + return false } } - return true + // Consume any trailing '*' in the pattern + for patternIndex < len(pattern) && pattern[patternIndex] == '*' { + patternIndex++ + } + + // Match is valid only if the entire pattern is consumed + return patternIndex == len(pattern) } // matchWildcardRegex uses WildcardMatcher for patterns with ? wildcards