From 2396933ca99c6bfb53bda9e53968760316646e01 Mon Sep 17 00:00:00 2001 From: David Wobrock Date: Mon, 6 Mar 2023 16:18:03 +0100 Subject: [PATCH] Fixed #34384 -- Fixed session validation when rotation secret keys. Bug in 0dcd549bbe36c060f536ec270d34d9e7d4b8e6c7. Thanks Eric Zarowny for the report. --- django/contrib/auth/__init__.py | 24 +++++++++++++++++++----- django/contrib/auth/base_user.py | 9 +++++++++ docs/ref/contrib/auth.txt | 9 ++++++++- docs/releases/4.1.8.txt | 3 ++- docs/topics/auth/customizing.txt | 7 +++++++ tests/auth_tests/test_basic.py | 24 ++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 7 deletions(-) diff --git a/django/contrib/auth/__init__.py b/django/contrib/auth/__init__.py index 155330c596..2c81d62a0c 100644 --- a/django/contrib/auth/__init__.py +++ b/django/contrib/auth/__init__.py @@ -199,12 +199,26 @@ def get_user(request): # Verify the session if hasattr(user, "get_session_auth_hash"): session_hash = request.session.get(HASH_SESSION_KEY) - session_hash_verified = session_hash and constant_time_compare( - session_hash, user.get_session_auth_hash() - ) + if not session_hash: + session_hash_verified = False + else: + session_auth_hash = user.get_session_auth_hash() + session_hash_verified = constant_time_compare( + session_hash, session_auth_hash + ) if not session_hash_verified: - request.session.flush() - user = None + # If the current secret does not verify the session, try + # with the fallback secrets and stop when a matching one is + # found. + if session_hash and any( + constant_time_compare(session_hash, fallback_auth_hash) + for fallback_auth_hash in user.get_session_auth_fallback_hash() + ): + request.session.cycle_key() + request.session[HASH_SESSION_KEY] = session_auth_hash + else: + request.session.flush() + user = None return user or AnonymousUser() diff --git a/django/contrib/auth/base_user.py b/django/contrib/auth/base_user.py index 5ee30bf59c..e205ccccf2 100644 --- a/django/contrib/auth/base_user.py +++ b/django/contrib/auth/base_user.py @@ -5,6 +5,7 @@ not in INSTALLED_APPS. import unicodedata import warnings +from django.conf import settings from django.contrib.auth import password_validation from django.contrib.auth.hashers import ( check_password, @@ -135,10 +136,18 @@ class AbstractBaseUser(models.Model): """ Return an HMAC of the password field. """ + return self._get_session_auth_hash() + + def get_session_auth_fallback_hash(self): + for fallback_secret in settings.SECRET_KEY_FALLBACKS: + yield self._get_session_auth_hash(secret=fallback_secret) + + def _get_session_auth_hash(self, secret=None): key_salt = "django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash" return salted_hmac( key_salt, self.password, + secret=secret, algorithm="sha256", ).hexdigest() diff --git a/docs/ref/contrib/auth.txt b/docs/ref/contrib/auth.txt index 241a0219bd..90ae5904a8 100644 --- a/docs/ref/contrib/auth.txt +++ b/docs/ref/contrib/auth.txt @@ -695,10 +695,17 @@ Utility functions ``get_user()`` method to retrieve the user model instance and then verifies the session by calling the user model's :meth:`~django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash` - method. + method. If the verification fails and :setting:`SECRET_KEY_FALLBACKS` are + provided, it verifies the session against each fallback key using + :meth:`~django.contrib.auth.models.AbstractBaseUser.\ + get_session_auth_fallback_hash`. Returns an instance of :class:`~django.contrib.auth.models.AnonymousUser` if the authentication backend stored in the session is no longer in :setting:`AUTHENTICATION_BACKENDS`, if a user isn't returned by the backend's ``get_user()`` method, or if the session auth hash doesn't validate. + + .. versionchanged:: 4.1.8 + + Fallback verification with :setting:`SECRET_KEY_FALLBACKS` was added. diff --git a/docs/releases/4.1.8.txt b/docs/releases/4.1.8.txt index 685580f33c..9f3dd167ed 100644 --- a/docs/releases/4.1.8.txt +++ b/docs/releases/4.1.8.txt @@ -9,4 +9,5 @@ Django 4.1.8 fixes several bugs in 4.1.7. Bugfixes ======== -* ... +* Fixed a bug in Django 4.1 that caused invalidation of sessions when rotating + secret keys with ``SECRET_KEY_FALLBACKS`` (:ticket:`34384`). diff --git a/docs/topics/auth/customizing.txt b/docs/topics/auth/customizing.txt index 3b688c8b5c..6cc48cacb1 100644 --- a/docs/topics/auth/customizing.txt +++ b/docs/topics/auth/customizing.txt @@ -722,6 +722,13 @@ The following attributes and methods are available on any subclass of Returns an HMAC of the password field. Used for :ref:`session-invalidation-on-password-change`. + .. method:: models.AbstractBaseUser.get_session_auth_fallback_hash() + + .. versionadded:: 4.1.8 + + Yields the HMAC of the password field using + :setting:`SECRET_KEY_FALLBACKS`. Used by ``get_user()``. + :class:`~models.AbstractUser` subclasses :class:`~models.AbstractBaseUser`: .. class:: models.AbstractUser diff --git a/tests/auth_tests/test_basic.py b/tests/auth_tests/test_basic.py index 4b491e521e..c341aeb8c9 100644 --- a/tests/auth_tests/test_basic.py +++ b/tests/auth_tests/test_basic.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.contrib.auth import get_user, get_user_model from django.contrib.auth.models import AnonymousUser, User from django.core.exceptions import ImproperlyConfigured @@ -138,3 +139,26 @@ class TestGetUser(TestCase): user = get_user(request) self.assertIsInstance(user, User) self.assertEqual(user.username, created_user.username) + + def test_get_user_fallback_secret(self): + created_user = User.objects.create_user( + "testuser", "test@example.com", "testpw" + ) + self.client.login(username="testuser", password="testpw") + request = HttpRequest() + request.session = self.client.session + prev_session_key = request.session.session_key + with override_settings( + SECRET_KEY="newsecret", + SECRET_KEY_FALLBACKS=[settings.SECRET_KEY], + ): + user = get_user(request) + self.assertIsInstance(user, User) + self.assertEqual(user.username, created_user.username) + self.assertNotEqual(request.session.session_key, prev_session_key) + # Remove the fallback secret. + # The session hash should be updated using the current secret. + with override_settings(SECRET_KEY="newsecret"): + user = get_user(request) + self.assertIsInstance(user, User) + self.assertEqual(user.username, created_user.username)