Merge pull request #6640 from mitake/bcrypt-async

auth, etcdserver: check password at API layer

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

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)
 	}

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
 
-	internalReq := &pb.InternalAuthenticateRequest{
-		Name:        r.Name,
-		Password:    r.Password,
-		SimpleToken: st,
-	}
+	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
+		}
 
-	result, err := s.processInternalRaftRequestOnce(ctx, pb.InternalRaftRequest{Authenticate: internalReq})
-	if err != nil {
-		return nil, err
-	}
-	if result.err != nil {
-		return nil, result.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
 	}
+
 	return result.resp.(*pb.AuthenticateResponse), nil
 }