From 6e3525188fdfbe7fabd665e21df2068280471689 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Thu, 15 Jun 2023 12:59:03 -0700 Subject: [PATCH] [S-RBAC] Fix policies for CUD subnets APIs In new, secure RBAC policies for create subnet there was rule "ADMIN_OR_PROJECT_MEMBER" used and that was wrong as this rule is basically allows any member (PROJECT_MEMBER) create subnet in networks visible to them, not necessarily this project needs to be owner of that network. So it allowed users to create new subnets in the shared or provider networks as well. Now policy for create subnet is ADMIN OR NET_OWNER_MEMBER to avoid that. Additionally this patch also fixes policies for update and delete subnet APIs where there was rule NET_OWNER used and that effectively allowed to update or delete subnet to the network owner who has READER role only. Now this is also fixed by using NET_OWNER_MEMBER rule instead. Closes-Bug: #2023679 Change-Id: Ia494872b58f368581fb29fa40b7da17e1071db22 --- neutron/conf/policies/subnet.py | 13 ++++++++---- .../tests/unit/conf/policies/test_subnet.py | 20 ++++++++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/neutron/conf/policies/subnet.py b/neutron/conf/policies/subnet.py index 7e93045626e..cc3fdb1b48c 100644 --- a/neutron/conf/policies/subnet.py +++ b/neutron/conf/policies/subnet.py @@ -36,13 +36,18 @@ ACTION_GET = [ {'method': 'GET', 'path': RESOURCE_PATH}, ] +# TODO(slaweq): remove it once network will be added to the +# EXT_PARENT_RESOURCE_MAPPING in neutron_lib and rule base.PARENT_OWNER_MEMBER +# will be possible to use instead of RULE_NET_OWNER_MEMBER +RULE_NET_OWNER_MEMBER = 'role:member and ' + base.RULE_NET_OWNER + rules = [ policy.DocumentedRuleDefault( name='create_subnet', check_str=neutron_policy.policy_or( - base.ADMIN_OR_PROJECT_MEMBER, - base.RULE_NET_OWNER), + base.ADMIN, + RULE_NET_OWNER_MEMBER), scope_types=['project'], description='Create a subnet', operations=ACTION_POST, @@ -112,7 +117,7 @@ rules = [ name='update_subnet', check_str=neutron_policy.policy_or( base.ADMIN_OR_PROJECT_MEMBER, - base.RULE_NET_OWNER), + RULE_NET_OWNER_MEMBER), scope_types=['project'], description='Update a subnet', operations=ACTION_PUT, @@ -150,7 +155,7 @@ rules = [ name='delete_subnet', check_str=neutron_policy.policy_or( base.ADMIN_OR_PROJECT_MEMBER, - base.RULE_NET_OWNER), + RULE_NET_OWNER_MEMBER), scope_types=['project'], description='Delete a subnet', operations=ACTION_DELETE, diff --git a/neutron/tests/unit/conf/policies/test_subnet.py b/neutron/tests/unit/conf/policies/test_subnet.py index cee00fc70d7..6c3d8ef9eb2 100644 --- a/neutron/tests/unit/conf/policies/test_subnet.py +++ b/neutron/tests/unit/conf/policies/test_subnet.py @@ -29,19 +29,33 @@ class SubnetAPITestCase(base.PolicyBaseTestCase): self.network = { 'id': uuidutils.generate_uuid(), + 'tenant_id': self.project_id, 'project_id': self.project_id} + self.alt_network = { + 'id': uuidutils.generate_uuid(), + 'tenant_id': self.alt_project_id, + 'project_id': self.alt_project_id} + + networks = { + self.network['id']: self.network, + self.alt_network['id']: self.alt_network} self.target = { 'project_id': self.project_id, + 'tenant_id': self.project_id, 'network_id': self.network['id'], 'ext_parent_network_id': self.network['id']} self.alt_target = { 'project_id': self.alt_project_id, - 'network_id': self.network['id'], - 'ext_parent_network_id': self.network['id']} + 'tenant_id': self.alt_project_id, + 'network_id': self.alt_network['id'], + 'ext_parent_network_id': self.alt_network['id']} + + def get_network(context, id, fields=None): + return networks.get(id) self.plugin_mock = mock.Mock() - self.plugin_mock.get_network.return_value = self.network + self.plugin_mock.get_network.side_effect = get_network mock.patch( 'neutron_lib.plugins.directory.get_plugin', return_value=self.plugin_mock).start()