[release-4.12] OCPBUGS-97988: CVE-2026-44918: Prevent rehoming resources to nodes with different owner

ironic/api/controllers/v1/port.py

@@ -636,12 +636,9 @@ def patch(self, port_uuid, patch):
             'baremetal:port:update', port_uuid)
 
         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
@@ -652,7 +649,10 @@ def patch(self, port_uuid, 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'):
@@ -667,16 +667,6 @@ def patch(self, port_uuid, 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)
 

ironic/api/controllers/v1/portgroup.py

@@ -470,29 +470,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)
 

ironic/api/controllers/v1/utils.py

@@ -25,6 +25,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
@@ -46,6 +47,8 @@
 
 CONF = cfg.CONF
 
+LOG = log.getLogger(__name__)
+
 
 _JSONPATCH_EXCEPTIONS = (jsonpatch.JsonPatchConflict,
                          jsonpatch.JsonPatchException,
@@ -294,28 +297,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.
@@ -1470,6 +1451,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 check_owner_policy(object_type, policy_name, owner, lessee=None,
                        conceal_node=False):
     """Check if the policy authorizes this request on an object.

ironic/api/controllers/v1/volume_connector.py

@@ -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)

ironic/api/controllers/v1/volume_target.py

@@ -345,9 +345,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()
@@ -361,29 +360,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)

ironic/tests/unit/api/controllers/v1/test_node.py

@@ -2695,6 +2695,12 @@ def test_update_ok(self, mock_notify):
                          timeutils.parse_isotime(response.json['updated_at']))
         self.mock_update_node.assert_called_once_with(
             mock.ANY, mock.ANY, mock.ANY, 'test-topic', None)
+        # NOTE(TheJulia) As we save a hydrated database object back, we need
+        # to double check what gets saved back out to the database to validate
+        # handling is proper.
+        updated_node = self.mock_update_node.call_args.args[2]
+        self.assertEqual(updated_node.instance_uuid,
+                         'aaaaaaaa-1111-bbbb-2222-cccccccccccc')
         mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update',
                                       obj_fields.NotificationLevel.INFO,
                                       obj_fields.NotificationStatus.START,

ironic/tests/unit/api/controllers/v1/test_utils.py

@@ -931,24 +931,6 @@ def test_replace_node_uuid_with_id_not_found(self, mock_gbu, mock_pr):
                               utils.replace_node_uuid_with_id, to_dict)
         self.assertEqual(400, e.code)
 
-    @mock.patch.object(objects.Node, 'get_by_id', autospec=True)
-    def test_replace_node_id_with_uuid(self, mock_gbi, mock_pr):
-        node = obj_utils.get_test_node(self.context, uuid=self.valid_uuid)
-        mock_gbi.return_value = node
-        to_dict = {'node_id': 1}
-
-        self.assertEqual(node, utils.replace_node_id_with_uuid(to_dict))
-        self.assertEqual({'node_uuid': self.valid_uuid}, to_dict)
-
-    @mock.patch.object(objects.Node, 'get_by_id', autospec=True)
-    def test_replace_node_id_with_uuid_not_found(self, mock_gbi, mock_pr):
-        to_dict = {'node_id': 1}
-        mock_gbi.side_effect = exception.NodeNotFound(node=1)
-
-        e = self.assertRaises(exception.NodeNotFound,
-                              utils.replace_node_id_with_uuid, to_dict)
-        self.assertEqual(400, e.code)
-
 
 class TestVendorPassthru(base.TestCase):
 

releasenotes/notes/2150450-owners-b4019d5fa63a8bc1.yaml

@@ -0,0 +1,14 @@
+---
+security:
+  - |
+    Prevents changing the ``node_uuid`` of ports, port groups, volume targets,
+    and volume connectors to point to a node with a different owner from the
+    initial node. In some cases this is not normally permitted due to the
+    database model, but additional access checking was added across these
+    similar resources for consistency in the event the Ironic project fixes
+    `12150252 <https://bugs.launchpad.net/ironic/+bug/2150252>`_.
+issues:
+  - |
+    System operators should note that changing the owner field on a node does
+    not affect its child or parent nodes, potentially resulting in these nodes
+    being in different projects.

tox.ini

@@ -136,7 +136,7 @@ filename = *.py,app.wsgi
 exclude=.*,dist,doc,*lib/python*,*egg,build
 import-order-style = pep8
 application-import-names = ironic
-max-complexity=19
+max-complexity=21
 # [H106] Don't put vim configuration in source files.
 # [H203] Use assertIs(Not)None to check for None.
 # [H204] Use assert(Not)Equal to check for equality.