From 6481e2aac5c435217f673e224e51fb48d2d61e99 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Tue, 9 Jan 2024 22:56:16 -0800 Subject: [PATCH] fix: cleanup backend ACLs This adds the default ACL to the CreateBucket backend method so that the backend doesn't need to know how to construct and ACL. This also moves the s3proxy ACLs to a tag key/value because the gateway ACLs are not the same accounts as the backend s3 server. TODO: we may need to mask this tag key/value if we add support for the Get/PutBucketTagging API. --- backend/azure/azure.go | 24 +--- backend/backend.go | 4 +- backend/posix/posix.go | 11 +- backend/s3proxy/s3.go | 167 ++++++++++++++------------ s3api/controllers/backend_moq_test.go | 14 ++- s3api/controllers/base.go | 11 +- s3api/controllers/base_test.go | 2 +- 7 files changed, 119 insertions(+), 114 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 876b71dd..562e23a7 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -19,7 +19,6 @@ import ( "context" "encoding/base64" "encoding/binary" - "encoding/json" "errors" "fmt" "io" @@ -37,7 +36,6 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/container" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" - "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3response" @@ -109,17 +107,11 @@ func (az *Azure) String() string { return "Azure Blob Gateway" } -func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput) error { - owner := string(input.ObjectOwnership) - acl := auth.ACL{ACL: "private", Owner: owner, Grantees: []auth.Grantee{}} - jsonACL, err := json.Marshal(acl) - if err != nil { - return fmt.Errorf("marshal acl: %w", err) - } +func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, acl []byte) error { meta := map[string]*string{ - aclKey: getStringPtr(string(jsonACL)), + aclKey: backend.GetStringPtr(string(acl)), } - _, err = az.client.CreateContainer(ctx, *input.Bucket, &container.CreateOptions{Metadata: meta}) + _, err := az.client.CreateContainer(ctx, *input.Bucket, &container.CreateOptions{Metadata: meta}) return azureErrToS3Err(err) } @@ -357,8 +349,8 @@ func (az *Azure) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInput } else { errs = append(errs, types.Error{ Key: obj.Key, - Code: getStringPtr("InternalError"), - Message: getStringPtr(err.Error()), + Code: backend.GetStringPtr("InternalError"), + Message: backend.GetStringPtr(err.Error()), }) } } @@ -652,7 +644,7 @@ func (az *Azure) PutBucketAcl(ctx context.Context, bucket string, data []byte) e return err } meta := map[string]*string{ - aclKey: getStringPtr(string(data)), + aclKey: backend.GetStringPtr(string(data)), } _, err = client.SetMetadata(ctx, &container.SetMetadataOptions{ Metadata: meta, @@ -782,10 +774,6 @@ func getString(str *string) string { return *str } -func getStringPtr(str string) *string { - return &str -} - // Parses azure ResponseError into AWS APIError func azureErrToS3Err(apiErr error) error { var azErr *azcore.ResponseError diff --git a/backend/backend.go b/backend/backend.go index 9a70ca14..ce397ab7 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -35,7 +35,7 @@ type Backend interface { ListBuckets(_ context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) HeadBucket(context.Context, *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) GetBucketAcl(context.Context, *s3.GetBucketAclInput) ([]byte, error) - CreateBucket(context.Context, *s3.CreateBucketInput) error + CreateBucket(_ context.Context, _ *s3.CreateBucketInput, defaultACL []byte) error PutBucketAcl(_ context.Context, bucket string, data []byte) error DeleteBucket(context.Context, *s3.DeleteBucketInput) error @@ -95,7 +95,7 @@ func (BackendUnsupported) HeadBucket(context.Context, *s3.HeadBucketInput) (*s3. func (BackendUnsupported) GetBucketAcl(context.Context, *s3.GetBucketAclInput) ([]byte, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) } -func (BackendUnsupported) CreateBucket(context.Context, *s3.CreateBucketInput) error { +func (BackendUnsupported) CreateBucket(context.Context, *s3.CreateBucketInput, []byte) error { return s3err.GetAPIError(s3err.ErrNotImplemented) } func (BackendUnsupported) PutBucketAcl(_ context.Context, bucket string, data []byte) error { diff --git a/backend/posix/posix.go b/backend/posix/posix.go index a5c88da7..784d5872 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -161,13 +161,12 @@ func (p *Posix) HeadBucket(_ context.Context, input *s3.HeadBucketInput) (*s3.He return &s3.HeadBucketOutput{}, nil } -func (p *Posix) CreateBucket(_ context.Context, input *s3.CreateBucketInput) error { +func (p *Posix) CreateBucket(_ context.Context, input *s3.CreateBucketInput, acl []byte) error { if input.Bucket == nil { return s3err.GetAPIError(s3err.ErrInvalidBucketName) } bucket := *input.Bucket - owner := string(input.ObjectOwnership) err := os.Mkdir(bucket, 0777) if err != nil && os.IsExist(err) { @@ -177,13 +176,7 @@ func (p *Posix) CreateBucket(_ context.Context, input *s3.CreateBucketInput) err return fmt.Errorf("mkdir bucket: %w", err) } - acl := auth.ACL{ACL: "private", Owner: owner, Grantees: []auth.Grantee{}} - jsonACL, err := json.Marshal(acl) - if err != nil { - return fmt.Errorf("marshal acl: %w", err) - } - - if err := xattr.Set(bucket, aclkey, jsonACL); err != nil { + if err := xattr.Set(bucket, aclkey, acl); err != nil { return fmt.Errorf("set acl: %w", err) } diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index 8225193c..55db70bd 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -17,6 +17,7 @@ package s3proxy import ( "context" "crypto/sha256" + "encoding/base64" "encoding/hex" "encoding/json" "errors" @@ -32,12 +33,13 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/aws/smithy-go" - "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3response" ) +const aclKey string = "versitygwAcl" + type S3Proxy struct { backend.BackendUnsupported @@ -72,9 +74,8 @@ func New(access, secret, endpoint, region string, disableChecksum, sslSkipVerify func (s *S3Proxy) ListBuckets(ctx context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) { output, err := s.client.ListBuckets(ctx, &s3.ListBucketsInput{}) - err = handleError(err) if err != nil { - return s3response.ListAllMyBucketsResult{}, err + return s3response.ListAllMyBucketsResult{}, handleError(err) } var buckets []s3response.ListAllMyBucketsEntry @@ -97,13 +98,27 @@ func (s *S3Proxy) ListBuckets(ctx context.Context, owner string, isAdmin bool) ( func (s *S3Proxy) HeadBucket(ctx context.Context, input *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) { out, err := s.client.HeadBucket(ctx, input) - err = handleError(err) - - return out, err + return out, handleError(err) } -func (s *S3Proxy) CreateBucket(ctx context.Context, input *s3.CreateBucketInput) error { +func (s *S3Proxy) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, acl []byte) error { _, err := s.client.CreateBucket(ctx, input) + if err != nil { + return handleError(err) + } + + var tagSet []types.Tag + tagSet = append(tagSet, types.Tag{ + Key: backend.GetStringPtr(aclKey), + Value: backend.GetStringPtr(base64Encode(acl)), + }) + + _, err = s.client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: input.Bucket, + Tagging: &types.Tagging{ + TagSet: tagSet, + }, + }) return handleError(err) } @@ -114,27 +129,23 @@ func (s *S3Proxy) DeleteBucket(ctx context.Context, input *s3.DeleteBucketInput) func (s *S3Proxy) CreateMultipartUpload(ctx context.Context, input *s3.CreateMultipartUploadInput) (*s3.CreateMultipartUploadOutput, error) { out, err := s.client.CreateMultipartUpload(ctx, input) - err = handleError(err) - return out, err + return out, handleError(err) } func (s *S3Proxy) CompleteMultipartUpload(ctx context.Context, input *s3.CompleteMultipartUploadInput) (*s3.CompleteMultipartUploadOutput, error) { out, err := s.client.CompleteMultipartUpload(ctx, input) - err = handleError(err) - return out, err + return out, handleError(err) } func (s *S3Proxy) AbortMultipartUpload(ctx context.Context, input *s3.AbortMultipartUploadInput) error { _, err := s.client.AbortMultipartUpload(ctx, input) - err = handleError(err) - return err + return handleError(err) } func (s *S3Proxy) ListMultipartUploads(ctx context.Context, input *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResult, error) { output, err := s.client.ListMultipartUploads(ctx, input) - err = handleError(err) if err != nil { - return s3response.ListMultipartUploadsResult{}, err + return s3response.ListMultipartUploadsResult{}, handleError(err) } var uploads []s3response.Upload @@ -180,9 +191,8 @@ func (s *S3Proxy) ListMultipartUploads(ctx context.Context, input *s3.ListMultip func (s *S3Proxy) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3response.ListPartsResult, error) { output, err := s.client.ListParts(ctx, input) - err = handleError(err) if err != nil { - return s3response.ListPartsResult{}, err + return s3response.ListPartsResult{}, handleError(err) } var parts []s3response.Part @@ -233,9 +243,8 @@ func (s *S3Proxy) UploadPart(ctx context.Context, input *s3.UploadPartInput) (et output, err := s.client.UploadPart(ctx, input, s3.WithAPIOptions( v4.SwapComputePayloadSHA256ForUnsignedPayloadMiddleware, )) - err = handleError(err) if err != nil { - return "", err + return "", handleError(err) } return *output.ETag, nil @@ -243,9 +252,8 @@ func (s *S3Proxy) UploadPart(ctx context.Context, input *s3.UploadPartInput) (et func (s *S3Proxy) UploadPartCopy(ctx context.Context, input *s3.UploadPartCopyInput) (s3response.CopyObjectResult, error) { output, err := s.client.UploadPartCopy(ctx, input) - err = handleError(err) if err != nil { - return s3response.CopyObjectResult{}, err + return s3response.CopyObjectResult{}, handleError(err) } return s3response.CopyObjectResult{ @@ -260,9 +268,8 @@ func (s *S3Proxy) PutObject(ctx context.Context, input *s3.PutObjectInput) (stri output, err := s.client.PutObject(ctx, input, s3.WithAPIOptions( v4.SwapComputePayloadSHA256ForUnsignedPayloadMiddleware, )) - err = handleError(err) if err != nil { - return "", err + return "", handleError(err) } return *output.ETag, nil @@ -270,16 +277,13 @@ func (s *S3Proxy) PutObject(ctx context.Context, input *s3.PutObjectInput) (stri func (s *S3Proxy) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) { out, err := s.client.HeadObject(ctx, input) - err = handleError(err) - - return out, err + return out, handleError(err) } func (s *S3Proxy) GetObject(ctx context.Context, input *s3.GetObjectInput, w io.Writer) (*s3.GetObjectOutput, error) { output, err := s.client.GetObject(ctx, input) - err = handleError(err) if err != nil { - return nil, err + return nil, handleError(err) } defer output.Body.Close() @@ -293,30 +297,22 @@ func (s *S3Proxy) GetObject(ctx context.Context, input *s3.GetObjectInput, w io. func (s *S3Proxy) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttributesInput) (*s3.GetObjectAttributesOutput, error) { out, err := s.client.GetObjectAttributes(ctx, input) - err = handleError(err) - - return out, err + return out, handleError(err) } func (s *S3Proxy) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.CopyObjectOutput, error) { out, err := s.client.CopyObject(ctx, input) - err = handleError(err) - - return out, err + return out, handleError(err) } func (s *S3Proxy) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (*s3.ListObjectsOutput, error) { out, err := s.client.ListObjects(ctx, input) - err = handleError(err) - - return out, err + return out, handleError(err) } func (s *S3Proxy) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input) (*s3.ListObjectsV2Output, error) { out, err := s.client.ListObjectsV2(ctx, input) - err = handleError(err) - - return out, err + return out, handleError(err) } func (s *S3Proxy) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) error { @@ -330,9 +326,8 @@ func (s *S3Proxy) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInpu } output, err := s.client.DeleteObjects(ctx, input) - err = handleError(err) if err != nil { - return s3response.DeleteObjectsResult{}, err + return s3response.DeleteObjectsResult{}, handleError(err) } return s3response.DeleteObjectsResult{ @@ -342,53 +337,58 @@ func (s *S3Proxy) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInpu } func (s *S3Proxy) GetBucketAcl(ctx context.Context, input *s3.GetBucketAclInput) ([]byte, error) { - output, err := s.client.GetBucketAcl(ctx, input) - err = handleError(err) + tagout, err := s.client.GetBucketTagging(ctx, &s3.GetBucketTaggingInput{ + Bucket: input.Bucket, + }) if err != nil { - return nil, err + return nil, handleError(err) } - var acl auth.ACL - - acl.Owner = *output.Owner.ID - for _, el := range output.Grants { - acl.Grantees = append(acl.Grantees, auth.Grantee{ - Permission: el.Permission, - Access: *el.Grantee.ID, - }) + for _, tag := range tagout.TagSet { + if *tag.Key == aclKey { + acl, err := base64Decode(*tag.Value) + if err != nil { + return nil, handleError(err) + } + return acl, nil + } } - return json.Marshal(acl) + return []byte{}, nil } func (s *S3Proxy) PutBucketAcl(ctx context.Context, bucket string, data []byte) error { - acl, err := auth.ParseACL(data) - if err != nil { - return err - } - - input := &s3.PutBucketAclInput{ + tagout, err := s.client.GetBucketTagging(ctx, &s3.GetBucketTaggingInput{ Bucket: &bucket, - ACL: acl.ACL, - AccessControlPolicy: &types.AccessControlPolicy{ - Owner: &types.Owner{ - ID: &acl.Owner, - }, - }, + }) + if err != nil { + return handleError(err) + } + + var found bool + for i, tag := range tagout.TagSet { + if *tag.Key == aclKey { + tagout.TagSet[i] = types.Tag{ + Key: backend.GetStringPtr(aclKey), + Value: backend.GetStringPtr(base64Encode(data)), + } + found = true + break + } } - - for _, el := range acl.Grantees { - acc := el.Access - input.AccessControlPolicy.Grants = append(input.AccessControlPolicy.Grants, types.Grant{ - Permission: el.Permission, - Grantee: &types.Grantee{ - ID: &acc, - Type: types.TypeCanonicalUser, - }, + if !found { + tagout.TagSet = append(tagout.TagSet, types.Tag{ + Key: backend.GetStringPtr(aclKey), + Value: backend.GetStringPtr(base64Encode(data)), }) } - _, err = s.client.PutBucketAcl(ctx, input) + _, err = s.client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: &bucket, + Tagging: &types.Tagging{ + TagSet: tagout.TagSet, + }, + }) return handleError(err) } @@ -416,9 +416,8 @@ func (s *S3Proxy) GetObjectTagging(ctx context.Context, bucket, object string) ( Bucket: &bucket, Key: &object, }) - err = handleError(err) if err != nil { - return nil, err + return nil, handleError(err) } tags := make(map[string]string) @@ -532,3 +531,15 @@ func handleError(err error) error { } return err } + +func base64Encode(input []byte) string { + return base64.StdEncoding.EncodeToString(input) +} + +func base64Decode(encoded string) ([]byte, error) { + decoded, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + return nil, err + } + return decoded, nil +} diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index 80eb64df..b8a8b263 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -35,7 +35,7 @@ var _ backend.Backend = &BackendMock{} // CopyObjectFunc: func(contextMoqParam context.Context, copyObjectInput *s3.CopyObjectInput) (*s3.CopyObjectOutput, error) { // panic("mock out the CopyObject method") // }, -// CreateBucketFunc: func(contextMoqParam context.Context, createBucketInput *s3.CreateBucketInput) error { +// CreateBucketFunc: func(contextMoqParam context.Context, createBucketInput *s3.CreateBucketInput, defaultACL []byte) error { // panic("mock out the CreateBucket method") // }, // CreateMultipartUploadFunc: func(contextMoqParam context.Context, createMultipartUploadInput *s3.CreateMultipartUploadInput) (*s3.CreateMultipartUploadOutput, error) { @@ -142,7 +142,7 @@ type BackendMock struct { CopyObjectFunc func(contextMoqParam context.Context, copyObjectInput *s3.CopyObjectInput) (*s3.CopyObjectOutput, error) // CreateBucketFunc mocks the CreateBucket method. - CreateBucketFunc func(contextMoqParam context.Context, createBucketInput *s3.CreateBucketInput) error + CreateBucketFunc func(contextMoqParam context.Context, createBucketInput *s3.CreateBucketInput, defaultACL []byte) error // CreateMultipartUploadFunc mocks the CreateMultipartUpload method. CreateMultipartUploadFunc func(contextMoqParam context.Context, createMultipartUploadInput *s3.CreateMultipartUploadInput) (*s3.CreateMultipartUploadOutput, error) @@ -266,6 +266,8 @@ type BackendMock struct { ContextMoqParam context.Context // CreateBucketInput is the createBucketInput argument value. CreateBucketInput *s3.CreateBucketInput + // DefaultACL is the defaultACL argument value. + DefaultACL []byte } // CreateMultipartUpload holds details about calls to the CreateMultipartUpload method. CreateMultipartUpload []struct { @@ -652,21 +654,23 @@ func (mock *BackendMock) CopyObjectCalls() []struct { } // CreateBucket calls CreateBucketFunc. -func (mock *BackendMock) CreateBucket(contextMoqParam context.Context, createBucketInput *s3.CreateBucketInput) error { +func (mock *BackendMock) CreateBucket(contextMoqParam context.Context, createBucketInput *s3.CreateBucketInput, defaultACL []byte) error { if mock.CreateBucketFunc == nil { panic("BackendMock.CreateBucketFunc: method is nil but Backend.CreateBucket was just called") } callInfo := struct { ContextMoqParam context.Context CreateBucketInput *s3.CreateBucketInput + DefaultACL []byte }{ ContextMoqParam: contextMoqParam, CreateBucketInput: createBucketInput, + DefaultACL: defaultACL, } mock.lockCreateBucket.Lock() mock.calls.CreateBucket = append(mock.calls.CreateBucket, callInfo) mock.lockCreateBucket.Unlock() - return mock.CreateBucketFunc(contextMoqParam, createBucketInput) + return mock.CreateBucketFunc(contextMoqParam, createBucketInput, defaultACL) } // CreateBucketCalls gets all the calls that were made to CreateBucket. @@ -676,10 +680,12 @@ func (mock *BackendMock) CreateBucket(contextMoqParam context.Context, createBuc func (mock *BackendMock) CreateBucketCalls() []struct { ContextMoqParam context.Context CreateBucketInput *s3.CreateBucketInput + DefaultACL []byte } { var calls []struct { ContextMoqParam context.Context CreateBucketInput *s3.CreateBucketInput + DefaultACL []byte } mock.lockCreateBucket.RLock() calls = mock.calls.CreateBucket diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index d7cc0c42..8b3a47a2 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -16,6 +16,7 @@ package controllers import ( "bytes" + "encoding/json" "encoding/xml" "errors" "fmt" @@ -407,10 +408,16 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidBucketName), &MetaOpts{Logger: c.logger, Action: "CreateBucket"}) } - err := c.be.CreateBucket(ctx.Context(), &s3.CreateBucketInput{ + defACL := auth.ACL{ACL: "private", Owner: acct.Access, Grantees: []auth.Grantee{}} + jsonACL, err := json.Marshal(defACL) + if err != nil { + return SendResponse(ctx, fmt.Errorf("marshal acl: %w", err), &MetaOpts{Logger: c.logger, Action: "CreateBucket", BucketOwner: acct.Access}) + } + + err = c.be.CreateBucket(ctx.Context(), &s3.CreateBucketInput{ Bucket: &bucket, ObjectOwnership: types.ObjectOwnership(acct.Access), - }) + }, jsonACL) return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "CreateBucket", BucketOwner: acct.Access}) } diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index d4d7531a..8facf76f 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -500,7 +500,7 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { PutBucketAclFunc: func(context.Context, string, []byte) error { return nil }, - CreateBucketFunc: func(context.Context, *s3.CreateBucketInput) error { + CreateBucketFunc: func(context.Context, *s3.CreateBucketInput, []byte) error { return nil }, },