fw4: fix family auto-selection for config nat rules

 - Ensure that a nat rule without any AF specifics and no explictly set
   family ends up being IPv4 only

 - Ensure that an IPv6 address without explicitly set family results in
   an IPv6 rule

 - Ensure that explicitly setting family any results in a IPv4/IPv6 rule

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

2 files changed, 74 insertions, 16 deletions

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 28865cd..05e8646 100644
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -2881,7 +2881,7 @@ return {
 			enabled: [ "bool", "1" ],
 
 			name: [ "string", this.section_id(data[".name"]) ],
-			family: [ "family", "4" ],
+			family: [ "family" ],
 
 			src: [ "zone_ref" ],
 			device: [ "string" ],
@@ -2956,9 +2956,6 @@ return {
 			return;
 		}
 
-		if (snat.src && snat.src.zone)
-			snat.src.zone.dflags.snat = true;
-
 		let add_rule = (family, proto, saddrs, daddrs, raddrs, sport, dport, rport, snat) => {
 			let n = {
 				...snat,
@@ -3008,19 +3005,21 @@ return {
 			if (length(rip[0]) > 1 || length(rip[1]) > 1)
 				this.warn_section(data, "specifies multiple rewrite addresses, using only first one");
 
-			/* inherit family restrictions from related zones */
-			if (family === 0 || family === null) {
-				let f = (rule.src && rule.src.zone) ? rule.src.zone.family : 0;
-
-				if (f) {
-					this.warn_section(r,
-						sprintf("inheriting %s restriction from src %s",
-						        this.nfproto(f1, true), rule.src.zone.name));
+			family = infer_family(family, [
+				sip, "source IP",
+				dip, "destination IP",
+				rip, "rewrite IP",
+				snat.src?.zone, "source zone"
+			]);
 
-					family = f;
-				}
+			if (type(family) == "string") {
+				this.warn_section(data, family + ", skipping");
+				continue;
 			}
 
+			if (snat.src?.zone)
+				snat.src.zone.dflags.snat = true;
+
 			/* if no family was configured, infer target family from IP addresses */
 			if (family === null) {
 				if ((length(sip[0]) || length(dip[0]) || length(rip[0])) && !length(sip[1]) && !length(dip[1]) && !length(rip[1]))
@@ -3028,7 +3027,7 @@ return {
 				else if ((length(sip[1]) || length(dip[1]) || length(rip[1])) && !length(sip[0]) && !length(dip[0]) && !length(rip[0]))
 					family = 6;
 				else
-					family = 0;
+					family = 4; /* default to IPv4 only for backwards compatibility, unless an explict family any was configured */
 			}
 
 			/* check if there's no AF specific bits, in this case we can do an AF agnostic rule */
diff --git a/tests/03_rules/08_family_inheritance b/tests/03_rules/08_family_inheritance
index 9a6aa59..a1fd39f 100644
--- a/tests/03_rules/08_family_inheritance
+++ b/tests/03_rules/08_family_inheritance
@@ -88,7 +88,7 @@ Testing various option constraints.
 	],
 	"redirect": [
 		{
-			".description": "Redirects rhose family conflicts with the referenced zone family should be skipped",
+			".description": "Redirects whose family conflicts with the referenced zone family should be skipped",
 			"src": "ipv4only",
 			"proto": "tcp",
 			"src_dport": "22",
@@ -96,6 +96,55 @@ Testing various option constraints.
 			"name": "Redirect #1",
 			"target": "dnat"
 		},
+	],
+	"nat": [
+		{
+			".description": "NAT rules whose family conflicts with the referenced zone family should be skipped",
+			"name": "NAT #1",
+			"family": "ipv6",
+			"src": "ipv4only",
+			"target": "masquerade"
+		},
+
+		{
+			".description": "NAT rules whose family conflicts with their addresses should be skipped",
+			"name": "NAT #2",
+			"family": "ipv4",
+			"src": "*",
+			"src_ip": "fc00::/7",
+			"target": "masquerade"
+		},
+
+		{
+			".description": "NAT rules without any AF specific bits and unspecified family should default to IPv4 for backwards compatibility",
+			"name": "NAT #3",
+			"src": "*",
+			"target": "masquerade"
+		},
+
+		{
+			".description": "NAT rules without explicit family but IPv6 specific bits should be IPv6",
+			"name": "NAT #4",
+			"src": "*",
+			"src_ip": "fc00::/7",
+			"target": "masquerade"
+		},
+
+
+		{
+			".description": "NAT rules with explicit family any should inherit zone restrictions",
+			"name": "NAT #5",
+			"src": "ipv4only",
+			"target": "masquerade"
+		},
+
+		{
+			".description": "NAT rules without any AF specific bits but explicit family any should be IPv4/IPv6",
+			"name": "NAT #6",
+			"family": "any",
+			"src": "*",
+			"target": "masquerade"
+		}
 	]
 }
 -- End --
@@ -106,6 +155,8 @@ Testing various option constraints.
 [!] Section @rule[2] (Rule #3) is restricted to IPv6 but referenced source zone is IPv4 only, skipping
 [!] Section @rule[3] (Rule #4) is restricted to IPv6 but referenced set match is IPv4 only, skipping
 [!] Section @redirect[0] (Redirect #1) is restricted to IPv6 but referenced source zone is IPv4 only, skipping
+[!] Section @nat[0] (NAT #1) is restricted to IPv6 but referenced source zone is IPv4 only, skipping
+[!] Section @nat[1] (NAT #2) is restricted to IPv4 but referenced source IP is IPv6 only, skipping
 -- End --
 
 -- Expect stdout --
@@ -209,11 +260,19 @@ table inet fw4 {
 
 	chain srcnat {
 		type nat hook postrouting priority srcnat; policy accept;
+		meta nfproto ipv4 masquerade comment "!fw4: NAT #3"
+		ip6 saddr fc00::/7 masquerade comment "!fw4: NAT #4"
+		masquerade comment "!fw4: NAT #6"
+		meta nfproto ipv4 ip daddr 192.168.1.0/24 jump srcnat_ipv4only comment "!fw4: Handle ipv4only IPv4 srcnat traffic"
 	}
 
 	chain dstnat_ipv4only {
 	}
 
+	chain srcnat_ipv4only {
+		meta nfproto ipv4 masquerade comment "!fw4: NAT #5"
+	}
+
 
 	#
 	# Raw rules (notrack & helper)