acl: rework source/destination

For ACE 'source' and 'destination' is now possible to specify one of:
- ipv4 or ipv6 address
- ipv4 or ipv6 prefix
- name of a prefixlist

The validator resolves the src/dst network list, optionally filtering
this with the desired 'family' (which defaults to 'any'). Errors are
raised if the resulting src/dst network lists do not overlap, that is
to say if all src entries are IPv4 but there are no IPv4 dst entries
and vise-versa.

*  Update the example to have a 'trusted' prefixlist.
*  Update the unit tests to use the new error message(s).

vppcfg/config/acl.py

@@ -40,28 +40,12 @@ def get_by_name(yaml, aclname):
 def hydrate_term(acl_term):
     """Adds all defaults to an ACL term"""
 
-    # Figure out the address family
     if "family" not in acl_term:
-        if "source" in acl_term and ":" in acl_term["source"]:
+        acl_term["family"] = "any"
-            acl_term["family"] = "ipv6"
+    if "source" not in acl_term:
-        elif "destination" in acl_term and ":" in acl_term["destination"]:
+        acl_term["source"] = "any"
-            acl_term["family"] = "ipv6"
+    if "destination" not in acl_term:
-        elif "source" in acl_term and "." in acl_term["source"]:
+        acl_term["destination"] = "any"
-            acl_term["family"] = "ipv4"
-        elif "destination" in acl_term and "." in acl_term["destination"]:
-            acl_term["family"] = "ipv4"
-        else:
-            acl_term["family"] = "any"
-
-    # Set source/destionation based on family, if they're omitted
-    if acl_term["family"] == "ipv4" and "source" not in acl_term:
-        acl_term["source"] = "0.0.0.0/0"
-    if acl_term["family"] == "ipv4" and "destination" not in acl_term:
-        acl_term["destination"] = "0.0.0.0/0"
-    if acl_term["family"] == "ipv6" and "source" not in acl_term:
-        acl_term["source"] = "::/0"
-    if acl_term["family"] == "ipv6" and "destination" not in acl_term:
-        acl_term["destination"] = "::/0"
 
     if "protocol" not in acl_term or acl_term["protocol"] == "any":
         acl_term["protocol"] = 0
@@ -181,6 +165,13 @@ def get_network_list(yaml, network_string, want_ipv4=True, want_ipv6=True):
             ret = [ipn]
         return ret
 
+    if network_string == "any":
+        if want_ipv4:
+            ret.append(ipaddress.ip_network("0.0.0.0/0"))
+        if want_ipv6:
+            ret.append(ipaddress.ip_network("::/0"))
+        return ret
+
     return prefixlist.get_network_list(
         yaml, network_string, want_ipv4=want_ipv4, want_ipv6=want_ipv6
     )
@@ -211,6 +202,16 @@ def get_protocol(protostring):
     return None
 
 
+def network_list_has_family(network_list, version):
+    """Returns True if the given list of ip_network() elements has at least one
+    element with the specified version, which can be either 4 or 6. Return False
+    otherwise"""
+    for m in network_list:
+        if m.version == version:
+            return True
+    return False
+
+
 def validate_acls(yaml):
     """Validate the semantics of all YAML 'acls' entries"""
     result = True
@@ -230,25 +231,53 @@ def validate_acls(yaml):
             logger.debug(
                 f"acl {aclname} term {terms} orig {orig_acl_term} hydrated {acl_term}"
             )
-            if acl_term["family"] == "any":
+            if acl_term["family"] == "ipv4":
-                if "source" in acl_term:
+                want_ipv4 = True
-                    msgs.append(
+                want_ipv6 = False
-                        f"acl {aclname} term {terms} family any cannot have source"
+            elif acl_term["family"] == "ipv6":
-                    )
+                want_ipv4 = False
-                    result = False
+                want_ipv6 = True
-                if "destination" in acl_term:
-                    msgs.append(
-                        f"acl {aclname} term {terms} family any cannot have destination"
-                    )
-                    result = False
             else:
-                src = ipaddress.ip_network(acl_term["source"])
+                want_ipv4 = True
-                dst = ipaddress.ip_network(acl_term["destination"])
+                want_ipv6 = True
-                if src.version != dst.version:
+
-                    msgs.append(
+            src_network_list = get_network_list(
-                        f"acl {aclname} term {terms} source and destination have different address family"
+                yaml, acl_term["source"], want_ipv4=want_ipv4, want_ipv6=want_ipv6
-                    )
+            )
-                    result = False
+            dst_network_list = get_network_list(
+                yaml, acl_term["destination"], want_ipv4=want_ipv4, want_ipv6=want_ipv6
+            )
+            logger.debug(
+                f"acl {aclname} term {terms} src: {src_network_list} dst: {dst_network_list}"
+            )
+            if len(src_network_list) == 0:
+                msgs.append(
+                    f"acl {aclname} term {terms} family {acl_term['family']} has no source"
+                )
+                result = False
+            if len(dst_network_list) == 0:
+                msgs.append(
+                    f"acl {aclname} term {terms} family {acl_term['family']} has no destination"
+                )
+                result = False
+            if len(dst_network_list) == 0 or len(src_network_list) == 0:
+                ## Pointless to continue if there's no src/dst at all
+                continue
+
+            src_network_has_ipv4 = network_list_has_family(src_network_list, 4)
+            dst_network_has_ipv4 = network_list_has_family(dst_network_list, 4)
+            src_network_has_ipv6 = network_list_has_family(src_network_list, 6)
+            dst_network_has_ipv6 = network_list_has_family(dst_network_list, 6)
+
+            if (
+                src_network_has_ipv4 != dst_network_has_ipv4
+                and src_network_has_ipv6 != dst_network_has_ipv6
+            ):
+                msgs.append(
+                    f"acl {aclname} term {terms} source and destination family do not overlap"
+                )
+                result = False
+                continue
 
             proto = get_protocol(acl_term["protocol"])
             if proto is None:
@@ -266,8 +295,7 @@ def validate_acls(yaml):
                         f"acl {aclname} term {terms} destination-port can only be specified for protocol tcp or udp"
                     )
                     result = False
-
+            else:
-            if proto in [6, 17]:
                 src_low_port, src_high_port = get_port_low_high(acl_term["source-port"])
                 dst_low_port, dst_high_port = get_port_low_high(
                     acl_term["destination-port"]
@@ -328,7 +356,7 @@ def validate_acls(yaml):
                         f"acl {aclname} term {terms} icmp-type can only be specified for protocol icmp or icmp-ipv6"
                     )
                     result = False
-            if proto in [1, 58]:
+            else:
                 icmp_code_low, icmp_code_high = get_icmp_low_high(acl_term["icmp-code"])
                 icmp_type_low, icmp_type_high = get_icmp_low_high(acl_term["icmp-type"])
                 if icmp_code_low > icmp_code_high:

vppcfg/config/test_acl.py

@@ -152,3 +152,20 @@ class TestACLMethods(unittest.TestCase):
         l = acl.get_network_list(self.cfg, "pl-notexist")
         self.assertIsInstance(l, list)
         self.assertEquals(0, len(l))
+
+    def test_network_list_has_family(self):
+        l = acl.get_network_list(self.cfg, "trusted")
+        self.assertTrue(acl.network_list_has_family(l, 4))
+        self.assertTrue(acl.network_list_has_family(l, 6))
+
+        l = acl.get_network_list(self.cfg, "trusted", want_ipv4=False)
+        self.assertFalse(acl.network_list_has_family(l, 4))
+        self.assertTrue(acl.network_list_has_family(l, 6))
+
+        l = acl.get_network_list(self.cfg, "trusted", want_ipv6=False)
+        self.assertTrue(acl.network_list_has_family(l, 4))
+        self.assertFalse(acl.network_list_has_family(l, 6))
+
+        l = acl.get_network_list(self.cfg, "trusted", want_ipv4=False, want_ipv6=False)
+        self.assertFalse(acl.network_list_has_family(l, 4))
+        self.assertFalse(acl.network_list_has_family(l, 6))

vppcfg/example.yaml

@@ -126,6 +126,7 @@ prefixlists:
       - 192.0.2.0/24
       - 2001:db8::1
       - 2001:db8::/64
+      - 2001:db8::/48
   empty:
     description: "An empty prefixlist"
     members: []
@@ -134,6 +135,13 @@ acls:
   acl01:
      description: "Test ACL"
      terms:
+       - description: "Allow a prefixlist"
+         action: permit
+         source: trusted
+       - description: "Allow a prefixlist for one family only"
+         family: ipv4
+         action: permit
+         source: trusted
        - description: "Allow a specific IPv6 TCP flow"
          action: permit
          source: 2001:db8::/64

vppcfg/schema.yaml

@@ -5,7 +5,7 @@ bridgedomains: map(include('bridgedomain'),key=str(matches='bd[0-9]+'),required=
 vxlan_tunnels: map(include('vxlan'),key=str(matches='vxlan_tunnel[0-9]+'),required=False)
 taps: map(include('tap'),key=str(matches='tap[0-9]+'),required=False)
 prefixlists: map(include('prefixlist'),key=str(matches='[a-z][a-z0-9\-]+',min=1,max=64),required=False)
-acls: map(include('acl'),key=str(matches='[a-z][a-z0-9\-]+'),required=False)
+acls: map(include('acl'),key=str(matches='[a-z][a-z0-9\-]+',min=1,max=56),required=False)
 ---
 vxlan:
   description: str(exclude='\'"',len=64,required=False)
@@ -96,11 +96,11 @@ acl-term:
   description: str(exclude='\'"',len=64,required=False)
   action: enum('permit','deny','permit+reflect')
   family: enum('ipv4','ipv6','any',required=False)
-  source: any(ip(), ip_interface(),required=False)
+  source: any(ip(),ip_interface(),str(min=1,max=56),required=False)
-  destination: any(ip(), ip_interface(),required=False)
+  destination: any(ip(),ip_interface(),str(min=1,max=56),required=False)
   protocol: any(int(min=1,max=255),regex('^[a-z][a-z0-9-]*$'),required=False)
-  source-port: include('acl-term-port-int-range-symbolic', required=False)
+  source-port: include('acl-term-port-int-range-symbolic',required=False)
-  destination-port: include('acl-term-port-int-range-symbolic', required=False)
+  destination-port: include('acl-term-port-int-range-symbolic',required=False)
   icmp-type: include('acl-term-icmp-int-range',required=False)
   icmp-code: include('acl-term-icmp-int-range',required=False)
 ---

vppcfg/unittest/yaml/correct-acl.yaml

@@ -3,10 +3,23 @@ test:
   errors:
     count: 0
 ---
+prefixlists:
+  trusted:
+    description: "Trusted IPv4 nd IPv6 hosts"
+    members:
+      - 192.0.2.1
+      - 192.0.2.0/24
+      - 2001:db8::1
+      - 2001:db8::/64
+      - 2001:db8::/48
+
 acls:
   acl01:
      description: "Test ACL"
      terms:
+       - description: "Allow a prefixlist"
+         action: permit
+         source: trusted
        - description: "Allow a specific IPv6 TCP flow"
          action: permit
          source: 2001:db8::/64

vppcfg/unittest/yaml/error-acl1.yaml

@@ -1,22 +0,0 @@
-test:
-  description: "Family any precludes source/destination"
-  errors:
-    expected:
-      - "acl .* term .* family any cannot have (source|destination)"
-    count: 4
----
-acls:
-  acl01:
-    terms:
-     - family: any
-       source: 0.0.0.0/0
-       action: permit
-     - family: any
-       source: ::/0
-       action: permit
-     - family: any
-       destination: 0.0.0.0/0
-       action: permit
-     - family: any
-       destination: ::/0
-       action: permit

vppcfg/unittest/yaml/error-acl2.yaml

@@ -2,9 +2,20 @@ test:
   description: "Source and Destination must have the same address family"
   errors:
     expected:
-      - "acl .* term .* source and destination have different address family"
+      - "acl .* term .* source and destination family do not overlap"
-    count: 4
+    count: 6
 ---
+prefixlists:
+  v4only:
+    members:
+     - 192.0.2.1
+     - 192.0.2.0/24
+  v6only:
+    members:
+     - 2001:db8::1
+     - 2001:db8::/64
+     - 2001:db8::/48
+
 acls:
   acl01:
     terms:
@@ -12,6 +23,14 @@ acls:
        source: 0.0.0.0/0
        destination: ::/0
        action: permit
+     - description: "Error, source prefixlist is IPv4 and destination prefixlist is IPv6"
+       source: v4only
+       destination: v6only
+       action: permit
+     - description: "Error, source prefixlist is IPv6 and destination is IPv4"
+       source: v6only
+       destination: 0.0.0.0/0
+       action: permit
      - description: "Error, source is IPv6 and destination is IPv4"
        source: ::/0
        destination: 192.168.0.1
@@ -32,3 +51,13 @@ acls:
        source: 192.168.0.1
        destination: 10.0.0.0/8
        action: permit
+     - description: "OK"
+       source: v4only
+       action: permit
+     - description: "OK"
+       source: v6only
+       action: permit
+     - description: "OK"
+       source: v4only
+       destination: v4only
+       action: permit