Opened 3 weeks ago
Closed 2 weeks ago
#37078 closed Cleanup/optimization (fixed)
Change default algorithm of salted_hmac() from SHA-1 to SHA-256
| Reported by: | Denny Biasiolli | Owned by: | Denny Biasiolli |
|---|---|---|---|
| Component: | Utilities | Version: | dev |
| Severity: | Normal | Keywords: | security, crypto |
| Cc: | Denny Biasiolli | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The salted_hmac() function (crypto.py:19) defaults to algorithm="sha1". While HMAC-SHA1 is not cryptographically broken (HMAC construction is resistant to collision attacks), SHA-1 is deprecated by NIST and modern security standards recommend SHA-256 or stronger for all new applications.
All security-sensitive callers within Django already override this default — Signer uses sha256 (signing.py:193), PasswordResetTokenGenerator passes sha256 explicitly, and session auth hashes use SHA-256. However, any third-party code or custom application calling salted_hmac() without specifying an algorithm will silently use SHA-1.
## Steps to Reproduce
- In any Django project, call:
`python from django.utils.crypto import salted_hmac mac = salted_hmac("my_salt", "my_value") print(mac.digest_size) # 20 bytes = SHA-1` - Observe the HMAC uses SHA-1 without any explicit algorithm selection
## Expected Behavior
salted_hmac() should default to "sha256" to match modern cryptographic best practices and align with Django's own internal usage.
## Actual Behavior
salted_hmac() defaults to algorithm="sha1" (line 19 of crypto.py).
Change History (11)
comment:1 by , 3 weeks ago
| Has patch: | set |
|---|
comment:2 by , 3 weeks ago
comment:3 by , 3 weeks ago
A deprecation might make sense, but just to check my understanding -- Tim, does it make a difference that base64_hmac() isn't documented? (And that all uses in Django have already migrated?)
comment:4 by , 3 weeks ago
| Has patch: | unset |
|---|
There's at least one usage of base64_hmac() in healthchecks. Even if undocumented, I wouldn't modify security-related functionality lightly.
comment:5 by , 3 weeks ago
| Summary: | salted_hmac() defaults to SHA-1 algorithm despite SHA-256 being preferred everywhere else → Change default algorithm of salted_hmac() from SHA-1 to SHA-256 |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Version: | → dev |
Makes good sense -- I agree we should go through a deprecation here.
comment:6 by , 3 weeks ago
I'd be happy to help, but do you have suggestions about the steps I need to take to fix this?
My PR with the fix (without the deprecation) was here: https://github.com/django/django/pull/21190
comment:7 by , 3 weeks ago
Sure thing, and thanks for the offer.
- Set yourself in the owner field here on this ticket.
- Check the deprecation guide for places to add documentation.
- I assume you will need to do something like change the default value for the argument to a
NOT_PROVIDEDsentinel, check for it, and issue the warning and fall back to SHA-1.
comment:8 by , 3 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:9 by , 3 weeks ago
| Has patch: | set |
|---|
comment:10 by , 2 weeks ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Previous work was done in #27468. We cannot just change default value of the parameter due to backward compatibility. The change would have to go through a deprecation.