| From bff03b5726fe5cac93d44a66715eea49b89c8cb0 Mon Sep 17 00:00:00 2001 |
| From: Brant Knudson <bknudson@us.ibm.com> |
| 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 <https://bugs.launchpad.net/keystone/+bug/1490804>`_] |
| + Audit IDs are included in the token revocation list. |
| +security: |
| + - > |
| + [`bug 1490804 <https://bugs.launchpad.net/keystone/+bug/1490804>`_] |
| + [`CVE-2015-7546 <http://cve.mitre.org/cgi-bin/cvename.cgi?name=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 |
| |
