Page MenuHomePhabricator

Make password hashing modular
ClosedPublic

Authored by epriestley on Feb 18 2014, 5:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 24, 9:14 PM
Unknown Object (File)
Wed, Mar 20, 10:33 PM
Unknown Object (File)
Tue, Mar 19, 7:42 AM
Unknown Object (File)
Tue, Mar 19, 7:42 AM
Unknown Object (File)
Tue, Mar 19, 7:42 AM
Unknown Object (File)
Tue, Mar 19, 7:12 AM
Unknown Object (File)
Tue, Mar 19, 6:48 AM
Unknown Object (File)
Sat, Mar 16, 7:10 PM
Subscribers

Details

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

epriestley updated this revision to Unknown Object (????).Feb 18 2014, 8:07 PM
  • Fix spelling.

Whoops, I meant to accept earlier.