From df176d9b9b4cc67ae509ae2ff17a02f2520cc881 Mon Sep 17 00:00:00 2001 From: Nicolas Fort Date: Tue, 29 Oct 2024 19:05:52 +0000 Subject: [PATCH 1/4] T6841: firewall: improve config parsing for ZBF when using VRFs and interfaces attached to VRFs --- data/templates/firewall/nftables-zone.j2 | 57 +++++++++-- interface-definitions/firewall.xml.in | 45 ++++++--- .../include/version/firewall-version.xml.i | 2 +- python/vyos/utils/network.py | 4 +- smoketest/scripts/cli/test_firewall.py | 94 ++++++++++++++++++- src/conf_mode/firewall.py | 28 +++++- src/migration-scripts/firewall/16-to-17 | 0 src/migration-scripts/firewall/17-to-18 | 36 +++++++ 8 files changed, 235 insertions(+), 31 deletions(-) mode change 100755 => 100644 interface-definitions/firewall.xml.in mode change 100755 => 100644 src/migration-scripts/firewall/16-to-17 create mode 100755 src/migration-scripts/firewall/17-to-18 diff --git a/data/templates/firewall/nftables-zone.j2 b/data/templates/firewall/nftables-zone.j2 index e787250794..1f1d8cf24b 100644 --- a/data/templates/firewall/nftables-zone.j2 +++ b/data/templates/firewall/nftables-zone.j2 @@ -8,7 +8,15 @@ {% endif %} {% for zone_name, zone_conf in zone.items() %} {% if 'local_zone' not in zone_conf %} - oifname { {{ zone_conf.interface | join(',') }} } counter jump VZONE_{{ zone_name }} +{% if 'name' in zone_conf.interface %} + oifname { {{ zone_conf.interface.name | join(',') }} } counter jump VZONE_{{ zone_name }} +{% endif %} +{% if 'vrf' in zone_conf.interface %} +{% for vrf_name in zone_conf.interface.vrf %} + oifname { {{ zone_conf['vrf_interfaces'][vrf_name] }} } counter jump VZONE_{{ zone_name }} + #oifname { {{ zone_conf.interface.vrf | join(',') }} } counter jump VZONE_{{ zone_name }} +{% endfor %} +{% endif %} {% endif %} {% endfor %} } @@ -40,8 +48,15 @@ iifname lo counter return {% if zone_conf.from is vyos_defined %} {% for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall[fw_name] is vyos_defined %} - iifname { {{ zone[from_zone].interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - iifname { {{ zone[from_zone].interface | join(",") }} } counter return + +{% if 'name' in zone[from_zone].interface %} + iifname { {{ zone[from_zone].interface.name | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].interface.name | join(",") }} } counter return +{% endif %} +{% if 'vrf' in zone[from_zone].interface %} + iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter return +{% endif %} {% endfor %} {% endif %} {{ zone_conf | nft_default_rule('zone_' + zone_name, family) }} @@ -50,23 +65,47 @@ oifname lo counter return {% if zone_conf.from_local is vyos_defined %} {% for from_zone, from_conf in zone_conf.from_local.items() if from_conf.firewall[fw_name] is vyos_defined %} - oifname { {{ zone[from_zone].interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - oifname { {{ zone[from_zone].interface | join(",") }} } counter return +{% if 'name' in zone[from_zone].interface %} + oifname { {{ zone[from_zone].interface.name | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + oifname { {{ zone[from_zone].interface.name | join(",") }} } counter return +{% endif %} +{% if 'vrf' in zone[from_zone].interface %} +{% for vrf_name in zone[from_zone].interface.vrf %} + oifname { {{ zone[from_zone]['vrf_interfaces'][vrf_name] }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + oifname { {{ zone[from_zone]['vrf_interfaces'][vrf_name] }} } counter return +{% endfor %} +{% endif %} {% endfor %} {% endif %} {{ zone_conf | nft_default_rule('zone_' + zone_name, family) }} } {% else %} chain VZONE_{{ zone_name }} { - iifname { {{ zone_conf.interface | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }} +{% if 'name' in zone_conf.interface %} + iifname { {{ zone_conf.interface.name | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }} +{% endif %} +{% if 'vrf' in zone_conf.interface %} + iifname { {{ zone_conf.interface.vrf | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }} +{% endif %} {% if zone_conf.intra_zone_filtering is vyos_defined %} - iifname { {{ zone_conf.interface | join(",") }} } counter return +{% if 'name' in zone_conf.interface %} + iifname { {{ zone_conf.interface.name | join(",") }} } counter return +{% endif %} +{% if 'vrf' in zone_conf.interface %} + iifname { {{ zone_conf.interface.vrf | join(",") }} } counter return +{% endif %} {% endif %} {% if zone_conf.from is vyos_defined %} {% for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall[fw_name] is vyos_defined %} {% if zone[from_zone].local_zone is not defined %} - iifname { {{ zone[from_zone].interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - iifname { {{ zone[from_zone].interface | join(",") }} } counter return +{% if 'name' in zone[from_zone].interface %} + iifname { {{ zone[from_zone].interface.name | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].interface.name | join(",") }} } counter return +{% endif %} +{% if 'vrf' in zone[from_zone].interface %} + iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter return +{% endif %} {% endif %} {% endfor %} {% endif %} diff --git a/interface-definitions/firewall.xml.in b/interface-definitions/firewall.xml.in old mode 100755 new mode 100644 index 07c88f799e..5bb269ae6d --- a/interface-definitions/firewall.xml.in +++ b/interface-definitions/firewall.xml.in @@ -464,24 +464,39 @@ - + Interface associated with zone - - txt - Interface associated with zone - - - vrf - VRF associated with zone - - - - vrf name - - - + + + + Interface associated with zone + + txt + Interface associated with zone + + + + + + + + + + VRF associated with zone + + vrf + VRF associated with zone + + + vrf name + + + + + + Intra-zone filtering diff --git a/interface-definitions/include/version/firewall-version.xml.i b/interface-definitions/include/version/firewall-version.xml.i index a15cf0eece..1a8098297e 100644 --- a/interface-definitions/include/version/firewall-version.xml.i +++ b/interface-definitions/include/version/firewall-version.xml.i @@ -1,3 +1,3 @@ - + diff --git a/python/vyos/utils/network.py b/python/vyos/utils/network.py index 8fce08de09..dc0c0a6d6a 100644 --- a/python/vyos/utils/network.py +++ b/python/vyos/utils/network.py @@ -69,7 +69,9 @@ def get_vrf_members(vrf: str) -> list: answer = json.loads(output) for data in answer: if 'ifname' in data: - interfaces.append(data.get('ifname')) + # Skip PIM interfaces which appears in VRF + if 'pim' not in data.get('ifname'): + interfaces.append(data.get('ifname')) except: pass return interfaces diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py index 6420afa382..b71c062cc0 100755 --- a/smoketest/scripts/cli/test_firewall.py +++ b/smoketest/scripts/cli/test_firewall.py @@ -906,7 +906,7 @@ def test_timeout_sysctl(self): def test_zone_basic(self): self.cli_set(['firewall', 'ipv4', 'name', 'smoketest', 'default-action', 'drop']) self.cli_set(['firewall', 'ipv6', 'name', 'smoketestv6', 'default-action', 'drop']) - self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'interface', 'eth0']) + self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'interface', 'name', 'eth0']) self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'from', 'smoketest-local', 'firewall', 'name', 'smoketest']) self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'intra-zone-filtering', 'firewall', 'ipv6-name', 'smoketestv6']) self.cli_set(['firewall', 'zone', 'smoketest-local', 'local-zone']) @@ -964,6 +964,98 @@ def test_zone_basic(self): self.verify_nftables(nftables_search, 'ip vyos_filter') self.verify_nftables(nftables_search_v6, 'ip6 vyos_filter') + def test_zone_with_vrf(self): + self.cli_set(['firewall', 'ipv4', 'name', 'ZONE1-to-LOCAL', 'default-action', 'accept']) + self.cli_set(['firewall', 'ipv4', 'name', 'ZONE2_to_ZONE1', 'default-action', 'continue']) + self.cli_set(['firewall', 'ipv6', 'name', 'LOCAL_to_ZONE2_v6', 'default-action', 'drop']) + self.cli_set(['firewall', 'zone', 'LOCAL', 'from', 'ZONE1', 'firewall', 'name', 'ZONE1-to-LOCAL']) + self.cli_set(['firewall', 'zone', 'LOCAL', 'local-zone']) + self.cli_set(['firewall', 'zone', 'ZONE1', 'from', 'ZONE2', 'firewall', 'name', 'ZONE2_to_ZONE1']) + self.cli_set(['firewall', 'zone', 'ZONE1', 'interface', 'name', 'eth1']) + self.cli_set(['firewall', 'zone', 'ZONE1', 'interface', 'name', 'eth2']) + self.cli_set(['firewall', 'zone', 'ZONE1', 'interface', 'vrf', 'VRF-1']) + self.cli_set(['firewall', 'zone', 'ZONE2', 'from', 'LOCAL', 'firewall', 'ipv6-name', 'LOCAL_to_ZONE2_v6']) + self.cli_set(['firewall', 'zone', 'ZONE2', 'interface', 'name', 'vtun66']) + self.cli_set(['firewall', 'zone', 'ZONE2', 'interface', 'vrf', 'VRF-2']) + + self.cli_set(['vrf', 'name', 'VRF-1', 'table', '101']) + self.cli_set(['vrf', 'name', 'VRF-2', 'table', '102']) + self.cli_set(['interfaces', 'ethernet', 'eth0', 'vrf', 'VRF-1']) + self.cli_set(['interfaces', 'vti', 'vti1', 'vrf', 'VRF-2']) + + self.cli_commit() + + nftables_search = [ + ['chain NAME_ZONE1-to-LOCAL'], + ['counter', 'accept', 'comment "NAM-ZONE1-to-LOCAL default-action accept"'], + ['chain NAME_ZONE2_to_ZONE1'], + ['counter', 'continue', 'comment "NAM-ZONE2_to_ZONE1 default-action continue"'], + ['chain VYOS_ZONE_FORWARD'], + ['type filter hook forward priority filter + 1'], + ['oifname { "eth1", "eth2" }', 'counter packets', 'jump VZONE_ZONE1'], + ['oifname "eth0"', 'counter packets', 'jump VZONE_ZONE1'], + ['oifname "vtun66"', 'counter packets', 'jump VZONE_ZONE2'], + ['oifname "vti1"', 'counter packets', 'jump VZONE_ZONE2'], + ['chain VYOS_ZONE_LOCAL'], + ['type filter hook input priority filter + 1'], + ['counter packets', 'jump VZONE_LOCAL_IN'], + ['chain VYOS_ZONE_OUTPUT'], + ['type filter hook output priority filter + 1'], + ['counter packets', 'jump VZONE_LOCAL_OUT'], + ['chain VZONE_LOCAL_IN'], + ['iifname { "eth1", "eth2" }', 'counter packets', 'jump NAME_ZONE1-to-LOCAL'], + ['iifname "VRF-1"', 'counter packets', 'jump NAME_ZONE1-to-LOCAL'], + ['counter packets', 'drop', 'comment "zone_LOCAL default-action drop"'], + ['chain VZONE_LOCAL_OUT'], + ['counter packets', 'drop', 'comment "zone_LOCAL default-action drop"'], + ['chain VZONE_ZONE1'], + ['iifname { "eth1", "eth2" }', 'counter packets', 'return'], + ['iifname "VRF-1"', 'counter packets', 'return'], + ['iifname "vtun66"', 'counter packets', 'jump NAME_ZONE2_to_ZONE1'], + ['iifname "vtun66"', 'counter packets', 'return'], + ['iifname "VRF-2"', 'counter packets', 'jump NAME_ZONE2_to_ZONE1'], + ['iifname "VRF-2"', 'counter packets', 'return'], + ['counter packets', 'drop', 'comment "zone_ZONE1 default-action drop"'], + ['chain VZONE_ZONE2'], + ['iifname "vtun66"', 'counter packets', 'return'], + ['iifname "VRF-2"', 'counter packets', 'return'], + ['counter packets', 'drop', 'comment "zone_ZONE2 default-action drop"'] + ] + + nftables_search_v6 = [ + ['chain NAME6_LOCAL_to_ZONE2_v6'], + ['counter', 'drop', 'comment "NAM-LOCAL_to_ZONE2_v6 default-action drop"'], + ['chain VYOS_ZONE_FORWARD'], + ['type filter hook forward priority filter + 1'], + ['oifname { "eth1", "eth2" }', 'counter packets', 'jump VZONE_ZONE1'], + ['oifname "eth0"', 'counter packets', 'jump VZONE_ZONE1'], + ['oifname "vtun66"', 'counter packets', 'jump VZONE_ZONE2'], + ['oifname "vti1"', 'counter packets', 'jump VZONE_ZONE2'], + ['chain VYOS_ZONE_LOCAL'], + ['type filter hook input priority filter + 1'], + ['counter packets', 'jump VZONE_LOCAL_IN'], + ['chain VYOS_ZONE_OUTPUT'], + ['type filter hook output priority filter + 1'], + ['counter packets', 'jump VZONE_LOCAL_OUT'], + ['chain VZONE_LOCAL_IN'], + ['counter packets', 'drop', 'comment "zone_LOCAL default-action drop"'], + ['chain VZONE_LOCAL_OUT'], + ['oifname "vtun66"', 'counter packets', 'jump NAME6_LOCAL_to_ZONE2_v6'], + ['oifname "vti1"', 'counter packets', 'jump NAME6_LOCAL_to_ZONE2_v6'], + ['counter packets', 'drop', 'comment "zone_LOCAL default-action drop"'], + ['chain VZONE_ZONE1'], + ['iifname { "eth1", "eth2" }', 'counter packets', 'return'], + ['iifname "VRF-1"', 'counter packets', 'return'], + ['counter packets', 'drop', 'comment "zone_ZONE1 default-action drop"'], + ['chain VZONE_ZONE2'], + ['iifname "vtun66"', 'counter packets', 'return'], + ['iifname "VRF-2"', 'counter packets', 'return'], + ['counter packets', 'drop', 'comment "zone_ZONE2 default-action drop"'] + ] + + self.verify_nftables(nftables_search, 'ip vyos_filter') + self.verify_nftables(nftables_search_v6, 'ip6 vyos_filter') + def test_flow_offload(self): self.cli_set(['interfaces', 'ethernet', 'eth0', 'vif', '10']) self.cli_set(['firewall', 'flowtable', 'smoketest', 'interface', 'eth0.10']) diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py index 10d389d730..c09f934aa9 100755 --- a/src/conf_mode/firewall.py +++ b/src/conf_mode/firewall.py @@ -34,6 +34,8 @@ from vyos.utils.process import call from vyos.utils.process import cmd from vyos.utils.process import rc_cmd +from vyos.utils.network import get_vrf_members +from vyos.utils.network import get_interface_vrf from vyos import ConfigError from vyos import airbag from pathlib import Path @@ -441,6 +443,7 @@ def verify(firewall): local_zone = False zone_interfaces = [] + zone_vrf = [] if 'zone' in firewall: for zone, zone_conf in firewall['zone'].items(): @@ -457,12 +460,23 @@ def verify(firewall): local_zone = True if 'interface' in zone_conf: - found_duplicates = [intf for intf in zone_conf['interface'] if intf in zone_interfaces] + if 'name'in zone_conf['interface']: - if found_duplicates: - raise ConfigError(f'Interfaces cannot be assigned to multiple zones') + for iface in zone_conf['interface']['name']: - zone_interfaces += zone_conf['interface'] + if iface in zone_interfaces: + raise ConfigError(f'Interfaces cannot be assigned to multiple zones') + + iface_vrf = get_interface_vrf(iface) + if iface_vrf != 'default': + Warning(f"Interface {iface} assigned to zone {zone} is in VRF {iface_vrf}. This might not work as expected.") + zone_interfaces += iface + + if 'vrf' in zone_conf['interface']: + for vrf in zone_conf['interface']['vrf']: + if vrf in zone_vrf: + raise ConfigError(f'VRF cannot be assigned to multiple zones') + zone_vrf += vrf if 'intra_zone_filtering' in zone_conf: intra_zone = zone_conf['intra_zone_filtering'] @@ -504,6 +518,12 @@ def generate(firewall): if 'zone' in firewall: for local_zone, local_zone_conf in firewall['zone'].items(): if 'local_zone' not in local_zone_conf: + # Get physical interfaces assigned to the zone if vrf is used: + if 'vrf' in local_zone_conf['interface']: + local_zone_conf['vrf_interfaces'] = {} + for vrf_name in local_zone_conf['interface']['vrf']: + local_zone_conf['vrf_interfaces'][vrf_name] = ','.join(get_vrf_members(vrf_name)) + #local_zone_conf['interface']['vrf'][vrf_name] = ''.join(get_vrf_members(vrf_name)) continue local_zone_conf['from_local'] = {} diff --git a/src/migration-scripts/firewall/16-to-17 b/src/migration-scripts/firewall/16-to-17 old mode 100755 new mode 100644 diff --git a/src/migration-scripts/firewall/17-to-18 b/src/migration-scripts/firewall/17-to-18 new file mode 100755 index 0000000000..af16ba8ec9 --- /dev/null +++ b/src/migration-scripts/firewall/17-to-18 @@ -0,0 +1,36 @@ +# Copyright (C) 2024 VyOS maintainers and contributors +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library. If not, see . + +# From + # set firewall zone interface +# To + # set firewall zone interface name + # or + # set firewall zone interface vrf + + +from vyos.configtree import ConfigTree + +base = ['firewall', 'zone'] + +def migrate(config: ConfigTree) -> None: + if not config.exists(base): + # Nothing to do + return + + for zone in config.list_nodes(base): + if config.exists(base + [zone, 'interface']): + for iface in config.return_values(base + [zone, 'interface']): + config.set(base + [zone, 'interface', 'name'], value=iface, replace=False) \ No newline at end of file From 4a194b32509ffcd9574bb7571a5a6347f7dc4e42 Mon Sep 17 00:00:00 2001 From: aapostoliuk Date: Tue, 17 Dec 2024 13:39:49 +0200 Subject: [PATCH 2/4] T6841: firewall: Fixed issues in ZBF when using VRFs Improve config parsing for ZBF when using VRFs and interfaces attached to VRFs --- data/templates/firewall/nftables-zone.j2 | 59 ++++++++++---------- interface-definitions/firewall.xml.in | 4 +- smoketest/scripts/cli/test_firewall.py | 14 ++--- src/conf_mode/firewall.py | 69 ++++++++++++------------ src/migration-scripts/firewall/17-to-18 | 7 +-- src/op_mode/zone.py | 11 ++-- 6 files changed, 86 insertions(+), 78 deletions(-) diff --git a/data/templates/firewall/nftables-zone.j2 b/data/templates/firewall/nftables-zone.j2 index 1f1d8cf24b..645a387067 100644 --- a/data/templates/firewall/nftables-zone.j2 +++ b/data/templates/firewall/nftables-zone.j2 @@ -8,13 +8,12 @@ {% endif %} {% for zone_name, zone_conf in zone.items() %} {% if 'local_zone' not in zone_conf %} -{% if 'name' in zone_conf.interface %} - oifname { {{ zone_conf.interface.name | join(',') }} } counter jump VZONE_{{ zone_name }} +{% if 'interface' in zone_conf.member %} + oifname { {{ zone_conf.member.interface | join(',') }} } counter jump VZONE_{{ zone_name }} {% endif %} -{% if 'vrf' in zone_conf.interface %} -{% for vrf_name in zone_conf.interface.vrf %} +{% if 'vrf' in zone_conf.member %} +{% for vrf_name in zone_conf.member.vrf %} oifname { {{ zone_conf['vrf_interfaces'][vrf_name] }} } counter jump VZONE_{{ zone_name }} - #oifname { {{ zone_conf.interface.vrf | join(',') }} } counter jump VZONE_{{ zone_name }} {% endfor %} {% endif %} {% endif %} @@ -49,13 +48,13 @@ {% if zone_conf.from is vyos_defined %} {% for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall[fw_name] is vyos_defined %} -{% if 'name' in zone[from_zone].interface %} - iifname { {{ zone[from_zone].interface.name | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - iifname { {{ zone[from_zone].interface.name | join(",") }} } counter return +{% if 'interface' in zone[from_zone].member %} + iifname { {{ zone[from_zone].member.interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].member.interface | join(",") }} } counter return {% endif %} -{% if 'vrf' in zone[from_zone].interface %} - iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter return +{% if 'vrf' in zone[from_zone].member %} + iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter return {% endif %} {% endfor %} {% endif %} @@ -65,12 +64,12 @@ oifname lo counter return {% if zone_conf.from_local is vyos_defined %} {% for from_zone, from_conf in zone_conf.from_local.items() if from_conf.firewall[fw_name] is vyos_defined %} -{% if 'name' in zone[from_zone].interface %} - oifname { {{ zone[from_zone].interface.name | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - oifname { {{ zone[from_zone].interface.name | join(",") }} } counter return +{% if 'interface' in zone[from_zone].member %} + oifname { {{ zone[from_zone].member.interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + oifname { {{ zone[from_zone].member.interface | join(",") }} } counter return {% endif %} -{% if 'vrf' in zone[from_zone].interface %} -{% for vrf_name in zone[from_zone].interface.vrf %} +{% if 'vrf' in zone[from_zone].member %} +{% for vrf_name in zone[from_zone].member.vrf %} oifname { {{ zone[from_zone]['vrf_interfaces'][vrf_name] }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} oifname { {{ zone[from_zone]['vrf_interfaces'][vrf_name] }} } counter return {% endfor %} @@ -81,30 +80,30 @@ } {% else %} chain VZONE_{{ zone_name }} { -{% if 'name' in zone_conf.interface %} - iifname { {{ zone_conf.interface.name | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }} +{% if 'interface' in zone_conf.member %} + iifname { {{ zone_conf.member.interface | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }} {% endif %} -{% if 'vrf' in zone_conf.interface %} - iifname { {{ zone_conf.interface.vrf | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }} +{% if 'vrf' in zone_conf.member %} + iifname { {{ zone_conf.member.vrf | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }} {% endif %} {% if zone_conf.intra_zone_filtering is vyos_defined %} -{% if 'name' in zone_conf.interface %} - iifname { {{ zone_conf.interface.name | join(",") }} } counter return +{% if 'interface' in zone_conf.member %} + iifname { {{ zone_conf.member.interface | join(",") }} } counter return {% endif %} -{% if 'vrf' in zone_conf.interface %} - iifname { {{ zone_conf.interface.vrf | join(",") }} } counter return +{% if 'vrf' in zone_conf.member %} + iifname { {{ zone_conf.member.vrf | join(",") }} } counter return {% endif %} {% endif %} {% if zone_conf.from is vyos_defined %} {% for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall[fw_name] is vyos_defined %} {% if zone[from_zone].local_zone is not defined %} -{% if 'name' in zone[from_zone].interface %} - iifname { {{ zone[from_zone].interface.name | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - iifname { {{ zone[from_zone].interface.name | join(",") }} } counter return +{% if 'interface' in zone[from_zone].member %} + iifname { {{ zone[from_zone].member.interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].member.interface | join(",") }} } counter return {% endif %} -{% if 'vrf' in zone[from_zone].interface %} - iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} - iifname { {{ zone[from_zone].interface.vrf | join(",") }} } counter return +{% if 'vrf' in zone[from_zone].member %} + iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }} + iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter return {% endif %} {% endif %} {% endfor %} diff --git a/interface-definitions/firewall.xml.in b/interface-definitions/firewall.xml.in index 5bb269ae6d..018e0dbdfb 100644 --- a/interface-definitions/firewall.xml.in +++ b/interface-definitions/firewall.xml.in @@ -464,12 +464,12 @@ - + Interface associated with zone - + Interface associated with zone diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py index b71c062cc0..10301831e4 100755 --- a/smoketest/scripts/cli/test_firewall.py +++ b/smoketest/scripts/cli/test_firewall.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2021-2023 VyOS maintainers and contributors +# Copyright (C) 2021-2024 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 @@ -906,7 +906,7 @@ def test_timeout_sysctl(self): def test_zone_basic(self): self.cli_set(['firewall', 'ipv4', 'name', 'smoketest', 'default-action', 'drop']) self.cli_set(['firewall', 'ipv6', 'name', 'smoketestv6', 'default-action', 'drop']) - self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'interface', 'name', 'eth0']) + self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'member', 'interface', 'eth0']) self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'from', 'smoketest-local', 'firewall', 'name', 'smoketest']) self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'intra-zone-filtering', 'firewall', 'ipv6-name', 'smoketestv6']) self.cli_set(['firewall', 'zone', 'smoketest-local', 'local-zone']) @@ -971,12 +971,12 @@ def test_zone_with_vrf(self): self.cli_set(['firewall', 'zone', 'LOCAL', 'from', 'ZONE1', 'firewall', 'name', 'ZONE1-to-LOCAL']) self.cli_set(['firewall', 'zone', 'LOCAL', 'local-zone']) self.cli_set(['firewall', 'zone', 'ZONE1', 'from', 'ZONE2', 'firewall', 'name', 'ZONE2_to_ZONE1']) - self.cli_set(['firewall', 'zone', 'ZONE1', 'interface', 'name', 'eth1']) - self.cli_set(['firewall', 'zone', 'ZONE1', 'interface', 'name', 'eth2']) - self.cli_set(['firewall', 'zone', 'ZONE1', 'interface', 'vrf', 'VRF-1']) + self.cli_set(['firewall', 'zone', 'ZONE1', 'member', 'interface', 'eth1']) + self.cli_set(['firewall', 'zone', 'ZONE1', 'member', 'interface', 'eth2']) + self.cli_set(['firewall', 'zone', 'ZONE1', 'member', 'vrf', 'VRF-1']) self.cli_set(['firewall', 'zone', 'ZONE2', 'from', 'LOCAL', 'firewall', 'ipv6-name', 'LOCAL_to_ZONE2_v6']) - self.cli_set(['firewall', 'zone', 'ZONE2', 'interface', 'name', 'vtun66']) - self.cli_set(['firewall', 'zone', 'ZONE2', 'interface', 'vrf', 'VRF-2']) + self.cli_set(['firewall', 'zone', 'ZONE2', 'member', 'interface', 'vtun66']) + self.cli_set(['firewall', 'zone', 'ZONE2', 'member', 'vrf', 'VRF-2']) self.cli_set(['vrf', 'name', 'VRF-1', 'table', '101']) self.cli_set(['vrf', 'name', 'VRF-2', 'table', '102']) diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py index c09f934aa9..768bb127d5 100755 --- a/src/conf_mode/firewall.py +++ b/src/conf_mode/firewall.py @@ -18,7 +18,6 @@ import re from sys import exit - from vyos.base import Warning from vyos.config import Config from vyos.configdict import is_node_changed @@ -135,6 +134,27 @@ def get_config(config=None): fqdn_config_parse(firewall, 'firewall') + if not os.path.exists(nftables_conf): + firewall['first_install'] = True + + if 'zone' in firewall: + for local_zone, local_zone_conf in firewall['zone'].items(): + if 'local_zone' not in local_zone_conf: + # Get physical interfaces assigned to the zone if vrf is used: + if 'vrf' in local_zone_conf['member']: + local_zone_conf['vrf_interfaces'] = {} + for vrf_name in local_zone_conf['member']['vrf']: + local_zone_conf['vrf_interfaces'][vrf_name] = ','.join(get_vrf_members(vrf_name)) + continue + + local_zone_conf['from_local'] = {} + + for zone, zone_conf in firewall['zone'].items(): + if zone == local_zone or 'from' not in zone_conf: + continue + if local_zone in zone_conf['from']: + local_zone_conf['from_local'][zone] = zone_conf['from'][local_zone] + set_dependents('conntrack', conf) return firewall @@ -447,22 +467,21 @@ def verify(firewall): if 'zone' in firewall: for zone, zone_conf in firewall['zone'].items(): - if 'local_zone' not in zone_conf and 'interface' not in zone_conf: + if 'local_zone' not in zone_conf and 'member' not in zone_conf: raise ConfigError(f'Zone "{zone}" has no interfaces and is not the local zone') if 'local_zone' in zone_conf: if local_zone: raise ConfigError('There cannot be multiple local zones') - if 'interface' in zone_conf: + if 'member' in zone_conf: raise ConfigError('Local zone cannot have interfaces assigned') if 'intra_zone_filtering' in zone_conf: raise ConfigError('Local zone cannot use intra-zone-filtering') local_zone = True - if 'interface' in zone_conf: - if 'name'in zone_conf['interface']: - - for iface in zone_conf['interface']['name']: + if 'member' in zone_conf: + if 'interface' in zone_conf['member']: + for iface in zone_conf['member']['interface']: if iface in zone_interfaces: raise ConfigError(f'Interfaces cannot be assigned to multiple zones') @@ -470,13 +489,19 @@ def verify(firewall): iface_vrf = get_interface_vrf(iface) if iface_vrf != 'default': Warning(f"Interface {iface} assigned to zone {zone} is in VRF {iface_vrf}. This might not work as expected.") - zone_interfaces += iface + zone_interfaces.append(iface) - if 'vrf' in zone_conf['interface']: - for vrf in zone_conf['interface']['vrf']: + if 'vrf' in zone_conf['member']: + for vrf in zone_conf['member']['vrf']: if vrf in zone_vrf: raise ConfigError(f'VRF cannot be assigned to multiple zones') - zone_vrf += vrf + zone_vrf.append(vrf) + + if 'vrf_interfaces' in zone_conf: + for vrf_name, vrf_interfaces in zone_conf['vrf_interfaces'].items(): + if not vrf_interfaces: + raise ConfigError( + f'VRF "{vrf_name}" cannot be a member of any zone. It does not contain any interfaces.') if 'intra_zone_filtering' in zone_conf: intra_zone = zone_conf['intra_zone_filtering'] @@ -512,28 +537,6 @@ def verify(firewall): return None def generate(firewall): - if not os.path.exists(nftables_conf): - firewall['first_install'] = True - - if 'zone' in firewall: - for local_zone, local_zone_conf in firewall['zone'].items(): - if 'local_zone' not in local_zone_conf: - # Get physical interfaces assigned to the zone if vrf is used: - if 'vrf' in local_zone_conf['interface']: - local_zone_conf['vrf_interfaces'] = {} - for vrf_name in local_zone_conf['interface']['vrf']: - local_zone_conf['vrf_interfaces'][vrf_name] = ','.join(get_vrf_members(vrf_name)) - #local_zone_conf['interface']['vrf'][vrf_name] = ''.join(get_vrf_members(vrf_name)) - continue - - local_zone_conf['from_local'] = {} - - for zone, zone_conf in firewall['zone'].items(): - if zone == local_zone or 'from' not in zone_conf: - continue - if local_zone in zone_conf['from']: - local_zone_conf['from_local'][zone] = zone_conf['from'][local_zone] - render(nftables_conf, 'firewall/nftables.j2', firewall) render(sysctl_file, 'firewall/sysctl-firewall.conf.j2', firewall) return None diff --git a/src/migration-scripts/firewall/17-to-18 b/src/migration-scripts/firewall/17-to-18 index af16ba8ec9..891f9f1950 100755 --- a/src/migration-scripts/firewall/17-to-18 +++ b/src/migration-scripts/firewall/17-to-18 @@ -16,9 +16,9 @@ # From # set firewall zone interface # To - # set firewall zone interface name + # set firewall zone member interface # or - # set firewall zone interface vrf + # set firewall zone member vrf from vyos.configtree import ConfigTree @@ -33,4 +33,5 @@ def migrate(config: ConfigTree) -> None: for zone in config.list_nodes(base): if config.exists(base + [zone, 'interface']): for iface in config.return_values(base + [zone, 'interface']): - config.set(base + [zone, 'interface', 'name'], value=iface, replace=False) \ No newline at end of file + config.set(base + [zone, 'member', 'interface'], value=iface, replace=False) + config.delete(base + [zone, 'interface']) \ No newline at end of file diff --git a/src/op_mode/zone.py b/src/op_mode/zone.py index 49fecdf284..df39549d2f 100644 --- a/src/op_mode/zone.py +++ b/src/op_mode/zone.py @@ -56,10 +56,15 @@ def _convert_one_zone_data(zone: str, zone_config: dict) -> dict: from_zone_dict['firewall_v6'] = dict_search( 'firewall.ipv6_name', from_zone_config) list_of_rules.append(from_zone_dict) + zone_members =[] + interface_members = dict_search('member.interface', zone_config) + vrf_members = dict_search('member.vrf', zone_config) + zone_members += interface_members if interface_members is not None else [] + zone_members += vrf_members if vrf_members is not None else [] zone_dict = { 'name': zone, - 'interface': dict_search('interface', zone_config), + 'members': zone_members, 'type': 'LOCAL' if dict_search('local_zone', zone_config) is not None else None, } @@ -126,7 +131,7 @@ def output_zone_list(zone_conf: dict) -> list: if zone_conf['type'] == 'LOCAL': zone_info.append('LOCAL') else: - zone_info.append("\n".join(zone_conf['interface'])) + zone_info.append("\n".join(zone_conf['members'])) from_zone = [] firewall = [] @@ -175,7 +180,7 @@ def get_formatted_output(zone_policy: list) -> str: :rtype: str """ headers = ["Zone", - "Interfaces", + "Members", "From Zone", "Firewall IPv4", "Firewall IPv6" From 3b04cc29f0baf618351d67b4d6aa47f55b54bb20 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 23 Dec 2024 13:14:11 +0100 Subject: [PATCH 3/4] T6841: firewall: re-use existing generic-interface-multi.xml.i XML building block --- interface-definitions/firewall.xml.in | 34 +++------------------------ 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/interface-definitions/firewall.xml.in b/interface-definitions/firewall.xml.in index 018e0dbdfb..e4fe9a5087 100644 --- a/interface-definitions/firewall.xml.in +++ b/interface-definitions/firewall.xml.in @@ -16,15 +16,7 @@ #include - - - Interfaces to use this flowtable - - - - - - + #include Offloading method @@ -155,15 +147,7 @@ Name of firewall group can only contain alphanumeric letters, hyphen, underscores and dot - - - Interface-group member - - - - - - + #include Include another interface-group @@ -469,19 +453,7 @@ Interface associated with zone - - - Interface associated with zone - - txt - Interface associated with zone - - - - - - - + #include VRF associated with zone From dda428fc42c44decb3e661a7b6ba4e55b178dc4f Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Mon, 6 Jan 2025 11:56:53 +0100 Subject: [PATCH 4/4] T6841: firewall: migrate existing VRF in zone based firewall VRF support was introduced in VyOS 1.4.0. If a VRF is added as an interface in the zone based firewall, it will be migrated to the new syntax. OLD: set firewall zone FOO interface RED set firewall zone FOO interface eth0 NEW: set firewall zone FOO member vrf RED set firewall zone FOO member interface eth0 --- src/migration-scripts/firewall/17-to-18 | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/migration-scripts/firewall/17-to-18 b/src/migration-scripts/firewall/17-to-18 index 891f9f1950..34ce6aa070 100755 --- a/src/migration-scripts/firewall/17-to-18 +++ b/src/migration-scripts/firewall/17-to-18 @@ -1,4 +1,4 @@ -# Copyright (C) 2024 VyOS maintainers and contributors +# Copyright (C) 2024-2025 VyOS maintainers and contributors # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -14,12 +14,11 @@ # along with this library. If not, see . # From - # set firewall zone interface +# set firewall zone interface RED +# set firewall zone interface eth0 # To - # set firewall zone member interface - # or - # set firewall zone member vrf - +# set firewall zone member vrf RED +# set firewall zone member interface eth0 from vyos.configtree import ConfigTree @@ -31,7 +30,12 @@ def migrate(config: ConfigTree) -> None: return for zone in config.list_nodes(base): - if config.exists(base + [zone, 'interface']): - for iface in config.return_values(base + [zone, 'interface']): - config.set(base + [zone, 'member', 'interface'], value=iface, replace=False) - config.delete(base + [zone, 'interface']) \ No newline at end of file + zone_iface_base = base + [zone, 'interface'] + zone_member_base = base + [zone, 'member'] + if config.exists(zone_iface_base): + for iface in config.return_values(zone_iface_base): + if config.exists(['vrf', 'name', iface]): + config.set(zone_member_base + ['vrf'], value=iface, replace=False) + else: + config.set(zone_member_base + ['interface'], value=iface, replace=False) + config.delete(zone_iface_base)