Enhance: Upgrade encrypt alg to sha256

previous sha1 will still used for old password

Signed-off-by: DQ <dengq@vmware.com>

make/migrations/postgresql/0010_1.9.0_schema.up.sql

@@ -186,3 +186,5 @@ create table notification_policy (
 ALTER TABLE replication_task ADD COLUMN status_revision int DEFAULT 0;
 DELETE FROM project_metadata WHERE deleted = TRUE;
 ALTER TABLE project_metadata DROP COLUMN deleted;
+ALTER TABLE harbor_user ADD COLUMN password_version varchar(16) Default 'sha256';
+UPDATE harbor_user SET password_version = 'sha1';

src/common/dao/dao_test.go

@@ -324,7 +324,12 @@ func TestResetUserPassword(t *testing.T) {
 		t.Errorf("Error occurred in UpdateUserResetUuid: %v", err)
 	}
 
-	err = ResetUserPassword(models.User{UserID: currentUser.UserID, Password: "HarborTester12345", ResetUUID: uuid, Salt: currentUser.Salt})
+	err = ResetUserPassword(
+		models.User{
+			UserID:          currentUser.UserID,
+			PasswordVersion: utils.SHA256,
+			ResetUUID:       uuid,
+			Salt:            currentUser.Salt}, "HarborTester12345")
 	if err != nil {
 		t.Errorf("Error occurred in ResetUserPassword: %v", err)
 	}
@@ -346,7 +351,12 @@ func TestChangeUserPassword(t *testing.T) {
 		t.Errorf("Error occurred when get user salt")
 	}
 	currentUser.Salt = query.Salt
-	err = ChangeUserPassword(models.User{UserID: currentUser.UserID, Password: "NewHarborTester12345", Salt: currentUser.Salt})
+	err = ChangeUserPassword(
+		models.User{
+			UserID:          currentUser.UserID,
+			Password:        "NewHarborTester12345",
+			PasswordVersion: utils.SHA256,
+			Salt:            currentUser.Salt})
 	if err != nil {
 		t.Errorf("Error occurred in ChangeUserPassword: %v", err)
 	}

src/common/dao/register.go

@@ -29,10 +29,10 @@ func Register(user models.User) (int64, error) {
 	now := time.Now()
 	salt := utils.GenerateRandomString()
 	sql := `insert into harbor_user
-				(username, password, realname, email, comment, salt, sysadmin_flag, creation_time, update_time)
+				(username, password, password_version, realname, email, comment, salt, sysadmin_flag, creation_time, update_time)
-				 values (?, ?, ?, ?, ?, ?, ?, ?, ?) RETURNING user_id`
+				 values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) RETURNING user_id`
 	var userID int64
-	err := o.Raw(sql, user.Username, utils.Encrypt(user.Password, salt), user.Realname, user.Email,
+	err := o.Raw(sql, user.Username, utils.Encrypt(user.Password, salt, utils.SHA256), utils.SHA256, user.Realname, user.Email,
 		user.Comment, salt, user.HasAdminRole, now, now).QueryRow(&userID)
 	if err != nil {
 		return 0, err

src/common/dao/user.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
-
 	"github.com/goharbor/harbor/src/common/utils/log"
 )
 
@@ -32,7 +31,7 @@ func GetUser(query models.User) (*models.User, error) {
 
 	o := GetOrmer()
 
-	sql := `select user_id, username, password, email, realname, comment, reset_uuid, salt,
+	sql := `select user_id, username, password, password_version, email, realname, comment, reset_uuid, salt,
 		sysadmin_flag, creation_time, update_time
 		from harbor_user u
 		where deleted = false `
@@ -76,9 +75,9 @@ func GetUser(query models.User) (*models.User, error) {
 
 // LoginByDb is used for user to login with database auth mode.
 func LoginByDb(auth models.AuthModel) (*models.User, error) {
+	var users []models.User
 	o := GetOrmer()
 
-	var users []models.User
 	n, err := o.Raw(`select * from harbor_user where (username = ? or email = ?) and deleted = false`,
 		auth.Principal, auth.Principal).QueryRows(&users)
 	if err != nil {
@@ -90,12 +89,10 @@ func LoginByDb(auth models.AuthModel) (*models.User, error) {
 
 	user := users[0]
 
-	if user.Password != utils.Encrypt(auth.Password, user.Salt) {
+	if !matchPassword(&user, auth.Password) {
 		return nil, nil
 	}
-
 	user.Password = "" // do not return the password
-
 	return &user, nil
 }
 
@@ -165,23 +162,34 @@ func ToggleUserAdminRole(userID int, hasAdmin bool) error {
 func ChangeUserPassword(u models.User) error {
 	u.UpdateTime = time.Now()
 	u.Salt = utils.GenerateRandomString()
-	u.Password = utils.Encrypt(u.Password, u.Salt)
+	u.Password = utils.Encrypt(u.Password, u.Salt, utils.SHA256)
-	_, err := GetOrmer().Update(&u, "Password", "Salt", "UpdateTime")
+	var err error
+	if u.PasswordVersion == utils.SHA1 {
+		u.PasswordVersion = utils.SHA256
+		_, err = GetOrmer().Update(&u, "Password", "PasswordVersion", "Salt", "UpdateTime")
+	} else {
+		_, err = GetOrmer().Update(&u, "Password", "Salt", "UpdateTime")
+	}
 	return err
 }
 
 // ResetUserPassword ...
-func ResetUserPassword(u models.User) error {
+func ResetUserPassword(u models.User, rawPassword string) error {
-	o := GetOrmer()
+	var rowsAffected int64
-	r, err := o.Raw(`update harbor_user set password=?, reset_uuid=? where reset_uuid=?`, utils.Encrypt(u.Password, u.Salt), "", u.ResetUUID).Exec()
+	var err error
+	u.UpdateTime = time.Now()
+	u.Password = utils.Encrypt(rawPassword, u.Salt, utils.SHA256)
+	u.ResetUUID = ""
+	if u.PasswordVersion == utils.SHA1 {
+		u.PasswordVersion = utils.SHA256
+		rowsAffected, err = GetOrmer().Update(&u, "Password", "PasswordVersion", "ResetUUID", "UpdateTime")
+	} else {
+		rowsAffected, err = GetOrmer().Update(&u, "Password", "ResetUUID", "UpdateTime")
+	}
 	if err != nil {
 		return err
 	}
-	count, err := r.RowsAffected()
+	if rowsAffected == 0 {
-	if err != nil {
-		return err
-	}
-	if count == 0 {
 		return errors.New("no record be changed, reset password failed")
 	}
 	return nil
@@ -282,3 +290,11 @@ func CleanUser(id int64) error {
 	}
 	return nil
 }
+
+// MatchPassword returns true is password matched
+func matchPassword(u *models.User, password string) bool {
+	if u.Password != utils.Encrypt(password, u.Salt, u.PasswordVersion) {
+		return false
+	}
+	return true
+}

src/common/models/user.go

@@ -27,6 +27,7 @@ type User struct {
 	Username        string `orm:"column(username)" json:"username"`
 	Email           string `orm:"column(email)" json:"email"`
 	Password        string `orm:"column(password)" json:"password"`
+	PasswordVersion string `orm:"column(password_version)" json:"password_version"`
 	Realname        string `orm:"column(realname)" json:"realname"`
 	Comment         string `orm:"column(comment)" json:"comment"`
 	Deleted         bool   `orm:"column(deleted)" json:"deleted"`

src/common/utils/encrypt.go

@@ -19,25 +19,37 @@ import (
 	"crypto/cipher"
 	"crypto/rand"
 	"crypto/sha1"
+	"crypto/sha256"
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"hash"
 	"io"
 	"strings"
 
 	"golang.org/x/crypto/pbkdf2"
 )
 
-// Encrypt encrypts the content with salt
-func Encrypt(content string, salt string) string {
-	return fmt.Sprintf("%x", pbkdf2.Key([]byte(content), []byte(salt), 4096, 16, sha1.New))
-}
-
 const (
 	// EncryptHeaderV1 ...
 	EncryptHeaderV1 = "<enc-v1>"
+	// SHA1 is the name of sha1 hash alg
+	SHA1 = "sha1"
+	// SHA256 is the name of sha256 hash alg
+	SHA256 = "sha256"
 )
 
+// HashAlg used to get correct alg for hash
+var HashAlg = map[string]func() hash.Hash{
+	SHA1:   sha1.New,
+	SHA256: sha256.New,
+}
+
+// Encrypt encrypts the content with salt
+func Encrypt(content string, salt string, encrptAlg string) string {
+	return fmt.Sprintf("%x", pbkdf2.Key([]byte(content), []byte(salt), 4096, 16, HashAlg[encrptAlg]))
+}
+
 // ReversibleEncrypt encrypts the str with aes/base64
 func ReversibleEncrypt(str, key string) (string, error) {
 	keyBytes := []byte(key)

src/common/utils/test/database.go

@@ -89,7 +89,6 @@ func updateUserInitialPassword(userID int, password string) error {
 		if err != nil {
 			return fmt.Errorf("Failed to update user encrypted password, userID: %d, err: %v", userID, err)
 		}
-	} else {
 	}
 	return nil
 }

src/common/utils/utils_test.go

@@ -17,6 +17,7 @@ package utils
 import (
 	"encoding/base64"
 	"net/http/httptest"
+	"reflect"
 	"strconv"
 	"strings"
 	"testing"
@@ -91,12 +92,21 @@ func TestParseRepository(t *testing.T) {
 }
 
 func TestEncrypt(t *testing.T) {
-	content := "content"
+	tests := map[string]struct {
-	salt := "salt"
+		content string
-	result := Encrypt(content, salt)
+		salt    string
+		alg     string
+		want    string
+	}{
+		"sha1 test":   {content: "content", salt: "salt", alg: SHA1, want: "dc79e76c88415c97eb089d9cc80b4ab0"},
+		"sha256 test": {content: "content", salt: "salt", alg: SHA256, want: "83d3d6f3e7cacb040423adf7ced63d21"},
+	}
 
-	if result != "dc79e76c88415c97eb089d9cc80b4ab0" {
+	for name, tc := range tests {
-		t.Errorf("unexpected result: %s != %s", result, "dc79e76c88415c97eb089d9cc80b4ab0")
+		got := Encrypt(tc.content, tc.salt, tc.alg)
+		if !reflect.DeepEqual(tc.want, got) {
+			t.Errorf("%s: expected: %v, got: %v", name, tc.want, got)
+		}
 	}
 }
 

src/core/api/user.go

@@ -17,6 +17,10 @@ package api
 import (
 	"errors"
 	"fmt"
+	"net/http"
+	"regexp"
+	"strconv"
+
 	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
@@ -25,9 +29,6 @@ import (
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/core/config"
-	"net/http"
-	"regexp"
-	"strconv"
 )
 
 // UserAPI handles request to /api/users/{}
@@ -416,13 +417,13 @@ func (ua *UserAPI) ChangePassword() {
 		return
 	}
 	if changePwdOfOwn {
-		if user.Password != utils.Encrypt(req.OldPassword, user.Salt) {
+		if user.Password != utils.Encrypt(req.OldPassword, user.Salt, user.PasswordVersion) {
 			log.Info("incorrect old_password")
 			ua.SendForbiddenError(errors.New("incorrect old_password"))
 			return
 		}
 	}
-	if user.Password == utils.Encrypt(req.NewPassword, user.Salt) {
+	if user.Password == utils.Encrypt(req.NewPassword, user.Salt, user.PasswordVersion) {
 		ua.SendBadRequestError(errors.New("the new password can not be same with the old one"))
 		return
 	}
@@ -430,6 +431,7 @@ func (ua *UserAPI) ChangePassword() {
 	updatedUser := models.User{
 		UserID:          ua.userID,
 		Password:        req.NewPassword,
+		PasswordVersion: user.PasswordVersion,
 	}
 	if err = dao.ChangeUserPassword(updatedUser); err != nil {
 		ua.SendInternalServerError(fmt.Errorf("failed to change password of user %d: %v", ua.userID, err))

src/core/controllers/base.go

@@ -17,7 +17,6 @@ package controllers
 import (
 	"bytes"
 	"context"
-	"github.com/goharbor/harbor/src/core/filter"
 	"html/template"
 	"net"
 	"net/http"
@@ -26,6 +25,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/goharbor/harbor/src/core/filter"
+
 	"github.com/astaxie/beego"
 	"github.com/beego/i18n"
 	"github.com/goharbor/harbor/src/common"
@@ -252,11 +253,10 @@ func (cc *CommonController) ResetPassword() {
 		cc.CustomAbort(http.StatusForbidden, http.StatusText(http.StatusForbidden))
 	}
 
-	password := cc.GetString("password")
+	rawPassword := cc.GetString("password")
 
-	if password != "" {
+	if rawPassword != "" {
-		user.Password = password
+		err = dao.ResetUserPassword(*user, rawPassword)
-		err = dao.ResetUserPassword(*user)
 		if err != nil {
 			log.Errorf("Error occurred in ResetUserPassword: %v", err)
 			cc.CustomAbort(http.StatusInternalServerError, "Internal error.")