From 1492fe9728a7ffc7429e6c3f5adeb83af138ffa7 Mon Sep 17 00:00:00 2001 From: Nick Daly Date: Sat, 23 Mar 2013 19:59:20 -0500 Subject: [PATCH] Unify authentication errors. Give the same error if the username doesn't exist or if the password is wrong. If we deliver separate errors, we tell the attacker whether they've picked a valid password or not. Also, if username doesn't exist, hash the password anyway to avoid this timing side-channel attack: 1. Invalid Username: A. User tries to log in with invalid username. B. User name is not found in database. C. Password is never hashed. 2. Invalid Password: A. User tries to log in with valid username. B. User name is found in database. C. Password is hashed. Given that proper password hashing will take a minute, *not* hashing the password takes so much less time that we've effectively indicated to the attacker that the username didn't exist, regardless of the error message. This way, no such error occurs. --- modules/installed/lib/auth.py | 44 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/modules/installed/lib/auth.py b/modules/installed/lib/auth.py index 988f8dd9d..4aa5be5c6 100644 --- a/modules/installed/lib/auth.py +++ b/modules/installed/lib/auth.py @@ -11,6 +11,7 @@ import cherrypy import urllib, hashlib import cfg +import random cfg.session_key = '_cp_username' @@ -18,29 +19,28 @@ def check_credentials(username, passphrase): """Verifies credentials for username and passphrase. Returns None on success or a string describing the error on failure""" + start = time.clock() + + if not username or not passphrase: + error = "No username or password." + cfg.log(error) + return error + u = cfg.users[username] - if u is None: - cfg.log("Unknown user: %s" % username) - return u"Username %s is unknown to me." % username - if u['passphrase'] != hashlib.md5(passphrase).hexdigest(): - return u"Incorrect passphrase." + elif u is None: + # hash the password whether the user exists, to foil timing + # side-channel attacks + hashlib.md5(passphrase).hexdigest() + error = "Bad user-name or password." + elif u['passphrase'] != hashlib.md5(passphrase).hexdigest(): + error = "Bad user-name or password." + else: + error = None -def check_auth(*args, **kwargs): - """A tool that looks in config for 'auth.require'. If found and it - is not None, a login is required and the entry is evaluated as a - list of conditions that the user must fulfill""" - conditions = cherrypy.request.config.get('auth.require', None) - if conditions is not None: - username = cherrypy.session.get(cfg.session_key) - if username: - cherrypy.request.login = username - for condition in conditions: - # A condition is just a callable that returns true or false - if not condition(): - raise cherrypy.HTTPRedirect("/auth/login") - else: - raise cherrypy.HTTPRedirect("/auth/login") + if error: + cfg.log(error) + return error def check_auth(*args, **kwargs): """A tool that looks in config for 'auth.require'. If found and it @@ -60,8 +60,8 @@ def check_auth(*args, **kwargs): raise cherrypy.HTTPRedirect("/auth/login?from_page=%s" % get_params) else: # Send old page as from_page parameter - raise cherrypy.HTTPRedirect("/auth/login?from_page=%s" % get_params) - + raise cherrypy.HTTPRedirect("/auth/login?from_page=%s" % get_params) + cherrypy.tools.auth = cherrypy.Tool('before_handler', check_auth) def require(*conditions):