From 2e0742e309be0ba931abab7548fe1bf2d3426a05 Mon Sep 17 00:00:00 2001 From: karthic rao Date: Thu, 4 Aug 2016 13:11:32 +0530 Subject: [PATCH] bucket policy: Support for '?' wildcard. (#2353) - Support for '?' wildcard for resource matching. - Wildcard package is added with Match functions. - Wildcard.Match supports '*' and wild.MatchExtended supports both '*' and '?' wildcards in the pattern string. - Tests for the same for the wide range of cases. --- bucket-policy-handlers.go | 33 +-- bucket-policy-handlers_test.go | 72 ----- pkg/wildcard/match.go | 138 +++++++++ pkg/wildcard/match_test.go | 518 +++++++++++++++++++++++++++++++++ queues.go | 3 +- 5 files changed, 661 insertions(+), 103 deletions(-) create mode 100644 pkg/wildcard/match.go create mode 100644 pkg/wildcard/match_test.go diff --git a/bucket-policy-handlers.go b/bucket-policy-handlers.go index fb190f01e..419b0333d 100644 --- a/bucket-policy-handlers.go +++ b/bucket-policy-handlers.go @@ -21,9 +21,9 @@ import ( "io" "io/ioutil" "net/http" - "strings" mux "github.com/gorilla/mux" + "github.com/minio/minio/pkg/wildcard" ) // maximum supported access policy size. @@ -71,41 +71,14 @@ func bucketPolicyActionMatch(action string, statement policyStatement) bool { return false } -// Match function matches wild cards in 'pattern' for 'text'. -func wildCardMatch(pattern, text string) bool { - if pattern == "" { - return text == pattern - } - if pattern == "*" { - return true - } - parts := strings.Split(pattern, "*") - if len(parts) == 1 { - return text == pattern - } - tGlob := strings.HasSuffix(pattern, "*") - end := len(parts) - 1 - if !strings.HasPrefix(text, parts[0]) { - return false - } - for i := 1; i < end; i++ { - if !strings.Contains(text, parts[i]) { - return false - } - idx := strings.Index(text, parts[i]) + len(parts[i]) - text = text[idx:] - } - return tGlob || strings.HasSuffix(text, parts[end]) -} - // Match function matches wild cards in 'pattern' for resource. func resourceMatch(pattern, resource string) bool { - return wildCardMatch(pattern, resource) + return wildcard.MatchExtended(pattern, resource) } // Match function matches wild cards in 'pattern' for action. func actionMatch(pattern, action string) bool { - return wildCardMatch(pattern, action) + return wildcard.Match(pattern, action) } // Verify if given resource matches with policy statement. diff --git a/bucket-policy-handlers_test.go b/bucket-policy-handlers_test.go index b8fc0d71c..bd6fb081f 100644 --- a/bucket-policy-handlers_test.go +++ b/bucket-policy-handlers_test.go @@ -237,78 +237,6 @@ func TestBucketPolicyActionMatch(t *testing.T) { } } -// TestWildCardMatch - Tests validate the logic of wild card matching. -// Its used to match the action and resources of the policy statement and the request. -func TestWildCardMatch(t *testing.T) { - testCases := []struct { - pattern string - text string - expectedResult bool - }{ - // Test case - 1. - // Test case with pattern "*". Expected to match any text. - {"*", "s3:GetObject", true}, - // Test case - 2. - // Test case with empty pattern. This only matches empty string. - {"", "s3:GetObject", false}, - // Test case - 3. - // Test case with empty pattern. This only matches empty string. - {"", "", true}, - // Test case - 4. - // Test case with single "*" at the end. - {"s3:*", "s3:ListMultipartUploadParts", true}, - // Test case - 5. - // Test case with a no "*". In this case the pattern and text should be the same. - {"s3:ListBucketMultipartUploads", "s3:ListBucket", false}, - // Test case - 6. - // Test case with a no "*". In this case the pattern and text should be the same. - {"s3:ListBucket", "s3:ListBucket", true}, - // Test case - 7. - // Test case with a no "*". In this case the pattern and text should be the same. - {"s3:ListBucketMultipartUploads", "s3:ListBucketMultipartUploads", true}, - // Test case - 8. - // Test case with pattern containing key name with a prefix. Should accept the same text without a "*". - {"my-bucket/oo*", "my-bucket/oo", true}, - // Test case - 9. - // Test case with "*" at the end of the pattern. - {"my-bucket/In*", "my-bucket/India/Karnataka/", true}, - // Test case - 10. - // Test case with prefixes shuffled. - // This should fail. - {"my-bucket/In*", "my-bucket/Karnataka/India/", false}, - // Test case - 11. - // Test case with text expanded to the wildcards in the pattern. - {"my-bucket/In*/Ka*/Ban", "my-bucket/India/Karnataka/Ban", true}, - // Test case - 12. - // Test case with the keyname part is repeated as prefix several times. - // This is valid. - {"my-bucket/In*/Ka*/Ban", "my-bucket/India/Karnataka/Ban/Ban/Ban/Ban/Ban", true}, - // Test case - 13. - // Test case to validate that `*` can be expanded into multiple prefixes. - {"my-bucket/In*/Ka*/Ban", "my-bucket/India/Karnataka/Area1/Area2/Area3/Ban", true}, - // Test case to validate that `*` can be expanded into multiple prefixes. - {"my-bucket/In*/Ka*/Ban", "my-bucket/India/State1/State2/Karnataka/Area1/Area2/Area3/Ban", true}, - // Test case - 14. - // Test case where the keyname part of the pattern is expanded in the text. - {"my-bucket/In*/Ka*/Ban", "my-bucket/India/Karnataka/Bangalore", false}, - // Test case - 15. - // Test case with prefixes and wildcard expanded for all "*". - {"my-bucket/In*/Ka*/Ban*", "my-bucket/India/Karnataka/Bangalore", true}, - // Test case - 16. - // Test case with keyname part being a wildcard in the pattern. - {"my-bucket/*", "my-bucket/India", true}, - // Test case - 17. - {"my-bucket/oo*", "my-bucket/odo", false}, - } - // Iterating over the test cases, call the function under test and asert the output. - for i, testCase := range testCases { - actualResult := wildCardMatch(testCase.pattern, testCase.text) - if testCase.expectedResult != actualResult { - t.Errorf("Test %d: Expected the result to be `%v`, but instead found it to be `%v`", i+1, testCase.expectedResult, actualResult) - } - } -} - // Wrapper for calling Put Bucket Policy HTTP handler tests for both XL multiple disks and single node setup. func TestPutBucketPolicyHandler(t *testing.T) { ExecObjectLayerTest(t, testPutBucketPolicyHandler) diff --git a/pkg/wildcard/match.go b/pkg/wildcard/match.go new file mode 100644 index 000000000..e1119eeb6 --- /dev/null +++ b/pkg/wildcard/match.go @@ -0,0 +1,138 @@ +/* + * Minio Cloud Storage, (C) 2015, 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package wildcard + +import ( + "strings" + "unicode/utf8" +) + +// Match - finds whether the text matches/satisfies the pattern string. +// supports only '*' wildcard in the pattern. +// considers a file system path as a flat name space. +func Match(pattern, text string) bool { + if pattern == "" { + return text == pattern + } + if pattern == "*" { + return true + } + parts := strings.Split(pattern, "*") + if len(parts) == 1 { + return text == pattern + } + tGlob := strings.HasSuffix(pattern, "*") + end := len(parts) - 1 + if !strings.HasPrefix(text, parts[0]) { + return false + } + for i := 1; i < end; i++ { + if !strings.Contains(text, parts[i]) { + return false + } + idx := strings.Index(text, parts[i]) + len(parts[i]) + text = text[idx:] + } + return tGlob || strings.HasSuffix(text, parts[end]) +} + +// MatchExtended - finds whether the text matches/satisfies the pattern string. +// supports '*' and '?' wildcards in the pattern string. +// unlike path.Match(), considers a path as a flat name space while matching the pattern. +// The difference is illustrated in the example here https://play.golang.org/p/Ega9qgD4Qz . +func MatchExtended(pattern, name string) (matched bool) { +Pattern: + for len(pattern) > 0 { + var star bool + var chunk string + star, chunk, pattern = scanChunk(pattern) + if star && chunk == "" { + // Trailing * matches rest of string. + return true + } + // Look for match at current position. + t, ok := matchChunk(chunk, name) + // if we're the last chunk, make sure we've exhausted the name + // otherwise we'll give a false result even if we could still match + // using the star + if ok && (len(t) == 0 || len(pattern) > 0) { + name = t + continue + } + if star { + // Look for match skipping i+1 bytes. + for i := 0; i < len(name); i++ { + t, ok := matchChunk(chunk, name[i+1:]) + if ok { + // if we're the last chunk, make sure we exhausted the name + if len(pattern) == 0 && len(t) > 0 { + continue + } + name = t + continue Pattern + } + } + } + return false + } + return len(name) == 0 +} + +// scanChunk gets the next segment of pattern, which is a non-star string +// possibly preceded by a star. +func scanChunk(pattern string) (star bool, chunk, rest string) { + for len(pattern) > 0 && pattern[0] == '*' { + pattern = pattern[1:] + star = true + } + inrange := false + var i int +Scan: + for i = 0; i < len(pattern); i++ { + switch pattern[i] { + case '*': + if !inrange { + break Scan + } + } + } + return star, pattern[0:i], pattern[i:] +} + +// matchChunk checks whether chunk matches the beginning of s. +// If so, it returns the remainder of s (after the match). +// Chunk is all single-character operators: literals, char classes, and ?. +func matchChunk(chunk, s string) (rest string, ok bool) { + for len(chunk) > 0 { + if len(s) == 0 { + return + } + switch chunk[0] { + case '?': + _, n := utf8.DecodeRuneInString(s) + s = s[n:] + chunk = chunk[1:] + default: + if chunk[0] != s[0] { + return + } + s = s[1:] + chunk = chunk[1:] + } + } + return s, true +} diff --git a/pkg/wildcard/match_test.go b/pkg/wildcard/match_test.go new file mode 100644 index 000000000..d796b5208 --- /dev/null +++ b/pkg/wildcard/match_test.go @@ -0,0 +1,518 @@ +/* + * Minio Cloud Storage, (C) 2015, 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package wildcard + +import ( + "testing" +) + +// TestMatchExtended - Tests validate the logic of wild card matching. +// `MatchExtended` supports '*' and '?' wildcards. +// Sample usage: In resource matching for bucket policy validation. +func TestMatchExtended(t *testing.T) { + testCases := []struct { + pattern string + text string + matched bool + }{ + // Test case - 1. + // Test case with pattern "*". Expected to match any text. + { + pattern: "*", + text: "s3:GetObject", + matched: true, + }, + // Test case - 2. + // Test case with empty pattern. This only matches empty string. + { + pattern: "", + text: "s3:GetObject", + matched: false, + }, + // Test case - 3. + // Test case with empty pattern. This only matches empty string. + { + pattern: "", + text: "", + matched: true, + }, + // Test case - 4. + // Test case with single "*" at the end. + { + pattern: "s3:*", + text: "s3:ListMultipartUploadParts", + matched: true, + }, + // Test case - 5. + // Test case with a no "*". In this case the pattern and text should be the same. + { + pattern: "s3:ListBucketMultipartUploads", + text: "s3:ListBucket", + matched: false, + }, + // Test case - 6. + // Test case with a no "*". In this case the pattern and text should be the same. + { + pattern: "s3:ListBucket", + text: "s3:ListBucket", + matched: true, + }, + // Test case - 7. + // Test case with a no "*". In this case the pattern and text should be the same. + { + pattern: "s3:ListBucketMultipartUploads", + text: "s3:ListBucketMultipartUploads", + matched: true, + }, + // Test case - 8. + // Test case with pattern containing key name with a prefix. Should accept the same text without a "*". + { + pattern: "my-bucket/oo*", + text: "my-bucket/oo", + matched: true, + }, + // Test case - 9. + // Test case with "*" at the end of the pattern. + { + pattern: "my-bucket/In*", + text: "my-bucket/India/Karnataka/", + matched: true, + }, + // Test case - 10. + // Test case with prefixes shuffled. + // This should fail. + { + pattern: "my-bucket/In*", + text: "my-bucket/Karnataka/India/", + matched: false, + }, + // Test case - 11. + // Test case with text expanded to the wildcards in the pattern. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Ban", + matched: true, + }, + // Test case - 12. + // Test case with the keyname part is repeated as prefix several times. + // This is valid. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Ban/Ban/Ban/Ban/Ban", + matched: true, + }, + // Test case - 13. + // Test case to validate that `*` can be expanded into multiple prefixes. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Area1/Area2/Area3/Ban", + matched: true, + }, + // Test case - 14. + // Test case to validate that `*` can be expanded into multiple prefixes. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/State1/State2/Karnataka/Area1/Area2/Area3/Ban", + matched: true, + }, + // Test case - 15. + // Test case where the keyname part of the pattern is expanded in the text. + {pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Bangalore", + matched: false, + }, + // Test case - 16. + // Test case with prefixes and wildcard expanded for all "*". + { + pattern: "my-bucket/In*/Ka*/Ban*", + text: "my-bucket/India/Karnataka/Bangalore", + matched: true, + }, + // Test case - 17. + // Test case with keyname part being a wildcard in the pattern. + {pattern: "my-bucket/*", + text: "my-bucket/India", + matched: true, + }, + // Test case - 18. + { + pattern: "my-bucket/oo*", + text: "my-bucket/odo", + matched: false, + }, + + // Test case with pattern containing wildcard '?'. + // Test case - 19. + // "my-bucket?/" matches "my-bucket1/", "my-bucket2/", "my-bucket3" etc... + // doesn't match "mybucket/". + { + pattern: "my-bucket?/abc*", + text: "mybucket/abc", + matched: false, + }, + // Test case - 20. + { + pattern: "my-bucket?/abc*", + text: "my-bucket1/abc", + matched: true, + }, + // Test case - 21. + { + pattern: "my-?-bucket/abc*", + text: "my--bucket/abc", + matched: false, + }, + // Test case - 22. + { + pattern: "my-?-bucket/abc*", + text: "my-1-bucket/abc", + matched: true, + }, + // Test case - 23. + { + pattern: "my-?-bucket/abc*", + text: "my-k-bucket/abc", + matched: true, + }, + // Test case - 24. + { + pattern: "my??bucket/abc*", + text: "mybucket/abc", + matched: false, + }, + // Test case - 25. + { + pattern: "my??bucket/abc*", + text: "my4abucket/abc", + matched: true, + }, + // Test case - 26. + { + pattern: "my-bucket?abc*", + text: "my-bucket/abc", + matched: true, + }, + // Test case 27-28. + // '?' matches '/' too. (works with s3). + // This is because the namespace is considered flat. + // "abc?efg" matches both "abcdefg" and "abc/efg". + { + pattern: "my-bucket/abc?efg", + text: "my-bucket/abcdefg", + matched: true, + }, + { + pattern: "my-bucket/abc?efg", + text: "my-bucket/abc/efg", + matched: true, + }, + // Test case - 29. + { + pattern: "my-bucket/abc????", + text: "my-bucket/abc", + matched: false, + }, + // Test case - 30. + { + pattern: "my-bucket/abc????", + text: "my-bucket/abcde", + matched: false, + }, + // Test case - 31. + { + pattern: "my-bucket/abc????", + text: "my-bucket/abcdefg", + matched: true, + }, + // Test case 32-34. + // test case with no '*'. + { + pattern: "my-bucket/abc?", + text: "my-bucket/abc", + matched: false, + }, + { + pattern: "my-bucket/abc?", + text: "my-bucket/abcd", + matched: true, + }, + { + pattern: "my-bucket/abc?", + text: "my-bucket/abcde", + matched: false, + }, + // Test case 35. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mnop", + matched: false, + }, + // Test case 36. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mnopqrst/mnopqr", + matched: true, + }, + // Test case 37. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mnopqrst/mnopqrs", + matched: true, + }, + // Test case 38. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mnop", + matched: false, + }, + // Test case 39. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mnopq", + matched: true, + }, + // Test case 40. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mnopqr", + matched: true, + }, + // Test case 41. + { + pattern: "my-bucket/mnop*?and", + text: "my-bucket/mnopqand", + matched: true, + }, + // Test case 42. + { + pattern: "my-bucket/mnop*?and", + text: "my-bucket/mnopand", + matched: false, + }, + // Test case 43. + { + pattern: "my-bucket/mnop*?and", + text: "my-bucket/mnopqand", + matched: true, + }, + // Test case 44. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mn", + matched: false, + }, + // Test case 45. + { + pattern: "my-bucket/mnop*?", + text: "my-bucket/mnopqrst/mnopqrs", + matched: true, + }, + // Test case 46. + { + pattern: "my-bucket/mnop*??", + text: "my-bucket/mnopqrst", + matched: true, + }, + // Test case 47. + { + pattern: "my-bucket/mnop*qrst", + text: "my-bucket/mnopabcdegqrst", + matched: true, + }, + // Test case 48. + { + pattern: "my-bucket/mnop*?and", + text: "my-bucket/mnopqand", + matched: true, + }, + // Test case 49. + { + pattern: "my-bucket/mnop*?and", + text: "my-bucket/mnopand", + matched: false, + }, + // Test case 50. + { + pattern: "my-bucket/mnop*?and?", + text: "my-bucket/mnopqanda", + matched: true, + }, + // Test case 51. + { + pattern: "my-bucket/mnop*?and", + text: "my-bucket/mnopqanda", + matched: false, + }, + // Test case 52. + + { + pattern: "my-?-bucket/abc*", + text: "my-bucket/mnopqanda", + matched: false, + }, + } + // Iterating over the test cases, call the function under test and asert the output. + for i, testCase := range testCases { + actualResult := MatchExtended(testCase.pattern, testCase.text) + if testCase.matched != actualResult { + t.Errorf("Test %d: Expected the result to be `%v`, but instead found it to be `%v`", i+1, testCase.matched, actualResult) + } + } +} + +// TestMatch - Tests validate the logic of wild card matching. +// `Match` supports matching for only '*' in the pattern string. +func TestMatch(t *testing.T) { + testCases := []struct { + pattern string + text string + matched bool + }{ + // Test case - 1. + // Test case with pattern "*". Expected to match any text. + { + pattern: "*", + text: "s3:GetObject", + matched: true, + }, + // Test case - 2. + // Test case with empty pattern. This only matches empty string. + { + pattern: "", + text: "s3:GetObject", + matched: false, + }, + // Test case - 3. + // Test case with empty pattern. This only matches empty string. + { + pattern: "", + text: "", + matched: true, + }, + // Test case - 4. + // Test case with single "*" at the end. + { + pattern: "s3:*", + text: "s3:ListMultipartUploadParts", + matched: true, + }, + // Test case - 5. + // Test case with a no "*". In this case the pattern and text should be the same. + { + pattern: "s3:ListBucketMultipartUploads", + text: "s3:ListBucket", + matched: false, + }, + // Test case - 6. + // Test case with a no "*". In this case the pattern and text should be the same. + { + pattern: "s3:ListBucket", + text: "s3:ListBucket", + matched: true, + }, + // Test case - 7. + // Test case with a no "*". In this case the pattern and text should be the same. + { + pattern: "s3:ListBucketMultipartUploads", + text: "s3:ListBucketMultipartUploads", + matched: true, + }, + // Test case - 8. + // Test case with pattern containing key name with a prefix. Should accept the same text without a "*". + { + pattern: "my-bucket/oo*", + text: "my-bucket/oo", + matched: true, + }, + // Test case - 9. + // Test case with "*" at the end of the pattern. + { + pattern: "my-bucket/In*", + text: "my-bucket/India/Karnataka/", + matched: true, + }, + // Test case - 10. + // Test case with prefixes shuffled. + // This should fail. + { + pattern: "my-bucket/In*", + text: "my-bucket/Karnataka/India/", + matched: false, + }, + // Test case - 11. + // Test case with text expanded to the wildcards in the pattern. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Ban", + matched: true, + }, + // Test case - 12. + // Test case with the keyname part is repeated as prefix several times. + // This is valid. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Ban/Ban/Ban/Ban/Ban", + matched: true, + }, + // Test case - 13. + // Test case to validate that `*` can be expanded into multiple prefixes. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Area1/Area2/Area3/Ban", + matched: true, + }, + // Test case - 14. + // Test case to validate that `*` can be expanded into multiple prefixes. + { + pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/State1/State2/Karnataka/Area1/Area2/Area3/Ban", + matched: true, + }, + // Test case - 15. + // Test case where the keyname part of the pattern is expanded in the text. + {pattern: "my-bucket/In*/Ka*/Ban", + text: "my-bucket/India/Karnataka/Bangalore", + matched: false, + }, + // Test case - 16. + // Test case with prefixes and wildcard expanded for all "*". + { + pattern: "my-bucket/In*/Ka*/Ban*", + text: "my-bucket/India/Karnataka/Bangalore", + matched: true, + }, + // Test case - 17. + // Test case with keyname part being a wildcard in the pattern. + {pattern: "my-bucket/*", + text: "my-bucket/India", + matched: true, + }, + // Test case - 18. + { + pattern: "my-bucket/oo*", + text: "my-bucket/odo", + matched: false, + }, + } + // Iterating over the test cases, call the function under test and asert the output. + for i, testCase := range testCases { + actualResult := Match(testCase.pattern, testCase.text) + if testCase.matched != actualResult { + t.Errorf("Test %d: Expected the result to be `%v`, but instead found it to be `%v`", i+1, testCase.matched, actualResult) + } + } +} diff --git a/queues.go b/queues.go index e1451f897..28ca2c980 100644 --- a/queues.go +++ b/queues.go @@ -23,6 +23,7 @@ import ( "time" "github.com/Sirupsen/logrus" + "github.com/minio/minio/pkg/wildcard" ) const ( @@ -85,7 +86,7 @@ func isElasticQueue(sqsArn arnMinioSqs) bool { // Match function matches wild cards in 'pattern' for events. func eventMatch(eventType EventName, events []string) (ok bool) { for _, event := range events { - ok = wildCardMatch(event, eventType.String()) + ok = wildcard.Match(event, eventType.String()) if ok { break }