mirror of https://github.com/milvus-io/milvus.git
Fix password verification miss cache (#18730)
Signed-off-by: yun.zhang <yun.zhang@zilliz.com> Signed-off-by: yun.zhang <yun.zhang@zilliz.com>pull/18741/head
parent
c8b585c13f
commit
516fd928f9
|
@ -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 {
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
}
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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{
|
||||
|
|
Loading…
Reference in New Issue