openstack/ironic
Code Issues Proposed changes

Allow configuring IPMI cipher suite

Browse Source 
Negotiation fails for some hardware, let's allow an explicit setting.

Change-Id: I04a3391f85412dcabc6105bd91beb1da25bdfc19
(cherry picked from commit 2773c5fb25)
This commit is contained in:
Dmitry Tantsur 2020-09-16 15:29:55 +02:00 committed by Julia Kreger
parent 145d68e8f5
commit ac2d4c086b
5 changed files with 115 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
doc/source/admin/drivers/ipmitool.rst
View File

@ -168,6 +168,28 @@ protocol version::
Version *1.5* of the IPMI protocol does not support encryption.
Therefore, it is highly recommended that version 2.0 is used.
Cipher suites
~~~~~~~~~~~~~
IPMI 2.0 introduces support for encryption and allows setting which cipher
suite to use. Traditionally, ``ipmitool`` was using cipher suite 3 by default,
but since SHA1 no longer complies with modern security requirement, recent
versions (e.g. the one used in RHEL 8.2) are switching to suite 17.
Normally, the cipher suite to use is negotiated with the BMC using the special
command. On some hardware the negotiation yields incorrect results and IPMI
commands fail with
::
Error in open session response message : no matching cipher suite
Error: Unable to establish IPMI v2 / RMCP+ session
Another possible problem is ``ipmitool`` commands taking very long (tens of
seconds or even minutes) because the BMC does not support cipher suite
negotiation. In both cases you can specify the required suite yourself, e.g.::
openstack baremetal node set <UUID or name> --driver-info ipmi_cipher_suite=3
Static boot order configuration
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

35
ironic/drivers/modules/ipmitool.py
View File

@ -109,6 +109,11 @@ OPTIONAL_PROPERTIES = {
'[ipmi]disable_boot_timeout will be '
'used if this option is not set. '
'Optional.'),
'ipmi_cipher_suite': _('The number of a cipher suite to use. Only 3 '
'(AES-128 with SHA1) or 17 (AES-128 with SHA256) '
'should ever be used; 17 is preferred. The '
'default value depends on the ipmitool version, '
'some recent versions have switched from 3 to 17.'),
}
COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)
@ -298,6 +303,7 @@ def _parse_driver_info(node):
target_address = info.get('ipmi_target_address')
protocol_version = str(info.get('ipmi_protocol_version', '2.0'))
force_boot_device = info.get('ipmi_force_boot_device', False)
cipher_suite = info.get('ipmi_cipher_suite')
if not username:
LOG.warning('ipmi_username is not defined or empty for node %s: '
@ -370,6 +376,16 @@ def _parse_driver_info(node):
raise exception.InvalidParameterValue(_(
"Number of ipmi_hex_kg_key characters is not even"))
if cipher_suite is not None:
try:
int(cipher_suite)
except (TypeError, ValueError):
raise exception.InvalidParameterValue(_(
"Invalid cipher suite %s, expected a number") % cipher_suite)
if protocol_version == '1.5':
raise exception.InvalidParameterValue(_(
"Cipher suites cannot be used with IPMI 1.5"))
return {
'address': address,
'dest_port': dest_port,
@ -386,6 +402,7 @@ def _parse_driver_info(node):
'target_address': target_address,
'protocol_version': protocol_version,
'force_boot_device': force_boot_device,
'cipher_suite': cipher_suite,
}
@ -425,17 +442,13 @@ def _get_ipmitool_args(driver_info, pw_file=None):
'-L', driver_info['priv_level']
]
if driver_info['dest_port']:
args.append('-p')
args.append(driver_info['dest_port'])
if driver_info['username']:
args.append('-U')
args.append(driver_info['username'])
if driver_info['hex_kg_key']:
args.append('-y')
args.append(driver_info['hex_kg_key'])
for field, arg in [('dest_port', '-p'),
('username', '-U'),
('hex_kg_key', '-y'),
('cipher_suite', '-C')]:
if driver_info[field]:
args.append(arg)
args.append(driver_info[field])
for name, option in BRIDGING_OPTIONS:
if driver_info[name] is not None:

3
ironic/tests/unit/conductor/test_manager.py
View File

@ -5639,7 +5639,8 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
'force_persistent_boot_device', 'ipmi_protocol_version',
'ipmi_force_boot_device', 'deploy_forces_oob_reboot',
'rescue_kernel', 'rescue_ramdisk',
'ipmi_disable_boot_timeout', 'ipmi_hex_kg_key']
'ipmi_disable_boot_timeout', 'ipmi_hex_kg_key',
'ipmi_cipher_suite']
self._check_driver_properties("ipmi", expected)
def test_driver_properties_snmp(self):

49
ironic/tests/unit/drivers/modules/test_ipmitool.py
View File

@ -807,6 +807,28 @@ class IPMIToolPrivateMethodTestCase(
ret = ipmi._parse_driver_info(node)
self.assertEqual(623, ret['dest_port'])
def test__parse_driver_info_ipmi_cipher_suite(self):
info = dict(INFO_DICT)
info['ipmi_cipher_suite'] = 0 # absolute power!
node = obj_utils.get_test_node(self.context, driver_info=info)
ret = ipmi._parse_driver_info(node)
self.assertEqual(0, ret['cipher_suite'])
def test__parse_driver_info_ipmi_cipher_suite_not_a_number(self):
info = dict(INFO_DICT)
info['ipmi_cipher_suite'] = 'I can haz accez'
node = obj_utils.get_test_node(self.context, driver_info=info)
self.assertRaises(exception.InvalidParameterValue,
ipmi._parse_driver_info, node)
def test__parse_driver_info_ipmi_cipher_suite_ipmi_1_5(self):
info = dict(INFO_DICT)
info['ipmi_cipher_suite'] = 0
info['ipmi_protocol_version'] = '1.5'
node = obj_utils.get_test_node(self.context, driver_info=info)
self.assertRaises(exception.InvalidParameterValue,
ipmi._parse_driver_info, node)
@mock.patch.object(ipmi.LOG, 'warning', spec_set=True, autospec=True)
def test__parse_driver_info_undefined_credentials(self, mock_log):
info = dict(INFO_DICT)
@ -1360,6 +1382,33 @@ class IPMIToolPrivateMethodTestCase(
mock_exec.assert_called_once_with(*args)
self.assertFalse(self.mock_sleep.called)
@mock.patch.object(ipmi, '_is_option_supported', autospec=True)
@mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub)
@mock.patch.object(utils, 'execute', autospec=True)
def test__exec_ipmitool_cipher_suite(self, mock_exec, mock_support):
self.info['cipher_suite'] = '3'
ipmi.LAST_CMD_TIME = {}
args = [
'ipmitool',
'-I', 'lanplus',
'-H', self.info['address'],
'-L', self.info['priv_level'],
'-U', self.info['username'],
'-C', '3',
'-v',
'-f', awesome_password_filename,
'A', 'B', 'C',
]
mock_support.return_value = False
mock_exec.return_value = (None, None)
ipmi._exec_ipmitool(self.info, 'A B C')
mock_support.assert_called_once_with('timing')
mock_exec.assert_called_once_with(*args)
self.assertFalse(self.mock_sleep.called)
@mock.patch.object(ipmi, '_is_option_supported', autospec=True)
@mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub)
@mock.patch.object(utils, 'execute', autospec=True)

18
releasenotes/notes/ipmi-cipher-suite-499097740f7c86ee.yaml Normal file
View File

@ -0,0 +1,18 @@
---
fixes:
- |
Allows configuring IPMI cipher suite via the new ``driver_info``
parameter ``ipmi_cipher_suite`` to enable operators to navigate
``ipmitool`` behavior changes around supported ciphers.
issues:
- Some ``ipmitool`` builds, in particular on machines running
Red Hat Enterprise Linux 8.2, have changed the default cipher suite
being offered which can cause ``ipmitool`` to completely fail to
negotiate a connection with the BMC. Operators who encounter this
situation should use the ``ipmi_cipher_suite`` parameter in the
``driver_info`` field to override and directly assert the required
cipher. Because of potential security implications of attempting
second level auto-negotiation and known BMC vendor behaviors,
this must be identified by the operator and explicitly set as
logic to attempt to navigate through situations like this may
have undesirable results.