Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR] Increase password hashing iterations (BACKDROP_HASH_COUNT) #5810

Open
klonos opened this issue Sep 29, 2022 · 4 comments · May be fixed by backdrop/backdrop#4336, backdrop/backdrop#4337, backdrop/backdrop#4338 or backdrop/backdrop#4339

Comments

@klonos
Copy link
Member

klonos commented Sep 29, 2022

This was brought up in #5655, but since that issue may prove tricky(ier), I'm splitting this task here into its own issue, to see if we can do it sooner.

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

...

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.

There is a respective issue in d.o: https://www.drupal.org/project/drupal/issues/3110670, where @mcdruid has performed a benchmark to test how bigger numbers of iterations affect execution time, in which it was concluded that it is OK to increase the iterations, but not too much.

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

count (iterations) time
16 0m1.801s
17 0m1.835s
18 0m1.926s
19 0m2.386s
20 0m3.512s
21 0m5.230s
22 0m8.693s
23 0m15.590s
24 0m29.829s
25 0m58.770s
26 1m55.109s
27 3m50.611s
28 7m37.155s
30 30m27.600s

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.

@klonos klonos changed the title Increase password hashing iterations [SR] Increase password hashing iterations Sep 29, 2022
@klonos klonos changed the title [SR] Increase password hashing iterations [SR] Increase password hashing iterations (BACKDROP_HASH_COUNT) Sep 29, 2022
@klonos
Copy link
Member Author

klonos commented Sep 29, 2022

...so let the bikeshedding begin 😅

It seems to me that if we increased the current value from 16 to either 17 or 18, the change would be negligible. Some might argue that that applies to 19 or even 20. But if we go to 21 iterations, we'd be increasing the time from less than 2sec to 5sec+, which might be more noticeable.

@mcdruid
Copy link

mcdruid commented Sep 29, 2022

There's also a whole different bike shed in https://www.drupal.org/project/drupal/issues/1845004

phpass's author recommends:

...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.

https://www.openwall.com/phpass/

Perhaps if you can, you should.. rather than tweaking the custom phpass implementation which I believe pre-dated the availability of a decent native alternative in PHP.

@klonos
Copy link
Member Author

klonos commented Sep 29, 2022

Thanks @mcdruid ...yes, we have #5655 for that 👍🏼

@klonos
Copy link
Member Author

klonos commented Feb 12, 2023

I have filed 4 PRs (increasing the number of iterations from 16 to 17, 18, 19, and 20), to compare how this might affect our test times. Here are the results of total runtime (with sums of the 3 parts of each php version test + a total for all tests):

nr. of iterations → 16 (current) 17 18 19 20
php 5.6 ~23m 23m 54s 25m 34s 31m 10s 39m 9s
php 7.4 ~17m 18m 58s 18m 40s 23m 24s 27m 36s
php 8.1 ~15m 17m 24s 18m 38s 27m 14s 29m 32s
total runtime of all tests ~55m 60m 16s 62m 52s 81m 48s 96m 17s

This is indicative of course, since the test parts run in parallel. So actual time is less than those on the table. Feel free to re-run the PRs to get fresh results.

This is on GitHub shared server hardware (so times I imagine are subject to current platform load etc.), but it is real-life test runs nevertheless. I was planing to purchase a Pi for some time now, so I'll do that when I'm back to Melbourne next week, and test there soon as I get it and set it up. I will also test on a single-CPU Nanode. I believe that benchmarking on a ~$130-$150AUD hardware and $5US/month hosting should be acceptable as current "minimal" (*).

(*) trying to keep in mind people in less fortunate parts of the world or from different socio-economic backgrounds, for whom I understand that these may be considerable amounts of money - not being disrespectful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment