diff --git a/doc/source/admin/drivers/irmc.rst b/doc/source/admin/drivers/irmc.rst index 15d245c105..53f5517a67 100644 --- a/doc/source/admin/drivers/irmc.rst +++ b/doc/source/admin/drivers/irmc.rst @@ -111,6 +111,9 @@ Here is a command example to enroll a node with ``irmc`` hardware type. Node configuration ^^^^^^^^^^^^^^^^^^ +Configuration via ``driver_info`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * Each node is configured for ``irmc`` hardware type by setting the following ironic node object's properties: @@ -126,6 +129,44 @@ Node configuration UEFI Secure Boot is required. Please refer to `UEFI Secure Boot Support`_ for more information. +* If ``port`` in ``[irmc]`` section of ``/etc/ironic/ironic.conf`` or + ``driver_info/irmc_port`` is set to 443, ``driver_info/irmc_verify_ca`` + will take effect: + + ``driver_info/irmc_verify_ca`` property takes one of 4 value (default value + is ``True``): + + - ``True``: When set to ``True``, which certification file iRMC driver uses + is determined by ``requests`` Python module. + + Value of ``driver_info/irmc_verify_ca`` is passed to ``verify`` argument + of functions defined in ``requests`` Python module. So which certification + will be used is depend on behavior of ``requests`` module. + (maybe certification provided by ``certifi`` Python module) + + - ``False``: When set to ``False``, iRMC driver won't verify server + certification with certification file during HTTPS connection with iRMC. + Just stop to verify server certification, but does HTTPS. + + .. warning:: + When set to ``False``, user must notice that it can result in + vulnerable situation. Stopping verification of server certification + during HTTPS connection means it cannot prevent Man-in-the-middle + attack. When set to ``False``, Ironic user must take enough care + around infrastructure environment in terms of security. + (e.g. make sure network between Ironic conductor and iRMC is secure) + + - string representing filesystem path to directory which contains + certification file: In this case, iRMC driver uses certification file + stored at specified directory. Ironic conductor must be able to access + that directory. For iRMC to recongnize certification file, Ironic user + must run ``openssl rehash ``. + + - string representing filesystem path to certification file: In this case, + iRMC driver uses certification file specified. Ironic conductor must have + access to that file. + + * The following properties are also required if ``irmc-virtual-media`` boot interface is used: @@ -150,6 +191,9 @@ Node configuration - ``driver_info/irmc_snmp_priv_password`` property to be the privacy protocol pass phrase. The length of pass phrase should be at least 8 characters. +Configuration via ``ironic.conf`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * All of the nodes are configured by setting the following configuration options in the ``[irmc]`` section of ``/etc/ironic/ironic.conf``: @@ -198,6 +242,10 @@ Node configuration ``driver_info/irmc_snmp_user`` parameter for each node if SNMPv3 inspection is needed. + +Override ``ironic.conf`` configuration via ``driver_info`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * Each node can be further configured by setting the following ironic node object's properties which override the parameter values in ``[irmc]`` section of ``/etc/ironic/ironic.conf``: @@ -215,6 +263,7 @@ Node configuration - ``driver_info/irmc_snmp_priv_proto`` property overrides ``snmp_priv_proto``. + Optional functionalities for the ``irmc`` hardware type ======================================================= diff --git a/ironic/drivers/modules/irmc/common.py b/ironic/drivers/modules/irmc/common.py index 00b7c06255..7a8fc0f1d8 100644 --- a/ironic/drivers/modules/irmc/common.py +++ b/ironic/drivers/modules/irmc/common.py @@ -15,8 +15,11 @@ """ Common functionalities shared between different iRMC modules. """ +import os + from oslo_log import log as logging from oslo_utils import importutils +from oslo_utils import strutils from ironic.common import exception from ironic.common.i18n import _ @@ -46,6 +49,16 @@ OPTIONAL_PROPERTIES = { "'ipmitool' or 'scci'. The default value is " "'ipmitool'. Optional."), } +OPTIONAL_DRIVER_INFO_PROPERTIES = { + 'irmc_verify_ca': _('Either a Boolean value, a path to a CA_BUNDLE ' + 'file or directory with certificates of trusted ' + 'CAs. If set to True the driver will verify the ' + 'host certificates; if False the driver will ' + 'ignore verifying the SSL certificate. If it\'s ' + 'a path the driver will use the specified ' + 'certificate or one of the certificates in the ' + 'directory. Defaults to True. Optional'), +} SNMP_PROPERTIES = { 'irmc_snmp_version': _("SNMP protocol version; either 'v1', 'v2c', or " @@ -84,6 +97,7 @@ SNMP_V3_DEPRECATED_PROPERTIES = { COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy() COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES) +COMMON_PROPERTIES.update(OPTIONAL_DRIVER_INFO_PROPERTIES) COMMON_PROPERTIES.update(SNMP_PROPERTIES) COMMON_PROPERTIES.update(SNMP_V3_REQUIRED_PROPERTIES) COMMON_PROPERTIES.update(SNMP_V3_OPTIONAL_PROPERTIES) @@ -116,7 +130,9 @@ def parse_driver_info(node): # corresponding config names don't have 'irmc_' prefix opt = {param: info.get(param, CONF.irmc.get(param[len('irmc_'):])) for param in OPTIONAL_PROPERTIES} - d_info = dict(req, **opt) + opt_driver_info = {param: info.get(param) + for param in OPTIONAL_DRIVER_INFO_PROPERTIES} + d_info = dict(req, **opt, **opt_driver_info) d_info['irmc_port'] = utils.validate_network_port( d_info['irmc_port'], 'irmc_port') @@ -137,6 +153,38 @@ def parse_driver_info(node): error_msgs.append( _("Value '%s' is not supported for 'irmc_sensor_method'.") % d_info['irmc_sensor_method']) + + verify_ca = d_info.get('irmc_verify_ca') + if verify_ca is None: + d_info['irmc_verify_ca'] = verify_ca = CONF.webserver_verify_ca + + # Check if verify_ca is a Boolean or a file/directory in the file-system + if isinstance(verify_ca, str): + if ((os.path.isdir(verify_ca) and os.path.isabs(verify_ca)) + or (os.path.isfile(verify_ca) and os.path.isabs(verify_ca))): + # If it's fullpath and dir/file, we don't need to do anything + pass + else: + try: + d_info['irmc_verify_ca'] = strutils.bool_from_string( + verify_ca, strict=True) + except ValueError: + error_msgs.append( + _('Invalid value type set in driver_info/' + 'irmc_verify_ca on node %(node)s. ' + 'The value should be a Boolean or the path ' + 'to a file/directory, not "%(value)s"' + ) % {'value': verify_ca, 'node': node.uuid}) + elif isinstance(verify_ca, bool): + # If it's a boolean it's grand, we don't need to do anything + pass + else: + error_msgs.append( + _('Invalid value type set in driver_info/irmc_verify_ca ' + 'on node %(node)s. The value should be a Boolean or the path ' + 'to a file/directory, not "%(value)s"') % {'value': verify_ca, + 'node': node.uuid}) + if error_msgs: msg = (_("The following errors were encountered while parsing " "driver_info:\n%s") % "\n".join(error_msgs)) @@ -287,6 +335,7 @@ def get_irmc_client(node): :raises: InvalidParameterValue on invalid inputs. :raises: MissingParameterValue if some mandatory information is missing on the node + :raises: IRMCOperationError if iRMC operation failed """ driver_info = parse_driver_info(node) @@ -296,6 +345,7 @@ def get_irmc_client(node): driver_info['irmc_password'], port=driver_info['irmc_port'], auth_method=driver_info['irmc_auth_method'], + verify=driver_info.get('irmc_verify_ca'), client_timeout=driver_info['irmc_client_timeout']) return scci_client @@ -338,6 +388,7 @@ def get_irmc_report(node): driver_info['irmc_password'], port=driver_info['irmc_port'], auth_method=driver_info['irmc_auth_method'], + verify=driver_info.get('irmc_verify_ca'), client_timeout=driver_info['irmc_client_timeout']) diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 1822b99652..736422ac81 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -63,6 +63,7 @@ PARSED_IFNO = { 'irmc_snmp_port': 161, 'irmc_snmp_version': snmp.SNMP_V2C, 'irmc_sensor_method': 'ipmitool', + 'irmc_verify_ca': True, } diff --git a/ironic/tests/unit/drivers/modules/irmc/test_common.py b/ironic/tests/unit/drivers/modules/irmc/test_common.py index 7598fc16b2..9dbb380baf 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_common.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_common.py @@ -16,6 +16,7 @@ Test class for common methods used by iRMC modules. """ +import os from unittest import mock from oslo_config import cfg @@ -71,6 +72,7 @@ class IRMCValidateParametersTestCase(BaseIRMCTest): self.assertEqual(snmp.SNMP_V2C, info['irmc_snmp_version']) self.assertEqual(161, info['irmc_snmp_port']) self.assertEqual('public', info['irmc_snmp_community']) + self.assertTrue(info['irmc_verify_ca']) def test_parse_driver_info_snmpv3(self): self.node.driver_info['irmc_snmp_version'] = 'v3' @@ -111,6 +113,7 @@ class IRMCValidateParametersTestCase(BaseIRMCTest): self.assertEqual(443, info['irmc_port']) self.assertEqual(60, info['irmc_client_timeout']) self.assertEqual('ipmitool', info['irmc_sensor_method']) + self.assertEqual(True, info['irmc_verify_ca']) def test_parse_driver_info_missing_address(self): del self.node.driver_info['irmc_address'] @@ -274,6 +277,41 @@ class IRMCValidateParametersTestCase(BaseIRMCTest): self.assertRaises(exception.InvalidParameterValue, irmc_common.parse_driver_info, self.node) + @mock.patch.object(os.path, 'isabs', return_value=True, autospec=True) + @mock.patch.object(os.path, 'isdir', return_value=True, autospec=True) + def test_parse_driver_info_dir_path_verify_ca(self, mock_isdir, + mock_isabs): + fake_path = 'absolute/path/to/a/valid/CA' + self.node.driver_info['irmc_verify_ca'] = fake_path + info = irmc_common.parse_driver_info(self.node) + self.assertEqual(fake_path, info['irmc_verify_ca']) + mock_isdir.assert_called_once_with(fake_path) + mock_isabs.assert_called_once_with(fake_path) + + @mock.patch.object(os.path, 'isabs', return_value=True, autospec=True) + @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) + def test_parse_driver_info_file_path_verify_ca(self, mock_isfile, + mock_isabs): + fake_path = 'absolute/path/to/a/valid/ca.pem' + self.node.driver_info['irmc_verify_ca'] = fake_path + info = irmc_common.parse_driver_info(self.node) + self.assertEqual(fake_path, info['irmc_verify_ca']) + mock_isfile.assert_called_once_with(fake_path) + mock_isabs.assert_called_once_with(fake_path) + + def test_parse_driver_info_string_bool_verify_ca(self): + self.node.driver_info['irmc_verify_ca'] = "False" + info = irmc_common.parse_driver_info(self.node) + self.assertFalse(info['irmc_verify_ca']) + + def test_parse_driver_info_invalid_verify_ca(self): + self.node.driver_info['irmc_verify_ca'] = "1234" + self.assertRaises(exception.InvalidParameterValue, + irmc_common.parse_driver_info, self.node) + self.node.driver_info['irmc_verify_ca'] = 1234 + self.assertRaises(exception.InvalidParameterValue, + irmc_common.parse_driver_info, self.node) + class IRMCCommonMethodsTestCase(BaseIRMCTest): @@ -283,6 +321,7 @@ class IRMCCommonMethodsTestCase(BaseIRMCTest): self.info['irmc_port'] = 80 self.info['irmc_auth_method'] = 'digest' self.info['irmc_client_timeout'] = 60 + self.info['irmc_verify_ca'] = True mock_scci.get_client.return_value = 'get_client' returned_mock_scci_get_client = irmc_common.get_irmc_client(self.node) mock_scci.get_client.assert_called_with( @@ -291,6 +330,7 @@ class IRMCCommonMethodsTestCase(BaseIRMCTest): self.info['irmc_password'], port=self.info['irmc_port'], auth_method=self.info['irmc_auth_method'], + verify=self.info['irmc_verify_ca'], client_timeout=self.info['irmc_client_timeout']) self.assertEqual('get_client', returned_mock_scci_get_client) @@ -314,6 +354,7 @@ class IRMCCommonMethodsTestCase(BaseIRMCTest): self.info['irmc_port'] = 80 self.info['irmc_auth_method'] = 'digest' self.info['irmc_client_timeout'] = 60 + self.info['irmc_verify_ca'] = True mock_scci.get_report.return_value = 'get_report' returned_mock_scci_get_report = irmc_common.get_irmc_report(self.node) mock_scci.get_report.assert_called_with( @@ -322,6 +363,7 @@ class IRMCCommonMethodsTestCase(BaseIRMCTest): self.info['irmc_password'], port=self.info['irmc_port'], auth_method=self.info['irmc_auth_method'], + verify=self.info['irmc_verify_ca'], client_timeout=self.info['irmc_client_timeout']) self.assertEqual('get_report', returned_mock_scci_get_report) diff --git a/ironic/tests/unit/drivers/modules/irmc/test_raid.py b/ironic/tests/unit/drivers/modules/irmc/test_raid.py index eefe7ff3a5..54a0a0dd62 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_raid.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_raid.py @@ -702,8 +702,8 @@ class IRMCRaidConfigurationInternalMethodsTestCase(test_common.BaseIRMCTest): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: raid._commit_raid_config(task) - get_raid_adapter_mock.assert_called_once_with( - irmc_common.parse_driver_info(task.node)) + irmc_info = irmc_common.parse_driver_info(task.node) + get_raid_adapter_mock.assert_called_once_with(irmc_info) update_raid_info_mock.assert_called_once_with( task.node, task.node.raid_config) set_async_step_flags_mock.assert_called_once_with( diff --git a/releasenotes/notes/irmc-add-certification-file-option-34e7a0062c768e58.yaml b/releasenotes/notes/irmc-add-certification-file-option-34e7a0062c768e58.yaml new file mode 100644 index 0000000000..14e20864b0 --- /dev/null +++ b/releasenotes/notes/irmc-add-certification-file-option-34e7a0062c768e58.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Adds driver_info/irmc_verify_ca option to specify certification file. + Default value of driver_info/irmc_verify_ca is True. +security: + - | + Modifies the ``irmc`` hardware type to include a capability to control + enforcement of HTTPS certificate verification. By default this is enforced. + python-scciclient >= 0.12.0 is required.