VERSION.txt: this is v1.20.2

Change-Id: I0d0905dd4f709591002e52a53846af8fba0aa660
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

VERSION.txt

@@ -1 +1 @@
-1.19.0
+1.20.2

cmd/tailscale/cli/cli_test.go

@@ -786,6 +786,16 @@ func TestUpdatePrefs(t *testing.T) {
 			wantSimpleUp:   true,
 			wantJustEditMP: &ipn.MaskedPrefs{WantRunningSet: true},
 		},
+		{
+			name:  "just_edit_reset",
+			flags: []string{"--reset"},
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				Persist:    &persist.Persist{LoginName: "crawshaw.github"},
+			},
+			env:            upCheckEnv{backendState: "Running"},
+			wantJustEditMP: &ipn.MaskedPrefs{WantRunningSet: true},
+		},
 		{
 			name:  "control_synonym",
 			flags: []string{},

cmd/tailscale/cli/up.go

@@ -413,7 +413,6 @@ func updatePrefs(prefs, curPrefs *ipn.Prefs, env upCheckEnv) (simpleUp bool, jus
 
 	justEdit := env.backendState == ipn.Running.String() &&
 		!env.upArgs.forceReauth &&
-		!env.upArgs.reset &&
 		env.upArgs.authKeyOrFile == "" &&
 		!controlURLChanged &&
 		!tagsChanged

cmd/tailscale/cli/web.go

@@ -375,7 +375,7 @@ func webHandler(w http.ResponseWriter, r *http.Request) {
 			data.AdvertiseExitNode = true
 		} else {
 			if data.AdvertiseRoutes != "" {
-				data.AdvertiseRoutes = ","
+				data.AdvertiseRoutes += ","
 			}
 			data.AdvertiseRoutes += r.String()
 		}

net/dns/manager_openbsd.go

@@ -4,8 +4,71 @@
 
 package dns
 
-import "tailscale.com/types/logger"
+import (
+	"bytes"
+	"fmt"
+	"os"
 
-func NewOSConfigurator(logf logger.Logf, _ string) (OSConfigurator, error) {
+	"tailscale.com/types/logger"
+)
+
+type kv struct {
+	k, v string
+}
+
+func (kv kv) String() string {
+	return fmt.Sprintf("%s=%s", kv.k, kv.v)
+}
+
+func NewOSConfigurator(logf logger.Logf, interfaceName string) (OSConfigurator, error) {
+	return newOSConfigurator(logf, interfaceName,
+		newOSConfigEnv{
+			rcIsResolvd: rcIsResolvd,
+			fs:          directFS{},
+		})
+}
+
+// newOSConfigEnv are the funcs newOSConfigurator needs, pulled out for testing.
+type newOSConfigEnv struct {
+	fs          directFS
+	rcIsResolvd func(resolvConfContents []byte) bool
+}
+
+func newOSConfigurator(logf logger.Logf, interfaceName string, env newOSConfigEnv) (ret OSConfigurator, err error) {
+	var debug []kv
+	dbg := func(k, v string) {
+		debug = append(debug, kv{k, v})
+	}
+	defer func() {
+		if ret != nil {
+			dbg("ret", fmt.Sprintf("%T", ret))
+		}
+		logf("dns: %v", debug)
+	}()
+
+	bs, err := env.fs.ReadFile(resolvConf)
+	if os.IsNotExist(err) {
+		dbg("rc", "missing")
+		return newDirectManager(logf), nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("reading /etc/resolv.conf: %w", err)
+	}
+
+	if env.rcIsResolvd(bs) {
+		dbg("resolvd", "yes")
+		return newResolvdManager(logf, interfaceName)
+	}
+
+	dbg("resolvd", "missing")
 	return newDirectManager(logf), nil
 }
+
+func rcIsResolvd(resolvConfContents []byte) bool {
+	// If we have the string "# resolvd:" in resolv.conf resolvd(8) is
+	// managing things.
+	if bytes.Contains(resolvConfContents, []byte("# resolvd:")) {
+		return true
+	}
+	return false
+}

net/dns/manager_windows.go

@@ -7,8 +7,10 @@ package dns
 import (
 	"errors"
 	"fmt"
+	"os"
 	"os/exec"
 	"sort"
+	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -34,6 +36,8 @@ const (
 	versionKey = `SOFTWARE\Microsoft\Windows NT\CurrentVersion`
 )
 
+var configureWSL, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_CONFIGURE_WSL"))
+
 type windowsManager struct {
 	logf       logger.Logf
 	guid       string
@@ -307,13 +311,15 @@ func (m windowsManager) SetDNS(cfg OSConfig) error {
 
 	// On initial setup of WSL, the restart caused by --shutdown is slow,
 	// so we do it out-of-line.
-	go func() {
-		if err := m.wslManager.SetDNS(cfg); err != nil {
-			m.logf("WSL SetDNS: %v", err) // continue
-		} else {
-			m.logf("WSL SetDNS: success")
-		}
-	}()
+	if configureWSL {
+		go func() {
+			if err := m.wslManager.SetDNS(cfg); err != nil {
+				m.logf("WSL SetDNS: %v", err) // continue
+			} else {
+				m.logf("WSL SetDNS: success")
+			}
+		}()
+	}
 
 	return nil
 }

net/dns/resolvd.go

@@ -0,0 +1,161 @@
+// Copyright (c) 2020 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.
+
+//go:build openbsd
+// +build openbsd
+
+package dns
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"os"
+	"os/exec"
+	"regexp"
+	"strings"
+
+	"inet.af/netaddr"
+	"tailscale.com/types/logger"
+	"tailscale.com/util/dnsname"
+)
+
+func newResolvdManager(logf logger.Logf, interfaceName string) (*resolvdManager, error) {
+	return &resolvdManager{
+		logf:   logf,
+		ifName: interfaceName,
+		fs:     directFS{},
+	}, nil
+}
+
+// resolvdManager is an OSConfigurator which uses route(1) to teach OpenBSD's
+// resolvd(8) about DNS servers.
+type resolvdManager struct {
+	logf   logger.Logf
+	ifName string
+	fs     directFS
+}
+
+func (m *resolvdManager) SetDNS(config OSConfig) error {
+	args := []string{
+		"nameserver",
+		m.ifName,
+	}
+
+	origResolv, err := m.readAndCopy(resolvConf, backupConf, 0644)
+	if err != nil {
+		return err
+	}
+	newResolvConf := removeSearchLines(origResolv)
+
+	for _, ns := range config.Nameservers {
+		args = append(args, ns.String())
+	}
+
+	var newSearch = []string{
+		"search",
+	}
+	for _, s := range config.SearchDomains {
+		newSearch = append(newSearch, s.WithoutTrailingDot())
+	}
+
+	newResolvConf = append(newResolvConf, []byte(strings.Join(newSearch, " "))...)
+
+	err = m.fs.WriteFile(resolvConf, newResolvConf, 0644)
+	if err != nil {
+		return err
+	}
+
+	cmd := exec.Command("/sbin/route", args...)
+	return cmd.Run()
+}
+
+func (m *resolvdManager) SupportsSplitDNS() bool {
+	return false
+}
+
+func (m *resolvdManager) GetBaseConfig() (OSConfig, error) {
+	cfg, err := m.readResolvConf()
+	if err != nil {
+		return OSConfig{}, err
+	}
+
+	return cfg, nil
+}
+
+func (m *resolvdManager) Close() error {
+	// resolvd handles teardown of nameservers so we only need to write back the original
+	// config and be done.
+
+	_, err := m.readAndCopy(backupConf, resolvConf, 0644)
+	if err != nil {
+		return err
+	}
+
+	return m.fs.Remove(backupConf)
+}
+
+func (m *resolvdManager) readAndCopy(a, b string, mode os.FileMode) ([]byte, error) {
+	orig, err := m.fs.ReadFile(a)
+	if err != nil {
+		return nil, err
+	}
+	err = m.fs.WriteFile(b, orig, mode)
+	if err != nil {
+		return nil, err
+	}
+
+	return orig, nil
+}
+
+func (m resolvdManager) readResolvConf() (config OSConfig, err error) {
+	b, err := m.fs.ReadFile(resolvConf)
+	if err != nil {
+		return OSConfig{}, err
+	}
+
+	scanner := bufio.NewScanner(bytes.NewReader(b))
+	for scanner.Scan() {
+		line := strings.TrimSpace(scanner.Text())
+
+		// resolvd manages "nameserver" lines, we only need to handle
+		// "search".
+		if strings.HasPrefix(line, "search") {
+			domain := strings.TrimPrefix(line, "search")
+			domain = strings.TrimSpace(domain)
+			fqdn, err := dnsname.ToFQDN(domain)
+			if err != nil {
+				return OSConfig{}, fmt.Errorf("parsing search domains %q: %w", line, err)
+			}
+			config.SearchDomains = append(config.SearchDomains, fqdn)
+			continue
+		}
+
+		if strings.HasPrefix(line, "nameserver") {
+			s := strings.TrimPrefix(line, "nameserver")
+			parts := strings.Split(s, " # ")
+			if len(parts) == 0 {
+				return OSConfig{}, err
+			}
+			nameserver := strings.TrimSpace(parts[0])
+			ip, err := netaddr.ParseIP(nameserver)
+			if err != nil {
+				return OSConfig{}, err
+			}
+			config.Nameservers = append(config.Nameservers, ip)
+			continue
+		}
+	}
+
+	if err = scanner.Err(); err != nil {
+		return OSConfig{}, err
+	}
+
+	return config, nil
+}
+
+func removeSearchLines(orig []byte) []byte {
+	re := regexp.MustCompile(`(?m)^search\s+.+$`)
+	return re.ReplaceAll(orig, []byte(""))
+}

net/dns/resolver/tsdns.go

@@ -529,6 +529,10 @@ func stubResolverForOS() (ip netaddr.IP, err error) {
 	if c, ok := resolvConfCacheValue.Load().(resolvConfCache); ok && c.mod == cur.mod && c.size == cur.size {
 		return c.ip, nil
 	}
+	// TODO(bradfitz): unify this /etc/resolv.conf parsing code with readResolv
+	// in net/dns, which we can't use due to circular dependency reasons.
+	// Move it to a leaf, including the OSConfig type (perhaps in its own dnstype
+	// package?)
 	err = lineread.File("/etc/resolv.conf", func(line []byte) error {
 		if !ip.IsZero() {
 			return nil
@@ -537,6 +541,12 @@ func stubResolverForOS() (ip netaddr.IP, err error) {
 		if len(line) == 0 || line[0] == '#' {
 			return nil
 		}
+		// Normalize tabs to spaces to simplify parsing code later.
+		for i, b := range line {
+			if b == '\t' {
+				line[i] = ' '
+			}
+		}
 		if mem.HasPrefix(mem.B(line), mem.S("nameserver ")) {
 			s := strings.TrimSpace(strings.TrimPrefix(string(line), "nameserver "))
 			ip, err = netaddr.ParseIP(s)

net/dnscache/dnscache.go

@@ -457,9 +457,7 @@ func TLSDialer(fwd DialContextFunc, dnsCache *Resolver, tlsConfigBase *tls.Confi
 			}
 		}()
 		go func() {
-			err := tlsConn.Handshake()
-			handshakeTimeoutCancel()
-			errc <- err
+			errc <- tlsConn.Handshake()
 		}()
 		if err := <-errc; err != nil {
 			tcpConn.Close()

net/netns/netns_linux.go

@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"net"
 	"os"
-	"os/exec"
 	"sync"
 	"syscall"
 
@@ -67,8 +66,7 @@ func socketMarkWorks() bool {
 // If it doesn't, we have to use SO_BINDTODEVICE on our sockets instead.
 func useSocketMark() bool {
 	socketMarkWorksOnce.Do(func() {
-		ipRuleWorks := exec.Command("ip", "rule").Run() == nil
-		socketMarkWorksOnce.v = ipRuleWorks && socketMarkWorks()
+		socketMarkWorksOnce.v = socketMarkWorks()
 	})
 	return socketMarkWorksOnce.v
 }

net/packet/packet.go

@@ -22,12 +22,16 @@ const minFrag = 60 + 20 // max IPv4 header + basic TCP header
 type TCPFlag uint8
 
 const (
-	TCPFin    TCPFlag = 0x01
-	TCPSyn    TCPFlag = 0x02
-	TCPRst    TCPFlag = 0x04
-	TCPPsh    TCPFlag = 0x08
-	TCPAck    TCPFlag = 0x10
-	TCPSynAck TCPFlag = TCPSyn | TCPAck
+	TCPFin     TCPFlag = 0x01
+	TCPSyn     TCPFlag = 0x02
+	TCPRst     TCPFlag = 0x04
+	TCPPsh     TCPFlag = 0x08
+	TCPAck     TCPFlag = 0x10
+	TCPUrg     TCPFlag = 0x20
+	TCPECNEcho TCPFlag = 0x40
+	TCPCWR     TCPFlag = 0x80
+	TCPSynAck  TCPFlag = TCPSyn | TCPAck
+	TCPECNBits TCPFlag = TCPECNEcho | TCPCWR
 )
 
 // Parsed is a minimal decoding of a packet suitable for use in filters.
@@ -52,7 +56,7 @@ type Parsed struct {
 	Src netaddr.IPPort
 	// DstIP4 is the destination address. Family matches IPVersion.
 	Dst netaddr.IPPort
-	// TCPFlags is the packet's TCP flag bigs. Valid iff IPProto == TCP.
+	// TCPFlags is the packet's TCP flag bits. Valid iff IPProto == TCP.
 	TCPFlags TCPFlag
 }
 
@@ -180,7 +184,7 @@ func (q *Parsed) decode4(b []byte) {
 			}
 			q.Src = q.Src.WithPort(binary.BigEndian.Uint16(sub[0:2]))
 			q.Dst = q.Dst.WithPort(binary.BigEndian.Uint16(sub[2:4]))
-			q.TCPFlags = TCPFlag(sub[13]) & 0x3F
+			q.TCPFlags = TCPFlag(sub[13])
 			headerLength := (sub[12] & 0xF0) >> 2
 			q.dataofs = q.subofs + int(headerLength)
 			return
@@ -282,7 +286,7 @@ func (q *Parsed) decode6(b []byte) {
 		}
 		q.Src = q.Src.WithPort(binary.BigEndian.Uint16(sub[0:2]))
 		q.Dst = q.Dst.WithPort(binary.BigEndian.Uint16(sub[2:4]))
-		q.TCPFlags = TCPFlag(sub[13]) & 0x3F
+		q.TCPFlags = TCPFlag(sub[13])
 		headerLength := (sub[12] & 0xF0) >> 2
 		q.dataofs = q.subofs + int(headerLength)
 		return
@@ -374,8 +378,14 @@ func (q *Parsed) Payload() []byte {
 	return q.b[q.dataofs:q.length]
 }
 
-// IsTCPSyn reports whether q is a TCP SYN packet
-// (i.e. the first packet in a new connection).
+// Transport returns the transport header and payload (IP subprotocol, such as TCP or UDP).
+// This is a read-only view; that is, p retains the ownership of the buffer.
+func (p *Parsed) Transport() []byte {
+	return p.b[p.subofs:]
+}
+
+// IsTCPSyn reports whether q is a TCP SYN packet,
+// without ACK set. (i.e. the first packet in a new connection)
 func (q *Parsed) IsTCPSyn() bool {
 	return (q.TCPFlags & TCPSynAck) == TCPSyn
 }
@@ -424,6 +434,40 @@ func (q *Parsed) IsEchoResponse() bool {
 	}
 }
 
+// RemoveECNBits modifies p and its underlying memory buffer to remove
+// ECN bits, if any. It reports whether it did so.
+//
+// It currently only does the TCP flags.
+func (p *Parsed) RemoveECNBits() bool {
+	if p.IPVersion == 0 {
+		return false
+	}
+	if p.IPProto != ipproto.TCP {
+		// TODO(bradfitz): handle non-TCP too? for now only trying to
+		// fix the Issue 2642 problem.
+		return false
+	}
+	if p.TCPFlags&TCPECNBits == 0 {
+		// Nothing to do.
+		return false
+	}
+
+	// Clear flags.
+
+	// First in the parsed output.
+	p.TCPFlags = p.TCPFlags & ^TCPECNBits
+
+	// Then in the underlying memory.
+	tcp := p.Transport()
+	old := binary.BigEndian.Uint16(tcp[12:14])
+	tcp[13] = byte(p.TCPFlags)
+	new := binary.BigEndian.Uint16(tcp[12:14])
+	oldSum := binary.BigEndian.Uint16(tcp[16:18])
+	newSum := ^checksumUpdate2ByteAlignedUint16(^oldSum, old, new)
+	binary.BigEndian.PutUint16(tcp[16:18], newSum)
+	return true
+}
+
 func Hexdump(b []byte) string {
 	out := new(strings.Builder)
 	for i := 0; i < len(b); i += 16 {
@@ -455,3 +499,26 @@ func Hexdump(b []byte) string {
 	}
 	return out.String()
 }
+
+// From gVisor's unexported API:
+
+// checksumUpdate2ByteAlignedUint16 updates a uint16 value in a calculated
+// checksum.
+//
+// The value MUST begin at a 2-byte boundary in the original buffer.
+func checksumUpdate2ByteAlignedUint16(xsum, old, new uint16) uint16 {
+	// As per RFC 1071 page 4,
+	//(4)  Incremental Update
+	//
+	//        ...
+	//
+	//        To update the checksum, simply add the differences of the
+	//        sixteen bit integers that have been changed.  To see why this
+	//        works, observe that every 16-bit integer has an additive inverse
+	//        and that addition is associative.  From this it follows that
+	//        given the original value m, the new value m', and the old
+	//        checksum C, the new checksum C' is:
+	//
+	//                C' = C + (-m) + m' = C + (m' - m)
+	return checksumCombine(xsum, checksumCombine(new, ^old))
+}

net/packet/packet_test.go

@@ -6,7 +6,9 @@ package packet
 
 import (
 	"bytes"
+	"encoding/hex"
 	"reflect"
+	"regexp"
 	"testing"
 
 	"inet.af/netaddr"
@@ -561,3 +563,57 @@ func BenchmarkString(b *testing.B) {
 		})
 	}
 }
+
+func TestRemoveECNBits(t *testing.T) {
+	// withECNHex is a TCP SYN packet with ECN bits set in the TCP
+	// header as captured by Wireshark on macOS against the
+	// Tailscale interface. In this packet (because it's a SYN
+	// control packet), the ECN bits are not set in the IP header.
+	const withECNHex = `45 00 00 40 00 00 40 00
+ 40 06 0c 66 64 7b 65 28 64 7f 00 30 f1 ab 00 16
+ 5a 7a 63 e8 00 00 00 00 b0 c2 ff ff 97 76 00 00
+ 02 04 04 d8 01 03 03 06 01 01 08 0a 03 e1 bd 49
+ 00 00 00 00 04 02 00 00`
+
+	// Generated by hand-editing a pcap file in hexl-mode to set
+	// the TCP flags to just SYN (0x02), then loading that pcap
+	// file in wireshark to get the expected checksum value, then
+	// putting that checksum value (0x9836) in the file.
+	const wantStrippedHex = `45 00 00 40 00 00 40 00
+ 40 06 0c 66 64 7b 65 28 64 7f 00 30 f1 ab 00 16
+ 5a 7a 63 e8 00 00 00 00 b0 02 ff ff 98 36 00 00
+ 02 04 04 d8 01 03 03 06 01 01 08 0a 03 e1 bd 49
+ 00 00 00 00 04 02 00 00`
+
+	var p Parsed
+	pktBuf := bytesOfHex(withECNHex)
+	p.Decode(pktBuf)
+	if want := TCPCWR | TCPECNEcho | TCPSyn; p.TCPFlags != want {
+		t.Fatalf("pre flags = %v; want %v", p.TCPFlags, want)
+	}
+
+	if !p.RemoveECNBits() {
+		t.Fatal("didn't remove bits")
+	}
+	if want := TCPSyn; p.TCPFlags != want {
+		t.Fatalf("post flags = %v; want %v", p.TCPFlags, want)
+	}
+	wantPkt := bytesOfHex(wantStrippedHex)
+	if !bytes.Equal(pktBuf, wantPkt) {
+		t.Fatalf("wrong result.\n got: % 2x\nwant: % 2x\n", pktBuf, wantPkt)
+	}
+
+	if p.RemoveECNBits() {
+		t.Fatal("unexpected true return value on second call")
+	}
+}
+
+var nonHex = regexp.MustCompile(`[^0-9a-fA-F]+`)
+
+func bytesOfHex(s string) []byte {
+	b, err := hex.DecodeString(nonHex.ReplaceAllString(s, ""))
+	if err != nil {
+		panic(err)
+	}
+	return b
+}

tailcfg/tailcfg.go

@@ -48,7 +48,8 @@ import (
 //    23: 2021-08-25: DNSConfig.Routes values may be empty (for ExtraRecords support in 1.14.1+)
 //    24: 2021-09-18: MapResponse.Health from control to node; node shows in "tailscale status"
 //    25: 2021-11-01: MapResponse.Debug.Exit
-const CurrentMapRequestVersion = 25
+//    26: 2022-01-12: (nothing, just bumping for 1.20.0)
+const CurrentMapRequestVersion = 26
 
 type StableID string
 

wgengine/magicsock/magicsock.go

@@ -265,6 +265,7 @@ type Conn struct {
 	stunReceiveFunc atomic.Value // of func(p []byte, fromAddr *net.UDPAddr)
 
 	// derpRecvCh is used by receiveDERP to read DERP messages.
+	// It must have buffer size > 0; see issue 3736.
 	derpRecvCh chan derpReadResult
 
 	// bind is the wireguard-go conn.Bind for Conn.
@@ -299,11 +300,18 @@ type Conn struct {
 	havePrivateKey  syncs.AtomicBool
 	publicKeyAtomic atomic.Value // of key.NodePublic (or NodeKey zero value if !havePrivateKey)
 
+	// derpMapAtomic is the same as derpMap, but without requiring
+	// sync.Mutex. For use with NewRegionClient's callback, to avoid
+	// lock ordering deadlocks. See issue 3726 and mu field docs.
+	derpMapAtomic atomic.Value // of *tailcfg.DERPMap
+
 	// port is the preferred port from opts.Port; 0 means auto.
 	port syncs.AtomicUint32
 
 	// ============================================================
-	// mu guards all following fields; see userspaceEngine lock ordering rules
+	// mu guards all following fields; see userspaceEngine lock
+	// ordering rules against the engine. For derphttp, mu must
+	// be held before derphttp.Client.mu.
 	mu     sync.Mutex
 	muCond *sync.Cond
 
@@ -522,7 +530,7 @@ func (o *Options) derpActiveFunc() func() {
 // of NewConn. Mostly for tests.
 func newConn() *Conn {
 	c := &Conn{
-		derpRecvCh:   make(chan derpReadResult),
+		derpRecvCh:   make(chan derpReadResult, 1), // must be buffered, see issue 3736
 		derpStarted:  make(chan struct{}),
 		peerLastDerp: make(map[key.NodePublic]int),
 		peerMap:      newPeerMap(),
@@ -1351,19 +1359,23 @@ func (c *Conn) derpWriteChanOfAddr(addr netaddr.IPPort, peer key.NodePublic) cha
 	}
 
 	// Note that derphttp.NewRegionClient does not dial the server
-	// so it is safe to do under the mu lock.
+	// (it doesn't block) so it is safe to do under the c.mu lock.
 	dc := derphttp.NewRegionClient(c.privateKey, c.logf, func() *tailcfg.DERPRegion {
+		// Warning: it is not legal to acquire
+		// magicsock.Conn.mu from this callback.
+		// It's run from derphttp.Client.connect (via Send, etc)
+		// and the lock ordering rules are that magicsock.Conn.mu
+		// must be acquired before derphttp.Client.mu.
+		// See https://github.com/tailscale/tailscale/issues/3726
 		if c.connCtx.Err() != nil {
-			// If we're closing, don't try to acquire the lock.
-			// We might already be in Conn.Close and the Lock would deadlock.
+			// We're closing anyway; return nil to stop dialing.
 			return nil
 		}
-		c.mu.Lock()
-		defer c.mu.Unlock()
-		if c.derpMap == nil {
+		derpMap, _ := c.derpMapAtomic.Load().(*tailcfg.DERPMap)
+		if derpMap == nil {
 			return nil
 		}
-		return c.derpMap.Regions[regionID]
+		return derpMap.Regions[regionID]
 	})
 
 	dc.SetCanAckPings(true)
@@ -2252,6 +2264,7 @@ func (c *Conn) SetDERPMap(dm *tailcfg.DERPMap) {
 		return
 	}
 
+	c.derpMapAtomic.Store(dm)
 	old := c.derpMap
 	c.derpMap = dm
 	if dm == nil {
@@ -2629,6 +2642,7 @@ func (c *connBind) Close() error {
 	}
 	// Send an empty read result to unblock receiveDERP,
 	// which will then check connBind.Closed.
+	// connBind.Closed takes c.mu, but c.derpRecvCh is buffered.
 	c.derpRecvCh <- derpReadResult{}
 	return nil
 }

wgengine/netstack/netstack.go

@@ -41,6 +41,7 @@ import (
 	"tailscale.com/syncs"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
+	"tailscale.com/version/distro"
 	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/magicsock"
@@ -71,13 +72,15 @@ type Impl struct {
 	// It can only be set before calling Start.
 	ProcessSubnets bool
 
-	ipstack *stack.Stack
-	linkEP  *channel.Endpoint
-	tundev  *tstun.Wrapper
-	e       wgengine.Engine
-	mc      *magicsock.Conn
-	logf    logger.Logf
-	dialer  *tsdial.Dialer
+	ipstack   *stack.Stack
+	linkEP    *channel.Endpoint
+	tundev    *tstun.Wrapper
+	e         wgengine.Engine
+	mc        *magicsock.Conn
+	logf      logger.Logf
+	dialer    *tsdial.Dialer
+	ctx       context.Context    // alive until Close
+	ctxCancel context.CancelFunc // called on Close
 
 	// atomicIsLocalIPFunc holds a func that reports whether an IP
 	// is a local (non-subnet) Tailscale IP address of this
@@ -151,10 +154,16 @@ func Create(logf logger.Logf, tundev *tstun.Wrapper, e wgengine.Engine, mc *magi
 		dialer:              dialer,
 		connsOpenBySubnetIP: make(map[netaddr.IP]int),
 	}
+	ns.ctx, ns.ctxCancel = context.WithCancel(context.Background())
 	ns.atomicIsLocalIPFunc.Store(tsaddr.NewContainsIPFunc(nil))
 	return ns, nil
 }
 
+func (ns *Impl) Close() error {
+	ns.ctxCancel()
+	return nil
+}
+
 // wrapProtoHandler returns protocol handler h wrapped in a version
 // that dynamically reconfigures ns's subnet addresses as needed for
 // outbound traffic.
@@ -346,8 +355,12 @@ func (ns *Impl) DialContextUDP(ctx context.Context, ipp netaddr.IPPort) (*gonet.
 
 func (ns *Impl) injectOutbound() {
 	for {
-		packetInfo, ok := ns.linkEP.ReadContext(context.Background())
+		packetInfo, ok := ns.linkEP.ReadContext(ns.ctx)
 		if !ok {
+			if ns.ctx.Err() != nil {
+				// Return without logging.
+				return
+			}
 			ns.logf("[v2] ReadContext-for-write = ok=false")
 			continue
 		}
@@ -394,8 +407,14 @@ func (ns *Impl) shouldProcessInbound(p *packet.Parsed, t *tstun.Wrapper) bool {
 	return false
 }
 
+// setAmbientCapsRaw is non-nil on Linux for Synology, to run ping with
+// CAP_NET_RAW from tailscaled's binary.
+var setAmbientCapsRaw func(*exec.Cmd)
+
 var userPingSem = syncs.NewSemaphore(20) // 20 child ping processes at once
 
+var isSynology = runtime.GOOS == "linux" && distro.Get() == distro.Synology
+
 // userPing tried to ping dstIP and if it succeeds, injects pingResPkt
 // into the tundev.
 //
@@ -419,6 +438,11 @@ func (ns *Impl) userPing(dstIP netaddr.IP, pingResPkt []byte) {
 	switch runtime.GOOS {
 	case "windows":
 		err = exec.Command("ping", "-n", "1", "-w", "3000", dstIP.String()).Run()
+	case "darwin":
+		// Note: 2000 ms is actually 1 second + 2,000
+		// milliseconds extra for 3 seconds total.
+		// See https://github.com/tailscale/tailscale/pull/3753 for details.
+		err = exec.Command("ping", "-c", "1", "-W", "2000", dstIP.String()).Run()
 	case "android":
 		ping := "/system/bin/ping"
 		if dstIP.Is6() {
@@ -426,11 +450,29 @@ func (ns *Impl) userPing(dstIP netaddr.IP, pingResPkt []byte) {
 		}
 		err = exec.Command(ping, "-c", "1", "-w", "3", dstIP.String()).Run()
 	default:
-		err = exec.Command("ping", "-c", "1", "-W", "3", dstIP.String()).Run()
+		ping := "ping"
+		if isSynology {
+			ping = "/bin/ping"
+		}
+		cmd := exec.Command(ping, "-c", "1", "-W", "3", dstIP.String())
+		if isSynology && os.Getuid() != 0 {
+			// On DSM7 we run as non-root and need to pass
+			// CAP_NET_RAW if our binary has it.
+			setAmbientCapsRaw(cmd)
+		}
+		err = cmd.Run()
 	}
 	d := time.Since(t0)
 	if err != nil {
-		ns.logf("exec ping of %v failed in %v", dstIP, d)
+		if d < time.Second/2 {
+			// If it failed quicker than the 3 second
+			// timeout we gave above (500 ms is a
+			// reasonable threshold), then assume the ping
+			// failed for problems finding/running
+			// ping. We don't want to log if the host is
+			// just down.
+			ns.logf("exec ping of %v failed in %v: %v", dstIP, d, err)
+		}
 		return
 	}
 	if debugNetstack {
@@ -470,6 +512,7 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons
 	case 6:
 		pn = header.IPv6ProtocolNumber
 	}
+	p.RemoveECNBits() // Issue 2642
 	if debugPackets {
 		ns.logf("[v2] packet in (from %v): % x", p.Src, p.Buffer())
 	}
@@ -478,6 +521,7 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons
 		Data: vv,
 	})
 	ns.linkEP.InjectInbound(pn, packetBuf)
+	packetBuf.DecRef()
 
 	// We've now delivered this to netstack, so we're done.
 	// Instead of returning a filter.Accept here (which would also
@@ -508,7 +552,7 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 	clientRemoteIP := netaddrIPFromNetstackIP(reqDetails.RemoteAddress)
 	if !clientRemoteIP.IsValid() {
 		ns.logf("invalid RemoteAddress in TCP ForwarderRequest: %s", stringifyTEI(reqDetails))
-		r.Complete(true)
+		r.Complete(true) // sends a RST
 		return
 	}
 
@@ -524,7 +568,8 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 	var wq waiter.Queue
 	ep, err := r.CreateEndpoint(&wq)
 	if err != nil {
-		r.Complete(true)
+		ns.logf("CreateEndpoint error for %s: %v", stringifyTEI(reqDetails), err)
+		r.Complete(true) // sends a RST
 		return
 	}
 	r.Complete(false)

wgengine/netstack/netstack_linux.go

@@ -0,0 +1,20 @@
+// 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 netstack
+
+import (
+	"os/exec"
+	"syscall"
+
+	"golang.org/x/sys/unix"
+)
+
+func init() {
+	setAmbientCapsRaw = func(cmd *exec.Cmd) {
+		cmd.SysProcAttr = &syscall.SysProcAttr{
+			AmbientCaps: []uintptr{unix.CAP_NET_RAW},
+		}
+	}
+}

wgengine/netstack/netstack_test.go

@@ -0,0 +1,76 @@
+// 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 netstack
+
+import (
+	"runtime"
+	"testing"
+
+	"inet.af/netaddr"
+	"tailscale.com/net/packet"
+	"tailscale.com/net/tsdial"
+	"tailscale.com/net/tstun"
+	"tailscale.com/wgengine"
+	"tailscale.com/wgengine/filter"
+)
+
+// TestInjectInboundLeak tests that injectInbound doesn't leak memory.
+// See https://github.com/tailscale/tailscale/issues/3762
+func TestInjectInboundLeak(t *testing.T) {
+	tunDev := tstun.NewFake()
+	dialer := new(tsdial.Dialer)
+	logf := func(format string, args ...interface{}) {
+		if !t.Failed() {
+			t.Logf(format, args...)
+		}
+	}
+	eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{
+		Tun:    tunDev,
+		Dialer: dialer,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer eng.Close()
+	ig, ok := eng.(wgengine.InternalsGetter)
+	if !ok {
+		t.Fatal("not an InternalsGetter")
+	}
+	tunWrap, magicSock, ok := ig.GetInternals()
+	if !ok {
+		t.Fatal("failed to get internals")
+	}
+
+	ns, err := Create(logf, tunWrap, eng, magicSock, dialer)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ns.Close()
+	ns.ProcessLocalIPs = true
+	if err := ns.Start(); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	ns.atomicIsLocalIPFunc.Store(func(netaddr.IP) bool { return true })
+
+	pkt := &packet.Parsed{}
+	const N = 10_000
+	ms0 := getMemStats()
+	for i := 0; i < N; i++ {
+		outcome := ns.injectInbound(pkt, tunWrap)
+		if outcome != filter.DropSilently {
+			t.Fatalf("got outcome %v; want DropSilently", outcome)
+		}
+	}
+	ms1 := getMemStats()
+	if grew := int64(ms1.HeapObjects) - int64(ms0.HeapObjects); grew >= N {
+		t.Fatalf("grew by %v (which is too much and >= the %v packets we sent)", grew, N)
+	}
+}
+
+func getMemStats() (ms runtime.MemStats) {
+	runtime.GC()
+	runtime.ReadMemStats(&ms)
+	return
+}