From 36b1b4af2fc338806a517fc2bd91b207192ec1b3 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 15 Jun 2024 21:26:44 -0700 Subject: [PATCH] wgengine/filter: split local+logging lookups by IPv4-vs-IPv6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we already know it's an incoming IPv4 packet, no need to match against the set of IPv6s and vice versa. goos: darwin goarch: arm64 pkg: tailscale.com/wgengine/filter │ before │ after │ │ sec/op │ sec/op vs base │ FilterMatch/not-local-v4-8 21.40n ± 3% 16.04n ± 1% -25.09% (p=0.000 n=10) FilterMatch/not-local-v6-8 20.75n ± 9% 15.71n ± 0% -24.31% (p=0.000 n=10) FilterMatch/no-match-v4-8 81.37n ± 1% 78.57n ± 3% -3.43% (p=0.005 n=10) FilterMatch/no-match-v6-8 77.73n ± 2% 73.71n ± 3% -5.18% (p=0.002 n=10) FilterMatch/tcp-not-syn-v4-8 21.41n ± 3% 16.86n ± 0% -21.25% (p=0.000 n=10) FilterMatch/tcp-not-syn-v4-no-logs-8 10.04n ± 0% 10.05n ± 0% ~ (p=0.446 n=10) geomean 29.07n 25.05n -13.84% Updates #12486 Change-Id: I70e5024af03893327d26629a994ab2aa9811f4f3 Signed-off-by: Brad Fitzpatrick --- wgengine/filter/filter.go | 52 ++++++++++++++++++++++------------ wgengine/filter/filter_test.go | 3 +- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/wgengine/filter/filter.go b/wgengine/filter/filter.go index 0e01c848d..a545acb79 100644 --- a/wgengine/filter/filter.go +++ b/wgengine/filter/filter.go @@ -23,21 +23,23 @@ import ( "tailscale.com/types/logger" "tailscale.com/types/views" "tailscale.com/util/mak" + "tailscale.com/util/slicesx" ) // Filter is a stateful packet filter. type Filter struct { logf logger.Logf - // local is the set of IPs prefixes that we know to be "local" to - // this node. All packets coming in over tailscale must have a - // destination within local, regardless of the policy filter - // below. - local func(netip.Addr) bool + // local4 and local6 report whether an IP is "local" to this node, for the + // respective address family. All packets coming in over tailscale must have + // a destination within local, regardless of the policy filter below. + local4 func(netip.Addr) bool + local6 func(netip.Addr) bool // logIPs is the set of IPs that are allowed to appear in flow // logs. If a packet is to or from an IP not in logIPs, it will // never be logged. - logIPs func(netip.Addr) bool + logIPs4 func(netip.Addr) bool + logIPs6 func(netip.Addr) bool // matches4 and matches6 are lists of match->action rules // applied to all packets arriving over tailscale @@ -184,23 +186,31 @@ func New(matches []Match, localNets, logIPs *netipx.IPSet, shareStateWith *Filte } } - containsFunc := func(s *netipx.IPSet) func(netip.Addr) bool { - if s == nil { - return tsaddr.FalseContainsIPFunc() - } - return tsaddr.NewContainsIPFunc(views.SliceOf(s.Prefixes())) - } - f := &Filter{ logf: logf, matches4: matchesFamily(matches, netip.Addr.Is4), matches6: matchesFamily(matches, netip.Addr.Is6), cap4: capMatchesFunc(matches, netip.Addr.Is4), cap6: capMatchesFunc(matches, netip.Addr.Is6), - local: containsFunc(localNets), - logIPs: containsFunc(logIPs), + local4: tsaddr.FalseContainsIPFunc(), + local6: tsaddr.FalseContainsIPFunc(), + logIPs4: tsaddr.FalseContainsIPFunc(), + logIPs6: tsaddr.FalseContainsIPFunc(), state: state, } + if localNets != nil { + p := localNets.Prefixes() + p4, p6 := slicesx.Partition(p, func(p netip.Prefix) bool { return p.Addr().Is4() }) + f.local4 = tsaddr.NewContainsIPFunc(views.SliceOf(p4)) + f.local6 = tsaddr.NewContainsIPFunc(views.SliceOf(p6)) + } + if logIPs != nil { + p := logIPs.Prefixes() + p4, p6 := slicesx.Partition(p, func(p netip.Prefix) bool { return p.Addr().Is4() }) + f.logIPs4 = tsaddr.NewContainsIPFunc(views.SliceOf(p4)) + f.logIPs6 = tsaddr.NewContainsIPFunc(views.SliceOf(p6)) + } + return f } @@ -431,7 +441,7 @@ func (f *Filter) runIn4(q *packet.Parsed) (r Response, why string) { // A compromised peer could try to send us packets for // destinations we didn't explicitly advertise. This check is to // prevent that. - if !f.local(q.Dst.Addr()) { + if !f.local4(q.Dst.Addr()) { return Drop, "destination not allowed" } @@ -491,7 +501,7 @@ func (f *Filter) runIn6(q *packet.Parsed) (r Response, why string) { // A compromised peer could try to send us packets for // destinations we didn't explicitly advertise. This check is to // prevent that. - if !f.local(q.Dst.Addr()) { + if !f.local6(q.Dst.Addr()) { return Drop, "destination not allowed" } @@ -617,7 +627,13 @@ func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response { // loggingAllowed reports whether p can appear in logs at all. func (f *Filter) loggingAllowed(p *packet.Parsed) bool { - return f.logIPs(p.Src.Addr()) && f.logIPs(p.Dst.Addr()) + switch p.IPVersion { + case 4: + return f.logIPs4(p.Src.Addr()) && f.logIPs4(p.Dst.Addr()) + case 6: + return f.logIPs6(p.Src.Addr()) && f.logIPs6(p.Dst.Addr()) + } + return false } // omitDropLogging reports whether packet p, which has already been diff --git a/wgengine/filter/filter_test.go b/wgengine/filter/filter_test.go index 568e2dd4b..23b0a936a 100644 --- a/wgengine/filter/filter_test.go +++ b/wgengine/filter/filter_test.go @@ -440,10 +440,11 @@ func TestLoggingPrivacy(t *testing.T) { } f := newFilter(logf) - f.logIPs = tsaddr.NewContainsIPFunc(views.SliceOf([]netip.Prefix{ + f.logIPs4 = tsaddr.NewContainsIPFunc(views.SliceOf([]netip.Prefix{ tsaddr.CGNATRange(), tsaddr.TailscaleULARange(), })) + f.logIPs6 = f.logIPs4 var ( ts4 = netip.AddrPortFrom(tsaddr.CGNATRange().Addr().Next(), 1234)