[D10][SR] Deprecate or wrap existing functions in password.inc around the `password_hash()` function (built into PHP 5.5+)

[ghost]

This is the respective issue as Replace custom password hashing library with PHP 5.5 password_hash() for Drupal.

password.inc seems to be quite old and, possibly, no longer necessary/safe...

For example, the comment at the top says:

Based on the Portable PHP password hashing framework.

• @see http://www.openwall.com/phpass/

That site says:

At this time, if your new project can afford to require PHP 5.5+, which it should, please use PHP's native password_hash() / password_verify() API instead of phpass.

Additionally, the documentation for BACKDROP_HASH_COUNT says:

This should increase by 1 every Backdrop version in order to counteract increases in the speed and power of computers available to crack the hashes.

Now admittedly this was written for Drupal, and presumably refers to their major versions (6, 7, 8, etc.), but even so, the last time this was updated was back in Drupal - Backdrop's never updated this value.

So I'm wondering if we should remove (or deprecate) this file and its functions in favour of PHP's built-in functions, or at the very least keep it updated and secure. Thoughts?

[indigoxela]

Interesting...

if your new project can afford to require PHP 5.5+...

Backdrop - since version 1.22.0 - does require php 5.6 as a minimum version. The user module uses functions from password.inc. It might be time to evaluate that.

[stpaultim]

@BWPanda - I don't have a lot to contribute to the technical side of this discussion. You have convinced me that at a minimum we should review this file and decide if it's worth keeping or updating. I am not sure if there are backward compatibility issues, but I assume not.

I look forward to hearing what others think.

[quicksketch]

I think it would be great if we could deprecate or wrap our existing functions around PHP's built-in password_hash() function. I'm not sure password_hash() is compatible with the hashes we save in the database today though. We could update password hashes as users log in, but it would be ideal if we could bulk update the password hashes, or if we didn't need to update them at all. I'm not sure what's possible.

[klonos]

I think it would be great if we could deprecate or wrap our existing functions around PHP's built-in password_hash() function. ...

I've added a link to https://www.drupal.org/project/drupal/issues/1845004 (the respective issue in d.o) in the issue summary.

This should increase by 1 every Backdrop version in order to counteract increases in the speed and power of computers available to crack the hashes.

Now admittedly this was written for Drupal, and presumably refers to their major versions (6, 7, 8, etc.), but even so, the last time this was updated was back in Drupal - Backdrop's never updated this value.

I have split that task into a separate issue here: #5810

[BillTheGoat]

As per phpass page linked above: The preferred (most secure) hashing method supported by phpass is the OpenBSD-style Blowfish-based **bcrypt**. This is also the best form of hashing supported by Apache2, and the one Drupal is currently using (visible via issue linked above).

At the top of password.inc, it looks easy to replace since there are only 3 public functions:

 * An alternative or custom version of this password hashing API may be
 * used by setting the variable password_inc to the name of the PHP file
 * containing replacement user_hash_password(), user_check_password(), and
 * user_needs_new_hash() functions.
 *

0) Use PHP's native password_hash() / password_verify() functions using bcrypt as the new hash.

1) There are already fall backs in user_check_password() for older style md5 password checks, which could be used as a template to allow log in with the existing SHA512 hashes (simple).

2) Since there are no stored passwords, the best way to update hashes is when user logs in and we can access their plain text password, which is likely when user_needs_new_hash() is called already (I did not check Backdrop code, but this is the way current Drupal does it). It would need to be modified to check according to the new hash format.

3) We need to check whether the db column is compatible with the new hash length / format, and update / future proof if required (255 char column recommended I believe).

I think that is it?

If I jump in to make Backdrop and Apache2 mod_db talk nicely as per today's office hours, this would be the way I would do it, so if nobody else gets to this, there is some chance I might.

[avpaderno]

The documentation for password_hash() says that a database column that can store 255 characters would be a good choice, even if the actual hash is not longer than 125 characters. The database column for the password can store 128 characters, which is sufficient, for now.

[avpaderno]

Using password_hash() with the PASSWORD_BCRYPT algorithm will cause the password to be truncated to 72 characters, which is what happens with crypt() and the CRYPT_BLOWFISH algorithm. Backdrop allows longer passwords.

Caution Using the PASSWORD_BCRYPT as the algorithm, will result in the password parameter being truncated to a maximum length of 72 bytes.

Caution Using the CRYPT_BLOWFISH algorithm, will result in the string parameter being truncated to a maximum length of 72 bytes.

PASSWORD_ARGON2I and PASSWORD_ARGON2ID are only available when PHP is compiled with Argon2 support.

[klonos]

I'm not sure password_hash() is compatible with the hashes we save in the database today though. We could update password hashes as users log in, but it would be ideal if we could bulk update the password hashes, or if we didn't need to update them at all. I'm not sure what's possible.

This change was released for Drupal 10.1.0 back in April. See change record here: https://www.drupal.org/node/3322420

In Drupal core, they've introduced a new Password Compatibility module, which holds the previous algorithm and is enabled on existing installations, but not in new sites. We could do the same and then get rid of that module (or move it to contrib) in 2.x.

Quoting the Drupal change record, to answer your question @quicksketch:

When a site updates to Drupal 10.1.0 or later, a database-update function installs the Password Compatibility module. This allows existing users to use their credentials. If the module is not installed, existing users can login by requesting a new password.

The first time an existing user logs in, a new hash is and saved to the database. From then on, the user can log in with the same credentials whether this module is installed or not.

Users who login with some other method, such as a one-time login link, and do not update their passwords, will not have the new hash.

There's more details in the change record (like how to check how many user accounts still use the old hash types, and when to know whether the helper module can be safely uninstalled), but in general it seems like a solid implementation and very doable with minimal impact and actions required.