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 1b4764cea4fa04d4810b4786b4c28e9b59c84c17..cdc6e94bebc04cc9c766938d8159d6b4b23bceb6 100644 (file)
--- 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 a2d48b5eea1b75a283353f63a4e845e6df260b66..b01a609efec320b9d924dbeec29e9a746bc95fe8 100644 (file)
--- 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