Increase password hashing iterations

Why doesn't core use password_hash?

I wrote the comment about increasing this every version assuming we were on ~18-24 month release cycles.

Heh...

We'd do this in a minor release. It was even backported to a patch release of D7. ;)

The todo needs fix because it doesn't mention minor or major version should affect it
https://git.drupalcode.org/project/drupal/-/blob/9.0.x/core/core.service...

I'd suggest making the value configurable giving the right to the site builders/owners. For example: adding an item to settings.php

$settings['drupal_hash_count'] = 18;

In core, just set a default value as the recommended count by the community.

following are my considerations:

1. The requirements of security are varied:

   a) For firewall-protected internal web applications, the security is less important probably. The value could be less than the recommended one.

b) For some security-sensitive web applications, they may want to sacrifice the time/UE to enhance the security so that they may want to set a higher count value than the recommended one.

2. The servers are varied too when we increase the count, the servers are still the old ones, for most of the sites. the execution time is longer a little bit right after being updated when the end-users requesting the relevant page(s).

If they set a value lower than the recommended one, a warning could be given on the status page if it is necessary.

cilefen commented 8 February 2020

Why doesn't core use password_hash?

Yeah, it should nowadays. Linking #1845004: Replace custom password hashing library with PHP password_hash().

As far as this issue goes, we can do benchmarks but IIUC the point of increasing the hashing iterations is to make cracking Drupal password hashes more costly.

A typical Drupal site doesn't need to - for example - go through a word list trying to crack hashes. (Although there is a module for that :).

I don't think the cost of the hashing matters particularly for a Drupal site (within reason).

Having said all that I did some benchmarking and the results were interesting - tl;dr we should not increase this iteration count too much.

Methodology

Incremenent the count, clear cache and time setting one new password using drush.

n.b. don't set real passwords like this - there's far too little entropy (the value actually shouldn't make any difference to the benchmark).

$ for i in {16..32} ; do sed -i "s/arguments: \[[0-9]\{2\}\]/arguments: [$i]/g" core/core.services.yml ; git diff core/core.services.yml | grep argument ; drush cr ; time drush upwd admin "$(date | sha256sum | awk '{ print $1 }')" ; done

 [success] Cache rebuild complete.
 [success] Changed password for admin.

real    0m1.801s
user    0m1.201s
sys     0m0.124s

-    arguments: [16]
+    arguments: [17]
 [success] Cache rebuild complete.
 [success] Changed password for admin.

real    0m1.835s
user    0m1.322s
sys     0m0.089s

...snip...

This is on an Intel(R) Core(TM) i7-4702MQ CPU @ 2.20GHz. Pretty sure this is single-threaded.

Results

...I gave up there.

Conclusion

I think it's okay to keep incrementing the count for D9 and the next few major releases, but we probably don't want to increase the count to the point where hashing a single password takes tens of seconds (or more) on typical hardware.

Benchmarks are above, but still needs work to pass tests

Better title

I did some quick benchmarking to see where things are at and found that password hashing is approximately twice as fast on a newish laptop compared to a ~10-year-old, budget-grade server. And the laptop was actually running around a third of its max clock rate - maxing out one core was not enough to get it to switch to a higher clock rate.

Based on this, I'd say increasing to 17 iterations is very reasonable - it's a little slow on old hardware but still under 100 milliseconds. And those hosting on better hardware could reasonably go higher than 17. Of course, I'd hope that phpass will be phased out any day now in the related issue, i.e. fingers crossed this issue doesn't need to be worked on?

Newish laptop (AMD Ryzen 7 PRO 6850U):

./vendor/bin/drush scr test.php
16 => 20 milliseconds
17 => 41 milliseconds
18 => 82 milliseconds
19 => 163 milliseconds
20 => 327 milliseconds
21 => 654 milliseconds
22 => 1312 milliseconds
23 => 2628 milliseconds
24 => 5259 milliseconds

Old server (Intel Xeon E3-1265L v3):

./vendor/bin/drush scr test.php
16 => 44 milliseconds
17 => 87 milliseconds
18 => 175 milliseconds
19 => 349 milliseconds
20 => 698 milliseconds
21 => 1397 milliseconds
22 => 2797 milliseconds
23 => 5593 milliseconds
24 => 11177 milliseconds

Test script is as follows:

$start = microtime(TRUE);
$end = microtime(TRUE);
$iterations = range(16, 24);
foreach ($iterations as $countLog2) {
  $password = new Drupal\Core\Password\PhpassHashedPassword($countLog2);
  $start = microtime(TRUE);
  $password->hash('lololololololololol');
  $end = microtime(TRUE);
  echo $countLog2, ' => ', round(1000 * ($end - $start)), ' milliseconds', PHP_EOL;
} 

I think this issue should be closed in favor of #1845004: Replace custom password hashing library with PHP password_hash() - but setting to needs review until that happens

Do agree this can be closed in favor of #1845004: Replace custom password hashing library with PHP password_hash() that one seems like a closer. I'll move over credit.