changeset 5217:9a02f9ef28d7 stable

utils: make API key generator more random The API key generator abused temporary filenames in what seems to be an attempt of creating keys that unambiguously specified the user and thus were unique across users. A final hashing did however remove that property. More importantly, tempfile is not documented to use secure random numbers ... and it only uses 6 characters, giving approximately 36 bits of entropy. Instead, use the cryptographically secure os.urandom directly to generate keys with the same length but with the full 160 bits of entropy. Reported and fixed by Andrew Bartlett.
author Mads Kiilerich <madski@unity3d.com>
date Tue, 07 Jul 2015 02:09:35 +0200
parents 3e81e6534cad
children c0da0ef508da
files kallithea/controllers/admin/my_account.py kallithea/controllers/admin/users.py kallithea/lib/dbmigrate/schema/db_1_2_0.py kallithea/lib/utils2.py kallithea/model/api_key.py kallithea/model/user.py kallithea/tests/functional/test_login.py
diffstat 7 files changed, 13 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/controllers/admin/my_account.py	Tue Jul 07 02:09:35 2015 +0200
+++ b/kallithea/controllers/admin/my_account.py	Tue Jul 07 02:09:35 2015 +0200
@@ -260,7 +260,7 @@
         if request.POST.get('del_api_key_builtin'):
             user = User.get(user_id)
             if user:
-                user.api_key = generate_api_key(user.username)
+                user.api_key = generate_api_key()
                 Session().add(user)
                 Session().commit()
                 h.flash(_("Api key successfully reset"), category='success')
--- a/kallithea/controllers/admin/users.py	Tue Jul 07 02:09:35 2015 +0200
+++ b/kallithea/controllers/admin/users.py	Tue Jul 07 02:09:35 2015 +0200
@@ -324,7 +324,7 @@
         if request.POST.get('del_api_key_builtin'):
             user = User.get(c.user.user_id)
             if user:
-                user.api_key = generate_api_key(user.username)
+                user.api_key = generate_api_key()
                 Session().add(user)
                 Session().commit()
                 h.flash(_("Api key successfully reset"), category='success')
--- a/kallithea/lib/dbmigrate/schema/db_1_2_0.py	Tue Jul 07 02:09:35 2015 +0200
+++ b/kallithea/lib/dbmigrate/schema/db_1_2_0.py	Tue Jul 07 02:09:35 2015 +0200
@@ -336,7 +336,7 @@
                     v = get_crypt_password(v)
                 setattr(new_user, k, v)
 
-            new_user.api_key = generate_api_key(form_data['username'])
+            new_user.api_key = generate_api_key()
             Session.add(new_user)
             Session.commit()
             return new_user
--- a/kallithea/lib/utils2.py	Tue Jul 07 02:09:35 2015 +0200
+++ b/kallithea/lib/utils2.py	Tue Jul 07 02:09:35 2015 +0200
@@ -32,8 +32,10 @@
 import time
 import uuid
 import datetime
+import urllib
+import binascii
+
 import webob
-import urllib
 import urlobject
 
 from pylons.i18n.translation import _, ungettext
@@ -161,23 +163,11 @@
         return default
 
 
-def generate_api_key(username, salt=None):
+def generate_api_key():
     """
-    Generates unique API key for given username, if salt is not given
-    it'll be generated from some random string
-
-    :param username: username as string
-    :param salt: salt to hash generate KEY
-    :rtype: str
-    :returns: sha1 hash from username+salt
+    Generates a random (presumably unique) API key.
     """
-    from tempfile import _RandomNameSequence
-    import hashlib
-
-    if salt is None:
-        salt = _RandomNameSequence().next()
-
-    return hashlib.sha1(username + salt).hexdigest()
+    return binascii.hexlify(os.urandom(20))
 
 
 def safe_int(val, default=None):
--- a/kallithea/model/api_key.py	Tue Jul 07 02:09:35 2015 +0200
+++ b/kallithea/model/api_key.py	Tue Jul 07 02:09:35 2015 +0200
@@ -50,7 +50,7 @@
         user = self._get_user(user)
 
         new_api_key = UserApiKeys()
-        new_api_key.api_key = generate_api_key(user.username)
+        new_api_key.api_key = generate_api_key()
         new_api_key.user_id = user.user_id
         new_api_key.description = description
         new_api_key.expires = time.time() + (lifetime * 60) if lifetime != -1 else -1
--- a/kallithea/model/user.py	Tue Jul 07 02:09:35 2015 +0200
+++ b/kallithea/model/user.py	Tue Jul 07 02:09:35 2015 +0200
@@ -100,7 +100,7 @@
                 k = 'name'
             setattr(new_user, k, v)
 
-        new_user.api_key = generate_api_key(form_data['username'])
+        new_user.api_key = generate_api_key()
         self.sa.add(new_user)
 
         log_create_user(new_user.get_dict(), cur_user)
@@ -159,7 +159,7 @@
             new_user.lastname = lastname
 
             if not edit:
-                new_user.api_key = generate_api_key(username)
+                new_user.api_key = generate_api_key()
 
             # set password only if creating an user or password is changed
             password_change = new_user.password and not check_password(password,
--- a/kallithea/tests/functional/test_login.py	Tue Jul 07 02:09:35 2015 +0200
+++ b/kallithea/tests/functional/test_login.py	Tue Jul 07 02:09:35 2015 +0200
@@ -260,7 +260,7 @@
         new.email = email
         new.name = name
         new.lastname = lastname
-        new.api_key = generate_api_key(username)
+        new.api_key = generate_api_key()
         Session().add(new)
         Session().commit()