firewall: T4651: re-implement packet-length CLI option to use <multi/>

4 files changed, 88 insertions, 53 deletions

diff --git a/interface-definitions/include/firewall/packet-length.xml.i b/interface-definitions/include/firewall/packet-length.xml.i
index 866a76bbb..043f56d16 100644
--- a/interface-definitions/include/firewall/packet-length.xml.i
+++ b/interface-definitions/include/firewall/packet-length.xml.i
@@ -1,18 +1,38 @@
 <!-- include start from firewall/packet-length.xml.i -->
 <leafNode name="packet-length">
   <properties>
-    <help>Payload size in bytes, including header and data</help>
+    <help>Payload size in bytes, including header and data to match</help>
     <valueHelp>
       <format>u32:1-65535</format>
-      <description>Packet length value. Multiple values can be specified as a comma-separated list. Inverted match is also supported</description>
+      <description>Packet length to match</description>
     </valueHelp>
     <valueHelp>
       <format>&lt;start-end&gt;</format>
-      <description>Packet length range. Inverted match is also supported (e.g. 1001-1005 or !1001-1005)</description>
+      <description>Packet length range to match</description>
     </valueHelp>
     <constraint>
-      <validator name="packet-length"/>
+      <validator name="numeric" argument="--range 1-65535"/>
+      <validator name="range" argument="--min=1 --max=65535"/>
     </constraint>
+    <multi/>
+  </properties>
+</leafNode>
+<leafNode name="packet-length-exclude">
+  <properties>
+    <help>Payload size in bytes, including header and data not to match</help>
+    <valueHelp>
+      <format>u32:1-65535</format>
+      <description>Packet length not to match</description>
+    </valueHelp>
+    <valueHelp>
+      <format>&lt;start-end&gt;</format>
+      <description>Packet length range not to match</description>
+    </valueHelp>
+    <constraint>
+      <validator name="numeric" argument="--range 1-65535"/>
+      <validator name="range" argument="--min=1 --max=65535"/>
+    </constraint>
+    <multi/>
   </properties>
 </leafNode>
 <!-- include end -->
diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py
index ea28aa91d..0bc5378db 100644
--- a/python/vyos/firewall.py
+++ b/python/vyos/firewall.py
@@ -150,7 +150,7 @@ def parse_rule(rule_conf, fw_name, rule_id, ip_name):
                 if suffix[0] == '!':
                     suffix = f'!= {suffix[1:]}'
                 output.append(f'{ip_name} {prefix}addr {suffix}')
-            
+
             if dict_search_args(side_conf, 'geoip', 'country_code'):
                 operator = ''
                 if dict_search_args(side_conf, 'geoip', 'inverse_match') != None:
@@ -267,25 +267,12 @@ def parse_rule(rule_conf, fw_name, rule_id, ip_name):
 
 
     if 'packet_length' in rule_conf:
-        #proto = rule_conf['protocol']
-        length = rule_conf['packet_length'].split(',')
-
-        lengths = []
-        negated_lengths = []
-
-        for p in length:
-            if p[0] == '!':
-                negated_lengths.append(p[1:])
-            else:
-                lengths.append(p)
-
-        if lengths:
-            lengths_str = ','.join(lengths)
-            output.append(f'ip{def_suffix} length {{{lengths_str}}}')
+        lengths_str = ','.join(rule_conf['packet_length'])
+        output.append(f'ip{def_suffix} length {{{lengths_str}}}')
 
-        if negated_lengths:
-            negated_lengths_str = ','.join(negated_lengths)
-            output.append(f'ip{def_suffix} length != {{{negated_lengths_str}}}')
+    if 'packet_length_exclude' in rule_conf:
+        negated_lengths_str = ','.join(rule_conf['packet_length_exclude'])
+        output.append(f'ip{def_suffix} length != {{{negated_lengths_str}}}')
 
 
     if 'ipsec' in rule_conf:
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index c5756a027..1517180de 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -48,7 +48,6 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         cls.cli_delete(cls, ['firewall'])
 
         cls.cli_set(cls, ['interfaces', 'ethernet', 'eth0', 'address', eth0_addr])
-        cls.debug=True
 
     @classmethod
     def tearDownClass(cls):
@@ -233,6 +232,35 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
 
         self.verify_nftables(nftables_search, 'ip filter')
 
+    def test_ipv4_packet_length(self):
+        name = 'smoketest-plen'
+        interface = 'eth0'
+
+        self.cli_set(['firewall', 'name', name, 'default-action', 'drop'])
+        self.cli_set(['firewall', 'name', name, 'enable-default-log'])
+
+        self.cli_set(['firewall', 'name', name, 'rule', '6', 'action', 'accept'])
+        self.cli_set(['firewall', 'name', name, 'rule', '6', 'packet-length', '64'])
+        self.cli_set(['firewall', 'name', name, 'rule', '6', 'packet-length', '512'])
+        self.cli_set(['firewall', 'name', name, 'rule', '6', 'packet-length', '1024'])
+
+        self.cli_set(['firewall', 'name', name, 'rule', '7', 'action', 'accept'])
+        self.cli_set(['firewall', 'name', name, 'rule', '7', 'packet-length', '1-30000'])
+        self.cli_set(['firewall', 'name', name, 'rule', '7', 'packet-length-exclude', '60000-65535'])
+
+        self.cli_set(['interfaces', 'ethernet', interface, 'firewall', 'in', 'name', name])
+
+        self.cli_commit()
+
+        nftables_search = [
+            [f'iifname "{interface}"', f'jump NAME_{name}'],
+            ['ip length { 64, 512, 1024 }', 'return'],
+            ['ip length { 1-30000 }', 'ip length != { 60000-65535 }', 'return'],
+            [f'log prefix "[{name}-default-D]" drop']
+        ]
+
+        self.verify_nftables(nftables_search, 'ip filter')
+
     def test_ipv6_basic_rules(self):
         name = 'v6-smoketest'
         interface = 'eth0'
@@ -263,6 +291,35 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
 
         self.verify_nftables(nftables_search, 'ip6 filter')
 
+    def test_ipv6_packet_length(self):
+        name = 'v6-smoketest-plen'
+        interface = 'eth0'
+
+        self.cli_set(['firewall', 'ipv6-name', name, 'default-action', 'drop'])
+        self.cli_set(['firewall', 'ipv6-name', name, 'enable-default-log'])
+
+        self.cli_set(['firewall', 'ipv6-name', name, 'rule', '3', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv6-name', name, 'rule', '3', 'packet-length', '65'])
+        self.cli_set(['firewall', 'ipv6-name', name, 'rule', '3', 'packet-length', '513'])
+        self.cli_set(['firewall', 'ipv6-name', name, 'rule', '3', 'packet-length', '1025'])
+
+        self.cli_set(['firewall', 'ipv6-name', name, 'rule', '4', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv6-name', name, 'rule', '4', 'packet-length', '1-1999'])
+        self.cli_set(['firewall', 'ipv6-name', name, 'rule', '4', 'packet-length-exclude', '60000-65535'])
+
+        self.cli_set(['interfaces', 'ethernet', interface, 'firewall', 'in', 'ipv6-name', name])
+
+        self.cli_commit()
+
+        nftables_search = [
+            [f'iifname "{interface}"', f'jump NAME6_{name}'],
+            ['ip6 length { 65, 513, 1025 }', 'return'],
+            ['ip6 length { 1-1999 }', 'ip6 length != { 60000-65535 }', 'return'],
+            [f'log prefix "[{name}-default-D]"', 'drop']
+        ]
+
+        self.verify_nftables(nftables_search, 'ip6 filter')
+
     def test_state_policy(self):
         self.cli_set(['firewall', 'state-policy', 'established', 'action', 'accept'])
         self.cli_set(['firewall', 'state-policy', 'related', 'action', 'accept'])
diff --git a/src/validators/packet-length b/src/validators/packet-length
deleted file mode 100755
index d96093849..000000000
--- a/src/validators/packet-length
+++ /dev/null
@@ -1,29 +0,0 @@
-#!/usr/bin/python3
-
-from sys import argv
-from sys import exit
-import re
-
-if __name__ == '__main__':
-    if len(argv)>1:
-        lengths = argv[1].split(",")
-
-        for length in lengths:
-            if length and length[0] == '!':
-                length = length[1:]
-            if re.match('^[0-9]{1,5}-[0-9]{1,5}$', length):
-                length_1, length_2 = length.split('-')
-                if int(length_1) not in range(0, 65536) or int(length_2) not in range(0, 65536):
-                    print(f'Error: {length} is not a valid length range')
-                    exit(1)
-                if int(length_1) > int(length_2):
-                    print(f'Error: {length} is not a valid length range')
-                    exit(1)
-            elif length.isnumeric():
-                if int(length) not in range(0, 65536):
-                    print(f'Error: {length} is not a valid length value')
-                    exit(1)
-    else:
-        exit(2)
-
-    exit(0)
\ No newline at end of file