bcrypt the passwords

keystone/common/utils.py

@@ -25,9 +25,16 @@ import subprocess
 import sys
 import urllib
 
+import bcrypt
+
+from keystone import config
 from keystone.common import logging
 
 
+CONF = config.CONF
+config.register_int('bcrypt_strength', default=12)
+
+
 def import_class(import_str):
     """Returns a class from a string including module and class."""
     mod_str, _sep, class_str = import_str.rpartition('.')
@@ -134,6 +141,24 @@ class Ec2Signer(object):
         return b64
 
 
+def hash_password(password):
+    """Hash a password. Hard."""
+    salt = bcrypt.gensalt(CONF.bcrypt_strength)
+    return bcrypt.hashpw(password, salt)
+
+
+def check_password(password, hashed):
+    """Check that a plaintext password matches hashed.
+
+    Due to the way bcrypt works, hashing a password with the hashed
+    version of that password as salt will return the hashed version
+    of that password (mostly). Neat!
+
+    """
+    check = bcrypt.hashpw(password, hashed[:29])
+    return check == hashed
+
+
 # From python 2.7
 def check_output(*popenargs, **kwargs):
   r"""Run command with arguments and return its output as a byte string.

keystone/config.py

@@ -114,6 +114,18 @@ def register_cli_bool(*args, **kw):
     return conf.register_cli_opt(cfg.BoolOpt(*args, **kw), group=group)
 
 
+def register_int(*args, **kw):
+    conf = kw.pop('conf', CONF)
+    group = _ensure_group(kw, conf)
+    return conf.register_opt(cfg.IntOpt(*args, **kw), group=group)
+
+
+def register_cli_int(*args, **kw):
+    conf = kw.pop('conf', CONF)
+    group = _ensure_group(kw, conf)
+    return conf.register_cli_opt(cfg.IntOpt(*args, **kw), group=group)
+
+
 def _ensure_group(kw, conf):
     group = kw.pop('group', None)
     if group:

keystone/identity/backends/kvs.py

@@ -2,6 +2,7 @@
 
 from keystone import identity
 from keystone.common import kvs
+from keystone.common import utils
 
 
 def _filter_user(user_ref):
@@ -12,6 +13,13 @@ def _filter_user(user_ref):
     return user_ref
 
 
+def _ensure_hashed_password(user_ref):
+    pw = user_ref.get('password', None)
+    if pw is not None:
+        user_ref['password'] = utils.hash_password(pw)
+    return user_ref
+
+
 class Identity(kvs.Base, identity.Driver):
     # Public interface
     def authenticate(self, user_id=None, tenant_id=None, password=None):
@@ -24,7 +32,8 @@ class Identity(kvs.Base, identity.Driver):
         user_ref = self._get_user(user_id)
         tenant_ref = None
         metadata_ref = None
-        if not user_ref or user_ref.get('password') != password:
+        if (not user_ref
+            or not utils.check_password(password, user_ref.get('password'))):
             raise AssertionError('Invalid user / password')
         if tenant_id and tenant_id not in user_ref['tenants']:
             raise AssertionError('Invalid tenant')
@@ -78,15 +87,13 @@ class Identity(kvs.Base, identity.Driver):
         user_ref = self._get_user(user_id)
         tenants = set(user_ref.get('tenants', []))
         tenants.add(tenant_id)
-        user_ref['tenants'] = list(tenants)
-        self.update_user(user_id, user_ref)
+        self.update_user(user_id, {'tenants': list(tenants)})
 
     def remove_user_from_tenant(self, tenant_id, user_id):
         user_ref = self._get_user(user_id)
         tenants = set(user_ref.get('tenants', []))
         tenants.remove(tenant_id)
-        user_ref['tenants'] = list(tenants)
-        self.update_user(user_id, user_ref)
+        self.update_user(user_id, {'tenants': list(tenants)})
 
     def get_tenants_for_user(self, user_id):
         user_ref = self._get_user(user_id)
@@ -118,6 +125,7 @@ class Identity(kvs.Base, identity.Driver):
 
     # CRUD
     def create_user(self, user_id, user):
+        user = _ensure_hashed_password(user)
         self.db.set('user-%s' % user_id, user)
         self.db.set('user_name-%s' % user['name'], user)
         user_list = set(self.db.get('user_list', []))
@@ -128,10 +136,13 @@ class Identity(kvs.Base, identity.Driver):
     def update_user(self, user_id, user):
         # get the old name and delete it too
         old_user = self.db.get('user-%s' % user_id)
+        new_user = old_user.copy()
+        user = _ensure_hashed_password(user)
+        new_user.update(user)
         self.db.delete('user_name-%s' % old_user['name'])
-        self.db.set('user-%s' % user_id, user)
-        self.db.set('user_name-%s' % user['name'], user)
-        return user
+        self.db.set('user-%s' % user_id, new_user)
+        self.db.set('user_name-%s' % new_user['name'], new_user)
+        return new_user
 
     def delete_user(self, user_id):
         old_user = self.db.get('user-%s' % user_id)

keystone/identity/backends/sql.py

@@ -2,6 +2,7 @@
 
 from keystone import identity
 from keystone.common import sql
+from keystone.common import utils
 from keystone.common.sql import migration
 
 
@@ -11,6 +12,13 @@ def _filter_user(user_ref):
     return user_ref
 
 
+def _ensure_hashed_password(user_ref):
+    pw = user_ref.get('password', None)
+    if pw is not None:
+        user_ref['password'] = utils.hash_password(pw)
+    return user_ref
+
+
 class User(sql.ModelBase, sql.DictBase):
     __tablename__ = 'user'
     id = sql.Column(sql.String(64), primary_key=True)
@@ -106,7 +114,8 @@ class Identity(sql.Base, identity.Driver):
         user_ref = self._get_user(user_id)
         tenant_ref = None
         metadata_ref = None
-        if not user_ref or user_ref.get('password') != password:
+        if (not user_ref
+            or not utils.check_password(password, user_ref.get('password'))):
             raise AssertionError('Invalid user / password')
 
         tenants = self.get_tenants_for_user(user_id)
@@ -246,6 +255,7 @@ class Identity(sql.Base, identity.Driver):
 
     # CRUD
     def create_user(self, user_id, user):
+        user = _ensure_hashed_password(user)
         session = self.get_session()
         with session.begin():
             user_ref = User.from_dict(user)
@@ -258,6 +268,7 @@ class Identity(sql.Base, identity.Driver):
         with session.begin():
             user_ref = session.query(User).filter_by(id=user_id).first()
             old_user_dict = user_ref.to_dict()
+            user = _ensure_hashed_password(user)
             for k in user:
                 old_user_dict[k] = user[k]
             new_user = User.from_dict(old_user_dict)

keystone/identity/core.py

@@ -334,13 +334,10 @@ class UserController(wsgi.Application):
             self.identity_api.add_user_to_tenant(tenant_id, user_id)
         return {'user': new_user_ref}
 
-    # NOTE(termie): this is really more of a patch than a put
     def update_user(self, context, user_id, user):
+        # NOTE(termie): this is really more of a patch than a put
         self.assert_admin(context)
-        user_ref = self.identity_api._get_user(context, user_id)
-        del user['id']
-        user_ref.update(user)
-        self.identity_api.update_user(context, user_id, user_ref)
+        user_ref = self.identity_api.update_user(context, user_id, user)
         return {'user': user_ref}
 
     def delete_user(self, context, user_id):

keystone/test.py

@@ -107,6 +107,11 @@ class TestCase(unittest.TestCase):
 
     def setUp(self):
         super(TestCase, self).setUp()
+        self.config()
+
+    def config(self):
+        CONF(config_files=[etcdir('keystone.conf'),
+                           testsdir('test_overrides.conf')])
 
     def tearDown(self):
         for path in self._paths:
@@ -137,10 +142,11 @@ class TestCase(unittest.TestCase):
         for user in fixtures.USERS:
             user_copy = user.copy()
             tenants = user_copy.pop('tenants')
-            rv = self.identity_api.create_user(user['id'], user_copy)
+            rv = self.identity_api.create_user(user['id'], user_copy.copy())
             for tenant_id in tenants:
                 self.identity_api.add_user_to_tenant(tenant_id, user['id'])
-            setattr(self, 'user_%s' % user['id'], rv)
+            setattr(self, 'user_%s' % user['id'], user_copy)
+            print user_copy
 
         for role in fixtures.ROLES:
             rv = self.identity_api.create_role(role['id'], role)

tests/test_backend.py

@@ -45,6 +45,11 @@ class IdentityTests(object):
     self.assertDictEquals(tenant_ref, self.tenant_bar)
     self.assertDictEquals(metadata_ref, self.metadata_foobar)
 
+  def test_password_hashed(self):
+    user_ref = self.identity_api._get_user(self.user_foo['id'])
+    self.assertNotEqual(user_ref['password'], self.user_foo['password'])
+
+
   def test_get_tenant_bad_tenant(self):
     tenant_ref = self.identity_api.get_tenant(
         tenant_id=self.tenant_bar['id'] + 'WRONG')

tests/test_keystoneclient.py

@@ -45,7 +45,6 @@ class KcMasterTestCase(CompatTestCase):
         from keystoneclient.v2_0 import client as ks_client
         reload(ks_client)
 
-        self._config()
         self.public_app = self.loadapp('keystone', name='main')
         self.admin_app = self.loadapp('keystone', name='admin')
 
@@ -63,10 +62,6 @@ class KcMasterTestCase(CompatTestCase):
             self.user_foo['id'], self.tenant_bar['id'],
             dict(roles=['keystone_admin'], is_admin='1'))
 
-    def _config(self):
-        CONF(config_files=[test.etcdir('keystone.conf'),
-                           test.testsdir('test_overrides.conf')])
-
     def get_client(self, user_ref=None, tenant_ref=None):
         if user_ref is None:
             user_ref = self.user_foo
@@ -87,10 +82,9 @@ class KcMasterTestCase(CompatTestCase):
         self.assertEquals(tenants[0].id, self.tenant_bar['id'])
 
     def test_authenticate_tenant_id_and_tenants(self):
-        client = self._client(username='FOO',
-                              password='foo2',
+        client = self._client(username=self.user_foo['name'],
+                              password=self.user_foo['password'],
                               tenant_id='bar')
-
         tenants = client.tenants.list()
         self.assertEquals(tenants[0].id, self.tenant_bar['id'])
 

tests/test_keystoneclient_sql.py

@@ -11,7 +11,7 @@ CONF = config.CONF
 
 
 class KcMasterSqlTestCase(test_keystoneclient.KcMasterTestCase):
-    def _config(self):
+    def config(self):
         CONF(config_files=[test.etcdir('keystone.conf'),
                            test.testsdir('test_overrides.conf'),
                            test.testsdir('backend_sql.conf')])

tests/test_overrides.conf

@@ -1,2 +1,5 @@
+[DEFAULT]
+bcrypt_strength = 2
+
 [catalog]
 template_file = default_catalog.templates

tools/pip-requires

@@ -7,5 +7,6 @@ routes
 pycli
 sqlalchemy
 sqlalchemy-migrate
+py-bcrypt
 
 -e git+https://github.com/openstack/python-keystoneclient.git#egg=python-keystoneclient