XH 1.7: Password hashing

Discussions and requests related to new CMSimple features, plugins, templates etc. and how to develop.
Please don't ask for support at this forums!
Post Reply
cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

XH 1.7: Password hashing

Post by cmb » Tue Feb 24, 2015 3:27 pm

Hello Community,

since CMSimple_XH 1.5.4 we adopted PasswordHash for hashing the admin password. For portability we opted in to the portable hashes (based on MD5). However, these hashes are relatively weak[1], and since PHP 5.3.0 (5.3.2?) PasswordHash is able to support the stronger bcrypt in a portable way. Therefore I suggest to switch to its bcrypt hashes. Note that the old passwords would be still valid (only not as safe).

An alternative might be to use PHP's native crypt(), but I don't know how to use that in a portable way. password_hash() appears to be the best option, but that is only available since PHP 5.5.0. So for now (i.e. until we require PHP 5.5) I suggest to stick with PasswordHash.

An additional benefit of PasswordHash would be the get_random_bytes() method. I'm not an expert on CSRNGs, but I'm confident that get_random_bytes() is far superior to mt_rand() or even rand() for cryptographic purposes. A presumably better alternative would be openssl_random_pseudo_bytes(), but that may not be widely available.

Anyhow, if we stick with PasswordHash, I suggest that we fully adopt it, and modify it according to our CS and use a PHP 5 constructor, add visibility specifiers and remove the fallbacks for PHP < 5. Besides the constructor, HashPassword(), CheckPassword() and get_random_bytes() (renamed to getRandomBytes()) should be public; all other methods and properties protected/private. And of course we should rename the class to XH_PasswordHash, so it can be autoloaded by the general autoloader.

I suggest to apply this patch.

Thoughts?

See also: [1] For once, it uses md5() which is not collision safe, and furthermore the stretching is implemented in PHP, which is slower. On my PC bcrypt is roughly able to use the double amount of rounds (i.e. cost+1) than the md5 based variant. Therefore we can easily switch to cost 9, and maybe we should increase that further.
Christoph M. Becker – Plugins for CMSimple_XH

manu
Posts: 1086
Joined: Wed Jun 04, 2008 12:05 pm
Location: St. Gallen - Schweiz
Contact:

Re: XH 1.7: Password hashing

Post by manu » Wed Feb 25, 2015 10:22 am

+1
It's all high discipline, but sounds reasonable.

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH 1.7: Password hashing

Post by cmb » Mon Mar 23, 2015 12:51 am

Done (r1517).
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH 1.7: Password hashing

Post by cmb » Fri Apr 14, 2017 3:34 pm

Well, more than two years have passed, and now I see the following severe issue with XH\PasswordHash:

::getRandomBytes() tries to read /dev/urandom (what might not be the best option, anyway; php_random_bytes() uses it as last resort), but if that fails (e.g. on Windows), it falls back to constructing random bytes mostly based on microtime() calls. This is definitely not cryptographically secure, but crypt() relies on a good salt:
Make sure to specify a strong enough salt for better security.
Furthermore, password_hash() and friends are supposed to offer strong password hashes in a simple and portable way as of PHP 5.5.0, and there is a "Polyfill" available for PHP 5.3.7: password_compat.

Therefore I suggest to replace XH\PasswordHash with password_hash() and friends, and to include password_compat (it's only a single file). For maximum portability we can use PASSWORD_BCRYPT as algorithm, which currently doesn't matter anyway as PHP_PASSWORD_DEFAULT is still PASSWORD_BCRYPT in current PHP master. This way users of recent PHP versions have the best support available, and older versions would have an acceptable fallback (I'm not concerned to raise the requirements from PHP 5.3.0 to 5.3.7).

We would need a public replacement for ::getRandomBytes(), though, as it's used elsewhere in the core. random_bytes() is available as of PHP 7.0.0, and for older versions the respective code from password_compat might already be sufficient; otherwise we could use (relevant parts of) random_compat. random_compat has the advantage that it doesn't fall back on weak algorithms, but rather raises an Exception if sufficiently strong sources of randomness are not available, as does random_bytes(). We could still fall back to a weak entropy source in this case, but also show a warning to the user, or maybe simply not offer related functionality (currently, only the password forgotten link and configuration fields of type "random").
Christoph M. Becker – Plugins for CMSimple_XH

manu
Posts: 1086
Joined: Wed Jun 04, 2008 12:05 pm
Location: St. Gallen - Schweiz
Contact:

Re: XH 1.7: Password hashing

Post by manu » Fri Apr 14, 2017 6:33 pm

Thanks for seeing ahead wisely.

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH 1.7: Password hashing

Post by cmb » Fri Apr 14, 2017 7:02 pm

manu wrote:Thanks for seeing ahead wisely.
Hopefully. :)
Christoph M. Becker – Plugins for CMSimple_XH

Post Reply