From 22791e26f444766dc9f9e1729b72893208f58079 Mon Sep 17 00:00:00 2001 From: zsdc Date: Mon, 12 Jul 2021 22:59:48 +0300 Subject: VRF: T3655: proper connection tracking for VRFs Currently, all VRFs share the same connection tracking table, which can lead to problems: - traffic leaks to a wrong VRF - improper NAT rules handling when multiple VRFs contain the same IP networks - stateful firewall rules issues The commit implements connection tracking zones support. Each VRF utilizes its own zone, so connections will never mix up. It also adds some restrictions to VRF names and assigned table numbers, because of nftables and conntrack requirements: - VRF name should always start from a letter (interfaces that start from numbers are not supported in nftables rules) - table number must be in the 100-65535 range because conntrack supports only 65535 zones --- data/templates/firewall/nftables-vrf-zones.tmpl | 17 +++ interface-definitions/vrf.xml.in | 8 +- python/vyos/ifconfig/interface.py | 23 ++++ src/conf_mode/vrf.py | 24 ++++ src/migration-scripts/vrf/2-to-3 | 144 ++++++++++++++++++++++++ src/validators/vrf-name | 4 +- 6 files changed, 214 insertions(+), 6 deletions(-) create mode 100644 data/templates/firewall/nftables-vrf-zones.tmpl mode change 100644 => 100755 python/vyos/ifconfig/interface.py create mode 100755 src/migration-scripts/vrf/2-to-3 diff --git a/data/templates/firewall/nftables-vrf-zones.tmpl b/data/templates/firewall/nftables-vrf-zones.tmpl new file mode 100644 index 000000000..eecf47b78 --- /dev/null +++ b/data/templates/firewall/nftables-vrf-zones.tmpl @@ -0,0 +1,17 @@ +table inet vrf_zones { + # Map of interfaces and connections tracking zones + map ct_iface_map { + typeof iifname : ct zone + } + # Assign unique zones for each VRF + # Chain for inbound traffic + chain vrf_zones_ct_in { + type filter hook prerouting priority raw; policy accept; + counter ct zone set iifname map @ct_iface_map + } + # Chain for locally-generated traffic + chain vrf_zones_ct_out { + type filter hook output priority raw; policy accept; + counter ct zone set oifname map @ct_iface_map + } +} diff --git a/interface-definitions/vrf.xml.in b/interface-definitions/vrf.xml.in index 426884a11..9d513945c 100644 --- a/interface-definitions/vrf.xml.in +++ b/interface-definitions/vrf.xml.in @@ -19,7 +19,7 @@ - VRF instance name must be 15 characters or less and can not\nbe named as regular network interfaces.\n + VRF instance name must be 15 characters or less and can not\nbe named as regular network interfaces.\nA name must starts from a letter.\n txt VRF instance name @@ -76,13 +76,13 @@ Routing table associated with this instance - 100-2147483647 + 100-65535 Routing table ID - + - VRF routing table must be in range from 100 to 2147483647 + VRF routing table must be in range from 100 to 65535 #include diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py old mode 100644 new mode 100755 index 08b7af90b..a1928ba51 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -311,6 +311,28 @@ class Interface(Control): cmd = 'ip link del dev {ifname}'.format(**self.config) return self._cmd(cmd) + def _set_vrf_ct_zone(self, vrf): + """ + Add/Remove rules in nftables to associate traffic in VRF to an + individual conntack zone + """ + if vrf: + # Get routing table ID for VRF + vrf_table_id = get_interface_config(vrf).get('linkinfo', {}).get( + 'info_data', {}).get('table') + # Add map element with interface and zone ID + if vrf_table_id: + self._cmd( + f'nft add element inet vrf_zones ct_iface_map {{ "{self.ifname}" : {vrf_table_id} }}' + ) + else: + nft_del_element = f'delete element inet vrf_zones ct_iface_map {{ "{self.ifname}" }}' + # Check if deleting is possible first to avoid raising errors + _, err = self._popen(f'nft -c {nft_del_element}') + if not err: + # Remove map element + self._cmd(f'nft {nft_del_element}') + def get_min_mtu(self): """ Get hardware minimum supported MTU @@ -401,6 +423,7 @@ class Interface(Control): >>> Interface('eth0').set_vrf() """ self.set_interface('vrf', vrf) + self._set_vrf_ct_zone(vrf) def set_arp_cache_tmo(self, tmo): """ diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py index 936561edc..fbfce646f 100755 --- a/src/conf_mode/vrf.py +++ b/src/conf_mode/vrf.py @@ -18,6 +18,7 @@ import os from sys import exit from json import loads +from tempfile import NamedTemporaryFile from vyos.config import Config from vyos.configdict import node_changed @@ -26,6 +27,7 @@ from vyos.template import render from vyos.template import render_to_string from vyos.util import call from vyos.util import cmd +from vyos.util import popen from vyos.util import dict_search from vyos.util import get_interface_config from vyos import ConfigError @@ -125,11 +127,17 @@ def verify(vrf): return None + def generate(vrf): render(config_file, 'vrf/vrf.conf.tmpl', vrf) vrf['new_frr_config'] = render_to_string('frr/vrf.frr.tmpl', vrf) + # Render nftables zones config + vrf['nft_vrf_zones'] = NamedTemporaryFile().name + render(vrf['nft_vrf_zones'], 'firewall/nftables-vrf-zones.tmpl', vrf) + return None + def apply(vrf): # Documentation # @@ -151,8 +159,19 @@ def apply(vrf): call(f'ip -4 route del vrf {tmp} unreachable default metric 4278198272') call(f'ip -6 route del vrf {tmp} unreachable default metric 4278198272') call(f'ip link delete dev {tmp}') + # Remove nftables conntrack zone map item + nft_del_element = f'delete element inet vrf_zones ct_iface_map {{ "{tmp}" }}' + cmd(f'nft {nft_del_element}') if 'name' in vrf: + # Separate VRFs in conntrack table + # check if table already exists + _, err = popen('nft list table inet vrf_zones') + # If not, create a table + if err: + cmd(f'nft -f {vrf["nft_vrf_zones"]}') + os.unlink(vrf['nft_vrf_zones']) + for name, config in vrf['name'].items(): table = config['table'] @@ -182,6 +201,9 @@ def apply(vrf): # reconfiguration. state = 'down' if 'disable' in config else 'up' vrf_if.set_admin_state(state) + # Add nftables conntrack zone map item + nft_add_element = f'add element inet vrf_zones ct_iface_map {{ "{name}" : {table} }}' + cmd(f'nft {nft_add_element}') # Linux routing uses rules to find tables - routing targets are then # looked up in those tables. If the lookup got a matching route, the @@ -214,6 +236,8 @@ def apply(vrf): # clean out l3mdev-table rule if present if 1000 in [r.get('priority') for r in list_rules() if r.get('priority') == 1000]: call(f'ip {af} rule del pref 1000') + # Remove VRF zones table from nftables + cmd('nft delete table inet vrf_zones') # add configuration to FRR frr_cfg = frr.FRRConfig() diff --git a/src/migration-scripts/vrf/2-to-3 b/src/migration-scripts/vrf/2-to-3 new file mode 100755 index 000000000..8e0f97141 --- /dev/null +++ b/src/migration-scripts/vrf/2-to-3 @@ -0,0 +1,144 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2021 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 . + +# Since connection tracking zones are int16, VRFs tables maximum value must +# be limited to 65535 +# Also, interface names in nftables cannot start from numbers, +# so VRF name should not start from a number + +from sys import argv +from sys import exit +from random import randrange +from random import choice +from string import ascii_lowercase +from vyos.configtree import ConfigTree +import re + + +# Helper function to find all config items with a VRF name +def _search_vrfs(config_commands, vrf_name): + vrf_values = [] + # Regex to find path of config command with old VRF + regex_filter = re.compile(rf'^set (?P[^\']+vrf) \'{vrf_name}\'$') + # Check each command for VRF value + for config_command in config_commands: + search_result = regex_filter.search(config_command) + if search_result: + # Append VRF command to a list + vrf_values.append(search_result.group('cmd_path').split()) + if vrf_values: + return vrf_values + else: + return None + + +# Helper function to find all config items with a table number +def _search_tables(config_commands, table_num): + table_items = {'table_tags': [], 'table_values': []} + # Regex to find values and nodes with a table number + regex_tags = re.compile(rf'^set (?P[^\']+table {table_num}) ?.*$') + regex_values = re.compile( + rf'^set (?P[^\']+table) \'{table_num}\'$') + for config_command in config_commands: + # Search for tag nodes + search_result = regex_tags.search(config_command) + if search_result: + # Append table node path to a tag nodes list + cmd_path = search_result.group('cmd_path').split() + if cmd_path not in table_items['table_tags']: + table_items['table_tags'].append(cmd_path) + # Search for value nodes + search_result = regex_values.search(config_command) + if search_result: + # Append table node path to a value nodes list + table_items['table_values'].append( + search_result.group('cmd_path').split()) + return table_items + + +if (len(argv) < 2): + print("Must specify file name!") + exit(1) + +file_name = argv[1] + +with open(file_name, 'r') as f: + config_file = f.read() + +base = ['vrf', 'name'] +config = ConfigTree(config_file) + +if not config.exists(base): + # Nothing to do + exit(0) + +# Get a list of all currently used VRFs and tables +vrfs_current = {} +for vrf in config.list_nodes(base): + vrfs_current[vrf] = int(config.return_value(base + [vrf, 'table'])) + +# Check VRF names and table numbers +name_regex = re.compile(r'^\d.*$') +for vrf_name, vrf_table in vrfs_current.items(): + # Check table number + if vrf_table > 65535: + # Find new unused table number + vrfs_current[vrf_name] = None + while not vrfs_current[vrf_name]: + table_random = randrange(100, 65535) + if table_random not in vrfs_current.values(): + vrfs_current[vrf_name] = table_random + # Update number to a new one + config.set(['vrf', 'name', vrf_name, 'table'], + vrfs_current[vrf_name], + replace=True) + # Check config items with old table number and replace to new one + config_commands = config.to_commands().split('\n') + table_config_lines = _search_tables(config_commands, vrf_table) + # Rename table nodes + if table_config_lines.get('table_tags'): + for table_config_path in table_config_lines.get('table_tags'): + config.rename(table_config_path, f'{vrfs_current[vrf_name]}') + # Replace table values + if table_config_lines.get('table_values'): + for table_config_path in table_config_lines.get('table_values'): + config.set(table_config_path, + f'{vrfs_current[vrf_name]}', + replace=True) + + # Check VRF name + if name_regex.match(vrf_name): + vrf_name_new = None + while not vrf_name_new: + vrf_name_rand = f'{choice(ascii_lowercase)}{vrf_name}'[:15] + if vrf_name_rand not in vrfs_current: + vrf_name_new = vrf_name_rand + # Update VRF name to a new one + config.rename(['vrf', 'name', vrf_name], vrf_name_new) + # Check config items with old VRF name and replace to new one + config_commands = config.to_commands().split('\n') + vrf_config_lines = _search_vrfs(config_commands, vrf_name) + # Rename VRF to a new name + if vrf_config_lines: + for vrf_value_path in vrf_config_lines: + config.set(vrf_value_path, vrf_name_new, replace=True) + +try: + with open(file_name, 'w') as f: + f.write(config.to_string()) +except OSError as e: + print("Failed to save the modified config: {}".format(e)) + exit(1) diff --git a/src/validators/vrf-name b/src/validators/vrf-name index c78a80776..29167c635 100755 --- a/src/validators/vrf-name +++ b/src/validators/vrf-name @@ -33,8 +33,8 @@ if __name__ == '__main__': if vrf == "lo": exit(1) - pattern = "^(?!(bond|br|dum|eth|lan|eno|ens|enp|enx|gnv|ipoe|l2tp|l2tpeth|" \ - "vtun|ppp|pppoe|peth|tun|vti|vxlan|wg|wlan|wwan)\d+(\.\d+(v.+)?)?$).*$" + pattern = r'^(?!(bond|br|dum|eth|lan|eno|ens|enp|enx|gnv|ipoe|l2tp|l2tpeth|\ + vtun|ppp|pppoe|peth|tun|vti|vxlan|wg|wlan|wwan|\d)\d*(\.\d+)?(v.+)?).*$' if not re.match(pattern, vrf): exit(1) -- cgit v1.2.3