From da7609a68543e9ccf0581d763a2ac373a0978328 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 15 Jan 2023 21:41:58 +0000 Subject: [PATCH 01/46] acls: Syntax schema, example and docs First stab at integrating the acl-plugin from VPP. Allow to craft ACLs consisting of one-or-more ACEs (this is ensured by 'terms' being required with min=1), and a rich language to be able to set any L3 and L4 (UDP, ICMP, TCP) matchers that the plugin provides. Explain how the syntax will look like, although for now only YAMALE syntax checking can be performed (semantic validation is next). TESTED: pim@hippo:~/src/vppcfg/vppcfg$ ./vppcfg.py check -c example.yaml [INFO ] root.main: Loading configfile example.yaml [INFO ] vppcfg.config.valid_config: Configuration validated successfully [INFO ] root.main: Configuration is valid --- docs/config-guide.md | 76 ++++++++++++++++++++++++++++++++++++++++++++ vppcfg/example.yaml | 20 ++++++++++++ vppcfg/schema.yaml | 22 +++++++++++++ 3 files changed, 118 insertions(+) diff --git a/docs/config-guide.md b/docs/config-guide.md index f4838b9..947fad4 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -358,3 +358,79 @@ interfaces: dot1q: 200 exact-match: False ``` + +### Access Control Lists + +In VPP, a common firewall function is provided by the `acl-plugin`. The anatomy of this plugin +is as follows. First, an ACL consists of one or more Access Control Elements or `ACE`s. These +can match on IPv4 or IPv6 source/destination, an IP protocol, and then for TCP/UDP a range +of source- and destination ports, and for ICMP a range of ICMP type and codes. Any matching +packets then either perform an action of `permit` or `deny` (for stateless) or `permit+reflect` +(stateful). The full syntax is as follows: + +* ***description***: A string, no longer than 64 characters, and excluding the single quote ' + and double quote ". This string is currently not used anywhere, and serves for enduser + documentation purposes. +* ***terms***: A list of Access Control Elements: + * ***action***: What to do upon match, can be either `permit`, `deny` or `permit+reflect`. + This is the only required field. + * ***family***: Which IP address family to match, can be either `ipv4`, or `ipv6` or `any`, + which is the default. If `any` is used, this term will also operate on any source and + destination addresses, and it will emit two ACEs, one for each address family. + * ***source***: The IPv4 or IPv6 source prefix, eg. `192.0.2.0/24` or `2001:db8::/64`. If + left empty, this means any (ie. `0.0.0.0/0` or `::/0`). + * ***destination***: Similar to `source`, but for the destination field of the packets. + * ***protocol***: The L4 protocol, can be either a numeric value (eg. `6`), or a symbolic + string value from `/etc/protocols` (eg. `tcp`). If omitted, only L3 matches are performed. + * ***source-port***: When `TCP` or `UDP` are specified, this field specified which source + port(s) are matched. It can be either a numeric value (eg. `80`), a symbolic string value + from `/etc/services` (eg. `www`), a numeric range with start and/or end ranges (eg. `-1024` + for all ports from 0-1024 inclusive; or `1024-` for all ports from 1024-65535 inclusive, + or an actual range `49152-65535`). The default keyword `any` is also permitted, which results + in range `0-65535`, and is the default if the field is not specified. + * ***destination-port***: Similar to `source-port` but for the destination port field in the + `TCP` or `UDP` header. + * ***icmp-type***: It can be either a numeric value (eg. `3`), a numeric range with start + and/or end ranges (eg. `-10` for all types from 0-10 inclusive; or `10-` for all types from + 10-255 inclusive, or an actual range `10-15`). The default keyword `any` is also permitted, + which results in range `0-255`, and is the default if the field is not specified. This field + can only be specified if the `protocol` field is `icmp` (or `1`). + * ***icmp-code***: Similar to `icmp-type` but for the ICMP code field. This field can only be + specified if the `protocol` field is `icmp` (or `1`). + +An example ACL with three ACE terms: +``` +acls: + acl01: + description: "Test ACL" + terms: + - description: "Allow a specific IPv6 TCP flow" + action: permit + source: 2001:db8::/64 + destination: 2001:db8:1::/64 + protocol: tcp + destination-port: www + source-port: "1024-65535" + - description: "Allow IPv4 ICMP Destination Unreachable, any code" + family: ipv4 + action: permit + protocol: icmp + icmp-type: 3 + icmp-code: any + - description: "Deny any IPv4 or IPv6" + action: deny +``` + +One or more of these ACLs are then applied to an interface in either the `input` or the `output` +direction: + +``` +interfaces: + GigabitEthernet3/0/0: + acls: + input: acl01 + output: [ acl02, acl03 ] +``` +The configuration here is tolerant of either a singleton (a literal string referring to the one +ACL that must be applied), or a _list_ of strings to more than one ACL, in which case they will +be tested in order (with a first-match return value). diff --git a/vppcfg/example.yaml b/vppcfg/example.yaml index 85578b8..a7700f5 100644 --- a/vppcfg/example.yaml +++ b/vppcfg/example.yaml @@ -117,3 +117,23 @@ taps: name: vpp-tap101 mtu: 1500 bridge: br1 + +acls: + acl01: + description: "Test ACL" + terms: + - description: "Allow a specific IPv6 TCP flow" + action: permit + source: 2001:db8::/64 + destination: 2001:db8:1::/64 + protocol: tcp + destination-port: www + source-port: "1024-65535" + - description: "Allow IPv4 ICMP Destination Unreachable, any code" + family: ipv4 + action: permit + protocol: icmp + icmp-type: 3 + icmp-code: any + - description: "Deny any IPv4 or IPv6" + action: deny diff --git a/vppcfg/schema.yaml b/vppcfg/schema.yaml index 2baadb6..2c5e739 100644 --- a/vppcfg/schema.yaml +++ b/vppcfg/schema.yaml @@ -4,6 +4,7 @@ loopbacks: map(include('loopback'),key=str(matches='loop[0-9]+'),required=False) bridgedomains: map(include('bridgedomain'),key=str(matches='bd[0-9]+'),required=False) 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) +acls: map(include('acl'),key=str(matches='[a-z][a-z0-9\-]+'),required=False) --- vxlan: description: str(exclude='\'"',len=64,required=False) @@ -79,3 +80,24 @@ tap: namespace-create: bool(required=False) rx-ring-size: int(min=8,max=32768,required=False) tx-ring-size: int(min=8,max=32768,required=False) +--- +# Valid: 80 "www" "-1024" "1024-" "1024-65535", and "any" +acl-term-port-int-range-symbolic: any(int(min=1,max=65535),str(equals="any"),regex('^([1-9][0-9]*-|-[1-9][0-9]*|[1-9][0-9]*-[1-9][0-9]*)$'),regex('^[a-z][a-z0-9-]*$')) +# Valid: 80 "-245" "10-" "10-245", and "any" +acl-term-icmp-int-range: any(int(min=0,max=255),str(equals="any"),regex('^([0-9]+-|-[1-9][0-9]*|[0-9]*-[1-9][0-9]*)$')) +--- +acl-term: + description: str(exclude='\'"',len=64,required=False) + action: enum('permit','deny','permit+reflect') + family: enum('ipv4','ipv6','any',required=False) + source: ip_interface(required=False) + destination: ip_interface(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) + 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) +--- +acl: + description: str(exclude='\'"',len=64,required=False) + terms: list(include('acl-term'), min=1, max=100, required=True) From b08e97107e09220994724fb2593aa7a5e510db7a Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 15 Jan 2023 22:24:13 +0000 Subject: [PATCH 02/46] Add first semantic check + unittest --- vppcfg/config/__init__.py | 2 + vppcfg/config/acl.py | 60 ++++++++++++++++++++++++++++ vppcfg/unittest/yaml/error-acl1.yaml | 22 ++++++++++ 3 files changed, 84 insertions(+) create mode 100644 vppcfg/config/acl.py create mode 100644 vppcfg/unittest/yaml/error-acl1.yaml diff --git a/vppcfg/config/__init__.py b/vppcfg/config/__init__.py index e62dd98..0bd0cbc 100644 --- a/vppcfg/config/__init__.py +++ b/vppcfg/config/__init__.py @@ -38,6 +38,7 @@ from .interface import validate_interfaces from .bridgedomain import validate_bridgedomains from .vxlan_tunnel import validate_vxlan_tunnels from .tap import validate_taps +from .acl import validate_acls class IPInterfaceWithPrefixLength(validators.Validator): @@ -89,6 +90,7 @@ class Validator: validate_bridgedomains, validate_vxlan_tunnels, validate_taps, + validate_acls, ] def validate(self, yaml): diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py new file mode 100644 index 0000000..c98f879 --- /dev/null +++ b/vppcfg/config/acl.py @@ -0,0 +1,60 @@ +# +# Copyright (c) 2023 Pim van Pelt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at: +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" A vppcfg configuration module that validates acls """ +import logging + + +def get_aclx(yaml): + """Return a list of all acls.""" + ret = [] + if "acls" in yaml: + for aclname, _acl in yaml["acls"].items(): + ret.append(aclname) + return ret + + +def get_by_name(yaml, aclname): + """Return the acl by name, if it exists. Return None otherwise.""" + try: + if aclname in yaml["acls"]: + return aclname, yaml["acls"][aclname] + except KeyError: + pass + return None, None + + +def validate_acls(yaml): + """Validate the semantics of all YAML 'acls' entries""" + result = True + msgs = [] + logger = logging.getLogger("vppcfg.config") + logger.addHandler(logging.NullHandler()) + + if not "acls" in yaml: + return result, msgs + + for aclname, acl in yaml["acls"].items(): + logger.debug(f"acl {acl}") + terms = 0 + for acl_term in acl["terms"]: + terms += 1 + if "family" in acl_term and "any" in acl_term["family"]: + if "source" in acl_term: + msgs.append(f"acl term {terms} family any cannot have source") + result = False + if "destination" in acl_term: + msgs.append(f"acl term {terms} family any cannot have destination") + result = False + + return result, msgs diff --git a/vppcfg/unittest/yaml/error-acl1.yaml b/vppcfg/unittest/yaml/error-acl1.yaml new file mode 100644 index 0000000..1812395 --- /dev/null +++ b/vppcfg/unittest/yaml/error-acl1.yaml @@ -0,0 +1,22 @@ +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 From 6990fb691dc8fc3bdc73720bdc74243540cc5511 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 00:16:17 +0000 Subject: [PATCH 03/46] Allow src/dst to also be an IP address --- vppcfg/schema.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vppcfg/schema.yaml b/vppcfg/schema.yaml index 2c5e739..2bffd49 100644 --- a/vppcfg/schema.yaml +++ b/vppcfg/schema.yaml @@ -90,8 +90,8 @@ acl-term: description: str(exclude='\'"',len=64,required=False) action: enum('permit','deny','permit+reflect') family: enum('ipv4','ipv6','any',required=False) - source: ip_interface(required=False) - destination: ip_interface(required=False) + source: any(ip(), ip_interface(),required=False) + destination: any(ip(), ip_interface(),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) destination-port: include('acl-term-port-int-range-symbolic', required=False) From 56ffe52e20fa01fd5f9cb4c3d6d345b63035f6e7 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 01:09:23 +0000 Subject: [PATCH 04/46] acl: semantic validation --- vppcfg/config/acl.py | 242 +++++++++++++++++++++++++- vppcfg/config/test_acl.py | 95 ++++++++++ vppcfg/unittest/yaml/correct-acl.yaml | 37 ++++ vppcfg/unittest/yaml/error-acl2.yaml | 34 ++++ vppcfg/unittest/yaml/error-acl3.yaml | 42 +++++ vppcfg/unittest/yaml/error-acl4.yaml | 13 ++ vppcfg/unittest/yaml/error-acl5.yaml | 42 +++++ 7 files changed, 503 insertions(+), 2 deletions(-) create mode 100644 vppcfg/config/test_acl.py create mode 100644 vppcfg/unittest/yaml/correct-acl.yaml create mode 100644 vppcfg/unittest/yaml/error-acl2.yaml create mode 100644 vppcfg/unittest/yaml/error-acl3.yaml create mode 100644 vppcfg/unittest/yaml/error-acl4.yaml create mode 100644 vppcfg/unittest/yaml/error-acl5.yaml diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index c98f879..d395cb8 100644 --- a/vppcfg/config/acl.py +++ b/vppcfg/config/acl.py @@ -13,6 +13,8 @@ # """ A vppcfg configuration module that validates acls """ import logging +import socket +import ipaddress def get_aclx(yaml): @@ -34,6 +36,146 @@ def get_by_name(yaml, aclname): return None, None +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"] = "ipv6" + elif "destination" in acl_term and ":" in acl_term["destination"]: + acl_term["family"] = "ipv6" + elif "source" in acl_term and "." in acl_term["source"]: + 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 + + if "source-port" not in acl_term: + acl_term["source-port"] = "any" + if "destination-port" not in acl_term: + acl_term["destination-port"] = "any" + if "icmp-code" not in acl_term: + acl_term["icmp-code"] = "any" + if "icmp-type" not in acl_term: + acl_term["icmp-type"] = "any" + + return acl_term + + +def get_icmp_low_high(icmpstring): + """For a given icmp string, which can be either an integer or a range of + integers including start/stop being omitted, eg 0-255, 10- or -10, or the + string "any", return a tuple of (lowport, highport) or (None, None) upon + error""" + if isinstance(icmpstring, int): + return int(icmpstring), int(icmpstring) + if "any" == icmpstring: + return 0, 255 + + try: + icmp = int(icmpstring) + if icmp > 0: + return icmp, icmp + except: + pass + + if icmpstring.startswith("-"): + icmp = int(icmpstring[1:]) + return 0, icmp + + if icmpstring.endswith("-"): + icmp = int(icmpstring[:-1]) + return icmp, 255 + + try: + icmps = icmpstring.split("-") + return int(icmps[0]), int(icmps[1]) + except: + pass + + return None, None + + +def get_port_low_high(portstring): + """For a given port string, which can be either an integer, a symbolic port name + in /etc/services, a range of integers including start/stop being omitted, eg + 0-65535, 1024- or -1024, or the string "any", return a tuple of + (lowport, highport) or (None, None) upon error""" + if isinstance(portstring, int): + return int(portstring), int(portstring) + if "any" == portstring: + return 0, 65535 + + try: + port = int(portstring) + if port > 0: + return port, port + except: + pass + + try: + port = socket.getservbyname(portstring) + return port, port + except: + pass + + if portstring.startswith("-"): + port = int(portstring[1:]) + return 0, port + + if portstring.endswith("-"): + port = int(portstring[:-1]) + return port, 65535 + + try: + ports = portstring.split("-") + return int(ports[0]), int(ports[1]) + except: + pass + + return None, None + + +def get_protocol(protostring): + """For a given protocol string, which can be either an integer or a symbolic port + name in /etc/protocols, return the protocol number as integer, or None if it cannot + be determined.""" + if isinstance(protostring, int): + return int(protostring) + if "any" == protostring: + return 0 + + try: + proto = int(protostring) + if proto > 0: + return proto + except: + pass + + try: + proto = socket.getprotobyname(protostring) + return proto + except: + pass + + return None + + def validate_acls(yaml): """Validate the semantics of all YAML 'acls' entries""" result = True @@ -45,16 +187,112 @@ def validate_acls(yaml): return result, msgs for aclname, acl in yaml["acls"].items(): - logger.debug(f"acl {acl}") terms = 0 for acl_term in acl["terms"]: terms += 1 - if "family" in acl_term and "any" in acl_term["family"]: + orig_acl_term = acl_term.copy() + acl_term = hydrate_term(acl_term) + logger.debug(f"acl term {terms} orig {orig_acl_term} hydrated {acl_term}") + if acl_term["family"] == "any": if "source" in acl_term: msgs.append(f"acl term {terms} family any cannot have source") result = False if "destination" in acl_term: msgs.append(f"acl term {terms} family any cannot have destination") result = False + else: + src = ipaddress.ip_network(acl_term["source"]) + dst = ipaddress.ip_network(acl_term["destination"]) + if src.version != dst.version: + msgs.append( + f"acl term {terms} source and destination have different address family" + ) + result = False + + proto = get_protocol(acl_term["protocol"]) + if proto is None: + msgs.append(f"acl term {terms} could not understand protocol") + result = False + + if not proto in [6, 17]: + if "source-port" in orig_acl_term: + msgs.append( + f"acl term {terms} source-port can only be specified for protocol tcp or udp" + ) + result = False + if "destination-port" in orig_acl_term: + msgs.append( + f"acl term {terms} destination-port can only be specified for protocol tcp or udp" + ) + result = False + + 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"] + ) + + if src_low_port is None or src_high_port is None: + msgs.append(f"acl term {terms} could not understand source port") + result = False + else: + if src_low_port > src_high_port: + msgs.append( + f"acl term {terms} source low port is higher than source high port" + ) + result = False + if src_low_port < 0 or src_low_port > 65535: + msgs.append( + f"acl term {terms} source low port is not between [0,65535]" + ) + result = False + if src_high_port < 0 or src_high_port > 65535: + msgs.append( + f"acl term {terms} source high port is not between [0,65535]" + ) + result = False + + if dst_low_port is None or dst_high_port is None: + msgs.append( + f"acl term {terms} could not understand destination port" + ) + result = False + else: + if dst_low_port > dst_high_port: + msgs.append( + f"acl term {terms} destination low port is higher than destination high port" + ) + result = False + if dst_low_port < 0 or dst_low_port > 65535: + msgs.append( + f"acl term {terms} destination low port is not between [0,65535]" + ) + result = False + if dst_high_port < 0 or dst_high_port > 65535: + msgs.append( + f"acl term {terms} destination high port is not between [0,65535]" + ) + result = False + + if not proto in [1, 58]: + if "icmp-code" in orig_acl_term: + msgs.append( + f"acl term {terms} icmp-code can only be specified for protocol icmp or icmp-ipv6" + ) + result = False + if "icmp-type" in orig_acl_term: + msgs.append( + f"acl term {terms} icmp-type can only be specified for protocol icmp or icmp-ipv6" + ) + result = False + if proto in [1, 58]: + 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: + msgs.append(f"acl term {terms} icmp-code low value is higher than high value") + result = False + if icmp_type_low > icmp_type_high: + msgs.append(f"acl term {terms} icmp-type low value is higher than high value") + result = False return result, msgs diff --git a/vppcfg/config/test_acl.py b/vppcfg/config/test_acl.py new file mode 100644 index 0000000..e02ffa8 --- /dev/null +++ b/vppcfg/config/test_acl.py @@ -0,0 +1,95 @@ +# +# Copyright (c) 2022 Pim van Pelt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at: +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# -*- coding: utf-8 -*- +""" Unit tests for taps """ +import unittest +from . import acl + + +class TestACLMethods(unittest.TestCase): + def test_get_port_low_high(self): + lo, hi = acl.get_port_low_high(80) + self.assertEqual(80, lo) + self.assertEqual(80, hi) + + lo, hi = acl.get_port_low_high("80") + self.assertEqual(80, lo) + self.assertEqual(80, hi) + + lo, hi = acl.get_port_low_high("www") + self.assertEqual(80, lo) + self.assertEqual(80, hi) + + lo, hi = acl.get_port_low_high("any") + self.assertEqual(0, lo) + self.assertEqual(65535, hi) + + lo, hi = acl.get_port_low_high("-1024") + self.assertEqual(0, lo) + self.assertEqual(1024, hi) + + lo, hi = acl.get_port_low_high("1024-") + self.assertEqual(1024, lo) + self.assertEqual(65535, hi) + + lo, hi = acl.get_port_low_high("1000-2000") + self.assertEqual(1000, lo) + self.assertEqual(2000, hi) + + lo, hi = acl.get_port_low_high("0-65535") + self.assertEqual(0, lo) + self.assertEqual(65535, hi) + + lo, hi = acl.get_port_low_high("bla") + self.assertIsNone(lo) + self.assertIsNone(hi) + + lo, hi = acl.get_port_low_high("foo-bar") + self.assertIsNone(lo) + self.assertIsNone(hi) + + def test_get_protocol(self): + proto = acl.get_protocol(1) + self.assertEqual(1, proto) + + proto = acl.get_protocol("icmp") + self.assertEqual(1, proto) + + proto = acl.get_protocol("unknown") + self.assertIsNone(proto) + + def test_get_icmp_low_high(self): + lo, hi = acl.get_icmp_low_high(3) + self.assertEqual(3, lo) + self.assertEqual(3, hi) + + lo, hi = acl.get_icmp_low_high("3") + self.assertEqual(3, lo) + self.assertEqual(3, hi) + + lo, hi = acl.get_icmp_low_high("any") + self.assertEqual(0, lo) + self.assertEqual(255, hi) + + lo, hi = acl.get_icmp_low_high("10-") + self.assertEqual(10, lo) + self.assertEqual(255, hi) + + lo, hi = acl.get_icmp_low_high("-10") + self.assertEqual(0, lo) + self.assertEqual(10, hi) + + lo, hi = acl.get_icmp_low_high("10-20") + self.assertEqual(10, lo) + self.assertEqual(20, hi) diff --git a/vppcfg/unittest/yaml/correct-acl.yaml b/vppcfg/unittest/yaml/correct-acl.yaml new file mode 100644 index 0000000..58e0611 --- /dev/null +++ b/vppcfg/unittest/yaml/correct-acl.yaml @@ -0,0 +1,37 @@ +test: + description: "A bunch of ACLs that are wellformed" + errors: + count: 0 +--- +acls: + acl01: + description: "Test ACL" + terms: + - description: "Allow a specific IPv6 TCP flow" + action: permit + source: 2001:db8::/64 + destination: 2001:db8:1::/64 + protocol: tcp + destination-port: www + source-port: "1024-65535" + - description: "Allow IPv4 ICMP Destination Unreachable, any code" + family: ipv4 + action: permit + protocol: icmp + icmp-type: 3 + icmp-code: any + - description: "Using an IPv4 address is OK" + action: permit + source: 192.168.0.1 + - description: "Using an IPv6 address is OK" + action: permit + destination: 2001:db8::1 + - description: "Protocol using number" + action: permit + protocol: 1 + - description: "Protocol using symbolic name" + action: permit + protocol: icmp + - description: "Deny any IPv4 or IPv6" + protocol: any + action: deny diff --git a/vppcfg/unittest/yaml/error-acl2.yaml b/vppcfg/unittest/yaml/error-acl2.yaml new file mode 100644 index 0000000..f088f63 --- /dev/null +++ b/vppcfg/unittest/yaml/error-acl2.yaml @@ -0,0 +1,34 @@ +test: + description: "Source and Destination must have the same address family" + errors: + expected: + - "acl term .* source and destination have different address family" + count: 4 +--- +acls: + acl01: + terms: + - description: "Error, source is IPv4 and destination is IPv6" + source: 0.0.0.0/0 + destination: ::/0 + action: permit + - description: "Error, source is IPv6 and destination is IPv4" + source: ::/0 + destination: 192.168.0.1 + action: permit + - description: "Error, source is IPv4 and destination is IPv6" + source: 0.0.0.0/0 + destination: 2001:db8::1 + action: permit + - description: "Error, source is IPv6 and destination is IPv4" + source: ::/0 + destination: 192.168.0.0/16 + action: permit + - description: "OK" + source: ::/0 + destination: 2001:db8::1 + action: permit + - description: "OK" + source: 192.168.0.1 + destination: 10.0.0.0/8 + action: permit diff --git a/vppcfg/unittest/yaml/error-acl3.yaml b/vppcfg/unittest/yaml/error-acl3.yaml new file mode 100644 index 0000000..cc1961b --- /dev/null +++ b/vppcfg/unittest/yaml/error-acl3.yaml @@ -0,0 +1,42 @@ +test: + description: "Ways in which port ranges can fail" + errors: + expected: + - "acl term .* could not understand source port" + - "acl term .* could not understand destination port" + - "acl term .* source low port is higher than source high port" + - "acl term .* source (high|low) port is not between \\[0,65535\\]" + - "acl term .* destination (high|low) port is not between \\[0,65535\\]" + - "acl term .* source-port can only be specified for protocol tcp or udp" + - "acl term .* destination-port can only be specified for protocol tcp or udp" + count: 7 +--- +acls: + acl01: + terms: + - description: "Port is not known in /etc/services" + action: permit + protocol: tcp + source-port: "unknown" + - description: "Port is not known in /etc/services" + action: permit + destination-port: "unknown-range" + protocol: tcp + - description: "Low port is higher than High port" + action: permit + source-port: "20-10" + protocol: udp + - description: "High port is > 65535" + action: permit + source-port: "10-65536" + protocol: udp + - description: "High port is > 65535" + action: permit + protocol: tcp + destination-port: "10-65536" + - description: "ports are not allowed if protocol is not TCP or UDP" + action: permit + source-port: 80 + - description: "ports are not allowed if protocol is not TCP or UDP" + action: permit + destination-port: 80-1024 diff --git a/vppcfg/unittest/yaml/error-acl4.yaml b/vppcfg/unittest/yaml/error-acl4.yaml new file mode 100644 index 0000000..dc32e40 --- /dev/null +++ b/vppcfg/unittest/yaml/error-acl4.yaml @@ -0,0 +1,13 @@ +test: + description: "Ways in which ACE protocol can fail" + errors: + expected: + - "acl term .* could not understand protocol" + count: 1 +--- +acls: + acl01: + terms: + - description: "Protocol is not known in /etc/protocols" + action: permit + protocol: "unknown" diff --git a/vppcfg/unittest/yaml/error-acl5.yaml b/vppcfg/unittest/yaml/error-acl5.yaml new file mode 100644 index 0000000..952fa87 --- /dev/null +++ b/vppcfg/unittest/yaml/error-acl5.yaml @@ -0,0 +1,42 @@ +test: + description: "Ways in which ICMP code and type can fail" + errors: + expected: + - "acl term .* icmp-type can only be specified for protocol icmp or icmp-ipv6" + - "acl term .* icmp-code can only be specified for protocol icmp or icmp-ipv6" + - "acl term .* icmp-code low value is higher than high value" + - "acl term .* icmp-type low value is higher than high value" + count: 8 +--- +acls: + acl01: + terms: + - description: "code and type are not allowed if protocol is not icmp or icmp-ipv6" + action: permit + icmp-code: 1 + icmp-type: 1 + - description: "code and type are not allowed if protocol is not icmp or icmp-ipv6" + action: permit + protocol: udp + icmp-code: 1 + icmp-type: 1 + - description: "code and type are not allowed if protocol is not icmp or icmp-ipv6" + action: permit + protocol: tcp + icmp-code: 1 + icmp-type: 1 + - description: "Ranges invalid" + action: permit + protocol: icmp + icmp-code: 5-4 + icmp-type: 20-10 + - description: "OK" + action: permit + protocol: icmp + icmp-code: 1 + icmp-type: 1 + - description: "OK" + action: permit + protocol: ipv6-icmp + icmp-code: 1 + icmp-type: 1 From 7fd47c085489a38a201a34021f56f31c89dd3039 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 01:12:16 +0000 Subject: [PATCH 05/46] acl: Add the aclname to error messages --- vppcfg/config/acl.py | 38 ++++++++++++++-------------- vppcfg/unittest/yaml/error-acl1.yaml | 2 +- vppcfg/unittest/yaml/error-acl2.yaml | 2 +- vppcfg/unittest/yaml/error-acl3.yaml | 14 +++++----- vppcfg/unittest/yaml/error-acl4.yaml | 2 +- vppcfg/unittest/yaml/error-acl5.yaml | 8 +++--- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index d395cb8..dc788f0 100644 --- a/vppcfg/config/acl.py +++ b/vppcfg/config/acl.py @@ -192,37 +192,37 @@ def validate_acls(yaml): terms += 1 orig_acl_term = acl_term.copy() acl_term = hydrate_term(acl_term) - logger.debug(f"acl term {terms} orig {orig_acl_term} hydrated {acl_term}") + logger.debug(f"acl {aclname} term {terms} orig {orig_acl_term} hydrated {acl_term}") if acl_term["family"] == "any": if "source" in acl_term: - msgs.append(f"acl term {terms} family any cannot have source") + msgs.append(f"acl {aclname} term {terms} family any cannot have source") result = False if "destination" in acl_term: - msgs.append(f"acl term {terms} family any cannot have destination") + msgs.append(f"acl {aclname} term {terms} family any cannot have destination") result = False else: src = ipaddress.ip_network(acl_term["source"]) dst = ipaddress.ip_network(acl_term["destination"]) if src.version != dst.version: msgs.append( - f"acl term {terms} source and destination have different address family" + f"acl {aclname} term {terms} source and destination have different address family" ) result = False proto = get_protocol(acl_term["protocol"]) if proto is None: - msgs.append(f"acl term {terms} could not understand protocol") + msgs.append(f"acl {aclname} term {terms} could not understand protocol") result = False if not proto in [6, 17]: if "source-port" in orig_acl_term: msgs.append( - f"acl term {terms} source-port can only be specified for protocol tcp or udp" + f"acl {aclname} term {terms} source-port can only be specified for protocol tcp or udp" ) result = False if "destination-port" in orig_acl_term: msgs.append( - f"acl term {terms} destination-port can only be specified for protocol tcp or udp" + f"acl {aclname} term {terms} destination-port can only be specified for protocol tcp or udp" ) result = False @@ -233,66 +233,66 @@ def validate_acls(yaml): ) if src_low_port is None or src_high_port is None: - msgs.append(f"acl term {terms} could not understand source port") + msgs.append(f"acl {aclname} term {terms} could not understand source port") result = False else: if src_low_port > src_high_port: msgs.append( - f"acl term {terms} source low port is higher than source high port" + f"acl {aclname} term {terms} source low port is higher than source high port" ) result = False if src_low_port < 0 or src_low_port > 65535: msgs.append( - f"acl term {terms} source low port is not between [0,65535]" + f"acl {aclname} term {terms} source low port is not between [0,65535]" ) result = False if src_high_port < 0 or src_high_port > 65535: msgs.append( - f"acl term {terms} source high port is not between [0,65535]" + f"acl {aclname} term {terms} source high port is not between [0,65535]" ) result = False if dst_low_port is None or dst_high_port is None: msgs.append( - f"acl term {terms} could not understand destination port" + f"acl {aclname} term {terms} could not understand destination port" ) result = False else: if dst_low_port > dst_high_port: msgs.append( - f"acl term {terms} destination low port is higher than destination high port" + f"acl {aclname} term {terms} destination low port is higher than destination high port" ) result = False if dst_low_port < 0 or dst_low_port > 65535: msgs.append( - f"acl term {terms} destination low port is not between [0,65535]" + f"acl {aclname} term {terms} destination low port is not between [0,65535]" ) result = False if dst_high_port < 0 or dst_high_port > 65535: msgs.append( - f"acl term {terms} destination high port is not between [0,65535]" + f"acl {aclname} term {terms} destination high port is not between [0,65535]" ) result = False if not proto in [1, 58]: if "icmp-code" in orig_acl_term: msgs.append( - f"acl term {terms} icmp-code can only be specified for protocol icmp or icmp-ipv6" + f"acl {aclname} term {terms} icmp-code can only be specified for protocol icmp or icmp-ipv6" ) result = False if "icmp-type" in orig_acl_term: msgs.append( - f"acl term {terms} icmp-type can only be specified for protocol icmp or icmp-ipv6" + f"acl {aclname} term {terms} icmp-type can only be specified for protocol icmp or icmp-ipv6" ) result = False if proto in [1, 58]: 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: - msgs.append(f"acl term {terms} icmp-code low value is higher than high value") + msgs.append(f"acl {aclname} term {terms} icmp-code low value is higher than high value") result = False if icmp_type_low > icmp_type_high: - msgs.append(f"acl term {terms} icmp-type low value is higher than high value") + msgs.append(f"acl {aclname} term {terms} icmp-type low value is higher than high value") result = False return result, msgs diff --git a/vppcfg/unittest/yaml/error-acl1.yaml b/vppcfg/unittest/yaml/error-acl1.yaml index 1812395..a758e42 100644 --- a/vppcfg/unittest/yaml/error-acl1.yaml +++ b/vppcfg/unittest/yaml/error-acl1.yaml @@ -2,7 +2,7 @@ test: description: "Family any precludes source/destination" errors: expected: - - "acl term .* family any cannot have (source|destination)" + - "acl .* term .* family any cannot have (source|destination)" count: 4 --- acls: diff --git a/vppcfg/unittest/yaml/error-acl2.yaml b/vppcfg/unittest/yaml/error-acl2.yaml index f088f63..3f18e00 100644 --- a/vppcfg/unittest/yaml/error-acl2.yaml +++ b/vppcfg/unittest/yaml/error-acl2.yaml @@ -2,7 +2,7 @@ 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 have different address family" count: 4 --- acls: diff --git a/vppcfg/unittest/yaml/error-acl3.yaml b/vppcfg/unittest/yaml/error-acl3.yaml index cc1961b..fb16337 100644 --- a/vppcfg/unittest/yaml/error-acl3.yaml +++ b/vppcfg/unittest/yaml/error-acl3.yaml @@ -2,13 +2,13 @@ test: description: "Ways in which port ranges can fail" errors: expected: - - "acl term .* could not understand source port" - - "acl term .* could not understand destination port" - - "acl term .* source low port is higher than source high port" - - "acl term .* source (high|low) port is not between \\[0,65535\\]" - - "acl term .* destination (high|low) port is not between \\[0,65535\\]" - - "acl term .* source-port can only be specified for protocol tcp or udp" - - "acl term .* destination-port can only be specified for protocol tcp or udp" + - "acl .* term .* could not understand source port" + - "acl .* term .* could not understand destination port" + - "acl .* term .* source low port is higher than source high port" + - "acl .* term .* source (high|low) port is not between \\[0,65535\\]" + - "acl .* term .* destination (high|low) port is not between \\[0,65535\\]" + - "acl .* term .* source-port can only be specified for protocol tcp or udp" + - "acl .* term .* destination-port can only be specified for protocol tcp or udp" count: 7 --- acls: diff --git a/vppcfg/unittest/yaml/error-acl4.yaml b/vppcfg/unittest/yaml/error-acl4.yaml index dc32e40..45cdb4c 100644 --- a/vppcfg/unittest/yaml/error-acl4.yaml +++ b/vppcfg/unittest/yaml/error-acl4.yaml @@ -2,7 +2,7 @@ test: description: "Ways in which ACE protocol can fail" errors: expected: - - "acl term .* could not understand protocol" + - "acl .* term .* could not understand protocol" count: 1 --- acls: diff --git a/vppcfg/unittest/yaml/error-acl5.yaml b/vppcfg/unittest/yaml/error-acl5.yaml index 952fa87..c212978 100644 --- a/vppcfg/unittest/yaml/error-acl5.yaml +++ b/vppcfg/unittest/yaml/error-acl5.yaml @@ -2,10 +2,10 @@ test: description: "Ways in which ICMP code and type can fail" errors: expected: - - "acl term .* icmp-type can only be specified for protocol icmp or icmp-ipv6" - - "acl term .* icmp-code can only be specified for protocol icmp or icmp-ipv6" - - "acl term .* icmp-code low value is higher than high value" - - "acl term .* icmp-type low value is higher than high value" + - "acl .* term .* icmp-type can only be specified for protocol icmp or icmp-ipv6" + - "acl .* term .* icmp-code can only be specified for protocol icmp or icmp-ipv6" + - "acl .* term .* icmp-code low value is higher than high value" + - "acl .* term .* icmp-type low value is higher than high value" count: 8 --- acls: From adf7c7eb244b3323575a63dc4995373581ea876d Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 01:13:27 +0000 Subject: [PATCH 06/46] formatting with black --- vppcfg/config/acl.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index dc788f0..3580cee 100644 --- a/vppcfg/config/acl.py +++ b/vppcfg/config/acl.py @@ -192,13 +192,19 @@ def validate_acls(yaml): terms += 1 orig_acl_term = acl_term.copy() acl_term = hydrate_term(acl_term) - logger.debug(f"acl {aclname} term {terms} orig {orig_acl_term} hydrated {acl_term}") + logger.debug( + f"acl {aclname} term {terms} orig {orig_acl_term} hydrated {acl_term}" + ) if acl_term["family"] == "any": if "source" in acl_term: - msgs.append(f"acl {aclname} term {terms} family any cannot have source") + msgs.append( + f"acl {aclname} term {terms} family any cannot have source" + ) result = False if "destination" in acl_term: - msgs.append(f"acl {aclname} term {terms} family any cannot have destination") + msgs.append( + f"acl {aclname} term {terms} family any cannot have destination" + ) result = False else: src = ipaddress.ip_network(acl_term["source"]) @@ -233,7 +239,9 @@ def validate_acls(yaml): ) if src_low_port is None or src_high_port is None: - msgs.append(f"acl {aclname} term {terms} could not understand source port") + msgs.append( + f"acl {aclname} term {terms} could not understand source port" + ) result = False else: if src_low_port > src_high_port: @@ -289,10 +297,14 @@ def validate_acls(yaml): 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: - msgs.append(f"acl {aclname} term {terms} icmp-code low value is higher than high value") + msgs.append( + f"acl {aclname} term {terms} icmp-code low value is higher than high value" + ) result = False if icmp_type_low > icmp_type_high: - msgs.append(f"acl {aclname} term {terms} icmp-type low value is higher than high value") + msgs.append( + f"acl {aclname} term {terms} icmp-type low value is higher than high value" + ) result = False return result, msgs From f0da3abe6e01ea87e77d181a6c8abd25986c26b8 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 09:42:22 +0000 Subject: [PATCH 07/46] Add an ACL yaml unit test, to cover get_acls() and get_by_name() --- vppcfg/config/acl.py | 2 +- vppcfg/config/test_acl.py | 20 ++++++++++++++++++++ vppcfg/unittest/test_acl.yaml | 23 +++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 vppcfg/unittest/test_acl.yaml diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index 3580cee..8aef561 100644 --- a/vppcfg/config/acl.py +++ b/vppcfg/config/acl.py @@ -17,7 +17,7 @@ import socket import ipaddress -def get_aclx(yaml): +def get_acls(yaml): """Return a list of all acls.""" ret = [] if "acls" in yaml: diff --git a/vppcfg/config/test_acl.py b/vppcfg/config/test_acl.py index e02ffa8..984b7b1 100644 --- a/vppcfg/config/test_acl.py +++ b/vppcfg/config/test_acl.py @@ -14,10 +14,30 @@ # -*- coding: utf-8 -*- """ Unit tests for taps """ import unittest +import yaml from . import acl +from .unittestyaml import UnitTestYaml class TestACLMethods(unittest.TestCase): + def setUp(self): + with UnitTestYaml("test_acl.yaml") as f: + self.cfg = yaml.load(f, Loader=yaml.FullLoader) + + def test_get_acls(self): + acllist = acl.get_acls(self.cfg) + self.assertIsInstance(acllist, list) + self.assertEqual(2, len(acllist)) + + def test_get_by_name(self): + aclname, _acl = acl.get_by_name(self.cfg, "deny-all") + self.assertIsNotNone(_acl) + self.assertEqual("deny-all", aclname) + + aclname, _acl = acl.get_by_name(self.cfg, "acl-noexist") + self.assertIsNone(aclname) + self.assertIsNone(_acl) + def test_get_port_low_high(self): lo, hi = acl.get_port_low_high(80) self.assertEqual(80, lo) diff --git a/vppcfg/unittest/test_acl.yaml b/vppcfg/unittest/test_acl.yaml new file mode 100644 index 0000000..80bbf5b --- /dev/null +++ b/vppcfg/unittest/test_acl.yaml @@ -0,0 +1,23 @@ +acls: + acl01: + description: "Test ACL #1" + terms: + - description: "Allow a specific IPv6 TCP flow" + action: permit + source: 2001:db8::/64 + destination: 2001:db8:1::/64 + protocol: tcp + destination-port: www + source-port: "1024-65535" + - description: "Allow IPv4 ICMP Destination Unreachable, any code" + family: ipv4 + action: permit + protocol: icmp + icmp-type: 3 + icmp-code: any + - description: "Deny any IPv4 or IPv6" + action: deny + deny-all: + description: "Test ACL #2" + terms: + - action: deny From 597981e79b1acbe51e5042c3fcea4e38b5868bb3 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 10:15:57 +0000 Subject: [PATCH 08/46] Add prefixlist (mixed IPv4 and IPv6, containing either IP addresses or prefixes + tests --- vppcfg/config/__init__.py | 2 + vppcfg/config/prefixlist.py | 104 +++++++++++++++++++++++++++ vppcfg/config/test_prefixlist.py | 77 ++++++++++++++++++++ vppcfg/example.yaml | 12 ++++ vppcfg/schema.yaml | 6 ++ vppcfg/unittest/test_prefixlist.yaml | 26 +++++++ 6 files changed, 227 insertions(+) create mode 100644 vppcfg/config/prefixlist.py create mode 100644 vppcfg/config/test_prefixlist.py create mode 100644 vppcfg/unittest/test_prefixlist.yaml diff --git a/vppcfg/config/__init__.py b/vppcfg/config/__init__.py index 0bd0cbc..e8221e8 100644 --- a/vppcfg/config/__init__.py +++ b/vppcfg/config/__init__.py @@ -38,6 +38,7 @@ from .interface import validate_interfaces from .bridgedomain import validate_bridgedomains from .vxlan_tunnel import validate_vxlan_tunnels from .tap import validate_taps +from .prefixlist import validate_prefixlists from .acl import validate_acls @@ -90,6 +91,7 @@ class Validator: validate_bridgedomains, validate_vxlan_tunnels, validate_taps, + validate_prefixlists, validate_acls, ] diff --git a/vppcfg/config/prefixlist.py b/vppcfg/config/prefixlist.py new file mode 100644 index 0000000..eff8ad9 --- /dev/null +++ b/vppcfg/config/prefixlist.py @@ -0,0 +1,104 @@ +# +# Copyright (c) 2023 Pim van Pelt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at: +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" A vppcfg configuration module that validates prefixlists """ +import logging +import socket +import ipaddress + + +def get_prefixlists(yaml): + """Return a list of all prefixlists.""" + ret = [] + if "prefixlists" in yaml: + for plname, _pl in yaml["prefixlists"].items(): + ret.append(plname) + return ret + + +def get_by_name(yaml, plname): + """Return the prefixlist by name, if it exists. Return None otherwise.""" + try: + if plname in yaml["prefixlists"]: + return plname, yaml["prefixlists"][plname] + except KeyError: + pass + return None, None + + +def count(yaml, plname): + """Return the number of IPv4 and IPv6 entries in the prefixlist. + Returns 0, 0 if it doesn't exist""" + v4, v6 = 0, 0 + + plname, pl = get_by_name(yaml, plname) + if not pl: + return 0, 0 + for m in pl["members"]: + ipn = ipaddress.ip_network(m, strict=False) + if ipn.version == 4: + v4 += 1 + elif ipn.version == 6: + v6 += 1 + + return v4, v6 + + +def count_ipv4(yaml, plname): + """Return the number of IPv4 entries in the prefixlist.""" + v4, v6 = count(yaml, plname) + return v4 + + +def count_ipv6(yaml, plname): + """Return the number of IPv6 entries in the prefixlist.""" + v4, v6 = count(yaml, plname) + return v6 + + +def has_ipv4(yaml, plname): + """Return True if the prefixlist has at least one IPv4 entry.""" + v4, v6 = count(yaml, plname) + return v4 > 0 + + +def has_ipv6(yaml, plname): + """Return True if the prefixlist has at least one IPv6 entry.""" + v4, v6 = count(yaml, plname) + return v6 > 0 + + +def is_empty(yaml, plname): + """Return True if the prefixlist has no entries.""" + v4, v6 = count(yaml, plname) + return v4 + v6 == 0 + + +def validate_prefixlists(yaml): + """Validate the semantics of all YAML 'prefixlists' entries""" + result = True + msgs = [] + logger = logging.getLogger("vppcfg.config") + logger.addHandler(logging.NullHandler()) + + if not "prefixlists" in yaml: + return result, msgs + + for plname, pl in yaml["prefixlists"].items(): + logger.debug(f"prefixlist {plname}: {pl}") + members = 0 + for pl_member in pl["members"]: + members += 1 + logger.debug(f"prefixlist {plname} member {members} is {pl_member}") + + return result, msgs diff --git a/vppcfg/config/test_prefixlist.py b/vppcfg/config/test_prefixlist.py new file mode 100644 index 0000000..5b1eab0 --- /dev/null +++ b/vppcfg/config/test_prefixlist.py @@ -0,0 +1,77 @@ +# +# Copyright (c) 2022 Pim van Pelt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at: +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# -*- coding: utf-8 -*- +""" Unit tests for taps """ +import unittest +import yaml +from . import prefixlist +from .unittestyaml import UnitTestYaml + + +class TestACLMethods(unittest.TestCase): + def setUp(self): + with UnitTestYaml("test_prefixlist.yaml") as f: + self.cfg = yaml.load(f, Loader=yaml.FullLoader) + + def test_get_prefixlists(self): + plist = prefixlist.get_prefixlists(self.cfg) + self.assertIsInstance(plist, list) + self.assertEqual(5, len(plist)) + + def test_get_by_name(self): + plname, _pl = prefixlist.get_by_name(self.cfg, "trusted") + self.assertIsNotNone(_pl) + self.assertEqual("trusted", plname) + + plname, _pl = prefixlist.get_by_name(self.cfg, "pl-noexist") + self.assertIsNone(plname) + self.assertIsNone(_pl) + + def test_count(self): + v4, v6 = prefixlist.count(self.cfg, "trusted") + self.assertEqual(2, v4) + self.assertEqual(2, v6) + + v4, v6 = prefixlist.count(self.cfg, "empty") + self.assertEqual(0, v4) + self.assertEqual(0, v6) + + v4, v6 = prefixlist.count(self.cfg, "pl-noexist") + self.assertEqual(0, v4) + self.assertEqual(0, v6) + + def test_count_ipv4(self): + self.assertEqual(2, prefixlist.count_ipv4(self.cfg, "trusted")) + self.assertEqual(0, prefixlist.count_ipv4(self.cfg, "empty")) + self.assertEqual(0, prefixlist.count_ipv4(self.cfg, "pl-noexist")) + + def test_count_ipv6(self): + self.assertEqual(2, prefixlist.count_ipv6(self.cfg, "trusted")) + self.assertEqual(0, prefixlist.count_ipv6(self.cfg, "empty")) + self.assertEqual(0, prefixlist.count_ipv6(self.cfg, "pl-noexist")) + + def test_has_ipv4(self): + self.assertTrue(prefixlist.has_ipv4(self.cfg, "trusted")) + self.assertFalse(prefixlist.has_ipv4(self.cfg, "empty")) + self.assertFalse(prefixlist.has_ipv4(self.cfg, "pl-noexist")) + + def test_has_ipv6(self): + self.assertTrue(prefixlist.has_ipv6(self.cfg, "trusted")) + self.assertFalse(prefixlist.has_ipv6(self.cfg, "empty")) + self.assertFalse(prefixlist.has_ipv6(self.cfg, "pl-noexist")) + + def test_is_empty(self): + self.assertFalse(prefixlist.is_empty(self.cfg, "trusted")) + self.assertTrue(prefixlist.is_empty(self.cfg, "empty")) + self.assertTrue(prefixlist.is_empty(self.cfg, "pl-noexist")) diff --git a/vppcfg/example.yaml b/vppcfg/example.yaml index a7700f5..b54fdd8 100644 --- a/vppcfg/example.yaml +++ b/vppcfg/example.yaml @@ -118,6 +118,18 @@ taps: mtu: 1500 bridge: br1 +prefixlists: + trusted: + description: "All IPv4 and IPv6 trusted networks" + members: + - 192.0.2.1 + - 192.0.2.0/24 + - 2001:db8::1 + - 2001:db8::/64 + empty: + description: "An empty prefixlist" + members: [] + acls: acl01: description: "Test ACL" diff --git a/vppcfg/schema.yaml b/vppcfg/schema.yaml index 2bffd49..12d62db 100644 --- a/vppcfg/schema.yaml +++ b/vppcfg/schema.yaml @@ -4,6 +4,7 @@ loopbacks: map(include('loopback'),key=str(matches='loop[0-9]+'),required=False) bridgedomains: map(include('bridgedomain'),key=str(matches='bd[0-9]+'),required=False) 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) --- vxlan: @@ -81,8 +82,13 @@ tap: rx-ring-size: int(min=8,max=32768,required=False) tx-ring-size: int(min=8,max=32768,required=False) --- +prefixlist: + description: str(exclude='\'"',len=64,required=False) + members: list(any(ip_interface(),ip())) +--- # Valid: 80 "www" "-1024" "1024-" "1024-65535", and "any" acl-term-port-int-range-symbolic: any(int(min=1,max=65535),str(equals="any"),regex('^([1-9][0-9]*-|-[1-9][0-9]*|[1-9][0-9]*-[1-9][0-9]*)$'),regex('^[a-z][a-z0-9-]*$')) +--- # Valid: 80 "-245" "10-" "10-245", and "any" acl-term-icmp-int-range: any(int(min=0,max=255),str(equals="any"),regex('^([0-9]+-|-[1-9][0-9]*|[0-9]*-[1-9][0-9]*)$')) --- diff --git a/vppcfg/unittest/test_prefixlist.yaml b/vppcfg/unittest/test_prefixlist.yaml new file mode 100644 index 0000000..47e7739 --- /dev/null +++ b/vppcfg/unittest/test_prefixlist.yaml @@ -0,0 +1,26 @@ +prefixlists: + trusted: + description: "All IPv4 and IPv6 trusted networks" + members: + - 192.0.2.1 + - 192.0.2.0/24 + - 2001:db8::1 + - 2001:db8::/64 + deny-all: + description: "Default for IPv4 and IPv6" + members: + - 0.0.0.0/0 + - ::/0 + ipv4-only: + description: "Only contains IPv4" + members: + - 192.0.2.1 + - 192.0.2.0/24 + ipv6-only: + description: "Only contains IPv6" + members: + - 2001:db8::1 + - 2001:db8::/64 + empty: + description: "An empty list" + members: [] From a274fdc2af883b18a9a65db599d05ba6e174a2fb Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 12:01:29 +0000 Subject: [PATCH 09/46] Add prefixlist.get_network_list() + tests --- vppcfg/config/prefixlist.py | 14 ++++++++++++++ vppcfg/config/test_prefixlist.py | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/vppcfg/config/prefixlist.py b/vppcfg/config/prefixlist.py index eff8ad9..b7cc16c 100644 --- a/vppcfg/config/prefixlist.py +++ b/vppcfg/config/prefixlist.py @@ -36,6 +36,20 @@ def get_by_name(yaml, plname): return None, None +def get_network_list(yaml, plname): + """Returns a list of 0 or more ip_network elements, that represent the members + in a prefixlist of given name. Return the empty list if the prefixlist doesn't + exist""" + ret = [] + plname, pl = get_by_name(yaml, plname) + if not pl: + return ret + for m in pl["members"]: + ipn = ipaddress.ip_network(m, strict=False) + ret.append(ipn) + return ret + + def count(yaml, plname): """Return the number of IPv4 and IPv6 entries in the prefixlist. Returns 0, 0 if it doesn't exist""" diff --git a/vppcfg/config/test_prefixlist.py b/vppcfg/config/test_prefixlist.py index 5b1eab0..fc41a2b 100644 --- a/vppcfg/config/test_prefixlist.py +++ b/vppcfg/config/test_prefixlist.py @@ -75,3 +75,12 @@ class TestACLMethods(unittest.TestCase): self.assertFalse(prefixlist.is_empty(self.cfg, "trusted")) self.assertTrue(prefixlist.is_empty(self.cfg, "empty")) self.assertTrue(prefixlist.is_empty(self.cfg, "pl-noexist")) + + def test_get_network_list(self): + l = prefixlist.get_network_list(self.cfg, "trusted") + self.assertIsInstance(l, list) + self.assertEquals(4, len(l)) + + l = prefixlist.get_network_list(self.cfg, "pl-notexist") + self.assertIsInstance(l, list) + self.assertEquals(0, len(l)) From 4e2354c3d8c1cf18ad79b2aa2f6b9767fb4072b4 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 12:03:34 +0000 Subject: [PATCH 10/46] Add acl.get_network_list() + tests; Update docs to reference the ability to use prefixlist as a source/destination --- docs/config-guide.md | 39 +++++++++++++++++++++++++++++++++++++-- vppcfg/config/acl.py | 27 +++++++++++++++++++++++++++ vppcfg/config/test_acl.py | 27 +++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/docs/config-guide.md b/docs/config-guide.md index 947fad4..3beaaac 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -359,6 +359,39 @@ interfaces: exact-match: False ``` +### Prefix Lists + +This construct allows to enumerate a list of IPv4 or IPv6 host addresses and/or networks. Each +prefixlist has a name which consists of anywhere between 1 and 56 characters, and it must start +with a letter. The syntax is straight forward: + +* ***description***: A string, no longer than 64 characters, and excluding the single quote ' + and double quote ". This string is currently not used anywhere, and serves for enduser + documentation purposes. +* ***members***: A list of zero or more entries which can take the form: + * ***IPv4 Host***: an IPv4 address, eg. `192.0.2.1` + * ***IPv4 Prefix***: an IPv6 prefix, eg. `192.0.2.0/24` + * ***IPv6 Host***: an IPv4 address, eg. `2001:db8::1` + * ***IPv6 Prefix***: an IPv6 prefix, eg. `2001:db8::0/64` + +***NOTE***: It is valid to have host addresses with prefixlen, for example `192.168.1.1/24` +in other words, the prefix can be either a network or a host. + +A few examples: +``` +prefixlists: + example: + description: "An example prefixlist with hosts and prefixes" + members: + - 192.0.2.1 + - 192.0.2.0/24 + - 2001:db8::1 + - 2001:db8::/64 + empty: + description: "An empty prefixlist" + members: [] +``` + ### Access Control Lists In VPP, a common firewall function is provided by the `acl-plugin`. The anatomy of this plugin @@ -377,8 +410,10 @@ packets then either perform an action of `permit` or `deny` (for stateless) or ` * ***family***: Which IP address family to match, can be either `ipv4`, or `ipv6` or `any`, which is the default. If `any` is used, this term will also operate on any source and destination addresses, and it will emit two ACEs, one for each address family. - * ***source***: The IPv4 or IPv6 source prefix, eg. `192.0.2.0/24` or `2001:db8::/64`. If - left empty, this means any (ie. `0.0.0.0/0` or `::/0`). + * ***source***: Either an IPv4 or IPv6 host (without prefixlen, eg. `192.0.2.1` or + `2001:db8::1`), an IPv4 or IPv6 prefix (with prefixlen, eg. `192.0.2.0/24` or + `2001:db8::/64`), or a reference to the name of an existing _prefixlist_ (eg. `trusted`). + If left empty, this means all IPv4 and IPv6 (ie. `[ 0.0.0.0/0, ::/0 ]`). * ***destination***: Similar to `source`, but for the destination field of the packets. * ***protocol***: The L4 protocol, can be either a numeric value (eg. `6`), or a symbolic string value from `/etc/protocols` (eg. `tcp`). If omitted, only L3 matches are performed. diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index 8aef561..68d9e74 100644 --- a/vppcfg/config/acl.py +++ b/vppcfg/config/acl.py @@ -15,6 +15,7 @@ import logging import socket import ipaddress +from . import prefixlist def get_acls(yaml): @@ -151,6 +152,32 @@ def get_port_low_high(portstring): return None, None +def is_ip(ip_string): + """Return True if the given ip_string is either an IPv4/IPv6 address or prefix.""" + if not isinstance(ip_string, str): + return False + + try: + ipn = ipaddress.ip_network(ip_string, strict=False) + return True + except: + pass + return False + + +def get_network_list(yaml, network_string): + """Return the full list of source or destination address(es). This function resolves the + 'source' or 'destination' field, which can either be an IP address, a Prefix, or the name + of a Prefix List. It returns a list of ip_network() objects, including prefix. IP addresses + will receive prefixlen /32 or /128.""" + + if is_ip(network_string): + ipn = ipaddress.ip_network(network_string, strict=False) + return [ipn] + + return prefixlist.get_network_list(yaml, network_string) + + def get_protocol(protostring): """For a given protocol string, which can be either an integer or a symbolic port name in /etc/protocols, return the protocol number as integer, or None if it cannot diff --git a/vppcfg/config/test_acl.py b/vppcfg/config/test_acl.py index 984b7b1..d307849 100644 --- a/vppcfg/config/test_acl.py +++ b/vppcfg/config/test_acl.py @@ -113,3 +113,30 @@ class TestACLMethods(unittest.TestCase): lo, hi = acl.get_icmp_low_high("10-20") self.assertEqual(10, lo) self.assertEqual(20, hi) + + def test_is_ip(self): + self.assertTrue(acl.is_ip("192.0.2.1")) + self.assertTrue(acl.is_ip("192.0.2.1/24")) + self.assertTrue(acl.is_ip("192.0.2.0/24")) + self.assertTrue(acl.is_ip("2001:db8::1")) + self.assertTrue(acl.is_ip("2001:db8::1/64")) + self.assertTrue(acl.is_ip("2001:db8::/64")) + self.assertFalse(acl.is_ip(True)) + self.assertFalse(acl.is_ip("String")) + self.assertFalse(acl.is_ip([])) + self.assertFalse(acl.is_ip({})) + + def test_get_network_list(self): + for s in ["192.0.2.1", "192.0.2.1/24", "2001:db8::1", "2001:db8::1/64"]: + l = acl.get_network_list(self.cfg, s) + self.assertIsInstance(l, list) + self.assertEquals(1, len(l)) + n = l[0] + + l = acl.get_network_list(self.cfg, "trusted") + self.assertIsInstance(l, list) + self.assertEquals(4, len(l)) + + l = acl.get_network_list(self.cfg, "pl-notexist") + self.assertIsInstance(l, list) + self.assertEquals(0, len(l)) From 8a7c690ee50388391371fe3ba35f124c9610fcb7 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 12:15:24 +0000 Subject: [PATCH 11/46] Add ability to filter get_network_list() by ipv4 or ipv6, and add tests --- vppcfg/config/acl.py | 16 ++++++++++++---- vppcfg/config/prefixlist.py | 9 ++++++--- vppcfg/config/test_acl.py | 14 +++++++++++++- vppcfg/config/test_prefixlist.py | 20 +++++++++++++++++--- vppcfg/unittest/test_acl.yaml | 12 ++++++++++++ vppcfg/unittest/test_prefixlist.yaml | 1 + 6 files changed, 61 insertions(+), 11 deletions(-) diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index 68d9e74..d7b477c 100644 --- a/vppcfg/config/acl.py +++ b/vppcfg/config/acl.py @@ -165,17 +165,25 @@ def is_ip(ip_string): return False -def get_network_list(yaml, network_string): +def get_network_list(yaml, network_string, want_ipv4=True, want_ipv6=True): """Return the full list of source or destination address(es). This function resolves the 'source' or 'destination' field, which can either be an IP address, a Prefix, or the name of a Prefix List. It returns a list of ip_network() objects, including prefix. IP addresses - will receive prefixlen /32 or /128.""" + will receive prefixlen /32 or /128. Optionally, want_ipv4 or want_ipv6 can be set to False + to filter the list.""" + ret = [] if is_ip(network_string): ipn = ipaddress.ip_network(network_string, strict=False) - return [ipn] + if ipn.version == 4 and want_ipv4: + ret = [ipn] + if ipn.version == 6 and want_ipv6: + ret = [ipn] + return ret - return prefixlist.get_network_list(yaml, network_string) + return prefixlist.get_network_list( + yaml, network_string, want_ipv4=want_ipv4, want_ipv6=want_ipv6 + ) def get_protocol(protostring): diff --git a/vppcfg/config/prefixlist.py b/vppcfg/config/prefixlist.py index b7cc16c..8160b60 100644 --- a/vppcfg/config/prefixlist.py +++ b/vppcfg/config/prefixlist.py @@ -36,17 +36,20 @@ def get_by_name(yaml, plname): return None, None -def get_network_list(yaml, plname): +def get_network_list(yaml, plname, want_ipv4=True, want_ipv6=True): """Returns a list of 0 or more ip_network elements, that represent the members in a prefixlist of given name. Return the empty list if the prefixlist doesn't - exist""" + exist. Optionally, want_ipv4 or want_ipv6 can be set to False to filter the list.""" ret = [] plname, pl = get_by_name(yaml, plname) if not pl: return ret for m in pl["members"]: ipn = ipaddress.ip_network(m, strict=False) - ret.append(ipn) + if ipn.version == 4 and want_ipv4: + ret.append(ipn) + if ipn.version == 6 and want_ipv6: + ret.append(ipn) return ret diff --git a/vppcfg/config/test_acl.py b/vppcfg/config/test_acl.py index d307849..b699daa 100644 --- a/vppcfg/config/test_acl.py +++ b/vppcfg/config/test_acl.py @@ -135,7 +135,19 @@ class TestACLMethods(unittest.TestCase): l = acl.get_network_list(self.cfg, "trusted") self.assertIsInstance(l, list) - self.assertEquals(4, len(l)) + self.assertEquals(5, len(l)) + + l = acl.get_network_list(self.cfg, "trusted", want_ipv6=False) + self.assertIsInstance(l, list) + self.assertEquals(2, len(l)) + + l = acl.get_network_list(self.cfg, "trusted", want_ipv4=False) + self.assertIsInstance(l, list) + self.assertEquals(3, len(l)) + + l = acl.get_network_list(self.cfg, "trusted", want_ipv4=False, want_ipv6=False) + self.assertIsInstance(l, list) + self.assertEquals(0, len(l)) l = acl.get_network_list(self.cfg, "pl-notexist") self.assertIsInstance(l, list) diff --git a/vppcfg/config/test_prefixlist.py b/vppcfg/config/test_prefixlist.py index fc41a2b..d0f37bc 100644 --- a/vppcfg/config/test_prefixlist.py +++ b/vppcfg/config/test_prefixlist.py @@ -41,7 +41,7 @@ class TestACLMethods(unittest.TestCase): def test_count(self): v4, v6 = prefixlist.count(self.cfg, "trusted") self.assertEqual(2, v4) - self.assertEqual(2, v6) + self.assertEqual(3, v6) v4, v6 = prefixlist.count(self.cfg, "empty") self.assertEqual(0, v4) @@ -57,7 +57,7 @@ class TestACLMethods(unittest.TestCase): self.assertEqual(0, prefixlist.count_ipv4(self.cfg, "pl-noexist")) def test_count_ipv6(self): - self.assertEqual(2, prefixlist.count_ipv6(self.cfg, "trusted")) + self.assertEqual(3, prefixlist.count_ipv6(self.cfg, "trusted")) self.assertEqual(0, prefixlist.count_ipv6(self.cfg, "empty")) self.assertEqual(0, prefixlist.count_ipv6(self.cfg, "pl-noexist")) @@ -79,7 +79,21 @@ class TestACLMethods(unittest.TestCase): def test_get_network_list(self): l = prefixlist.get_network_list(self.cfg, "trusted") self.assertIsInstance(l, list) - self.assertEquals(4, len(l)) + self.assertEquals(5, len(l)) + + l = prefixlist.get_network_list(self.cfg, "trusted", want_ipv6=False) + self.assertIsInstance(l, list) + self.assertEquals(2, len(l)) + + l = prefixlist.get_network_list(self.cfg, "trusted", want_ipv4=False) + self.assertIsInstance(l, list) + self.assertEquals(3, len(l)) + + l = prefixlist.get_network_list( + self.cfg, "trusted", want_ipv4=False, want_ipv6=False + ) + self.assertIsInstance(l, list) + self.assertEquals(0, len(l)) l = prefixlist.get_network_list(self.cfg, "pl-notexist") self.assertIsInstance(l, list) diff --git a/vppcfg/unittest/test_acl.yaml b/vppcfg/unittest/test_acl.yaml index 80bbf5b..f3bf8a7 100644 --- a/vppcfg/unittest/test_acl.yaml +++ b/vppcfg/unittest/test_acl.yaml @@ -1,7 +1,19 @@ +prefixlists: + trusted: + members: + - 192.0.2.1 + - 192.0.2.0/24 + - 2001:db8::1 + - 2001:db8::/64 + - 2001:db8::/48 + acls: acl01: description: "Test ACL #1" terms: + - description: "Allow a Prefixlist" + action: permit + source: trusted - description: "Allow a specific IPv6 TCP flow" action: permit source: 2001:db8::/64 diff --git a/vppcfg/unittest/test_prefixlist.yaml b/vppcfg/unittest/test_prefixlist.yaml index 47e7739..47c6826 100644 --- a/vppcfg/unittest/test_prefixlist.yaml +++ b/vppcfg/unittest/test_prefixlist.yaml @@ -6,6 +6,7 @@ prefixlists: - 192.0.2.0/24 - 2001:db8::1 - 2001:db8::/64 + - 2001:db8::/48 deny-all: description: "Default for IPv4 and IPv6" members: From 0e4490fc0651f92947f57b455cb6e9de56b57143 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 14:20:07 +0000 Subject: [PATCH 12/46] Make 'any' a reserved name for prefixlists --- docs/config-guide.md | 2 +- vppcfg/config/prefixlist.py | 6 ++++++ vppcfg/unittest/yaml/error-prefixlist1.yaml | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 vppcfg/unittest/yaml/error-prefixlist1.yaml diff --git a/docs/config-guide.md b/docs/config-guide.md index 3beaaac..2f40390 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -363,7 +363,7 @@ interfaces: This construct allows to enumerate a list of IPv4 or IPv6 host addresses and/or networks. Each prefixlist has a name which consists of anywhere between 1 and 56 characters, and it must start -with a letter. The syntax is straight forward: +with a letter. The prefixlist name `any` is reserved. The syntax is straight forward: * ***description***: A string, no longer than 64 characters, and excluding the single quote ' and double quote ". This string is currently not used anywhere, and serves for enduser diff --git a/vppcfg/config/prefixlist.py b/vppcfg/config/prefixlist.py index 8160b60..bd8192e 100644 --- a/vppcfg/config/prefixlist.py +++ b/vppcfg/config/prefixlist.py @@ -113,6 +113,12 @@ def validate_prefixlists(yaml): for plname, pl in yaml["prefixlists"].items(): logger.debug(f"prefixlist {plname}: {pl}") + if plname in ["any"]: + ## Note: ACL 'source' and 'destination', when they are empty, will resolve + ## to 'any', and can thus never refer to a prefixlist called 'any'. + msgs.append(f"prefixlist {plname} is a reserved name") + result = False + members = 0 for pl_member in pl["members"]: members += 1 diff --git a/vppcfg/unittest/yaml/error-prefixlist1.yaml b/vppcfg/unittest/yaml/error-prefixlist1.yaml new file mode 100644 index 0000000..e000d9c --- /dev/null +++ b/vppcfg/unittest/yaml/error-prefixlist1.yaml @@ -0,0 +1,18 @@ +test: + description: "Some prefixlist names are reserved" + errors: + expected: + - "prefixlist any is a reserved name" + count: 1 +--- +prefixlists: + any: + description: "any is a reserved name" + members: + - 192.0.2.1 + - 192.0.2.0/24 + v6only: + members: + - 2001:db8::1 + - 2001:db8::/64 + - 2001:db8::/48 From a282a5358ad1d76611e47ec358269da514bfc1b0 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 14:24:36 +0000 Subject: [PATCH 13/46] 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 | 112 ++++++++++++++++---------- vppcfg/config/test_acl.py | 17 ++++ vppcfg/example.yaml | 8 ++ vppcfg/schema.yaml | 10 +-- vppcfg/unittest/yaml/correct-acl.yaml | 13 +++ vppcfg/unittest/yaml/error-acl1.yaml | 22 ----- vppcfg/unittest/yaml/error-acl2.yaml | 33 +++++++- 7 files changed, 144 insertions(+), 71 deletions(-) delete mode 100644 vppcfg/unittest/yaml/error-acl1.yaml diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index d7b477c..9f86b8b 100644 --- a/vppcfg/config/acl.py +++ b/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"] = "ipv6" - elif "destination" in acl_term and ":" in acl_term["destination"]: - acl_term["family"] = "ipv6" - elif "source" in acl_term and "." in acl_term["source"]: - 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" + acl_term["family"] = "any" + if "source" not in acl_term: + acl_term["source"] = "any" + if "destination" not in acl_term: + acl_term["destination"] = "any" 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 "source" in acl_term: - msgs.append( - f"acl {aclname} term {terms} family any cannot have source" - ) - result = False - if "destination" in acl_term: - msgs.append( - f"acl {aclname} term {terms} family any cannot have destination" - ) - result = False + if acl_term["family"] == "ipv4": + want_ipv4 = True + want_ipv6 = False + elif acl_term["family"] == "ipv6": + want_ipv4 = False + want_ipv6 = True else: - src = ipaddress.ip_network(acl_term["source"]) - dst = ipaddress.ip_network(acl_term["destination"]) - if src.version != dst.version: - msgs.append( - f"acl {aclname} term {terms} source and destination have different address family" - ) - result = False + want_ipv4 = True + want_ipv6 = True + + src_network_list = get_network_list( + yaml, acl_term["source"], want_ipv4=want_ipv4, want_ipv6=want_ipv6 + ) + 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 - - if proto in [6, 17]: + else: 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: diff --git a/vppcfg/config/test_acl.py b/vppcfg/config/test_acl.py index b699daa..6b719a4 100644 --- a/vppcfg/config/test_acl.py +++ b/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)) diff --git a/vppcfg/example.yaml b/vppcfg/example.yaml index b54fdd8..c11813d 100644 --- a/vppcfg/example.yaml +++ b/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 diff --git a/vppcfg/schema.yaml b/vppcfg/schema.yaml index 12d62db..d2fb830 100644 --- a/vppcfg/schema.yaml +++ b/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) - destination: any(ip(), ip_interface(),required=False) + source: any(ip(),ip_interface(),str(min=1,max=56),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) - destination-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) icmp-type: include('acl-term-icmp-int-range',required=False) icmp-code: include('acl-term-icmp-int-range',required=False) --- diff --git a/vppcfg/unittest/yaml/correct-acl.yaml b/vppcfg/unittest/yaml/correct-acl.yaml index 58e0611..9024b5a 100644 --- a/vppcfg/unittest/yaml/correct-acl.yaml +++ b/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 diff --git a/vppcfg/unittest/yaml/error-acl1.yaml b/vppcfg/unittest/yaml/error-acl1.yaml deleted file mode 100644 index a758e42..0000000 --- a/vppcfg/unittest/yaml/error-acl1.yaml +++ /dev/null @@ -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 diff --git a/vppcfg/unittest/yaml/error-acl2.yaml b/vppcfg/unittest/yaml/error-acl2.yaml index 3f18e00..a9404e4 100644 --- a/vppcfg/unittest/yaml/error-acl2.yaml +++ b/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" - count: 4 + - "acl .* term .* source and destination family do not overlap" + 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 From 5824af966622d436685b48fd252e5a321d905098 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 14:30:56 +0000 Subject: [PATCH 14/46] Add a unit test for empty src/dst --- vppcfg/unittest/yaml/error-acl2.yaml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/vppcfg/unittest/yaml/error-acl2.yaml b/vppcfg/unittest/yaml/error-acl2.yaml index a9404e4..e54c914 100644 --- a/vppcfg/unittest/yaml/error-acl2.yaml +++ b/vppcfg/unittest/yaml/error-acl2.yaml @@ -3,7 +3,9 @@ test: errors: expected: - "acl .* term .* source and destination family do not overlap" - count: 6 + - "acl .* term .* family any has no source" + - "acl .* term .* family any has no destination" + count: 8 --- prefixlists: v4only: @@ -15,6 +17,8 @@ prefixlists: - 2001:db8::1 - 2001:db8::/64 - 2001:db8::/48 + empty: + members: [] acls: acl01: @@ -43,6 +47,14 @@ acls: source: ::/0 destination: 192.168.0.0/16 action: permit + - description: "Error, can never match an empty prefixlist" + source: empty + destination: 192.0.2.1 + action: permit + - description: "Error, can never match an empty prefixlist" + source: 2001:db8::1 + destination: empty + action: permit - description: "OK" source: ::/0 destination: 2001:db8::1 From 9a175e1bbad53921e0b4a98643db44f718134b5a Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 14:36:05 +0000 Subject: [PATCH 15/46] Add an ACE with a an example prefixlist --- docs/config-guide.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/config-guide.md b/docs/config-guide.md index 2f40390..3852e89 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -433,12 +433,25 @@ packets then either perform an action of `permit` or `deny` (for stateless) or ` * ***icmp-code***: Similar to `icmp-type` but for the ICMP code field. This field can only be specified if the `protocol` field is `icmp` (or `1`). -An example ACL with three ACE terms: +An example ACL with four ACE terms: ``` +prefixlists: + example: + description: "An example prefixlist with hosts and prefixes" + members: + - 192.0.2.1 + - 192.0.2.0/24 + - 2001:db8::1 + - 2001:db8::/64 + acls: acl01: description: "Test ACL" terms: + - description: "Allow a prefixlist, but only for IPv6" + family: ipv6 + action: permit + source: example - description: "Allow a specific IPv6 TCP flow" action: permit source: 2001:db8::/64 From efef03ea4279841ab47de82e4fbeb4b4514015dc Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 14:41:07 +0000 Subject: [PATCH 16/46] address pylint --- vppcfg/config/prefixlist.py | 51 ++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/vppcfg/config/prefixlist.py b/vppcfg/config/prefixlist.py index bd8192e..c034d7c 100644 --- a/vppcfg/config/prefixlist.py +++ b/vppcfg/config/prefixlist.py @@ -13,7 +13,6 @@ # """ A vppcfg configuration module that validates prefixlists """ import logging -import socket import ipaddress @@ -41,11 +40,11 @@ def get_network_list(yaml, plname, want_ipv4=True, want_ipv6=True): in a prefixlist of given name. Return the empty list if the prefixlist doesn't exist. Optionally, want_ipv4 or want_ipv6 can be set to False to filter the list.""" ret = [] - plname, pl = get_by_name(yaml, plname) - if not pl: + plname, plist = get_by_name(yaml, plname) + if not plist: return ret - for m in pl["members"]: - ipn = ipaddress.ip_network(m, strict=False) + for member in plist["members"]: + ipn = ipaddress.ip_network(member, strict=False) if ipn.version == 4 and want_ipv4: ret.append(ipn) if ipn.version == 6 and want_ipv6: @@ -56,49 +55,49 @@ def get_network_list(yaml, plname, want_ipv4=True, want_ipv6=True): def count(yaml, plname): """Return the number of IPv4 and IPv6 entries in the prefixlist. Returns 0, 0 if it doesn't exist""" - v4, v6 = 0, 0 + ipv4, ipv6 = 0, 0 - plname, pl = get_by_name(yaml, plname) - if not pl: + plname, plist = get_by_name(yaml, plname) + if not plist: return 0, 0 - for m in pl["members"]: - ipn = ipaddress.ip_network(m, strict=False) + for member in plist["members"]: + ipn = ipaddress.ip_network(member, strict=False) if ipn.version == 4: - v4 += 1 + ipv4 += 1 elif ipn.version == 6: - v6 += 1 + ipv6 += 1 - return v4, v6 + return ipv4, ipv6 def count_ipv4(yaml, plname): """Return the number of IPv4 entries in the prefixlist.""" - v4, v6 = count(yaml, plname) - return v4 + ipv4, _ = count(yaml, plname) + return ipv4 def count_ipv6(yaml, plname): """Return the number of IPv6 entries in the prefixlist.""" - v4, v6 = count(yaml, plname) - return v6 + _, ipv6 = count(yaml, plname) + return ipv6 def has_ipv4(yaml, plname): """Return True if the prefixlist has at least one IPv4 entry.""" - v4, v6 = count(yaml, plname) - return v4 > 0 + ipv4, _ = count(yaml, plname) + return ipv4 > 0 def has_ipv6(yaml, plname): """Return True if the prefixlist has at least one IPv6 entry.""" - v4, v6 = count(yaml, plname) - return v6 > 0 + _, ipv6 = count(yaml, plname) + return ipv6 > 0 def is_empty(yaml, plname): """Return True if the prefixlist has no entries.""" - v4, v6 = count(yaml, plname) - return v4 + v6 == 0 + ipv4, ipv6 = count(yaml, plname) + return ipv4 + ipv6 == 0 def validate_prefixlists(yaml): @@ -111,8 +110,8 @@ def validate_prefixlists(yaml): if not "prefixlists" in yaml: return result, msgs - for plname, pl in yaml["prefixlists"].items(): - logger.debug(f"prefixlist {plname}: {pl}") + for plname, plist in yaml["prefixlists"].items(): + logger.debug(f"prefixlist {plname}: {plist}") if plname in ["any"]: ## Note: ACL 'source' and 'destination', when they are empty, will resolve ## to 'any', and can thus never refer to a prefixlist called 'any'. @@ -120,7 +119,7 @@ def validate_prefixlists(yaml): result = False members = 0 - for pl_member in pl["members"]: + for pl_member in plist["members"]: members += 1 logger.debug(f"prefixlist {plname} member {members} is {pl_member}") From 02ca2e22cdbac69c276ace4b3413137eead8d22f Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 17:12:48 +0000 Subject: [PATCH 17/46] acl: add dumper for acls A reasonable attempt will be made to shorten the output of terms, but due to the nature of the ACL plugin in VPP, all ACLs will be unrolled into their individual ACEs (called 'terms'). - src/dst-port will only be emitted with UDP/TCP - icmp-typc/code will only be emitted with ICMP/ICMPv6 - icmp-code/type and source/destination-ports ranges will be collapsed where appropriate. - if protocol is 0, only L3 information will be emitted NOTE: a bug in the VPP plugin will allow for ICMP 'sport' and 'dport' upper value to be 16 bits. If an ACE is retrieved from the dataplane regarding an ICMP or ICMPv6 (referring the 16 bit values to icmp type and code), they will be truncated and a warning issued. --- vppcfg/vpp/dumper.py | 89 ++++++++++++++++++++++++++++++++++++++++++++ vppcfg/vpp/vppapi.py | 29 +++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/vppcfg/vpp/dumper.py b/vppcfg/vpp/dumper.py index 1d31721..6c23dba 100644 --- a/vppcfg/vpp/dumper.py +++ b/vppcfg/vpp/dumper.py @@ -65,6 +65,8 @@ class Dumper(VPPApi): "bridgedomains": {}, "vxlan_tunnels": {}, "taps": {}, + "prefixlists": {}, + "acls": {}, } for idx, bond_iface in self.cache["bondethernets"].items(): bond = {"description": ""} @@ -246,5 +248,92 @@ class Dumper(VPPApi): bridge["interfaces"] = members bridge["mtu"] = mtu config["bridgedomains"][bridge_name] = bridge + for idx, acl in self.cache["acls"].items(): + aclname = f"vppacl{acl.acl_index}" + + config_acl = {"description": "", "terms": []} + terms = 0 + for acl_rule in acl.r: + terms += 1 + action = "deny" + if acl_rule.is_permit == 1: + action = "permit" + elif acl_rule.is_permit == 2: + action = "permit+reflect" + + config_term = { + "action": action, + "source": str(acl_rule.src_prefix), + "destination": str(acl_rule.dst_prefix), + } + if acl_rule.proto == 0: + pass + elif acl_rule.proto in [1, 58]: + if acl_rule.proto == 1: + config_term["protocol"] = "icmp" + else: + config_term["protocol"] = "ipv6-icmp" + maxval = acl_rule.srcport_or_icmptype_last + if maxval > 255: + self.logger.warning( + f"icmp type > 255 on acl {acl.acl_index} term {terms}" + ) + maxval = 255 + if acl_rule.srcport_or_icmptype_first == maxval: + config_term["icmp-type"] = int( + acl_rule.srcport_or_icmptype_first + ) + else: + config_term[ + "icmp-type" + ] = f"{acl_rule.srcport_or_icmptype_first}-{maxval}" + + maxval = acl_rule.dstport_or_icmpcode_last + if maxval > 255: + self.logger.warning( + f"icmp code > 255 on acl {acl.acl_index} term {terms}" + ) + maxval = 255 + if acl_rule.dstport_or_icmpcode_first == maxval: + config_term["icmp-code"] = int( + acl_rule.dstport_or_icmpcode_first + ) + else: + config_term[ + "icmp-code" + ] = f"{acl_rule.dstport_or_icmpcode_first}-{maxval}" + elif acl_rule.proto in [6, 17]: + if acl_rule.proto == 6: + config_term["protocol"] = "tcp" + else: + config_term["protocol"] = "udp" + if ( + acl_rule.srcport_or_icmptype_first + == acl_rule.srcport_or_icmptype_last + ): + config_term["source-port"] = int( + acl_rule.srcport_or_icmptype_first + ) + else: + config_term[ + "source-port" + ] = f"{acl_rule.srcport_or_icmptype_first}-{acl_rule.srcport_or_icmptype_last}" + if ( + acl_rule.dstport_or_icmpcode_first + == acl_rule.dstport_or_icmpcode_last + ): + config_term["destination-port"] = int( + acl_rule.dstport_or_icmpcode_first + ) + else: + config_term[ + "destination-port" + ] = f"{acl_rule.dstport_or_icmpcode_first}-{acl_rule.dstport_or_icmpcode_last}" + else: + config_term["protocol"] = int(acl_rule.proto) + + config_acl["terms"].append(config_term) + + config["acls"][aclname] = config_acl return config diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 942b9f6..57fa7af 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -119,12 +119,14 @@ class VPPApi: "interface_names": {}, "interfaces": {}, "interface_addresses": {}, + "interface_acls": {}, "bondethernets": {}, "bondethernet_members": {}, "bridgedomains": {}, "vxlan_tunnels": {}, "l2xcs": {}, "taps": {}, + "acls": {}, } return True @@ -196,6 +198,7 @@ class VPPApi: if len(self.cache["interface_addresses"][iface.sw_if_index]) > 0: self.logger.warning(f"Not all addresses were removed on {ifname}") del self.cache["interface_addresses"][iface.sw_if_index] + del self.cache["interface_acls"][iface.sw_if_index] del self.cache["interface_names"][ifname] ## Use my_dict.pop('key', None), as it allows 'key' to be absent @@ -246,6 +249,14 @@ class VPPApi: interface_dev_type="local", tag="mock", ) + self.cache["interface_acls"][idx] = self.vpp_messages[ + "acl_interface_list_details" + ].tuple( + sw_if_index=idx, + count=0, + n_input=0, + acls=[], + ) ## Add mock PHYs for ifname, iface in yaml_config["interfaces"].items(): if not "device-type" in iface or iface["device-type"] not in ["dpdk"]: @@ -277,6 +288,14 @@ class VPPApi: interface_dev_type=iface["device-type"], tag="mock", ) + self.cache["interface_acls"][idx] = self.vpp_messages[ + "acl_interface_list_details" + ].tuple( + sw_if_index=idx, + count=0, + n_input=0, + acls=[], + ) ## Create interface_names and interface_address indexes for idx, iface in self.cache["interfaces"].items(): @@ -332,6 +351,16 @@ class VPPApi: str(addr.prefix) ) + self.logger.debug("Retrieving ACLs") + api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) + for acl in api_response: + self.cache["acls"][acl.acl_index] = acl + + self.logger.debug("Retrieving interface ACLs") + api_response = self.vpp.api.acl_interface_list_dump() + for iface in api_response: + self.cache["interface_acls"][iface.sw_if_index] = iface + self.logger.debug("Retrieving bondethernets") api_response = self.vpp.api.sw_bond_interface_dump() for iface in api_response: From f654e78ed584a5bed30727452f71977a129f62b6 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 18:00:24 +0000 Subject: [PATCH 18/46] Fix pylint warning --- vppcfg/vpp/vppapi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 57fa7af..3071841 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -218,7 +218,7 @@ class VPPApi: enumerating the 'interfaces' scope from yaml_config""" if not "interfaces" in yaml_config: - self.logger.error(f"YAML config does not contain any interfaces") + self.logger.error("YAML config does not contain any interfaces") return False self.logger.debug(f"config: {yaml_config['interfaces']}") From ace08ac0526b6d0b0503d1016a0168eec235dc65 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 19:07:04 +0000 Subject: [PATCH 19/46] Refuse to work with ACLs if there are duplicate tags -- it means something/somebody has been inserting them outside of vppcfg, and this breaks the requirement that vppcfg.acls. is the same uniquely identified vpp.acl.tag --- vppcfg/vpp/vppapi.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 3071841..4b11a0e 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -127,6 +127,7 @@ class VPPApi: "l2xcs": {}, "taps": {}, "acls": {}, + "acl_tags": {}, } return True @@ -355,6 +356,12 @@ class VPPApi: api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) for acl in api_response: self.cache["acls"][acl.acl_index] = acl + if acl.tag in self.cache["acl_tags"]: + self.logger.error( + f"Duplicate ACL tag '{acl.tag}' found - cannot safely preoceed, bailing" + ) + return False + self.cache["acl_tags"][acl.tag] = acl.acl_index self.logger.debug("Retrieving interface ACLs") api_response = self.vpp.api.acl_interface_list_dump() From 16e946c92c14fb782fafbd532331cfecfb5b65ed Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 19:12:33 +0000 Subject: [PATCH 20/46] Copy over the acl.tag into the description when dumping --- vppcfg/foo | 115 +++++++++++++++++++++++++++++++++++++++++++ vppcfg/vpp/dumper.py | 3 +- 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 vppcfg/foo diff --git a/vppcfg/foo b/vppcfg/foo new file mode 100644 index 0000000..6dcc196 --- /dev/null +++ b/vppcfg/foo @@ -0,0 +1,115 @@ +acls: + vppacl0: + description: tag tag0 + terms: + - action: permit + destination: 0.0.0.0/0 + source: 0.0.0.0/0 + vppacl1: + description: tag tag1 + terms: + - action: deny + destination: 0.0.0.0/0 + source: 0.0.0.0/0 + vppacl2: + description: tag tag2 + terms: + - action: permit + destination: 0.0.0.0/0 + icmp-code: 0-255 + icmp-type: 0-255 + protocol: icmp + source: 0.0.0.0/0 + vppacl3: + description: tag tag3 + terms: + - action: permit + destination: 0.0.0.0/0 + destination-port: 80 + protocol: tcp + source: 0.0.0.0/0 + source-port: 1024-65535 + vppacl4: + description: tag tag4 + terms: + - action: permit + destination: 0.0.0.0/0 + destination-port: 80 + protocol: tcp + source: 0.0.0.0/0 + source-port: 1024-65535 + vppacl5: + description: tag tag5 + terms: + - action: permit + destination: 2001:db8::1/128 + destination-port: 80 + protocol: tcp + source: ::/0 + source-port: 1024-65535 + - action: permit + destination: 0.0.0.0/0 + source: 0.0.0.0/0 +bondethernets: + BondEthernet0: + description: '' + interfaces: + - GigabitEthernet3/0/0 + - GigabitEthernet3/0/1 + load-balance: l2 + mac: 02:fe:0f:f0:a7:bb + mode: lacp +bridgedomains: {} +interfaces: + BondEthernet0: + description: '' + lcp: be0 + mtu: 9000 + sub-interfaces: + 100: + description: '' + encapsulation: + dot1q: 100 + exact-match: false + l2xc: BondEthernet0.200 + mtu: 2500 + 200: + description: '' + encapsulation: + dot1q: 200 + exact-match: false + l2xc: BondEthernet0.100 + mtu: 2500 + 500: + description: '' + encapsulation: + dot1ad: 500 + exact-match: false + mtu: 2000 + 501: + description: '' + encapsulation: + dot1ad: 501 + exact-match: false + mtu: 2000 + GigabitEthernet3/0/0: + description: '' + mac: 00:25:90:0c:05:00 + mtu: 9000 + GigabitEthernet3/0/1: + description: '' + mac: 00:25:90:0c:05:00 + mtu: 9000 + HundredGigabitEthernet13/0/0: + description: '' + mac: b4:96:91:b3:b1:10 + mtu: 1500 + HundredGigabitEthernet13/0/1: + description: '' + mac: b4:96:91:b3:b1:11 + mtu: 1500 +loopbacks: {} +prefixlists: {} +taps: {} +vxlan_tunnels: {} + diff --git a/vppcfg/vpp/dumper.py b/vppcfg/vpp/dumper.py index 6c23dba..c3faebf 100644 --- a/vppcfg/vpp/dumper.py +++ b/vppcfg/vpp/dumper.py @@ -251,7 +251,8 @@ class Dumper(VPPApi): for idx, acl in self.cache["acls"].items(): aclname = f"vppacl{acl.acl_index}" - config_acl = {"description": "", "terms": []} + descr = "tag " + acl.tag.replace('"', "").replace("'", "") + config_acl = {"description": descr, "terms": []} terms = 0 for acl_rule in acl.r: terms += 1 From 5fd2d0859cda593085f0d56beedd911f8ee95214 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 19:52:46 +0000 Subject: [PATCH 21/46] Remove dangling file --- vppcfg/foo | 115 ----------------------------------------------------- 1 file changed, 115 deletions(-) delete mode 100644 vppcfg/foo diff --git a/vppcfg/foo b/vppcfg/foo deleted file mode 100644 index 6dcc196..0000000 --- a/vppcfg/foo +++ /dev/null @@ -1,115 +0,0 @@ -acls: - vppacl0: - description: tag tag0 - terms: - - action: permit - destination: 0.0.0.0/0 - source: 0.0.0.0/0 - vppacl1: - description: tag tag1 - terms: - - action: deny - destination: 0.0.0.0/0 - source: 0.0.0.0/0 - vppacl2: - description: tag tag2 - terms: - - action: permit - destination: 0.0.0.0/0 - icmp-code: 0-255 - icmp-type: 0-255 - protocol: icmp - source: 0.0.0.0/0 - vppacl3: - description: tag tag3 - terms: - - action: permit - destination: 0.0.0.0/0 - destination-port: 80 - protocol: tcp - source: 0.0.0.0/0 - source-port: 1024-65535 - vppacl4: - description: tag tag4 - terms: - - action: permit - destination: 0.0.0.0/0 - destination-port: 80 - protocol: tcp - source: 0.0.0.0/0 - source-port: 1024-65535 - vppacl5: - description: tag tag5 - terms: - - action: permit - destination: 2001:db8::1/128 - destination-port: 80 - protocol: tcp - source: ::/0 - source-port: 1024-65535 - - action: permit - destination: 0.0.0.0/0 - source: 0.0.0.0/0 -bondethernets: - BondEthernet0: - description: '' - interfaces: - - GigabitEthernet3/0/0 - - GigabitEthernet3/0/1 - load-balance: l2 - mac: 02:fe:0f:f0:a7:bb - mode: lacp -bridgedomains: {} -interfaces: - BondEthernet0: - description: '' - lcp: be0 - mtu: 9000 - sub-interfaces: - 100: - description: '' - encapsulation: - dot1q: 100 - exact-match: false - l2xc: BondEthernet0.200 - mtu: 2500 - 200: - description: '' - encapsulation: - dot1q: 200 - exact-match: false - l2xc: BondEthernet0.100 - mtu: 2500 - 500: - description: '' - encapsulation: - dot1ad: 500 - exact-match: false - mtu: 2000 - 501: - description: '' - encapsulation: - dot1ad: 501 - exact-match: false - mtu: 2000 - GigabitEthernet3/0/0: - description: '' - mac: 00:25:90:0c:05:00 - mtu: 9000 - GigabitEthernet3/0/1: - description: '' - mac: 00:25:90:0c:05:00 - mtu: 9000 - HundredGigabitEthernet13/0/0: - description: '' - mac: b4:96:91:b3:b1:10 - mtu: 1500 - HundredGigabitEthernet13/0/1: - description: '' - mac: b4:96:91:b3:b1:11 - mtu: 1500 -loopbacks: {} -prefixlists: {} -taps: {} -vxlan_tunnels: {} - From c190dfbd2bdde8e53dec1bb9428e14c4212d0083 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 20:57:49 +0000 Subject: [PATCH 22/46] Add a warning in case the tag contains ' or " characters --- vppcfg/vpp/dumper.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/vppcfg/vpp/dumper.py b/vppcfg/vpp/dumper.py index c3faebf..15cd7b4 100644 --- a/vppcfg/vpp/dumper.py +++ b/vppcfg/vpp/dumper.py @@ -251,7 +251,12 @@ class Dumper(VPPApi): for idx, acl in self.cache["acls"].items(): aclname = f"vppacl{acl.acl_index}" - descr = "tag " + acl.tag.replace('"', "").replace("'", "") + descr = acl.tag.replace('"', "").replace("'", "") + if descr != acl.tag: + self.logger.warning( + f"acl tag {acl.tag} has invalid characters, stripping" + ) + descr = "tag " + descr config_acl = {"description": descr, "terms": []} terms = 0 for acl_rule in acl.r: From 7914659fa5d3efe08d8cb5f6bb0e9441445a6ffb Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 21:09:08 +0000 Subject: [PATCH 23/46] icmp-type/code also match for proto 58 (ipv6-icmp) --- docs/config-guide.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/config-guide.md b/docs/config-guide.md index 3852e89..f94667a 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -429,9 +429,9 @@ packets then either perform an action of `permit` or `deny` (for stateless) or ` and/or end ranges (eg. `-10` for all types from 0-10 inclusive; or `10-` for all types from 10-255 inclusive, or an actual range `10-15`). The default keyword `any` is also permitted, which results in range `0-255`, and is the default if the field is not specified. This field - can only be specified if the `protocol` field is `icmp` (or `1`). + can only be specified if the `protocol` field is `icmp` (1) or `ipv6-icmp` (58). * ***icmp-code***: Similar to `icmp-type` but for the ICMP code field. This field can only be - specified if the `protocol` field is `icmp` (or `1`). + specified if the `protocol` field is `icmp` (1) or `ipv6-icmp` (58). An example ACL with four ACE terms: ``` From 818c45e09c17e3fff53d581eb97fbd4609d5d39b Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 22:20:41 +0000 Subject: [PATCH 24/46] acl: consistency in error messages, reformatted, and updated unittests --- vppcfg/config/acl.py | 24 ++++++++++++------------ vppcfg/unittest/yaml/error-acl2.yaml | 3 +-- vppcfg/unittest/yaml/error-acl3.yaml | 11 ++++------- vppcfg/unittest/yaml/error-acl5.yaml | 8 ++++---- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/vppcfg/config/acl.py b/vppcfg/config/acl.py index 9f86b8b..6838b71 100644 --- a/vppcfg/config/acl.py +++ b/vppcfg/config/acl.py @@ -303,57 +303,57 @@ def validate_acls(yaml): if src_low_port is None or src_high_port is None: msgs.append( - f"acl {aclname} term {terms} could not understand source port" + f"acl {aclname} term {terms} could not understand source-port" ) result = False else: if src_low_port > src_high_port: msgs.append( - f"acl {aclname} term {terms} source low port is higher than source high port" + f"acl {aclname} term {terms} source-port low value is greater than high value" ) result = False if src_low_port < 0 or src_low_port > 65535: msgs.append( - f"acl {aclname} term {terms} source low port is not between [0,65535]" + f"acl {aclname} term {terms} source-port low value is not between [0,65535]" ) result = False if src_high_port < 0 or src_high_port > 65535: msgs.append( - f"acl {aclname} term {terms} source high port is not between [0,65535]" + f"acl {aclname} term {terms} source-port high value is not between [0,65535]" ) result = False if dst_low_port is None or dst_high_port is None: msgs.append( - f"acl {aclname} term {terms} could not understand destination port" + f"acl {aclname} term {terms} could not understand destination-port" ) result = False else: if dst_low_port > dst_high_port: msgs.append( - f"acl {aclname} term {terms} destination low port is higher than destination high port" + f"acl {aclname} term {terms} destination-port low value is greater than high value" ) result = False if dst_low_port < 0 or dst_low_port > 65535: msgs.append( - f"acl {aclname} term {terms} destination low port is not between [0,65535]" + f"acl {aclname} term {terms} destination-port low value is not between [0,65535]" ) result = False if dst_high_port < 0 or dst_high_port > 65535: msgs.append( - f"acl {aclname} term {terms} destination high port is not between [0,65535]" + f"acl {aclname} term {terms} destination-port high value is not between [0,65535]" ) result = False if not proto in [1, 58]: if "icmp-code" in orig_acl_term: msgs.append( - f"acl {aclname} term {terms} icmp-code can only be specified for protocol icmp or icmp-ipv6" + f"acl {aclname} term {terms} icmp-code can only be specified for protocol icmp or ipv6-icmp" ) result = False if "icmp-type" in orig_acl_term: msgs.append( - f"acl {aclname} term {terms} icmp-type can only be specified for protocol icmp or icmp-ipv6" + f"acl {aclname} term {terms} icmp-type can only be specified for protocol icmp or ipv6-icmp" ) result = False else: @@ -361,12 +361,12 @@ def validate_acls(yaml): icmp_type_low, icmp_type_high = get_icmp_low_high(acl_term["icmp-type"]) if icmp_code_low > icmp_code_high: msgs.append( - f"acl {aclname} term {terms} icmp-code low value is higher than high value" + f"acl {aclname} term {terms} icmp-code low value is greater than high value" ) result = False if icmp_type_low > icmp_type_high: msgs.append( - f"acl {aclname} term {terms} icmp-type low value is higher than high value" + f"acl {aclname} term {terms} icmp-type low value is greater than high value" ) result = False diff --git a/vppcfg/unittest/yaml/error-acl2.yaml b/vppcfg/unittest/yaml/error-acl2.yaml index e54c914..1f749ec 100644 --- a/vppcfg/unittest/yaml/error-acl2.yaml +++ b/vppcfg/unittest/yaml/error-acl2.yaml @@ -3,8 +3,7 @@ test: errors: expected: - "acl .* term .* source and destination family do not overlap" - - "acl .* term .* family any has no source" - - "acl .* term .* family any has no destination" + - "acl .* term .* family any has no (source|destination)" count: 8 --- prefixlists: diff --git a/vppcfg/unittest/yaml/error-acl3.yaml b/vppcfg/unittest/yaml/error-acl3.yaml index fb16337..74f75bb 100644 --- a/vppcfg/unittest/yaml/error-acl3.yaml +++ b/vppcfg/unittest/yaml/error-acl3.yaml @@ -2,13 +2,10 @@ test: description: "Ways in which port ranges can fail" errors: expected: - - "acl .* term .* could not understand source port" - - "acl .* term .* could not understand destination port" - - "acl .* term .* source low port is higher than source high port" - - "acl .* term .* source (high|low) port is not between \\[0,65535\\]" - - "acl .* term .* destination (high|low) port is not between \\[0,65535\\]" - - "acl .* term .* source-port can only be specified for protocol tcp or udp" - - "acl .* term .* destination-port can only be specified for protocol tcp or udp" + - "acl .* term .* could not understand (source|destination)-port" + - "acl .* term .* (source|destination)-port low value is greater than high value" + - "acl .* term .* (source|destination)-port (low|high) value is not between \\[0,65535\\]" + - "acl .* term .* (source|destination)-port can only be specified for protocol tcp or udp" count: 7 --- acls: diff --git a/vppcfg/unittest/yaml/error-acl5.yaml b/vppcfg/unittest/yaml/error-acl5.yaml index c212978..5b2eaaa 100644 --- a/vppcfg/unittest/yaml/error-acl5.yaml +++ b/vppcfg/unittest/yaml/error-acl5.yaml @@ -2,10 +2,10 @@ test: description: "Ways in which ICMP code and type can fail" errors: expected: - - "acl .* term .* icmp-type can only be specified for protocol icmp or icmp-ipv6" - - "acl .* term .* icmp-code can only be specified for protocol icmp or icmp-ipv6" - - "acl .* term .* icmp-code low value is higher than high value" - - "acl .* term .* icmp-type low value is higher than high value" + - "acl .* term .* icmp-type can only be specified for protocol icmp or ipv6-icmp" + - "acl .* term .* icmp-code can only be specified for protocol icmp or ipv6-icmp" + - "acl .* term .* icmp-code low value is greater than high value" + - "acl .* term .* icmp-type low value is greater than high value" count: 8 --- acls: From b890a08c7e61556e6cedd131552efb80e19367aa Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 22:22:27 +0000 Subject: [PATCH 25/46] Collapse the error messages to force consistency --- vppcfg/unittest/yaml/error-acl5.yaml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vppcfg/unittest/yaml/error-acl5.yaml b/vppcfg/unittest/yaml/error-acl5.yaml index 5b2eaaa..ff1916c 100644 --- a/vppcfg/unittest/yaml/error-acl5.yaml +++ b/vppcfg/unittest/yaml/error-acl5.yaml @@ -2,10 +2,8 @@ test: description: "Ways in which ICMP code and type can fail" errors: expected: - - "acl .* term .* icmp-type can only be specified for protocol icmp or ipv6-icmp" - - "acl .* term .* icmp-code can only be specified for protocol icmp or ipv6-icmp" - - "acl .* term .* icmp-code low value is greater than high value" - - "acl .* term .* icmp-type low value is greater than high value" + - "acl .* term .* icmp-(type|code) can only be specified for protocol icmp or ipv6-icmp" + - "acl .* term .* icmp-(type|code) low value is greater than high value" count: 8 --- acls: From 8cf915e873218d8ea871ddc4b34a613ec62e7076 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sat, 25 Feb 2023 13:15:24 +0100 Subject: [PATCH 26/46] Bugfix: Run vppcfg plan --novpp cleanly with bondethernet and MAC addresses --- vppcfg/vpp/reconciler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vppcfg/vpp/reconciler.py b/vppcfg/vpp/reconciler.py index 4cee762..504b3db 100644 --- a/vppcfg/vpp/reconciler.py +++ b/vppcfg/vpp/reconciler.py @@ -1017,10 +1017,11 @@ class Reconciler: if not member_iface or member_ifname not in vpp_members: if ( len(vpp_members) == 0 + and member_iface and member_iface.l2_address != "00:00:00:00:00:00" ): bondmac = member_iface.l2_address - cli = f"bond add {config_bond_ifname} {member_iface.interface_name}" + cli = f"bond add {config_bond_ifname} {member_ifname}" self.cli["sync"].append(cli) if ( vpp_iface From 3249432681be5d4d01c402597c02278224b0007e Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sat, 25 Feb 2023 13:37:42 +0100 Subject: [PATCH 27/46] Move to checkout@v3 for node 16 --- .github/workflows/black.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml index a25f477..eff29ab 100644 --- a/.github/workflows/black.yml +++ b/.github/workflows/black.yml @@ -6,7 +6,7 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: psf/black@stable with: options: "--check --verbose --diff" From cf5f1f094441d48e0abd7a8186d41360651b38c5 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Thu, 25 May 2023 16:54:01 +0200 Subject: [PATCH 28/46] Add device-type, to ensure that plan --novpp generates MTU statements --- docs/config-guide.md | 3 +++ docs/user-guide.md | 3 +++ vppcfg/config/__init__.py | 3 ++- vppcfg/example.yaml | 4 ++++ 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/config-guide.md b/docs/config-guide.md index f94667a..b7fd14f 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -304,6 +304,8 @@ exist as a PHY in VPP (ie. `HundredGigabitEthernet12/0/0`) or as a specified `Bo target interface. * ***state***: An optional string that configures the link admin state, either `up` or `down`. If it is not specified, the link is considered admin 'up'. +* ***device-type***: An optional interface type in VPP. Currently the only supported vlaue is + `dpdk`, and it is used to generate correct mock interfaces if the `--novpp` flag is used. Further, top-level interfaces, that is to say those that do not have an encapsulation, are permitted to have any number of sub-interfaces specified by `subid`, an integer between [0,2G), which further @@ -324,6 +326,7 @@ Examples: ``` interfaces: HundredGigabitEthernet12/0/0: + device-type: dpdk lcp: "ice0" mtu: 9000 addresses: [ 192.0.2.1/30, 2001:db8:1::1/64 ] diff --git a/docs/user-guide.md b/docs/user-guide.md index c40c894..51612b6 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -271,6 +271,9 @@ with `head.vpp` (think of things like custom logging, plugin defaults, DPDK affi then letting `vppcfg` do its part, and finally leaving the ability to also program the dataplane with things that `vppcfg` does not (yet) support in `tail.vpp`. +***NOTE***: For MTU values to be generated in `--novpp` mode, the interface device type must be +set (typically using `device-type: dpdk` in the PHY interface definition). + ### vppcfg apply Applying state is not (yet) implemented. Don't worry, it's not much work, but this is punted until diff --git a/vppcfg/config/__init__.py b/vppcfg/config/__init__.py index e8221e8..d7b469e 100644 --- a/vppcfg/config/__init__.py +++ b/vppcfg/config/__init__.py @@ -77,7 +77,8 @@ class Validator: The purpose is to ensure that the YAML file is both syntactically correct, which is ensured by Yamale, and semantically correct, which is ensured by a set - of built-in validators, and user-added validators (see the add_validator() method).""" + of built-in validators, and user-added validators (see the add_validator() method). + """ def __init__(self, schema): self.logger = logging.getLogger("vppcfg.config") diff --git a/vppcfg/example.yaml b/vppcfg/example.yaml index c11813d..6df2d61 100644 --- a/vppcfg/example.yaml +++ b/vppcfg/example.yaml @@ -7,13 +7,16 @@ bondethernets: interfaces: GigabitEthernet3/0/0: + device-type: dpdk mtu: 9000 description: "LAG #1" GigabitEthernet3/0/1: + device-type: dpdk mtu: 9000 description: "LAG #2" HundredGigabitEthernet12/0/0: + device-type: dpdk lcp: "ice12-0-0" mac: f2:01:00:12:00:00 mtu: 9000 @@ -34,6 +37,7 @@ interfaces: exact-match: True HundredGigabitEthernet12/0/1: + device-type: dpdk mtu: 2000 description: "Bridged" From f961e41ce6984a0b3d3cc33f112d43228e2999b4 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Thu, 25 May 2023 18:24:46 +0200 Subject: [PATCH 29/46] Add address.get_canonical() and is_canonical() These functions will take either an IPv4/IPv6 address, or an IPv4/IPv6 prefix, and cast them to their canonical form. Notably for IPv6 addresses, this means lower case and with the 0-tuples correctly formatted: 2001:DB8::1 becomes 2001:db8::1 2001:db8:0:0::1 becomes 2001:db8::1 This avoids spurious diffs in vppcfg when comparing to the output of the VPP dataplane. --- vppcfg/config/address.py | 30 +++++++++++++++ vppcfg/config/interface.py | 7 ++++ vppcfg/config/test_address.py | 44 ++++++++++++++++++++++ vppcfg/example.yaml | 2 +- vppcfg/unittest/yaml/correct-example1.yaml | 2 +- vppcfg/unittest/yaml/error-address1.yaml | 8 +++- 6 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 vppcfg/config/test_address.py diff --git a/vppcfg/config/address.py b/vppcfg/config/address.py index 2afa3d9..2e1fb48 100644 --- a/vppcfg/config/address.py +++ b/vppcfg/config/address.py @@ -113,3 +113,33 @@ def is_allowed(yaml, ifname, iface_addresses, ip_interface): return False return True + + +def get_canonical(iface_address): + """Returns the canonical form of an interface address, which can be either an address + '2001:db8::1' or a prefix '2001:db8::1/64' for either IPv4 of IPv6. + + This function the string representation of the canonical address.""" + + is_prefix = "/" in iface_address + if is_prefix: + iface_address, prefixlen = iface_address.split("/") + + ip_address = ipaddress.ip_address(iface_address) + if is_prefix: + return str(ip_address) + "/" + prefixlen + return str(ip_address) + + +def is_canonical(iface_address): + """Checks to see that the interface address is written in a canonical form, useful to + ensure that IPv6 addresses are written in such a way that they don't trigger spurious + diffs when comparing to VPP. As an example, '2001:DB8:0:0::1/64' is fine, but VPP will + show this as '2001:db8::1/64'. + + Input can be either an address 2001:db8::1 or a prefix 2001:db8::1/128 + + This function returns False if the iface_address isn't canonical, and True otherwise. + """ + can = get_canonical(iface_address) + return can == iface_address diff --git a/vppcfg/config/interface.py b/vppcfg/config/interface.py index 4755a93..5d73560 100644 --- a/vppcfg/config/interface.py +++ b/vppcfg/config/interface.py @@ -13,6 +13,7 @@ # """ A vppcfg configuration module that validates interfaces """ import logging +import ipaddress from . import bondethernet from . import bridgedomain from . import loopback @@ -500,6 +501,12 @@ def validate_interfaces(yaml): f"interface {ifname} IP address {addr} conflicts with another" ) result = False + if not address.is_canonical(addr): + canonical = address.get_canonical(addr) + msgs.append( + f"interface {ifname} IP address {addr} is not canonical, use {canonical}" + ) + result = False if "l2xc" in iface: if has_sub(yaml, ifname): diff --git a/vppcfg/config/test_address.py b/vppcfg/config/test_address.py new file mode 100644 index 0000000..dd4c354 --- /dev/null +++ b/vppcfg/config/test_address.py @@ -0,0 +1,44 @@ +# +# Copyright (c) 2022 Pim van Pelt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at: +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# -*- coding: utf-8 -*- +""" Unit tests for addresses """ +import unittest +import yaml +from . import address + + +class TestAddressMethods(unittest.TestCase): + def test_get_canonical(self): + self.assertEqual(address.get_canonical("0.0.0.0"), "0.0.0.0") + self.assertEqual(address.get_canonical("0.0.0.0/0"), "0.0.0.0/0") + self.assertEqual(address.get_canonical("192.168.1.1"), "192.168.1.1") + self.assertEqual(address.get_canonical("192.168.1.1/32"), "192.168.1.1/32") + self.assertEqual(address.get_canonical("2001:db8::1"), "2001:db8::1") + self.assertEqual(address.get_canonical("2001:db8::1/64"), "2001:db8::1/64") + + self.assertEqual(address.get_canonical("2001:dB8::1/128"), "2001:db8::1/128") + self.assertEqual(address.get_canonical("2001:db8:0::1/128"), "2001:db8::1/128") + self.assertEqual(address.get_canonical("2001:db8::0:1"), "2001:db8::1") + + def test_is_canonical(self): + self.assertTrue(address.is_canonical("0.0.0.0")) + self.assertTrue(address.is_canonical("0.0.0.0/0")) + self.assertTrue(address.is_canonical("192.168.1.1")) + self.assertTrue(address.is_canonical("192.168.1.1/32")) + self.assertTrue(address.is_canonical("2001:db8::1")) + self.assertTrue(address.is_canonical("2001:db8::1/64")) + + self.assertFalse(address.is_canonical("2001:dB8::1/128")) # Capitals + self.assertFalse(address.is_canonical("2001:db8:0::1/128")) # Spurious 0 + self.assertFalse(address.is_canonical("2001:db8::0:1")) # Spurious 0 diff --git a/vppcfg/example.yaml b/vppcfg/example.yaml index 6df2d61..a5990d8 100644 --- a/vppcfg/example.yaml +++ b/vppcfg/example.yaml @@ -20,7 +20,7 @@ interfaces: lcp: "ice12-0-0" mac: f2:01:00:12:00:00 mtu: 9000 - addresses: [ 192.0.2.17/30, 2001:db8:3::1/64 ] + addresses: [ 192.0.2.17/30, 2001:DB8:3::1/64 ] sub-interfaces: 1234: mtu: 1200 diff --git a/vppcfg/unittest/yaml/correct-example1.yaml b/vppcfg/unittest/yaml/correct-example1.yaml index 41b8057..19214cf 100644 --- a/vppcfg/unittest/yaml/correct-example1.yaml +++ b/vppcfg/unittest/yaml/correct-example1.yaml @@ -13,7 +13,7 @@ interfaces: GigabitEthernet1/0/0: description: "Infra: nikhef-core-1.nl.switch.coloclue.net e1/34" lcp: e0-0 - addresses: [ 94.142.244.85/24, 2A02:898::146:1/64 ] + addresses: [ 94.142.244.85/24, 2a02:898::146:1/64 ] sub-interfaces: 100: description: "Cust: hvn0.nlams0.ipng.ch" diff --git a/vppcfg/unittest/yaml/error-address1.yaml b/vppcfg/unittest/yaml/error-address1.yaml index daf8701..b9645bd 100644 --- a/vppcfg/unittest/yaml/error-address1.yaml +++ b/vppcfg/unittest/yaml/error-address1.yaml @@ -5,7 +5,8 @@ test: - "interface .* IP address .* conflicts with another" - "sub-interface .* IP address .* conflicts with another" - "loopback .* IP address .* conflicts with another" - count: 14 + - "interface .* IP address .* is not canonical, use .*" + count: 15 --- interfaces: GigabitEthernet1/0/0: @@ -15,7 +16,7 @@ interfaces: GigabitEthernet1/0/1: lcp: e1-0-1 - addresses: [ 192.0.2.1/29, 2001:db8:1::1/64 ] + addresses: [ 192.0.2.1/29, 2001:DB8:1::1/64 ] sub-interfaces: 100: description: "These addresses overlap with Gi1/0/1" @@ -23,6 +24,9 @@ interfaces: 101: description: "These addresses overlap with loop0" addresses: [ 192.0.2.10/29, 2001:db8:2::2/64 ] + 102: + description: "This address is not canonical" + addresses: [ 2001:DB8:5::1/64 ] GigabitEthernet1/0/2: lcp: e0-2 From 367aad8dbe6fbee6c5bc4ca7f8e481c1682113a5 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Thu, 25 May 2023 18:24:46 +0200 Subject: [PATCH 30/46] Add address.get_canonical() and is_canonical() These functions will take either an IPv4/IPv6 address, or an IPv4/IPv6 prefix, and cast them to their canonical form. Notably for IPv6 addresses, this means lower case and with the 0-tuples correctly formatted: 2001:DB8::1 becomes 2001:db8::1 2001:db8:0:0::1 becomes 2001:db8::1 This avoids spurious diffs in vppcfg when comparing to the output of the VPP dataplane. --- vppcfg/config/loopback.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vppcfg/config/loopback.py b/vppcfg/config/loopback.py index 5db307f..a5f1d2d 100644 --- a/vppcfg/config/loopback.py +++ b/vppcfg/config/loopback.py @@ -83,6 +83,12 @@ def validate_loopbacks(yaml): f"loopback {ifname} IP address {addr} conflicts with another" ) result = False + if not address.is_canonical(addr): + canonical = address.get_canonical(addr) + msgs.append( + f"loopback {ifname} IP address {addr} is not canonical, use {canonical}" + ) + result = False if "mac" in iface and mac.is_multicast(iface["mac"]): msgs.append( f"loopback {ifname} MAC address {iface['mac']} cannot be multicast" From 8077d20bc7527c4ce49f3590ee113fb2368c18de Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Thu, 25 May 2023 18:29:35 +0200 Subject: [PATCH 31/46] Cut 0.0.4 --- debian/changelog | 7 +++++++ setup.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/debian/changelog b/debian/changelog index d1c0049..c4de77b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +vppcfg (0.0.4) unstable; urgency=low + + Feature release: + * Bugfix, ensuring IPv6 addresses are in their canonical form, to avoid + spurious diffs when comparing to VPP dataplane addresses. + * Document the 'device-type' field for interfaces + -- Pim van Pelt Sat, 03 Dec 2022 14:15:00 +0000 vppcfg (0.0.3) unstable; urgency=low Feature release: diff --git a/setup.py b/setup.py index 17c2393..32bf82d 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ from setuptools import setup setup( name="vppcfg", - version="0.0.3", + version="0.0.4", install_requires=[ "requests", 'importlib-metadata; python_version >= "3.8"', From b442bc1aae36d2ad9ee36aa51e64b21628b38492 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Thu, 25 May 2023 18:32:54 +0200 Subject: [PATCH 32/46] Fix some formatting issues --- vppcfg/tests.py | 3 ++- vppcfg/vpp/applier.py | 3 ++- vppcfg/vpp/reconciler.py | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/vppcfg/tests.py b/vppcfg/tests.py index 7958148..fcbc076 100755 --- a/vppcfg/tests.py +++ b/vppcfg/tests.py @@ -44,7 +44,8 @@ def example_validator(_yaml): class YAMLTest(unittest.TestCase): """This test suite takes a YAML configuration file and holds it against the syntax - (Yamale) and semantic validators, returning errors in case of validation failures.""" + (Yamale) and semantic validators, returning errors in case of validation failures. + """ def __init__(self, testName, yaml_filename, yaml_schema): # calling the super class init varies for different python versions. This works for 2.7 diff --git a/vppcfg/vpp/applier.py b/vppcfg/vpp/applier.py index 9934bb8..c160da8 100644 --- a/vppcfg/vpp/applier.py +++ b/vppcfg/vpp/applier.py @@ -125,7 +125,8 @@ class Applier(VPPApi): def lcp_create(self, ifname, host_if_name): """Create a linux control plane interface pair for an interface given by name - (ie GigabitEthernet3/0/0) under a Linux TAP device name host_if_name (ie e3-0-0)""" + (ie GigabitEthernet3/0/0) under a Linux TAP device name host_if_name (ie e3-0-0) + """ pass def set_interface_mac(self, ifname, mac): diff --git a/vppcfg/vpp/reconciler.py b/vppcfg/vpp/reconciler.py index 504b3db..e3d0b7f 100644 --- a/vppcfg/vpp/reconciler.py +++ b/vppcfg/vpp/reconciler.py @@ -181,7 +181,8 @@ class Reconciler: def __prune_bridgedomains(self): """Remove bridge-domains from VPP, if they do not occur in the config. If any interfaces are - found in to-be removed bridge-domains, they are returned to L3 mode, and tag-rewrites removed.""" + found in to-be removed bridge-domains, they are returned to L3 mode, and tag-rewrites removed. + """ for idx, bridge in self.vpp.cache["bridgedomains"].items(): bridgename = f"bd{int(idx)}" _config_ifname, config_iface = bridgedomain.get_by_name( From 0f27a15bee695caf82f35840149d44d0930b64ff Mon Sep 17 00:00:00 2001 From: najieb Date: Wed, 31 May 2023 10:04:36 +0700 Subject: [PATCH 33/46] Update schema.yaml --- vppcfg/schema.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vppcfg/schema.yaml b/vppcfg/schema.yaml index d2fb830..5bd935e 100644 --- a/vppcfg/schema.yaml +++ b/vppcfg/schema.yaml @@ -39,7 +39,7 @@ loopback: bondethernet: description: str(exclude='\'"',len=64,required=False) mac: mac(required=False) - interfaces: list(str(matches='.*GigabitEthernet[0-9]+/[0-9]+/[0-9]+'),required=False) + interfaces: list(str(matches='.*GigabitEthernet[0-9a-z]+/[0-9]+/[0-9]+'),required=False) mode: enum('round-robin','active-backup','broadcast','lacp','xor',required=False) load-balance: enum('l2','l23','l34',required=False) --- From 52b8cb5477da13a26253464b50bd07a88a6fef81 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sat, 10 Jun 2023 15:31:09 +0200 Subject: [PATCH 34/46] Add MPLS config option and interface.is_mpls() Also add tests and documentation --- docs/config-guide.md | 4 ++++ vppcfg/config/interface.py | 11 +++++++++++ vppcfg/config/test_interface.py | 7 +++++++ vppcfg/example.yaml | 3 ++- vppcfg/schema.yaml | 2 ++ vppcfg/unittest/test_interface.yaml | 2 ++ 6 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/config-guide.md b/docs/config-guide.md index b7fd14f..97c1fb1 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -306,6 +306,8 @@ exist as a PHY in VPP (ie. `HundredGigabitEthernet12/0/0`) or as a specified `Bo If it is not specified, the link is considered admin 'up'. * ***device-type***: An optional interface type in VPP. Currently the only supported vlaue is `dpdk`, and it is used to generate correct mock interfaces if the `--novpp` flag is used. +* ***mpls***: An optional boolean that configures MPLS on the interface or sub-interface. The + default value is `false`, if the field is not specified, which means MPLS will not be enabled. Further, top-level interfaces, that is to say those that do not have an encapsulation, are permitted to have any number of sub-interfaces specified by `subid`, an integer between [0,2G), which further @@ -330,6 +332,7 @@ interfaces: lcp: "ice0" mtu: 9000 addresses: [ 192.0.2.1/30, 2001:db8:1::1/64 ] + mpls: true sub-interfaces: 1234: mtu: 9000 @@ -338,6 +341,7 @@ interfaces: 1235: mtu: 1500 lcp: "ice0.qinq" + mpls: true addresses: [ 192.0.2.9/30, 2001:db8:3::1/64 ] encapsulation: dot1q: 1234 diff --git a/vppcfg/config/interface.py b/vppcfg/config/interface.py index 5d73560..ac77adf 100644 --- a/vppcfg/config/interface.py +++ b/vppcfg/config/interface.py @@ -701,3 +701,14 @@ def validate_interfaces(yaml): result = False return result, msgs + + +def is_mpls(yaml, ifname): + """Returns True if the interface exists and has mpls enabled. Returns false otherwise.""" + ifname, iface = get_by_name(yaml, ifname) + try: + if iface["mpls"] == True: + return True + except: + pass + return False diff --git a/vppcfg/config/test_interface.py b/vppcfg/config/test_interface.py index c677874..3d765c4 100644 --- a/vppcfg/config/test_interface.py +++ b/vppcfg/config/test_interface.py @@ -277,3 +277,10 @@ class TestInterfaceMethods(unittest.TestCase): self.assertFalse( interface.get_admin_state(self.cfg, "GigabitEthernet1/0/0.102") ) + + def test_is_mpls(self): + self.assertTrue(interface.is_mpls(self.cfg, "GigabitEthernet1/0/1")) + self.assertTrue(interface.is_mpls(self.cfg, "GigabitEthernet1/0/1.101")) + self.assertFalse(interface.is_mpls(self.cfg, "GigabitEthernet1/0/0")) + self.assertFalse(interface.is_mpls(self.cfg, "GigabitEthernet1/0/0.100")) + self.assertFalse(interface.is_mpls(self.cfg, "notexist")) diff --git a/vppcfg/example.yaml b/vppcfg/example.yaml index a5990d8..b735c49 100644 --- a/vppcfg/example.yaml +++ b/vppcfg/example.yaml @@ -20,7 +20,8 @@ interfaces: lcp: "ice12-0-0" mac: f2:01:00:12:00:00 mtu: 9000 - addresses: [ 192.0.2.17/30, 2001:DB8:3::1/64 ] + addresses: [ 192.0.2.17/30, 2001:db8:3::1/64 ] + mpls: true sub-interfaces: 1234: mtu: 1200 diff --git a/vppcfg/schema.yaml b/vppcfg/schema.yaml index 5bd935e..cd7f6df 100644 --- a/vppcfg/schema.yaml +++ b/vppcfg/schema.yaml @@ -52,6 +52,7 @@ interface: sub-interfaces: map(include('sub-interface'),key=int(min=1,max=4294967295),required=False) l2xc: str(required=False) state: enum('up', 'down', required=False) + mpls: bool(required=False) device-type: enum('dpdk', required=False) --- sub-interface: @@ -62,6 +63,7 @@ sub-interface: encapsulation: include('encapsulation',required=False) l2xc: str(required=False) state: enum('up', 'down', required=False) + mpls: bool(required=False) --- encapsulation: dot1q: int(min=1,max=4095,required=False) diff --git a/vppcfg/unittest/test_interface.yaml b/vppcfg/unittest/test_interface.yaml index 648c2eb..b48811c 100644 --- a/vppcfg/unittest/test_interface.yaml +++ b/vppcfg/unittest/test_interface.yaml @@ -23,6 +23,7 @@ interfaces: mtu: 9216 lcp: "e1" addresses: [ "192.0.2.1/30", "2001:db8:1::1/64" ] + mpls: true sub-interfaces: 100: lcp: "foo" @@ -33,6 +34,7 @@ interfaces: exact-match: True lcp: "e1.100" addresses: [ "10.0.2.1/30" ] + mpls: true 102: encapsulation: dot1ad: 100 From 9a42cc91c3e9b2d84eb0bca4dc4a0b407f439158 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 11 Jun 2023 18:18:05 +0200 Subject: [PATCH 35/46] Allow MPLS on loopbacks too -- needed for BVIs and such. Add tests. --- vppcfg/config/loopback.py | 11 +++++++++++ vppcfg/config/test_loopback.py | 5 +++++ vppcfg/example.yaml | 1 + vppcfg/schema.yaml | 1 + vppcfg/unittest/test_loopback.yaml | 1 + 5 files changed, 19 insertions(+) diff --git a/vppcfg/config/loopback.py b/vppcfg/config/loopback.py index a5f1d2d..4bf6294 100644 --- a/vppcfg/config/loopback.py +++ b/vppcfg/config/loopback.py @@ -96,3 +96,14 @@ def validate_loopbacks(yaml): result = False return result, msgs + + +def is_mpls(yaml, ifname): + """Returns True if the loopback exists and has mpls enabled. Returns false otherwise.""" + ifname, iface = get_by_name(yaml, ifname) + try: + if iface["mpls"] == True: + return True + except: + pass + return False diff --git a/vppcfg/config/test_loopback.py b/vppcfg/config/test_loopback.py index 9c148c5..7b0345c 100644 --- a/vppcfg/config/test_loopback.py +++ b/vppcfg/config/test_loopback.py @@ -50,3 +50,8 @@ class TestLoopbackMethods(unittest.TestCase): self.assertIn("loop1", ifs) self.assertIn("loop2", ifs) self.assertNotIn("loop-noexist", ifs) + + def test_is_mpls(self): + self.assertTrue(loopback.is_mpls(self.cfg, "loop1")) + self.assertFalse(loopback.is_mpls(self.cfg, "loop2")) + self.assertFalse(loopback.is_mpls(self.cfg, "loop-noexist")) diff --git a/vppcfg/example.yaml b/vppcfg/example.yaml index b735c49..d9bcbff 100644 --- a/vppcfg/example.yaml +++ b/vppcfg/example.yaml @@ -91,6 +91,7 @@ loopbacks: mtu: 1500 mac: 02:de:ad:11:be:ef addresses: [ 10.0.2.1/24, 2001:db8:2::1/64 ] + mpls: true bridgedomains: bd1: diff --git a/vppcfg/schema.yaml b/vppcfg/schema.yaml index cd7f6df..f79b1a5 100644 --- a/vppcfg/schema.yaml +++ b/vppcfg/schema.yaml @@ -35,6 +35,7 @@ loopback: lcp: str(max=15,matches='[a-z]+[a-z0-9-]*',required=False) mtu: int(min=128,max=9216,required=False) addresses: list(ip_interface(),min=1,max=6,required=False) + mpls: bool(required=False) --- bondethernet: description: str(exclude='\'"',len=64,required=False) diff --git a/vppcfg/unittest/test_loopback.yaml b/vppcfg/unittest/test_loopback.yaml index 2030dfb..413fbb6 100644 --- a/vppcfg/unittest/test_loopback.yaml +++ b/vppcfg/unittest/test_loopback.yaml @@ -6,6 +6,7 @@ loopbacks: mtu: 2000 lcp: "loop56789012345" addresses: [ 192.0.2.1/29, 2001:db8::1/64 ] + mpls: true loop2: description: "Loopback, invalid because it has an address but no LCP" mtu: 2000 From 1c9590dccefba4ecf5699cafa0d4f461b2d563b8 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 11 Jun 2023 18:43:43 +0200 Subject: [PATCH 36/46] Set MPLS for loopback and interface. Allow for --novpp and VPP changes --- vppcfg/vpp/dumper.py | 2 ++ vppcfg/vpp/reconciler.py | 33 +++++++++++++++++++++++++++++++++ vppcfg/vpp/vppapi.py | 6 ++++++ 3 files changed, 41 insertions(+) diff --git a/vppcfg/vpp/dumper.py b/vppcfg/vpp/dumper.py index 15cd7b4..692735c 100644 --- a/vppcfg/vpp/dumper.py +++ b/vppcfg/vpp/dumper.py @@ -110,6 +110,8 @@ class Dumper(VPPApi): loop["addresses"] = self.cache["interface_addresses"][ iface.sw_if_index ] + if iface.sw_if_index in self.cache["interface_mpls"]: + loop["mpls"] = self.cache["interface_mpls"][iface.sw_if_index] config["loopbacks"][iface.interface_name] = loop elif iface.interface_dev_type in [ "bond", diff --git a/vppcfg/vpp/reconciler.py b/vppcfg/vpp/reconciler.py index e3d0b7f..9e8b757 100644 --- a/vppcfg/vpp/reconciler.py +++ b/vppcfg/vpp/reconciler.py @@ -947,6 +947,9 @@ class Reconciler: if not self.__sync_phys(): self.logger.warning("Could not sync PHYs in VPP") ret = False + if not self.__sync_mpls_state(): + self.logger.warning("Could not sync interface MPLS state in VPP") + ret = False if not self.__sync_admin_state(): self.logger.warning("Could not sync interface adminstate in VPP") ret = False @@ -1291,6 +1294,36 @@ class Reconciler: ret = False return ret + def __sync_mpls_state(self): + """Synchronize the VPP Dataplane configuration for interface and loopback MPLS state""" + for ifname in interface.get_interfaces(self.cfg) + loopback.get_loopbacks( + self.cfg + ): + if ifname.startswith("loop"): + vpp_ifname, config_iface = loopback.get_by_name(self.cfg, ifname) + else: + vpp_ifname, config_iface = interface.get_by_name(self.cfg, ifname) + + try: + config_mpls = config_iface["mpls"] + except KeyError: + config_mpls = False + + vpp_mpls = False + if vpp_ifname in self.vpp.cache["interface_names"]: + sw_if_index = self.vpp.cache["interface_names"][vpp_ifname] + try: + vpp_mpls = self.vpp.cache["interface_mpls"][sw_if_index] + except KeyError: + pass + if vpp_mpls != config_mpls: + state = "disable" + if config_mpls: + state = "enable" + cli = f"set interface mpls {vpp_ifname} {state}" + self.cli["sync"].append(cli) + return True + def __sync_addresses(self): """Synchronize the VPP Dataplane configuration for interface addresses""" for ifname in interface.get_interfaces(self.cfg) + loopback.get_loopbacks( diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 4b11a0e..c6a2be4 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -120,6 +120,7 @@ class VPPApi: "interfaces": {}, "interface_addresses": {}, "interface_acls": {}, + "interface_mpls": {}, "bondethernets": {}, "bondethernet_members": {}, "bridgedomains": {}, @@ -368,6 +369,11 @@ class VPPApi: for iface in api_response: self.cache["interface_acls"][iface.sw_if_index] = iface + self.logger.debug("Retrieving interface MPLS state") + api_response = self.vpp.api.mpls_interface_dump() + for iface in api_response: + self.cache["interface_mpls"][iface.sw_if_index] = True + self.logger.debug("Retrieving bondethernets") api_response = self.vpp.api.sw_bond_interface_dump() for iface in api_response: From 52864aac2cd6ad9c5e6283fc8aa4feed96828fcc Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 11 Jun 2023 18:45:30 +0200 Subject: [PATCH 37/46] Add documentation for MPLS on loopback interfaces --- docs/config-guide.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/config-guide.md b/docs/config-guide.md index 97c1fb1..a328149 100644 --- a/docs/config-guide.md +++ b/docs/config-guide.md @@ -73,6 +73,9 @@ following fields: in CIDR format. VPP requires IP addresses to be unique in the entire dataplane, with one notable exception: Multiple IP addresses in the same prefix/len can be added on one and the same interface. +* ***mpls***: An optional boolean that configures MPLS on the interface or sub-interface. The + default value is `false`, if the field is not specified, which means MPLS will not be enabled. + This allows BVIs, represented by Loopbacks, to participate in MPLS . Although VPP would allow it, `vppcfg` does not allow for loopbacks to have sub-interfaces. @@ -87,6 +90,7 @@ loopbacks: lcp: bvi1 mtu: 9000 addresses: [ 10.0.1.1/24, 10.0.1.2/24, 2001:db8:1::1/64 ] + mpls: true ``` ### Bridge Domains From 7a69f657dbe21946f438a68f9ebb8449b927df26 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 11 Jun 2023 18:55:44 +0200 Subject: [PATCH 38/46] Protect API calls that are missing, print a warning --- vppcfg/vpp/vppapi.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index c6a2be4..007256d 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -326,9 +326,7 @@ class VPPApi: self.cache["lcps"][lcp.phy_sw_if_index] = lcp self.lcp_enabled = True except AttributeError as err: - self.logger.warning( - f"linux-cp not found, will not reconcile Linux Control Plane: {err}" - ) + self.logger.warning(f"LinuxCP API not found - missing plugin: {err}") self.logger.debug("Retrieving interfaces") api_response = self.vpp.api.sw_interface_dump() @@ -374,6 +372,16 @@ class VPPApi: for iface in api_response: self.cache["interface_mpls"][iface.sw_if_index] = True + try: ## TODO(pim): Remove after 23.10 release + self.logger.debug("Retrieving interface MPLS state") + api_response = self.vpp.api.mpls_interface_dump() + for iface in api_response: + self.cache["interface_mpls"][iface.sw_if_index] = True + except AttributeError: + self.logger.warning( + f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" + ) + self.logger.debug("Retrieving bondethernets") api_response = self.vpp.api.sw_bond_interface_dump() for iface in api_response: @@ -391,10 +399,13 @@ class VPPApi: for bridge in api_response: self.cache["bridgedomains"][bridge.bd_id] = bridge - self.logger.debug("Retrieving vxlan_tunnels") - api_response = self.vpp.api.vxlan_tunnel_v2_dump() - for vxlan in api_response: - self.cache["vxlan_tunnels"][vxlan.sw_if_index] = vxlan + try: + self.logger.debug("Retrieving vxlan_tunnels") + api_response = self.vpp.api.vxlan_tunnel_v2_dump() + for vxlan in api_response: + self.cache["vxlan_tunnels"][vxlan.sw_if_index] = vxlan + except AttributeError as err: + self.logger.warning(f"VXLAN API not found - missing plugin: {err}") self.logger.debug("Retrieving L2 Cross Connects") api_response = self.vpp.api.l2_xconnect_dump() From 871e5a7d8b6cf16454f108f57dac83cf005d51f3 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 17:12:48 +0000 Subject: [PATCH 39/46] acl: add dumper for acls A reasonable attempt will be made to shorten the output of terms, but due to the nature of the ACL plugin in VPP, all ACLs will be unrolled into their individual ACEs (called 'terms'). - src/dst-port will only be emitted with UDP/TCP - icmp-typc/code will only be emitted with ICMP/ICMPv6 - icmp-code/type and source/destination-ports ranges will be collapsed where appropriate. - if protocol is 0, only L3 information will be emitted NOTE: a bug in the VPP plugin will allow for ICMP 'sport' and 'dport' upper value to be 16 bits. If an ACE is retrieved from the dataplane regarding an ICMP or ICMPv6 (referring the 16 bit values to icmp type and code), they will be truncated and a warning issued. --- vppcfg/vpp/vppapi.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 007256d..639c3f1 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -119,8 +119,8 @@ class VPPApi: "interface_names": {}, "interfaces": {}, "interface_addresses": {}, - "interface_acls": {}, "interface_mpls": {}, + "interface_acls": {}, "bondethernets": {}, "bondethernet_members": {}, "bridgedomains": {}, @@ -382,6 +382,19 @@ class VPPApi: f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" ) + try: + self.logger.debug("Retrieving ACLs") + api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) + for acl in api_response: + self.cache["acls"][acl.acl_index] = acl + + self.logger.debug("Retrieving interface ACLs") + api_response = self.vpp.api.acl_interface_list_dump() + for iface in api_response: + self.cache["interface_acls"][iface.sw_if_index] = iface + except AttributeError: + self.logger.warning(f"ACL API not found - missing plugin: {err}") + self.logger.debug("Retrieving bondethernets") api_response = self.vpp.api.sw_bond_interface_dump() for iface in api_response: From a4dfbf055fe264b6be34e35584f0dd37b8da88bb Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 19:07:04 +0000 Subject: [PATCH 40/46] Refuse to work with ACLs if there are duplicate tags -- it means something/somebody has been inserting them outside of vppcfg, and this breaks the requirement that vppcfg.acls. is the same uniquely identified vpp.acl.tag --- vppcfg/vpp/vppapi.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 639c3f1..edc1ee7 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -381,6 +381,18 @@ class VPPApi: self.logger.warning( f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" ) +======= + self.logger.debug("Retrieving ACLs") + api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) + for acl in api_response: + self.cache["acls"][acl.acl_index] = acl + if acl.tag in self.cache["acl_tags"]: + self.logger.error( + f"Duplicate ACL tag '{acl.tag}' found - cannot safely preoceed, bailing" + ) + return False + self.cache["acl_tags"][acl.tag] = acl.acl_index +>>>>>>> ace08ac (Refuse to work with ACLs if there are duplicate tags -- it means something/somebody has been inserting them outside of vppcfg, and this breaks the requirement that vppcfg.acls. is the same uniquely identified vpp.acl.tag) try: self.logger.debug("Retrieving ACLs") From c7ba451016a97f7c09ab0037b1ff51ac18bcd78a Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 22:22:27 +0000 Subject: [PATCH 41/46] Collapse the error messages to force consistency --- vppcfg/vpp/vppapi.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index edc1ee7..e0d3de2 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -381,18 +381,20 @@ class VPPApi: self.logger.warning( f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" ) -======= - self.logger.debug("Retrieving ACLs") - api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) - for acl in api_response: - self.cache["acls"][acl.acl_index] = acl - if acl.tag in self.cache["acl_tags"]: - self.logger.error( - f"Duplicate ACL tag '{acl.tag}' found - cannot safely preoceed, bailing" - ) - return False - self.cache["acl_tags"][acl.tag] = acl.acl_index ->>>>>>> ace08ac (Refuse to work with ACLs if there are duplicate tags -- it means something/somebody has been inserting them outside of vppcfg, and this breaks the requirement that vppcfg.acls. is the same uniquely identified vpp.acl.tag) + + try: + self.logger.debug("Retrieving ACLs") + api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) + for acl in api_response: + self.cache["acls"][acl.acl_index] = acl + if acl.tag in self.cache["acl_tags"]: + self.logger.error( + f"Duplicate ACL tag '{acl.tag}' found - cannot safely preoceed, bailing" + ) + return False + self.cache["acl_tags"][acl.tag] = acl.acl_index + except AttributeError as err: + self.logger.warning(f"ACL API not found - missing plugin: {err}") try: self.logger.debug("Retrieving ACLs") From 603e26a313d53566354672ba3df5e8adc23c23f4 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 11 Jun 2023 18:43:43 +0200 Subject: [PATCH 42/46] Set MPLS for loopback and interface. Allow for --novpp and VPP changes --- vppcfg/vpp/vppapi.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index e0d3de2..1103807 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -409,6 +409,8 @@ class VPPApi: except AttributeError: self.logger.warning(f"ACL API not found - missing plugin: {err}") +======= +>>>>>>> 0cf4473 (Set MPLS for loopback and interface. Allow for --novpp and VPP changes) self.logger.debug("Retrieving bondethernets") api_response = self.vpp.api.sw_bond_interface_dump() for iface in api_response: From 28b8ba148561b3b1a709864fe845d0eed6a2399f Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 11 Jun 2023 18:55:44 +0200 Subject: [PATCH 43/46] Protect API calls that are missing, print a warning --- vppcfg/vpp/vppapi.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 1103807..e27a9f7 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -382,6 +382,16 @@ class VPPApi: f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" ) + try: ## TODO(pim): Remove after 23.10 release + self.logger.debug("Retrieving interface MPLS state") + api_response = self.vpp.api.mpls_interface_dump() + for iface in api_response: + self.cache["interface_mpls"][iface.sw_if_index] = True + except AttributeError: + self.logger.warning( + f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" + ) + try: self.logger.debug("Retrieving ACLs") api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) From 1bebb8d68da1269bcd1d2871d1aab9ef9e3caf6d Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 17:12:48 +0000 Subject: [PATCH 44/46] acl: add dumper for acls A reasonable attempt will be made to shorten the output of terms, but due to the nature of the ACL plugin in VPP, all ACLs will be unrolled into their individual ACEs (called 'terms'). - src/dst-port will only be emitted with UDP/TCP - icmp-typc/code will only be emitted with ICMP/ICMPv6 - icmp-code/type and source/destination-ports ranges will be collapsed where appropriate. - if protocol is 0, only L3 information will be emitted NOTE: a bug in the VPP plugin will allow for ICMP 'sport' and 'dport' upper value to be 16 bits. If an ACE is retrieved from the dataplane regarding an ICMP or ICMPv6 (referring the 16 bit values to icmp type and code), they will be truncated and a warning issued. --- vppcfg/vpp/vppapi.py | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index e27a9f7..639c3f1 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -382,30 +382,6 @@ class VPPApi: f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" ) - try: ## TODO(pim): Remove after 23.10 release - self.logger.debug("Retrieving interface MPLS state") - api_response = self.vpp.api.mpls_interface_dump() - for iface in api_response: - self.cache["interface_mpls"][iface.sw_if_index] = True - except AttributeError: - self.logger.warning( - f"MPLS state retrieval requires https://gerrit.fd.io/r/c/vpp/+/39022" - ) - - try: - self.logger.debug("Retrieving ACLs") - api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) - for acl in api_response: - self.cache["acls"][acl.acl_index] = acl - if acl.tag in self.cache["acl_tags"]: - self.logger.error( - f"Duplicate ACL tag '{acl.tag}' found - cannot safely preoceed, bailing" - ) - return False - self.cache["acl_tags"][acl.tag] = acl.acl_index - except AttributeError as err: - self.logger.warning(f"ACL API not found - missing plugin: {err}") - try: self.logger.debug("Retrieving ACLs") api_response = self.vpp.api.acl_dump(acl_index=0xFFFFFFFF) @@ -419,8 +395,6 @@ class VPPApi: except AttributeError: self.logger.warning(f"ACL API not found - missing plugin: {err}") -======= ->>>>>>> 0cf4473 (Set MPLS for loopback and interface. Allow for --novpp and VPP changes) self.logger.debug("Retrieving bondethernets") api_response = self.vpp.api.sw_bond_interface_dump() for iface in api_response: From 7d615fd33811abb78076601a98df408b71d4e026 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 11 Jun 2023 18:43:43 +0200 Subject: [PATCH 45/46] Set MPLS for loopback and interface. Allow for --novpp and VPP changes --- vppcfg/vpp/vppapi.py | 1 + 1 file changed, 1 insertion(+) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 639c3f1..189ae1e 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -121,6 +121,7 @@ class VPPApi: "interface_addresses": {}, "interface_mpls": {}, "interface_acls": {}, + "interface_mpls": {}, "bondethernets": {}, "bondethernet_members": {}, "bridgedomains": {}, From 91122c70a42f416800630ca01b220cef974f97ed Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 16 Jan 2023 17:12:48 +0000 Subject: [PATCH 46/46] acl: add dumper for acls A reasonable attempt will be made to shorten the output of terms, but due to the nature of the ACL plugin in VPP, all ACLs will be unrolled into their individual ACEs (called 'terms'). - src/dst-port will only be emitted with UDP/TCP - icmp-typc/code will only be emitted with ICMP/ICMPv6 - icmp-code/type and source/destination-ports ranges will be collapsed where appropriate. - if protocol is 0, only L3 information will be emitted NOTE: a bug in the VPP plugin will allow for ICMP 'sport' and 'dport' upper value to be 16 bits. If an ACE is retrieved from the dataplane regarding an ICMP or ICMPv6 (referring the 16 bit values to icmp type and code), they will be truncated and a warning issued. --- vppcfg/vpp/vppapi.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vppcfg/vpp/vppapi.py b/vppcfg/vpp/vppapi.py index 189ae1e..639c3f1 100644 --- a/vppcfg/vpp/vppapi.py +++ b/vppcfg/vpp/vppapi.py @@ -121,7 +121,6 @@ class VPPApi: "interface_addresses": {}, "interface_mpls": {}, "interface_acls": {}, - "interface_mpls": {}, "bondethernets": {}, "bondethernet_members": {}, "bridgedomains": {},