From 674cfb7cacd99bb236b13db8754b21ae2f555ca8 Mon Sep 17 00:00:00 2001 From: Morgan Bazalgette Date: Mon, 8 Jan 2018 23:28:18 +0100 Subject: [PATCH] Change EncodePasswd to HashPassword (#3329) * Change EncodePasswd to HashPassword * Create test+benchmark for HashPassword --- cmd/admin.go | 2 +- models/user.go | 8 ++++---- models/user_test.go | 39 ++++++++++++++++++++++++++++++++++++ routers/admin/users.go | 2 +- routers/api/v1/admin/user.go | 2 +- routers/user/auth.go | 2 +- routers/user/setting.go | 2 +- 7 files changed, 48 insertions(+), 9 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index 4df28358a2..5000fc2d4d 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -111,7 +111,7 @@ func runChangePassword(c *cli.Context) error { if user.Salt, err = models.GetUserSalt(); err != nil { return fmt.Errorf("%v", err) } - user.EncodePasswd() + user.HashPassword() if err := models.UpdateUserCols(user, "passwd", "salt"); err != nil { return fmt.Errorf("%v", err) } diff --git a/models/user.go b/models/user.go index 3839e14590..c30c66d6c3 100644 --- a/models/user.go +++ b/models/user.go @@ -388,8 +388,8 @@ func (u *User) NewGitSig() *git.Signature { } } -// EncodePasswd encodes password to safe format. -func (u *User) EncodePasswd() { +// HashPassword hashes a password using PBKDF. +func (u *User) HashPassword() { newPasswd := pbkdf2.Key([]byte(u.Passwd), []byte(u.Salt), 10000, 50, sha256.New) u.Passwd = fmt.Sprintf("%x", newPasswd) } @@ -397,7 +397,7 @@ func (u *User) EncodePasswd() { // ValidatePassword checks if given password matches the one belongs to the user. func (u *User) ValidatePassword(passwd string) bool { newUser := &User{Passwd: passwd, Salt: u.Salt} - newUser.EncodePasswd() + newUser.HashPassword() return subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(newUser.Passwd)) == 1 } @@ -711,7 +711,7 @@ func CreateUser(u *User) (err error) { if u.Salt, err = GetUserSalt(); err != nil { return err } - u.EncodePasswd() + u.HashPassword() u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization u.MaxRepoCreation = -1 diff --git a/models/user_test.go b/models/user_test.go index 03ab54aaf7..c0b7dace7f 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -5,6 +5,7 @@ package models import ( + "math/rand" "testing" "code.gitea.io/gitea/modules/setting" @@ -123,3 +124,41 @@ func TestDeleteUser(t *testing.T) { test(8) test(11) } + +func TestHashPasswordDeterministic(t *testing.T) { + b := make([]byte, 16) + rand.Read(b) + u := &User{Salt: string(b)} + for i := 0; i < 50; i++ { + // generate a random password + rand.Read(b) + pass := string(b) + + // save the current password in the user - hash it and store the result + u.Passwd = pass + u.HashPassword() + r1 := u.Passwd + + // run again + u.Passwd = pass + u.HashPassword() + r2 := u.Passwd + + // assert equal (given the same salt+pass, the same result is produced) + assert.Equal(t, r1, r2) + } +} + +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} + b.ResetTimer() + for i := 0; i < b.N; i++ { + u.HashPassword() + u.Passwd = pass + } +} diff --git a/routers/admin/users.go b/routers/admin/users.go index 1081f3092f..02b8334564 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -200,7 +200,7 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) { ctx.Handle(500, "UpdateUser", err) return } - u.EncodePasswd() + u.HashPassword() } u.LoginName = form.LoginName diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index fc1d3da2f1..5bbb8a7afd 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -132,7 +132,7 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) { ctx.Error(500, "UpdateUser", err) return } - u.EncodePasswd() + u.HashPassword() } u.LoginName = form.LoginName diff --git a/routers/user/auth.go b/routers/user/auth.go index c353db8984..f7fb1512a1 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -994,7 +994,7 @@ func ResetPasswdPost(ctx *context.Context) { ctx.Handle(500, "UpdateUser", err) return } - u.EncodePasswd() + u.HashPassword() if err := models.UpdateUserCols(u, "passwd", "rands", "salt"); err != nil { ctx.Handle(500, "UpdateUser", err) return diff --git a/routers/user/setting.go b/routers/user/setting.go index a2f32e3e19..d83c62d89d 100644 --- a/routers/user/setting.go +++ b/routers/user/setting.go @@ -235,7 +235,7 @@ func SettingsSecurityPost(ctx *context.Context, form auth.ChangePasswordForm) { ctx.Handle(500, "UpdateUser", err) return } - ctx.User.EncodePasswd() + ctx.User.HashPassword() if err := models.UpdateUserCols(ctx.User, "salt", "passwd"); err != nil { ctx.Handle(500, "UpdateUser", err) return