diff --git a/acl_loader/main.py b/acl_loader/main.py index e0df7eec8b71..6a5311be5136 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -103,6 +103,7 @@ class AclLoader(object): "IP_RSVP": 46, "IP_GRE": 47, "IP_AUTH": 51, + "IP_ICMPV6": 58, "IP_L2TP": 115 } @@ -290,6 +291,14 @@ def is_table_mirror(self, tname): """ return self.tables_db_info[tname]['type'].upper().startswith(self.ACL_TABLE_TYPE_MIRROR) + def is_table_ipv6(self, tname): + """ + Check if ACL table type is IPv6 (L3V6 or MIRRORV6) + :param tname: ACL table name + :return: True if table type is IPv6 else False + """ + return self.tables_db_info[tname]["type"].upper() in ("L3V6", "MIRRORV6") + def is_table_control_plane(self, tname): """ Check if ACL table type is ACL_TABLE_TYPE_CTRLPLANE @@ -409,9 +418,18 @@ def convert_l2(self, table_name, rule_idx, rule): else: try: rule_props["ETHER_TYPE"] = int(rule.l2.config.ethertype) - except: - raise AclLoaderException("Failed to convert ethertype %s table %s rule %s" % ( - rule.l2.config.ethertype, table_name, rule_idx)) + except Exception as e: + raise AclLoaderException( + "Failed to convert ethertype %s; table %s rule %s; exception=%s" % + (rule.l2.config.ethertype, table_name, rule_idx, str(e))) + + if rule.l2.config.vlan_id != "" and rule.l2.config.vlan_id != "null": + vlan_id = rule.l2.config.vlan_id + + if vlan_id <= 0 or vlan_id >= 4096: + raise AclLoaderException("VLAN ID %d is out of bounds (0, 4096)" % (vlan_id)) + + rule_props["VLAN_ID"] = vlan_id return rule_props @@ -422,7 +440,12 @@ def convert_ip(self, table_name, rule_idx, rule): # so there isn't currently a good way to check if the user defined proto=0 or not. if rule.ip.config.protocol: if rule.ip.config.protocol in self.ip_protocol_map: - rule_props["IP_PROTOCOL"] = self.ip_protocol_map[rule.ip.config.protocol] + # Special case: ICMP has different protocol numbers for IPv4 and IPv6, so if we receive + # "IP_ICMP" we need to pick the correct protocol number for the IP version + if rule.ip.config.protocol == "IP_ICMP" and self.is_table_ipv6(table_name): + rule_props["IP_PROTOCOL"] = self.ip_protocol_map["IP_ICMPV6"] + else: + rule_props["IP_PROTOCOL"] = self.ip_protocol_map[rule.ip.config.protocol] else: try: int(rule.ip.config.protocol) @@ -453,6 +476,31 @@ def convert_ip(self, table_name, rule_idx, rule): return rule_props + def convert_icmp(self, table_name, rule_idx, rule): + rule_props = {} + + is_table_v6 = self.is_table_ipv6(table_name) + type_key = "ICMPV6_TYPE" if is_table_v6 else "ICMP_TYPE" + code_key = "ICMPV6_CODE" if is_table_v6 else "ICMP_CODE" + + if rule.icmp.config.type != "" and rule.icmp.config.type != "null": + icmp_type = rule.icmp.config.type + + if icmp_type < 0 or icmp_type > 255: + raise AclLoaderException("ICMP type %d is out of bounds [0, 255]" % (icmp_type)) + + rule_props[type_key] = icmp_type + + if rule.icmp.config.code != "" and rule.icmp.config.code != "null": + icmp_code = rule.icmp.config.code + + if icmp_code < 0 or icmp_code > 255: + raise AclLoaderException("ICMP code %d is out of bounds [0, 255]" % (icmp_code)) + + rule_props[code_key] = icmp_code + + return rule_props + def convert_port(self, port): """ Convert port field format from openconfig ACL to Config DB schema @@ -527,6 +575,7 @@ def convert_rule_to_db_schema(self, table_name, rule): deep_update(rule_props, self.convert_action(table_name, rule_idx, rule)) deep_update(rule_props, self.convert_l2(table_name, rule_idx, rule)) deep_update(rule_props, self.convert_ip(table_name, rule_idx, rule)) + deep_update(rule_props, self.convert_icmp(table_name, rule_idx, rule)) deep_update(rule_props, self.convert_transport(table_name, rule_idx, rule)) deep_update(rule_props, self.convert_input_interface(table_name, rule_idx, rule)) diff --git a/tests/acl_input/acl1.json b/tests/acl_input/acl1.json index 1149070f44ba..adfde60f3a84 100644 --- a/tests/acl_input/acl1.json +++ b/tests/acl_input/acl1.json @@ -141,6 +141,108 @@ "config": { "name": "everflowV6" } + }, + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "3", + "code": "0" + } + } + }, + "2": { + "config": { + "sequence-id": 2 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "vlan-id": "369" + } + }, + "ip": { + "config": { + "protocol": "IP_TCP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + } + } + } + } + }, + "DATAACLV6": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "::1/128", + "destination-ip-address": "::1/128" + } + }, + "icmp": { + "config": { + "type": "1", + "code": "0" + } + } + }, + "2": { + "config": { + "sequence-id": 100 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "::1/128", + "destination-ip-address": "::1/128" + } + }, + "icmp": { + "config": { + "type": "128" + } + } + } + } + } } } } diff --git a/tests/acl_input/illegal_icmp_code_300.json b/tests/acl_input/illegal_icmp_code_300.json new file mode 100644 index 000000000000..f3eff2e443c9 --- /dev/null +++ b/tests/acl_input/illegal_icmp_code_300.json @@ -0,0 +1,37 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "3", + "code": "300" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_icmp_code_nan.json b/tests/acl_input/illegal_icmp_code_nan.json new file mode 100644 index 000000000000..382056d165bd --- /dev/null +++ b/tests/acl_input/illegal_icmp_code_nan.json @@ -0,0 +1,37 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "3", + "code": "foo" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_icmp_code_neg_1.json b/tests/acl_input/illegal_icmp_code_neg_1.json new file mode 100644 index 000000000000..ff9364a7b095 --- /dev/null +++ b/tests/acl_input/illegal_icmp_code_neg_1.json @@ -0,0 +1,37 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "3", + "code": "-1" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_icmp_type_300.json b/tests/acl_input/illegal_icmp_type_300.json new file mode 100644 index 000000000000..c21c91e083f3 --- /dev/null +++ b/tests/acl_input/illegal_icmp_type_300.json @@ -0,0 +1,37 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "300", + "code": "0" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_icmp_type_nan.json b/tests/acl_input/illegal_icmp_type_nan.json new file mode 100644 index 000000000000..d6ca81f9d233 --- /dev/null +++ b/tests/acl_input/illegal_icmp_type_nan.json @@ -0,0 +1,37 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "foo", + "code": "0" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_icmp_type_neg_1.json b/tests/acl_input/illegal_icmp_type_neg_1.json new file mode 100644 index 000000000000..cd8d6ff7027d --- /dev/null +++ b/tests/acl_input/illegal_icmp_type_neg_1.json @@ -0,0 +1,37 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "ip": { + "config": { + "protocol": "IP_ICMP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + }, + "icmp": { + "config": { + "type": "-1", + "code": "0" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_vlan_0.json b/tests/acl_input/illegal_vlan_0.json new file mode 100644 index 000000000000..ae3cd3491069 --- /dev/null +++ b/tests/acl_input/illegal_vlan_0.json @@ -0,0 +1,36 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "vlan-id": "0" + } + }, + "ip": { + "config": { + "protocol": "IP_TCP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_vlan_9000.json b/tests/acl_input/illegal_vlan_9000.json new file mode 100644 index 000000000000..65d56b20d7fc --- /dev/null +++ b/tests/acl_input/illegal_vlan_9000.json @@ -0,0 +1,36 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "vlan-id": "9000" + } + }, + "ip": { + "config": { + "protocol": "IP_TCP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_input/illegal_vlan_nan.json b/tests/acl_input/illegal_vlan_nan.json new file mode 100644 index 000000000000..ba0d24dba319 --- /dev/null +++ b/tests/acl_input/illegal_vlan_nan.json @@ -0,0 +1,36 @@ +{ + "acl": { + "acl-sets": { + "acl-set": { + "DATAACL": { + "acl-entries": { + "acl-entry": { + "1": { + "config": { + "sequence-id": 1 + }, + "actions": { + "config": { + "forwarding-action": "ACCEPT" + } + }, + "l2": { + "config": { + "vlan-id": "nan" + } + }, + "ip": { + "config": { + "protocol": "IP_TCP", + "source-ip-address": "20.0.0.2/32", + "destination-ip-address": "30.0.0.3/32" + } + } + } + } + } + } + } + } + } +} diff --git a/tests/acl_loader_test.py b/tests/acl_loader_test.py index ae98cd957140..9a33819eb2cb 100644 --- a/tests/acl_loader_test.py +++ b/tests/acl_loader_test.py @@ -10,8 +10,9 @@ from acl_loader.main import * class TestAclLoader(object): - def setUp(self): - pass + @pytest.fixture(scope="class") + def acl_loader(self): + yield AclLoader() def test_acl_empty(self): yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/empty_acl.json')) @@ -19,13 +20,13 @@ def test_acl_empty(self): def test_valid(self): yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/acl1.json')) - assert len(yang_acl.acl.acl_sets.acl_set) == 4 + assert len(yang_acl.acl.acl_sets.acl_set) == 6 def test_invalid(self): with pytest.raises(AclLoaderException): yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/acl2.json')) - def test_validate_mirror_action(self): + def test_validate_mirror_action(self, acl_loader): ingress_mirror_rule_props = { "MIRROR_INGRESS_ACTION": "everflow0" } @@ -34,7 +35,6 @@ def test_validate_mirror_action(self): "mirror_egress_action": "everflow0" } - acl_loader = AclLoader() # switch capability taken from mock_tables/state_db.json SWITCH_CAPABILITY table assert acl_loader.validate_actions("EVERFLOW", ingress_mirror_rule_props) assert not acl_loader.validate_actions("EVERFLOW", egress_mirror_rule_props) @@ -53,3 +53,97 @@ def test_validate_mirror_action(self): # switch capability taken from mock_tables/state_db.json SWITCH_CAPABILITY table assert acl_loader.validate_actions("DATAACL", forward_packet_action) assert not acl_loader.validate_actions("DATAACL", drop_packet_action) + + def test_vlan_id_translation(self, acl_loader): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json')) + assert acl_loader.rules_info[("DATAACL", "RULE_2")] + assert acl_loader.rules_info[("DATAACL", "RULE_2")] == { + "VLAN_ID": 369, + "IP_PROTOCOL": 6, + "SRC_IP": "20.0.0.2/32", + "DST_IP": "30.0.0.3/32", + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9998" + } + + def test_vlan_id_lower_bound(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_vlan_0.json')) + + def test_vlan_id_upper_bound(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_vlan_9000.json')) + + def test_vlan_id_not_a_number(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_vlan_nan.json')) + + def test_icmp_translation(self, acl_loader): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json')) + assert acl_loader.rules_info[("DATAACL", "RULE_1")] + assert acl_loader.rules_info[("DATAACL", "RULE_1")] == { + "ICMP_TYPE": 3, + "ICMP_CODE": 0, + "IP_PROTOCOL": 1, + "SRC_IP": "20.0.0.2/32", + "DST_IP": "30.0.0.3/32", + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9999" + } + + def test_icmpv6_translation(self, acl_loader): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json')) + print(acl_loader.rules_info) + assert acl_loader.rules_info[("DATAACLV6", "RULE_1")] == { + "ICMPV6_TYPE": 1, + "ICMPV6_CODE": 0, + "IP_PROTOCOL": 58, + "SRC_IPV6": "::1/128", + "DST_IPV6": "::1/128", + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9999" + } + assert acl_loader.rules_info[("DATAACLV6", "RULE_100")] == { + "ICMPV6_TYPE": 128, + "IP_PROTOCOL": 58, + "SRC_IPV6": "::1/128", + "DST_IPV6": "::1/128", + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9900" + } + + def test_icmp_type_lower_bound(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_icmp_type_neg_1.json')) + + def test_icmp_type_upper_bound(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_icmp_type_300.json')) + + def test_icmp_type_not_a_number(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_icmp_type_nan.json')) + + def test_icmp_code_lower_bound(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_icmp_code_neg_1.json')) + + def test_icmp_code_upper_bound(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_icmp_code_300.json')) + + def test_icmp_code_not_a_number(self, acl_loader): + with pytest.raises(ValueError): + acl_loader.rules_info = {} + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/illegal_icmp_code_nan.json')) diff --git a/tests/aclshow_test.py b/tests/aclshow_test.py index 5003771296d3..a2c122ddd7f3 100644 --- a/tests/aclshow_test.py +++ b/tests/aclshow_test.py @@ -78,7 +78,7 @@ # Expected output for aclshow -r RULE_4,RULE_6 -vv rule4_rule6_verbose_output = '' + \ """Reading ACL info... -Total number of ACL Tables: 5 +Total number of ACL Tables: 6 Total number of ACL Rules: 11 RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 858fe349b224..f9a7459680bc 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -400,6 +400,11 @@ "ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet64,Ethernet68,Ethernet72,Ethernet76,Ethernet80,Ethernet84,Ethernet88,Ethernet92,Ethernet96,Ethernet100,Ethernet104,Ethernet108,Ethernet112,Ethernet116,Ethernet120,Ethernet124", "type": "L3" }, + "ACL_TABLE|DATAACLV6": { + "policy_desc": "DATAACLV6", + "ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet64,Ethernet68,Ethernet72,Ethernet76,Ethernet80,Ethernet84,Ethernet88,Ethernet92,Ethernet96,Ethernet100,Ethernet104,Ethernet108,Ethernet112,Ethernet116,Ethernet120,Ethernet124", + "type": "L3V6" + }, "ACL_TABLE|EVERFLOW": { "policy_desc": "EVERFLOW", "ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet100,Ethernet104,Ethernet92,Ethernet96,Ethernet84,Ethernet88,Ethernet76,Ethernet80,Ethernet108,Ethernet112,Ethernet64,Ethernet120,Ethernet116,Ethernet124,Ethernet72,Ethernet68",