[wp-trac] [WordPress Trac] #50027: Retire Phpass and use PHP native password hashing

[WordPress Trac]

#50027: Retire Phpass and use PHP native password hashing
-------------------------------------------------+-------------------------
 Reporter:  ayeshrajans                          |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  Security                             |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  2nd-opinion needs-unit-tests needs-  |     Focuses:
  patch                                          |
-------------------------------------------------+-------------------------

Comment (by ayeshrajans):

 Thanks a lot for the in-depth comment @Otto42, very helpful.

 In that plugin, one of the use cases was to make it possible for WordPress
 and a custom site to share login, so having raw passwords gone through
 `password_hash` helped to keep things cleaner, so it became a design
 choice to ignore the 72 char limit.

 I see the points raised about bcrypt's 72 byte limitation, and while they
 are valid and can solve the very problem of "unlimited" length passwords,
 I would like to propose that we do not pre-hash passwords.

 Can we simplify this? We have two approaches:

 - Ignore that bcrypt truncates beyond 72 bytes. Hard-reject at 4096 bytes
 regardless of the hashing algorithm (bcrypt, argon2, etc). This introduces
 no BC backwards, and will allow us to accept longer passwords (So `correct
 horse battery staple`-akin passwords are allowed). As for security, I
 would argue that beyond the sensible password length or a 30-40 bytes,
 brute-forcing passwords is impractical for everyone unless they have
 billions of years to do it.

 - If the password hashing algo is bcrypt, we throw an error for inputs
 larger than 72 bytes. This is exactly what
 [https://github.com/symfony/security/blob/3dcb8400076290d67bcd13832460da0d93c7a35c/Core/Encoder/NativePasswordEncoder.php#L65
 symfony/security component does], and it probably is the least surprising
 way, although we break BC for those who use quarter a tweet for their
 password.


 Both approaches above will make the password hashes truly portable and
 make it easier to integrate WordPress with other platforms, be it
 importing users to WordPress, exporting WordPress users to another, or
 cross-authentication.



 > c) @mbijon suggested using batch processing and usermeta. This is not
 necessary, nor actually possible. The conversion process would need the
 original password input by the user, so it can only be done at the time
 they actually login, same as the old MD5 upgrade process. So, we modify
 the existing process to upgrade from old $P passwords to new ones in the
 exact same manner as it currently does for MD5 hashes.

 You are right we will not be able to batch-process passwords, nor it would
 be a good idea to have millions of WordPress sites suddenly bcrypt
 millions of strings during an update.


 > d) Centralize the use of password_hash (and password_verify), so that
 filters can be applied to it across the board, in case somebody wants to
 change $algo or $options.

 Someone also mentioned config.php directive too, noting that it is too
 much of a responsibility for a plugin to alter hashing algorithms mid-
 flight. But I agree that it should be neatly centralized and allow room
 for modifications.

 > e) For obvious reasons, the phpass library will need to remain to verify
 existing hashes before they are upgraded. No plans to remove or deprecate
 it are necessary. We still convert old MD5 hashes, after all.

 Yep. It's a small-enough library and it is only loaded on-demand even at
 this point. It's a third party library, but it would be nicer to have
 `hash_equals()` in there instead of the current `===` comparator.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50027#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform