From 7b3674441a2a65d69f48f6e4f0d4a188d39a6ac9 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Tue, 10 Apr 2018 18:14:12 -0700 Subject: [PATCH] Allow one Service ACL to bind to multiple services (#1576) * [caclmgrd] Also ignore IP protocol if found in rule; we will only use our predefined protocols --- dockers/docker-snmp-sv2/snmpd-config-updater | 4 +- files/image_config/caclmgrd/caclmgrd | 138 +++++++++--------- files/image_config/ssh/sshd-config-updater | 2 +- src/sonic-config-engine/minigraph.py | 24 ++- .../tests/t0-sample-graph.xml | 10 ++ src/sonic-config-engine/tests/test_cfggen.py | 10 +- src/sonic-utilities | 2 +- 7 files changed, 106 insertions(+), 84 deletions(-) diff --git a/dockers/docker-snmp-sv2/snmpd-config-updater b/dockers/docker-snmp-sv2/snmpd-config-updater index 10c2a2b57ef3..ea837e5a787f 100755 --- a/dockers/docker-snmp-sv2/snmpd-config-updater +++ b/dockers/docker-snmp-sv2/snmpd-config-updater @@ -48,8 +48,8 @@ class ConfigUpdater(object): if table_data["type"] != self.ACL_TABLE_TYPE_CTRLPLANE: continue - # Ignore non-SSH service ACLs - if table_data["service"] != self.ACL_SERVICE_SNMP: + # Ignore non-SNMP service ACLs + if self.ACL_SERVICE_SNMP not in table_data["services"]: continue acl_rules = {} diff --git a/files/image_config/caclmgrd/caclmgrd b/files/image_config/caclmgrd/caclmgrd index a61c2437a5b7..dee85d2e26ed 100755 --- a/files/image_config/caclmgrd/caclmgrd +++ b/files/image_config/caclmgrd/caclmgrd @@ -123,80 +123,76 @@ class ControlPlaneAclManager(object): if table_data["type"] != self.ACL_TABLE_TYPE_CTRLPLANE: continue - acl_service = table_data["service"] + acl_services = table_data["services"] - if acl_service not in self.ACL_SERVICES: - log_warning("Ignoring control plane ACL '{}' with unrecognized service '{}'" - .format(table_name, acl_service)) - continue - - log_info("Translating ACL rules for control plane ACL '{}' (service: '{}')" - .format(table_name, acl_service)) - - # Obtain default IP protocol(s) and destination port(s) for this service - ip_protocols = self.ACL_SERVICES[acl_service]["ip_protocols"] - dst_ports = self.ACL_SERVICES[acl_service]["dst_ports"] - - acl_rules = {} - - for ((rule_table_name, rule_id), rule_props) in self._rules_db_info.iteritems(): - if rule_table_name == table_name: - acl_rules[rule_props["PRIORITY"]] = rule_props - - # For each ACL rule in this table (in descending order of priority) - for priority in sorted(acl_rules.iterkeys(), reverse=True): - rule_props = acl_rules[priority] - - if "PACKET_ACTION" not in rule_props: - log_error("ACL rule does not contain PACKET_ACTION property") + for acl_service in acl_services: + if acl_service not in self.ACL_SERVICES: + log_warning("Ignoring control plane ACL '{}' with unrecognized service '{}'" + .format(table_name, acl_service)) continue - # If the rule contains an IP protocol, we will use it. - # Otherwise, we will apply the rule to the default - # protocol(s) for this ACL service - if "IP_PROTOCOL" in rule_props: - ip_protocols = [rule_props["IP_PROTOCOL"]] - - for ip_protocol in ip_protocols: - for dst_port in dst_ports: - rule_cmd = "iptables -A INPUT -p {}".format(ip_protocol) - - if "SRC_IP" in rule_props and rule_props["SRC_IP"]: - rule_cmd += " -s {}".format(rule_props["SRC_IP"]) - - rule_cmd += " --dport {}".format(dst_port) - - # If there are TCP flags present, append them - if "TCP_FLAGS" in rule_props and rule_props["TCP_FLAGS"]: - tcp_flags = int(rule_props["TCP_FLAGS"], 16) - - if tcp_flags > 0: - rule_cmd += " --tcp-flags " - - if tcp_flags & 0x01: - rule_cmd += "FIN," - if tcp_flags & 0x02: - rule_cmd += "SYN," - if tcp_flags & 0x04: - rule_cmd += "RST," - if tcp_flags & 0x08: - rule_cmd += "PSH," - if tcp_flags & 0x10: - rule_cmd += "ACK," - if tcp_flags & 0x20: - rule_cmd += "URG," - if tcp_flags & 0x40: - rule_cmd += "ECE," - if tcp_flags & 0x80: - rule_cmd += "CWR," - - # Delete the trailing comma - rule_cmd = rule_cmd[:-1] - - # Append the packet action as the jump target - rule_cmd += " -j {}".format(rule_props["PACKET_ACTION"]) - - iptables_cmds.append(rule_cmd) + log_info("Translating ACL rules for control plane ACL '{}' (service: '{}')" + .format(table_name, acl_service)) + + # Obtain default IP protocol(s) and destination port(s) for this service + ip_protocols = self.ACL_SERVICES[acl_service]["ip_protocols"] + dst_ports = self.ACL_SERVICES[acl_service]["dst_ports"] + + acl_rules = {} + + for ((rule_table_name, rule_id), rule_props) in self._rules_db_info.iteritems(): + if rule_table_name == table_name: + acl_rules[rule_props["PRIORITY"]] = rule_props + + # For each ACL rule in this table (in descending order of priority) + for priority in sorted(acl_rules.iterkeys(), reverse=True): + rule_props = acl_rules[priority] + + if "PACKET_ACTION" not in rule_props: + log_error("ACL rule does not contain PACKET_ACTION property") + continue + + # Apply the rule to the default protocol(s) for this ACL service + for ip_protocol in ip_protocols: + for dst_port in dst_ports: + rule_cmd = "iptables -A INPUT -p {}".format(ip_protocol) + + if "SRC_IP" in rule_props and rule_props["SRC_IP"]: + rule_cmd += " -s {}".format(rule_props["SRC_IP"]) + + rule_cmd += " --dport {}".format(dst_port) + + # If there are TCP flags present, append them + if "TCP_FLAGS" in rule_props and rule_props["TCP_FLAGS"]: + tcp_flags = int(rule_props["TCP_FLAGS"], 16) + + if tcp_flags > 0: + rule_cmd += " --tcp-flags " + + if tcp_flags & 0x01: + rule_cmd += "FIN," + if tcp_flags & 0x02: + rule_cmd += "SYN," + if tcp_flags & 0x04: + rule_cmd += "RST," + if tcp_flags & 0x08: + rule_cmd += "PSH," + if tcp_flags & 0x10: + rule_cmd += "ACK," + if tcp_flags & 0x20: + rule_cmd += "URG," + if tcp_flags & 0x40: + rule_cmd += "ECE," + if tcp_flags & 0x80: + rule_cmd += "CWR," + + # Delete the trailing comma + rule_cmd = rule_cmd[:-1] + + # Append the packet action as the jump target + rule_cmd += " -j {}".format(rule_props["PACKET_ACTION"]) + + iptables_cmds.append(rule_cmd) return iptables_cmds diff --git a/files/image_config/ssh/sshd-config-updater b/files/image_config/ssh/sshd-config-updater index ad1ee000febe..5f7a4ce17f98 100755 --- a/files/image_config/ssh/sshd-config-updater +++ b/files/image_config/ssh/sshd-config-updater @@ -49,7 +49,7 @@ class ConfigUpdater(object): continue # Ignore non-SSH service ACLs - if table_data["service"] != self.ACL_SERVICE_SSH: + if self.ACL_SERVICE_SSH not in table_data["services"]: continue acl_rules = {} diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index fb098cee3268..731f449ab028 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -188,6 +188,10 @@ def parse_dpg(dpg, hname): aclattach = aclintf.find(str(QName(ns, "AttachTo"))).text.split(';') acl_intfs = [] is_mirror = False + + # TODO: Ensure that acl_intfs will only ever contain front-panel interfaces (e.g., + # maybe we should explicity ignore management and loopback interfaces?) because we + # decide an ACL is a Control Plane ACL if acl_intfs is empty below. for member in aclattach: member = member.strip() if pcs.has_key(member): @@ -209,12 +213,22 @@ def parse_dpg(dpg, hname): # This ACL has no interfaces to attach to -- consider this a control plane ACL try: aclservice = aclintf.find(str(QName(ns, "Type"))).text - acls[aclname] = {'policy_desc': aclname, - 'ports': acl_intfs, - 'type': 'CTRLPLANE', - 'service': aclservice if aclservice is not None else 'UNKNOWN'} + + # If we already have an ACL with this name and this ACL is bound to a different service, + # append the service to our list of services + if aclname in acls: + if acls[aclname]['type'] != 'CTRLPLANE': + print >> sys.stderr, "Warning: ACL '%s' type mismatch. Not updating ACL." % aclname + elif acls[aclname]['services'] == aclservice: + print >> sys.stderr, "Warning: ACL '%s' already contains service '%s'. Not updating ACL." % (aclname, aclservice) + else: + acls[aclname]['services'].append(aclservice) + else: + acls[aclname] = {'policy_desc': aclname, + 'type': 'CTRLPLANE', + 'services': [aclservice]} except: - print >> sys.stderr, "Warning: Ingore Control Plane ACL %s without type" % aclname + print >> sys.stderr, "Warning: Ignoring Control Plane ACL %s without type" % aclname return intfs, lo_intfs, mgmt_intf, vlans, vlan_members, pcs, acls return None, None, None, None, None, None, None diff --git a/src/sonic-config-engine/tests/t0-sample-graph.xml b/src/sonic-config-engine/tests/t0-sample-graph.xml index a12605c8c3e1..bda5517771e5 100644 --- a/src/sonic-config-engine/tests/t0-sample-graph.xml +++ b/src/sonic-config-engine/tests/t0-sample-graph.xml @@ -281,6 +281,16 @@ SSH_ACL SSH + + SSH + ROUTER-PROTECT + SSH + + + SNMP + ROUTER-PROTECT + SNMP + NTP NTP_ACL diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 64162e49417e..6059798cbd6e 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -78,11 +78,13 @@ def test_render_template(self): def test_minigraph_acl(self): argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v ACL_TABLE' output = self.run_script(argument, True) - self.assertEqual(output.strip(), "Warning: Ingore Control Plane ACL NTP_ACL without type\n" - "{'SSH_ACL': {'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL', 'service': 'SSH', 'ports': []}," - " 'SNMP_ACL': {'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL', 'service': 'SNMP', 'ports': []}," + self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n" + "{'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'}," + " 'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'}," " 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124']}," - " 'NTP_ACL': {'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL', 'service': 'NTP', 'ports': []}}") + " 'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL'}," + " 'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT'}}") + def test_minigraph_everflow(self): argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v MIRROR_SESSION' output = self.run_script(argument) diff --git a/src/sonic-utilities b/src/sonic-utilities index 3f2d7bb4beaf..5e476d6d549c 160000 --- a/src/sonic-utilities +++ b/src/sonic-utilities @@ -1 +1 @@ -Subproject commit 3f2d7bb4beaf3987db0c3e6fb57ae7ba51390a16 +Subproject commit 5e476d6d549cf40b68e86ae01dcb703b567b85e3