Drop passlib, switch to argon2id hashes #6657
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Unfortunately passlib appears unmaintained with no commits in the past 2 years and literal spam tickets in their issue tracker. It brings in ~20k lines of code (per tokei), but we only need the argon2 functionality that is in argon2-cffi already. Using argon2-cffi directly is roughly the same, with just a few renames (e.g. rounds → time_cost) and different method signatures (verify() throws an exception instead of returning false). We explicitly specify a type=argon2i, in the next commit we'll switch over to argon2id. Fixes #6631.
argon2id is the current recommended hashing algorithm, as it is able to take advantage of both the argon2d and argon2i algorithms. It's the default in argon2-cffi as well. argon2-cffi automatically handles verifying argon2i passphrases, this is verified in the new `test_passphrase_argon2i_migration` test case. We use the `check_needs_rehash()` function to see if it needs an update, and rehash if so. valid_password() has been slight adjusted that we return False as soon as we know the passphrase is invalid rather than needing to guard the migration checks against is_valid. The test_valid_login_calls_argon2 test is dropped because it broke with the new migration checking and IMO provides little value. Fixes #6655.
eaon
approved these changes
Oct 19, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
Ready for review
Description of Changes
The individual commit messages have more details on specifics.
Fixes #6631.
Fixes #6655.
Testing
HEAD~2since develop will have moved forward since I wrote this). Log in asjournalist. Look in the database (e.g.SELECT username, passphrase_hash FROM journalists) and verify the hash starts with$argon2i$....journalistagain. Look in the database and verify the hash now starts with$argon2id$...$argon2id$...Note that there's a 50% chance the generated journalist uses the even more legacy pw_hash/pw_salt scrypt mechanism if you notice that passphrase_hash is sometimes null.
Deployment
Any special considerations for deployment?
Checklist
make lint) and tests (make test) pass in the development container