[RFC PATCH] auth: detect password hashing algorithm when storing and checking passwords

[Andrew Shadura]

Hi,

I was thinking about unifying the hashing algorithms we use on different
systems, and here's one of the ways of attacking the problem.

I don't know anything about the status of bcrypt on Windows. Dominik, could
you please verify what is it? Does bcrypt work at all on Windows? Does it
work effeciently enough? Are there any issues?

This approach has a downside: users will continue to use SHA256 until they
change their password, so if the database leaks, attackers may check
(unsalted) hashes against known popular password hashes.

Another way to drop SHA256 would be to remove support for SHA256 authentication
completely and add a bit of code to force users to change their passwords to
passwords with different SHA256 hashes.

Any thoughts on this?

# HG changeset patch
# User Andrew Shadura <andrew at shadura.me>
# Date 1491588381 -7200
#      Fri Apr 07 20:06:21 2017 +0200
# Node ID 1f5519aa30fbdd0f1d1ffe33bd6b66539198a5eb
# Parent  1ae319cb41b1b69ba5c3c1f9d782876b2e367922
auth: detect password hashing algorithm when storing and checking passwords

Detect the availabity of bcrypt and default to using it on all platforms.
When checking passwords, detect the hashing algorithms used to store the
password hach and check appropriately:

 - bcrypt hash must start with $2a$ or $2b$
 - SHA256 hashes are 64 characters long

This allows for a gradual transition to bcrypt or a stronger hash in the future.

diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py
--- a/kallithea/lib/auth.py
+++ b/kallithea/lib/auth.py
@@ -56,8 +56,14 @@ from kallithea.lib.utils import get_repo
     get_user_group_slug, conditional_cache
 from kallithea.lib.caching_query import FromCache
 
+log = logging.getLogger(__name__)
 
-log = logging.getLogger(__name__)
+try:
+    import bcrypt
+    default_hash_algo = 'bcrypt'
+except ImportError:
+    log.warning('bcrypt not available, using less secure SHA256 password hashing algorithm')
+    default_hash_algo = 'sha256'
 
 
 class PasswordGenerator(object):
@@ -99,33 +105,32 @@ def get_crypt_password(password):
 
     :param password: password to hash
     """
-    if is_windows:
-        return hashlib.sha256(password).hexdigest()
-    elif is_unix:
-        import bcrypt
+    if default_hash_algo == 'bcrypt':
         return bcrypt.hashpw(safe_str(password), bcrypt.gensalt(10))
     else:
-        raise Exception('Unknown or unsupported platform %s' \
-                        % __platform__)
+        return hashlib.sha256(password).hexdigest()
 
 
 def check_password(password, hashed):
     """
     Checks matching password with it's hashed value, runs different
-    implementation based on platform it runs on
+    implementation depending on what the hashed data looks like.
+
+    Bcrypt should be prefixed by one of $2a$, $2b$ or $2y$, with
+    Python bcrypt implementation generating only $2a$ and $2b$.
+
+    SHA256 hashes are always 64 characters long.
 
     :param password: password
     :param hashed: password in hashed form
     """
 
-    if is_windows:
+    if hashed.startswith(('$2a$', '$2b$')):
+        return bcrypt.checkpw(safe_str(password), safe_str(hashed))
+    elif len(hashed) == 64:
         return hashlib.sha256(password).hexdigest() == hashed
-    elif is_unix:
-        import bcrypt
-        return bcrypt.checkpw(safe_str(password), safe_str(hashed))
     else:
-        raise Exception('Unknown or unsupported platform %s' \
-                        % __platform__)
+        raise Exception('Unknown or unsupported hash algorithm')
 
 
 def _cached_perms_data(user_id, user_is_admin, user_inherit_default_permissions,