From: Jo-Philipp Wich Date: Thu, 28 Apr 2022 09:49:47 +0000 (+0200) Subject: fw4: fix family auto-selection for config nat rules X-Git-Url: http://git.cdn.openwrt.org/?a=commitdiff_plain;h=b479815b198900a65fa42833b12bce689f8f395f;p=project%2Ffirewall4.git 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 --- 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)