From f16525175deb69ae3b9193573550992b4d5fd951 Mon Sep 17 00:00:00 2001
From: sarthurdev <965089+sarthurdev@users.noreply.github.com>
Date: Mon, 10 Jan 2022 23:14:28 +0100
Subject: firewall: policy: T4159: T4164: Fix empty firewall groups, create
separate file for group definitions.
---
data/templates/firewall/nftables-policy.tmpl | 2 ++
1 file changed, 2 insertions(+)
(limited to 'data/templates/firewall/nftables-policy.tmpl')
diff --git a/data/templates/firewall/nftables-policy.tmpl b/data/templates/firewall/nftables-policy.tmpl
index ecc7e0fbd..668ec7388 100644
--- a/data/templates/firewall/nftables-policy.tmpl
+++ b/data/templates/firewall/nftables-policy.tmpl
@@ -1,5 +1,7 @@
#!/usr/sbin/nft -f
+include "/run/nftables_defines.conf"
+
table ip mangle {
{% if first_install is defined %}
chain VYOS_PBR_PREROUTING {
--
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 'data/templates/firewall/nftables-policy.tmpl')
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