Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Add max concurrent hashes setting #14713

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func runChangePassword(c *cli.Context) error {
if err != nil {
return err
}
if err = user.SetPassword(c.String("password")); err != nil {
if err = user.SetPassword(context.Background(), c.String("password")); err != nil {
return err
}

Expand Down Expand Up @@ -427,7 +427,7 @@ func runCreateUser(c *cli.Context) error {
Theme: setting.UI.DefaultTheme,
}

if err := models.CreateUser(u); err != nil {
if err := models.CreateUser(context.Background(), u); err != nil {
return fmt.Errorf("CreateUser: %v", err)
}

Expand Down
2 changes: 2 additions & 0 deletions custom/conf/app.example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,8 @@ INTERNAL_TOKEN=
;;
;; Validate against https://haveibeenpwned.com/Passwords to see if a password has been exposed
;PASSWORD_CHECK_PWN = false
;; Only allow a certain number of password hashes to be performed at a time. Set to 0 to have no limit.
;MAXIMUM_CONCURRENT_HASHES = 0

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
1 change: 1 addition & 0 deletions docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ relation to port exhaustion.
- spec - use one or more special characters as ``!"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~``
- off - do not check password complexity
- `PASSWORD_CHECK_PWN`: **false**: Check [HaveIBeenPwned](https://haveibeenpwned.com/Passwords) to see if a password has been exposed.
- `MAXIMUM_CONCURRENT_HASHES`: **0**: Only allow a certain number of password hashes to be performed at a time. Set to 0 to have no limit.

## OpenID (`openid`)

Expand Down
35 changes: 18 additions & 17 deletions models/login_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package models

import (
"context"
"crypto/tls"
"errors"
"fmt"
Expand Down Expand Up @@ -487,7 +488,7 @@ func composeFullName(firstname, surname, username string) string {

// LoginViaLDAP queries if login/password is valid against the LDAP directory pool,
// and create a local user if success when enabled.
func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*User, error) {
func LoginViaLDAP(ctx context.Context, user *User, login, password string, source *LoginSource) (*User, error) {
sr := source.Cfg.(*LDAPConfig).SearchEntry(login, password, source.Type == LoginDLDAP)
if sr == nil {
// User not in LDAP, do nothing
Expand Down Expand Up @@ -557,7 +558,7 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*Use
IsRestricted: sr.IsRestricted,
}

err := CreateUser(user)
err := CreateUser(ctx, user)

if err == nil && isAttributeSSHPublicKeySet && addLdapSSHPublicKeys(user, source, sr.SSHPublicKey) {
err = RewriteAllPublicKeys()
Expand Down Expand Up @@ -635,7 +636,7 @@ func SMTPAuth(a smtp.Auth, cfg *SMTPConfig) error {

// LoginViaSMTP queries if login/password is valid against the SMTP,
// and create a local user if success when enabled.
func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPConfig) (*User, error) {
func LoginViaSMTP(ctx context.Context, user *User, login, password string, sourceID int64, cfg *SMTPConfig) (*User, error) {
// Verify allowed domains.
if len(cfg.AllowedDomains) > 0 {
idx := strings.Index(login, "@")
Expand Down Expand Up @@ -686,7 +687,7 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC
LoginName: login,
IsActive: true,
}
return user, CreateUser(user)
return user, CreateUser(ctx, user)
}

// __________ _____ _____
Expand All @@ -698,7 +699,7 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC

// LoginViaPAM queries if login/password is valid against the PAM,
// and create a local user if success when enabled.
func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMConfig) (*User, error) {
func LoginViaPAM(ctx context.Context, user *User, login, password string, sourceID int64, cfg *PAMConfig) (*User, error) {
pamLogin, err := pam.Auth(cfg.ServiceName, login, password)
if err != nil {
if strings.Contains(err.Error(), "Authentication failure") {
Expand Down Expand Up @@ -739,23 +740,23 @@ func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMCon
LoginName: login, // This is what the user typed in
IsActive: true,
}
return user, CreateUser(user)
return user, CreateUser(ctx, user)
}

// ExternalUserLogin attempts a login using external source types.
func ExternalUserLogin(user *User, login, password string, source *LoginSource) (*User, error) {
func ExternalUserLogin(ctx context.Context, user *User, login, password string, source *LoginSource) (*User, error) {
if !source.IsActived {
return nil, ErrLoginSourceNotActived
}

var err error
switch source.Type {
case LoginLDAP, LoginDLDAP:
user, err = LoginViaLDAP(user, login, password, source)
user, err = LoginViaLDAP(ctx, user, login, password, source)
case LoginSMTP:
user, err = LoginViaSMTP(user, login, password, source.ID, source.Cfg.(*SMTPConfig))
user, err = LoginViaSMTP(ctx, user, login, password, source.ID, source.Cfg.(*SMTPConfig))
case LoginPAM:
user, err = LoginViaPAM(user, login, password, source.ID, source.Cfg.(*PAMConfig))
user, err = LoginViaPAM(ctx, user, login, password, source.ID, source.Cfg.(*PAMConfig))
default:
return nil, ErrUnsupportedLoginType
}
Expand All @@ -774,7 +775,7 @@ func ExternalUserLogin(user *User, login, password string, source *LoginSource)
}

// UserSignIn validates user name and password.
func UserSignIn(username, password string) (*User, error) {
func UserSignIn(ctx context.Context, username, password string) (*User, error) {
var user *User
if strings.Contains(username, "@") {
user = &User{Email: strings.ToLower(strings.TrimSpace(username))}
Expand Down Expand Up @@ -805,11 +806,11 @@ func UserSignIn(username, password string) (*User, error) {
if hasUser {
switch user.LoginType {
case LoginNoType, LoginPlain, LoginOAuth2:
if user.IsPasswordSet() && user.ValidatePassword(password) {
if user.IsPasswordSet() && user.ValidatePassword(ctx, password) {

// Update password hash if server password hash algorithm have changed
if user.PasswdHashAlgo != setting.PasswordHashAlgo {
if err = user.SetPassword(password); err != nil {
if err = user.SetPassword(ctx, password); err != nil {
return nil, err
}
if err = UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
Expand All @@ -830,19 +831,19 @@ func UserSignIn(username, password string) (*User, error) {

default:
var source LoginSource
hasSource, err := x.ID(user.LoginSource).Get(&source)
hasSource, err := x.Context(ctx).ID(user.LoginSource).Get(&source)
if err != nil {
return nil, err
} else if !hasSource {
return nil, ErrLoginSourceNotExist{user.LoginSource}
}

return ExternalUserLogin(user, user.LoginName, password, &source)
return ExternalUserLogin(ctx, user, user.LoginName, password, &source)
}
}

sources := make([]*LoginSource, 0, 5)
if err = x.Where("is_actived = ?", true).Find(&sources); err != nil {
if err = x.Context(ctx).Where("is_actived = ?", true).Find(&sources); err != nil {
return nil, err
}

Expand All @@ -851,7 +852,7 @@ func UserSignIn(username, password string) (*User, error) {
// don't try to authenticate against OAuth2 and SSPI sources here
continue
}
authUser, err := ExternalUserLogin(nil, username, password, source)
authUser, err := ExternalUserLogin(ctx, nil, username, password, source)
if err == nil {
return authUser, nil
}
Expand Down
91 changes: 74 additions & 17 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"path/filepath"
"regexp"
"strings"
"sync"
"time"
"unicode/utf8"

Expand Down Expand Up @@ -376,13 +377,43 @@ func (u *User) NewGitSig() *git.Signature {
}
}

func hashPassword(passwd, salt, algo string) string {
var lock = sync.Mutex{}
var cond = sync.NewCond(&lock)
var concurrentHashes = 0

func hashPassword(ctx context.Context, passwd, salt, algo string) (string, error) {
if setting.MaximumConcurrentHashes > 0 {
cond.L.Lock()
for concurrentHashes > setting.MaximumConcurrentHashes {
select {
case <-ctx.Done():
cond.L.Unlock()
return "", ctx.Err()
default:
}
cond.Wait()
}
concurrentHashes++
cond.L.Unlock()
defer func() {
cond.L.Lock()
concurrentHashes--
cond.Signal()
cond.L.Unlock()
}()
select {
case <-ctx.Done():
return "", ctx.Err()
default:
}
}

var tempPasswd []byte

switch algo {
case algoBcrypt:
tempPasswd, _ = bcrypt.GenerateFromPassword([]byte(passwd), bcrypt.DefaultCost)
return string(tempPasswd)
return string(tempPasswd), nil
case algoScrypt:
tempPasswd, _ = scrypt.Key([]byte(passwd), []byte(salt), 65536, 16, 2, 50)
case algoArgon2:
Expand All @@ -393,12 +424,12 @@ func hashPassword(passwd, salt, algo string) string {
tempPasswd = pbkdf2.Key([]byte(passwd), []byte(salt), 10000, 50, sha256.New)
}

return fmt.Sprintf("%x", tempPasswd)
return fmt.Sprintf("%x", 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) {
func (u *User) SetPassword(ctx context.Context, passwd string) (err error) {
if len(passwd) == 0 {
u.Passwd = ""
u.Salt = ""
Expand All @@ -409,23 +440,49 @@ func (u *User) SetPassword(passwd string) (err error) {
if u.Salt, err = GetUserSalt(); err != nil {
return err
}
hashedPassword, err := hashPassword(ctx, passwd, u.Salt, setting.PasswordHashAlgo)
if err != nil {
return err
}

u.Passwd = hashedPassword
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.
func (u *User) ValidatePassword(passwd string) bool {
tempHash := hashPassword(passwd, u.Salt, u.PasswdHashAlgo)

if u.PasswdHashAlgo != algoBcrypt && subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(tempHash)) == 1 {
return true
func (u *User) ValidatePassword(ctx context.Context, passwd string) bool {
if u.PasswdHashAlgo != algoBcrypt {
tempHash, err := hashPassword(ctx, passwd, u.Salt, u.PasswdHashAlgo)
if err != nil {
return false
}
return subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(tempHash)) == 1
}
if u.PasswdHashAlgo == algoBcrypt && bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil {
return true

// Handle maximum concurrent hashes
if setting.MaximumConcurrentHashes > 0 {
cond.L.Lock()
for concurrentHashes > setting.MaximumConcurrentHashes {
cond.Wait()
select {
case <-ctx.Done():
return false
default:
}
}
concurrentHashes++
cond.L.Unlock()
defer func() {
cond.L.Lock()
concurrentHashes--
cond.Signal()
cond.L.Unlock()
}()
}
return false

return bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil
}

// IsPasswordSet checks if the password is set or left empty
Expand Down Expand Up @@ -850,12 +907,12 @@ func IsUsableUsername(name string) error {
}

// CreateUser creates record of a new user.
func CreateUser(u *User) (err error) {
func CreateUser(ctx context.Context, u *User) (err error) {
if err = IsUsableUsername(u.Name); err != nil {
return err
}

sess := x.NewSession()
sess := x.NewSession().Context(ctx)
defer sess.Close()
if err = sess.Begin(); err != nil {
return err
Expand Down Expand Up @@ -891,7 +948,7 @@ func CreateUser(u *User) (err error) {
if u.Rands, err = GetUserSalt(); err != nil {
return err
}
if err = u.SetPassword(u.Passwd); err != nil {
if err = u.SetPassword(ctx, u.Passwd); err != nil {
return err
}
u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization && !setting.Admin.DisableRegularOrgCreation
Expand Down Expand Up @@ -1925,7 +1982,7 @@ func SyncExternalUsers(ctx context.Context, updateExisting bool) error {
IsActive: true,
}

err = CreateUser(usr)
err = CreateUser(ctx, usr)

if err != nil {
log.Error("SyncExternalUsers[%s]: Error creating user %s: %v", s.Name, su.Username, err)
Expand Down
15 changes: 8 additions & 7 deletions models/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package models

import (
"context"
"fmt"
"math/rand"
"strings"
Expand Down Expand Up @@ -230,15 +231,15 @@ func TestHashPasswordDeterministic(t *testing.T) {
pass := string(b)

// save the current password in the user - hash it and store the result
u.SetPassword(pass)
u.SetPassword(context.Background(), pass)
r1 := u.Passwd

// run again
u.SetPassword(pass)
u.SetPassword(context.Background(), pass)
r2 := u.Passwd

assert.NotEqual(t, r1, r2)
assert.True(t, u.ValidatePassword(pass))
assert.True(t, u.ValidatePassword(context.Background(), pass))
}
}
}
Expand All @@ -250,7 +251,7 @@ func BenchmarkHashPassword(b *testing.B) {
u := &User{Passwd: pass}
b.ResetTimer()
for i := 0; i < b.N; i++ {
u.SetPassword(pass)
u.SetPassword(context.Background(), pass)
}
}

Expand Down Expand Up @@ -317,7 +318,7 @@ func TestCreateUser(t *testing.T) {
MustChangePassword: false,
}

assert.NoError(t, CreateUser(user))
assert.NoError(t, CreateUser(context.Background(), user))

assert.NoError(t, DeleteUser(user))
}
Expand All @@ -332,7 +333,7 @@ func TestCreateUserInvalidEmail(t *testing.T) {
MustChangePassword: false,
}

err := CreateUser(user)
err := CreateUser(context.Background(), user)
assert.Error(t, err)
assert.True(t, IsErrEmailInvalid(err))
}
Expand All @@ -356,7 +357,7 @@ func TestCreateUser_Issue5882(t *testing.T) {
for _, v := range tt {
setting.Admin.DisableRegularOrgCreation = v.disableOrgCreation

assert.NoError(t, CreateUser(v.user))
assert.NoError(t, CreateUser(context.Background(), v.user))

u, err := GetUserByEmail(v.user.Email)
assert.NoError(t, err)
Expand Down
Loading