1356795 - Kuma hashes passwords using the SHA-256 hash function.

Status: RESOLVED FIXED

(developer.mozilla.org :: Security, enhancement)

Description

[Ed]

Dear Mozilla,

Summary
=========

Kuma hashes passwords using the SHA-256 hash function, which is not a secure method of password storage. SHA-256 is too fast and you should therefore use a slow algorithm as suggested in the "How can this be fixed?" section.

Why does this vulnerability exist?
==============================

Kuma has a Sha256Hasher() class which generates salted SHA-256 hash digests. The problem here is that you have chosen SHA-256, which shouldn't be used to store passwords. Even if you do add a salt to the password, this form of password storage remains insecure.

The issue is located in "kuma/users/backends.py". SHA-256 is implemented as follows:

class Sha256Hasher(BasePasswordHasher):
    ""
"
SHA - 256 password hasher.
""
"
algorithm = 'sha256'
digest = hashlib.sha256

def encode(self, password, salt):
    assert password
assert salt and '$'
not in salt
hash = self.digest(salt + password).hexdigest()
return "%s$%s$%s" % (self.algorithm, salt, hash)

def verify(self, password, encoded):
    algorithm, salt, hash = encoded.split('$', 2)
assert algorithm == self.algorithm
encoded_2 = self.encode(password, salt)
return constant_time_compare(encoded, encoded_2)

def safe_summary(self, encoded):
    algorithm, salt, hash = encoded.split('$', 2)
assert algorithm == self.algorithm
return collections.OrderedDict([
    (ugettext('algorithm'), algorithm), (ugettext('salt'), mask_hash(salt, show = 2)), (ugettext('hash'), mask_hash(hash)),
])

Link: https://github.com/mozilla/kuma/blob/master/kuma/users/backends.py#L9-L35

What is SHA-256?
================

If you would like to learn more about the SHA-2 hash function, I wrote a chapter on it in Laurens Van Houtven's "Crypto 101" book: https://github.com/crypto101/book/blob/master/Crypto101.org#sha-2 

Comment 1

[Ed]

That is weird. The end of my report was cut off.

Why shouldn't SHA-256 be used for hashing passwords?
=====================================================

In order to securely hash passwords one must use a slow hashing function, which SHA-256 isn't. By using a fast hash function like SHA-256 you make password cracking easier.

How can this be fixed?
=======================

For better security, use a slow algorithm such as PBKDF2 or bcrypt.

Comment 2

[Greg G]

Thanks Ed! 

Assigning the issue sec-moderate. This report isn't eligible for a bug bounty, since MDN isn't a valid bug bounty domain: https://www.mozilla.org/en-US/security/bug-bounty/faq-webapp/#eligible-bugs

We should definitely use cpu or memory-hard KDF for user passwords like PDKDF2, bcrypt or scrypt. 

Probably easiest to add the included django PBKDF2 and bcrypt hashers https://docs.djangoproject.com/en/1.10/topics/auth/passwords/#included-hashers above 'kuma.users.backends.Sha256Hasher' in the PASSWORD_HASHERS tuple in settings.py

+jwhitlock

Comment 3

[John Whitlock [:jwhitlock]]

I agree w/ :g-k, I'll add one or more of the included hashers into PASSWORD_HASHERS. We're on Django 1.8, so our list is different:

https://docs.djangoproject.com/en/1.8/ref/settings/#std:setting-PASSWORD_HASHERS

I'll do this in phases:

1. Update the hashers list to the Django 1.8 default, w/ the Sha256Hasher included. Ship
2. When all passwords are updated, drop Sha256Hasher
3. Get the new defaults as we upgrade Django

I'll close after step #1 ships.

Comment 4

[John Whitlock [:jwhitlock]]

New hashers merged, deployed to staging and production.

We don't use password login in production, but we do set random passwords in order to reuse Django's account recovery workflow.  The random passwords are not used in account recovery, but instead a token is emailed to the user, which expires after 3 days.  In three days, I'll clear these random passwords, and then close the bug as fixed.

Comment 5

[Ed]

Just a quick question, why is the following stated in users/backends.py?

> SHA-256 password hasher. Good option for 2010.

SHA-256 was never ideal for password hashing. It is a digest function.

Comment 6

[John Whitlock [:jwhitlock]]

The Sha256Hasher was added to the code in November 2010:

https://github.com/mozilla/kuma/commit/7213950d4bfa32928fa5c21d543a2ed96c4b914e

In Django 1.2, there was built-in support for three password hashing functions - MD5, SHA1, and crypt:

https://github.com/django/django/blob/973bf6f4856ec3769398caa643ccc61dff4b19f4/django/contrib/auth/models.py#L16-L33

It appears the Django developers were supporting Python 2.4, and those were the strongest hashing algorithms available. It required patching the framework to improve the situation:

http://fredericiana.com/2010/10/12/adding-support-for-stronger-password-hashes-to-django/

So, in 2010, using SHA-256 was an improvement over the default of MD5, SHA1, or crypt.

Comment 7

[Ed]

Fair enough, but it is important to note that neither MD5, SHA1 nor SHA2 are password hashing functions. Either way, I am happy with the quick responses and keep up the good work. :)

Comment 8

[John Whitlock [:jwhitlock]]

The Sha256Hasher is now unused in in production and staging, so closing as fixed and removing security flag.  Follow on actions, which will probably be tied to this bug:

1) Regenerate sample database with PDKDF2-based passwords
2) Drop Sha256Hasher and use the Django default PASSWORD_HASHERS

Comment 9

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

Not an eligible site for the bounty, however, we'll hall of fame this.

Comment 10

[[github robot]]

Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/c58e7d4dc28e3c37d8c7c144ca12f4e694c32fb1
bug 1356795: Use default PASSWORD_HASHERS

Use Django 1.8's default PASSWORD_HASHERS, and drop support for
custom Sha256Hasher.

https://github.com/mozilla/kuma/commit/25cb4795b31ea872a2f3b37f5796ebaedd86f5ad
Merge pull request #4268 from jwhitlock/drop-sha256-hasher-1356795

bug 1356795: Use default PASSWORD_HASHERS

Comment 11

[[github robot]]

Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/775750af470a5dea17c3ce75b08f37159c2468ef
bug 1356795: Remove unused Sha256Hasher

https://github.com/mozilla/kuma/commit/fcf7a0e10e8e0612133f764530e5b470f9943a25
Merge pull request #4296 from jwhitlock/drop_sha256_1356795

bug 1356795: Remove unused Sha256Hasher

Comment 12

[April King [:April]]

Ed, how would you like to be credited on our hall of fame for this? If you want us to link somewhere, we can do that too. Thanks!

Comment 13

[Ed]

Hi April,

"EdOverflow" and https://edoverflow.com should be the easiest way for people to find me.

Best regards,
Ed

Comment 14

[April King [:April]]

Unfortunately we've already pushed our updates for Q3 2017, but I've already updated it locally and will make sure it gets pushed during Q4's update. Sorry, and thanks for your patience!