Changeset - 9a02f9ef28d7
[Not reviewed]
stable
0 7 0
Mads Kiilerich - 10 years ago 2015-07-07 02:09:35
madski@unity3d.com
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.
7 files changed with 13 insertions and 23 deletions:
0 comments (0 inline, 0 general)
kallithea/controllers/admin/my_account.py
Show inline comments
 
@@ -251,22 +251,22 @@ class MyAccountController(BaseController
 
        description = request.POST.get('description')
 
        ApiKeyModel().create(self.authuser.user_id, description, lifetime)
 
        Session().commit()
 
        h.flash(_("Api key successfully created"), category='success')
 
        return redirect(url('my_account_api_keys'))
 

	
 
    def my_account_api_keys_delete(self):
 
        api_key = request.POST.get('del_api_key')
 
        user_id = self.authuser.user_id
 
        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')
 
        elif api_key:
 
            ApiKeyModel().delete(api_key, self.authuser.user_id)
 
            Session().commit()
 
            h.flash(_("Api key successfully deleted"), category='success')
 

	
 
        return redirect(url('my_account_api_keys'))
kallithea/controllers/admin/users.py
Show inline comments
 
@@ -315,25 +315,25 @@ class UsersController(BaseController):
 
        return redirect(url('edit_user_api_keys', id=c.user.user_id))
 

	
 
    def delete_api_key(self, id):
 
        c.user = User.get_or_404(id)
 
        if c.user.username == User.DEFAULT_USER:
 
            h.flash(_("You can't edit this user"), category='warning')
 
            return redirect(url('users'))
 

	
 
        api_key = request.POST.get('del_api_key')
 
        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')
 
        elif api_key:
 
            ApiKeyModel().delete(api_key, c.user.user_id)
 
            Session().commit()
 
            h.flash(_("Api key successfully deleted"), category='success')
 

	
 
        return redirect(url('edit_user_api_keys', id=c.user.user_id))
 

	
 
    def update_account(self, id):
 
        pass
kallithea/lib/dbmigrate/schema/db_1_2_0.py
Show inline comments
 
@@ -327,25 +327,25 @@ class User(Base, BaseModel):
 

	
 
    @classmethod
 
    def create(cls, form_data):
 
        from kallithea.lib.auth import get_crypt_password
 

	
 
        try:
 
            new_user = cls()
 
            for k, v in form_data.items():
 
                if k == 'password':
 
                    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
 
        except:
 
            log.error(traceback.format_exc())
 
            Session.rollback()
 
            raise
 

	
 
class UserLog(Base, BaseModel):
 
    __tablename__ = 'user_logs'
 
    __table_args__ = {'extend_existing':True}
 
    user_log_id = Column("user_log_id", Integer(), nullable=False, unique=True, default=None, primary_key=True)
kallithea/lib/utils2.py
Show inline comments
 
@@ -23,26 +23,28 @@ Original author and date, and relevant c
 
:author: marcink
 
:copyright: (c) 2013 RhodeCode GmbH, and others.
 
:license: GPLv3, see LICENSE.md for more details.
 
"""
 

	
 

	
 
import os
 
import re
 
import sys
 
import time
 
import uuid
 
import datetime
 
import urllib
 
import binascii
 

	
 
import webob
 
import urllib
 
import urlobject
 

	
 
from pylons.i18n.translation import _, ungettext
 
from kallithea.lib.vcs.utils.lazy import LazyProperty
 
from kallithea.lib.compat import json
 

	
 

	
 
def __get_lem():
 
    """
 
    Get language extension map based on what's inside pygments lexers
 
    """
 
    from pygments import lexers
 
@@ -152,41 +154,29 @@ def detect_mode(line, default):
 
    :return: value of line end on of 0 - Unix, 1 - Mac, 2 - DOS
 
    """
 
    if line.endswith('\r\n'):
 
        return 2
 
    elif line.endswith('\n'):
 
        return 0
 
    elif line.endswith('\r'):
 
        return 1
 
    else:
 
        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):
 
    """
 
    Returns int() of val if val is not convertable to int use default
 
    instead
 

	
 
    :param val:
 
    :param default:
 
    """
 

	
 
    try:
kallithea/model/api_key.py
Show inline comments
 
@@ -41,25 +41,25 @@ log = logging.getLogger(__name__)
 
class ApiKeyModel(BaseModel):
 
    cls = UserApiKeys
 

	
 
    def create(self, user, description, lifetime=-1):
 
        """
 
        :param user: user or user_id
 
        :param description: description of ApiKey
 
        :param lifetime: expiration time in seconds
 
        """
 
        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
 
        Session().add(new_api_key)
 

	
 
        return new_api_key
 

	
 
    def delete(self, api_key, user=None):
 
        """
 
        Deletes given api_key, if user is set it also filters the object for
 
        deletion by given user.
 
        """
kallithea/model/user.py
Show inline comments
 
@@ -91,25 +91,25 @@ class UserModel(BaseModel):
 
        # raises UserCreationError if it's not allowed
 
        check_allowed_create_user(user_data, cur_user)
 
        from kallithea.lib.auth import get_crypt_password
 

	
 
        new_user = User()
 
        for k, v in form_data.items():
 
            if k == 'password':
 
                v = get_crypt_password(v)
 
            if k == 'firstname':
 
                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)
 
        return new_user
 

	
 
    def create_or_update(self, username, password, email, firstname='',
 
                         lastname='', active=True, admin=False,
 
                         extern_type=None, extern_name=None, cur_user=None):
 
        """
 
        Creates a new instance if not found, or updates current one
 

	
 
        :param username:
 
@@ -150,25 +150,25 @@ class UserModel(BaseModel):
 

	
 
        try:
 
            new_user.username = username
 
            new_user.admin = admin
 
            new_user.email = email
 
            new_user.active = active
 
            new_user.extern_name = safe_unicode(extern_name) if extern_name else None
 
            new_user.extern_type = safe_unicode(extern_type) if extern_type else None
 
            new_user.name = firstname
 
            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,
 
                                                            new_user.password)
 
            if not edit or password_change:
 
                reason = 'new password' if edit else 'new user'
 
                log.debug('Updating password reason=>%s' % (reason,))
 
                new_user.password = get_crypt_password(password) if password else None
 

	
 
            self.sa.add(new_user)
 

	
 
            if not edit:
kallithea/tests/functional/test_login.py
Show inline comments
 
@@ -251,25 +251,25 @@ class TestLoginController(TestController
 
        username = 'test_password_reset_1'
 
        password = 'qweqwe'
 
        email = 'marcin@python-works.com'
 
        name = 'passwd'
 
        lastname = 'reset'
 

	
 
        new = User()
 
        new.username = username
 
        new.password = password
 
        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()
 

	
 
        response = self.app.post(url(controller='login',
 
                                     action='password_reset'),
 
                                 {'email': email, })
 

	
 
        self.checkSessionFlash(response, 'Your password reset link was sent')
 

	
 
        response = response.follow()
 

	
 
        # BAD KEY
0 comments (0 inline, 0 general)