From df5a862beb84145dfc8434efde7d7fee783199cf Mon Sep 17 00:00:00 2001
From: sarthurdev <965089+sarthurdev@users.noreply.github.com>
Date: Thu, 13 Jan 2022 12:58:37 +0100
Subject: firewall: T4178: Use lowercase for TCP flags and add an validator
---
.../include/firewall/common-rule.xml.i | 34 ++++++++++++++++++++--
.../include/policy/route-common-rule-ipv6.xml.i | 34 ++++++++++++++++++++--
.../include/policy/route-common-rule.xml.i | 34 ++++++++++++++++++++--
python/vyos/firewall.py | 7 ++---
src/conf_mode/firewall.py | 3 ++
src/conf_mode/policy-route.py | 10 +++----
src/validators/tcp-flag | 19 ++++++++++++
7 files changed, 126 insertions(+), 15 deletions(-)
create mode 100755 src/validators/tcp-flag
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 @@
TCP flags to match
txt
- TCP flags to match
+ Multiple comma-separated flags
+
+
+ syn
+ Syncronise flag
+
+
+ ack
+ Acknowledge flag
+
+
+ fin
+ Finish flag
+
+
+ rst
+ Reset flag
+
+
+ urg
+ Urgent flag
+
+
+ psh
+ Push flag
- \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
+ \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
+
+
+
+
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 @@
TCP flags to match
txt
- TCP flags to match
+ Multiple comma-separated flags
+
+
+ syn
+ Syncronise flag
+
+
+ ack
+ Acknowledge flag
+
+
+ fin
+ Finish flag
+
+
+ rst
+ Reset flag
+
+
+ urg
+ Urgent flag
+
+
+ psh
+ Push flag
- \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
+ \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
+
+
+
+
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 @@
TCP flags to match
txt
- TCP flags to match
+ Multiple comma-separated flags
+
+
+ syn
+ Syncronise flag
+
+
+ ack
+ Acknowledge flag
+
+
+ fin
+ Finish flag
+
+
+ rst
+ Reset flag
+
+
+ urg
+ Urgent flag
+
+
+ psh
+ Push flag
- \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
+ \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
+
+
+
+
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)
--
cgit v1.2.3