Move utils.crypto.salted_hmac() from SHA1 toward SHA256

[Tim Graham]

The salted_hmac() function uses SHA-1 in two places, for key derivation and as the hash function of the HMAC algorithm. HMAC-SHA1 is ​still considered secure, but it would be nice to move toward SHA256. Some work is needed to ensure backward compatibility (like we've done with password storage).

Ramifications:

• Changing the algorithm will make all the old HMACs invalid. This means that when users upgrade, old sessions, signed cookies, and session authentication hashes will be rejected as invalid and replaced (hence the need for a backwards-compatibility layer of some sort).
• SHA-256 is slower than SHA-1, though hopefully no Django servers are bottlenecked on hashing speed.

Thanks Predrag Gruevski (obi1kenobi on Github) for the report.

[Srinivas Reddy Thatiparthy]

A WIP PR has been raised here - ​https://github.com/django/django/pull/8773

[Carlton Gibson]

There are a number of review comments (from multiple reviewers) on the PR that need to be addressed in order to move this patch forwards.

[Claude Paroz]

In this ​PR, I did the first step of allowing different hash algorithms in the salted_hmac() function, and tried to apply the new capability to the contrib.messages CookieStorage. We can then work on other salted_hmac() usages if that approach is validated.

[Collin Anderson]

Maybe there could be a SIGNING_HASHERS similar to PASSWORD_HASHERS? Add a "sha256:" prefix to new signatures, and a signature without an algorithm should be assumed to be SHA1, similar to how a password without an algorithm prefix is assumed to be md5.

That should provide a backwards-compatible transition path.

The default SIGNING_HASHERS could be something like ['django.core.signing.hashers.SHA256', 'django.core.signing.hashers.SHA1'] (or maybe just ['sha256', 'sha1'])

You could still override by passing in the algoritm name in the function signature.

[Claude Paroz]

Replying to Collin Anderson:

...Add a "sha256:" prefix to new signatures, and a signature without an algorithm should be assumed to be SHA1, similar to how a password without an algorithm prefix is assumed to be md5.

That's the approach I choose for the CookieStorage part of my patch, I also think it's a good upgrade path.

The default SIGNING_HASHERS could be something like ['django.core.signing.hashers.SHA256', 'django.core.signing.hashers.SHA1'] (or maybe just ['sha256', 'sha1'])

Your idea about SIGNING_HASHERS could be handled separately. I'm not yet convinced about use cases for that, but let's keep it in mind.

[Mariusz Felisiak <felisiak.mariusz@…>]

In b5a62bd1:

Refs #27468 -- Added explicit tests for django.utils.crypto.salted_hmac()

[Mariusz Felisiak <felisiak.mariusz@…>]

In 50cf183d:

Refs #27468 -- Added algorithm parameter to django.utils.crypto.salted_hmac().

[Claude Paroz]

Should we keep this only ticket to track all Django usage of salted_hmac or should we create tickets for each one:

• User session auth hash (django.contrib.auth.base_user)
• Password rest tokens (django.contrib.auth.tokens) ​PR
• Cookie storage of messages (django.contrib.messages.storage.cookie) (After #27604 is resolved, will use Signer)
• Session hashes (django.contrib.sessions.backends.base)
• Signing utility (django.core.signing)

I think that depending on the usage, different backwards-compatibility strategies may be involved.

[Simon Charette]

I'd definitely want to see django.core.signing default to SHA256, happy to submit a ticket and patch if it's not already on your plate Claude?

[Claude Paroz]

I have ​this PR for the rest password token part, and part of ​this PR (which needs a rebase) which tries to address the Signer part. Feel free to continue/improve my work.

[Mariusz Felisiak <felisiak.mariusz@…>]

In da4923e:

Refs #27468 -- Made PasswordResetTokenGenerator use SHA-256 algorithm.

[Claude Paroz]

Next step is ​this PR (include algorithm in Signer signature).
.

[Claude Paroz]

We have now two proposed strategies for handling an algorithm change in django.core.signing.Signer:

• the ​first one was to include the signer algorithm in the signature, so when unsigning, we use that algorithm taken from the signature.

That approach was not approved by @ubernostrum, so base on his comments, an alternative is:

• ​set the algorithm in the Signer class (with an optional legacy_algorithm value to handle the transition periods), so unsigning is forced using that algorithm, which avoids trusting a possibly mangled algorithm in the signature itself.

[Mariusz Felisiak]

I prefer the second approach (​PR).

[Mariusz Felisiak <felisiak.mariusz@…>]

In 71c4fb7b:

Refs #27468 -- Changed default Signer algorithm to SHA-256.

[Mariusz Felisiak]

Claude, are you going to work on the last point, i.e. user session auth hash? I would really like to close it in Django 3.1. I can prepare it if you're busy, just let me know. Thanks.

[Claude Paroz]

WIP patch for user sessions

[Claude Paroz]

Mariusz, I attached a "very" WIP patch (no tests, no docs) I was working on some days ago. It might not even be the good approach, but feel free to get some inspiration if it has any merit. I'll happily review your version.

[Mariusz Felisiak]

Thanks! I don't think we need to persist legacy sessions in login(), what's more flushing them should help not using them anymore.

​PR

[GitHub <noreply@…>]

In 54646a42:

Refs #27468 -- Made user sessions use SHA-256 algorithm.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 1d6fdca:

Refs #27468 -- Added tests and release notes for signing.dumps()/loads() changes.

Follow up to 71c4fb7beb8e3293243140e4bd74e53989196440.

[Mariusz Felisiak <felisiak.mariusz@…>]

In b84b192:

[3.1.x] Refs #27468 -- Added tests and release notes for signing.dumps()/loads() changes.

Follow up to 71c4fb7beb8e3293243140e4bd74e53989196440.

Backport of 1d6fdca557e674b9a789b51caadca8985e588492 from master

[GitHub <noreply@…>]

In 7c929fcf:

Refs #27468 -- Fixed TestSigner.test_dumps_loads_legacy_signature.

Added in 1d6fdca557e674b9a789b51caadca8985e588492.

[Mariusz Felisiak <felisiak.mariusz@…>]

In c67314ff:

[3.1.x] Refs #27468 -- Fixed TestSigner.test_dumps_loads_legacy_signature.

Added in 1d6fdca557e674b9a789b51caadca8985e588492.
Backport of 7c929fcf7c8913d0bd78a8400eafe866ceed9aa6 from master

[Mariusz Felisiak <felisiak.mariusz@…>]

In 66b4046d:

Refs #27468 -- Removed support for the pre-Django 3.1 password reset tokens.

Per deprecation timeline.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d32a232f:

Refs #27468 -- Removed support for the pre-Django 3.1 signatures in Signer and signing.dumps()/loads().

Per deprecation timeline.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 6b4941dd:

Refs #27468 -- Removed support for the pre-Django 3.1 user sessions.

Per deprecation timeline.

[GitHub <noreply@…>]

In a94ae4cb:

Refs #27468 -- Updated django.core.signing docstring.

Follow up to 71c4fb7beb8e3293243140e4bd74e53989196440.