Some mostly cosmetic changes to CSRF
authorElrond <elrond+mediagoblin.org@samba-tng.org>
Mon, 3 Oct 2011 22:12:03 +0000 (00:12 +0200)
committerElrond <elrond+mediagoblin.org@samba-tng.org>
Fri, 14 Oct 2011 19:46:17 +0000 (21:46 +0200)
* remove max_age - A session cookie is better, because it's
  a session thing, really.
* Call the cookie mediagoblin_csrftoken, much clearer.
* Use the SCRIPT_NAME for the path of the cookie, so that
  the cookie is sent back to the right place only.
  Alternatively the path= parameter could be removed, so
  that it defaults to '/'.
* call the randomness function only once, instead of twice.
  64 bits should be enough. If really more bits are needed,
  increase the number.
* Just give the number as cookie. No point in md5 and
  hexdigest in my view (those functions just make another
  representation).
* getrandbits gets a bit count directly, simpler API

mediagoblin/config_spec.ini
mediagoblin/middleware/csrf.py

index 298a6951632774d615e9bb7957292ba6ce27f3e3..900957cee4b23b3741659c5bc256b06142a1e881 100644 (file)
@@ -42,7 +42,7 @@ celery_setup_elsewhere = boolean(default=False)
 allow_attachments = boolean(default=False)
 
 # Cookie stuff
-csrf_cookie_name = string(default='mediagoblin_nonce')
+csrf_cookie_name = string(default='mediagoblin_csrftoken')
 
 [storage:publicstore]
 storage_class = string(default="mediagoblin.storage.filestorage:BasicFileStorage")
index 44b799d54f42e42d0bd511e872f00508d919188d..7a5e352e8e88a808431a9d5cf6faf34cfea8101c 100644 (file)
@@ -25,9 +25,9 @@ from mediagoblin import mg_globals
 # Use the system (hardware-based) random number generator if it exists.
 # -- this optimization is lifted from Django
 if hasattr(random, 'SystemRandom'):
-    randrange = random.SystemRandom().randrange
+    getrandbits = random.SystemRandom().getrandbits
 else:
-    randrange = random.randrange
+    getrandbits = random.getrandbits
 
 
 class CsrfForm(Form):
@@ -54,7 +54,7 @@ class CsrfMiddleware(object):
     and matches the form token for non-safe requests.
     """
 
-    MAX_CSRF_KEY = 2 << 63
+    CSRF_KEYLEN = 64
     SAFE_HTTP_METHODS = ("GET", "HEAD", "OPTIONS", "TRACE")
 
     def __init__(self, mg_app):
@@ -92,9 +92,8 @@ class CsrfMiddleware(object):
         response.set_cookie(
             mg_globals.app_config['csrf_cookie_name'],
             request.environ['CSRF_TOKEN'],
-            max_age=60 * 60 * 24 * 7 * 52,
-            path='/',
-            domain=mg_globals.app_config.get('csrf_cookie_domain', None),
+            path=request.environ['SCRIPT_NAME'],
+            domain=mg_globals.app_config.get('csrf_cookie_domain'),
             secure=(request.scheme.lower() == 'https'),
             httponly=True)
 
@@ -104,9 +103,7 @@ class CsrfMiddleware(object):
     def _make_token(self, request):
         """Generate a new token to use for CSRF protection."""
 
-        return hashlib.md5("%s%s" %
-                           (randrange(0, self.MAX_CSRF_KEY),
-                            randrange(0, self.MAX_CSRF_KEY))).hexdigest()
+        return "%s" % (getrandbits(self.CSRF_KEYLEN),)
 
     def verify_tokens(self, request):
         """Verify that the CSRF Cookie exists and that it matches the