diff --git a/auth/bucket_policy_resources.go b/auth/bucket_policy_resources.go index 99c5635c..e105e95a 100644 --- a/auth/bucket_policy_resources.go +++ b/auth/bucket_policy_resources.go @@ -102,21 +102,45 @@ func (r Resources) Validate(bucket string) error { func (r Resources) FindMatch(resource string) bool { for res := range r { - if strings.HasSuffix(res, "*") { - pattern := strings.TrimSuffix(res, "*") - if strings.HasPrefix(resource, pattern) { - return true - } - } else { - if res == resource { - return true - } + if r.Match(res, resource) { + return true } } return false } +// Match checks if the input string matches the given pattern with wildcards (`*`, `?`). +// - `?` matches exactly one occurrence of any character. +// - `*` matches arbitrary many (including zero) occurrences of any character. +func (r Resources) Match(pattern, input string) bool { + pIdx, sIdx := 0, 0 + starIdx, matchIdx := -1, 0 + + for sIdx < len(input) { + if pIdx < len(pattern) && (pattern[pIdx] == '?' || pattern[pIdx] == input[sIdx]) { + sIdx++ + pIdx++ + } else if pIdx < len(pattern) && pattern[pIdx] == '*' { + starIdx = pIdx + matchIdx = sIdx + pIdx++ + } else if starIdx != -1 { + pIdx = starIdx + 1 + matchIdx++ + sIdx = matchIdx + } else { + return false + } + } + + for pIdx < len(pattern) && pattern[pIdx] == '*' { + pIdx++ + } + + return pIdx == len(pattern) +} + // Checks the resource to have arn prefix and not starting with / func isValidResource(rc string) (isValid bool, pattern string) { if !strings.HasPrefix(rc, ResourceArnPrefix) { diff --git a/auth/bucket_policy_resources_test.go b/auth/bucket_policy_resources_test.go new file mode 100644 index 00000000..9b362fc9 --- /dev/null +++ b/auth/bucket_policy_resources_test.go @@ -0,0 +1,182 @@ +// Copyright 2023 Versity Software +// This file is 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 auth + +import ( + "encoding/json" + "testing" +) + +func TestUnmarshalJSON(t *testing.T) { + var r Resources + + cases := []struct { + input string + expected int + wantErr bool + }{ + {`"arn:aws:s3:::my-bucket/*"`, 1, false}, + {`["arn:aws:s3:::my-bucket/*", "arn:aws:s3:::other-bucket"]`, 2, false}, + {`""`, 0, true}, + {`[]`, 0, true}, + {`["invalid-bucket"]`, 0, true}, + } + + for _, tc := range cases { + r = Resources{} + err := json.Unmarshal([]byte(tc.input), &r) + if (err != nil) != tc.wantErr { + t.Errorf("Unexpected error status for input %s: %v", tc.input, err) + } + if len(r) != tc.expected { + t.Errorf("Expected %d resources, got %d", tc.expected, len(r)) + } + } +} + +func TestAdd(t *testing.T) { + r := Resources{} + + cases := []struct { + input string + wantErr bool + }{ + {"arn:aws:s3:::valid-bucket/*", false}, + {"arn:aws:s3:::valid-bucket/object", false}, + {"invalid-bucket/*", true}, + {"/invalid-start", true}, + } + + for _, tc := range cases { + err := r.Add(tc.input) + if (err != nil) != tc.wantErr { + t.Errorf("Unexpected error status for input %s: %v", tc.input, err) + } + } +} + +func TestContainsObjectPattern(t *testing.T) { + cases := []struct { + resources []string + expected bool + }{ + {[]string{"arn:aws:s3:::my-bucket/my-object"}, true}, + {[]string{"arn:aws:s3:::my-bucket/*"}, true}, + {[]string{"arn:aws:s3:::my-bucket"}, false}, + } + + for _, tc := range cases { + r := Resources{} + for _, res := range tc.resources { + r.Add(res) + } + if r.ContainsObjectPattern() != tc.expected { + t.Errorf("Expected object pattern to be %v for %v", tc.expected, tc.resources) + } + } +} + +func TestContainsBucketPattern(t *testing.T) { + cases := []struct { + resources []string + expected bool + }{ + {[]string{"arn:aws:s3:::my-bucket"}, true}, + {[]string{"arn:aws:s3:::my-bucket/*"}, false}, + {[]string{"arn:aws:s3:::my-bucket/object"}, false}, + } + + for _, tc := range cases { + r := Resources{} + for _, res := range tc.resources { + r.Add(res) + } + if r.ContainsBucketPattern() != tc.expected { + t.Errorf("Expected bucket pattern to be %v for %v", tc.expected, tc.resources) + } + } +} + +func TestValidate(t *testing.T) { + cases := []struct { + resources []string + bucket string + expected bool + }{ + {[]string{"arn:aws:s3:::valid-bucket/*"}, "valid-bucket", true}, + {[]string{"arn:aws:s3:::wrong-bucket/*"}, "valid-bucket", false}, + {[]string{"arn:aws:s3:::valid-bucket/*", "arn:aws:s3:::valid-bucket/object/*"}, "valid-bucket", true}, + } + + for _, tc := range cases { + r := Resources{} + for _, res := range tc.resources { + r.Add(res) + } + if (r.Validate(tc.bucket) == nil) != tc.expected { + t.Errorf("Expected validation to be %v for bucket %s", tc.expected, tc.bucket) + } + } +} + +func TestFindMatch(t *testing.T) { + cases := []struct { + resources []string + input string + expected bool + }{ + {[]string{"arn:aws:s3:::my-bucket/*"}, "my-bucket/my-object", true}, + {[]string{"arn:aws:s3:::my-bucket/object"}, "other-bucket/my-object", false}, + {[]string{"arn:aws:s3:::my-bucket/object"}, "my-bucket/object", true}, + {[]string{"arn:aws:s3:::my-bucket/*", "arn:aws:s3:::other-bucket/*"}, "other-bucket/something", true}, + } + + for _, tc := range cases { + r := Resources{} + for _, res := range tc.resources { + r.Add(res) + } + if r.FindMatch(tc.input) != tc.expected { + t.Errorf("Expected FindMatch to be %v for input %s", tc.expected, tc.input) + } + } +} + +func TestMatch(t *testing.T) { + r := Resources{} + cases := []struct { + pattern string + input string + expected bool + }{ + {"my-bucket/*", "my-bucket/object", true}, + {"my-bucket/?bject", "my-bucket/object", true}, + {"my-bucket/*", "other-bucket/object", false}, + {"*", "any-bucket/object", true}, + {"my-bucket/*", "my-bucket/subdir/object", true}, + {"my-bucket/*", "other-bucket", false}, + {"my-bucket/*/*", "my-bucket/hello", false}, + {"my-bucket/*/*", "my-bucket/hello/world", true}, + {"foo/???/bar", "foo/qux/bar", true}, + {"foo/???/bar", "foo/quxx/bar", false}, + {"foo/???/bar/*/?", "foo/qux/bar/hello/g", true}, + {"foo/???/bar/*/?", "foo/qux/bar/hello/smth", false}, + } + for _, tc := range cases { + if r.Match(tc.pattern, tc.input) != tc.expected { + t.Errorf("Match(%s, %s) failed, expected %v", tc.pattern, tc.input, tc.expected) + } + } +} diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index e6f15a87..f377d195 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -472,6 +472,8 @@ func TestPutBucketPolicy(s *S3Conf) { PutBucketPolicy_object_action_on_bucket_resource(s) PutBucketPolicy_bucket_action_on_object_resource(s) PutBucketPolicy_explicit_deny(s) + PutBucketPolicy_multi_wildcard_resource(s) + PutBucketPolicy_any_char_match(s) PutBucketPolicy_success(s) } @@ -1045,6 +1047,8 @@ func GetIntTests() IntTests { "PutBucketPolicy_object_action_on_bucket_resource": PutBucketPolicy_object_action_on_bucket_resource, "PutBucketPolicy_explicit_deny": PutBucketPolicy_explicit_deny, "PutBucketPolicy_bucket_action_on_object_resource": PutBucketPolicy_bucket_action_on_object_resource, + "PutBucketPolicy_multi_wildcard_resource": PutBucketPolicy_multi_wildcard_resource, + "PutBucketPolicy_any_char_match": PutBucketPolicy_any_char_match, "PutBucketPolicy_success": PutBucketPolicy_success, "GetBucketPolicy_non_existing_bucket": GetBucketPolicy_non_existing_bucket, "GetBucketPolicy_not_set": GetBucketPolicy_not_set, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 7fbe5c41..cecf0a80 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -11195,7 +11195,6 @@ func PutBucketPolicy_bucket_action_on_object_resource(s *S3Conf) error { return nil }) } - func PutBucketPolicy_explicit_deny(s *S3Conf) error { testName := "PutBucketPolicy_object_action_on_bucket_resource" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -11281,6 +11280,80 @@ func PutBucketPolicy_explicit_deny(s *S3Conf) error { }) } +func PutBucketPolicy_multi_wildcard_resource(s *S3Conf) error { + testName := "PutBucketPolicy_multi_wildcard_resource" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + usr := user{"grt1", "grt1secret", "user"} + if err := createUsers(s, []user{usr}); err != nil { + return err + } + + resource := fmt.Sprintf(`["arn:aws:s3:::%v/*/*", "arn:aws:s3:::%v"]`, bucket, bucket) + principal := fmt.Sprintf("\"%v\"", usr.access) + doc := genPolicyDoc("Allow", principal, `"s3:*"`, resource) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + if err != nil { + return err + } + + userClient := getUserS3Client(usr, s) + _, err = putObjects(userClient, []string{"foo"}, bucket) + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil { + return err + } + + _, err = putObjects(userClient, []string{"bar/quxx", "foo/bar/baz", "foo/bar/xyz/quxx"}, bucket) + if err != nil { + return err + } + + return nil + }) +} + +func PutBucketPolicy_any_char_match(s *S3Conf) error { + testName := "PutBucketPolicy_any_char_match" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + usr := user{"grt1", "grt1secret", "user"} + if err := createUsers(s, []user{usr}); err != nil { + return err + } + + resource := fmt.Sprintf(`["arn:aws:s3:::%v/m?-obj/*"]`, bucket) + principal := fmt.Sprintf("\"%v\"", usr.access) + doc := genPolicyDoc("Allow", principal, `"s3:*"`, resource) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + if err != nil { + return err + } + + userClient := getUserS3Client(usr, s) + _, err = putObjects(userClient, []string{"myy-obj/hello", "rand/foo", "my-objj/bar"}, bucket) + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil { + return err + } + + _, err = putObjects(userClient, []string{"my-obj/hello", "mk-obj/foo", "m--obj/bar"}, bucket) + if err != nil { + return err + } + + return nil + }) +} + func PutBucketPolicy_success(s *S3Conf) error { testName := "PutBucketPolicy_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {