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>

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 28865cd926fbad36fbec14e161dbd51ae87b01a8..05e86469979db4fd6f0aaaf9f96af985c5298b33 100644 (file)
--- 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 9a6aa59365805c423050bd872a7e694dbf031bac..a1fd39f2eadd2079d1f27cec9cb98d005bc898cd 100644 (file)
--- 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)