From 623c93ff46798e86636dba24f5a48bb35e12b9a5 Mon Sep 17 00:00:00 2001 From: Gusted Date: Tue, 4 Jan 2022 15:13:52 +0000 Subject: Increase Salt randomness (#18179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The current implementation of `RandomString` doesn't give you a most-possible unique randomness. It gives you 6*`length` instead of the possible 8*`length` bits(or as `length`x bytes) randomness. This is because `RandomString` is being limited to a max value of 63, this in order to represent the random byte as a letter/digit. - The recommendation of pbkdf2 is to use 64+ bit salt, which the `RandomString` doesn't give with a length of 10, instead of increasing 10 to a higher number, this patch adds a new function called `RandomBytes` which does give you the guarentee of 8*`length` randomness and thus corresponding of `length`x bytes randomness. - Use hexadecimal to store the bytes value in the database, as mentioned, it doesn't play nice in order to convert it to a string. This will always be a length of 32(with `length` being 16). - When we detect on `Authenticate`(source: db) that a user has the old format of salt, re-hash the password such that the user will have it's password hashed with increased salt. Thanks to @zeripath for working out the rouge edges from my first commit 😄. Co-authored-by: lafriks Co-authored-by: zeripath --- services/auth/source/db/authenticate.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'services/auth') diff --git a/services/auth/source/db/authenticate.go b/services/auth/source/db/authenticate.go index e0e439c2fe..f062f66ae0 100644 --- a/services/auth/source/db/authenticate.go +++ b/services/auth/source/db/authenticate.go @@ -21,7 +21,9 @@ func Authenticate(user *user_model.User, login, password string) (*user_model.Us } // Update password hash if server password hash algorithm have changed - if user.PasswdHashAlgo != setting.PasswordHashAlgo { + // Or update the password when the salt length doesn't match the current + // recommended salt length, this in order to migrate user's salts to a more secure salt. + if user.PasswdHashAlgo != setting.PasswordHashAlgo || len(user.Salt) != user_model.SaltByteLength*2 { if err := user.SetPassword(password); err != nil { return nil, err } -- cgit v1.2.3