Merge pull request #1174 from sarthurdev/firewall

firewall: T4178: T3873: tcp flags syntax refactor, intra-zone-filtering fix

15 files changed, 237 insertions, 182 deletions

diff --git a/data/templates/zone_policy/nftables.tmpl b/data/templates/zone_policy/nftables.tmpl
index fae6e8c4f..e59208a0d 100644
--- a/data/templates/zone_policy/nftables.tmpl
+++ b/data/templates/zone_policy/nftables.tmpl
@@ -29,6 +29,9 @@ table ip filter {
 {%     else %}
     chain VZONE_{{ zone_name }} {
         iifname { {{ zone_conf.interface | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6=False) }}
+{%       if zone_conf.intra_zone_filtering is defined %}
+        iifname { {{ zone_conf.interface | join(",") }} } counter return
+{%       endif %}
 {%       for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall.name is defined %}
 {%         if zone[from_zone].local_zone is not defined %}
         iifname { {{ zone[from_zone].interface | join(",") }} } counter jump {{ from_conf.firewall.name }}
@@ -63,6 +66,9 @@ table ip6 filter {
 {%     else %}
     chain VZONE6_{{ zone_name }} {
         iifname { {{ zone_conf.interface | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6=True) }}
+{%       if zone_conf.intra_zone_filtering is defined %}
+        iifname { {{ zone_conf.interface | join(",") }} } counter return
+{%       endif %}
 {%       for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall.ipv6_name is defined %}
 {%         if zone[from_zone].local_zone is not defined %}
         iifname { {{ zone[from_zone].interface | join(",") }} } counter jump {{ from_conf.firewall.ipv6_name }}
diff --git a/interface-definitions/include/firewall/common-rule.xml.i b/interface-definitions/include/firewall/common-rule.xml.i
index 6e8203c88..5ffbd639c 100644
--- a/interface-definitions/include/firewall/common-rule.xml.i
+++ b/interface-definitions/include/firewall/common-rule.xml.i
@@ -264,56 +264,7 @@
     </leafNode>
   </children>
 </node>
-<node name="tcp">
-  <properties>
-    <help>TCP flags to match</help>
-  </properties>
-  <children>
-    <leafNode name="flags">
-      <properties>
-        <help>TCP flags to match</help>
-        <valueHelp>
-          <format>txt</format>
-          <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 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>
-</node>
+#include <include/firewall/tcp-flags.xml.i>
 <node name="time">
   <properties>
     <help>Time to match rule</help>
diff --git a/interface-definitions/include/firewall/tcp-flags.xml.i b/interface-definitions/include/firewall/tcp-flags.xml.i
new file mode 100644
index 000000000..b99896687
--- /dev/null
+++ b/interface-definitions/include/firewall/tcp-flags.xml.i
@@ -0,0 +1,119 @@
+<!-- include start from firewall/tcp-flags.xml.i -->
+<node name="tcp">
+  <properties>
+    <help>TCP flags to match</help>
+  </properties>
+  <children>
+    <node name="flags">
+      <properties>
+        <help>TCP flags to match</help>
+      </properties>
+      <children>
+        <leafNode name="syn">
+          <properties>
+            <help>Synchronise flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <leafNode name="ack">
+          <properties>
+            <help>Acknowledge flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <leafNode name="fin">
+          <properties>
+            <help>Finish flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <leafNode name="rst">
+          <properties>
+            <help>Reset flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <leafNode name="urg">
+          <properties>
+            <help>Urgent flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <leafNode name="psh">
+          <properties>
+            <help>Push flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <leafNode name="ecn">
+          <properties>
+            <help>Explicit Congestion Notification flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <leafNode name="cwr">
+          <properties>
+            <help>Congestion Window Reduced flag</help>
+            <valueless/>
+          </properties>
+        </leafNode>
+        <node name="not">
+          <properties>
+            <help>Match flags not set</help>
+          </properties>
+          <children>
+            <leafNode name="syn">
+              <properties>
+                <help>Synchronise flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+            <leafNode name="ack">
+              <properties>
+                <help>Acknowledge flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+            <leafNode name="fin">
+              <properties>
+                <help>Finish flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+            <leafNode name="rst">
+              <properties>
+                <help>Reset flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+            <leafNode name="urg">
+              <properties>
+                <help>Urgent flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+            <leafNode name="psh">
+              <properties>
+                <help>Push flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+            <leafNode name="ecn">
+              <properties>
+                <help>Explicit Congestion Notification flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+            <leafNode name="cwr">
+              <properties>
+                <help>Congestion Window Reduced flag</help>
+                <valueless/>
+              </properties>
+            </leafNode>
+          </children>
+        </node>
+      </children>
+    </node>
+  </children>
+</node>
+<!-- include end -->
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 b8fee4b7b..735edbd48 100644
--- a/interface-definitions/include/policy/route-common-rule-ipv6.xml.i
+++ b/interface-definitions/include/policy/route-common-rule-ipv6.xml.i
@@ -320,56 +320,7 @@
     </leafNode>
   </children>
 </node>
-<node name="tcp">
-  <properties>
-    <help>TCP flags to match</help>
-  </properties>
-  <children>
-    <leafNode name="flags">
-      <properties>
-        <help>TCP flags to match</help>
-        <valueHelp>
-          <format>txt</format>
-          <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 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>
-</node>
+#include <include/firewall/tcp-flags.xml.i>
 <node name="time">
   <properties>
     <help>Time to match rule</help>
diff --git a/interface-definitions/include/policy/route-common-rule.xml.i b/interface-definitions/include/policy/route-common-rule.xml.i
index 17b47474d..4452f78fc 100644
--- a/interface-definitions/include/policy/route-common-rule.xml.i
+++ b/interface-definitions/include/policy/route-common-rule.xml.i
@@ -320,56 +320,7 @@
     </leafNode>
   </children>
 </node>
-<node name="tcp">
-  <properties>
-    <help>TCP flags to match</help>
-  </properties>
-  <children>
-    <leafNode name="flags">
-      <properties>
-        <help>TCP flags to match</help>
-        <valueHelp>
-          <format>txt</format>
-          <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 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>
-</node>
+#include <include/firewall/tcp-flags.xml.i>
 <node name="time">
   <properties>
     <help>Time to match rule</help>
diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py
index acde9f913..ad84393df 100644
--- a/python/vyos/firewall.py
+++ b/python/vyos/firewall.py
@@ -185,14 +185,8 @@ def parse_rule(rule_conf, fw_name, rule_id, ip_name):
     return " ".join(output)
 
 def parse_tcp_flags(flags):
-    all_flags = []
-    include = []
-    for flag in flags.split(","):
-        if flag[0] == '!':
-            flag = flag[1:].lower()
-        else:
-            include.append(flag.lower())
-        all_flags.append(flag.lower())
+    include = [flag for flag in flags if flag != 'not']
+    all_flags = include + [flag for flag in flags['not']] if 'not' in flags else []
     return f'tcp flags & ({"|".join(all_flags)}) == {"|".join(include)}'
 
 def parse_time(time):
diff --git a/smoketest/configs/dialup-router-medium-vpn b/smoketest/configs/dialup-router-medium-vpn
index 7ca540b66..63d955738 100644
--- a/smoketest/configs/dialup-router-medium-vpn
+++ b/smoketest/configs/dialup-router-medium-vpn
@@ -6,6 +6,15 @@ firewall {
     ipv6-src-route disable
     ip-src-route disable
     log-martians enable
+    name test_tcp_flags {
+        rule 1 {
+            action drop
+            protocol tcp
+            tcp {
+                flags SYN,ACK,!RST,!FIN
+            }
+        }
+    }
     options {
         interface vtun0 {
             adjust-mss 1380
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index 2b3b354ba..c70743a9f 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -53,7 +53,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'source', 'group', 'network-group', 'smoketest_network'])
         self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'destination', 'address', '172.16.10.10'])
         self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'destination', 'group', 'port-group', 'smoketest_port'])
-        self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'protocol', 'tcp'])
+        self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'protocol', 'tcp_udp'])
 
         self.cli_set(['interfaces', 'ethernet', 'eth0', 'firewall', 'in', 'name', 'smoketest'])
 
@@ -61,7 +61,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
 
         nftables_search = [
             ['iifname "eth0"', 'jump smoketest'],
-            ['ip saddr { 172.16.99.0/24 }', 'ip daddr 172.16.10.10', 'tcp dport { 53, 123 }', 'return'],
+            ['ip saddr { 172.16.99.0/24 }', 'ip daddr 172.16.10.10', 'th dport { 53, 123 }', 'return'],
         ]
 
         nftables_output = cmd('sudo nft list table ip filter')
@@ -72,7 +72,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
                 if all(item in line for item in search):
                     matched = True
                     break
-            self.assertTrue(matched)
+            self.assertTrue(matched, msg=search)
 
     def test_basic_rules(self):
         self.cli_set(['firewall', 'name', 'smoketest', 'default-action', 'drop'])
@@ -80,8 +80,10 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'source', 'address', '172.16.20.10'])
         self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'destination', 'address', '172.16.10.10'])
         self.cli_set(['firewall', 'name', 'smoketest', 'rule', '2', 'action', 'reject'])
-        self.cli_set(['firewall', 'name', 'smoketest', 'rule', '2', 'protocol', 'tcp_udp'])
+        self.cli_set(['firewall', 'name', 'smoketest', 'rule', '2', 'protocol', 'tcp'])
         self.cli_set(['firewall', 'name', 'smoketest', 'rule', '2', 'destination', 'port', '8888'])
+        self.cli_set(['firewall', 'name', 'smoketest', 'rule', '2', 'tcp', 'flags', 'syn'])
+        self.cli_set(['firewall', 'name', 'smoketest', 'rule', '2', 'tcp', 'flags', 'not', 'ack'])
 
         self.cli_set(['interfaces', 'ethernet', 'eth0', 'firewall', 'in', 'name', 'smoketest'])
 
@@ -90,7 +92,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         nftables_search = [
             ['iifname "eth0"', 'jump smoketest'],
             ['saddr 172.16.20.10', 'daddr 172.16.10.10', 'return'],
-            ['meta l4proto { tcp, udp }', 'th dport { 8888 }', 'reject'],
+            ['tcp flags & (syn | ack) == syn', 'tcp dport { 8888 }', 'reject'],
             ['smoketest default-action', 'drop']
         ]
 
@@ -102,7 +104,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
                 if all(item in line for item in search):
                     matched = True
                     break
-            self.assertTrue(matched)
+            self.assertTrue(matched, msg=search)
 
     def test_basic_rules_ipv6(self):
         self.cli_set(['firewall', 'ipv6-name', 'v6-smoketest', 'default-action', 'drop'])
@@ -132,7 +134,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
                 if all(item in line for item in search):
                     matched = True
                     break
-            self.assertTrue(matched)
+            self.assertTrue(matched, msg=search)
 
     def test_state_policy(self):
         self.cli_set(['firewall', 'state-policy', 'established', 'action', 'accept'])
diff --git a/smoketest/scripts/cli/test_policy_route.py b/smoketest/scripts/cli/test_policy_route.py
index 4463a2255..9035f0832 100755
--- a/smoketest/scripts/cli/test_policy_route.py
+++ b/smoketest/scripts/cli/test_policy_route.py
@@ -63,8 +63,10 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
             self.assertTrue(matched)
 
     def test_pbr_table(self):
-        self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'protocol', 'tcp_udp'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'protocol', 'tcp'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'destination', 'port', '8888'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'tcp', 'flags', 'syn'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'tcp', 'flags', 'not', 'ack'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'set', 'table', table_id])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '1', 'protocol', 'tcp_udp'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '1', 'destination', 'port', '8888'])
@@ -81,7 +83,7 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
 
         nftables_search = [
             ['iifname "eth0"', 'jump VYOS_PBR_smoketest'],
-            ['meta l4proto { tcp, udp }', 'th dport { 8888 }', 'meta mark set ' + mark_hex]
+            ['tcp flags & (syn | ack) == syn', 'tcp dport { 8888 }', 'meta mark set ' + mark_hex]
         ]
 
         nftables_output = cmd('sudo nft list table ip mangle')
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index 853470fd8..906d477b0 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -142,8 +142,16 @@ 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')
+    tcp_flags = dict_search_args(rule_conf, 'tcp', 'flags')
+    if tcp_flags:
+        if dict_search_args(rule_conf, 'protocol') != 'tcp':
+            raise ConfigError('Protocol must be tcp when specifying tcp flags')
+
+        not_flags = dict_search_args(rule_conf, 'tcp', 'flags', 'not')
+        if not_flags:
+            duplicates = [flag for flag in tcp_flags if flag in not_flags]
+            if duplicates:
+                raise ConfigError(f'Cannot match a tcp flag as set and not set')
 
     for side in ['destination', 'source']:
         if side in rule_conf:
diff --git a/src/conf_mode/policy-route.py b/src/conf_mode/policy-route.py
index 30597ef4e..eb13788dd 100755
--- a/src/conf_mode/policy-route.py
+++ b/src/conf_mode/policy-route.py
@@ -97,11 +97,19 @@ def verify_rule(policy, name, rule_conf, ipv6):
     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(","):
+            if not tcp_flags or 'syn' not in tcp_flags:
                 raise ConfigError(f'{name} rule {rule_id}: TCP SYN flag must be set to modify TCP-MSS')
 
-    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')
+    tcp_flags = dict_search_args(rule_conf, 'tcp', 'flags')
+    if tcp_flags:
+        if dict_search_args(rule_conf, 'protocol') != 'tcp':
+            raise ConfigError('Protocol must be tcp when specifying tcp flags')
+
+        not_flags = dict_search_args(rule_conf, 'tcp', 'flags', 'not')
+        if not_flags:
+            duplicates = [flag for flag in tcp_flags if flag in not_flags]
+            if duplicates:
+                raise ConfigError(f'Cannot match a tcp flag as set and not set')
 
     for side in ['destination', 'source']:
         if side in rule_conf:
diff --git a/src/migration-scripts/firewall/6-to-7 b/src/migration-scripts/firewall/6-to-7
index 4a4097d56..bc0b19325 100755
--- a/src/migration-scripts/firewall/6-to-7
+++ b/src/migration-scripts/firewall/6-to-7
@@ -17,6 +17,7 @@
 # T2199: Remove unavailable nodes due to XML/Python implementation using nftables
 #        monthdays: nftables does not have a monthdays equivalent
 #        utc: nftables userspace uses localtime and calculates the UTC offset automatically
+# T4178: Update tcp flags to use multi value node
 
 from sys import argv
 from sys import exit
@@ -45,6 +46,7 @@ if config.exists(base + ['name']):
         if config.exists(base + ['name', name, 'rule']):
             for rule in config.list_nodes(base + ['name', name, 'rule']):
                 rule_time = base + ['name', name, 'rule', rule, 'time']
+                rule_tcp_flags = base + ['name', name, 'rule', rule, 'tcp', 'flags']
 
                 if config.exists(rule_time + ['monthdays']):
                     config.delete(rule_time + ['monthdays'])
@@ -52,11 +54,21 @@ if config.exists(base + ['name']):
                 if config.exists(rule_time + ['utc']):
                     config.delete(rule_time + ['utc'])
 
+                if config.exists(rule_tcp_flags):
+                    tmp = config.return_value(rule_tcp_flags)
+                    config.delete(rule_tcp_flags)
+                    for flag in tmp.split(","):
+                        if flag[0] == '!':
+                            config.set(rule_tcp_flags + ['not', flag[1:].lower()])
+                        else:
+                            config.set(rule_tcp_flags + [flag.lower()])
+
 if config.exists(base + ['ipv6-name']):
     for name in config.list_nodes(base + ['ipv6-name']):
         if config.exists(base + ['ipv6-name', name, 'rule']):
             for rule in config.list_nodes(base + ['ipv6-name', name, 'rule']):
                 rule_time = base + ['ipv6-name', name, 'rule', rule, 'time']
+                rule_tcp_flags = base + ['ipv6-name', name, 'rule', rule, 'tcp', 'flags']
 
                 if config.exists(rule_time + ['monthdays']):
                     config.delete(rule_time + ['monthdays'])
@@ -64,6 +76,15 @@ if config.exists(base + ['ipv6-name']):
                 if config.exists(rule_time + ['utc']):
                     config.delete(rule_time + ['utc'])
 
+                if config.exists(rule_tcp_flags):
+                    tmp = config.return_value(rule_tcp_flags)
+                    config.delete(rule_tcp_flags)
+                    for flag in tmp.split(","):
+                        if flag[0] == '!':
+                            config.set(rule_tcp_flags + ['not', flag[1:].lower()])
+                        else:
+                            config.set(rule_tcp_flags + [flag.lower()])
+
 try:
     with open(file_name, 'w') as f:
         f.write(config.to_string())
diff --git a/src/migration-scripts/policy/1-to-2 b/src/migration-scripts/policy/1-to-2
index 7ffceef22..eebbf9d41 100755
--- a/src/migration-scripts/policy/1-to-2
+++ b/src/migration-scripts/policy/1-to-2
@@ -16,6 +16,7 @@
 
 # T4170: rename "policy ipv6-route" to "policy route6" to match common
 #        IPv4/IPv6 schema
+# T4178: Update tcp flags to use multi value node
 
 from sys import argv
 from sys import exit
@@ -41,6 +42,24 @@ if not config.exists(base):
 config.rename(base, 'route6')
 config.set_tag(['policy', 'route6'])
 
+for route in ['route', 'route6']:
+    route_path = ['policy', route]
+    if config.exists(route_path):
+        for name in config.list_nodes(route_path):
+            if config.exists(route_path + [name, 'rule']):
+                for rule in config.list_nodes(route_path + [name, 'rule']):
+                    rule_tcp_flags = route_path + [name, 'rule', rule, 'tcp', 'flags']
+
+                    if config.exists(rule_tcp_flags):
+                        tmp = config.return_value(rule_tcp_flags)
+                        config.delete(rule_tcp_flags)
+                        for flag in tmp.split(","):
+                            for flag in tmp.split(","):
+                                if flag[0] == '!':
+                                    config.set(rule_tcp_flags + ['not', flag[1:].lower()])
+                                else:
+                                    config.set(rule_tcp_flags + [flag.lower()])
+
 if config.exists(['interfaces']):
     def if_policy_rename(config, path):
         if config.exists(path + ['policy', 'ipv6-route']):
diff --git a/src/validators/port-range b/src/validators/port-range
index 6c01048f0..5468000a7 100755
--- a/src/validators/port-range
+++ b/src/validators/port-range
@@ -3,6 +3,19 @@
 import sys
 import re
 
+from vyos.util import read_file
+
+services_file = '/etc/services'
+
+def get_services():
+    names = []
+    service_data = read_file(services_file, "")
+    for line in service_data.split("\n"):
+        if not line or line[0] == '#':
+            continue
+        names.append(line.split(None, 1)[0])
+    return names
+
 def error(port_range):
     print(f'Error: {port_range} is not a valid port or port range')
     sys.exit(1)
@@ -16,8 +29,11 @@ if __name__ == '__main__':
                 error(port_range)
             if int(port_1) > int(port_2):
                 error(port_range)
-        elif not port_range.isnumeric() or int(port_range) not in range(1, 65536):
-                error(port_range)
+        elif port_range.isnumeric() and int(port_range) not in range(1, 65536):
+            error(port_range)
+        elif not port_range.isnumeric() and port_range not in get_services():
+            print(f'Error: {port_range} is not a valid service name')
+            sys.exit(1)
     else:
         sys.exit(2)
 
diff --git a/src/validators/tcp-flag b/src/validators/tcp-flag
index 86ebec189..1496b904a 100755
--- a/src/validators/tcp-flag
+++ b/src/validators/tcp-flag
@@ -5,14 +5,12 @@ 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)
+        flag = sys.argv[1]
+        if flag and flag[0] == '!':
+            flag = flag[1:]
+        if flag not in ['syn', 'ack', 'rst', 'fin', 'urg', 'psh', 'ecn', 'cwr']:
+            print(f'Error: {flag} is not a valid TCP flag')
+            sys.exit(1)
     else:
         sys.exit(2)