Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[acl-loader] Add support for matching on ICMP and VLAN info #1469

Merged
merged 7 commits into from
Mar 3, 2021
Merged

[acl-loader] Add support for matching on ICMP and VLAN info #1469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fbbfa11
[acl-loader] Add support for matching on ICMP and VLAN info
daall Feb 26, 2021
b87bbba
Enhance input validation and add more test cases
daall Mar 1, 2021
38da2ce
Explicitly check for 0
daall Mar 2, 2021
f7948d4
Make test checks more explicit
daall Mar 2, 2021
aa2ff41
Fix protocol number for ICMPv6 when table type is IPv6
daall Mar 3, 2021
3869af8
Fix aclshow test
daall Mar 3, 2021
bb79143
Clean up ICMP input checks and make comments more clear
daall Mar 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions acl_loader/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ class AclLoader(object):
"IP_RSVP": 46,
"IP_GRE": 47,
"IP_AUTH": 51,
"IP_ICMPV6": 58,
"IP_L2TP": 115
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))

Expand Down
102 changes: 102 additions & 0 deletions tests/acl_input/acl1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
}
}
}
}
}
Expand Down
37 changes: 37 additions & 0 deletions tests/acl_input/illegal_icmp_code_300.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
}
}
}
}
}
37 changes: 37 additions & 0 deletions tests/acl_input/illegal_icmp_code_nan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
}
}
}
}
}
37 changes: 37 additions & 0 deletions tests/acl_input/illegal_icmp_code_neg_1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
}
}
}
}
}
37 changes: 37 additions & 0 deletions tests/acl_input/illegal_icmp_type_300.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
}
}
}
}
}
37 changes: 37 additions & 0 deletions tests/acl_input/illegal_icmp_type_nan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
}
}
}
}
}
Loading