From c2a01b875995eaeff86712f074fc60567785f76b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 17 Feb 2021 19:48:04 +0000 Subject: [PATCH 1/7] Make modules/context.Context a go context.Context Simplest solution is to map all the functions on to the Request directly Signed-off-by: Andrew Thornton --- modules/context/context.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/modules/context/context.go b/modules/context/context.go index bc48c1415de2d..1898b45569890 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -69,6 +69,26 @@ type Context struct { Org *Organization } +// Deadline is part of go's context.Context interface +func (ctx *Context) Deadline() (deadline time.Time, ok bool) { + return ctx.Req.Context().Deadline() +} + +// Done is part of go's context.Context interface +func (ctx *Context) Done() <-chan struct{} { + return ctx.Req.Context().Done() +} + +// Err is part of go's context.Context interface +func (ctx *Context) Err() error { + return ctx.Req.Context().Err() +} + +// Value is part of go's context.Context interface +func (ctx *Context) Value(key interface{}) interface{} { + return ctx.Req.Context().Value(key) +} + // GetData returns the data func (ctx *Context) GetData() map[string]interface{} { return ctx.Data From 5ddff31e5a61ac10a517ecb40274f00189d58037 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 17 Feb 2021 20:02:09 +0000 Subject: [PATCH 2/7] change references from ctx.Req.Context() to simply ctx Signed-off-by: Andrew Thornton --- routers/admin/users.go | 4 ++-- routers/api/v1/admin/user.go | 4 ++-- routers/events/events.go | 2 +- routers/install.go | 4 ++-- routers/private/manager.go | 2 +- routers/repo/blame.go | 2 +- routers/user/auth.go | 12 ++++++------ routers/user/auth_openid.go | 4 ++-- services/archiver/archiver.go | 4 ++-- 9 files changed, 19 insertions(+), 19 deletions(-) diff --git a/routers/admin/users.go b/routers/admin/users.go index 2d40a883af3db..6f86ae6045666 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -112,7 +112,7 @@ func NewUserPost(ctx *context.Context) { ctx.RenderWithErr(password.BuildComplexityError(ctx), tplUserNew, &form) return } - pwned, err := password.IsPwned(ctx.Req.Context(), form.Password) + pwned, err := password.IsPwned(ctx, form.Password) if pwned { ctx.Data["Err_Password"] = true errMsg := ctx.Tr("auth.password_pwned") @@ -255,7 +255,7 @@ func EditUserPost(ctx *context.Context) { ctx.RenderWithErr(password.BuildComplexityError(ctx), tplUserEdit, &form) return } - pwned, err := password.IsPwned(ctx.Req.Context(), form.Password) + pwned, err := password.IsPwned(ctx, form.Password) if pwned { ctx.Data["Err_Password"] = true errMsg := ctx.Tr("auth.password_pwned") diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index f148710c79715..ad69b1e406422 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -88,7 +88,7 @@ func CreateUser(ctx *context.APIContext) { ctx.Error(http.StatusBadRequest, "PasswordComplexity", err) return } - pwned, err := password.IsPwned(ctx.Req.Context(), form.Password) + pwned, err := password.IsPwned(ctx, form.Password) if pwned { if err != nil { log.Error(err.Error()) @@ -162,7 +162,7 @@ func EditUser(ctx *context.APIContext) { ctx.Error(http.StatusBadRequest, "PasswordComplexity", err) return } - pwned, err := password.IsPwned(ctx.Req.Context(), form.Password) + pwned, err := password.IsPwned(ctx, form.Password) if pwned { if err != nil { log.Error(err.Error()) diff --git a/routers/events/events.go b/routers/events/events.go index a1131f29e3e14..ee072f150fae6 100644 --- a/routers/events/events.go +++ b/routers/events/events.go @@ -27,7 +27,7 @@ func Events(ctx *context.Context) { ctx.Resp.WriteHeader(http.StatusOK) // Listen to connection close and un-register messageChan - notify := ctx.Req.Context().Done() + notify := ctx.Done() ctx.Resp.Flush() shutdownCtx := graceful.GetManager().ShutdownContext() diff --git a/routers/install.go b/routers/install.go index 69ae428a54894..0ec9d4b3e0ea4 100644 --- a/routers/install.go +++ b/routers/install.go @@ -393,7 +393,7 @@ func InstallPost(ctx *context.Context) { } // Re-read settings - PostInstallInit(ctx.Req.Context()) + PostInstallInit(ctx) // Create admin account if len(form.AdminName) > 0 { @@ -446,7 +446,7 @@ func InstallPost(ctx *context.Context) { // Now get the http.Server from this request and shut it down // NB: This is not our hammerable graceful shutdown this is http.Server.Shutdown - srv := ctx.Req.Context().Value(http.ServerContextKey).(*http.Server) + srv := ctx.Value(http.ServerContextKey).(*http.Server) go func() { if err := srv.Shutdown(graceful.GetManager().HammerContext()); err != nil { log.Error("Unable to shutdown the install server! Error: %v", err) diff --git a/routers/private/manager.go b/routers/private/manager.go index e5b4583fd1b2e..c7b6f348cb6c2 100644 --- a/routers/private/manager.go +++ b/routers/private/manager.go @@ -35,7 +35,7 @@ func FlushQueues(ctx *context.PrivateContext) { }) return } - err := queue.GetManager().FlushAll(ctx.Req.Context(), opts.Timeout) + err := queue.GetManager().FlushAll(ctx, opts.Timeout) if err != nil { ctx.JSON(http.StatusRequestTimeout, map[string]interface{}{ "err": fmt.Sprintf("%v", err), diff --git a/routers/repo/blame.go b/routers/repo/blame.go index 9be1ea05af9b4..1f5d91ca4bada 100644 --- a/routers/repo/blame.go +++ b/routers/repo/blame.go @@ -123,7 +123,7 @@ func RefBlame(ctx *context.Context) { return } - blameReader, err := git.CreateBlameReader(ctx.Req.Context(), models.RepoPath(userName, repoName), commitID, fileName) + blameReader, err := git.CreateBlameReader(ctx, models.RepoPath(userName, repoName), commitID, fileName) if err != nil { ctx.NotFound("CreateBlameReader", err) return diff --git a/routers/user/auth.go b/routers/user/auth.go index de74055d565f5..cb308eda95c92 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -920,9 +920,9 @@ func LinkAccountPostRegister(ctx *context.Context) { case setting.ImageCaptcha: valid = context.GetImageCaptcha().VerifyReq(ctx.Req) case setting.ReCaptcha: - valid, err = recaptcha.Verify(ctx.Req.Context(), form.GRecaptchaResponse) + valid, err = recaptcha.Verify(ctx, form.GRecaptchaResponse) case setting.HCaptcha: - valid, err = hcaptcha.Verify(ctx.Req.Context(), form.HcaptchaResponse) + valid, err = hcaptcha.Verify(ctx, form.HcaptchaResponse) default: ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType)) return @@ -1111,9 +1111,9 @@ func SignUpPost(ctx *context.Context) { case setting.ImageCaptcha: valid = context.GetImageCaptcha().VerifyReq(ctx.Req) case setting.ReCaptcha: - valid, err = recaptcha.Verify(ctx.Req.Context(), form.GRecaptchaResponse) + valid, err = recaptcha.Verify(ctx, form.GRecaptchaResponse) case setting.HCaptcha: - valid, err = hcaptcha.Verify(ctx.Req.Context(), form.HcaptchaResponse) + valid, err = hcaptcha.Verify(ctx, form.HcaptchaResponse) default: ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType)) return @@ -1149,7 +1149,7 @@ func SignUpPost(ctx *context.Context) { ctx.RenderWithErr(password.BuildComplexityError(ctx), tplSignUp, &form) return } - pwned, err := password.IsPwned(ctx.Req.Context(), form.Password) + pwned, err := password.IsPwned(ctx, form.Password) if pwned { errMsg := ctx.Tr("auth.password_pwned") if err != nil { @@ -1480,7 +1480,7 @@ func ResetPasswdPost(ctx *context.Context) { ctx.Data["Err_Password"] = true ctx.RenderWithErr(password.BuildComplexityError(ctx), tplResetPassword, nil) return - } else if pwned, err := password.IsPwned(ctx.Req.Context(), passwd); pwned || err != nil { + } else if pwned, err := password.IsPwned(ctx, passwd); pwned || err != nil { errMsg := ctx.Tr("auth.password_pwned") if err != nil { log.Error(err.Error()) diff --git a/routers/user/auth_openid.go b/routers/user/auth_openid.go index e79085e94d400..82e7b5c75340e 100644 --- a/routers/user/auth_openid.go +++ b/routers/user/auth_openid.go @@ -378,13 +378,13 @@ func RegisterOpenIDPost(ctx *context.Context) { ctx.ServerError("", err) return } - valid, err = recaptcha.Verify(ctx.Req.Context(), form.GRecaptchaResponse) + valid, err = recaptcha.Verify(ctx, form.GRecaptchaResponse) case setting.HCaptcha: if err := ctx.Req.ParseForm(); err != nil { ctx.ServerError("", err) return } - valid, err = hcaptcha.Verify(ctx.Req.Context(), form.HcaptchaResponse) + valid, err = hcaptcha.Verify(ctx, form.HcaptchaResponse) default: ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType)) return diff --git a/services/archiver/archiver.go b/services/archiver/archiver.go index 359fc8b627d91..dfa6334d9536c 100644 --- a/services/archiver/archiver.go +++ b/services/archiver/archiver.go @@ -76,7 +76,7 @@ func (aReq *ArchiveRequest) IsComplete() bool { func (aReq *ArchiveRequest) WaitForCompletion(ctx *context.Context) bool { select { case <-aReq.cchan: - case <-ctx.Req.Context().Done(): + case <-ctx.Done(): } return aReq.IsComplete() @@ -92,7 +92,7 @@ func (aReq *ArchiveRequest) TimedWaitForCompletion(ctx *context.Context, dur tim case <-time.After(dur): timeout = true case <-aReq.cchan: - case <-ctx.Req.Context().Done(): + case <-ctx.Done(): } return aReq.IsComplete(), timeout From 3c163c396fc859909f44eba65bed7fa5c97c11e4 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 17 Feb 2021 20:21:34 +0000 Subject: [PATCH 3/7] Make CreateUser and password checking context cancellable Signed-off-by: Andrew Thornton --- cmd/admin.go | 4 ++-- models/login_source.go | 35 ++++++++++++++------------- models/user.go | 41 ++++++++++++++++++-------------- models/user_test.go | 15 ++++++------ modules/auth/sso/basic.go | 2 +- modules/auth/sso/reverseproxy.go | 2 +- modules/lfs/locks.go | 2 +- modules/lfs/server.go | 7 +++--- routers/admin/users.go | 4 ++-- routers/api/v1/admin/user.go | 4 ++-- routers/install.go | 2 +- routers/org/setting.go | 2 +- routers/repo/http.go | 2 +- routers/user/auth.go | 14 +++++------ routers/user/auth_openid.go | 4 ++-- routers/user/setting/account.go | 8 +++---- 16 files changed, 78 insertions(+), 70 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index 3ddf8eddf9e6c..c2542c79b08bd 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -349,7 +349,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 } @@ -426,7 +426,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) } diff --git a/models/login_source.go b/models/login_source.go index d351f12861f79..835aedd2d940e 100644 --- a/models/login_source.go +++ b/models/login_source.go @@ -6,6 +6,7 @@ package models import ( + "context" "crypto/tls" "encoding/json" "errors" @@ -460,7 +461,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 @@ -530,7 +531,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() @@ -608,7 +609,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, "@") @@ -659,7 +660,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) } // __________ _____ _____ @@ -671,7 +672,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") { @@ -701,11 +702,11 @@ 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 } @@ -713,11 +714,11 @@ func ExternalUserLogin(user *User, login, password string, source *LoginSource) 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 } @@ -736,7 +737,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))} @@ -767,11 +768,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 { @@ -792,19 +793,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 } @@ -813,7 +814,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 } diff --git a/models/user.go b/models/user.go index 495fed1ff4d03..b0e5b71b2e248 100644 --- a/models/user.go +++ b/models/user.go @@ -374,13 +374,13 @@ func (u *User) NewGitSig() *git.Signature { } } -func hashPassword(passwd, salt, algo string) string { +func hashPassword(ctx context.Context, passwd, salt, algo string) (string, error) { 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: @@ -391,12 +391,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 = "" @@ -407,23 +407,28 @@ 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 - } - if u.PasswdHashAlgo == algoBcrypt && bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil { - 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 } - return false + + return bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil } // IsPasswordSet checks if the password is set or left empty @@ -845,12 +850,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 @@ -895,7 +900,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 @@ -1919,7 +1924,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) diff --git a/models/user_test.go b/models/user_test.go index ac40015969aed..9513b9b0a15e2 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -5,6 +5,7 @@ package models import ( + "context" "fmt" "math/rand" "strings" @@ -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)) } } } @@ -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) } } @@ -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)) } @@ -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)) } @@ -357,7 +358,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) diff --git a/modules/auth/sso/basic.go b/modules/auth/sso/basic.go index d2d25c6cece65..21cda0641be0e 100644 --- a/modules/auth/sso/basic.go +++ b/modules/auth/sso/basic.go @@ -98,7 +98,7 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D } if u == nil { - u, err = models.UserSignIn(uname, passwd) + u, err = models.UserSignIn(req.Context(), uname, passwd) if err != nil { if !models.IsErrUserNotExist(err) { log.Error("UserSignIn: %v", err) diff --git a/modules/auth/sso/reverseproxy.go b/modules/auth/sso/reverseproxy.go index ca9450e71429d..7f1a2f46a2913 100644 --- a/modules/auth/sso/reverseproxy.go +++ b/modules/auth/sso/reverseproxy.go @@ -105,7 +105,7 @@ func (r *ReverseProxy) newUser(req *http.Request) *models.User { Passwd: username, IsActive: true, } - if err := models.CreateUser(user); err != nil { + if err := models.CreateUser(req.Context(), user); err != nil { // FIXME: should I create a system notice? log.Error("CreateUser: %v", err) return nil diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index cf62492c7e1e9..b88b361914432 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -30,7 +30,7 @@ func checkIsValidRequest(ctx *context.Context) bool { return false } if !ctx.IsSigned { - user, _, _, err := parseToken(ctx.Req.Header.Get("Authorization")) + user, _, _, err := parseToken(ctx, ctx.Req.Header.Get("Authorization")) if err != nil { ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") writeStatus(ctx, 401) diff --git a/modules/lfs/server.go b/modules/lfs/server.go index be21a4de82917..f543e5e07a996 100644 --- a/modules/lfs/server.go +++ b/modules/lfs/server.go @@ -5,6 +5,7 @@ package lfs import ( + gocontext "context" "encoding/base64" "encoding/json" "fmt" @@ -609,7 +610,7 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza return true } - user, repo, opStr, err := parseToken(authorization) + user, repo, opStr, err := parseToken(ctx, authorization) if err != nil { // Most of these are Warn level - the true internal server errors are logged in parseToken already log.Warn("Authentication failure for provided token with Error: %v", err) @@ -633,7 +634,7 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza return false } -func parseToken(authorization string) (*models.User, *models.Repository, string, error) { +func parseToken(ctx gocontext.Context, authorization string) (*models.User, *models.Repository, string, error) { if authorization == "" { return nil, nil, "unknown", fmt.Errorf("No token") } @@ -681,7 +682,7 @@ func parseToken(authorization string) (*models.User, *models.Repository, string, log.Error("Unable to GetUserByName[%d]: Error: %v", user, err) return nil, nil, "basic", err } - if !u.IsPasswordSet() || !u.ValidatePassword(password) { + if !u.IsPasswordSet() || !u.ValidatePassword(ctx, password) { return nil, nil, "basic", fmt.Errorf("Basic auth failed") } return u, nil, "basic", nil diff --git a/routers/admin/users.go b/routers/admin/users.go index 6f86ae6045666..b90ea02aa0809 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -125,7 +125,7 @@ func NewUserPost(ctx *context.Context) { } u.MustChangePassword = form.MustChangePassword } - if err := models.CreateUser(u); err != nil { + if err := models.CreateUser(ctx, u); err != nil { switch { case models.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true @@ -270,7 +270,7 @@ func EditUserPost(ctx *context.Context) { ctx.ServerError("UpdateUser", err) return } - if err = u.SetPassword(form.Password); err != nil { + if err = u.SetPassword(ctx, form.Password); err != nil { ctx.ServerError("SetPassword", err) return } diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index ad69b1e406422..7c46071c8a386 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -97,7 +97,7 @@ func CreateUser(ctx *context.APIContext) { ctx.Error(http.StatusBadRequest, "PasswordPwned", errors.New("PasswordPwned")) return } - if err := models.CreateUser(u); err != nil { + if err := models.CreateUser(ctx, u); err != nil { if models.IsErrUserAlreadyExist(err) || models.IsErrEmailAlreadyUsed(err) || models.IsErrNameReserved(err) || @@ -175,7 +175,7 @@ func EditUser(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "UpdateUser", err) return } - if err = u.SetPassword(form.Password); err != nil { + if err = u.SetPassword(ctx, form.Password); err != nil { ctx.InternalServerError(err) return } diff --git a/routers/install.go b/routers/install.go index 0ec9d4b3e0ea4..e427ef8ed616e 100644 --- a/routers/install.go +++ b/routers/install.go @@ -404,7 +404,7 @@ func InstallPost(ctx *context.Context) { IsAdmin: true, IsActive: true, } - if err = models.CreateUser(u); err != nil { + if err = models.CreateUser(ctx, u); err != nil { if !models.IsErrUserAlreadyExist(err) { setting.InstallLock = false ctx.Data["Err_AdminName"] = true diff --git a/routers/org/setting.go b/routers/org/setting.go index ac120662581d3..aad8d7d63a1e0 100644 --- a/routers/org/setting.go +++ b/routers/org/setting.go @@ -145,7 +145,7 @@ func SettingsDelete(ctx *context.Context) { org := ctx.Org.Organization if ctx.Req.Method == "POST" { - if _, err := models.UserSignIn(ctx.User.Name, ctx.Query("password")); err != nil { + if _, err := models.UserSignIn(ctx, ctx.User.Name, ctx.Query("password")); err != nil { if models.IsErrUserNotExist(err) { ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_password"), tplSettingsDelete, nil) } else { diff --git a/routers/repo/http.go b/routers/repo/http.go index 0377979e8bb5c..e2eaa916b6264 100644 --- a/routers/repo/http.go +++ b/routers/repo/http.go @@ -237,7 +237,7 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { if authUser == nil { // Check username and password - authUser, err = models.UserSignIn(authUsername, authPasswd) + authUser, err = models.UserSignIn(ctx, authUsername, authPasswd) if err != nil { if models.IsErrUserProhibitLogin(err) { ctx.HandleText(http.StatusForbidden, "User is not permitted to login") diff --git a/routers/user/auth.go b/routers/user/auth.go index cb308eda95c92..017984e22ec6f 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -171,7 +171,7 @@ func SignInPost(ctx *context.Context) { } form := web.GetForm(ctx).(*auth.SignInForm) - u, err := models.UserSignIn(form.UserName, form.Password) + u, err := models.UserSignIn(ctx, form.UserName, form.Password) if err != nil { if models.IsErrUserNotExist(err) { ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplSignIn, &form) @@ -822,7 +822,7 @@ func LinkAccountPostSignIn(ctx *context.Context) { return } - u, err := models.UserSignIn(signInForm.UserName, signInForm.Password) + u, err := models.UserSignIn(ctx, signInForm.UserName, signInForm.Password) if err != nil { if models.IsErrUserNotExist(err) { ctx.Data["user_exists"] = true @@ -974,7 +974,7 @@ func LinkAccountPostRegister(ctx *context.Context) { } //nolint: dupl - if err := models.CreateUser(u); err != nil { + if err := models.CreateUser(ctx, u); err != nil { switch { case models.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true @@ -1167,7 +1167,7 @@ func SignUpPost(ctx *context.Context) { Passwd: form.Password, IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), } - if err := models.CreateUser(u); err != nil { + if err := models.CreateUser(ctx, u); err != nil { switch { case models.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true @@ -1267,7 +1267,7 @@ func Activate(ctx *context.Context) { ctx.HTML(200, TplActivate) return } - if !user.ValidatePassword(password) { + if !user.ValidatePassword(ctx, password) { ctx.Data["IsActivateFailed"] = true ctx.HTML(200, TplActivate) return @@ -1529,7 +1529,7 @@ func ResetPasswdPost(ctx *context.Context) { ctx.ServerError("UpdateUser", err) return } - if err = u.SetPassword(passwd); err != nil { + if err = u.SetPassword(ctx, passwd); err != nil { ctx.ServerError("UpdateUser", err) return } @@ -1603,7 +1603,7 @@ func MustChangePasswordPost(ctx *context.Context) { } var err error - if err = u.SetPassword(form.Password); err != nil { + if err = u.SetPassword(ctx, form.Password); err != nil { ctx.ServerError("UpdateUser", err) return } diff --git a/routers/user/auth_openid.go b/routers/user/auth_openid.go index 82e7b5c75340e..f3c893ebadd1e 100644 --- a/routers/user/auth_openid.go +++ b/routers/user/auth_openid.go @@ -289,7 +289,7 @@ func ConnectOpenIDPost(ctx *context.Context) { ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp ctx.Data["OpenID"] = oid - u, err := models.UserSignIn(form.UserName, form.Password) + u, err := models.UserSignIn(ctx, form.UserName, form.Password) if err != nil { if models.IsErrUserNotExist(err) { ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplConnectOID, &form) @@ -418,7 +418,7 @@ func RegisterOpenIDPost(ctx *context.Context) { IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), } //nolint: dupl - if err := models.CreateUser(u); err != nil { + if err := models.CreateUser(ctx, u); err != nil { switch { case models.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go index 4900bba14ac0c..c55a2942ccaea 100644 --- a/routers/user/setting/account.go +++ b/routers/user/setting/account.go @@ -51,13 +51,13 @@ func AccountPost(ctx *context.Context) { if len(form.Password) < setting.MinPasswordLength { ctx.Flash.Error(ctx.Tr("auth.password_too_short", setting.MinPasswordLength)) - } else if ctx.User.IsPasswordSet() && !ctx.User.ValidatePassword(form.OldPassword) { + } else if ctx.User.IsPasswordSet() && !ctx.User.ValidatePassword(ctx, form.OldPassword) { 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(password.BuildComplexityError(ctx)) - } else if pwned, err := password.IsPwned(ctx.Req.Context(), form.Password); pwned || err != nil { + } else if pwned, err := password.IsPwned(ctx, form.Password); pwned || err != nil { errMsg := ctx.Tr("auth.password_pwned") if err != nil { log.Error(err.Error()) @@ -66,7 +66,7 @@ func AccountPost(ctx *context.Context) { ctx.Flash.Error(errMsg) } else { var err error - if err = ctx.User.SetPassword(form.Password); err != nil { + if err = ctx.User.SetPassword(ctx, form.Password); err != nil { ctx.ServerError("UpdateUser", err) return } @@ -226,7 +226,7 @@ func DeleteAccount(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true - if _, err := models.UserSignIn(ctx.User.Name, ctx.Query("password")); err != nil { + if _, err := models.UserSignIn(ctx, ctx.User.Name, ctx.Query("password")); err != nil { if models.IsErrUserNotExist(err) { loadAccountData(ctx) From 51892821acf1e3bb1c986e3eced0cb806a1682a7 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 17 Feb 2021 20:22:14 +0000 Subject: [PATCH 4/7] Add Maximum Concurrent Hashes settings Signed-off-by: Andrew Thornton --- custom/conf/app.example.ini | 2 + .../doc/advanced/config-cheat-sheet.en-us.md | 1 + models/user.go | 46 +++++++++++++++++++ modules/setting/setting.go | 2 + 4 files changed, 51 insertions(+) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index a6965c2cf642b..78795671678b2 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -570,6 +570,8 @@ PASSWORD_HASH_ALGO = pbkdf2 CSRF_COOKIE_HTTP_ONLY = true ; 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 [openid] ; 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 acd02dab08c78..17d582ce7e5eb 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -411,6 +411,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`) diff --git a/models/user.go b/models/user.go index b0e5b71b2e248..d3ac8cdc09c27 100644 --- a/models/user.go +++ b/models/user.go @@ -18,6 +18,7 @@ import ( "path/filepath" "regexp" "strings" + "sync" "time" "unicode/utf8" @@ -374,7 +375,31 @@ func (u *User) NewGitSig() *git.Signature { } } +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 { + cond.Wait() + select { + case <-ctx.Done(): + return "", ctx.Err() + default: + } + } + concurrentHashes++ + cond.L.Unlock() + defer func() { + cond.L.Lock() + concurrentHashes-- + cond.Signal() + cond.L.Unlock() + }() + } + var tempPasswd []byte switch algo { @@ -428,6 +453,27 @@ func (u *User) ValidatePassword(ctx context.Context, passwd string) bool { return subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(tempHash)) == 1 } + // 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 bcrypt.CompareHashAndPassword([]byte(u.Passwd), []byte(passwd)) == nil } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index be7ec16e10cc1..6ebfe55ca2cd9 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -160,6 +160,7 @@ var ( OnlyAllowPushIfGiteaEnvironmentSet bool PasswordComplexity []string PasswordHashAlgo string + MaximumConcurrentHashes int PasswordCheckPwn bool // UI settings @@ -805,6 +806,7 @@ func NewContext() { DisableWebhooks = sec.Key("DISABLE_WEBHOOKS").MustBool(false) OnlyAllowPushIfGiteaEnvironmentSet = sec.Key("ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET").MustBool(true) PasswordHashAlgo = sec.Key("PASSWORD_HASH_ALGO").MustString("pbkdf2") + MaximumConcurrentHashes = sec.Key("MAXIMUM_CONCURRENT_HASHES").MustInt(0) CSRFCookieHTTPOnly = sec.Key("CSRF_COOKIE_HTTP_ONLY").MustBool(true) PasswordCheckPwn = sec.Key("PASSWORD_CHECK_PWN").MustBool(false) From e7a0291f46fdce92514692966aa1d8620e333146 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 17 Feb 2021 20:50:24 +0000 Subject: [PATCH 5/7] windows fix Signed-off-by: Andrew Thornton --- modules/auth/sso/sspi_windows.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/auth/sso/sspi_windows.go b/modules/auth/sso/sspi_windows.go index 46f7ad9d97a0a..c66c4dfe4f101 100644 --- a/modules/auth/sso/sspi_windows.go +++ b/modules/auth/sso/sspi_windows.go @@ -5,6 +5,7 @@ package sso import ( + "context" "errors" "net/http" "strings" @@ -128,7 +129,7 @@ func (s *SSPI) VerifyAuthData(req *http.Request, w http.ResponseWriter, store Da log.Error("User '%s' not found", username) return nil } - user, err = s.newUser(username, cfg) + user, err = s.newUser(req.Context(), username, cfg) if err != nil { log.Error("CreateUser: %v", err) return nil @@ -177,7 +178,7 @@ func (s *SSPI) shouldAuthenticate(req *http.Request) (shouldAuth bool) { // newUser creates a new user object for the purpose of automatic registration // and populates its name and email with the information present in request headers. -func (s *SSPI) newUser(username string, cfg *models.SSPIConfig) (*models.User, error) { +func (s *SSPI) newUser(ctx context.Context, username string, cfg *models.SSPIConfig) (*models.User, error) { email := gouuid.New().String() + "@localhost.localdomain" user := &models.User{ Name: username, @@ -190,7 +191,7 @@ func (s *SSPI) newUser(username string, cfg *models.SSPIConfig) (*models.User, e Avatar: models.DefaultAvatarLink(), EmailNotificationsPreference: models.EmailNotificationsDisabled, } - if err := models.CreateUser(user); err != nil { + if err := models.CreateUser(ctx, user); err != nil { return nil, err } return user, nil From 7c0f84f0473524dae03dbf284678f3d0aab29d65 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 9 Mar 2021 18:52:05 +0000 Subject: [PATCH 6/7] fix cond.Wait() lock-up Signed-off-by: Andrew Thornton --- models/user.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/models/user.go b/models/user.go index 3ddbd763f14b6..2c1f6013d09f6 100644 --- a/models/user.go +++ b/models/user.go @@ -393,12 +393,13 @@ func hashPassword(ctx context.Context, passwd, salt, algo string) (string, error if setting.MaximumConcurrentHashes > 0 { cond.L.Lock() for concurrentHashes > setting.MaximumConcurrentHashes { - cond.Wait() select { case <-ctx.Done(): + cond.L.Unlock() return "", ctx.Err() default: } + cond.Wait() } concurrentHashes++ cond.L.Unlock() @@ -408,6 +409,11 @@ func hashPassword(ctx context.Context, passwd, salt, algo string) (string, error cond.Signal() cond.L.Unlock() }() + select { + case <-ctx.Done(): + return "", ctx.Err() + default: + } } var tempPasswd []byte From 912fd15adddf413a56158f139a2b6d701eef96a1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 14 Jun 2021 18:12:08 +0100 Subject: [PATCH 7/7] fix merge Signed-off-by: Andrew Thornton --- modules/context/context.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/modules/context/context.go b/modules/context/context.go index 497fd0a5780bd..492b3f80ded6f 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -68,26 +68,6 @@ type Context struct { Org *Organization } -// Deadline is part of go's context.Context interface -func (ctx *Context) Deadline() (deadline time.Time, ok bool) { - return ctx.Req.Context().Deadline() -} - -// Done is part of go's context.Context interface -func (ctx *Context) Done() <-chan struct{} { - return ctx.Req.Context().Done() -} - -// Err is part of go's context.Context interface -func (ctx *Context) Err() error { - return ctx.Req.Context().Err() -} - -// Value is part of go's context.Context interface -func (ctx *Context) Value(key interface{}) interface{} { - return ctx.Req.Context().Value(key) -} - // GetData returns the data func (ctx *Context) GetData() map[string]interface{} { return ctx.Data