From 454a7b0ec172763bbb9531107310d8228131943f Mon Sep 17 00:00:00 2001 From: Pavlo Shchelokovskyy Date: Thu, 20 Aug 2015 14:39:14 +0000 Subject: [PATCH] Add resource_type-specific policies Heat's `policy.json` now can contain policies of the following schema: "resource_types:": "rule" This will allow cloud admins to control resource access utilizing user roles, names, tenants and any other oslo.policy-supported rules. Basic usage is to facilitate fail-early for stacks with resources that a given user will not be able to actually create due to role restrictions. Default policy is 'allow to everyone' (who has passed previous policy checks on REST API layer). Resource types that the user will not be able to use due to resources policy restrictions are hidden from `resource-type-list`. Current operations that are prohibited if the user does not pass policy check for a particular "forbidden" resource: - show resource type for forbidden resource type - show resource template for forbidden resource type - create a stack containing a forbidden resource - delete a stack containing a forbidden resource - update a stack that already has a forbidden resource - update a stack initroducing a new forbidden resource - restore a stack snapshot to a stack that currently has forbidden resource Not yet prohibited, need to be fixed: - restore a stack snapshot that will create a forbidden resource As first step (and for testing purposes) OS::Nova::Flavor is forbidden to create for non-admin users. Simple functional test using this resource is added. Change-Id: I337306c4f1624552a2631e0ffbb43f0d3102813d Implements blueprint conditional-resource-exposure --- etc/heat/policy.json | 4 +- heat/common/exception.py | 5 +- heat/common/policy.py | 31 +++++++++++++ heat/engine/environment.py | 14 ++++++ heat/engine/resource.py | 2 +- heat/engine/service.py | 14 ++++++ heat/tests/common.py | 7 ++- heat/tests/policy/resources.json | 6 +++ heat/tests/test_common_policy.py | 46 ++++++++++++++++++- .../functional/test_conditional_exposure.py | 33 +++++++++++++ 10 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 heat/tests/policy/resources.json diff --git a/etc/heat/policy.json b/etc/heat/policy.json index a843ddabf7..06314cd8db 100644 --- a/etc/heat/policy.json +++ b/etc/heat/policy.json @@ -73,5 +73,7 @@ "software_deployments:delete": "rule:deny_stack_user", "software_deployments:metadata": "", - "service:index": "rule:context_is_admin" + "service:index": "rule:context_is_admin", + + "resource_types:OS::Nova::Flavor": "rule:context_is_admin" } diff --git a/heat/common/exception.py b/heat/common/exception.py index a67769b379..478797333a 100644 --- a/heat/common/exception.py +++ b/heat/common/exception.py @@ -107,7 +107,10 @@ class NotAuthenticated(HeatException): class Forbidden(HeatException): - msg_fmt = _("You are not authorized to complete this action.") + msg_fmt = _("You are not authorized to use %(action)s.") + + def __init__(self, action='this action'): + super(Forbidden, self).__init__(action=action) # NOTE(bcwaldon): here for backwards-compatibility, need to deprecate. diff --git a/heat/common/policy.py b/heat/common/policy.py index 10d5fb683f..3132e0d8a7 100644 --- a/heat/common/policy.py +++ b/heat/common/policy.py @@ -18,14 +18,18 @@ """Policy Engine For Heat""" from oslo_config import cfg +from oslo_log import log as logging from oslo_policy import policy +import six from heat.common import exception CONF = cfg.CONF +LOG = logging.getLogger(__name__) DEFAULT_RULES = policy.Rules.from_dict({'default': '!'}) +DEFAULT_RESOURCE_RULES = policy.Rules.from_dict({'default': '@'}) class Enforcer(object): @@ -82,3 +86,30 @@ class Enforcer(object): :returns: A non-False value if the user is admin according to policy """ return self._check(context, 'context_is_admin', target={}, exc=None) + + +class ResourceEnforcer(Enforcer): + def __init__(self, default_rule=DEFAULT_RESOURCE_RULES['default'], + **kwargs): + super(ResourceEnforcer, self).__init__( + default_rule=default_rule, **kwargs) + + def enforce(self, context, res_type, scope=None, target=None): + # NOTE(pas-ha): try/except just to log the exception + try: + result = super(ResourceEnforcer, self).enforce( + context, res_type, + scope=scope or 'resource_types', + target=target) + except self.exc as ex: + LOG.info(six.text_type(ex)) + raise + if not result: + if self.exc: + raise self.exc(action=res_type) + else: + return result + + def enforce_stack(self, stack, scope=None, target=None): + for res in stack.resources.values(): + self.enforce(stack.context, res.type(), scope=scope, target=target) diff --git a/heat/engine/environment.py b/heat/engine/environment.py index b061125b2f..2cc03bc4df 100644 --- a/heat/engine/environment.py +++ b/heat/engine/environment.py @@ -28,6 +28,7 @@ from heat.common.i18n import _ from heat.common.i18n import _LE from heat.common.i18n import _LI from heat.common.i18n import _LW +from heat.common import policy from heat.engine import support LOG = log.getLogger(__name__) @@ -458,10 +459,23 @@ class ResourceRegistry(object): def not_hidden_matches(cls): return cls.get_class().support_status.status != support.HIDDEN + def is_allowed(enforcer, name): + if cnxt is None: + return True + try: + enforcer.enforce(cnxt, name) + except enforcer.exc: + return False + else: + return True + + enforcer = policy.ResourceEnforcer() + return [name for name, cls in six.iteritems(self._registry) if (is_resource(name) and status_matches(cls) and is_available(cls) and + is_allowed(enforcer, name) and not_hidden_matches(cls))] diff --git a/heat/engine/resource.py b/heat/engine/resource.py index bbab9a8a03..9c9451eda1 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -176,7 +176,7 @@ class Resource(object): service_name=ResourceClass.default_client_name, resource_type=definition.resource_type ) - LOG.error(six.text_type(ex)) + LOG.info(six.text_type(ex)) raise ex diff --git a/heat/engine/service.py b/heat/engine/service.py index a76f1372fb..7dece62540 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -39,6 +39,7 @@ from heat.common.i18n import _LI from heat.common.i18n import _LW from heat.common import identifier from heat.common import messaging as rpc_messaging +from heat.common import policy from heat.common import service_utils from heat.engine import api from heat.engine import attributes @@ -293,6 +294,7 @@ class EngineService(service.Service): self.manage_thread_grp = None self._rpc_server = None self.software_config = service_software_config.SoftwareConfigService() + self.resource_enforcer = policy.ResourceEnforcer() if cfg.CONF.trusts_delegated_roles: warnings.warn('The default value of "trusts_delegated_roles" ' @@ -641,6 +643,7 @@ class EngineService(service.Service): args, convergence=conv_eng) + self.resource_enforcer.enforce_stack(stack) return api.format_stack_preview(stack) @context.request_context @@ -700,6 +703,7 @@ class EngineService(service.Service): nested_depth, user_creds_id, stack_user_project_id, convergence, parent_resource_name) + self.resource_enforcer.enforce_stack(stack) stack.store() _create_stack_user(stack) if convergence: @@ -734,6 +738,7 @@ class EngineService(service.Service): LOG.info(_LI('Updating stack %s'), db_stack.name) current_stack = parser.Stack.load(cnxt, stack=db_stack) + self.resource_enforcer.enforce_stack(current_stack) if current_stack.action == current_stack.SUSPEND: msg = _('Updating a stack when it is suspended') @@ -778,6 +783,7 @@ class EngineService(service.Service): current_kwargs.update(common_params) updated_stack = parser.Stack(cnxt, stack_name, tmpl, **current_kwargs) + self.resource_enforcer.enforce_stack(updated_stack) updated_stack.parameters.set_stack_id(current_stack.identifier()) self._validate_deferred_auth_context(cnxt, updated_stack) @@ -957,6 +963,7 @@ class EngineService(service.Service): st = self._get_stack(cnxt, stack_identity) LOG.info(_LI('Deleting stack %s'), st.name) stack = parser.Stack.load(cnxt, stack=st) + self.resource_enforcer.enforce_stack(stack) if stack.convergence: template = templatem.Template.create_empty_template() @@ -1067,6 +1074,7 @@ class EngineService(service.Service): :param cnxt: RPC context. :param type_name: Name of the resource type to obtain the schema of. """ + self.resource_enforcer.enforce(cnxt, type_name) try: resource_class = resources.global_env().get_class(type_name) except (exception.InvalidResourceType, @@ -1112,6 +1120,7 @@ class EngineService(service.Service): :param type_name: Name of the resource type to generate a template for. :param template_type: the template type to generate, cfn or hot. """ + self.resource_enforcer.enforce(cnxt, type_name) try: resource_class = resources.global_env().get_class(type_name) if resource_class.support_status.status == support.HIDDEN: @@ -1314,6 +1323,7 @@ class EngineService(service.Service): s = self._get_stack(cnxt, stack_identity) stack = parser.Stack.load(cnxt, stack=s) + self.resource_enforcer.enforce_stack(stack) self.thread_group_mgr.start_with_lock(cnxt, stack, self.engine_id, _stack_suspend, stack) @@ -1329,6 +1339,7 @@ class EngineService(service.Service): s = self._get_stack(cnxt, stack_identity) stack = parser.Stack.load(cnxt, stack=s) + self.resource_enforcer.enforce_stack(stack) self.thread_group_mgr.start_with_lock(cnxt, stack, self.engine_id, _stack_resume, stack) @@ -1415,8 +1426,11 @@ class EngineService(service.Service): s = self._get_stack(cnxt, stack_identity) stack = parser.Stack.load(cnxt, stack=s) + self.resource_enforcer.enforce_stack(stack) snapshot = snapshot_object.Snapshot.get_snapshot_by_stack( cnxt, snapshot_id, s) + # FIXME(pas-ha) has to be ammended to deny restoring stacks + # that have disallowed for current user self.thread_group_mgr.start_with_lock(cnxt, stack, self.engine_id, _stack_restore, stack, snapshot) diff --git a/heat/tests/common.py b/heat/tests/common.py index f6a857d647..d669eb1b46 100644 --- a/heat/tests/common.py +++ b/heat/tests/common.py @@ -25,6 +25,7 @@ import testtools from heat.common import context from heat.common import messaging +from heat.common import policy from heat.engine.clients.os import cinder from heat.engine.clients.os import glance from heat.engine.clients.os import keystone @@ -78,7 +79,8 @@ class FakeLogMixin(object): class HeatTestCase(testscenarios.WithScenarios, testtools.TestCase, FakeLogMixin): - def setUp(self, mock_keystone=True, quieten_logging=True): + def setUp(self, mock_keystone=True, mock_resource_policy=True, + quieten_logging=True): super(HeatTestCase, self).setUp() self.m = mox.Mox() self.addCleanup(self.m.UnsetStubs) @@ -124,6 +126,9 @@ class HeatTestCase(testscenarios.WithScenarios, if mock_keystone: self.stub_keystoneclient() + if mock_resource_policy: + self.mock_resource_policy = self.patchobject( + policy.ResourceEnforcer, 'enforce') utils.setup_dummy_db() self.register_test_resources() self.addCleanup(utils.reset_dummy_db) diff --git a/heat/tests/policy/resources.json b/heat/tests/policy/resources.json new file mode 100644 index 0000000000..045e60d29c --- /dev/null +++ b/heat/tests/policy/resources.json @@ -0,0 +1,6 @@ +{ + "context_is_admin": "role:admin", + + "resource_types:OS::Test::AdminOnly": "rule:context_is_admin" + +} diff --git a/heat/tests/test_common_policy.py b/heat/tests/test_common_policy.py index a8cca5e275..3289a8ea73 100644 --- a/heat/tests/test_common_policy.py +++ b/heat/tests/test_common_policy.py @@ -40,7 +40,7 @@ class TestPolicyEnforcer(common.HeatTestCase): "PutMetricAlarm", "PutMetricData", "SetAlarmState") def setUp(self): - super(TestPolicyEnforcer, self).setUp() + super(TestPolicyEnforcer, self).setUp(mock_resource_policy=False) opts = [ cfg.StrOpt('config_dir', default=policy_path), cfg.StrOpt('config_file', default='foo'), @@ -183,3 +183,47 @@ class TestPolicyEnforcer(common.HeatTestCase): False, exc=None).AndReturn(True) self.m.ReplayAll() self.assertTrue(enforcer.check_is_admin(ctx)) + + def test_resource_default_rule(self): + context = utils.dummy_context(roles=['non-admin']) + enforcer = policy.ResourceEnforcer( + policy_file=self.get_policy_file('resources.json')) + res_type = "OS::Test::NotInPolicy" + self.assertIsNone(enforcer.enforce(context, res_type)) + + def test_resource_enforce_success(self): + context = utils.dummy_context(roles=['admin']) + enforcer = policy.ResourceEnforcer( + policy_file=self.get_policy_file('resources.json')) + res_type = "OS::Test::AdminOnly" + self.assertIsNone(enforcer.enforce(context, res_type)) + + def test_resource_enforce_fail(self): + context = utils.dummy_context(roles=['non-admin']) + enforcer = policy.ResourceEnforcer( + policy_file=self.get_policy_file('resources.json')) + res_type = "OS::Test::AdminOnly" + ex = self.assertRaises(exception.Forbidden, + enforcer.enforce, + context, res_type) + self.assertIn(res_type, ex.message) + + def test_resource_enforce_returns_false(self): + context = utils.dummy_context(roles=['non-admin']) + enforcer = policy.ResourceEnforcer( + policy_file=self.get_policy_file('resources.json'), + exc=None) + res_type = "OS::Test::AdminOnly" + self.assertFalse(enforcer.enforce(context, res_type)) + + def test_resource_enforce_exc_on_false(self): + context = utils.dummy_context(roles=['non-admin']) + enforcer = policy.ResourceEnforcer( + policy_file=self.get_policy_file('resources.json')) + res_type = "OS::Test::AdminOnly" + self.patchobject(base_policy.Enforcer, 'enforce', + return_value=False) + ex = self.assertRaises(exception.Forbidden, + enforcer.enforce, + context, res_type) + self.assertIn(res_type, ex.message) diff --git a/heat_integrationtests/functional/test_conditional_exposure.py b/heat_integrationtests/functional/test_conditional_exposure.py index 99e76ee38d..d03771235f 100644 --- a/heat_integrationtests/functional/test_conditional_exposure.py +++ b/heat_integrationtests/functional/test_conditional_exposure.py @@ -66,3 +66,36 @@ resources: template=self.unavailable_template) self.assertIn('ResourceTypeUnavailable', ex.message) self.assertIn('OS::Sahara::NodeGroupTemplate', ex.message) + + +class RoleBasedExposureTest(functional_base.FunctionalTestsBase): + forbidden_resource_type = "OS::Nova::Flavor" + fl_tmpl = """ +heat_template_version: 2015-10-15 + +resources: + not4everyone: + type: OS::Nova::Flavor + properties: + ram: 20000 + vcpus: 10 +""" + + def test_non_admin_forbidden_create_flavors(self): + """Fail to create Flavor resource w/o admin role + + Integration tests job runs as normal OpenStack user, + and OS::Nova:Flavor is configured to require + admin role in default policy file of Heat. + """ + stack_name = self._stack_rand_name() + ex = self.assertRaises(exc.Forbidden, + self.client.stacks.create, + stack_name=stack_name, + template=self.fl_tmpl) + self.assertIn(self.forbidden_resource_type, ex.message) + + def test_forbidden_resource_not_listed(self): + resources = self.client.resource_types.list() + self.assertNotIn(self.forbidden_resource_type, + (r.resource_type for r in resources))