diff --git a/auth/store.go b/auth/store.go index 1db89c03f..325af5b84 100644 --- a/auth/store.go +++ b/auth/store.go @@ -146,6 +146,9 @@ type AuthStore interface { // Revision gets current revision of authStore Revision() uint64 + + // CheckPassword checks a given pair of username and password is correct + CheckPassword(username, password string) (uint64, error) } type authStore struct { @@ -232,11 +235,6 @@ func (as *authStore) Authenticate(ctx context.Context, username, password string return nil, ErrAuthFailed } - if bcrypt.CompareHashAndPassword(user.Password, []byte(password)) != nil { - plog.Noticef("authentication failed, invalid password for user %s", username) - return &pb.AuthenticateResponse{}, ErrAuthFailed - } - token := fmt.Sprintf("%s.%d", simpleToken, index) as.assignSimpleTokenToUser(username, token) @@ -244,6 +242,24 @@ func (as *authStore) Authenticate(ctx context.Context, username, password string return &pb.AuthenticateResponse{Token: token}, nil } +func (as *authStore) CheckPassword(username, password string) (uint64, error) { + tx := as.be.BatchTx() + tx.Lock() + defer tx.Unlock() + + user := getUser(tx, username) + if user == nil { + return 0, ErrAuthFailed + } + + if bcrypt.CompareHashAndPassword(user.Password, []byte(password)) != nil { + plog.Noticef("authentication failed, invalid password for user %s", username) + return 0, ErrAuthFailed + } + + return getRevision(tx), nil +} + func (as *authStore) Recover(be backend.Backend) { enabled := false as.be = be diff --git a/auth/store_test.go b/auth/store_test.go index 68174ea39..89feeac95 100644 --- a/auth/store_test.go +++ b/auth/store_test.go @@ -67,7 +67,7 @@ func enableAuthAndCreateRoot(as *authStore) error { return as.AuthEnable() } -func TestAuthenticate(t *testing.T) { +func TestCheckPassword(t *testing.T) { b, tPath := backend.NewDefaultTmpBackend() defer func() { b.Close() @@ -87,8 +87,7 @@ func TestAuthenticate(t *testing.T) { } // auth a non-existing user - ctx1 := context.WithValue(context.WithValue(context.TODO(), "index", uint64(1)), "simpleToken", "dummy") - _, err = as.Authenticate(ctx1, "foo-test", "bar") + _, err = as.CheckPassword("foo-test", "bar") if err == nil { t.Fatalf("expected %v, got %v", ErrAuthFailed, err) } @@ -97,15 +96,13 @@ func TestAuthenticate(t *testing.T) { } // auth an existing user with correct password - ctx2 := context.WithValue(context.WithValue(context.TODO(), "index", uint64(2)), "simpleToken", "dummy") - _, err = as.Authenticate(ctx2, "foo", "bar") + _, err = as.CheckPassword("foo", "bar") if err != nil { t.Fatal(err) } // auth an existing user but with wrong password - ctx3 := context.WithValue(context.WithValue(context.TODO(), "index", uint64(3)), "simpleToken", "dummy") - _, err = as.Authenticate(ctx3, "foo", "") + _, err = as.CheckPassword("foo", "") if err == nil { t.Fatalf("expected %v, got %v", ErrAuthFailed, err) } diff --git a/etcdserver/v3_server.go b/etcdserver/v3_server.go index 6abe7dfaa..0e7f74938 100644 --- a/etcdserver/v3_server.go +++ b/etcdserver/v3_server.go @@ -419,24 +419,42 @@ func (s *EtcdServer) AuthDisable(ctx context.Context, r *pb.AuthDisableRequest) } func (s *EtcdServer) Authenticate(ctx context.Context, r *pb.AuthenticateRequest) (*pb.AuthenticateResponse, error) { - st, err := s.AuthStore().GenSimpleToken() - if err != nil { - return nil, err + var result *applyResult + + for { + checkedRevision, err := s.AuthStore().CheckPassword(r.Name, r.Password) + if err != nil { + plog.Errorf("invalid authentication request to user %s was issued", r.Name) + return nil, err + } + + st, err := s.AuthStore().GenSimpleToken() + if err != nil { + return nil, err + } + + internalReq := &pb.InternalAuthenticateRequest{ + Name: r.Name, + Password: r.Password, + SimpleToken: st, + } + + result, err = s.processInternalRaftRequestOnce(ctx, pb.InternalRaftRequest{Authenticate: internalReq}) + if err != nil { + return nil, err + } + if result.err != nil { + return nil, result.err + } + + if checkedRevision != s.AuthStore().Revision() { + plog.Infof("revision when password checked is obsolete, retrying") + continue + } + + break } - internalReq := &pb.InternalAuthenticateRequest{ - Name: r.Name, - Password: r.Password, - SimpleToken: st, - } - - result, err := s.processInternalRaftRequestOnce(ctx, pb.InternalRaftRequest{Authenticate: internalReq}) - if err != nil { - return nil, err - } - if result.err != nil { - return nil, result.err - } return result.resp.(*pb.AuthenticateResponse), nil }