From 5f093bf5ee9f8ed201f01bab9c9afbde0423df07 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Sat, 26 Jan 2019 23:06:00 +0100 Subject: [PATCH] Add validation of app cred access rules This commit adds a validation step in the auth_token middleware to check for the presence of an access_rules attribute in an application credential token and to validate the request against the permissions granted for that token. During token validation it sends a header to keystone to indicate that it is capable of validating these access rules, and not providing this header for a token like this would result in the token failing validation. This disregards access rules for a service request made by a service on behalf of a user, such as nova making a request to glance, because such a request is not under the control of the user and is not expected to be explicitly allowed in the access rules. bp whitelist-extension-for-app-creds Depends-On: https://review.opendev.org/670377 Change-Id: I185e0541d5df538d74edadf9976b3034a2470c88 --- keystonemiddleware/auth_token/__init__.py | 74 ++++++++++- keystonemiddleware/auth_token/_identity.py | 5 +- keystonemiddleware/auth_token/_opts.py | 4 + .../auth_token/test_auth_token_middleware.py | 104 +++++++++++++++ .../tests/unit/client_fixtures.py | 124 ++++++++++++++++++ .../tests/unit/test_access_rules.py | 54 ++++++++ keystonemiddleware/tests/unit/test_opts.py | 2 + lower-constraints.txt | 4 +- ...ension-for-app-creds-badf088c8ad584bb.yaml | 7 + requirements.txt | 4 +- 10 files changed, 376 insertions(+), 6 deletions(-) create mode 100644 keystonemiddleware/tests/unit/test_access_rules.py create mode 100644 releasenotes/notes/bp-whitelist-extension-for-app-creds-badf088c8ad584bb.yaml diff --git a/keystonemiddleware/auth_token/__init__.py b/keystonemiddleware/auth_token/__init__.py index dcf166ba..b29b63a0 100644 --- a/keystonemiddleware/auth_token/__init__.py +++ b/keystonemiddleware/auth_token/__init__.py @@ -218,6 +218,7 @@ object is stored. """ import copy +import re from keystoneauth1 import access from keystoneauth1 import adapter @@ -277,6 +278,26 @@ def list_opts(): return [(g, copy.deepcopy(o)) for g, o in AUTH_TOKEN_OPTS] +def _path_matches(request_path, path_pattern): + # The fnmatch module doesn't provide the ability to match * versus **, + # so convert to regex. + token_regex = (r'(?P{[^}]*})|' # {tag} # nosec + '(?P\*(?=$|[^\*]))|' # * + '(?P\*\*)|' # ** + '(?P[^{}\*])') # anything else + path_regex = '' + for match in re.finditer(token_regex, path_pattern): + token = match.groupdict() + if token['tag'] or token['wild']: + path_regex += '[^\/]+' + if token['rec_wild']: + path_regex += '.*' + if token['literal']: + path_regex += token['literal'] + path_regex = r'^%s$' % path_regex + return re.match(path_regex, request_path) + + class _BIND_MODE(object): DISABLED = 'disabled' PERMISSIVE = 'permissive' @@ -301,13 +322,15 @@ class BaseAuthProtocol(object): log=_LOG, enforce_token_bind=_BIND_MODE.PERMISSIVE, service_token_roles=None, - service_token_roles_required=False): + service_token_roles_required=False, + service_type=None): self.log = log self._app = app self._enforce_token_bind = enforce_token_bind self._service_token_roles = set(service_token_roles or []) self._service_token_roles_required = service_token_roles_required self._service_token_warning_emitted = False + self._service_type = service_type @webob.dec.wsgify(RequestClass=_request._AuthTokenRequest) def __call__(self, req): @@ -388,6 +411,8 @@ class BaseAuthProtocol(object): allow_expired=allow_expired) self._validate_token(user_auth_ref, allow_expired=allow_expired) + if user_auth_ref.version != 'v2.0': + self.validate_allowed_request(request, data['token']) if not request.service_token: self._confirm_token_bind(user_auth_ref, request) except ksm_exceptions.InvalidToken: @@ -516,6 +541,53 @@ class BaseAuthProtocol(object): {'bind_type': bind_type, 'identifier': identifier}) self._invalid_user_token() + def validate_allowed_request(self, request, token): + self.log.debug("Validating token access rules against request") + app_cred = token.get('application_credential') + if not app_cred: + return + access_rules = app_cred.get('access_rules') + if access_rules is None: + return + if hasattr(self, '_conf'): + my_service_type = self._conf.get('service_type') + else: + my_service_type = self._service_type + if not my_service_type: + self.log.warning('Cannot validate request with restricted' + ' access rules. Set service_type in' + ' [keystone_authtoken] to allow access rule' + ' validation.') + raise ksm_exceptions.InvalidToken(_('Token authorization failed')) + # token can always be validated regardless of access rules + if (my_service_type == 'identity' and + request.method == 'GET' and + request.path.endswith('/v3/auth/tokens')): + return + catalog = token['catalog'] + # validate service type is in catalog + catalog_svcs = [s for s in catalog if s['type'] == my_service_type] + if len(catalog_svcs) == 0: + self.log.warning('Cannot validate request with restricted' + ' access rules. service_type in' + ' [keystone_authtoken] is not a valid service' + ' type in the catalog.') + raise ksm_exceptions.InvalidToken(_('Token authorization failed')) + if request.service_token: + # The request may not match an allowed request, but the presence + # of the service token indicates this is a chain of requests and + # hence this request was not user-facing + return + for access_rule in access_rules: + method = access_rule['method'] + path = access_rule['path'] + service = access_rule['service'] + if request.method == method and \ + service == my_service_type and \ + _path_matches(request.path, path): + return + raise ksm_exceptions.InvalidToken(_('Token authorization failed')) + class AuthProtocol(BaseAuthProtocol): """Middleware that handles authenticating client calls.""" diff --git a/keystonemiddleware/auth_token/_identity.py b/keystonemiddleware/auth_token/_identity.py index 4c105215..3967c23a 100644 --- a/keystonemiddleware/auth_token/_identity.py +++ b/keystonemiddleware/auth_token/_identity.py @@ -21,6 +21,8 @@ from keystonemiddleware.auth_token import _auth from keystonemiddleware.auth_token import _exceptions as ksm_exceptions from keystonemiddleware.i18n import _ +ACCESS_RULES_SUPPORT = '1' + class _RequestStrategy(object): @@ -69,7 +71,8 @@ class _V3RequestStrategy(_RequestStrategy): auth_ref = self._client.tokens.validate( token, include_catalog=self._include_service_catalog, - allow_expired=allow_expired) + allow_expired=allow_expired, + access_rules_support=ACCESS_RULES_SUPPORT) if not auth_ref: msg = _('Failed to fetch token data from identity server') diff --git a/keystonemiddleware/auth_token/_opts.py b/keystonemiddleware/auth_token/_opts.py index b5514079..f16d3f81 100644 --- a/keystonemiddleware/auth_token/_opts.py +++ b/keystonemiddleware/auth_token/_opts.py @@ -178,6 +178,10 @@ _OPTS = [ ' service tokens pass that don\'t pass the service_token_roles' ' check as valid. Setting this true will become the default' ' in a future release and should be enabled if possible.'), + cfg.StrOpt('service_type', + help='The name or type of the service as it appears in the' + ' service catalog. This is used to validate tokens that have' + ' restricted access rules.'), ] diff --git a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py index 04e605cd..9fd0328a 100644 --- a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py +++ b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py @@ -1413,6 +1413,110 @@ class v3AuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest, e = self.requests_mock.request_history[3].qs.get('allow_expired') self.assertIsNone(e) + def test_app_cred_token_without_access_rules(self): + self.set_middleware(conf={'service_type': 'compute'}) + token = self.examples.v3_APP_CRED_TOKEN + token_data = self.examples.TOKEN_RESPONSES[token] + resp = self.call_middleware(headers={'X-Auth-Token': token}) + self.assertEqual(FakeApp.SUCCESS, resp.body) + token_auth = resp.request.environ['keystone.token_auth'] + self.assertEqual(token_data.application_credential_id, + token_auth.user.application_credential_id) + + def test_app_cred_access_rules_token(self): + self.set_middleware(conf={'service_type': 'compute'}) + token = self.examples.v3_APP_CRED_ACCESS_RULES + token_data = self.examples.TOKEN_RESPONSES[token] + resp = self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=200, + method='GET', path='/v2.1/servers') + token_auth = resp.request.environ['keystone.token_auth'] + self.assertEqual(token_data.application_credential_id, + token_auth.user.application_credential_id) + self.assertEqual(token_data.application_credential_access_rules, + token_auth.user.application_credential_access_rules) + resp = self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', + path='/v2.1/servers/someuuid') + token_auth = resp.request.environ['keystone.token_auth'] + self.assertEqual(token_data.application_credential_id, + token_auth.user.application_credential_id) + self.assertEqual(token_data.application_credential_access_rules, + token_auth.user.application_credential_access_rules) + + def test_app_cred_access_rules_service_request(self): + self.set_middleware(conf={'service_type': 'image'}) + token = self.examples.v3_APP_CRED_ACCESS_RULES + headers = {'X-Auth-Token': token} + self.call_middleware(headers=headers, + expected_status=401, + method='GET', path='/v2/images') + service_token = self.examples.v3_UUID_SERVICE_TOKEN_DEFAULT + headers['X-Service-Token'] = service_token + self.call_middleware(headers=headers, + expected_status=200, + method='GET', path='/v2/images') + + def test_app_cred_no_access_rules_token(self): + self.set_middleware(conf={'service_type': 'compute'}) + token = self.examples.v3_APP_CRED_EMPTY_ACCESS_RULES + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/v2.1/servers') + service_token = self.examples.v3_UUID_SERVICE_TOKEN_DEFAULT + headers = { + 'X-Auth-Token': token, + 'X-Service-Token': service_token + } + self.call_middleware(headers=headers, expected_status=401, + method='GET', path='/v2.1/servers') + + def test_app_cred_matching_rules(self): + self.set_middleware(conf={'service_type': 'compute'}) + token = self.examples.v3_APP_CRED_MATCHING_RULES + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=200, + method='GET', path='/v2.1/servers/foobar') + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/v2.1/servers/foobar/barfoo') + self.set_middleware(conf={'service_type': 'image'}) + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=200, + method='GET', path='/v2/images/foobar') + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/v2/images/foobar/barfoo') + self.set_middleware(conf={'service_type': 'identity'}) + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=200, + method='GET', + path='/v3/projects/123/users/456/roles/member') + self.set_middleware(conf={'service_type': 'block-storage'}) + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=200, + method='GET', path='/v3/123/types/456') + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/v3/123/types') + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/v2/123/types/456') + self.set_middleware(conf={'service_type': 'object-store'}) + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=200, + method='GET', path='/v1/1/2/3') + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/v1/1/2') + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/v2/1/2') + self.call_middleware(headers={'X-Auth-Token': token}, + expected_status=401, + method='GET', path='/info') + class DelayedAuthTests(BaseAuthTokenMiddlewareTest): diff --git a/keystonemiddleware/tests/unit/client_fixtures.py b/keystonemiddleware/tests/unit/client_fixtures.py index a8070548..27ba4826 100644 --- a/keystonemiddleware/tests/unit/client_fixtures.py +++ b/keystonemiddleware/tests/unit/client_fixtures.py @@ -64,6 +64,11 @@ class Examples(fixtures.Fixture): self.v3_UUID_SERVICE_TOKEN_BIND = 'be705e4426d0449a89e35ae21c380a05' self.v3_NOT_IS_ADMIN_PROJECT = uuid.uuid4().hex + self.v3_APP_CRED_TOKEN = '6f506fa9641448bbaecbd12dd30678a9' + self.v3_APP_CRED_ACCESS_RULES = 'c417747898c44629b08791f2579e40a5' + self.v3_APP_CRED_EMPTY_ACCESS_RULES = 'c75905c307f04fdd9979126582d7aae' + self.v3_APP_CRED_MATCHING_RULES = 'ad49decc7106489d95ca9ed874b6cb66' + # JSON responses keyed by token ID self.TOKEN_RESPONSES = {} @@ -86,6 +91,8 @@ class Examples(fixtures.Fixture): SERVICE_ROLE_NAME1 = 'service' SERVICE_ROLE_NAME2 = 'service_role2' + APP_CRED_ID = 'app_cred_id1' + self.SERVICE_TYPE = 'identity' self.UNVERSIONED_SERVICE_URL = 'https://keystone.example.com:1234/' self.SERVICE_URL = self.UNVERSIONED_SERVICE_URL + 'v2.0' @@ -293,6 +300,123 @@ class Examples(fixtures.Fixture): svc.add_endpoint('public', self.SERVICE_URL) self.TOKEN_RESPONSES[self.v3_NOT_IS_ADMIN_PROJECT] = token + # Application credential token + token = fixture.V3Token(user_id=USER_ID, + user_name=USER_NAME, + user_domain_id=DOMAIN_ID, + user_domain_name=DOMAIN_NAME, + project_id=PROJECT_ID, + project_name=PROJECT_NAME, + project_domain_id=DOMAIN_ID, + project_domain_name=DOMAIN_NAME, + application_credential_id=APP_CRED_ID) + token.add_role(name=ROLE_NAME1) + token.add_role(name=ROLE_NAME2) + svc = token.add_service(self.SERVICE_TYPE) + svc.add_endpoint('public', self.SERVICE_URL) + svc = token.add_service('compute') + svc.add_endpoint('public', 'https://nova.openstack.example.org/v2.1') + self.TOKEN_RESPONSES[self.v3_APP_CRED_TOKEN] = token + + # Application credential with access_rules token + access_rules = [{ + 'path': '/v2.1/servers', + 'method': 'GET', + 'service': 'compute' + }] + token = fixture.V3Token( + user_id=USER_ID, + user_name=USER_NAME, + user_domain_id=DOMAIN_ID, + user_domain_name=DOMAIN_NAME, + project_id=PROJECT_ID, + project_name=PROJECT_NAME, + project_domain_id=DOMAIN_ID, + project_domain_name=DOMAIN_NAME, + application_credential_id=APP_CRED_ID, + application_credential_access_rules=access_rules) + token.add_role(name=ROLE_NAME1) + token.add_role(name=ROLE_NAME2) + svc = token.add_service(self.SERVICE_TYPE) + svc.add_endpoint('public', self.SERVICE_URL) + svc = token.add_service('compute') + svc.add_endpoint('public', 'https://nova.openstack.example.org') + svc = token.add_service('image') + svc.add_endpoint('public', 'https://glance.openstack.example.org') + self.TOKEN_RESPONSES[self.v3_APP_CRED_ACCESS_RULES] = token + + # Application credential with explicitly empty access_rules + access_rules = [] + token = fixture.V3Token( + user_id=USER_ID, + user_name=USER_NAME, + user_domain_id=DOMAIN_ID, + user_domain_name=DOMAIN_NAME, + project_id=PROJECT_ID, + project_name=PROJECT_NAME, + project_domain_id=DOMAIN_ID, + project_domain_name=DOMAIN_NAME, + application_credential_id=APP_CRED_ID, + application_credential_access_rules=access_rules) + token.add_role(name=ROLE_NAME1) + token.add_role(name=ROLE_NAME2) + svc = token.add_service(self.SERVICE_TYPE) + svc.add_endpoint('public', self.SERVICE_URL) + self.TOKEN_RESPONSES[self.v3_APP_CRED_EMPTY_ACCESS_RULES] = token + + # Application credential with matching rules + access_rules = [ + { + 'path': '/v2.1/servers/{server_id}', + 'method': 'GET', + 'service': 'compute' + }, + { + 'path': '/v2/images/*', + 'method': 'GET', + 'service': 'image' + }, + { + 'path': '**', + 'method': 'GET', + 'service': 'identity' + }, + { + 'path': '/v3/{project_id}/types/{volume_type_id}', + 'method': 'GET', + 'service': 'block-storage' + }, + { + 'path': '/v1/*/*/*', + 'method': 'GET', + 'service': 'object-store' + } + ] + token = fixture.V3Token( + user_id=USER_ID, + user_name=USER_NAME, + user_domain_id=DOMAIN_ID, + user_domain_name=DOMAIN_NAME, + project_id=PROJECT_ID, + project_name=PROJECT_NAME, + project_domain_id=DOMAIN_ID, + project_domain_name=DOMAIN_NAME, + application_credential_id=APP_CRED_ID, + application_credential_access_rules=access_rules) + token.add_role(name=ROLE_NAME1) + token.add_role(name=ROLE_NAME2) + svc = token.add_service(self.SERVICE_TYPE) + svc.add_endpoint('public', self.SERVICE_URL) + svc = token.add_service('compute') + svc.add_endpoint('public', 'https://nova.openstack.example.org') + svc = token.add_service('image') + svc.add_endpoint('public', 'https://glance.openstack.example.org') + svc = token.add_service('block-storage') + svc.add_endpoint('public', 'https://cinder.openstack.example.org') + svc = token.add_service('object-store') + svc.add_endpoint('public', 'https://swift.openstack.example.org') + self.TOKEN_RESPONSES[self.v3_APP_CRED_MATCHING_RULES] = token + self.JSON_TOKEN_RESPONSES = dict([(k, jsonutils.dumps(v)) for k, v in self.TOKEN_RESPONSES.items()]) diff --git a/keystonemiddleware/tests/unit/test_access_rules.py b/keystonemiddleware/tests/unit/test_access_rules.py new file mode 100644 index 00000000..663b806c --- /dev/null +++ b/keystonemiddleware/tests/unit/test_access_rules.py @@ -0,0 +1,54 @@ +# Copyright 2019 SUSE LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from keystonemiddleware.auth_token import _path_matches +from keystonemiddleware.tests.unit import utils + + +class TestAccessRules(utils.BaseTestCase): + + def test_path_matches(self): + good_matches = [ + ('/v2/servers', '/v2/servers'), + ('/v2/servers/123', '/v2/servers/{server_id}'), + ('/v2/servers/123/', '/v2/servers/{server_id}/'), + ('/v2/servers/123', '/v2/servers/*'), + ('/v2/servers/123/', '/v2/servers/*/'), + ('/v2/servers/123', '/v2/servers/**'), + ('/v2/servers/123/', '/v2/servers/**'), + ('/v2/servers/123/456', '/v2/servers/**'), + ('/v2/servers', '**'), + ('/v2/servers/', '**'), + ('/v2/servers/123', '**'), + ('/v2/servers/123/456', '**'), + ('/v2/servers/123/volume/456', '**'), + ('/v2/servers/123/456', '/v2/*/*/*'), + ('/v2/123/servers/466', '/v2/{project_id}/servers/{server_id}'), + ] + for (request, pattern) in good_matches: + self.assertIsNotNone(_path_matches(request, pattern)) + bad_matches = [ + ('/v2/servers/someuuid', '/v2/servers'), + ('/v2/servers//', '/v2/servers/{server_id}'), + ('/v2/servers/123/', '/v2/servers/{server_id}'), + ('/v2/servers/123/456', '/v2/servers/{server_id}'), + ('/v2/servers/123/456', '/v2/servers/*'), + ('/v2/servers', 'v2/servers'), + ('/v2/servers/123/456/789', '/v2/*/*/*'), + ('/v2/servers/123/', '/v2/*/*/*'), + ('/v2/servers/', '/v2/servers/{server_id}'), + ('/v2/servers', '/v2/servers/{server_id}'), + ] + for (request, pattern) in bad_matches: + self.assertIsNone(_path_matches(request, pattern)) diff --git a/keystonemiddleware/tests/unit/test_opts.py b/keystonemiddleware/tests/unit/test_opts.py index 7700d17f..799bdd08 100644 --- a/keystonemiddleware/tests/unit/test_opts.py +++ b/keystonemiddleware/tests/unit/test_opts.py @@ -69,6 +69,7 @@ class OptsTestCase(utils.TestCase): 'auth_section', 'service_token_roles', 'service_token_roles_required', + 'service_type', ] opt_names = [o.name for (g, l) in result_of_old_opts for o in l] self.assertThat(opt_names, matchers.HasLength(len(expected_opt_names))) @@ -113,6 +114,7 @@ class OptsTestCase(utils.TestCase): 'auth_section', 'service_token_roles', 'service_token_roles_required', + 'service_type', ] opt_names = [o.name for (g, l) in result for o in l] self.assertThat(opt_names, matchers.HasLength(len(expected_opt_names))) diff --git a/lower-constraints.txt b/lower-constraints.txt index 9ea603fe..90c49e47 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -23,7 +23,7 @@ GitPython==2.1.8 hacking==0.10.0 idna==2.6 iso8601==0.1.12 -keystoneauth1==3.4.0 +keystoneauth1==3.12.0 linecache2==1.0.0 mccabe==0.2.1 mock==2.0.0 @@ -57,7 +57,7 @@ pyinotify==0.9.6 pyparsing==2.2.0 pyperclip==1.6.0 python-dateutil==2.7.0 -python-keystoneclient==3.10.0 +python-keystoneclient==3.20.0 python-memcached==1.59 python-mimeparse==1.6.0 python-subunit==1.2.0 diff --git a/releasenotes/notes/bp-whitelist-extension-for-app-creds-badf088c8ad584bb.yaml b/releasenotes/notes/bp-whitelist-extension-for-app-creds-badf088c8ad584bb.yaml new file mode 100644 index 00000000..a6e68d41 --- /dev/null +++ b/releasenotes/notes/bp-whitelist-extension-for-app-creds-badf088c8ad584bb.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + [`spec `_] + The auth_token middleware now has support for accepting or denying incoming + requests based on access rules provided by users in their keystone + application credentials. diff --git a/requirements.txt b/requirements.txt index 80b26d43..d3f07ceb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -keystoneauth1>=3.4.0 # Apache-2.0 +keystoneauth1>=3.12.0 # Apache-2.0 oslo.cache>=1.26.0 # Apache-2.0 oslo.config>=5.2.0 # Apache-2.0 oslo.context>=2.19.2 # Apache-2.0 @@ -12,7 +12,7 @@ oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 pbr!=2.1.0,>=2.0.0 # Apache-2.0 pycadf!=2.0.0,>=1.1.0 # Apache-2.0 -python-keystoneclient>=3.10.0 # Apache-2.0 +python-keystoneclient>=3.20.0 # Apache-2.0 requests>=2.14.2 # Apache-2.0 six>=1.10.0 # MIT WebOb>=1.7.1 # MIT