From d469a72213f66a356acdd74107f7dd733acab8f4 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Fri, 15 Mar 2024 15:47:10 -0400 Subject: [PATCH 1/3] feat: Implemented Put/Get/DeletBucketPolicy s3 actions in posix backend. Implemented policy document validation function --- auth/bucket_policy.go | 90 +++++++++++ auth/bucket_policy_actions.go | 212 ++++++++++++++++++++++++++ auth/bucket_policy_effect.go | 34 +++++ auth/bucket_policy_principals.go | 86 +++++++++++ auth/bucket_policy_resources.go | 126 +++++++++++++++ backend/backend.go | 5 + backend/posix/posix.go | 49 ++++++ s3api/controllers/backend_moq_test.go | 52 ++++++- s3api/controllers/base.go | 32 ++++ 9 files changed, 685 insertions(+), 1 deletion(-) create mode 100644 auth/bucket_policy.go create mode 100644 auth/bucket_policy_actions.go create mode 100644 auth/bucket_policy_effect.go create mode 100644 auth/bucket_policy_principals.go create mode 100644 auth/bucket_policy_resources.go diff --git a/auth/bucket_policy.go b/auth/bucket_policy.go new file mode 100644 index 00000000..02a5c482 --- /dev/null +++ b/auth/bucket_policy.go @@ -0,0 +1,90 @@ +// 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" + "fmt" + "net/http" + + "github.com/versity/versitygw/s3err" +) + +type BucketPolicy struct { + Statement []BucketPolicyItem `json:"Statement"` +} + +func (bp *BucketPolicy) Validate(bucket string, iam IAMService) error { + for _, statement := range bp.Statement { + err := statement.Validate(bucket, iam) + if err != nil { + return err + } + } + + return nil +} + +type BucketPolicyItem struct { + Effect BucketPolicyAccessType `json:"Effect"` + Principals Principals `json:"Principal"` + Actions Actions `json:"Action"` + Resources Resources `json:"Resource"` +} + +func (bpi *BucketPolicyItem) Validate(bucket string, iam IAMService) error { + if err := bpi.Principals.Validate(iam); err != nil { + return err + } + if err := bpi.Effect.Validate(); err != nil { + return err + } + + containsObjectAction := bpi.Resources.ContainsObjectPattern() + containsBucketAction := bpi.Resources.ContainsBucketPattern() + + for action := range bpi.Actions { + isObjectAction := action.IsObjectAction() + if isObjectAction && !containsObjectAction { + return fmt.Errorf("unsupported object action '%v' on the specified resources", action) + } + if !isObjectAction && !containsBucketAction { + return fmt.Errorf("unsupported bucket action '%v', on the specified resources", action) + } + } + + return nil +} + +func getMalformedPolicyError(err error) error { + return s3err.APIError{ + Code: "MalformedPolicy", + Description: err.Error(), + HTTPStatusCode: http.StatusBadRequest, + } +} + +func ValidatePolicyDocument(policyBin []byte, bucket string, iam IAMService) error { + var policy BucketPolicy + if err := json.Unmarshal(policyBin, &policy); err != nil { + return getMalformedPolicyError(err) + } + + if err := policy.Validate(bucket, iam); err != nil { + return getMalformedPolicyError(err) + } + + return nil +} diff --git a/auth/bucket_policy_actions.go b/auth/bucket_policy_actions.go new file mode 100644 index 00000000..ab80a383 --- /dev/null +++ b/auth/bucket_policy_actions.go @@ -0,0 +1,212 @@ +// 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" + "fmt" + "strings" +) + +type Action string + +const ( + ListBuckets Action = "s3:ListBuckets" + HeadBucketAction Action = "s3:HeadBucket" + GetBucketAclAction Action = "s3:GetBucketAcl" + CreateBucketAction Action = "s3:CreateBucket" + PutBucketAclAction Action = "s3:PutBucketAcl" + DeleteBucketAction Action = "s3:DeleteBucket" + PutBucketVersioningAction Action = "s3:PutBucketVersioning" + GetBucketVersioningAction Action = "s3:GetBucketVersioning" + PutBucketPolicyAction Action = "s3:PutBucketPolicy" + GetBucketPolicyAction Action = "s3:GetBucketPolicy" + CreateMultipartUploadAction Action = "s3:CreateMultipartUpload" + CompleteMultipartUploadAction Action = "s3:CompleteMultipartUpload" + AbortMultipartUploadAction Action = "s3:AbortMultipartUpload" + ListMultipartUploadsAction Action = "s3:ListMultipartUploads" + ListPartsAction Action = "s3:ListParts" + UploadPartAction Action = "s3:UploadPart" + UploadPartCopyAction Action = "s3:UploadPartCopy" + PutObjectAction Action = "s3:PutObject" + HeadObjectAction Action = "s3:HeadObject" + GetObjectAction Action = "s3:GetObject" + GetObjectAclAction Action = "s3:GetObjectAcl" + GetObjectAttributesAction Action = "s3:GetObjectAttributes" + CopyObjectAction Action = "s3:CopyObject" + ListObjectsAction Action = "s3:ListObjects" + ListObjectsV2Action Action = "s3:ListObjectsV2" + DeleteObjectAction Action = "s3:DeleteObject" + DeleteObjectsAction Action = "s3:DeleteObjects" + PutObjectAclAction Action = "s3:PutObjectAcl" + ListObjectVersionsAction Action = "s3:ListObjectVersions" + RestoreObjectAction Action = "s3:RestoreObject" + SelectObjectContentAction Action = "s3:SelectObjectContent" + GetBucketTaggingAction Action = "s3:GetBucketTagging" + PutBucketTaggingAction Action = "s3:PutBucketTagging" + DeleteBucketTaggingAction Action = "s3:DeleteBucketTagging" + GetObjectTaggingAction Action = "s3:GetObjectTagging" + PutObjectTaggingAction Action = "s3:PutObjectTagging" + DeleteObjectTaggingAction Action = "s3:DeleteObjectTagging" + AllActions Action = "s3:*" +) + +var supportedActionList = map[Action]struct{}{ + ListBuckets: {}, + HeadBucketAction: {}, + GetBucketAclAction: {}, + CreateBucketAction: {}, + PutBucketAclAction: {}, + DeleteBucketAction: {}, + PutBucketVersioningAction: {}, + GetBucketVersioningAction: {}, + PutBucketPolicyAction: {}, + GetBucketPolicyAction: {}, + CreateMultipartUploadAction: {}, + CompleteMultipartUploadAction: {}, + AbortMultipartUploadAction: {}, + ListMultipartUploadsAction: {}, + ListPartsAction: {}, + UploadPartAction: {}, + UploadPartCopyAction: {}, + PutObjectAction: {}, + HeadObjectAction: {}, + GetObjectAction: {}, + GetObjectAclAction: {}, + GetObjectAttributesAction: {}, + CopyObjectAction: {}, + ListObjectsAction: {}, + ListObjectsV2Action: {}, + DeleteObjectAction: {}, + DeleteObjectsAction: {}, + PutObjectAclAction: {}, + ListObjectVersionsAction: {}, + RestoreObjectAction: {}, + SelectObjectContentAction: {}, + GetBucketTaggingAction: {}, + PutBucketTaggingAction: {}, + DeleteBucketTaggingAction: {}, + GetObjectTaggingAction: {}, + PutObjectTaggingAction: {}, + DeleteObjectTaggingAction: {}, + AllActions: {}, +} + +var supportedObjectActionList = map[Action]struct{}{ + CreateMultipartUploadAction: {}, + CompleteMultipartUploadAction: {}, + AbortMultipartUploadAction: {}, + ListMultipartUploadsAction: {}, + ListPartsAction: {}, + UploadPartAction: {}, + UploadPartCopyAction: {}, + PutObjectAction: {}, + HeadObjectAction: {}, + GetObjectAction: {}, + GetObjectAclAction: {}, + GetObjectAttributesAction: {}, + CopyObjectAction: {}, + ListObjectsAction: {}, + ListObjectsV2Action: {}, + DeleteObjectAction: {}, + DeleteObjectsAction: {}, + PutObjectAclAction: {}, + ListObjectVersionsAction: {}, + RestoreObjectAction: {}, + SelectObjectContentAction: {}, + GetObjectTaggingAction: {}, + PutObjectTaggingAction: {}, + DeleteObjectTaggingAction: {}, + AllActions: {}, +} + +// Validates Action: it should either wildcard match with supported actions list or be in it +func (a Action) IsValid() bool { + if a[len(a)-1] == '*' { + pattern := strings.TrimSuffix(string(a), "*") + for act := range supportedActionList { + if strings.HasPrefix(string(act), pattern) { + return true + } + } + + return false + } + + _, found := supportedActionList[a] + return found +} + +// Checks if the action is object action +func (a Action) IsObjectAction() bool { + if a[len(a)-1] == '*' { + pattern := strings.TrimSuffix(string(a), "*") + for act := range supportedObjectActionList { + if strings.HasPrefix(string(act), pattern) { + return true + } + } + + return false + } + + _, found := supportedObjectActionList[a] + return found +} + +type Actions map[Action]struct{} + +// Override UnmarshalJSON method to decode both []string and string properties +func (a *Actions) UnmarshalJSON(data []byte) error { + ss := []string{} + var err error + if err = json.Unmarshal(data, &ss); err == nil { + if len(ss) == 0 { + return fmt.Errorf("actions can't be empty") + } + *a = make(Actions) + for _, s := range ss { + err = a.Add(s) + if err != nil { + return err + } + } + } else { + var s string + if err = json.Unmarshal(data, &s); err == nil { + if s == "" { + return fmt.Errorf("actions can't be empty") + } + *a = make(Actions) + err = a.Add(s) + if err != nil { + return err + } + } + } + + return err +} + +// Validates and adds a new Action to Actions map +func (a Actions) Add(str string) error { + action := Action(str) + if !action.IsValid() { + return fmt.Errorf("invalid action") + } + + a[action] = struct{}{} + return nil +} diff --git a/auth/bucket_policy_effect.go b/auth/bucket_policy_effect.go new file mode 100644 index 00000000..c2ccc31d --- /dev/null +++ b/auth/bucket_policy_effect.go @@ -0,0 +1,34 @@ +// 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 "fmt" + +type BucketPolicyAccessType string + +const ( + BucketPolicyAccessTypeDeny BucketPolicyAccessType = "Deny" + BucketPolicyAccessTypeAllow BucketPolicyAccessType = "Allow" +) + +// Checks policy statement Effect to be valid ("Deny", "Allow") +func (bpat BucketPolicyAccessType) Validate() error { + switch bpat { + case BucketPolicyAccessTypeAllow, BucketPolicyAccessTypeDeny: + return nil + } + + return fmt.Errorf("invalid effect: %v", bpat) +} diff --git a/auth/bucket_policy_principals.go b/auth/bucket_policy_principals.go new file mode 100644 index 00000000..7abe49e9 --- /dev/null +++ b/auth/bucket_policy_principals.go @@ -0,0 +1,86 @@ +// 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" + "fmt" +) + +type Principals map[string]struct{} + +func (p Principals) Add(key string) { + p[key] = struct{}{} +} + +// Override UnmarshalJSON method to decode both []string and string properties +func (p *Principals) UnmarshalJSON(data []byte) error { + ss := []string{} + var err error + if err = json.Unmarshal(data, &ss); err == nil { + if len(ss) == 0 { + return fmt.Errorf("principals can't be empty") + } + *p = make(Principals) + for _, s := range ss { + p.Add(s) + } + } else { + var s string + if err = json.Unmarshal(data, &s); err == nil { + if s == "" { + return fmt.Errorf("principals can't be empty") + } + *p = make(Principals) + p.Add(s) + } + } + + return err +} + +// Converts Principals map to a slice, by omitting "*" +func (p Principals) ToSlice() []string { + principals := []string{} + for p := range p { + if p == "*" { + continue + } + principals = append(principals, p) + } + + return principals +} + +// Validates Principals by checking user account access keys existence +func (p Principals) Validate(iam IAMService) error { + _, containsWildCard := p["*"] + if containsWildCard { + if len(p) == 1 { + return nil + } + return fmt.Errorf("principals should either contain * or user access keys") + } + + accs, err := CheckIfAccountsExist(p.ToSlice(), iam) + if err != nil { + return err + } + if len(accs) > 0 { + return fmt.Errorf("users doesn't exist: %v", accs) + } + + return nil +} diff --git a/auth/bucket_policy_resources.go b/auth/bucket_policy_resources.go new file mode 100644 index 00000000..be02d132 --- /dev/null +++ b/auth/bucket_policy_resources.go @@ -0,0 +1,126 @@ +// 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" + "fmt" + "strings" +) + +type Resources map[string]struct{} + +const ResourceArnPrefix = "arn:aws:s3:::" + +// Override UnmarshalJSON method to decode both []string and string properties +func (r *Resources) UnmarshalJSON(data []byte) error { + ss := []string{} + var err error + if err = json.Unmarshal(data, &ss); err == nil { + if len(ss) == 0 { + return fmt.Errorf("resources can't be empty") + } + *r = make(Resources) + for _, s := range ss { + err = r.Add(s) + if err != nil { + return err + } + } + } else { + var s string + if err = json.Unmarshal(data, &s); err == nil { + if s == "" { + return fmt.Errorf("resources can't be empty") + } + *r = make(Resources) + err = r.Add(s) + if err != nil { + return err + } + } + } + + return err +} + +// Adds and validates a new resource to Resources map +func (r Resources) Add(rc string) error { + _, found := r[rc] + if found { + return fmt.Errorf("duplicate resource") + } + + ok, pattern := isValidResource(rc) + + if !ok { + return fmt.Errorf("invalid resource") + } + + r[pattern] = struct{}{} + + return nil +} + +// Checks if the resources contain object pattern +func (r Resources) ContainsObjectPattern() bool { + for resource := range r { + if resource == "*" || strings.Contains(resource, "/") { + return true + } + } + + return false +} + +// Checks if the resources contain bucket pattern +func (r Resources) ContainsBucketPattern() bool { + for resource := range r { + if resource == "*" || !strings.Contains(resource, "/") { + return true + } + } + + return false +} + +// Bucket resources should start with bucket name: arn:aws:s3:::MyBucket/* +func (r Resources) Validate(bucket string) error { + for resource := range r { + if !strings.HasPrefix(resource, bucket) { + return fmt.Errorf("invalid bucket resource") + } + } + + return nil +} + +// Checks the resource to have arn prefix and not starting with / +func isValidResource(rc string) (isValid bool, pattern string) { + if !strings.HasPrefix(rc, ResourceArnPrefix) { + return false, "" + } + + res := strings.TrimPrefix(rc, ResourceArnPrefix) + if res == "" { + return false, "" + } + // The resource can't start with / (bucket name comes first) + if res == "/" { + return false, "" + } + + return true, res +} diff --git a/backend/backend.go b/backend/backend.go index 64ab197d..b56e654a 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -42,6 +42,8 @@ type Backend interface { GetBucketVersioning(_ context.Context, bucket string) (*s3.GetBucketVersioningOutput, error) PutBucketPolicy(_ context.Context, bucket string, policy []byte) error GetBucketPolicy(_ context.Context, bucket string) ([]byte, error) + DeleteBucketPolicy(_ context.Context, bucket string) error + // multipart operations CreateMultipartUpload(context.Context, *s3.CreateMultipartUploadInput) (*s3.CreateMultipartUploadOutput, error) CompleteMultipartUpload(context.Context, *s3.CompleteMultipartUploadInput) (*s3.CompleteMultipartUploadOutput, error) @@ -125,6 +127,9 @@ func (BackendUnsupported) PutBucketPolicy(_ context.Context, bucket string, poli func (BackendUnsupported) GetBucketPolicy(_ context.Context, bucket string) ([]byte, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) } +func (BackendUnsupported) DeleteBucketPolicy(_ context.Context, bucket string) error { + return s3err.GetAPIError(s3err.ErrNotImplemented) +} func (BackendUnsupported) CreateMultipartUpload(context.Context, *s3.CreateMultipartUploadInput) (*s3.CreateMultipartUploadOutput, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index a3f53a96..19106444 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -61,6 +61,7 @@ const ( emptyMD5 = "d41d8cd98f00b204e9800998ecf8427e" aclkey = "user.acl" etagkey = "user.etag" + policykey = "user.policy" ) func New(rootdir string) (*Posix, error) { @@ -1851,6 +1852,54 @@ func (p *Posix) DeleteObjectTagging(ctx context.Context, bucket, object string) return p.PutObjectTagging(ctx, bucket, object, nil) } +func (p *Posix) PutBucketPolicy(ctx context.Context, bucket string, policy []byte) error { + _, err := os.Stat(bucket) + if errors.Is(err, fs.ErrNotExist) { + return s3err.GetAPIError(s3err.ErrNoSuchBucket) + } + if err != nil { + return fmt.Errorf("stat bucket: %w", err) + } + + if policy == nil { + if err := xattr.Remove(bucket, policykey); err != nil { + return fmt.Errorf("remove policy: %w", err) + } + + return nil + } + + if err := xattr.Set(bucket, policykey, policy); err != nil { + return fmt.Errorf("set policy: %w", err) + } + + return nil +} + +func (p *Posix) GetBucketPolicy(ctx context.Context, bucket string) ([]byte, error) { + _, err := os.Stat(bucket) + if errors.Is(err, fs.ErrNotExist) { + return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) + } + if err != nil { + return nil, fmt.Errorf("stat bucket: %w", err) + } + + policy, err := xattr.Get(bucket, policykey) + if isNoAttr(err) { + return []byte{}, nil + } + if err != nil { + return nil, fmt.Errorf("get bucket policy: %w", err) + } + + return policy, nil +} + +func (p *Posix) DeleteBucketPolicy(ctx context.Context, bucket string) error { + return p.PutBucketPolicy(ctx, bucket, nil) +} + func (p *Posix) ChangeBucketOwner(ctx context.Context, bucket, newOwner string) error { _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index 60fbc8c9..d8b97cb0 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -44,6 +44,9 @@ var _ backend.Backend = &BackendMock{} // DeleteBucketFunc: func(contextMoqParam context.Context, deleteBucketInput *s3.DeleteBucketInput) error { // panic("mock out the DeleteBucket method") // }, +// DeleteBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) error { +// panic("mock out the DeleteBucketPolicy method") +// }, // DeleteBucketTaggingFunc: func(contextMoqParam context.Context, bucket string) error { // panic("mock out the DeleteBucketTagging method") // }, @@ -53,7 +56,7 @@ var _ backend.Backend = &BackendMock{} // DeleteObjectTaggingFunc: func(contextMoqParam context.Context, bucket string, object string) error { // panic("mock out the DeleteObjectTagging method") // }, -// DeleteObjectsFunc: func(contextMoqParam context.Context, deleteObjectsInput *s3.DeleteObjectsInput) (s3response.DeleteObjectsResult, error) { +// DeleteObjectsFunc: func(contextMoqParam context.Context, deleteObjectsInput *s3.DeleteObjectsInput) (s3response.DeleteResult, error) { // panic("mock out the DeleteObjects method") // }, // GetBucketAclFunc: func(contextMoqParam context.Context, getBucketAclInput *s3.GetBucketAclInput) ([]byte, error) { @@ -174,6 +177,9 @@ type BackendMock struct { // DeleteBucketFunc mocks the DeleteBucket method. DeleteBucketFunc func(contextMoqParam context.Context, deleteBucketInput *s3.DeleteBucketInput) error + // DeleteBucketPolicyFunc mocks the DeleteBucketPolicy method. + DeleteBucketPolicyFunc func(contextMoqParam context.Context, bucket string) error + // DeleteBucketTaggingFunc mocks the DeleteBucketTagging method. DeleteBucketTaggingFunc func(contextMoqParam context.Context, bucket string) error @@ -331,6 +337,13 @@ type BackendMock struct { // DeleteBucketInput is the deleteBucketInput argument value. DeleteBucketInput *s3.DeleteBucketInput } + // DeleteBucketPolicy holds details about calls to the DeleteBucketPolicy method. + DeleteBucketPolicy []struct { + // ContextMoqParam is the contextMoqParam argument value. + ContextMoqParam context.Context + // Bucket is the bucket argument value. + Bucket string + } // DeleteBucketTagging holds details about calls to the DeleteBucketTagging method. DeleteBucketTagging []struct { // ContextMoqParam is the contextMoqParam argument value. @@ -585,6 +598,7 @@ type BackendMock struct { lockCreateBucket sync.RWMutex lockCreateMultipartUpload sync.RWMutex lockDeleteBucket sync.RWMutex + lockDeleteBucketPolicy sync.RWMutex lockDeleteBucketTagging sync.RWMutex lockDeleteObject sync.RWMutex lockDeleteObjectTagging sync.RWMutex @@ -881,6 +895,42 @@ func (mock *BackendMock) DeleteBucketCalls() []struct { return calls } +// DeleteBucketPolicy calls DeleteBucketPolicyFunc. +func (mock *BackendMock) DeleteBucketPolicy(contextMoqParam context.Context, bucket string) error { + if mock.DeleteBucketPolicyFunc == nil { + panic("BackendMock.DeleteBucketPolicyFunc: method is nil but Backend.DeleteBucketPolicy was just called") + } + callInfo := struct { + ContextMoqParam context.Context + Bucket string + }{ + ContextMoqParam: contextMoqParam, + Bucket: bucket, + } + mock.lockDeleteBucketPolicy.Lock() + mock.calls.DeleteBucketPolicy = append(mock.calls.DeleteBucketPolicy, callInfo) + mock.lockDeleteBucketPolicy.Unlock() + return mock.DeleteBucketPolicyFunc(contextMoqParam, bucket) +} + +// DeleteBucketPolicyCalls gets all the calls that were made to DeleteBucketPolicy. +// Check the length with: +// +// len(mockedBackend.DeleteBucketPolicyCalls()) +func (mock *BackendMock) DeleteBucketPolicyCalls() []struct { + ContextMoqParam context.Context + Bucket string +} { + var calls []struct { + ContextMoqParam context.Context + Bucket string + } + mock.lockDeleteBucketPolicy.RLock() + calls = mock.calls.DeleteBucketPolicy + mock.lockDeleteBucketPolicy.RUnlock() + return calls +} + // DeleteBucketTagging calls DeleteBucketTaggingFunc. func (mock *BackendMock) DeleteBucketTagging(contextMoqParam context.Context, bucket string) error { if mock.DeleteBucketTaggingFunc == nil { diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 7332c2c4..4a55b30b 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -708,6 +708,17 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { }) } + err = auth.ValidatePolicyDocument(ctx.Body(), bucket, c.iam) + if err != nil { + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.logger, + Action: "PutBucketPolicy", + BucketOwner: parsedAcl.Owner, + }, + ) + } + err = c.be.PutBucketPolicy(ctx.Context(), bucket, ctx.Body()) return SendResponse(ctx, err, &MetaOpts{ @@ -1298,6 +1309,27 @@ func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { }) } + if ctx.Request().URI().QueryArgs().Has("policy") { + err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + if err != nil { + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.logger, + Action: "DeleteBucketPolicy", + BucketOwner: parsedAcl.Owner, + }) + } + + err = c.be.DeleteBucketPolicy(ctx.Context(), bucket) + return SendResponse(ctx, err, + &MetaOpts{ + Logger: c.logger, + Action: "DeleteBucketPolicy", + BucketOwner: parsedAcl.Owner, + Status: http.StatusNoContent, + }) + } + err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) if err != nil { return SendResponse(ctx, err, From af641e5368802884490bf9a2afa130c2cdf8ec8c Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Wed, 20 Mar 2024 17:31:52 -0400 Subject: [PATCH 2/3] feat: Added integration test cases for Put/Get/DeleteBucketPolicy actions. Made some bug fixes in these actions implementations --- auth/bucket_policy.go | 7 +- auth/bucket_policy_actions.go | 24 +- auth/bucket_policy_principals.go | 2 +- auth/bucket_policy_resources.go | 17 +- backend/posix/posix.go | 4 + s3api/controllers/base_test.go | 24 +- tests/integration/group-tests.go | 64 ++++ tests/integration/tests.go | 542 +++++++++++++++++++++++++++++++ tests/integration/utils.go | 36 +- 9 files changed, 698 insertions(+), 22 deletions(-) diff --git a/auth/bucket_policy.go b/auth/bucket_policy.go index 02a5c482..469d5bd2 100644 --- a/auth/bucket_policy.go +++ b/auth/bucket_policy.go @@ -45,10 +45,13 @@ type BucketPolicyItem struct { } func (bpi *BucketPolicyItem) Validate(bucket string, iam IAMService) error { + if err := bpi.Effect.Validate(); err != nil { + return err + } if err := bpi.Principals.Validate(iam); err != nil { return err } - if err := bpi.Effect.Validate(); err != nil { + if err := bpi.Resources.Validate(bucket); err != nil { return err } @@ -61,7 +64,7 @@ func (bpi *BucketPolicyItem) Validate(bucket string, iam IAMService) error { return fmt.Errorf("unsupported object action '%v' on the specified resources", action) } if !isObjectAction && !containsBucketAction { - return fmt.Errorf("unsupported bucket action '%v', on the specified resources", action) + return fmt.Errorf("unsupported bucket action '%v' on the specified resources", action) } } diff --git a/auth/bucket_policy_actions.go b/auth/bucket_policy_actions.go index ab80a383..09814b36 100644 --- a/auth/bucket_policy_actions.go +++ b/auth/bucket_policy_actions.go @@ -133,20 +133,31 @@ var supportedObjectActionList = map[Action]struct{}{ } // Validates Action: it should either wildcard match with supported actions list or be in it -func (a Action) IsValid() bool { +func (a Action) IsValid() error { + if !strings.HasPrefix(string(a), "s3:") { + return fmt.Errorf("invalid action: %v", a) + } + + if a == AllActions { + return nil + } + if a[len(a)-1] == '*' { pattern := strings.TrimSuffix(string(a), "*") for act := range supportedActionList { if strings.HasPrefix(string(act), pattern) { - return true + return nil } } - return false + return fmt.Errorf("invalid wildcard usage: %v prefix is not in the supported actions list", pattern) } _, found := supportedActionList[a] - return found + if !found { + return fmt.Errorf("unsupported action: %v", a) + } + return nil } // Checks if the action is object action @@ -203,8 +214,9 @@ func (a *Actions) UnmarshalJSON(data []byte) error { // Validates and adds a new Action to Actions map func (a Actions) Add(str string) error { action := Action(str) - if !action.IsValid() { - return fmt.Errorf("invalid action") + err := action.IsValid() + if err != nil { + return err } a[action] = struct{}{} diff --git a/auth/bucket_policy_principals.go b/auth/bucket_policy_principals.go index 7abe49e9..0ae98df9 100644 --- a/auth/bucket_policy_principals.go +++ b/auth/bucket_policy_principals.go @@ -79,7 +79,7 @@ func (p Principals) Validate(iam IAMService) error { return err } if len(accs) > 0 { - return fmt.Errorf("users doesn't exist: %v", accs) + return fmt.Errorf("user accounts don't exist: %v", accs) } return nil diff --git a/auth/bucket_policy_resources.go b/auth/bucket_policy_resources.go index be02d132..704ca884 100644 --- a/auth/bucket_policy_resources.go +++ b/auth/bucket_policy_resources.go @@ -58,15 +58,14 @@ func (r *Resources) UnmarshalJSON(data []byte) error { // Adds and validates a new resource to Resources map func (r Resources) Add(rc string) error { - _, found := r[rc] - if found { - return fmt.Errorf("duplicate resource") + ok, pattern := isValidResource(rc) + if !ok { + return fmt.Errorf("invalid resource: %v", rc) } - ok, pattern := isValidResource(rc) - - if !ok { - return fmt.Errorf("invalid resource") + _, found := r[pattern] + if found { + return fmt.Errorf("duplicate resource: %v", rc) } r[pattern] = struct{}{} @@ -100,7 +99,7 @@ func (r Resources) ContainsBucketPattern() bool { func (r Resources) Validate(bucket string) error { for resource := range r { if !strings.HasPrefix(resource, bucket) { - return fmt.Errorf("invalid bucket resource") + return fmt.Errorf("incorrect bucket name in %v", resource) } } @@ -118,7 +117,7 @@ func isValidResource(rc string) (isValid bool, pattern string) { return false, "" } // The resource can't start with / (bucket name comes first) - if res == "/" { + if strings.HasPrefix(res, "/") { return false, "" } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 19106444..a94f0609 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1863,6 +1863,10 @@ func (p *Posix) PutBucketPolicy(ctx context.Context, bucket string, policy []byt if policy == nil { if err := xattr.Remove(bucket, policykey); err != nil { + if isNoAttr(err) { + return nil + } + return fmt.Errorf("remove policy: %w", err) } diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index ae80d366..97a5fd9a 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -570,6 +570,19 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { ` + policyBody := ` + { + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::my-bucket/*" + } + ] + } + ` + s3ApiController := S3ApiController{ be: &BackendMock{ GetBucketAclFunc: func(context.Context, *s3.GetBucketAclInput) ([]byte, error) { @@ -667,12 +680,21 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { statusCode: 200, }, { - name: "Put-bucket-policy-success", + name: "Put-bucket-policy-invalid-body", app: app, args: args{ req: httptest.NewRequest(http.MethodPut, "/my-bucket?policy", nil), }, wantErr: false, + statusCode: 400, + }, + { + name: "Put-bucket-policy-success", + app: app, + args: args{ + req: httptest.NewRequest(http.MethodPut, "/my-bucket?policy", strings.NewReader(policyBody)), + }, + wantErr: false, statusCode: 200, }, { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index c09fecbd..009d978b 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -241,6 +241,41 @@ func TestGetBucketAcl(s *S3Conf) { GetBucketAcl_success(s) } +func TestPutBucketPolicy(s *S3Conf) { + PutBucketPolicy_non_existing_bucket(s) + PutBucketPolicy_invalid_effect(s) + PutBucketPolicy_empty_actions_string(s) + PutBucketPolicy_empty_actions_array(s) + PutBucketPolicy_invalid_action(s) + PutBucketPolicy_unsupported_action(s) + PutBucketPolicy_incorrect_action_wildcard_usage(s) + PutBucketPolicy_empty_principals_string(s) + PutBucketPolicy_empty_principals_array(s) + PutBucketPolicy_principals_incorrect_wildcard_usage(s) + PutBucketPolicy_non_existing_principals(s) + PutBucketPolicy_empty_resources_string(s) + PutBucketPolicy_empty_resources_array(s) + PutBucketPolicy_invalid_resource_prefix(s) + 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_success(s) +} + +func TestGetBucketPolicy(s *S3Conf) { + GetBucketPolicy_non_existing_bucket(s) + GetBucketPolicy_default_empty_policy(s) + GetBucketPolicy_success(s) +} + +func TestDeleteBucketPolicy(s *S3Conf) { + DeleteBucketPolicy_non_existing_bucket(s) + DeleteBucketPolicy_remove_before_setting(s) + DeleteBucketPolicy_success(s) +} + func TestFullFlow(s *S3Conf) { TestAuthentication(s) TestPresignedAuthentication(s) @@ -270,6 +305,9 @@ func TestFullFlow(s *S3Conf) { TestCompleteMultipartUpload(s) TestPutBucketAcl(s) TestGetBucketAcl(s) + TestPutBucketPolicy(s) + TestGetBucketPolicy(s) + TestDeleteBucketPolicy(s) } func TestPosix(s *S3Conf) { @@ -443,6 +481,32 @@ func GetIntTests() IntTests { "GetBucketAcl_non_existing_bucket": GetBucketAcl_non_existing_bucket, "GetBucketAcl_access_denied": GetBucketAcl_access_denied, "GetBucketAcl_success": GetBucketAcl_success, + "PutBucketPolicy_non_existing_bucket": PutBucketPolicy_non_existing_bucket, + "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_unsupported_action": PutBucketPolicy_unsupported_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_incorrect_wildcard_usage": PutBucketPolicy_principals_incorrect_wildcard_usage, + "PutBucketPolicy_non_existing_principals": PutBucketPolicy_non_existing_principals, + "PutBucketPolicy_empty_resources_string": PutBucketPolicy_empty_resources_string, + "PutBucketPolicy_empty_resources_array": PutBucketPolicy_empty_resources_array, + "PutBucketPolicy_invalid_resource_prefix": PutBucketPolicy_invalid_resource_prefix, + "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_bucket_action_on_object_resource": PutBucketPolicy_bucket_action_on_object_resource, + "PutBucketPolicy_success": PutBucketPolicy_success, + "GetBucketPolicy_non_existing_bucket": GetBucketPolicy_non_existing_bucket, + "GetBucketPolicy_default_empty_policy": GetBucketPolicy_default_empty_policy, + "GetBucketPolicy_success": GetBucketPolicy_success, + "DeleteBucketPolicy_non_existing_bucket": DeleteBucketPolicy_non_existing_bucket, + "DeleteBucketPolicy_remove_before_setting": DeleteBucketPolicy_remove_before_setting, + "DeleteBucketPolicy_success": DeleteBucketPolicy_success, "PutObject_overwrite_dir_obj": PutObject_overwrite_dir_obj, "PutObject_overwrite_file_obj": PutObject_overwrite_file_obj, "PutObject_dir_obj_with_data": PutObject_dir_obj_with_data, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index d8fb45b2..2d509301 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -5177,6 +5177,548 @@ func GetBucketAcl_success(s *S3Conf) error { }) } +func PutBucketPolicy_non_existing_bucket(s *S3Conf) error { + testName := "PutBucketPolicy_non_existing_bucket" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + doc := genPolicyDoc("Allow", `"*"`, `"s3:*"`, fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket)) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: getPtr("non_existing_bucket"), + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_invalid_effect(s *S3Conf) error { + testName := "PutBucketPolicy_invalid_effect" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("invalid_effect", `"*"`, `"s3:*"`, `"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("invalid effect: invalid_effect")); err != nil { + return err + } + return nil + }) +} + +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("actions can't be empty")); 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("actions can't be empty")); 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("invalid action: ListObjects")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_unsupported_action(s *S3Conf) error { + testName := "PutBucketPolicy_unsupported_action" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `"*"`, `"s3:PutLifecycleConfiguration"`, `"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("unsupported action: s3:PutLifecycleConfiguration")); 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:::*"`) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError("invalid wildcard usage: s3:hello prefix is not in the supported actions list")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_empty_principals_string(s *S3Conf) error { + testName := "PutBucketPolicy_empty_principals_string" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `""`, `"s3:*"`, `"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("principals can't be empty")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_empty_principals_array(s *S3Conf) error { + testName := "PutBucketPolicy_empty_principals_array" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `[]`, `"s3:*"`, `"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("principals can't be empty")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_principals_incorrect_wildcard_usage(s *S3Conf) error { + testName := "PutBucketPolicy_principals_incorrect_wildcard_usage" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `["*", "grt1"]`, `"s3:*"`, `"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("principals should either contain * or user access keys")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_non_existing_principals(s *S3Conf) error { + testName := "PutBucketPolicy_non_existing_principals" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `["a_rarely_existing_user_account_1", "a_rarely_existing_user_account_2"]`, `"s3:*"`, `"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(fmt.Sprintf("user accounts don't exist: %v", []string{"a_rarely_existing_user_account_1", "a_rarely_existing_user_account_2"}))); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_empty_resources_string(s *S3Conf) error { + testName := "PutBucketPolicy_empty_resources_string" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `["*"]`, `"s3:*"`, `""`) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError("resources can't be empty")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_empty_resources_array(s *S3Conf) error { + testName := "PutBucketPolicy_empty_resources_array" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `["*"]`, `"s3:*"`, `[]`) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError("resources can't be empty")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_invalid_resource_prefix(s *S3Conf) error { + testName := "PutBucketPolicy_invalid_resource_prefix" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + resource := fmt.Sprintf(`"arn:aws:iam:::%v"`, bucket) + doc := genPolicyDoc("Allow", `["*"]`, `"s3:*"`, resource) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("invalid resource: %v", resource[1:len(resource)-1]))); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_invalid_resource_with_starting_slash(s *S3Conf) error { + testName := "PutBucketPolicy_invalid_resource_with_starting_slash" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + resource := fmt.Sprintf(`"arn:aws:s3:::/%v"`, bucket) + doc := genPolicyDoc("Allow", `["*"]`, `"s3:*"`, resource) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("invalid resource: %v", resource[1:len(resource)-1]))); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_duplicate_resource(s *S3Conf) error { + testName := "PutBucketPolicy_duplicate_resource" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + resource := fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket) + doc := genPolicyDoc("Allow", `["*"]`, `"s3:*"`, fmt.Sprintf("[%v, %v]", resource, resource)) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("duplicate resource: %v", resource[1:len(resource)-1]))); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_incorrect_bucket_name(s *S3Conf) error { + testName := "PutBucketPolicy_incorrect_bucket_name" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + resource := fmt.Sprintf(`"arn:aws:s3:::prefix-%v"`, bucket) + doc := genPolicyDoc("Allow", `["*"]`, `"s3:*"`, resource) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("incorrect bucket name in prefix-%v", bucket))); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_object_action_on_bucket_resource(s *S3Conf) error { + testName := "PutBucketPolicy_object_action_on_bucket_resource" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + resource := fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket) + doc := genPolicyDoc("Allow", `["*"]`, `"s3:ListObjects"`, resource) + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + + if err := checkApiErr(err, getMalformedPolicyError("unsupported object action 's3:ListObjects' on the specified resources")); err != nil { + return err + } + return nil + }) +} + +func PutBucketPolicy_bucket_action_on_object_resource(s *S3Conf) error { + testName := "PutBucketPolicy_object_action_on_bucket_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("unsupported bucket action 's3:DeleteBucket' on the specified resources")); 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 { + err := createUsers(s, []user{ + {"grt1", "grt1secret", "user"}, + {"grt2", "grt2secret", "user"}, + }) + if err != nil { + return err + } + + bucketResource := fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket) + objectResource := fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket) + + for _, doc := range []string{ + genPolicyDoc("Allow", `["grt1", "grt2"]`, `["s3:DeleteBucket", "s3:GetBucketAcl"]`, bucketResource), + genPolicyDoc("Deny", `"*"`, `"s3:DeleteBucket"`, fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket)), + genPolicyDoc("Allow", `"grt1"`, `["s3:CompleteMultipartUpload", "s3:UploadPart", "s3:HeadBucket"]`, fmt.Sprintf(`[%v, %v]`, bucketResource, objectResource)), + genPolicyDoc("Allow", `"*"`, `"s3:*"`, fmt.Sprintf(`[%v, %v]`, bucketResource, objectResource)), + genPolicyDoc("Allow", `"*"`, `"s3:Get*"`, objectResource), + genPolicyDoc("Deny", `"*"`, `"s3:Create*"`, fmt.Sprintf(`[%v, %v]`, bucketResource, objectResource)), + } { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &doc, + }) + cancel() + if err != nil { + return err + } + } + + return nil + }) +} + +func GetBucketPolicy_non_existing_bucket(s *S3Conf) error { + testName := "GetBucketPolicy_non_existing_bucket" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.GetBucketPolicy(ctx, &s3.GetBucketPolicyInput{ + Bucket: getPtr("non_existing_bucket"), + }) + cancel() + + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { + return err + } + return nil + }) +} + +func GetBucketPolicy_default_empty_policy(s *S3Conf) error { + testName := "GetBucketPolicy_default_empty_policy" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetBucketPolicy(ctx, &s3.GetBucketPolicyInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if out.Policy != nil { + return fmt.Errorf("expected policy to be nil, instead got %s", *out.Policy) + } + + return nil + }) +} + +func GetBucketPolicy_success(s *S3Conf) error { + testName := "GetBucketPolicy_success" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `"*"`, `["s3:DeleteBucket", "s3:GetBucketTagging"]`, 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 != nil { + return err + } + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetBucketPolicy(ctx, &s3.GetBucketPolicyInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if out.Policy == nil { + return fmt.Errorf("expected non nil policy result") + } + + if *out.Policy != doc { + return fmt.Errorf("expected the bucket policy to be %v, instead got %v", doc, *out.Policy) + } + + return nil + }) +} + +func DeleteBucketPolicy_non_existing_bucket(s *S3Conf) error { + testName := "DeleteBucketPolicy_non_existing_bucket" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.DeleteBucketPolicy(ctx, &s3.DeleteBucketPolicyInput{ + Bucket: getPtr("non_existing_bucket"), + }) + cancel() + + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { + return err + } + return nil + }) +} + +func DeleteBucketPolicy_remove_before_setting(s *S3Conf) error { + testName := "DeleteBucketPolicy_remove_before_setting" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.DeleteBucketPolicy(ctx, &s3.DeleteBucketPolicyInput{ + Bucket: &bucket, + }) + cancel() + + return err + }) +} + +func DeleteBucketPolicy_success(s *S3Conf) error { + testName := "DeleteBucketPolicy_success" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + doc := genPolicyDoc("Allow", `"*"`, `["s3:DeleteBucket", "s3:GetBucketTagging"]`, 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 != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.DeleteBucketPolicy(ctx, &s3.DeleteBucketPolicyInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetBucketPolicy(ctx, &s3.GetBucketPolicyInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if out.Policy != nil { + return fmt.Errorf("expected policy to be nil, instead got %s", *out.Policy) + } + + return err + }) +} + // IAM related tests // multi-user iam tests func IAM_user_access_denied(s *S3Conf) error { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 563ae283..626907c3 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -219,12 +219,17 @@ func checkApiErr(err error, apiErr s3err.APIError) error { } var ae smithy.APIError if errors.As(err, &ae) { - if ae.ErrorCode() == apiErr.Code && ae.ErrorMessage() == apiErr.Description { - return nil + if ae.ErrorCode() != apiErr.Code { + return fmt.Errorf("expected error code to be %v, instead got %v", apiErr.Code, ae.ErrorCode()) } - return fmt.Errorf("expected %v, instead got %v", apiErr.Code, ae.ErrorCode()) + if ae.ErrorMessage() != apiErr.Description { + return fmt.Errorf("expected error message to be %v, instead got %v", apiErr.Description, ae.ErrorMessage()) + } + + return nil } + return fmt.Errorf("expected aws api error, instead got: %w", err) } @@ -603,3 +608,28 @@ func changeAuthCred(uri, newVal string, index int) (string, error) { return urlParsed.String(), nil } + +func genPolicyDoc(effect, principal, action, resource string) string { + jsonTemplate := ` + { + "Statement": [ + { + "Effect": "%s", + "Principal": %s, + "Action": %s, + "Resource": %s + } + ] + } + ` + + return fmt.Sprintf(jsonTemplate, effect, principal, action, resource) +} + +func getMalformedPolicyError(msg string) s3err.APIError { + return s3err.APIError{ + Code: "MalformedPolicy", + Description: msg, + HTTPStatusCode: http.StatusBadRequest, + } +} From 754c221c4d9eee40030dfec15f9b8790b43f154f Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Mon, 25 Mar 2024 16:00:35 -0400 Subject: [PATCH 3/3] feat: Added bucket policy access verifier function implementation. Changed the default behaviour of bucket ACLs. Fixed the supported actions list for bucket policy. Implemented Copy* actions access checker function --- auth/acl.go | 90 +++++++- auth/bucket_policy.go | 52 +++++ auth/bucket_policy_actions.go | 196 +++++++++-------- auth/bucket_policy_principals.go | 11 + auth/bucket_policy_resources.go | 17 ++ backend/azure/azure.go | 10 - backend/posix/posix.go | 17 -- s3api/controllers/base.go | 347 +++++++++++++++++++++++++++---- tests/integration/tests.go | 36 ++-- 9 files changed, 592 insertions(+), 184 deletions(-) diff --git a/auth/acl.go b/auth/acl.go index 166b6868..6e6e501f 100644 --- a/auth/acl.go +++ b/auth/acl.go @@ -15,12 +15,14 @@ package auth import ( + "context" "encoding/json" "fmt" "strings" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" ) @@ -203,13 +205,14 @@ func splitUnique(s, divider string) []string { return result } -func VerifyACL(acl ACL, access string, permission types.Permission, isRoot bool) error { - if isRoot { - return nil - } +func verifyACL(acl ACL, access string, permission types.Permission) error { + // Default disabled ACL case + if acl.ACL == "" && len(acl.Grantees) == 0 { + if acl.Owner == access { + return nil + } - if acl.Owner == access { - return nil + return s3err.GetAPIError(s3err.ErrAccessDenied) } if acl.ACL != "" { @@ -273,3 +276,78 @@ func IsAdminOrOwner(acct Account, isRoot bool, acl ACL) error { // Return access denied in all other cases return s3err.GetAPIError(s3err.ErrAccessDenied) } + +type AccessOptions struct { + Acl ACL + AclPermission types.Permission + IsRoot bool + Acc Account + Bucket string + Object string + Action Action +} + +func VerifyAccess(ctx context.Context, be backend.Backend, opts AccessOptions) error { + if opts.IsRoot { + return nil + } + if opts.Acc.Role == RoleAdmin { + return nil + } + if opts.Acc.Access == opts.Acl.Owner { + return nil + } + + if err := verifyACL(opts.Acl, opts.Acc.Access, opts.AclPermission); err != nil { + return err + } + if err := verifyBucketPolicy(ctx, be, opts.Acc.Access, opts.Bucket, opts.Object, opts.Action); err != nil { + return err + } + + return nil +} + +func VerifyObjectCopyAccess(ctx context.Context, be backend.Backend, copySource string, opts AccessOptions) error { + if opts.IsRoot { + return nil + } + if opts.Acc.Role == RoleAdmin { + return nil + } + + // Verify destination bucket access + if err := VerifyAccess(ctx, be, opts); err != nil { + return err + } + // Verify source bucket access + srcBucket, srcObject, found := strings.Cut(copySource, "/") + if !found { + return s3err.GetAPIError(s3err.ErrInvalidCopySource) + } + + // Get source bucket ACL + srcBucketACLBytes, err := be.GetBucketAcl(ctx, &s3.GetBucketAclInput{Bucket: &srcBucket}) + if err != nil { + return err + } + + var srcBucketAcl ACL + if err := json.Unmarshal(srcBucketACLBytes, &srcBucketAcl); err != nil { + return err + } + + if err := VerifyAccess(ctx, be, AccessOptions{ + Acl: srcBucketAcl, + AclPermission: types.PermissionRead, + IsRoot: opts.IsRoot, + Acc: opts.Acc, + Bucket: srcBucket, + Object: srcObject, + Action: GetObjectAction, + }); err != nil { + return err + } + + return nil +} diff --git a/auth/bucket_policy.go b/auth/bucket_policy.go index 469d5bd2..ed9545aa 100644 --- a/auth/bucket_policy.go +++ b/auth/bucket_policy.go @@ -15,10 +15,12 @@ package auth import ( + "context" "encoding/json" "fmt" "net/http" + "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" ) @@ -37,6 +39,16 @@ func (bp *BucketPolicy) Validate(bucket string, iam IAMService) error { return nil } +func (bp *BucketPolicy) isAllowed(principal string, action Action, resource string) bool { + for _, statement := range bp.Statement { + if statement.isAllowed(principal, action, resource) { + return true + } + } + + return false +} + type BucketPolicyItem struct { Effect BucketPolicyAccessType `json:"Effect"` Principals Principals `json:"Principal"` @@ -71,6 +83,19 @@ func (bpi *BucketPolicyItem) Validate(bucket string, iam IAMService) error { return nil } +func (bpi *BucketPolicyItem) isAllowed(principal string, action Action, resource string) bool { + if bpi.Principals.Contains(principal) && bpi.Actions.FindMatch(action) && bpi.Resources.FindMatch(resource) { + switch bpi.Effect { + case BucketPolicyAccessTypeAllow: + return true + case BucketPolicyAccessTypeDeny: + return false + } + } + + return false +} + func getMalformedPolicyError(err error) error { return s3err.APIError{ Code: "MalformedPolicy", @@ -91,3 +116,30 @@ func ValidatePolicyDocument(policyBin []byte, bucket string, iam IAMService) err return nil } + +func verifyBucketPolicy(ctx context.Context, be backend.Backend, access, bucket, object string, action Action) error { + policyDoc, err := be.GetBucketPolicy(ctx, bucket) + if err != nil { + return err + } + // If bucket policy is not set + if len(policyDoc) == 0 { + return nil + } + + var bucketPolicy BucketPolicy + if err := json.Unmarshal(policyDoc, &bucketPolicy); err != nil { + return err + } + + resource := bucket + if object != "" { + resource += "" + object + } + + if !bucketPolicy.isAllowed(access, action, resource) { + return s3err.GetAPIError(s3err.ErrAccessDenied) + } + + return nil +} diff --git a/auth/bucket_policy_actions.go b/auth/bucket_policy_actions.go index 09814b36..028a7b9c 100644 --- a/auth/bucket_policy_actions.go +++ b/auth/bucket_policy_actions.go @@ -23,113 +23,79 @@ import ( type Action string const ( - ListBuckets Action = "s3:ListBuckets" - HeadBucketAction Action = "s3:HeadBucket" - GetBucketAclAction Action = "s3:GetBucketAcl" - CreateBucketAction Action = "s3:CreateBucket" - PutBucketAclAction Action = "s3:PutBucketAcl" - DeleteBucketAction Action = "s3:DeleteBucket" - PutBucketVersioningAction Action = "s3:PutBucketVersioning" - GetBucketVersioningAction Action = "s3:GetBucketVersioning" - PutBucketPolicyAction Action = "s3:PutBucketPolicy" - GetBucketPolicyAction Action = "s3:GetBucketPolicy" - CreateMultipartUploadAction Action = "s3:CreateMultipartUpload" - CompleteMultipartUploadAction Action = "s3:CompleteMultipartUpload" - AbortMultipartUploadAction Action = "s3:AbortMultipartUpload" - ListMultipartUploadsAction Action = "s3:ListMultipartUploads" - ListPartsAction Action = "s3:ListParts" - UploadPartAction Action = "s3:UploadPart" - UploadPartCopyAction Action = "s3:UploadPartCopy" - PutObjectAction Action = "s3:PutObject" - HeadObjectAction Action = "s3:HeadObject" - GetObjectAction Action = "s3:GetObject" - GetObjectAclAction Action = "s3:GetObjectAcl" - GetObjectAttributesAction Action = "s3:GetObjectAttributes" - CopyObjectAction Action = "s3:CopyObject" - ListObjectsAction Action = "s3:ListObjects" - ListObjectsV2Action Action = "s3:ListObjectsV2" - DeleteObjectAction Action = "s3:DeleteObject" - DeleteObjectsAction Action = "s3:DeleteObjects" - PutObjectAclAction Action = "s3:PutObjectAcl" - ListObjectVersionsAction Action = "s3:ListObjectVersions" - RestoreObjectAction Action = "s3:RestoreObject" - SelectObjectContentAction Action = "s3:SelectObjectContent" - GetBucketTaggingAction Action = "s3:GetBucketTagging" - PutBucketTaggingAction Action = "s3:PutBucketTagging" - DeleteBucketTaggingAction Action = "s3:DeleteBucketTagging" - GetObjectTaggingAction Action = "s3:GetObjectTagging" - PutObjectTaggingAction Action = "s3:PutObjectTagging" - DeleteObjectTaggingAction Action = "s3:DeleteObjectTagging" - AllActions Action = "s3:*" + GetBucketAclAction Action = "s3:GetBucketAcl" + CreateBucketAction Action = "s3:CreateBucket" + PutBucketAclAction Action = "s3:PutBucketAcl" + DeleteBucketAction Action = "s3:DeleteBucket" + PutBucketVersioningAction Action = "s3:PutBucketVersioning" + GetBucketVersioningAction Action = "s3:GetBucketVersioning" + PutBucketPolicyAction Action = "s3:PutBucketPolicy" + GetBucketPolicyAction Action = "s3:GetBucketPolicy" + DeleteBucketPolicyAction Action = "s3:DeleteBucketPolicy" + AbortMultipartUploadAction Action = "s3:AbortMultipartUpload" + ListMultipartUploadPartsAction Action = "s3:ListMultipartUploadParts" + ListBucketMultipartUploadsAction Action = "s3:ListBucketMultipartUploads" + PutObjectAction Action = "s3:PutObject" + GetObjectAction Action = "s3:GetObject" + DeleteObjectAction Action = "s3:DeleteObject" + GetObjectAclAction Action = "s3:GetObjectAcl" + GetObjectAttributesAction Action = "s3:GetObjectAttributes" + PutObjectAclAction Action = "s3:PutObjectAcl" + RestoreObjectAction Action = "s3:RestoreObject" + GetBucketTaggingAction Action = "s3:GetBucketTagging" + PutBucketTaggingAction Action = "s3:PutBucketTagging" + GetObjectTaggingAction Action = "s3:GetObjectTagging" + PutObjectTaggingAction Action = "s3:PutObjectTagging" + DeleteObjectTaggingAction Action = "s3:DeleteObjectTagging" + ListBucketVersionsAction Action = "s3:ListBucketVersions" + ListBucketAction Action = "s3:ListBucket" + AllActions Action = "s3:*" ) var supportedActionList = map[Action]struct{}{ - ListBuckets: {}, - HeadBucketAction: {}, - GetBucketAclAction: {}, - CreateBucketAction: {}, - PutBucketAclAction: {}, - DeleteBucketAction: {}, - PutBucketVersioningAction: {}, - GetBucketVersioningAction: {}, - PutBucketPolicyAction: {}, - GetBucketPolicyAction: {}, - CreateMultipartUploadAction: {}, - CompleteMultipartUploadAction: {}, - AbortMultipartUploadAction: {}, - ListMultipartUploadsAction: {}, - ListPartsAction: {}, - UploadPartAction: {}, - UploadPartCopyAction: {}, - PutObjectAction: {}, - HeadObjectAction: {}, - GetObjectAction: {}, - GetObjectAclAction: {}, - GetObjectAttributesAction: {}, - CopyObjectAction: {}, - ListObjectsAction: {}, - ListObjectsV2Action: {}, - DeleteObjectAction: {}, - DeleteObjectsAction: {}, - PutObjectAclAction: {}, - ListObjectVersionsAction: {}, - RestoreObjectAction: {}, - SelectObjectContentAction: {}, - GetBucketTaggingAction: {}, - PutBucketTaggingAction: {}, - DeleteBucketTaggingAction: {}, - GetObjectTaggingAction: {}, - PutObjectTaggingAction: {}, - DeleteObjectTaggingAction: {}, - AllActions: {}, + GetBucketAclAction: {}, + CreateBucketAction: {}, + PutBucketAclAction: {}, + DeleteBucketAction: {}, + PutBucketVersioningAction: {}, + GetBucketVersioningAction: {}, + PutBucketPolicyAction: {}, + GetBucketPolicyAction: {}, + DeleteBucketPolicyAction: {}, + AbortMultipartUploadAction: {}, + ListMultipartUploadPartsAction: {}, + ListBucketMultipartUploadsAction: {}, + PutObjectAction: {}, + GetObjectAction: {}, + DeleteObjectAction: {}, + GetObjectAclAction: {}, + GetObjectAttributesAction: {}, + PutObjectAclAction: {}, + RestoreObjectAction: {}, + GetBucketTaggingAction: {}, + PutBucketTaggingAction: {}, + GetObjectTaggingAction: {}, + PutObjectTaggingAction: {}, + DeleteObjectTaggingAction: {}, + ListBucketVersionsAction: {}, + ListBucketAction: {}, + AllActions: {}, } var supportedObjectActionList = map[Action]struct{}{ - CreateMultipartUploadAction: {}, - CompleteMultipartUploadAction: {}, - AbortMultipartUploadAction: {}, - ListMultipartUploadsAction: {}, - ListPartsAction: {}, - UploadPartAction: {}, - UploadPartCopyAction: {}, - PutObjectAction: {}, - HeadObjectAction: {}, - GetObjectAction: {}, - GetObjectAclAction: {}, - GetObjectAttributesAction: {}, - CopyObjectAction: {}, - ListObjectsAction: {}, - ListObjectsV2Action: {}, - DeleteObjectAction: {}, - DeleteObjectsAction: {}, - PutObjectAclAction: {}, - ListObjectVersionsAction: {}, - RestoreObjectAction: {}, - SelectObjectContentAction: {}, - GetObjectTaggingAction: {}, - PutObjectTaggingAction: {}, - DeleteObjectTaggingAction: {}, - AllActions: {}, + AbortMultipartUploadAction: {}, + ListMultipartUploadPartsAction: {}, + PutObjectAction: {}, + GetObjectAction: {}, + DeleteObjectAction: {}, + GetObjectAclAction: {}, + GetObjectAttributesAction: {}, + PutObjectAclAction: {}, + RestoreObjectAction: {}, + GetObjectTaggingAction: {}, + PutObjectTaggingAction: {}, + DeleteObjectTaggingAction: {}, + AllActions: {}, } // Validates Action: it should either wildcard match with supported actions list or be in it @@ -177,6 +143,14 @@ func (a Action) IsObjectAction() bool { 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 +} + type Actions map[Action]struct{} // Override UnmarshalJSON method to decode both []string and string properties @@ -222,3 +196,23 @@ func (a Actions) Add(str string) error { a[action] = struct{}{} return nil } + +func (a Actions) FindMatch(action Action) bool { + _, ok := a[AllActions] + if ok { + return true + } + // First O(1) check for non wildcard actions + _, found := a[action] + if found { + return true + } + + for act := range a { + if strings.HasSuffix(string(act), "*") && act.WildCardMatch(action) { + return true + } + } + + return false +} diff --git a/auth/bucket_policy_principals.go b/auth/bucket_policy_principals.go index 0ae98df9..d09cc554 100644 --- a/auth/bucket_policy_principals.go +++ b/auth/bucket_policy_principals.go @@ -84,3 +84,14 @@ func (p Principals) Validate(iam IAMService) error { return nil } + +func (p Principals) Contains(userAccess string) bool { + // "*" means it matches for any user account + _, ok := p["*"] + if ok { + return true + } + + _, found := p[userAccess] + return found +} diff --git a/auth/bucket_policy_resources.go b/auth/bucket_policy_resources.go index 704ca884..3c05552b 100644 --- a/auth/bucket_policy_resources.go +++ b/auth/bucket_policy_resources.go @@ -106,6 +106,23 @@ func (r Resources) Validate(bucket string) error { return nil } +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 + } + } + } + + return false +} + // 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/backend/azure/azure.go b/backend/azure/azure.go index 2961de64..7e489cf0 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -466,16 +466,6 @@ func (az *Azure) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3 return nil, azureErrToS3Err(err) } - dstContainerAcl, err := getAclFromMetadata(res.Metadata, aclKeyCapital) - if err != nil { - return nil, err - } - - err = auth.VerifyACL(*dstContainerAcl, *input.ExpectedBucketOwner, types.PermissionWrite, false) - if err != nil { - return nil, err - } - if strings.Join([]string{*input.Bucket, *input.Key}, "/") == *input.CopySource && isMetaSame(res.Metadata, input.Metadata) { return nil, s3err.GetAPIError(s3err.ErrInvalidCopyDest) } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index a94f0609..f42d864f 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1423,7 +1423,6 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3. } dstBucket := *input.Bucket dstObject := *input.Key - owner := *input.ExpectedBucketOwner _, err := os.Stat(srcBucket) if errors.Is(err, fs.ErrNotExist) { @@ -1441,22 +1440,6 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3. return nil, fmt.Errorf("stat bucket: %w", err) } - dstBucketACLBytes, err := xattr.Get(dstBucket, aclkey) - if err != nil { - return nil, fmt.Errorf("get dst bucket acl tag: %w", err) - } - - var dstBucketACL auth.ACL - err = json.Unmarshal(dstBucketACLBytes, &dstBucketACL) - if err != nil { - return nil, fmt.Errorf("parse dst bucket acl: %w", err) - } - - err = auth.VerifyACL(dstBucketACL, owner, types.PermissionWrite, false) - if err != nil { - return nil, err - } - objPath := filepath.Join(srcBucket, srcObject) f, err := os.Open(objPath) if errors.Is(err, fs.ErrNotExist) { diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 4a55b30b..7467adf5 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -80,7 +80,15 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("tagging") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.GetObjectTaggingAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -139,7 +147,15 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { } } - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.ListMultipartUploadPartsAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -169,7 +185,15 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("acl") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ_ACP", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionReadAcp, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.GetObjectAclAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -191,7 +215,15 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { } if attrs := ctx.Get("X-Amz-Object-Attributes"); attrs != "" { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.GetObjectAttributesAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -218,7 +250,15 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { }) } - err := auth.VerifyACL(parsedAcl, acct.Access, "READ_ACP", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.GetObjectAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -341,7 +381,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) if ctx.Request().URI().QueryArgs().Has("tagging") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.GetBucketTaggingAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -378,7 +425,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("versioning") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.GetBucketVersioningAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -407,7 +461,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("policy") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.GetBucketPolicyAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -427,7 +488,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("versions") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.ListBucketVersionsAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -464,7 +532,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("acl") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ_ACP", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionReadAcp, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.GetBucketAclAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -490,7 +565,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("uploads") { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.ListBucketMultipartUploadsAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -525,7 +607,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { } if ctx.QueryInt("list-type") == 2 { - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.ListBucketAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -560,7 +649,14 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { }) } - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.ListBucketAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -640,7 +736,14 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { tags[tag.Key] = tag.Value } - err = auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.PutBucketTaggingAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -661,7 +764,14 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { if ctx.Request().URI().QueryArgs().Has("versioning") { parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.PutBucketVersioningAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -698,7 +808,14 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { if ctx.Request().URI().QueryArgs().Has("policy") { parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.PutBucketPolicyAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -735,7 +852,14 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { var accessControlPolicy auth.AccessControlPolicy parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE_ACP", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWriteAcp, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.PutBucketAclAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -855,9 +979,7 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { } defACL := auth.ACL{ - ACL: "private", - Owner: acct.Access, - Grantees: []auth.Grantee{}, + Owner: acct.Access, } updAcl, err := auth.UpdateACL(&s3.PutBucketAclInput{ @@ -958,7 +1080,15 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { tags[tag.Key] = tag.Value } - err = auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: keyStart, + Action: auth.PutBucketTaggingAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -992,6 +1122,24 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { }) } + err := auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: keyStart, + Action: auth.PutObjectAction, + }) + if err != nil { + return SendXMLResponse(ctx, nil, err, + &MetaOpts{ + Logger: c.logger, + Action: "UploadPartCopy", + BucketOwner: parsedAcl.Owner, + }) + } + resp, err := c.be.UploadPartCopy(ctx.Context(), &s3.UploadPartCopyInput{ Bucket: &bucket, Key: &keyStart, @@ -1021,7 +1169,15 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { }) } - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: keyStart, + Action: auth.PutObjectAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1160,7 +1316,15 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { } if copySource != "" { - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyObjectCopyAccess(ctx.Context(), c.be, copySource, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: keyStart, + Action: auth.PutObjectAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -1233,7 +1397,15 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { metadata := utils.GetUserMetaData(&ctx.Request().Header) - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: keyStart, + Action: auth.PutObjectAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1289,7 +1461,14 @@ func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) if ctx.Request().URI().QueryArgs().Has("tagging") { - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.PutBucketTaggingAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1310,7 +1489,14 @@ func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("policy") { - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.DeleteBucketPolicyAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1330,7 +1516,14 @@ func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { }) } - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.DeleteBucketAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1369,7 +1562,14 @@ func (c S3ApiController) DeleteObjects(ctx *fiber.Ctx) error { }) } - err = auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.DeleteObjectAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1409,12 +1609,20 @@ func (c S3ApiController) DeleteActions(ctx *fiber.Ctx) error { } if ctx.Request().URI().QueryArgs().Has("tagging") { - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.DeleteObjectTaggingAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ Logger: c.logger, - Action: "RemoveObjectTagging", + Action: "DeleteObjectTagging", BucketOwner: parsedAcl.Owner, }) } @@ -1434,7 +1642,15 @@ func (c S3ApiController) DeleteActions(ctx *fiber.Ctx) error { expectedBucketOwner := ctx.Get("X-Amz-Expected-Bucket-Owner") requestPayer := ctx.Get("X-Amz-Request-Payer") - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.AbortMultipartUploadAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1461,7 +1677,15 @@ func (c S3ApiController) DeleteActions(ctx *fiber.Ctx) error { }) } - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.DeleteObjectAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1494,7 +1718,14 @@ func (c S3ApiController) HeadBucket(ctx *fiber.Ctx) error { isRoot := ctx.Locals("isRoot").(bool) parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.ListBucketAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1531,7 +1762,15 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { key = strings.Join([]string{key, keyEnd}, "/") } - err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.GetObjectAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1637,7 +1876,15 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { }) } - err = auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.RestoreObjectAction, + }) if err != nil { return SendResponse(ctx, err, &MetaOpts{ @@ -1675,7 +1922,15 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { }) } - err = auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot) + err = auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.GetObjectAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -1717,7 +1972,15 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { }) } - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.PutObjectAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ @@ -1756,7 +2019,15 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { }) } - err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot) + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Acl: parsedAcl, + AclPermission: types.PermissionWrite, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Object: key, + Action: auth.PutObjectAction, + }) if err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{ diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 2d509301..aa100d2d 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -1448,8 +1448,6 @@ func PresignedAuth_Put_GetObject_with_data(s *S3Conf) error { return fmt.Errorf("read get object response body %w", err) } - fmt.Println(resp.Request.Method, resp.ContentLength, string(respBody)) - if string(respBody) != data { return fmt.Errorf("expected get object response body to be %v, instead got %s", data, respBody) } @@ -3201,28 +3199,35 @@ func CopyObject_not_owned_source_bucket(s *S3Conf) error { } usr := user{ - access: "admin1", - secret: "admin1secret", - role: "admin", + access: "grt1", + secret: "grt1secret", + role: "user", } cfg := *s cfg.awsID = usr.access cfg.awsSecret = usr.secret + userS3Client := s3.NewFromConfig(cfg.Config()) + err = createUsers(s, []user{usr}) if err != nil { return err } dstBucket := getBucketName() - err = setup(&cfg, dstBucket) + err = setup(s, dstBucket) + if err != nil { + return err + } + + err = changeBucketsOwner(s, []string{bucket}, usr.access) if err != nil { return err } ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{ + _, err = userS3Client.CopyObject(ctx, &s3.CopyObjectInput{ Bucket: &dstBucket, Key: getPtr("obj-1"), CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, srcObj)), @@ -5378,9 +5383,16 @@ func PutBucketPolicy_non_existing_principals(s *S3Conf) error { }) cancel() - if err := checkApiErr(err, getMalformedPolicyError(fmt.Sprintf("user accounts don't exist: %v", []string{"a_rarely_existing_user_account_1", "a_rarely_existing_user_account_2"}))); err != nil { - return err + apiErr1 := getMalformedPolicyError(fmt.Sprintf("user accounts don't exist: %v", []string{"a_rarely_existing_user_account_1", "a_rarely_existing_user_account_2"})) + apiErr2 := getMalformedPolicyError(fmt.Sprintf("user accounts don't exist: %v", []string{"a_rarely_existing_user_account_2", "a_rarely_existing_user_account_1"})) + + err1 := checkApiErr(err, apiErr1) + err2 := checkApiErr(err, apiErr2) + + if err1 != nil && err2 != nil { + return err1 } + return nil }) } @@ -5507,7 +5519,7 @@ func PutBucketPolicy_object_action_on_bucket_resource(s *S3Conf) error { testName := "PutBucketPolicy_object_action_on_bucket_resource" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { resource := fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket) - doc := genPolicyDoc("Allow", `["*"]`, `"s3:ListObjects"`, resource) + doc := genPolicyDoc("Allow", `["*"]`, `"s3:PutObjectTagging"`, resource) ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ @@ -5516,7 +5528,7 @@ func PutBucketPolicy_object_action_on_bucket_resource(s *S3Conf) error { }) cancel() - if err := checkApiErr(err, getMalformedPolicyError("unsupported object action 's3:ListObjects' on the specified resources")); err != nil { + if err := checkApiErr(err, getMalformedPolicyError("unsupported object action 's3:PutObjectTagging' on the specified resources")); err != nil { return err } return nil @@ -5560,7 +5572,7 @@ func PutBucketPolicy_success(s *S3Conf) error { for _, doc := range []string{ genPolicyDoc("Allow", `["grt1", "grt2"]`, `["s3:DeleteBucket", "s3:GetBucketAcl"]`, bucketResource), genPolicyDoc("Deny", `"*"`, `"s3:DeleteBucket"`, fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket)), - genPolicyDoc("Allow", `"grt1"`, `["s3:CompleteMultipartUpload", "s3:UploadPart", "s3:HeadBucket"]`, fmt.Sprintf(`[%v, %v]`, bucketResource, objectResource)), + genPolicyDoc("Allow", `"grt1"`, `["s3:PutBucketVersioning", "s3:ListMultipartUploadParts", "s3:ListBucket"]`, fmt.Sprintf(`[%v, %v]`, bucketResource, objectResource)), genPolicyDoc("Allow", `"*"`, `"s3:*"`, fmt.Sprintf(`[%v, %v]`, bucketResource, objectResource)), genPolicyDoc("Allow", `"*"`, `"s3:Get*"`, objectResource), genPolicyDoc("Deny", `"*"`, `"s3:Create*"`, fmt.Sprintf(`[%v, %v]`, bucketResource, objectResource)),