From bf838242ac4ccca4d7341e2d1ee8187e3101d425 Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Fri, 19 Feb 2021 05:22:25 +0000 Subject: [PATCH] Fail to start if authorization and policy is misconfigured This informs operators of glance's support status for secure RBAC as of the Wallaby release. Eventually, this message will be removed when glance adopts more support for secure RBAC personas. This also forces glance to fail if it's configured improperly. This is done to explicitly prevent ambiguity with authoritative decisions. Related: blueprint secure-rbac Change-Id: I06293de08dd3fdfbd60b9a65501d1198f40ff434 --- devstack/plugin.sh | 1 + glance/api/policy.py | 11 ++++++- glance/cmd/api.py | 8 ++++++ glance/common/config.py | 25 ++++++++++++++++ glance/common/wsgi_app.py | 12 ++++++++ glance/tests/unit/common/test_wsgi_app.py | 35 +++++++++++++++++++++++ glance/tests/unit/test_policy.py | 21 ++++++++++++++ 7 files changed, 112 insertions(+), 1 deletion(-) diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 6a9ea741ba..68d50b95d5 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -15,6 +15,7 @@ function configure_enforce_scope { iniset $GLANCE_CONF_DIR/glance-api.conf oslo_policy enforce_scope true iniset $GLANCE_CONF_DIR/glance-api.conf oslo_policy enforce_new_defaults true + iniset $GLANCE_CONF_DIR/glance-api.conf DEFAULT enforce_secure_rbac true sudo systemctl restart devstack@g-api } diff --git a/glance/api/policy.py b/glance/api/policy.py index 2bd4fa71af..62ba6c2460 100644 --- a/glance/api/policy.py +++ b/glance/api/policy.py @@ -26,7 +26,7 @@ from oslo_policy import policy from glance.common import exception import glance.domain.proxy -from glance.i18n import _ +from glance.i18n import _, _LW from glance import policies @@ -48,6 +48,15 @@ class Enforcer(policy.Enforcer): def __init__(self): super(Enforcer, self).__init__(CONF, use_conf=True, overwrite=False) self.register_defaults(policies.list_rules()) + if CONF.enforce_secure_rbac and CONF.oslo_policy.enforce_new_defaults: + LOG.warning(_LW( + "Deploying glance with secure RBAC personas enabled via " + "`glance-api.conf [DEFAULT] enforce_secure_rbac=True` and " + "`glance-api.conf [oslo_policy] enforce_new_defaults=True` " + "is marked as EXPERIMENTAL in Wallaby. The status of this " + "feature will graduate to SUPPORTED as glance adopts more " + "personas, specifically for system-scope." + )) def add_rules(self, rules): """Add new rules to the Rules object""" diff --git a/glance/cmd/api.py b/glance/cmd/api.py index 47e433ed76..2b6fbef31c 100644 --- a/glance/cmd/api.py +++ b/glance/cmd/api.py @@ -108,6 +108,14 @@ def main(): host=CONF.bind_host ) + if CONF.enforce_secure_rbac != CONF.oslo_policy.enforce_new_defaults: + fail_message = ( + "[DEFAULT] enforce_secure_rbac does not match " + "[oslo_policy] enforce_new_defaults. Please set both to " + "True to enable secure RBAC personas. Otherwise, make sure " + "both are False.") + raise exception.ServerError(fail_message) + # NOTE(danms): Configure system-wide threading model to use eventlet glance.async_.set_threadpool_model('eventlet') diff --git a/glance/common/config.py b/glance/common/config.py index 76ee2e20b5..538a83356e 100644 --- a/glance/common/config.py +++ b/glance/common/config.py @@ -566,6 +566,31 @@ Related options: Related options: * [DEFAULT]/node_staging_uri""")), + cfg.BoolOpt('enforce_secure_rbac', default=False, + deprecated_for_removal=True, + deprecated_reason=_(""" +This option has been introduced to require operators to opt into enforcing +authorization based on common RBAC personas, which is EXPERIMENTAL as of the +Wallaby release. This behavior will be the default and STABLE in a future +release, allowing this option to be removed. +"""), + deprecated_since='Wallaby', + help=_(""" +Enforce API access based on common persona definitions used across OpenStack. +Enabling this option formalizes project-specific read/write operations, like +creating private images or updating the status of shared image, behind the +`member` role. It also formalizes a read-only variant useful for +project-specific API operations, like listing private images in a project, +behind the `reader` role. + +Operators should take an opportunity to understand glance's new image policies, +audit assignments in their deployment, and update permissions using the default +roles in keystone (e.g., `admin`, `member`, and `reader`). + +Related options: + * [oslo_policy]/enforce_new_defaults + +""")), ] wsgi_opts = [ diff --git a/glance/common/wsgi_app.py b/glance/common/wsgi_app.py index d91952e501..130d4a6851 100644 --- a/glance/common/wsgi_app.py +++ b/glance/common/wsgi_app.py @@ -21,6 +21,7 @@ import osprofiler.initializer from glance.api import common import glance.async_ from glance.common import config +from glance.common import exception from glance.common import store_utils from glance.i18n import _ from glance import notifier @@ -69,6 +70,16 @@ def _setup_os_profiler(): host=CONF.bind_host) +def _validate_policy_enforcement_configuration(): + if CONF.enforce_secure_rbac != CONF.oslo_policy.enforce_new_defaults: + fail_message = ( + "[DEFAULT] enforce_secure_rbac does not match " + "[oslo_policy] enforce_new_defaults. Please set both to " + "True to enable secure RBAC personas. Otherwise, make sure " + "both are False.") + raise exception.ServerError(fail_message) + + def drain_threadpools(): # NOTE(danms): If there are any other named pools that we need to # drain before exit, they should be in this list. @@ -112,4 +123,5 @@ def init_app(): glance_store.verify_default_store() _setup_os_profiler() + _validate_policy_enforcement_configuration() return config.load_paste_app('glance-api') diff --git a/glance/tests/unit/common/test_wsgi_app.py b/glance/tests/unit/common/test_wsgi_app.py index f9e0a7687b..c8e3487cbf 100644 --- a/glance/tests/unit/common/test_wsgi_app.py +++ b/glance/tests/unit/common/test_wsgi_app.py @@ -18,6 +18,7 @@ from unittest import mock from glance.api import common import glance.async_ +from glance.common import exception from glance.common import wsgi_app from glance.tests import utils as test_utils @@ -63,3 +64,37 @@ class TestWsgiAppInit(test_utils.BaseTestCase): # Make sure that shutdown() was called on the tasks_pool # ThreadPoolExecutor mock_shutdown.assert_called_once_with() + + @mock.patch('glance.common.config.load_paste_app') + @mock.patch('glance.async_.set_threadpool_model') + @mock.patch('glance.common.wsgi_app._get_config_files') + def test_policy_enforcement_kills_service_if_misconfigured( + self, mock_load_app, mock_set, mock_config_files): + self.config(enforce_new_defaults=True, group='oslo_policy') + self.config(enforce_secure_rbac=False) + self.assertRaises(exception.ServerError, wsgi_app.init_app) + + self.config(enforce_new_defaults=False, group='oslo_policy') + self.config(enforce_secure_rbac=True) + self.assertRaises(exception.ServerError, wsgi_app.init_app) + + @mock.patch('glance.common.config.load_paste_app') + @mock.patch('glance.async_.set_threadpool_model') + @mock.patch('glance.common.wsgi_app._get_config_files') + def test_policy_enforcement_valid_truthy_configuration( + self, mock_load_app, mock_set, mock_config_files): + self.config(enforce_new_defaults=True, group='oslo_policy') + self.config(enforce_secure_rbac=True) + self.assertTrue(wsgi_app.init_app()) + + @mock.patch('glance.common.config.load_paste_app') + @mock.patch('glance.async_.set_threadpool_model') + @mock.patch('glance.common.wsgi_app._get_config_files') + def test_policy_enforcement_valid_falsy_configuration( + self, mock_load_app, mock_set, mock_config_files): + # This is effectively testing the default values, but we're doing that + # to make sure nothing bad happens at runtime in the default case when + # validating policy enforcement configuration. + self.config(enforce_new_defaults=False, group='oslo_policy') + self.config(enforce_secure_rbac=False) + self.assertTrue(wsgi_app.init_app()) diff --git a/glance/tests/unit/test_policy.py b/glance/tests/unit/test_policy.py index d1e8dee916..903dbb0be2 100644 --- a/glance/tests/unit/test_policy.py +++ b/glance/tests/unit/test_policy.py @@ -373,6 +373,27 @@ class TestPolicyEnforcer(base.IsolatedUnitTest): enforcer.check(context, 'foo', {}) mock_enforcer.assert_called_once_with('foo', {}, context) + def test_ensure_experimental_warning_is_logged_for_secure_rbac(self): + self.config(enforce_new_defaults=True, group='oslo_policy') + self.config(enforce_secure_rbac=True) + expected_log_string = ( + "Deploying glance with secure RBAC personas enabled via " + "`glance-api.conf [DEFAULT] enforce_secure_rbac=True` and " + "`glance-api.conf [oslo_policy] enforce_new_defaults=True` " + "is marked as EXPERIMENTAL in Wallaby. The status of this " + "feature will graduate to SUPPORTED as glance adopts more " + "personas, specifically for system-scope." + ) + with mock.patch.object(glance.api.policy, 'LOG') as mock_log: + glance.api.policy.Enforcer() + mock_log.warning.assert_called_once_with(expected_log_string) + + def test_ensure_experimental_warning_is_not_logged_for_legacy_rbac(self): + self.config(enforce_new_defaults=False, group='oslo_policy') + with mock.patch.object(glance.api.policy, 'LOG') as mock_log: + glance.api.policy.Enforcer() + mock_log.warning.assert_not_called() + class TestPolicyEnforcerNoFile(base.IsolatedUnitTest):