Skip to content

Commit

Permalink
[S-RBAC] Fix policies for CUD subnets APIs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
slawqo committed Jun 21, 2023
1 parent 5932785 commit 6e35251
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
13 changes: 9 additions & 4 deletions neutron/conf/policies/subnet.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,18 @@
{'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,
Expand Down Expand Up @@ -112,7 +117,7 @@
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,
Expand Down Expand Up @@ -150,7 +155,7 @@
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,
Expand Down
20 changes: 17 additions & 3 deletions neutron/tests/unit/conf/policies/test_subnet.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,33 @@ def setUp(self):

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()
Expand Down

0 comments on commit 6e35251

Please sign in to comment.