From 6cf5767524b8519f86981943ab71ff288bf77d67 Mon Sep 17 00:00:00 2001
From: sarthurdev <965089+sarthurdev@users.noreply.github.com>
Date: Tue, 11 Jan 2022 01:10:59 +0100
Subject: policy: T2199: Refactor policy route script for better error handling
* Migrates all policy route references from `ipv6-route` to `route6`
* Update test config `dialup-router-medium-vpn` to test migration of `ipv6-route` to `route6`
---
smoketest/configs/dialup-router-medium-vpn | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
(limited to 'smoketest/configs/dialup-router-medium-vpn')
diff --git a/smoketest/configs/dialup-router-medium-vpn b/smoketest/configs/dialup-router-medium-vpn
index af7c075e4..7ca540b66 100644
--- a/smoketest/configs/dialup-router-medium-vpn
+++ b/smoketest/configs/dialup-router-medium-vpn
@@ -83,6 +83,7 @@ interfaces {
}
policy {
route LAN-POLICY-BASED-ROUTING
+ ipv6-route LAN6-POLICY-BASED-ROUTING
}
smp-affinity auto
speed auto
@@ -383,6 +384,29 @@ nat {
}
}
policy {
+ ipv6-route LAN6-POLICY-BASED-ROUTING {
+ rule 10 {
+ destination {
+ }
+ disable
+ set {
+ table 10
+ }
+ source {
+ address 2002::1
+ }
+ }
+ rule 20 {
+ destination {
+ }
+ set {
+ table 100
+ }
+ source {
+ address 2008::f
+ }
+ }
+ }
prefix-list user2-routes {
rule 1 {
action permit
--
cgit v1.2.3
From 64668771d5f14fc4b68fff382d166238c164bdde Mon Sep 17 00:00:00 2001
From: sarthurdev <965089+sarthurdev@users.noreply.github.com>
Date: Sat, 15 Jan 2022 12:48:48 +0100
Subject: firewall: policy: T4178: Migrate and refactor tcp flags
* Add support for ECN and CWR flags
---
.../include/firewall/common-rule.xml.i | 51 +--------
.../include/firewall/tcp-flags.xml.i | 119 +++++++++++++++++++++
.../include/policy/route-common-rule-ipv6.xml.i | 51 +--------
.../include/policy/route-common-rule.xml.i | 51 +--------
python/vyos/firewall.py | 10 +-
smoketest/configs/dialup-router-medium-vpn | 9 ++
smoketest/scripts/cli/test_firewall.py | 16 +--
smoketest/scripts/cli/test_policy_route.py | 6 +-
src/conf_mode/firewall.py | 12 ++-
src/conf_mode/policy-route.py | 14 ++-
src/migration-scripts/firewall/6-to-7 | 21 ++++
src/migration-scripts/policy/1-to-2 | 19 ++++
src/validators/tcp-flag | 14 ++-
13 files changed, 213 insertions(+), 180 deletions(-)
create mode 100644 interface-definitions/include/firewall/tcp-flags.xml.i
(limited to 'smoketest/configs/dialup-router-medium-vpn')
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 @@
-
-
- TCP flags to match
-
-
-
-
- TCP flags to match
-
- txt
- Multiple comma-separated flags
-
-
- syn
- Syncronise flag
-
-
- ack
- Acknowledge flag
-
-
- fin
- Finish flag
-
-
- rst
- Reset flag
-
-
- urg
- Urgent flag
-
-
- psh
- Push flag
-
-
-
- \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
-
-
- syn ack fin rst urg psh
-
-
-
-
-
-
-
-
+#include
Time to match rule
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 @@
+
+
+
+ TCP flags to match
+
+
+
+
+ TCP flags to match
+
+
+
+
+ Synchronise flag
+
+
+
+
+
+ Acknowledge flag
+
+
+
+
+
+ Finish flag
+
+
+
+
+
+ Reset flag
+
+
+
+
+
+ Urgent flag
+
+
+
+
+
+ Push flag
+
+
+
+
+
+ Explicit Congestion Notification flag
+
+
+
+
+
+ Congestion Window Reduced flag
+
+
+
+
+
+ Match flags not set
+
+
+
+
+ Synchronise flag
+
+
+
+
+
+ Acknowledge flag
+
+
+
+
+
+ Finish flag
+
+
+
+
+
+ Reset flag
+
+
+
+
+
+ Urgent flag
+
+
+
+
+
+ Push flag
+
+
+
+
+
+ Explicit Congestion Notification flag
+
+
+
+
+
+ Congestion Window Reduced flag
+
+
+
+
+
+
+
+
+
+
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 @@
-
-
- TCP flags to match
-
-
-
-
- TCP flags to match
-
- txt
- Multiple comma-separated flags
-
-
- syn
- Syncronise flag
-
-
- ack
- Acknowledge flag
-
-
- fin
- Finish flag
-
-
- rst
- Reset flag
-
-
- urg
- Urgent flag
-
-
- psh
- Push flag
-
-
-
- \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
-
-
- syn ack fin rst urg psh
-
-
-
-
-
-
-
-
+#include
Time to match rule
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 @@
-
-
- TCP flags to match
-
-
-
-
- TCP flags to match
-
- txt
- Multiple comma-separated flags
-
-
- syn
- Syncronise flag
-
-
- ack
- Acknowledge flag
-
-
- fin
- Finish flag
-
-
- rst
- Reset flag
-
-
- urg
- Urgent flag
-
-
- psh
- Push flag
-
-
-
- \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
-
-
- syn ack fin rst urg psh
-
-
-
-
-
-
-
-
+#include
Time to match rule
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/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)
--
cgit v1.2.3