From 2dc8d249326458b4d70e0cf1efbc956dccb12d3f Mon Sep 17 00:00:00 2001 From: Elrond Date: Tue, 4 Oct 2011 00:12:03 +0200 Subject: [PATCH] Some mostly cosmetic changes to CSRF * 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 | 2 +- mediagoblin/middleware/csrf.py | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/mediagoblin/config_spec.ini b/mediagoblin/config_spec.ini index 298a6951..900957ce 100644 --- a/mediagoblin/config_spec.ini +++ b/mediagoblin/config_spec.ini @@ -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") diff --git a/mediagoblin/middleware/csrf.py b/mediagoblin/middleware/csrf.py index 44b799d5..7a5e352e 100644 --- a/mediagoblin/middleware/csrf.py +++ b/mediagoblin/middleware/csrf.py @@ -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 -- 2.25.1