From 9791258d7d5320d3a8bfa45d43b59fd35e8a2131 Mon Sep 17 00:00:00 2001
From: sarthurdev <965089+sarthurdev@users.noreply.github.com>
Date: Fri, 10 Jun 2022 16:57:21 +0200
Subject: firewall: T478: Add support for nesting groups
---
data/templates/firewall/nftables-defines.j2 | 32 ++++++++++-------
interface-definitions/firewall.xml.in | 54 +++++++++++++++++++++++++++++
python/vyos/template.py | 33 ++++++++++++++++++
smoketest/scripts/cli/test_firewall.py | 39 +++++++++++++++++++++
src/conf_mode/firewall.py | 29 ++++++++++++++++
5 files changed, 174 insertions(+), 13 deletions(-)
diff --git a/data/templates/firewall/nftables-defines.j2 b/data/templates/firewall/nftables-defines.j2
index 4fa92f2e3..12146879d 100644
--- a/data/templates/firewall/nftables-defines.j2
+++ b/data/templates/firewall/nftables-defines.j2
@@ -1,32 +1,38 @@
{% if group is vyos_defined %}
{% if group.address_group is vyos_defined %}
-{% for group_name, group_conf in group.address_group.items() %}
-define A_{{ group_name }} = { {{ group_conf.address | join(",") }} }
+{% for group_name, group_conf in group.address_group | sort_nested_groups %}
+{% set includes = group_conf.include if group_conf.include is vyos_defined else [] %}
+define A_{{ group_name }} = { {{ group_conf.address | nft_nested_group(includes, 'A_') | join(",") }} }
{% endfor %}
{% endif %}
{% if group.ipv6_address_group is vyos_defined %}
-{% for group_name, group_conf in group.ipv6_address_group.items() %}
-define A6_{{ group_name }} = { {{ group_conf.address | join(",") }} }
+{% for group_name, group_conf in group.ipv6_address_group | sort_nested_groups %}
+{% set includes = group_conf.include if group_conf.include is vyos_defined else [] %}
+define A6_{{ group_name }} = { {{ group_conf.address | nft_nested_group(includes, 'A6_') | join(",") }} }
{% endfor %}
{% endif %}
{% if group.mac_group is vyos_defined %}
-{% for group_name, group_conf in group.mac_group.items() %}
-define M_{{ group_name }} = { {{ group_conf.mac_address | join(",") }} }
+{% for group_name, group_conf in group.mac_group | sort_nested_groups %}
+{% set includes = group_conf.include if group_conf.include is vyos_defined else [] %}
+define M_{{ group_name }} = { {{ group_conf.mac_address | nft_nested_group(includes, 'M_') | join(",") }} }
{% endfor %}
{% endif %}
{% if group.network_group is vyos_defined %}
-{% for group_name, group_conf in group.network_group.items() %}
-define N_{{ group_name }} = { {{ group_conf.network | join(",") }} }
+{% for group_name, group_conf in group.network_group | sort_nested_groups %}
+{% set includes = group_conf.include if group_conf.include is vyos_defined else [] %}
+define N_{{ group_name }} = { {{ group_conf.network | nft_nested_group(includes, 'N_') | join(",") }} }
{% endfor %}
{% endif %}
{% if group.ipv6_network_group is vyos_defined %}
-{% for group_name, group_conf in group.ipv6_network_group.items() %}
-define N6_{{ group_name }} = { {{ group_conf.network | join(",") }} }
+{% for group_name, group_conf in group.ipv6_network_group | sort_nested_groups %}
+{% set includes = group_conf.include if group_conf.include is vyos_defined else [] %}
+define N6_{{ group_name }} = { {{ group_conf.network | nft_nested_group(includes, 'N6_') | join(",") }} }
{% endfor %}
{% endif %}
{% if group.port_group is vyos_defined %}
-{% for group_name, group_conf in group.port_group.items() %}
-define P_{{ group_name }} = { {{ group_conf.port | join(",") }} }
+{% for group_name, group_conf in group.port_group | sort_nested_groups %}
+{% set includes = group_conf.include if group_conf.include is vyos_defined else [] %}
+define P_{{ group_name }} = { {{ group_conf.port | nft_nested_group(includes, 'P_') | join(",") }} }
{% endfor %}
{% endif %}
-{% endif %}
\ No newline at end of file
+{% endif %}
diff --git a/interface-definitions/firewall.xml.in b/interface-definitions/firewall.xml.in
index 63095bc20..6ab11c790 100644
--- a/interface-definitions/firewall.xml.in
+++ b/interface-definitions/firewall.xml.in
@@ -97,6 +97,15 @@
+
+
+ Include another address-group
+
+ firewall group address-group
+
+
+
+
#include
@@ -151,6 +160,15 @@
+
+
+ Include another ipv6-address-group
+
+ firewall group ipv6-address-group
+
+
+
+
#include
@@ -176,6 +194,15 @@
+
+
+ Include another ipv6-network-group
+
+ firewall group ipv6-network-group
+
+
+
+
@@ -200,6 +227,15 @@
+
+
+ Include another mac-group
+
+ firewall group mac-group
+
+
+
+
@@ -224,6 +260,15 @@
+
+
+ Include another network-group
+
+ firewall group network-group
+
+
+
+
@@ -256,6 +301,15 @@
+
+
+ Include another port-group
+
+ firewall group port-group
+
+
+
+
diff --git a/python/vyos/template.py b/python/vyos/template.py
index ee82f8f8f..3feda47c8 100644
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -591,6 +591,39 @@ def nft_intra_zone_action(zone_conf, ipv6=False):
return f'jump {name_prefix}{name}'
return 'return'
+@register_filter('nft_nested_group')
+def nft_nested_group(out_list, includes, prefix):
+ if not vyos_defined(out_list):
+ out_list = []
+ for name in includes:
+ out_list.append(f'${prefix}{name}')
+ return out_list
+
+@register_filter('sort_nested_groups')
+def sort_nested_groups(groups):
+ seen = []
+ out = {}
+
+ def include_iterate(group_name):
+ group = groups[group_name]
+ if 'include' not in group:
+ if group_name not in out:
+ out[group_name] = groups[group_name]
+ return
+
+ for inc_group_name in group['include']:
+ if inc_group_name not in seen:
+ seen.append(inc_group_name)
+ include_iterate(inc_group_name)
+
+ if group_name not in out:
+ out[group_name] = groups[group_name]
+
+ for group_name in groups:
+ include_iterate(group_name)
+
+ return out.items()
+
@register_test('vyos_defined')
def vyos_defined(value, test_value=None, var_type=None):
"""
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index b1fd663d2..2462e9a6a 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -20,6 +20,7 @@ from glob import glob
from base_vyostest_shim import VyOSUnitTestSHIM
+from vyos.configsession import ConfigSessionError
from vyos.util import cmd
sysfs_config = {
@@ -108,6 +109,44 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
self.cli_delete(['system', 'static-host-mapping'])
self.cli_commit()
+ def test_nested_groups(self):
+ self.cli_set(['firewall', 'group', 'network-group', 'smoketest_network', 'network', '172.16.99.0/24'])
+ self.cli_set(['firewall', 'group', 'network-group', 'smoketest_network1', 'network', '172.16.101.0/24'])
+ self.cli_set(['firewall', 'group', 'network-group', 'smoketest_network1', 'include', 'smoketest_network'])
+ self.cli_set(['firewall', 'group', 'port-group', 'smoketest_port', 'port', '53'])
+ self.cli_set(['firewall', 'group', 'port-group', 'smoketest_port1', 'port', '123'])
+ self.cli_set(['firewall', 'group', 'port-group', 'smoketest_port1', 'include', 'smoketest_port'])
+ self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'action', 'accept'])
+ self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'source', 'group', 'network-group', 'smoketest_network1'])
+ self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'destination', 'group', 'port-group', 'smoketest_port1'])
+ self.cli_set(['firewall', 'name', 'smoketest', 'rule', '1', 'protocol', 'tcp_udp'])
+
+ self.cli_set(['interfaces', 'ethernet', 'eth0', 'firewall', 'in', 'name', 'smoketest'])
+
+ self.cli_commit()
+
+ # Test circular includes
+ self.cli_set(['firewall', 'group', 'network-group', 'smoketest_network', 'include', 'smoketest_network1'])
+ with self.assertRaises(ConfigSessionError):
+ self.cli_commit()
+
+ self.cli_delete(['firewall', 'group', 'network-group', 'smoketest_network', 'include', 'smoketest_network1'])
+
+ nftables_search = [
+ ['iifname "eth0"', 'jump NAME_smoketest'],
+ ['ip saddr { 172.16.99.0/24, 172.16.101.0/24 }', 'th dport { 53, 123 }', 'return']
+ ]
+
+ nftables_output = cmd('sudo nft list table ip filter')
+
+ for search in nftables_search:
+ matched = False
+ for line in nftables_output.split("\n"):
+ if all(item in line for item in search):
+ matched = True
+ break
+ self.assertTrue(matched, msg=search)
+
def test_basic_rules(self):
self.cli_set(['firewall', 'name', 'smoketest', 'default-action', 'drop'])
self.cli_set(['firewall', 'name', 'smoketest', 'enable-default-log'])
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index 335098bf1..82a51f4af 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -85,10 +85,16 @@ nft6_iface_chains = ['VYOS_FW6_FORWARD', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']
valid_groups = [
'address_group',
+ 'domain_group',
'network_group',
'port_group'
]
+group_types = [
+ 'address_group', 'network_group', 'port_group',
+ 'ipv6_address_group', 'ipv6_network_group'
+]
+
snmp_change_type = {
'unknown': 0,
'add': 1,
@@ -241,11 +247,34 @@ def verify_rule(firewall, rule_conf, ipv6):
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_nested_group(group_name, group, groups, seen):
+ if 'include' not in group:
+ return
+
+ for g in group['include']:
+ if g not in groups:
+ raise ConfigError(f'Nested group "{g}" does not exist')
+
+ if g in seen:
+ raise ConfigError(f'Group "{group_name}" has a circular reference')
+
+ seen.append(g)
+
+ if 'include' in groups[g]:
+ verify_nested_group(g, groups[g], groups, seen)
+
def verify(firewall):
if 'config_trap' in firewall and firewall['config_trap'] == 'enable':
if not firewall['trap_targets']:
raise ConfigError(f'Firewall config-trap enabled but "service snmp trap-target" is not defined')
+ if 'group' in firewall:
+ for group_type in group_types:
+ if group_type in firewall['group']:
+ groups = firewall['group'][group_type]
+ for group_name, group in groups.items():
+ verify_nested_group(group_name, group, groups, [])
+
for name in ['name', 'ipv6_name']:
if name in firewall:
for name_id, name_conf in firewall[name].items():
--
cgit v1.2.3