[Refactor] Passwort Hash/Set (#14282)
* move SaltGeneration into HashPasswort and rename it to what it does * Migration: Where Password is Valid with Empty String delete it * prohibit empty password hash * let SetPassword("") unset pwd stuff
This commit is contained in:
parent
6b3b6f1833
commit
74a0481586
10 changed files with 158 additions and 32 deletions
|
@ -349,12 +349,11 @@ func runChangePassword(c *cli.Context) error {
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if user.Salt, err = models.GetUserSalt(); err != nil {
|
||||
if err = user.SetPassword(c.String("password")); err != nil {
|
||||
return err
|
||||
}
|
||||
user.HashPassword(c.String("password"))
|
||||
|
||||
if err := models.UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
|
||||
if err = models.UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
|
@ -771,8 +771,10 @@ func UserSignIn(username, password string) (*User, error) {
|
|||
|
||||
// Update password hash if server password hash algorithm have changed
|
||||
if user.PasswdHashAlgo != setting.PasswordHashAlgo {
|
||||
user.HashPassword(password)
|
||||
if err := UpdateUserCols(user, "passwd", "passwd_hash_algo"); err != nil {
|
||||
if err = user.SetPassword(password); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err = UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
|
|
@ -277,6 +277,8 @@ var migrations = []Migration{
|
|||
NewMigration("Add scope and nonce columns to oauth2_grant table", addScopeAndNonceColumnsToOAuth2Grant),
|
||||
// v165 -> v166
|
||||
NewMigration("Convert hook task type from char(16) to varchar(16) and trim the column", convertHookTaskTypeToVarcharAndTrim),
|
||||
// v166 -> v167
|
||||
NewMigration("Where Password is Valid with Empty String delete it", recalculateUserEmptyPWD),
|
||||
}
|
||||
|
||||
// GetCurrentDBVersion returns the current db version
|
||||
|
|
115
models/migrations/v166.go
Normal file
115
models/migrations/v166.go
Normal file
|
@ -0,0 +1,115 @@
|
|||
// Copyright 2021 The Gitea Authors. All rights reserved.
|
||||
// Use of this source code is governed by a MIT-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
package migrations
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"fmt"
|
||||
|
||||
"golang.org/x/crypto/argon2"
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
"golang.org/x/crypto/pbkdf2"
|
||||
"golang.org/x/crypto/scrypt"
|
||||
"xorm.io/builder"
|
||||
"xorm.io/xorm"
|
||||
)
|
||||
|
||||
func recalculateUserEmptyPWD(x *xorm.Engine) (err error) {
|
||||
const (
|
||||
algoBcrypt = "bcrypt"
|
||||
algoScrypt = "scrypt"
|
||||
algoArgon2 = "argon2"
|
||||
algoPbkdf2 = "pbkdf2"
|
||||
)
|
||||
|
||||
type User struct {
|
||||
ID int64 `xorm:"pk autoincr"`
|
||||
Passwd string `xorm:"NOT NULL"`
|
||||
PasswdHashAlgo string `xorm:"NOT NULL DEFAULT 'argon2'"`
|
||||
MustChangePassword bool `xorm:"NOT NULL DEFAULT false"`
|
||||
LoginType int
|
||||
LoginName string
|
||||
Type int
|
||||
Salt string `xorm:"VARCHAR(10)"`
|
||||
}
|
||||
|
||||
// hashPassword hash password based on algo and salt
|
||||
// state 461406070c
|
||||
hashPassword := func(passwd, salt, algo string) string {
|
||||
var tempPasswd []byte
|
||||
|
||||
switch algo {
|
||||
case algoBcrypt:
|
||||
tempPasswd, _ = bcrypt.GenerateFromPassword([]byte(passwd), bcrypt.DefaultCost)
|
||||
return string(tempPasswd)
|
||||
case algoScrypt:
|
||||
tempPasswd, _ = scrypt.Key([]byte(passwd), []byte(salt), 65536, 16, 2, 50)
|
||||
case algoArgon2:
|
||||
tempPasswd = argon2.IDKey([]byte(passwd), []byte(salt), 2, 65536, 8, 50)
|
||||
case algoPbkdf2:
|
||||
fallthrough
|
||||
default:
|
||||
tempPasswd = pbkdf2.Key([]byte(passwd), []byte(salt), 10000, 50, sha256.New)
|
||||
}
|
||||
|
||||
return fmt.Sprintf("%x", tempPasswd)
|
||||
}
|
||||
|
||||
// ValidatePassword checks if given password matches the one belongs to the user.
|
||||
// state 461406070c, changed since it's not necessary to be time constant
|
||||
ValidatePassword := func(u *User, passwd string) bool {
|
||||
tempHash := hashPassword(passwd, u.Salt, u.PasswdHashAlgo)
|
||||
|
||||
if u.PasswdHashAlgo != algoBcrypt && u.Passwd == tempHash {
|
||||
return true
|
||||
}
|
||||
if u.PasswdHashAlgo == algoBcrypt && bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
sess := x.NewSession()
|
||||
defer sess.Close()
|
||||
|
||||
const batchSize = 100
|
||||
|
||||
for start := 0; ; start += batchSize {
|
||||
users := make([]*User, 0, batchSize)
|
||||
if err = sess.Limit(batchSize, start).Where(builder.Neq{"passwd": ""}, 0).Find(&users); err != nil {
|
||||
return
|
||||
}
|
||||
if len(users) == 0 {
|
||||
break
|
||||
}
|
||||
|
||||
if err = sess.Begin(); err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
for _, user := range users {
|
||||
if ValidatePassword(user, "") {
|
||||
user.Passwd = ""
|
||||
user.Salt = ""
|
||||
user.PasswdHashAlgo = ""
|
||||
if _, err = sess.ID(user.ID).Cols("passwd", "salt", "passwd_hash_algo").Update(user); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if err = sess.Commit(); err != nil {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// delete salt and algo where password is empty
|
||||
if _, err = sess.Where(builder.Eq{"passwd": ""}.And(builder.Neq{"salt": ""}.Or(builder.Neq{"passwd_hash_algo": ""}))).
|
||||
Cols("salt", "passwd_hash_algo").Update(&User{}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return sess.Commit()
|
||||
}
|
|
@ -395,10 +395,23 @@ func hashPassword(passwd, salt, algo string) string {
|
|||
return fmt.Sprintf("%x", tempPasswd)
|
||||
}
|
||||
|
||||
// HashPassword hashes a password using the algorithm defined in the config value of PASSWORD_HASH_ALGO.
|
||||
func (u *User) HashPassword(passwd string) {
|
||||
// SetPassword hashes a password using the algorithm defined in the config value of PASSWORD_HASH_ALGO
|
||||
// change passwd, salt and passwd_hash_algo fields
|
||||
func (u *User) SetPassword(passwd string) (err error) {
|
||||
if len(passwd) == 0 {
|
||||
u.Passwd = ""
|
||||
u.Salt = ""
|
||||
u.PasswdHashAlgo = ""
|
||||
return nil
|
||||
}
|
||||
|
||||
if u.Salt, err = GetUserSalt(); err != nil {
|
||||
return err
|
||||
}
|
||||
u.PasswdHashAlgo = setting.PasswordHashAlgo
|
||||
u.Passwd = hashPassword(passwd, u.Salt, setting.PasswordHashAlgo)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidatePassword checks if given password matches the one belongs to the user.
|
||||
|
@ -416,7 +429,7 @@ func (u *User) ValidatePassword(passwd string) bool {
|
|||
|
||||
// IsPasswordSet checks if the password is set or left empty
|
||||
func (u *User) IsPasswordSet() bool {
|
||||
return !u.ValidatePassword("")
|
||||
return len(u.Passwd) != 0
|
||||
}
|
||||
|
||||
// IsOrganization returns true if user is actually a organization.
|
||||
|
@ -826,10 +839,9 @@ func CreateUser(u *User) (err error) {
|
|||
if u.Rands, err = GetUserSalt(); err != nil {
|
||||
return err
|
||||
}
|
||||
if u.Salt, err = GetUserSalt(); err != nil {
|
||||
if err = u.SetPassword(u.Passwd); err != nil {
|
||||
return err
|
||||
}
|
||||
u.HashPassword(u.Passwd)
|
||||
u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization && !setting.Admin.DisableRegularOrgCreation
|
||||
u.EmailNotificationsPreference = setting.Admin.DefaultEmailNotification
|
||||
u.MaxRepoCreation = -1
|
||||
|
|
|
@ -220,8 +220,7 @@ func TestEmailNotificationPreferences(t *testing.T) {
|
|||
|
||||
func TestHashPasswordDeterministic(t *testing.T) {
|
||||
b := make([]byte, 16)
|
||||
rand.Read(b)
|
||||
u := &User{Salt: string(b)}
|
||||
u := &User{}
|
||||
algos := []string{"argon2", "pbkdf2", "scrypt", "bcrypt"}
|
||||
for j := 0; j < len(algos); j++ {
|
||||
u.PasswdHashAlgo = algos[j]
|
||||
|
@ -231,19 +230,15 @@ func TestHashPasswordDeterministic(t *testing.T) {
|
|||
pass := string(b)
|
||||
|
||||
// save the current password in the user - hash it and store the result
|
||||
u.HashPassword(pass)
|
||||
u.SetPassword(pass)
|
||||
r1 := u.Passwd
|
||||
|
||||
// run again
|
||||
u.HashPassword(pass)
|
||||
u.SetPassword(pass)
|
||||
r2 := u.Passwd
|
||||
|
||||
// assert equal (given the same salt+pass, the same result is produced) except bcrypt
|
||||
if u.PasswdHashAlgo == "bcrypt" {
|
||||
assert.NotEqual(t, r1, r2)
|
||||
} else {
|
||||
assert.Equal(t, r1, r2)
|
||||
}
|
||||
assert.NotEqual(t, r1, r2)
|
||||
assert.True(t, u.ValidatePassword(pass))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -252,12 +247,10 @@ func BenchmarkHashPassword(b *testing.B) {
|
|||
// BenchmarkHashPassword ensures that it takes a reasonable amount of time
|
||||
// to hash a password - in order to protect from brute-force attacks.
|
||||
pass := "password1337"
|
||||
bs := make([]byte, 16)
|
||||
rand.Read(bs)
|
||||
u := &User{Salt: string(bs), Passwd: pass}
|
||||
u := &User{Passwd: pass}
|
||||
b.ResetTimer()
|
||||
for i := 0; i < b.N; i++ {
|
||||
u.HashPassword(pass)
|
||||
u.SetPassword(pass)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -267,7 +267,10 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) {
|
|||
ctx.ServerError("UpdateUser", err)
|
||||
return
|
||||
}
|
||||
u.HashPassword(form.Password)
|
||||
if err = u.SetPassword(form.Password); err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if len(form.UserName) != 0 && u.Name != form.UserName {
|
||||
|
|
|
@ -174,7 +174,10 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) {
|
|||
ctx.Error(http.StatusInternalServerError, "UpdateUser", err)
|
||||
return
|
||||
}
|
||||
u.HashPassword(form.Password)
|
||||
if err = u.SetPassword(form.Password); err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if form.MustChangePassword != nil {
|
||||
|
|
|
@ -1517,11 +1517,10 @@ func ResetPasswdPost(ctx *context.Context) {
|
|||
ctx.ServerError("UpdateUser", err)
|
||||
return
|
||||
}
|
||||
if u.Salt, err = models.GetUserSalt(); err != nil {
|
||||
if err = u.SetPassword(passwd); err != nil {
|
||||
ctx.ServerError("UpdateUser", err)
|
||||
return
|
||||
}
|
||||
u.HashPassword(passwd)
|
||||
u.MustChangePassword = false
|
||||
if err := models.UpdateUserCols(u, "must_change_password", "passwd", "passwd_hash_algo", "rands", "salt"); err != nil {
|
||||
ctx.ServerError("UpdateUser", err)
|
||||
|
@ -1591,12 +1590,11 @@ func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form aut
|
|||
}
|
||||
|
||||
var err error
|
||||
if u.Salt, err = models.GetUserSalt(); err != nil {
|
||||
if err = u.SetPassword(form.Password); err != nil {
|
||||
ctx.ServerError("UpdateUser", err)
|
||||
return
|
||||
}
|
||||
|
||||
u.HashPassword(form.Password)
|
||||
u.MustChangePassword = false
|
||||
|
||||
if err := models.UpdateUserCols(u, "must_change_password", "passwd", "passwd_hash_algo", "salt"); err != nil {
|
||||
|
|
|
@ -63,11 +63,10 @@ func AccountPost(ctx *context.Context, form auth.ChangePasswordForm) {
|
|||
ctx.Flash.Error(errMsg)
|
||||
} else {
|
||||
var err error
|
||||
if ctx.User.Salt, err = models.GetUserSalt(); err != nil {
|
||||
if err = ctx.User.SetPassword(form.Password); err != nil {
|
||||
ctx.ServerError("UpdateUser", err)
|
||||
return
|
||||
}
|
||||
ctx.User.HashPassword(form.Password)
|
||||
if err := models.UpdateUserCols(ctx.User, "salt", "passwd_hash_algo", "passwd"); err != nil {
|
||||
ctx.ServerError("UpdateUser", err)
|
||||
return
|
||||
|
|
Reference in a new issue