CVE-2012-3426: Permissions, Privileges, and Access Controls (CWE-264)
value: OpenStack Keystone before 2012.1.1, as used in OpenStack Folsom before Folsom-1 and OpenStack Essex, does not properly implement token expiration, which allows remote authenticated users to bypass intended authorization restrictions by (1) creating new tokens through token chaining, (2) leveraging possession of a token for a disabled user account, or (3) leveraging possession of a token for an account with a changed password.
CWE description: Weaknesses in this category are related to the management of permissions, privileges, and other security features that are used to perform access control.
Published: 2012-07-31T10:45Z
Repository: https://github.com/openstack/keystone
Language: Python
Files changed: keystone/identity/core.py, keystone/service.py, tests/test_keystoneclient.py
Commit date: 2012-05-16 23:46:48 +0100
Commit message: Invalidate user tokens when a user is disabled

Fixes Bug 997194

Delete valid tokens for a user when they have been disabled

Moved logic to delete tokens into update_user, as this can be called
directly form the REST API.

Also checks if a user is enabled when creating a token from another
token, this helps in cases there the backend didn't support listing of
tokens (and as a result weren't deleted)

Change-Id: Ib5ed73a7873bfa66ef31bf6d0f0322f50e677688

Diff: diff --git a/keystone/identity/core.py b/keystone/identity/core.py
index 5efd142e..83b1798e 100644
--- a/keystone/identity/core.py
+++ b/keystone/identity/core.py
@@ -406,10 +406,21 @@ class UserController(wsgi.Application):
         self.assert_admin(context)
         if self.identity_api.get_user(context, user_id) is None:
             raise exception.UserNotFound(user_id=user_id)
 
         user_ref = self.identity_api.update_user(context, user_id, user)
+
+        # If the password was changed or the user was disabled we clear tokens
+        if user.get('password') or user.get('enabled', True) == False:
+            try:
+                for token_id in self.token_api.list_tokens(context, user_id):
+                    self.token_api.delete_token(context, token_id)
+            except exception.NotImplemented:
+                # The users status has been changed but tokens remain valid for
+                # backends that can't list tokens for users
+                LOG.warning('User %s status has changed, but existing tokens '
+                            'remain valid' % user_id)
         return {'user': user_ref}
 
     def delete_user(self, context, user_id):
         self.assert_admin(context)
         if self.identity_api.get_user(context, user_id) is None:
@@ -419,20 +430,11 @@ class UserController(wsgi.Application):
 
     def set_user_enabled(self, context, user_id, user):
         return self.update_user(context, user_id, user)
 
     def set_user_password(self, context, user_id, user):
-        user_ref = self.update_user(context, user_id, user)
-        try:
-            for token_id in self.token_api.list_tokens(context, user_id):
-                self.token_api.delete_token(context, token_id)
-        except exception.NotImplemented:
-            # The password has been changed but tokens remain valid for
-            # backends that can't list tokens for users
-            LOG.warning('Password changed for %s, but existing tokens remain '
-                        'valid' % user_id)
-        return user_ref
+        return self.update_user(context, user_id, user)
 
     def update_user_tenant(self, context, user_id, user):
         """Update the default tenant."""
         # ensure that we're a member of that tenant
         tenant_id = user.get('tenantId')
diff --git a/keystone/service.py b/keystone/service.py
index 1cb455dd..2b024f4e 100644
--- a/keystone/service.py
+++ b/keystone/service.py
@@ -26,10 +26,13 @@ from keystone import token
 from keystone.common import logging
 from keystone.common import utils
 from keystone.common import wsgi
 
 
+LOG = logging.getLogger(__name__)
+
+
 class AdminRouter(wsgi.ComposingRouter):
     def __init__(self):
         mapper = routes.Mapper()
 
         version_controller = VersionController('admin')
@@ -273,11 +276,12 @@ class TokenController(wsgi.Application):
                                                            tenant_id=tenant_id)
                 (user_ref, tenant_ref, metadata_ref) = auth_info
 
                 # If the user is disabled don't allow them to authenticate
                 if not user_ref.get('enabled', True):
-                    raise exception.Forbidden(message='User has been disabled')
+                    LOG.warning('User %s is disabled' % user_id)
+                    raise exception.Unauthorized()
             except AssertionError as e:
                 raise exception.Unauthorized(e.message)
 
             token_ref = self.token_api.create_token(
                     context, token_id, dict(id=token_id,
@@ -312,10 +316,18 @@ class TokenController(wsgi.Application):
             except exception.NotFound:
                 raise exception.Unauthorized()
 
             user_ref = old_token_ref['user']
 
+            # If the user is disabled don't allow them to authenticate
+            current_user_ref = self.identity_api.get_user(
+                                                    context=context,
+                                                    user_id=user_ref['id'])
+            if not current_user_ref.get('enabled', True):
+                LOG.warning('User %s is disabled' % user_ref['id'])
+                raise exception.Unauthorized()
+
             tenants = self.identity_api.get_tenants_for_user(context,
                                                              user_ref['id'])
             if tenant_id:
                 assert tenant_id in tenants
 
diff --git a/tests/test_keystoneclient.py b/tests/test_keystoneclient.py
index a02af87c..37f877f2 100644
--- a/tests/test_keystoneclient.py
+++ b/tests/test_keystoneclient.py
@@ -307,10 +307,27 @@ class KeystoneClientTests(object):
         # authenticate with a token should not work after a password change
         self.assertRaises(client_exceptions.Unauthorized,
                           client.tokens.authenticate,
                           token=token_id)
 
+    def test_disable_user_invalidates_token(self):
+        from keystoneclient import exceptions as client_exceptions
+
+        admin_client = self.get_client(admin=True)
+        foo_client = self.get_client(self.user_foo)
+
+        admin_client.users.update_enabled(user=self.user_foo['id'],
+                                          enabled=False)
+
+        self.assertRaises(client_exceptions.Unauthorized,
+                          foo_client.tokens.authenticate,
+                          token=foo_client.auth_token)
+
+        self.assertRaises(client_exceptions.Unauthorized,
+                          self.get_client,
+                          self.user_foo)
+
     def test_user_create_update_delete(self):
         from keystoneclient import exceptions as client_exceptions
 
         test_username = 'new_user'
         client = self.get_client(admin=True)
@@ -330,11 +347,11 @@ class KeystoneClientTests(object):
         # NOTE(termie): update_enabled doesn't return anything, probably a bug
         client.users.update_enabled(user=user, enabled=False)
         user = client.users.get(user.id)
         self.assertFalse(user.enabled)
 
-        self.assertRaises(client_exceptions.AuthorizationFailure,
+        self.assertRaises(client_exceptions.Unauthorized,
                   self._client,
                   username=test_username,
                   password='password')
         client.users.update_enabled(user, True)
 
@@ -878,11 +895,11 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
         # NOTE(termie): update_enabled doesn't return anything, probably a bug
         client.users.update_enabled(user=user, enabled=False)
         user = client.users.get(user.id)
         self.assertFalse(user.enabled)
 
-        self.assertRaises(client_exceptions.AuthorizationFailure,
+        self.assertRaises(client_exceptions.Unauthorized,
                   self._client,
                   username=test_username,
                   password='password')
         client.users.update_enabled(user, True)
 

Vulnerable code:         user_ref = self.update_user(context, user_id, user)
        try:
            for token_id in self.token_api.list_tokens(context, user_id):
                self.token_api.delete_token(context, token_id)
        except exception.NotImplemented:
            # The password has been changed but tokens remain valid for
            # backends that can't list tokens for users
            LOG.warning('Password changed for %s, but existing tokens remain '
                        'valid' % user_id)
        return user_ref
                    raise exception.Forbidden(message='User has been disabled')
        self.assertRaises(client_exceptions.AuthorizationFailure,
        self.assertRaises(client_exceptions.AuthorizationFailure,
Fixed code: 
        # If the password was changed or the user was disabled we clear tokens
        if user.get('password') or user.get('enabled', True) == False:
            try:
                for token_id in self.token_api.list_tokens(context, user_id):
                    self.token_api.delete_token(context, token_id)
            except exception.NotImplemented:
                # The users status has been changed but tokens remain valid for
                # backends that can't list tokens for users
                LOG.warning('User %s status has changed, but existing tokens '
                            'remain valid' % user_id)
        return self.update_user(context, user_id, user)
LOG = logging.getLogger(__name__)


                    LOG.warning('User %s is disabled' % user_id)
                    raise exception.Unauthorized()
            # If the user is disabled don't allow them to authenticate
            current_user_ref = self.identity_api.get_user(
                                                    context=context,
                                                    user_id=user_ref['id'])
            if not current_user_ref.get('enabled', True):
                LOG.warning('User %s is disabled' % user_ref['id'])
                raise exception.Unauthorized()

    def test_disable_user_invalidates_token(self):
        from keystoneclient import exceptions as client_exceptions

        admin_client = self.get_client(admin=True)
        foo_client = self.get_client(self.user_foo)

        admin_client.users.update_enabled(user=self.user_foo['id'],
                                          enabled=False)

        self.assertRaises(client_exceptions.Unauthorized,
                          foo_client.tokens.authenticate,
                          token=foo_client.auth_token)

        self.assertRaises(client_exceptions.Unauthorized,
                          self.get_client,
                          self.user_foo)

        self.assertRaises(client_exceptions.Unauthorized,
        self.assertRaises(client_exceptions.Unauthorized,