From d8b424d123300aad13b934b25f5670506396da7b Mon Sep 17 00:00:00 2001 From: Konstantin Lebedev <9497591+kmlebedev@users.noreply.github.com> Date: Thu, 21 Sep 2023 20:19:11 +0500 Subject: [PATCH 1/2] [s3] optimization iam lookup for reducing algorithm complexity (#4857) optimization iam lookup for reducing algorithm complexity https://github.com/seaweedfs/seaweedfs/issues/4519 Co-authored-by: Konstantin Lebedev <9497591+kmlebedev@users.noreply.github.co> --- weed/s3api/auth_credentials.go | 49 +++++++-------- weed/s3api/auth_credentials_test.go | 9 +++ weed/s3api/auto_signature_v4_test.go | 90 +++++++++++++++++++--------- 3 files changed, 93 insertions(+), 55 deletions(-) diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 234dc100b..38ff2b5ca 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -18,8 +18,6 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) -var IdentityAnonymous *Identity - type Action string type Iam interface { @@ -29,12 +27,14 @@ type Iam interface { type IdentityAccessManagement struct { m sync.RWMutex - identities []*Identity - isAuthEnabled bool - domain string - hashes map[string]*sync.Pool - hashCounters map[string]*int32 - hashMu sync.RWMutex + identities []*Identity + accessKeyIdent map[string]*Identity + hashes map[string]*sync.Pool + hashCounters map[string]*int32 + identityAnonymous *Identity + hashMu sync.RWMutex + domain string + isAuthEnabled bool } type Identity struct { @@ -136,6 +136,8 @@ func (iam *IdentityAccessManagement) LoadS3ApiConfigurationFromBytes(content []b func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3ApiConfiguration) error { var identities []*Identity + var identityAnonymous *Identity + accessKeyIdent := make(map[string]*Identity) for _, ident := range config.Identities { t := &Identity{ Name: ident.Name, @@ -149,7 +151,7 @@ func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3Api glog.Warningf("anonymous identity is associated with a non-anonymous account ID, the association is invalid") } t.AccountId = s3account.AccountAnonymous.Id - IdentityAnonymous = t + identityAnonymous = t } else { if len(ident.AccountId) > 0 { t.AccountId = ident.AccountId @@ -164,19 +166,15 @@ func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3Api AccessKey: cred.AccessKey, SecretKey: cred.SecretKey, }) + accessKeyIdent[cred.AccessKey] = t } identities = append(identities, t) } - - if IdentityAnonymous == nil { - IdentityAnonymous = &Identity{ - Name: s3account.AccountAnonymous.Name, - AccountId: s3account.AccountAnonymous.Id, - } - } iam.m.Lock() // atomically switch iam.identities = identities + iam.identityAnonymous = identityAnonymous + iam.accessKeyIdent = accessKeyIdent if !iam.isAuthEnabled { // one-directional, no toggling iam.isAuthEnabled = len(identities) > 0 } @@ -189,14 +187,12 @@ func (iam *IdentityAccessManagement) isEnabled() bool { } func (iam *IdentityAccessManagement) lookupByAccessKey(accessKey string) (identity *Identity, cred *Credential, found bool) { - iam.m.RLock() defer iam.m.RUnlock() - for _, ident := range iam.identities { - for _, cred := range ident.Credentials { - // println("checking", ident.Name, cred.AccessKey) - if cred.AccessKey == accessKey { - return ident, cred, true + if ident, ok := iam.accessKeyIdent[accessKey]; ok { + for _, credential := range ident.Credentials { + if credential.AccessKey == accessKey { + return ident, credential, true } } } @@ -207,10 +203,8 @@ func (iam *IdentityAccessManagement) lookupByAccessKey(accessKey string) (identi func (iam *IdentityAccessManagement) lookupAnonymous() (identity *Identity, found bool) { iam.m.RLock() defer iam.m.RUnlock() - for _, ident := range iam.identities { - if ident.isAnonymous() { - return ident, true - } + if iam.identityAnonymous != nil { + return iam.identityAnonymous, true } return nil, false } @@ -270,8 +264,7 @@ func (iam *IdentityAccessManagement) authRequest(r *http.Request, action Action) return identity, s3err.ErrNotImplemented case authTypeAnonymous: authType = "Anonymous" - identity, found = iam.lookupAnonymous() - if !found { + if identity, found = iam.lookupAnonymous(); !found { r.Header.Set(s3_constants.AmzAuthType, authType) return identity, s3err.ErrAccessDenied } diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index 1f0ffc1cc..645932aba 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -126,6 +126,15 @@ func TestCanDo(t *testing.T) { } assert.Equal(t, true, ident5.canDo(ACTION_READ, "special_bucket", "/a/b/c/d.txt")) assert.Equal(t, true, ident5.canDo(ACTION_WRITE, "special_bucket", "/a/b/c/d.txt")) + + // anonymous buckets + ident6 := &Identity{ + Name: "anonymous", + Actions: []Action{ + "Read", + }, + } + assert.Equal(t, true, ident6.canDo(ACTION_READ, "anything_bucket", "/a/b/c/d.txt")) } type LoadS3ApiConfigurationTestCase struct { diff --git a/weed/s3api/auto_signature_v4_test.go b/weed/s3api/auto_signature_v4_test.go index 41b54db63..ccee8b885 100644 --- a/weed/s3api/auto_signature_v4_test.go +++ b/weed/s3api/auto_signature_v4_test.go @@ -8,8 +8,8 @@ import ( "encoding/hex" "errors" "fmt" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "io" "net/http" "net/url" @@ -60,22 +60,24 @@ func TestIsRequestPresignedSignatureV4(t *testing.T) { // Tests is requested authenticated function, tests replies for s3 errors. func TestIsReqAuthenticated(t *testing.T) { - option := S3ApiServerOption{ - GrpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()), + iam := &IdentityAccessManagement{ + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), } - iam := NewIdentityAccessManagement(&option) - iam.identities = []*Identity{ - { - Name: "someone", - Credentials: []*Credential{ - { - AccessKey: "access_key_1", - SecretKey: "secret_key_1", + _ = iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + { + Name: "someone", + Credentials: []*iam_pb.Credential{ + { + AccessKey: "access_key_1", + SecretKey: "secret_key_1", + }, }, + Actions: []string{}, }, - Actions: nil, }, - } + }) // List of test cases for validating http request authentication. testCases := []struct { @@ -97,24 +99,58 @@ func TestIsReqAuthenticated(t *testing.T) { } } -func TestCheckAdminRequestAuthType(t *testing.T) { - option := S3ApiServerOption{ - GrpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()), +func TestCheckaAnonymousRequestAuthType(t *testing.T) { + iam := &IdentityAccessManagement{ + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), } - iam := NewIdentityAccessManagement(&option) - iam.identities = []*Identity{ - { - Name: "someone", - Credentials: []*Credential{ - { - AccessKey: "access_key_1", - SecretKey: "secret_key_1", - }, + _ = iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + { + Name: "anonymous", + Actions: []string{s3_constants.ACTION_READ}, }, - Actions: nil, }, + }) + testCases := []struct { + Request *http.Request + ErrCode s3err.ErrorCode + Action Action + }{ + {Request: mustNewRequest("GET", "http://127.0.0.1:9000/bucket", 0, nil, t), ErrCode: s3err.ErrNone, Action: s3_constants.ACTION_READ}, + {Request: mustNewRequest("PUT", "http://127.0.0.1:9000/bucket", 0, nil, t), ErrCode: s3err.ErrAccessDenied, Action: s3_constants.ACTION_WRITE}, + } + for i, testCase := range testCases { + _, s3Error := iam.authRequest(testCase.Request, testCase.Action) + if s3Error != testCase.ErrCode { + t.Errorf("Test %d: Unexpected s3error returned wanted %d, got %d", i, testCase.ErrCode, s3Error) + } + if testCase.Request.Header.Get(s3_constants.AmzAuthType) != "Anonymous" { + t.Errorf("Test %d: Unexpected AuthType returned wanted %s, got %s", i, "Anonymous", testCase.Request.Header.Get(s3_constants.AmzAuthType)) + } } +} + +func TestCheckAdminRequestAuthType(t *testing.T) { + iam := &IdentityAccessManagement{ + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), + } + _ = iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + { + Name: "someone", + Credentials: []*iam_pb.Credential{ + { + AccessKey: "access_key_1", + SecretKey: "secret_key_1", + }, + }, + Actions: []string{}, + }, + }, + }) testCases := []struct { Request *http.Request ErrCode s3err.ErrorCode From a46f873edd8a5fb0c25aeb1c5c3c33e925ed63dd Mon Sep 17 00:00:00 2001 From: Konstantin Lebedev <9497591+kmlebedev@users.noreply.github.com> Date: Thu, 21 Sep 2023 20:20:05 +0500 Subject: [PATCH 2/2] [s3acl] Step 0: Put bucket ACL only responds success if the ACL is private. (#4856) * Passing test: test_bucket_acl_default test_bucket_acl_canned_private_to_private https://github.com/seaweedfs/seaweedfs/issues/4519 * Update weed/s3api/s3api_bucket_handlers.go --------- Co-authored-by: Konstantin Lebedev <9497591+kmlebedev@users.noreply.github.co> Co-authored-by: Chris Lu --- docker/compose/s3tests.conf | 4 +- weed/s3api/s3api_bucket_handlers.go | 68 ++++++++++++++++-------- weed/s3api/s3api_bucket_skip_handlers.go | 6 --- 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/docker/compose/s3tests.conf b/docker/compose/s3tests.conf index 68d9ddeb7..2bffe20d4 100644 --- a/docker/compose/s3tests.conf +++ b/docker/compose/s3tests.conf @@ -18,10 +18,10 @@ bucket prefix = yournamehere-{random}- [s3 main] # main display_name set in vstart.sh -display_name = M. Tester +display_name = s3_tests # main user_idname set in vstart.sh -user_id = testid +user_id = s3_tests # main email set in vstart.sh email = tester@ceph.com diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index d4d81905d..d2e987a25 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -259,32 +259,54 @@ func (s3a *S3ApiServer) GetBucketAclHandler(w http.ResponseWriter, r *http.Reque return } - response := AccessControlPolicy{} - for _, ident := range s3a.iam.identities { - if len(ident.Credentials) == 0 { - continue + identityId := r.Header.Get(s3_constants.AmzIdentityId) + response := AccessControlPolicy{ + Owner: CanonicalUser{ + ID: identityId, + DisplayName: identityId, + }, + } + response.AccessControlList.Grant = append(response.AccessControlList.Grant, Grant{ + Grantee: Grantee{ + ID: identityId, + DisplayName: identityId, + Type: "CanonicalUser", + XMLXSI: "CanonicalUser", + XMLNS: "http://www.w3.org/2001/XMLSchema-instance"}, + Permission: s3.PermissionFullControl, + }) + writeSuccessResponseXML(w, r, response) +} + +// PutBucketAclHandler Put bucket ACL only responds success if the ACL is private. +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html // +func (s3a *S3ApiServer) PutBucketAclHandler(w http.ResponseWriter, r *http.Request) { + // collect parameters + bucket, _ := s3_constants.GetBucketAndObject(r) + glog.V(3).Infof("PutBucketAclHandler %s", bucket) + + if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, err) + return + } + cannedAcl := r.Header.Get(s3_constants.AmzCannedAcl) + switch { + case cannedAcl == "": + acl := &s3.AccessControlPolicy{} + if err := xmlDecoder(r.Body, acl, r.ContentLength); err != nil { + glog.Errorf("PutBucketAclHandler: %s", err) + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + return } - for _, action := range ident.Actions { - if !action.overBucket(bucket) || action.getPermission() == "" { - continue - } - id := ident.Credentials[0].AccessKey - if response.Owner.DisplayName == "" && action.isOwner(bucket) && len(ident.Credentials) > 0 { - response.Owner.DisplayName = ident.Name - response.Owner.ID = id - } - response.AccessControlList.Grant = append(response.AccessControlList.Grant, Grant{ - Grantee: Grantee{ - ID: id, - DisplayName: ident.Name, - Type: "CanonicalUser", - XMLXSI: "CanonicalUser", - XMLNS: "http://www.w3.org/2001/XMLSchema-instance"}, - Permission: action.getPermission(), - }) + if len(acl.Grants) == 1 && acl.Grants[0].Permission != nil && *acl.Grants[0].Permission == s3_constants.PermissionFullControl { + writeSuccessResponseEmpty(w, r) + return } + case cannedAcl == s3_constants.CannedAclPrivate: + writeSuccessResponseEmpty(w, r) + return } - writeSuccessResponseXML(w, r, response) + s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) } // GetBucketLifecycleConfigurationHandler Get Bucket Lifecycle configuration diff --git a/weed/s3api/s3api_bucket_skip_handlers.go b/weed/s3api/s3api_bucket_skip_handlers.go index 70fd38424..62d5b8ce7 100644 --- a/weed/s3api/s3api_bucket_skip_handlers.go +++ b/weed/s3api/s3api_bucket_skip_handlers.go @@ -41,9 +41,3 @@ func (s3a *S3ApiServer) PutBucketPolicyHandler(w http.ResponseWriter, r *http.Re func (s3a *S3ApiServer) DeleteBucketPolicyHandler(w http.ResponseWriter, r *http.Request) { s3err.WriteErrorResponse(w, r, http.StatusNoContent) } - -// PutBucketAclHandler Put bucket ACL -// https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html -func (s3a *S3ApiServer) PutBucketAclHandler(w http.ResponseWriter, r *http.Request) { - s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) -}