Skip to content

Commit

Permalink
Add audit IDs to revocation events
Browse files Browse the repository at this point in the history 
The revoked tokens' audit ID is now included in the data returned in
the revocation list.

Closes-Bug: 1490804
Change-Id: Ifcf88f1158bebddc4f927121fbf4136fb53b659f
  • Loading branch information
Brant Knudson committed Dec 17, 2015
1 parent 4c3071d commit d5378f1
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 15 deletions.
39 changes: 26 additions & 13 deletions keystone/tests/unit/test_backend.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4124,7 +4124,9 @@ def test_token_crud(self):
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')
Expand Down Expand Up @@ -4159,7 +4161,8 @@ def create_token_sample_data(self, token_id=None, tenant_id=None,
# 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:
Expand All @@ -4168,7 +4171,7 @@ def create_token_sample_data(self, token_id=None, tenant_id=None,
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.
Expand Down Expand Up @@ -4334,17 +4337,21 @@ def test_null_expires_token(self):

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)
Expand All @@ -4356,7 +4363,7 @@ def delete_token(self):
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']
Expand Down Expand Up @@ -4407,12 +4414,16 @@ def test_revocation_list_cache(self):
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)
Expand Down Expand Up @@ -4447,7 +4458,8 @@ def test_revocation_list_cache(self):
def _test_predictable_revoked_pki_token_id(self, hash_fn):
token_id = self._create_token_id()
token_id_hash = hash_fn(token_id.encode('utf-8')).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)
Expand All @@ -4469,7 +4481,8 @@ def test_predictable_revoked_pki_token_id_sha256(self):

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)
Expand Down
3 changes: 2 additions & 1 deletion keystone/tests/unit/test_backend_sql.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ def test_token_revocation_list_uses_right_columns(self):
# 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()
Expand Down
9 changes: 9 additions & 0 deletions keystone/token/persistence/backends/kvs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,15 @@ def _add_to_revocation_list(self, data, lock):
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
Expand Down
12 changes: 11 additions & 1 deletion keystone/token/persistence/backends/sql.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,23 @@ def list_revoked_tokens(self):
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
Expand Down
13 changes: 13 additions & 0 deletions releasenotes/notes/bug-1490804-de58a9606edb31eb.yaml
View file
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit d5378f1

Please sign in to comment.