Fix iRMC driver to use certification file in HTTPS

This patch modifies iRMC driver to use certification file
when it connects to iRMC via HTTPS

Depends-On: https://review.opendev.org/c/openstack/ironic/+/852250
Change-Id: If69ce1cf2789d9d60fb8e544596cf7d29eab514d
Co-authored-by: Kobayashi Daisuke <kobayashi.da-06@fujitsu.com>
Co-authored-by: Song Shukun <song.shukun@jp.fujitsu.com>
Story: 2009801
Task: 44345

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 <path_to_dir>``.
+
+  - 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
 =======================================================
 

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'])
 
 

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,
 }
 
 

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)
 

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_info = irmc_common.parse_driver_info(task.node)
-                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(

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.