From 54675c2cc9aa9c1315478107cce14e5ba23d865e Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Tue, 11 Jan 2022 10:27:13 +0100 Subject: policy: T4170: rename "policy ipv6-route" -> "policy route6" In order to have a consistent looking CLI we should rename this CLI node. There is: * access-list and access-list6 (policy) * prefix-list and prefix-list6 (policy) * route and route6 (static routes) --- src/migration-scripts/policy/1-to-2 | 49 +++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100755 src/migration-scripts/policy/1-to-2 (limited to 'src/migration-scripts/policy/1-to-2') diff --git a/src/migration-scripts/policy/1-to-2 b/src/migration-scripts/policy/1-to-2 new file mode 100755 index 000000000..3e46227de --- /dev/null +++ b/src/migration-scripts/policy/1-to-2 @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2022 VyOS maintainers and contributors +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 or later as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# T4170: rename "policy ipv6-route" to "policy route6" to match common +# IPv4/IPv6 schema + +from sys import argv +from sys import exit + +from vyos.configtree import ConfigTree + +if (len(argv) < 1): + print("Must specify file name!") + exit(1) + +file_name = argv[1] + +with open(file_name, 'r') as f: + config_file = f.read() + +base = ['policy', 'ipv6-route'] +config = ConfigTree(config_file) + +if not config.exists(base): + # Nothing to do + exit(0) + +config.rename(base, 'route6') +config.set_tag(['policy', 'route6']) + +try: + with open(file_name, 'w') as f: + f.write(config.to_string()) +except OSError as e: + print(f'Failed to save the modified config: {e}') + exit(1) -- cgit v1.2.3 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` --- data/templates/firewall/nftables-policy.tmpl | 6 + .../include/interface/interface-policy-vif-c.xml.i | 4 +- .../include/interface/interface-policy-vif.xml.i | 4 +- .../include/interface/interface-policy.xml.i | 4 +- smoketest/configs/dialup-router-medium-vpn | 24 +++ smoketest/scripts/cli/test_policy_route.py | 28 +++- src/conf_mode/policy-route-interface.py | 8 +- src/conf_mode/policy-route.py | 169 +++++++++++++++------ src/migration-scripts/policy/1-to-2 | 18 +++ src/op_mode/policy_route.py | 10 +- 10 files changed, 213 insertions(+), 62 deletions(-) (limited to 'src/migration-scripts/policy/1-to-2') diff --git a/data/templates/firewall/nftables-policy.tmpl b/data/templates/firewall/nftables-policy.tmpl index 668ec7388..484b6f203 100644 --- a/data/templates/firewall/nftables-policy.tmpl +++ b/data/templates/firewall/nftables-policy.tmpl @@ -1,5 +1,11 @@ #!/usr/sbin/nft -f +{% if cleanup_commands is defined %} +{% for command in cleanup_commands %} +{{ command }} +{% endfor %} +{% endif %} + include "/run/nftables_defines.conf" table ip mangle { diff --git a/interface-definitions/include/interface/interface-policy-vif-c.xml.i b/interface-definitions/include/interface/interface-policy-vif-c.xml.i index 5dad6422b..866fcd5c0 100644 --- a/interface-definitions/include/interface/interface-policy-vif-c.xml.i +++ b/interface-definitions/include/interface/interface-policy-vif-c.xml.i @@ -13,11 +13,11 @@ - + IPv6 policy route ruleset for interface - policy ipv6-route + policy route6 diff --git a/interface-definitions/include/interface/interface-policy-vif.xml.i b/interface-definitions/include/interface/interface-policy-vif.xml.i index 5ee80ae13..83510fe59 100644 --- a/interface-definitions/include/interface/interface-policy-vif.xml.i +++ b/interface-definitions/include/interface/interface-policy-vif.xml.i @@ -13,11 +13,11 @@ - + IPv6 policy route ruleset for interface - policy ipv6-route + policy route6 diff --git a/interface-definitions/include/interface/interface-policy.xml.i b/interface-definitions/include/interface/interface-policy.xml.i index 06f025af1..42a8fd009 100644 --- a/interface-definitions/include/interface/interface-policy.xml.i +++ b/interface-definitions/include/interface/interface-policy.xml.i @@ -13,11 +13,11 @@ - + IPv6 policy route ruleset for interface - policy ipv6-route + policy route6 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 diff --git a/smoketest/scripts/cli/test_policy_route.py b/smoketest/scripts/cli/test_policy_route.py index 70a234187..4463a2255 100755 --- a/smoketest/scripts/cli/test_policy_route.py +++ b/smoketest/scripts/cli/test_policy_route.py @@ -31,8 +31,9 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase): def tearDown(self): self.cli_delete(['interfaces', 'ethernet', 'eth0']) + self.cli_delete(['protocols', 'static']) self.cli_delete(['policy', 'route']) - self.cli_delete(['policy', 'ipv6-route']) + self.cli_delete(['policy', 'route6']) self.cli_commit() def test_pbr_mark(self): @@ -65,13 +66,19 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase): self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'protocol', 'tcp_udp']) self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'destination', 'port', '8888']) 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']) + self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '1', 'set', 'table', table_id]) self.cli_set(['interfaces', 'ethernet', 'eth0', 'policy', 'route', 'smoketest']) + self.cli_set(['interfaces', 'ethernet', 'eth0', 'policy', 'route6', 'smoketest6']) self.cli_commit() mark_hex = "{0:#010x}".format(table_mark_offset - int(table_id)) + # IPv4 + nftables_search = [ ['iifname "eth0"', 'jump VYOS_PBR_smoketest'], ['meta l4proto { tcp, udp }', 'th dport { 8888 }', 'meta mark set ' + mark_hex] @@ -87,6 +94,25 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase): break self.assertTrue(matched) + # IPv6 + + nftables6_search = [ + ['iifname "eth0"', 'jump VYOS_PBR6_smoketest'], + ['meta l4proto { tcp, udp }', 'th dport { 8888 }', 'meta mark set ' + mark_hex] + ] + + nftables6_output = cmd('sudo nft list table ip6 mangle') + + for search in nftables6_search: + matched = False + for line in nftables6_output.split("\n"): + if all(item in line for item in search): + matched = True + break + self.assertTrue(matched) + + # IP rule fwmark -> table + ip_rule_search = [ ['fwmark ' + hex(table_mark_offset - int(table_id)), 'lookup ' + table_id] ] diff --git a/src/conf_mode/policy-route-interface.py b/src/conf_mode/policy-route-interface.py index e81135a74..1108aebe6 100755 --- a/src/conf_mode/policy-route-interface.py +++ b/src/conf_mode/policy-route-interface.py @@ -52,7 +52,7 @@ def verify(if_policy): if not if_policy: return None - for route in ['route', 'ipv6_route']: + for route in ['route', 'route6']: if route in if_policy: if route not in if_policy['policy']: raise ConfigError('Policy route not configured') @@ -71,7 +71,7 @@ def cleanup_rule(table, chain, ifname, new_name=None): results = cmd(f'nft -a list chain {table} {chain}').split("\n") retval = None for line in results: - if f'oifname "{ifname}"' in line: + if f'ifname "{ifname}"' in line: if new_name and f'jump {new_name}' in line: # new_name is used to clear rules for any previously referenced chains # returns true when rule exists and doesn't need to be created @@ -98,8 +98,8 @@ def apply(if_policy): else: cleanup_rule('ip mangle', route_chain, ifname) - if 'ipv6_route' in if_policy: - name = 'VYOS_PBR6_' + if_policy['ipv6_route'] + if 'route6' in if_policy: + name = 'VYOS_PBR6_' + if_policy['route6'] rule_exists = cleanup_rule('ip6 mangle', ipv6_route_chain, ifname, name) if not rule_exists: diff --git a/src/conf_mode/policy-route.py b/src/conf_mode/policy-route.py index 9edab4b47..c5904309f 100755 --- a/src/conf_mode/policy-route.py +++ b/src/conf_mode/policy-route.py @@ -31,6 +31,35 @@ airbag.enable() mark_offset = 0x7FFFFFFF nftables_conf = '/run/nftables_policy.conf' +preserve_chains = [ + 'VYOS_PBR_PREROUTING', + 'VYOS_PBR_POSTROUTING', + 'VYOS_PBR6_PREROUTING', + 'VYOS_PBR6_POSTROUTING' +] + +valid_groups = [ + 'address_group', + 'network_group', + 'port_group' +] + +def get_policy_interfaces(conf): + out = {} + interfaces = conf.get_config_dict(['interfaces'], key_mangling=('-', '_'), get_first_key=True, + no_tag_node_value_mangle=True) + def find_interfaces(iftype_conf, output={}, prefix=''): + for ifname, if_conf in iftype_conf.items(): + if 'policy' in if_conf: + output[prefix + ifname] = if_conf['policy'] + for vif in ['vif', 'vif_s', 'vif_c']: + if vif in if_conf: + output.update(find_interfaces(if_conf[vif], output, f'{prefix}{ifname}.')) + return output + for iftype, iftype_conf in interfaces.items(): + out.update(find_interfaces(iftype_conf)) + return out + def get_config(config=None): if config: conf = config @@ -38,61 +67,117 @@ def get_config(config=None): conf = Config() base = ['policy'] - if not conf.exists(base + ['route']) and not conf.exists(base + ['ipv6-route']): - return None - policy = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True, no_tag_node_value_mangle=True) + policy['firewall_group'] = conf.get_config_dict(['firewall', 'group'], key_mangling=('-', '_'), get_first_key=True, + no_tag_node_value_mangle=True) + policy['interfaces'] = get_policy_interfaces(conf) + return policy -def verify(policy): - # bail out early - looks like removal from running config - if not policy: - return None +def verify_rule(policy, rule_conf, ipv6): + icmp = 'icmp' if not ipv6 else 'icmpv6' + if icmp in rule_conf: + icmp_defined = False + if 'type_name' in rule_conf[icmp]: + icmp_defined = True + if 'code' in rule_conf[icmp] or 'type' in rule_conf[icmp]: + raise ConfigError(f'{name} rule {rule_id}: Cannot use ICMP type/code with ICMP type-name') + if 'code' in rule_conf[icmp]: + icmp_defined = True + if 'type' not in rule_conf[icmp]: + raise ConfigError(f'{name} rule {rule_id}: ICMP code can only be defined if ICMP type is defined') + if 'type' in rule_conf[icmp]: + icmp_defined = True + + 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': + raise ConfigError(f'{name} rule {rule_id}: TCP flags can only be set if protocol is set to TCP') + + for side in ['destination', 'source']: + if side in rule_conf: + side_conf = rule_conf[side] + + if 'group' in side_conf: + if {'address_group', 'network_group'} <= set(side_conf['group']): + raise ConfigError('Only one address-group or network-group can be specified') + + for group in valid_groups: + if group in side_conf['group']: + group_name = side_conf['group'][group] + fw_group = f'ipv6_{group}' if ipv6 and group in ['address_group', 'network_group'] else group + error_group = fw_group.replace("_", "-") + group_obj = dict_search_args(policy['firewall_group'], fw_group, group_name) + + if group_obj is None: + raise ConfigError(f'Invalid {error_group} "{group_name}" on policy route rule') + + if not group_obj: + print(f'WARNING: {error_group} "{group_name}" has no members') + + if 'port' in side_conf or dict_search_args(side_conf, 'group', 'port_group'): + if 'protocol' not in rule_conf: + raise ConfigError('Protocol must be defined if specifying a port or port-group') + + if rule_conf['protocol'] not in ['tcp', 'udp', 'tcp_udp']: + raise ConfigError('Protocol must be tcp, udp, or tcp_udp when specifying a port or port-group') +def verify(policy): for route in ['route', 'route6']: + ipv6 = route == 'route6' if route in policy: for name, pol_conf in policy[route].items(): if 'rule' in pol_conf: - for rule_id, rule_conf in pol_conf.items(): - icmp = 'icmp' if route == 'route' else 'icmpv6' - if icmp in rule_conf: - icmp_defined = False - if 'type_name' in rule_conf[icmp]: - icmp_defined = True - if 'code' in rule_conf[icmp] or 'type' in rule_conf[icmp]: - raise ConfigError(f'{name} rule {rule_id}: Cannot use ICMP type/code with ICMP type-name') - if 'code' in rule_conf[icmp]: - icmp_defined = True - if 'type' not in rule_conf[icmp]: - raise ConfigError(f'{name} rule {rule_id}: ICMP code can only be defined if ICMP type is defined') - if 'type' in rule_conf[icmp]: - icmp_defined = True - - 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': - raise ConfigError(f'{name} rule {rule_id}: TCP flags can only be set if protocol is set to TCP') + for rule_id, rule_conf in pol_conf['rule'].items(): + verify_rule(policy, rule_conf, ipv6) + + for ifname, if_policy in policy['interfaces'].items(): + name = dict_search_args(if_policy, 'route') + ipv6_name = dict_search_args(if_policy, 'route6') + if name and not dict_search_args(policy, 'route', name): + raise ConfigError(f'Policy route "{name}" is still referenced on interface {ifname}') + + if ipv6_name and not dict_search_args(policy, 'route6', ipv6_name): + raise ConfigError(f'Policy route6 "{ipv6_name}" is still referenced on interface {ifname}') return None -def generate(policy): - if not policy: - if os.path.exists(nftables_conf): - os.unlink(nftables_conf) - return None +def cleanup_commands(policy): + commands = [] + for table in ['ip mangle', 'ip6 mangle']: + json_str = cmd(f'nft -j list table {table}') + obj = loads(json_str) + if 'nftables' not in obj: + continue + for item in obj['nftables']: + if 'chain' in item: + chain = item['chain']['name'] + if not chain.startswith("VYOS_PBR"): + continue + if chain not in preserve_chains: + if table == 'ip mangle' and dict_search_args(policy, 'route', chain.replace("VYOS_PBR_", "", 1)): + commands.append(f'flush chain {table} {chain}') + elif table == 'ip6 mangle' and dict_search_args(policy, 'route6', chain.replace("VYOS_PBR6_", "", 1)): + commands.append(f'flush chain {table} {chain}') + else: + commands.append(f'delete chain {table} {chain}') + return commands +def generate(policy): if not os.path.exists(nftables_conf): policy['first_install'] = True + else: + policy['cleanup_commands'] = cleanup_commands(policy) render(nftables_conf, 'firewall/nftables-policy.tmpl', policy) return None @@ -124,14 +209,6 @@ def cleanup_table_marks(): cmd(f'ip rule del fwmark {fwmark} table {table}') def apply(policy): - if not policy or 'first_install' not in policy: - run(f'nft flush table ip mangle') - run(f'nft flush table ip6 mangle') - - if not policy: - cleanup_table_marks() - return None - install_result = run(f'nft -f {nftables_conf}') if install_result == 1: raise ConfigError('Failed to apply policy based routing') diff --git a/src/migration-scripts/policy/1-to-2 b/src/migration-scripts/policy/1-to-2 index 3e46227de..7ffceef22 100755 --- a/src/migration-scripts/policy/1-to-2 +++ b/src/migration-scripts/policy/1-to-2 @@ -41,6 +41,24 @@ if not config.exists(base): config.rename(base, 'route6') config.set_tag(['policy', 'route6']) +if config.exists(['interfaces']): + def if_policy_rename(config, path): + if config.exists(path + ['policy', 'ipv6-route']): + config.rename(path + ['policy', 'ipv6-route'], 'route6') + + for if_type in config.list_nodes(['interfaces']): + for ifname in config.list_nodes(['interfaces', if_type]): + if_path = ['interfaces', if_type, ifname] + if_policy_rename(config, if_path) + + for vif_type in ['vif', 'vif-s']: + if config.exists(if_path + [vif_type]): + for vifname in config.list_nodes(if_path + [vif_type]): + if_policy_rename(config, if_path + [vif_type, vifname]) + + if config.exists(if_path + [vif_type, vifname, 'vif-c']): + for vifcname in config.list_nodes(if_path + [vif_type, vifname, 'vif-c']): + if_policy_rename(config, if_path + [vif_type, vifname, 'vif-c', vifcname]) try: with open(file_name, 'w') as f: f.write(config.to_string()) diff --git a/src/op_mode/policy_route.py b/src/op_mode/policy_route.py index e0b4ac514..95a7eadac 100755 --- a/src/op_mode/policy_route.py +++ b/src/op_mode/policy_route.py @@ -26,7 +26,7 @@ def get_policy_interfaces(conf, policy, name=None, ipv6=False): interfaces = conf.get_config_dict(['interfaces'], key_mangling=('-', '_'), get_first_key=True, no_tag_node_value_mangle=True) - routes = ['route', 'ipv6_route'] + routes = ['route', 'route6'] def parse_if(ifname, if_conf): if 'policy' in if_conf: @@ -52,7 +52,7 @@ def get_policy_interfaces(conf, policy, name=None, ipv6=False): def get_config_policy(conf, name=None, ipv6=False, interfaces=True): config_path = ['policy'] if name: - config_path += ['ipv6-route' if ipv6 else 'route', name] + config_path += ['route6' if ipv6 else 'route', name] policy = conf.get_config_dict(config_path, key_mangling=('-', '_'), get_first_key=True, no_tag_node_value_mangle=True) @@ -64,7 +64,7 @@ def get_config_policy(conf, name=None, ipv6=False, interfaces=True): for route_name, route_conf in policy['route'].items(): route_conf['interface'] = [] - if 'ipv6_route' in policy: + if 'route6' in policy: for route_name, route_conf in policy['ipv6_route'].items(): route_conf['interface'] = [] @@ -151,8 +151,8 @@ def show_policy(ipv6=False): for route, route_conf in policy['route'].items(): output_policy_route(route, route_conf, ipv6=False) - if ipv6 and 'ipv6_route' in policy: - for route, route_conf in policy['ipv6_route'].items(): + if ipv6 and 'route6' in policy: + for route, route_conf in policy['route6'].items(): output_policy_route(route, route_conf, ipv6=True) def show_policy_name(name, ipv6=False): -- 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 'src/migration-scripts/policy/1-to-2') 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