From: Jo-Philipp Wich <jo@mein.io>
Date: Wed, 26 Jan 2022 22:09:01 +0000 (+0100)
Subject: ruleset: consolidate zone matches for raw_prerouting and raw_output chains
X-Git-Url: http://git.lede-project.org./?a=commitdiff_plain;h=f85bb2d21d04c4579509988ff47c270dc30e68e2;p=project%2Ffirewall4.git

ruleset: consolidate zone matches for raw_prerouting and raw_output chains

Instead of duplicating the zone match rule expressions, reuse the
`zone-match.uc` template emit the correct match expressions.

This simplifies the code somewhat and ensures that wildcard interfaces
are also properly handled for notrack/helper rules.

Signed-off-by: Jo-Philipp Wich <jo@mein.io>
---

diff --git a/root/usr/share/firewall4/templates/ruleset.uc b/root/usr/share/firewall4/templates/ruleset.uc
index 4e519bf..2a0a8a8 100644
--- a/root/usr/share/firewall4/templates/ruleset.uc
+++ b/root/usr/share/firewall4/templates/ruleset.uc
@@ -278,19 +278,12 @@ table inet fw4 {
 {%  for (let zone in fw4.zones()): %}
 {%   if (zone.dflags[target]): %}
 {%    for (let rule in zone.match_rules): %}
-{%     let devs = fw4.filter_loopback_devs(rule.devices_pos, false); %}
-{%     let nets = fw4.filter_loopback_addrs(rule.subnets_pos, false); %}
-{%     if (rule.devices_neg || rule.subnets_neg || length(devs) || length(nets)): %}
+{%     let devices_pos = fw4.filter_loopback_devs(rule.devices_pos, false); %}
+{%     let subnets_pos = fw4.filter_loopback_addrs(rule.subnets_pos, false); %}
+{%     if (rule.devices_neg || rule.subnets_neg || devices_pos || subnets_pos): %}
 		{%+ if (rule.family): -%}
 			meta nfproto {{ fw4.nfproto(rule.family) }} {%+ endif -%}
-		{%+ if (length(devs)): -%}
-			iifname {{ fw4.set(devs) }} {%+ endif -%}
-		{%+ if (rule.devices_neg): -%}
-			iifname != {{ fw4.set(rule.devices_neg) }} {%+ endif -%}
-		{%+ if (length(nets)): -%}
-			{{ fw4.ipproto(rule.family) }} saddr {{ fw4.set(nets) }} {%+ endif -%}
-		{%+ if (rule.subnets_neg): -%}
-			{{ fw4.ipproto(rule.family) }} saddr != {{ fw4.set(rule.subnets_neg) }} {%+ endif -%}
+		{%+ include("zone-match.uc", { fw4, rule: { ...rule, devices_pos, subnets_pos } }) -%}
 		jump {{ target }}_{{ zone.name }} comment "!fw4: {{ zone.name }} {{ fw4.nfproto(rule.family, true) }} {{
 			(target == "helper") ? "CT helper assignment" : "CT bypass"
 		}}"
@@ -307,15 +300,12 @@ table inet fw4 {
 {%  for (let zone in fw4.zones()): %}
 {%   if (zone.dflags[target]): %}
 {%    for (let rule in zone.match_rules): %}
-{%     let devs = fw4.filter_loopback_devs(rule.devices_pos, true); %}
-{%     let nets = fw4.filter_loopback_addrs(rule.subnets_pos, true); %}
-{%     if (length(devs) || length(nets)): %}
+{%     let devices_pos = fw4.filter_loopback_devs(rule.devices_pos, true); %}
+{%     let subnets_pos = fw4.filter_loopback_addrs(rule.subnets_pos, true); %}
+{%     if (devices_pos || subnets_pos): %}
 		{%+ if (rule.family): -%}
 			meta nfproto {{ fw4.nfproto(rule.family) }} {%+ endif -%}
-		{%+ if (length(devs)): -%}
-			iifname {{ fw4.set(devs) }} {%+ endif -%}
-		{%+ if (length(nets)): -%}
-			{{ fw4.ipproto(rule.family) }} saddr {{ fw4.set(nets) }} {%+ endif -%}
+		{%+ include("zone-match.uc", { fw4, rule: { ...rule, devices_pos, subnets_pos } }) -%}
 		jump {{ target }}_{{ zone.name }} comment "!fw4: {{ zone.name }} {{ fw4.nfproto(rule.family, true) }} {{
 			(target == "helper") ? "CT helper assignment" : "CT bypass"
 		}}"
diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 8564ca5..b84983d 100644
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -1498,13 +1498,13 @@ return {
 	},
 
 	filter_loopback_devs: function(devs, invert) {
-		let self = this;
-		return filter(devs, d => (self.is_loopback_dev(d) == invert));
+		let rv = filter(devs, d => (this.is_loopback_dev(d) == invert));
+		return length(rv) ? rv : null;
 	},
 
 	filter_loopback_addrs: function(addrs, invert) {
-		let self = this;
-		return filter(addrs, a => (self.is_loopback_addr(a) == invert));
+		let rv = filter(addrs, a => (this.is_loopback_addr(a) == invert));
+		return length(rv) ? rv : null;
 	},
 
 
diff --git a/tests/02_zones/04_wildcard_devices b/tests/02_zones/04_wildcard_devices
index aea5853..ad38734 100644
--- a/tests/02_zones/04_wildcard_devices
+++ b/tests/02_zones/04_wildcard_devices
@@ -299,9 +299,9 @@ table inet fw4 {
 		iifname "foo*" jump helper_test4 comment "!fw4: test4 IPv4/IPv6 CT helper assignment"
 		iifname "bar*" jump helper_test4 comment "!fw4: test4 IPv4/IPv6 CT helper assignment"
 		iifname { "test1", "test2" } jump helper_test4 comment "!fw4: test4 IPv4/IPv6 CT helper assignment"
-		iifname "foo*" iifname != { "test3", "test4" } jump helper_test5 comment "!fw4: test5 IPv4/IPv6 CT helper assignment"
-		iifname "bar*" iifname != { "test3", "test4" } jump helper_test5 comment "!fw4: test5 IPv4/IPv6 CT helper assignment"
-		iifname { "test1", "test2" } iifname != { "test3", "test4" } jump helper_test5 comment "!fw4: test5 IPv4/IPv6 CT helper assignment"
+		iifname "foo*" iifname != { "test3", "test4" } iifname != "baz*" iifname != "qrx*" jump helper_test5 comment "!fw4: test5 IPv4/IPv6 CT helper assignment"
+		iifname "bar*" iifname != { "test3", "test4" } iifname != "baz*" iifname != "qrx*" jump helper_test5 comment "!fw4: test5 IPv4/IPv6 CT helper assignment"
+		iifname { "test1", "test2" } iifname != { "test3", "test4" } iifname != "baz*" iifname != "qrx*" jump helper_test5 comment "!fw4: test5 IPv4/IPv6 CT helper assignment"
 	}
 
 	chain raw_output {