Wikimedia Bugzilla is closed!

To "kill" the setting, we should first retain it only for comparison against old-style, unprefixed password hashes. (I think change I0a9c9729 tries to do this.)

Then to deal with existing hashes, we could provide a maintenance script to convert both old-style and :A: hashes to :B: hashes or a future, work-hardened successor (bug 28419).

Note that when :A: and :B: hashes were introduced in r35923, an updater was provided to convert old-style hashes using a single SQL query. It was added in 1.13 and removed in 1.15 because it made the updater dependent on wiki configuration, and thus did not correctly convert unsalted hashes from within the web installer (bug 18846). Though in any case it did not add salt where none existed.

I mentioned $wgPasswordSalt a little while back in a wikitech-l email. After being prodded by Tim I did come up with an idea on how to handle the backwards compatibility of ancient hash-only password entries without $wgPasswordSalt.

The simple idea was to just try both. If it's an ancient hash try it both with and without salt. If either matches then it's ok and you continue. ((Which in the case of adding this on top of bug 28419 "continue" would mean converting that to fresh PBKDF based password storage))

(In reply to comment #2)
> The simple idea was to just try both. If it's an ancient hash try it both
> with
> and without salt. If either matches then it's ok and you continue. ((Which in
> the case of adding this on top of bug 28419 "continue" would mean converting
> that to fresh PBKDF based password storage))

Of course, a minor problem with "try both" is that if a (user_name, user_id, old-style unsalted user_password) combo were to leak, it would be possible to log in with it directly, without even having to look it up in a (rainbow) table.

Perhaps rehashing during the upgrade process could deal with that, provided that the original hash has not already leaked. Such rehashed passwords could get a special flag so it would not be necessary to apply the hack to :A: and :B: hashes. (Unless, of course, a wiki with $wgPasswordSalt = false upgrades to 1.13 or 1.14 first, only discovering bug 18846 too late...)

(In reply to comment #3)
> (In reply to comment #2)
> > The simple idea was to just try both. If it's an ancient hash try it both
> > with
> > and without salt. If either matches then it's ok and you continue. ((Which in
> > the case of adding this on top of bug 28419 "continue" would mean converting
> > that to fresh PBKDF based password storage))
> 
> Of course, a minor problem with "try both" is that if a (user_name, user_id,
> old-style unsalted user_password) combo were to leak, it would be possible to
> log in with it directly, without even having to look it up in a (rainbow)
> table.

Hmmm, yeah you're right about that.

> Perhaps rehashing during the upgrade process could deal with that, provided
> that the original hash has not already leaked. Such rehashed passwords could
> get a special flag so it would not be necessary to apply the hack to :A: and
> :B: hashes. (Unless, of course, a wiki with $wgPasswordSalt = false upgrades
> to
> 1.13 or 1.14 first, only discovering bug 18846 too late...)

Sounds like a nice idea. Actually if we did that we could eliminate all the back-compat code we have in User to deal with old password hashes and do everything in the new Password system. (It's there because the old passwords are salted with user id and the new password system is isolated so it has nothing to do with user ids.)

To implement your idea all we'd have to do is implement an :old: password type in the format ":old:{user_id}:{newhash}". The updater would find all rows with old hash-only password storage. Then hash the hash and insert it with that format into the database including the user id. Then since the user_id is now embedded into the storage data the password system no longer needs the state of what User the password belongs to in order to handle the password.

;) we get to kill two pieces of legacy compat with one change.

Regarding "try both", the unsolved (and unsolvable with migration hashes[1]) problem is that if the user reused the password on *another* wiki or site using unsalted MD5 hashing, the original hash could be taken from that other site.

(In reply to comment #4)
> To implement your idea all we'd have to do is implement an :old: password
> type
> in the format ":old:{user_id}:{newhash}". The updater would find all rows
> with
> old hash-only password storage. Then hash the hash and insert it with that
> format into the database including the user id. Then since the user_id is now
> embedded into the storage data the password system no longer needs the state
> of
> what User the password belongs to in order to handle the password.
> 
> ;) we get to kill two pieces of legacy compat with one change.

Defining a separate :old: password type (as described above, except that {newhash} would instead be the original hash) would avoid the problem described in bug 18846. Only the class for that password type would have to care whether $wgPasswordSalt is true or false - not the web installer or the User class. (And if the "try both" compromise were made, nothing would have to care.)

The migration hash mechanism should be generic enough to apply to all superseded types (including :old:, :A:, and :B:) and allow specifying additional salt that was not part of the original hash.

Wikimedia would still need some B/C code while migration hashes are being calculated and stored (would it really make sense to update every row in the table not once, but twice?), though limited to:

if ( preg_match( '/^[0-9a-f]{32}$/', $row->user_password ) ) {
    $row->user_password = ":old:{$this->mId}:{$row->user_password}";
}

rather than:

if ( preg_match( '/^[0-9a-f]{32}$/', $row->user_password ) ) {
    global $wgPasswordSalt;
    if ( $wgPasswordSalt ) {
        $row->user_password = ":B:{$this->mId}:{$row->user_password}";
    } else {
        $row->user_password = ":A:{$row->user_password}";
    }
}

[1]: "Migration hash" is the term Mozilla uses in <https://wiki.mozilla.org/WebAppSec/Secure_Coding_Guidelines#Password_Storage>.

Change 77645 had a related patch set uploaded by Parent5446:
Added password hashing API

https://gerrit.wikimedia.org/r/77645

Change 77645 merged by jenkins-bot:
Added password hashing API

https://gerrit.wikimedia.org/r/77645

$wgPasswordSalt officially deprecated. Will be removed in future release.