Opened 3 years ago

Last modified 16 months ago

#32800 closed Cleanup/optimization

CsrfViewMiddleware unnecessarily masks CSRF cookie — at Version 1

Reported by: Chris Jerdonek Owned by: nobody
Component: CSRF Version: dev
Severity: Normal Keywords:
Cc: Shai Berger, Florian Apolloner 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 (last modified by Chris Jerdonek)

I noticed that CsrfViewMiddleware unnecessarily masks the CSRF cookie. See, for example:
https://github.com/django/django/blob/d270dd584e0af12fe6229fb712d0704c232dc7e5/django/middleware/csrf.py#L91

My understanding of the BREACH attack is that the vulnerability comes from not masking the CSRF token in the response body (e.g. what is included in the HTML). Masking the cookie itself doesn't help with this. (Django also doesn't change the mask of the cookie with each request, so the mask wouldn't help in this regard anyways.)

Some advantages of not masking the cookie are: It would simplify the code in CsrfViewMiddleware because it would remove some complexity and operations that aren't needed for security. Currently, masking the CSRF cookie is a red herring for someone wanting to understand the various security features. Also, not masking the cookie would make requests and responses smaller when CSRF_USE_SESSIONS is false or when true and cookie-based sessions are used. This is because masking doubles the length of the string.

This can be changed fairly easily while (1) continuing to respect masked cookie values, and (2) not forcing session stores to update their cookie in its unmasked form if they are currently storing it masked.

Change History (1)

comment:1 by Chris Jerdonek, 3 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top