From 843620235b0465557d678fc66af28aa53fcfb2ac Mon Sep 17 00:00:00 2001 From: niksis02 Date: Wed, 27 Aug 2025 20:44:30 +0400 Subject: [PATCH] fix: adds full wildcard and any character match for bucket policy actions Fixes #1488 Adds full wildcard (`*`) and single-character (`?`) support for bucket policy actions, fixes resource detection with wildcards, and includes unit tests for `bucket_policy_actions`, `bucket_policy_effect`, and `bucket_policy_principals`. --- auth/bucket_policy.go | 31 +++++ auth/bucket_policy_actions.go | 65 +++++----- auth/bucket_policy_actions_test.go | 175 ++++++++++++++++++++++++++ auth/bucket_policy_effect_test.go | 57 +++++++++ auth/bucket_policy_principals_test.go | 106 ++++++++++++++++ auth/bucket_policy_resources.go | 30 +---- tests/integration/group-tests.go | 12 +- tests/integration/tests.go | 153 +++++++++------------- 8 files changed, 465 insertions(+), 164 deletions(-) create mode 100644 auth/bucket_policy_actions_test.go create mode 100644 auth/bucket_policy_effect_test.go create mode 100644 auth/bucket_policy_principals_test.go diff --git a/auth/bucket_policy.go b/auth/bucket_policy.go index c103832f..94987c81 100644 --- a/auth/bucket_policy.go +++ b/auth/bucket_policy.go @@ -257,3 +257,34 @@ func VerifyPublicBucketPolicy(policy []byte, bucket, object string, action Actio return nil } + +// matchPattern checks if the input string matches the given pattern with wildcard(`*`) and any character(`?`). +// - `?` matches exactly one occurrence of any character. +// - `*` matches arbitrary many (including zero) occurrences of any character. +func matchPattern(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) +} diff --git a/auth/bucket_policy_actions.go b/auth/bucket_policy_actions.go index 8812e1e7..716fc402 100644 --- a/auth/bucket_policy_actions.go +++ b/auth/bucket_policy_actions.go @@ -192,55 +192,54 @@ func (a Action) IsValid() error { return nil } - if a[len(a)-1] == '*' { - pattern := strings.TrimSuffix(string(a), "*") - for act := range supportedActionList { - if strings.HasPrefix(string(act), pattern) { - return nil - } + // first check for an exact match + if _, ok := supportedActionList[a]; ok { + return nil + } + + // walk through the supported actions and try wildcard match + for action := range supportedActionList { + if action.Match(a) { + return nil } - - return policyErrInvalidAction } - _, found := supportedActionList[a] - if !found { - return policyErrInvalidAction - } - return nil + return policyErrInvalidAction } func getBoolPtr(bl bool) *bool { return &bl } +// String converts the action to string +func (a Action) String() string { + return string(a) +} + +// Match wildcard matches the given pattern to the action +func (a Action) Match(pattern Action) bool { + return matchPattern(pattern.String(), a.String()) +} + // Checks if the action is object action // nil points to 's3:*' func (a Action) IsObjectAction() *bool { if a == AllActions { return nil } - if a[len(a)-1] == '*' { - pattern := strings.TrimSuffix(string(a), "*") - for act := range supportedObjectActionList { - if strings.HasPrefix(string(act), pattern) { - return getBoolPtr(true) - } + + // first find an exact match + if _, ok := supportedObjectActionList[a]; ok { + return &ok + } + + for action := range supportedObjectActionList { + if action.Match(a) { + return getBoolPtr(true) } - - return getBoolPtr(false) } - _, found := supportedObjectActionList[a] - return &found -} - -func (a Action) WildCardMatch(act Action) bool { - if strings.HasSuffix(string(a), "*") { - pattern := strings.TrimSuffix(string(a), "*") - return strings.HasPrefix(string(act), pattern) - } - return false + return getBoolPtr(false) } type Actions map[Action]struct{} @@ -289,6 +288,7 @@ func (a Actions) Add(str string) error { return nil } +// FindMatch tries to match the given action to the actions list func (a Actions) FindMatch(action Action) bool { _, ok := a[AllActions] if ok { @@ -300,8 +300,9 @@ func (a Actions) FindMatch(action Action) bool { return true } + // search for a wildcard match for act := range a { - if strings.HasSuffix(string(act), "*") && act.WildCardMatch(action) { + if action.Match(act) { return true } } diff --git a/auth/bucket_policy_actions_test.go b/auth/bucket_policy_actions_test.go new file mode 100644 index 00000000..7b236d3d --- /dev/null +++ b/auth/bucket_policy_actions_test.go @@ -0,0 +1,175 @@ +// 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" + + "github.com/stretchr/testify/assert" +) + +func TestAction_IsValid(t *testing.T) { + tests := []struct { + name string + action Action + wantErr bool + }{ + {"valid exact action", GetObjectAction, false}, + {"valid all actions", AllActions, false}, + {"invalid prefix", "invalid:Action", true}, + {"unsupported action 1", "s3:Unsupported", true}, + {"unsupported action 2", "s3:HeadObject", true}, + {"valid wildcard match 1", "s3:Get*", false}, + {"valid wildcard match 2", "s3:*Object*", false}, + {"valid wildcard match 3", "s3:*Multipart*", false}, + {"any char match 1", "s3:Get?bject", false}, + {"any char match 2", "s3:Get??bject", true}, + {"any char match 3", "s3:???", true}, + {"mixed match 1", "s3:Get?*", false}, + {"mixed match 2", "s3:*Object?????", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.action.IsValid() + if tt.wantErr { + assert.EqualValues(t, policyErrInvalidAction, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestAction_String(t *testing.T) { + a := Action("s3:TestAction") + assert.Equal(t, "s3:TestAction", a.String()) +} + +func TestAction_Match(t *testing.T) { + tests := []struct { + name string + action Action + pattern Action + want bool + }{ + {"exact match", "s3:GetObject", "s3:GetObject", true}, + {"wildcard match", "s3:GetObject", "s3:Get*", true}, + {"wildcard mismatch", "s3:PutObject", "s3:Get*", false}, + {"any character match", "s3:Get1", "s3:Get?", true}, + {"any character mismatch", "s3:Get12", "s3:Get?", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.action.Match(tt.pattern) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestAction_IsObjectAction(t *testing.T) { + tests := []struct { + name string + action Action + want *bool + }{ + {"all actions", AllActions, nil}, + {"object action exact", GetObjectAction, getBoolPtr(true)}, + {"object action wildcard", "s3:Get*", getBoolPtr(true)}, + {"non object action", GetBucketAclAction, getBoolPtr(false)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.action.IsObjectAction() + if tt.want == nil { + assert.Nil(t, got) + } else { + assert.NotNil(t, got) + assert.Equal(t, *tt.want, *got) + } + }) + } +} + +func TestActions_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + }{ + {"valid slice", `["s3:GetObject","s3:PutObject"]`, false}, + {"empty slice", `[]`, true}, + {"invalid action in slice", `["s3:Invalid"]`, true}, + {"valid string", `"s3:GetObject"`, false}, + {"empty string", `""`, true}, + {"invalid string", `"s3:Invalid"`, true}, + {"invalid json", `{}`, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var a Actions + err := json.Unmarshal([]byte(tt.input), &a) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestActions_Add(t *testing.T) { + tests := []struct { + name string + action string + wantErr bool + }{ + {"valid add", "s3:GetObject", false}, + {"invalid add", "s3:InvalidAction", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := make(Actions) + err := a.Add(tt.action) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + _, ok := a[Action(tt.action)] + assert.True(t, ok) + } + }) + } +} + +func TestActions_FindMatch(t *testing.T) { + tests := []struct { + name string + actions Actions + check Action + want bool + }{ + {"all actions present", Actions{AllActions: {}}, GetObjectAction, true}, + {"exact match", Actions{GetObjectAction: {}}, GetObjectAction, true}, + {"wildcard match", Actions{"s3:Get*": {}}, GetObjectAction, true}, + {"no match", Actions{"s3:Put*": {}}, GetObjectAction, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.actions.FindMatch(tt.check) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/auth/bucket_policy_effect_test.go b/auth/bucket_policy_effect_test.go new file mode 100644 index 00000000..e27f2a64 --- /dev/null +++ b/auth/bucket_policy_effect_test.go @@ -0,0 +1,57 @@ +// 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 ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBucketPolicyAccessType_Validate(t *testing.T) { + tests := []struct { + name string + input BucketPolicyAccessType + wantErr bool + errMsg string + }{ + { + name: "valid allow", + input: BucketPolicyAccessTypeAllow, + wantErr: false, + }, + { + name: "valid deny", + input: BucketPolicyAccessTypeDeny, + wantErr: false, + }, + { + name: "invalid type", + input: BucketPolicyAccessType("InvalidValue"), + wantErr: true, + errMsg: "Invalid effect: InvalidValue", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.input.Validate() + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/auth/bucket_policy_principals_test.go b/auth/bucket_policy_principals_test.go new file mode 100644 index 00000000..766b7628 --- /dev/null +++ b/auth/bucket_policy_principals_test.go @@ -0,0 +1,106 @@ +// 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" + + "github.com/stretchr/testify/assert" +) + +func TestPrincipals_Add(t *testing.T) { + p := make(Principals) + p.Add("user1") + _, ok := p["user1"] + assert.True(t, ok) +} + +func TestPrincipals_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want Principals + wantErr bool + }{ + {"valid slice", `["user1","user2"]`, Principals{"user1": {}, "user2": {}}, false}, + {"empty slice", `[]`, nil, true}, + {"valid string", `"user1"`, Principals{"user1": {}}, false}, + {"empty string", `""`, nil, true}, + {"valid AWS object", `{"AWS":"user1"}`, Principals{"user1": {}}, false}, + {"empty AWS object", `{"AWS":""}`, nil, true}, + {"valid AWS array", `{"AWS":["user1","user2"]}`, Principals{"user1": {}, "user2": {}}, false}, + {"empty AWS array", `{"AWS":[]}`, nil, true}, + {"invalid json", `{invalid}`, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var p Principals + err := json.Unmarshal([]byte(tt.input), &p) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, p) + } + }) + } +} + +func TestPrincipals_ToSlice(t *testing.T) { + p := Principals{"user1": {}, "user2": {}, "*": {}} + got := p.ToSlice() + assert.Contains(t, got, "user1") + assert.Contains(t, got, "user2") + assert.NotContains(t, got, "*") +} + +func TestPrincipals_Validate(t *testing.T) { + iamSingle := NewIAMServiceSingle(Account{ + Access: "user1", + }) + tests := []struct { + name string + principals Principals + mockIAM IAMService + err error + }{ + {"only wildcard", Principals{"*": {}}, iamSingle, nil}, + {"wildcard and user", Principals{"*": {}, "user1": {}}, iamSingle, policyErrInvalidPrincipal}, + {"accounts exist returns err", Principals{"user2": {}, "user3": {}}, iamSingle, policyErrInvalidPrincipal}, + {"accounts exist non-empty", Principals{"user1": {}}, iamSingle, nil}, + {"accounts valid", Principals{"user1": {}}, iamSingle, nil}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.principals.Validate(tt.mockIAM) + assert.EqualValues(t, tt.err, err) + }) + } +} + +func TestPrincipals_Contains(t *testing.T) { + p := Principals{"user1": {}} + assert.True(t, p.Contains("user1")) + assert.False(t, p.Contains("user2")) + + p = Principals{"*": {}} + assert.True(t, p.Contains("anyuser")) +} + +func TestPrincipals_isPublic(t *testing.T) { + assert.True(t, Principals{"*": {}}.isPublic()) + assert.False(t, Principals{"user1": {}}.isPublic()) +} diff --git a/auth/bucket_policy_resources.go b/auth/bucket_policy_resources.go index cde9c36e..2dfd2494 100644 --- a/auth/bucket_policy_resources.go +++ b/auth/bucket_policy_resources.go @@ -110,35 +110,9 @@ func (r Resources) FindMatch(resource string) bool { 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. +// Match matches the given input resource with the pattern 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) + return matchPattern(pattern, input) } // Checks the resource to have arn prefix and not starting with / diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index b4557dd6..764aa218 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -494,10 +494,7 @@ func TestPutBucketPolicy(s *S3Conf) { PutBucketPolicy_statement_not_provided(s) PutBucketPolicy_empty_statement(s) PutBucketPolicy_invalid_effect(s) - PutBucketPolicy_empty_actions_string(s) - PutBucketPolicy_empty_actions_array(s) PutBucketPolicy_invalid_action(s) - PutBucketPolicy_incorrect_action_wildcard_usage(s) PutBucketPolicy_empty_principals_string(s) PutBucketPolicy_empty_principals_array(s) PutBucketPolicy_principals_aws_struct_empty_string(s) @@ -510,8 +507,7 @@ func TestPutBucketPolicy(s *S3Conf) { PutBucketPolicy_invalid_resource_with_starting_slash(s) PutBucketPolicy_duplicate_resource(s) PutBucketPolicy_incorrect_bucket_name(s) - PutBucketPolicy_object_action_on_bucket_resource(s) - PutBucketPolicy_bucket_action_on_object_resource(s) + PutBucketPolicy_action_resource_mismatch(s) PutBucketPolicy_explicit_deny(s) PutBucketPolicy_multi_wildcard_resource(s) PutBucketPolicy_any_char_match(s) @@ -1328,10 +1324,7 @@ func GetIntTests() IntTests { "PutBucketPolicy_statement_not_provided": PutBucketPolicy_statement_not_provided, "PutBucketPolicy_empty_statement": PutBucketPolicy_empty_statement, "PutBucketPolicy_invalid_effect": PutBucketPolicy_invalid_effect, - "PutBucketPolicy_empty_actions_string": PutBucketPolicy_empty_actions_string, - "PutBucketPolicy_empty_actions_array": PutBucketPolicy_empty_actions_array, "PutBucketPolicy_invalid_action": PutBucketPolicy_invalid_action, - "PutBucketPolicy_incorrect_action_wildcard_usage": PutBucketPolicy_incorrect_action_wildcard_usage, "PutBucketPolicy_empty_principals_string": PutBucketPolicy_empty_principals_string, "PutBucketPolicy_empty_principals_array": PutBucketPolicy_empty_principals_array, "PutBucketPolicy_principals_aws_struct_empty_string": PutBucketPolicy_principals_aws_struct_empty_string, @@ -1344,9 +1337,8 @@ func GetIntTests() IntTests { "PutBucketPolicy_invalid_resource_with_starting_slash": PutBucketPolicy_invalid_resource_with_starting_slash, "PutBucketPolicy_duplicate_resource": PutBucketPolicy_duplicate_resource, "PutBucketPolicy_incorrect_bucket_name": PutBucketPolicy_incorrect_bucket_name, - "PutBucketPolicy_object_action_on_bucket_resource": PutBucketPolicy_object_action_on_bucket_resource, + "PutBucketPolicy_action_resource_mismatch": PutBucketPolicy_action_resource_mismatch, "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, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index c068a53e..07fd0b94 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -12938,78 +12938,42 @@ func PutBucketPolicy_invalid_effect(s *S3Conf) error { }) } -func PutBucketPolicy_empty_actions_string(s *S3Conf) error { - testName := "PutBucketPolicy_empty_actions_string" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - doc := genPolicyDoc("Allow", `"*"`, `""`, `"arn:aws:s3:::*"`) - - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ - Bucket: &bucket, - Policy: &doc, - }) - cancel() - - if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil { - return err - } - return nil - }) -} - -func PutBucketPolicy_empty_actions_array(s *S3Conf) error { - testName := "PutBucketPolicy_empty_actions_array" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - doc := genPolicyDoc("Allow", `"*"`, `[]`, `"arn:aws:s3:::*"`) - - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ - Bucket: &bucket, - Policy: &doc, - }) - cancel() - - if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil { - return err - } - return nil - }) -} - func PutBucketPolicy_invalid_action(s *S3Conf) error { testName := "PutBucketPolicy_invalid_action" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - doc := genPolicyDoc("Allow", `"*"`, `"ListObjects"`, `"arn:aws:s3:::*"`) - - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ - Bucket: &bucket, - Policy: &doc, - }) - cancel() - - if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil { + err := createUsers(s, []user{testuser1}) + if err != nil { return err } - return nil - }) -} -func PutBucketPolicy_incorrect_action_wildcard_usage(s *S3Conf) error { - testName := "PutBucketPolicy_incorrect_action_wildcard_usage" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - doc := genPolicyDoc("Allow", `"*"`, `"s3:hello*"`, `"arn:aws:s3:::*"`) + for _, action := range []string{ + // empty actions + `""`, "[]", + // completely invalid action + `"invalid_action"`, `["invalid_action"]`, + // only prefix + `"s3"`, `"s3:"`, + // malformed prefix + `"s4:GetObject"`, `"ss3:ListBucket"`, `"s3x:PutBucketAcl"`, `":GetObject"`, `"s3GetObject"`, + // bad separator + `"s3::GetObject"`, `"s3:Put-Object"`, `"s3:GetObject:"`, `"s3:Put(Object)"`, + // wildcard abuse + `"s3:*Obj??ect*"`, `"s3:????"`, `"s3:*:"`, `"*GetObject"`, `"???PutObject"`, `"s3:Abort?"`, `"s3:??Abort*"`, + } { + doc := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser1.access), action, fmt.Sprintf(`"arn:aws:s3:::%s"`, bucket)) - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ - Bucket: &bucket, - Policy: &doc, - }) - cancel() + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() - if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil { - return err + if err := checkApiErr(err, getMalformedPolicyError("Policy has invalid action")); err != nil { + return err + } } + return nil }) } @@ -13247,45 +13211,46 @@ func PutBucketPolicy_incorrect_bucket_name(s *S3Conf) error { }) } -func PutBucketPolicy_object_action_on_bucket_resource(s *S3Conf) error { - testName := "PutBucketPolicy_object_action_on_bucket_resource" +func PutBucketPolicy_action_resource_mismatch(s *S3Conf) error { + testName := "PutBucketPolicy_action_resource_mismatch" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - resource := fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket) - doc := genPolicyDoc("Allow", `["*"]`, `"s3:PutObjectTagging"`, resource) + bucketResource := fmt.Sprintf(`"arn:aws:s3:::%s"`, bucket) + objectResource := fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket) - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ - Bucket: &bucket, - Policy: &doc, - }) - cancel() - if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil { - return err + for _, test := range []struct { + resource string + action string + }{ + // bucket resources + {bucketResource, `"s3:GetObject"`}, + {bucketResource, `"s3:PutObjectTagging"`}, + {bucketResource, `"s3:GetObjec?"`}, + {bucketResource, `"s3:Abort*"`}, + {bucketResource, `"s3:*Multipart*"`}, + {bucketResource, `"s3:???Object"`}, + // object resources + {objectResource, `"s3:ListBucket"`}, + {objectResource, `"s3:GetBucketTagging"`}, + {objectResource, `"s3:???BucketVersioning"`}, + {objectResource, `"s3:*Bucket*"`}, + {objectResource, `"s3:GetBucket*"`}, + } { + doc := genPolicyDoc("Allow", `["*"]`, test.action, test.resource) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil { + return err + } } return nil }) } -func PutBucketPolicy_bucket_action_on_object_resource(s *S3Conf) error { - testName := "PutBucketPolicy_bucket_action_on_object_resource" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - resource := fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket) - doc := genPolicyDoc("Allow", `["*"]`, `"s3:DeleteBucket"`, resource) - - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ - Bucket: &bucket, - Policy: &doc, - }) - cancel() - - if err := checkApiErr(err, getMalformedPolicyError("Action does not apply to any resource(s) in statement")); err != nil { - return err - } - return nil - }) -} func PutBucketPolicy_explicit_deny(s *S3Conf) error { testName := "PutBucketPolicy_explicit_deny" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {