Allow configuring IPMI cipher suite

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

Change-Id: I04a3391f85412dcabc6105bd91beb1da25bdfc19

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 <UUID or name> --driver-info ipmi_cipher_suite=3
+
 Static boot order configuration
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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,
     }
 
 
@@ -400,17 +417,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:

ironic/tests/unit/conductor/test_manager.py

@@ -5800,7 +5800,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):

ironic/tests/unit/drivers/modules/test_ipmitool.py

@@ -808,6 +808,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)
@@ -1384,6 +1406,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)

releasenotes/notes/ipmi-cipher-suite-499097740f7c86ee.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Allows configuring IPMI cipher suite via the new ``driver_info``
+    parameter ``ipmi_cipher_suite``.