Differential D8268

Make password hashing modular
ClosedPublic
Actions

Authored by epriestley on Feb 18 2014, 5:57 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T4443: Use bcrypt / password_hash() to hash passwords if available

Commits

Restricted Diffusion Commit
rP3c9153079f13: Make password hashing modular

Summary

Ref T4443. Make hashing algorithms pluggable and extensible so we can deal with the attendant complexities more easily.

This moves "Iterated MD5" to a modular implementation, and adds a tiny bit of hack-glue so we don't need to migrate the DB in this patch. I'll migrate in the next patch, then add bcrypt.

Test Plan

Verified that the same stuff gets stored in the DB (i.e., no functional changes):
- Logged into an old password account.
- Changed password.
- Registered a new account.
- Changed password.
- Switched back to master.
- Logged in / out, changed password.
- Switched back, logged in.
Ran unit tests (they aren't super extensive, but cover some of the basics).

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

Here's the new table to help administrators understand hashing:

{F115237}

I was going to ask "why?" Then I had the fun read of T4443. You sir are quite kind to existing installs re: php extensions. :)

src/infrastructure/util/password/PhabricatorPasswordHasher.php
10	hasher

Gave this a quick read-through; looks well-designed to handle corner cases such as moving to an install that has fewer hashers available, etc.

Yeah, on the one hand this is sort of overkill. If we had reasonable access to bcrypt in PHP 5.2.3 without extensions I would probably just switch all new hashing over and then opportunistically migrate the md5 stuff. However, we don't have convenient access until 5.5.0, and have no non-extension access to scrypt.

In defense of the complexity involved here, the underlying problem really is a migration problem (i.e., the cost factor of the hashing algorithm should theoretically be keeping pace with the speed of hardware), and this isn't that complex, and lets us reframe the problem completely as a migration problem: you specify the best algorithm, and everything moves toward it on average. After D8271, we can bump the cost factor every couple of years with one line of code.

I like all the transparency this brings around algorithms, too. And we can also share all this code in the VCS password stuff (coming shortly).