[Django] #16859: CSRF Improvements
Django
-------------------------------------+-------------------------------------
Reporter: PaulM | Owner: PaulM
Type: | Status: new
Cleanup/optimization | Component:
Milestone: | contrib.csrf
Version: 1.3 | Severity: Normal
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
This is a ticket to keep track of general CSRF improvements we want to add
to Django.
This includes:
* #16010 - add Origin checking
* Optionally tie CSRF to sessions
* Use signing to improve CSRF (maybe with sessions)
* Improve domain/host checking - deal with the subdomain to subdomain
problem
--
Ticket URL: <https://code.djangoproject.com/ticket/16859>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: PaulM | Owner: PaulM
Type: Cleanup/optimization | Status: new
Component: contrib.csrf | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Japneet Singh):
This ticket requires some cleanup and some makeover.Optional tie setup may
work or may may not as it has some vulnerabilities.I would kike to add
that we build a basic framework for these things to happen.
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:1>
Django
--------------------------------------+------------------------------------
Reporter: PaulM | Owner: PaulM
Type: Cleanup/optimization | Status: new
Component: CSRF | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by collinanderson):
* cc: cmawebsite@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: PaulM | Owner: PaulM
Type: Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by auvipy):
* version: 1.3 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:3>
Django
--------------------------------------+------------------------------------
Reporter: PaulM | Owner: PaulM
Type: Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by raphaelm):
In case other people at the #duth sprint are looking into this: Shai
berger is working on a new approach to generating the tokens and I'm
working on the usage of sessions for token storage.
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: PaulM | Owner: PaulM
Type: Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by raphaelm):
* cc: mail@… (added)
Comment:
I submitted a first version of a patch for session storage of CSRF tokens:
https://github.com/django/django/pull/5600
I'd love to have some review on this, but I'm fine with postponing the
merge after Shai landed his changes to CSRF handling, as those two will
get merge conflicts and his one will be the bigger change.
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Paul McMillan | Owner: Paul
Type: | McMillan
Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Paul McMillan | Owner: Paul
Type: | McMillan
Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"ddf169cdaca91e92dd5bfe6796bb6f38369ecb68" ddf169c]:
{{{
#!CommitTicketReference repository=""
revision="ddf169cdaca91e92dd5bfe6796bb6f38369ecb68"
Refs #16859 -- Allowed storing CSRF tokens in sessions.
Major thanks to Shai for helping to refactor the tests, and to
Shai, Tim, Florian, and others for extensive and helpful review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Paul McMillan | Owner: Paul
Type: | McMillan
Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Paul McMillan | Owner: Paul
Type: | McMillan
Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"33e86b3488dbf29f5aeb38cf0ee6597190d33c59" 33e86b34]:
{{{
#!CommitTicketReference repository=""
revision="33e86b3488dbf29f5aeb38cf0ee6597190d33c59"
Refs #16859 -- Disabled CSRF_COOKIE_* checks when using CSRF_USE_SESSIONS.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Paul McMillan | Owner: Paul
Type: | McMillan
Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"503e944ac792498e7b38c799d8e4b06f74e9d65a" 503e944a]:
{{{
#!CommitTicketReference repository=""
revision="503e944ac792498e7b38c799d8e4b06f74e9d65a"
Refs #16859 -- Updated CSRF FAQ to mention CSRF_USE_SESSIONS setting.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Paul McMillan | Owner: Paul
Type: | McMillan
Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"6bb01b0b3cc6e5b2cf8d75ed2fd00a442d5caf52" 6bb01b0]:
{{{
#!CommitTicketReference repository=""
revision="6bb01b0b3cc6e5b2cf8d75ed2fd00a442d5caf52"
[1.11.x] Refs #16859 -- Updated CSRF FAQ to mention CSRF_USE_SESSIONS
setting.
Backport of 503e944ac792498e7b38c799d8e4b06f74e9d65a from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Paul McMillan | Owner: Paul
Type: | McMillan
Cleanup/optimization | Status: new
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ed Morley):
Currently the CSRF middleware performs strict `Referer` header checking,
to (a) mitigate MITM attacks that set a cookie via plain HTTP, and (b)
prevent issues with malicious subdomains.
If the new `CSRF_USE_SESSIONS` is set to `True`, does that mean both of
those issues can no longer occur, and so the strict referrer checking is
then not required? (Along the lines of:
https://github.com/django/django/pull/5600#issuecomment-154797097)
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:12>
Django
-------------------------------+------------------------------------------
Reporter: Paul McMillan | Owner: Raphael Michel
Type: New feature | Status: assigned
Component: CSRF | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* owner: Paul McMillan => Raphael Michel
* status: new => assigned
* type: Cleanup/optimization => New feature
Old description:
> This is a ticket to keep track of general CSRF improvements we want to
> add to Django.
>
> This includes:
>
> * #16010 - add Origin checking
> * Optionally tie CSRF to sessions
> * Use signing to improve CSRF (maybe with sessions)
> * Improve domain/host checking - deal with the subdomain to subdomain
> problem
New description:
Storing the CSRF token in a cookie (Django's default) is safe, but storing
it in the session is common practice in other web frameworks and therefore
sometimes demanded by security auditors.
--
Comment:
I'm going to retitle this ticket for the work that was done and close it.
If there's remaining work from items in the old description (below), let's
open new tickets to track them individually.
> This is a ticket to keep track of general CSRF improvements we want to
add to Django. This includes:
> * #16010 - add Origin checking
> * Optionally tie CSRF to sessions [done in this ticket]
> * Use signing to improve CSRF (maybe with sessions)
> * Improve domain/host checking - deal with the subdomain to subdomain
problem
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:13>
Django
-------------------------------+------------------------------------------
Reporter: Paul McMillan | Owner: Raphael Michel
Component: CSRF | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: assigned => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/16859#comment:14>