policy/matcher: fix bug using contains instead of overlap (#2556)

* policy/matcher: slices.ContainsFunc

Signed-off-by: Kristoffer Dalby <[email protected]>

* policy/matcher: slices.ContainsFunc, correct contains vs overlap

Signed-off-by: Kristoffer Dalby <[email protected]>

* policy: add tests to validate fix for 2181

Fixes #2181

Signed-off-by: Kristoffer Dalby <[email protected]>

---------

Signed-off-by: Kristoffer Dalby <[email protected]>

Author: kradalby

hscontrol/policy/matcher/matcher.go

@@ -3,6 +3,8 @@ package matcher
 import (
 	"net/netip"
 
+	"slices"
+
 	"github.com/juanfont/headscale/hscontrol/util"
 	"go4.org/netipx"
 	"tailscale.com/tailcfg"
@@ -58,41 +60,17 @@ func MatchFromStrings(sources, destinations []string) Match {
 }
 
 func (m *Match) SrcsContainsIPs(ips ...netip.Addr) bool {
-	for _, ip := range ips {
-		if m.srcs.Contains(ip) {
-			return true
-		}
-	}
-
-	return false
+	return slices.ContainsFunc(ips, m.srcs.Contains)
 }
 
 func (m *Match) DestsContainsIP(ips ...netip.Addr) bool {
-	for _, ip := range ips {
-		if m.dests.Contains(ip) {
-			return true
-		}
-	}
-
-	return false
+	return slices.ContainsFunc(ips, m.dests.Contains)
 }
 
 func (m *Match) SrcsOverlapsPrefixes(prefixes ...netip.Prefix) bool {
-	for _, prefix := range prefixes {
-		if m.srcs.ContainsPrefix(prefix) {
-			return true
-		}
-	}
-
-	return false
+	return slices.ContainsFunc(prefixes, m.srcs.OverlapsPrefix)
 }
 
 func (m *Match) DestsOverlapsPrefixes(prefixes ...netip.Prefix) bool {
-	for _, prefix := range prefixes {
-		if m.dests.ContainsPrefix(prefix) {
-			return true
-		}
-	}
-
-	return false
+	return slices.ContainsFunc(prefixes, m.dests.OverlapsPrefix)
 }

hscontrol/policy/policy_test.go

@@ -2,10 +2,11 @@ package policy
 
 import (
 	"fmt"
-	"github.com/juanfont/headscale/hscontrol/policy/matcher"
 	"net/netip"
 	"testing"
 
+	"github.com/juanfont/headscale/hscontrol/policy/matcher"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/util"
@@ -1370,7 +1371,6 @@ func TestFilterNodesByACL(t *testing.T) {
 				},
 			},
 		},
-
 		{
 			name: "subnet-router-with-only-route",
 			args: args{
@@ -1422,6 +1422,108 @@ func TestFilterNodesByACL(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "subnet-router-with-only-route-smaller-mask-2181",
+			args: args{
+				nodes: []*types.Node{
+					{
+						ID:       1,
+						IPv4:     ap("100.64.0.1"),
+						Hostname: "router",
+						User:     types.User{Name: "router"},
+						Hostinfo: &tailcfg.Hostinfo{
+							RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+						},
+						ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+					},
+					{
+						ID:       2,
+						IPv4:     ap("100.64.0.2"),
+						Hostname: "node",
+						User:     types.User{Name: "node"},
+					},
+				},
+				rules: []tailcfg.FilterRule{
+					{
+						SrcIPs: []string{
+							"100.64.0.2/32",
+						},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "10.99.0.2/32", Ports: tailcfg.PortRangeAny},
+						},
+					},
+				},
+				node: &types.Node{
+					ID:       1,
+					IPv4:     ap("100.64.0.1"),
+					Hostname: "router",
+					User:     types.User{Name: "router"},
+					Hostinfo: &tailcfg.Hostinfo{
+						RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+					},
+					ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+				},
+			},
+			want: []*types.Node{
+				{
+					ID:       2,
+					IPv4:     ap("100.64.0.2"),
+					Hostname: "node",
+					User:     types.User{Name: "node"},
+				},
+			},
+		},
+		{
+			name: "node-to-subnet-router-with-only-route-smaller-mask-2181",
+			args: args{
+				nodes: []*types.Node{
+					{
+						ID:       1,
+						IPv4:     ap("100.64.0.1"),
+						Hostname: "router",
+						User:     types.User{Name: "router"},
+						Hostinfo: &tailcfg.Hostinfo{
+							RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+						},
+						ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+					},
+					{
+						ID:       2,
+						IPv4:     ap("100.64.0.2"),
+						Hostname: "node",
+						User:     types.User{Name: "node"},
+					},
+				},
+				rules: []tailcfg.FilterRule{
+					{
+						SrcIPs: []string{
+							"100.64.0.2/32",
+						},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "10.99.0.2/32", Ports: tailcfg.PortRangeAny},
+						},
+					},
+				},
+				node: &types.Node{
+					ID:       2,
+					IPv4:     ap("100.64.0.2"),
+					Hostname: "node",
+					User:     types.User{Name: "node"},
+				},
+			},
+			want: []*types.Node{
+				{
+					ID:       1,
+					IPv4:     ap("100.64.0.1"),
+					Hostname: "router",
+					User:     types.User{Name: "router"},
+					Hostinfo: &tailcfg.Hostinfo{
+						RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+					},
+					ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {