-
Notifications
You must be signed in to change notification settings - Fork 103
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
Updates for bcrypt support #238
Updates for bcrypt support #238
Conversation
* avoid errors about undef values in strings from Perl strict mode * only accept old hashes when password_hash_update is set to true upgrade_sympa_password.pl.in: * new command line options and associated POD documentation * support for precaching hashes (useful for big sites using bcrypt) * measure how long it takes to generate a hash (useful for bcrypt users) * report on progress every 100 users processed when in verbose mode
Reading changes of code, I feel this PR is promising. I'll merge it. |
Thank you for your review and for accepting this code! |
@mpkut, if possible, how about adding your description about usage to documentation? Currently there are no detailed description. |
@ikedas I have submitted a small PR for that page related to the password_hash_update parameter. As for instructions on how one might precalculate hashes at a big site, would it be acceptable to put that in an |
@mpkut, thanks! Your pr will be merged in some days. I think you may either include If I had to choose, latter may make license problem clear. |
@ikedas - OK, will put the instructions in the admin manual page. Query: what does "license problem" mean in this context? |
Administration Manual is planned to be released under CC BY-SA at its beginning. However PODs are not the case: As they are part of source files, probably GPL may be applied, but it doesn't suit for documentation works. IMO clearer license is better. |
Got, it thanks! |
This PR contains updates for the bcrypt hash support, based on tag 6.2.25b.3.
-- Fix some undef string interpolation errors in
User.pm
when running in Perl strict mode--
password_fingerprint()
now usespassword_hash_update
as a flag to control whether old hash types are honored when a new hash algorithm is chosen.**
1
= honor old hashes and update the user_table upon user login, for a graceful transition**
0
= only use the system configured hash type; users with old hashes must set a new passwordThis behavior better fits expectations and the recently created documentation.
Just to note in case it should be changed, the default for
password_hash_update
is currently1
.-- Calculating updated hashes for tens of thousands of users with an intentionally expensive algorithm like Bcrypt can take many, many hours. At the same time, many sites (including ours) have limited maintenance windows for updates, even major upgrades.
The hash update could be done in the background after returning to service. However that would prevent taking a final backup of Sympa in its upgraded-but-quiescent state before going into production. At least at our site, it would be helpful if we could reduce the time required to actually perform the user_table update.
To do this, we updated
upgrade_sympa_password.pl.in
to add a cache of previously calculated hashes (--cache cachefile
), and an option (--noupdateuser
) to disable actually updating the user table. The final piece is an alternate config file (--config filename
) that contains the new password hash settings. With these options we can calculate hashes ahead of time:upgrade_sympa_password.pl --config /path/to/new-config --cache /path/to/cachefile --noupdateuser -v >&/tmp/precalc.log
When it's time to finally upgrade, the update time is reduced to whatever is required to actually update the user database:
upgrade_sympa_password.pl -c /path/to/cachefile -v >&/tmp/upgrade.log
BTW the cache file does not contain cleartext passwords, but does contain MD5 checksums of them as a way to confirm that they have not changed since the hashes were precalculated.
Again, only particularly large sites will find much use for this update, but it will help ours a lot.
Thank you again for your consideration of these PR's!