Opened 11 years ago

Closed 10 years ago

#19043 closed New feature (duplicate)

Mutable Password Hash Strength

Reported by: Jason Buckner Owned by: nobody
Component: contrib.auth Version: dev
Severity: Normal Keywords: auth, bcrypt, pbkdf2
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Django 1.4 introduced automatic conversion of passwords from one hashing algorithm to another. Some algorithms, notably Bcrypt and PBKDF2 can have different work factors and iterations. Currently there's no way to change the difficulty of the particular algorithm without subclassing it and writing your own hasher. Even then, it won't re-hash the password with the new difficulty.

This patch adds two Django settings, BCRYPT_ROUNDS (as used in django-bcrypt) and PBKDF2_ITERATIONS, depending on your preferred algorithm. If you change these values, the next time check_password() is called when a user logs in, it will re-hash their password with the new difficulty.

We did this by introducing an is_current() method in the django.contrib.auth.hashers.BasePasswordHasher that returns whether or not the hash matches the desired difficulty. For instance, in the BCryptPasswordHasher, we compare the safe_summary['work factor'] against the BCRYPT_ROUNDS setting and if they differ, re-hash the password.

There are also tests included.

Attachments (1)

password_hashing.diff (5.4 KB ) - added by Jason Buckner 11 years ago.

Download all attachments as: .zip

Change History (8)

by Jason Buckner, 11 years ago

Attachment: password_hashing.diff added

comment:1 by Jason Buckner, 11 years ago

I have also opened a pull request for this ticket.

comment:2 by Preston Holmes, 11 years ago

The bar is pretty high for introducing new settings - I'm not sure this passes that bar.

The subclassing of the hashers is very straightforward and even well documented:

https://docs.djangoproject.com/en/dev/topics/auth/#increasing-the-work-factor

You don't have to actually write your own hasher when subclassing in this case, just change a few attributes.

I do think an argument could be made for changing the must_update flag to check if the same hasher is being used, not just the same algorithm. That would address the second part of your changes.

Another reason not to introduce these settings - is it makes our sane security defaults a little too easy to muck with. The project has to tread a line between defaults that are robust and not easily circumvented by genuine accident, while still allowing those who know what they are doing to make the changes they need to. Subclassing seems to strike that balance better than a pair of settings.

comment:3 by Tomek Paczkowski, 11 years ago

If you want to force rehashing, change the algorithm name when subclassing it, say from pbkdf2_sha256 to pbkdf2_sha256_100000.

Last edited 11 years ago by Tomek Paczkowski (previous) (diff)

comment:4 by Aymeric Augustin, 11 years ago

Triage Stage: UnreviewedAccepted

If I understand correctly, the technique described in the documentation to increase the work factor will only work for new passwords; existing passwords won't be upgraded. I'm accepting the ticket on this basis.

However, I'd prefer an implementation based on subclassing, for the reasons described by ptone, and also because it's generally more flexible.

The solution proposed by oinopion looks more like a workaround with the current code than like something we'd like to document.

comment:5 by Aymeric Augustin, 11 years ago

Really, this doesn't pass the threshold for introducing new settings.

Since GitHub doesn't offer any options besides open/close, I've closed the pull request; you're welcome to reopen it with a patch that:

  • doesn't introduce new settings
  • updates the documentation on increasing the work factor
  • changes the implementation of must_update to take into account the number of rounds

comment:6 by Tim Graham, 11 years ago

Needs documentation: set
Patch needs improvement: set

comment:7 by Tim Graham, 10 years ago

Resolution: duplicate
Status: newclosed

Existing passwords should be updated as of #21535 (Django 1.6.1).

Note: See TracTickets for help on using tickets.
Back to Top