Opened 12 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)
Change History (8)
by , 12 years ago
Attachment: | password_hashing.diff added |
---|
comment:1 by , 12 years ago
comment:2 by , 12 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 , 12 years ago
If you want to force rehashing, change the algorithm name when subclassing it, say from pbkdf2_sha256
to pbkdf2_sha256_100000
.
comment:4 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
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 , 12 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 , 11 years ago
Needs documentation: | set |
---|---|
Patch needs improvement: | set |
comment:7 by , 10 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Existing passwords should be updated as of #21535 (Django 1.6.1).
I have also opened a pull request for this ticket.