Page MenuHomePhabricator

Password hashes should not be stored in live User objects
Closed, ResolvedPublic

Description

Bugs like T45518 where raw User objects are accidentally exposed would be much less frightening if things like the password hashes weren't included in the live object.

Ideally we should only look up a hash when comparing against it, and shouldn't keep it in memory the rest of the time.

Details

Reference
bz45716

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 1:18 AM
bzimport set Reference to bz45716.
bzimport added a subscriber: Unknown Object (MLST).
brion created this task.Mar 4 2013, 8:19 PM
Krinkle updated the task description. (Show Details)Dec 10 2014, 2:00 PM
Krinkle added a project: Technical-Debt.
Krinkle set Security to None.
Krinkle removed a subscriber: Unknown Object (MLST).
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 1 2015, 4:15 AM
Tgr added a subscriber: Tgr.Sep 1 2015, 4:15 AM

T91699 removes passsword handling from User.

Krenair updated the task description. (Show Details)Sep 1 2015, 4:45 AM
Krenair added a subscriber: Krenair.

Change 236044 had a related patch set uploaded (by Anomie):
WIP: User: Mostly remove password handling

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

Change 236044 merged by jenkins-bot:
User: Mostly remove password handling

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

Tgr added a comment.Oct 14 2015, 1:09 AM

Almost completely removed in 236044, except for the case when User::setPassword() is called when there is no DB record for the user yet. In that case, the password (in cleartext - it would probably make sense to hash it immediately) is temporarily stored in the object until the next User::addToDatabase() call. Apart from the cleartext part, I don't think you can get better than that.

Change 246445 had a related patch set uploaded (by Gergő Tisza):
Hash the password early

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

Change 250020 had a related patch set uploaded (by Anomie):
Disallow User::setPassword() on users not in database

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

Change 246445 abandoned by Gergő Tisza:
Hash the password early

Reason:
That's a lot cleaner indeed, thanks!

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

Change 250020 merged by jenkins-bot:
Disallow User::setPassword() on users not in database

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

Tgr closed this task as Resolved.Oct 31 2015, 1:41 AM
Tgr claimed this task.

Change 250025 had a related patch set uploaded (by Gergő Tisza):
Avoid calling User::setPassword() on users not in database

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

Change 250025 merged by Gergő Tisza:
Avoid calling User::setPassword() on users not in database

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