diff --git a/scripts/caclmgrd b/scripts/caclmgrd index 258991e3..7224e1bf 100755 --- a/scripts/caclmgrd +++ b/scripts/caclmgrd @@ -132,6 +132,7 @@ class ControlPlaneAclManager(logger.Logger): VxlanAllowed = False VxlanSrcIP = "" RedfishAllowed = False + DhcpServerSyslogAllowed = False # a map from dpu name to port dashHaPortMap = {} @@ -170,6 +171,7 @@ class ControlPlaneAclManager(logger.Logger): feature_tb = self.config_db_map[DEFAULT_NAMESPACE].get_table(self.FEATURE_TABLE) self.RedfishAllowed = feature_tb.get("redfish", {}).get("state") == "enabled" + self.DhcpServerSyslogAllowed = feature_tb.get("dhcp_server", {}).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': @@ -668,6 +670,10 @@ class ControlPlaneAclManager(logger.Logger): iptables_cmds.append(self.iptables_cmd_ns_prefix[namespace] + ['iptables', '-A', 'INPUT', '-s', '127.0.0.1', '-i', 'lo', '-j', 'ACCEPT']) iptables_cmds.append(self.iptables_cmd_ns_prefix[namespace] + ['ip6tables', '-A', 'INPUT', '-s', '::1', '-i', 'lo', '-j', 'ACCEPT']) + # dhcp_server docker0 syslog (RELP tcp/2514) exception; host namespace / IPv4 only + if namespace == DEFAULT_NAMESPACE and self.DhcpServerSyslogAllowed: + iptables_cmds.append(self.iptables_cmd_ns_prefix[namespace] + ['iptables', '-A', 'INPUT', '-i', 'docker0', '-p', 'tcp', '--dport', '2514', '-j', 'ACCEPT', '-m', 'comment', '--comment', 'dhcp_server_syslog']) + if self.bfdAllowed: iptables_cmds += self.get_bfd_iptable_commands(namespace) @@ -1094,27 +1100,28 @@ class ControlPlaneAclManager(logger.Logger): 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. - """ + def handle_feature_state_events(self, subscribe_feature_table, namespace, ctrl_plane_acl_notification): + """Drain FEATURE-table events and toggle redfish / dhcp_server INPUT handling.""" 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) + if key == "redfish": + if new_state == self.RedfishAllowed: + continue + if new_state: + self.allow_redfish() + else: + self.block_redfish() + ctrl_plane_acl_notification.add(namespace) + elif key == "dhcp_server": + if new_state == self.DhcpServerSyslogAllowed: + continue + self.DhcpServerSyslogAllowed = new_state + self.log_info("{} dhcp_server docker0 syslog control plane ACL handling" + .format("Enabled" if new_state else "Disabled")) + ctrl_plane_acl_notification.add(namespace) def remove_dash_ha_rules(self, namespace, port): iptables_cmds = [] @@ -1329,7 +1336,7 @@ class ControlPlaneAclManager(logger.Logger): if "dash-ha" in self.feature_present: self.update_dash_ha_rules(namespace, key, op, fvs) - self.handle_redfish_feature_events(subscribe_feature_table, namespace, ctrl_plane_acl_notification) + self.handle_feature_state_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_dhcp_server_syslog_test.py b/tests/caclmgrd/caclmgrd_dhcp_server_syslog_test.py new file mode 100644 index 00000000..fbb5326d --- /dev/null +++ b/tests/caclmgrd/caclmgrd_dhcp_server_syslog_test.py @@ -0,0 +1,176 @@ +import os +import sys + +from swsscommon import swsscommon +from sonic_py_common.general import load_module_from_source +from unittest import TestCase, mock +from pyfakefs.fake_filesystem_unittest import patchfs + +from tests.common.mock_configdb import MockConfigDb + + +DBCONFIG_PATH = '/var/run/redis/sonic-db/database_config.json' + +# Must stay byte-identical to the container-side -C check in docker_image_ctl.j2. +DHCP_SYSLOG_RULE = ( + 'iptables', '-A', 'INPUT', '-i', 'docker0', '-p', 'tcp', '--dport', '2514', + '-j', 'ACCEPT', '-m', 'comment', '--comment', 'dhcp_server_syslog', +) + + +class TestCaclmgrdDhcpServerSyslog(TestCase): + """ + Verifies caclmgrd owns the dhcp_server docker0 syslog (RELP tcp/2514) INPUT + exception, gated on FEATURE.dhcp_server, and re-emits it before the + catch-all DROP on every rebuild (sonic-net/sonic-buildimage#27584). + """ + 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) + + def setup_daemon(self, config_db): + MockConfigDb.set_config_db(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.generate_allow_internal_docker_ip_traffic_commands = mock.MagicMock(return_value=[]) + self.caclmgrd.ControlPlaneAclManager.generate_allow_internal_chasis_midplane_traffic = 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='') + return self.caclmgrd.ControlPlaneAclManager("caclmgrd") + + @patchfs + def test_init_seeds_flag_from_feature_state(self, fs): + """DhcpServerSyslogAllowed is seeded from the persisted FEATURE state at __init__.""" + if not os.path.exists(DBCONFIG_PATH): + fs.create_file(DBCONFIG_PATH) + + enabled = self.setup_daemon({"DEVICE_METADATA": {"localhost": {}}, + "FEATURE": {"dhcp_server": {"state": "enabled"}}}) + self.assertTrue(enabled.DhcpServerSyslogAllowed) + + disabled = self.setup_daemon({"DEVICE_METADATA": {"localhost": {}}, + "FEATURE": {"dhcp_server": {"state": "disabled"}}}) + self.assertFalse(disabled.DhcpServerSyslogAllowed) + + # Feature absent entirely (platform without dhcp_server) -> not allowed. + absent = self.setup_daemon({"DEVICE_METADATA": {"localhost": {}}, "FEATURE": {}}) + self.assertFalse(absent.DhcpServerSyslogAllowed) + + @patchfs + def test_rule_emitted_when_feature_enabled(self, fs): + """When enabled, the docker0/2514 ACCEPT rule is present in the host rebuild.""" + if not os.path.exists(DBCONFIG_PATH): + fs.create_file(DBCONFIG_PATH) + + daemon = self.setup_daemon({"DEVICE_METADATA": {"localhost": {}}, + "FEATURE": {"dhcp_server": {"state": "enabled"}}}) + self.assertTrue(daemon.DhcpServerSyslogAllowed) + + cmds, _ = daemon.get_acl_rules_and_translate_to_iptables_commands('', MockConfigDb()) + self.assertIn(DHCP_SYSLOG_RULE, [tuple(c) for c in cmds]) + + @patchfs + def test_rule_absent_when_feature_disabled(self, fs): + """When disabled, the rule is not emitted, so the next rebuild drops it.""" + if not os.path.exists(DBCONFIG_PATH): + fs.create_file(DBCONFIG_PATH) + + daemon = self.setup_daemon({"DEVICE_METADATA": {"localhost": {}}, + "FEATURE": {"dhcp_server": {"state": "disabled"}}}) + self.assertFalse(daemon.DhcpServerSyslogAllowed) + + cmds, _ = daemon.get_acl_rules_and_translate_to_iptables_commands('', MockConfigDb()) + self.assertNotIn(DHCP_SYSLOG_RULE, [tuple(c) for c in cmds]) + + @patchfs + def test_rule_reinserted_before_catch_all_drop(self, fs): + """With a CACL rule present (so the catch-all DROP exists), the exception appears + AND strictly before the DROP -- there is never a rebuild window where a DROP + exists without it.""" + if not os.path.exists(DBCONFIG_PATH): + fs.create_file(DBCONFIG_PATH) + + config_db = { + "ACL_TABLE": { + "SSH_ONLY": {"stage": "INGRESS", "type": "CTRLPLANE", "services": ["SSH"]}, + }, + "ACL_RULE": { + "SSH_ONLY|RULE_1": {"PACKET_ACTION": "ACCEPT", "PRIORITY": "9999", "SRC_IP": "10.0.0.0/8"}, + }, + "DEVICE_METADATA": {"localhost": {}}, + "FEATURE": {"dhcp_server": {"state": "enabled"}}, + } + daemon = self.setup_daemon(config_db) + + cmds, _ = daemon.get_acl_rules_and_translate_to_iptables_commands('', MockConfigDb()) + cmds = [tuple(c) for c in cmds] + catch_all_drop = ('iptables', '-A', 'INPUT', '-j', 'DROP') + self.assertIn(DHCP_SYSLOG_RULE, cmds, "exception must be present in rebuild") + self.assertIn(catch_all_drop, cmds, "test setup should produce a catch-all DROP") + self.assertLess(cmds.index(DHCP_SYSLOG_RULE), cmds.index(catch_all_drop), + "exception must come before the catch-all DROP") + + @patchfs + def test_rule_host_namespace_only(self, fs): + """docker0 is a host bridge / IPv4 only; ASIC namespaces must not emit the rule + even when the flag is set.""" + if not os.path.exists(DBCONFIG_PATH): + fs.create_file(DBCONFIG_PATH) + + daemon = self.setup_daemon({"DEVICE_METADATA": {"localhost": {}}, + "FEATURE": {"dhcp_server": {"state": "enabled"}}}) + self.assertTrue(daemon.DhcpServerSyslogAllowed) + daemon.iptables_cmd_ns_prefix['asic0'] = [] + + cmds, _ = daemon.get_acl_rules_and_translate_to_iptables_commands('asic0', MockConfigDb()) + self.assertNotIn(DHCP_SYSLOG_RULE, [tuple(c) for c in cmds]) + + @patchfs + def test_handle_feature_state_events_dhcp_server(self, fs): + """Drive the FEATURE-table handler for the dhcp_server branch: enable transition, + disable transition, unrelated key ignored, and same-state no-op.""" + if not os.path.exists(DBCONFIG_PATH): + fs.create_file(DBCONFIG_PATH) + + daemon = self.setup_daemon({"DEVICE_METADATA": {"localhost": {}}, + "FEATURE": {"dhcp_server": {"state": "disabled"}}}) + self.assertFalse(daemon.DhcpServerSyslogAllowed) + + 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() + daemon.handle_feature_state_events( + sub_with([("dhcp_server", "SET", (("state", "enabled"),))]), "", notif) + self.assertTrue(daemon.DhcpServerSyslogAllowed) + self.assertIn("", notif) + + # disable: flag flips False and namespace queued + notif = set() + daemon.handle_feature_state_events( + sub_with([("dhcp_server", "SET", (("state", "disabled"),))]), "", notif) + self.assertFalse(daemon.DhcpServerSyslogAllowed) + self.assertIn("", notif) + + # unrelated FEATURE event: dhcp_server flag untouched, nothing queued + notif = set() + daemon.handle_feature_state_events( + sub_with([("bgp", "SET", (("state", "enabled"),))]), "", notif) + self.assertFalse(daemon.DhcpServerSyslogAllowed) + self.assertEqual(notif, set()) + + # same-state event (already disabled): no-op, nothing queued + notif = set() + daemon.handle_feature_state_events( + sub_with([("dhcp_server", "SET", (("state", "disabled"),))]), "", notif) + self.assertFalse(daemon.DhcpServerSyslogAllowed) + self.assertEqual(notif, set()) diff --git a/tests/caclmgrd/caclmgrd_redfish_acl_test.py b/tests/caclmgrd/caclmgrd_redfish_acl_test.py index 43186061..043f6064 100644 --- a/tests/caclmgrd/caclmgrd_redfish_acl_test.py +++ b/tests/caclmgrd/caclmgrd_redfish_acl_test.py @@ -92,7 +92,7 @@ def test_caclmgrd_redfish_acl_skipped_when_feature_disabled(self, test_name, tes @patchfs def test_handle_redfish_feature_events(self, fs): """ - Drive handle_redfish_feature_events (the FEATURE-table subscription + Drive handle_feature_state_events (the FEATURE-table subscription handler) directly, covering every branch: enable transition, disable transition, non-redfish event ignored, and same-state no-op. """ @@ -118,28 +118,28 @@ def sub_with(events): # enable: flag flips True and namespace queued for re-walk notif = set() - caclmgrd_daemon.handle_redfish_feature_events( + caclmgrd_daemon.handle_feature_state_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( + caclmgrd_daemon.handle_feature_state_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( + caclmgrd_daemon.handle_feature_state_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( + caclmgrd_daemon.handle_feature_state_events( sub_with([("redfish", "SET", (("state", "disabled"),))]), "", notif) self.assertFalse(caclmgrd_daemon.RedfishAllowed) self.assertEqual(notif, set())