From efb6b3fca0ba0ad768b3e803a324043095d326e2 Mon Sep 17 00:00:00 2001 From: Dolph Mathews Date: Fri, 7 Sep 2012 14:35:21 -0500 Subject: 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: Iecf891f274b67408f568b949a7028362c4c30312 --- 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 c9c496c2..085a3dd1 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -590,6 +590,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} @@ -614,7 +616,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): @@ -657,6 +659,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} @@ -684,3 +688,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 d6e9d38d..d3c1b67a 100644 --- a/keystone/token/core.py +++ b/keystone/token/core.py @@ -39,6 +39,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.""" @@ -106,6 +110,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 944ca790..f42c3e70 100644 --- a/tests/test_keystoneclient.py +++ b/tests/test_keystoneclient.py @@ -794,15 +794,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 @@ -1001,16 +1001,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']): @@ -1018,11 +1018,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]) -- cgit