Merge pull request #1167 from sarthurdev/firewall

firewall: T4178: Use lowercase for TCP flags and add an validator

7 files changed, 126 insertions, 15 deletions

diff --git a/interface-definitions/include/firewall/common-rule.xml.i b/interface-definitions/include/firewall/common-rule.xml.i
index 92950cc68..6e8203c88 100644
--- a/interface-definitions/include/firewall/common-rule.xml.i
+++ b/interface-definitions/include/firewall/common-rule.xml.i
@@ -274,12 +274,42 @@
         <help>TCP flags to match</help>
         <valueHelp>
           <format>txt</format>
-          <description>TCP flags to match</description>
+          <description>Multiple comma-separated flags</description>
+        </valueHelp>
+        <valueHelp>
+          <format>syn</format>
+          <description>Syncronise flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>ack</format>
+          <description>Acknowledge flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>fin</format>
+          <description>Finish flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>rst</format>
+          <description>Reset flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>urg</format>
+          <description>Urgent flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>psh</format>
+          <description>Push flag</description>
         </valueHelp>
         <valueHelp>
           <format> </format>
-          <description>\n\n  Allowed values for TCP flags : SYN ACK FIN RST URG PSH ALL\n  When specifying more than one flag, flags should be comma-separated.\n For example : value of 'SYN,!ACK,!FIN,!RST' will only match packets with\n  the SYN flag set, and the ACK, FIN and RST flags unset</description>
+          <description>\n When specifying more than one flag, flags should be comma-separated.\n For example: value of 'SYN,!ACK,!FIN,!RST' will only match packets with\n the SYN flag set, and the ACK, FIN and RST flags unset</description>
         </valueHelp>
+        <completionHelp>
+          <list>syn ack fin rst urg psh</list>
+        </completionHelp>
+        <constraint>
+          <validator name="tcp-flag"/>
+        </constraint>
       </properties>
     </leafNode>
   </children>
diff --git a/interface-definitions/include/policy/route-common-rule-ipv6.xml.i b/interface-definitions/include/policy/route-common-rule-ipv6.xml.i
index 2d6adcd1d..b8fee4b7b 100644
--- a/interface-definitions/include/policy/route-common-rule-ipv6.xml.i
+++ b/interface-definitions/include/policy/route-common-rule-ipv6.xml.i
@@ -330,12 +330,42 @@
         <help>TCP flags to match</help>
         <valueHelp>
           <format>txt</format>
-          <description>TCP flags to match</description>
+          <description>Multiple comma-separated flags</description>
+        </valueHelp>
+        <valueHelp>
+          <format>syn</format>
+          <description>Syncronise flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>ack</format>
+          <description>Acknowledge flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>fin</format>
+          <description>Finish flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>rst</format>
+          <description>Reset flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>urg</format>
+          <description>Urgent flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>psh</format>
+          <description>Push flag</description>
         </valueHelp>
         <valueHelp>
           <format> </format>
-          <description>\n\n  Allowed values for TCP flags : SYN ACK FIN RST URG PSH ALL\n  When specifying more than one flag, flags should be comma-separated.\n For example : value of 'SYN,!ACK,!FIN,!RST' will only match packets with\n  the SYN flag set, and the ACK, FIN and RST flags unset</description>
+          <description>\n When specifying more than one flag, flags should be comma-separated.\n For example: value of 'SYN,!ACK,!FIN,!RST' will only match packets with\n the SYN flag set, and the ACK, FIN and RST flags unset</description>
         </valueHelp>
+        <completionHelp>
+          <list>syn ack fin rst urg psh</list>
+        </completionHelp>
+        <constraint>
+          <validator name="tcp-flag"/>
+        </constraint>
       </properties>
     </leafNode>
   </children>
diff --git a/interface-definitions/include/policy/route-common-rule.xml.i b/interface-definitions/include/policy/route-common-rule.xml.i
index c4deefd2a..17b47474d 100644
--- a/interface-definitions/include/policy/route-common-rule.xml.i
+++ b/interface-definitions/include/policy/route-common-rule.xml.i
@@ -330,12 +330,42 @@
         <help>TCP flags to match</help>
         <valueHelp>
           <format>txt</format>
-          <description>TCP flags to match</description>
+          <description>Multiple comma-separated flags</description>
+        </valueHelp>
+        <valueHelp>
+          <format>syn</format>
+          <description>Syncronise flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>ack</format>
+          <description>Acknowledge flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>fin</format>
+          <description>Finish flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>rst</format>
+          <description>Reset flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>urg</format>
+          <description>Urgent flag</description>
+        </valueHelp>
+        <valueHelp>
+          <format>psh</format>
+          <description>Push flag</description>
         </valueHelp>
         <valueHelp>
           <format> </format>
-          <description>\n\n  Allowed values for TCP flags : SYN ACK FIN RST URG PSH ALL\n  When specifying more than one flag, flags should be comma-separated.\n For example : value of 'SYN,!ACK,!FIN,!RST' will only match packets with\n  the SYN flag set, and the ACK, FIN and RST flags unset</description>
+          <description>\n When specifying more than one flag, flags should be comma-separated.\n For example: value of 'SYN,!ACK,!FIN,!RST' will only match packets with\n the SYN flag set, and the ACK, FIN and RST flags unset</description>
         </valueHelp>
+        <completionHelp>
+          <list>syn ack fin rst urg psh</list>
+        </completionHelp>
+        <constraint>
+          <validator name="tcp-flag"/>
+        </constraint>
       </properties>
     </leafNode>
   </children>
diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py
index 66dc8bc40..acde9f913 100644
--- a/python/vyos/firewall.py
+++ b/python/vyos/firewall.py
@@ -171,7 +171,6 @@ def parse_rule(rule_conf, fw_name, rule_id, ip_name):
     if tcp_flags:
         output.append(parse_tcp_flags(tcp_flags))
 
-
     output.append('counter')
 
     if 'set' in rule_conf:
@@ -190,10 +189,10 @@ def parse_tcp_flags(flags):
     include = []
     for flag in flags.split(","):
         if flag[0] == '!':
-            flag = flag[1:]
+            flag = flag[1:].lower()
         else:
-            include.append(flag)
-        all_flags.append(flag)
+            include.append(flag.lower())
+        all_flags.append(flag.lower())
     return f'tcp flags & ({"|".join(all_flags)}) == {"|".join(include)}'
 
 def parse_time(time):
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index 7b491a325..853470fd8 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -142,6 +142,9 @@ def verify_rule(firewall, rule_conf, ipv6):
         if not {'count', 'time'} <= set(rule_conf['recent']):
             raise ConfigError('Recent "count" and "time" values must be defined')
 
+    if dict_search_args(rule_conf, 'tcp', 'flags') and dict_search_args(rule_conf, 'protocol') != 'tcp':
+        raise ConfigError('Protocol must be tcp when specifying tcp flags')
+
     for side in ['destination', 'source']:
         if side in rule_conf:
             side_conf = rule_conf[side]
diff --git a/src/conf_mode/policy-route.py b/src/conf_mode/policy-route.py
index c5904309f..30597ef4e 100755
--- a/src/conf_mode/policy-route.py
+++ b/src/conf_mode/policy-route.py
@@ -76,7 +76,7 @@ def get_config(config=None):
 
     return policy
 
-def verify_rule(policy, rule_conf, ipv6):
+def verify_rule(policy, name, rule_conf, ipv6):
     icmp = 'icmp' if not ipv6 else 'icmpv6'
     if icmp in rule_conf:
         icmp_defined = False
@@ -93,14 +93,14 @@ def verify_rule(policy, rule_conf, ipv6):
 
         if icmp_defined and 'protocol' not in rule_conf or rule_conf['protocol'] != icmp:
             raise ConfigError(f'{name} rule {rule_id}: ICMP type/code or type-name can only be defined if protocol is ICMP')
+
     if 'set' in rule_conf:
         if 'tcp_mss' in rule_conf['set']:
             tcp_flags = dict_search_args(rule_conf, 'tcp', 'flags')
             if not tcp_flags or 'SYN' not in tcp_flags.split(","):
                 raise ConfigError(f'{name} rule {rule_id}: TCP SYN flag must be set to modify TCP-MSS')
-    if 'tcp' in rule_conf:
-        if 'flags' in rule_conf['tcp']:
-            if 'protocol' not in rule_conf or rule_conf['protocol'] != 'tcp':
+
+    if dict_search_args(rule_conf, 'tcp', 'flags') and dict_search_args(rule_conf, 'protocol') != 'tcp':
                 raise ConfigError(f'{name} rule {rule_id}: TCP flags can only be set if protocol is set to TCP')
 
     for side in ['destination', 'source']:
@@ -138,7 +138,7 @@ def verify(policy):
             for name, pol_conf in policy[route].items():
                 if 'rule' in pol_conf:
                     for rule_id, rule_conf in pol_conf['rule'].items():
-                        verify_rule(policy, rule_conf, ipv6)
+                        verify_rule(policy, name, rule_conf, ipv6)
 
     for ifname, if_policy in policy['interfaces'].items():
         name = dict_search_args(if_policy, 'route')
diff --git a/src/validators/tcp-flag b/src/validators/tcp-flag
new file mode 100755
index 000000000..86ebec189
--- /dev/null
+++ b/src/validators/tcp-flag
@@ -0,0 +1,19 @@
+#!/usr/bin/python3
+
+import sys
+import re
+
+if __name__ == '__main__':
+    if len(sys.argv)>1:
+        flags = sys.argv[1].split(",")
+
+        for flag in flags:
+            if flag and flag[0] == '!':
+                flag = flag[1:]
+            if flag.lower() not in ['syn', 'ack', 'rst', 'fin', 'urg', 'psh']:
+                print(f'Error: {flag} is not a valid TCP flag')
+                sys.exit(1)
+    else:
+        sys.exit(2)
+
+    sys.exit(0)