From 3f09bdcf952621b75e7a0dfc9e001542b2bcc94f Mon Sep 17 00:00:00 2001
From: Vanou Ishii <ishii.vanou@fujitsu.com>
Date: Sun, 23 Apr 2023 19:57:12 -0400
Subject: [PATCH] [iRMC] Fix parse_driver_info bug enforcing SNMP v3 under FIPS
 mode

This patch fixes a condition where iRMC driver interfaces would have
the FIPS enforcement logic check applied if the SNMP version was not
set to SNMP v3, even if the interfaces did not use SNMP.

With this patch, if FIPS enabled, iRMC driver enforces SNMP
version to be version 3 only when any xxx_interface of iRMC
driver actually uses SNMP.

Story: 2010713
Task: 47879
Change-Id: I774c459a5e11b7cd01f7a65754d5a2c7cc573476
---
 ironic/drivers/modules/irmc/common.py         | 12 ++++++
 .../unit/drivers/modules/irmc/test_common.py  | 42 +++++++++++++++++++
 ...ing-snmpv3-with-fips-e45971d363925ec3.yaml |  6 +++
 3 files changed, 60 insertions(+)
 create mode 100644 releasenotes/notes/fix-irmc-enforcing-snmpv3-with-fips-e45971d363925ec3.yaml

diff --git a/ironic/drivers/modules/irmc/common.py b/ironic/drivers/modules/irmc/common.py
index 4341a82f40..d554a12d35 100644
--- a/ironic/drivers/modules/irmc/common.py
+++ b/ironic/drivers/modules/irmc/common.py
@@ -51,6 +51,12 @@ IPMI_ENABLED_BY_DEFAULT_RANGES = {
 
 ELCM_STATUS_PATH = '/rest/v1/Oem/eLCM/eLCMStatus'
 
+# List of xxx_interface & implementation pair which uses SNMP internally
+# and iRMC driver supports
+INTERFACE_IMPL_LIST_WITH_SNMP = {
+    'inspect_interface': {'irmc', },
+    'power_interface': {'irmc', }}
+
 REQUIRED_PROPERTIES = {
     'irmc_address': _("IP address or hostname of the iRMC. Required."),
     'irmc_username': _("Username for the iRMC with administrator privileges. "
@@ -234,6 +240,12 @@ def _parse_snmp_driver_info(node, info):
                       "v2c": snmp.SNMP_V2C,
                       "v3": snmp.SNMP_V3}
 
+    for int_name, impl_list in INTERFACE_IMPL_LIST_WITH_SNMP.items():
+        if getattr(node, int_name) in impl_list:
+            break
+    else:
+        return snmp_info
+
     if snmp_info['irmc_snmp_version'].lower() not in valid_versions:
         raise exception.InvalidParameterValue(_(
             "Value '%s' is not supported for 'irmc_snmp_version'.") %
diff --git a/ironic/tests/unit/drivers/modules/irmc/test_common.py b/ironic/tests/unit/drivers/modules/irmc/test_common.py
index f125d7bd5a..322b9514b0 100644
--- a/ironic/tests/unit/drivers/modules/irmc/test_common.py
+++ b/ironic/tests/unit/drivers/modules/irmc/test_common.py
@@ -37,6 +37,8 @@ from ironic.tests.unit.objects import utils as obj_utils
 class BaseIRMCTest(db_base.DbTestCase):
 
     boot_interface = 'irmc-pxe'
+    inspect_interface = 'irmc'
+    power_interface = 'irmc'
 
     def setUp(self):
         super(BaseIRMCTest, self).setUp()
@@ -51,6 +53,8 @@ class BaseIRMCTest(db_base.DbTestCase):
             self.context,
             driver='irmc',
             boot_interface=self.boot_interface,
+            inspect_interface=self.inspect_interface,
+            power_interface=self.power_interface,
             driver_info=self.info,
             uuid=uuidutils.generate_uuid())
 
@@ -74,6 +78,44 @@ class IRMCValidateParametersTestCase(BaseIRMCTest):
         self.assertEqual('public', info['irmc_snmp_community'])
         self.assertTrue(info['irmc_verify_ca'])
 
+    @mock.patch.object(utils, 'is_fips_enabled',
+                       return_value=False, autospec=True)
+    def test_parse_snmp_driver_info_with_snmp(self, mock_check_fips):
+        test_list = [{'interfaces': [{'interface': 'inspect_interface',
+                                      'impl': 'irmc'},
+                                     {'interface': 'power_interface',
+                                      'impl': 'irmc'}],
+                      'snmp': True},
+                     {'interfaces': [{'interface': 'inspect_interface',
+                                      'impl': 'inspector'},
+                                     {'interface': 'power_interface',
+                                      'impl': 'irmc'}],
+                      'snmp': True},
+                     {'interfaces': [{'interface': 'inspect_interface',
+                                      'impl': 'irmc'},
+                                     {'interface': 'power_interface',
+                                      'impl': 'ipmitool'}],
+                      'snmp': True},
+                     {'interfaces': [{'interface': 'inspect_interface',
+                                      'impl': 'inspector'},
+                                     {'interface': 'power_interface',
+                                      'impl': 'ipmitool'}],
+                      'snmp': False}
+                     ]
+
+        for t_conf in test_list:
+            with self.subTest(t_conf=t_conf):
+                for int_conf in t_conf['interfaces']:
+                    setattr(self.node, int_conf['interface'], int_conf['impl'])
+                irmc_common.parse_driver_info(self.node)
+
+                if t_conf['snmp']:
+                    mock_check_fips.assert_called()
+                else:
+                    mock_check_fips.assert_not_called()
+
+                mock_check_fips.reset_mock()
+
     def test_parse_driver_info_snmpv3(self):
         self.node.driver_info['irmc_snmp_version'] = 'v3'
         self.node.driver_info['irmc_snmp_user'] = 'admin0'
diff --git a/releasenotes/notes/fix-irmc-enforcing-snmpv3-with-fips-e45971d363925ec3.yaml b/releasenotes/notes/fix-irmc-enforcing-snmpv3-with-fips-e45971d363925ec3.yaml
new file mode 100644
index 0000000000..8b6be9982b
--- /dev/null
+++ b/releasenotes/notes/fix-irmc-enforcing-snmpv3-with-fips-e45971d363925ec3.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes bug of iRMC driver in parse_driver_info where, if FIPS is enabled,
+    SNMP version is always required to be version 3 even though iRMC driver's
+    xxx_interface doesn't use SNMP actually.