Skip to content

Commit 9408008

Browse files
ralonsohAlex-Welsh
authored andcommitted
Fix the tagging policy engine
The service tagging policy engine should consider the parent resource or the upper parent resource project ID when checking the policies against the caller project ID. Before this patch, as introduced in [1], the target was incorrectly populated with the caller project ID instead of using the resource ID. [1]https://review.opendev.org/c/openstack/neutron/+/896509/13/neutron/extensions/tagging.py OSSA-2024-005 CVE-2024-53916 Closes-Bug: #2088986 Change-Id: Id7d0c8e7ba37993b1084519d05e7e2eac095b81b (cherry picked from commit fb75d3c)
1 parent 9270119 commit 9408008

File tree

2 files changed

+348
-42
lines changed

2 files changed

+348
-42
lines changed

neutron/extensions/tagging.py

Lines changed: 169 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@
1212
# under the License.
1313

1414
import abc
15+
import collections
1516
import copy
17+
import functools
18+
import itertools
1619

1720
from neutron_lib.api.definitions import port
1821
from neutron_lib.api import extensions as api_extensions
@@ -28,6 +31,16 @@
2831
from neutron._i18n import _
2932
from neutron.api import extensions
3033
from neutron.api.v2 import resource as api_resource
34+
from neutron.objects import network as network_obj
35+
from neutron.objects import network_segment_range as network_segment_range_obj
36+
from neutron.objects import ports as ports_obj
37+
from neutron.objects.qos import policy as policy_obj
38+
from neutron.objects import router as router_obj
39+
from neutron.objects import securitygroup as securitygroup_obj
40+
from neutron.objects import subnet as subnet_obj
41+
from neutron.objects import subnetpool as subnetpool_obj
42+
from neutron.objects import trunk as trunk_obj
43+
from neutron import policy
3144

3245

3346
TAG = 'tag'
@@ -55,6 +68,34 @@
5568
'validate': {'type:list_of_unique_strings': MAX_TAG_LEN},
5669
'default': [], 'is_visible': True, 'is_filter': True
5770
}
71+
PARENTS = {
72+
'floatingips': router_obj.FloatingIP,
73+
'network_segment_ranges': network_segment_range_obj.NetworkSegmentRange,
74+
'networks': network_obj.Network,
75+
'policies': policy_obj.QosPolicy,
76+
'ports': ports_obj.Port,
77+
'routers': router_obj.Router,
78+
'security_groups': securitygroup_obj.SecurityGroup,
79+
'subnets': ('networks', subnet_obj.Subnet),
80+
'subnetpools': subnetpool_obj.SubnetPool,
81+
'trunks': trunk_obj.Trunk,
82+
}
83+
ResourceInfo = collections.namedtuple(
84+
'ResourceInfo', ['project_id',
85+
'parent_type',
86+
'parent_id',
87+
'upper_parent_type',
88+
'upper_parent_id',
89+
])
90+
EMPTY_RESOURCE_INFO = ResourceInfo(None, None, None, None, None)
91+
92+
93+
def _policy_init(f):
94+
@functools.wraps(f)
95+
def func(self, *args, **kwargs):
96+
policy.init()
97+
return f(self, *args, **kwargs)
98+
return func
5899

59100

60101
class TagResourceNotFound(exceptions.NotFound):
@@ -95,75 +136,161 @@ def __init__(self):
95136
self.plugin = directory.get_plugin(TAG_PLUGIN_TYPE)
96137
self.supported_resources = TAG_SUPPORTED_RESOURCES
97138

98-
def _get_parent_resource_and_id(self, kwargs):
99-
for key in kwargs:
100-
for resource in self.supported_resources:
101-
if key == self.supported_resources[resource] + '_id':
102-
return resource, kwargs[key]
103-
return None, None
139+
def _get_target(self, res_info):
140+
target = {'id': res_info.parent_id,
141+
'tenant_id': res_info.project_id,
142+
'project_id': res_info.project_id}
143+
if res_info.upper_parent_type:
144+
res_id = (self.supported_resources[res_info.upper_parent_type] +
145+
'_id')
146+
target[res_id] = res_info.upper_parent_id
147+
return target
148+
149+
def _get_resource_info(self, context, kwargs):
150+
"""Return the tag parent resource information
151+
152+
Some parent resources, like the subnets, depend on other upper parent
153+
resources (networks). In that case, it is needed to provide the upper
154+
parent resource information.
155+
156+
:param kwargs: dictionary with the parent resource ID, along with other
157+
information not needed. It is formated as
158+
{"resource_id": "id", ...}
159+
:return: ``ResourceInfo`` named tuple with the parent and upper parent
160+
information and the project ID (of the parent or upper
161+
parent).
162+
"""
163+
for key, parent_type in itertools.product(
164+
kwargs.keys(), self.supported_resources.keys()):
165+
if key != self.supported_resources[parent_type] + '_id':
166+
continue
167+
168+
parent_id = kwargs[key]
169+
parent_obj = PARENTS[parent_type]
170+
if isinstance(parent_obj, tuple):
171+
upper_parent_type = parent_obj[0]
172+
parent_obj = parent_obj[1]
173+
res_id = (self.supported_resources[upper_parent_type] +
174+
'_id')
175+
upper_parent_id = parent_obj.get_values(
176+
context.elevated(), res_id, id=parent_id)[0]
177+
else:
178+
upper_parent_type = upper_parent_id = None
179+
180+
try:
181+
project_id = parent_obj.get_values(
182+
context.elevated(), 'project_id', id=parent_id)[0]
183+
except IndexError:
184+
return EMPTY_RESOURCE_INFO
185+
186+
return ResourceInfo(project_id, parent_type, parent_id,
187+
upper_parent_type, upper_parent_id)
188+
189+
# This should never be returned.
190+
return EMPTY_RESOURCE_INFO
104191

105192
def index(self, request, **kwargs):
106-
# GET /v2.0/networks/{network_id}/tags
107-
parent, parent_id = self._get_parent_resource_and_id(kwargs)
108-
return self.plugin.get_tags(request.context, parent, parent_id)
193+
# GET /v2.0/{parent_resource}/{parent_resource_id}/tags
194+
ctx = request.context
195+
rinfo = self._get_resource_info(ctx, kwargs)
196+
target = self._get_target(rinfo)
197+
policy.enforce(ctx, 'get_{}_{}'.format(rinfo.parent_type, TAGS),
198+
target)
199+
return self.plugin.get_tags(ctx, rinfo.parent_type, rinfo.parent_id)
109200

110201
def show(self, request, id, **kwargs):
111202
# GET /v2.0/networks/{network_id}/tags/{tag}
112203
# id == tag
113204
validate_tag(id)
114-
parent, parent_id = self._get_parent_resource_and_id(kwargs)
115-
return self.plugin.get_tag(request.context, parent, parent_id, id)
116-
117-
def create(self, request, **kwargs):
118-
# not supported
119-
# POST /v2.0/networks/{network_id}/tags
120-
raise webob.exc.HTTPNotFound("not supported")
205+
ctx = request.context
206+
rinfo = self._get_resource_info(ctx, kwargs)
207+
target = self._get_target(rinfo)
208+
policy.enforce(ctx, 'get_{}_{}'.format(rinfo.parent_type, TAGS),
209+
target)
210+
return self.plugin.get_tag(ctx, rinfo.parent_type, rinfo.parent_id, id)
211+
212+
@_policy_init
213+
def create(self, request, body, **kwargs):
214+
# POST /v2.0/{parent_resource}/{parent_resource_id}/tags
215+
# body: {"tags": ["aaa", "bbb"]}
216+
validate_tags(body)
217+
ctx = request.context
218+
rinfo = self._get_resource_info(ctx, kwargs)
219+
target = self._get_target(rinfo)
220+
policy.enforce(ctx, 'create_{}_{}'.format(rinfo.parent_type, TAGS),
221+
target)
222+
notify_tag_action(ctx, 'create.start', rinfo.parent_type,
223+
rinfo.parent_id, body['tags'])
224+
result = self.plugin.create_tags(ctx, rinfo.parent_type,
225+
rinfo.parent_id, body)
226+
notify_tag_action(ctx, 'create.end', rinfo.parent_type,
227+
rinfo.parent_id, body['tags'])
228+
return result
121229

122230
def update(self, request, id, **kwargs):
123231
# PUT /v2.0/networks/{network_id}/tags/{tag}
124232
# id == tag
125233
validate_tag(id)
126-
parent, parent_id = self._get_parent_resource_and_id(kwargs)
127-
notify_tag_action(request.context, 'create.start',
128-
parent, parent_id, [id])
129-
result = self.plugin.update_tag(request.context, parent, parent_id, id)
130-
notify_tag_action(request.context, 'create.end',
131-
parent, parent_id, [id])
234+
ctx = request.context
235+
rinfo = self._get_resource_info(ctx, kwargs)
236+
target = self._get_target(rinfo)
237+
policy.enforce(ctx, 'update_{}_{}'.format(rinfo.parent_type, TAGS),
238+
target)
239+
notify_tag_action(ctx, 'create.start', rinfo.parent_type,
240+
rinfo.parent_id, [id])
241+
result = self.plugin.update_tag(ctx, rinfo.parent_type,
242+
rinfo.parent_id, id)
243+
notify_tag_action(ctx, 'create.end', rinfo.parent_type,
244+
rinfo.parent_id, [id])
132245
return result
133246

134247
def update_all(self, request, body, **kwargs):
135248
# PUT /v2.0/networks/{network_id}/tags
136249
# body: {"tags": ["aaa", "bbb"]}
137250
validate_tags(body)
138-
parent, parent_id = self._get_parent_resource_and_id(kwargs)
139-
notify_tag_action(request.context, 'update.start',
140-
parent, parent_id, body['tags'])
141-
result = self.plugin.update_tags(request.context, parent,
142-
parent_id, body)
143-
notify_tag_action(request.context, 'update.end',
144-
parent, parent_id, body['tags'])
251+
ctx = request.context
252+
rinfo = self._get_resource_info(ctx, kwargs)
253+
target = self._get_target(rinfo)
254+
policy.enforce(ctx, 'update_{}_{}'.format(rinfo.parent_type, TAGS),
255+
target)
256+
notify_tag_action(ctx, 'update.start', rinfo.parent_type,
257+
rinfo.parent_id, body['tags'])
258+
result = self.plugin.update_tags(ctx, rinfo.parent_type,
259+
rinfo.parent_id, body)
260+
notify_tag_action(ctx, 'update.end', rinfo.parent_type,
261+
rinfo.parent_id, body['tags'])
145262
return result
146263

147264
def delete(self, request, id, **kwargs):
148265
# DELETE /v2.0/networks/{network_id}/tags/{tag}
149266
# id == tag
150267
validate_tag(id)
151-
parent, parent_id = self._get_parent_resource_and_id(kwargs)
152-
notify_tag_action(request.context, 'delete.start',
153-
parent, parent_id, [id])
154-
result = self.plugin.delete_tag(request.context, parent, parent_id, id)
155-
notify_tag_action(request.context, 'delete.end',
156-
parent, parent_id, [id])
268+
ctx = request.context
269+
rinfo = self._get_resource_info(ctx, kwargs)
270+
target = self._get_target(rinfo)
271+
policy.enforce(ctx, 'delete_{}_{}'.format(rinfo.parent_type, TAGS),
272+
target)
273+
notify_tag_action(ctx, 'delete.start', rinfo.parent_type,
274+
rinfo.parent_id, [id])
275+
result = self.plugin.delete_tag(ctx, rinfo.parent_type,
276+
rinfo.parent_id, id)
277+
notify_tag_action(ctx, 'delete.end', rinfo.parent_type,
278+
rinfo.parent_id, [id])
157279
return result
158280

159281
def delete_all(self, request, **kwargs):
160-
# DELETE /v2.0/networks/{network_id}/tags
161-
parent, parent_id = self._get_parent_resource_and_id(kwargs)
162-
notify_tag_action(request.context, 'delete_all.start',
163-
parent, parent_id)
164-
result = self.plugin.delete_tags(request.context, parent, parent_id)
165-
notify_tag_action(request.context, 'delete_all.end',
166-
parent, parent_id)
282+
# DELETE /v2.0/{parent_resource}/{parent_resource_id}/tags
283+
ctx = request.context
284+
rinfo = self._get_resource_info(ctx, kwargs)
285+
target = self._get_target(rinfo)
286+
policy.enforce(ctx, 'delete_{}_{}'.format(rinfo.parent_type, TAGS),
287+
target)
288+
notify_tag_action(ctx, 'delete_all.start', rinfo.parent_type,
289+
rinfo.parent_id)
290+
result = self.plugin.delete_tags(ctx, rinfo.parent_type,
291+
rinfo.parent_id)
292+
notify_tag_action(ctx, 'delete_all.end', rinfo.parent_type,
293+
rinfo.parent_id)
167294
return result
168295

169296

0 commit comments

Comments
 (0)