firewall: T6951: Add a configuration command for ethertypes that bridge firewalls should always accept

9 files changed, 201 insertions, 15 deletions

diff --git a/data/templates/firewall/nftables.j2 b/data/templates/firewall/nftables.j2
index bf051bb57..39ef72059 100755
--- a/data/templates/firewall/nftables.j2
+++ b/data/templates/firewall/nftables.j2
@@ -410,15 +410,11 @@ table bridge vyos_filter {
 {%         for prior, conf in bridge.output.items() %}
     chain VYOS_OUTPUT_{{ prior }} {
         type filter hook output priority {{ prior }}; policy accept;
-{%             if global_options.apply_to_bridged_traffic is vyos_defined %}
-{%                 if 'invalid_connections' in global_options.apply_to_bridged_traffic %}
-        ct state invalid udp sport 67 udp dport 68 counter accept
-        ct state invalid ether type arp counter accept
-        ct state invalid ether type 8021q counter accept
-        ct state invalid ether type 8021ad counter accept
-        ct state invalid ether type 0x8863 counter accept
-        ct state invalid ether type 0x8864 counter accept
-        ct state invalid ether type 0x0842 counter accept
+{%             if global_options.apply_to_bridged_traffic.accept_invalid is vyos_defined %}
+{%                 if 'ethernet_type' in global_options.apply_to_bridged_traffic.accept_invalid %}
+{%                     for ether_type in global_options.apply_to_bridged_traffic.accept_invalid.ethernet_type %}
+        {{ ether_type | nft_accept_invalid() }}
+{%                     endfor %}
 {%                 endif %}
 {%             endif %}
 {%             if global_options.state_policy is vyos_defined %}
diff --git a/interface-definitions/include/firewall/global-options.xml.i b/interface-definitions/include/firewall/global-options.xml.i
index 794da4f9d..e19f3a7c5 100644
--- a/interface-definitions/include/firewall/global-options.xml.i
+++ b/interface-definitions/include/firewall/global-options.xml.i
@@ -49,12 +49,53 @@
         <help>Apply configured firewall rules to traffic switched by bridges</help>
       </properties>
       <children>
-        <leafNode name="invalid-connections">
+        <node name="accept-invalid">
           <properties>
-            <help>Accept ARP, 802.1q, 802.1ad, DHCP, PPPoE and WoL despite being marked as invalid connections</help>
-            <valueless/>
+            <help>Accept connections despite they are marked as invalid</help>
           </properties>
-        </leafNode>
+          <children>
+            <leafNode name="ethernet-type">
+              <properties>
+                <help>Ethernet type</help>
+                <completionHelp>
+                  <list>arp dhcp pppoe 802.1q 802.1ad pppoe-discovery wol</list>
+                </completionHelp>
+                <valueHelp>
+                  <format>arp</format>
+                  <description>Adress Resolution Protocol (ARP)</description>
+                </valueHelp>
+                <valueHelp>
+                  <format>dhcp</format>
+                  <description>Dynamic Host Configuration Protocol (DHCP)</description>
+                </valueHelp>
+                <valueHelp>
+                  <format>pppoe</format>
+                  <description>Point to Point over Ethernet (PPPoE) Session</description>
+                </valueHelp>
+                <valueHelp>
+                  <format>pppoe-discovery</format>
+                  <description>PPPoE Discovery</description>
+                </valueHelp>
+                <valueHelp>
+                  <format>802.1q</format>
+                  <description>Customer VLAN tag type (802.1Q)</description>
+                </valueHelp>
+                <valueHelp>
+                  <format>802.1ad</format>
+                  <description>Service VLAN tag type (802.1ad)</description>
+                </valueHelp>
+                <valueHelp>
+                  <format>wol</format>
+                  <description>Wake-on-LAN magic packet</description>
+                </valueHelp>
+                <constraint>
+                  <regex>(arp|dhcp|pppoe|pppoe-discovery|802.1q|802.1ad|wol)</regex>
+                </constraint>
+                <multi/>
+              </properties>
+            </leafNode>
+          </children>
+        </node>
         <leafNode name="ipv4">
           <properties>
             <help>Apply configured IPv4 firewall rules</help>
diff --git a/interface-definitions/include/version/firewall-version.xml.i b/interface-definitions/include/version/firewall-version.xml.i
index 1a8098297..1f3b779d5 100644
--- a/interface-definitions/include/version/firewall-version.xml.i
+++ b/interface-definitions/include/version/firewall-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/firewall-version.xml.i -->
-<syntaxVersion component='firewall' version='18'></syntaxVersion>
+<syntaxVersion component='firewall' version='19'></syntaxVersion>
 <!-- include end -->
diff --git a/python/vyos/template.py b/python/vyos/template.py
index bf7928914..bf2f13183 100755
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -674,6 +674,29 @@ def nft_nested_group(out_list, includes, groups, key):
         add_includes(name)
     return out_list
 
+@register_filter('nft_accept_invalid')
+def nft_accept_invalid(ether_type):
+    ether_type_mapping = {
+        'dhcp': 'udp sport 67 udp dport 68',
+        'arp': 'arp',
+        'pppoe-discovery': '0x8863',
+        'pppoe': '0x8864',
+        '802.1q': '8021q',
+        '802.1ad': '8021ad',
+        'wol': '0x0842',
+    }
+    if ether_type not in ether_type_mapping:
+        raise RuntimeError(f'Ethernet type "{ether_type}" not found in ' \
+                           'available ethernet types!')
+    out = 'ct state invalid '
+
+    if ether_type != 'dhcp':
+        out += 'ether type '
+
+    out += f'{ether_type_mapping[ether_type]} counter accept'
+
+    return out
+
 @register_filter('nat_rule')
 def nat_rule(rule_conf, rule_id, nat_type, ipv6=False):
     from vyos.nat import parse_nat_rule
diff --git a/smoketest/config-tests/firewall-bridged-global-options b/smoketest/config-tests/firewall-bridged-global-options
new file mode 100644
index 000000000..1d960d6c1
--- /dev/null
+++ b/smoketest/config-tests/firewall-bridged-global-options
@@ -0,0 +1,21 @@
+set firewall bridge prerouting filter rule 10 action 'accept'
+set firewall bridge prerouting filter rule 10 ethernet-type 'arp'
+set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type 'dhcp'
+set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type 'arp'
+set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type 'pppoe-discovery'
+set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type 'pppoe'
+set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type '802.1q'
+set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type '802.1ad'
+set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type 'wol'
+set firewall global-options state-policy established action 'accept'
+set firewall global-options state-policy invalid action 'drop'
+set firewall global-options state-policy related action 'accept'
+set interfaces ethernet eth0 duplex 'auto'
+set interfaces ethernet eth0 speed 'auto'
+set interfaces ethernet eth1 duplex 'auto'
+set interfaces ethernet eth1 speed 'auto'
+set system console device ttyS0 speed '115200'
+set system domain-name 'vyos-ci-test.net'
+set system host-name 'vyos'
+set system login user vyos authentication encrypted-password '$6$O5gJRlDYQpj$MtrCV9lxMnZPMbcxlU7.FI793MImNHznxGoMFgm3Q6QP3vfKJyOSRCt3Ka/GzFQyW1yZS4NS616NLHaIPPFHc0'
+set system login user vyos authentication plaintext-password ''
diff --git a/smoketest/configs/firewall-bridged-global-options b/smoketest/configs/firewall-bridged-global-options
new file mode 100644
index 000000000..a7e1428d8
--- /dev/null
+++ b/smoketest/configs/firewall-bridged-global-options
@@ -0,0 +1,60 @@
+firewall {
+    bridge {
+        prerouting {
+            filter {
+                rule 10 {
+                    action "accept"
+                    ethernet-type "arp"
+                }
+            }
+        }
+    }
+    global-options {
+        apply-to-bridged-traffic {
+            invalid-connections {
+            }
+        }
+        state-policy {
+            established {
+                action "accept"
+            }
+            invalid {
+                action "drop"
+            }
+            related {
+                action "accept"
+            }
+        }
+    }
+}
+interfaces {
+    ethernet eth0 {
+        duplex "auto"
+        speed "auto"
+    }
+    ethernet eth1 {
+        duplex auto
+        speed auto
+    }
+}
+system {
+    console {
+        device ttyS0 {
+            speed 115200
+        }
+    }
+    domain-name vyos-ci-test.net
+    host-name vyos
+    login {
+        user vyos {
+            authentication {
+                encrypted-password $6$O5gJRlDYQpj$MtrCV9lxMnZPMbcxlU7.FI793MImNHznxGoMFgm3Q6QP3vfKJyOSRCt3Ka/GzFQyW1yZS4NS616NLHaIPPFHc0
+                plaintext-password ""
+            }
+        }
+    }
+}
+
+// Warning: Do not remove the following line.
+// vyos-config-version: "bgp@6:broadcast-relay@1:cluster@2:config-management@1:conntrack@6:conntrack-sync@2:container@2:dhcp-relay@2:dhcp-server@11:dhcpv6-server@6:dns-dynamic@4:dns-forwarding@4:firewall@18:flow-accounting@2:https@7:ids@2:interfaces@33:ipoe-server@4:ipsec@13:isis@3:l2tp@9:lldp@3:mdns@1:monitoring@2:nat@8:nat66@3:nhrp@1:ntp@3:openconnect@3:openvpn@4:ospf@2:pim@1:policy@9:pppoe-server@11:pptp@5:qos@3:quagga@12:reverse-proxy@3:rip@1:rpki@2:salt@1:snmp@3:ssh@2:sstp@6:system@29:vpp@1:vrf@3:vrrp@4:vyos-accel-ppp@2:wanloadbalance@4:webproxy@2"
+// Release version: 2025.06.17-0020-rolling
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index 2d850dfdf..455c704d0 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -728,7 +728,13 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         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-to-bridged-traffic', 'ipv4'])
-        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'invalid-connections'])
+        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'accept-invalid', 'ethernet-type', 'dhcp'])
+        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'accept-invalid', 'ethernet-type', 'arp'])
+        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'accept-invalid', 'ethernet-type', 'pppoe'])
+        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'accept-invalid', 'ethernet-type', 'pppoe-discovery'])
+        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'accept-invalid', 'ethernet-type', '802.1q'])
+        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'accept-invalid', 'ethernet-type', '802.1ad'])
+        self.cli_set(['firewall', 'global-options', 'apply-to-bridged-traffic', 'accept-invalid', 'ethernet-type', 'wol'])
 
         self.cli_set(['firewall', 'bridge', 'name', name, 'default-action', 'accept'])
         self.cli_set(['firewall', 'bridge', 'name', name, 'default-log'])
diff --git a/src/migration-scripts/firewall/18-to-19 b/src/migration-scripts/firewall/18-to-19
new file mode 100644
index 000000000..3564e0e01
--- /dev/null
+++ b/src/migration-scripts/firewall/18-to-19
@@ -0,0 +1,35 @@
+# Copyright (C) 2024-2025 VyOS maintainers and contributors
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+# From
+#   set firewall global-options apply-to-bridged-traffic invalid-connections
+# To
+#   set firewall global-options apply-to-bridged-traffic accept-invalid ethernet-type <ethertype>
+
+from vyos.configtree import ConfigTree
+
+base = ['firewall', 'global-options', 'apply-to-bridged-traffic']
+
+def migrate(config: ConfigTree) -> None:
+    if not config.exists(base + ['invalid-connections']):
+        # Nothing to do
+        return
+
+    ether_types = ['dhcp', 'arp', 'pppoe-discovery', 'pppoe', '802.1q', '802.1ad', 'wol']
+
+    for ether_type in ether_types:
+        config.set(base + ['accept-invalid', 'ethernet-type'], value=ether_type, replace=False)
+
+    config.delete(base + ['invalid-connections'])
diff --git a/src/tests/test_template.py b/src/tests/test_template.py
index 4660c0038..09315d398 100644
--- a/src/tests/test_template.py
+++ b/src/tests/test_template.py
@@ -199,8 +199,12 @@ class TestVyOSTemplate(TestCase):
             vyos.template.get_default_config_file('UNKNOWN')
         with self.assertRaises(RuntimeError):
             vyos.template.get_default_port('UNKNOWN')
+        with self.assertRaises(RuntimeError):
+            vyos.template.nft_accept_invalid('UNKNOWN')
 
         self.assertEqual(vyos.template.get_default_config_file('sshd_user_ca'),
                          config_files['sshd_user_ca'])
         self.assertEqual(vyos.template.get_default_port('certbot_haproxy'),
                          internal_ports['certbot_haproxy'])
+        self.assertEqual(vyos.template.nft_accept_invalid('arp'),
+                         'ct state invalid ether type arp counter accept')