summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--cmd/admin.go5
-rw-r--r--models/login_source.go6
-rw-r--r--models/migrations/migrations.go2
-rw-r--r--models/migrations/v166.go115
-rw-r--r--models/user.go22
-rw-r--r--models/user_test.go21
-rw-r--r--routers/admin/users.go5
-rw-r--r--routers/api/v1/admin/user.go5
-rw-r--r--routers/user/auth.go6
-rw-r--r--routers/user/setting/account.go3
10 files changed, 158 insertions, 32 deletions
diff --git a/cmd/admin.go b/cmd/admin.go
index b4e0c3cb5e..3ddf8eddf9 100644
--- a/cmd/admin.go
+++ b/cmd/admin.go
@@ -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
}
diff --git a/models/login_source.go b/models/login_source.go
index f1941c3e78..d351f12861 100644
--- a/models/login_source.go
+++ b/models/login_source.go
@@ -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
}
}
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index 4f3d823b9a..f62bba2a71 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -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
diff --git a/models/migrations/v166.go b/models/migrations/v166.go
new file mode 100644
index 0000000000..3d6cf4f57f
--- /dev/null
+++ b/models/migrations/v166.go
@@ -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()
+}
diff --git a/models/user.go b/models/user.go
index d3f1b16c2e..dbd2372fcf 100644
--- a/models/user.go
+++ b/models/user.go
@@ -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
diff --git a/models/user_test.go b/models/user_test.go
index 224acd5f3c..69cb21b975 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -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)
}
}
diff --git a/routers/admin/users.go b/routers/admin/users.go
index 8a848b1f9d..9fe5c3ef7d 100644
--- a/routers/admin/users.go
+++ b/routers/admin/users.go
@@ -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 {
diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
index 93b28c2175..670baf020b 100644
--- a/routers/api/v1/admin/user.go
+++ b/routers/api/v1/admin/user.go
@@ -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 {
diff --git a/routers/user/auth.go b/routers/user/auth.go
index 540a0d2f1a..bce801847d 100644
--- a/routers/user/auth.go
+++ b/routers/user/auth.go
@@ -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 {
diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go
index 4fb2e4be40..ca9b5b3c32 100644
--- a/routers/user/setting/account.go
+++ b/routers/user/setting/account.go
@@ -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