upgrade to argon password hasher #3410
Conversation
|
Automated message from Dropbox CLA bot @sinwar, it looks like you've already signed the Dropbox CLA. Thanks! |
zproject/settings.py
Outdated
| @@ -489,6 +489,7 @@ def get_secret(key): | |||
| # Use fast password hashing for creating testing users when not | |||
| # PRODUCTION. Saves a bunch of time. | |||
| PASSWORD_HASHERS = ( | |||
| 'django.contrib.auth.hashers.Argon2PasswordHasher', | |||
There was a problem hiding this comment.
This config is only used in development! This will just make the tests run slow.
zproject/settings.py
Outdated
| @@ -490,12 +490,23 @@ def get_secret(key): | |||
| # PRODUCTION. Saves a bunch of time. | |||
| PASSWORD_HASHERS = ( | |||
| 'django.contrib.auth.hashers.SHA1PasswordHasher', | |||
| 'django.contrib.auth.hashers.PBKDF2PasswordHasher' | |||
| 'django.contrib.auth.hashers.PBKDF2PasswordHasher', | |||
| 'django.contrib.auth.hashers.Argon2PasswordHasher' | |||
There was a problem hiding this comment.
I included argon here for checking because it is preferred in production.
sinwar
force-pushed
the
pswrdhash
branch
8 times, most recently
from
e71053c
to
3f232d1
Compare
zproject/settings.py
Outdated
| @@ -496,6 +496,11 @@ def get_secret(key): | |||
| # can query using ./manage.py print_initial_password | |||
| INITIAL_PASSWORD_SALT = get_secret("initial_password_salt") | |||
|
|
|||
| # Use best password hashing algorithm argon2 for PRODUCTION | |||
| if PRODUCTION: | |||
There was a problem hiding this comment.
This should probably be an else.
zproject/settings.py
Outdated
| # Use best password hashing algorithm argon2 for PRODUCTION | ||
| if PRODUCTION: | ||
| PASSWORD_HASHERS = ('django.contrib.auth.hashers.Argon2PasswordHasher', | ||
| 'django.contrib.auth.hashers.PBKDF2PasswordHasher') |
There was a problem hiding this comment.
Maybe add a comment that Zulip was originally on PBKDF2 so we need it for compatibility, but we're now defaulting to Argon2?
|
@sinwar how carefully have you tested this? Ideally, one would enable password auth locally on master (zproject/dev_settings.py), register a new account with password, and then apply this change, and verify: |
|
@timabbott I verified for all the test cases. |
zproject/settings.py
Outdated
| @@ -495,6 +495,11 @@ def get_secret(key): | |||
| # Also we auto-generate passwords for the default users which you | |||
| # can query using ./manage.py print_initial_password | |||
| INITIAL_PASSWORD_SALT = get_secret("initial_password_salt") | |||
| else: | |||
| # Use best password hashing algorithm argon2 for PRODUCTION | |||
There was a problem hiding this comment.
If the algorithm is called "Argon2" then - for ease of grepping - we should call it that in this comment as well:
For production, use the best password hashing algorithm: Argon2
There was a problem hiding this comment.
ok @brainwane changing it
There was a problem hiding this comment.
@sinwar, have you updated this?
|
So I have verified this and it works for all the cases mentioned above. The only observation I want to make is that for the tests to pass in the development environment, I had to do |
I see that we are not planning to use this hasher in our tests. |
|
@umairwaheed this will slower down our test so we are not planning to use in development. |
|
Cool. Lgtm! |
|
the production tests were not working yesterday. I think now we should run it again. It seems to pass all test. |
|
Merged, after adding a PROVISION_VERSION update. Thanks @sinwar! |
This changed in commit 483a351 (zulip#3410). Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes #3362