search

DOMjudge / domjudge

DOMjudge programming contest jury system
https://www.domjudge.org
GNU General Public License v2.0
720 stars 254 forks source link

Current password hashing is not secure #237

Closed bertptrs closed 7 years ago

bertptrs commented 7 years ago

The current hashing technique used for hashing password is md5($username . '#' . $password). While this does prevent users with the same password showing in the database, it is not a secure password hashing scheme as it can be brute forced quite easily.

Would it be possible to implement a secure hashing scheme, like PHP's own password_hash and password_verify? For backwards compatibility, a package such as password_compat can be used. That way the PHP requirement does not have to be bumped.

thijskh commented 7 years ago

Agreed that this is the case. We shied away from changing it earlier because it would mean that when upgrading DOMjudge everyone's password would need to be reset (or a complicated migration scheme implemented). We could consider to accept the breakage and just change it in the next major release.

bertptrs commented 7 years ago

If I were to create a patch and send a PR, would you consider it?

thijskh commented 7 years ago

Yes!

eldering commented 7 years ago

I would suggest to create dj_* wrappers (see lib/lib.wrappers.php) around the functions password_verify and password_needs_rehash to detect our old-style md5($username . '#' . $password) hashes and be able to update these when a user logs in.

eldering commented 7 years ago

@bertptrs: I see you already pushed a PR, great! Do you think implementing my suggestion would be easy/sensible?