T5637: firewall: extend rule for default-action to firewall bridge, in order to be able to catch logs using separte rule for default-action

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 @@
       </properties>
       <children>
         #include <include/firewall/default-action-base-chains.xml.i>
+        #include <include/firewall/enable-default-log.xml.i>
         #include <include/generic-description.xml.i>
         <tagNode name="rule">
           <properties>
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