From 19bbd0e8a3215bec0009d5b65fc11165ba43a9d8 Mon Sep 17 00:00:00 2001
From: Pim van Pelt <pim@ipng.nl>
Date: Sun, 13 Mar 2022 23:45:40 +0000
Subject: [PATCH] IP Address validator
Returns True if there is at most one occurence of the ip_interface (an IPv4/IPv6 prefix+len)
in the entire config. That said, we need the 'iface_addresses' because VPP is a bit fickle in
this regard.
IP addresses from the same prefix/len can be added to a given interface (ie 192.0.2.1/24 and
192.0.2.2/24), but other than that, any prefix can not occur as a more-specific or less-specific
of any other interface.
So, we will allow:
- any ip_interface that is of equal network/len of existing one(s) _on the same interface_
And, we will reject
- any ip_interface that is a more specific of any existing one
- any ip_interface that is a less specific of any existing one
Update unit tests to ensure ip_interfaces are allowed in all cases.
---
unittest/correct-address.yaml | 32 ++++++++
unittest/correct-example1.yaml | 2 +-
unittest/error-address1.yaml | 64 ++++++++++++++++
unittest/error-subinterface5.yaml | 2 +-
validator/address.py | 122 ++++++++++++++++++++++++++++++
validator/bridgedomain.py | 6 ++
validator/interface.py | 11 +++
validator/loopback.py | 6 ++
8 files changed, 243 insertions(+), 2 deletions(-)
create mode 100644 unittest/correct-address.yaml
create mode 100644 unittest/error-address1.yaml
create mode 100644 validator/address.py
diff --git a/unittest/correct-address.yaml b/unittest/correct-address.yaml
new file mode 100644
index 0000000..eea1343
--- /dev/null
+++ b/unittest/correct-address.yaml
@@ -0,0 +1,32 @@
+test:
+ description: "A bunch of IP addresses that are wellformed"
+ errors:
+ count: 0
+---
+interfaces:
+ GigabitEthernet1/0/0:
+ lcp: e0-0
+ addresses: [ 192.0.2.1/29, 2001:db8:1::1/64 ]
+ sub-interfaces:
+ 100:
+ description: "Overlapping IP addresses are fine, if in the same prefix"
+ addresses: [ 192.0.2.9/29, 192.0.2.10/29 ]
+ 101:
+ description: ".. and for IPv6 also, provided the same prefix is used"
+ addresses: [ 2001:db8:2::1/64, 2001:db8:2::2/64 ]
+
+ GigabitEthernet3/0/0:
+ mtu: 2000
+
+loopbacks:
+ loop0:
+ lcp: "loop0"
+ addresses: [ 10.0.0.1/32, 2001:db8::1/128 ]
+
+bridgedomains:
+ bd10:
+ description: "Bridge Domain 10"
+ mtu: 2000
+ lcp: "bvi10"
+ addresses: [ 10.0.0.2/32, 2001:db8::2/128 ]
+ interfaces: [ GigabitEthernet3/0/0 ]
diff --git a/unittest/correct-example1.yaml b/unittest/correct-example1.yaml
index 7e8c9a7..6a09354 100644
--- a/unittest/correct-example1.yaml
+++ b/unittest/correct-example1.yaml
@@ -48,7 +48,7 @@ interfaces:
encapsulation:
dot1q: 1000
inner-dot1q: 1234
- addresses: [ 192.0.2.1/24 ]
+ addresses: [ 192.168.1.1/24 ]
203:
encapsulation:
dot1ad: 1000
diff --git a/unittest/error-address1.yaml b/unittest/error-address1.yaml
new file mode 100644
index 0000000..b7538a5
--- /dev/null
+++ b/unittest/error-address1.yaml
@@ -0,0 +1,64 @@
+test:
+ description: "IP interfaces cannot be more- or less-specific of any other interface"
+ errors:
+ expected:
+ - "interface .* IP address .* is not allowed"
+ - "sub-interface .* IP address .* is not allowed"
+ - "loopback .* IP address .* is not allowed"
+ - "bridgedomain .* IP address .* is not allowed"
+ count: 18
+---
+interfaces:
+ GigabitEthernet1/0/0:
+ lcp: e1-0-0
+ description: "The 2nd/3rd addresses all are more/less specifics of the first"
+ addresses: [ 172.16.12.1/29, 172.16.12.2/30, 172.16.12.3/28 ]
+
+ GigabitEthernet1/0/1:
+ lcp: e1-0-1
+ addresses: [ 192.0.2.1/29, 2001:db8:1::1/64 ]
+ sub-interfaces:
+ 100:
+ description: "These addresses overlap with Gi1/0/1"
+ addresses: [ 192.0.2.2/29, 2001:db8:1::2/64 ]
+ 101:
+ description: "These addresses overlap with loop0"
+ addresses: [ 192.0.2.10/29, 2001:db8:2::2/64 ]
+
+ GigabitEthernet1/0/2:
+ lcp: e0-2
+ description: "These addresses overlap with bd1"
+ addresses: [ 192.0.2.18/29, 2001:db8:3::2/64 ]
+
+ GigabitEthernet1/0/3:
+ lcp: e0-3
+ description: "These addresses are more-specific to Gi1/0/4"
+ addresses: [ 10.0.0.1/24, 2001:db8:3::1/64 ]
+
+ GigabitEthernet1/0/4:
+ lcp: e0-4
+ description: "These addresses are less-specific to Gi1/0/3"
+ addresses: [ 10.0.0.2/23, 2001:db8:3::2/60 ]
+
+ GigabitEthernet1/0/5:
+ lcp: e0-5
+ description: "These addresses are more-specific to Gi1/0/3"
+ addresses: [ 10.0.0.3/25, 2001:db8:3::3/112 ]
+
+ GigabitEthernet3/0/0:
+ description: "Bridge Domain bd1, member #1"
+ mtu: 2000
+
+loopbacks:
+ loop0:
+ description: "These addresses overlap with Gi1/0/1.101"
+ lcp: "loop0"
+ addresses: [ 192.0.2.9/29, 2001:db8:2::1/64 ]
+
+bridgedomains:
+ bd1:
+ description: "These addresses overlap with Gi1/0/2"
+ mtu: 2000
+ lcp: "bvi1"
+ addresses: [ 192.0.2.17/29, 2001:db8:3::1/64 ]
+ interfaces: [ GigabitEthernet3/0/0 ]
diff --git a/unittest/error-subinterface5.yaml b/unittest/error-subinterface5.yaml
index 3bb5c97..e9254c8 100644
--- a/unittest/error-subinterface5.yaml
+++ b/unittest/error-subinterface5.yaml
@@ -13,5 +13,5 @@ interfaces:
lcp: "xe2-0-0"
sub-interfaces:
100:
- addresses: [ 192.0.2.1/24 ]
+ addresses: [ 192.168.1.1/24 ]
diff --git a/validator/address.py b/validator/address.py
new file mode 100644
index 0000000..efd6920
--- /dev/null
+++ b/validator/address.py
@@ -0,0 +1,122 @@
+#
+# 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.
+#
+import logging
+import validator.interface as interface
+import ipaddress
+
+def get_all_addresses_except_ifname(yaml, except_ifname):
+ """ Return a list of all ipaddress.ip_interface() instances in the entire config,
+ except for those that belong to 'ifname'.
+ """
+ ret = []
+ if 'interfaces' in yaml:
+ for ifname, iface in yaml['interfaces'].items():
+ if ifname == except_ifname:
+ continue
+
+ if 'addresses' in iface:
+ for a in iface['addresses']:
+ ret.append(ipaddress.ip_interface(a))
+ if 'sub-interfaces' in iface:
+ for subid, sub_iface in iface['sub-interfaces'].items():
+ sub_ifname = "%s.%d" % (ifname, subid)
+ if sub_ifname == except_ifname:
+ continue
+
+ if 'addresses' in sub_iface:
+ for a in sub_iface['addresses']:
+ ret.append(ipaddress.ip_interface(a))
+ if 'loopbacks' in yaml:
+ for ifname, iface in yaml['loopbacks'].items():
+ if ifname == except_ifname:
+ continue
+
+ if 'addresses' in iface:
+ for a in iface['addresses']:
+ ret.append(ipaddress.ip_interface(a))
+ if 'bridgedomains' in yaml:
+ for ifname, iface in yaml['bridgedomains'].items():
+ if ifname == except_ifname:
+ continue
+
+ if 'addresses' in iface:
+ for a in iface['addresses']:
+ ret.append(ipaddress.ip_interface(a))
+
+ return ret
+
+def is_allowed(yaml, ifname, iface_addresses, ip_interface):
+ """ Returns True if there is at most one occurence of the ip_interface (an IPv4/IPv6 prefix+len)
+ in the entire config. That said, we need the 'iface_addresses' because VPP is a bit fickle in
+ this regard.
+
+ IP addresses from the same prefix/len can be added to a given interface (ie 192.0.2.1/24 and
+ 192.0.2.2/24), but other than that, any prefix can not occur as a more-specific or less-specific
+ of any other interface.
+
+ So, we will allow:
+ - any ip_interface that is of equal network/len of existing one(s) _on the same interface_
+
+ And, we will reject
+ - any ip_interface that is a more specific of any existing one
+ - any ip_interface that is a less specific of any existing one
+
+ Examples:
+ vpp# set interface ip address loop0 192.0.2.1/24
+ vpp# set interface ip address loop0 192.0.2.2/24
+ vpp# set interface ip address loop0 192.0.2.1/29
+ set interface ip address: failed to add 192.0.2.1/29 on loop0 which conflicts with 192.0.2.1/24 for interface loop0
+ vpp# set interface ip address loop0 192.0.2.3/23
+ set interface ip address: failed to add 192.0.2.3/23 on loop0 which conflicts with 192.0.2.1/24 for interface loop0
+ """
+ ## print("input: (%s,%s)" % (iface_addresses, ip_interface))
+ all_other_addresses = get_all_addresses_except_ifname(yaml, ifname)
+ ## print("All IPs: %s" % all_other_addresses)
+
+ my_ip_network = ipaddress.ip_network(ip_interface, strict=False)
+
+ for ipi in all_other_addresses:
+ if ipi.version != my_ip_network.version:
+ continue
+
+ if ipaddress.ip_network(ipi, strict=False) == my_ip_network:
+ ## print("Same: %s == %s" % (ip_interface, ipi))
+ return False
+
+ if ipaddress.ip_network(ipi, strict=False).subnet_of(my_ip_network):
+ ## print("More: %s == %s" % (ip_interface, ipi))
+ return False
+
+ if my_ip_network.subnet_of(ipaddress.ip_network(ipi, strict=False)):
+ ## print("Less: %s == %s" % (ip_interface, ipi))
+ return False
+
+ for ip in iface_addresses:
+ ipi = ipaddress.ip_interface(ip)
+ if ipi.version != my_ip_network.version:
+ continue
+
+ if ipaddress.ip_network(ipi, strict=False) == my_ip_network:
+ ## print("iface same: %s == %s" % (ip_interface, ipi))
+ return True
+
+ if ipaddress.ip_network(ipi, strict=False).subnet_of(my_ip_network):
+ ## print("iface more: %s == %s" % (ip_interface, ipi))
+ return False
+
+ if my_ip_network.subnet_of(ipaddress.ip_network(ipi, strict=False)):
+ ## print("iface less: %s == %s" % (ip_interface, ipi))
+ return False
+
+ return True
diff --git a/validator/bridgedomain.py b/validator/bridgedomain.py
index 0dd4f84..6d24196 100644
--- a/validator/bridgedomain.py
+++ b/validator/bridgedomain.py
@@ -14,6 +14,7 @@
import logging
import validator.interface as interface
import validator.lcp as lcp
+import validator.address as address
class NullHandler(logging.Handler):
def emit(self, record):
@@ -52,6 +53,11 @@ def validate_bridgedomains(yaml):
if 'lcp' in iface and not lcp.is_unique(yaml, iface['lcp']):
msgs.append("bridgedomain %s does not have a unique LCP name %s" % (ifname, iface['lcp']))
result = False
+ if 'addresses' in iface:
+ for a in iface['addresses']:
+ if not address.is_allowed(yaml, ifname, iface['addresses'], a):
+ msgs.append("bridgedomain %s IP address %s is not allowed" % (ifname, a))
+ result = False
if 'interfaces' in iface:
for member in iface['interfaces']:
diff --git a/validator/interface.py b/validator/interface.py
index dacb4f5..2827c46 100644
--- a/validator/interface.py
+++ b/validator/interface.py
@@ -14,6 +14,7 @@
import logging
import validator.bondethernet as bondethernet
import validator.lcp as lcp
+import validator.address as address
class NullHandler(logging.Handler):
def emit(self, record):
@@ -283,6 +284,12 @@ def validate_interfaces(yaml):
msgs.append("interface %s does not have a unique LCP name %s" % (ifname, iface_lcp))
result = False
+ if 'addresses' in iface:
+ for a in iface['addresses']:
+ if not address.is_allowed(yaml, ifname, iface['addresses'], a):
+ msgs.append("interface %s IP address %s is not allowed" % (ifname, a))
+ result = False
+
if has_sub(yaml, ifname):
for sub_id, sub_iface in yaml['interfaces'][ifname]['sub-interfaces'].items():
logger.debug("sub-interface %s" % sub_iface)
@@ -312,6 +319,10 @@ def validate_interfaces(yaml):
if not iface_lcp:
msgs.append("sub-interface %s has an address but %s does not have LCP" % (sub_ifname, ifname))
result = False
+ for a in sub_iface['addresses']:
+ if not address.is_allowed(yaml, sub_ifname, sub_iface['addresses'], a):
+ msgs.append("sub-interface %s IP address %s is not allowed" % (sub_ifname, a))
+ result = False
if not valid_encapsulation(yaml, sub_ifname):
msgs.append("sub-interface %s has invalid encapsulation" % (sub_ifname))
result = False
diff --git a/validator/loopback.py b/validator/loopback.py
index b62068a..fbfe285 100644
--- a/validator/loopback.py
+++ b/validator/loopback.py
@@ -13,6 +13,7 @@
#
import logging
import validator.lcp as lcp
+import validator.address as address
class NullHandler(logging.Handler):
def emit(self, record):
@@ -46,5 +47,10 @@ def validate_loopbacks(yaml):
if 'lcp' in iface and not lcp.is_unique(yaml, iface['lcp']):
msgs.append("loopback %s does not have a unique LCP name %s" % (ifname, iface['lcp']))
result = False
+ if 'addresses' in iface:
+ for a in iface['addresses']:
+ if not address.is_allowed(yaml, ifname, iface['addresses'], a):
+ msgs.append("loopback %s IP address %s is not allowed" % (ifname, a))
+ result = False
return result, msgs