From db657192d0349f7b10a62515fbf085d3a48d88f9 Mon Sep 17 00:00:00 2001 From: Maxim Tkachenko Date: Mon, 14 Oct 2019 22:24:26 +0700 Subject: [PATCH] Password Complexity Checks (#6230) Add password complexity checks. The default settings require a lowercase, uppercase, number and a special character within passwords. Co-Authored-By: T-M-A Co-Authored-By: Lanre Adelowo Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com> Co-Authored-By: Lauris BH --- cmd/admin.go | 19 ++--- custom/conf/app.ini.sample | 5 +- .../doc/advanced/config-cheat-sheet.en-us.md | 5 ++ modules/password/password.go | 73 +++++++++++++++++ modules/setting/setting.go | 22 +++++ options/locale/locale_en-US.ini | 1 + routers/admin/users.go | 10 ++- routers/api/v1/admin/user.go | 14 +++- routers/user/auth.go | 11 +-- routers/user/setting/account.go | 3 + routers/user/setting/account_test.go | 81 ++++++++++++++----- 11 files changed, 207 insertions(+), 37 deletions(-) create mode 100644 modules/password/password.go diff --git a/cmd/admin.go b/cmd/admin.go index 4c4d6f9b6..4346159fe 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -13,9 +13,9 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth/oauth2" - "code.gitea.io/gitea/modules/generate" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + pwd "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "github.com/urfave/cli" @@ -233,7 +233,9 @@ func runChangePassword(c *cli.Context) error { if err := initDB(); err != nil { return err } - + if !pwd.IsComplexEnough(c.String("password")) { + return errors.New("Password does not meet complexity requirements") + } uname := c.String("username") user, err := models.GetUserByName(uname) if err != nil { @@ -243,6 +245,7 @@ func runChangePassword(c *cli.Context) error { return err } user.HashPassword(c.String("password")) + if err := models.UpdateUserCols(user, "passwd", "salt"); err != nil { return err } @@ -275,26 +278,24 @@ func runCreateUser(c *cli.Context) error { fmt.Fprintf(os.Stderr, "--name flag is deprecated. Use --username instead.\n") } - var password string + if err := initDB(); err != nil { + return err + } + var password string if c.IsSet("password") { password = c.String("password") } else if c.IsSet("random-password") { var err error - password, err = generate.GetRandomString(c.Int("random-password-length")) + password, err = pwd.Generate(c.Int("random-password-length")) if err != nil { return err } - fmt.Printf("generated random password is '%s'\n", password) } else { return errors.New("must set either password or random-password flag") } - if err := initDB(); err != nil { - return err - } - // always default to true var changePassword = true diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index fd8d928ed..79d996005 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -332,6 +332,9 @@ MIN_PASSWORD_LENGTH = 6 IMPORT_LOCAL_PATHS = false ; Set to true to prevent all users (including admin) from creating custom git hooks DISABLE_GIT_HOOKS = false +;Comma separated list of character classes required to pass minimum complexity. +;If left empty or no valid values are specified, the default values (`lower,upper,digit,spec`) will be used. +PASSWORD_COMPLEXITY = lower,upper,digit,spec ; Password Hash algorithm, either "pbkdf2", "argon2", "scrypt" or "bcrypt" PASSWORD_HASH_ALGO = pbkdf2 ; Set false to allow JavaScript to read CSRF cookie @@ -415,7 +418,7 @@ DEFAULT_ALLOW_CREATE_ORGANIZATION = true ; Public is for everyone DEFAULT_ORG_VISIBILITY = public ; Default value for DefaultOrgMemberVisible -; True will make the membership of the users visible when added to the organisation +; True will make the membership of the users visible when added to the organisation DEFAULT_ORG_MEMBER_VISIBLE = false ; Default value for EnableDependencies ; Repositories will use dependencies by default depending on this setting diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index b927793a5..100bb229e 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -208,6 +208,11 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `INTERNAL_TOKEN_URI`: ****: 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 \[pbkdf2, argon2, scrypt, bcrypt\]. - `CSRF_COOKIE_HTTP_ONLY`: **true**: Set false to allow JavaScript to read CSRF cookie. +- `PASSWORD_COMPLEXITY`: **lower,upper,digit,spec**: Comma separated list of character classes required to pass minimum complexity. If left empty or no valid values are specified, the default values will be used. Possible values are: + - lower - use one or more lower latin characters + - upper - use one or more upper latin characters + - digit - use one or more digits + - spec - use one or more special characters as ``][!"#$%&'()*+,./:;<=>?@\^_{|}~`-`` and space symbol. ## OpenID (`openid`) diff --git a/modules/password/password.go b/modules/password/password.go new file mode 100644 index 000000000..54131b964 --- /dev/null +++ b/modules/password/password.go @@ -0,0 +1,73 @@ +// Copyright 2019 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 password + +import ( + "crypto/rand" + "math/big" + "regexp" + "sync" + + "code.gitea.io/gitea/modules/setting" +) + +var matchComplexities = map[string]regexp.Regexp{} +var matchComplexityOnce sync.Once +var validChars string +var validComplexities = map[string]string{ + "lower": "abcdefghijklmnopqrstuvwxyz", + "upper": "ABCDEFGHIJKLMNOPQRSTUVWXYZ", + "digit": "0123456789", + "spec": `][ !"#$%&'()*+,./:;<=>?@\^_{|}~` + "`-", +} + +// NewComplexity for preparation +func NewComplexity() { + matchComplexityOnce.Do(func() { + if len(setting.PasswordComplexity) > 0 { + for key, val := range setting.PasswordComplexity { + matchComplexity := regexp.MustCompile(val) + matchComplexities[key] = *matchComplexity + validChars += validComplexities[key] + } + } else { + for _, val := range validComplexities { + validChars += val + } + } + }) +} + +// IsComplexEnough return True if password is Complexity +func IsComplexEnough(pwd string) bool { + if len(setting.PasswordComplexity) > 0 { + NewComplexity() + for _, val := range matchComplexities { + if !val.MatchString(pwd) { + return false + } + } + } + return true +} + +// Generate a random password +func Generate(n int) (string, error) { + NewComplexity() + buffer := make([]byte, n) + max := big.NewInt(int64(len(validChars))) + for { + for j := 0; j < n; j++ { + rnd, err := rand.Int(rand.Reader, max) + if err != nil { + return "", err + } + buffer[j] = validChars[rnd.Int64()] + } + if IsComplexEnough(string(buffer)) && string(buffer[0]) != " " && string(buffer[n-1]) != " " { + return string(buffer), nil + } + } +} diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 8c61bdbb7..278ed4b10 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -146,6 +146,7 @@ var ( MinPasswordLength int ImportLocalPaths bool DisableGitHooks bool + PasswordComplexity map[string]string PasswordHashAlgo string // UI settings @@ -774,6 +775,27 @@ func NewContext() { InternalToken = loadInternalToken(sec) + var dictPC = map[string]string{ + "lower": "[a-z]+", + "upper": "[A-Z]+", + "digit": "[0-9]+", + "spec": `][ !"#$%&'()*+,./:;<=>?@\\^_{|}~` + "`-", + } + PasswordComplexity = make(map[string]string) + cfgdata := sec.Key("PASSWORD_COMPLEXITY").Strings(",") + for _, y := range cfgdata { + ts := strings.TrimSpace(y) + for a := range dictPC { + if strings.ToLower(ts) == a { + PasswordComplexity[ts] = dictPC[ts] + break + } + } + } + if len(PasswordComplexity) == 0 { + PasswordComplexity = dictPC + } + sec = Cfg.Section("attachment") AttachmentPath = sec.Key("PATH").MustString(path.Join(AppDataPath, "attachments")) if !filepath.IsAbs(AttachmentPath) { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index e6c5839a6..4a92b0803 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -315,6 +315,7 @@ team_no_units_error = Allow access to at least one repository section. email_been_used = The email address is already used. openid_been_used = The OpenID address '%s' is already used. username_password_incorrect = Username or password is incorrect. +password_complexity = Password does not pass complexity requirements. enterred_invalid_repo_name = The repository name you entered is incorrect. enterred_invalid_owner_name = The new owner name is not valid. enterred_invalid_password = The password you entered is incorrect. diff --git a/routers/admin/users.go b/routers/admin/users.go index 660f11668..fdc4e0e37 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers" "code.gitea.io/gitea/services/mailer" @@ -94,7 +95,10 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) { u.LoginName = form.LoginName } } - + if !password.IsComplexEnough(form.Password) { + ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplUserNew, &form) + return + } if err := models.CreateUser(u); err != nil { switch { case models.IsErrUserAlreadyExist(err): @@ -201,6 +205,10 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) { ctx.ServerError("UpdateUser", err) return } + if !password.IsComplexEnough(form.Password) { + ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplUserEdit, &form) + return + } u.HashPassword(form.Password) } diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 70076b626..f35ad297b 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -6,9 +6,12 @@ package admin import ( + "errors" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/password" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/routers/api/v1/convert" "code.gitea.io/gitea/routers/api/v1/user" @@ -73,7 +76,11 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) { if ctx.Written() { return } - + if !password.IsComplexEnough(form.Password) { + err := errors.New("PasswordComplexity") + ctx.Error(400, "PasswordComplexity", err) + return + } if err := models.CreateUser(u); err != nil { if models.IsErrUserAlreadyExist(err) || models.IsErrEmailAlreadyUsed(err) || @@ -131,6 +138,11 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) { } if len(form.Password) > 0 { + if !password.IsComplexEnough(form.Password) { + err := errors.New("PasswordComplexity") + ctx.Error(400, "PasswordComplexity", err) + return + } var err error if u.Salt, err = models.GetUserSalt(); err != nil { ctx.Error(500, "UpdateUser", err) diff --git a/routers/user/auth.go b/routers/user/auth.go index 212d535a0..82a508e4d 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/recaptcha" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" @@ -1334,6 +1335,11 @@ func ResetPasswdPost(ctx *context.Context) { ctx.Data["Err_Password"] = true ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplResetPassword, nil) return + } else if !password.IsComplexEnough(passwd) { + ctx.Data["IsResetForm"] = true + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplResetPassword, nil) + return } var err error @@ -1364,7 +1370,6 @@ func ResetPasswdPost(ctx *context.Context) { func MustChangePassword(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("auth.must_change_password") ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password" - ctx.HTML(200, tplMustChangePassword) } @@ -1372,16 +1377,12 @@ func MustChangePassword(ctx *context.Context) { // account was created by an admin func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form auth.MustChangePasswordForm) { ctx.Data["Title"] = ctx.Tr("auth.must_change_password") - ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password" - if ctx.HasError() { ctx.HTML(200, tplMustChangePassword) return } - u := ctx.User - // Make sure only requests for users who are eligible to change their password via // this method passes through if !u.MustChangePassword { diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go index 71d98fd3b..c78222421 100644 --- a/routers/user/setting/account.go +++ b/routers/user/setting/account.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/services/mailer" @@ -52,6 +53,8 @@ func AccountPost(ctx *context.Context, form auth.ChangePasswordForm) { ctx.Flash.Error(ctx.Tr("settings.password_incorrect")) } else if form.Password != form.Retype { ctx.Flash.Error(ctx.Tr("form.password_not_match")) + } else if !password.IsComplexEnough(form.Password) { + ctx.Flash.Error(ctx.Tr("settings.password_complexity")) } else { var err error if ctx.User.Salt, err = models.GetUserSalt(); err != nil { diff --git a/routers/user/setting/account_test.go b/routers/user/setting/account_test.go index 59fbda156..497ee658b 100644 --- a/routers/user/setting/account_test.go +++ b/routers/user/setting/account_test.go @@ -19,36 +19,77 @@ import ( func TestChangePassword(t *testing.T) { oldPassword := "password" setting.MinPasswordLength = 6 + setting.PasswordComplexity = map[string]string{ + "lower": "[a-z]+", + "upper": "[A-Z]+", + "digit": "[0-9]+", + "spec": "[-_]+", + } + var pcLUN = map[string]string{ + "lower": "[a-z]+", + "upper": "[A-Z]+", + "digit": "[0-9]+", + } + var pcLU = map[string]string{ + "lower": "[a-z]+", + "upper": "[A-Z]+", + } for _, req := range []struct { - OldPassword string - NewPassword string - Retype string - Message string + OldPassword string + NewPassword string + Retype string + Message string + PasswordComplexity map[string]string }{ { - OldPassword: oldPassword, - NewPassword: "123456", - Retype: "123456", - Message: "", + OldPassword: oldPassword, + NewPassword: "Qwerty123456-", + Retype: "Qwerty123456-", + Message: "", + PasswordComplexity: setting.PasswordComplexity, }, { - OldPassword: oldPassword, - NewPassword: "12345", - Retype: "12345", - Message: "auth.password_too_short", + OldPassword: oldPassword, + NewPassword: "12345", + Retype: "12345", + Message: "auth.password_too_short", + PasswordComplexity: setting.PasswordComplexity, }, { - OldPassword: "12334", - NewPassword: "123456", - Retype: "123456", - Message: "settings.password_incorrect", + OldPassword: "12334", + NewPassword: "123456", + Retype: "123456", + Message: "settings.password_incorrect", + PasswordComplexity: setting.PasswordComplexity, }, { - OldPassword: oldPassword, - NewPassword: "123456", - Retype: "12345", - Message: "form.password_not_match", + OldPassword: oldPassword, + NewPassword: "123456", + Retype: "12345", + Message: "form.password_not_match", + PasswordComplexity: setting.PasswordComplexity, + }, + { + OldPassword: oldPassword, + NewPassword: "Qwerty", + Retype: "Qwerty", + Message: "settings.password_complexity", + PasswordComplexity: setting.PasswordComplexity, + }, + { + OldPassword: oldPassword, + NewPassword: "Qwerty", + Retype: "Qwerty", + Message: "settings.password_complexity", + PasswordComplexity: pcLUN, + }, + { + OldPassword: oldPassword, + NewPassword: "QWERTY", + Retype: "QWERTY", + Message: "settings.password_complexity", + PasswordComplexity: pcLU, }, } { models.PrepareTestEnv(t)