Provide the ability to set password hash algorithm parameters

This PR refactors and improves the password hashing code within gitea
and makes it possible for server administrators to set the password
hashing parameters

In addition it takes the opportunity to adjust the settings for `pbkdf2`
in order to make the hashing a little stronger.

The majority of this work was inspired by PR go-gitea#14751 and I would like to
thank @boppy for their work on this.

Thanks to @Gusted for the suggestion to adjust the `pbkdf2` hashing
parameters.

Close go-gitea#14751

Signed-off-by: Andrew Thornton <[email protected]>

Author: zeripath

cmd/admin_user_change_password.go

@@ -9,7 +9,7 @@ import (
 	"fmt"
 
 	user_model "code.gitea.io/gitea/models/user"
-	pwd "code.gitea.io/gitea/modules/password"
+	pwd "code.gitea.io/gitea/modules/auth/password"
 	"code.gitea.io/gitea/modules/setting"
 
 	"github.com/urfave/cli"

cmd/admin_user_create.go

@@ -10,7 +10,7 @@ import (
 
 	auth_model "code.gitea.io/gitea/models/auth"
 	user_model "code.gitea.io/gitea/models/user"
-	pwd "code.gitea.io/gitea/modules/password"
+	pwd "code.gitea.io/gitea/modules/auth/password"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -568,7 +568,21 @@ Certain queues have defaults that override the defaults set in `[queue]` (this o
 - `IMPORT_LOCAL_PATHS`: **false**: Set to `false` to prevent all users (including admin) from importing local path on server.
 - `INTERNAL_TOKEN`: **\<random at every install if no uri set\>**: Secret used to validate communication within Gitea binary.
 - `INTERNAL_TOKEN_URI`: **<empty>**: Instead of defining INTERNAL_TOKEN in the configuration, this configuration option can be used to give Gitea a path to a file that contains the internal token (example value: `file:/etc/gitea/internal_token`)
-- `PASSWORD_HASH_ALGO`: **pbkdf2**: The hash algorithm to use \[argon2, pbkdf2, scrypt, bcrypt\], argon2 will spend more memory than others.
+- `PASSWORD_HASH_ALGO`: **pbkdf2**: The hash algorithm to use \[argon2, pbkdf2, pbkdf2_v1, scrypt, bcrypt\], argon2 and scrypt will spend significant amounts of memory.
+  - Note: The default parameters for `pbkdf2` hashing have changed - the previous settings are available as `pbkdf2_v1` but are not recommended.
+  - The hash functions may be tuned by using `$` after the algorithm:
+    - `argon2$<time>$<memory>$<threads>$<key-length>`
+    - `pbkdf2$<iterations>$<key-length>`
+    - `scrypt$<n>$<r>$<p>$<key-length>`
+    - `bcrypt$<cost>`
+  - The defaults are:
+    - `argon2`:    `argon2$2$65536$8$50`
+    - `bcrypt`:    `bcrypt$10`
+    - `scrypt`:    `scrypt$65536$16$2$50`
+    - `pbkdf2`:    `pbkdf2$320000$50`
+    - `pbkdf2_v1`: `pbkdf2$10000$50`
+    - `pbkdf2_v2`: `pbkdf2$320000$50`
+  - Adjusting the algorithm parameters using this functionality is done at your own risk.
 - `CSRF_COOKIE_HTTP_ONLY`: **true**: Set false to allow JavaScript to read CSRF cookie.
 - `MIN_PASSWORD_LENGTH`: **6**: Minimum password length for new users.
 - `PASSWORD_COMPLEXITY`: **off**: Comma separated list of character classes required to pass minimum complexity. If left empty or no valid values are specified, checking is disabled (off):

models/user/user.go

@@ -6,8 +6,6 @@ package user
 
 import (
 	"context"
-	"crypto/sha256"
-	"crypto/subtle"
 	"encoding/hex"
 	"fmt"
 	"net/url"
@@ -21,6 +19,7 @@ import (
 	"code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/modules/auth/openid"
+	"code.gitea.io/gitea/modules/auth/password/hash"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
@@ -30,10 +29,6 @@ import (
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/validation"
 
-	"golang.org/x/crypto/argon2"
-	"golang.org/x/crypto/bcrypt"
-	"golang.org/x/crypto/pbkdf2"
-	"golang.org/x/crypto/scrypt"
 	"xorm.io/builder"
 )
 
@@ -48,21 +43,6 @@ const (
 	UserTypeOrganization
 )
 
-const (
-	algoBcrypt = "bcrypt"
-	algoScrypt = "scrypt"
-	algoArgon2 = "argon2"
-	algoPbkdf2 = "pbkdf2"
-)
-
-// AvailableHashAlgorithms represents the available password hashing algorithms
-var AvailableHashAlgorithms = []string{
-	algoPbkdf2,
-	algoArgon2,
-	algoScrypt,
-	algoBcrypt,
-}
-
 const (
 	// EmailNotificationsEnabled indicates that the user would like to receive all email notifications except your own
 	EmailNotificationsEnabled = "enabled"
@@ -377,42 +357,6 @@ func (u *User) NewGitSig() *git.Signature {
 	}
 }
 
-func hashPassword(passwd, salt, algo string) (string, error) {
-	var tempPasswd []byte
-	var saltBytes []byte
-
-	// There are two formats for the Salt value:
-	// * The new format is a (32+)-byte hex-encoded string
-	// * The old format was a 10-byte binary format
-	// We have to tolerate both here but Authenticate should
-	// regenerate the Salt following a successful validation.
-	if len(salt) == 10 {
-		saltBytes = []byte(salt)
-	} else {
-		var err error
-		saltBytes, err = hex.DecodeString(salt)
-		if err != nil {
-			return "", err
-		}
-	}
-
-	switch algo {
-	case algoBcrypt:
-		tempPasswd, _ = bcrypt.GenerateFromPassword([]byte(passwd), bcrypt.DefaultCost)
-		return string(tempPasswd), nil
-	case algoScrypt:
-		tempPasswd, _ = scrypt.Key([]byte(passwd), saltBytes, 65536, 16, 2, 50)
-	case algoArgon2:
-		tempPasswd = argon2.IDKey([]byte(passwd), saltBytes, 2, 65536, 8, 50)
-	case algoPbkdf2:
-		fallthrough
-	default:
-		tempPasswd = pbkdf2.Key([]byte(passwd), saltBytes, 10000, 50, sha256.New)
-	}
-
-	return hex.EncodeToString(tempPasswd), nil
-}
-
 // 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) {
@@ -426,28 +370,17 @@ func (u *User) SetPassword(passwd string) (err error) {
 	if u.Salt, err = GetUserSalt(); err != nil {
 		return err
 	}
-	if u.Passwd, err = hashPassword(passwd, u.Salt, setting.PasswordHashAlgo); err != nil {
+	if u.Passwd, err = hash.Parse(setting.PasswordHashAlgo).Hash(passwd, u.Salt); err != nil {
 		return err
 	}
 	u.PasswdHashAlgo = setting.PasswordHashAlgo
 
 	return nil
 }
 
-// ValidatePassword checks if given password matches the one belongs to the user.
+// ValidatePassword checks if the given password matches the one belonging to the user.
 func (u *User) ValidatePassword(passwd string) bool {
-	tempHash, err := hashPassword(passwd, u.Salt, u.PasswdHashAlgo)
-	if err != nil {
-		return false
-	}
-
-	if u.PasswdHashAlgo != algoBcrypt && subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(tempHash)) == 1 {
-		return true
-	}
-	if u.PasswdHashAlgo == algoBcrypt && bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil {
-		return true
-	}
-	return false
+	return hash.Parse(u.PasswdHashAlgo).VerifyPassword(passwd, u.Passwd, u.Salt)
 }
 
 // IsPasswordSet checks if the password is set or left empty

models/user/user_test.go

@@ -14,6 +14,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/auth/password/hash"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
@@ -164,7 +165,7 @@ func TestEmailNotificationPreferences(t *testing.T) {
 func TestHashPasswordDeterministic(t *testing.T) {
 	b := make([]byte, 16)
 	u := &user_model.User{}
-	algos := []string{"argon2", "pbkdf2", "scrypt", "bcrypt"}
+	algos := hash.RecommendedHashAlgorithms
 	for j := 0; j < len(algos); j++ {
 		u.PasswdHashAlgo = algos[j]
 		for i := 0; i < 50; i++ {

modules/auth/password/hash/argon2.go

@@ -0,0 +1,76 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package hash
+
+import (
+	"encoding/hex"
+	"strings"
+
+	"code.gitea.io/gitea/modules/log"
+	"golang.org/x/crypto/argon2"
+)
+
+func init() {
+	registerHasher("argon2", NewArgon2Hasher)
+}
+
+// Argon2Hasher implements PasswordHasher
+// and uses the Argon2 key derivation function, hybrant variant
+type Argon2Hasher struct {
+	time    uint32
+	memory  uint32
+	threads uint8
+	keyLen  uint32
+}
+
+// HashWithSaltBytes a provided password and salt
+func (hasher *Argon2Hasher) HashWithSaltBytes(password string, salt []byte) string {
+	if hasher == nil {
+		return ""
+	}
+	return hex.EncodeToString(argon2.IDKey([]byte(password), salt, hasher.time, hasher.memory, hasher.threads, hasher.keyLen))
+}
+
+// NewArgon2Hasher is a factory method to create an Argon2Hasher
+// The provided config should be either empty or of the form:
+// "<time>$<memory>$<threads>$<keyLen>", where <x> is the string representation
+// of an integer
+func NewArgon2Hasher(config string) *Argon2Hasher {
+	// This default configuration uses the following parameters:
+	// time=2, memory=64*1024, threads=8, keyLen=50.
+	// It will make two passes through the memory, using 64MiB in total.
+	hasher := &Argon2Hasher{
+		time:    2,
+		memory:  1 << 16,
+		threads: 8,
+		keyLen:  50,
+	}
+
+	if config == "" {
+		return hasher
+	}
+
+	vals := strings.SplitN(config, "$", 4)
+	if len(vals) != 4 {
+		log.Error("invalid argon2 hash spec %s", config)
+		return nil
+	}
+
+	parsed, err := parseUIntParam(vals[0], "time", "argon2", config, nil)
+	hasher.time = uint32(parsed)
+
+	parsed, err = parseUIntParam(vals[1], "memory", "argon2", config, err)
+	hasher.memory = uint32(parsed)
+
+	parsed, err = parseUIntParam(vals[2], "threads", "argon2", config, err)
+	hasher.threads = uint8(parsed)
+
+	parsed, err = parseUIntParam(vals[3], "keyLen", "argon2", config, err)
+	hasher.keyLen = uint32(parsed)
+	if err != nil {
+		return nil
+	}
+
+	return hasher
+}

modules/auth/password/hash/bcrypt.go

@@ -0,0 +1,51 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package hash
+
+import (
+	"golang.org/x/crypto/bcrypt"
+)
+
+func init() {
+	registerHasher("bcrypt", NewBcryptHasher)
+}
+
+// BcryptHasher implements PasswordHasher
+// and uses the bcrypt password hash function.
+type BcryptHasher struct {
+	cost int
+}
+
+// HashWithSaltBytes a provided password and salt
+func (hasher *BcryptHasher) HashWithSaltBytes(password string, salt []byte) string {
+	if hasher == nil {
+		return ""
+	}
+	hashedPassword, _ := bcrypt.GenerateFromPassword([]byte(password), hasher.cost)
+	return string(hashedPassword)
+}
+
+func (hasher *BcryptHasher) VerifyPassword(password, hashedPassword, salt string) bool {
+	return bcrypt.CompareHashAndPassword([]byte(hashedPassword), []byte(password)) == nil
+}
+
+// NewBcryptHasher is a factory method to create an BcryptHasher
+// The provided config should be either empty or the string representation of the "<cost>"
+// as an integer
+func NewBcryptHasher(config string) *BcryptHasher {
+	hasher := &BcryptHasher{
+		cost: 10, // cost=10. i.e. 2^10 rounds of key expansion.
+	}
+
+	if config == "" {
+		return hasher
+	}
+	var err error
+	hasher.cost, err = parseIntParam(config, "cost", "bcrypt", config, nil)
+	if err != nil {
+		return nil
+	}
+
+	return hasher
+}

modules/auth/password/hash/common.go

@@ -0,0 +1,28 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package hash
+
+import (
+	"strconv"
+
+	"code.gitea.io/gitea/modules/log"
+)
+
+func parseIntParam(value, param, algorithmName, config string, previousErr error) (int, error) {
+	parsed, err := strconv.Atoi(value)
+	if err != nil {
+		log.Error("invalid integer for %s representation in %s hash spec %s", param, algorithmName, config)
+		return 0, err
+	}
+	return parsed, previousErr // <- Keep the previous error as this function should still return an error once everything has been checked if any call failed
+}
+
+func parseUIntParam(value, param, algorithmName, config string, previousErr error) (uint64, error) {
+	parsed, err := strconv.ParseUint(value, 10, 64)
+	if err != nil {
+		log.Error("invalid integer for %s representation in %s hash spec %s", param, algorithmName, config)
+		return 0, err
+	}
+	return parsed, previousErr // <- Keep the previous error as this function should still return an error once everything has been checked if any call failed
+}