From e3626da307d66c34e4ce12c7c38cdb8fa5da9889 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 30 Mar 2022 19:18:09 +0200 Subject: vyos.util: T4319: add is_ipv6_enabled() helper function (cherry picked from commit df0fbfeedce0f163e9d10be21d58ad4dc797a28a) --- python/vyos/ifconfig/interface.py | 4 ++-- python/vyos/ifconfig/loopback.py | 13 +++++-------- python/vyos/util.py | 4 ++++ src/tests/test_util.py | 14 +++++++++++++- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index a2fa96d82..99a06b1dd 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -38,7 +38,7 @@ from vyos.util import dict_search from vyos.util import read_file from vyos.util import get_interface_config from vyos.util import is_systemd_service_active -from vyos.util import sysctl_read +from vyos.util import is_ipv6_enabled from vyos.template import is_ipv4 from vyos.template import is_ipv6 from vyos.validate import is_intf_addr_assigned @@ -1359,7 +1359,7 @@ class Interface(Control): self.set_ipv4_source_validation(value) # Only change IPv6 parameters if IPv6 was not explicitly disabled - if sysctl_read('net.ipv6.conf.all.disable_ipv6') == '0': + if is_ipv6_enabled(): # IPv6 forwarding tmp = dict_search('ipv6.disable_forwarding', config) value = '0' if (tmp != None) else '1' diff --git a/python/vyos/ifconfig/loopback.py b/python/vyos/ifconfig/loopback.py index de554ef44..30c890fdf 100644 --- a/python/vyos/ifconfig/loopback.py +++ b/python/vyos/ifconfig/loopback.py @@ -13,9 +13,8 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library. If not, see . -import vyos.util - from vyos.ifconfig.interface import Interface +from vyos.util import is_ipv6_enabled @Interface.register class LoopbackIf(Interface): @@ -34,8 +33,6 @@ class LoopbackIf(Interface): } } - name = 'loopback' - def remove(self): """ Loopback interface can not be deleted from operating system. We can @@ -62,11 +59,11 @@ class LoopbackIf(Interface): on any interface. """ addr = config.get('address', []) - # We must ensure that the loopback addresses are never deleted from the system - addr += ['127.0.0.1/8'] - if (vyos.util.sysctl_read('net.ipv6.conf.all.disable_ipv6') == '0'): - addr += ['::1/128'] + # We must ensure that the loopback addresses are never deleted from the system + addr.append('127.0.0.1/8') + if is_ipv6_enabled(): + addr.append('::1/128') # Update IP address entry in our dictionary config.update({'address' : addr}) diff --git a/python/vyos/util.py b/python/vyos/util.py index b5d81fba5..4283e604c 100644 --- a/python/vyos/util.py +++ b/python/vyos/util.py @@ -827,3 +827,7 @@ def sysctl_write(name, value): call(f'sysctl -wq {name}={value}') return True return False + +def is_ipv6_enabled() -> bool: + """ Check if IPv6 support on the system is enabled or not """ + return (sysctl_read('net.ipv6.conf.all.disable_ipv6') == '0') diff --git a/src/tests/test_util.py b/src/tests/test_util.py index 22bc085c5..8afa531d0 100644 --- a/src/tests/test_util.py +++ b/src/tests/test_util.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2020 VyOS maintainers and contributors +# Copyright (C) 2020-2022 VyOS maintainers and contributors # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License version 2 or later as @@ -24,3 +24,15 @@ class TestVyOSUtil(TestCase): new_data = mangle_dict_keys(data, '-', '_') self.assertEqual(new_data, expected_data) + def test_sysctl_read(self): + self.assertEqual(sysctl_read('net.ipv4.conf.lo.forwarding'), '1') + + def test_ipv6_enabled(self): + tmp = sysctl_read('net.ipv6.conf.all.disable_ipv6') + # We need to test for both variants as this depends on how the + # Docker container is started (with or without IPv6 support) - so we + # will simply check both cases to not make the users life miserable. + if tmp == '0': + self.assertTrue(is_ipv6_enabled()) + else: + self.assertFalse(is_ipv6_enabled()) -- cgit v1.2.3 From 138d8ecd528ea0104b2194af0a1e8b330438da8f Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 30 Mar 2022 19:19:24 +0200 Subject: vrf: T4319: do not add IPv6 localhost address if IPv6 is disabled (cherry picked from commit c33a96f6f0f0259808992b246b1a550fcf9a454a) --- src/conf_mode/vrf.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py index dd1739087..fb2182fff 100755 --- a/src/conf_mode/vrf.py +++ b/src/conf_mode/vrf.py @@ -27,6 +27,7 @@ from vyos.util import call from vyos.util import cmd from vyos.util import dict_search from vyos.util import sysctl_write +from vyos.util import is_ipv6_enabled from vyos import ConfigError from vyos import airbag airbag.enable() @@ -194,10 +195,11 @@ def apply(vrf): # set VRF description for e.g. SNMP monitoring vrf_if = Interface(name) - # We also should add proper loopback IP addresses to the newly - # created VRFs for services bound to the loopback address (SNMP, NTP) + # We also should add proper loopback IP addresses to the newly added + # VRF for services bound to the loopback address (SNMP, NTP) vrf_if.add_addr('127.0.0.1/8') - vrf_if.add_addr('::1/128') + if is_ipv6_enabled(): + vrf_if.add_addr('::1/128') # add VRF description if available vrf_if.set_alias(config.get('description', '')) -- cgit v1.2.3 From 24ffc55877ac1b791f876fd454473d0fd9ab750b Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 30 Mar 2022 19:19:54 +0200 Subject: vyos.ifconfig: T4319: add_addr() should not add IPv6 address if it's disabled (cherry picked from commit 60f009defadb9d36bf84def1e839cb11a0b3d619) --- python/vyos/ifconfig/interface.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 99a06b1dd..b06fd1294 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -990,6 +990,10 @@ class Interface(Control): "Can't configure both static IPv4 and DHCP address " "on the same interface")) + # Failsave - do not add IPv6 address if IPv6 is disabled + if is_ipv6(addr) and not is_ipv6_enabled(): + return False + # add to interface if addr == 'dhcp': self.set_dhcp(True) -- cgit v1.2.3 From 9da1b20e40b9f524a27d27e73a9a13d937a49aa6 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 30 Mar 2022 21:57:13 +0200 Subject: smoketest: T4319: verify correct assignment of loopback IP addresses (cherry picked from commit a33b737b753843501c86eee744aef75137d2b64e) --- smoketest/scripts/cli/test_system_ipv6.py | 19 +++++++++++ smoketest/scripts/cli/test_vrf.py | 56 +++++++++++++++++++++++++++---- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/smoketest/scripts/cli/test_system_ipv6.py b/smoketest/scripts/cli/test_system_ipv6.py index 3112d2e46..6fe58701b 100755 --- a/smoketest/scripts/cli/test_system_ipv6.py +++ b/smoketest/scripts/cli/test_system_ipv6.py @@ -17,7 +17,11 @@ import unittest from base_vyostest_shim import VyOSUnitTestSHIM + +from vyos.template import is_ipv4 from vyos.util import read_file +from vyos.util import is_ipv6_enabled +from vyos.validate import is_intf_addr_assigned base_path = ['system', 'ipv6'] @@ -42,6 +46,14 @@ class TestSystemIPv6(VyOSUnitTestSHIM.TestCase): self.assertEqual(read_file(file_forwarding), '0') def test_system_ipv6_disable(self): + # Verify previous "enable" state + self.assertEqual(read_file(file_disable), '0') + self.assertTrue(is_ipv6_enabled()) + + loopbacks = ['127.0.0.1', '::1'] + for addr in loopbacks: + self.assertTrue(is_intf_addr_assigned('lo', addr)) + # Do not assign any IPv6 address on interfaces, this requires a reboot # which can not be tested, but we can read the config file :) self.cli_set(base_path + ['disable']) @@ -49,6 +61,13 @@ class TestSystemIPv6(VyOSUnitTestSHIM.TestCase): # Verify configuration file self.assertEqual(read_file(file_disable), '1') + self.assertFalse(is_ipv6_enabled()) + + for addr in loopbacks: + if is_ipv4(addr): + self.assertTrue(is_intf_addr_assigned('lo', addr)) + else: + self.assertFalse(is_intf_addr_assigned('lo', addr)) def test_system_ipv6_strict_dad(self): # This defaults to 1 diff --git a/smoketest/scripts/cli/test_vrf.py b/smoketest/scripts/cli/test_vrf.py index 0f006ca3c..938fd4798 100755 --- a/smoketest/scripts/cli/test_vrf.py +++ b/smoketest/scripts/cli/test_vrf.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2020-2021 VyOS maintainers and contributors +# Copyright (C) 2020-2022 VyOS maintainers and contributors # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License version 2 or later as @@ -26,9 +26,10 @@ from netifaces import interfaces from vyos.configsession import ConfigSessionError from vyos.ifconfig import Interface from vyos.ifconfig import Section -from vyos.template import is_ipv6 +from vyos.template import is_ipv4 from vyos.util import cmd from vyos.util import read_file +from vyos.util import get_interface_config from vyos.validate import is_intf_addr_assigned base_path = ['vrf'] @@ -108,9 +109,14 @@ class VRFTest(VyOSUnitTestSHIM.TestCase): # ... regex = f'{table}\s+{vrf}\s+#\s+{description}' self.assertTrue(re.findall(regex, iproute2_config)) + + tmp = get_interface_config(vrf) + self.assertEqual(int(table), tmp['linkinfo']['info_data']['table']) + + # Increment table ID for the next run table = str(int(table) + 1) - def test_vrf_loopback_ips(self): + def test_vrf_loopbacks_ips(self): table = '2000' for vrf in vrfs: base = base_path + ['name', vrf] @@ -121,10 +127,48 @@ class VRFTest(VyOSUnitTestSHIM.TestCase): self.cli_commit() # Verify VRF configuration + loopbacks = ['127.0.0.1', '::1'] for vrf in vrfs: - self.assertTrue(vrf in interfaces()) - self.assertTrue(is_intf_addr_assigned(vrf, '127.0.0.1')) - self.assertTrue(is_intf_addr_assigned(vrf, '::1')) + # Ensure VRF was created + self.assertIn(vrf, interfaces()) + # Test for proper loopback IP assignment + for addr in loopbacks: + self.assertTrue(is_intf_addr_assigned(vrf, addr)) + + def test_vrf_loopbacks_no_ipv6(self): + table = '2002' + for vrf in vrfs: + base = base_path + ['name', vrf] + self.cli_set(base + ['table', str(table)]) + table = str(int(table) + 1) + + # Globally disable IPv6 - this will remove all IPv6 interface addresses + self.cli_set(['system', 'ipv6', 'disable']) + + # commit changes + self.cli_commit() + + # Verify VRF configuration + table = '2002' + loopbacks = ['127.0.0.1', '::1'] + for vrf in vrfs: + # Ensure VRF was created + self.assertIn(vrf, interfaces()) + + # Verify VRF table ID + tmp = get_interface_config(vrf) + self.assertEqual(int(table), tmp['linkinfo']['info_data']['table']) + + # Test for proper loopback IP assignment + for addr in loopbacks: + if is_ipv4(addr): + self.assertTrue(is_intf_addr_assigned(vrf, addr)) + else: + self.assertFalse(is_intf_addr_assigned(vrf, addr)) + + table = str(int(table) + 1) + + self.cli_delete(['system', 'ipv6']) def test_vrf_bind_all(self): table = '2000' -- cgit v1.2.3 From 18ac045fe6d11065b4fa518372fabb844c347146 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 30 Mar 2022 23:15:52 +0200 Subject: test: vyos.util build time tests should import all functions --- src/tests/test_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/test_util.py b/src/tests/test_util.py index 8afa531d0..91890262c 100644 --- a/src/tests/test_util.py +++ b/src/tests/test_util.py @@ -15,7 +15,7 @@ # along with this program. If not, see . from unittest import TestCase -from vyos.util import mangle_dict_keys +from vyos.util import * class TestVyOSUtil(TestCase): def test_key_mangline(self): -- cgit v1.2.3 From b99043b645eba05667909cfb573a3ad532659308 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 31 Mar 2022 21:13:19 +0200 Subject: smoketest: vrf: verify routes via FRR cli --- smoketest/scripts/cli/test_vrf.py | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/smoketest/scripts/cli/test_vrf.py b/smoketest/scripts/cli/test_vrf.py index 938fd4798..75be9e4f8 100755 --- a/smoketest/scripts/cli/test_vrf.py +++ b/smoketest/scripts/cli/test_vrf.py @@ -26,6 +26,7 @@ from netifaces import interfaces from vyos.configsession import ConfigSessionError from vyos.ifconfig import Interface from vyos.ifconfig import Section +from vyos.template import is_ipv6 from vyos.template import is_ipv4 from vyos.util import cmd from vyos.util import read_file @@ -62,6 +63,8 @@ class VRFTest(VyOSUnitTestSHIM.TestCase): def tearDown(self): # delete all VRFs self.cli_delete(base_path) + self.cli_delete(['interfaces', 'dummy']) + self.cli_delete(['protocols', 'vrf']) self.cli_commit() for vrf in vrfs: self.assertNotIn(vrf, interfaces()) @@ -248,14 +251,14 @@ class VRFTest(VyOSUnitTestSHIM.TestCase): }, } + # required interface for leaking to default table + self.cli_set(['interfaces', 'ethernet', 'eth0', 'address', '192.0.2.1/24']) + table = '2000' for vrf in vrfs: base = base_path + ['name', vrf] self.cli_set(base + ['table', str(table)]) - # required interface for leaking to default table - self.cli_set(['interfaces', 'ethernet', 'eth0', 'address', '192.0.2.1/24']) - # we also need an interface in "UP" state to install routes self.cli_set(['interfaces', 'dummy', f'dum{table}', 'vrf', vrf]) self.cli_set(['interfaces', 'dummy', f'dum{table}', 'address', '192.0.2.1/24']) @@ -277,28 +280,24 @@ class VRFTest(VyOSUnitTestSHIM.TestCase): self.cli_commit() # Verify routes - table = '2000' for vrf in vrfs: - for route, route_config in routes.items(): - if is_ipv6(route): - tmp = get_vrf_ipv6_routes(vrf) - else: - tmp = get_vrf_ipv4_routes(vrf) + self.assertIn(vrf, interfaces()) + frrconfig = self.getFRRconfig(f'vrf {vrf}') + for prefix, prefix_config in routes.items(): + tmp = 'ip' + if is_ipv6(prefix): + tmp += 'v6' - found = False - for result in tmp: - if 'dst' in result and result['dst'] == route: - if 'gateway' in result and result['gateway'] == route_config['next_hop']: - found = True + tmp += f' route {prefix} {prefix_config["next_hop"]}' + if 'distance' in prefix_config: + tmp += ' ' + prefix_config['distance'] + if 'next_hop_vrf' in prefix_config: + tmp += ' nexthop-vrf ' + prefix_config['next_hop_vrf'] - self.assertTrue(found) + self.assertIn(tmp, frrconfig) - # Cleanup - self.cli_delete(['protocols', 'vrf', vrf]) - self.cli_delete(['interfaces', 'dummy', f'dum{table}']) - self.cli_delete(['interfaces', 'ethernet', 'eth0', 'address', '192.0.2.1/24']) + self.cli_delete(['interfaces', 'ethernet', 'eth0', 'address']) - table = str(int(table) + 1) if __name__ == '__main__': unittest.main(verbosity=2) -- cgit v1.2.3 From 8ab73ce0f84e10ae3c4f6bdb341b7b6b11db66b4 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 31 Mar 2022 21:38:17 +0200 Subject: vyos.ifconfig: T4330: bugfix changing MTU when IPv6 is disabled Commit f8b3d8999c ("ipv6: T4319: do not configure IPv6 related settings if it's disabled") moved the MTU configuration part under the code path which is only run if IPv6 is enabled on the system. This prevented MTU changes on IPv6 disabled systems. (cherry picked from commit 53e20097d227ebf4bdb4dc6c85427ec9c5ec3982) --- python/vyos/ifconfig/interface.py | 8 ++++---- smoketest/scripts/cli/test_system_ipv6.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index b06fd1294..e8278b103 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -1386,10 +1386,6 @@ class Interface(Control): value = tmp if (tmp != None) else '1' self.set_ipv6_dad_messages(value) - # MTU - Maximum Transfer Unit - if 'mtu' in config: - self.set_mtu(config.get('mtu')) - # Delete old IPv6 EUI64 addresses before changing MAC for addr in (dict_search('ipv6.address.eui64_old', config) or []): self.del_ipv6_eui64_address(addr) @@ -1406,6 +1402,10 @@ class Interface(Control): for addr in tmp: self.add_ipv6_eui64_address(addr) + # MTU - Maximum Transfer Unit + if 'mtu' in config: + self.set_mtu(config.get('mtu')) + # re-add ourselves to any bridge we might have fallen out of if 'is_bridge_member' in config: bridge_dict = config.get('is_bridge_member') diff --git a/smoketest/scripts/cli/test_system_ipv6.py b/smoketest/scripts/cli/test_system_ipv6.py index 6fe58701b..837d1dc12 100755 --- a/smoketest/scripts/cli/test_system_ipv6.py +++ b/smoketest/scripts/cli/test_system_ipv6.py @@ -21,6 +21,7 @@ from base_vyostest_shim import VyOSUnitTestSHIM from vyos.template import is_ipv4 from vyos.util import read_file from vyos.util import is_ipv6_enabled +from vyos.util import get_interface_config from vyos.validate import is_intf_addr_assigned base_path = ['system', 'ipv6'] @@ -69,6 +70,17 @@ class TestSystemIPv6(VyOSUnitTestSHIM.TestCase): else: self.assertFalse(is_intf_addr_assigned('lo', addr)) + # T4330: Verify MTU can be changed with IPv6 disabled + mtu = '1600' + eth_if = 'eth0' + self.cli_set(['interfaces', 'ethernet', eth_if, 'mtu', mtu]) + self.cli_commit() + + tmp = get_interface_config(eth_if) + self.assertEqual(tmp['mtu'], int(mtu)) + + self.cli_delete(['interfaces', 'ethernet', eth_if, 'mtu']) + def test_system_ipv6_strict_dad(self): # This defaults to 1 self.assertEqual(read_file(file_dad), '1') -- cgit v1.2.3 From 5b11ad0024552f9072c8bc046bb8ce7ad5a193b2 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 31 Mar 2022 21:38:17 +0200 Subject: vyos.ifconfig: T4330: MTU must be configured prior to any IPv6 option change This extends the fix from 53e20097 ("vyos.ifconfig: T4330: bugfix changing MTU when IPv6 is disabled") by ordering the execution in a way the Kernel does not complain. (cherry picked from commit 601ab19fd8c81a998b3c966cc83b85ed60ac5ae0) --- python/vyos/ifconfig/interface.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index e8278b103..214c8dcb1 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -1362,6 +1362,13 @@ class Interface(Control): value = tmp if (tmp != None) else '0' self.set_ipv4_source_validation(value) + # MTU - Maximum Transfer Unit has a default value. It must ALWAYS be set + # before mangling any IPv6 option. If MTU is less then 1280 IPv6 will be + # automatically disabled by the kernel. Also MTU must be increased before + # configuring any IPv6 address on the interface. + if 'mtu' in config: + self.set_mtu(config.get('mtu')) + # Only change IPv6 parameters if IPv6 was not explicitly disabled if is_ipv6_enabled(): # IPv6 forwarding @@ -1402,10 +1409,6 @@ class Interface(Control): for addr in tmp: self.add_ipv6_eui64_address(addr) - # MTU - Maximum Transfer Unit - if 'mtu' in config: - self.set_mtu(config.get('mtu')) - # re-add ourselves to any bridge we might have fallen out of if 'is_bridge_member' in config: bridge_dict = config.get('is_bridge_member') -- cgit v1.2.3