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

WordPress Trac noreply at wordpress.org
Wed Apr 29 14:20:44 UTC 2020


#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 Otto42):

 @ayeshrajans Looking at your plugin, you are hashing the raw password,
 which has the problem of truncating it at 72 characters, as was still
 being discussed over in #21022.

 Other than that, creating a patch is relatively straightforward. Filters
 could be added to allow changes to the $algo and $options parameters of
 password_hash, however those should be left as the default settings in
 most/all cases.

 The only issue remaining for a patch is the 72 character limitation.

 One of the solutions suggested in the previous ticket is to essentially do
 this to the password:

 `$prehash = base64_encode( hash( 'sha512', $raw_password, true ) )`

 And then to run the prehash through password_hash. The purpose of this is
 to reduce the size of the password from whatever length it is down to
 small enough to fit into the limit for password_hash. This is always
 better than truncation, really.

 The reason for the base64 is speed, simplicity, and avoiding null bytes in
 the hash. These are valid reasons.

 Minor problem is that sha512 produces a 64 byte result, and base64'ing it
 produces an 88 byte string. Slightly longer than what we want. Better than
 nothing even with truncation, but still.

 A viable alternative that ticks all the boxes would be to use sha384
 instead.
 @paragoninitiativeenterprises : please weigh in on this idea:

 This produces a 64 byte string. Always:

 `$prehash = base64_encode( hash( 'sha384', $raw_password, true ) )`

 It's not sha512. But the purpose of using a hash here is to reduce the
 possibly very long passphrase down into a reasonable length. The
 substantial problem and difference in using various hashes here is thus
 the possibility of collision. There is a higher collision chance with 384
 vs. 512. However, given that the longer 512 result would be truncated by a
 whole 16 bytes anyway (which is equivalent to 12 bytes in the 4/3
 conversion from base64), then the difference is not particularly
 significant.

 As for support, I believe the sha384 algorithm should be equally available
 in all php versions where sha512 exists.

 So, I suggest the following:

 a) Switch all password hashing to `password_hash()` using `$prehash =
 base64_encode( hash( 'sha384', $raw_password, true ) )` as a size reducer
 to allow for "unlimited" password length.

 b) Due to speed of hashing considerations and DOS attacks, prefilter the
 allowed password length down to 4096 characters, the same as we already
 have in WordPress and for the exact same reasons. This code will remain
 essentially the same.

 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.

 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.

 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.

 Given all that, a patch to implement this is relatively straightforward,
 if we can agree on step A and using a prehash method to overcome the 72
 character truncation problem of password_hash.

 I suggest sha384. Any objections?

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


More information about the wp-trac mailing list