#34384 closed Bug (fixed)
SECRET_KEY_FALLBACKS is not used for sessions
Reported by: | Eric Zarowny | Owned by: | David Wobrock |
---|---|---|---|
Component: | contrib.auth | Version: | 4.1 |
Severity: | Release blocker | Keywords: | |
Cc: | Timothy Schilling, Florian Apolloner, Joey Lange, Carlton Gibson, Andreas Pelme, terrameijar, David Wobrock | 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
I recently rotated my secret key, made the old one available in SECRET_KEY_FALLBACKS
and I'm pretty sure everyone on our site is logged out now.
I think the docs for SECRET_KEY_FALLBACKS may be incorrect when stating the following:
In order to rotate your secret keys, set a new SECRET_KEY and move the previous value to the beginning of SECRET_KEY_FALLBACKS. Then remove the old values from the end of the SECRET_KEY_FALLBACKS when you are ready to expire the sessions, password reset tokens, and so on, that make use of them.
When looking at the Django source code, I see that the salted_hmac function uses the SECRET_KEY
by default and the AbstractBaseUser.get_session_auth_hash method does not call salted_hmac
with a value for the secret
keyword argument.
Change History (14)
comment:1 by , 21 months ago
Cc: | added |
---|
comment:2 by , 21 months ago
Cc: | added |
---|
follow-up: 6 comment:4 by , 21 months ago
Cc: | added |
---|---|
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
Thanks for the report. Agreed, we should check fallback session hashes.
Bug in 0dcd549bbe36c060f536ec270d34d9e7d4b8e6c7.
In particular for user sessions, using fallback keys in the
AuthenticationMiddleware
/auth.get_user(request)
will keep existing_auth_user_hash
values from before the rotation being seen as valid, which is nice during the rotation period, but without any upgrading of the_auth_user_hash
values, when the rotation is finished and the fallback keys are removed, all of those sessions will essentially be invalidated again.
So, I think possibly an additional need here is a way to upgrade the cookies when a fallback key is used? Or at least documentation calling out this drawback.
Edit: It's possible I'm conflating a cookie value and a session value, but either way I think the principle of what I wrote stands?
As far as I'm aware, this is a new feature request not a bug in #30360, so we should discuss it separately. Maybe we could call update_session_auth_hash()
when a fallback hash is valid 🤔
comment:5 by , 21 months ago
Cc: | added |
---|---|
Has patch: | set |
Owner: | changed from | to
Status: | new → assigned |
follow-up: 7 comment:6 by , 21 months ago
Replying to Mariusz Felisiak:
Maybe we could call
update_session_auth_hash()
when a fallback hash is valid 🤔
Most likely yes, we don't want to pay the calculation overhead every request :)
comment:7 by , 21 months ago
Replying to Florian Apolloner:
Replying to Mariusz Felisiak:
Maybe we could call
update_session_auth_hash()
when a fallback hash is valid 🤔
Most likely yes, we don't want to pay the calculation overhead every request :)
OK, so we can accept this as a part of the bugfix.
comment:8 by , 21 months ago
Needs documentation: | set |
---|---|
Patch needs improvement: | set |
comment:9 by , 21 months ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | unset |
comment:10 by , 21 months ago
Patch needs improvement: | set |
---|
comment:11 by , 21 months ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Hi! I'm a colleague of Eric's, and we were discussing some of the ramifications of fixing this issue and I thought I'd write them here for posterity.
In particular for user sessions, using fallback keys in the
AuthenticationMiddleware
/auth.get_user(request)
will keep existing_auth_user_hash
values from before the rotation being seen as valid, which is nice during the rotation period, but without any upgrading of the_auth_user_hash
values, when the rotation is finished and the fallback keys are removed, all of those sessions will essentially be invalidated again.So, I think possibly an additional need here is a way to upgrade the cookies when a fallback key is used? Or at least documentation calling out this drawback.