From e051d9d6d69174cfbf3b4ebea30c7f0b20fd90e5 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Thu, 16 Apr 2020 20:35:46 -0700 Subject: [PATCH 3/3] Ensure OAuth1 authorized roles are respected Without this patch, when an OAuth1 request token is authorized with a limited set of roles, the roles for the access token are ignored when the user uses it to request a keystone token. Since they are ignored in the access token, they are also ignored when used to create an EC2 credential. In both cases, a user of an access token can use it to escallate their role assignments beyond what was authorized by the creator. This patch fixes the issue by ensuring the token model accounts for an OAuth1-scoped token and correctly populating the roles for it, as well as ensuring that an EC2 token request also respects the roles in the access token. Change-Id: I02f9836fbd4d7e629653977fc341476cfd89859e Closes-bug: #1873290 Related-bug: #1872735 --- keystone/api/_shared/EC2_S3_Resource.py | 12 +- keystone/api/credentials.py | 11 +- keystone/models/token_model.py | 18 +++ keystone/tests/unit/test_v3_credential.py | 149 ++++++++++++++++++ keystone/tests/unit/test_v3_oauth1.py | 13 ++ .../notes/bug-1873290-ff7f8e4cee15b75a.yaml | 19 +++ 6 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/bug-1873290-ff7f8e4cee15b75a.yaml diff --git a/keystone/api/_shared/EC2_S3_Resource.py b/keystone/api/_shared/EC2_S3_Resource.py index 5a4a0f1b4..287ada2bb 100644 --- a/keystone/api/_shared/EC2_S3_Resource.py +++ b/keystone/api/_shared/EC2_S3_Resource.py @@ -84,7 +84,8 @@ class ResourceBase(ks_flask.ResourceBase): access=loaded.get('access'), secret=loaded.get('secret'), trust_id=loaded.get('trust_id'), - app_cred_id=loaded.get('app_cred_id') + app_cred_id=loaded.get('app_cred_id'), + access_token_id=loaded.get('access_token_id') ) # validate the signature @@ -103,6 +104,7 @@ class ResourceBase(ks_flask.ResourceBase): raise ks_exceptions.Unauthorized from e trustee_user_id = None + auth_context = None if cred_data['trust_id']: trust = PROVIDERS.trust_api.get_trust(cred_data['trust_id']) roles = [r['id'] for r in trust['roles']] @@ -120,6 +122,11 @@ class ResourceBase(ks_flask.ResourceBase): app_cred = ac_client.get_application_credential( cred_data['app_cred_id']) roles = [r['id'] for r in app_cred['roles']] + elif cred_data['access_token_id']: + access_token = PROVIDERS.oauth_api.get_access_token( + cred_data['access_token_id']) + roles = jsonutils.loads(access_token['role_ids']) + auth_context = {'access_token_id': cred_data['access_token_id']} else: roles = PROVIDERS.assignment_api.get_roles_for_user_and_project( user_ref['id'], project_ref['id']) @@ -141,5 +148,6 @@ class ResourceBase(ks_flask.ResourceBase): user_id=user_id, method_names=method_names, project_id=project_ref['id'], trust_id=cred_data['trust_id'], - app_cred_id=cred_data['app_cred_id']) + app_cred_id=cred_data['app_cred_id'], + auth_context=auth_context) return token diff --git a/keystone/api/credentials.py b/keystone/api/credentials.py index d477ed864..9a88620ff 100644 --- a/keystone/api/credentials.py +++ b/keystone/api/credentials.py @@ -74,7 +74,8 @@ class CredentialResource(ks_flask.ResourceBase): target='credential') return blob - def _assign_unique_id(self, ref, trust_id=None, app_cred_id=None): + def _assign_unique_id( + self, ref, trust_id=None, app_cred_id=None, access_token_id=None): # Generates an assigns a unique identifier to a credential reference. if ref.get('type', '').lower() == 'ec2': blob = self._validate_blob_json(ref) @@ -91,6 +92,9 @@ class CredentialResource(ks_flask.ResourceBase): if app_cred_id is not None: blob['app_cred_id'] = app_cred_id ref['blob'] = jsonutils.dumps(blob) + if access_token_id is not None: + blob['access_token_id'] = access_token_id + ref['blob'] = jsonutils.dumps(blob) return ref else: return super(CredentialResource, self)._assign_unique_id(ref) @@ -155,9 +159,12 @@ class CredentialResource(ks_flask.ResourceBase): trust_id = getattr(self.oslo_context, 'trust_id', None) app_cred_id = getattr( self.auth_context['token'], 'application_credential_id', None) + access_token_id = getattr( + self.auth_context['token'], 'access_token_id', None) ref = self._assign_unique_id( self._normalize_dict(credential), - trust_id=trust_id, app_cred_id=app_cred_id) + trust_id=trust_id, app_cred_id=app_cred_id, + access_token_id=access_token_id) ref = PROVIDERS.credential_api.create_credential( ref['id'], ref, initiator=self.audit_initiator) return self.wrap_member(ref), http.client.CREATED diff --git a/keystone/models/token_model.py b/keystone/models/token_model.py index 54c45f42a..d68b8eb96 100644 --- a/keystone/models/token_model.py +++ b/keystone/models/token_model.py @@ -13,6 +13,7 @@ """Unified in-memory token model.""" from oslo_log import log +from oslo_serialization import jsonutils from oslo_serialization import msgpackutils from oslo_utils import reflection @@ -327,6 +328,21 @@ class TokenModel(object): return roles + def _get_oauth_roles(self): + roles = [] + access_token_roles = self.access_token['role_ids'] + access_token_roles = [ + {'role_id': r} for r in jsonutils.loads(access_token_roles)] + effective_access_token_roles = ( + PROVIDERS.assignment_api.add_implied_roles(access_token_roles) + ) + user_roles = [r['id'] for r in self._get_project_roles()] + for role in effective_access_token_roles: + if role['role_id'] in user_roles: + role = PROVIDERS.role_api.get_role(role['role_id']) + roles.append({'id': role['id'], 'name': role['name']}) + return roles + def _get_federated_roles(self): roles = [] group_ids = [group['id'] for group in self.federated_groups] @@ -430,6 +446,8 @@ class TokenModel(object): roles = self._get_system_roles() elif self.trust_scoped: roles = self._get_trust_roles() + elif self.oauth_scoped: + roles = self._get_oauth_roles() elif self.is_federated and not self.unscoped: roles = self._get_federated_roles() elif self.domain_scoped: diff --git a/keystone/tests/unit/test_v3_credential.py b/keystone/tests/unit/test_v3_credential.py index fd08362b7..2bde25b6c 100644 --- a/keystone/tests/unit/test_v3_credential.py +++ b/keystone/tests/unit/test_v3_credential.py @@ -21,12 +21,14 @@ import http.client from keystoneclient.contrib.ec2 import utils as ec2_utils from oslo_db import exception as oslo_db_exception from testtools import matchers +import urllib from keystone.api import ec2tokens from keystone.common import provider_api from keystone.common import utils from keystone.credential.providers import fernet as credential_fernet from keystone import exception +from keystone import oauth1 from keystone.tests import unit from keystone.tests.unit import ksfixtures from keystone.tests.unit import test_v3 @@ -644,6 +646,153 @@ class TestCredentialAppCreds(CredentialBaseTestCase): expected_status=http.client.CONFLICT) +class TestCredentialAccessToken(CredentialBaseTestCase): + """Test credential with access token.""" + + def setUp(self): + super(TestCredentialAccessToken, self).setUp() + self.useFixture( + ksfixtures.KeyRepository( + self.config_fixture, + 'credential', + credential_fernet.MAX_ACTIVE_KEYS + ) + ) + self.base_url = 'http://localhost/v3' + + def _urllib_parse_qs_text_keys(self, content): + results = urllib.parse.parse_qs(content) + return {key.decode('utf-8'): value for key, value in results.items()} + + def _create_single_consumer(self): + endpoint = '/OS-OAUTH1/consumers' + + ref = {'description': uuid.uuid4().hex} + resp = self.post(endpoint, body={'consumer': ref}) + return resp.result['consumer'] + + def _create_request_token(self, consumer, project_id, base_url=None): + endpoint = '/OS-OAUTH1/request_token' + client = oauth1.Client(consumer['key'], + client_secret=consumer['secret'], + signature_method=oauth1.SIG_HMAC, + callback_uri="oob") + headers = {'requested_project_id': project_id} + if not base_url: + base_url = self.base_url + url, headers, body = client.sign(base_url + endpoint, + http_method='POST', + headers=headers) + return endpoint, headers + + def _create_access_token(self, consumer, token, base_url=None): + endpoint = '/OS-OAUTH1/access_token' + client = oauth1.Client(consumer['key'], + client_secret=consumer['secret'], + resource_owner_key=token.key, + resource_owner_secret=token.secret, + signature_method=oauth1.SIG_HMAC, + verifier=token.verifier) + if not base_url: + base_url = self.base_url + url, headers, body = client.sign(base_url + endpoint, + http_method='POST') + headers.update({'Content-Type': 'application/json'}) + return endpoint, headers + + def _get_oauth_token(self, consumer, token): + client = oauth1.Client(consumer['key'], + client_secret=consumer['secret'], + resource_owner_key=token.key, + resource_owner_secret=token.secret, + signature_method=oauth1.SIG_HMAC) + endpoint = '/auth/tokens' + url, headers, body = client.sign(self.base_url + endpoint, + http_method='POST') + headers.update({'Content-Type': 'application/json'}) + ref = {'auth': {'identity': {'oauth1': {}, 'methods': ['oauth1']}}} + return endpoint, headers, ref + + def _authorize_request_token(self, request_id): + if isinstance(request_id, bytes): + request_id = request_id.decode() + return '/OS-OAUTH1/authorize/%s' % (request_id) + + def _get_access_token(self): + consumer = self._create_single_consumer() + consumer_id = consumer['id'] + consumer_secret = consumer['secret'] + consumer = {'key': consumer_id, 'secret': consumer_secret} + + url, headers = self._create_request_token(consumer, self.project_id) + content = self.post( + url, headers=headers, + response_content_type='application/x-www-form-urlencoded') + credentials = self._urllib_parse_qs_text_keys(content.result) + request_key = credentials['oauth_token'][0] + request_secret = credentials['oauth_token_secret'][0] + request_token = oauth1.Token(request_key, request_secret) + + url = self._authorize_request_token(request_key) + body = {'roles': [{'id': self.role_id}]} + resp = self.put(url, body=body, expected_status=http.client.OK) + verifier = resp.result['token']['oauth_verifier'] + + request_token.set_verifier(verifier) + url, headers = self._create_access_token(consumer, request_token) + content = self.post( + url, headers=headers, + response_content_type='application/x-www-form-urlencoded') + credentials = self._urllib_parse_qs_text_keys(content.result) + access_key = credentials['oauth_token'][0] + access_secret = credentials['oauth_token_secret'][0] + access_token = oauth1.Token(access_key, access_secret) + + url, headers, body = self._get_oauth_token(consumer, access_token) + content = self.post(url, headers=headers, body=body) + return access_key, content.headers['X-Subject-Token'] + + def test_access_token_ec2_credential(self): + """Test creating ec2 credential from an oauth access token. + + Call ``POST /credentials``. + """ + access_key, token_id = self._get_access_token() + + # Create the credential with the access token + blob, ref = unit.new_ec2_credential(user_id=self.user_id, + project_id=self.project_id) + r = self.post('/credentials', body={'credential': ref}, token=token_id) + + # We expect the response blob to contain the access_token_id + ret_ref = ref.copy() + ret_blob = blob.copy() + ret_blob['access_token_id'] = access_key.decode('utf-8') + ret_ref['blob'] = json.dumps(ret_blob) + self.assertValidCredentialResponse(r, ref=ret_ref) + + # Assert credential id is same as hash of access key id for + # ec2 credentials + access = blob['access'].encode('utf-8') + self.assertEqual(hashlib.sha256(access).hexdigest(), + r.result['credential']['id']) + + # Create a role assignment to ensure that it is ignored and only the + # roles in the access token are used + role = unit.new_role_ref(name='reader') + role_id = role['id'] + PROVIDERS.role_api.create_role(role_id, role) + PROVIDERS.assignment_api.add_role_to_user_and_project( + self.user_id, self.project_id, role_id) + + ret_blob = json.loads(r.result['credential']['blob']) + ec2token = self._test_get_token( + access=ret_blob['access'], secret=ret_blob['secret']) + ec2_roles = [role['id'] for role in ec2token['roles']] + self.assertIn(self.role_id, ec2_roles) + self.assertNotIn(role_id, ec2_roles) + + class TestCredentialEc2(CredentialBaseTestCase): """Test v3 credential compatibility with ec2tokens.""" diff --git a/keystone/tests/unit/test_v3_oauth1.py b/keystone/tests/unit/test_v3_oauth1.py index eaa2d128e..6b6942b61 100644 --- a/keystone/tests/unit/test_v3_oauth1.py +++ b/keystone/tests/unit/test_v3_oauth1.py @@ -308,6 +308,19 @@ class OAuthFlowTests(OAuth1Tests): self.keystone_token = content.result['token'] self.assertIsNotNone(self.keystone_token_id) + # add a new role assignment to ensure it is ignored in the access token + new_role = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + PROVIDERS.role_api.create_role(new_role['id'], new_role) + PROVIDERS.assignment_api.add_role_to_user_and_project( + user_id=self.user_id, + project_id=self.project_id, + role_id=new_role['id']) + content = self.post(url, headers=headers, body=body) + token = content.result['token'] + token_roles = [r['id'] for r in token['roles']] + self.assertIn(self.role_id, token_roles) + self.assertNotIn(new_role['id'], token_roles) + class AccessTokenCRUDTests(OAuthFlowTests): def test_delete_access_token_dne(self): diff --git a/releasenotes/notes/bug-1873290-ff7f8e4cee15b75a.yaml b/releasenotes/notes/bug-1873290-ff7f8e4cee15b75a.yaml new file mode 100644 index 000000000..de5fa680c --- /dev/null +++ b/releasenotes/notes/bug-1873290-ff7f8e4cee15b75a.yaml @@ -0,0 +1,19 @@ +--- +security: + - | + [`bug 1873290 `_] + [`bug 1872735 `_] + Fixed the token model to respect the roles authorized OAUTH1 access tokens. + Previously, the list of roles authorized for an OAUTH1 access token were + ignored, so when an access token was created, the token would contain every + role assignment the creator had for the project. This also fixed EC2 + credentials to respect those roles as well. +fixes: + - | + [`bug 1873290 `_] + [`bug 1872735 `_] + Fixed the token model to respect the roles authorized OAUTH1 access tokens. + Previously, the list of roles authorized for an OAUTH1 access token were + ignored, so when an access token was created, the token would contain every + role assignment the creator had for the project. This also fixed EC2 + credentials to respect those roles as well. -- 2.26.0