From 5d80843ebc5376d00f98bf2a6aadbada4c29365c Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Tue, 17 Aug 2021 09:13:13 -0400 Subject: [PATCH] Fixed #32800 -- Changed CsrfViewMiddleware not to mask the CSRF secret. This also adds CSRF_COOKIE_MASKED transitional setting helpful in migrating multiple instance of the same project to Django 4.1+. Thanks Florian Apolloner and Shai Berger for reviews. Co-Authored-By: Mariusz Felisiak --- django/conf/__init__.py | 10 + django/conf/global_settings.py | 4 + django/middleware/csrf.py | 103 +++++++---- docs/internals/deprecation.txt | 2 + docs/ref/csrf.txt | 43 ++--- docs/ref/settings.txt | 16 ++ docs/releases/4.1.txt | 28 +++ tests/csrf_tests/test_context_processor.py | 6 +- tests/csrf_tests/tests.py | 185 +++++++++++-------- tests/deprecation/test_csrf_cookie_masked.py | 30 +++ 10 files changed, 284 insertions(+), 143 deletions(-) create mode 100644 tests/deprecation/test_csrf_cookie_masked.py diff --git a/django/conf/__init__.py b/django/conf/__init__.py index 80f3115d476..57623db42eb 100644 --- a/django/conf/__init__.py +++ b/django/conf/__init__.py @@ -34,6 +34,11 @@ USE_L10N_DEPRECATED_MSG = ( 'display numbers and dates using the format of the current locale.' ) +CSRF_COOKIE_MASKED_DEPRECATED_MSG = ( + 'The CSRF_COOKIE_MASKED transitional setting is deprecated. Support for ' + 'it will be removed in Django 5.0.' +) + class SettingsReference(str): """ @@ -206,6 +211,9 @@ class Settings: if self.is_overridden('USE_DEPRECATED_PYTZ'): warnings.warn(USE_DEPRECATED_PYTZ_DEPRECATED_MSG, RemovedInDjango50Warning) + if self.is_overridden('CSRF_COOKIE_MASKED'): + warnings.warn(CSRF_COOKIE_MASKED_DEPRECATED_MSG, RemovedInDjango50Warning) + if hasattr(time, 'tzset') and self.TIME_ZONE: # When we can, attempt to validate the timezone. If we can't find # this file, no check happens and it's harmless. @@ -254,6 +262,8 @@ class UserSettingsHolder: self._deleted.discard(name) if name == 'USE_L10N': warnings.warn(USE_L10N_DEPRECATED_MSG, RemovedInDjango50Warning) + if name == 'CSRF_COOKIE_MASKED': + warnings.warn(CSRF_COOKIE_MASKED_DEPRECATED_MSG, RemovedInDjango50Warning) super().__setattr__(name, value) if name == 'USE_DEPRECATED_PYTZ': warnings.warn(USE_DEPRECATED_PYTZ_DEPRECATED_MSG, RemovedInDjango50Warning) diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py index 769e7c99af2..66180702820 100644 --- a/django/conf/global_settings.py +++ b/django/conf/global_settings.py @@ -557,6 +557,10 @@ CSRF_HEADER_NAME = 'HTTP_X_CSRFTOKEN' CSRF_TRUSTED_ORIGINS = [] CSRF_USE_SESSIONS = False +# Whether to mask CSRF cookie value. It's a transitional setting helpful in +# migrating multiple instance of the same project to Django 4.1+. +CSRF_COOKIE_MASKED = False + ############ # MESSAGES # ############ diff --git a/django/middleware/csrf.py b/django/middleware/csrf.py index 1545014711d..41bf8640d6e 100644 --- a/django/middleware/csrf.py +++ b/django/middleware/csrf.py @@ -83,7 +83,12 @@ def _add_new_csrf_cookie(request): """Generate a new random CSRF_COOKIE value, and add it to request.META.""" csrf_secret = _get_new_csrf_string() request.META.update({ - 'CSRF_COOKIE': _mask_cipher_secret(csrf_secret), + # RemovedInDjango50Warning: when the deprecation ends, replace + # with: 'CSRF_COOKIE': csrf_secret + 'CSRF_COOKIE': ( + _mask_cipher_secret(csrf_secret) + if settings.CSRF_COOKIE_MASKED else csrf_secret + ), 'CSRF_COOKIE_NEEDS_UPDATE': True, }) return csrf_secret @@ -100,7 +105,7 @@ def get_token(request): function lazily, as is done by the csrf context processor. """ if 'CSRF_COOKIE' in request.META: - csrf_secret = _unmask_cipher_token(request.META["CSRF_COOKIE"]) + csrf_secret = request.META['CSRF_COOKIE'] # Since the cookie is being used, flag to send the cookie in # process_response() (even if the client already has it) in order to # renew the expiry timer. @@ -124,29 +129,33 @@ class InvalidTokenFormat(Exception): def _sanitize_token(token): + """ + Raise an InvalidTokenFormat error if the token has an invalid length or + characters that aren't allowed. The token argument can be a CSRF cookie + secret or non-cookie CSRF token, and either masked or unmasked. + """ if len(token) not in (CSRF_TOKEN_LENGTH, CSRF_SECRET_LENGTH): raise InvalidTokenFormat(REASON_INCORRECT_LENGTH) # Make sure all characters are in CSRF_ALLOWED_CHARS. if invalid_token_chars_re.search(token): raise InvalidTokenFormat(REASON_INVALID_CHARACTERS) - if len(token) == CSRF_SECRET_LENGTH: - # Older Django versions set cookies to values of CSRF_SECRET_LENGTH - # alphanumeric characters. For backwards compatibility, accept - # such values as unmasked secrets. - # It's easier to mask here and be consistent later, rather than add - # different code paths in the checks, although that might be a tad more - # efficient. - return _mask_cipher_secret(token) - return token -def _does_token_match(request_csrf_token, csrf_token): - # Assume both arguments are sanitized -- that is, strings of - # length CSRF_TOKEN_LENGTH, all CSRF_ALLOWED_CHARS. - return constant_time_compare( - _unmask_cipher_token(request_csrf_token), - _unmask_cipher_token(csrf_token), - ) +def _does_token_match(request_csrf_token, csrf_secret): + """ + Return whether the given CSRF token matches the given CSRF secret, after + unmasking the token if necessary. + + This function assumes that the request_csrf_token argument has been + validated to have the correct length (CSRF_SECRET_LENGTH or + CSRF_TOKEN_LENGTH characters) and allowed characters, and that if it has + length CSRF_TOKEN_LENGTH, it is a masked secret. + """ + # Only unmask tokens that are exactly CSRF_TOKEN_LENGTH characters long. + if len(request_csrf_token) == CSRF_TOKEN_LENGTH: + request_csrf_token = _unmask_cipher_token(request_csrf_token) + assert len(request_csrf_token) == CSRF_SECRET_LENGTH + return constant_time_compare(request_csrf_token, csrf_secret) class RejectRequest(Exception): @@ -206,10 +215,17 @@ class CsrfViewMiddleware(MiddlewareMixin): ) return response - def _get_token(self, request): + def _get_secret(self, request): + """ + Return the CSRF secret originally associated with the request, or None + if it didn't have one. + + If the CSRF_USE_SESSIONS setting is false, raises InvalidTokenFormat if + the request's secret has invalid characters or an invalid length. + """ if settings.CSRF_USE_SESSIONS: try: - return request.session.get(CSRF_SESSION_KEY) + csrf_secret = request.session.get(CSRF_SESSION_KEY) except AttributeError: raise ImproperlyConfigured( 'CSRF_USE_SESSIONS is enabled, but request.session is not ' @@ -218,18 +234,18 @@ class CsrfViewMiddleware(MiddlewareMixin): ) else: try: - cookie_token = request.COOKIES[settings.CSRF_COOKIE_NAME] + csrf_secret = request.COOKIES[settings.CSRF_COOKIE_NAME] except KeyError: - return None - - # This can raise InvalidTokenFormat. - csrf_token = _sanitize_token(cookie_token) - - if csrf_token != cookie_token: - # Then the cookie token had length CSRF_SECRET_LENGTH, so flag - # to replace it with the masked version. - request.META['CSRF_COOKIE_NEEDS_UPDATE'] = True - return csrf_token + csrf_secret = None + else: + # This can raise InvalidTokenFormat. + _sanitize_token(csrf_secret) + if csrf_secret is None: + return None + # Django versions before 4.0 masked the secret before storing. + if len(csrf_secret) == CSRF_TOKEN_LENGTH: + csrf_secret = _unmask_cipher_token(csrf_secret) + return csrf_secret def _set_csrf_cookie(self, request, response): if settings.CSRF_USE_SESSIONS: @@ -328,15 +344,15 @@ class CsrfViewMiddleware(MiddlewareMixin): return f'CSRF token from {token_source} {reason}.' def _check_token(self, request): - # Access csrf_token via self._get_token() as rotate_token() may have + # Access csrf_secret via self._get_secret() as rotate_token() may have # been called by an authentication middleware during the # process_request() phase. try: - csrf_token = self._get_token(request) + csrf_secret = self._get_secret(request) except InvalidTokenFormat as exc: raise RejectRequest(f'CSRF cookie {exc.reason}.') - if csrf_token is None: + if csrf_secret is None: # No CSRF cookie. For POST requests, we insist on a CSRF cookie, # and in this way we can avoid all CSRF attacks, including login # CSRF. @@ -358,6 +374,10 @@ class CsrfViewMiddleware(MiddlewareMixin): # Fall back to X-CSRFToken, to make things easier for AJAX, and # possible for PUT/DELETE. try: + # This can have length CSRF_SECRET_LENGTH or CSRF_TOKEN_LENGTH, + # depending on whether the client obtained the token from + # the DOM or the cookie (and if the cookie, whether the cookie + # was masked or unmasked). request_csrf_token = request.META[settings.CSRF_HEADER_NAME] except KeyError: raise RejectRequest(REASON_CSRF_TOKEN_MISSING) @@ -366,24 +386,27 @@ class CsrfViewMiddleware(MiddlewareMixin): token_source = 'POST' try: - request_csrf_token = _sanitize_token(request_csrf_token) + _sanitize_token(request_csrf_token) except InvalidTokenFormat as exc: reason = self._bad_token_message(exc.reason, token_source) raise RejectRequest(reason) - if not _does_token_match(request_csrf_token, csrf_token): + if not _does_token_match(request_csrf_token, csrf_secret): reason = self._bad_token_message('incorrect', token_source) raise RejectRequest(reason) def process_request(self, request): try: - csrf_token = self._get_token(request) + csrf_secret = self._get_secret(request) except InvalidTokenFormat: _add_new_csrf_cookie(request) else: - if csrf_token is not None: - # Use same token next time. - request.META['CSRF_COOKIE'] = csrf_token + if csrf_secret is not None: + # Use the same secret next time. If the secret was originally + # masked, this also causes it to be replaced with the unmasked + # form, but only in cases where the secret is already getting + # saved anyways. + request.META['CSRF_COOKIE'] = csrf_secret def process_view(self, request, callback, callback_args, callback_kwargs): if getattr(request, 'csrf_processing_done', False): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 13c800e0ee9..7ee87d448af 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -67,6 +67,8 @@ details on these changes. * The ``SitemapIndexItem.__str__()`` method will be removed. +* The ``CSRF_COOKIE_MASKED`` transitional setting will be removed. + .. _deprecation-removed-in-4.1: 4.1 diff --git a/docs/ref/csrf.txt b/docs/ref/csrf.txt index e9c402cf4de..dcd86c53ea9 100644 --- a/docs/ref/csrf.txt +++ b/docs/ref/csrf.txt @@ -110,11 +110,12 @@ The above code could be simplified by using the `JavaScript Cookie library .. note:: - The CSRF token is also present in the DOM, but only if explicitly included - using :ttag:`csrf_token` in a template. The cookie contains the canonical - token; the ``CsrfViewMiddleware`` will prefer the cookie to the token in - the DOM. Regardless, you're guaranteed to have the cookie if the token is - present in the DOM, so you should use the cookie! + The CSRF token is also present in the DOM in a masked form, but only if + explicitly included using :ttag:`csrf_token` in a template. The cookie + contains the canonical, unmasked token. The + :class:`~django.middleware.csrf.CsrfViewMiddleware` will accept either. + However, in order to protect against `BREACH`_ attacks, it's recommended to + use a masked token. .. warning:: @@ -231,25 +232,21 @@ How it works The CSRF protection is based on the following things: -#. A CSRF cookie that is based on a random secret value, which other sites - will not have access to. +#. A CSRF cookie that is a random secret value, which other sites will not have + access to. - This cookie is set by ``CsrfViewMiddleware``. It is sent with every - response that has called ``django.middleware.csrf.get_token()`` (the - function used internally to retrieve the CSRF token), if it wasn't already - set on the request. + ``CsrfViewMiddleware`` sends this cookie with the response whenever + ``django.middleware.csrf.get_token()`` is called. It can also send it in + other cases. For security reasons, the value of the secret is changed each + time a user logs in. - In order to protect against `BREACH`_ attacks, the token is not simply the - secret; a random mask is prepended to the secret and used to scramble it. +#. A hidden form field with the name 'csrfmiddlewaretoken', present in all + outgoing POST forms. - For security reasons, the value of the secret is changed each time a - user logs in. - -#. A hidden form field with the name 'csrfmiddlewaretoken' present in all - outgoing POST forms. The value of this field is, again, the value of the - secret, with a mask which is both added to it and used to scramble it. The - mask is regenerated on every call to ``get_token()`` so that the form field - value is changed in every such response. + In order to protect against `BREACH`_ attacks, the value of this field is + not simply the secret. It is scrambled differently with each response using + a mask. The mask is generated randomly on every call to ``get_token()``, so + the form field value is different each time. This part is done by the template tag. @@ -294,6 +291,10 @@ The CSRF protection is based on the following things: ``Origin`` checking was added, as described above. +.. versionchanged:: 4.1 + + In older versions, the CSRF cookie value was masked. + This ensures that only forms that have originated from trusted domains can be used to POST data back. diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 2a6f082842e..54d9c992c94 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -347,6 +347,22 @@ form input ` instead of :ref:`from the cookie See :setting:`SESSION_COOKIE_HTTPONLY` for details on ``HttpOnly``. +.. setting:: CSRF_COOKIE_MASKED + +``CSRF_COOKIE_MASKED`` +---------------------- + +.. versionadded:: 4.1 + +Default: ``False`` + +Whether to mask the CSRF cookie. See +:ref:`release notes ` for usage details. + +.. deprecated:: 4.1 + + This transitional setting is deprecated and will be removed in Django 5.0. + .. setting:: CSRF_COOKIE_NAME ``CSRF_COOKIE_NAME`` diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index 9998802105b..34611b4d4d5 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -26,6 +26,25 @@ officially support the latest release of each series. What's new in Django 4.1 ======================== +.. _csrf-cookie-masked-usage: + +``CSRF_COOKIE_MASKED`` setting +------------------------------ + +The new :setting:`CSRF_COOKIE_MASKED` transitional setting allows specifying +whether to mask the CSRF cookie. + +:class:`~django.middleware.csrf.CsrfViewMiddleware` no longer masks the CSRF +cookie like it does the CSRF token in the DOM. If you are upgrading multiple +instances of the same project to Django 4.1, you should set +:setting:`CSRF_COOKIE_MASKED` to ``True`` during the transition, in +order to allow compatibility with the older versions of Django. Once the +transition to 4.1 is complete you can stop overriding +:setting:`CSRF_COOKIE_MASKED`. + +This setting is deprecated as of this release and will be removed in Django +5.0. + Minor features -------------- @@ -270,6 +289,13 @@ Miscellaneous * The Django test runner now returns a non-zero error code for unexpected successes from tests marked with :py:func:`unittest.expectedFailure`. +* :class:`~django.middleware.csrf.CsrfViewMiddleware` no longer masks the CSRF + cookie like it does the CSRF token in the DOM. + +* :class:`~django.middleware.csrf.CsrfViewMiddleware` now uses + ``request.META['CSRF_COOKIE']`` for storing the unmasked CSRF secret rather + than a masked version. This is an undocumented, private API. + .. _deprecated-features-4.1: Features deprecated in 4.1 @@ -283,6 +309,8 @@ Miscellaneous :ref:`context variables `, expecting a list of objects with ``location`` and optional ``lastmod`` attributes. +* ``CSRF_COOKIE_MASKED`` transitional setting is deprecated. + Features removed in 4.1 ======================= diff --git a/tests/csrf_tests/test_context_processor.py b/tests/csrf_tests/test_context_processor.py index 0949ed4e347..26a2b7aedb2 100644 --- a/tests/csrf_tests/test_context_processor.py +++ b/tests/csrf_tests/test_context_processor.py @@ -9,7 +9,7 @@ class TestContextProcessor(CsrfFunctionTestMixin, SimpleTestCase): def test_force_token_to_string(self): request = HttpRequest() - test_token = '1bcdefghij2bcdefghij3bcdefghij4bcdefghij5bcdefghij6bcdefghijABCD' - request.META['CSRF_COOKIE'] = test_token + test_secret = 32 * 'a' + request.META['CSRF_COOKIE'] = test_secret token = csrf(request).get('csrf_token') - self.assertMaskedSecretCorrect(token, 'lcccccccX2kcccccccY2jcccccccssIC') + self.assertMaskedSecretCorrect(token, test_secret) diff --git a/tests/csrf_tests/tests.py b/tests/csrf_tests/tests.py index 60f1e32ba54..203ef0f4193 100644 --- a/tests/csrf_tests/tests.py +++ b/tests/csrf_tests/tests.py @@ -12,6 +12,8 @@ from django.middleware.csrf import ( _unmask_cipher_token, get_token, rotate_token, ) from django.test import SimpleTestCase, override_settings +from django.test.utils import ignore_warnings +from django.utils.deprecation import RemovedInDjango50Warning from django.views.decorators.csrf import csrf_exempt, requires_csrf_token from .views import ( @@ -76,13 +78,12 @@ class CsrfFunctionTests(CsrfFunctionTestMixin, SimpleTestCase): def test_get_token_csrf_cookie_set(self): request = HttpRequest() - request.META['CSRF_COOKIE'] = MASKED_TEST_SECRET1 + request.META['CSRF_COOKIE'] = TEST_SECRET self.assertNotIn('CSRF_COOKIE_NEEDS_UPDATE', request.META) token = get_token(request) - self.assertNotEqual(token, MASKED_TEST_SECRET1) self.assertMaskedSecretCorrect(token, TEST_SECRET) # The existing cookie is preserved. - self.assertEqual(request.META['CSRF_COOKIE'], MASKED_TEST_SECRET1) + self.assertEqual(request.META['CSRF_COOKIE'], TEST_SECRET) self.assertIs(request.META['CSRF_COOKIE_NEEDS_UPDATE'], True) def test_get_token_csrf_cookie_not_set(self): @@ -91,38 +92,32 @@ class CsrfFunctionTests(CsrfFunctionTestMixin, SimpleTestCase): self.assertNotIn('CSRF_COOKIE_NEEDS_UPDATE', request.META) token = get_token(request) cookie = request.META['CSRF_COOKIE'] - self.assertEqual(len(cookie), CSRF_TOKEN_LENGTH) - unmasked_cookie = _unmask_cipher_token(cookie) - self.assertMaskedSecretCorrect(token, unmasked_cookie) + self.assertMaskedSecretCorrect(token, cookie) self.assertIs(request.META['CSRF_COOKIE_NEEDS_UPDATE'], True) def test_rotate_token(self): request = HttpRequest() - request.META['CSRF_COOKIE'] = MASKED_TEST_SECRET1 + request.META['CSRF_COOKIE'] = TEST_SECRET self.assertNotIn('CSRF_COOKIE_NEEDS_UPDATE', request.META) rotate_token(request) # The underlying secret was changed. cookie = request.META['CSRF_COOKIE'] - self.assertEqual(len(cookie), CSRF_TOKEN_LENGTH) - unmasked_cookie = _unmask_cipher_token(cookie) - self.assertNotEqual(unmasked_cookie, TEST_SECRET) + self.assertEqual(len(cookie), CSRF_SECRET_LENGTH) + self.assertNotEqual(cookie, TEST_SECRET) self.assertIs(request.META['CSRF_COOKIE_NEEDS_UPDATE'], True) - def test_sanitize_token_masked(self): - # Tokens of length CSRF_TOKEN_LENGTH are preserved. + def test_sanitize_token_valid(self): cases = [ - (MASKED_TEST_SECRET1, MASKED_TEST_SECRET1), - (64 * 'a', 64 * 'a'), + # A token of length CSRF_SECRET_LENGTH. + TEST_SECRET, + # A token of length CSRF_TOKEN_LENGTH. + MASKED_TEST_SECRET1, + 64 * 'a', ] - for token, expected in cases: + for token in cases: with self.subTest(token=token): actual = _sanitize_token(token) - self.assertEqual(actual, expected) - - def test_sanitize_token_unmasked(self): - # A token of length CSRF_SECRET_LENGTH is masked. - actual = _sanitize_token(TEST_SECRET) - self.assertMaskedSecretCorrect(actual, TEST_SECRET) + self.assertIsNone(actual) def test_sanitize_token_invalid(self): cases = [ @@ -136,14 +131,26 @@ class CsrfFunctionTests(CsrfFunctionTestMixin, SimpleTestCase): def test_does_token_match(self): cases = [ - ((MASKED_TEST_SECRET1, MASKED_TEST_SECRET2), True), - ((MASKED_TEST_SECRET1, 64 * 'a'), False), + # Masked tokens match. + ((MASKED_TEST_SECRET1, TEST_SECRET), True), + ((MASKED_TEST_SECRET2, TEST_SECRET), True), + ((64 * 'a', _unmask_cipher_token(64 * 'a')), True), + # Unmasked tokens match. + ((TEST_SECRET, TEST_SECRET), True), + ((32 * 'a', 32 * 'a'), True), + # Incorrect tokens don't match. + ((32 * 'a', TEST_SECRET), False), + ((64 * 'a', TEST_SECRET), False), ] - for (token1, token2), expected in cases: - with self.subTest(token1=token1, token2=token2): - actual = _does_token_match(token1, token2) + for (token, secret), expected in cases: + with self.subTest(token=token, secret=secret): + actual = _does_token_match(token, secret) self.assertIs(actual, expected) + def test_does_token_match_wrong_token_length(self): + with self.assertRaises(AssertionError): + _does_token_match(16 * 'a', TEST_SECRET) + class TestingSessionStore(SessionStore): """ @@ -215,14 +222,6 @@ class CsrfViewMiddlewareTestMixin(CsrfFunctionTestMixin): """ raise NotImplementedError('This method must be implemented by a subclass.') - def assertCookiesSet(self, req, resp, expected_secrets): - """ - Assert that set_cookie() was called with the given sequence of secrets. - """ - cookies_set = self._get_cookies_set(req, resp) - secrets_set = [_unmask_cipher_token(cookie) for cookie in cookies_set] - self.assertEqual(secrets_set, expected_secrets) - def _get_request(self, method=None, cookie=None, request_class=None): if method is None: method = 'GET' @@ -280,11 +279,9 @@ class CsrfViewMiddlewareTestMixin(CsrfFunctionTestMixin): ) # This method depends on _unmask_cipher_token() being correct. - def _check_token_present(self, response, csrf_token=None): - if csrf_token is None: + def _check_token_present(self, response, csrf_secret=None): + if csrf_secret is None: csrf_secret = TEST_SECRET - else: - csrf_secret = _unmask_cipher_token(csrf_token) text = str(response.content, response.charset) match = re.search('name="csrfmiddlewaretoken" value="(.*?)"', text) self.assertTrue( @@ -482,10 +479,12 @@ class CsrfViewMiddlewareTestMixin(CsrfFunctionTestMixin): req = self._get_POST_request_with_token() resp = sandwiched_rotate_token_view(req) self.assertContains(resp, 'OK') - csrf_cookie = self._read_csrf_cookie(req, resp) - actual_secret = _unmask_cipher_token(csrf_cookie) + actual_secret = self._read_csrf_cookie(req, resp) # set_cookie() was called a second time with a different secret. - self.assertCookiesSet(req, resp, [TEST_SECRET, actual_secret]) + cookies_set = self._get_cookies_set(req, resp) + # Only compare the last two to exclude a spurious entry that's present + # when CsrfViewMiddlewareUseSessionsTests is running. + self.assertEqual(cookies_set[-2:], [TEST_SECRET, actual_secret]) self.assertNotEqual(actual_secret, TEST_SECRET) # Tests for the template tag method @@ -498,7 +497,8 @@ class CsrfViewMiddlewareTestMixin(CsrfFunctionTestMixin): token = get_token(req) self.assertIsNotNone(token) - self._check_token_present(resp, token) + csrf_secret = _unmask_cipher_token(token) + self._check_token_present(resp, csrf_secret) def test_token_node_empty_csrf_cookie(self): """ @@ -511,7 +511,8 @@ class CsrfViewMiddlewareTestMixin(CsrfFunctionTestMixin): token = get_token(req) self.assertIsNotNone(token) - self._check_token_present(resp, token) + csrf_secret = _unmask_cipher_token(token) + self._check_token_present(resp, csrf_secret) def test_token_node_with_csrf_cookie(self): """ @@ -568,7 +569,7 @@ class CsrfViewMiddlewareTestMixin(CsrfFunctionTestMixin): resp = mw(req) csrf_cookie = self._read_csrf_cookie(req, resp) self.assertEqual( - csrf_cookie, self._csrf_id_cookie, + csrf_cookie, TEST_SECRET, 'CSRF cookie was changed on an accepted request', ) @@ -1108,7 +1109,7 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase): mw.process_view(req, token_view, (), {}) resp = mw(req) csrf_cookie = self._read_csrf_cookie(req, resp) - self.assertEqual(len(csrf_cookie), CSRF_TOKEN_LENGTH) + self.assertEqual(len(csrf_cookie), CSRF_SECRET_LENGTH) def test_process_view_token_invalid_chars(self): """ @@ -1121,7 +1122,7 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase): mw.process_view(req, token_view, (), {}) resp = mw(req) csrf_cookie = self._read_csrf_cookie(req, resp) - self.assertEqual(len(csrf_cookie), CSRF_TOKEN_LENGTH) + self.assertEqual(len(csrf_cookie), CSRF_SECRET_LENGTH) self.assertNotEqual(csrf_cookie, token) def test_masked_unmasked_combinations(self): @@ -1151,20 +1152,19 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase): resp = mw.process_view(req, token_view, (), {}) self.assertIsNone(resp) - def test_cookie_reset_only_once(self): + def test_set_cookie_called_only_once(self): """ - A CSRF cookie that needs to be reset is reset only once when the view - is decorated with both ensure_csrf_cookie and csrf_protect. + set_cookie() is called only once when the view is decorated with both + ensure_csrf_cookie and csrf_protect. """ - # Pass an unmasked cookie to trigger a cookie reset. - req = self._get_POST_request_with_token(cookie=TEST_SECRET) + req = self._get_POST_request_with_token() resp = ensured_and_protected_view(req) self.assertContains(resp, 'OK') csrf_cookie = self._read_csrf_cookie(req, resp) - actual_secret = _unmask_cipher_token(csrf_cookie) - self.assertEqual(actual_secret, TEST_SECRET) + self.assertEqual(csrf_cookie, TEST_SECRET) # set_cookie() was called only once and with the expected secret. - self.assertCookiesSet(req, resp, [TEST_SECRET]) + cookies_set = self._get_cookies_set(req, resp) + self.assertEqual(cookies_set, [TEST_SECRET]) def test_invalid_cookie_replaced_on_GET(self): """ @@ -1175,28 +1175,28 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase): self.assertContains(resp, 'OK') csrf_cookie = self._read_csrf_cookie(req, resp) self.assertTrue(csrf_cookie, msg='No CSRF cookie was sent.') - self.assertEqual(len(csrf_cookie), CSRF_TOKEN_LENGTH) + self.assertEqual(len(csrf_cookie), CSRF_SECRET_LENGTH) - def test_unmasked_secret_replaced_on_GET(self): - """An unmasked CSRF cookie is replaced during a GET request.""" - req = self._get_request(cookie=TEST_SECRET) - resp = protected_view(req) - self.assertContains(resp, 'OK') - csrf_cookie = self._read_csrf_cookie(req, resp) - self.assertTrue(csrf_cookie, msg='No CSRF cookie was sent.') - self.assertMaskedSecretCorrect(csrf_cookie, TEST_SECRET) - - def test_masked_secret_not_replaced_on_GET(self): - """A masked CSRF cookie is not replaced during a GET request.""" - req = self._get_request(cookie=MASKED_TEST_SECRET1) - resp = protected_view(req) - self.assertContains(resp, 'OK') - csrf_cookie = self._read_csrf_cookie(req, resp) - self.assertFalse(csrf_cookie, msg='A CSRF cookie was sent.') - - def test_masked_secret_accepted_and_not_replaced(self): + def test_valid_secret_not_replaced_on_GET(self): """ - The csrf cookie is left unchanged if originally masked. + Masked and unmasked CSRF cookies are not replaced during a GET request. + """ + cases = [ + TEST_SECRET, + MASKED_TEST_SECRET1, + ] + for cookie in cases: + with self.subTest(cookie=cookie): + req = self._get_request(cookie=cookie) + resp = protected_view(req) + self.assertContains(resp, 'OK') + csrf_cookie = self._read_csrf_cookie(req, resp) + self.assertFalse(csrf_cookie, msg='A CSRF cookie was sent.') + + def test_masked_secret_accepted_and_replaced(self): + """ + For a view that uses the csrf_token, the csrf cookie is replaced with + the unmasked version if originally masked. """ req = self._get_POST_request_with_token(cookie=MASKED_TEST_SECRET1) mw = CsrfViewMiddleware(token_view) @@ -1205,12 +1205,12 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase): self.assertIsNone(resp) resp = mw(req) csrf_cookie = self._read_csrf_cookie(req, resp) - self.assertEqual(csrf_cookie, MASKED_TEST_SECRET1) + self.assertEqual(csrf_cookie, TEST_SECRET) self._check_token_present(resp, csrf_cookie) - def test_bare_secret_accepted_and_replaced(self): + def test_bare_secret_accepted_and_not_replaced(self): """ - The csrf cookie is reset (masked) if originally not masked. + The csrf cookie is left unchanged if originally not masked. """ req = self._get_POST_request_with_token(cookie=TEST_SECRET) mw = CsrfViewMiddleware(token_view) @@ -1219,8 +1219,7 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase): self.assertIsNone(resp) resp = mw(req) csrf_cookie = self._read_csrf_cookie(req, resp) - # This also checks that csrf_cookie now has length CSRF_TOKEN_LENGTH. - self.assertMaskedSecretCorrect(csrf_cookie, TEST_SECRET) + self.assertEqual(csrf_cookie, TEST_SECRET) self._check_token_present(resp, csrf_cookie) @override_settings(ALLOWED_HOSTS=['www.example.com'], CSRF_COOKIE_DOMAIN='.example.com', USE_X_FORWARDED_PORT=True) @@ -1407,3 +1406,31 @@ class CsrfInErrorHandlingViewsTests(CsrfFunctionTestMixin, SimpleTestCase): token2 = response.content.decode('ascii') secret2 = _unmask_cipher_token(token2) self.assertMaskedSecretCorrect(token1, secret2) + + +@ignore_warnings(category=RemovedInDjango50Warning) +class CsrfCookieMaskedTests(CsrfFunctionTestMixin, SimpleTestCase): + @override_settings(CSRF_COOKIE_MASKED=True) + def test_get_token_csrf_cookie_not_set(self): + request = HttpRequest() + self.assertNotIn('CSRF_COOKIE', request.META) + self.assertNotIn('CSRF_COOKIE_NEEDS_UPDATE', request.META) + token = get_token(request) + cookie = request.META['CSRF_COOKIE'] + self.assertEqual(len(cookie), CSRF_TOKEN_LENGTH) + unmasked_cookie = _unmask_cipher_token(cookie) + self.assertMaskedSecretCorrect(token, unmasked_cookie) + self.assertIs(request.META['CSRF_COOKIE_NEEDS_UPDATE'], True) + + @override_settings(CSRF_COOKIE_MASKED=True) + def test_rotate_token(self): + request = HttpRequest() + request.META['CSRF_COOKIE'] = MASKED_TEST_SECRET1 + self.assertNotIn('CSRF_COOKIE_NEEDS_UPDATE', request.META) + rotate_token(request) + # The underlying secret was changed. + cookie = request.META['CSRF_COOKIE'] + self.assertEqual(len(cookie), CSRF_TOKEN_LENGTH) + unmasked_cookie = _unmask_cipher_token(cookie) + self.assertNotEqual(unmasked_cookie, TEST_SECRET) + self.assertIs(request.META['CSRF_COOKIE_NEEDS_UPDATE'], True) diff --git a/tests/deprecation/test_csrf_cookie_masked.py b/tests/deprecation/test_csrf_cookie_masked.py new file mode 100644 index 00000000000..74592fd0b76 --- /dev/null +++ b/tests/deprecation/test_csrf_cookie_masked.py @@ -0,0 +1,30 @@ +import sys +from types import ModuleType + +from django.conf import CSRF_COOKIE_MASKED_DEPRECATED_MSG, Settings, settings +from django.test import SimpleTestCase +from django.utils.deprecation import RemovedInDjango50Warning + + +class CsrfCookieMaskedDeprecationTests(SimpleTestCase): + msg = CSRF_COOKIE_MASKED_DEPRECATED_MSG + + def test_override_settings_warning(self): + with self.assertRaisesMessage(RemovedInDjango50Warning, self.msg): + with self.settings(CSRF_COOKIE_MASKED=True): + pass + + def test_settings_init_warning(self): + settings_module = ModuleType('fake_settings_module') + settings_module.USE_TZ = False + settings_module.CSRF_COOKIE_MASKED = True + sys.modules['fake_settings_module'] = settings_module + try: + with self.assertRaisesMessage(RemovedInDjango50Warning, self.msg): + Settings('fake_settings_module') + finally: + del sys.modules['fake_settings_module'] + + def test_access(self): + # Warning is not raised on access. + self.assertEqual(settings.CSRF_COOKIE_MASKED, False)