Opened 3 weeks ago

Closed 10 days ago

Last modified 9 days ago

#37375 closed Bug (wontfix)

ScryptPasswordHasher cannot verify hashes with derived key lengths greater than 64 bytes

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 (last modified by Dominic Roy)

ScryptPasswordHasher currently hardcodes dklen=64 when calling
hashlib.scrypt().

As a result, Django cannot verify otherwise valid scrypt password hashes
generated by another implementation when that implementation uses a larger
derived key length.

This came up while working on password hash compatibility in authentik
(​https://goauthentik.io), where imported scrypt hashes may come from systems
using different dklen values.

For example, using the same password, salt, N, r, and p values:

scrypt$16384$django-scrypt-example$8$1$D5reUgzHkb7jsUr+tu+tLDdM7Zx9ogzquC0AJYM1U6irspdgjDMDL7GforSHakP8eTjb3aDsBw+VLv+odzzPpA==
scrypt$16384$django-scrypt-example$8$1$D5reUgzHkb7jsUr+tu+tLDdM7Zx9ogzquC0AJYM1U6irspdgjDMDL7GforSHakP8eTjb3aDsBw+VLv+odzzPpDHtdiQLBYg3cABNaUmj/LOy7C0gbqe9Tt3TA/GAuuaql9RhoeGh57L7aDdxGhddkCbrwxLVvzJxa6YRW1XqdCs=

The first uses dklen=64 and can be verified by Django. The second uses
dklen=128 and currently cannot, even though all other scrypt parameters are
the same.

Django should probably keep 64 bytes as the minimum accepted derived key
length rather than accepting arbitrary shorter hashes. However, hashes with a
larger dklen, for example 128 bytes, currently fail verification because
verify() always regenerates a 64-byte result.

The derived key length can be recovered from the length of the Base64-decoded
digest. verify() could use that value when it is at least 64, while Django
continues generating new hashes with dklen=64.

If appropriate under Django's backport policy, it would also be useful to
include this in Django 5.2 LTS.

I'm happy to submit a patch with tests if this approach is accepted.

Change History (7)

comment:1 by Dominic Roy, 3 weeks ago

Description: modified (diff)

comment:2 by Dominic Roy, 3 weeks ago

Summary: `ScryptPasswordHasher` cannot verify hashes with non-default derived key lengths → ScryptPasswordHasher cannot verify hashes with derived key lengths greater than 64 bytes

comment:3 by Dominic Roy, 3 weeks ago

Description: modified (diff)

comment:4 by David Smith, 10 days ago

As you are using hashes that were not generated by Django, I wonder if a subclass of ScryptPasswordHasher in your project would be the way to go here?

If appropriate under Django's backport policy, it would also be useful to include this in Django 5.2 LTS.

This would be a new feature so would not qualify for a backport. Further, 5.2 is also now in extended support and so it only receives backports for security and data loss issues.

comment:5 by Dominic Roy, 10 days ago

Hey David, thanks for taking a look!

A subclass would work, but I think this is worth handling upstream as well. For example, authentik supports importing password hashes from other systems, and a user migrating from one that uses a 128 byte scrypt derived key would need a custom hasher just to verify an otherwise valid hash.

Since the derived key length can already be inferred from the stored digest, Django could support these hashes without changing how it generates new ones or lowering its existing 64-byte minimum.

Version 0, edited 10 days ago by Dominic Roy (next)

comment:6 by Sarah Boyce, 10 days ago

Resolution: → wontfix
Status: new → closed

ScryptPasswordHasher generates hashes with dklen=64 and verifies hashes in that format. Inferring dklen from the stored digest would change the hasher’s design assumption that it verifies the format it generates. It would also require an upper bound on the accepted length to avoid excessive resource use.

I think a custom password hasher is a better fit for these imported hashes, since it can define the derived key lengths it supports.

comment:7 by Dominic Roy, 9 days ago

Perfect, thanks!

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