From bff03b5726fe5cac93d44a66715eea49b89c8cb0 Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Tue, 1 Dec 2015 11:09:14 -0600 Subject: [PATCH] Add audit IDs to revocation events The revoked tokens' audit ID is now included in the data returned in the revocation list. Closes-Bug: 1490804 Change-Id: Ifcf88f1158bebddc4f927121fbf4136fb53b659f (cherry picked from commit d5378f173da14a34ca010271477337879002d6d0) Conflicts: keystone/tests/unit/test_backend.py --- keystone/tests/unit/test_backend.py | 39 ++++++++++++++-------- keystone/tests/unit/test_backend_sql.py | 3 +- keystone/token/persistence/backends/kvs.py | 9 +++++ keystone/token/persistence/backends/sql.py | 12 ++++++- .../notes/bug-1490804-de58a9606edb31eb.yaml | 13 ++++++++ 5 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/bug-1490804-de58a9606edb31eb.yaml diff --git a/keystone/tests/unit/test_backend.py b/keystone/tests/unit/test_backend.py index 2340645..1273736 100644 --- a/keystone/tests/unit/test_backend.py +++ b/keystone/tests/unit/test_backend.py @@ -4426,7 +4426,9 @@ class TokenTests(object): token_id = self._create_token_id() data = {'id': token_id, 'a': 'b', 'trust_id': None, - 'user': {'id': 'testuserid'}} + 'user': {'id': 'testuserid'}, + 'token_data': {'access': {'token': { + 'audit_ids': [uuid.uuid4().hex]}}}} data_ref = self.token_provider_api._persistence.create_token(token_id, data) expires = data_ref.pop('expires') @@ -4461,7 +4463,8 @@ class TokenTests(object): # FIXME(morganfainberg): These tokens look nothing like "Real" tokens. # This should be fixed when token issuance is cleaned up. data = {'id': token_id, 'a': 'b', - 'user': {'id': user_id}} + 'user': {'id': user_id}, + 'access': {'token': {'audit_ids': [uuid.uuid4().hex]}}} if tenant_id is not None: data['tenant'] = {'id': tenant_id, 'name': tenant_id} if tenant_id is NULL_OBJECT: @@ -4470,7 +4473,7 @@ class TokenTests(object): data['expires'] = expires if trust_id is not None: data['trust_id'] = trust_id - data.setdefault('access', {}).setdefault('trust', {}) + data['access'].setdefault('trust', {}) # Testuserid2 is used here since a trustee will be different in # the cases of impersonation and therefore should not match the # token's user_id. @@ -4633,17 +4636,21 @@ class TokenTests(object): self.assertEqual(data_ref, new_data_ref) - def check_list_revoked_tokens(self, token_ids): - revoked_ids = [x['id'] - for x in self.token_provider_api.list_revoked_tokens()] + def check_list_revoked_tokens(self, token_infos): + revocation_list = self.token_provider_api.list_revoked_tokens() + revoked_ids = [x['id'] for x in revocation_list] + revoked_audit_ids = [x['audit_id'] for x in revocation_list] self._assert_revoked_token_list_matches_token_persistence(revoked_ids) - for token_id in token_ids: + for token_id, audit_id in token_infos: self.assertIn(token_id, revoked_ids) + self.assertIn(audit_id, revoked_audit_ids) def delete_token(self): token_id = uuid.uuid4().hex + audit_id = uuid.uuid4().hex data = {'id_hash': token_id, 'id': token_id, 'a': 'b', - 'user': {'id': 'testuserid'}} + 'user': {'id': 'testuserid'}, + 'token_data': {'token': {'audit_ids': [audit_id]}}} data_ref = self.token_provider_api._persistence.create_token(token_id, data) self.token_provider_api._persistence.delete_token(token_id) @@ -4655,7 +4662,7 @@ class TokenTests(object): exception.TokenNotFound, self.token_provider_api._persistence.delete_token, data_ref['id']) - return token_id + return (token_id, audit_id) def test_list_revoked_tokens_returns_empty_list(self): revoked_ids = [x['id'] @@ -4706,12 +4713,16 @@ class TokenTests(object): token_data = {'id_hash': token_id, 'id': token_id, 'a': 'b', 'expires': expire_time, 'trust_id': None, - 'user': {'id': 'testuserid'}} + 'user': {'id': 'testuserid'}, + 'token_data': {'token': { + 'audit_ids': [uuid.uuid4().hex]}}} token2_id = uuid.uuid4().hex token2_data = {'id_hash': token2_id, 'id': token2_id, 'a': 'b', 'expires': expire_time, 'trust_id': None, - 'user': {'id': 'testuserid'}} + 'user': {'id': 'testuserid'}, + 'token_data': {'token': { + 'audit_ids': [uuid.uuid4().hex]}}} # Create 2 Tokens. self.token_provider_api._persistence.create_token(token_id, token_data) @@ -4746,7 +4757,8 @@ class TokenTests(object): def _test_predictable_revoked_pki_token_id(self, hash_fn): token_id = self._create_token_id() token_id_hash = hash_fn(token_id).hexdigest() - token = {'user': {'id': uuid.uuid4().hex}} + token = {'user': {'id': uuid.uuid4().hex}, + 'token_data': {'token': {'audit_ids': [uuid.uuid4().hex]}}} self.token_provider_api._persistence.create_token(token_id, token) self.token_provider_api._persistence.delete_token(token_id) @@ -4768,7 +4780,8 @@ class TokenTests(object): def test_predictable_revoked_uuid_token_id(self): token_id = uuid.uuid4().hex - token = {'user': {'id': uuid.uuid4().hex}} + token = {'user': {'id': uuid.uuid4().hex}, + 'token_data': {'token': {'audit_ids': [uuid.uuid4().hex]}}} self.token_provider_api._persistence.create_token(token_id, token) self.token_provider_api._persistence.delete_token(token_id) diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index 69fac63..51221a3 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -492,7 +492,8 @@ class SqlToken(SqlTests, test_backend.TokenTests): # necessary. expected_query_args = (token_sql.TokenModel.id, - token_sql.TokenModel.expires) + token_sql.TokenModel.expires, + token_sql.TokenModel.extra,) with mock.patch.object(token_sql, 'sql') as mock_sql: tok = token_sql.Token() diff --git a/keystone/token/persistence/backends/kvs.py b/keystone/token/persistence/backends/kvs.py index 5193158..60f7931 100644 --- a/keystone/token/persistence/backends/kvs.py +++ b/keystone/token/persistence/backends/kvs.py @@ -210,6 +210,15 @@ class Token(token.persistence.TokenDriverV8): subsecond=True) revoked_token_data['id'] = data['id'] + token_data = data['token_data'] + if 'access' in token_data: + # It's a v2 token. + audit_ids = token_data['access']['token']['audit_ids'] + else: + # It's a v3 token. + audit_ids = token_data['token']['audit_ids'] + revoked_token_data['audit_id'] = audit_ids[0] + token_list = self._get_key_or_default(self.revocation_key, default=[]) if not isinstance(token_list, list): # NOTE(morganfainberg): In the case that the revocation list is not diff --git a/keystone/token/persistence/backends/sql.py b/keystone/token/persistence/backends/sql.py index 6fc1d22..d677620 100644 --- a/keystone/token/persistence/backends/sql.py +++ b/keystone/token/persistence/backends/sql.py @@ -228,13 +228,23 @@ class Token(token.persistence.TokenDriverV8): session = sql.get_session() tokens = [] now = timeutils.utcnow() - query = session.query(TokenModel.id, TokenModel.expires) + query = session.query(TokenModel.id, TokenModel.expires, + TokenModel.extra) query = query.filter(TokenModel.expires > now) token_references = query.filter_by(valid=False) for token_ref in token_references: + token_data = token_ref[2]['token_data'] + if 'access' in token_data: + # It's a v2 token. + audit_ids = token_data['access']['token']['audit_ids'] + else: + # It's a v3 token. + audit_ids = token_data['token']['audit_ids'] + record = { 'id': token_ref[0], 'expires': token_ref[1], + 'audit_id': audit_ids[0], } tokens.append(record) return tokens diff --git a/releasenotes/notes/bug-1490804-de58a9606edb31eb.yaml b/releasenotes/notes/bug-1490804-de58a9606edb31eb.yaml new file mode 100644 index 0000000..0d5c203 --- /dev/null +++ b/releasenotes/notes/bug-1490804-de58a9606edb31eb.yaml @@ -0,0 +1,13 @@ +--- +features: + - > + [`bug 1490804 `_] + Audit IDs are included in the token revocation list. +security: + - > + [`bug 1490804 `_] + [`CVE-2015-7546 `_] + A bug is fixed where an attacker could avoid token revocation when the PKI + or PKIZ token provider is used. The complete remediation for this + vulnerability requires the corresponding fix in the keystonemiddleware + project. -- 1.9.1