Opened 2 hours ago
Last modified 113 minutes ago
#37375 new Bug
`ScryptPasswordHasher` cannot verify hashes with non-default derived key lengths — at Initial Version
| Reported by: | Dominic Roy | Owned by: | |
|---|---|---|---|
| Component: | contrib.auth | Version: | 5.2 |
| Severity: | Normal | Keywords: | scrypt password hasher dklen |
| Cc: | Dominic Roy | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
ScryptPasswordHasher currently hardcodes dklen=64 when calling
hashlib.scrypt().
This means Django cannot verify an otherwise valid scrypt password hash
generated by another implementation if that implementation used a different
derived key length.
This came up while working on password hash compatibility in authentik (https://goauthentik.io), where
we need to be able to verify scrypt hashes imported from other systems.
For example, these two hashes use the same password, salt, N, r, and p values,
but different derived key lengths:
scrypt$1024$salt$8$5$Xo0CV8Rk/C+B2LSvQ/MgyKJmMtW6yik2PCmVEtsZyMu01U+2KSQrCJnGfrVo4+hqUkv7Oq+AyCG9eaNdpJSn5w== scrypt$1024$salt$8$5$Xo0CV8Rk/C+B2LSvQ/MgyKJmMtW6yik2PCmVEtsZyMu01U+2KSQrCJnGfrVo4+hqUkv7Oq+AyCE=
The first uses dklen=64 and verifies with Django. The second uses
dklen=56 and does not.
Django's encoded scrypt format does not contain an explicit dklen field,
but the value can be recovered from the length of the Base64-decoded digest.
verify() could as a result derive the original key length from the stored
hash and pass it to hashlib.scrypt().
Django could continue generating new hashes with dklen=64. Hashes using a
different derived key length could also be considered for upgrade by
must_update(), so they are re-encoded using Django's preferred parameters
after a successful login.
This would make importing existing scrypt password hashes from other
implementations possible without requiring otherwise valid passwords to be
reset.
If this qualifies for backporting, it would also be useful to have the fix in
Django 5.2 LTS.
I'm happy to submit a patch if this approach is accepted!