Skip to content

Commit

Permalink
T6841: firewall: Fixed issues in ZBF when using VRFs
Browse files Browse the repository at this point in the history 
Improve config parsing for ZBF when using VRFs and interfaces attached to VRFs
  • Loading branch information
@aapostoliuk
aapostoliuk committed Dec 17, 2024
1 parent 409766f commit e12d521
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 e12d521

Please sign in to comment.