Added CACL entry for Redfish (#380)

shreyansh-nexthop · web-flow · commit fbc6c4158ea6 · 2026-06-07T16:09:39.000-07:00
What: Adds a REDFISH entry (tcp/443) to caclmgrd's ACL_SERVICES and gates it on FEATURE.redfish — init-time seed, walker skip when disabled, and a FEATURE subscriber that flips the flag and triggers a re-walk. Includes unit tests for the walker (v4+v6, enabled+disabled) and the handler branches. Why: Without a REDFISH registration, operator ACL_TABLE entries referencing REDFISH were silently dropped as "unrecognized service", leaving the BMC's tcp/443 (bmcweb in docker-sonic-redfish) ungoverned by CACL. How: Mirrors existing SSH/SNMP/NTP entries in ACL_SERVICES; introduces RedfishAllowed flag analogous to bfdAllowed/VxlanAllowed; subscribes to FEATURE table to converge iptables on redfish enable/disable. On non-BMC platforms a stray REDFISH template is a no-op (warn + ignore). Testing: Azure CI green (build + sonic-net.sonic-host-services), Analyze/DCO/EasyCLA/Semgrep pass. New unit tests cover v4+v6 enabled (rules emitted) and disabled (rules skipped) states plus handler toggles. Companion buildimage PR sonic-net/sonic-buildimage#27349. Signed-off-by: shreyansh-nexthop <shreyansh@nexthop.ai>
diff --git a/scripts/caclmgrd b/scripts/caclmgrd
@@ -108,6 +108,11 @@ class ControlPlaneAclManager(logger.Logger):
             "dst_ports": ["22"],
             "multi_asic_ns_to_host_fwd":True
         },
+        "REDFISH": {
+            "ip_protocols": ["tcp"],
+            "dst_ports": ["443"],
+            "multi_asic_ns_to_host_fwd":False
+        },
         "EXTERNAL_CLIENT": {
             "ip_protocols": ["tcp"],
             "multi_asic_ns_to_host_fwd":False
@@ -126,6 +131,7 @@ class ControlPlaneAclManager(logger.Logger):
     bfdAllowed = False
     VxlanAllowed = False
     VxlanSrcIP = ""
+    RedfishAllowed = False
     # a map from dpu name to port
     dashHaPortMap = {}
 
@@ -162,6 +168,9 @@ class ControlPlaneAclManager(logger.Logger):
         self.feature_present = {}
         self.update_feature_present()
 
+        feature_tb = self.config_db_map[DEFAULT_NAMESPACE].get_table(self.FEATURE_TABLE)
+        self.RedfishAllowed = feature_tb.get("redfish", {}).get("state") == "enabled"
+
         metadata = self.config_db_map[DEFAULT_NAMESPACE].get_table(self.DEVICE_METADATA_TABLE)
         if 'subtype' in metadata['localhost'] and metadata['localhost']['subtype'] == 'DualToR':
             self.DualToR = True
@@ -751,6 +760,11 @@ class ControlPlaneAclManager(logger.Logger):
                                      .format(table_name, acl_service))
                     continue
 
+                if acl_service == "REDFISH" and not self.RedfishAllowed:
+                    self.log_warning("Ignoring control plane ACL '{}': REDFISH service is not enabled on this platform"
+                                     .format(table_name))
+                    continue
+
                 self.log_info("Translating ACL rules for control plane ACL '{}' (service: '{}')"
                               .format(table_name, acl_service))
 
@@ -1057,6 +1071,36 @@ class ControlPlaneAclManager(logger.Logger):
         self.VxlanSrcIP = ""
         return True
 
+    def allow_redfish(self):
+        self.RedfishAllowed = True
+        self.log_info("Enabled REDFISH control plane ACL handling")
+
+    def block_redfish(self):
+        self.RedfishAllowed = False
+        self.log_info("Disabled REDFISH control plane ACL handling")
+
+    def handle_redfish_feature_events(self, subscribe_feature_table, namespace, ctrl_plane_acl_notification):
+        """Drain FEATURE table events and toggle REDFISH control plane ACL handling.
+
+        REDFISH iptables rules are only emitted when FEATURE.redfish is enabled. On a
+        state change, flip RedfishAllowed and request a re-walk so any existing REDFISH
+        ACL rules get (de)programmed immediately.
+        """
+        while True:
+            key, op, fvs = subscribe_feature_table.pop()
+            if not key:
+                break
+            if key != "redfish":
+                continue
+            new_state = (op == 'SET' and dict(fvs).get("state") == "enabled")
+            if new_state == self.RedfishAllowed:
+                continue
+            if new_state:
+                self.allow_redfish()
+            else:
+                self.block_redfish()
+            ctrl_plane_acl_notification.add(namespace)
+
     def remove_dash_ha_rules(self, namespace, port):
         iptables_cmds = []
         # Remove iptables/ip6tables commands that allow DASH-HA packets
@@ -1162,6 +1206,9 @@ class ControlPlaneAclManager(logger.Logger):
 
         subscribe_dpu_table = swsscommon.SubscriberStateTable(config_db_connector, self.DPU_TABLE)
         sel.addSelectable(subscribe_dpu_table)
+
+        subscribe_feature_table = swsscommon.SubscriberStateTable(config_db_connector, self.FEATURE_TABLE)
+        sel.addSelectable(subscribe_feature_table)
         # Map of Namespace <--> susbcriber table's object
         config_db_subscriber_table_map = {}
 
@@ -1248,6 +1295,8 @@ class ControlPlaneAclManager(logger.Logger):
                         self.update_dhcp_acl(key, op, dict(fvs), mark)
                 continue
 
+            ctrl_plane_acl_notification = set()
+
             if db_id == config_db_id:
                 while True:
                     key, op, fvs = subscribe_vxlan_table.pop()
@@ -1265,7 +1314,7 @@ class ControlPlaneAclManager(logger.Logger):
                     if "dash-ha" in self.feature_present:
                         self.update_dash_ha_rules(namespace, key, op, fvs)
 
-            ctrl_plane_acl_notification = set()
+                self.handle_redfish_feature_events(subscribe_feature_table, namespace, ctrl_plane_acl_notification)
 
             # Pop data of both Subscriber Table object of namespace that got config db acl table event
             for table in config_db_subscriber_table_map[namespace]:
diff --git a/tests/caclmgrd/caclmgrd_redfish_acl_test.py b/tests/caclmgrd/caclmgrd_redfish_acl_test.py
@@ -0,0 +1,145 @@
+import os
+import sys
+
+from swsscommon import swsscommon
+from parameterized import parameterized
+from sonic_py_common.general import load_module_from_source
+from unittest import TestCase, mock
+from pyfakefs.fake_filesystem_unittest import patchfs
+
+from .test_redfish_acl_vectors import REDFISH_ACL_TEST_VECTOR, REDFISH_ACL_DISABLED_FEATURE_VECTORS
+from tests.common.mock_configdb import MockConfigDb
+
+
+DBCONFIG_PATH = '/var/run/redis/sonic-db/database_config.json'
+
+
+class TestCaclmgrdRedfishAcl(TestCase):
+    """
+        Verifies that an ACL_TABLE referencing the REDFISH service produces
+        iptables rules on tcp/443. Guards against the ACL_SERVICES["REDFISH"]
+        entry being removed or renamed.
+    """
+    def setUp(self):
+        swsscommon.ConfigDBConnector = MockConfigDb
+        test_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+        modules_path = os.path.dirname(test_path)
+        scripts_path = os.path.join(modules_path, "scripts")
+        sys.path.insert(0, modules_path)
+        caclmgrd_path = os.path.join(scripts_path, 'caclmgrd')
+        self.caclmgrd = load_module_from_source('caclmgrd', caclmgrd_path)
+
+    @parameterized.expand(REDFISH_ACL_TEST_VECTOR)
+    @patchfs
+    def test_caclmgrd_redfish_acl(self, test_name, test_data, fs):
+        if not os.path.exists(DBCONFIG_PATH):
+            fs.create_file(DBCONFIG_PATH)
+
+        MockConfigDb.set_config_db(test_data["config_db"])
+        self.caclmgrd.ControlPlaneAclManager.get_namespace_mgmt_ip = mock.MagicMock()
+        self.caclmgrd.ControlPlaneAclManager.get_namespace_mgmt_ipv6 = mock.MagicMock()
+        self.caclmgrd.ControlPlaneAclManager.generate_block_ip2me_traffic_iptables_commands = mock.MagicMock(return_value=[])
+        self.caclmgrd.ControlPlaneAclManager.get_chain_list = mock.MagicMock(return_value=["INPUT", "FORWARD", "OUTPUT"])
+        self.caclmgrd.ControlPlaneAclManager.get_chassis_midplane_interface_ip = mock.MagicMock(return_value='')
+        caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd")
+
+        self.assertTrue(caclmgrd_daemon.RedfishAllowed,
+                        "Seed at __init__ should set RedfishAllowed=True for these vectors")
+
+        self.assertIn("REDFISH", caclmgrd_daemon.ACL_SERVICES,
+                      "ACL_SERVICES is missing the 'REDFISH' entry — see scripts/caclmgrd")
+        self.assertEqual(caclmgrd_daemon.ACL_SERVICES["REDFISH"]["dst_ports"], ["443"])
+        self.assertEqual(caclmgrd_daemon.ACL_SERVICES["REDFISH"]["ip_protocols"], ["tcp"])
+
+        iptables_rules_ret, _ = caclmgrd_daemon.get_acl_rules_and_translate_to_iptables_commands('', MockConfigDb())
+        test_data['return'] = [tuple(i) for i in test_data['return']]
+        iptables_rules_ret = [tuple(i) for i in iptables_rules_ret]
+        self.assertEqual(set(test_data["return"]).issubset(set(iptables_rules_ret)), True,
+                         "Expected iptables rules not produced. Got: {}".format(iptables_rules_ret))
+
+    @parameterized.expand(REDFISH_ACL_DISABLED_FEATURE_VECTORS)
+    @patchfs
+    def test_caclmgrd_redfish_acl_skipped_when_feature_disabled(self, test_name, test_data, fs):
+        """
+            REDFISH only ships on BMC-equipped platforms. When the redfish FEATURE
+            is "disabled", RedfishAllowed stays False and an operator template
+            referencing the REDFISH service must not program any tcp/443 iptables
+            rules -- otherwise it would silently govern whatever else (if anything)
+            listens on 443. Parametrized over IPv4 and IPv6 to confirm the gate
+            skips REDFISH regardless of IP family.
+        """
+        if not os.path.exists(DBCONFIG_PATH):
+            fs.create_file(DBCONFIG_PATH)
+
+        MockConfigDb.set_config_db(test_data["config_db"])
+        self.caclmgrd.ControlPlaneAclManager.get_namespace_mgmt_ip = mock.MagicMock()
+        self.caclmgrd.ControlPlaneAclManager.get_namespace_mgmt_ipv6 = mock.MagicMock()
+        self.caclmgrd.ControlPlaneAclManager.generate_block_ip2me_traffic_iptables_commands = mock.MagicMock(return_value=[])
+        self.caclmgrd.ControlPlaneAclManager.get_chain_list = mock.MagicMock(return_value=["INPUT", "FORWARD", "OUTPUT"])
+        self.caclmgrd.ControlPlaneAclManager.get_chassis_midplane_interface_ip = mock.MagicMock(return_value='')
+        caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd")
+
+        self.assertFalse(caclmgrd_daemon.RedfishAllowed,
+                         "Seed at __init__ should set RedfishAllowed=False when FEATURE.redfish.state=disabled")
+
+        iptables_rules_ret, _ = caclmgrd_daemon.get_acl_rules_and_translate_to_iptables_commands('', MockConfigDb())
+
+        port_443_rules = [r for r in iptables_rules_ret if "--dport" in r and "443" in r]
+        self.assertEqual(port_443_rules, test_data["return"],
+                         "REDFISH ACL should be skipped when RedfishAllowed=False, "
+                         "but found tcp/443 rules: {}".format(port_443_rules))
+
+    @patchfs
+    def test_handle_redfish_feature_events(self, fs):
+        """
+            Drive handle_redfish_feature_events (the FEATURE-table subscription
+            handler) directly, covering every branch: enable transition, disable
+            transition, non-redfish event ignored, and same-state no-op.
+        """
+        if not os.path.exists(DBCONFIG_PATH):
+            fs.create_file(DBCONFIG_PATH)
+
+        MockConfigDb.set_config_db({
+            "DEVICE_METADATA": {"localhost": {}},
+            "FEATURE": {"redfish": {"state": "disabled"}},
+        })
+        self.caclmgrd.ControlPlaneAclManager.get_namespace_mgmt_ip = mock.MagicMock()
+        self.caclmgrd.ControlPlaneAclManager.get_namespace_mgmt_ipv6 = mock.MagicMock()
+        self.caclmgrd.ControlPlaneAclManager.generate_block_ip2me_traffic_iptables_commands = mock.MagicMock(return_value=[])
+        self.caclmgrd.ControlPlaneAclManager.get_chain_list = mock.MagicMock(return_value=["INPUT", "FORWARD", "OUTPUT"])
+        self.caclmgrd.ControlPlaneAclManager.get_chassis_midplane_interface_ip = mock.MagicMock(return_value='')
+        caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd")
+        self.assertFalse(caclmgrd_daemon.RedfishAllowed)
+
+        def sub_with(events):
+            sub = mock.MagicMock()
+            sub.pop.side_effect = events + [("", None, None)]
+            return sub
+
+        # enable: flag flips True and namespace queued for re-walk
+        notif = set()
+        caclmgrd_daemon.handle_redfish_feature_events(
+            sub_with([("redfish", "SET", (("state", "enabled"),))]), "", notif)
+        self.assertTrue(caclmgrd_daemon.RedfishAllowed)
+        self.assertIn("", notif)
+
+        # disable: flag flips False and namespace queued
+        notif = set()
+        caclmgrd_daemon.handle_redfish_feature_events(
+            sub_with([("redfish", "SET", (("state", "disabled"),))]), "", notif)
+        self.assertFalse(caclmgrd_daemon.RedfishAllowed)
+        self.assertIn("", notif)
+
+        # non-redfish FEATURE event: ignored, nothing queued
+        notif = set()
+        caclmgrd_daemon.handle_redfish_feature_events(
+            sub_with([("snmp", "SET", (("state", "enabled"),))]), "", notif)
+        self.assertFalse(caclmgrd_daemon.RedfishAllowed)
+        self.assertEqual(notif, set())
+
+        # same-state event (already disabled): no-op, nothing queued
+        notif = set()
+        caclmgrd_daemon.handle_redfish_feature_events(
+            sub_with([("redfish", "SET", (("state", "disabled"),))]), "", notif)
+        self.assertFalse(caclmgrd_daemon.RedfishAllowed)
+        self.assertEqual(notif, set())
diff --git a/tests/caclmgrd/test_redfish_acl_vectors.py b/tests/caclmgrd/test_redfish_acl_vectors.py
