]> source.dussan.org Git - gitea.git/commitdiff
[Refactor] Passwort Hash/Set (#14282)
author6543 <6543@obermui.de>
Sun, 10 Jan 2021 18:05:18 +0000 (19:05 +0100)
committerGitHub <noreply@github.com>
Sun, 10 Jan 2021 18:05:18 +0000 (20:05 +0200)
* 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

cmd/admin.go
models/login_source.go
models/migrations/migrations.go
models/migrations/v166.go [new file with mode: 0644]
models/user.go
models/user_test.go
routers/admin/users.go
routers/api/v1/admin/user.go
routers/user/auth.go
routers/user/setting/account.go

index b4e0c3cb5ee6b6de63e907757638ca6fa3d7903b..3ddf8eddf9e6c9e96122cae69a6f5f788c8fa44a 100644 (file)
@@ -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
        }
 
index f1941c3e786c6c57c28474ca68f826a14f2e3204..d351f12861f7906229a7eb6905089269b97bd6de 100644 (file)
@@ -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
                                        }
                                }
index 4f3d823b9afa14020c39e6049caeb2b051e9e459..f62bba2a71906db9a51e1b511dc957670356c7d7 100644 (file)
@@ -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 (file)
index 0000000..3d6cf4f
--- /dev/null
@@ -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()
+}
index d3f1b16c2e7f2b3951ed23061e0a89050fe142e2..dbd2372fcfb099ad21a48fdd4b24b0409332ca28 100644 (file)
@@ -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
index 224acd5f3c5908e9b979210fb04615cf38878a0f..69cb21b975fa795dfe92895781383562cbe330ac 100644 (file)
@@ -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)
        }
 }
 
index 8a848b1f9ddcf38a421403e909def3d69d7bc00e..9fe5c3ef7deec2dbbcc5c20060b0f24882e84daf 100644 (file)
@@ -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 {
index 93b28c2175bd49c4e595cc799f788ba12ee3ea87..670baf020b95b22c3970180518ab45723549625d 100644 (file)
@@ -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 {
index 540a0d2f1a8e57c0cc4d0a0309677bf3a288b937..bce801847d2f61436004adb9864dad55b489c31b 100644 (file)
@@ -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 {
index 4fb2e4be402eeff5c1736660523176f0d032744f..ca9b5b3c3205b39ceef937ad604b1b0fbbbd2bb6 100644 (file)
@@ -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