Skip to content

Commit

Permalink
Merge pull request #1 from aapostoliuk/nicolas-fort-zbf
Browse files Browse the repository at this point in the history 
T6841: firewall: Fixed issues in ZBF when using VRFs
  • Loading branch information
@nicolas-fort
nicolas-fort authored Dec 17, 2024
2 parents 409766f + e12d521 commit 48da00d
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 78 deletions.
59 changes: 29 additions & 30 deletions data/templates/firewall/nftables-zone.j2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
Expand Down Expand Up @@ -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 %}
Expand All @@ -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 %}
Expand All @@ -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 %}
Expand Down
4 changes: 2 additions & 2 deletions interface-definitions/firewall.xml.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,12 @@
</node>
</children>
</tagNode>
<node name="interface">
<node name="member">
<properties>
<help>Interface associated with zone</help>
</properties>
<children>
<leafNode name="name">
<leafNode name="interface">
<properties>
<help>Interface associated with zone</help>
<valueHelp>
Expand Down
14 changes: 7 additions & 7 deletions smoketest/scripts/cli/test_firewall.py
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -905,7 +905,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'])
Expand Down Expand Up @@ -970,12 +970,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'])
Expand Down
69 changes: 36 additions & 33 deletions src/conf_mode/firewall.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -136,6 +135,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
Expand Down Expand Up @@ -448,36 +468,41 @@ 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')

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']
Expand Down Expand Up @@ -513,28 +538,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
Expand Down
7 changes: 4 additions & 3 deletions src/migration-scripts/firewall/17-to-18
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
# From
# set firewall zone <zone> interface <iface>
# To
# set firewall zone <zone> interface name <iface>
# set firewall zone <zone> member interface <iface>
# or
# set firewall zone <zone> interface vrf <vrf>
# set firewall zone <zone> member vrf <vrf>


from vyos.configtree import ConfigTree
Expand All @@ -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)
config.set(base + [zone, 'member', 'interface'], value=iface, replace=False)
config.delete(base + [zone, 'interface'])
11 changes: 8 additions & 3 deletions src/op_mode/zone.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -175,7 +180,7 @@ def get_formatted_output(zone_policy: list) -> str:
:rtype: str
"""
headers = ["Zone",
"Interfaces",
"Members",
"From Zone",
"Firewall IPv4",
"Firewall IPv6"
Expand Down

0 comments on commit 48da00d

Please sign in to comment.