From 198ef70c2b754b73c06deb66e351d054d81dac07 Mon Sep 17 00:00:00 2001 From: Pavlo Shchelokovskyy Date: Wed, 27 Sep 2017 09:26:58 +0000 Subject: [PATCH] Add request context and policy enforcement this patch introduces an oslo.policy-based API access policy enforcement engine to ironic-inspector. As part of implementation, a proper oslo.context-based request context is also generated and assigned to each request. Short overview of changes: - added custom RequestContext class - extends oslo.context to handle of "is_public_api" flag (False by default) - added context to request in each API route - '/continue' api sets the "is_public_api" flag to True - added documented definitions for API access policies and their defaults - added enforcement of these policies on API requests - added oslo.policy-specific entry points to setup.cfg - added autogenerated policy sample file with defaults - added documentation with autogenerated policies Change-Id: Iff6f98fa9950d78608f0a7c325d132c11a1383b3 Closes-Bug: #1719812 --- .gitignore | 1 + config-generator.conf | 1 + doc/source/conf.py | 5 + doc/source/configuration/index.rst | 2 + doc/source/configuration/policy.rst | 9 + doc/source/configuration/sample-policy.rst | 13 ++ example.conf | 21 ++ ironic_inspector/common/context.py | 45 ++++ ironic_inspector/main.py | 95 +++++--- ironic_inspector/policy.py | 217 ++++++++++++++++++ ironic_inspector/test/base.py | 2 + ironic_inspector/test/unit/policy_fixture.py | 40 ++++ ironic_inspector/test/unit/test_utils.py | 38 +-- ironic_inspector/utils.py | 16 +- policy-generator.conf | 3 + policy.yaml.sample | 59 +++++ .../notes/policy-engine-c44828e3131e6c62.yaml | 35 +++ requirements.txt | 2 + setup.cfg | 4 + tox.ini | 5 + 20 files changed, 557 insertions(+), 56 deletions(-) create mode 100644 doc/source/configuration/policy.rst create mode 100644 doc/source/configuration/sample-policy.rst create mode 100644 ironic_inspector/common/context.py create mode 100644 ironic_inspector/policy.py create mode 100644 ironic_inspector/test/unit/policy_fixture.py create mode 100644 policy-generator.conf create mode 100644 policy.yaml.sample create mode 100644 releasenotes/notes/policy-engine-c44828e3131e6c62.yaml diff --git a/.gitignore b/.gitignore index 4153f8687..8426a2ba9 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ # Sphinx _build doc/source/contributor/api/ +doc/source/_static/*.sample # release notes build releasenotes/build diff --git a/config-generator.conf b/config-generator.conf index 1509509c8..b3c8f42b2 100644 --- a/config-generator.conf +++ b/config-generator.conf @@ -10,3 +10,4 @@ namespace = keystonemiddleware.auth_token namespace = oslo.db namespace = oslo.log namespace = oslo.middleware.cors +namespace = oslo.policy diff --git a/doc/source/conf.py b/doc/source/conf.py index 8f58f1756..7f342a2a3 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -7,6 +7,8 @@ # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones. extensions = ['sphinx.ext.autodoc', 'sphinx.ext.viewcode', + 'oslo_policy.sphinxext', + 'oslo_policy.sphinxpolicygen', 'oslo_config.sphinxext', 'oslo_config.sphinxconfiggen'] @@ -43,6 +45,9 @@ copyright = u'OpenStack Foundation' config_generator_config_file = '../../config-generator.conf' sample_config_basename = '_static/ironic-inspector' +policy_generator_config_file = '../../policy-generator.conf' +sample_policy_basename = '_static/ironic-inspector' + # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the # built documents. diff --git a/doc/source/configuration/index.rst b/doc/source/configuration/index.rst index 612edd9d0..c9fad8ef5 100644 --- a/doc/source/configuration/index.rst +++ b/doc/source/configuration/index.rst @@ -9,5 +9,7 @@ file. The overview of configuration file options follow. Ironic Inspector Configuration Options Sample Ironic Inspector Configuration + Policies + Sample policy file diff --git a/doc/source/configuration/policy.rst b/doc/source/configuration/policy.rst new file mode 100644 index 000000000..f14ac5a8c --- /dev/null +++ b/doc/source/configuration/policy.rst @@ -0,0 +1,9 @@ +======== +Policies +======== + +The following is an overview of all available policies in **ironic inspector**. +For a sample configuration file, refer to :doc:`sample-policy`. + +.. show-policy:: + :config-file: policy-generator.conf diff --git a/doc/source/configuration/sample-policy.rst b/doc/source/configuration/sample-policy.rst new file mode 100644 index 000000000..3b3c9ea89 --- /dev/null +++ b/doc/source/configuration/sample-policy.rst @@ -0,0 +1,13 @@ +======================= +Ironic Inspector Policy +======================= + +The following is a sample **ironic-inspector** policy file, autogenerated from +Ironic Inspector when this documentation is built. +To avoid issues, make sure your version of **ironic-inspector** +matches that of the example policy file. + +The sample policy can also be downloaded as a :download:`file +`. + +.. literalinclude:: /_static/ironic-inspector.policy.yaml.sample diff --git a/example.conf b/example.conf index 7a7346eae..90cabf2c3 100644 --- a/example.conf +++ b/example.conf @@ -666,6 +666,27 @@ #auth_section = +[oslo_policy] + +# +# From oslo.policy +# + +# The file that defines policies. (string value) +#policy_file = policy.json + +# Default rule. Enforced when a requested rule is not found. (string +# value) +#policy_default_rule = default + +# Directories where policy configuration files are stored. They can be +# relative to any directory in the search path defined by the +# config_dir option, or absolute paths. The file defined by +# policy_file must exist for these directories to be searched. +# Missing or empty directories are ignored. (multi valued) +#policy_dirs = policy.d + + [pci_devices] # diff --git a/ironic_inspector/common/context.py b/ironic_inspector/common/context.py new file mode 100644 index 000000000..06e8c42e3 --- /dev/null +++ b/ironic_inspector/common/context.py @@ -0,0 +1,45 @@ +# +# 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 oslo_context import context + + +class RequestContext(context.RequestContext): + """Extends security contexts from the oslo.context library.""" + + def __init__(self, is_public_api=False, **kwargs): + """Initialize the RequestContext + + :param is_public_api: Specifies whether the request should be processed + without authentication. + :param kwargs: additional arguments passed to oslo.context. + """ + super(RequestContext, self).__init__(**kwargs) + self.is_public_api = is_public_api + + def to_policy_values(self): + policy_values = super(RequestContext, self).to_policy_values() + policy_values.update({'is_public_api': self.is_public_api}) + return policy_values + + @classmethod + def from_dict(cls, values, **kwargs): + kwargs.setdefault('is_public_api', values.get('is_public_api', False)) + return super(RequestContext, RequestContext).from_dict(values, + **kwargs) + + @classmethod + def from_environ(cls, environ, **kwargs): + kwargs.setdefault('is_public_api', environ.get('is_public_api', False)) + return super(RequestContext, RequestContext).from_environ(environ, + **kwargs) diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index 665c57ade..d186a7b92 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -21,6 +21,7 @@ from oslo_utils import uuidutils import werkzeug from ironic_inspector import api_tools +from ironic_inspector.common import context from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils from ironic_inspector.common import swift @@ -146,8 +147,44 @@ def generate_introspection_status(node): return status -@app.route('/', methods=['GET']) -@convert_exceptions +def api(path, is_public_api=False, rule=None, verb_to_rule_map=None, + **flask_kwargs): + """Decorator to wrap api methods. + + Performs flask routing, exception convertion, + generation of oslo context for request and API access policy enforcement. + + :param path: flask app route path + :param is_public_api: whether this API path should be treated + as public, with minimal access enforcement + :param rule: API access policy rule to enforce. + If rule is None, the 'default' policy rule will be enforced, + which is "deny all" if not overridden in policy confif file. + :param verb_to_rule_map: if both rule and this are given, + defines mapping between http verbs (uppercase) + and strings to format the 'rule' string with + :param kwargs: all the rest kwargs are passed to flask app.route + """ + def outer(func): + @app.route(path, **flask_kwargs) + @convert_exceptions + @functools.wraps(func) + def wrapper(*args, **kwargs): + flask.request.context = context.RequestContext.from_environ( + flask.request.environ, + is_public_api=is_public_api) + if verb_to_rule_map and rule: + policy_rule = rule.format( + verb_to_rule_map[flask.request.method.upper()]) + else: + policy_rule = rule + utils.check_auth(flask.request, rule=policy_rule) + return func(*args, **kwargs) + return wrapper + return outer + + +@api('/', rule='introspection', is_public_api=True, methods=['GET']) def api_root(): versions = [ { @@ -163,8 +200,8 @@ def api_root(): return flask.jsonify(versions=versions) -@app.route('/', methods=['GET']) -@convert_exceptions +@api('/', rule='introspection:version', is_public_api=True, + methods=['GET']) def version_root(version): pat = re.compile("^\/%s\/[^\/]*?$" % version) @@ -179,8 +216,8 @@ def version_root(version): return flask.jsonify(resources=generate_resource_data(resources)) -@app.route('/v1/continue', methods=['POST']) -@convert_exceptions +@api('/v1/continue', rule="introspection:continue", is_public_api=True, + methods=['POST']) def api_continue(): data = flask.request.get_json(force=True) if not isinstance(data, dict): @@ -196,11 +233,11 @@ def api_continue(): # TODO(sambetts) Add API discovery for this endpoint -@app.route('/v1/introspection/', methods=['GET', 'POST']) -@convert_exceptions +@api('/v1/introspection/', + rule="introspection:{}", + verb_to_rule_map={'GET': 'status', 'POST': 'start'}, + methods=['GET', 'POST']) def api_introspection(node_id): - utils.check_auth(flask.request) - if flask.request.method == 'POST': introspect.introspect(node_id, token=flask.request.headers.get('X-Auth-Token')) @@ -210,11 +247,8 @@ def api_introspection(node_id): return flask.json.jsonify(generate_introspection_status(node_info)) -@app.route('/v1/introspection', methods=['GET']) -@convert_exceptions +@api('/v1/introspection', rule='introspection:status', methods=['GET']) def api_introspection_statuses(): - utils.check_auth(flask.request) - nodes = node_cache.get_node_list( marker=api_tools.marker_field(), limit=api_tools.limit_field(default=CONF.api_max_limit) @@ -226,19 +260,16 @@ def api_introspection_statuses(): return flask.json.jsonify(data) -@app.route('/v1/introspection//abort', methods=['POST']) -@convert_exceptions +@api('/v1/introspection//abort', rule="introspection:abort", + methods=['POST']) def api_introspection_abort(node_id): - utils.check_auth(flask.request) introspect.abort(node_id, token=flask.request.headers.get('X-Auth-Token')) return '', 202 -@app.route('/v1/introspection//data', methods=['GET']) -@convert_exceptions +@api('/v1/introspection//data', rule="introspection:data", + methods=['GET']) def api_introspection_data(node_id): - utils.check_auth(flask.request) - if CONF.processing.store_data == 'swift': if not uuidutils.is_uuid_like(node_id): node = ir_utils.get_node(node_id, fields=['uuid']) @@ -252,11 +283,9 @@ def api_introspection_data(node_id): code=404) -@app.route('/v1/introspection//data/unprocessed', methods=['POST']) -@convert_exceptions +@api('/v1/introspection//data/unprocessed', + rule="introspection:reapply", methods=['POST']) def api_introspection_reapply(node_id): - utils.check_auth(flask.request) - if flask.request.content_length: return error_response(_('User data processing is not ' 'supported yet'), code=400) @@ -280,11 +309,11 @@ def rule_repr(rule, short): return result -@app.route('/v1/rules', methods=['GET', 'POST', 'DELETE']) -@convert_exceptions +@api('/v1/rules', + rule="introspection:rule:{}", + verb_to_rule_map={'GET': 'get', 'POST': 'create', 'DELETE': 'delete'}, + methods=['GET', 'POST', 'DELETE']) def api_rules(): - utils.check_auth(flask.request) - if flask.request.method == 'GET': res = [rule_repr(rule, short=True) for rule in rules.get_all()] return flask.jsonify(rules=res) @@ -306,11 +335,11 @@ def api_rules(): flask.jsonify(rule_repr(rule, short=False)), response_code) -@app.route('/v1/rules/', methods=['GET', 'DELETE']) -@convert_exceptions +@api('/v1/rules/', + rule="introspection:rule:{}", + verb_to_rule_map={'GET': 'get', 'DELETE': 'delete'}, + methods=['GET', 'DELETE']) def api_rule(uuid): - utils.check_auth(flask.request) - if flask.request.method == 'GET': rule = rules.get(uuid) return flask.jsonify(rule_repr(rule, short=False)) diff --git a/ironic_inspector/policy.py b/ironic_inspector/policy.py new file mode 100644 index 000000000..82723348d --- /dev/null +++ b/ironic_inspector/policy.py @@ -0,0 +1,217 @@ +# 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. + +import itertools +import sys + +from oslo_concurrency import lockutils +from oslo_config import cfg +from oslo_policy import policy + +CONF = cfg.CONF + +_ENFORCER = None + +default_policies = [ + policy.RuleDefault( + 'is_admin', + 'role:admin or role:administrator or role:baremetal_admin', + description='Full read/write API access'), + policy.RuleDefault( + 'is_observer', + 'role:baremetal_observer', + description='Read-only API access'), + policy.RuleDefault( + 'public_api', + 'is_public_api:True', + description='Internal flag for public API routes'), + policy.RuleDefault( + 'default', + '!', + description='Default API access policy'), +] + +api_version_policies = [ + policy.DocumentedRuleDefault( + 'introspection', + 'rule:public_api', + 'Access the API root for available versions information', + [{'path': '/', 'method': 'GET'}] + ), + policy.DocumentedRuleDefault( + 'introspection:version', + 'rule:public_api', + 'Access the versioned API root for version information', + [{'path': '/{version}', 'method': 'GET'}] + ), +] + + +introspection_policies = [ + policy.DocumentedRuleDefault( + 'introspection:continue', + 'rule:public_api', + 'Ramdisk callback to continue introspection', + [{'path': '/continue', 'method': 'POST'}] + ), + policy.DocumentedRuleDefault( + 'introspection:status', + 'rule:is_admin or rule:is_observer', + 'Get introspection status', + [{'path': '/introspection', 'method': 'GET'}, + {'path': '/introspection/{node_id}', 'method': 'GET'}] + ), + policy.DocumentedRuleDefault( + 'introspection:start', + 'rule:is_admin', + 'Start introspection', + [{'path': '/introspection/{node_id}', 'method': 'POST'}] + ), + policy.DocumentedRuleDefault( + 'introspection:abort', + 'rule:is_admin', + 'Abort introspection', + [{'path': '/introspection/{node_id}/abort', 'method': 'POST'}] + ), + policy.DocumentedRuleDefault( + 'introspection:data', + 'rule:is_admin', + 'Get introspection data', + [{'path': '/introspection/{node_id}/data', 'method': 'GET'}] + ), + policy.DocumentedRuleDefault( + 'introspection:reapply', + 'rule:is_admin', + 'Reapply introspection on stored data', + [{'path': '/introspection/{node_id}/data/unprocessed', + 'method': 'POST'}] + ), +] + +rule_policies = [ + policy.DocumentedRuleDefault( + 'introspection:rule:get', + 'rule:is_admin', + 'Get introspection rule(s)', + [{'path': '/rules', 'method': 'GET'}, + {'path': '/rules/{rule_id}', 'method': 'GET'}] + ), + policy.DocumentedRuleDefault( + 'introspection:rule:delete', + 'rule:is_admin', + 'Delete introspection rule(s)', + [{'path': '/rules', 'method': 'DELETE'}, + {'path': '/rules/{rule_id}', 'method': 'DELETE'}] + ), + policy.DocumentedRuleDefault( + 'introspection:rule:create', + 'rule:is_admin', + 'Create introspection rule', + [{'path': '/rules', 'method': 'POST'}] + ), +] + + +def list_policies(): + """Get list of all policies defined in code. + + Used to register them all at runtime, + and by oslo-config-generator to generate sample policy files. + """ + policies = itertools.chain( + default_policies, + api_version_policies, + introspection_policies, + rule_policies) + return policies + + +@lockutils.synchronized('policy_enforcer') +def init_enforcer(policy_file=None, rules=None, + default_rule=None, use_conf=True): + """Synchronously initializes the policy enforcer + + :param policy_file: Custom policy file to use, if none is specified, + `CONF.oslo_policy.policy_file` will be used. + :param rules: Default dictionary / Rules to use. It will be + considered just in the first instantiation. + :param default_rule: Default rule to use, + CONF.oslo_policy.policy_default_rule will + be used if none is specified. + :param use_conf: Whether to load rules from config file. + """ + global _ENFORCER + + if _ENFORCER: + return + _ENFORCER = policy.Enforcer(CONF, policy_file=policy_file, + rules=rules, + default_rule=default_rule, + use_conf=use_conf) + _ENFORCER.register_defaults(list_policies()) + + +def get_enforcer(): + """Provides access to the single instance of Policy enforcer.""" + if not _ENFORCER: + init_enforcer() + return _ENFORCER + + +def get_oslo_policy_enforcer(): + """Get the enforcer instance to generate policy files. + + This method is for use by oslopolicy CLI scripts. + Those scripts need the 'output-file' and 'namespace' options, + but having those in sys.argv means loading the inspector config options + will fail as those are not expected to be present. + So we pass in an arg list with those stripped out. + """ + + conf_args = [] + # Start at 1 because cfg.CONF expects the equivalent of sys.argv[1:] + i = 1 + while i < len(sys.argv): + if sys.argv[i].strip('-') in ['namespace', 'output-file']: + # e.g. --namespace + i += 2 + continue + conf_args.append(sys.argv[i]) + i += 1 + + cfg.CONF(conf_args, project='ironic-inspector') + + return get_enforcer() + + +def authorize(rule, target, creds, *args, **kwargs): + """A shortcut for policy.Enforcer.authorize() + + Checks authorization of a rule against the target and credentials, and + raises an exception if the rule is not defined. + args and kwargs are passed directly to oslo.policy Enforcer.authorize + Always returns True if CONF.auth_strategy != keystone. + + :param rule: name of a registered oslo.policy rule + :param target: dict-like structure to check rule against + :param creds: dict of policy values from request + :returns: True if request is authorized against given policy, + False otherwise + :raises: oslo_policy.policy.PolicyNotRegistered if supplied policy + is not registered in oslo_policy + """ + if CONF.auth_strategy != 'keystone': + return True + enforcer = get_enforcer() + rule = CONF.oslo_policy.policy_default_rule if rule is None else rule + return enforcer.authorize(rule, target, creds, *args, **kwargs) diff --git a/ironic_inspector/test/base.py b/ironic_inspector/test/base.py index ea0ec2706..887098866 100644 --- a/ironic_inspector/test/base.py +++ b/ironic_inspector/test/base.py @@ -30,6 +30,7 @@ from ironic_inspector import db from ironic_inspector import introspection_state as istate from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base +from ironic_inspector.test.unit import policy_fixture from ironic_inspector import utils CONF = conf.cfg.CONF @@ -64,6 +65,7 @@ class BaseTest(test_base.BaseTestCase): self.cfg.set_default('slave_connection', None, group='database') self.cfg.set_default('max_retries', 10, group='database') conf.parse_args([], default_config_files=[]) + self.policy = self.useFixture(policy_fixture.PolicyFixture()) def assertPatchEqual(self, expected, actual): expected = sorted(expected, key=lambda p: p['path']) diff --git a/ironic_inspector/test/unit/policy_fixture.py b/ironic_inspector/test/unit/policy_fixture.py new file mode 100644 index 000000000..e3e0feeea --- /dev/null +++ b/ironic_inspector/test/unit/policy_fixture.py @@ -0,0 +1,40 @@ +# +# 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. + +import os + +import fixtures +from oslo_config import cfg +from oslo_policy import opts as policy_opts + +from ironic_inspector import policy as inspector_policy + +CONF = cfg.CONF + +policy_data = """{ +} +""" + + +class PolicyFixture(fixtures.Fixture): + def setUp(self): + super(PolicyFixture, self).setUp() + self.policy_dir = self.useFixture(fixtures.TempDir()) + self.policy_file_name = os.path.join(self.policy_dir.path, + 'policy.json') + with open(self.policy_file_name, 'w') as policy_file: + policy_file.write(policy_data) + policy_opts.set_defaults(CONF) + CONF.set_override('policy_file', self.policy_file_name, 'oslo_policy') + inspector_policy._ENFORCER = None + self.addCleanup(inspector_policy.get_enforcer().clear) diff --git a/ironic_inspector/test/unit/test_utils.py b/ironic_inspector/test/unit/test_utils.py index 171503d1f..44be419b4 100644 --- a/ironic_inspector/test/unit/test_utils.py +++ b/ironic_inspector/test/unit/test_utils.py @@ -14,6 +14,7 @@ from keystonemiddleware import auth_token from oslo_config import cfg +from ironic_inspector.common import context from ironic_inspector import node_cache from ironic_inspector.test import base from ironic_inspector import utils @@ -30,17 +31,16 @@ CONF = cfg.CONF class TestCheckAuth(base.BaseTest): def setUp(self): super(TestCheckAuth, self).setUp() - CONF.set_override('auth_strategy', 'keystone') + self.cfg.config(auth_strategy='keystone') @mock.patch.object(auth_token, 'AuthProtocol') def test_middleware(self, mock_auth): - CONF.set_override('admin_user', 'admin', 'keystone_authtoken') - CONF.set_override('admin_tenant_name', 'admin', 'keystone_authtoken') - CONF.set_override('admin_password', 'password', 'keystone_authtoken') - CONF.set_override('auth_uri', 'http://127.0.0.1:5000', - 'keystone_authtoken') - CONF.set_override('identity_uri', 'http://127.0.0.1:35357', - 'keystone_authtoken') + self.cfg.config(group='keystone_authtoken', + admin_user='admin', + admin_tenant_name='admin', + admin_password='password', + auth_uri='http://127.0.0.1:5000', + identity_uri='http://127.0.0.1:35357') app = mock.Mock(wsgi_app=mock.sentinel.app) utils.add_auth_middleware(app) @@ -57,25 +57,31 @@ class TestCheckAuth(base.BaseTest): self.assertEqual('http://127.0.0.1:5000', args1['auth_uri']) self.assertEqual('http://127.0.0.1:35357', args1['identity_uri']) - def test_ok(self): - request = mock.Mock(headers={'X-Identity-Status': 'Confirmed', - 'X-Roles': 'admin,member'}) - utils.check_auth(request) + def test_admin(self): + request = mock.Mock(headers={'X-Identity-Status': 'Confirmed'}) + request.context = context.RequestContext(roles=['admin']) + utils.check_auth(request, rule="is_admin") def test_invalid(self): request = mock.Mock(headers={'X-Identity-Status': 'Invalid'}) self.assertRaises(utils.Error, utils.check_auth, request) def test_not_admin(self): - request = mock.Mock(headers={'X-Identity-Status': 'Confirmed', - 'X-Roles': 'member'}) - self.assertRaises(utils.Error, utils.check_auth, request) + request = mock.Mock(headers={'X-Identity-Status': 'Confirmed'}) + request.context = context.RequestContext(roles=['member']) + self.assertRaises(utils.Error, utils.check_auth, request, + rule="is_admin") def test_disabled(self): - CONF.set_override('auth_strategy', 'noauth') + self.cfg.config(auth_strategy='noauth') request = mock.Mock(headers={'X-Identity-Status': 'Invalid'}) utils.check_auth(request) + def test_public_api(self): + request = mock.Mock(headers={'X-Identity-Status': 'Invalid'}) + request.context = context.RequestContext(is_public_api=True) + utils.check_auth(request, "public_api") + class TestProcessingLogger(base.BaseTest): def test_prefix_no_info(self): diff --git a/ironic_inspector/utils.py b/ironic_inspector/utils.py index 237d1b70d..274686f0f 100644 --- a/ironic_inspector/utils.py +++ b/ironic_inspector/utils.py @@ -24,6 +24,7 @@ import pytz from ironic_inspector.common.i18n import _ from ironic_inspector import conf # noqa +from ironic_inspector import policy CONF = cfg.CONF @@ -170,20 +171,21 @@ def add_cors_middleware(app): app.wsgi_app = cors_middleware.CORS(app.wsgi_app, CONF) -def check_auth(request): +def check_auth(request, rule=None, target=None): """Check authentication on request. :param request: Flask request + :param rule: policy rule to check the request against :raises: utils.Error if access is denied """ if CONF.auth_strategy == 'noauth': return - if request.headers.get('X-Identity-Status').lower() == 'invalid': - raise Error(_('Authentication required'), code=401) - roles = (request.headers.get('X-Roles') or '').split(',') - if 'admin' not in roles: - LOG.error('Role "admin" not in user role list %s', roles) - raise Error(_('Access denied'), code=403) + if not request.context.is_public_api: + if request.headers.get('X-Identity-Status', '').lower() == 'invalid': + raise Error(_('Authentication required'), code=401) + target = {} if target is None else target + if not policy.authorize(rule, target, request.context.to_policy_values()): + raise Error(_("Access denied by policy"), code=403) def get_valid_macs(data): diff --git a/policy-generator.conf b/policy-generator.conf new file mode 100644 index 000000000..e5d39d907 --- /dev/null +++ b/policy-generator.conf @@ -0,0 +1,3 @@ +[DEFAULT] +output_file = policy.yaml.sample +namespace = ironic_inspector.api diff --git a/policy.yaml.sample b/policy.yaml.sample new file mode 100644 index 000000000..b94c2e7fd --- /dev/null +++ b/policy.yaml.sample @@ -0,0 +1,59 @@ +# Full read/write API access +#"is_admin": "role:admin or role:administrator or role:baremetal_admin" + +# Read-only API access +#"is_observer": "role:baremetal_observer" + +# Internal flag for public API routes +#"public_api": "is_public_api:True" + +# Default API access policy +#"default": "!" + +# Access the API root for available versions information +# GET / +#"introspection": "rule:public_api" + +# Access the versioned API root for version information +# GET /{version} +#"introspection:version": "rule:public_api" + +# Ramdisk callback to continue introspection +# POST /continue +#"introspection:continue": "rule:public_api" + +# Get introspection status +# GET /introspection +# GET /introspection/{node_id} +#"introspection:status": "rule:is_admin or rule:is_observer" + +# Start introspection +# POST /introspection/{node_id} +#"introspection:start": "rule:is_admin" + +# Abort introspection +# POST /introspection/{node_id}/abort +#"introspection:abort": "rule:is_admin" + +# Get introspection data +# GET /introspection/{node_id}/data +#"introspection:data": "rule:is_admin" + +# Reapply introspection on stored data +# POST /introspection/{node_id}/data/unprocessed +#"introspection:reapply": "rule:is_admin" + +# Get introspection rule(s) +# GET /rules +# GET /rules/{rule_id} +#"introspection:rule:get": "rule:is_admin" + +# Delete introspection rule(s) +# DELETE /rules +# DELETE /rules/{rule_id} +#"introspection:rule:delete": "rule:is_admin" + +# Create introspection rule +# POST /rules +#"introspection:rule:create": "rule:is_admin" + diff --git a/releasenotes/notes/policy-engine-c44828e3131e6c62.yaml b/releasenotes/notes/policy-engine-c44828e3131e6c62.yaml new file mode 100644 index 000000000..0bfef657c --- /dev/null +++ b/releasenotes/notes/policy-engine-c44828e3131e6c62.yaml @@ -0,0 +1,35 @@ +--- +features: + - | + Added an API access policy enforcment (based on oslo.policy rules). + Similar to other OpenStack services, operators now can configure + fine-grained access policies using ``policy.yaml`` file. + See example ``policy.yaml.sample`` file included in the code tree + for the list of available policies and their default rules. + This file can also be generated from the code tree + with ``tox -egenpolicy`` command. + + See ``oslo.policy`` package documentation for more information + on using and configuring API access policies. + +upgrade: + - | + Due to the choice of default values for API access policies rules, + some API parts of the ironic-inspector service will become available + to wider range of users after upgrade: + + - general access to the whole API is by default granted to a user + with either ``admin``, ``administrator`` or ``baremetal_admin`` + role (previously it allowed access only to a user with ``admin`` + role) + - listing of current introspections and showing a given + introspection is by default also allowed to the user with the + ``baremetal_observer`` role + + If these access policies are not suiting a given deployment before + upgrade, operator will have to create a ``policy.json`` file + in the inspector configuration folder (usually ``/etc/inspector``) + that redefines the API rules as required. + + See ``oslo.policy`` package documentation for more information + on using and configuring API access policies. diff --git a/requirements.txt b/requirements.txt index 8531d56fd..6fa8deece 100644 --- a/requirements.txt +++ b/requirements.txt @@ -20,10 +20,12 @@ python-swiftclient>=3.2.0 # Apache-2.0 pytz>=2013.6 # MIT oslo.concurrency>=3.20.0 # Apache-2.0 oslo.config>=4.6.0 # Apache-2.0 +oslo.context>=2.14.0,!=2.19.1 # Apache-2.0 oslo.db>=4.27.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.log>=3.30.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 +oslo.policy>=1.23.0 # Apache-2.0 oslo.rootwrap>=5.8.0 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.utils>=3.28.0 # Apache-2.0 diff --git a/setup.cfg b/setup.cfg index 5200b5a3f..2c2193132 100644 --- a/setup.cfg +++ b/setup.cfg @@ -68,6 +68,10 @@ oslo.config.opts = ironic_inspector.plugins.pci_devices = ironic_inspector.plugins.pci_devices:list_opts oslo.config.opts.defaults = ironic_inspector = ironic_inspector.conf:set_config_defaults +oslo.policy.enforcer = + ironic_inspector = ironic_inspector.policy:get_oslo_policy_enforcer +oslo.policy.policies = + ironic_inspector.api = ironic_inspector.policy:list_policies tempest.test_plugins = ironic_inspector_tests = ironic_inspector.test.inspector_tempest_plugin.plugin:InspectorTempestPlugin diff --git a/tox.ini b/tox.ini index fd77bfc00..4f781afdd 100644 --- a/tox.ini +++ b/tox.ini @@ -54,6 +54,11 @@ commands = envdir = {toxworkdir}/venv commands = oslo-config-generator --config-file config-generator.conf +[testenv:genpolicy] +sitepackages = False +envdir = {toxworkdir}/venv +commands = oslopolicy-sample-generator --config-file {toxinidir}/policy-generator.conf + [testenv:genstates] deps = {[testenv]deps} commands = {toxinidir}/tools/states_to_dot.py -f {toxinidir}/doc/source/images/states.svg --format svg