From: Stijn Tintel <stijn@linux-ipv6.be>
Date: Mon, 1 Aug 2022 09:40:14 +0000 (+0300)
Subject: fw4: don't inherit zone family from ct helpers
X-Git-Url: http://git.lede-project.org./?a=commitdiff_plain;h=111a7f780ab26b1377960c6eb47e1e5f169cc178;p=project%2Ffirewall4.git

fw4: don't inherit zone family from ct helpers

It's perfectly valid to use a conntrack helper that only supports a
single address family in a zone where both IPv4 and IPv6 are used.
Restricting a zone to a certain family due to limitations of the
associated conntrack helpers may result in unexpected behaviour, which
in turn may have unintended security implications.

Don't inherit zone family from conntrack helper restrictions to avoid
this and add test coverage.

Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Acked-by: Jo-Philipp Wich <jo@mein.io>
---

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 1b4764c..cdc6e94 100644
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -2006,7 +2006,6 @@ return {
 		};
 
 		let family = infer_family(zone.family, [
-			zone.helper, "ct helper",
 			match_subnets, "subnet list"
 		]);
 
diff --git a/tests/02_zones/06_family_selections b/tests/02_zones/06_family_selections
index a2d48b5..b01a609 100644
--- a/tests/02_zones/06_family_selections
+++ b/tests/02_zones/06_family_selections
@@ -15,7 +15,18 @@ specified or not.
 -- End --
 
 -- File uci/helpers.json --
-{}
+{
+	"helper" : [
+		{
+			"description" : "An example IPv4-only conntrack helper",
+			"family" : "ipv4",
+			"module" : "nf_conntrack_dummy",
+			"name" : "test",
+			"port" : 1234,
+			"proto" : "tcp"
+		}
+	]
+}
 -- End --
 
 -- File uci/firewall.json --
@@ -59,6 +70,14 @@ specified or not.
 			"family": "ipv6",
 			"device": [ "eth0" ],
 			"auto_helper": 0
+		},
+
+		{
+			".description": "Family restrictions of associated ct helpers should not influence zone family selection",
+			"name": "test6",
+			"family": "any",
+			"device": [ "br-lan" ],
+			"helper": [ "test" ]
 		}
 	]
 }
@@ -69,6 +88,15 @@ table inet fw4
 flush table inet fw4
 
 table inet fw4 {
+	#
+	# CT helper definitions
+	#
+
+	ct helper test {
+		type "test" protocol tcp;
+	}
+
+
 	#
 	# Defines
 	#
@@ -88,6 +116,9 @@ table inet fw4 {
 	define test5_devices = { "eth0" }
 	define test5_subnets = {  }
 
+	define test6_devices = { "br-lan" }
+	define test6_subnets = {  }
+
 
 	#
 	# User includes
@@ -111,6 +142,7 @@ table inet fw4 {
 		meta nfproto ipv6 ip6 saddr 2001:db8:1234::/64 jump input_test3 comment "!fw4: Handle test3 IPv6 input traffic"
 		meta nfproto ipv6 ip6 saddr 2001:db8:1234::/64 jump input_test4 comment "!fw4: Handle test4 IPv6 input traffic"
 		meta nfproto ipv6 iifname "eth0" jump input_test5 comment "!fw4: Handle test5 IPv6 input traffic"
+		iifname "br-lan" jump input_test6 comment "!fw4: Handle test6 IPv4/IPv6 input traffic"
 	}
 
 	chain forward {
@@ -122,6 +154,7 @@ table inet fw4 {
 		meta nfproto ipv6 ip6 saddr 2001:db8:1234::/64 jump forward_test3 comment "!fw4: Handle test3 IPv6 forward traffic"
 		meta nfproto ipv6 ip6 saddr 2001:db8:1234::/64 jump forward_test4 comment "!fw4: Handle test4 IPv6 forward traffic"
 		meta nfproto ipv6 iifname "eth0" jump forward_test5 comment "!fw4: Handle test5 IPv6 forward traffic"
+		iifname "br-lan" jump forward_test6 comment "!fw4: Handle test6 IPv4/IPv6 forward traffic"
 	}
 
 	chain output {
@@ -135,10 +168,12 @@ table inet fw4 {
 		meta nfproto ipv6 ip6 daddr 2001:db8:1234::/64 jump output_test3 comment "!fw4: Handle test3 IPv6 output traffic"
 		meta nfproto ipv6 ip6 daddr 2001:db8:1234::/64 jump output_test4 comment "!fw4: Handle test4 IPv6 output traffic"
 		meta nfproto ipv6 oifname "eth0" jump output_test5 comment "!fw4: Handle test5 IPv6 output traffic"
+		oifname "br-lan" jump output_test6 comment "!fw4: Handle test6 IPv4/IPv6 output traffic"
 	}
 
 	chain prerouting {
 		type filter hook prerouting priority filter; policy accept;
+		iifname "br-lan" jump helper_test6 comment "!fw4: Handle test6 IPv4/IPv6 helper assignment"
 	}
 
 	chain handle_reject {
@@ -246,6 +281,30 @@ table inet fw4 {
 		meta nfproto ipv6 oifname "eth0" counter drop comment "!fw4: drop test5 IPv6 traffic"
 	}
 
+	chain input_test6 {
+		jump drop_from_test6
+	}
+
+	chain output_test6 {
+		jump drop_to_test6
+	}
+
+	chain forward_test6 {
+		jump drop_to_test6
+	}
+
+	chain helper_test6 {
+		meta nfproto ipv4 meta l4proto tcp tcp dport 1234 ct helper set "test" comment "!fw4: An example IPv4-only conntrack helper"
+	}
+
+	chain drop_from_test6 {
+		iifname "br-lan" counter drop comment "!fw4: drop test6 IPv4/IPv6 traffic"
+	}
+
+	chain drop_to_test6 {
+		oifname "br-lan" counter drop comment "!fw4: drop test6 IPv4/IPv6 traffic"
+	}
+
 
 	#
 	# NAT rules