Просмотр исходного кода

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 <[email protected]>
Brad Fitzpatrick 4 лет назад
Родитель
Сommit
7d9b1de3aa

+ 1 - 0
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/flowtrack                                  from tailscale.com/wgengine/filter+
      💣 tailscale.com/net/interfaces                                 from tailscale.com/cmd/tailscale/cli+
      💣 tailscale.com/net/interfaces                                 from tailscale.com/cmd/tailscale/cli+
         tailscale.com/net/netcheck                                   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/netknob                                    from tailscale.com/net/netns
         tailscale.com/net/netns                                      from tailscale.com/derp/derphttp+
         tailscale.com/net/netns                                      from tailscale.com/derp/derphttp+
         tailscale.com/net/packet                                     from tailscale.com/wgengine/filter
         tailscale.com/net/packet                                     from tailscale.com/wgengine/filter

+ 1 - 0
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/flowtrack                                  from tailscale.com/net/packet+
      💣 tailscale.com/net/interfaces                                 from tailscale.com/cmd/tailscaled+
      💣 tailscale.com/net/interfaces                                 from tailscale.com/cmd/tailscaled+
         tailscale.com/net/netcheck                                   from tailscale.com/wgengine/magicsock
         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/netknob                                    from tailscale.com/logpolicy+
         tailscale.com/net/netns                                      from tailscale.com/cmd/tailscaled+
         tailscale.com/net/netns                                      from tailscale.com/cmd/tailscaled+
      💣 tailscale.com/net/netstat                                    from tailscale.com/ipn/ipnserver
      💣 tailscale.com/net/netstat                                    from tailscale.com/ipn/ipnserver

+ 3 - 2
net/netcheck/netcheck.go

@@ -27,6 +27,7 @@ import (
 	"inet.af/netaddr"
 	"inet.af/netaddr"
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/interfaces"
+	"tailscale.com/net/neterror"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/portmapper"
 	"tailscale.com/net/portmapper"
 	"tailscale.com/net/stun"
 	"tailscale.com/net/stun"
@@ -1234,7 +1235,7 @@ func (rs *reportState) runProbe(ctx context.Context, dm *tailcfg.DERPMap, probe
 	case probeIPv4:
 	case probeIPv4:
 		metricSTUNSend4.Add(1)
 		metricSTUNSend4.Add(1)
 		n, err := rs.pc4.WriteTo(req, addr)
 		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.mu.Lock()
 			rs.report.IPv4CanSend = true
 			rs.report.IPv4CanSend = true
 			rs.mu.Unlock()
 			rs.mu.Unlock()
@@ -1242,7 +1243,7 @@ func (rs *reportState) runProbe(ctx context.Context, dm *tailcfg.DERPMap, probe
 	case probeIPv6:
 	case probeIPv6:
 		metricSTUNSend6.Add(1)
 		metricSTUNSend6.Add(1)
 		n, err := rs.pc6.WriteTo(req, addr)
 		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.mu.Lock()
 			rs.report.IPv6CanSend = true
 			rs.report.IPv6CanSend = true
 			rs.mu.Unlock()
 			rs.mu.Unlock()

+ 42 - 0
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
+}

+ 55 - 0
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)
+			}
+		})
+	}
+
+}

+ 10 - 0
net/portmapper/portmapper.go

@@ -20,6 +20,7 @@ import (
 	"go4.org/mem"
 	"go4.org/mem"
 	"inet.af/netaddr"
 	"inet.af/netaddr"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/interfaces"
+	"tailscale.com/net/neterror"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/netns"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/clientmetric"
 	"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.
 		// Only do PCP mapping in the case when PMP did not appear to be available recently.
 		pkt := buildPCPRequestMappingPacket(myIP, localPort, prevPort, pcpMapLifetimeSec, wildcardIP)
 		pkt := buildPCPRequestMappingPacket(myIP, localPort, prevPort, pcpMapLifetimeSec, wildcardIP)
 		if _, err := uc.WriteTo(pkt, pxpAddru); err != nil {
 		if _, err := uc.WriteTo(pkt, pxpAddru); err != nil {
+			if neterror.TreatAsLostUDP(err) {
+				err = NoMappingError{ErrNoPortMappingServices}
+			}
 			return netaddr.IPPort{}, err
 			return netaddr.IPPort{}, err
 		}
 		}
 	} else {
 	} else {
 		// Ask for our external address if needed.
 		// Ask for our external address if needed.
 		if m.external.IP().IsZero() {
 		if m.external.IP().IsZero() {
 			if _, err := uc.WriteTo(pmpReqExternalAddrPacket, pxpAddru); err != nil {
 			if _, err := uc.WriteTo(pmpReqExternalAddrPacket, pxpAddru); err != nil {
+				if neterror.TreatAsLostUDP(err) {
+					err = NoMappingError{ErrNoPortMappingServices}
+				}
 				return netaddr.IPPort{}, err
 				return netaddr.IPPort{}, err
 			}
 			}
 		}
 		}
 
 
 		pkt := buildPMPRequestMappingPacket(localPort, prevPort, pmpMapLifetimeSec)
 		pkt := buildPMPRequestMappingPacket(localPort, prevPort, pmpMapLifetimeSec)
 		if _, err := uc.WriteTo(pkt, pxpAddru); err != nil {
 		if _, err := uc.WriteTo(pkt, pxpAddru); err != nil {
+			if neterror.TreatAsLostUDP(err) {
+				err = NoMappingError{ErrNoPortMappingServices}
+			}
 			return netaddr.IPPort{}, err
 			return netaddr.IPPort{}, err
 		}
 		}
 	}
 	}

+ 3 - 2
wgengine/magicsock/magicsock.go

@@ -39,6 +39,7 @@ import (
 	"tailscale.com/net/dnscache"
 	"tailscale.com/net/dnscache"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/netcheck"
 	"tailscale.com/net/netcheck"
+	"tailscale.com/net/neterror"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/portmapper"
 	"tailscale.com/net/portmapper"
 	"tailscale.com/net/stun"
 	"tailscale.com/net/stun"
@@ -1212,7 +1213,7 @@ func (c *Conn) sendUDPStd(addr *net.UDPAddr, b []byte) (sent bool, err error) {
 	switch {
 	switch {
 	case addr.IP.To4() != nil:
 	case addr.IP.To4() != nil:
 		_, err = c.pconn4.WriteTo(b, addr)
 		_, err = c.pconn4.WriteTo(b, addr)
-		if err != nil && c.noV4.Get() {
+		if err != nil && (c.noV4.Get() || neterror.TreatAsLostUDP(err)) {
 			return false, nil
 			return false, nil
 		}
 		}
 	case len(addr.IP) == net.IPv6len:
 	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
 			return false, nil
 		}
 		}
 		_, err = c.pconn6.WriteTo(b, addr)
 		_, err = c.pconn6.WriteTo(b, addr)
-		if err != nil && c.noV6.Get() {
+		if err != nil && (c.noV6.Get() || neterror.TreatAsLostUDP(err)) {
 			return false, nil
 			return false, nil
 		}
 		}
 	default:
 	default: