From df265218b2ad9de53d6d81d0e6e3a181376225af Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Fri, 21 Jan 2022 18:48:27 +0100 Subject: [PATCH 1/4] caclmgrd: correct IP2ME address logic Ensure that the destination IP that is permitted is the interface IP only, and not the whole subnet. Signed-off-by: Christian Svensson --- src/sonic-host-services/scripts/caclmgrd | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/sonic-host-services/scripts/caclmgrd b/src/sonic-host-services/scripts/caclmgrd index a65f05a3452..ca3b7b252c7 100755 --- a/src/sonic-host-services/scripts/caclmgrd +++ b/src/sonic-host-services/scripts/caclmgrd @@ -215,18 +215,12 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): for key, _ in iface_table.items(): if not _ip_prefix_in_key(key): continue - iface_name, iface_cidr = key - ip_ntwrk = ipaddress.ip_network(iface_cidr, strict=False) - - # For VLAN interfaces, the IP address we want to block is the default gateway (i.e., - # the first available host IP address of the VLAN subnet) - ip_addr = next(ip_ntwrk.hosts()) if iface_table_name == "VLAN_INTERFACE" else ip_ntwrk.network_address - - if isinstance(ip_ntwrk, ipaddress.IPv4Network): - block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "iptables -A INPUT -d {}/{} -j DROP".format(ip_addr, ip_ntwrk.max_prefixlen)) - elif isinstance(ip_ntwrk, ipaddress.IPv6Network): - block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "ip6tables -A INPUT -d {}/{} -j DROP".format(ip_addr, ip_ntwrk.max_prefixlen)) + ip_iface = ipaddress.ip_interface(iface_cidr) + if isinstance(ip_iface, ipaddress.IPv4Interface): + block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "iptables -A INPUT -d {} -j DROP -m comment --comment 'Block IP2ME on interface {}'".format(ip_iface.ip, iface_name)) + elif isinstance(ip_iface, ipaddress.IPv6Interface): + block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "ip6tables -A INPUT -d {} -j DROP -m comment --comment 'Block IP2ME on interface {}'".format(ip_iface.ip, iface_name)) else: self.log_warning("Unrecognized IP address type on interface '{}': {}".format(iface_name, ip_ntwrk)) From 637d8999f060c21a2a49e4b35694feba1b32a59e Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Sat, 22 Jan 2022 10:17:32 +0100 Subject: [PATCH 2/4] caclmgrd: Exclude mgmt interfaces from block IP2ME Signed-off-by: Christian Svensson --- src/sonic-host-services/scripts/caclmgrd | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sonic-host-services/scripts/caclmgrd b/src/sonic-host-services/scripts/caclmgrd index ca3b7b252c7..e60b618daf9 100755 --- a/src/sonic-host-services/scripts/caclmgrd +++ b/src/sonic-host-services/scripts/caclmgrd @@ -198,9 +198,10 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): return tcp_flags_str def generate_block_ip2me_traffic_iptables_commands(self, namespace): + # We do not block MGMT_INTERFACE IPs by default to allow users + # to manage the box with default configuration. INTERFACE_TABLE_NAME_LIST = [ "LOOPBACK_INTERFACE", - "MGMT_INTERFACE", "VLAN_INTERFACE", "PORTCHANNEL_INTERFACE", "INTERFACE" From 3af2865cd2afefc01f5f591d96d05ba24caa0f9e Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Thu, 27 Jan 2022 22:40:17 +0100 Subject: [PATCH 3/4] caclmgrd: Tests for ip2me block Signed-off-by: Christian Svensson --- .../tests/caclmgrd/caclmgrd_dhcp_test.py | 21 ++-- .../tests/caclmgrd/caclmgrd_ip2me_test.py | 41 ++++++++ .../tests/caclmgrd/test_ip2me_vectors.py | 97 +++++++++++++++++++ .../tests/common/mock_configdb.py | 5 +- 4 files changed, 153 insertions(+), 11 deletions(-) create mode 100644 src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py create mode 100644 src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py diff --git a/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py b/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py index 176dec1b508..b16a7a3482b 100644 --- a/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py +++ b/src/sonic-host-services/tests/caclmgrd/caclmgrd_dhcp_test.py @@ -14,19 +14,20 @@ DBCONFIG_PATH = '/var/run/redis/sonic-db/database_config.json' -swsscommon.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') -caclmgrd = load_module_from_source('caclmgrd', caclmgrd_path) - - class TestCaclmgrdDhcp(TestCase): """ Test caclmgrd dhcp """ + def setUp(self): + + swsscommon.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(CACLMGRD_DHCP_TEST_VECTOR) @patchfs def test_caclmgrd_dhcp(self, test_name, test_data, fs): @@ -46,7 +47,7 @@ def test_caclmgrd_dhcp(self, test_name, test_data, fs): mark = test_data["mark"] - caclmgrd_daemon = caclmgrd.ControlPlaneAclManager("caclmgrd") + caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") mux_update = test_data["mux_update"] for key,data in mux_update: diff --git a/src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py b/src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py new file mode 100644 index 00000000000..f2541aea340 --- /dev/null +++ b/src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py @@ -0,0 +1,41 @@ +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_ip2me_vectors import CACLMGRD_IP2ME_TEST_VECTOR +from tests.common.mock_configdb import MockConfigDb + + +DBCONFIG_PATH = '/var/run/redis/sonic-db/database_config.json' + + +class TestCaclmgrdIP2Me(TestCase): + """ + Test caclmgrd IP2Me + """ + 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(CACLMGRD_IP2ME_TEST_VECTOR) + @patchfs + def test_caclmgrd_ip2me(self, test_name, test_data, fs): + if not os.path.exists(DBCONFIG_PATH): + fs.create_file(DBCONFIG_PATH) # fake database_config.json + + 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() + caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") + ret = caclmgrd_daemon.generate_block_ip2me_traffic_iptables_commands('') + self.assertEqual(ret, test_data["return"]) diff --git a/src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py b/src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py new file mode 100644 index 00000000000..634442c9325 --- /dev/null +++ b/src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py @@ -0,0 +1,97 @@ +from unittest.mock import call + +""" + caclmgrd ip2me block test vector +""" +CACLMGRD_IP2ME_TEST_VECTOR = [ + [ + "Only MGMT interface, no block", + { + "config_db": { + "MGMT_INTERFACE": { + "eth0|172.18.0.100/24": { + "gwaddr": "172.18.0.1" + } + }, + "LOOPBACK_INTERFACE": {}, + "VLAN_INTERFACE": {}, + "PORTCHANNEL_INTERFACE": {}, + "INTERFACE": {}, + "DEVICE_METADATA": { + "localhost": { + } + }, + }, + "return": [ + ], + }, + ], + [ + "One interface of each type, /32 - block all interfaces but MGMT", + { + "config_db": { + "LOOPBACK_INTERFACE": { + "Loopback0|10.10.10.10/32": {}, + }, + "VLAN_INTERFACE": { + "Vlan110|10.10.11.10/32": {}, + }, + "PORTCHANNEL_INTERFACE": { + "PortChannel0001|10.10.12.10/32": {}, + }, + "INTERFACE": { + "Ethernet0|10.10.13.10/32": {} + }, + "MGMT_INTERFACE": { + "eth0|172.18.0.100/24": { + "gwaddr": "172.18.0.1" + } + }, + "DEVICE_METADATA": { + "localhost": { + } + }, + }, + "return": [ + "iptables -A INPUT -d 10.10.10.10 -j DROP -m comment --comment 'Block IP2ME on interface Loopback0'", + "iptables -A INPUT -d 10.10.11.10 -j DROP -m comment --comment 'Block IP2ME on interface Vlan110'", + "iptables -A INPUT -d 10.10.12.10 -j DROP -m comment --comment 'Block IP2ME on interface PortChannel0001'", + "iptables -A INPUT -d 10.10.13.10 -j DROP -m comment --comment 'Block IP2ME on interface Ethernet0'" + ], + }, + ], + [ + "One interface of each type, /25 - block all interfaces but MGMT", + { + "config_db": { + "LOOPBACK_INTERFACE": { + "Loopback0|10.10.10.150/25": {}, + }, + "VLAN_INTERFACE": { + "Vlan110|10.10.11.100/25": {}, + }, + "PORTCHANNEL_INTERFACE": { + "PortChannel0001|10.10.12.100/25": {}, + }, + "INTERFACE": { + "Ethernet0|10.10.13.1/25": {} + }, + "MGMT_INTERFACE": { + "eth0|172.18.0.100/24": { + "gwaddr": "172.18.0.1" + } + }, + "DEVICE_METADATA": { + "localhost": { + } + }, + }, + "return": [ + "iptables -A INPUT -d 10.10.10.150 -j DROP -m comment --comment 'Block IP2ME on interface Loopback0'", + "iptables -A INPUT -d 10.10.11.100 -j DROP -m comment --comment 'Block IP2ME on interface Vlan110'", + "iptables -A INPUT -d 10.10.12.100 -j DROP -m comment --comment 'Block IP2ME on interface PortChannel0001'", + "iptables -A INPUT -d 10.10.13.1 -j DROP -m comment --comment 'Block IP2ME on interface Ethernet0'" + ], + }, + ] +] diff --git a/src/sonic-host-services/tests/common/mock_configdb.py b/src/sonic-host-services/tests/common/mock_configdb.py index f0b12b11abf..1ee279a6b9a 100644 --- a/src/sonic-host-services/tests/common/mock_configdb.py +++ b/src/sonic-host-services/tests/common/mock_configdb.py @@ -43,7 +43,10 @@ def set_entry(self, key, field, data): MockConfigDb.CONFIG_DB[key][field] = data def get_table(self, table_name): - return MockConfigDb.CONFIG_DB[table_name] + data = {} + for k, v in MockConfigDb.CONFIG_DB[table_name].items(): + data[self.deserialize_key(k)] = v + return data def subscribe(self, table_name, callback): self.handlers[table_name] = callback From 53ae2d398270c58bbddef2bb85ac0eabd4a7f5ac Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Wed, 16 Feb 2022 20:21:06 +0100 Subject: [PATCH 4/4] caclmgrd: IPv6 test and re-add first-IP behavior It was decided to keep this current behavior for now. Signed-off-by: Christian Svensson --- src/sonic-host-services/scripts/caclmgrd | 18 +++++- .../tests/caclmgrd/caclmgrd_ip2me_test.py | 3 +- .../tests/caclmgrd/test_ip2me_vectors.py | 62 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/sonic-host-services/scripts/caclmgrd b/src/sonic-host-services/scripts/caclmgrd index e60b618daf9..bee93a12928 100755 --- a/src/sonic-host-services/scripts/caclmgrd +++ b/src/sonic-host-services/scripts/caclmgrd @@ -218,12 +218,28 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): continue iface_name, iface_cidr = key ip_iface = ipaddress.ip_interface(iface_cidr) + ip_ntwrk = ipaddress.ip_network(iface_cidr, strict=False) + ip_first_addr = next(iter(ip_ntwrk.hosts()), None) + if isinstance(ip_iface, ipaddress.IPv4Interface): block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "iptables -A INPUT -d {} -j DROP -m comment --comment 'Block IP2ME on interface {}'".format(ip_iface.ip, iface_name)) elif isinstance(ip_iface, ipaddress.IPv6Interface): block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "ip6tables -A INPUT -d {} -j DROP -m comment --comment 'Block IP2ME on interface {}'".format(ip_iface.ip, iface_name)) else: - self.log_warning("Unrecognized IP address type on interface '{}': {}".format(iface_name, ip_ntwrk)) + self.log_warning("Unrecognized IP address type on interface '{}': {}".format(iface_name, ip_iface)) + + # For more information about this part, see + # https://github.com/Azure/sonic-buildimage/pull/9826 + # For VLAN interfaces, we additionally block the first IP + # in the subnet. + if (iface_table_name == "VLAN_INTERFACE" and + ip_first_addr is not None and ip_first_addr != ip_iface.ip): + if isinstance(ip_ntwrk, ipaddress.IPv4Network): + block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "iptables -A INPUT -d {} -j DROP -m comment --comment 'Block IP2ME (first IP) on interface {}'".format(ip_first_addr, iface_name)) + elif isinstance(ip_ntwrk, ipaddress.IPv6Network): + block_ip2me_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "ip6tables -A INPUT -d {} -j DROP -m comment --comment 'Block IP2ME (first IP) on interface {}'".format(ip_first_addr, iface_name)) + else: + self.log_warning("Unrecognized IP address type on interface '{}': {}".format(iface_name, ip_ntwrk)) return block_ip2me_cmds diff --git a/src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py b/src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py index f2541aea340..698a110d784 100644 --- a/src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py +++ b/src/sonic-host-services/tests/caclmgrd/caclmgrd_ip2me_test.py @@ -26,6 +26,7 @@ def setUp(self): sys.path.insert(0, modules_path) caclmgrd_path = os.path.join(scripts_path, 'caclmgrd') self.caclmgrd = load_module_from_source('caclmgrd', caclmgrd_path) + self.maxDiff = None @parameterized.expand(CACLMGRD_IP2ME_TEST_VECTOR) @patchfs @@ -38,4 +39,4 @@ def test_caclmgrd_ip2me(self, test_name, test_data, fs): self.caclmgrd.ControlPlaneAclManager.get_namespace_mgmt_ipv6 = mock.MagicMock() caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd") ret = caclmgrd_daemon.generate_block_ip2me_traffic_iptables_commands('') - self.assertEqual(ret, test_data["return"]) + self.assertListEqual(test_data["return"], ret) diff --git a/src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py b/src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py index 634442c9325..639e0b1b673 100644 --- a/src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py +++ b/src/sonic-host-services/tests/caclmgrd/test_ip2me_vectors.py @@ -89,9 +89,71 @@ "return": [ "iptables -A INPUT -d 10.10.10.150 -j DROP -m comment --comment 'Block IP2ME on interface Loopback0'", "iptables -A INPUT -d 10.10.11.100 -j DROP -m comment --comment 'Block IP2ME on interface Vlan110'", + "iptables -A INPUT -d 10.10.11.1 -j DROP -m comment --comment 'Block IP2ME (first IP) on interface Vlan110'", "iptables -A INPUT -d 10.10.12.100 -j DROP -m comment --comment 'Block IP2ME on interface PortChannel0001'", "iptables -A INPUT -d 10.10.13.1 -j DROP -m comment --comment 'Block IP2ME on interface Ethernet0'" ], }, + ], + [ + "One VLAN interface, /24, we are .1 -- regression testing to ensure compatibility with old behavior", + { + "config_db": { + "MGMT_INTERFACE": { + "eth0|172.18.0.100/24": { + "gwaddr": "172.18.0.1" + } + }, + "LOOPBACK_INTERFACE": {}, + "VLAN_INTERFACE": { + "Vlan110|10.10.11.1/24": {}, + }, + "PORTCHANNEL_INTERFACE": {}, + "INTERFACE": {}, + "DEVICE_METADATA": { + "localhost": { + } + }, + }, + "return": [ + "iptables -A INPUT -d 10.10.11.1 -j DROP -m comment --comment 'Block IP2ME on interface Vlan110'", + ], + }, + ], + [ + "One interface of each type, IPv6, /64 - block all interfaces but MGMT", + { + "config_db": { + "LOOPBACK_INTERFACE": { + "Loopback0|2001:db8:10::/64": {}, + }, + "VLAN_INTERFACE": { + "Vlan110|2001:db8:11::/64": {}, + }, + "PORTCHANNEL_INTERFACE": { + "PortChannel0001|2001:db8:12::/64": {}, + }, + "INTERFACE": { + "Ethernet0|2001:db8:13::/64": {} + }, + "MGMT_INTERFACE": { + "eth0|2001:db8:200::200/64": { + "gwaddr": "2001:db8:200::100" + } + }, + "DEVICE_METADATA": { + "localhost": { + } + }, + }, + "return": [ + "ip6tables -A INPUT -d 2001:db8:10:: -j DROP -m comment --comment 'Block IP2ME on interface Loopback0'", + "ip6tables -A INPUT -d 2001:db8:11:: -j DROP -m comment --comment 'Block IP2ME on interface Vlan110'", + "ip6tables -A INPUT -d 2001:db8:11::1 -j DROP -m comment --comment 'Block IP2ME (first IP) on interface Vlan110'", + "ip6tables -A INPUT -d 2001:db8:12:: -j DROP -m comment --comment 'Block IP2ME on interface PortChannel0001'", + "ip6tables -A INPUT -d 2001:db8:13:: -j DROP -m comment --comment 'Block IP2ME on interface Ethernet0'" + ], + }, ] + ]