Skip to content

Commit 7bbe5a2

Browse files
juliakregerjayofdoom
authored andcommitted
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. Related-Bug: 2150458 Change-Id: I1f88315346ff19a4fcde34f954350b32cf02f96c Signed-off-by: Julia Kreger <juliaashleykreger@gmail.com> Signed-off-by: Jay Faulkner <jay@jvf.cc>
1 parent 95eb164 commit 7bbe5a2

3 files changed

Lines changed: 110 additions & 5 deletions

File tree

ironic/conf/api.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,15 +195,15 @@ def __call__(self, value):
195195
'Each middleware must be a callable that accepts a '
196196
'WSGI application and returns a wrapped application.')),
197197
cfg.ListOpt('disallow_deploy_steps',
198-
default=[],
198+
default=['vendor.send_raw'],
199199
mutable=True,
200200
help=_("List of steps not allowed across the deploy "
201201
"workflow. Each entry should be in 'interface.step' "
202202
"format, e.g. ['raid.apply_configuration']. "
203203
"Applies to user-requested steps, deploy template "
204204
"steps, and driver steps alike.")),
205205
cfg.ListOpt('disallow_service_steps',
206-
default=[],
206+
default=['vendor.send_raw'],
207207
mutable=True,
208208
help=_("List of steps not allowed across the service "
209209
"workflow. Each entry should be in 'interface.step' "
@@ -212,7 +212,7 @@ def __call__(self, value):
212212
"Applies to user-requested steps and driver steps "
213213
"alike.")),
214214
cfg.ListOpt('disallow_clean_steps',
215-
default=[],
215+
default=['vendor.send_raw'],
216216
mutable=True,
217217
help=_("List of steps not allowed across the clean "
218218
"workflow. Each entry should be in 'interface.step' "

ironic/tests/unit/conductor/test_manager.py

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2176,6 +2176,30 @@ def test_do_node_deploy_disallowed_step_raises(self, mock_start,
21762176
node.refresh()
21772177
self.assertEqual(states.AVAILABLE, node.provision_state)
21782178

2179+
@mock.patch.object(deployments, 'start_deploy', autospec=True)
2180+
def test_do_node_deploy_disallowed_step_raises_send_raw(
2181+
self, mock_start, mock_iwdi):
2182+
mock_iwdi.return_value = False
2183+
self._start_service()
2184+
deploy_steps = [
2185+
{'step': 'send_raw',
2186+
'priority': 7,
2187+
'interface': 'vendor'}
2188+
]
2189+
node = obj_utils.create_test_node(
2190+
self.context, driver='fake-hardware',
2191+
provision_state=states.AVAILABLE,
2192+
target_provision_state=states.NOSTATE)
2193+
exc = self.assertRaises(messaging.rpc.ExpectedException,
2194+
self.service.do_node_deploy,
2195+
self.context, node.uuid,
2196+
deploy_steps=deploy_steps)
2197+
self.assertEqual(exception.StepNotAllowed, exc.exc_info[0])
2198+
# start_deploy must NOT have been called
2199+
self.assertFalse(mock_start.called)
2200+
node.refresh()
2201+
self.assertEqual(states.AVAILABLE, node.provision_state)
2202+
21792203
@mock.patch.object(deployments, 'start_deploy', autospec=True)
21802204
def test_do_node_deploy_allowed_step_proceeds(self, mock_start,
21812205
mock_iwdi):
@@ -3535,6 +3559,36 @@ def test_do_node_clean_disallowed_step_raises(self, mock_power_valid,
35353559
# Node stays in original state
35363560
self.assertEqual(states.MANAGEABLE, node.provision_state)
35373561

3562+
@mock.patch('ironic.conductor.task_manager.TaskManager.process_event',
3563+
autospec=True)
3564+
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
3565+
autospec=True)
3566+
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
3567+
autospec=True)
3568+
def test_do_node_clean_disallowed_step_raises_send_raw(
3569+
self, mock_power_valid,
3570+
mock_network_valid,
3571+
mock_process):
3572+
node = obj_utils.create_test_node(
3573+
self.context, driver='fake-hardware',
3574+
provision_state=states.MANAGEABLE,
3575+
target_provision_state=states.NOSTATE)
3576+
self._start_service()
3577+
clean_steps = [
3578+
{'step': 'send_raw',
3579+
'priority': 7,
3580+
'interface': 'vendor'}
3581+
]
3582+
exc = self.assertRaises(messaging.rpc.ExpectedException,
3583+
self.service.do_node_clean,
3584+
self.context, node.uuid, clean_steps)
3585+
self.assertEqual(exception.StepNotAllowed, exc.exc_info[0])
3586+
# process_event must NOT have been called
3587+
self.assertFalse(mock_process.called)
3588+
node.refresh()
3589+
# Node stays in original state
3590+
self.assertEqual(states.MANAGEABLE, node.provision_state)
3591+
35383592
@mock.patch('ironic.conductor.task_manager.TaskManager.process_event',
35393593
autospec=True)
35403594
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
@@ -3750,15 +3804,19 @@ def test_do_node_service(self, mock_pv, mock_nv, mock_event):
37503804
target_provision_state=states.NOSTATE)
37513805
self._start_service()
37523806
self.service.do_node_service(self.context,
3753-
node.uuid, {'foo': 'bar'})
3807+
node.uuid,
3808+
[{'step': 'foo',
3809+
'priority': 7,
3810+
'interface': 'management'}])
37543811
self.assertTrue(mock_pv.called)
37553812
self.assertTrue(mock_nv.called)
37563813
mock_event.assert_called_once_with(
37573814
mock.ANY,
37583815
'service',
37593816
callback=mock.ANY,
37603817
call_args=(servicing.do_node_service, mock.ANY,
3761-
{'foo': 'bar'}, False),
3818+
[{'step': 'foo', 'priority': 7,
3819+
'interface': 'management'}], False),
37623820
err_handler=mock.ANY, target_state='active')
37633821

37643822
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
@@ -3805,6 +3863,33 @@ def test_do_node_service_disallowed_step_raises(self, mock_pv, mock_nv,
38053863
node.refresh()
38063864
self.assertEqual(states.ACTIVE, node.provision_state)
38073865

3866+
@mock.patch('ironic.conductor.task_manager.TaskManager.process_event',
3867+
autospec=True)
3868+
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
3869+
autospec=True)
3870+
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
3871+
autospec=True)
3872+
def test_do_node_service_disallowed_step_raises_on_send_raw(
3873+
self, mock_pv, mock_nv,
3874+
mock_event):
3875+
node = obj_utils.create_test_node(
3876+
self.context, driver='fake-hardware',
3877+
provision_state=states.ACTIVE,
3878+
target_provision_state=states.NOSTATE)
3879+
self._start_service()
3880+
service_steps = [
3881+
{'step': 'send_raw',
3882+
'priority': 7,
3883+
'interface': 'vendor'}
3884+
]
3885+
exc = self.assertRaises(messaging.rpc.ExpectedException,
3886+
self.service.do_node_service,
3887+
self.context, node.uuid, service_steps)
3888+
self.assertEqual(exception.StepNotAllowed, exc.exc_info[0])
3889+
self.assertFalse(mock_event.called)
3890+
node.refresh()
3891+
self.assertEqual(states.ACTIVE, node.provision_state)
3892+
38083893
@mock.patch('ironic.conductor.task_manager.TaskManager.process_event',
38093894
autospec=True)
38103895
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
security:
3+
- |
4+
The IPMI Vendor-Passthru interface method ``send_raw`` step has been
5+
disabled by default due to security implications. A malicious user with
6+
sufficient Ironic access could utilzie this interface to make manual
7+
changes to BMCs when the ``ipmitool`` vendor-passthru interface was
8+
enabled.
9+
fixes:
10+
- |
11+
Fixes a bug where ironic allowed the ability for operators to send
12+
raw commands when the ``ipmitool`` vendor interface was enabled
13+
and configured for a baremetal node utilizing the IPMI protocol.
14+
15+
The Ironic project generally recommends the use of ``redfish``
16+
instead of IPMI, however recognizes that is not universally possible
17+
for all operators.
18+
19+
More information can be found in `bug 2150458
20+
<https://bugs.launchpad.net/ironic/+bug/2150458>`_.

0 commit comments

Comments
 (0)