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/token/backends/kvs.py, keystone/token/backends/sql.py, keystone/token/core.py, tests/test_keystoneclient.py
Commit date: 2012-05-11 15:29:39 +0100
Commit message: Invalidate user tokens when password is changed

Fixes bug 996595

This commit will cause all valid tokens to be deleted for a user
who's password is changed (implemented for the sql and kvs backends)

Change-Id: I6ad7da8957b7041983a3fc91d9ba9368667d06ac

Diff: diff --git a/keystone/identity/core.py b/keystone/identity/core.py
index ee225264..5efd142e 100644
--- a/keystone/identity/core.py
+++ b/keystone/identity/core.py
@@ -22,16 +22,19 @@ import urlparse
 
 from keystone import config
 from keystone import exception
 from keystone import policy
 from keystone import token
+from keystone.common import logging
 from keystone.common import manager
 from keystone.common import wsgi
 
 
 CONF = config.CONF
 
+LOG = logging.getLogger(__name__)
+
 
 class Manager(manager.Manager):
     """Default pivot point for the Identity backend.
 
     See :mod:`keystone.common.manager.Manager` for more details on how this
@@ -416,11 +419,20 @@ 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):
-        return self.update_user(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
 
     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/token/backends/kvs.py b/keystone/token/backends/kvs.py
index 71b86f8e..b033dd31 100644
--- a/keystone/token/backends/kvs.py
+++ b/keystone/token/backends/kvs.py
@@ -42,5 +42,20 @@ class Token(kvs.Base, token.Driver):
     def delete_token(self, token_id):
         try:
             return self.db.delete('token-%s' % token_id)
         except KeyError:
             raise exception.TokenNotFound(token_id=token_id)
+
+    def list_tokens(self, user_id):
+        tokens = []
+        now = datetime.datetime.utcnow()
+        for token, user_ref in self.db.items():
+            if not token.startswith('token-'):
+                continue
+            if 'user' not in user_ref:
+                continue
+            if user_ref['user'].get('id') != user_id:
+                continue
+            if user_ref.get('expires') and user_ref.get('expires') < now:
+                continue
+            tokens.append(token.split('-', 1)[1])
+        return tokens
diff --git a/keystone/token/backends/sql.py b/keystone/token/backends/sql.py
index 7a9a5515..543f9628 100644
--- a/keystone/token/backends/sql.py
+++ b/keystone/token/backends/sql.py
@@ -79,5 +79,19 @@ class Token(sql.Base, token.Driver):
             raise exception.TokenNotFound(token_id=token_id)
 
         with session.begin():
             session.delete(token_ref)
             session.flush()
+
+    def list_tokens(self, user_id):
+        session = self.get_session()
+        tokens = []
+        now = datetime.datetime.utcnow()
+        for token_ref in session.query(TokenModel)\
+                                      .filter(TokenModel.expires > now):
+            token_ref_dict = token_ref.to_dict()
+            if 'user' not in token_ref_dict:
+                continue
+            if token_ref_dict['user'].get('id') != user_id:
+                continue
+            tokens.append(token_ref['id'])
+        return tokens
diff --git a/keystone/token/core.py b/keystone/token/core.py
index 0a28e382..0d1101db 100644
--- a/keystone/token/core.py
+++ b/keystone/token/core.py
@@ -85,10 +85,20 @@ class Driver(object):
         :raises: keystone.exception.TokenNotFound
 
         """
         raise exception.NotImplemented()
 
+    def list_tokens(self, user_id):
+        """Returns a list of current token_id's for a user
+
+        :param user_id: identity of the user
+        :type user_id: string
+        :returns: list of token_id's
+
+        """
+        raise exception.NotImplemented()
+
     def _get_default_expire_time(self):
         """Determine when a token should expire based on the config.
 
         :returns: a naive utc datetime.datetime object
 
diff --git a/tests/test_keystoneclient.py b/tests/test_keystoneclient.py
index 064a8e34..a02af87c 100644
--- a/tests/test_keystoneclient.py
+++ b/tests/test_keystoneclient.py
@@ -284,10 +284,33 @@ class KeystoneClientTests(object):
         self.assertRaises(client_exceptions.Unauthorized,
                           self._client,
                           username='blah',
                           password='blah')
 
+    def test_change_password_invalidates_token(self):
+        from keystoneclient import exceptions as client_exceptions
+
+        client = self.get_client(admin=True)
+
+        username = uuid.uuid4().hex
+        passwd = uuid.uuid4().hex
+        user = client.users.create(name=username, password=passwd,
+                                   email=uuid.uuid4().hex)
+
+        token_id = client.tokens.authenticate(username=username,
+                                              password=passwd).id
+
+        # authenticate with a token should work before a password change
+        client.tokens.authenticate(token=token_id)
+
+        client.users.update_password(user=user.id, password=uuid.uuid4().hex)
+
+        # authenticate with a token should not work after a password change
+        self.assertRaises(client_exceptions.Unauthorized,
+                          client.tokens.authenticate,
+                          token=token_id)
+
     def test_user_create_update_delete(self):
         from keystoneclient import exceptions as client_exceptions
 
         test_username = 'new_user'
         client = self.get_client(admin=True)

Vulnerable code:         return self.update_user(context, user_id, user)
Fixed code: from keystone.common import logging
LOG = logging.getLogger(__name__)

        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

    def list_tokens(self, user_id):
        tokens = []
        now = datetime.datetime.utcnow()
        for token, user_ref in self.db.items():
            if not token.startswith('token-'):
                continue
            if 'user' not in user_ref:
                continue
            if user_ref['user'].get('id') != user_id:
                continue
            if user_ref.get('expires') and user_ref.get('expires') < now:
                continue
            tokens.append(token.split('-', 1)[1])
        return tokens

    def list_tokens(self, user_id):
        session = self.get_session()
        tokens = []
        now = datetime.datetime.utcnow()
        for token_ref in session.query(TokenModel)\
                                      .filter(TokenModel.expires > now):
            token_ref_dict = token_ref.to_dict()
            if 'user' not in token_ref_dict:
                continue
            if token_ref_dict['user'].get('id') != user_id:
                continue
            tokens.append(token_ref['id'])
        return tokens
    def list_tokens(self, user_id):
        """Returns a list of current token_id's for a user

        :param user_id: identity of the user
        :type user_id: string
        :returns: list of token_id's

        """
        raise exception.NotImplemented()

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

        client = self.get_client(admin=True)

        username = uuid.uuid4().hex
        passwd = uuid.uuid4().hex
        user = client.users.create(name=username, password=passwd,
                                   email=uuid.uuid4().hex)

        token_id = client.tokens.authenticate(username=username,
                                              password=passwd).id

        # authenticate with a token should work before a password change
        client.tokens.authenticate(token=token_id)

        client.users.update_password(user=user.id, password=uuid.uuid4().hex)

        # authenticate with a token should not work after a password change
        self.assertRaises(client_exceptions.Unauthorized,
                          client.tokens.authenticate,
                          token=token_id)