Stored passwords with a stronger hash algorithm

Bug #843568 reported by François Marier
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Medium
Hugh Davenport

Bug Description

MD5 is broken, we should switch to something better.

Ideally, we should use PHP 5.3.2's crypt() function (http://nz.php.net/manual/en/function.crypt.php) with the CRYPT_BLOWFISH algorithm. Not sure what cost parameter we should use, but ideally a large number (we should do tests here).

Note that bulk creation of users will be slowed down by using a slow hash. So perhaps in that case, we should use SHA256. Which means that Mahara needs to recognize 3 hash formats at least:

- the existing MD5-hashed passwords
- the new Blowfish ones
- the new SHA256 ones

Tags: passwords
Changed in mahara:
status: New → Triaged
importance: Undecided → Medium
milestone: none → 1.5.0
Revision history for this message
Ruslan Kabalin (rkabalin) wrote :

Using both Blowfish and SHA256 is not ideal as some users will have stronger passwords than others. We probably may use Blowfish as the main method. With regard of bulk user creation, we indeed can use SHA256 for speed, but upon the login of such user, after SHA256 verification, password hash will be replaced with generated Blowfish one. We might force conversion of existing MD5 passwords to Blowfish as well.

Revision history for this message
François Marier (fmarier) wrote :

I like Ruslan's hash upgrade idea. We should do that.

That way Mahara will slowly migrate to better hashes for all of its active users as they login.

Revision history for this message
François Marier (fmarier) wrote :

The Mozilla Secure Coding Guidelines suggest an interesting migration procedure:

  https://wiki.mozilla.org/WebAppSec/Secure_Coding_Guidelines#Password_Storage

Revision history for this message
Dan Poltawski (dan-poltawski) wrote :

Just thought i'd link to this article I read a while ago suggesting bcrypt:
http://codahale.com/how-to-safely-store-a-password/ and a sample implementation from Marco.org: https://gist.github.com/1053158

Revision history for this message
Hugh Davenport (hugh-davenport) wrote :
Changed in mahara:
assignee: nobody → Hugh Davenport (hugh-catalyst)
status: Triaged → In Progress
Revision history for this message
Hugh Davenport (hugh-davenport) wrote :
Revision history for this message
Hugh Davenport (hugh-davenport) wrote :

above review was abandoned. new review(s) at

remote: https://reviews.mahara.org/854
remote: https://reviews.mahara.org/855

tags: added: passwords
removed: password
Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/854
Committed: http://gitorious.org/mahara/mahara/commit/75ff6aae2212f9b5988faad46a3e3c6cc6540e3c
Submitter: Francois Marier (<email address hidden>)
Branch: master

commit 75ff6aae2212f9b5988faad46a3e3c6cc6540e3c
Author: Hugh Davenport <email address hidden>
Date: Mon Nov 14 17:58:05 2011 +1300

    Change internal password algorithm to bcrypt

    This changes the internal authentication plugin to use
    bcrypt instead of sha1.

    It also introduces a fast hash (SHA512) for bulk operations.
    This hash is updated on user login to the bcrypt hash.

    Bug #843568

    See https://wiki.mahara.org/index.php/Developer_Area/Specifications_in_Development/Improve_Password_Storage

    Change-Id: Ibf2f71bb5b5a5279dbc16ccda781ad99e81c59b8
    Signed-off-by: Hugh Davenport <email address hidden>

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/855
Committed: http://gitorious.org/mahara/mahara/commit/5a714bf73796693bf71ffa75fcb89800dc3c0ed3
Submitter: Francois Marier (<email address hidden>)
Branch: master

commit 5a714bf73796693bf71ffa75fcb89800dc3c0ed3
Author: Hugh Davenport <email address hidden>
Date: Tue Nov 15 12:52:43 2011 +1300

    Add a sitewide salt that isn't in the db

    This salt is used to add an extra layer of salting that
    isn't visible from the database. This requires attackers
    to obtain both the database, and the config.php file to
    get the true salt value that is passed to crypt.

    Bug #843568

    See http://docs.moodle.org/20/en/Password_salting

    Change-Id: Iaa575a4724e387104f9e436c07b336ef8c7ebef5
    Signed-off-by: Hugh Davenport <email address hidden>
    Signed-off-by: Francois Marier <email address hidden>

Changed in mahara:
status: In Progress → Fix Committed
Melissa Draper (melissa)
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.