Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 11 additions & 19 deletions ironic/api/controllers/v1/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -2981,11 +2981,9 @@ def post(self, node):
chassis = _replace_chassis_uuid_with_id(node)
chassis_uuid = chassis and chassis.uuid or None

if ('parent_node' in node
and not uuidutils.is_uuid_like(node['parent_node'])):

parent_node = api_utils.check_node_policy_and_retrieve(
'baremetal:node:get', node['parent_node'])
if node.get('parent_node'):
parent_node = api_utils.authorize_node_link(
node, node['parent_node'], 'parent_node')
node['parent_node'] = parent_node.uuid

new_node = objects.Node(context, **node)
Expand Down Expand Up @@ -3051,20 +3049,6 @@ def _validate_patch(self, patch, reset_interfaces):

for network_data in network_data_fields:
validate_network_data(network_data)
parent_node = api_utils.get_patch_values(patch, '/parent_node')
if parent_node:
# At this point, if there *is* a parent_node, there is a value
# in the patch value list.
try:
# Verify we can see the parent node
req_parent_node = api_utils.check_node_policy_and_retrieve(
'baremetal:node:get', parent_node[0])
# If we can't see the node, an exception gets raised.
except Exception:
msg = _("Unable to apply the requested parent_node. "
"Requested value was invalid.")
raise exception.Invalid(msg)
corrected_values['parent_node'] = req_parent_node.uuid
return corrected_values

def _authorize_patch_and_get_node(self, node_ident, patch):
Expand Down Expand Up @@ -3152,6 +3136,14 @@ def patch(self, node_ident, reset_interfaces=None, patch=None):

context = api.request.context
rpc_node = self._authorize_patch_and_get_node(node_ident, patch)
if parent_node := api_utils.authorize_node_link_patch(
rpc_node, patch, 'parent_node'):
# parent_node would be set to the UUID to verify that the request
# is authorized against the returned parent_node.uuid, and is then
# set as the authorized to node. IFF there is a mismatch, then
# we need to force override the parent node to the result.
if parent_node.uuid != rpc_node.uuid:
corrected_values['parent_node'] = parent_node.uuid

remove_inst_uuid_patch = [{'op': 'remove', 'path': '/instance_uuid'}]
if rpc_node.maintenance and patch == remove_inst_uuid_patch:
Expand Down
18 changes: 4 additions & 14 deletions ironic/api/controllers/v1/port.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,12 +689,9 @@ def patch(self, port_ident, patch):
'baremetal:port:update', port_ident)

port_dict = rpc_port.as_dict()
# NOTE(lucasagomes):
# 1) Remove node_id because it's an internal value and
# NOTE(lucasagomes): Remove node_id because it's an internal value and
# not present in the API object
# 2) Add node_uuid
port_dict.pop('node_id', None)
port_dict['node_uuid'] = rpc_node.uuid
# NOTE(vsaienko):
# 1) Remove portgroup_id because it's an internal value and
# not present in the API object
Expand All @@ -705,7 +702,10 @@ def patch(self, port_ident, patch):
context, port_dict.pop('portgroup_id'))
port_dict['portgroup_uuid'] = portgroup and portgroup.uuid or None

rpc_node = api_utils.authorize_node_link_patch(
rpc_node, patch, 'node_uuid')
port_dict = api_utils.apply_jsonpatch(port_dict, patch)
port_dict['node_uuid'] = rpc_node.uuid

try:
if api_utils.is_path_updated(patch, '/portgroup_uuid'):
Expand All @@ -720,16 +720,6 @@ def patch(self, port_ident, patch):
e.code = http_client.BAD_REQUEST # BadRequest
raise

try:
if port_dict['node_uuid'] != rpc_node.uuid:
rpc_node = objects.Node.get(
api.request.context, port_dict['node_uuid'])
except exception.NodeNotFound as e:
# Change error code because 404 (NotFound) is inappropriate
# response for a PATCH request to change a Port
e.code = http_client.BAD_REQUEST # BadRequest
raise

api_utils.patched_validate_with_schema(
port_dict, PORT_PATCH_SCHEMA, PORT_PATCH_VALIDATOR)

Expand Down
21 changes: 6 additions & 15 deletions ironic/api/controllers/v1/portgroup.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,29 +472,20 @@ def patch(self, portgroup_ident, patch):

portgroup_dict = rpc_portgroup.as_dict()

# NOTE:
# 1) Remove node_id because it's an internal value and
# NOTE: Remove node_id because it's an internal value and
# not present in the API object
# 2) Add node_uuid
portgroup_dict.pop('node_id')
portgroup_dict['node_uuid'] = rpc_node.uuid

rpc_node = api_utils.authorize_node_link_patch(
rpc_node, patch, 'node_uuid')

portgroup_dict = api_utils.apply_jsonpatch(portgroup_dict, patch)
portgroup_dict['node_uuid'] = rpc_node.uuid

if 'mode' not in portgroup_dict:
msg = _("'mode' is a mandatory attribute and can not be removed")
raise exception.ClientSideError(msg)

try:
if portgroup_dict['node_uuid'] != rpc_node.uuid:
rpc_node = objects.Node.get(api.request.context,
portgroup_dict['node_uuid'])

except exception.NodeNotFound as e:
# Change error code because 404 (NotFound) is inappropriate
# response for a POST request to patch a Portgroup
e.code = http_client.BAD_REQUEST # BadRequest
raise

api_utils.patched_validate_with_schema(
portgroup_dict, PORTGROUP_PATCH_SCHEMA, PORTGROUP_PATCH_VALIDATOR)

Expand Down
76 changes: 54 additions & 22 deletions ironic/api/controllers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from jsonschema import exceptions as json_schema_exc
import os_traits
from oslo_config import cfg
from oslo_log import log
from oslo_policy import policy as oslo_policy
from oslo_utils import uuidutils
from pecan import rest
Expand All @@ -47,6 +48,8 @@

CONF = cfg.CONF

LOG = log.getLogger(__name__)


_JSONPATCH_EXCEPTIONS = (jsonpatch.JsonPatchConflict,
jsonpatch.JsonPatchException,
Expand Down Expand Up @@ -325,28 +328,6 @@ def replace_node_uuid_with_id(to_dict):
return node


def replace_node_id_with_uuid(to_dict):
"""Replace ``node_id`` dict value with ``node_uuid``

``node_uuid`` is found by fetching the node by id lookup.

:param to_dict: Dict to set ``node_uuid`` value on
:returns: The node object from the lookup
:raises: NodeNotFound with status_code set to 400 BAD_REQUEST
when node is not found.
"""
try:
node = objects.Node.get_by_id(api.request.context,
to_dict.pop('node_id'))
to_dict['node_uuid'] = node.uuid
except exception.NodeNotFound as e:
# Change error code because 404 (NotFound) is inappropriate
# response for requests acting on non-nodes
e.code = http_client.BAD_REQUEST # BadRequest
raise
return node


def patch_update_changed_fields(from_dict, rpc_object, fields,
schema, id_map=None):
"""Update rpc object based on changed fields in a dict.
Expand Down Expand Up @@ -1574,6 +1555,57 @@ def check_policy_true(policy_name):
return policy.check_policy(policy_name, cdict, api.request.context)


def authorize_node_link(orig_node, new_uuid, field_name):
"""Authorize creating or changing a link to the node on a resource.

:param orig_node: Node that currently owns the resource.
:param new_uuid: UUID of the new node.
:param field_name: Human-readable field name for logging and error message.
:raises: Invalid on access error
:returns: The new Node object
"""
msg = _("Unable to apply the requested %s '%s'. "
"Requested value was invalid.")

try:
new_node = check_node_policy_and_retrieve(
'baremetal:node:get', new_uuid)
except Exception as exc:
LOG.debug("Rejecting %s %s: %s", field_name, new_uuid, exc)
# Important: do not disclose if the node exists
raise exception.Invalid(msg % (field_name, new_uuid))

if isinstance(orig_node, objects.Node):
orig_node = orig_node.as_dict() # adjust for different callers

if orig_node.get('owner') != new_node.owner:
LOG.warning("Project mismatch on setting or changing %(field)s: "
"current owner '%(orig)s', new %(field)s owner '%(new)s'",
{'orig': orig_node.get('owner'), 'new': new_node.owner,
'field': field_name})
# Important: same error message as when the node does not exist
raise exception.Invalid(msg % (field_name, new_uuid))

return new_node


def authorize_node_link_patch(orig_node, patch, field_name):
"""Authorize changing a link to the node on a resource.

:param orig_node: Node that currently owns the resource.
:param patch: JSON patch being applied.
:param field_name: Field names that contains the link.
:raises: Invalid on access error
:returns: The new Node object or the old one if not changed
"""
new_node_id = get_patch_values(patch, f'/{field_name}')
if new_node_id:
return authorize_node_link(
orig_node, new_node_id[0], field_name)

return orig_node


def duplicate_steps(name, value):
"""Argument validator to check template for duplicate steps"""
# TODO(mgoddard): Determine the consequences of allowing duplicate
Expand Down
21 changes: 7 additions & 14 deletions ironic/api/controllers/v1/volume_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,23 +332,16 @@ def patch(self, connector_uuid, patch):
raise exception.InvalidUUID(message=message)

connector_dict = rpc_connector.as_dict()
# NOTE(smoriya):
# 1) Remove node_id because it's an internal value and
# NOTE(smoriya): Remove node_id because it's an internal value and
# not present in the API object
# 2) Add node_uuid
rpc_node = api_utils.replace_node_id_with_uuid(connector_dict)
connector_dict.pop('node_id', None)
# NOTE(dtantsur): Patch won't apply if the field does not exist.
connector_dict['node_uuid'] = None

rpc_node = api_utils.authorize_node_link_patch(
rpc_node, patch, 'node_uuid')
connector_dict = api_utils.apply_jsonpatch(connector_dict, patch)

try:
if connector_dict['node_uuid'] != rpc_node.uuid:
rpc_node = objects.Node.get(
api.request.context, connector_dict['node_uuid'])
except exception.NodeNotFound as e:
# Change error code because 404 (NotFound) is inappropriate
# response for a PATCH request to change a Port
e.code = http_client.BAD_REQUEST # BadRequest
raise
connector_dict['node_uuid'] = rpc_node.uuid

api_utils.patched_validate_with_schema(
connector_dict, CONNECTOR_SCHEMA, CONNECTOR_VALIDATOR)
Expand Down
31 changes: 9 additions & 22 deletions ironic/api/controllers/v1/volume_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,8 @@ def patch(self, target_uuid, patch):
"""
context = api.request.context

api_utils.check_volume_policy_and_retrieve('baremetal:volume:update',
target_uuid,
target=True)
rpc_target, rpc_node = api_utils.check_volume_policy_and_retrieve(
'baremetal:volume:update', target_uuid, target=True)

if self.parent_node_ident:
raise exception.OperationNotPermitted()
Expand All @@ -369,29 +368,17 @@ def patch(self, target_uuid, patch):
"%(uuid)s.") % {'uuid': str(value)}
raise exception.InvalidUUID(message=message)

rpc_target = objects.VolumeTarget.get_by_uuid(context, target_uuid)
target_dict = rpc_target.as_dict()
# NOTE(smoriya):
# 1) Remove node_id because it's an internal value and
# NOTE(smoriya): Remove node_id because it's an internal value and
# not present in the API object
# 2) Add node_uuid
rpc_node = api_utils.replace_node_id_with_uuid(target_dict)
target_dict.pop('node_id', None)
# NOTE(dtantsur): Patch won't apply if the field does not exist.
target_dict['node_uuid'] = None

rpc_node = api_utils.authorize_node_link_patch(
rpc_node, patch, 'node_uuid')
target_dict = api_utils.apply_jsonpatch(target_dict, patch)

try:
if target_dict['node_uuid'] != rpc_node.uuid:

# TODO(TheJulia): I guess the intention is to
# permit the mapping to be changed
# should we even allow this at all?
rpc_node = objects.Node.get(
api.request.context, target_dict['node_uuid'])
except exception.NodeNotFound as e:
# Change error code because 404 (NotFound) is inappropriate
# response for a PATCH request to change a volume target
e.code = http_client.BAD_REQUEST # BadRequest
raise
target_dict['node_uuid'] = rpc_node.uuid

api_utils.patched_validate_with_schema(
target_dict, TARGET_SCHEMA, TARGET_VALIDATOR)
Expand Down
37 changes: 37 additions & 0 deletions ironic/cmd/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
from oslo_upgradecheck import common_checks
from oslo_upgradecheck import upgradecheck
import sqlalchemy
from sqlalchemy import orm

from ironic.cmd import dbsync
from ironic.common import driver_factory
from ironic.common.i18n import _
from ironic.common import policy # noqa importing to load policy config.
import ironic.conf
from ironic.db import api as db_api
from ironic.db.sqlalchemy import models

CONF = ironic.conf.CONF

Expand Down Expand Up @@ -181,6 +183,38 @@ def _check_hardware_types_interfaces(self):
else:
return upgradecheck.Result(upgradecheck.Code.SUCCESS)

def _check_parent_child_owners(self):
engine = enginefacade.reader.get_engine()
parent_node = orm.aliased(models.Node)
broken = []
with engine.connect() as conn, conn.begin():
# NOTE(dtantsur): since NULL handling in SQL is insane, we need to
# handle 3 possible cases separately.
conditions = (
(parent_node.owner != models.Node.owner)
| (models.Node.owner.is_not(None)
& parent_node.owner.is_(None))
| (models.Node.owner.is_(None)
& parent_node.owner.is_not(None))
)
query = (sqlalchemy.select(models.Node.uuid,
models.Node.owner,
parent_node.uuid,
parent_node.owner)
.join(parent_node,
models.Node.parent_node == parent_node.uuid)
.where(conditions))
res = conn.execute(query)
for child_uuid, child_owner, parent_uuid, parent_owner in res:
broken.append(f"- {child_uuid}: owner {child_owner}, "
f"parent {parent_uuid} owner {parent_owner}")
if broken:
msg = ("Some nodes have an owner mismatch with parents:\n"
+ "\n".join(broken))
return upgradecheck.Result(upgradecheck.Code.WARNING, details=msg)
else:
return upgradecheck.Result(upgradecheck.Code.SUCCESS)

# A tuple of check tuples of (<name of check>, <check function>).
# The name of the check will be used in the output of this command.
# The check function takes no arguments and returns an
Expand All @@ -199,6 +233,9 @@ def _check_hardware_types_interfaces(self):
(common_checks.check_policy_json, {'conf': CONF})),
(_('Hardware Types and Interfaces Check'),
_check_hardware_types_interfaces),
# Cases of CVE-2026-44918
(_('Mismatch between Child and Parent Owners'),
_check_parent_child_owners),
)


Expand Down
4 changes: 2 additions & 2 deletions ironic/common/neutron.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def add_ports_to_network(task, network_uuid, security_groups=None):
:raises: NetworkError
:returns: a dictionary in the form {port.uuid: neutron_port['id']}
"""
client = get_client(context=task.context)
client = get_client(context=task.context, auth_from_config=True)
node = task.node
add_all_ports = CONF.neutron.add_all_ports

Expand Down Expand Up @@ -444,7 +444,7 @@ def remove_neutron_ports(task, params):
:param params: Dict of params to filter ports.
:raises: NetworkError
"""
client = get_client(context=task.context)
client = get_client(context=task.context, auth_from_config=True)
node_uuid = task.node.uuid

try:
Expand Down
Loading