From e86a50d7c962cfcc2deaae8f1f74a9a11de1506e 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 and forbidding users from modifying the access token in the credential. Conflicts due to six removal in e2d83ae9: keystone/tests/unit/test_v3_credential.py Change-Id: I02f9836fbd4d7e629653977fc341476cfd89859e Closes-bug: #1873290 Related-bug: #1872735 Related-bug: #1872755 (cherry picked from commit c3162a578a9e234c326f6a0a058f949d9eee7c81) (cherry picked from commit 49bc11807661cc4d7aeb0031796cf144ecd1bd5f) --- keystone/api/_shared/EC2_S3_Resource.py | 12 +- keystone/api/credentials.py | 24 ++- keystone/models/token_model.py | 18 ++ keystone/tests/unit/test_v3_credential.py | 178 +++++++++++++++++- keystone/tests/unit/test_v3_oauth1.py | 13 ++ .../notes/bug-1873290-ff7f8e4cee15b75a.yaml | 19 ++ 6 files changed, 251 insertions(+), 13 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 46623f031..e044c308e 100644 --- a/keystone/api/_shared/EC2_S3_Resource.py +++ b/keystone/api/_shared/EC2_S3_Resource.py @@ -87,7 +87,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 @@ -109,6 +110,7 @@ class ResourceBase(ks_flask.ResourceBase): sys.exc_info()[2]) 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']] @@ -126,6 +128,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']) @@ -147,5 +154,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 3a1122244..95098dcc8 100644 --- a/keystone/api/credentials.py +++ b/keystone/api/credentials.py @@ -75,7 +75,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) @@ -92,6 +93,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) @@ -156,9 +160,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 @@ -169,14 +176,11 @@ class CredentialResource(ks_flask.ResourceBase): old_blob = credential.get('blob') if isinstance(old_blob, six.string_types): old_blob = jsonutils.loads(old_blob) - # if there was a trust set, prevent changing it or unsetting it - if old_blob.get('trust_id') != new_blob.get('trust_id'): - raise exception.ValidationError( - message=_('trust_id can not be updated for credential')) - # if there was an app cred set, prevent changing it or unsetting it - if old_blob.get('app_cred_id') != new_blob.get('app_cred_id'): - raise exception.ValidationError( - message=_('app_cred_id can not be updated for credential')) + # if there was a scope set, prevent changing it or unsetting it + for key in ['trust_id', 'app_cred_id', 'access_token_id']: + if old_blob.get(key) != new_blob.get(key): + message = _('%s can not be updated for credential') % key + raise exception.ValidationError(message=message) def patch(self, credential_id): # Update Credential diff --git a/keystone/models/token_model.py b/keystone/models/token_model.py index 7f190286a..413545b53 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 import six @@ -325,6 +326,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] @@ -428,6 +444,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 0c9121502..0345bb38d 100644 --- a/keystone/tests/unit/test_v3_credential.py +++ b/keystone/tests/unit/test_v3_credential.py @@ -19,7 +19,7 @@ import uuid from keystoneclient.contrib.ec2 import utils as ec2_utils import mock from oslo_db import exception as oslo_db_exception -from six.moves import http_client +from six.moves import http_client, urllib from testtools import matchers from keystone.api import ec2tokens @@ -27,6 +27,7 @@ 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 @@ -341,6 +342,34 @@ class CredentialTestCase(CredentialBaseTestCase): body={'credential': update_ref}, expected_status=http_client.BAD_REQUEST) + def test_update_ec2_credential_change_access_token_id(self): + """Call ``PATCH /credentials/{credential_id}``.""" + blob, ref = unit.new_ec2_credential(user_id=self.user['id'], + project_id=self.project_id) + blob['access_token_id'] = uuid.uuid4().hex + ref['blob'] = json.dumps(blob) + r = self.post( + '/credentials', + body={'credential': ref}) + self.assertValidCredentialResponse(r, ref) + credential_id = r.result.get('credential')['id'] + # Try changing to a different access token + blob['access_token_id'] = uuid.uuid4().hex + update_ref = {'blob': json.dumps(blob)} + self.patch( + '/credentials/%(credential_id)s' % { + 'credential_id': credential_id}, + body={'credential': update_ref}, + expected_status=http_client.BAD_REQUEST) + # Try removing the access token + del blob['access_token_id'] + update_ref = {'blob': json.dumps(blob)} + self.patch( + '/credentials/%(credential_id)s' % { + 'credential_id': credential_id}, + body={'credential': update_ref}, + expected_status=http_client.BAD_REQUEST) + def test_delete_credential(self): """Call ``DELETE /credentials/{credential_id}``.""" self.delete( @@ -644,6 +673,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 90378214e..4c648a23e 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..ad35a3047 --- /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 used to request a keystone token, the + keystone 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 used to request a keystone token, the + keystone 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