From 060ca2ee369af1fff3bf833d466f33d1889fb72e Mon Sep 17 00:00:00 2001 From: Ade Lee Date: Tue, 9 Mar 2021 00:30:29 -0500 Subject: [PATCH] Implement secure RBAC for secretstore API Add new system scope specific RBAC rules for the secretstore API. The new rules allow all roles to list and get secret stores. Change-Id: Ibb19e9854e8bafd2a454c0792503c6f4360e7cf7 --- barbican/common/policies/secretstores.py | 22 +++++----- barbican/tests/api/test_resources_policy.py | 41 +++++++------------ .../api/v1/functional/test_secretstores.py | 15 +++++-- ...c-secretstore-policy-ffa782850082add8.yaml | 9 ++++ 4 files changed, 47 insertions(+), 40 deletions(-) create mode 100644 releasenotes/notes/secure-rbac-secretstore-policy-ffa782850082add8.yaml diff --git a/barbican/common/policies/secretstores.py b/barbican/common/policies/secretstores.py index 6e7b035fe..38abe15ce 100644 --- a/barbican/common/policies/secretstores.py +++ b/barbican/common/policies/secretstores.py @@ -13,11 +13,13 @@ from oslo_policy import policy +_READER = "role:reader" + rules = [ policy.DocumentedRuleDefault( name='secretstores:get', - check_str='rule:admin', - scope_types=[], + check_str=f'rule:all_users or {_READER}', + scope_types=['project', 'system'], description='Get list of available secret store backends.', operations=[ { @@ -28,8 +30,8 @@ rules = [ ), policy.DocumentedRuleDefault( name='secretstores:get_global_default', - check_str='rule:admin', - scope_types=[], + check_str=f'rule:all_users or {_READER}', + scope_types=['project', 'system'], description='Get a reference to the secret store that is used as ' + 'default secret store backend for the deployment.', operations=[ @@ -41,8 +43,8 @@ rules = [ ), policy.DocumentedRuleDefault( name='secretstores:get_preferred', - check_str='rule:admin', - scope_types=[], + check_str=f'rule:all_users or {_READER}', + scope_types=['project', 'system'], description='Get a reference to the preferred secret store if ' + 'assigned previously.', operations=[ @@ -55,7 +57,7 @@ rules = [ policy.DocumentedRuleDefault( name='secretstore_preferred:post', check_str='rule:admin', - scope_types=[], + scope_types=['project'], description='Set a secret store backend to be preferred store ' + 'backend for their project.', operations=[ @@ -68,7 +70,7 @@ rules = [ policy.DocumentedRuleDefault( name='secretstore_preferred:delete', check_str='rule:admin', - scope_types=[], + scope_types=['project'], description='Remove preferred secret store backend setting for ' + 'their project.', operations=[ @@ -80,8 +82,8 @@ rules = [ ), policy.DocumentedRuleDefault( name='secretstore:get', - check_str='rule:admin', - scope_types=[], + check_str=f'rule:all_users or {_READER}', + scope_types=['project', 'system'], description='Get details of secret store by its ID.', operations=[ { diff --git a/barbican/tests/api/test_resources_policy.py b/barbican/tests/api/test_resources_policy.py index a892aa562..3b00b6be8 100644 --- a/barbican/tests/api/test_resources_policy.py +++ b/barbican/tests/api/test_resources_policy.py @@ -1164,28 +1164,19 @@ class WhenTestingSecretStoresResource(BaseTestCase): self.assertIsNotNone(self.policy_enforcer.rules) def test_should_pass_get_all_secret_stores(self): - self._assert_pass_rbac(['admin'], - self._invoke_on_get) - - def test_should_raise_get_all_secret_stores(self): - self._assert_fail_rbac([None, 'creator', 'observer', 'audit'], - self._invoke_on_get) + self._assert_pass_rbac( + ['admin', 'observer', 'audit', 'creator', 'reader'], + self._invoke_on_get) def test_should_pass_get_global_default(self): - self._assert_pass_rbac(['admin'], - self._invoke_get_global_default) - - def test_should_raise_get_global_default(self): - self._assert_fail_rbac([None, 'creator', 'observer', 'audit'], - self._invoke_get_global_default) + self._assert_pass_rbac( + ['admin', 'observer', 'audit', 'creator', 'reader'], + self._invoke_get_global_default) def test_should_pass_get_preferred(self): - self._assert_pass_rbac(['admin'], - self._invoke_get_preferred) - - def test_should_raise_get_preferred(self): - self._assert_fail_rbac([None, 'creator', 'observer', 'audit'], - self._invoke_get_preferred) + self._assert_pass_rbac( + ['admin', 'observer', 'audit', 'creator', 'reader'], + self._invoke_get_preferred) def _invoke_on_get(self): self.resource.on_get(self.req, self.resp) @@ -1235,12 +1226,9 @@ class WhenTestingSecretStoreResource(BaseTestCase): self.assertIsNotNone(self.policy_enforcer.rules) def test_should_pass_get_a_secret_store(self): - self._assert_pass_rbac(['admin'], - self._invoke_on_get) - - def test_should_raise_get_a_secret_store(self): - self._assert_fail_rbac([None, 'creator', 'observer', 'audit'], - self._invoke_on_get) + self._assert_pass_rbac( + ['admin', 'observer', 'audit', 'creator', 'reader'], + self._invoke_on_get) def _invoke_on_get(self): self.resource.on_get(self.req, self.resp) @@ -1278,8 +1266,9 @@ class WhenTestingPreferredSecretStoreResource(BaseTestCase): self._invoke_on_post) def test_should_raise_set_preferred_secret_store(self): - self._assert_fail_rbac([None, 'creator', 'observer', 'audit'], - self._invoke_on_post) + self._assert_fail_rbac( + [None, 'creator', 'observer', 'audit', 'reader'], + self._invoke_on_post) def _invoke_on_post(self): self.resource.on_post(self.req, self.resp) diff --git a/functionaltests/api/v1/functional/test_secretstores.py b/functionaltests/api/v1/functional/test_secretstores.py index 759d47fdb..ac3a932b7 100644 --- a/functionaltests/api/v1/functional/test_secretstores.py +++ b/functionaltests/api/v1/functional/test_secretstores.py @@ -29,6 +29,13 @@ admin_b = CONF.rbac_users.admin_b observer_b = CONF.rbac_users.observer_b test_user_data_when_enabled = { + 'with_admin_a': {'user': admin_a, 'expected_return': 200}, + 'with_creator_a': {'user': creator_a, 'expected_return': 200}, + 'with_observer_a': {'user': observer_a, 'expected_return': 200}, + 'with_auditor_a': {'user': auditor_a, 'expected_return': 200}, +} + +test_user_data_admin_ops_when_enabled = { 'with_admin_a': {'user': admin_a, 'expected_return': 200}, 'with_creator_a': {'user': creator_a, 'expected_return': 403}, 'with_observer_a': {'user': observer_a, 'expected_return': 403}, @@ -37,9 +44,9 @@ test_user_data_when_enabled = { test_user_data_when_not_enabled = { 'with_admin_a': {'user': admin_a, 'expected_return': 404}, - 'with_creator_a': {'user': creator_a, 'expected_return': 403}, - 'with_observer_a': {'user': observer_a, 'expected_return': 403}, - 'with_auditor_a': {'user': auditor_a, 'expected_return': 403}, + 'with_creator_a': {'user': creator_a, 'expected_return': 404}, + 'with_observer_a': {'user': observer_a, 'expected_return': 404}, + 'with_auditor_a': {'user': auditor_a, 'expected_return': 404}, } @@ -183,7 +190,7 @@ class SecretStoresTestCase(base.TestCase): @testcase.skipUnless(base.conf_multiple_backends_enabled, 'executed only ' 'when multiple backends support is enabled in ' 'barbican server side') - @utils.parameterized_dataset(test_user_data_when_enabled) + @utils.parameterized_dataset(test_user_data_admin_ops_when_enabled) def test_unset_project_preferred_store_multiple_enabled(self, user, expected_return): diff --git a/releasenotes/notes/secure-rbac-secretstore-policy-ffa782850082add8.yaml b/releasenotes/notes/secure-rbac-secretstore-policy-ffa782850082add8.yaml new file mode 100644 index 000000000..86ba476b0 --- /dev/null +++ b/releasenotes/notes/secure-rbac-secretstore-policy-ffa782850082add8.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Implement secure-rbac for secretstores resource. +security: + - | + The current policy only allows users with the admin role to + list and get secretstore resources. The new policy allows all + users to perform these operations.