Changes between Version 14 and Version 15 of CsrfProtection


Ignore:
Timestamp:
Sep 25, 2009, 5:28:26 AM (15 years ago)
Author:
Luke Plant
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • CsrfProtection

    v14 v15  
    77== Summary ==
    88
    9 For Django 1.2, I (Luke Plant) propose:
     9For Django 1.2, Luke Plant, with feedback from other developers, proposes:
    1010
    1111 * We should move to using a session independent nonce as a CSRF token, instead of a hash of the session identifier as used in Django 1.1 and earlier.  This eliminates the false positives associated with session cycling, and removes the dependency on the session framework, making the middleware more generally useful, and also fixing login CSRF vulnerabilities (which were only partially and accidentally addressed before).
    1212 * Since the above introduces a subtle regression involving attacks on HTTPS connections, this should be fixed by adding strict referer checking for HTTPS connections.
    13  * The post-processing middleware (CsrfResponseMiddleware) should be deprecated and replaced with a template tag to insert the CSRF token.  This allows for streaming responses and fixes a potential vulnerability that existed with the post-processing middleware.  All contrib apps should be updated to use this, which makes the CSRF app a dependency of these apps.
     13 * The post-processing middleware (!CsrfResponseMiddleware) should be deprecated and replaced with a template tag to insert the CSRF token.  This allows for streaming responses and fixes a potential vulnerability that existed with the post-processing middleware.  All contrib apps should be updated to use this, which makes the CSRF app a dependency of these apps.
     14 * In response to feedback from other devs, additionally:
     15   * A decorator ``csrf_protect`` should be implemented and added to all contrib views that need it, so that even in the absence of the middleware, these apps are protected.
     16   * Everything needed for this should be baked into core template tags etc., so that no changes to settings are needed
     17   * ``django.contrib.csrf.*`` should move to core code somewhere.
     18 
     19No immediate changes are required for people upgrading if they want the admin and other contrib apps to continue to work.  Longer term (by Django 1.4), apps must be updated to switch away from the deprecated features and use the new template tag method.  Also, imports must be changed by Django 1.4.
    1420
    15 These changes require some immediate changes to settings.py for people upgrading if they want the admin and other contrib apps to continue to work, and, longer term, apps must be updated to switch away from the deprecated features and use the new template tag method, but otherwise the upgrade path should be fairly easy (and it is fully documented).
     21The patch for this is complete (see bottom).
     22
     23Also, Simon Willison is proposing:
     24 * a 'csrf_protect_form`` decorator that acts similarly to ``csrf_protect``, but moves the actual rejection of requests to part of form validation, by adding a special entry to ``request.POST`` which is checked in ``Form.is_valid()``.
     25 * extensions to the template tag to allow the CSRF token to be specific to the form for extra security.
     26
     27Simon's patch is currently in progress.
    1628
    1729== Background ==
     
    2638For the record, the most relevant discussion on django-developers are here, most recent first, but this page attempts to supersede them all by gathering the conclusions.
    2739
     40 * Discussion of this proposal -  http://groups.google.com/group/django-developers/browse_thread/thread/3d2dc750082103dc/f3beb18c27fb7152?lnk=gst
    2841 * Discussion of #9977 patch csrf_template_tag_14.diff - http://groups.google.com/group/django-developers/browse_thread/thread/c23f556b88cedbc7?pli=1
    2942 * Luke Plant's proposal - http://groups.google.com/group/django-developers/browse_thread/thread/ae525f270ed46933/c338b050f43741b2?lnk=gst
     
    5972The main contenders are:
    6073
    61  * 'HMAC of session identifier'.  This was provided by Django 1.0 in !CsrfMiddleware and in 1.1 in !CsrfViewMiddleware, and is referred to as the 'CSRF token'.  All incoming POST requests that have an active session are required to have a CSRF token that is a hash of the session identifier and the site's SECRET_KEY.
     74 * 'HMAC of session identifier'.  This was provided by Django 1.0 in !CsrfMiddleware and in 1.1 in !CsrfViewMiddleware, and is referred to as the 'CSRF token'.  All incoming POST requests that have an active session are required to have a CSRF token that is a hash of the session identifier and the site's SECRET_KEY.  (Technically Django 1.1 used straight MD5, not HMAC-MD5).
    6275 
    6376 * 'session independent nonce'.  A random value is stored in a cookie, unique to every user, and POST forms must contain the same value as a token.
     
    7386   * Modifying ``!HttpResponse.content`` can have nasty side effects and interactions with other middleware e.g. see #9163.
    7487
    75  * !SafeForm - a Django Form subclass that adds the token.  Proposal abandoned in favour of template tag for various reasons.
     88 * !SafeForm - a Django Form subclass that adds the token.  Proposal abandoned in favour of template tag for various reasons, especially:
     89   * requires a different API to the normal Django Form which is confusing and brings many complications.
     90   * much more invasive changes requires to switch to it, making it a big barrier for users.
    7691
    77  * Template tag - templates that need the CSRF token need to insert them manually using `{% load csrf %}` at the top and `{% csrf_token %}` in every `<form>`.  This also requires use of a context processor (usually via !RequestContext).  This method is assumed for the rest of the document.
     92 * Template tag - templates that need the CSRF token need to insert them manually using `{% csrf_token %}` in every `<form>` that is internal (and using `{% load csrf %}` at the top if the template tag is not a builtin).  This also requires use of a context processor (usually via !RequestContext).  This method is assumed for the rest of the document. 
    7893
    7994== Evaluation ==
     
    130145== Proposal ==
    131146
    132 (by Luke Plant)
     147(initially by Luke Plant)
    133148
    134149CSRF protection should be done by the following method:
     
    140155   * with a backwards compatible !CsrfResponseMiddleware which can be used at the same time as the template tag, to allow people to upgrade without upgrading all their apps.
    141156 * The middleware should be enabled by default.
     157 * A decorator derived from the middleware is added to all contrib views that need it, so there is protection even if the middleware is turned off.
    142158
    143159Compared to Django 1.1:
     
    147163 * some cross sub-domain vulnerabilities are opened up - '''regression'''.  This is deemed an acceptable compromise, since there were already cross sub-domain vulnerabilities, and giving sub-domains to untrusted parties seems to be increasingly uncommon as most people buy their own domains.  It also isn't possible to fix cross sub-domain session fixing without changes to browsers, so it is safer simply to say that sub-domains should only be given to trusted parties.
    148164 * for HTTPS connections, adding strict Referer checking closes the other vulnerabilities opened up by the change from 'HMAC of session identifier' to 'session independent nonce' (i.e.  CSRF + MITM under HTTPS)
    149  * there are required upgrade steps for contrib apps (including the admin) to continue working.  Given the fact that without CSRF protection enabled by default ticket #510 is a security bug that should not be considered closed, and the fact that the CSRF protection provided in Django 1.1 is considered inadequate (due to its performance and security problems), I think this is acceptable.
    150165
    151166This proposal is implemented in the lp-csrf_rework branch in http://bitbucket.org/spookylukey/django-trunk-lukeplant/ (with patches regularly copied to #9977).  It includes fixes to all the contrib apps and documentation.
Back to Top