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] [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