futuresight / futurebb

The forum system by FutureSight Technologies. A live sample can be seen at http://futuresight.org/forums
http://futurebb.futuresight.org
2 stars 2 forks source link

Do not use unsalted SHA1 for password hash #190

Open iggyvolz opened 7 years ago

iggyvolz commented 7 years ago

https://github.com/futuresight/futurebb/blob/f91c2384c8dd6065f3b25d09a237a48ec63eae49/app_resources/includes/functions.php#L163

At very least use sha256. Better yet - use the password_hash and password_verify functions (PHP 5.5+, all the supported versions of PHP, back to 5.3.7 with https://github.com/ircmaxell/password_compat) which computes a salt for you and puts it in the database.

iggyvolz commented 7 years ago

login_hash will also need to be expanded to fit the full sha256 hash - var_char(100) instead of var_char(50) worked for me.

jacob-g commented 7 years ago

The issue with this is that it will require converting passwords in all existing versions (it's not impossible, but if I'm going to do that, I want to make sure I choose the best algorithm).

iggyvolz commented 7 years ago

If you're converting from sha1 to sha256 you could do:

if(strlen($dbhash) === 40)
{
    // Check with sha1
    // Write sha256 hash to DB
}
else
{
    // Normal sha256 check
}

Or if you're moving to password_hash, then check if the first char is a $ (then it is a blowcrypt signature).

At this point, blowcrypt (the default password_hash algorithm) is the best thing out there as far as PHP goes. Argon2 is another algorithm that's more secure (i.e. validates the password beyond 40 characters), but that won't be in PHP until 7.2 (this fall). In any event - it's impossible to "convert" the passwords with an upgrade operation short of just cracking the passwords and then re-hashing them. You would have to do it at runtime as the user logs in - or the simpler (yet less user-friendly) way is to just invalidate all sha1 passwords and force them to reset their password.