From 7d9b1de3aaa1f9d8ca1e40f8ea6d589fc3c5d821 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 30 Dec 2021 11:11:50 -0800 Subject: [PATCH] netcheck,portmapper,magicsock: ignore some UDP write errors on Linux Treat UDP send EPERM errors as a lost UDP packet, not something super fatal. That's just the Linux firewall preventing it from going out. And add a leaf package net/neterror for that (and future) policy that all three packages can share, with tests. Updates #3619 Change-Id: Ibdb838c43ee9efe70f4f25f7fc7fdf4607ba9c1d Signed-off-by: Brad Fitzpatrick --- cmd/tailscale/depaware.txt | 1 + cmd/tailscaled/depaware.txt | 1 + net/netcheck/netcheck.go | 5 +-- net/neterror/neterror.go | 42 ++++++++++++++++++++++ net/neterror/neterror_linux_test.go | 55 +++++++++++++++++++++++++++++ net/portmapper/portmapper.go | 10 ++++++ wgengine/magicsock/magicsock.go | 5 +-- 7 files changed, 115 insertions(+), 4 deletions(-) create mode 100644 net/neterror/neterror.go create mode 100644 net/neterror/neterror_linux_test.go diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index f3d5d36d7..0f303b086 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -47,6 +47,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/net/flowtrack from tailscale.com/wgengine/filter+ 💣 tailscale.com/net/interfaces from tailscale.com/cmd/tailscale/cli+ tailscale.com/net/netcheck from tailscale.com/cmd/tailscale/cli + tailscale.com/net/neterror from tailscale.com/net/netcheck+ tailscale.com/net/netknob from tailscale.com/net/netns tailscale.com/net/netns from tailscale.com/derp/derphttp+ tailscale.com/net/packet from tailscale.com/wgengine/filter diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index c5d4ed01f..e8964704c 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -195,6 +195,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/flowtrack from tailscale.com/net/packet+ 💣 tailscale.com/net/interfaces from tailscale.com/cmd/tailscaled+ tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock + tailscale.com/net/neterror from tailscale.com/net/netcheck+ tailscale.com/net/netknob from tailscale.com/logpolicy+ tailscale.com/net/netns from tailscale.com/cmd/tailscaled+ 💣 tailscale.com/net/netstat from tailscale.com/ipn/ipnserver diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index 2c0e70287..72896dde0 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -27,6 +27,7 @@ import ( "inet.af/netaddr" "tailscale.com/derp/derphttp" "tailscale.com/net/interfaces" + "tailscale.com/net/neterror" "tailscale.com/net/netns" "tailscale.com/net/portmapper" "tailscale.com/net/stun" @@ -1234,7 +1235,7 @@ func (rs *reportState) runProbe(ctx context.Context, dm *tailcfg.DERPMap, probe case probeIPv4: metricSTUNSend4.Add(1) n, err := rs.pc4.WriteTo(req, addr) - if n == len(req) && err == nil { + if n == len(req) && err == nil || neterror.TreatAsLostUDP(err) { rs.mu.Lock() rs.report.IPv4CanSend = true rs.mu.Unlock() @@ -1242,7 +1243,7 @@ func (rs *reportState) runProbe(ctx context.Context, dm *tailcfg.DERPMap, probe case probeIPv6: metricSTUNSend6.Add(1) n, err := rs.pc6.WriteTo(req, addr) - if n == len(req) && err == nil { + if n == len(req) && err == nil || neterror.TreatAsLostUDP(err) { rs.mu.Lock() rs.report.IPv6CanSend = true rs.mu.Unlock() diff --git a/net/neterror/neterror.go b/net/neterror/neterror.go new file mode 100644 index 000000000..d122fa795 --- /dev/null +++ b/net/neterror/neterror.go @@ -0,0 +1,42 @@ +// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package neterror classifies network errors. +package neterror + +import ( + "errors" + "runtime" + "syscall" +) + +var errEPERM error = syscall.EPERM // box it into interface just once + +// TreatAsLostUDP reports whether err is an error from a UDP send +// operation that should be treated as a UDP packet that just got +// lost. +// +// Notably, on Linux this reports true for EPERM errors (from outbound +// firewall blocks) which aren't really send errors; they're just +// sends that are never going to make it because the local OS blocked +// it. +func TreatAsLostUDP(err error) bool { + if err == nil { + return false + } + switch runtime.GOOS { + case "linux": + // Linux, while not documented in the man page, + // returns EPERM when there's an OUTPUT rule with -j + // DROP or -j REJECT. We use this very specific + // Linux+EPERM check rather than something super broad + // like net.Error.Temporary which could be anything. + // + // For now we only do this on Linux, as such outgoing + // firewall violations mapping to syscall errors + // hasn't yet been observed on other OSes. + return errors.Is(err, errEPERM) + } + return false +} diff --git a/net/neterror/neterror_linux_test.go b/net/neterror/neterror_linux_test.go new file mode 100644 index 000000000..70db5ff01 --- /dev/null +++ b/net/neterror/neterror_linux_test.go @@ -0,0 +1,55 @@ +// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package neterror + +import ( + "errors" + "net" + "os" + "syscall" + "testing" +) + +func TestTreatAsLostUDP(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {"nil", nil, false}, + {"non-nil", errors.New("foo"), false}, + {"eperm", syscall.EPERM, true}, + { + name: "operror", + err: &net.OpError{ + Op: "write", + Err: &os.SyscallError{ + Syscall: "sendto", + Err: syscall.EPERM, + }, + }, + want: true, + }, + { + name: "host_unreach", + err: &net.OpError{ + Op: "write", + Err: &os.SyscallError{ + Syscall: "sendto", + Err: syscall.EHOSTUNREACH, + }, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := TreatAsLostUDP(tt.err); got != tt.want { + t.Errorf("got = %v; want %v", got, tt.want) + } + }) + } + +} diff --git a/net/portmapper/portmapper.go b/net/portmapper/portmapper.go index f352e9883..a47f176ee 100644 --- a/net/portmapper/portmapper.go +++ b/net/portmapper/portmapper.go @@ -20,6 +20,7 @@ import ( "go4.org/mem" "inet.af/netaddr" "tailscale.com/net/interfaces" + "tailscale.com/net/neterror" "tailscale.com/net/netns" "tailscale.com/types/logger" "tailscale.com/util/clientmetric" @@ -478,18 +479,27 @@ func (c *Client) createOrGetMapping(ctx context.Context) (external netaddr.IPPor // Only do PCP mapping in the case when PMP did not appear to be available recently. pkt := buildPCPRequestMappingPacket(myIP, localPort, prevPort, pcpMapLifetimeSec, wildcardIP) if _, err := uc.WriteTo(pkt, pxpAddru); err != nil { + if neterror.TreatAsLostUDP(err) { + err = NoMappingError{ErrNoPortMappingServices} + } return netaddr.IPPort{}, err } } else { // Ask for our external address if needed. if m.external.IP().IsZero() { if _, err := uc.WriteTo(pmpReqExternalAddrPacket, pxpAddru); err != nil { + if neterror.TreatAsLostUDP(err) { + err = NoMappingError{ErrNoPortMappingServices} + } return netaddr.IPPort{}, err } } pkt := buildPMPRequestMappingPacket(localPort, prevPort, pmpMapLifetimeSec) if _, err := uc.WriteTo(pkt, pxpAddru); err != nil { + if neterror.TreatAsLostUDP(err) { + err = NoMappingError{ErrNoPortMappingServices} + } return netaddr.IPPort{}, err } } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 72f68c275..5a143f7d4 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -39,6 +39,7 @@ import ( "tailscale.com/net/dnscache" "tailscale.com/net/interfaces" "tailscale.com/net/netcheck" + "tailscale.com/net/neterror" "tailscale.com/net/netns" "tailscale.com/net/portmapper" "tailscale.com/net/stun" @@ -1212,7 +1213,7 @@ func (c *Conn) sendUDPStd(addr *net.UDPAddr, b []byte) (sent bool, err error) { switch { case addr.IP.To4() != nil: _, err = c.pconn4.WriteTo(b, addr) - if err != nil && c.noV4.Get() { + if err != nil && (c.noV4.Get() || neterror.TreatAsLostUDP(err)) { return false, nil } case len(addr.IP) == net.IPv6len: @@ -1221,7 +1222,7 @@ func (c *Conn) sendUDPStd(addr *net.UDPAddr, b []byte) (sent bool, err error) { return false, nil } _, err = c.pconn6.WriteTo(b, addr) - if err != nil && c.noV6.Get() { + if err != nil && (c.noV6.Get() || neterror.TreatAsLostUDP(err)) { return false, nil } default: