Browse Source

Fixed #34384 -- Fixed session validation when rotation secret keys.

Bug in 0dcd549bbe36c060f536ec270d34d9e7d4b8e6c7.

Thanks Eric Zarowny for the report.
David Wobrock 2 years ago
parent
commit
2396933ca9

+ 19 - 5
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()
 

+ 9 - 0
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()
 

+ 8 - 1
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.

+ 2 - 1
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`).

+ 7 - 0
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

+ 24 - 0
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)