Skip to content

Commit

Permalink
Fixed #32800 -- Changed CsrfViewMiddleware not to mask the CSRF secret.
Browse files Browse the repository at this point in the history
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 <felisiak.mariusz@gmail.com>
  • Loading branch information
cjerdonek and felixxm committed Nov 29, 2021
1 parent 05e29da commit 5d80843
Show file tree
Hide file tree
Showing 10 changed files with 285 additions and 144 deletions.
10 changes: 10 additions & 0 deletions django/conf/__init__.py
Expand Up @@ -34,6 +34,11 @@
'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):
"""
Expand Down Expand Up @@ -206,6 +211,9 @@ def __init__(self, settings_module):
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.
Expand Down Expand Up @@ -254,6 +262,8 @@ def __setattr__(self, name, value):
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)
Expand Down
4 changes: 4 additions & 0 deletions django/conf/global_settings.py
Expand Up @@ -557,6 +557,10 @@ def gettext_noop(s):
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 #
############
Expand Down
107 changes: 65 additions & 42 deletions django/middleware/csrf.py
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -124,29 +129,33 @@ def __init__(self, reason):


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):
Expand Down Expand Up @@ -206,10 +215,17 @@ def _reject(self, request, reason):
)
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 '
Expand All @@ -218,18 +234,18 @@ def _get_token(self, request):
)
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:
Expand Down Expand Up @@ -328,15 +344,15 @@ def _bad_token_message(self, reason, token_source):
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.
Expand All @@ -358,6 +374,10 @@ def _check_token(self, request):
# 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)
Expand All @@ -366,24 +386,27 @@ def _check_token(self, request):
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):
Expand Down
2 changes: 2 additions & 0 deletions docs/internals/deprecation.txt
Expand Up @@ -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
Expand Down
43 changes: 22 additions & 21 deletions docs/ref/csrf.txt
Expand Up @@ -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::

Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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.

Expand Down
16 changes: 16 additions & 0 deletions docs/ref/settings.txt
Expand Up @@ -347,6 +347,22 @@ form input <acquiring-csrf-token-from-html>` 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 <csrf-cookie-masked-usage>` 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``
Expand Down
28 changes: 28 additions & 0 deletions docs/releases/4.1.txt
Expand Up @@ -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
--------------

Expand Down Expand Up @@ -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
Expand All @@ -283,6 +309,8 @@ Miscellaneous
:ref:`context variables <sitemap-index-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
=======================

Expand Down
6 changes: 3 additions & 3 deletions tests/csrf_tests/test_context_processor.py
Expand Up @@ -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)

0 comments on commit 5d80843

Please sign in to comment.