use hash_equals to check password hash

[hanshenrik]

today in wp-includes/class-phpass.php under function CheckPassword we find

		# This is not constant-time.  In order to keep the code simple,
		# for timing safety we currently rely on the salts being
		# unpredictable, which they are at least in the non-fallback
		# cases (that is, when we use /dev/urandom and bcrypt).
		return $hash === $stored_hash;

and while i agree that a constant-time comparison is probably not needed, it's a trivial change to fix it, and better safe than sorry. I suggest changing it to

		if(PHP_VERSION_ID >= 50600){
			return hash_equals($stored_hash, $hash);
		} else {
			# This is not constant-time.  In order to keep the code simple,
			# for timing safety we currently rely on the salts being
			# unpredictable, which they are at least in the non-fallback
			# cases (that is, when we use /dev/urandom and bcrypt).
			return $hash === $stored_hash;
		}

PHP_VERSION_ID was introduced in 5.2.7, and i doubt WordPress still need to support PHP5.2. Unsure if WordPress still support 5.5? if the answer is no, the entire PHP_VERSION_ID can be removed.

.. on PHP>=5.6.0
notably it would be possible to implement constant time on <5.6.0, but it's definitely not worth the effort. the PHP_VERSION_ID constant was introduced in PHP 5.2.7

Unsure if WordPress still support 5.5? if the answer is no, the entire PHP_VERSION_ID check can be removed.

Trac ticket: See https://core.trac.wordpress.org/ticket/56335

[hanshenrik]

turns out PHP<5.6 support has been dropped, that simplifies things, PR updated :)

Also fwiw PHP's built-in password_equals() also use constant-time hash check internally.

[desrosj]

I'm going to add a close suggestion here.

The ​PHPass class is an external library, though WordPress has made some changes to it over time (see the description of #51549).

I believe that the original intent of PHPass was to properly support password hashing on PHP < 5.5, which no longer applies to WordPress. But moving off of PHPass is a much larger discussion currently being had in #50027 and #21022. I think I prefer that we direct any effort and attention towards those tickets instead.

[johnbillion]

Closing in favour of #50027 and #21022. Thanks!