[openstack-keystone/f17] Delete user tokens after role grant/revoke (CVE-2012-4413)
Alan Pevec
apevec at fedoraproject.org
Wed Sep 12 17:56:05 UTC 2012
commit 0ac1bab8c403679219e089786eb8f29272cf133a
Author: Alan Pevec <apevec at redhat.com>
Date: Wed Sep 12 19:48:54 2012 +0200
Delete user tokens after role grant/revoke (CVE-2012-4413)
0005-List-tokens-for-memcached-backend.patch | 166 ++++++++++++++++++++
...elete-user-tokens-after-role-grant-revoke.patch | 142 +++++++++++++++++
openstack-keystone.spec | 9 +-
3 files changed, 316 insertions(+), 1 deletions(-)
---
diff --git a/0005-List-tokens-for-memcached-backend.patch b/0005-List-tokens-for-memcached-backend.patch
new file mode 100644
index 0000000..ab56729
--- /dev/null
+++ b/0005-List-tokens-for-memcached-backend.patch
@@ -0,0 +1,166 @@
+From fcede9b3700c5f98323333746d9ec54695828646 Mon Sep 17 00:00:00 2001
+From: Adam Young <ayoung at redhat.com>
+Date: Thu, 6 Sep 2012 11:54:04 -0400
+Subject: [PATCH] List tokens for memcached backend
+
+Creates and updates an index of tokens in a memcache entry keyed
+by the user id
+
+Bug 1046905
+
+Change-Id: I114810297009331f491dc069d667f358092f1e34
+---
+ keystone/token/backends/memcache.py | 23 ++++++++++++++++++-
+ tests/test_backend.py | 41 ++++++++++++++++++++++++++++++----
+ tests/test_backend_memcache.py | 17 ++++++++++++--
+ 3 files changed, 72 insertions(+), 9 deletions(-)
+
+diff --git a/keystone/token/backends/memcache.py b/keystone/token/backends/memcache.py
+index 91d547d..ae4ecc0 100644
+--- a/keystone/token/backends/memcache.py
++++ b/keystone/token/backends/memcache.py
+@@ -63,10 +63,31 @@ class Token(token.Driver):
+ expires_ts = utils.unixtime(data_copy['expires'])
+ kwargs['time'] = expires_ts
+ self.client.set(ptk, data_copy, **kwargs)
++ if 'id' in data['user']:
++ token_data = token_id
++ user_id = data['user']['id']
++ user_key = 'usertokens-%s' % user_id
++ if not self.client.append(user_key, ',%s' % token_data):
++ if not self.client.add(user_key, token_data):
++ if not self.client.append(user_key, ',%s' % token_data):
++ msg = _('Unable to add token user list.')
++ raise exception.UnexpectedError(msg)
+ return copy.deepcopy(data_copy)
+
+ def delete_token(self, token_id):
+ # Test for existence
+ self.get_token(token_id)
+ ptk = self._prefix_token_id(token_id)
+- return self.client.delete(ptk)
++ result = self.client.delete(ptk)
++ return result
++
++ def list_tokens(self, user_id):
++ tokens = []
++ user_record = self.client.get('usertokens-%s' % user_id) or ""
++ token_list = user_record.split(',')
++ for token_id in token_list:
++ ptk = self._prefix_token_id(token_id)
++ token = self.client.get(ptk)
++ if token:
++ tokens.append(token_id)
++ return tokens
+diff --git a/tests/test_backend.py b/tests/test_backend.py
+index 2914734..2a7ba4d 100644
+--- a/tests/test_backend.py
++++ b/tests/test_backend.py
+@@ -306,7 +306,8 @@ class IdentityTests(object):
+ class TokenTests(object):
+ def test_token_crud(self):
+ token_id = uuid.uuid4().hex
+- data = {'id': token_id, 'a': 'b'}
++ data = {'id': token_id, 'a': 'b',
++ 'user': {'id': 'testuserid'}}
+ data_ref = self.token_api.create_token(token_id, data)
+ expires = data_ref.pop('expires')
+ self.assertTrue(isinstance(expires, datetime.datetime))
+@@ -319,22 +320,52 @@ class TokenTests(object):
+
+ self.token_api.delete_token(token_id)
+ self.assertRaises(exception.TokenNotFound,
+- self.token_api.delete_token, token_id)
++ self.token_api.get_token, token_id)
+ self.assertRaises(exception.TokenNotFound,
+- self.token_api.get_token, token_id)
++ self.token_api.delete_token, token_id)
+
+ def test_expired_token(self):
+ token_id = uuid.uuid4().hex
+ expire_time = datetime.datetime.utcnow() - datetime.timedelta(minutes=1)
+- data = {'id': token_id, 'a': 'b', 'expires': expire_time}
++ data = {'id': token_id, 'a': 'b', 'expires': expire_time,
++ 'user': {'id': 'testuserid'}}
+ data_ref = self.token_api.create_token(token_id, data)
+ self.assertDictEquals(data_ref, data)
+ self.assertRaises(exception.TokenNotFound,
+ self.token_api.get_token, token_id)
+
++ def create_token_sample_data(self):
++ token_id = uuid.uuid4().hex
++ data = {'id': token_id, 'a': 'b',
++ 'user': {'id': 'testuserid'}}
++ self.token_api.create_token(token_id, data)
++ return token_id
++
++ def test_token_list(self):
++ tokens = self.token_api.list_tokens('testuserid')
++ self.assertEquals(len(tokens), 0)
++ token_id1 = self.create_token_sample_data()
++ tokens = self.token_api.list_tokens('testuserid')
++ self.assertEquals(len(tokens), 1)
++ self.assertIn(token_id1, tokens)
++ token_id2 = self.create_token_sample_data()
++ tokens = self.token_api.list_tokens('testuserid')
++ self.assertEquals(len(tokens), 2)
++ self.assertIn(token_id2, tokens)
++ self.assertIn(token_id1, tokens)
++ self.token_api.delete_token(token_id1)
++ tokens = self.token_api.list_tokens('testuserid')
++ self.assertIn(token_id2, tokens)
++ self.assertNotIn(token_id1, tokens)
++ self.token_api.delete_token(token_id2)
++ tokens = self.token_api.list_tokens('testuserid')
++ self.assertNotIn(token_id2, tokens)
++ self.assertNotIn(token_id1, tokens)
++
+ def test_null_expires_token(self):
+ token_id = uuid.uuid4().hex
+- data = {'id': token_id, 'a': 'b', 'expires': None}
++ data = {'id': token_id, 'id_hash': token_id, 'a': 'b', 'expires': None,
++ 'user': {'id': 'testuserid'}}
+ data_ref = self.token_api.create_token(token_id, data)
+ self.assertDictEquals(data_ref, data)
+ new_data_ref = self.token_api.get_token(token_id)
+diff --git a/tests/test_backend_memcache.py b/tests/test_backend_memcache.py
+index 2c07580..06f1c31 100644
+--- a/tests/test_backend_memcache.py
++++ b/tests/test_backend_memcache.py
+@@ -34,6 +34,18 @@ class MemcacheClient(object):
+ """Ignores the passed in args."""
+ self.cache = {}
+
++ def add(self, key, value):
++ if self.get(key):
++ return False
++ return self.set(key, value)
++
++ def append(self, key, value):
++ existing_value = self.get(key)
++ if existing_value:
++ self.set(key, existing_value + value)
++ return True
++ return False
++
+ def check_key(self, key):
+ if not isinstance(key, str):
+ raise memcache.Client.MemcachedStringEncodingError()
+@@ -45,8 +57,6 @@ class MemcacheClient(object):
+ now = time.mktime(datetime.datetime.utcnow().utctimetuple())
+ if obj and (obj[1] == 0 or obj[1] > now):
+ return obj[0]
+- else:
+- raise exception.TokenNotFound(token_id=key)
+
+ def set(self, key, value, time=0):
+ """Sets the value for a key."""
+@@ -71,6 +81,7 @@ class MemcacheToken(test.TestCase, test_backend.TokenTests):
+
+ def test_get_unicode(self):
+ token_id = unicode(uuid.uuid4().hex)
+- data = {'id': token_id, 'a': 'b'}
++ data = {'id': token_id, 'a': 'b',
++ 'user': {'id': 'testuserid'}}
+ self.token_api.create_token(token_id, data)
+ self.token_api.get_token(token_id)
diff --git a/0006-Delete-user-tokens-after-role-grant-revoke.patch b/0006-Delete-user-tokens-after-role-grant-revoke.patch
new file mode 100644
index 0000000..1b710f8
--- /dev/null
+++ b/0006-Delete-user-tokens-after-role-grant-revoke.patch
@@ -0,0 +1,142 @@
+From ea110beaa8a86fa4a0d11546b6dee4cf931950d7 Mon Sep 17 00:00:00 2001
+From: Dolph Mathews <dolph.mathews at gmail.com>
+Date: Fri, 7 Sep 2012 14:55:31 -0500
+Subject: [PATCH] Delete user tokens after role grant/revoke
+
+Delete user tokens when a new role is granted or revoked, in order to
+prevent old tokens to continue to be valid for the original set of
+roles for the remainder of the token's lifespan.
+
+Addresses CVE-2012-4413.
+Fixes bug 1041396.
+
+Change-Id: Ib11b5b3a933c6000afe0c875c3f71f1f101bb202
+---
+ keystone/identity/core.py | 7 ++++++-
+ keystone/token/core.py | 11 +++++++++++
+ tests/test_keystoneclient.py | 18 +++++++++---------
+ 3 files changed, 26 insertions(+), 10 deletions(-)
+
+diff --git a/keystone/identity/core.py b/keystone/identity/core.py
+index db3ce31..22df815 100644
+--- a/keystone/identity/core.py
++++ b/keystone/identity/core.py
+@@ -525,6 +525,8 @@ class RoleController(wsgi.Application):
+ self.identity_api.add_user_to_tenant(context, tenant_id, user_id)
+ self.identity_api.add_role_to_user_and_tenant(
+ context, user_id, tenant_id, role_id)
++ self.token_api.revoke_tokens(context, user_id)
++
+ role_ref = self.identity_api.get_role(context, role_id)
+ return {'role': role_ref}
+
+@@ -555,7 +557,7 @@ class RoleController(wsgi.Application):
+ if not roles:
+ self.identity_api.remove_user_from_tenant(
+ context, tenant_id, user_id)
+- return
++ self.token_api.revoke_tokens(context, user_id)
+
+ # COMPAT(diablo): CRUD extension
+ def get_role_refs(self, context, user_id):
+@@ -597,6 +599,8 @@ class RoleController(wsgi.Application):
+ self.identity_api.add_user_to_tenant(context, tenant_id, user_id)
+ self.identity_api.add_role_to_user_and_tenant(
+ context, user_id, tenant_id, role_id)
++ self.token_api.revoke_tokens(context, user_id)
++
+ role_ref = self.identity_api.get_role(context, role_id)
+ return {'role': role_ref}
+
+@@ -624,3 +628,4 @@ class RoleController(wsgi.Application):
+ if not roles:
+ self.identity_api.remove_user_from_tenant(
+ context, tenant_id, user_id)
++ self.token_api.revoke_tokens(context, user_id)
+diff --git a/keystone/token/core.py b/keystone/token/core.py
+index 0d1101d..a0c89e2 100644
+--- a/keystone/token/core.py
++++ b/keystone/token/core.py
+@@ -38,6 +38,10 @@ class Manager(manager.Manager):
+ def __init__(self):
+ super(Manager, self).__init__(CONF.token.driver)
+
++ def revoke_tokens(self, context, user_id):
++ for token_id in self.list_tokens(context, user_id):
++ self.delete_token(context, token_id)
++
+
+ class Driver(object):
+ """Interface description for a Token driver."""
+@@ -97,6 +101,13 @@ class Driver(object):
+ """
+ raise exception.NotImplemented()
+
++ def revoke_tokens(self, user_id):
++ """Invalidates all tokens held by a user.
++
++ :raises: keystone.exception.UserNotFound
++ """
++ raise exception.NotImplemented()
++
+ def _get_default_expire_time(self):
+ """Determine when a token should expire based on the config.
+
+diff --git a/tests/test_keystoneclient.py b/tests/test_keystoneclient.py
+index 0f6f628..6e45790 100644
+--- a/tests/test_keystoneclient.py
++++ b/tests/test_keystoneclient.py
+@@ -769,15 +769,15 @@ class KcMasterTestCase(CompatTestCase, KeystoneClientTests):
+ def test_tenant_add_and_remove_user(self):
+ client = self.get_client(admin=True)
+ client.roles.add_user_role(tenant=self.tenant_baz['id'],
+- user=self.user_foo['id'],
++ user=self.user_two['id'],
+ role=self.role_useless['id'])
+ user_refs = client.tenants.list_users(tenant=self.tenant_baz['id'])
+- self.assert_(self.user_foo['id'] in [x.id for x in user_refs])
++ self.assert_(self.user_two['id'] in [x.id for x in user_refs])
+ client.roles.remove_user_role(tenant=self.tenant_baz['id'],
+- user=self.user_foo['id'],
++ user=self.user_two['id'],
+ role=self.role_useless['id'])
+ user_refs = client.tenants.list_users(tenant=self.tenant_baz['id'])
+- self.assert_(self.user_foo['id'] not in [x.id for x in user_refs])
++ self.assert_(self.user_two['id'] not in [x.id for x in user_refs])
+
+ def test_user_role_add_404(self):
+ from keystoneclient import exceptions as client_exceptions
+@@ -890,16 +890,16 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
+ def test_tenant_add_and_remove_user(self):
+ client = self.get_client(admin=True)
+ client.roles.add_user_to_tenant(tenant_id=self.tenant_baz['id'],
+- user_id=self.user_foo['id'],
++ user_id=self.user_two['id'],
+ role_id=self.role_useless['id'])
+ role_refs = client.roles.get_user_role_refs(
+- user_id=self.user_foo['id'])
++ user_id=self.user_two['id'])
+ self.assert_(self.tenant_baz['id'] in [x.tenantId for x in role_refs])
+
+ # get the "role_refs" so we get the proper id, this is how the clients
+ # do it
+ roleref_refs = client.roles.get_user_role_refs(
+- user_id=self.user_foo['id'])
++ user_id=self.user_two['id'])
+ for roleref_ref in roleref_refs:
+ if (roleref_ref.roleId == self.role_useless['id']
+ and roleref_ref.tenantId == self.tenant_baz['id']):
+@@ -907,11 +907,11 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
+ break
+
+ client.roles.remove_user_from_tenant(tenant_id=self.tenant_baz['id'],
+- user_id=self.user_foo['id'],
++ user_id=self.user_two['id'],
+ role_id=roleref_ref.id)
+
+ role_refs = client.roles.get_user_role_refs(
+- user_id=self.user_foo['id'])
++ user_id=self.user_two['id'])
+ self.assert_(self.tenant_baz['id'] not in
+ [x.tenantId for x in role_refs])
+
diff --git a/openstack-keystone.spec b/openstack-keystone.spec
index 127a5a3..2d6ba98 100644
--- a/openstack-keystone.spec
+++ b/openstack-keystone.spec
@@ -9,7 +9,7 @@
Name: openstack-keystone
Version: 2012.1.2
-Release: 2%{?dist}
+Release: 3%{?dist}
Summary: OpenStack Identity Service
License: ASL 2.0
@@ -27,6 +27,8 @@ Patch0001: 0001-fix-man-page-build.patch
Patch0002: 0002-fix-sphinx-warnings.patch
Patch0003: 0003-match-egg-and-spec-requires.patch
Patch0004: 0004-Require-authz-to-update-user-s-tenant-bug-1040626.patch
+Patch0005: 0005-List-tokens-for-memcached-backend.patch
+Patch0006: 0006-Delete-user-tokens-after-role-grant-revoke.patch
BuildArch: noarch
BuildRequires: python2-devel
@@ -114,6 +116,8 @@ This package contains documentation for Keystone.
%patch0002 -p1
%patch0003 -p1
%patch0004 -p1
+%patch0005 -p1
+%patch0006 -p1
find . \( -name .gitignore -o -name .placeholder \) -delete
find keystone -name \*.py -exec sed -i '/\/usr\/bin\/env python/d' {} \;
@@ -251,6 +255,9 @@ fi
%endif
%changelog
+* Wed Sep 12 2012 Alan Pevec <apevec at redhat.com> 2012.1.2-3
+- Delete user tokens after role grant/revoke (CVE-2012-4413)
+
* Thu Aug 30 2012 Alan Pevec <apevec at redhat.com> 2012.1.2-2
- Require authz to update user's tenant (CVE-2012-3542)
More information about the scm-commits
mailing list