wgengine/magicsock: add a stress test

Updates tailscale/corp#3016

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

Dockerfile

@@ -59,8 +59,8 @@ RUN go install -tags=xversion -ldflags="\
       -X tailscale.com/version.Long=$VERSION_LONG \
       -X tailscale.com/version.Short=$VERSION_SHORT \
       -X tailscale.com/version.GitCommit=$VERSION_GIT_HASH" \
-      -v ./cmd/...
+      -v ./cmd/tailscale ./cmd/tailscaled
 
 FROM alpine:3.14
-RUN apk add --no-cache ca-certificates iptables iproute2
+RUN apk add --no-cache ca-certificates iptables iproute2 ip6tables
 COPY --from=build-env /go/bin/* /usr/local/bin/

VERSION.txt

@@ -1 +1 @@
-1.15.0
+1.16.2

disco/disco.go

@@ -26,6 +26,7 @@ import (
 	"net"
 
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
 )
 
 // Magic is the 6 byte header of all discovery messages.
@@ -106,12 +107,28 @@ func appendMsgHeader(b []byte, t MessageType, ver uint8, dataLen int) (all, data
 }
 
 type Ping struct {
+	// TxID is a random client-generated per-ping transaction ID.
 	TxID [12]byte
+
+	// NodeKey is allegedly the ping sender's wireguard public key.
+	// Old clients (~1.16.0 and earlier) don't send this field.
+	// It shouldn't be trusted by itself, but can be combined with
+	// netmap data to reduce the discokey:nodekey relation from 1:N to
+	// 1:1.
+	NodeKey tailcfg.NodeKey
 }
 
 func (m *Ping) AppendMarshal(b []byte) []byte {
-	ret, d := appendMsgHeader(b, TypePing, v0, 12)
-	copy(d, m.TxID[:])
+	dataLen := 12
+	hasKey := !m.NodeKey.IsZero()
+	if hasKey {
+		dataLen += len(m.NodeKey)
+	}
+	ret, d := appendMsgHeader(b, TypePing, v0, dataLen)
+	n := copy(d, m.TxID[:])
+	if hasKey {
+		copy(d[n:], m.NodeKey[:])
+	}
 	return ret
 }
 
@@ -120,7 +137,10 @@ func parsePing(ver uint8, p []byte) (m *Ping, err error) {
 		return nil, errShort
 	}
 	m = new(Ping)
-	copy(m.TxID[:], p)
+	p = p[copy(m.TxID[:], p):]
+	if len(p) >= len(m.NodeKey) {
+		copy(m.NodeKey[:], p)
+	}
 	return m, nil
 }
 

disco/disco_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
 )
 
 func TestMarshalAndParse(t *testing.T) {
@@ -26,6 +27,19 @@ func TestMarshalAndParse(t *testing.T) {
 			},
 			want: "01 00 01 02 03 04 05 06 07 08 09 0a 0b 0c",
 		},
+		{
+			name: "ping_with_nodekey_src",
+			m: &Ping{
+				TxID: [12]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12},
+				NodeKey: tailcfg.NodeKey{
+					1:  1,
+					2:  2,
+					30: 30,
+					31: 31,
+				},
+			},
+			want: "01 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 00 01 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 1e 1f",
+		},
 		{
 			name: "pong",
 			m: &Pong{

hostinfo/hostinfo.go

@@ -28,7 +28,7 @@ func New() *tailcfg.Hostinfo {
 		IPNVersion:  version.Long,
 		Hostname:    hostname,
 		OS:          version.OS(),
-		OSVersion:   getOSVersion(),
+		OSVersion:   GetOSVersion(),
 		Package:     packageType(),
 		GoArch:      runtime.GOARCH,
 		DeviceModel: deviceModel(),
@@ -37,7 +37,8 @@ func New() *tailcfg.Hostinfo {
 
 var osVersion func() string // non-nil on some platforms
 
-func getOSVersion() string {
+// GetOSVersion returns the OSVersion of current host if available.
+func GetOSVersion() string {
 	if s, _ := osVersionAtomic.Load().(string); s != "" {
 		return s
 	}
@@ -82,6 +83,7 @@ const (
 	AzureAppService = EnvType("az")
 	AWSFargate      = EnvType("fg")
 	FlyDotIo        = EnvType("fly")
+	Kubernetes      = EnvType("k8s")
 )
 
 var envType atomic.Value // of EnvType
@@ -136,6 +138,9 @@ func getEnvType() EnvType {
 	if inFlyDotIo() {
 		return FlyDotIo
 	}
+	if inKubernetes() {
+		return Kubernetes
+	}
 	return ""
 }
 
@@ -212,3 +217,10 @@ func inFlyDotIo() bool {
 	}
 	return false
 }
+
+func inKubernetes() bool {
+	if os.Getenv("KUBERNETES_SERVICE_HOST") != "" && os.Getenv("KUBERNETES_SERVICE_PORT") != "" {
+		return true
+	}
+	return false
+}

ipn/ipnlocal/local.go

@@ -1022,14 +1022,29 @@ var removeFromDefaultRoute = []netaddr.IPPrefix{
 // Given that "internal" routes don't leave the device, we choose to
 // trust them more, allowing access to them when an Exit Node is enabled.
 func internalAndExternalInterfaces() (internal, external []netaddr.IPPrefix, err error) {
-	if err := interfaces.ForeachInterfaceAddress(func(iface interfaces.Interface, pfx netaddr.IPPrefix) {
+	il, err := interfaces.GetList()
+	if err != nil {
+		return nil, nil, err
+	}
+	return internalAndExternalInterfacesFrom(il, runtime.GOOS)
+}
+
+func internalAndExternalInterfacesFrom(il interfaces.List, goos string) (internal, external []netaddr.IPPrefix, err error) {
+	// We use an IPSetBuilder here to canonicalize the prefixes
+	// and to remove any duplicate entries.
+	var internalBuilder, externalBuilder netaddr.IPSetBuilder
+	if err := il.ForeachInterfaceAddress(func(iface interfaces.Interface, pfx netaddr.IPPrefix) {
 		if tsaddr.IsTailscaleIP(pfx.IP()) {
 			return
 		}
 		if pfx.IsSingleIP() {
 			return
 		}
-		if runtime.GOOS == "windows" {
+		if iface.IsLoopback() {
+			internalBuilder.AddPrefix(pfx)
+			return
+		}
+		if goos == "windows" {
 			// Windows Hyper-V prefixes all MAC addresses with 00:15:5d.
 			// https://docs.microsoft.com/en-us/troubleshoot/windows-server/virtualization/default-limit-256-dynamic-mac-addresses
 			//
@@ -1040,16 +1055,24 @@ func internalAndExternalInterfaces() (internal, external []netaddr.IPPrefix, err
 			// configuration breaks WSL2 DNS without this.
 			mac := iface.Interface.HardwareAddr
 			if len(mac) == 6 && mac[0] == 0x00 && mac[1] == 0x15 && mac[2] == 0x5d {
-				internal = append(internal, pfx)
+				internalBuilder.AddPrefix(pfx)
 				return
 			}
 		}
-		external = append(external, pfx)
+		externalBuilder.AddPrefix(pfx)
 	}); err != nil {
 		return nil, nil, err
 	}
+	iSet, err := internalBuilder.IPSet()
+	if err != nil {
+		return nil, nil, err
+	}
+	eSet, err := externalBuilder.IPSet()
+	if err != nil {
+		return nil, nil, err
+	}
 
-	return internal, external, nil
+	return iSet.Prefixes(), eSet.Prefixes(), nil
 }
 
 func interfaceRoutes() (ips *netaddr.IPSet, hostIPs []netaddr.IP, err error) {

ipn/ipnlocal/local_test.go

@@ -6,6 +6,7 @@ package ipnlocal
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 	"reflect"
 	"testing"
@@ -494,3 +495,103 @@ func TestFileTargets(t *testing.T) {
 	}
 	// (other cases handled by TestPeerAPIBase above)
 }
+
+func TestInternalAndExternalInterfaces(t *testing.T) {
+	type interfacePrefix struct {
+		i   interfaces.Interface
+		pfx netaddr.IPPrefix
+	}
+
+	masked := func(ips ...interfacePrefix) (pfxs []netaddr.IPPrefix) {
+		for _, ip := range ips {
+			pfxs = append(pfxs, ip.pfx.Masked())
+		}
+		return pfxs
+	}
+	iList := func(ips ...interfacePrefix) (il interfaces.List) {
+		for _, ip := range ips {
+			il = append(il, ip.i)
+		}
+		return il
+	}
+	newInterface := func(name, pfx string, wsl2, loopback bool) interfacePrefix {
+		ippfx := netaddr.MustParseIPPrefix(pfx)
+		ip := interfaces.Interface{
+			Interface: &net.Interface{},
+			AltAddrs: []net.Addr{
+				ippfx.IPNet(),
+			},
+		}
+		if loopback {
+			ip.Flags = net.FlagLoopback
+		}
+		if wsl2 {
+			ip.HardwareAddr = []byte{0x00, 0x15, 0x5d, 0x00, 0x00, 0x00}
+		}
+		return interfacePrefix{i: ip, pfx: ippfx}
+	}
+	var (
+		en0      = newInterface("en0", "10.20.2.5/16", false, false)
+		en1      = newInterface("en1", "192.168.1.237/24", false, false)
+		wsl      = newInterface("wsl", "192.168.5.34/24", true, false)
+		loopback = newInterface("lo0", "127.0.0.1/8", false, true)
+	)
+
+	tests := []struct {
+		name    string
+		goos    string
+		il      interfaces.List
+		wantInt []netaddr.IPPrefix
+		wantExt []netaddr.IPPrefix
+	}{
+		{
+			name: "single-interface",
+			goos: "linux",
+			il: iList(
+				en0,
+				loopback,
+			),
+			wantInt: masked(loopback),
+			wantExt: masked(en0),
+		},
+		{
+			name: "multiple-interfaces",
+			goos: "linux",
+			il: iList(
+				en0,
+				en1,
+				wsl,
+				loopback,
+			),
+			wantInt: masked(loopback),
+			wantExt: masked(en0, en1, wsl),
+		},
+		{
+			name: "wsl2",
+			goos: "windows",
+			il: iList(
+				en0,
+				en1,
+				wsl,
+				loopback,
+			),
+			wantInt: masked(loopback, wsl),
+			wantExt: masked(en0, en1),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			gotInt, gotExt, err := internalAndExternalInterfacesFrom(tc.il, tc.goos)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !reflect.DeepEqual(gotInt, tc.wantInt) {
+				t.Errorf("unexpected internal prefixes\ngot %v\nwant %v", gotInt, tc.wantInt)
+			}
+			if !reflect.DeepEqual(gotExt, tc.wantExt) {
+				t.Errorf("unexpected external prefixes\ngot %v\nwant %v", gotExt, tc.wantExt)
+			}
+		})
+	}
+}

ipn/prefs.go

@@ -234,6 +234,20 @@ func (m *MaskedPrefs) Pretty() string {
 	mt := mv.Type()
 	mpv := reflect.ValueOf(&m.Prefs).Elem()
 	first := true
+
+	format := func(v reflect.Value) string {
+		switch v.Type().Kind() {
+		case reflect.String:
+			return "%s=%q"
+		case reflect.Slice:
+			// []string
+			if v.Type().Elem().Kind() == reflect.String {
+				return "%s=%q"
+			}
+		}
+		return "%s=%v"
+	}
+
 	for i := 1; i < mt.NumField(); i++ {
 		name := mt.Field(i).Name
 		if mv.Field(i).Bool() {
@@ -241,9 +255,10 @@ func (m *MaskedPrefs) Pretty() string {
 				sb.WriteString(" ")
 			}
 			first = false
-			fmt.Fprintf(&sb, "%s=%#v",
+			f := mpv.Field(i - 1)
+			fmt.Fprintf(&sb, format(f),
 				strings.TrimSuffix(name, "Set"),
-				mpv.Field(i-1).Interface())
+				f.Interface())
 		}
 	}
 	sb.WriteString("}")

ipn/prefs_test.go

@@ -615,12 +615,27 @@ func TestMaskedPrefsPretty(t *testing.T) {
 					OperatorUser:     "galaxybrain",
 					AllowSingleHosts: true,
 					RouteAll:         false,
+					ExitNodeID:       "foo",
+					AdvertiseTags:    []string{"tag:foo", "tag:bar"},
+					NetfilterMode:    preftype.NetfilterNoDivert,
 				},
-				RouteAllSet:     true,
-				HostnameSet:     true,
-				OperatorUserSet: true,
+				RouteAllSet:      true,
+				HostnameSet:      true,
+				OperatorUserSet:  true,
+				ExitNodeIDSet:    true,
+				AdvertiseTagsSet: true,
+				NetfilterModeSet: true,
 			},
-			want: `MaskedPrefs{RouteAll=false Hostname="bar" OperatorUser="galaxybrain"}`,
+			want: `MaskedPrefs{RouteAll=false ExitNodeID="foo" AdvertiseTags=["tag:foo" "tag:bar"] Hostname="bar" NetfilterMode=nodivert OperatorUser="galaxybrain"}`,
+		},
+		{
+			m: &MaskedPrefs{
+				Prefs: Prefs{
+					ExitNodeIP: netaddr.IPv4(100, 102, 104, 105),
+				},
+				ExitNodeIPSet: true,
+			},
+			want: `MaskedPrefs{ExitNodeIP=100.102.104.105}`,
 		},
 	}
 	for i, tt := range tests {

logtail/filch/filch.go

@@ -17,8 +17,11 @@ import (
 
 var stderrFD = 2 // a variable for testing
 
+const defaultMaxFileSize = 50 << 20
+
 type Options struct {
 	ReplaceStderr bool // dup over fd 2 so everything written to stderr comes here
+	MaxFileSize   int
 }
 
 // A Filch uses two alternating files as a simplistic ring buffer.
@@ -30,6 +33,10 @@ type Filch struct {
 	alt       *os.File
 	altscan   *bufio.Scanner
 	recovered int64
+
+	maxFileSize  int64
+	writeCounter int
+
 	// buf is an initial buffer for altscan.
 	// As of August 2021, 99.96% of all log lines
 	// are below 4096 bytes in length.
@@ -38,7 +45,7 @@ type Filch struct {
 	// so that the whole struct takes 4096 bytes
 	// (less on 32 bit platforms).
 	// This reduces allocation waste.
-	buf [4096 - 48]byte
+	buf [4096 - 64]byte
 }
 
 // TryReadline implements the logtail.Buffer interface.
@@ -91,6 +98,22 @@ func (f *Filch) scan() ([]byte, error) {
 func (f *Filch) Write(b []byte) (int, error) {
 	f.mu.Lock()
 	defer f.mu.Unlock()
+	if f.writeCounter == 100 {
+		// Check the file size every 100 writes.
+		f.writeCounter = 0
+		fi, err := f.cur.Stat()
+		if err != nil {
+			return 0, err
+		}
+		if fi.Size() >= f.maxFileSize {
+			// This most likely means we are not draining.
+			// To limit the amount of space we use, throw away the old logs.
+			if err := moveContents(f.alt, f.cur); err != nil {
+				return 0, err
+			}
+		}
+	}
+	f.writeCounter++
 
 	if len(b) == 0 || b[len(b)-1] != '\n' {
 		bnl := make([]byte, len(b)+1)
@@ -159,8 +182,13 @@ func New(filePrefix string, opts Options) (f *Filch, err error) {
 		return nil, err
 	}
 
+	mfs := defaultMaxFileSize
+	if opts.MaxFileSize > 0 {
+		mfs = opts.MaxFileSize
+	}
 	f = &Filch{
-		OrigStderr: os.Stderr, // temporary, for past logs recovery
+		OrigStderr:  os.Stderr, // temporary, for past logs recovery
+		maxFileSize: int64(mfs),
 	}
 
 	// Neither, either, or both files may exist and contain logs from
@@ -234,6 +262,9 @@ func moveContents(dst, src *os.File) (err error) {
 	if _, err := src.Seek(0, io.SeekStart); err != nil {
 		return err
 	}
+	if _, err := dst.Seek(0, io.SeekStart); err != nil {
+		return err
+	}
 	if _, err := io.Copy(dst, src); err != nil {
 		return err
 	}

logtail/filch/filch_test.go

@@ -57,6 +57,39 @@ func (f *filchTest) close(t *testing.T) {
 	}
 }
 
+func TestDropOldLogs(t *testing.T) {
+	const line1 = "123456789" // 10 bytes (9+newline)
+	tests := []struct {
+		write, read int
+	}{
+		{10, 10},
+		{100, 100},
+		{200, 200},
+		{250, 150},
+		{500, 200},
+	}
+	for _, tc := range tests {
+		t.Run(fmt.Sprintf("w%d-r%d", tc.write, tc.read), func(t *testing.T) {
+			filePrefix := t.TempDir()
+			f := newFilchTest(t, filePrefix, Options{ReplaceStderr: false, MaxFileSize: 1000})
+			defer f.close(t)
+			// Make filch rotate the logs 3 times
+			for i := 0; i < tc.write; i++ {
+				f.write(t, line1)
+			}
+			// We should only be able to read the last 150 lines
+			for i := 0; i < tc.read; i++ {
+				f.read(t, line1)
+				if t.Failed() {
+					t.Logf("could only read %d lines", i)
+					break
+				}
+			}
+			f.readEOF(t)
+		})
+	}
+}
+
 func TestQueue(t *testing.T) {
 	filePrefix := t.TempDir()
 	f := newFilchTest(t, filePrefix, Options{ReplaceStderr: false})

net/dns/resolver/forwarder.go

@@ -17,12 +17,14 @@ import (
 	"net/http"
 	"runtime"
 	"sort"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
 
 	dns "golang.org/x/net/dns/dnsmessage"
 	"inet.af/netaddr"
+	"tailscale.com/hostinfo"
 	"tailscale.com/net/netns"
 	"tailscale.com/types/dnstype"
 	"tailscale.com/types/logger"
@@ -170,19 +172,37 @@ func init() {
 	rand.Seed(time.Now().UnixNano())
 }
 
-func newForwarder(logf logger.Logf, responses chan packet, linkMon *monitor.Mon, linkSel ForwardLinkSelector) *forwarder {
-	maxDoHInFlight := 1000 // effectively unlimited
-	if runtime.GOOS == "ios" {
-		// No HTTP/2 on iOS yet (for size reasons), so DoH is
-		// pricier.
-		maxDoHInFlight = 10
+func maxDoHInFlight(goos string) int {
+	if goos != "ios" {
+		return 1000 // effectively unlimited
 	}
+	// iOS <  15 limits the memory to 15MB for NetworkExtensions.
+	// iOS >= 15 gives us 50MB.
+	// See: https://tailscale.com/blog/go-linker/
+	ver := hostinfo.GetOSVersion()
+	if ver == "" {
+		// Unknown iOS version, be cautious.
+		return 10
+	}
+	idx := strings.Index(ver, ".")
+	if idx == -1 {
+		// Unknown iOS version, be cautious.
+		return 10
+	}
+	major := ver[:idx]
+	if m, err := strconv.Atoi(major); err != nil || m < 15 {
+		return 10
+	}
+	return 1000
+}
+
+func newForwarder(logf logger.Logf, responses chan packet, linkMon *monitor.Mon, linkSel ForwardLinkSelector) *forwarder {
 	f := &forwarder{
 		logf:      logger.WithPrefix(logf, "forward: "),
 		linkMon:   linkMon,
 		linkSel:   linkSel,
 		responses: responses,
-		dohSem:    make(chan struct{}, maxDoHInFlight),
+		dohSem:    make(chan struct{}, maxDoHInFlight(runtime.GOOS)),
 	}
 	f.ctx, f.ctxCancel = context.WithCancel(context.Background())
 	return f
@@ -528,7 +548,6 @@ func (f *forwarder) forward(query packet) error {
 	switch runtime.GOOS {
 	case "ios", "darwin":
 		if hasRDNSBonjourPrefix(domain) {
-			f.logf("[v1] dropping %q", domain)
 			return nil
 		}
 	}

net/dns/resolver/forwarder_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	"tailscale.com/hostinfo"
 	"tailscale.com/types/dnstype"
 )
 
@@ -97,3 +98,30 @@ func TestResolversWithDelays(t *testing.T) {
 	}
 
 }
+
+func TestMaxDoHInFlight(t *testing.T) {
+	tests := []struct {
+		goos string
+		ver  string
+		want int
+	}{
+		{"ios", "", 10},
+		{"ios", "1532", 10},
+		{"ios", "9.3.2", 10},
+		{"ios", "14.3.2", 10},
+		{"ios", "15.3.2", 1000},
+		{"ios", "20.3.2", 1000},
+		{"android", "", 1000},
+		{"darwin", "", 1000},
+		{"linux", "", 1000},
+	}
+	for _, tc := range tests {
+		t.Run(fmt.Sprintf("%s-%s", tc.goos, tc.ver), func(t *testing.T) {
+			hostinfo.SetOSVersion(tc.ver)
+			got := maxDoHInFlight(tc.goos)
+			if got != tc.want {
+				t.Errorf("got %d; want %d", got, tc.want)
+			}
+		})
+	}
+}

net/dns/resolver/tsdns.go

@@ -598,11 +598,6 @@ const (
 //  dr._dns-sd._udp.<domain>.
 //  lb._dns-sd._udp.<domain>.
 func hasRDNSBonjourPrefix(name dnsname.FQDN) bool {
-	// Even the shortest name containing a Bonjour prefix is long,
-	// so check length (cheap) and bail early if possible.
-	if len(name) < len("*._dns-sd._udp.0.0.0.0.in-addr.arpa.") {
-		return false
-	}
 	s := name.WithTrailingDot()
 	dot := strings.IndexByte(s, '.')
 	if dot == -1 {

net/dns/resolver/tsdns_test.go

@@ -985,6 +985,7 @@ func TestTrimRDNSBonjourPrefix(t *testing.T) {
 		{"lb._dns-sd._udp.0.10.20.172.in-addr.arpa.", true},
 		{"qq._dns-sd._udp.0.10.20.172.in-addr.arpa.", false},
 		{"0.10.20.172.in-addr.arpa.", false},
+		{"lb._dns-sd._udp.ts-dns.test.", true},
 	}
 
 	for _, test := range tests {

net/interfaces/interfaces.go

@@ -183,14 +183,20 @@ func (i Interface) Addrs() ([]net.Addr, error) {
 	return i.Interface.Addrs()
 }
 
-// ForeachInterfaceAddress calls fn for each interface's address on
-// the machine. The IPPrefix's IP is the IP address assigned to the
-// interface, and Bits are the subnet mask.
+// ForeachInterfaceAddress is a wrapper for GetList, then
+// List.ForeachInterfaceAddress.
 func ForeachInterfaceAddress(fn func(Interface, netaddr.IPPrefix)) error {
-	ifaces, err := netInterfaces()
+	ifaces, err := GetList()
 	if err != nil {
 		return err
 	}
+	return ifaces.ForeachInterfaceAddress(fn)
+}
+
+// ForeachInterfaceAddress calls fn for each interface in ifaces, with
+// all its addresses. The IPPrefix's IP is the IP address assigned to
+// the interface, and Bits are the subnet mask.
+func (ifaces List) ForeachInterfaceAddress(fn func(Interface, netaddr.IPPrefix)) error {
 	for _, iface := range ifaces {
 		addrs, err := iface.Addrs()
 		if err != nil {
@@ -208,11 +214,21 @@ func ForeachInterfaceAddress(fn func(Interface, netaddr.IPPrefix)) error {
 	return nil
 }
 
-// ForeachInterface calls fn for each interface on the machine, with
+// ForeachInterface is a wrapper for GetList, then
+// List.ForeachInterface.
+func ForeachInterface(fn func(Interface, []netaddr.IPPrefix)) error {
+	ifaces, err := GetList()
+	if err != nil {
+		return err
+	}
+	return ifaces.ForeachInterface(fn)
+}
+
+// ForeachInterface calls fn for each interface in ifaces, with
 // all its addresses. The IPPrefix's IP is the IP address assigned to
 // the interface, and Bits are the subnet mask.
-func ForeachInterface(fn func(Interface, []netaddr.IPPrefix)) error {
-	ifaces, err := netInterfaces()
+func (ifaces List) ForeachInterface(fn func(Interface, []netaddr.IPPrefix)) error {
+	ifaces, err := GetList()
 	if err != nil {
 		return err
 	}
@@ -589,6 +605,14 @@ func RegisterInterfaceGetter(getInterfaces func() ([]Interface, error)) {
 	altNetInterfaces = getInterfaces
 }
 
+// List is a list of interfaces on the machine.
+type List []Interface
+
+// GetList returns the list of interfaces on the machine.
+func GetList() (List, error) {
+	return netInterfaces()
+}
+
 // netInterfaces is a wrapper around the standard library's net.Interfaces
 // that returns a []*Interface instead of a []net.Interface.
 // It exists because Android SDK 30 no longer permits Go's net.Interfaces

net/portmapper/portmapper.go

@@ -677,6 +677,7 @@ func (c *Client) Probe(ctx context.Context) (res ProbeResult, err error) {
 
 	pxpAddr := netaddr.IPPortFrom(gw, c.pxpPort()).UDPAddr()
 	upnpAddr := netaddr.IPPortFrom(gw, c.upnpPort()).UDPAddr()
+	upnpMulticastAddr := netaddr.IPPortFrom(netaddr.IPv4(239, 255, 255, 250), c.upnpPort()).UDPAddr()
 
 	// Don't send probes to services that we recently learned (for
 	// the same gw/myIP) are available. See
@@ -694,7 +695,47 @@ func (c *Client) Probe(ctx context.Context) (res ProbeResult, err error) {
 	if c.sawUPnPRecently() {
 		res.UPnP = true
 	} else if !DisableUPnP {
+		// Strictly speaking, you discover UPnP services by sending an
+		// SSDP query (which uPnPPacket is) to udp/1900 on the SSDP
+		// multicast address, and then get a flood of responses back
+		// from everything on your network.
+		//
+		// Empirically, many home routers also respond to SSDP queries
+		// directed at udp/1900 on their LAN unicast IP
+		// (e.g. 192.168.1.1). This is handy because it means we can
+		// probe the router directly and likely get a reply. However,
+		// the specs do not _require_ UPnP devices to respond to
+		// unicast SSDP queries, so some conformant UPnP
+		// implementations only respond to multicast queries.
+		//
+		// In theory, we could send just the multicast query and get
+		// all compliant devices to respond. However, we instead send
+		// to both a unicast and a multicast addresses, for a couple
+		// of reasons:
+		//
+		// First, some LANs and OSes have broken multicast in one way
+		// or another, so it's possible for the multicast query to be
+		// lost while the unicast query gets through. But we still
+		// have to send the multicast query to also get a response
+		// from strict-UPnP devices on multicast-working networks.
+		//
+		// Second, SSDP's packet dynamics are a bit weird: you send
+		// the SSDP query from your unicast IP to the SSDP multicast
+		// IP, but responses are from the UPnP devices's _unicast_ IP
+		// to your unicast IP. This can confuse some less-intelligent
+		// stateful host firewalls, who might block the responses. To
+		// work around this, we send the unicast query first, to teach
+		// the firewall to expect a unicast response from the router,
+		// and then send our multicast query. That way, even if the
+		// device doesn't respond to the unicast query, we've set the
+		// stage for the host firewall to accept the response to the
+		// multicast query.
+		//
+		// See https://github.com/tailscale/tailscale/issues/3197 for
+		// an example of a device that strictly implements UPnP, and
+		// only responds to multicast queries.
 		uc.WriteTo(uPnPPacket, upnpAddr)
+		uc.WriteTo(uPnPPacket, upnpMulticastAddr)
 	}
 
 	buf := make([]byte, 1500)
@@ -711,10 +752,14 @@ func (c *Client) Probe(ctx context.Context) (res ProbeResult, err error) {
 			}
 			return res, err
 		}
+		ip, ok := netaddr.FromStdIP(addr.(*net.UDPAddr).IP)
+		if !ok {
+			continue
+		}
 		port := uint16(addr.(*net.UDPAddr).Port)
 		switch port {
 		case c.upnpPort():
-			if mem.Contains(mem.B(buf[:n]), mem.S(":InternetGatewayDevice:")) {
+			if ip == gw && mem.Contains(mem.B(buf[:n]), mem.S(":InternetGatewayDevice:")) {
 				meta, err := parseUPnPDiscoResponse(buf[:n])
 				if err != nil {
 					c.logf("unrecognized UPnP discovery response; ignoring")

wgengine/magicsock/magicsock.go

@@ -72,7 +72,7 @@ func useDerpRoute() bool {
 // peerInfo is all the information magicsock tracks about a particular
 // peer.
 type peerInfo struct {
-	ep *endpoint // optional, if wireguard-go isn't currently talking to this peer.
+	ep *endpoint // always non-nil.
 	// ipPorts is an inverted version of peerMap.byIPPort (below), so
 	// that when we're deleting this node, we can rapidly find out the
 	// keys that need deleting from peerMap.byIPPort without having to
@@ -80,8 +80,9 @@ type peerInfo struct {
 	ipPorts map[netaddr.IPPort]bool
 }
 
-func newPeerInfo() *peerInfo {
+func newPeerInfo(ep *endpoint) *peerInfo {
 	return &peerInfo{
+		ep:      ep,
 		ipPorts: map[netaddr.IPPort]bool{},
 	}
 }
@@ -91,16 +92,19 @@ func newPeerInfo() *peerInfo {
 //
 // Doesn't do any locking, all access must be done with Conn.mu held.
 type peerMap struct {
-	byDiscoKey map[tailcfg.DiscoKey]*peerInfo
-	byNodeKey  map[tailcfg.NodeKey]*peerInfo
-	byIPPort   map[netaddr.IPPort]*peerInfo
+	byNodeKey map[tailcfg.NodeKey]*peerInfo
+	byIPPort  map[netaddr.IPPort]*peerInfo
+
+	// nodesOfDisco are contains the set of nodes that are using a
+	// DiscoKey. Usually those sets will be just one node.
+	nodesOfDisco map[tailcfg.DiscoKey]map[tailcfg.NodeKey]bool
 }
 
 func newPeerMap() peerMap {
 	return peerMap{
-		byDiscoKey: map[tailcfg.DiscoKey]*peerInfo{},
-		byNodeKey:  map[tailcfg.NodeKey]*peerInfo{},
-		byIPPort:   map[netaddr.IPPort]*peerInfo{},
+		byNodeKey:    map[tailcfg.NodeKey]*peerInfo{},
+		byIPPort:     map[netaddr.IPPort]*peerInfo{},
+		nodesOfDisco: map[tailcfg.DiscoKey]map[tailcfg.NodeKey]bool{},
 	}
 }
 
@@ -109,16 +113,10 @@ func (m *peerMap) nodeCount() int {
 	return len(m.byNodeKey)
 }
 
-// endpointForDiscoKey returns the endpoint for dk, or nil
-// if dk is not known to us.
-func (m *peerMap) endpointForDiscoKey(dk tailcfg.DiscoKey) (ep *endpoint, ok bool) {
-	if dk.IsZero() {
-		return nil, false
-	}
-	if info, ok := m.byDiscoKey[dk]; ok && info.ep != nil {
-		return info.ep, true
-	}
-	return nil, false
+// anyEndpointForDiscoKey reports whether there exists any
+// peers in the netmap with dk as their DiscoKey.
+func (m *peerMap) anyEndpointForDiscoKey(dk tailcfg.DiscoKey) bool {
+	return len(m.nodesOfDisco[dk]) > 0
 }
 
 // endpointForNodeKey returns the endpoint for nk, or nil if
@@ -127,7 +125,7 @@ func (m *peerMap) endpointForNodeKey(nk tailcfg.NodeKey) (ep *endpoint, ok bool)
 	if nk.IsZero() {
 		return nil, false
 	}
-	if info, ok := m.byNodeKey[nk]; ok && info.ep != nil {
+	if info, ok := m.byNodeKey[nk]; ok {
 		return info.ep, true
 	}
 	return nil, false
@@ -136,61 +134,87 @@ func (m *peerMap) endpointForNodeKey(nk tailcfg.NodeKey) (ep *endpoint, ok bool)
 // endpointForIPPort returns the endpoint for the peer we
 // believe to be at ipp, or nil if we don't know of any such peer.
 func (m *peerMap) endpointForIPPort(ipp netaddr.IPPort) (ep *endpoint, ok bool) {
-	if info, ok := m.byIPPort[ipp]; ok && info.ep != nil {
+	if info, ok := m.byIPPort[ipp]; ok {
 		return info.ep, true
 	}
 	return nil, false
 }
 
-// forEachDiscoEndpoint invokes f on every endpoint in m.
-func (m *peerMap) forEachDiscoEndpoint(f func(ep *endpoint)) {
+// forEachEndpoint invokes f on every endpoint in m.
+func (m *peerMap) forEachEndpoint(f func(ep *endpoint)) {
 	for _, pi := range m.byNodeKey {
-		if pi.ep != nil {
-			f(pi.ep)
-		}
+		f(pi.ep)
 	}
 }
 
-// upsertDiscoEndpoint stores endpoint in the peerInfo for
+// forEachEndpointWithDiscoKey invokes f on every endpoint in m
+// that has the provided DiscoKey.
+func (m *peerMap) forEachEndpointWithDiscoKey(dk tailcfg.DiscoKey, f func(ep *endpoint)) {
+	for nk := range m.nodesOfDisco[dk] {
+		pi, ok := m.byNodeKey[nk]
+		if !ok {
+			// Unexpected. Data structures would have to
+			// be out of sync.  But we don't have a logger
+			// here to log [unexpected], so just skip.
+			// Maybe log later once peerMap is merged back
+			// into Conn.
+			continue
+		}
+		f(pi.ep)
+	}
+}
+
+// upsertEndpoint stores endpoint in the peerInfo for
 // ep.publicKey, and updates indexes. m must already have a
 // tailcfg.Node for ep.publicKey.
-func (m *peerMap) upsertDiscoEndpoint(ep *endpoint) {
+func (m *peerMap) upsertEndpoint(ep *endpoint) {
 	pi := m.byNodeKey[ep.publicKey]
 	if pi == nil {
-		pi = newPeerInfo()
+		pi = newPeerInfo(ep)
 		m.byNodeKey[ep.publicKey] = pi
+	} else {
+		old := pi.ep
+		pi.ep = ep
+		if old.discoKey != ep.discoKey {
+			delete(m.nodesOfDisco[old.discoKey], ep.publicKey)
+		}
 	}
-	old := pi.ep
-	pi.ep = ep
-	if old != nil && old.discoKey != ep.discoKey {
-		delete(m.byDiscoKey, old.discoKey)
+	if !ep.discoKey.IsZero() {
+		set := m.nodesOfDisco[ep.discoKey]
+		if set == nil {
+			set = map[tailcfg.NodeKey]bool{}
+			m.nodesOfDisco[ep.discoKey] = set
+		}
+		set[ep.publicKey] = true
 	}
-	m.byDiscoKey[ep.discoKey] = pi
 }
 
-// SetDiscoKeyForIPPort makes future peer lookups by ipp return the
-// same peer info as the lookup by dk.
-func (m *peerMap) setDiscoKeyForIPPort(ipp netaddr.IPPort, dk tailcfg.DiscoKey) {
-	// Check for a prior mapping for ipp, may need to clean it up.
+// setNodeKeyForIPPort makes future peer lookups by ipp return the
+// same endpoint as a lookup by nk.
+//
+// This should only be called with a fully verified mapping of ipp to
+// nk, because calling this function defines the endpoint we hand to
+// WireGuard for packets received from ipp.
+func (m *peerMap) setNodeKeyForIPPort(ipp netaddr.IPPort, nk tailcfg.NodeKey) {
 	if pi := m.byIPPort[ipp]; pi != nil {
 		delete(pi.ipPorts, ipp)
 		delete(m.byIPPort, ipp)
 	}
-	if pi, ok := m.byDiscoKey[dk]; ok {
+	if pi, ok := m.byNodeKey[nk]; ok {
 		pi.ipPorts[ipp] = true
 		m.byIPPort[ipp] = pi
 	}
 }
 
-// deleteDiscoEndpoint deletes the peerInfo associated with ep, and
+// deleteEndpoint deletes the peerInfo associated with ep, and
 // updates indexes.
-func (m *peerMap) deleteDiscoEndpoint(ep *endpoint) {
+func (m *peerMap) deleteEndpoint(ep *endpoint) {
 	if ep == nil {
 		return
 	}
 	ep.stopAndReset()
 	pi := m.byNodeKey[ep.publicKey]
-	delete(m.byDiscoKey, ep.discoKey)
+	delete(m.nodesOfDisco[ep.discoKey], ep.publicKey)
 	delete(m.byNodeKey, ep.publicKey)
 	if pi == nil {
 		// Kneejerk paranoia from earlier issue 2801.
@@ -274,7 +298,8 @@ type Conn struct {
 	networkUp syncs.AtomicBool
 
 	// havePrivateKey is whether privateKey is non-zero.
-	havePrivateKey syncs.AtomicBool
+	havePrivateKey  syncs.AtomicBool
+	publicKeyAtomic atomic.Value // of tailcfg.NodeKey (or NodeKey zero value if !havePrivateKey)
 
 	// port is the preferred port from opts.Port; 0 means auto.
 	port syncs.AtomicUint32
@@ -336,9 +361,9 @@ type Conn struct {
 	// nodeOfDisco tracks the networkmap Node entity for each peer
 	// discovery key.
 	peerMap peerMap
-	// sharedDiscoKey is the precomputed nacl/box key for
-	// communication with the peer that has the given DiscoKey.
-	sharedDiscoKey map[tailcfg.DiscoKey]*[32]byte
+
+	// discoInfo is the state for an active DiscoKey.
+	discoInfo map[tailcfg.DiscoKey]*discoInfo
 
 	// netInfoFunc is a callback that provides a tailcfg.NetInfo when
 	// discovered network conditions change.
@@ -499,11 +524,11 @@ func (o *Options) derpActiveFunc() func() {
 // of NewConn. Mostly for tests.
 func newConn() *Conn {
 	c := &Conn{
-		derpRecvCh:     make(chan derpReadResult),
-		derpStarted:    make(chan struct{}),
-		peerLastDerp:   make(map[key.Public]int),
-		peerMap:        newPeerMap(),
-		sharedDiscoKey: make(map[tailcfg.DiscoKey]*[32]byte),
+		derpRecvCh:   make(chan derpReadResult),
+		derpStarted:  make(chan struct{}),
+		peerLastDerp: make(map[key.Public]int),
+		peerMap:      newPeerMap(),
+		discoInfo:    make(map[tailcfg.DiscoKey]*discoInfo),
 	}
 	c.bind = &connBind{Conn: c, closed: true}
 	c.muCond = sync.NewCond(&c.mu)
@@ -609,8 +634,13 @@ func (c *Conn) updateEndpoints(why string) {
 	}()
 	c.logf("[v1] magicsock: starting endpoint update (%s)", why)
 	if c.noV4Send.Get() {
-		c.logf("magicsock: last netcheck reported send error. Rebinding.")
-		c.Rebind()
+		c.mu.Lock()
+		closed := c.closed
+		c.mu.Unlock()
+		if !closed {
+			c.logf("magicsock: last netcheck reported send error. Rebinding.")
+			c.Rebind()
+		}
 	}
 
 	endpoints, err := c.determineEndpoints(c.connCtx)
@@ -811,10 +841,10 @@ func (c *Conn) callNetInfoCallbackLocked(ni *tailcfg.NetInfo) {
 // discoKey. It's used in tests to enable receiving of packets from
 // addr without having to spin up the entire active discovery
 // machinery.
-func (c *Conn) addValidDiscoPathForTest(discoKey tailcfg.DiscoKey, addr netaddr.IPPort) {
+func (c *Conn) addValidDiscoPathForTest(nodeKey tailcfg.NodeKey, addr netaddr.IPPort) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	c.peerMap.setDiscoKeyForIPPort(addr, discoKey)
+	c.peerMap.setNodeKeyForIPPort(addr, nodeKey)
 }
 
 func (c *Conn) SetNetInfoCallback(fn func(*tailcfg.NetInfo)) {
@@ -831,12 +861,12 @@ func (c *Conn) SetNetInfoCallback(fn func(*tailcfg.NetInfo)) {
 	}
 }
 
-// LastRecvActivityOfDisco describes the time we last got traffic from
+// LastRecvActivityOfNodeKey describes the time we last got traffic from
 // this endpoint (updated every ~10 seconds).
-func (c *Conn) LastRecvActivityOfDisco(dk tailcfg.DiscoKey) string {
+func (c *Conn) LastRecvActivityOfNodeKey(nk tailcfg.NodeKey) string {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	de, ok := c.peerMap.endpointForDiscoKey(dk)
+	de, ok := c.peerMap.endpointForNodeKey(nk)
 	if !ok {
 		return "never"
 	}
@@ -1136,8 +1166,12 @@ var udpAddrPool = &sync.Pool{
 // See sendAddr's docs on the return value meanings.
 func (c *Conn) sendUDP(ipp netaddr.IPPort, b []byte) (sent bool, err error) {
 	ua := udpAddrPool.Get().(*net.UDPAddr)
-	defer udpAddrPool.Put(ua)
-	return c.sendUDPStd(ipp.UDPAddrAt(ua), b)
+	sent, err = c.sendUDPStd(ipp.UDPAddrAt(ua), b)
+	if err == nil {
+		// Only return it to the pool on success; Issue 3122.
+		udpAddrPool.Put(ua)
+	}
+	return
 }
 
 // sendUDP sends UDP packet b to addr.
@@ -1385,8 +1419,8 @@ func (c *Conn) setPeerLastDerpLocked(peer key.Public, regionID, homeID int) {
 // out, which also releases the buffer.
 type derpReadResult struct {
 	regionID int
-	n        int        // length of data received
-	src      key.Public // may be zero until server deployment if v2+
+	n        int // length of data received
+	src      key.Public
 	// copyBuf is called to copy the data to dst.  It returns how
 	// much data was copied, which will be n if dst is large
 	// enough. copyBuf can only be called once.
@@ -1589,7 +1623,7 @@ func (c *Conn) receiveIP(b []byte, ipp netaddr.IPPort, cache *ippEndpointCache)
 		c.stunReceiveFunc.Load().(func([]byte, netaddr.IPPort))(b, ipp)
 		return nil, false
 	}
-	if c.handleDiscoMessage(b, ipp) {
+	if c.handleDiscoMessage(b, ipp, tailcfg.NodeKey{}) {
 		return nil, false
 	}
 	if !c.havePrivateKey.Get() {
@@ -1652,7 +1686,7 @@ func (c *Conn) processDERPReadResult(dm derpReadResult, b []byte) (n int, ep *en
 	}
 
 	ipp := netaddr.IPPortFrom(derpMagicIPAddr, uint16(regionID))
-	if c.handleDiscoMessage(b[:n], ipp) {
+	if c.handleDiscoMessage(b[:n], ipp, tailcfg.NodeKey(dm.src)) {
 		return 0, nil
 	}
 
@@ -1682,6 +1716,12 @@ const (
 	discoVerboseLog
 )
 
+// sendDiscoMessage sends discovery message m to dstDisco at dst.
+//
+// If dst is a DERP IP:port, then dstKey must be non-zero.
+//
+// The dstKey should only be non-zero if the dstDisco key
+// unambiguously maps to exactly one peer.
 func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey tailcfg.NodeKey, dstDisco tailcfg.DiscoKey, m disco.Message, logLevel discoLogLevel) (sent bool, err error) {
 	c.mu.Lock()
 	if c.closed {
@@ -1696,14 +1736,18 @@ func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey tailcfg.NodeKey, dstD
 	pkt = append(pkt, disco.Magic...)
 	pkt = append(pkt, c.discoPublic[:]...)
 	pkt = append(pkt, nonce[:]...)
-	sharedKey := c.sharedDiscoKeyLocked(dstDisco)
+	di := c.discoInfoLocked(dstDisco)
 	c.mu.Unlock()
 
-	pkt = box.SealAfterPrecomputation(pkt, m.AppendMarshal(nil), &nonce, sharedKey)
+	pkt = box.SealAfterPrecomputation(pkt, m.AppendMarshal(nil), &nonce, di.sharedKey)
 	sent, err = c.sendAddr(dst, key.Public(dstKey), pkt)
 	if sent {
 		if logLevel == discoLog || (logLevel == discoVerboseLog && debugDisco) {
-			c.logf("[v1] magicsock: disco: %v->%v (%v, %v) sent %v", c.discoShort, dstDisco.ShortString(), dstKey.ShortString(), derpStr(dst.String()), disco.MessageSummary(m))
+			node := "?"
+			if !dstKey.IsZero() {
+				node = dstKey.ShortString()
+			}
+			c.logf("[v1] magicsock: disco: %v->%v (%v, %v) sent %v", c.discoShort, dstDisco.ShortString(), node, derpStr(dst.String()), disco.MessageSummary(m))
 		}
 	} else if err == nil {
 		// Can't send. (e.g. no IPv6 locally)
@@ -1725,9 +1769,11 @@ func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey tailcfg.NodeKey, dstD
 //  * nonce             [24]byte
 //  * naclbox of payload (see tailscale.com/disco package for inner payload format)
 //
-// For messages received over DERP, the addr will be derpMagicIP (with
-// port being the region)
-func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bool) {
+// For messages received over DERP, the src.IP() will be derpMagicIP (with
+// src.Port() being the region ID) and the derpNodeSrc will be the node key
+// it was received from at the DERP layer. derpNodeSrc is zero when received
+// over UDP.
+func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort, derpNodeSrc tailcfg.NodeKey) (isDiscoMsg bool) {
 	const headerLen = len(disco.Magic) + len(tailcfg.DiscoKey{}) + disco.NonceLen
 	if len(msg) < headerLen || string(msg[:len(disco.Magic)]) != disco.Magic {
 		return false
@@ -1763,28 +1809,24 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bo
 		return
 	}
 
-	ep, ok := c.peerMap.endpointForDiscoKey(sender)
-	if !ok {
+	if !c.peerMap.anyEndpointForDiscoKey(sender) {
 		if debugDisco {
 			c.logf("magicsock: disco: ignoring disco-looking frame, don't know endpoint for %v", sender.ShortString())
 		}
 		return
 	}
-	if !ep.canP2P() {
-		// This endpoint allegedly sent us a disco packet, but we know
-		// they can't speak disco. Drop.
-		return
-	}
 
 	// We're now reasonably sure we're expecting communication from
 	// this peer, do the heavy crypto lifting to see what they want.
 	//
 	// From here on, peerNode and de are non-nil.
 
+	di := c.discoInfoLocked(sender)
+
 	var nonce [disco.NonceLen]byte
 	copy(nonce[:], msg[len(disco.Magic)+len(key.Public{}):])
 	sealedBox := msg[headerLen:]
-	payload, ok := box.OpenAfterPrecomputation(nil, sealedBox, &nonce, c.sharedDiscoKeyLocked(sender))
+	payload, ok := box.OpenAfterPrecomputation(nil, sealedBox, &nonce, di.sharedKey)
 	if !ok {
 		// This might be have been intended for a previous
 		// disco key.  When we restart we get a new disco key
@@ -1817,15 +1859,37 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bo
 
 	switch dm := dm.(type) {
 	case *disco.Ping:
-		c.handlePingLocked(dm, ep, src, sender)
+		c.handlePingLocked(dm, src, di, derpNodeSrc)
 	case *disco.Pong:
-		ep.handlePongConnLocked(dm, src)
+		// There might be multiple nodes for the sender's DiscoKey.
+		// Ask each to handle it, stopping once one reports that
+		// the Pong's TxID was theirs.
+		handled := false
+		c.peerMap.forEachEndpointWithDiscoKey(sender, func(ep *endpoint) {
+			if !handled && ep.handlePongConnLocked(dm, di, src) {
+				handled = true
+			}
+		})
 	case *disco.CallMeMaybe:
-		if src.IP() != derpMagicIPAddr {
+		if src.IP() != derpMagicIPAddr || derpNodeSrc.IsZero() {
 			// CallMeMaybe messages should only come via DERP.
 			c.logf("[unexpected] CallMeMaybe packets should only come via DERP")
 			return
 		}
+		nodeKey := tailcfg.NodeKey(derpNodeSrc)
+		ep, ok := c.peerMap.endpointForNodeKey(nodeKey)
+		if !ok {
+			c.logf("magicsock: disco: ignoring CallMeMaybe from %v; %v is unknown", sender.ShortString(), derpNodeSrc.ShortString())
+			return
+		}
+		if !ep.canP2P() {
+			return
+		}
+		if ep.discoKey != di.discoKey {
+			c.logf("[unexpected] CallMeMaybe from peer via DERP whose netmap discokey != disco source")
+			return
+		}
+		di.setNodeKey(nodeKey)
 		c.logf("[v1] magicsock: disco: %v<-%v (%v, %v)  got call-me-maybe, %d endpoints",
 			c.discoShort, ep.discoShort,
 			ep.publicKey.ShortString(), derpStr(src.String()),
@@ -1835,21 +1899,105 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bo
 	return
 }
 
-func (c *Conn) handlePingLocked(dm *disco.Ping, de *endpoint, src netaddr.IPPort, sender tailcfg.DiscoKey) {
-	likelyHeartBeat := src == de.lastPingFrom && time.Since(de.lastPingTime) < 5*time.Second
-	de.lastPingFrom = src
-	de.lastPingTime = time.Now()
-	if !likelyHeartBeat || debugDisco {
-		c.logf("[v1] magicsock: disco: %v<-%v (%v, %v)  got ping tx=%x", c.discoShort, de.discoShort, de.publicKey.ShortString(), src, dm.TxID[:6])
+// unambiguousNodeKeyOfPingLocked attempts to look up an unambiguous mapping
+// from a DiscoKey dk (which sent ping dm) to a NodeKey. ok is true
+// if there's the NodeKey is known unambiguously.
+//
+// derpNodeSrc is non-zero if the disco ping arrived via DERP.
+//
+// c.mu must be held.
+func (c *Conn) unambiguousNodeKeyOfPingLocked(dm *disco.Ping, dk tailcfg.DiscoKey, derpNodeSrc tailcfg.NodeKey) (nk tailcfg.NodeKey, ok bool) {
+	if !derpNodeSrc.IsZero() {
+		if ep, ok := c.peerMap.endpointForNodeKey(derpNodeSrc); ok && ep.discoKey == dk {
+			return derpNodeSrc, true
+		}
 	}
 
+	// Pings after 1.16.0 contains its node source. See if it maps back.
+	if !dm.NodeKey.IsZero() {
+		if ep, ok := c.peerMap.endpointForNodeKey(dm.NodeKey); ok && ep.discoKey == dk {
+			return dm.NodeKey, true
+		}
+	}
+
+	// If there's exactly 1 node in our netmap with DiscoKey dk,
+	// then it's not ambiguous which node key dm was from.
+	if set := c.peerMap.nodesOfDisco[dk]; len(set) == 1 {
+		for nk = range set {
+			return nk, true
+		}
+	}
+
+	return nk, false
+}
+
+// di is the discoInfo of the source of the ping.
+// derpNodeSrc is non-zero if the ping arrived via DERP.
+func (c *Conn) handlePingLocked(dm *disco.Ping, src netaddr.IPPort, di *discoInfo, derpNodeSrc tailcfg.NodeKey) {
+	likelyHeartBeat := src == di.lastPingFrom && time.Since(di.lastPingTime) < 5*time.Second
+	di.lastPingFrom = src
+	di.lastPingTime = time.Now()
+	isDerp := src.IP() == derpMagicIPAddr
+
+	// If we can figure out with certainty which node key this disco
+	// message is for, eagerly update our IP<>node and disco<>node
+	// mappings to make p2p path discovery faster in simple
+	// cases. Without this, disco would still work, but would be
+	// reliant on DERP call-me-maybe to establish the disco<>node
+	// mapping, and on subsequent disco handlePongLocked to establish
+	// the IP<>disco mapping.
+	if nk, ok := c.unambiguousNodeKeyOfPingLocked(dm, di.discoKey, derpNodeSrc); ok {
+		di.setNodeKey(nk)
+		if !isDerp {
+			c.peerMap.setNodeKeyForIPPort(src, nk)
+		}
+	}
+
+	// If we got a ping over DERP, then derpNodeSrc is non-zero and we reply
+	// over DERP (in which case ipDst is also a DERP address).
+	// But if the ping was over UDP (ipDst is not a DERP address), then dstKey
+	// will be zero here, but that's fine: sendDiscoMessage only requires
+	// a dstKey if the dst ip:port is DERP.
+	dstKey := derpNodeSrc
+
 	// Remember this route if not present.
-	c.setAddrToDiscoLocked(src, sender)
-	de.addCandidateEndpoint(src)
+	var numNodes int
+	if isDerp {
+		if ep, ok := c.peerMap.endpointForNodeKey(derpNodeSrc); ok {
+			ep.addCandidateEndpoint(src)
+			numNodes = 1
+		}
+	} else {
+		c.peerMap.forEachEndpointWithDiscoKey(di.discoKey, func(ep *endpoint) {
+			ep.addCandidateEndpoint(src)
+			numNodes++
+			if numNodes == 1 && dstKey.IsZero() {
+				dstKey = ep.publicKey
+			}
+		})
+		if numNodes > 1 {
+			// Zero it out if it's ambiguous, so sendDiscoMessage logging
+			// isn't confusing.
+			dstKey = tailcfg.NodeKey{}
+		}
+	}
+
+	if numNodes == 0 {
+		c.logf("[unexpected] got disco ping from %v/%v for node not in peers", src, derpNodeSrc)
+		return
+	}
+
+	if !likelyHeartBeat || debugDisco {
+		pingNodeSrcStr := dstKey.ShortString()
+		if numNodes > 1 {
+			pingNodeSrcStr = "[one-of-multi]"
+		}
+		c.logf("[v1] magicsock: disco: %v<-%v (%v, %v)  got ping tx=%x", c.discoShort, di.discoShort, pingNodeSrcStr, src, dm.TxID[:6])
+	}
 
 	ipDst := src
-	discoDest := sender
-	go c.sendDiscoMessage(ipDst, de.publicKey, discoDest, &disco.Pong{
+	discoDest := di.discoKey
+	go c.sendDiscoMessage(ipDst, dstKey, discoDest, &disco.Pong{
 		TxID: dm.TxID,
 		Src:  src,
 	}, discoVerboseLog)
@@ -1894,30 +2042,21 @@ func (c *Conn) enqueueCallMeMaybe(derpAddr netaddr.IPPort, de *endpoint) {
 	go de.sendDiscoMessage(derpAddr, &disco.CallMeMaybe{MyNumber: eps}, discoLog)
 }
 
-// setAddrToDiscoLocked records that newk is at src.
+// discoInfoLocked returns the previous or new discoInfo for k.
 //
 // c.mu must be held.
-func (c *Conn) setAddrToDiscoLocked(src netaddr.IPPort, newk tailcfg.DiscoKey) {
-	oldEp, ok := c.peerMap.endpointForIPPort(src)
+func (c *Conn) discoInfoLocked(k tailcfg.DiscoKey) *discoInfo {
+	di, ok := c.discoInfo[k]
 	if !ok {
-		c.logf("[v1] magicsock: disco: adding mapping of %v to %v", src, newk.ShortString())
-	} else if oldEp.discoKey != newk {
-		c.logf("[v1] magicsock: disco: changing mapping of %v from %x=>%x", src, oldEp.discoKey.ShortString(), newk.ShortString())
-	} else {
-		// No change
-		return
+		di = &discoInfo{
+			discoKey:   k,
+			discoShort: k.ShortString(),
+			sharedKey:  new([32]byte),
+		}
+		box.Precompute(di.sharedKey, key.Public(k).B32(), c.discoPrivate.B32())
+		c.discoInfo[k] = di
 	}
-	c.peerMap.setDiscoKeyForIPPort(src, newk)
-}
-
-func (c *Conn) sharedDiscoKeyLocked(k tailcfg.DiscoKey) *[32]byte {
-	if v, ok := c.sharedDiscoKey[k]; ok {
-		return v
-	}
-	shared := new([32]byte)
-	box.Precompute(shared, key.Public(k).B32(), c.discoPrivate.B32())
-	c.sharedDiscoKey[k] = shared
-	return shared
+	return di
 }
 
 func (c *Conn) SetNetworkUp(up bool) {
@@ -1970,6 +2109,12 @@ func (c *Conn) SetPrivateKey(privateKey wgkey.Private) error {
 	c.privateKey = newKey
 	c.havePrivateKey.Set(!newKey.IsZero())
 
+	if newKey.IsZero() {
+		c.publicKeyAtomic.Store(tailcfg.NodeKey{})
+	} else {
+		c.publicKeyAtomic.Store(tailcfg.NodeKey(newKey.Public()))
+	}
+
 	if oldKey.IsZero() {
 		c.everHadKey = true
 		c.logf("magicsock: SetPrivateKey called (init)")
@@ -1991,7 +2136,7 @@ func (c *Conn) SetPrivateKey(privateKey wgkey.Private) error {
 	}
 
 	if newKey.IsZero() {
-		c.peerMap.forEachDiscoEndpoint(func(ep *endpoint) {
+		c.peerMap.forEachEndpoint(func(ep *endpoint) {
 			ep.stopAndReset()
 		})
 	}
@@ -2072,23 +2217,10 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 		return
 	}
 
-	// For disco-capable peers, update the disco endpoint's state and
-	// check if the disco key migrated to a new node key.
 	numNoDisco := 0
 	for _, n := range nm.Peers {
 		if n.DiscoKey.IsZero() {
 			numNoDisco++
-			continue
-		}
-		if ep, ok := c.peerMap.endpointForDiscoKey(n.DiscoKey); ok && ep.publicKey == n.Key {
-			ep.updateFromNode(n)
-			c.peerMap.upsertDiscoEndpoint(ep) // maybe update discokey mappings in peerMap
-		} else if ep != nil {
-			// Endpoint no longer belongs to the same node. We'll
-			// create the new endpoint below.
-			c.logf("magicsock: disco key %v changed from node key %v to %v", n.DiscoKey, ep.publicKey.ShortString(), n.Key.ShortString())
-			ep.stopAndReset()
-			c.peerMap.deleteDiscoEndpoint(ep)
 		}
 	}
 
@@ -2106,7 +2238,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 	for _, n := range nm.Peers {
 		if ep, ok := c.peerMap.endpointForNodeKey(n.Key); ok {
 			ep.updateFromNode(n)
-			c.peerMap.upsertDiscoEndpoint(ep) // maybe update discokey mappings in peerMap
+			c.peerMap.upsertEndpoint(ep) // maybe update discokey mappings in peerMap
 			continue
 		}
 
@@ -2146,7 +2278,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 			}
 		}))
 		ep.updateFromNode(n)
-		c.peerMap.upsertDiscoEndpoint(ep)
+		c.peerMap.upsertEndpoint(ep)
 	}
 
 	// If the set of nodes changed since the last SetNetworkMap, the
@@ -2159,20 +2291,17 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 		for _, n := range nm.Peers {
 			keep[n.Key] = true
 		}
-		c.peerMap.forEachDiscoEndpoint(func(ep *endpoint) {
+		c.peerMap.forEachEndpoint(func(ep *endpoint) {
 			if !keep[ep.publicKey] {
-				c.peerMap.deleteDiscoEndpoint(ep)
-				if !ep.discoKey.IsZero() {
-					delete(c.sharedDiscoKey, ep.discoKey)
-				}
+				c.peerMap.deleteEndpoint(ep)
 			}
 		})
 	}
 
-	// discokeys might have changed in the above. Discard unused cached keys.
-	for discoKey := range c.sharedDiscoKey {
-		if _, ok := c.peerMap.endpointForDiscoKey(discoKey); !ok {
-			delete(c.sharedDiscoKey, discoKey)
+	// discokeys might have changed in the above. Discard unused info.
+	for dk := range c.discoInfo {
+		if !c.peerMap.anyEndpointForDiscoKey(dk) {
+			delete(c.discoInfo, dk)
 		}
 	}
 }
@@ -2374,7 +2503,7 @@ func (c *Conn) Close() error {
 	c.stopPeriodicReSTUNTimerLocked()
 	c.portMapper.Close()
 
-	c.peerMap.forEachDiscoEndpoint(func(ep *endpoint) {
+	c.peerMap.forEachEndpoint(func(ep *endpoint) {
 		ep.stopAndReset()
 	})
 
@@ -2628,7 +2757,7 @@ func (c *Conn) Rebind() {
 func (c *Conn) resetEndpointStates() {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	c.peerMap.forEachDiscoEndpoint(func(ep *endpoint) {
+	c.peerMap.forEachEndpoint(func(ep *endpoint) {
 		ep.noteConnectivityChange()
 	})
 }
@@ -2941,7 +3070,7 @@ func (c *Conn) UpdateStatus(sb *ipnstate.StatusBuilder) {
 		ss.TailAddrDeprecated = tailAddr4
 	})
 
-	c.peerMap.forEachDiscoEndpoint(func(ep *endpoint) {
+	c.peerMap.forEachEndpoint(func(ep *endpoint) {
 		ps := &ipnstate.PeerStatus{InMagicSock: true}
 		//ps.Addrs = append(ps.Addrs, n.Endpoints...)
 		ep.populatePeerStatus(ps)
@@ -2971,19 +3100,16 @@ type endpoint struct {
 
 	// These fields are initialized once and never modified.
 	c          *Conn
-	publicKey  tailcfg.NodeKey  // peer public key (for WireGuard + DERP)
-	discoKey   tailcfg.DiscoKey // for discovery messages. IsZero() if peer can't disco.
-	discoShort string           // ShortString of discoKey. Empty if peer can't disco.
-	fakeWGAddr netaddr.IPPort   // the UDP address we tell wireguard-go we're using
-	wgEndpoint string           // string from ParseEndpoint, holds a JSON-serialized wgcfg.Endpoints
-
-	// Owned by Conn.mu:
-	lastPingFrom netaddr.IPPort
-	lastPingTime time.Time
+	publicKey  tailcfg.NodeKey // peer public key (for WireGuard + DERP)
+	fakeWGAddr netaddr.IPPort  // the UDP address we tell wireguard-go we're using
+	wgEndpoint string          // string from ParseEndpoint, holds a JSON-serialized wgcfg.Endpoints
 
 	// mu protects all following fields.
 	mu sync.Mutex // Lock ordering: Conn.mu, then endpoint.mu
 
+	discoKey   tailcfg.DiscoKey // for discovery messages. IsZero() if peer can't disco.
+	discoShort string           // ShortString of discoKey. Empty if peer can't disco.
+
 	heartBeatTimer *time.Timer    // nil when idle
 	lastSend       mono.Time      // last time there was outgoing packets sent to this peer (from wireguard-go)
 	lastFullPing   mono.Time      // last time we pinged all endpoints
@@ -3338,7 +3464,11 @@ func (de *endpoint) removeSentPingLocked(txid stun.TxID, sp sentPing) {
 // The caller (startPingLocked) should've already been recorded the ping in
 // sentPing and set up the timer.
 func (de *endpoint) sendDiscoPing(ep netaddr.IPPort, txid stun.TxID, logLevel discoLogLevel) {
-	sent, _ := de.sendDiscoMessage(ep, &disco.Ping{TxID: [12]byte(txid)}, logLevel)
+	selfPubKey, _ := de.c.publicKeyAtomic.Load().(tailcfg.NodeKey)
+	sent, _ := de.sendDiscoMessage(ep, &disco.Ping{
+		TxID:    [12]byte(txid),
+		NodeKey: selfPubKey,
+	}, logLevel)
 	if !sent {
 		de.forgetPing(txid)
 	}
@@ -3523,7 +3653,9 @@ func (de *endpoint) noteConnectivityChange() {
 
 // handlePongConnLocked handles a Pong message (a reply to an earlier ping).
 // It should be called with the Conn.mu held.
-func (de *endpoint) handlePongConnLocked(m *disco.Pong, src netaddr.IPPort) {
+//
+// It reports whether m.TxID corresponds to a ping that this endpoint sent.
+func (de *endpoint) handlePongConnLocked(m *disco.Pong, di *discoInfo, src netaddr.IPPort) (knownTxID bool) {
 	de.mu.Lock()
 	defer de.mu.Unlock()
 
@@ -3531,10 +3663,12 @@ func (de *endpoint) handlePongConnLocked(m *disco.Pong, src netaddr.IPPort) {
 
 	sp, ok := de.sentPing[m.TxID]
 	if !ok {
-		// This is not a pong for a ping we sent. Ignore.
-		return
+		// This is not a pong for a ping we sent.
+		return false
 	}
+	knownTxID = true // for naked returns below
 	de.removeSentPingLocked(m.TxID, sp)
+	di.setNodeKey(de.publicKey)
 
 	now := mono.Now()
 	latency := now.Sub(sp.at)
@@ -3546,7 +3680,7 @@ func (de *endpoint) handlePongConnLocked(m *disco.Pong, src netaddr.IPPort) {
 			return
 		}
 
-		de.c.setAddrToDiscoLocked(src, de.discoKey)
+		de.c.peerMap.setNodeKeyForIPPort(src, de.publicKey)
 
 		st.addPongReplyLocked(pongReply{
 			latency: latency,
@@ -3584,6 +3718,7 @@ func (de *endpoint) handlePongConnLocked(m *disco.Pong, src netaddr.IPPort) {
 			de.trustBestAddrUntil = now.Add(trustUDPAddrDuration)
 		}
 	}
+	return
 }
 
 // addrLatency is an IPPort with an associated latency.
@@ -3766,3 +3901,51 @@ type ippEndpointCache struct {
 	gen int64
 	de  *endpoint
 }
+
+// discoInfo is the info and state for the DiscoKey
+// in the Conn.discoInfo map key.
+//
+// Note that a DiscoKey does not necessarily map to exactly one
+// node. In the case of shared nodes and users switching accounts, two
+// nodes in the NetMap may legitimately have the same DiscoKey.  As
+// such, no fields in here should be considered node-specific.
+type discoInfo struct {
+	// discoKey is the same as the Conn.discoInfo map key,
+	// just so you can pass around a *discoInfo alone.
+	// Not modifed once initiazed.
+	discoKey tailcfg.DiscoKey
+
+	// discoShort is discoKey.ShortString().
+	// Not modifed once initiazed;
+	discoShort string
+
+	// sharedKey is the precomputed nacl/box key for
+	// communication with the peer that has the DiscoKey
+	// used to look up this *discoInfo in Conn.discoInfo.
+	// Not modifed once initialized.
+	sharedKey *[32]byte
+
+	// Mutable fields follow, owned by Conn.mu:
+
+	// lastPingFrom is the src of a ping for discoKey.
+	lastPingFrom netaddr.IPPort
+
+	// lastPingTime is the last time of a ping for discoKey.
+	lastPingTime time.Time
+
+	// lastNodeKey is the last NodeKey seen using discoKey.
+	// It's only updated if the NodeKey is unambiguous.
+	lastNodeKey tailcfg.NodeKey
+
+	// lastNodeKeyTime is the time a NodeKey was last seen using
+	// this discoKey. It's only updated if the NodeKey is
+	// unambiguous.
+	lastNodeKeyTime time.Time
+}
+
+// setNodeKey sets the most recent mapping from di.discoKey to the
+// NodeKey nk.
+func (di *discoInfo) setNodeKey(nk tailcfg.NodeKey) {
+	di.lastNodeKey = nk
+	di.lastNodeKeyTime = time.Now()
+}

wgengine/magicsock/magicsock_test.go

@@ -12,6 +12,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"math/rand"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -1144,7 +1145,7 @@ func TestDiscoMessage(t *testing.T) {
 		Key:      tailcfg.NodeKey(key.NewPrivate().Public()),
 		DiscoKey: peer1Pub,
 	}
-	c.peerMap.upsertDiscoEndpoint(&endpoint{
+	c.peerMap.upsertEndpoint(&endpoint{
 		publicKey: n.Key,
 		discoKey:  n.DiscoKey,
 	})
@@ -1158,7 +1159,7 @@ func TestDiscoMessage(t *testing.T) {
 	pkt = append(pkt, nonce[:]...)
 
 	pkt = box.Seal(pkt, []byte(payload), &nonce, c.discoPrivate.Public().B32(), peer1Priv.B32())
-	got := c.handleDiscoMessage(pkt, netaddr.IPPort{})
+	got := c.handleDiscoMessage(pkt, netaddr.IPPort{}, tailcfg.NodeKey{})
 	if !got {
 		t.Error("failed to open it")
 	}
@@ -1248,7 +1249,7 @@ func addTestEndpoint(tb testing.TB, conn *Conn, sendConn net.PacketConn) (tailcf
 	if err != nil {
 		tb.Fatal(err)
 	}
-	conn.addValidDiscoPathForTest(discoKey, netaddr.MustParseIPPort(sendConn.LocalAddr().String()))
+	conn.addValidDiscoPathForTest(nodeKey, netaddr.MustParseIPPort(sendConn.LocalAddr().String()))
 	return nodeKey, discoKey
 }
 
@@ -1434,15 +1435,20 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) {
 		})
 	}
 
-	de, ok := conn.peerMap.endpointForDiscoKey(discoKey)
+	de, ok := conn.peerMap.endpointForNodeKey(nodeKey2)
 	if ok && de.publicKey != nodeKey2 {
 		t.Fatalf("discoEndpoint public key = %q; want %q", de.publicKey[:], nodeKey2[:])
 	}
+	if de.discoKey != discoKey {
+		t.Errorf("discoKey = %v; want %v", de.discoKey, discoKey)
+	}
+	if _, ok := conn.peerMap.endpointForNodeKey(nodeKey1); ok {
+		t.Errorf("didn't expect to find node for key1")
+	}
 
 	log := buf.String()
 	wantSub := map[string]int{
 		"magicsock: got updated network map; 1 peers": 2,
-		"magicsock: disco key discokey:0000000000000000000000000000000000000000000000000000000000000001 changed from node key [TksxA] to [TksyA]": 1,
 	}
 	for sub, want := range wantSub {
 		got := strings.Count(log, sub)
@@ -1634,3 +1640,59 @@ func epStrings(eps []tailcfg.Endpoint) (ret []string) {
 	}
 	return
 }
+
+func TestStressSetNetworkMap(t *testing.T) {
+	conn := newTestConn(t)
+	t.Cleanup(func() { conn.Close() })
+	var buf tstest.MemLogger
+	conn.logf = buf.Logf
+
+	conn.SetPrivateKey(wgkey.Private{0: 1})
+
+	const num = 5
+	present := make([]bool, num)
+	allPeers := make([]*tailcfg.Node, 0, num)
+	for i := 0; i < num; i++ {
+		present[i] = true
+		allPeers = append(allPeers, &tailcfg.Node{
+			DiscoKey:  randDiscoKey(),
+			Key:       randNodeKey(),
+			Endpoints: []string{"192.168.1.2:345"},
+		})
+	}
+
+	var peers []*tailcfg.Node
+	for i := 0; i < 1000; i++ {
+		which := rand.Intn(num)
+		action := rand.Intn(3)
+		switch action {
+		case 0:
+			present[which] = !present[which]
+		case 1:
+			allPeers[which].DiscoKey = randDiscoKey()
+		case 2:
+			allPeers[which].Key = randNodeKey()
+		default:
+			panic("unreachable")
+		}
+		peers = peers[:0]
+		for peerIdx, p := range allPeers {
+			if present[peerIdx] {
+				peers = append(peers, p)
+			}
+		}
+		conn.SetNetworkMap(&netmap.NetworkMap{
+			Peers: peers,
+		})
+	}
+}
+
+func randDiscoKey() (k tailcfg.DiscoKey) {
+	crand.Read(k[:])
+	return
+}
+
+func randNodeKey() (k tailcfg.NodeKey) {
+	crand.Read(k[:])
+	return
+}

wgengine/pendopen.go

@@ -231,7 +231,7 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) {
 	e.logf("open-conn-track: timeout opening %v to node %v; online=%v, lastRecv=%v",
 		flow, n.Key.ShortString(),
 		online,
-		e.magicConn.LastRecvActivityOfDisco(n.DiscoKey))
+		e.magicConn.LastRecvActivityOfNodeKey(n.Key))
 }
 
 func durFmt(t time.Time) string {

wgengine/router/router_linux.go

@@ -156,7 +156,7 @@ func newUserspaceRouter(logf logger.Logf, tunDev tun.Device, linkMon *monitor.Mo
 	}
 
 	cmd := osCommandRunner{
-		ambientCapNetAdmin: distro.Get() == distro.Synology,
+		ambientCapNetAdmin: useAmbientCaps(),
 	}
 
 	return newUserspaceRouterAdvanced(logf, tunname, linkMon, ipt4, ipt6, cmd, supportsV6, supportsV6NAT)
@@ -185,6 +185,17 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, linkMon *monit
 	return r, nil
 }
 
+func useAmbientCaps() bool {
+	if distro.Get() != distro.Synology {
+		return false
+	}
+	v, err := strconv.Atoi(os.Getenv("SYNOPKG_DSM_VERSION_MAJOR"))
+	if err != nil {
+		return false
+	}
+	return v >= 7
+}
+
 // onIPRuleDeleted is the callback from the link monitor for when an IP policy
 // rule is deleted. See Issue 1591.
 //
@@ -533,7 +544,21 @@ func (r *linuxRouter) addRouteDef(routeDef []string, cidr netaddr.IPPrefix) erro
 	if r.ipRuleAvailable {
 		args = append(args, "table", tailscaleRouteTable)
 	}
-	return r.cmd.run(args...)
+	err := r.cmd.run(args...)
+	if err == nil {
+		return nil
+	}
+
+	// TODO(bradfitz): remove this ugly hack to detect failure to
+	// add a route that already exists (as happens in when we're
+	// racing to add kernel-maintained routes when enabling exit
+	// nodes w/o Local LAN access, Issue 3060) and use netlink
+	// directly instead (Issue 391).
+	if errCode(err) == 2 && strings.Contains(err.Error(), "RTNETLINK answers: File exists") {
+		r.logf("ignoring route add of %v; already exists", cidr)
+		return nil
+	}
+	return err
 }
 
 // delRoute removes the route for cidr pointing to the tunnel
@@ -1079,6 +1104,14 @@ func (r *linuxRouter) delSNATRule() error {
 }
 
 func (r *linuxRouter) delLegacyNetfilter() error {
+	if distro.Get() == distro.Synology {
+		// We don't support netfilter on Synology, and unlike other platforms
+		// the following commands error out as the `comment` module doesn't
+		// exist in the iptables binary present on Synology. Albeit the errors
+		// are ignored it's nice to not have logspam.
+		return nil
+	}
+
 	del := func(table, chain string, args ...string) error {
 		exists, err := r.ipt4.Exists(table, chain, args...)
 		if err != nil {

wgengine/router/runner.go

@@ -10,6 +10,7 @@ package router
 import (
 	"errors"
 	"fmt"
+	"os"
 	"os/exec"
 	"strconv"
 	"strings"
@@ -68,6 +69,7 @@ func (o osCommandRunner) output(args ...string) ([]byte, error) {
 	}
 
 	cmd := exec.Command(args[0], args[1:]...)
+	cmd.Env = append(os.Environ(), "LC_ALL=C")
 	if o.ambientCapNetAdmin {
 		cmd.SysProcAttr = &syscall.SysProcAttr{
 			AmbientCaps: []uintptr{unix.CAP_NET_ADMIN},