Skip to content

Commit 8170fa8

Browse files
committed
Fix plural policy names in tagging controller and floatingip policy
The TaggingController.create() and update() methods enforce policy action names using the plural collection key (e.g. create_networks:tags) instead of the singular member name (e.g. create_network:tags). Since the registered policy rules use the singular form, the unmatched plural names fall through to oslo.policy's default rule, allowing project readers to mutate tags on same-project resources. Fix the delete_floatingips:tags policy rule name (should be singular delete_floatingip:tags) and add a unit test that validates _get_policy_action produces the correct singular form for all supported resources and actions, and that each generated name matches an actually registered policy rule. Conflicts: neutron/extensions/tagging.py Closes-Bug: #2150132 Signed-off-by: Rodolfo Alonso Hernandez <ralonsoh@redhat.com> Change-Id: I783510565e4fc4191b5494eb9a6dc0bdd3ace3fc (cherry picked from commit 7401244)
1 parent 1a22739 commit 8170fa8

4 files changed

Lines changed: 52 additions & 9 deletions

File tree

neutron/conf/policies/floatingip.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@
170170
deprecated_since=versionutils.deprecated.WALLABY)
171171
),
172172
policy.DocumentedRuleDefault(
173-
name='delete_floatingips:tags',
173+
name='delete_floatingip:tags',
174174
check_str=base.ADMIN_OR_PROJECT_MEMBER,
175175
description='Delete the floating IP tags',
176176
operations=ACTION_DELETE_TAGS,

neutron/extensions/tagging.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,10 @@ def index(self, request, **kwargs):
197197
# GET /v2.0/{obj_resource}/{obj_resource_id}/tags
198198
ctx = request.context
199199
rinfo = self._get_resource_info(ctx, kwargs)
200-
policy.enforce(ctx, 'get_{}_{}'.format(rinfo.obj_type, TAGS),
201-
rinfo.obj)
200+
policy.enforce(
201+
ctx,
202+
self._get_policy_action("get", rinfo.obj_type),
203+
rinfo.obj)
202204
return self.plugin.get_tags(ctx, rinfo.obj_type, rinfo.obj['id'])
203205

204206
def show(self, request, id, **kwargs):
@@ -207,8 +209,10 @@ def show(self, request, id, **kwargs):
207209
validate_tag(id)
208210
ctx = request.context
209211
rinfo = self._get_resource_info(ctx, kwargs)
210-
policy.enforce(ctx, 'get_{}:{}'.format(rinfo.obj_type, TAGS),
211-
rinfo.obj)
212+
policy.enforce(
213+
ctx,
214+
self._get_policy_action("get", rinfo.obj_type),
215+
rinfo.obj)
212216
return self.plugin.get_tag(ctx, rinfo.obj_type, rinfo.obj['id'], id)
213217

214218
def create(self, request, body, **kwargs):
@@ -217,8 +221,10 @@ def create(self, request, body, **kwargs):
217221
validate_tags(body)
218222
ctx = request.context
219223
rinfo = self._get_resource_info(ctx, kwargs, tags=body[TAGS])
220-
policy.enforce(ctx, 'create_{}:{}'.format(rinfo.obj_type, TAGS),
221-
rinfo.obj)
224+
policy.enforce(
225+
ctx,
226+
self._get_policy_action("create", rinfo.obj_type),
227+
rinfo.obj)
222228
validate_tags_limit(rinfo.obj_type, body['tags'])
223229
notify_tag_action(ctx, 'create.start', rinfo.obj_type,
224230
rinfo.obj['id'], body['tags'])
@@ -234,8 +240,10 @@ def update(self, request, id, **kwargs):
234240
validate_tag(id)
235241
ctx = request.context
236242
rinfo = self._get_resource_info(ctx, kwargs, tags=[id])
237-
policy.enforce(ctx, 'update_{}:{}'.format(rinfo.obj_type, TAGS),
238-
rinfo.obj)
243+
policy.enforce(
244+
ctx,
245+
self._get_policy_action("update", rinfo.obj_type),
246+
rinfo.obj)
239247
current_tags = self.plugin.get_tags(
240248
ctx, rinfo.obj_type, rinfo.obj['id'])['tags']
241249
new_tags = current_tags + [id]

neutron/tests/unit/extensions/test_tagging.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from neutron_lib.utils import net as net_utils
2525
from oslo_utils import uuidutils
2626

27+
from neutron.conf import policies as conf_policies
2728
from neutron.extensions import tagging
2829
from neutron.objects import network as network_obj
2930
from neutron.objects import network_segment_range as network_segment_range_obj
@@ -87,6 +88,22 @@ def test_all_ovo_cls_have_a_reference(self):
8788
ovo_resources = set(tagging.OVO_CLS.keys())
8889
self.assertEqual(tc_supported_resources, ovo_resources)
8990

91+
def test__get_policy_action_all_resources_and_actions(self):
92+
registered_rules = {rule.name for rule in conf_policies.list_rules()}
93+
actions = ('get', 'create', 'update', 'delete')
94+
for collection, member in self.tc.supported_resources.items():
95+
for action in actions:
96+
expected = f'{action}_{member}:tags'
97+
result = self.tc._get_policy_action(action, collection)
98+
self.assertEqual(
99+
expected, result,
100+
f'_get_policy_action("{action}", "{collection}") '
101+
f'returned "{result}" instead of "{expected}"')
102+
self.assertIn(
103+
result, registered_rules,
104+
f'Policy rule "{result}" is not registered in '
105+
f'neutron.conf.policies')
106+
90107
def _check_resource_info(self, obj, obj_type):
91108
id_key = self.tc.supported_resources[obj_type] + '_id'
92109
res = self.tc._get_resource_info(self.ctx, {id_key: obj['id']})
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
security:
3+
- |
4+
Fixed a security issue where the tagging controller's single-tag write
5+
endpoints (``POST /v2.0/{resource}/{id}/tags`` and
6+
``PUT /v2.0/{resource}/{id}/tags/{tag}``) enforced policy action names
7+
using the plural collection key (e.g. ``create_networks:tags``) instead
8+
of the singular member name (e.g. ``create_network:tags``). Because the
9+
plural names did not match any registered policy rule, they fell through
10+
to oslo.policy's default rule, allowing project readers to create and
11+
update tags on same-project resources.
12+
All tag-write endpoints now consistently use the singular policy action
13+
names that match the registered rules.
14+
fixes:
15+
- |
16+
Fixed the ``delete_floatingips:tags`` policy rule name to use the correct
17+
singular form ``delete_floatingip:tags``, consistent with all other
18+
tag-related policy rules.

0 commit comments

Comments
 (0)