From 3aed4c9c216375ff4f5ad3b0a000b6fd30836846 Mon Sep 17 00:00:00 2001 From: l0crian1 Date: Sat, 1 Aug 2026 17:56:37 -0400 Subject: T9152: Refactor source-validation to use vmaps - Refactor source-validation to use vmaps - Update broken source-validation tests - Fix source-validation failing to remove rules when interface is removed (T9153) --- python/vyos/ifconfig/interface.py | 119 +++++++++++++++++++++++++++----------- 1 file changed, 86 insertions(+), 33 deletions(-) (limited to 'python') diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 73f7544dd..b58eccd43 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -58,6 +58,7 @@ from vyos.ifconfig.control import Control from vyos.ifconfig.vrrp import VRRP from vyos.ifconfig.operational import Operational from vyos.ifconfig import Section +from vyos import ConfigError link_local_prefix = 'fe80::/64' @@ -418,6 +419,12 @@ class Interface(Control): # This will internally stop DHCP(v6) if running self.flush_addrs() + # Remove reverse-path filter entries before the interface is deleted. + # Map keys are interface indexes resolved from ifname, so this must + # happen while the interface still exists. + self.set_ipv4_source_validation('disable') + self.set_ipv6_source_validation('disable') + # remove interface from conntrack VRF interface map self._del_interface_from_ct_iface_map() @@ -882,13 +889,43 @@ class Interface(Control): return None return self.set_interface('ipv4_directed_broadcast', forwarding) - def _cleanup_ipv4_source_validation_rules(self, ifname): - results = self._cmdl(['nft', '-a', 'list', 'chain', 'ip', 'raw', 'vyos_rpfilter']).split("\n") + def _get_nft_set_elements(self, family, table, set_name): + """Return elements of an nftables set, or [] if none are present.""" + tmp = json.loads(self._cmdl(['nft', '-j', 'list', 'set', family, table, set_name])) + return dict_search('set.elem', tmp['nftables'][1], []) + + def _get_nft_rule_handle(self, family, table, chain, match): + """Return handle of first rule matching match, or None if not found.""" + results = self._cmdl(['nft', '-a', 'list', 'chain', family, table, chain]).split('\n') for line in results: - if f'iifname "{ifname}"' in line: - handle_search = re.search('handle (\d+)', line) + if match in line: + handle_search = re.search(r'handle (\d+)', line) if handle_search: - self._cmdl(['nft', 'delete', 'rule', 'ip', 'raw', 'vyos_rpfilter', 'handle', handle_search[1]]) + return handle_search[1] + return None + + def _get_rpf_interface_rules(self, family): + return self._get_nft_set_elements(family, 'raw', 'rpfilter_strict_ifaces'), self._get_nft_set_elements(family, 'raw', 'rpfilter_loose_ifaces') + + def _set_rpf_rule_deletes(self, family, rpf_dict, strict_needs_rule, loose_needs_rule): + if strict_needs_rule: + handle = self._get_nft_rule_handle(family, 'raw', 'vyos_rpfilter', '@rpfilter_strict_ifaces') + if handle: + rpf_dict['strict_delete'] = True + rpf_dict['strict_handle'] = handle + if loose_needs_rule: + handle = self._get_nft_rule_handle(family, 'raw', 'vyos_rpfilter', '@rpfilter_loose_ifaces') + if handle: + rpf_dict['loose_delete'] = True + rpf_dict['loose_handle'] = handle + + def _apply_rpf_nft_config(self, rpf_dict): + rpf_template = '/run/nftables-source-validation.conf' + render(rpf_template, 'firewall/nftables-source-validation.j2', rpf_dict) + tmp = run(['nft', '-c', '-f', rpf_template]) + if tmp > 0: + raise ConfigError('Source validation configuration file errors encountered!') + run(['nft', '-f', rpf_template]) def set_ipv4_source_validation(self, mode): """ @@ -902,23 +939,27 @@ class Interface(Control): if 'netns' in self.config: return None - self._cleanup_ipv4_source_validation_rules(self.ifname) - nft_prefix = ['nft', 'insert', 'rule', 'ip', 'raw', 'vyos_rpfilter', - 'iifname', f'"{self.ifname}"'] - if mode in ['strict', 'loose']: - self._cmdl(nft_prefix + ['counter', 'return']) - if mode == 'strict': - self._cmdl(nft_prefix + ['fib', 'saddr', '.', 'iif', 'oif', '0', 'counter', 'drop']) - elif mode == 'loose': - self._cmdl(nft_prefix + ['fib', 'saddr', 'oif', '0', 'counter', 'drop']) - - def _cleanup_ipv6_source_validation_rules(self, ifname): - results = self._cmdl(['nft', '-a', 'list', 'chain', 'ip6', 'raw', 'vyos_rpfilter']).split("\n") - for line in results: - if f'iifname "{ifname}"' in line: - handle_search = re.search('handle (\d+)', line) - if handle_search: - self._cmdl(['nft', 'delete', 'rule', 'ip6', 'raw', 'vyos_rpfilter', 'handle', handle_search[1]]) + strict_ifaces, loose_ifaces = self._get_rpf_interface_rules('ip') + + rpf_dict = { + 'family': 'ip', + 'iface': self.ifname, + 'mode': mode + } + + strict_needs_rule = (len(strict_ifaces) == 0 or + (len(strict_ifaces) == 1 and self.ifname in strict_ifaces)) + loose_needs_rule = (len(loose_ifaces) == 0 or + (len(loose_ifaces) == 1 and self.ifname in loose_ifaces)) + + self._set_rpf_rule_deletes('ip', rpf_dict, strict_needs_rule, loose_needs_rule) + + if mode == 'strict' and strict_needs_rule: + rpf_dict['strict_add'] = True + if mode == 'loose' and loose_needs_rule: + rpf_dict['loose_add'] = True + + self._apply_rpf_nft_config(rpf_dict) def set_ipv6_source_validation(self, mode): """ @@ -932,15 +973,27 @@ class Interface(Control): if 'netns' in self.config: return None - self._cleanup_ipv6_source_validation_rules(self.ifname) - nft_prefix = ['nft', 'insert', 'rule', 'ip6', 'raw', 'vyos_rpfilter', - 'iifname', f'"{self.ifname}"'] - if mode in ['strict', 'loose']: - self._cmdl(nft_prefix + ['counter', 'return']) - if mode == 'strict': - self._cmdl(nft_prefix + ['fib', 'saddr', '.', 'iif', 'oif', '0', 'counter', 'drop']) - elif mode == 'loose': - self._cmdl(nft_prefix + ['fib', 'saddr', 'oif', '0', 'counter', 'drop']) + strict_ifaces, loose_ifaces = self._get_rpf_interface_rules('ip6') + + rpf_dict = { + 'family': 'ip6', + 'iface': self.ifname, + 'mode': mode + } + + strict_needs_rule = (len(strict_ifaces) == 0 or + (len(strict_ifaces) == 1 and self.ifname in strict_ifaces)) + loose_needs_rule = (len(loose_ifaces) == 0 or + (len(loose_ifaces) == 1 and self.ifname in loose_ifaces)) + + self._set_rpf_rule_deletes('ip6', rpf_dict, strict_needs_rule, loose_needs_rule) + + if mode == 'strict' and strict_needs_rule: + rpf_dict['strict_add'] = True + if mode == 'loose' and loose_needs_rule: + rpf_dict['loose_add'] = True + + self._apply_rpf_nft_config(rpf_dict) def set_ipv6_accept_ra(self, accept_ra): """ @@ -1942,12 +1995,12 @@ class Interface(Control): # IPv4 source-validation tmp = dict_search('ip.source_validation', config) - value = tmp if (tmp != None) else '0' + value = tmp if (tmp != None) else 'disable' self.set_ipv4_source_validation(value) # IPv6 source-validation tmp = dict_search('ipv6.source_validation', config) - value = tmp if (tmp != None) else '0' + value = tmp if (tmp != None) else 'disable' self.set_ipv6_source_validation(value) # MTU - Maximum Transfer Unit has a default value. It must ALWAYS be set -- cgit v1.2.3 From da354b88d916f4e7ba1e888a0e3501a5307ec3b5 Mon Sep 17 00:00:00 2001 From: l0crian1 Date: Mon, 3 Aug 2026 13:58:16 -0400 Subject: T9152: Updated nft -c check to return actual error message --- python/vyos/ifconfig/interface.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'python') diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index b58eccd43..b567f4ca9 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -920,12 +920,23 @@ class Interface(Control): rpf_dict['loose_handle'] = handle def _apply_rpf_nft_config(self, rpf_dict): + from subprocess import STDOUT + rpf_template = '/run/nftables-source-validation.conf' render(rpf_template, 'firewall/nftables-source-validation.j2', rpf_dict) - tmp = run(['nft', '-c', '-f', rpf_template]) - if tmp > 0: - raise ConfigError('Source validation configuration file errors encountered!') - run(['nft', '-f', rpf_template]) + + cmdl( + ['nft', '-c', '--file', rpf_template], + stderr=STDOUT, + raising=ConfigError, + message='Source validation nftables check failed', + ) + cmdl( + ['nft', '-f', rpf_template], + stderr=STDOUT, + raising=ConfigError, + message='Failed to apply source validation nftables config', + ) def set_ipv4_source_validation(self, mode): """ -- cgit v1.2.3 From d15fb0e60f8f61fe8b965df571c74bf1c9e09fad Mon Sep 17 00:00:00 2001 From: l0crian1 Date: Mon, 10 Aug 2026 22:53:51 -0400 Subject: T9152: Refactor source-validation to use vmaps - Reduced rule processing of source-validation rules through hashable objects --- .../firewall/nftables-source-validation.j2 | 17 +++++---- data/vyos-firewall-init.conf | 12 ------- python/vyos/ifconfig/interface.py | 42 ++++++++++++---------- smoketest/scripts/cli/base_interfaces_test.py | 6 ++-- 4 files changed, 36 insertions(+), 41 deletions(-) (limited to 'python') diff --git a/data/templates/firewall/nftables-source-validation.j2 b/data/templates/firewall/nftables-source-validation.j2 index 28579a57e..26018e317 100644 --- a/data/templates/firewall/nftables-source-validation.j2 +++ b/data/templates/firewall/nftables-source-validation.j2 @@ -1,14 +1,17 @@ -{% if strict_delete is vyos_defined %} -destroy rule {{ family }} raw vyos_rpfilter handle {{ strict_handle }} +{% if strict_handles is vyos_defined %} +{% for handle in strict_handles %} +destroy rule {{ family }} raw vyos_rpfilter handle {{ handle }} +{% endfor %} {% endif %} -{% if loose_delete is vyos_defined %} -destroy rule {{ family }} raw vyos_rpfilter handle {{ loose_handle }} +{% if loose_handles is vyos_defined %} +{% for handle in loose_handles %} +destroy rule {{ family }} raw vyos_rpfilter handle {{ handle }} +{% endfor %} {% endif %} destroy element {{ family }} raw rpfilter_strict_ifaces { "{{ iface }}" } destroy element {{ family }} raw rpfilter_loose_ifaces { "{{ iface }}" } destroy element {{ family }} raw rpfilter_strict { "{{ iface }}" } -destroy element {{ family }} raw rpfilter_loose { "{{ iface }}" } {% if mode is vyos_defined('strict') %} add element {{ family }} raw rpfilter_strict_ifaces { "{{ iface }}" } @@ -18,8 +21,8 @@ insert rule {{ family }} raw vyos_rpfilter iifname @rpfilter_strict_ifaces fib s {% endif %} {% elif mode is vyos_defined('loose') %} add element {{ family }} raw rpfilter_loose_ifaces { "{{ iface }}" } -add element {{ family }} raw rpfilter_loose { "{{ iface }}" : accept } {% if loose_add is vyos_defined %} -insert rule {{ family }} raw vyos_rpfilter iifname @rpfilter_loose_ifaces fib saddr oif vmap @rpfilter_loose +insert rule {{ family }} raw vyos_rpfilter iifname @rpfilter_loose_ifaces counter drop +insert rule {{ family }} raw vyos_rpfilter iifname @rpfilter_loose_ifaces fib saddr oif != 0 counter accept {% endif %} {% endif %} diff --git a/data/vyos-firewall-init.conf b/data/vyos-firewall-init.conf index 6db8bc231..e32513c1a 100644 --- a/data/vyos-firewall-init.conf +++ b/data/vyos-firewall-init.conf @@ -14,12 +14,6 @@ table raw { elements = { 0 : drop } } - map rpfilter_loose { - typeof fib saddr oif : verdict - counter - elements = { 0 : drop } - } - set rpfilter_strict_ifaces { type ifname } @@ -53,12 +47,6 @@ table ip6 raw { elements = { 0 : drop } } - map rpfilter_loose { - typeof fib saddr oif : verdict - counter - elements = { 0 : drop } - } - set rpfilter_strict_ifaces { type ifname } diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index b567f4ca9..9d04e6a27 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -892,32 +892,36 @@ class Interface(Control): def _get_nft_set_elements(self, family, table, set_name): """Return elements of an nftables set, or [] if none are present.""" tmp = json.loads(self._cmdl(['nft', '-j', 'list', 'set', family, table, set_name])) - return dict_search('set.elem', tmp['nftables'][1], []) - - def _get_nft_rule_handle(self, family, table, chain, match): - """Return handle of first rule matching match, or None if not found.""" - results = self._cmdl(['nft', '-a', 'list', 'chain', family, table, chain]).split('\n') - for line in results: - if match in line: - handle_search = re.search(r'handle (\d+)', line) - if handle_search: - return handle_search[1] - return None + elems = dict_search('set.elem', tmp['nftables'][1], []) or [] + # nft may return a bare value for a single element instead of a list + if not isinstance(elems, list): + elems = [elems] + return elems + + def _get_nft_rule_handles(self, family, table, chain, set_name): + """Return handles of all rules referencing set_name.""" + handles = [] + tmp = json.loads(self._cmdl(['nft', '-j', 'list', 'chain', family, table, chain])) + for entry in tmp.get('nftables', []): + rule = entry.get('rule') + if not rule: + continue + if set_name in json.dumps(rule.get('expr', [])): + handles.append(str(rule['handle'])) + return handles def _get_rpf_interface_rules(self, family): return self._get_nft_set_elements(family, 'raw', 'rpfilter_strict_ifaces'), self._get_nft_set_elements(family, 'raw', 'rpfilter_loose_ifaces') def _set_rpf_rule_deletes(self, family, rpf_dict, strict_needs_rule, loose_needs_rule): if strict_needs_rule: - handle = self._get_nft_rule_handle(family, 'raw', 'vyos_rpfilter', '@rpfilter_strict_ifaces') - if handle: - rpf_dict['strict_delete'] = True - rpf_dict['strict_handle'] = handle + handles = self._get_nft_rule_handles(family, 'raw', 'vyos_rpfilter', 'rpfilter_strict_ifaces') + if handles: + rpf_dict['strict_handles'] = handles if loose_needs_rule: - handle = self._get_nft_rule_handle(family, 'raw', 'vyos_rpfilter', '@rpfilter_loose_ifaces') - if handle: - rpf_dict['loose_delete'] = True - rpf_dict['loose_handle'] = handle + handles = self._get_nft_rule_handles(family, 'raw', 'vyos_rpfilter', 'rpfilter_loose_ifaces') + if handles: + rpf_dict['loose_handles'] = handles def _apply_rpf_nft_config(self, rpf_dict): from subprocess import STDOUT diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index 774b3716d..4a7334905 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -1155,11 +1155,11 @@ class BasicInterfaceTest: self.assertEqual('1', tmp) if cli_defined(self._base_path + ['ip'], 'source-validation'): - base_options = f'iifname @rpfilter_loose_ifaces' - out = cmdl(['nft', 'list', 'chain', 'ip', 'raw', 'vyos_rpfilter'], sudo=True) + base_options = f'iifname @rpfilter_loose_ifaces fib saddr oif' + out = cmdl(['nft', '-s', 'list', 'chain', 'ip', 'raw', 'vyos_rpfilter'], sudo=True) for line in out.splitlines(): if base_options in line: - self.assertIn('iifname @rpfilter_loose_ifaces fib saddr oif vmap @rpfilter_loose', line) + self.assertIn('iifname @rpfilter_loose_ifaces fib saddr oif != 0 counter accept', line) def test_interface_ipv6_options(self): if not self._test_ipv6: -- cgit v1.2.3