[Django] #26628: CSRF violations should be logged
Django
---------------------------------------+-------------------------------
Reporter: jacobian | Owner: nobody
Type: New feature | Status: new
Component: CSRF | Version: 1.9
Severity: Normal | Keywords: csrf security
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+-------------------------------
It's currently possible to log CSRF exceptions with the 403 handler. But a
more sensible default would be to log them by default. This would be an
easy simple step that would make Django play more nicely with a SIEM
[One of a series of bugs from a discussion I had with @mallyvai about
improving the security of Django's admin - see
https://gist.github.com/mallyvai/bcb0bb827d6d53212879dff23cf15d03 for the
full list.]
--
Ticket URL: <https://code.djangoproject.com/ticket/26628>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jacobian | Owner: nobody
Type: New feature | Status: new
Component: CSRF | Version: 1.9
Keywords: csrf security | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
The `CsrfViewMiddleware` does have
[https://github.com/django/django/blob/f179113e6cbc8ba0a8d4e87e1d4410fb61d63e75/django/middleware/csrf.py#L100-L108
logging for rejected requests]. Is this insufficient? I couldn't find this
mentioned in the documentation so that could be added, at least.
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:1>
Django
Reporter: jacobian | Owner: nobody
Type: New feature | Status: closed
Component: CSRF | Version: 1.9
Severity: Normal | Resolution: worksforme
Keywords: csrf security | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => worksforme
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:2>
Django
Reporter: jacobian | Owner: nobody
Type: New feature | Status: closed
Component: CSRF | Version: 1.9
Severity: Normal | Resolution: worksforme
Keywords: csrf security | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mallyvai):
Hi Tim,
Definitely think it's a good idea to add to the docs.
My original thinking was that it would be a more sensible default to log
this as a security exception and use the django.security loggers (i'm not
100% sure about how that's wired up), since CSRFs are meant to be an
explicit security measure. It seems to be logged as a generic
logger.warning right now.
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:3>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: csrf security | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* type: New feature => Cleanup/optimization
* component: CSRF => Documentation
* resolution: worksforme =>
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:4>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Hwesta):
* owner: nobody => Hwesta
* needs_docs: 0 => 1
* status: new => assigned
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by carljm):
Sergio Campos (seocam) and I were just discussing this same issue at the
PyCon sprint (in the context of looking at the broader context in #26688).
I'm inclined to agree with mallyvai that this isn't just a doc bug -- we
should be logging CSRF failures to django.security by default. Updating
the ticket title accordingly -- please let me know if you don't agree, or
think this should be discussed on the mailing list.
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by timgraham):
Are you concerned at all about backwards-compatibility in the case of some
custom logging?
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by carljm):
I guess I'm not too concerned about back-compat, because it seems it would
only be an issue in a case where someone has configured logging to watch
`django.request` but ignore `django.security`, which seems like a pretty
strange choice. But I also don't feel strongly. I'd be inclined to merge
the doc fix that Hwesta has already submitted
(https://github.com/django/django/pull/6694/files) but as a `Refs`, and
leave this open for further discussion of the actual logger change.
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:8>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"ff9198ee0f1de24a5b2861d28849344e7a5714c4" ff9198e]:
{{{
#!CommitTicketReference repository=""
revision="ff9198ee0f1de24a5b2861d28849344e7a5714c4"
Refs #26628 -- Documented CSRF failure logging.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:9>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"cbc8ef6127a92cc59dfb28461e865242e5e39329" cbc8ef61]:
{{{
#!CommitTicketReference repository=""
revision="cbc8ef6127a92cc59dfb28461e865242e5e39329"
[1.9.x] Refs #26628 -- Documented CSRF failure logging.
Backport of ff9198ee0f1de24a5b2861d28849344e7a5714c4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:10>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"697ed75de5197a2175247464e1eee14fcf38562f" 697ed75]:
{{{
#!CommitTicketReference repository=""
revision="697ed75de5197a2175247464e1eee14fcf38562f"
[1.10.x] Refs #26628 -- Documented CSRF failure logging.
Backport of ff9198ee0f1de24a5b2861d28849344e7a5714c4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:11>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: csrf security | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
* needs_docs: 1 => 0
* has_patch: 0 => 1
* component: Documentation => CSRF
* needs_better_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/6707 PR] with comments for
improvement.
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:12>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: csrf security | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by timgraham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:13>
Django
--------------------------------------+------------------------------------
Reporter: jacobian | Owner: Hwesta
Component: CSRF | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: csrf security | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"55fec16aafed30a9daa06d6ecdf8ca3ad361279e" 55fec16a]:
{{{
#!CommitTicketReference repository=""
revision="55fec16aafed30a9daa06d6ecdf8ca3ad361279e"
Fixed #26628 -- Changed CSRF logger to django.security.csrf.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26628#comment:14>