From b029fb377001cfe5d727ead1d2a26535c720b381 Mon Sep 17 00:00:00 2001 From: Iury Gregory Melo Ferreira Date: Wed, 13 Jan 2021 03:13:59 +0100 Subject: [PATCH] Automaticaly set cipher suite In some cases the operator can't specify `ipmi_cipher_suite` for each node and the problem with session can still occour: `Error in open session response message : no matching cipher suite` This patch adds a new configuration option that will take a list of possible cipher suite versions that can be used when the error occurs and the node doesn't have the `ipmi_cipher_suite` set. Story: 2008739 Task: 42093 Change-Id: I6788585a83268e20ff6447e570995871bc9c25d5 --- doc/source/admin/drivers/ipmitool.rst | 22 ++ ironic/conf/ipmi.py | 5 + ironic/drivers/modules/ipmitool.py | 78 +++++ .../unit/drivers/modules/test_ipmitool.py | 289 ++++++++++++++++++ ...ipher_suite_versions-c64644860d3c220d.yaml | 6 + 5 files changed, 400 insertions(+) create mode 100644 releasenotes/notes/cipher_suite_versions-c64644860d3c220d.yaml diff --git a/doc/source/admin/drivers/ipmitool.rst b/doc/source/admin/drivers/ipmitool.rst index 9d9d949b74..f8ea5ca66d 100644 --- a/doc/source/admin/drivers/ipmitool.rst +++ b/doc/source/admin/drivers/ipmitool.rst @@ -190,6 +190,28 @@ negotiation. In both cases you can specify the required suite yourself, e.g.:: baremetal node set --driver-info ipmi_cipher_suite=3 +In scenarios where the operator can't specify the `ipmi_cipher_suite` for +each node, the configuration `[ipmi]/cipher_suite_versions` can be set to +a list of cipher suites that will be used, Ironic will attempt to find a value +that can be used from the list provided (from last to first).:: + + [ipmi] + cipher_suite_versions = ['1','2','3','6','7','8','11','12'] + +To find the suitable values for this configuration, you can check the field +`RMCP+ Cipher Suites` after running an `ipmitool` command, e.g:: + + $ ipmitool -I lanplus -H $HOST -U $USER -v -R 12 -N 5 lan print + # output + Set in Progress : Set Complete + Auth Type Support : NONE MD2 MD5 PASSWORD OEM + Auth Type Enable : Callback : NONE MD2 MD5 PASSWORD OEM + IP Address Source : Static Address + IP Address : + Subnet Mask : + MAC Address : + RMCP+ Cipher Suites : 0,1,2,3,6,7,8,11,12 + Static boot order configuration ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ironic/conf/ipmi.py b/ironic/conf/ipmi.py index 0ef39b7b29..1ea9aa4a6e 100644 --- a/ironic/conf/ipmi.py +++ b/ironic/conf/ipmi.py @@ -73,6 +73,11 @@ opts = [ 'additional debugging output. This is a separate ' 'option as ipmitool can log a substantial amount ' 'of misleading text when in this mode.')), + cfg.ListOpt('cipher_suite_versions', + default=[], + help=_('List of possible cipher suites versions that can ' + 'be supported by the hardware in case the field ' + '`cipher_suite` is not set for the node.')), ] diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 9020b8234f..6c17b77928 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -515,6 +515,75 @@ def _ipmitool_timing_args(): ] +def choose_cipher_suite(actual_ciper_suite): + """Gives the possible next avaible cipher suite version. + + Based on CONF.ipmi.cipher_suite_versions and the last cipher suite version + used that failed. This function is only called if the node doesn't have + cipher_suite set. Starts using the last element of the list and decreasing + the index. + + :param actual_ciper_suite: latest cipher suite used in the + ipmi call. + + :returns: the next possible cipher suite or None in case of empty + configuration. + """ + available_cs_versions = CONF.ipmi.cipher_suite_versions + if not available_cs_versions: + return None + + if actual_ciper_suite is None: + return available_cs_versions[-1] + else: + try: + cs_index = available_cs_versions.index(actual_ciper_suite) + except ValueError: + return available_cs_versions[-1] + + return available_cs_versions[max(cs_index - 1, 0)] + + +def check_cipher_suite_errors(cmd_stderr): + """Checks if the command stderr contains cipher suite errors. + + :param cmd_stderr: The command stderr. + + :returns: True if the cmd_stderr contains a cipher suite error, + False otherwise. + """ + cs_errors = ["Unsupported cipher suite ID", + "Error in open session response message :" + " no matching cipher suite"] + for cs_err in cs_errors: + if cmd_stderr is not None and cs_err in cmd_stderr: + return True + return False + + +def update_cipher_suite_cmd(actual_cs, args): + """Updates variables and the cipher suite cmd. + + This function updates the values for all parameters so they + can be used in the next retry of _exec_ipmitool. + + :param actual_cs: a string that represents the cipher suite that was + used in the command. + :param args: a list that contains the ipmitool command that was executed. + + :returns: a tuple with the new values (actual_cs, args) + """ + actual_cs = choose_cipher_suite(actual_cs) + if '-C' in args: + cs_index = args.index('-C') + 1 + args[cs_index] = actual_cs + else: + args.append('-C') + args.append(actual_cs) + + return (actual_cs, args) + + def _exec_ipmitool(driver_info, command, check_exit_code=None, kill_on_timeout=False): """Execute the ipmitool command. @@ -533,6 +602,10 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None, """ args = _get_ipmitool_args(driver_info) + change_cs = (CONF.ipmi.cipher_suite_versions != [] + and driver_info.get('cipher_suite') is None) + actual_cs = None + timeout = CONF.ipmi.command_retry_timeout args.extend(_ipmitool_timing_args()) @@ -570,6 +643,11 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None, out, err = utils.execute(*cmd_args, **extra_args) return out, err except processutils.ProcessExecutionError as e: + if change_cs and check_cipher_suite_errors(e.stderr): + actual_cs, args = update_cipher_suite_cmd( + actual_cs, args) + else: + change_cs = False with excutils.save_and_reraise_exception() as ctxt: err_list = [ x for x in ( diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 7a172ec2d4..1c9e61bf97 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -1458,6 +1458,295 @@ 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(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_cipher_suite_error_noconfig( + self, mock_exec, mock_check_cs, mock_support): + no_matching_error = 'Error in open session response message : '\ + 'no matching cipher suite\n\nError: '\ + 'Unable to establish IPMI v2 / RMCP+ session\n' + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + self.config(cipher_suite_versions=[], group='ipmi') + ipmi.LAST_CMD_TIME = {} + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-f', awesome_password_filename, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + ] + mock_check_cs.return_value = False + + self.assertRaises(processutils.ProcessExecutionError, + ipmi._exec_ipmitool, + self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + calls = [mock.call(*args), mock.call(*args)] + mock_exec.assert_has_calls(calls) + self.assertEqual(2, mock_exec.call_count) + self.assertEqual(0, mock_check_cs.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_cipher_suite_set_with_error_noconfig( + self, mock_exec, mock_check_cs, mock_support): + no_matching_error = 'Error in open session response message : '\ + 'no matching cipher suite\n\nError: '\ + 'Unable to establish IPMI v2 / RMCP+ session\n' + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + self.config(cipher_suite_versions=[], group='ipmi') + ipmi.LAST_CMD_TIME = {} + self.info['cipher_suite'] = '17' + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-C', '17', + '-v', + '-f', awesome_password_filename, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + ] + mock_check_cs.return_value = False + + self.assertRaises(processutils.ProcessExecutionError, + ipmi._exec_ipmitool, + self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + calls = [mock.call(*args), mock.call(*args)] + mock_exec.assert_has_calls(calls) + self.assertEqual(2, mock_exec.call_count) + self.assertEqual(0, mock_check_cs.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_cipher_suite_set_with_error_config( + self, mock_exec, mock_check_cs, mock_support): + no_matching_error = 'Error in open session response message : '\ + 'no matching cipher suite\n\nError: '\ + 'Unable to establish IPMI v2 / RMCP+ session\n' + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + self.config(cipher_suite_versions=[0, 1, 2, 3], group='ipmi') + ipmi.LAST_CMD_TIME = {} + self.info['cipher_suite'] = '17' + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-C', '17', + '-v', + '-f', awesome_password_filename, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + ] + mock_check_cs.return_value = False + + self.assertRaises(processutils.ProcessExecutionError, + ipmi._exec_ipmitool, + self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + calls = [mock.call(*args), mock.call(*args)] + mock_exec.assert_has_calls(calls) + self.assertEqual(2, mock_exec.call_count) + self.assertEqual(0, mock_check_cs.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_try_different_cipher_suite( + self, mock_exec, mock_check_cs, mock_support): + + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=4, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + cs_list = ['0', '1', '2', '3', '17'] + self.config(cipher_suite_versions=cs_list, group='ipmi') + no_matching_error = 'Error in open session response message : '\ + 'no matching cipher suite\n\nError: '\ + 'Unable to establish IPMI v2 / RMCP+ session\n' + unsupported_error = 'Unsupported cipher suite ID : 17\n\n'\ + 'Error: Unable to establish IPMI v2 / RMCP+ session\n' + ipmi.LAST_CMD_TIME = {} + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-f', awesome_password_filename, + 'A', 'B', 'C', + ] + + args_cs17 = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-C', '17', + '-f', awesome_password_filename, + 'A', 'B', 'C', + ] + + args_cs3 = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-C', '3', + '-f', awesome_password_filename, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=unsupported_error), + processutils.ProcessExecutionError(stderr="Unknown"), + ('', ''), + ] + mock_check_cs.side_effect = [True, True, False] + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + execute_calls = [mock.call(*args), mock.call(*args_cs17), + mock.call(*args_cs3), mock.call(*args_cs3)] + mock_exec.assert_has_calls(execute_calls) + self.assertEqual(4, mock_exec.call_count) + check_calls = [mock.call(no_matching_error), + mock.call(unsupported_error), mock.call('Unknown')] + mock_check_cs.assert_has_calls(check_calls) + self.assertEqual(3, mock_check_cs.call_count) + + def test__choose_cipher_suite_empty_list(self): + self.config(cipher_suite_versions=[], group='ipmi') + + value = ipmi.choose_cipher_suite(None) + self.assertIsNone(value) + + nextvalue = ipmi.choose_cipher_suite('3') + self.assertIsNone(nextvalue) + + def test__choose_cipher_suite_one_element(self): + cs_list = ['3'] + self.config(cipher_suite_versions=cs_list, group='ipmi') + + actual_cs = ipmi.choose_cipher_suite(None) + self.assertEqual(actual_cs, cs_list[-1]) + self.assertEqual(actual_cs, cs_list[0]) + self.assertEqual(actual_cs, '3') + + # Call again and ensure it will be the same. + new_cs = ipmi.choose_cipher_suite(actual_cs) + self.assertEqual(new_cs, cs_list[-1]) + self.assertEqual(new_cs, cs_list[0]) + self.assertEqual(new_cs, '3') + + self.assertEqual(new_cs, actual_cs) + + def test__choose_cipher_suite_returns_last_to_first(self): + cs_list = ['0', '1', '2', '3', '17'] + self.config(cipher_suite_versions=cs_list, group='ipmi') + + element_position = len(cs_list) - 1 + actual_cs = ipmi.choose_cipher_suite(None) + self.assertEqual(actual_cs, cs_list[-1]) + self.assertEqual(actual_cs, cs_list[element_position]) + self.assertEqual(actual_cs, '17') + + # iterate call choose_cipher_suite 5 more times + # this ensures the last two call needs to return the first + # element. + for i in range(len(cs_list)): + actual_cs = ipmi.choose_cipher_suite(actual_cs) + if element_position != 0: + element_position -= 1 + self.assertEqual(actual_cs, cs_list[element_position]) + + self.assertEqual(actual_cs, '0') + self.assertEqual(actual_cs, cs_list[0]) + + def test__check_cipher_suite_errors(self): + invalid_errors_stderr = [ + 'Unknow', 'x', '', 'test', + 'Problem\n\nError: Unable to establish IPMI v2 / RMCP+ session\n', + 'UnsupportedciphersuiteID:17\n\n' + ] + no_matching_error = 'Error in open session response message : '\ + 'no matching cipher suite\n\nError: '\ + 'Unable to establish IPMI v2 / RMCP+ session\n' + unsupported_error = 'Unsupported cipher suite ID : 17\n\n'\ + 'Error: Unable to establish IPMI v2 / RMCP+ session\n' + valid_errors_stderr = [no_matching_error, unsupported_error] + for invalid_err in invalid_errors_stderr: + self.assertFalse(ipmi.check_cipher_suite_errors(invalid_err)) + for valid_err in valid_errors_stderr: + self.assertTrue(ipmi.check_cipher_suite_errors(valid_err)) + @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/cipher_suite_versions-c64644860d3c220d.yaml b/releasenotes/notes/cipher_suite_versions-c64644860d3c220d.yaml new file mode 100644 index 0000000000..bf5d9930da --- /dev/null +++ b/releasenotes/notes/cipher_suite_versions-c64644860d3c220d.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Allows providing a list of IPMI cipher suite versions via the new + configuration option ``[ipmi]/cipher_suite_versions``. The configuration + is only used when ``ipmi_cipher_suite`` is not set in ``driver_info``. \ No newline at end of file