From 5756bc17049fbfa65c531eff08a8db5aea66b14a Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 20 Dec 2024 09:09:06 +0100 Subject: [PATCH] wgengine/filter: return drop reason for metrics Updates #14280 Signed-off-by: Kristoffer Dalby --- wgengine/filter/filter.go | 25 +++++++++++++------------ wgengine/filter/filter_test.go | 32 +++++++++++++++++--------------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/wgengine/filter/filter.go b/wgengine/filter/filter.go index 9e5d8a37f..6269b08eb 100644 --- a/wgengine/filter/filter.go +++ b/wgengine/filter/filter.go @@ -24,6 +24,7 @@ "tailscale.com/types/views" "tailscale.com/util/mak" "tailscale.com/util/slicesx" + "tailscale.com/util/usermetric" "tailscale.com/wgengine/filter/filtertype" ) @@ -410,7 +411,7 @@ func (f *Filter) ShieldsUp() bool { return f.shieldsUp } // Tailscale peer. func (f *Filter) RunIn(q *packet.Parsed, rf RunFlags) Response { dir := in - r := f.pre(q, rf, dir) + r, _ := f.pre(q, rf, dir) if r == Accept || r == Drop { // already logged return r @@ -431,16 +432,16 @@ func (f *Filter) RunIn(q *packet.Parsed, rf RunFlags) Response { // RunOut determines whether this node is allowed to send q to a // Tailscale peer. -func (f *Filter) RunOut(q *packet.Parsed, rf RunFlags) Response { +func (f *Filter) RunOut(q *packet.Parsed, rf RunFlags) (Response, usermetric.DropReason) { dir := out - r := f.pre(q, rf, dir) + r, reason := f.pre(q, rf, dir) if r == Accept || r == Drop { // already logged - return r + return r, reason } r, why := f.runOut(q) f.logRateLimit(rf, q, dir, r, why) - return r + return r, "" } var unknownProtoStringCache sync.Map // ipproto.Proto -> string @@ -610,33 +611,33 @@ func (d direction) String() string { // pre runs the direction-agnostic filter logic. dir is only used for // logging. -func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response { +func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) (Response, usermetric.DropReason) { if len(q.Buffer()) == 0 { // wireguard keepalive packet, always permit. - return Accept + return Accept, "" } if len(q.Buffer()) < 20 { f.logRateLimit(rf, q, dir, Drop, "too short") - return Drop + return Drop, usermetric.ReasonTooShort } if q.Dst.Addr().IsMulticast() { f.logRateLimit(rf, q, dir, Drop, "multicast") - return Drop + return Drop, usermetric.ReasonMulticast } if q.Dst.Addr().IsLinkLocalUnicast() && q.Dst.Addr() != gcpDNSAddr { f.logRateLimit(rf, q, dir, Drop, "link-local-unicast") - return Drop + return Drop, usermetric.ReasonLinkLocalUnicast } if q.IPProto == ipproto.Fragment { // Fragments after the first always need to be passed through. // Very small fragments are considered Junk by Parsed. f.logRateLimit(rf, q, dir, Accept, "fragment") - return Accept + return Accept, "" } - return noVerdict + return noVerdict, "" } // loggingAllowed reports whether p can appear in logs at all. diff --git a/wgengine/filter/filter_test.go b/wgengine/filter/filter_test.go index e7f71e6a4..68f206778 100644 --- a/wgengine/filter/filter_test.go +++ b/wgengine/filter/filter_test.go @@ -30,6 +30,7 @@ "tailscale.com/types/views" "tailscale.com/util/must" "tailscale.com/util/slicesx" + "tailscale.com/util/usermetric" "tailscale.com/wgengine/filter/filtertype" ) @@ -211,7 +212,7 @@ func TestUDPState(t *testing.T) { t.Fatalf("incoming initial packet not dropped, got=%v: %v", got, a4) } // We talk to that peer - if got := acl.RunOut(&b4, flags); got != Accept { + if got, _ := acl.RunOut(&b4, flags); got != Accept { t.Fatalf("outbound packet didn't egress, got=%v: %v", got, b4) } // Now, the same packet as before is allowed back. @@ -227,7 +228,7 @@ func TestUDPState(t *testing.T) { t.Fatalf("incoming initial packet not dropped: %v", a4) } // We talk to that peer - if got := acl.RunOut(&b6, flags); got != Accept { + if got, _ := acl.RunOut(&b6, flags); got != Accept { t.Fatalf("outbound packet didn't egress: %v", b4) } // Now, the same packet as before is allowed back. @@ -382,25 +383,26 @@ func BenchmarkFilter(b *testing.B) { func TestPreFilter(t *testing.T) { packets := []struct { - desc string - want Response - b []byte + desc string + want Response + wantReason usermetric.DropReason + b []byte }{ - {"empty", Accept, []byte{}}, - {"short", Drop, []byte("short")}, - {"junk", Drop, raw4default(ipproto.Unknown, 10)}, - {"fragment", Accept, raw4default(ipproto.Fragment, 40)}, - {"tcp", noVerdict, raw4default(ipproto.TCP, 0)}, - {"udp", noVerdict, raw4default(ipproto.UDP, 0)}, - {"icmp", noVerdict, raw4default(ipproto.ICMPv4, 0)}, + {"empty", Accept, "", []byte{}}, + {"short", Drop, usermetric.ReasonTooShort, []byte("short")}, + {"junk", Drop, "", raw4default(ipproto.Unknown, 10)}, + {"fragment", Accept, "", raw4default(ipproto.Fragment, 40)}, + {"tcp", noVerdict, "", raw4default(ipproto.TCP, 0)}, + {"udp", noVerdict, "", raw4default(ipproto.UDP, 0)}, + {"icmp", noVerdict, "", raw4default(ipproto.ICMPv4, 0)}, } f := NewAllowNone(t.Logf, &netipx.IPSet{}) for _, testPacket := range packets { p := &packet.Parsed{} p.Decode(testPacket.b) - got := f.pre(p, LogDrops|LogAccepts, in) - if got != testPacket.want { - t.Errorf("%q got=%v want=%v packet:\n%s", testPacket.desc, got, testPacket.want, packet.Hexdump(testPacket.b)) + got, gotReason := f.pre(p, LogDrops|LogAccepts, in) + if got != testPacket.want || gotReason != testPacket.wantReason { + t.Errorf("%q got=%v want=%v gotReason=%s wantReason=%s packet:\n%s", testPacket.desc, got, testPacket.want, gotReason, testPacket.wantReason, packet.Hexdump(testPacket.b)) } } }