From 4d5a3da4f1b3fea3de3d2b6d973f5da09336ac58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Wed, 17 Jun 2026 18:56:00 +0200 Subject: [PATCH 1/4] Fix bootfile_name_by_arch to consider boot mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pxe_bootfile_name_by_arch and ipxe_bootfile_name_by_arch options skipped the boot mode check when an architecture matched, making it impossible to specify different boot files for the same architecture in different boot modes. Support composite keys in the format arch-boot_mode (e.g. x86_64-uefi) with fallback to arch-only keys for backward compatibility. Closes-Bug: #2157027 Assisted-By: Claude - (Opus 4.6) Change-Id: I839d4b329a13aaa86873cbfcb6ec1cac0a2b4432 Signed-off-by: Harald Jensås (cherry picked from commit ab6a22bdff0bcfc3270a5d2d7f305e12e604a449) --- ironic/conf/pxe.py | 16 +++++- ironic/drivers/modules/deploy_utils.py | 30 +++++++--- .../unit/drivers/modules/test_deploy_utils.py | 56 +++++++++++++++++++ ...t-mode-composite-key-2b0a10764f3b71a9.yaml | 13 +++++ 4 files changed, 105 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/bootfile-by-arch-boot-mode-composite-key-2b0a10764f3b71a9.yaml diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py index e84438a47e..8dbe2c2b12 100644 --- a/ironic/conf/pxe.py +++ b/ironic/conf/pxe.py @@ -135,11 +135,23 @@ cfg.DictOpt('pxe_bootfile_name_by_arch', default={}, help=_('Bootfile DHCP parameter per node architecture. ' - 'For example: aarch64:grubaa64.efi')), + 'Keys can be a cpu_arch value (e.g. aarch64) or a ' + 'composite key of arch-boot_mode (e.g. ' + 'x86_64-uefi, x86_64-bios). The composite key is ' + 'tried first, then the arch-only key as a ' + 'fallback. ' + 'For example: aarch64:grubaa64.efi or ' + 'x86_64-bios:pxelinux.0,x86_64-uefi:bootx64.efi')), cfg.DictOpt('ipxe_bootfile_name_by_arch', default={}, help=_('Bootfile DHCP parameter per node architecture. ' - 'For example: aarch64:ipxe_aa64.efi')), + 'Keys can be a cpu_arch value (e.g. aarch64) or a ' + 'composite key of arch-boot_mode (e.g. ' + 'x86_64-uefi, x86_64-bios). The composite key is ' + 'tried first, then the arch-only key as a ' + 'fallback. ' + 'For example: aarch64:ipxe_aa64.efi or ' + 'x86_64-bios:undionly.kpxe,x86_64-uefi:snponly.efi')), cfg.StrOpt('ipxe_boot_script', default=os.path.join( '$pybasedir', 'drivers/modules/boot.ipxe'), diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 2f06f4a520..612e0ec626 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -400,16 +400,23 @@ def get_pxe_boot_file(node): """Return the PXE boot file name requested for deploy. This method returns PXE boot file name to be used for deploy. - Architecture specific boot file is searched first. BIOS/UEFI - boot file is used if no valid architecture specific file found. + Architecture and boot mode specific boot file is searched first + using a composite key of ``arch-boot_mode`` (e.g. ``x86_64-uefi``). + If not found, architecture-only key is tried for backward + compatibility. BIOS/UEFI boot file is used if no valid + architecture specific file found. :param node: A single Node. :returns: The PXE boot file name. """ cpu_arch = node.properties.get('cpu_arch') - boot_file = CONF.pxe.pxe_bootfile_name_by_arch.get(cpu_arch) + boot_mode = boot_mode_utils.get_boot_mode(node) + boot_file = CONF.pxe.pxe_bootfile_name_by_arch.get( + '%s-%s' % (cpu_arch, boot_mode)) if boot_file is None: - if boot_mode_utils.get_boot_mode(node) == 'uefi': + boot_file = CONF.pxe.pxe_bootfile_name_by_arch.get(cpu_arch) + if boot_file is None: + if boot_mode == 'uefi': boot_file = CONF.pxe.uefi_pxe_bootfile_name else: boot_file = CONF.pxe.pxe_bootfile_name @@ -421,8 +428,11 @@ def get_ipxe_boot_file(node): """Return the iPXE boot file name requested for deploy. This method returns iPXE boot file name to be used for deploy. - Architecture specific boot file is searched first. BIOS/UEFI - boot file is used if no valid architecture specific file found. + Architecture and boot mode specific boot file is searched first + using a composite key of ``arch-boot_mode`` (e.g. ``x86_64-bios``). + If not found, architecture-only key is tried for backward + compatibility. BIOS/UEFI boot file is used if no valid + architecture specific file found. If no valid value is found, the default reverts to the ``get_pxe_boot_file`` method and thus the @@ -433,9 +443,13 @@ def get_ipxe_boot_file(node): :returns: The iPXE boot file name. """ cpu_arch = node.properties.get('cpu_arch') - boot_file = CONF.pxe.ipxe_bootfile_name_by_arch.get(cpu_arch) + boot_mode = boot_mode_utils.get_boot_mode(node) + boot_file = CONF.pxe.ipxe_bootfile_name_by_arch.get( + '%s-%s' % (cpu_arch, boot_mode)) + if boot_file is None: + boot_file = CONF.pxe.ipxe_bootfile_name_by_arch.get(cpu_arch) if boot_file is None: - if boot_mode_utils.get_boot_mode(node) == 'uefi': + if boot_mode == 'uefi': boot_file = CONF.pxe.uefi_ipxe_bootfile_name else: boot_file = CONF.pxe.ipxe_bootfile_name diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 3b0a8d59ef..e7acf6452d 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -342,6 +342,34 @@ def test_get_pxe_boot_file_cpu_in_by_arch(self): result = utils.get_pxe_boot_file(self.node) self.assertEqual('aarch64-bootfile', result) + def test_get_pxe_boot_file_arch_boot_mode_composite_key(self): + bootfile_by_arch = {'x86_64-bios': 'pxelinux.0', + 'x86_64-uefi': 'bootx64.efi', + 'aarch64': 'grubaa64.efi'} + properties = {'cpu_arch': 'x86_64', 'capabilities': 'boot_mode:uefi'} + self.node.properties = properties + self.config(pxe_bootfile_name_by_arch=bootfile_by_arch, group='pxe') + result = utils.get_pxe_boot_file(self.node) + self.assertEqual('bootx64.efi', result) + + def test_get_pxe_boot_file_arch_boot_mode_composite_key_bios(self): + bootfile_by_arch = {'x86_64-bios': 'pxelinux.0', + 'x86_64-uefi': 'bootx64.efi'} + properties = {'cpu_arch': 'x86_64', 'capabilities': 'boot_mode:bios'} + self.node.properties = properties + self.config(pxe_bootfile_name_by_arch=bootfile_by_arch, group='pxe') + result = utils.get_pxe_boot_file(self.node) + self.assertEqual('pxelinux.0', result) + + def test_get_pxe_boot_file_arch_boot_mode_fallback_to_arch_only(self): + bootfile_by_arch = {'x86_64-bios': 'pxelinux.0', + 'aarch64': 'grubaa64.efi'} + properties = {'cpu_arch': 'aarch64', 'capabilities': 'boot_mode:uefi'} + self.node.properties = properties + self.config(pxe_bootfile_name_by_arch=bootfile_by_arch, group='pxe') + result = utils.get_pxe_boot_file(self.node) + self.assertEqual('grubaa64.efi', result) + def test_get_pxe_config_template_cpu_in_by_arch(self): properties = {'cpu_arch': 'aarch64', 'capabilities': 'boot_mode:uefi'} self.node.properties = properties @@ -392,6 +420,34 @@ def test_get_ipxe_boot_file_other_arch(self): result = utils.get_ipxe_boot_file(self.node) self.assertEqual('ipxe-aa64.efi', result) + def test_get_ipxe_boot_file_arch_boot_mode_composite_key(self): + arch_names = {'x86_64-bios': 'undionly.kpxe', + 'x86_64-uefi': 'snponly.efi', + 'aarch64': 'ipxe-aa64.efi'} + self.config(ipxe_bootfile_name_by_arch=arch_names, group='pxe') + properties = {'cpu_arch': 'x86_64', 'capabilities': 'boot_mode:uefi'} + self.node.properties = properties + result = utils.get_ipxe_boot_file(self.node) + self.assertEqual('snponly.efi', result) + + def test_get_ipxe_boot_file_arch_boot_mode_composite_key_bios(self): + arch_names = {'x86_64-bios': 'undionly.kpxe', + 'x86_64-uefi': 'snponly.efi'} + self.config(ipxe_bootfile_name_by_arch=arch_names, group='pxe') + properties = {'cpu_arch': 'x86_64', 'capabilities': 'boot_mode:bios'} + self.node.properties = properties + result = utils.get_ipxe_boot_file(self.node) + self.assertEqual('undionly.kpxe', result) + + def test_get_ipxe_boot_file_arch_boot_mode_fallback_to_arch_only(self): + arch_names = {'x86_64-bios': 'undionly.kpxe', + 'aarch64': 'ipxe-aa64.efi'} + self.config(ipxe_bootfile_name_by_arch=arch_names, group='pxe') + properties = {'cpu_arch': 'aarch64', 'capabilities': 'boot_mode:uefi'} + self.node.properties = properties + result = utils.get_ipxe_boot_file(self.node) + self.assertEqual('ipxe-aa64.efi', result) + def test_get_ipxe_boot_file_fallback(self): self.config(ipxe_bootfile_name=None, group='pxe') self.config(uefi_ipxe_bootfile_name=None, group='pxe') diff --git a/releasenotes/notes/bootfile-by-arch-boot-mode-composite-key-2b0a10764f3b71a9.yaml b/releasenotes/notes/bootfile-by-arch-boot-mode-composite-key-2b0a10764f3b71a9.yaml new file mode 100644 index 0000000000..4000a874f2 --- /dev/null +++ b/releasenotes/notes/bootfile-by-arch-boot-mode-composite-key-2b0a10764f3b71a9.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + The ``[pxe]pxe_bootfile_name_by_arch`` and + ``[pxe]ipxe_bootfile_name_by_arch`` options now support composite keys + in the format ``arch-boot_mode`` (e.g. ``x86_64-bios:undionly.kpxe``, + ``x86_64-uefi:snponly.efi``). The composite key is tried first, then + the arch-only key as a fallback, preserving backward compatibility. + Previously it was not possible to specify different boot files for the + same architecture in different boot modes (e.g. x86_64 in both UEFI + and BIOS), because the boot mode check was skipped when a by-arch + entry matched. + Fixes `bug 2157027 `_. From dd8c84f51b4d27843c11852282022cfb26e2edee Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 9 Jun 2026 12:43:41 -0700 Subject: [PATCH 2/4] security: block vendor.send_raw (CVE-2026-54423) A recent security report highlighted issues with the ability to invoke the ipmitool vendor pass-thru interface method of ``send_raw``. Where in the step model, this becomes "$interface_name"."$method_name", hence, "vendor.send_raw". This patch disables the method because standalone operators do not have the Role Based Access Control model to fallback on in order to prevent misuse of the interface. While the interface may still be useful, this disables the ability for standard API users to invoke the method by default. NOTE(JayF): Resolved some conflicts in ironic/conf/api.py Related-Bug: 2150458 Change-Id: I1f88315346ff19a4fcde34f954350b32cf02f96c Signed-off-by: Julia Kreger Signed-off-by: Jay Faulkner --- ironic/conf/api.py | 6 +- ironic/tests/unit/conductor/test_manager.py | 89 ++++++++++++++++++- .../disable_send_raw-f92217bf07c3eb38.yaml | 20 +++++ 3 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/disable_send_raw-f92217bf07c3eb38.yaml diff --git a/ironic/conf/api.py b/ironic/conf/api.py index 450695a901..3e27c0b4fe 100644 --- a/ironic/conf/api.py +++ b/ironic/conf/api.py @@ -104,7 +104,7 @@ def __call__(self, value): help=_("Specifies a list of boot modes that are not allowed " "during enrollment. Eg: ['bios']")), cfg.ListOpt('disallow_deploy_steps', - default=[], + default=['vendor.send_raw'], mutable=True, help=_("List of steps not allowed across the deploy " "workflow. Each entry should be in 'interface.step' " @@ -112,7 +112,7 @@ def __call__(self, value): "Applies to user-requested steps, deploy template " "steps, and driver steps alike.")), cfg.ListOpt('disallow_service_steps', - default=[], + default=['vendor.send_raw'], mutable=True, help=_("List of steps not allowed across the service " "workflow. Each entry should be in 'interface.step' " @@ -121,7 +121,7 @@ def __call__(self, value): "Applies to user-requested steps and driver steps " "alike.")), cfg.ListOpt('disallow_clean_steps', - default=[], + default=['vendor.send_raw'], mutable=True, help=_("List of steps not allowed across the clean " "workflow. Each entry should be in 'interface.step' " diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index d4b28f4778..5584b8e5e9 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2174,6 +2174,30 @@ def test_do_node_deploy_disallowed_step_raises(self, mock_start, node.refresh() self.assertEqual(states.AVAILABLE, node.provision_state) + @mock.patch.object(deployments, 'start_deploy', autospec=True) + def test_do_node_deploy_disallowed_step_raises_send_raw( + self, mock_start, mock_iwdi): + mock_iwdi.return_value = False + self._start_service() + deploy_steps = [ + {'step': 'send_raw', + 'priority': 7, + 'interface': 'vendor'} + ] + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.AVAILABLE, + target_provision_state=states.NOSTATE) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.do_node_deploy, + self.context, node.uuid, + deploy_steps=deploy_steps) + self.assertEqual(exception.StepNotAllowed, exc.exc_info[0]) + # start_deploy must NOT have been called + self.assertFalse(mock_start.called) + node.refresh() + self.assertEqual(states.AVAILABLE, node.provision_state) + @mock.patch.object(deployments, 'start_deploy', autospec=True) def test_do_node_deploy_allowed_step_proceeds(self, mock_start, mock_iwdi): @@ -3290,6 +3314,36 @@ def test_do_node_clean_disallowed_step_raises(self, mock_power_valid, # Node stays in original state self.assertEqual(states.MANAGEABLE, node.provision_state) + @mock.patch('ironic.conductor.task_manager.TaskManager.process_event', + autospec=True) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate', + autospec=True) + def test_do_node_clean_disallowed_step_raises_send_raw( + self, mock_power_valid, + mock_network_valid, + mock_process): + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.MANAGEABLE, + target_provision_state=states.NOSTATE) + self._start_service() + clean_steps = [ + {'step': 'send_raw', + 'priority': 7, + 'interface': 'vendor'} + ] + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.do_node_clean, + self.context, node.uuid, clean_steps) + self.assertEqual(exception.StepNotAllowed, exc.exc_info[0]) + # process_event must NOT have been called + self.assertFalse(mock_process.called) + node.refresh() + # Node stays in original state + self.assertEqual(states.MANAGEABLE, node.provision_state) + @mock.patch('ironic.conductor.task_manager.TaskManager.process_event', autospec=True) @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate', @@ -3509,7 +3563,10 @@ def test_do_node_service(self, mock_pv, mock_nv, mock_event): target_provision_state=states.NOSTATE) self._start_service() self.service.do_node_service(self.context, - node.uuid, {'foo': 'bar'}) + node.uuid, + [{'step': 'foo', + 'priority': 7, + 'interface': 'management'}]) self.assertTrue(mock_pv.called) self.assertTrue(mock_nv.called) mock_event.assert_called_once_with( @@ -3517,7 +3574,8 @@ def test_do_node_service(self, mock_pv, mock_nv, mock_event): 'service', callback=mock.ANY, call_args=(servicing.do_node_service, mock.ANY, - {'foo': 'bar'}, False), + [{'step': 'foo', 'priority': 7, + 'interface': 'management'}], False), err_handler=mock.ANY, target_state='active') @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', @@ -3565,6 +3623,33 @@ def test_do_node_service_disallowed_step_raises(self, mock_pv, mock_nv, node.refresh() self.assertEqual(states.ACTIVE, node.provision_state) + @mock.patch('ironic.conductor.task_manager.TaskManager.process_event', + autospec=True) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate', + autospec=True) + def test_do_node_service_disallowed_step_raises_on_send_raw( + self, mock_pv, mock_nv, + mock_event): + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.ACTIVE, + target_provision_state=states.NOSTATE) + self._start_service() + service_steps = [ + {'step': 'send_raw', + 'priority': 7, + 'interface': 'vendor'} + ] + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.do_node_service, + self.context, node.uuid, service_steps) + self.assertEqual(exception.StepNotAllowed, exc.exc_info[0]) + self.assertFalse(mock_event.called) + node.refresh() + self.assertEqual(states.ACTIVE, node.provision_state) + @mock.patch('ironic.conductor.task_manager.TaskManager.process_event', autospec=True) @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate', diff --git a/releasenotes/notes/disable_send_raw-f92217bf07c3eb38.yaml b/releasenotes/notes/disable_send_raw-f92217bf07c3eb38.yaml new file mode 100644 index 0000000000..654bcab0af --- /dev/null +++ b/releasenotes/notes/disable_send_raw-f92217bf07c3eb38.yaml @@ -0,0 +1,20 @@ +--- +security: + - | + The IPMI Vendor-Passthru interface method ``send_raw`` step has been + disabled by default due to security implications. A malicious user with + sufficient Ironic access could utilzie this interface to make manual + changes to BMCs when the ``ipmitool`` vendor-passthru interface was + enabled. +fixes: + - | + Fixes a bug where ironic allowed the ability for operators to send + raw commands when the ``ipmitool`` vendor interface was enabled + and configured for a baremetal node utilizing the IPMI protocol. + + The Ironic project generally recommends the use of ``redfish`` + instead of IPMI, however recognizes that is not universally possible + for all operators. + + More information can be found in `bug 2150458 + `_. From 54e4cf7a400dc32fb471829ac6d76a93749d0421 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 4 May 2026 17:14:07 +0200 Subject: [PATCH 3/4] CVE-2026-44918: Prevent rehoming resources to nodes with different owner Ironic has a few similar patterns where only the source node is verified during a create or update operation: 1. A node can be created with a parent from a different project 2. Node can be updated to have a parent from a different project 3. Ports, port groups, volume targets and volume connectors can be patched to be owned by a node from a different project. This patch addresses all these cases and introduces a single helper to cover all of them. Also makes sure that baremetal:node:get is checked even if the parent node ID is a UUID (it was missing before). Related-Bug: 2150450 Change-Id: Iaa512f380926a9fc032eefea5bcdcbc5040a4739 Signed-off-by: Julia Kreger Signed-off-by: Jay Faulkner (cherry picked from commit b58d4adc43551323b7ae8d5252636d796e32b949) (cherry picked from commit d99b35dcda7df5c09621e1c1a2c4def112d1d7c6) --- ironic/api/controllers/v1/node.py | 30 ++-- ironic/api/controllers/v1/port.py | 18 +-- ironic/api/controllers/v1/portgroup.py | 21 +-- ironic/api/controllers/v1/utils.py | 76 +++++++--- ironic/api/controllers/v1/volume_connector.py | 21 +-- ironic/api/controllers/v1/volume_target.py | 31 ++--- ironic/cmd/status.py | 37 +++++ .../unit/api/controllers/v1/test_node.py | 9 ++ .../unit/api/controllers/v1/test_utils.py | 18 --- .../unit/api/test_rbac_project_scoped.yaml | 130 +++++++++++++++++- ironic/tests/unit/cmd/test_status.py | 56 ++++++++ .../2150450-owners-b4019d5fa63a8bc1.yaml | 21 +++ tox.ini | 2 +- 13 files changed, 342 insertions(+), 128 deletions(-) create mode 100644 releasenotes/notes/2150450-owners-b4019d5fa63a8bc1.yaml diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 9ea4b7a38b..55730915f4 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -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) @@ -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): @@ -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: diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index f70c24093d..80889c12dd 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -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 @@ -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'): @@ -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) diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py index ec1035e112..9c30a90006 100644 --- a/ironic/api/controllers/v1/portgroup.py +++ b/ironic/api/controllers/v1/portgroup.py @@ -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) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 244940ab48..629cdcecba 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -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 @@ -47,6 +48,8 @@ CONF = cfg.CONF +LOG = log.getLogger(__name__) + _JSONPATCH_EXCEPTIONS = (jsonpatch.JsonPatchConflict, jsonpatch.JsonPatchException, @@ -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. @@ -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 diff --git a/ironic/api/controllers/v1/volume_connector.py b/ironic/api/controllers/v1/volume_connector.py index a94d4282ba..e7683d0630 100644 --- a/ironic/api/controllers/v1/volume_connector.py +++ b/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) diff --git a/ironic/api/controllers/v1/volume_target.py b/ironic/api/controllers/v1/volume_target.py index e7f477c85e..7d50649c68 100644 --- a/ironic/api/controllers/v1/volume_target.py +++ b/ironic/api/controllers/v1/volume_target.py @@ -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() @@ -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) diff --git a/ironic/cmd/status.py b/ironic/cmd/status.py index 63cfcc746f..dc735cda84 100644 --- a/ironic/cmd/status.py +++ b/ironic/cmd/status.py @@ -20,6 +20,7 @@ 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 @@ -27,6 +28,7 @@ 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 @@ -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 (, ). # The name of the check will be used in the output of this command. # The check function takes no arguments and returns an @@ -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), ) diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index d7f0fe0050..9056cd1216 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -2843,6 +2843,15 @@ 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') + # NOTE(TheJulia): Checking parent node here is to ensure we don't + # accidentally regress with auto-setting logic. + self.assertIsNone(updated_node.parent_node) mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 1f12798749..4bf1414e65 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/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): diff --git a/ironic/tests/unit/api/test_rbac_project_scoped.yaml b/ironic/tests/unit/api/test_rbac_project_scoped.yaml index 32782b1cf5..6f5c4606a5 100644 --- a/ironic/tests/unit/api/test_rbac_project_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_project_scoped.yaml @@ -2468,6 +2468,30 @@ third_party_admin_cannot_modify_portgroup: body: *portgroup_patch_body assert_status: 404 +owner_admin_cannot_rehome_portgroup: + path: '/v1/portgroups/{owner_portgroup_ident}' + method: patch + headers: *owner_admin_headers + body: &other_node_portgroup_patch_body + - op: replace + path: /node_uuid + value: 573208e5-cd41-4e26-8f06-ef44022b3793 + assert_status: 400 + +owner_manager_cannot_rehome_portgroup: + path: '/v1/portgroups/{owner_portgroup_ident}' + method: patch + headers: *owner_manager_headers + body: *other_node_portgroup_patch_body + assert_status: 400 + +owner_service_cannot_rehome_portgroup: + path: '/v1/portgroups/{owner_portgroup_ident}' + method: patch + headers: *service_headers_owner_project + body: *other_node_portgroup_patch_body + assert_status: 400 + owner_admin_can_delete_portgroup: path: '/v1/portgroups/{owner_portgroup_ident}' method: delete @@ -2768,6 +2792,30 @@ owner_admin_can_delete_port: headers: *owner_admin_headers assert_status: 503 +owner_admin_cannot_rehome_port: + path: '/v1/ports/{owner_port_ident}' + method: patch + headers: *owner_admin_headers + body: &other_node_port_patch_body + - op: replace + path: /node_uuid + value: 573208e5-cd41-4e26-8f06-ef44022b3793 + assert_status: 400 + +owner_manager_cannot_rehome_port: + path: '/v1/ports/{owner_port_ident}' + method: patch + headers: *owner_manager_headers + body: *other_node_port_patch_body + assert_status: 400 + +owner_service_cannot_rehome_port: + path: '/v1/ports/{owner_port_ident}' + method: patch + headers: *service_headers_owner_project + body: *other_node_port_patch_body + assert_status: 400 + owner_manager_can_delete_port: path: '/v1/ports/{owner_port_ident}' method: delete @@ -3044,6 +3092,30 @@ third_party_admin_cannot_patch_volume_connectors: body: *connector_patch_body assert_status: 404 +owner_admin_cannot_rehome_volume_connector: + path: '/v1/volume/connectors/{volume_connector_ident}' + method: patch + headers: *owner_admin_headers + body: &other_node_connector_patch_body + - op: replace + path: /node_uuid + value: 573208e5-cd41-4e26-8f06-ef44022b3793 + assert_status: 400 + +owner_manager_cannot_rehome_volume_connector: + path: '/v1/volume/connectors/{volume_connector_ident}' + method: patch + headers: *owner_manager_headers + body: *other_node_connector_patch_body + assert_status: 400 + +owner_service_cannot_rehome_volume_connector: + path: '/v1/volume/connectors/{volume_connector_ident}' + method: patch + headers: *service_headers_owner_project + body: *other_node_connector_patch_body + assert_status: 400 + owner_admin_can_delete_volume_connectors: path: '/v1/volume/connectors/{volume_connector_ident}' method: delete @@ -3245,6 +3317,30 @@ service_cannot_patch_volume_target: headers: *service_headers assert_status: 404 +owner_admin_cannot_rehome_volume_target: + path: '/v1/volume/targets/{volume_target_ident}' + method: patch + headers: *owner_admin_headers + body: &other_node_target_patch_body + - op: replace + path: /node_uuid + value: 573208e5-cd41-4e26-8f06-ef44022b3793 + assert_status: 400 + +owner_manager_cannot_rehome_volume_target: + path: '/v1/volume/targets/{volume_target_ident}' + method: patch + headers: *owner_manager_headers + body: *other_node_target_patch_body + assert_status: 400 + +owner_service_cannot_rehome_volume_target: + path: '/v1/volume/targets/{volume_target_ident}' + method: patch + headers: *service_headers_owner_project + body: *other_node_target_patch_body + assert_status: 400 + owner_admin_can_delete_volume_target: path: '/v1/volume/targets/{volume_target_ident}' method: delete @@ -3962,6 +4058,35 @@ shard_patch_set_node_shard_disallowed: value: 'TestShard' assert_status: 403 +# Create node with parent_node field + +admin_can_create_node_with_parent: + path: '/v1/nodes' + method: post + headers: *owner_admin_headers + body: + driver: fake + parent_node: *owned_node_ident + assert_status: 503 + +admin_cannot_create_node_with_non_existing_parent: + path: '/v1/nodes' + method: post + headers: *owner_admin_headers + body: + driver: fake + parent_node: 'f11853c7-fa9c-4db3-a477-c9d8e0dbbf13' + assert_status: 400 + +admin_cannot_create_node_with_parent_they_cannot_see: + path: '/v1/nodes' + method: post + headers: *owner_admin_headers + body: + driver: fake + parent_node: '{node_ident}' + assert_status: 400 + # Update node parent_node field - baremetal:node:update:parent_node parent_node_patch_by_admin: @@ -3996,8 +4121,7 @@ parent_node_patch_by_manager: assert_status: 403 parent_node_patch_by_cannot_see_node: - # This node cannot be seen, and also just doesn't exist. - # Just to verify we return a 400 on a node we can change. + # Avoid exposing whether the node exists if not authorized path: '/v1/nodes/{lessee_node_ident}' method: patch headers: *owner_admin_headers @@ -4005,7 +4129,7 @@ parent_node_patch_by_cannot_see_node: - op: replace path: /parent_node value: 'f11853c7-fa9c-4db3-a477-c9d8e0dbbf13' - assert_status: 400 + assert_status: 403 parent_node_children_can_get_list_of_children: path: '/v1/nodes/{owner_node_ident}/children' diff --git a/ironic/tests/unit/cmd/test_status.py b/ironic/tests/unit/cmd/test_status.py index 42ebe339c0..700e6f7787 100644 --- a/ironic/tests/unit/cmd/test_status.py +++ b/ironic/tests/unit/cmd/test_status.py @@ -16,6 +16,7 @@ from oslo_db import sqlalchemy from oslo_upgradecheck.upgradecheck import Code +from oslo_utils import uuidutils from sqlalchemy.engine import url as sa_url from ironic.cmd import dbsync @@ -146,3 +147,58 @@ def test__check_allocations_table_myiasm_both(self, mock_reader): 'table engine to utilize InnoDB, and reload the ' 'allocations table to utilize the InnoDB engine.') self.assertEqual(expected_msg, check_result.details) + + +class TestUpgradeChecksOwnerMismatch(db_base.DbTestCase): + + def setUp(self): + super().setUp() + self.cmd = status.Checks() + + def test_correct_nodes(self): + parents = [ + # Parent without an owner + {'uuid': uuidutils.generate_uuid()}, + # Parent with an owner + {'uuid': uuidutils.generate_uuid(), + 'owner': 'abcd'}, + ] + children = [ + {'parent_node': parents[0]['uuid']}, + {'parent_node': parents[1]['uuid'], + 'owner': parents[1]['owner']}, + ] + for node in parents + children: + self.dbapi.create_node(node) + + result = self.cmd._check_parent_child_owners() + self.assertEqual(Code.SUCCESS, result.code) + + def test_mismatch(self): + parents = [ + # Parent without an owner + {'uuid': uuidutils.generate_uuid()}, + # Parent with an owner + {'uuid': uuidutils.generate_uuid(), + 'owner': 'abcd'}, + ] + children = [ + # Owned child of a parent without owner + {'uuid': uuidutils.generate_uuid(), + 'parent_node': parents[0]['uuid'], + 'owner': 'abcd'}, + # Child of an owned node without owner + {'uuid': uuidutils.generate_uuid(), + 'parent_node': parents[1]['uuid']}, + # Mismatched owners + {'uuid': uuidutils.generate_uuid(), + 'parent_node': parents[1]['uuid'], + 'owner': parents[1]['owner'] + 'bad'}, + ] + for node in parents + children: + self.dbapi.create_node(node) + + result = self.cmd._check_parent_child_owners() + self.assertEqual(Code.WARNING, result.code) + for child in children: + self.assertIn(child['uuid'], result.details) diff --git a/releasenotes/notes/2150450-owners-b4019d5fa63a8bc1.yaml b/releasenotes/notes/2150450-owners-b4019d5fa63a8bc1.yaml new file mode 100644 index 0000000000..9d7e04e0de --- /dev/null +++ b/releasenotes/notes/2150450-owners-b4019d5fa63a8bc1.yaml @@ -0,0 +1,21 @@ +--- +security: + - | + Addresses a potential security issues where an admin of a project could + create nodes with ``parent_node`` set to a node from a different project. + - | + 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 `_. +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. +upgrade: + - | + Adds an upgrade check that issues a warning when any node has a different + owner from its parent node. diff --git a/tox.ini b/tox.ini index bb8ac9446e..669d842fab 100644 --- a/tox.ini +++ b/tox.ini @@ -169,7 +169,7 @@ filename = *.py,app.wsgi exclude=.*,dist,doc,*lib/python*,*egg,build import-order-style = pep8 application-import-names = ironic -max-complexity=20 +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. From 17d566791363ee5d9b72c315d637c6991806ff62 Mon Sep 17 00:00:00 2001 From: Sebastian Lohff Date: Mon, 18 May 2026 14:32:57 +0200 Subject: [PATCH 4/4] Use service auth for Neutron port operations When a user triggers a manual operation like cleaning, most requests towards the Neutron API are made with the authentication parameters provided by the user's context. In case of port creation this can lead to ports being created with an unexpected project id (taken from the project scope of the current user). To prevent this, we now use the service credentials from config for add_ports_to_network() and remove_neutron_ports(). Closes-Bug: #2121702 Change-Id: I0669782bacfb95b42dfa300e4f91ece62893847c Signed-off-by: Sebastian Lohff (cherry picked from commit 27083f06332ecc48d17911a3c75f2334b8723444) --- ironic/common/neutron.py | 4 ++-- ...e-auth-for-neutron-port-operations-03c8349ee5c06e3b.yaml | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/use-service-auth-for-neutron-port-operations-03c8349ee5c06e3b.yaml diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 3708e2def9..f7132f0a7c 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -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 @@ -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: diff --git a/releasenotes/notes/use-service-auth-for-neutron-port-operations-03c8349ee5c06e3b.yaml b/releasenotes/notes/use-service-auth-for-neutron-port-operations-03c8349ee5c06e3b.yaml new file mode 100644 index 0000000000..c320dc4e06 --- /dev/null +++ b/releasenotes/notes/use-service-auth-for-neutron-port-operations-03c8349ee5c06e3b.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Neutron port operations now always use service credentials from config, + rather than the user's token context. This prevents ports being created + with an unexpected project id (e.g. the project id of the user's context).