diff --git a/doc/source/admin/drivers/ipmitool.rst b/doc/source/admin/drivers/ipmitool.rst index dd051d9881..b9cca3e4ac 100644 --- a/doc/source/admin/drivers/ipmitool.rst +++ b/doc/source/admin/drivers/ipmitool.rst @@ -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 --driver-info ipmi_cipher_suite=3 + Static boot order configuration ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 92b9500e6b..befc637bfc 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -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: diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index cd0816a74c..a532979110 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -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): diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 6ab5681c97..b0852d58e1 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -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) diff --git a/releasenotes/notes/ipmi-cipher-suite-499097740f7c86ee.yaml b/releasenotes/notes/ipmi-cipher-suite-499097740f7c86ee.yaml new file mode 100644 index 0000000000..07c09e1f45 --- /dev/null +++ b/releasenotes/notes/ipmi-cipher-suite-499097740f7c86ee.yaml @@ -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.