From 754c221c4d9eee40030dfec15f9b8790b43f154f Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Mon, 25 Mar 2024 16:00:35 -0400 Subject: [PATCH] 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 166b686..6e6e501 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 469d5bd..ed9545a 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 09814b3..028a7b9 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 0ae98df..d09cc55 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 704ca88..3c05552 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 2961de6..7e489cf 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 a94f060..f42d864 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 4a55b30..7467adf 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 2d50930..aa100d2 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)),