From d810597414df46826671eef3d0605eba9a5a0a5f Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 2 May 2025 23:08:56 +0300 Subject: [PATCH] policy/matcher: fix bug using contains instead of overlap (#2556) * policy/matcher: slices.ContainsFunc Signed-off-by: Kristoffer Dalby * policy/matcher: slices.ContainsFunc, correct contains vs overlap Signed-off-by: Kristoffer Dalby * policy: add tests to validate fix for 2181 Fixes #2181 Signed-off-by: Kristoffer Dalby --------- Signed-off-by: Kristoffer Dalby --- hscontrol/policy/matcher/matcher.go | 34 ++------- hscontrol/policy/policy_test.go | 106 +++++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 30 deletions(-) diff --git a/hscontrol/policy/matcher/matcher.go b/hscontrol/policy/matcher/matcher.go index 1d4f09d2..ec07d19c 100644 --- a/hscontrol/policy/matcher/matcher.go +++ b/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) } diff --git a/hscontrol/policy/policy_test.go b/hscontrol/policy/policy_test.go index cebda65f..671ed829 100644 --- a/hscontrol/policy/policy_test.go +++ b/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 {