diff --git a/internal/proxy/authentication_interceptor.go b/internal/proxy/authentication_interceptor.go index 77c232de1a..262a1183b2 100644 --- a/internal/proxy/authentication_interceptor.go +++ b/internal/proxy/authentication_interceptor.go @@ -6,9 +6,6 @@ import ( "github.com/milvus-io/milvus/internal/util" - "go.uber.org/zap" - - "github.com/milvus-io/milvus/internal/log" "github.com/milvus-io/milvus/internal/util/crypto" "google.golang.org/grpc/metadata" @@ -31,13 +28,7 @@ func validAuth(ctx context.Context, authorization []string) bool { username := secrets[0] password := secrets[1] - credInfo, err := globalMetaCache.GetCredentialInfo(ctx, username) - if err != nil { - log.Error("found no credential", zap.String("username", username), zap.Error(err)) - return false - } - - return crypto.PasswordVerify(password, credInfo) + return passwordVerify(ctx, username, password, globalMetaCache) } func validSourceID(ctx context.Context, authorization []string) bool { diff --git a/internal/proxy/impl.go b/internal/proxy/impl.go index cda4912fc8..1e9bbb7fd7 100644 --- a/internal/proxy/impl.go +++ b/internal/proxy/impl.go @@ -3950,16 +3950,8 @@ func (node *Proxy) UpdateCredential(ctx context.Context, req *milvuspb.UpdateCre Reason: err.Error(), }, nil } - // check old password is correct - oldCredInfo, err := globalMetaCache.GetCredentialInfo(ctx, req.Username) - if err != nil { - log.Error("found no credential", zap.String("username", req.Username), zap.Error(err)) - return &commonpb.Status{ - ErrorCode: commonpb.ErrorCode_UpdateCredentialFailure, - Reason: "found no credential:" + req.Username, - }, nil - } - if !crypto.PasswordVerify(rawOldPassword, oldCredInfo) { + + if !passwordVerify(ctx, req.Username, rawOldPassword, globalMetaCache) { return &commonpb.Status{ ErrorCode: commonpb.ErrorCode_UpdateCredentialFailure, Reason: "old password is not correct:" + req.Username, diff --git a/internal/proxy/meta_cache.go b/internal/proxy/meta_cache.go index 9a80ef9c7b..935692e50e 100644 --- a/internal/proxy/meta_cache.go +++ b/internal/proxy/meta_cache.go @@ -536,7 +536,6 @@ func (m *MetaCache) GetCredentialInfo(ctx context.Context, username string) (*in Username: resp.Username, EncryptedPassword: resp.Password, } - m.UpdateCredential(credInfo) } return credInfo, nil @@ -552,15 +551,15 @@ func (m *MetaCache) RemoveCredential(username string) { func (m *MetaCache) UpdateCredential(credInfo *internalpb.CredentialInfo) { m.credMut.Lock() defer m.credMut.Unlock() - // update credMap username := credInfo.Username _, ok := m.credMap[username] if !ok { m.credMap[username] = &internalpb.CredentialInfo{} } + + // Do not cache encrypted password content m.credMap[username].Username = username m.credMap[username].Sha256Password = credInfo.Sha256Password - m.credMap[username].EncryptedPassword = credInfo.EncryptedPassword } // GetShards update cache if withCache == false diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 73b372c105..b5bee9af79 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -2270,9 +2270,7 @@ func TestProxy(t *testing.T) { getResp, err := rootCoordClient.GetCredential(ctx, getCredentialReq) assert.NoError(t, err) assert.Equal(t, commonpb.ErrorCode_Success, getResp.Status.ErrorCode) - assert.True(t, crypto.PasswordVerify(newPassword, &internalpb.CredentialInfo{ - EncryptedPassword: getResp.Password, - })) + assert.True(t, passwordVerify(ctx, username, newPassword, globalMetaCache)) getCredentialReq.Username = "(" getResp, err = rootCoordClient.GetCredential(ctx, getCredentialReq) diff --git a/internal/proxy/rootcoord_mock_test.go b/internal/proxy/rootcoord_mock_test.go index 14f81677f1..1b2694cec4 100644 --- a/internal/proxy/rootcoord_mock_test.go +++ b/internal/proxy/rootcoord_mock_test.go @@ -1144,6 +1144,7 @@ type DescribeSegmentsFunc func(ctx context.Context, request *rootcoordpb.Describ type ImportFunc func(ctx context.Context, req *milvuspb.ImportRequest) (*milvuspb.ImportResponse, error) type DropCollectionFunc func(ctx context.Context, request *milvuspb.DropCollectionRequest) (*commonpb.Status, error) type GetIndexStateFunc func(ctx context.Context, request *milvuspb.GetIndexStateRequest) (*indexpb.GetIndexStatesResponse, error) +type GetGetCredentialFunc func(ctx context.Context, req *rootcoordpb.GetCredentialRequest) (*rootcoordpb.GetCredentialResponse, error) type mockRootCoord struct { types.RootCoord @@ -1155,6 +1156,14 @@ type mockRootCoord struct { ImportFunc DropCollectionFunc GetIndexStateFunc + GetGetCredentialFunc +} + +func (m *mockRootCoord) GetCredential(ctx context.Context, request *rootcoordpb.GetCredentialRequest) (*rootcoordpb.GetCredentialResponse, error) { + if m.GetGetCredentialFunc != nil { + return m.GetGetCredentialFunc(ctx, request) + } + return nil, errors.New("mock") } func (m *mockRootCoord) DescribeCollection(ctx context.Context, request *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) { diff --git a/internal/proxy/util.go b/internal/proxy/util.go index 13bf3ddf26..017d291d6d 100644 --- a/internal/proxy/util.go +++ b/internal/proxy/util.go @@ -24,6 +24,8 @@ import ( "strings" "time" + "golang.org/x/crypto/bcrypt" + "github.com/milvus-io/milvus/internal/log" "go.uber.org/zap" @@ -701,3 +703,32 @@ func GetRole(username string) ([]string, error) { } return globalMetaCache.GetUserRole(username), nil } + +// PasswordVerify verify password +func passwordVerify(ctx context.Context, username, rawPwd string, globalMetaCache Cache) bool { + // it represents the cache miss if Sha256Password is empty within credInfo, which shall be updated first connection. + // meanwhile, generating Sha256Password depends on raw password and encrypted password will not cache. + credInfo, err := globalMetaCache.GetCredentialInfo(ctx, username) + if err != nil { + log.Error("found no credential", zap.String("username", username), zap.Error(err)) + return false + } + + // hit cache + sha256Pwd := crypto.SHA256(rawPwd, credInfo.Username) + if credInfo.Sha256Password != "" { + return sha256Pwd == credInfo.Sha256Password + } + + // miss cache, verify against encrypted password from etcd + if err := bcrypt.CompareHashAndPassword([]byte(credInfo.EncryptedPassword), []byte(rawPwd)); err != nil { + log.Error("Verify password failed", zap.Error(err)) + return false + } + + // update cache after miss cache + credInfo.Sha256Password = sha256Pwd + log.Debug("get credential miss cache, update cache with", zap.Any("credential", credInfo)) + globalMetaCache.UpdateCredential(credInfo) + return true +} diff --git a/internal/proxy/util_test.go b/internal/proxy/util_test.go index c3d013eec4..4c487542cd 100644 --- a/internal/proxy/util_test.go +++ b/internal/proxy/util_test.go @@ -23,6 +23,10 @@ import ( "strings" "testing" + "github.com/milvus-io/milvus/internal/proto/rootcoordpb" + + "github.com/milvus-io/milvus/internal/proto/internalpb" + "github.com/milvus-io/milvus/internal/proto/commonpb" "github.com/milvus-io/milvus/internal/proto/schemapb" "github.com/milvus-io/milvus/internal/util" @@ -721,3 +725,47 @@ func TestGetRole(t *testing.T) { assert.Nil(t, err) assert.Equal(t, 1, len(roles)) } + +func TestPasswordVerify(t *testing.T) { + username := "user-test00" + password := "PasswordVerify" + + // credential does not exist within cache + credCache := make(map[string]*internalpb.CredentialInfo, 0) + invokedCount := 0 + + mockedRootCoord := newMockRootCoord() + mockedRootCoord.GetGetCredentialFunc = func(ctx context.Context, req *rootcoordpb.GetCredentialRequest) (*rootcoordpb.GetCredentialResponse, error) { + invokedCount++ + return nil, fmt.Errorf("get cred not found credential") + } + + metaCache := &MetaCache{ + credMap: credCache, + rootCoord: mockedRootCoord, + } + ret, ok := credCache[username] + assert.False(t, ok) + assert.Nil(t, ret) + assert.False(t, passwordVerify(context.TODO(), username, password, metaCache)) + assert.Equal(t, 1, invokedCount) + + // Sha256Password has not been filled into cache during establish connection firstly + encryptedPwd, err := crypto.PasswordEncrypt(password) + assert.Nil(t, err) + credCache[username] = &internalpb.CredentialInfo{ + Username: username, + EncryptedPassword: encryptedPwd, + } + assert.True(t, passwordVerify(context.TODO(), username, password, metaCache)) + ret, ok = credCache[username] + assert.True(t, ok) + assert.NotNil(t, ret) + assert.Equal(t, username, ret.Username) + assert.NotNil(t, ret.Sha256Password) + assert.Equal(t, 1, invokedCount) + + // Sha256Password already exists within cache + assert.True(t, passwordVerify(context.TODO(), username, password, metaCache)) + assert.Equal(t, 1, invokedCount) +} diff --git a/internal/util/crypto/crypto.go b/internal/util/crypto/crypto.go index 2e8e1e15fd..cdbf032e6c 100644 --- a/internal/util/crypto/crypto.go +++ b/internal/util/crypto/crypto.go @@ -5,9 +5,6 @@ import ( "encoding/base64" "encoding/hex" - "github.com/milvus-io/milvus/internal/log" - "github.com/milvus-io/milvus/internal/proto/internalpb" - "go.uber.org/zap" "golang.org/x/crypto/bcrypt" ) @@ -30,23 +27,6 @@ func PasswordEncrypt(pwd string) (string, error) { return string(bytes), err } -// PasswordVerify verify password -func PasswordVerify(rawPwd string, credInfo *internalpb.CredentialInfo) bool { - // 1. hit cache - if credInfo.Sha256Password != "" { - encryped := SHA256(rawPwd, credInfo.Username) - return encryped == credInfo.Sha256Password - } - - // 2. miss cache, verify against encrypted password from etcd - err := bcrypt.CompareHashAndPassword([]byte(credInfo.EncryptedPassword), []byte(rawPwd)) - if err != nil { - log.Error("Verify password failed", zap.Error(err)) - } - - return err == nil -} - func Base64Decode(pwd string) (string, error) { bytes, err := base64.StdEncoding.DecodeString(pwd) if err != nil { diff --git a/internal/util/crypto/crypto_test.go b/internal/util/crypto/crypto_test.go index 286a7bcd03..73e1bdbce2 100644 --- a/internal/util/crypto/crypto_test.go +++ b/internal/util/crypto/crypto_test.go @@ -3,32 +3,10 @@ package crypto import ( "testing" - "github.com/milvus-io/milvus/internal/proto/internalpb" "github.com/stretchr/testify/assert" "golang.org/x/crypto/bcrypt" ) -func TestPasswordVerify_HitCache(t *testing.T) { - wrongPassword := "test_my_name" - correctPassword := "test_my_pass_new" - credInfo := &internalpb.CredentialInfo{ - Username: "root", - Sha256Password: "bcca79df9650cef1d7ed9f63449d7f8a27843d2678f5666f38ca65ab77d99a13", - } - assert.True(t, PasswordVerify(correctPassword, credInfo)) - assert.False(t, PasswordVerify(wrongPassword, credInfo)) -} - -func TestPasswordVerify_MissCache(t *testing.T) { - wrongPassword := "test_my_name" - correctPassword := "test_my_pass_new" - credInfo := &internalpb.CredentialInfo{ - EncryptedPassword: "$2a$10$3H9DLiHyPxJ29bMWRNyueOrGkbzJfE3BAR159ju3UetytAoKk7Ne2", - } - assert.True(t, PasswordVerify(correctPassword, credInfo)) - assert.False(t, PasswordVerify(wrongPassword, credInfo)) -} - //func BenchmarkPasswordVerify(b *testing.B) { // correctPassword := "test_my_pass_new" // credInfo := &internalpb.CredentialInfo{