From c45b695ca068f018910eb7513e01657adb7abbbe Mon Sep 17 00:00:00 2001 From: Nicolas Fort Date: Wed, 22 Nov 2023 11:38:00 +0000 Subject: T5637: firewall: extend rule for default-action to firewall bridge, in order to be able to catch logs using separte rule for default-action --- data/templates/firewall/nftables-bridge.j2 | 6 +++--- data/templates/firewall/nftables-zone.j2 | 18 ++++++++++++------ data/templates/firewall/nftables.j2 | 22 +++++++++++----------- .../include/firewall/bridge-hook-forward.xml.i | 1 + python/vyos/template.py | 6 +++--- smoketest/scripts/cli/test_firewall.py | 7 +++++-- src/op_mode/firewall.py | 21 ++++++++------------- 7 files changed, 43 insertions(+), 38 deletions(-) diff --git a/data/templates/firewall/nftables-bridge.j2 b/data/templates/firewall/nftables-bridge.j2 index 7f94e10d6..dec027bf9 100644 --- a/data/templates/firewall/nftables-bridge.j2 +++ b/data/templates/firewall/nftables-bridge.j2 @@ -2,9 +2,8 @@ {% set ns = namespace(sets=[]) %} {% if bridge.forward is vyos_defined %} {% for prior, conf in bridge.forward.items() %} -{% set def_action = conf.default_action %} chain VYOS_FORWARD_{{ prior }} { - type filter hook forward priority {{ prior }}; policy {{ def_action }}; + type filter hook forward priority {{ prior }}; policy accept; {% if conf.rule is vyos_defined %} {% for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %} {{ rule_conf | nft_rule('FWD', prior, rule_id, 'bri') }} @@ -13,6 +12,7 @@ {% endif %} {% endfor %} {% endif %} + {{ conf | nft_default_rule('FWD-filter', 'bri') }} } {% endfor %} {% endif %} @@ -28,7 +28,7 @@ {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule(name_text) }} + {{ conf | nft_default_rule(name_text, 'bri') }} } {% endfor %} {% endif %} diff --git a/data/templates/firewall/nftables-zone.j2 b/data/templates/firewall/nftables-zone.j2 index 1e9351f97..beb14ff00 100644 --- a/data/templates/firewall/nftables-zone.j2 +++ b/data/templates/firewall/nftables-zone.j2 @@ -1,7 +1,13 @@ -{% macro zone_chains(zone, ipv6=False) %} -{% set fw_name = 'ipv6_name' if ipv6 else 'name' %} -{% set suffix = '6' if ipv6 else '' %} +{% macro zone_chains(zone, family) %} +{% if family == 'ipv6' %} +{% set fw_name = 'ipv6_name' %} +{% set suffix = '6' %} +{% else %} +{% set fw_name = 'name' %} +{% set suffix = '' %} +{% endif %} + chain VYOS_ZONE_FORWARD { type filter hook forward priority 1; policy accept; {% for zone_name, zone_conf in zone.items() %} @@ -36,7 +42,7 @@ iifname { {{ zone[from_zone].interface | join(",") }} } counter return {% endfor %} {% endif %} - {{ zone_conf | nft_default_rule('zone_' + zone_name) }} + {{ zone_conf | nft_default_rule('zone_' + zone_name, family) }} } chain VZONE_{{ zone_name }}_OUT { oifname lo counter return @@ -46,7 +52,7 @@ oifname { {{ zone[from_zone].interface | join(",") }} } counter return {% endfor %} {% endif %} - {{ zone_conf | nft_default_rule('zone_' + zone_name) }} + {{ zone_conf | nft_default_rule('zone_' + zone_name, family) }} } {% else %} chain VZONE_{{ zone_name }} { @@ -62,7 +68,7 @@ {% endif %} {% endfor %} {% endif %} - {{ zone_conf | nft_default_rule('zone_' + zone_name) }} + {{ zone_conf | nft_default_rule('zone_' + zone_name, family) }} } {% endif %} {% endfor %} diff --git a/data/templates/firewall/nftables.j2 b/data/templates/firewall/nftables.j2 index e24a9655d..63195d25f 100644 --- a/data/templates/firewall/nftables.j2 +++ b/data/templates/firewall/nftables.j2 @@ -54,7 +54,7 @@ table ip vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule('FWD-filter') }} + {{ conf | nft_default_rule('FWD-filter', 'ipv4') }} } {% endfor %} {% endif %} @@ -71,7 +71,7 @@ table ip vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule('INP-filter') }} + {{ conf | nft_default_rule('INP-filter', 'ipv4') }} } {% endfor %} {% endif %} @@ -88,7 +88,7 @@ table ip vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule('OUT-filter') }} + {{ conf | nft_default_rule('OUT-filter', 'ipv4') }} } {% endfor %} {% endif %} @@ -108,7 +108,7 @@ table ip vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule('PRE-filter') }} + {{ conf | nft_default_rule('PRE-filter', 'ipv4') }} } {% endfor %} {% endif %} @@ -124,7 +124,7 @@ table ip vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule(name_text) }} + {{ conf | nft_default_rule(name_text, 'ipv4') }} } {% endfor %} {% endif %} @@ -154,7 +154,7 @@ table ip vyos_filter { {{ group_tmpl.groups(group, False, True) }} {% if zone is vyos_defined %} -{{ zone_tmpl.zone_chains(zone, False) }} +{{ zone_tmpl.zone_chains(zone, 'ipv4') }} {% endif %} } @@ -182,7 +182,7 @@ table ip6 vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule('FWD-filter', ipv6=True) }} + {{ conf | nft_default_rule('FWD-filter', 'ipv6') }} } {% endfor %} {% endif %} @@ -199,7 +199,7 @@ table ip6 vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule('INP-filter', ipv6=True) }} + {{ conf | nft_default_rule('INP-filter', 'ipv6') }} } {% endfor %} {% endif %} @@ -216,7 +216,7 @@ table ip6 vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule('OUT-filter', ipv6=True) }} + {{ conf | nft_default_rule('OUT-filter', 'ipv6') }} } {% endfor %} {% endif %} @@ -237,7 +237,7 @@ table ip6 vyos_filter { {% endif %} {% endfor %} {% endif %} - {{ conf | nft_default_rule(name_text, ipv6=True) }} + {{ conf | nft_default_rule(name_text, 'ipv6') }} } {% endfor %} {% endif %} @@ -266,7 +266,7 @@ table ip6 vyos_filter { {% endif %} {{ group_tmpl.groups(group, True, True) }} {% if zone is vyos_defined %} -{{ zone_tmpl.zone_chains(zone, True) }} +{{ zone_tmpl.zone_chains(zone, 'ipv6') }} {% endif %} } diff --git a/interface-definitions/include/firewall/bridge-hook-forward.xml.i b/interface-definitions/include/firewall/bridge-hook-forward.xml.i index 23d757070..ff86bf466 100644 --- a/interface-definitions/include/firewall/bridge-hook-forward.xml.i +++ b/interface-definitions/include/firewall/bridge-hook-forward.xml.i @@ -10,6 +10,7 @@ #include + #include #include diff --git a/python/vyos/template.py b/python/vyos/template.py index c778d0de8..1e683b605 100644 --- a/python/vyos/template.py +++ b/python/vyos/template.py @@ -579,10 +579,10 @@ def nft_rule(rule_conf, fw_hook, fw_name, rule_id, ip_name='ip'): return parse_rule(rule_conf, fw_hook, fw_name, rule_id, ip_name) @register_filter('nft_default_rule') -def nft_default_rule(fw_conf, fw_name, ipv6=False): +def nft_default_rule(fw_conf, fw_name, family): output = ['counter'] default_action = fw_conf['default_action'] - family = 'ipv6' if ipv6 else 'ipv4' + #family = 'ipv6' if ipv6 else 'ipv4' if 'enable_default_log' in fw_conf: action_suffix = default_action[:1].upper() @@ -592,7 +592,7 @@ def nft_default_rule(fw_conf, fw_name, ipv6=False): output.append(f'{default_action}') if 'default_jump_target' in fw_conf: target = fw_conf['default_jump_target'] - def_suffix = '6' if ipv6 else '' + def_suffix = '6' if family == 'ipv6' else '' output.append(f'NAME{def_suffix}_{target}') output.append(f'comment "{fw_name} default-action {default_action}"') diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py index 8c3e00a2a..cffa1c0be 100755 --- a/smoketest/scripts/cli/test_firewall.py +++ b/smoketest/scripts/cli/test_firewall.py @@ -586,6 +586,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase): self.cli_set(['firewall', 'bridge', 'name', name, 'rule', '1', 'log-options', 'level', 'crit']) self.cli_set(['firewall', 'bridge', 'forward', 'filter', 'default-action', 'drop']) + self.cli_set(['firewall', 'bridge', 'forward', 'filter', 'enable-default-log']) self.cli_set(['firewall', 'bridge', 'forward', 'filter', 'rule', '1', 'action', 'accept']) self.cli_set(['firewall', 'bridge', 'forward', 'filter', 'rule', '1', 'vlan', 'id', vlan_id]) self.cli_set(['firewall', 'bridge', 'forward', 'filter', 'rule', '2', 'action', 'jump']) @@ -596,11 +597,13 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase): nftables_search = [ ['chain VYOS_FORWARD_filter'], - ['type filter hook forward priority filter; policy drop;'], + ['type filter hook forward priority filter; policy accept;'], [f'vlan id {vlan_id}', 'accept'], [f'vlan pcp {vlan_prior}', f'jump NAME_{name}'], + ['log prefix "[bri-FWD-filter-default-D]"', 'drop', 'FWD-filter default-action drop'], [f'chain NAME_{name}'], - [f'ether saddr {mac_address}', f'iifname "{interface_in}"', f'log prefix "[bri-NAM-{name}-1-A]" log level crit', 'accept'] + [f'ether saddr {mac_address}', f'iifname "{interface_in}"', f'log prefix "[bri-NAM-{name}-1-A]" log level crit', 'accept'], + ['accept', f'{name} default-action accept'] ] self.verify_nftables(nftables_search, 'bridge vyos_filter') diff --git a/src/op_mode/firewall.py b/src/op_mode/firewall.py index 20f54b9ba..36bb013fe 100755 --- a/src/op_mode/firewall.py +++ b/src/op_mode/firewall.py @@ -113,19 +113,14 @@ def output_firewall_name(family, hook, priority, firewall_conf, single_rule_id=N if hook in ['input', 'forward', 'output']: def_action = firewall_conf['default_action'] if 'default_action' in firewall_conf else 'accept' - row = ['default', def_action, 'all'] - rule_details = details['default-action'] - row.append(rule_details.get('packets', 0)) - row.append(rule_details.get('bytes', 0)) - rows.append(row) + else: + def_action = firewall_conf['default_action'] if 'default_action' in firewall_conf else 'drop' + row = ['default', def_action, 'all'] + rule_details = details['default-action'] + row.append(rule_details.get('packets', 0)) + row.append(rule_details.get('bytes', 0)) - elif 'default_action' in firewall_conf and not single_rule_id: - row = ['default', firewall_conf['default_action'], 'all'] - if 'default-action' in details: - rule_details = details['default-action'] - row.append(rule_details.get('packets', 0)) - row.append(rule_details.get('bytes', 0)) - rows.append(row) + rows.append(row) if rows: header = ['Rule', 'Action', 'Protocol', 'Packets', 'Bytes', 'Conditions'] @@ -314,7 +309,7 @@ def show_firewall_group(name=None): family = ['ipv6'] group_type = 'network_group' else: - family = ['ipv4', 'ipv6'] + family = ['ipv4', 'ipv6', 'bridge'] for item in family: # Look references in firewall -- cgit v1.2.3