Skip to content

Commit

Permalink
T4072: change same helpers in xml definitions; add notrack action for…
Browse files Browse the repository at this point in the history
… prerouting chain; re introduce <set vrf> in policy; change global options for passing traffic to IPvX firewall; update smoketest
  • Loading branch information
nicolas-fort committed Aug 2, 2024
1 parent fa76492 commit c33cd61
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 82 deletions.
6 changes: 3 additions & 3 deletions data/templates/firewall/sysctl-firewall.conf.j2
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ net.ipv4.conf.*.send_redirects = {{ 1 if global_options.send_redirects == 'enabl
net.ipv4.tcp_syncookies = {{ 1 if global_options.syn_cookies == 'enable' else 0 }}
net.ipv4.tcp_rfc1337 = {{ 1 if global_options.twa_hazards_protection == 'enable' else 0 }}

{% if global_options.apply_for_bridge is vyos_defined %}
net.bridge.bridge-nf-call-iptables = {{ 1 if global_options.apply_for_bridge.ipv4 is vyos_defined else 0 }}
net.bridge.bridge-nf-call-ip6tables = {{ 1 if global_options.apply_for_bridge.ipv6 is vyos_defined else 0 }}
{% if global_options.apply_to_bridged_traffic is vyos_defined %}
net.bridge.bridge-nf-call-iptables = {{ 1 if global_options.apply_to_bridged_traffic.ipv4 is vyos_defined else 0 }}
net.bridge.bridge-nf-call-ip6tables = {{ 1 if global_options.apply_to_bridged_traffic.ipv6 is vyos_defined else 0 }}
{% else %}
net.bridge.bridge-nf-call-iptables = 0
net.bridge.bridge-nf-call-ip6tables = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
</properties>
<children>
#include <include/firewall/common-rule-bridge.xml.i>
#include <include/firewall/action-l2.xml.i>
#include <include/firewall/connection-mark.xml.i>
#include <include/firewall/connection-status.xml.i>
#include <include/firewall/state.xml.i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
</properties>
<children>
#include <include/firewall/common-rule-bridge.xml.i>
#include <include/firewall/action-l2.xml.i>
#include <include/firewall/connection-mark.xml.i>
#include <include/firewall/connection-status.xml.i>
#include <include/firewall/state.xml.i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
</properties>
<children>
#include <include/firewall/common-rule-bridge.xml.i>
#include <include/firewall/action-l2.xml.i>
#include <include/firewall/connection-mark.xml.i>
#include <include/firewall/connection-status.xml.i>
#include <include/firewall/state.xml.i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
</properties>
<children>
#include <include/firewall/common-rule-bridge.xml.i>
#include <include/firewall/action-l2.xml.i>
#include <include/firewall/connection-mark.xml.i>
#include <include/firewall/connection-status.xml.i>
#include <include/firewall/state.xml.i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include <include/generic-description.xml.i>
<tagNode name="rule">
<properties>
<help>Bridge Firewall prerouting filter rule number</help>
<help>Bridge firewall prerouting filter rule number</help>
<valueHelp>
<format>u32:1-999999</format>
<description>Number for this firewall rule</description>
Expand All @@ -26,7 +26,7 @@
</properties>
<children>
#include <include/firewall/common-rule-bridge.xml.i>
#include <include/firewall/set-packet-modifications.xml.i>
#include <include/firewall/action-and-notrack.xml.i>
#include <include/firewall/inbound-interface.xml.i>
</children>
</tagNode>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<!-- include start from firewall/common-rule-bridge.xml.i -->
#include <include/generic-description.xml.i>
#include <include/generic-disable-node.xml.i>
#include <include/firewall/action-l2.xml.i>
#include <include/firewall/dscp.xml.i>
#include <include/firewall/firewall-mark.xml.i>
#include <include/firewall/fragment.xml.i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
</properties>
<defaultValue>disable</defaultValue>
</leafNode>
<node name="apply-for-bridge">
<node name="apply-to-bridged-traffic">
<properties>
<help>Apply configured firewall rules to traffic switched by bridges</help>
</properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
<children>
<leafNode name="connection-mark">
<properties>
<help>Connection marking</help>
<help>Set connection mark</help>
<valueHelp>
<format>u32:0-2147483647</format>
<description>Connection marking</description>
<description>Connection mark</description>
</valueHelp>
<constraint>
<validator name="numeric" argument="--range 0-2147483647"/>
Expand All @@ -18,7 +18,7 @@
</leafNode>
<leafNode name="dscp">
<properties>
<help>Packet Differentiated Services Codepoint (DSCP)</help>
<help>Set DSCP (Packet Differentiated Services Codepoint) bits</help>
<valueHelp>
<format>u32:0-63</format>
<description>DSCP number</description>
Expand All @@ -30,10 +30,10 @@
</leafNode>
<leafNode name="mark">
<properties>
<help>Packet marking</help>
<help>Set packet mark</help>
<valueHelp>
<format>u32:1-2147483647</format>
<description>Packet marking</description>
<description>Packet mark</description>
</valueHelp>
<constraint>
<validator name="numeric" argument="--range 1-2147483647"/>
Expand All @@ -42,7 +42,7 @@
</leafNode>
<leafNode name="table">
<properties>
<help>Routing table to forward packet with</help>
<help>Set the routing table for matched packets</help>
<valueHelp>
<format>u32:1-200</format>
<description>Table number</description>
Expand All @@ -61,9 +61,27 @@
</completionHelp>
</properties>
</leafNode>
<leafNode name="vrf">
<properties>
<help>VRF to forward packet with</help>
<valueHelp>
<format>txt</format>
<description>VRF instance name</description>
</valueHelp>
<valueHelp>
<format>default</format>
<description>Forward into default global VRF</description>
</valueHelp>
<completionHelp>
<list>default</list>
<path>vrf name</path>
</completionHelp>
#include <include/constraint/vrf.xml.i>
</properties>
</leafNode>
<leafNode name="tcp-mss">
<properties>
<help>TCP Maximum Segment Size</help>
<help>Set TCP Maximum Segment Size</help>
<valueHelp>
<format>u32:500-1460</format>
<description>Explicitly set TCP MSS value</description>
Expand Down
7 changes: 3 additions & 4 deletions smoketest/scripts/cli/test_firewall.py
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ def test_bridge_firewall(self):

self.cli_set(['firewall', 'group', 'ipv6-address-group', 'AGV6', 'address', '2001:db1::1'])
self.cli_set(['firewall', 'global-options', 'state-policy', 'established', 'action', 'accept'])
self.cli_set(['firewall', 'global-options', 'apply-for-bridge', 'ipv4'])
self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'ipv4'])

self.cli_set(['firewall', 'bridge', 'name', name, 'default-action', 'accept'])
self.cli_set(['firewall', 'bridge', 'name', name, 'default-log'])
Expand All @@ -731,10 +731,9 @@ def test_bridge_firewall(self):
self.cli_set(['firewall', 'bridge', 'input', 'filter', 'rule', '1', 'source', 'address', '192.0.2.2'])
self.cli_set(['firewall', 'bridge', 'input', 'filter', 'rule', '1', 'state', 'new'])

self.cli_set(['firewall', 'bridge', 'prerouting', 'filter', 'rule', '1', 'action', 'drop'])
self.cli_set(['firewall', 'bridge', 'prerouting', 'filter', 'rule', '1', 'action', 'notrack'])
self.cli_set(['firewall', 'bridge', 'prerouting', 'filter', 'rule', '1', 'destination', 'group', 'ipv6-address-group', 'AGV6'])


self.cli_commit()

nftables_search = [
Expand All @@ -755,7 +754,7 @@ def test_bridge_firewall(self):
['ct state new', 'ip saddr 192.0.2.2', f'iifname "{interface_in}"', 'accept'],
['chain VYOS_PREROUTING_filter'],
['type filter hook prerouting priority filter; policy accept;'],
['ip6 daddr @A6_AGV6', 'drop']
['ip6 daddr @A6_AGV6', 'notrack']
]

self.verify_nftables(nftables_search, 'bridge vyos_filter')
Expand Down
117 changes: 53 additions & 64 deletions src/conf_mode/firewall.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@
'domain_group',
'network_group',
'port_group',
'interface_group'
'interface_group',
## Added for group ussage in bridge firewall
'ipv4_address_group',
'ipv6_address_group',
'ipv4_network_group',
'ipv6_network_group'
]

nested_group_types = [
Expand Down Expand Up @@ -129,49 +134,40 @@ def get_config(config=None):

return firewall

def verify_jump_target(firewall, root_chain, jump_target, ipv6, recursive=False):
def verify_jump_target(firewall, hook, jump_target, family, recursive=False):
targets_seen = []
targets_pending = [jump_target]

while targets_pending:
target = targets_pending.pop()

if not ipv6:
if target not in dict_search_args(firewall, 'ipv4', 'name'):
raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
target_rules = dict_search_args(firewall, 'ipv4', 'name', target, 'rule')
else:
if target not in dict_search_args(firewall, 'ipv6', 'name'):
raise ConfigError(f'Invalid jump-target. Firewall ipv6 name {target} does not exist on the system')
target_rules = dict_search_args(firewall, 'ipv6', 'name', target, 'rule')
if 'name' not in firewall[family]:
raise ConfigError(f'Invalid jump-target. Firewall {family} name {target} does not exist on the system')
elif target not in dict_search_args(firewall, family, 'name'):
raise ConfigError(f'Invalid jump-target. Firewall {family} name {target} does not exist on the system')

no_ipsec_in = root_chain in ('output', )
target_rules = dict_search_args(firewall, family, 'name', target, 'rule')
no_ipsec_in = hook in ('output', )

if target_rules:
for target_rule_conf in target_rules.values():
# Output hook types will not tolerate 'meta ipsec exists' matches even in jump targets:
if no_ipsec_in and (dict_search_args(target_rule_conf, 'ipsec', 'match_ipsec_in') is not None \
or dict_search_args(target_rule_conf, 'ipsec', 'match_none_in') is not None):
if not ipv6:
raise ConfigError(f'Invalid jump-target for {root_chain}. Firewall name {target} rules contain incompatible ipsec inbound matches')
else:
raise ConfigError(f'Invalid jump-target for {root_chain}. Firewall ipv6 name {target} rules contain incompatible ipsec inbound matches')
raise ConfigError(f'Invalid jump-target for {hook}. Firewall {family} name {target} rules contain incompatible ipsec inbound matches')
# Make sure we're not looping back on ourselves somewhere:
if recursive and 'jump_target' in target_rule_conf:
child_target = target_rule_conf['jump_target']
if child_target in targets_seen:
if not ipv6:
raise ConfigError(f'Loop detected in jump-targets, firewall name {target} refers to previously traversed name {child_target}')
else:
raise ConfigError(f'Loop detected in jump-targets, firewall ipv6 name {target} refers to previously traversed ipv6 name {child_target}')
raise ConfigError(f'Loop detected in jump-targets, firewall {family} name {target} refers to previously traversed {family} name {child_target}')
targets_pending.append(child_target)
if len(targets_seen) == 7:
path_txt = ' -> '.join(targets_seen)
Warning(f'Deep nesting of jump targets has reached 8 levels deep, following the path {path_txt} -> {child_target}!')

targets_seen.append(target)

def verify_rule(firewall, chain_name, rule_conf, ipv6):
def verify_rule(firewall, family, hook, priority, rule_id, rule_conf):
if 'action' not in rule_conf:
raise ConfigError('Rule action must be defined')

Expand All @@ -182,10 +178,10 @@ def verify_rule(firewall, chain_name, rule_conf, ipv6):
if 'jump' not in rule_conf['action']:
raise ConfigError('jump-target defined, but action jump needed and it is not defined')
target = rule_conf['jump_target']
if chain_name != 'name': # This is a bit clumsy, but consolidates a chunk of code.
verify_jump_target(firewall, chain_name, target, ipv6, recursive=True)
if hook != 'name': # This is a bit clumsy, but consolidates a chunk of code.
verify_jump_target(firewall, hook, target, family, recursive=True)
else:
verify_jump_target(firewall, chain_name, target, ipv6, recursive=False)
verify_jump_target(firewall, hook, target, family, recursive=False)

if rule_conf['action'] == 'offload':
if 'offload_target' not in rule_conf:
Expand Down Expand Up @@ -247,9 +243,9 @@ def verify_rule(firewall, chain_name, rule_conf, ipv6):
raise ConfigError(f'Cannot match a tcp flag as set and not set')

if 'protocol' in rule_conf:
if rule_conf['protocol'] == 'icmp' and ipv6:
if rule_conf['protocol'] == 'icmp' and family == 'ipv6':
raise ConfigError(f'Cannot match IPv4 ICMP protocol on IPv6, use ipv6-icmp')
if rule_conf['protocol'] == 'ipv6-icmp' and not ipv6:
if rule_conf['protocol'] == 'ipv6-icmp' and family == 'ipv4':
raise ConfigError(f'Cannot match IPv6 ICMP protocol on IPv4, use icmp')

for side in ['destination', 'source']:
Expand All @@ -267,7 +263,18 @@ def verify_rule(firewall, chain_name, rule_conf, ipv6):
if group in side_conf['group']:
group_name = side_conf['group'][group]

fw_group = f'ipv6_{group}' if ipv6 and group in ['address_group', 'network_group'] else group
if family == 'ipv6' and group in ['address_group', 'network_group']:
fw_group = f'ipv6_{group}'
elif family == 'bridge':
if group =='ipv4_address_group':
fw_group = 'address_group'
elif group == 'ipv4_network_group':
fw_group = 'network_group'
else:
fw_group = group
else:
fw_group = group

error_group = fw_group.replace("_", "-")

if group in ['address_group', 'network_group', 'domain_group']:
Expand Down Expand Up @@ -303,7 +310,7 @@ def verify_rule(firewall, chain_name, rule_conf, ipv6):
raise ConfigError(f'Dynamic address group must be defined.')
else:
target = rule_conf['add_address_to_group'][type]['address_group']
fwall_group = 'ipv6_address_group' if ipv6 else 'address_group'
fwall_group = 'ipv6_address_group' if family == 'ipv6' else 'address_group'
group_obj = dict_search_args(firewall, 'group', 'dynamic_group', fwall_group, target)
if group_obj is None:
raise ConfigError(f'Invalid dynamic address group on firewall rule')
Expand Down Expand Up @@ -380,43 +387,25 @@ def verify(firewall):
for group_name, group in groups.items():
verify_nested_group(group_name, group, groups, [])

if 'ipv4' in firewall:
for name in ['name','forward','input','output', 'prerouting']:
if name in firewall['ipv4']:
for name_id, name_conf in firewall['ipv4'][name].items():
if 'jump' in name_conf['default_action'] and 'default_jump_target' not in name_conf:
raise ConfigError('default-action set to jump, but no default-jump-target specified')
if 'default_jump_target' in name_conf:
target = name_conf['default_jump_target']
if 'jump' not in name_conf['default_action']:
raise ConfigError('default-jump-target defined, but default-action jump needed and it is not defined')
if name_conf['default_jump_target'] == name_id:
raise ConfigError(f'Loop detected on default-jump-target.')
verify_jump_target(firewall, name, target, False, recursive=True)

if 'rule' in name_conf:
for rule_id, rule_conf in name_conf['rule'].items():
verify_rule(firewall, name, rule_conf, False)

if 'ipv6' in firewall:
for name in ['name','forward','input','output', 'prerouting']:
if name in firewall['ipv6']:
for name_id, name_conf in firewall['ipv6'][name].items():
if 'jump' in name_conf['default_action'] and 'default_jump_target' not in name_conf:
raise ConfigError('default-action set to jump, but no default-jump-target specified')
if 'default_jump_target' in name_conf:
target = name_conf['default_jump_target']
if 'jump' not in name_conf['default_action']:
raise ConfigError('default-jump-target defined, but default-action jump needed and it is not defined')
if name_conf['default_jump_target'] == name_id:
raise ConfigError(f'Loop detected on default-jump-target.')
verify_jump_target(firewall, name, target, True, recursive=True)

if 'rule' in name_conf:
for rule_id, rule_conf in name_conf['rule'].items():
verify_rule(firewall, name, rule_conf, True)

#### ZONESSSS
for family in ['ipv4', 'ipv6', 'bridge']:
if family in firewall:
for chain in ['name','forward','input','output', 'prerouting']:
if chain in firewall[family]:
for priority, priority_conf in firewall[family][chain].items():
if 'jump' in priority_conf['default_action'] and 'default_jump_target' not in priority_conf:
raise ConfigError('default-action set to jump, but no default-jump-target specified')
if 'default_jump_target' in priority_conf:
target = priority_conf['default_jump_target']
if 'jump' not in priority_conf['default_action']:
raise ConfigError('default-jump-target defined, but default-action jump needed and it is not defined')
if priority_conf['default_jump_target'] == priority:
raise ConfigError(f'Loop detected on default-jump-target.')
if target not in dict_search_args(firewall[family], 'name'):
raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
if 'rule' in priority_conf:
for rule_id, rule_conf in priority_conf['rule'].items():
verify_rule(firewall, family, chain, priority, rule_id, rule_conf)

local_zone = False
zone_interfaces = []

Expand Down

0 comments on commit c33cd61

Please sign in to comment.