version/cmdname: s/path/filepath/ and fix version.ReadExe() fallback.

We were using the Go 'path' module, which apparently doesn't handle
backslashes correctly. path/filepath does.

However, the main bug turned out to be that we were not calling .Base()
on the path if version.ReadExe() fails, which it seems to do at least
on Windows 7. As a result, our logfile persistence was not working on
Windows, and logids would be regenerated on every restart.

Affects: #620

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>

cmd/cloner/cloner.go

@@ -123,7 +123,7 @@ const header = `// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserve
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// AUTO-GENERATED by: tailscale.com/cmd/cloner -type %s
+// Code generated by tailscale.com/cmd/cloner -type %s; DO NOT EDIT.
 
 package %s
 
@@ -168,8 +168,8 @@ func gen(buf *bytes.Buffer, imports map[string]struct{}, name string, typ *types
 			}
 			switch ft := ft.Underlying().(type) {
 			case *types.Slice:
-				n := importedName(ft.Elem())
 				if containsPointers(ft.Elem()) {
+					n := importedName(ft.Elem())
 					writef("dst.%s = make([]%s, len(src.%s))", fname, n, fname)
 					writef("for i := range dst.%s {", fname)
 					if _, isPtr := ft.Elem().(*types.Pointer); isPtr {
@@ -179,7 +179,7 @@ func gen(buf *bytes.Buffer, imports map[string]struct{}, name string, typ *types
 					}
 					writef("}")
 				} else {
-					writef("dst.%s = append([]%s(nil), src.%s...)", fname, n, fname)
+					writef("dst.%s = append(src.%s[:0:0], src.%s...)", fname, fname, fname)
 				}
 			case *types.Pointer:
 				if named, _ := ft.Elem().(*types.Named); named != nil && containsPointers(ft.Elem()) {

cmd/tailscaled/tailscaled.go

@@ -169,7 +169,7 @@ func run() error {
 		SurviveDisconnects: true,
 		DebugMux:           debugMux,
 	}
-	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), opts, e)
+	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), ipnserver.FixedEngine(e), opts)
 	// Cancelation is not an error: it is the only way to stop ipnserver.
 	if err != nil && err != context.Canceled {
 		logf("ipnserver.Run: %v", err)

control/controlclient/auto.go

@@ -517,7 +517,7 @@ func (c *Client) SetHostinfo(hi *tailcfg.Hostinfo) {
 		panic("nil Hostinfo")
 	}
 	if !c.direct.SetHostinfo(hi) {
-		c.logf("[unexpected] duplicate Hostinfo: %v", hi)
+		// No changes. Don't log.
 		return
 	}
 	c.logf("Hostinfo: %v", hi)

control/controlclient/controlclient_test.go

@@ -70,3 +70,10 @@ func TestStatusEqual(t *testing.T) {
 		}
 	}
 }
+
+func TestOSVersion(t *testing.T) {
+	if osVersion == nil {
+		t.Skip("not available for OS")
+	}
+	t.Logf("Got: %#q", osVersion())
+}

control/controlclient/direct.go

@@ -4,6 +4,8 @@
 
 package controlclient
 
+//go:generate go run tailscale.com/cmd/cloner -type=Persist -output=direct_clone.go
+
 import (
 	"bytes"
 	"context"
@@ -19,6 +21,7 @@ import (
 	"net/url"
 	"os"
 	"reflect"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -165,12 +168,20 @@ func NewDirect(opts Options) (*Direct, error) {
 	return c, nil
 }
 
+var osVersion func() string // non-nil on some platforms
+
 func NewHostinfo() *tailcfg.Hostinfo {
 	hostname, _ := os.Hostname()
+	var osv string
+	if osVersion != nil {
+		osv = osVersion()
+	}
 	return &tailcfg.Hostinfo{
 		IPNVersion: version.LONG,
 		Hostname:   hostname,
 		OS:         version.OS(),
+		OSVersion:  osv,
+		GoArch:     runtime.GOARCH,
 	}
 }
 
@@ -619,6 +630,7 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 			NodeKey:      tailcfg.NodeKey(persist.PrivateNodeKey.Public()),
 			PrivateKey:   persist.PrivateNodeKey,
 			Expiry:       resp.Node.KeyExpiry,
+			Name:         resp.Node.Name,
 			Addresses:    resp.Node.Addresses,
 			Peers:        resp.Peers,
 			LocalPort:    localPort,

control/controlclient/direct_clone.go

@@ -0,0 +1,20 @@
+// 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.
+
+// Code generated by tailscale.com/cmd/cloner -type Persist; DO NOT EDIT.
+
+package controlclient
+
+import ()
+
+// Clone makes a deep copy of Persist.
+// The result aliases no memory with the original.
+func (src *Persist) Clone() *Persist {
+	if src == nil {
+		return nil
+	}
+	dst := new(Persist)
+	*dst = *src
+	return dst
+}

control/controlclient/hostinfo_linux.go

@@ -0,0 +1,87 @@
+// 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.
+
+// +build linux,!android
+
+package controlclient
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"strings"
+	"syscall"
+
+	"go4.org/mem"
+	"tailscale.com/util/lineread"
+)
+
+func init() {
+	osVersion = osVersionLinux
+}
+
+func osVersionLinux() string {
+	m := map[string]string{}
+	lineread.File("/etc/os-release", func(line []byte) error {
+		eq := bytes.IndexByte(line, '=')
+		if eq == -1 {
+			return nil
+		}
+		k, v := string(line[:eq]), strings.Trim(string(line[eq+1:]), `"`)
+		m[k] = v
+		return nil
+	})
+
+	var un syscall.Utsname
+	syscall.Uname(&un)
+
+	var attrBuf strings.Builder
+	attrBuf.WriteString("; kernel=")
+	for _, b := range un.Release {
+		if b == 0 {
+			break
+		}
+		attrBuf.WriteByte(byte(b))
+	}
+	if inContainer() {
+		attrBuf.WriteString("; container")
+	}
+	attr := attrBuf.String()
+
+	id := m["ID"]
+
+	switch id {
+	case "debian":
+		slurp, _ := ioutil.ReadFile("/etc/debian_version")
+		return fmt.Sprintf("Debian %s (%s)%s", bytes.TrimSpace(slurp), m["VERSION_CODENAME"], attr)
+	case "ubuntu":
+		return fmt.Sprintf("Ubuntu %s%s", m["VERSION"], attr)
+	case "", "centos": // CentOS 6 has no /etc/os-release, so its id is ""
+		if cr, _ := ioutil.ReadFile("/etc/centos-release"); len(cr) > 0 { // "CentOS release 6.10 (Final)
+			return fmt.Sprintf("%s%s", bytes.TrimSpace(cr), attr)
+		}
+		fallthrough
+	case "fedora", "rhel", "alpine":
+		// Their PRETTY_NAME is fine as-is for all versions I tested.
+		fallthrough
+	default:
+		if v := m["PRETTY_NAME"]; v != "" {
+			return fmt.Sprintf("%s%s", v, attr)
+		}
+	}
+	return fmt.Sprintf("Other%s", attr)
+}
+
+func inContainer() (ret bool) {
+	lineread.File("/proc/1/cgroup", func(line []byte) error {
+		if mem.Contains(mem.B(line), mem.S("/docker/")) ||
+			mem.Contains(mem.B(line), mem.S("/lxc/")) {
+			ret = true
+			return io.EOF // arbitrary non-nil error to stop loop
+		}
+		return nil
+	})
+	return
+}

control/controlclient/hostinfo_windows.go

@@ -0,0 +1,26 @@
+// 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.
+
+package controlclient
+
+import (
+	"os/exec"
+	"strings"
+	"syscall"
+)
+
+func init() {
+	osVersion = osVersionWindows
+}
+
+func osVersionWindows() string {
+	cmd := exec.Command("cmd", "/c", "ver")
+	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
+	out, _ := cmd.Output() // "\nMicrosoft Windows [Version 10.0.19041.388]\n\n"
+	s := strings.TrimSpace(string(out))
+	s = strings.TrimPrefix(s, "Microsoft Windows [")
+	s = strings.TrimSuffix(s, "]")
+	s = strings.TrimPrefix(s, "Version ") // is this localized? do it last in case.
+	return s                              // "10.0.19041.388", ideally
+}

control/controlclient/netmap.go

@@ -23,9 +23,11 @@ import (
 type NetworkMap struct {
 	// Core networking
 
-	NodeKey       tailcfg.NodeKey
-	PrivateKey    wgcfg.PrivateKey
-	Expiry        time.Time
+	NodeKey    tailcfg.NodeKey
+	PrivateKey wgcfg.PrivateKey
+	Expiry     time.Time
+	// Name is the DNS name assigned to this node.
+	Name          string
 	Addresses     []wgcfg.CIDR
 	LocalPort     uint16 // used for debugging
 	MachineStatus tailcfg.MachineStatus

derp/derp_server.go

@@ -26,7 +26,6 @@ import (
 
 	"golang.org/x/crypto/nacl/box"
 	"golang.org/x/sync/errgroup"
-	"tailscale.com/disco"
 	"tailscale.com/metrics"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
@@ -52,13 +51,6 @@ type Server struct {
 	// before failing when writing to a client.
 	WriteTimeout time.Duration
 
-	// OnlyDisco controls whether, for tests, non-discovery packets
-	// are dropped. This is used by magicsock tests to verify that
-	// NAT traversal works (using DERP for out-of-band messaging)
-	// but the packets themselves aren't going via DERP.
-	OnlyDisco bool
-	_         [pad32bit]byte
-
 	privateKey key.Private
 	publicKey  key.Public
 	logf       logger.Logf
@@ -559,11 +551,6 @@ func (c *sclient) handleFrameSendPacket(ft frameType, fl uint32) error {
 		return fmt.Errorf("client %x: recvPacket: %v", c.key, err)
 	}
 
-	if s.OnlyDisco && !disco.LooksLikeDiscoWrapper(contents) {
-		s.packetsDropped.Add(1)
-		return nil
-	}
-
 	var fwd PacketForwarder
 	s.mu.Lock()
 	dst := s.clients[dstKey]

go.sum

@@ -86,6 +86,8 @@ github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJy
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/tailscale/winipcfg-go v0.0.0-20200413171540-609dcf2df55f h1:uFj5bslHsMzxIM8UTjAhq4VXeo6GfNW91rpoh/WMJaY=
 github.com/tailscale/winipcfg-go v0.0.0-20200413171540-609dcf2df55f/go.mod h1:x880GWw5fvrl2DVTQ04ttXQD4DuppTt1Yz6wLibbjNE=
+github.com/tailscale/wireguard-go v0.0.0-20200724155040-d554a2a5e7e1 h1:Ga895WFYzI9VOXyps7t9ax9P5zSKsP5Yqpiv2euuyeU=
+github.com/tailscale/wireguard-go v0.0.0-20200724155040-d554a2a5e7e1/go.mod h1:JPm5cTfu1K+qDFRbiHy0sOlHUylYQbpl356sdYFD8V4=
 github.com/tcnksm/go-httpstat v0.2.0 h1:rP7T5e5U2HfmOBmZzGgGZjBQ5/GluWUylujl0tJ04I0=
 github.com/tcnksm/go-httpstat v0.2.0/go.mod h1:s3JVJFtQxtBEBC9dwcdTTXS9xFnM3SXAZwPG41aurT8=
 github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9rrstGQ=

internal/deepprint/deepprint.go

@@ -18,13 +18,23 @@ import (
 	"reflect"
 )
 
-func Hash(v interface{}) string {
+func Hash(v ...interface{}) string {
 	h := sha256.New()
 	Print(h, v)
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-func Print(w io.Writer, v interface{}) {
+// UpdateHash sets last to the hash of v and reports whether its value changed.
+func UpdateHash(last *string, v ...interface{}) (changed bool) {
+	sig := Hash(v)
+	if *last != sig {
+		*last = sig
+		return true
+	}
+	return false
+}
+
+func Print(w io.Writer, v ...interface{}) {
 	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
 }
 

ipn/ipnserver/server.go

@@ -69,10 +69,6 @@ type Options struct {
 	// DebugMux, if non-nil, specifies an HTTP ServeMux in which
 	// to register a debug handler.
 	DebugMux *http.ServeMux
-
-	// ErrorMessage, if not empty, signals that the server will exist
-	// only to relay the provided critical error message to the user.
-	ErrorMessage string
 }
 
 // server is an IPN backend and its set of 0 or more active connections
@@ -152,7 +148,9 @@ func (s *server) writeToClients(b []byte) {
 	}
 }
 
-func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wgengine.Engine) error {
+// Run runs a Tailscale backend service.
+// The getEngine func is called repeatedly, once per connection, until it returns an engine successfully.
+func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (wgengine.Engine, error), opts Options) error {
 	runDone := make(chan struct{})
 	defer close(runDone)
 
@@ -179,25 +177,38 @@ func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wg
 
 	bo := backoff.NewBackoff("ipnserver", logf)
 
-	if opts.ErrorMessage != "" {
+	var unservedConn net.Conn // if non-nil, accepted, but hasn't served yet
+
+	eng, err := getEngine()
+	if err != nil {
+		logf("Initial getEngine call: %v", err)
 		for i := 1; ctx.Err() == nil; i++ {
-			s, err := listen.Accept()
+			c, err := listen.Accept()
 			if err != nil {
 				logf("%d: Accept: %v", i, err)
 				bo.BackOff(ctx, err)
 				continue
 			}
-			serverToClient := func(b []byte) {
-				ipn.WriteMsg(s, b)
+			logf("%d: trying getEngine again...", i)
+			eng, err = getEngine()
+			if err == nil {
+				logf("%d: GetEngine worked; exiting failure loop", i)
+				unservedConn = c
+				break
 			}
+			logf("%d: getEngine failed again: %v", i, err)
+			errMsg := err.Error()
 			go func() {
-				defer s.Close()
+				defer c.Close()
+				serverToClient := func(b []byte) { ipn.WriteMsg(c, b) }
 				bs := ipn.NewBackendServer(logf, nil, serverToClient)
-				bs.SendErrorMessage(opts.ErrorMessage)
-				s.Read(make([]byte, 1))
+				bs.SendErrorMessage(errMsg)
+				time.Sleep(time.Second)
 			}()
 		}
-		return ctx.Err()
+		if err := ctx.Err(); err != nil {
+			return err
+		}
 	}
 
 	var store ipn.StateStore
@@ -210,7 +221,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wg
 		store = &ipn.MemoryStore{}
 	}
 
-	b, err := ipn.NewLocalBackend(logf, logid, store, e)
+	b, err := ipn.NewLocalBackend(logf, logid, store, eng)
 	if err != nil {
 		return fmt.Errorf("NewLocalBackend: %v", err)
 	}
@@ -243,7 +254,14 @@ func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wg
 	}
 
 	for i := 1; ctx.Err() == nil; i++ {
-		c, err := listen.Accept()
+		var c net.Conn
+		var err error
+		if unservedConn != nil {
+			c = unservedConn
+			unservedConn = nil
+		} else {
+			c, err = listen.Accept()
+		}
 		if err != nil {
 			if ctx.Err() == nil {
 				logf("ipnserver: Accept: %v", err)
@@ -371,3 +389,8 @@ func BabysitProc(ctx context.Context, args []string, logf logger.Logf) {
 		}
 	}
 }
+
+// FixedEngine returns a func that returns eng and a nil error.
+func FixedEngine(eng wgengine.Engine) func() (wgengine.Engine, error) {
+	return func() (wgengine.Engine, error) { return eng, nil }
+}

ipn/ipnserver/server_test.go

@@ -72,6 +72,6 @@ func TestRunMultipleAccepts(t *testing.T) {
 		SocketPath: socketPath,
 	}
 	t.Logf("pre-Run")
-	err = ipnserver.Run(ctx, logTriggerTestf, "dummy_logid", opts, eng)
+	err = ipnserver.Run(ctx, logTriggerTestf, "dummy_logid", ipnserver.FixedEngine(eng), opts)
 	t.Logf("ipnserver.Run = %v", err)
 }

ipn/ipnstate/ipnstate.go

@@ -18,6 +18,7 @@ import (
 	"sync"
 	"time"
 
+	"inet.af/netaddr"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 )
@@ -25,6 +26,7 @@ import (
 // Status represents the entire state of the IPN network.
 type Status struct {
 	BackendState string
+	TailscaleIPs []netaddr.IP // Tailscale IP(s) assigned to this node
 	Peer         map[key.Public]*PeerStatus
 	User         map[tailcfg.UserID]tailcfg.UserProfile
 }
@@ -109,6 +111,18 @@ func (sb *StatusBuilder) AddUser(id tailcfg.UserID, up tailcfg.UserProfile) {
 	sb.st.User[id] = up
 }
 
+// AddIP adds a Tailscale IP address to the status.
+func (sb *StatusBuilder) AddTailscaleIP(ip netaddr.IP) {
+	sb.mu.Lock()
+	defer sb.mu.Unlock()
+	if sb.locked {
+		log.Printf("[unexpected] ipnstate: AddIP after Locked")
+		return
+	}
+
+	sb.st.TailscaleIPs = append(sb.st.TailscaleIPs, ip)
+}
+
 // AddPeer adds a peer node to the status.
 //
 // Its PeerStatus is mixed with any previous status already added.
@@ -218,6 +232,12 @@ table tbody tr:nth-child(even) td { background-color: #f5f5f5; }
 	//f("<p><b>logid:</b> %s</p>\n", logid)
 	//f("<p><b>opts:</b> <code>%s</code></p>\n", html.EscapeString(fmt.Sprintf("%+v", opts)))
 
+	ips := make([]string, 0, len(st.TailscaleIPs))
+	for _, ip := range st.TailscaleIPs {
+		ips = append(ips, ip.String())
+	}
+	f("<p>Tailscale IP: %s", strings.Join(ips, ", "))
+
 	f("<table>\n<thead>\n")
 	f("<tr><th>Peer</th><th>Node</th><th>Owner</th><th>Rx</th><th>Tx</th><th>Activity</th><th>Endpoints</th></tr>\n")
 	f("</thead>\n<tbody>\n")

ipn/local.go

@@ -16,6 +16,7 @@ import (
 	"golang.org/x/oauth2"
 	"inet.af/netaddr"
 	"tailscale.com/control/controlclient"
+	"tailscale.com/internal/deepprint"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/ipn/policy"
 	"tailscale.com/portlist"
@@ -51,12 +52,10 @@ type LocalBackend struct {
 	backendLogID    string
 	portpoll        *portlist.Poller // may be nil
 	portpollOnce    sync.Once
+	serverURL       string // tailcontrol URL
 	newDecompressor func() (controlclient.Decompressor, error)
 
-	// TODO: these fields are accessed unsafely by concurrent
-	// goroutines. They need to be protected.
-	serverURL       string // tailcontrol URL
-	lastFilterPrint time.Time
+	filterHash string
 
 	// The mutex protects the following elements.
 	mu       sync.Mutex
@@ -186,21 +185,56 @@ func (b *LocalBackend) SetDecompressor(fn func() (controlclient.Decompressor, er
 // setClientStatus is the callback invoked by the control client whenever it posts a new status.
 // Among other things, this is where we update the netmap, packet filters, DNS and DERP maps.
 func (b *LocalBackend) setClientStatus(st controlclient.Status) {
+	// The following do not depend on any data for which we need to lock b.
+	if st.Err != "" {
+		// TODO(crawshaw): display in the UI.
+		b.logf("Received error: %v", st.Err)
+		return
+	}
 	if st.LoginFinished != nil {
 		// Auth completed, unblock the engine
 		b.blockEngineUpdates(false)
 		b.authReconfig()
 		b.send(Notify{LoginFinished: &empty.Message{}})
 	}
+
+	prefsChanged := false
+
+	// Lock b once and do only the things that require locking.
+	b.mu.Lock()
+
+	prefs := b.prefs
+	stateKey := b.stateKey
+	netMap := b.netMap
+	interact := b.interact
+
 	if st.Persist != nil {
-		persist := *st.Persist // copy
+		if b.prefs.Persist.Equals(st.Persist) {
+			prefsChanged = true
+			b.prefs.Persist = st.Persist.Clone()
+		}
+	}
+	if st.NetMap != nil {
+		b.netMap = st.NetMap
+	}
+	if st.URL != "" {
+		b.authURL = st.URL
+	}
+	if b.state == NeedsLogin {
+		if !b.prefs.WantRunning {
+			prefsChanged = true
+		}
+		b.prefs.WantRunning = true
+	}
+	// Prefs will be written out; this is not safe unless locked or cloned.
+	if prefsChanged {
+		prefs = b.prefs.Clone()
+	}
 
-		b.mu.Lock()
-		b.prefs.Persist = &persist
-		prefs := b.prefs.Clone()
-		stateKey := b.stateKey
-		b.mu.Unlock()
+	b.mu.Unlock()
 
+	// Now complete the lock-free parts of what we started while locked.
+	if prefsChanged {
 		if stateKey != "" {
 			if err := b.store.WriteState(stateKey, prefs.ToBytes()); err != nil {
 				b.logf("Failed to save new controlclient state: %v", err)
@@ -209,63 +243,41 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		b.send(Notify{Prefs: prefs})
 	}
 	if st.NetMap != nil {
-		// Netmap is unchanged only when the diff is empty.
-		changed := true
-		b.mu.Lock()
-		if b.netMap != nil {
-			diff := st.NetMap.ConciseDiffFrom(b.netMap)
+		if netMap != nil {
+			diff := st.NetMap.ConciseDiffFrom(netMap)
 			if strings.TrimSpace(diff) == "" {
-				changed = false
 				b.logf("netmap diff: (none)")
 			} else {
 				b.logf("netmap diff:\n%v", diff)
 			}
 		}
-		disableDERP := b.prefs != nil && b.prefs.DisableDERP
-		b.netMap = st.NetMap
-		b.mu.Unlock()
 
-		b.send(Notify{NetMap: st.NetMap})
-		// There is nothing to update if the map hasn't changed.
-		if changed {
-			b.updateFilter(st.NetMap)
+		b.updateFilter(st.NetMap, prefs)
+		b.e.SetNetworkMap(st.NetMap)
+
+		if !dnsMapsEqual(st.NetMap, netMap) {
 			b.updateDNSMap(st.NetMap)
-			b.e.SetNetworkMap(st.NetMap)
 		}
+
+		disableDERP := prefs != nil && prefs.DisableDERP
 		if disableDERP {
 			b.e.SetDERPMap(nil)
 		} else {
 			b.e.SetDERPMap(st.NetMap.DERPMap)
 		}
+
+		b.send(Notify{NetMap: st.NetMap})
 	}
 	if st.URL != "" {
 		b.logf("Received auth URL: %.20v...", st.URL)
-
-		b.mu.Lock()
-		interact := b.interact
-		b.authURL = st.URL
-		b.mu.Unlock()
-
 		if interact > 0 {
 			b.popBrowserAuthNow()
 		}
 	}
-	if st.Err != "" {
-		// TODO(crawshaw): display in the UI.
-		b.logf("Received error: %v", st.Err)
-		return
-	}
-	if st.NetMap != nil {
-		b.mu.Lock()
-		if b.state == NeedsLogin {
-			b.prefs.WantRunning = true
-		}
-		prefs := b.prefs
-		b.mu.Unlock()
-
-		b.SetPrefs(prefs)
-	}
 	b.stateMachine()
+	// This is currently (2020-07-28) necessary; conditionally disabling it is fragile!
+	// This is where netmap information gets propagated to router and magicsock.
+	b.authReconfig()
 }
 
 // setWgengineStatus is the callback by the wireguard engine whenever it posts a new status.
@@ -360,7 +372,7 @@ func (b *LocalBackend) Start(opts Options) error {
 	persist := b.prefs.Persist
 	b.mu.Unlock()
 
-	b.updateFilter(nil)
+	b.updateFilter(nil, nil)
 
 	var discoPublic tailcfg.DiscoKey
 	if controlclient.Debug.Disco {
@@ -424,20 +436,30 @@ func (b *LocalBackend) Start(opts Options) error {
 
 // updateFilter updates the packet filter in wgengine based on the
 // given netMap and user preferences.
-func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
+func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap, prefs *Prefs) {
 	if netMap == nil {
 		// Not configured yet, block everything
 		b.logf("netmap packet filter: (not ready yet)")
 		b.e.SetFilter(filter.NewAllowNone(b.logf))
 		return
 	}
+	packetFilter := netMap.PacketFilter
+
+	var advRoutes []wgcfg.CIDR
+	if prefs != nil {
+		advRoutes = prefs.AdvertiseRoutes
+	}
+	// Be conservative while not ready.
+	shieldsUp := prefs == nil || prefs.ShieldsUp
+
+	changed := deepprint.UpdateHash(&b.filterHash, packetFilter, advRoutes, shieldsUp)
+	if !changed {
+		return
+	}
 
-	b.mu.Lock()
-	advRoutes := b.prefs.AdvertiseRoutes
-	b.mu.Unlock()
 	localNets := wgCIDRsToFilter(netMap.Addresses, advRoutes)
 
-	if b.shieldsAreUp() {
+	if shieldsUp {
 		// Shields up, block everything
 		b.logf("netmap packet filter: (shields up)")
 		var prevFilter *filter.Filter // don't reuse old filter state
@@ -445,44 +467,81 @@ func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
 		return
 	}
 
-	// TODO(apenwarr): don't replace filter at all if unchanged.
-	// TODO(apenwarr): print a diff instead of full filter.
-	now := time.Now()
-	if now.Sub(b.lastFilterPrint) > 1*time.Minute {
-		b.logf("netmap packet filter: %v", netMap.PacketFilter)
-		b.lastFilterPrint = now
-	} else {
-		b.logf("netmap packet filter: (length %d)", len(netMap.PacketFilter))
+	b.logf("netmap packet filter: %v", packetFilter)
+	b.e.SetFilter(filter.New(packetFilter, localNets, b.e.GetFilter(), b.logf))
+}
+
+// dnsCIDRsEqual determines whether two CIDR lists are equal
+// for DNS map construction purposes (that is, only the first entry counts).
+func dnsCIDRsEqual(newAddr, oldAddr []wgcfg.CIDR) bool {
+	if len(newAddr) != len(oldAddr) {
+		return false
 	}
-	b.e.SetFilter(filter.New(netMap.PacketFilter, localNets, b.e.GetFilter(), b.logf))
+	if len(newAddr) == 0 || newAddr[0] == oldAddr[0] {
+		return true
+	}
+	return false
+}
+
+// dnsMapsEqual determines whether the new and the old network map
+// induce the same DNS map. It does so without allocating memory,
+// at the expense of giving false negatives if peers are reordered.
+func dnsMapsEqual(new, old *controlclient.NetworkMap) bool {
+	if (old == nil) != (new == nil) {
+		return false
+	}
+	if old == nil && new == nil {
+		return true
+	}
+
+	if len(new.Peers) != len(old.Peers) {
+		return false
+	}
+
+	if new.Name != old.Name {
+		return false
+	}
+	if !dnsCIDRsEqual(new.Addresses, old.Addresses) {
+		return false
+	}
+
+	for i, newPeer := range new.Peers {
+		oldPeer := old.Peers[i]
+		if newPeer.Name != oldPeer.Name {
+			return false
+		}
+		if !dnsCIDRsEqual(newPeer.Addresses, oldPeer.Addresses) {
+			return false
+		}
+	}
+
+	return true
 }
 
 // updateDNSMap updates the domain map in the DNS resolver in wgengine
 // based on the given netMap and user preferences.
 func (b *LocalBackend) updateDNSMap(netMap *controlclient.NetworkMap) {
 	if netMap == nil {
+		b.logf("dns map: (not ready)")
 		return
 	}
 
-	domainToIP := make(map[string]netaddr.IP)
-	set := func(hostname string, addrs []wgcfg.CIDR) {
+	nameToIP := make(map[string]netaddr.IP)
+	set := func(name string, addrs []wgcfg.CIDR) {
 		if len(addrs) == 0 {
 			return
 		}
-		domain := hostname
-		// Like PeerStatus.SimpleHostName()
-		domain = strings.TrimSuffix(domain, ".local")
-		domain = strings.TrimSuffix(domain, ".localdomain")
-		domain = domain + ".b.tailscale.net"
-		domainToIP[domain] = netaddr.IPFrom16(addrs[0].IP.Addr)
+		nameToIP[name] = netaddr.IPFrom16(addrs[0].IP.Addr)
 	}
 
 	for _, peer := range netMap.Peers {
-		set(peer.Hostinfo.Hostname, peer.Addresses)
+		set(peer.Name, peer.Addresses)
 	}
-	set(netMap.Hostinfo.Hostname, netMap.Addresses)
+	set(netMap.Name, netMap.Addresses)
 
-	b.e.SetDNSMap(tsdns.NewMap(domainToIP))
+	dnsMap := tsdns.NewMap(nameToIP)
+	// map diff will be logged in tsdns.Resolver.SetMap.
+	b.e.SetDNSMap(dnsMap)
 }
 
 // readPoller is a goroutine that receives service lists from
@@ -721,37 +780,45 @@ func (b *LocalBackend) SetPrefs(new *Prefs) {
 	}
 
 	b.mu.Lock()
+
+	netMap := b.netMap
+	stateKey := b.stateKey
+
 	old := b.prefs
 	new.Persist = old.Persist // caller isn't allowed to override this
 	b.prefs = new
-	if b.stateKey != "" {
-		if err := b.store.WriteState(b.stateKey, b.prefs.ToBytes()); err != nil {
-			b.logf("Failed to save new controlclient state: %v", err)
-		}
-	}
+	// We do this to avoid holding the lock while doing everything else.
+	new = b.prefs.Clone()
+
 	oldHi := b.hostinfo
 	newHi := oldHi.Clone()
 	newHi.RoutableIPs = append([]wgcfg.CIDR(nil), b.prefs.AdvertiseRoutes...)
 	applyPrefsToHostinfo(newHi, new)
 	b.hostinfo = newHi
 	hostInfoChanged := !oldHi.Equal(newHi)
+
 	b.mu.Unlock()
 
+	if stateKey != "" {
+		if err := b.store.WriteState(stateKey, new.ToBytes()); err != nil {
+			b.logf("Failed to save new controlclient state: %v", err)
+		}
+	}
+
 	b.logf("SetPrefs: %v", new.Pretty())
 
 	if old.ShieldsUp != new.ShieldsUp || hostInfoChanged {
 		b.doSetHostinfoFilterServices(newHi)
 	}
 
-	b.updateFilter(b.netMap)
-	// TODO(dmytro): when Prefs gain an EnableTailscaleDNS toggle, updateDNSMap here.
+	b.updateFilter(netMap, new)
 
 	turnDERPOff := new.DisableDERP && !old.DisableDERP
 	turnDERPOn := !new.DisableDERP && old.DisableDERP
 	if turnDERPOff {
 		b.e.SetDERPMap(nil)
-	} else if turnDERPOn && b.netMap != nil {
-		b.e.SetDERPMap(b.netMap.DERPMap)
+	} else if turnDERPOn && netMap != nil {
+		b.e.SetDERPMap(netMap.DERPMap)
 	}
 
 	if old.WantRunning != new.WantRunning {

logtail/logtail.go

@@ -462,5 +462,6 @@ func (l *logger) Write(buf []byte) (int, error) {
 		}
 	}
 	b := l.encode(buf)
-	return l.send(b)
+	_, err := l.send(b)
+	return len(buf), err
 }

logtail/logtail_test.go

@@ -32,3 +32,18 @@ func TestLoggerEncodeTextAllocs(t *testing.T) {
 		t.Logf("allocs = %d; want 1", int(n))
 	}
 }
+
+func TestLoggerWriteLength(t *testing.T) {
+	lg := &logger{
+		timeNow: time.Now,
+		buffer:  NewMemoryBuffer(1024),
+	}
+	inBuf := []byte("some text to encode")
+	n, err := lg.Write(inBuf)
+	if err != nil {
+		t.Error(err)
+	}
+	if n != len(inBuf) {
+		t.Errorf("logger.Write wrote %d bytes, expected %d", n, len(inBuf))
+	}
+}

net/interfaces/interfaces_linux.go

@@ -5,8 +5,14 @@
 package interfaces
 
 import (
+	"bytes"
+	"log"
+	"os/exec"
+	"runtime"
+
 	"go4.org/mem"
 	"inet.af/netaddr"
+	"tailscale.com/syncs"
 	"tailscale.com/util/lineread"
 )
 
@@ -14,6 +20,8 @@ func init() {
 	likelyHomeRouterIP = likelyHomeRouterIPLinux
 }
 
+var procNetRouteErr syncs.AtomicBool
+
 /*
 Parse 10.0.0.1 out of:
 
@@ -23,9 +31,17 @@ ens18   00000000        0100000A        0003    0       0       0       00000000
 ens18   0000000A        00000000        0001    0       0       0       0000FFFF        0       0       0
 */
 func likelyHomeRouterIPLinux() (ret netaddr.IP, ok bool) {
+	if procNetRouteErr.Get() {
+		// If we failed to read /proc/net/route previously, don't keep trying.
+		// But if we're on Android, go into the Android path.
+		if runtime.GOOS == "android" {
+			return likelyHomeRouterIPAndroid()
+		}
+		return ret, false
+	}
 	lineNum := 0
 	var f []mem.RO
-	lineread.File("/proc/net/route", func(line []byte) error {
+	err := lineread.File("/proc/net/route", func(line []byte) error {
 		lineNum++
 		if lineNum == 1 {
 			// Skip header line.
@@ -55,5 +71,47 @@ func likelyHomeRouterIPLinux() (ret netaddr.IP, ok bool) {
 		}
 		return nil
 	})
+	if err != nil {
+		procNetRouteErr.Set(true)
+		if runtime.GOOS == "android" {
+			return likelyHomeRouterIPAndroid()
+		}
+		log.Printf("interfaces: failed to read /proc/net/route: %v", err)
+	}
+	return ret, !ret.IsZero()
+}
+
+// Android apps don't have permission to read /proc/net/route, at
+// least on Google devices and the Android emulator.
+func likelyHomeRouterIPAndroid() (ret netaddr.IP, ok bool) {
+	cmd := exec.Command("/system/bin/ip", "route", "show", "table", "0")
+	out, err := cmd.StdoutPipe()
+	if err != nil {
+		return
+	}
+	if err := cmd.Start(); err != nil {
+		log.Printf("interfaces: running /system/bin/ip: %v", err)
+		return
+	}
+	// Search for line like "default via 10.0.2.2 dev radio0 table 1016 proto static mtu 1500 "
+	lineread.Reader(out, func(line []byte) error {
+		const pfx = "default via "
+		if !mem.HasPrefix(mem.B(line), mem.S(pfx)) {
+			return nil
+		}
+		line = line[len(pfx):]
+		sp := bytes.IndexByte(line, ' ')
+		if sp == -1 {
+			return nil
+		}
+		ipb := line[:sp]
+		if ip, err := netaddr.ParseIP(string(ipb)); err == nil && ip.Is4() {
+			ret = ip
+			log.Printf("interfaces: found Android default route %v", ip)
+		}
+		return nil
+	})
+	cmd.Process.Kill()
+	cmd.Wait()
 	return ret, !ret.IsZero()
 }

net/netcheck/netcheck.go

@@ -18,7 +18,9 @@ import (
 	"log"
 	"net"
 	"net/http"
+	"os"
 	"sort"
+	"strconv"
 	"sync"
 	"time"
 
@@ -36,6 +38,45 @@ import (
 	"tailscale.com/types/opt"
 )
 
+// Debugging and experimentation tweakables.
+var (
+	debugNetcheck, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_NETCHECK"))
+)
+
+// The various default timeouts for things.
+const (
+	// overallProbeTimeout is the maximum amount of time netcheck will
+	// spend gathering a single report.
+	overallProbeTimeout = 5 * time.Second
+	// stunTimeout is the maximum amount of time netcheck will spend
+	// probing with STUN packets without getting a reply before
+	// switching to HTTP probing, on the assumption that outbound UDP
+	// is blocked.
+	stunProbeTimeout = 3 * time.Second
+	// hairpinCheckTimeout is the amount of time we wait for a
+	// hairpinned packet to come back.
+	hairpinCheckTimeout = 100 * time.Millisecond
+	// defaultActiveRetransmitTime is the retransmit interval we use
+	// for STUN probes when we're in steady state (not in start-up),
+	// but don't have previous latency information for a DERP
+	// node. This is a somewhat conservative guess because if we have
+	// no data, likely the DERP node is very far away and we have no
+	// data because we timed out the last time we probed it.
+	defaultActiveRetransmitTime = 200 * time.Millisecond
+	// defaultInitialRetransmitTime is the retransmit interval used
+	// when netcheck first runs. We have no past context to work with,
+	// and we want answers relatively quickly, so it's biased slightly
+	// more aggressive than defaultActiveRetransmitTime. A few extra
+	// packets at startup is fine.
+	defaultInitialRetransmitTime = 100 * time.Millisecond
+	// portMapServiceProbeTimeout is the time we wait for port mapping
+	// services (UPnP, NAT-PMP, PCP) to respond before we give up and
+	// decide that they're not there. Since these services are on the
+	// same LAN as this machine and a single L3 hop away, we don't
+	// give them much time to respond.
+	portMapServiceProbeTimeout = 100 * time.Millisecond
+)
+
 type Report struct {
 	UDP                   bool     // UDP works
 	IPv6                  bool     // IPv6 works
@@ -139,7 +180,7 @@ func (c *Client) logf(format string, a ...interface{}) {
 }
 
 func (c *Client) vlogf(format string, a ...interface{}) {
-	if c.Verbose {
+	if c.Verbose || debugNetcheck {
 		c.logf(format, a...)
 	}
 }
@@ -170,6 +211,8 @@ func (c *Client) MakeNextReportFull() {
 }
 
 func (c *Client) ReceiveSTUNPacket(pkt []byte, src netaddr.IPPort) {
+	c.vlogf("received STUN packet from %s", src)
+
 	c.mu.Lock()
 	if c.handleHairSTUNLocked(pkt, src) {
 		c.mu.Unlock()
@@ -330,7 +373,7 @@ func makeProbePlan(dm *tailcfg.DERPMap, ifState *interfaces.State, last *Report)
 			n := reg.Nodes[try%len(reg.Nodes)]
 			prevLatency := last.RegionLatency[reg.RegionID] * 120 / 100
 			if prevLatency == 0 {
-				prevLatency = 200 * time.Millisecond
+				prevLatency = defaultActiveRetransmitTime
 			}
 			delay := time.Duration(try) * prevLatency
 			if do4 {
@@ -353,16 +396,12 @@ func makeProbePlan(dm *tailcfg.DERPMap, ifState *interfaces.State, last *Report)
 func makeProbePlanInitial(dm *tailcfg.DERPMap, ifState *interfaces.State) (plan probePlan) {
 	plan = make(probePlan)
 
-	// initialSTUNTimeout is only 100ms because some extra retransmits
-	// when starting up is tolerable.
-	const initialSTUNTimeout = 100 * time.Millisecond
-
 	for _, reg := range dm.Regions {
 		var p4 []probe
 		var p6 []probe
 		for try := 0; try < 3; try++ {
 			n := reg.Nodes[try%len(reg.Nodes)]
-			delay := time.Duration(try) * initialSTUNTimeout
+			delay := time.Duration(try) * defaultInitialRetransmitTime
 			if ifState.HaveV4 && nodeMight4(n) {
 				p4 = append(p4, probe{delay: delay, node: n.Name, proto: probeIPv4})
 			}
@@ -518,7 +557,7 @@ func (rs *reportState) startHairCheckLocked(dst netaddr.IPPort) {
 	ua := dst.UDPAddr()
 	rs.pc4Hair.WriteTo(stun.Request(rs.hairTX), ua)
 	rs.c.vlogf("sent haircheck to %v", ua)
-	time.AfterFunc(500*time.Millisecond, func() { close(rs.hairTimeout) })
+	time.AfterFunc(hairpinCheckTimeout, func() { close(rs.hairTimeout) })
 }
 
 func (rs *reportState) waitHairCheck(ctx context.Context) {
@@ -539,6 +578,7 @@ func (rs *reportState) waitHairCheck(ctx context.Context) {
 	case <-rs.gotHairSTUN:
 		ret.HairPinning.Set(true)
 	case <-rs.hairTimeout:
+		rs.c.vlogf("hairCheck timeout")
 		ret.HairPinning.Set(false)
 	default:
 		select {
@@ -649,7 +689,7 @@ func (rs *reportState) probePortMapServices() {
 	}
 	defer uc.Close()
 	tempPort := uc.LocalAddr().(*net.UDPAddr).Port
-	uc.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
+	uc.SetReadDeadline(time.Now().Add(portMapServiceProbeTimeout))
 
 	// Send request packets for all three protocols.
 	uc.WriteTo(uPnPPacket, port1900)
@@ -727,15 +767,10 @@ func newReport() *Report {
 //
 // It may not be called concurrently with itself.
 func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, error) {
-	// Wait for STUN for 3 seconds, but then give HTTP probing
-	// another 2 seconds if all UDP failed.
-	const overallTimeout = 5 * time.Second
-	const stunTimeout = 3 * time.Second
-
 	// Mask user context with ours that we guarantee to cancel so
 	// we can depend on it being closed in goroutines later.
 	// (User ctx might be context.Background, etc)
-	ctx, cancel := context.WithTimeout(ctx, overallTimeout)
+	ctx, cancel := context.WithTimeout(ctx, overallProbeTimeout)
 	defer cancel()
 
 	if dm == nil {
@@ -844,7 +879,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
 		}(probeSet)
 	}
 
-	stunTimer := time.NewTimer(stunTimeout)
+	stunTimer := time.NewTimer(stunProbeTimeout)
 	defer stunTimer.Stop()
 
 	select {
@@ -857,7 +892,9 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
 	}
 
 	rs.waitHairCheck(ctx)
+	c.vlogf("hairCheck done")
 	rs.waitPortMap.Wait()
+	c.vlogf("portMap done")
 	rs.stopTimers()
 
 	// Try HTTPS latency check if all STUN probes failed due to UDP presumably being blocked.
@@ -912,7 +949,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
 
 func (c *Client) measureHTTPSLatency(ctx context.Context, reg *tailcfg.DERPRegion) (time.Duration, netaddr.IP, error) {
 	var result httpstat.Result
-	ctx, cancel := context.WithTimeout(httpstat.WithHTTPStat(ctx, &result), 5*time.Second)
+	ctx, cancel := context.WithTimeout(httpstat.WithHTTPStat(ctx, &result), overallProbeTimeout)
 	defer cancel()
 
 	var ip netaddr.IP

portlist/portlist_linux.go

@@ -10,12 +10,15 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"runtime"
 	"sort"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 
 	"golang.org/x/sys/unix"
+	"tailscale.com/syncs"
 )
 
 // Reading the sockfiles on Linux is very fast, so we can do it often.
@@ -26,13 +29,30 @@ const pollInterval = 1 * time.Second
 var sockfiles = []string{"/proc/net/tcp", "/proc/net/udp"}
 var protos = []string{"tcp", "udp"}
 
+var sawProcNetPermissionErr syncs.AtomicBool
+
 func listPorts() (List, error) {
+	if sawProcNetPermissionErr.Get() {
+		return nil, nil
+	}
 	l := []Port{}
 
 	for pi, fname := range sockfiles {
 		proto := protos[pi]
 
+		// Android 10+ doesn't allow access to this anymore.
+		// https://developer.android.com/about/versions/10/privacy/changes#proc-net-filesystem
+		// Ignore it rather than have the system log about our violation.
+		if runtime.GOOS == "android" && syscall.Access(fname, unix.R_OK) != nil {
+			sawProcNetPermissionErr.Set(true)
+			return nil, nil
+		}
+
 		f, err := os.Open(fname)
+		if os.IsPermission(err) {
+			sawProcNetPermissionErr.Set(true)
+			return nil, nil
+		}
 		if err != nil {
 			return nil, fmt.Errorf("%s: %s", fname, err)
 		}
@@ -96,7 +116,18 @@ func addProcesses(pl []Port) ([]Port, error) {
 	}
 
 	err := foreachPID(func(pid string) error {
-		fdDir, err := os.Open(fmt.Sprintf("/proc/%s/fd", pid))
+		fdPath := fmt.Sprintf("/proc/%s/fd", pid)
+
+		// Android logs a bunch of audit violations in logcat
+		// if we try to open things we don't have access
+		// to. So on Android only, ask if we have permission
+		// rather than just trying it to determine whether we
+		// have permission.
+		if runtime.GOOS == "android" && syscall.Access(fdPath, unix.R_OK) != nil {
+			return nil
+		}
+
+		fdDir, err := os.Open(fdPath)
 		if err != nil {
 			// Can't open fd list for this pid. Maybe
 			// don't have access. Ignore it.

tailcfg/tailcfg.go

@@ -4,6 +4,8 @@
 
 package tailcfg
 
+//go:generate go run tailscale.com/cmd/cloner -type=User,Node,Hostinfo,NetInfo -output=tailcfg_clone.go
+
 import (
 	"bytes"
 	"errors"
@@ -258,6 +260,7 @@ type Hostinfo struct {
 	OSVersion     string       // operating system version, with optional distro prefix ("Debian 10.4", "Windows 10 Pro 10.0.19041")
 	DeviceModel   string       // mobile phone model ("Pixel 3a", "iPhone 11 Pro")
 	Hostname      string       // name of the host the client runs on
+	GoArch        string       // the host's GOARCH value (of the running binary)
 	RoutableIPs   []wgcfg.CIDR `json:",omitempty"` // set of IP ranges this client can route
 	RequestTags   []string     `json:",omitempty"` // set of ACL tags this node wants to claim
 	Services      []Service    `json:",omitempty"` // services advertised by this machine
@@ -371,20 +374,6 @@ func (ni *NetInfo) BasicallyEqual(ni2 *NetInfo) bool {
 		ni.LinkType == ni2.LinkType
 }
 
-// Clone makes a deep copy of Hostinfo.
-// The result aliases no memory with the original.
-//
-// TODO: use cmd/cloner, reconcile len(0) vs. nil.
-func (h *Hostinfo) Clone() (res *Hostinfo) {
-	res = new(Hostinfo)
-	*res = *h
-
-	res.RoutableIPs = append([]wgcfg.CIDR{}, h.RoutableIPs...)
-	res.Services = append([]Service{}, h.Services...)
-	res.NetInfo = h.NetInfo.Clone()
-	return res
-}
-
 // Equal reports whether h and h2 are equal.
 func (h *Hostinfo) Equal(h2 *Hostinfo) bool {
 	if h == nil && h2 == nil {

tailcfg/tailcfg_clone.go

@@ -2,12 +2,11 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// AUTO-GENERATED by tailscale.com/cmd/cloner -type User,Node,NetInfo
+// Code generated by tailscale.com/cmd/cloner -type User,Node,Hostinfo,NetInfo; DO NOT EDIT.
 
 package tailcfg
 
 import (
-	"github.com/tailscale/wireguard-go/wgcfg"
 	"time"
 )
 
@@ -19,8 +18,8 @@ func (src *User) Clone() *User {
 	}
 	dst := new(User)
 	*dst = *src
-	dst.Logins = append([]LoginID(nil), src.Logins...)
-	dst.Roles = append([]RoleID(nil), src.Roles...)
+	dst.Logins = append(src.Logins[:0:0], src.Logins...)
+	dst.Roles = append(src.Roles[:0:0], src.Roles...)
 	return dst
 }
 
@@ -32,9 +31,9 @@ func (src *Node) Clone() *Node {
 	}
 	dst := new(Node)
 	*dst = *src
-	dst.Addresses = append([]wgcfg.CIDR(nil), src.Addresses...)
-	dst.AllowedIPs = append([]wgcfg.CIDR(nil), src.AllowedIPs...)
-	dst.Endpoints = append([]string(nil), src.Endpoints...)
+	dst.Addresses = append(src.Addresses[:0:0], src.Addresses...)
+	dst.AllowedIPs = append(src.AllowedIPs[:0:0], src.AllowedIPs...)
+	dst.Endpoints = append(src.Endpoints[:0:0], src.Endpoints...)
 	dst.Hostinfo = *src.Hostinfo.Clone()
 	if dst.LastSeen != nil {
 		dst.LastSeen = new(time.Time)
@@ -43,6 +42,21 @@ func (src *Node) Clone() *Node {
 	return dst
 }
 
+// Clone makes a deep copy of Hostinfo.
+// The result aliases no memory with the original.
+func (src *Hostinfo) Clone() *Hostinfo {
+	if src == nil {
+		return nil
+	}
+	dst := new(Hostinfo)
+	*dst = *src
+	dst.RoutableIPs = append(src.RoutableIPs[:0:0], src.RoutableIPs...)
+	dst.RequestTags = append(src.RequestTags[:0:0], src.RequestTags...)
+	dst.Services = append(src.Services[:0:0], src.Services...)
+	dst.NetInfo = src.NetInfo.Clone()
+	return dst
+}
+
 // Clone makes a deep copy of NetInfo.
 // The result aliases no memory with the original.
 func (src *NetInfo) Clone() *NetInfo {

tailcfg/tailcfg_test.go

@@ -24,7 +24,7 @@ func fieldsOf(t reflect.Type) (fields []string) {
 func TestHostinfoEqual(t *testing.T) {
 	hiHandles := []string{
 		"IPNVersion", "FrontendLogID", "BackendLogID", "OS", "OSVersion",
-		"DeviceModel", "Hostname", "RoutableIPs", "RequestTags", "Services",
+		"DeviceModel", "Hostname", "GoArch", "RoutableIPs", "RequestTags", "Services",
 		"NetInfo",
 	}
 	if have := fieldsOf(reflect.TypeOf(Hostinfo{})); !reflect.DeepEqual(have, hiHandles) {
@@ -390,3 +390,43 @@ func testKey(t *testing.T, prefix string, in keyIn, out encoding.TextUnmarshaler
 		t.Errorf("mismatch after unmarshal")
 	}
 }
+
+func TestCloneUser(t *testing.T) {
+	tests := []struct {
+		name string
+		u    *User
+	}{
+		{"nil_logins", &User{}},
+		{"zero_logins", &User{Logins: make([]LoginID, 0)}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			u2 := tt.u.Clone()
+			if !reflect.DeepEqual(tt.u, u2) {
+				t.Errorf("not equal")
+			}
+		})
+	}
+}
+
+func TestCloneNode(t *testing.T) {
+	tests := []struct {
+		name string
+		v    *Node
+	}{
+		{"nil_fields", &Node{}},
+		{"zero_fields", &Node{
+			Addresses:  make([]wgcfg.CIDR, 0),
+			AllowedIPs: make([]wgcfg.CIDR, 0),
+			Endpoints:  make([]string, 0),
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			v2 := tt.v.Clone()
+			if !reflect.DeepEqual(tt.v, v2) {
+				t.Errorf("not equal")
+			}
+		})
+	}
+}

tstest/natlab/natlab.go

@@ -93,9 +93,10 @@ func mustPrefix(s string) netaddr.IPPrefix {
 // NewInternet returns a network that simulates the internet.
 func NewInternet() *Network {
 	return &Network{
-		Name:    "internet",
-		Prefix4: mustPrefix("203.0.113.0/24"), // documentation netblock that looks Internet-y
-		Prefix6: mustPrefix("fc00:52::/64"),
+		Name: "internet",
+		// easily recognizable internett-y addresses
+		Prefix4: mustPrefix("1.0.0.0/24"),
+		Prefix6: mustPrefix("1111::/64"),
 	}
 }
 
@@ -184,6 +185,17 @@ func (n *Network) write(p *Packet) (num int, err error) {
 	defer n.mu.Unlock()
 	iface, ok := n.machine[p.Dst.IP]
 	if !ok {
+		// If the destination is within the network's authoritative
+		// range, no route to host.
+		if p.Dst.IP.Is4() && n.Prefix4.Contains(p.Dst.IP) {
+			p.Trace("no route to %v", p.Dst.IP)
+			return len(p.Payload), nil
+		}
+		if p.Dst.IP.Is6() && n.Prefix6.Contains(p.Dst.IP) {
+			p.Trace("no route to %v", p.Dst.IP)
+			return len(p.Payload), nil
+		}
+
 		if n.defaultGW == nil {
 			p.Trace("no route to %v", p.Dst.IP)
 			return len(p.Payload), nil

version/cmdname.go

@@ -8,7 +8,7 @@ package version
 
 import (
 	"os"
-	"path"
+	"path/filepath"
 	"strings"
 
 	"rsc.io/goversion/version"
@@ -26,12 +26,13 @@ func CmdName() string {
 	v, err := version.ReadExe(e)
 	if err != nil {
 		ret = strings.TrimSuffix(strings.ToLower(e), ".exe")
+		ret = filepath.Base(ret)
 	} else {
 		// v is like:
 		// "path\ttailscale.com/cmd/tailscale\nmod\ttailscale.com\t(devel)\t\ndep\tgithub.com/apenwarr/fixconsole\tv0.0.0-20191012055117-5a9f6489cc29\th1:muXWUcay7DDy1/hEQWrYlBy+g0EuwT70sBHg65SeUc4=\ndep\tgithub....
 		for _, line := range strings.Split(v.ModuleInfo, "\n") {
 			if strings.HasPrefix(line, "path\t") {
-				ret = path.Base(strings.TrimPrefix(line, "path\t"))
+				ret = filepath.Base(strings.TrimPrefix(line, "path\t"))
 				break
 			}
 		}

wgengine/filter/filter.go

@@ -136,9 +136,13 @@ func maybeHexdump(flag RunFlags, b []byte) string {
 var acceptBucket = rate.NewLimiter(rate.Every(10*time.Second), 3)
 var dropBucket = rate.NewLimiter(rate.Every(5*time.Second), 10)
 
-func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, r Response, why string) {
+func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, dir direction, r Response, why string) {
 	var verdict string
 
+	if r == Drop && f.omitDropLogging(q, dir) {
+		return
+	}
+
 	if r == Drop && (runflags&LogDrops) != 0 && dropBucket.Allow() {
 		verdict = "Drop"
 		runflags &= HexdumpDrops
@@ -157,26 +161,28 @@ func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, r Respo
 
 // RunIn determines whether this node is allowed to receive q from a Tailscale peer.
 func (f *Filter) RunIn(q *packet.ParsedPacket, rf RunFlags) Response {
-	r := f.pre(q, rf)
+	dir := in
+	r := f.pre(q, rf, dir)
 	if r == Accept || r == Drop {
 		// already logged
 		return r
 	}
 
 	r, why := f.runIn(q)
-	f.logRateLimit(rf, q, r, why)
+	f.logRateLimit(rf, q, dir, r, why)
 	return r
 }
 
 // RunOut determines whether this node is allowed to send q to a Tailscale peer.
 func (f *Filter) RunOut(q *packet.ParsedPacket, rf RunFlags) Response {
-	r := f.pre(q, rf)
+	dir := out
+	r := f.pre(q, rf, dir)
 	if r == Drop || r == Accept {
 		// already logged
 		return r
 	}
 	r, why := f.runOut(q)
-	f.logRateLimit(rf, q, r, why)
+	f.logRateLimit(rf, q, dir, r, why)
 	return r
 }
 
@@ -188,6 +194,11 @@ func (f *Filter) runIn(q *packet.ParsedPacket) (r Response, why string) {
 		return Drop, "destination not allowed"
 	}
 
+	if q.IPVersion == 6 {
+		// TODO: support IPv6.
+		return Drop, "no rules matched"
+	}
+
 	switch q.IPProto {
 	case packet.ICMP:
 		if q.IsEchoResponse() || q.IsError() {
@@ -247,30 +258,78 @@ func (f *Filter) runOut(q *packet.ParsedPacket) (r Response, why string) {
 	return Accept, "ok out"
 }
 
-func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags) Response {
+// direction is whether a packet was flowing in to this machine, or flowing out.
+type direction int
+
+const (
+	in direction = iota
+	out
+)
+
+func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags, dir direction) Response {
 	if len(q.Buffer()) == 0 {
 		// wireguard keepalive packet, always permit.
 		return Accept
 	}
 	if len(q.Buffer()) < 20 {
-		f.logRateLimit(rf, q, Drop, "too short")
+		f.logRateLimit(rf, q, dir, Drop, "too short")
 		return Drop
 	}
 
+	if q.IPVersion == 6 {
+		f.logRateLimit(rf, q, dir, Drop, "ipv6")
+		return Drop
+	}
 	switch q.IPProto {
 	case packet.Unknown:
 		// Unknown packets are dangerous; always drop them.
-		f.logRateLimit(rf, q, Drop, "unknown")
-		return Drop
-	case packet.IPv6:
-		f.logRateLimit(rf, q, Drop, "ipv6")
+		f.logRateLimit(rf, q, dir, Drop, "unknown")
 		return Drop
 	case packet.Fragment:
 		// Fragments after the first always need to be passed through.
 		// Very small fragments are considered Junk by ParsedPacket.
-		f.logRateLimit(rf, q, Accept, "fragment")
+		f.logRateLimit(rf, q, dir, Accept, "fragment")
 		return Accept
 	}
 
 	return noVerdict
 }
+
+// ipv6AllRoutersLinkLocal is ff02::2 (All link-local routers).
+const ipv6AllRoutersLinkLocal = "\xff\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02"
+
+// omitDropLogging reports whether packet p, which has already been
+// deemded a packet to Drop, should bypass the [rate-limited] logging.
+// We don't want to log scary & spammy reject warnings for packets that
+// are totally normal, like IPv6 route announcements.
+func (f *Filter) omitDropLogging(p *packet.ParsedPacket, dir direction) bool {
+	switch dir {
+	case out:
+		switch p.IPVersion {
+		case 4:
+			// Omit logging about outgoing IGMP queries being dropped.
+			if p.IPProto == packet.IGMP {
+				return true
+			}
+		case 6:
+			b := p.Buffer()
+			if len(b) < 40 {
+				return false
+			}
+			src, dst := b[8:8+16], b[24:24+16]
+			// Omit logging for outgoing IPv6 ICMP-v6 queries to ff02::2,
+			// as sent by the OS, looking for routers.
+			if p.IPProto == packet.ICMPv6 {
+				if isLinkLocalV6(src) && string(dst) == ipv6AllRoutersLinkLocal {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
+// isLinkLocalV6 reports whether src is in fe80::/10.
+func isLinkLocalV6(src []byte) bool {
+	return len(src) == 16 && src[0] == 0xfe && src[1]>>6 == 0x80>>6
+}

wgengine/filter/filter_test.go

@@ -219,7 +219,7 @@ func TestPreFilter(t *testing.T) {
 	for _, testPacket := range packets {
 		p := &ParsedPacket{}
 		p.Decode(testPacket.b)
-		got := f.pre(p, LogDrops|LogAccepts)
+		got := f.pre(p, LogDrops|LogAccepts, in)
 		if got != testPacket.want {
 			t.Errorf("%q got=%v want=%v packet:\n%s", testPacket.desc, got, testPacket.want, packet.Hexdump(testPacket.b))
 		}

wgengine/magicsock/magicsock.go

@@ -55,6 +55,43 @@ import (
 	"tailscale.com/version"
 )
 
+// Various debugging and experimental tweakables, set by environment
+// variable.
+var (
+	// logPacketDests prints the known addresses for a peer every time
+	// they change, in the legacy (non-discovery) endpoint code only.
+	logPacketDests, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_LOG_PACKET_DESTS"))
+	// debugDisco prints verbose logs of active discovery events as
+	// they happen.
+	debugDisco, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DISCO"))
+	// debugOmitLocalAddresses removes all local interface addresses
+	// from magicsock's discovered local endpoints. Used in some tests.
+	debugOmitLocalAddresses, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_OMIT_LOCAL_ADDRS"))
+	// debugUseDerpRoute temporarily (2020-03-22) controls whether DERP
+	// reverse routing is enabled (Issue 150). It will become always true
+	// later.
+	debugUseDerpRoute, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_ENABLE_DERP_ROUTE"))
+	// logDerpVerbose logs all received DERP packets, including their
+	// full payload.
+	logDerpVerbose, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DERP"))
+	// debugReSTUNStopOnIdle unconditionally enables the "shut down
+	// STUN if magicsock is idle" behavior that normally only triggers
+	// on mobile devices, lowers the shutdown interval, and logs more
+	// verbosely about idle measurements.
+	debugReSTUNStopOnIdle, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_RESTUN_STOP_ON_IDLE"))
+)
+
+// inTest reports whether the running program is a test that set the
+// IN_TS_TEST environment variable.
+//
+// Unlike the other debug tweakables above, this one needs to be
+// checked every time at runtime, because tests set this after program
+// startup.
+func inTest() bool {
+	inTest, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST"))
+	return inTest
+}
+
 // A Conn routes UDP packets and actively manages a list of its endpoints.
 // It implements wireguard/conn.Bind.
 type Conn struct {
@@ -138,8 +175,9 @@ type Conn struct {
 	derpMap     *tailcfg.DERPMap // nil (or zero regions/nodes) means DERP is disabled
 	netMap      *controlclient.NetworkMap
 	privateKey  key.Private
+	everHadKey  bool               // whether we ever had a non-zero private key
 	myDerp      int                // nearest DERP region ID; 0 means none/unknown
-	derpStarted chan struct{}      // closed on first connection to DERP; for tests
+	derpStarted chan struct{}      // closed on first connection to DERP; for tests & cleaner Close
 	activeDerp  map[int]activeDerp // DERP regionID -> connection to a node in that region
 	prevDerp    map[int]*syncs.WaitGroupChan
 
@@ -602,8 +640,6 @@ func (c *Conn) goDerpConnect(node int) {
 	go c.derpWriteChanOfAddr(netaddr.IPPort{IP: derpMagicIPAddr, Port: uint16(node)}, key.Public{})
 }
 
-var debugOmitLocalAddresses, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_OMIT_LOCAL_ADDRS"))
-
 // determineEndpoints returns the machine's endpoint addresses. It
 // does a STUN lookup (via netcheck) to determine its public address.
 //
@@ -705,10 +741,6 @@ func shouldSprayPacket(b []byte) bool {
 	return false
 }
 
-var logPacketDests, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_LOG_PACKET_DESTS"))
-
-var debugDisco, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DISCO"))
-
 const sprayPeriod = 3 * time.Second
 
 // appendDests appends to dsts the destinations that b should be
@@ -933,11 +965,6 @@ func (c *Conn) sendAddr(addr netaddr.IPPort, pubKey key.Public, b []byte) (sent
 // TODO: this is currently arbitrary. Figure out something better?
 const bufferedDerpWritesBeforeDrop = 32
 
-// debugUseDerpRoute temporarily (2020-03-22) controls whether DERP
-// reverse routing is enabled (Issue 150). It will become always true
-// later.
-var debugUseDerpRoute, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_ENABLE_DERP_ROUTE"))
-
 // derpWriteChanOfAddr returns a DERP client for fake UDP addresses that
 // represent DERP servers, creating them as necessary. For real UDP
 // addresses, it returns nil.
@@ -1006,6 +1033,11 @@ func (c *Conn) derpWriteChanOfAddr(addr netaddr.IPPort, peer key.Public) chan<-
 	// Note that derphttp.NewClient does not dial the server
 	// so it is safe to do under the mu lock.
 	dc := derphttp.NewRegionClient(c.privateKey, c.logf, func() *tailcfg.DERPRegion {
+		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.
+			return nil
+		}
 		c.mu.Lock()
 		defer c.mu.Unlock()
 		if c.derpMap == nil {
@@ -1108,8 +1140,6 @@ type derpReadResult struct {
 	copyBuf func(dst []byte) int
 }
 
-var logDerpVerbose, _ = strconv.ParseBool(os.Getenv("DEBUG_DERP_VERBOSE"))
-
 // runDerpReader runs in a goroutine for the life of a DERP
 // connection, handling received packets.
 func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, dc *derphttp.Client, wg *syncs.WaitGroupChan, startGate <-chan struct{}) {
@@ -1232,8 +1262,11 @@ func (c *Conn) runDerpWriter(ctx context.Context, dc *derphttp.Client, ch <-chan
 // The provided addr and ipp must match.
 //
 // TODO(bradfitz): add a fast path that returns nil here for normal
-// wireguard-go transport packets; IIRC wireguard-go only uses this
-// Endpoint for the relatively rare non-data packets.
+// wireguard-go transport packets; wireguard-go only uses this
+// Endpoint for the relatively rare non-data packets; but we need the
+// Endpoint to find the UDPAddr to return to wireguard anyway, so no
+// benefit unless we can, say, always return the same fake UDPAddr for
+// all packets.
 func (c *Conn) findEndpoint(ipp netaddr.IPPort, addr *net.UDPAddr) conn.Endpoint {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -1786,6 +1819,7 @@ func (c *Conn) SetPrivateKey(privateKey wgcfg.PrivateKey) error {
 	c.privateKey = newKey
 
 	if oldKey.IsZero() {
+		c.everHadKey = true
 		c.logf("magicsock: SetPrivateKey called (init)")
 		go c.ReSTUN("set-private-key")
 	} else if newKey.IsZero() {
@@ -2065,8 +2099,6 @@ func (c *Conn) Close() error {
 	return err
 }
 
-var debugReSTUNStopOnIdle, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_RESTUN_STOP_ON_IDLE"))
-
 func maxIdleBeforeSTUNShutdown() time.Duration {
 	if debugReSTUNStopOnIdle {
 		return time.Minute
@@ -2157,8 +2189,19 @@ func (c *Conn) ReSTUN(why string) {
 		// raced with a shutdown.
 		return
 	}
-	if c.privateKey.IsZero() {
-		c.logf("magicsock: ReSTUN(%q) ignored; no private key", why)
+
+	// If the user stopped the app, stop doing work. (When the
+	// user stops Tailscale via the GUI apps, ipn/local.go
+	// reconfigures the engine with a zero private key.)
+	//
+	// This used to just check c.privateKey.IsZero, but that broke
+	// some end-to-end tests tests that didn't ever set a private
+	// key somehow. So for now, only stop doing work if we ever
+	// had a key, which helps real users, but appeases tests for
+	// now. TODO: rewrite those tests to be less brittle or more
+	// realistic.
+	if c.privateKey.IsZero() && c.everHadKey {
+		c.logf("magicsock: ReSTUN(%q) ignored; stopped, no private key", why)
 		return
 	}
 
@@ -2192,7 +2235,7 @@ func (c *Conn) listenPacket(ctx context.Context, network, addr string) (net.Pack
 
 func (c *Conn) bind1(ruc **RebindingUDPConn, which string) error {
 	host := ""
-	if v, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST")); v {
+	if inTest() {
 		host = "127.0.0.1"
 	}
 	var pc net.PacketConn
@@ -2222,7 +2265,7 @@ func (c *Conn) bind1(ruc **RebindingUDPConn, which string) error {
 // It should be followed by a call to ReSTUN.
 func (c *Conn) Rebind() {
 	host := ""
-	if v, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST")); v {
+	if inTest() {
 		host = "127.0.0.1"
 	}
 	listenCtx := context.Background() // unused without DNS name to resolve
@@ -2832,6 +2875,17 @@ func (c *Conn) UpdateStatus(sb *ipnstate.StatusBuilder) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
+	if c.netMap != nil {
+		for _, addr := range c.netMap.Addresses {
+			if (addr.IP.Is4() && addr.Mask != 32) || (addr.IP.Is6() && addr.Mask != 128) {
+				continue
+			}
+			if ip, ok := netaddr.FromStdIP(addr.IP.IP()); ok {
+				sb.AddTailscaleIP(ip)
+			}
+		}
+	}
+
 	for dk, n := range c.nodeOfDisco {
 		ps := &ipnstate.PeerStatus{InMagicSock: true}
 		ps.Addrs = append(ps.Addrs, n.Endpoints...)

wgengine/magicsock/magicsock_test.go

@@ -6,6 +6,7 @@ package magicsock
 
 import (
 	"bytes"
+	"context"
 	crand "crypto/rand"
 	"crypto/tls"
 	"encoding/binary"
@@ -26,9 +27,11 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"golang.org/x/crypto/nacl/box"
 	"inet.af/netaddr"
+	"tailscale.com/control/controlclient"
 	"tailscale.com/derp"
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/derp/derpmap"
+	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/stun/stuntest"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
@@ -66,11 +69,6 @@ func runDERPAndStun(t *testing.T, logf logger.Logf, l nettype.PacketListener, st
 		t.Fatal(err)
 	}
 	d := derp.NewServer(serverPrivateKey, logf)
-	if l != (nettype.Std{}) {
-		// When using virtual networking, only allow DERP to forward
-		// discovery traffic, not actual packets.
-		d.OnlyDisco = true
-	}
 
 	httpsrv := httptest.NewUnstartedServer(derphttp.Handler(d))
 	httpsrv.Config.ErrorLog = logger.StdLogger(logf)
@@ -172,7 +170,7 @@ func newMagicStack(t *testing.T, logf logger.Logf, l nettype.PacketListener, der
 	// Wait for first endpoint update to be available
 	deadline := time.Now().Add(2 * time.Second)
 	for len(epCh) == 0 && time.Now().Before(deadline) {
-		time.Sleep(10 * time.Millisecond)
+		time.Sleep(100 * time.Millisecond)
 	}
 
 	return &magicStack{
@@ -185,11 +183,136 @@ func newMagicStack(t *testing.T, logf logger.Logf, l nettype.PacketListener, der
 	}
 }
 
+func (s *magicStack) String() string {
+	pub := s.Public()
+	return pub.ShortString()
+}
+
 func (s *magicStack) Close() {
 	s.dev.Close()
 	s.conn.Close()
 }
 
+func (s *magicStack) Public() key.Public {
+	return key.Public(s.privateKey.Public())
+}
+
+func (s *magicStack) Status() *ipnstate.Status {
+	var sb ipnstate.StatusBuilder
+	s.conn.UpdateStatus(&sb)
+	return sb.Status()
+}
+
+// IP returns the Tailscale IP address assigned to this magicStack.
+//
+// Something external needs to provide a NetworkMap and WireGuard
+// configs to the magicStack in order for it to acquire an IP
+// address. See meshStacks for one possible source of netmaps and IPs.
+func (s *magicStack) IP(t *testing.T) netaddr.IP {
+	for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) {
+		st := s.Status()
+		if len(st.TailscaleIPs) > 0 {
+			return st.TailscaleIPs[0]
+		}
+	}
+	t.Fatal("timed out waiting for magicstack to get an IP assigned")
+	panic("unreachable") // compiler doesn't know t.Fatal panics
+}
+
+// meshStacks monitors epCh on all given ms, and plumbs network maps
+// and WireGuard configs into everyone to form a full mesh that has up
+// to date endpoint info. Think of it as an extremely stripped down
+// and purpose-built Tailscale control plane.
+//
+// meshStacks only supports disco connections, not legacy logic.
+func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
+	ctx, cancel := context.WithCancel(context.Background())
+
+	// Serialize all reconfigurations globally, just to keep things
+	// simpler.
+	var (
+		mu  sync.Mutex
+		eps = make([][]string, len(ms))
+	)
+
+	buildNetmapLocked := func(myIdx int) *controlclient.NetworkMap {
+		me := ms[myIdx]
+		nm := &controlclient.NetworkMap{
+			PrivateKey: me.privateKey,
+			NodeKey:    tailcfg.NodeKey(me.privateKey.Public()),
+			Addresses:  []wgcfg.CIDR{{IP: wgcfg.IPv4(1, 0, 0, byte(myIdx+1)), Mask: 32}},
+		}
+		for i, peer := range ms {
+			if i == myIdx {
+				continue
+			}
+			addrs := []wgcfg.CIDR{{IP: wgcfg.IPv4(1, 0, 0, byte(i+1)), Mask: 32}}
+			peer := &tailcfg.Node{
+				ID:         tailcfg.NodeID(i + 1),
+				Name:       fmt.Sprintf("node%d", i+1),
+				Key:        tailcfg.NodeKey(peer.privateKey.Public()),
+				DiscoKey:   peer.conn.DiscoPublicKey(),
+				Addresses:  addrs,
+				AllowedIPs: addrs,
+				Endpoints:  eps[i],
+				DERP:       "127.3.3.40:1",
+			}
+			nm.Peers = append(nm.Peers, peer)
+		}
+
+		return nm
+	}
+
+	updateEps := func(idx int, newEps []string) {
+		mu.Lock()
+		defer mu.Unlock()
+
+		eps[idx] = newEps
+
+		for i, m := range ms {
+			netmap := buildNetmapLocked(i)
+			m.conn.SetNetworkMap(netmap)
+			peerSet := make(map[key.Public]struct{}, len(netmap.Peers))
+			for _, peer := range netmap.Peers {
+				peerSet[key.Public(peer.Key)] = struct{}{}
+			}
+			m.conn.UpdatePeers(peerSet)
+			wg, err := netmap.WGCfg(logf, controlclient.AllowSingleHosts, nil)
+			if err != nil {
+				// We're too far from the *testing.T to be graceful,
+				// blow up. Shouldn't happen anyway.
+				panic(fmt.Sprintf("failed to construct wgcfg from netmap: %v", err))
+			}
+			if err := m.dev.Reconfig(wg); err != nil {
+				panic(fmt.Sprintf("device reconfig failed: %v", err))
+			}
+		}
+	}
+
+	var wg sync.WaitGroup
+	wg.Add(len(ms))
+	for i := range ms {
+		go func(myIdx int) {
+			defer wg.Done()
+
+			for {
+				select {
+				case <-ctx.Done():
+					return
+				case eps := <-ms[myIdx].epCh:
+					logf("conn%d endpoints update", myIdx+1)
+					updateEps(myIdx, eps)
+				}
+			}
+		}(i)
+	}
+
+	return func() {
+		cancel()
+		wg.Wait()
+	}
+}
+
 func TestNewConn(t *testing.T) {
 	tstest.PanicOnLog()
 	rc := tstest.NewResourceCheck()
@@ -433,65 +556,136 @@ func makeNestable(t *testing.T) (logf logger.Logf, setT func(t *testing.T)) {
 }
 
 func TestTwoDevicePing(t *testing.T) {
-	t.Run("real", func(t *testing.T) {
-		l, ip := nettype.Std{}, netaddr.IPv4(127, 0, 0, 1)
+	l, ip := nettype.Std{}, netaddr.IPv4(127, 0, 0, 1)
+	n := &devices{
+		m1:     l,
+		m1IP:   ip,
+		m2:     l,
+		m2IP:   ip,
+		stun:   l,
+		stunIP: ip,
+	}
+	testTwoDevicePing(t, n)
+}
+
+func TestActiveDiscovery(t *testing.T) {
+	t.Run("simple_internet", func(t *testing.T) {
+		t.Parallel()
+		mstun := &natlab.Machine{Name: "stun"}
+		m1 := &natlab.Machine{Name: "m1"}
+		m2 := &natlab.Machine{Name: "m2"}
+		inet := natlab.NewInternet()
+		sif := mstun.Attach("eth0", inet)
+		m1if := m1.Attach("eth0", inet)
+		m2if := m2.Attach("eth0", inet)
+
 		n := &devices{
-			m1:     l,
-			m1IP:   ip,
-			m2:     l,
-			m2IP:   ip,
-			stun:   l,
-			stunIP: ip,
+			m1:     m1,
+			m1IP:   m1if.V4(),
+			m2:     m2,
+			m2IP:   m2if.V4(),
+			stun:   mstun,
+			stunIP: sif.V4(),
 		}
-		testTwoDevicePing(t, n)
+		testActiveDiscovery(t, n)
 	})
-	t.Run("natlab", func(t *testing.T) {
-		t.Run("simple internet", func(t *testing.T) {
-			mstun := &natlab.Machine{Name: "stun"}
-			m1 := &natlab.Machine{Name: "m1"}
-			m2 := &natlab.Machine{Name: "m2"}
-			inet := natlab.NewInternet()
-			sif := mstun.Attach("eth0", inet)
-			m1if := m1.Attach("eth0", inet)
-			m2if := m2.Attach("eth0", inet)
 
-			n := &devices{
-				m1:     m1,
-				m1IP:   m1if.V4(),
-				m2:     m2,
-				m2IP:   m2if.V4(),
-				stun:   mstun,
-				stunIP: sif.V4(),
-			}
-			testTwoDevicePing(t, n)
-		})
+	t.Run("facing_easy_firewalls", func(t *testing.T) {
+		mstun := &natlab.Machine{Name: "stun"}
+		m1 := &natlab.Machine{
+			Name:          "m1",
+			PacketHandler: &natlab.Firewall{},
+		}
+		m2 := &natlab.Machine{
+			Name:          "m2",
+			PacketHandler: &natlab.Firewall{},
+		}
+		inet := natlab.NewInternet()
+		sif := mstun.Attach("eth0", inet)
+		m1if := m1.Attach("eth0", inet)
+		m2if := m2.Attach("eth0", inet)
 
-		t.Run("facing firewalls", func(t *testing.T) {
-			mstun := &natlab.Machine{Name: "stun"}
-			m1 := &natlab.Machine{
-				Name:          "m1",
-				PacketHandler: &natlab.Firewall{},
-			}
-			m2 := &natlab.Machine{
-				Name:          "m2",
-				PacketHandler: &natlab.Firewall{},
-			}
-			inet := natlab.NewInternet()
-			sif := mstun.Attach("eth0", inet)
-			m1if := m1.Attach("eth0", inet)
-			m2if := m2.Attach("eth0", inet)
-
-			n := &devices{
-				m1:     m1,
-				m1IP:   m1if.V4(),
-				m2:     m2,
-				m2IP:   m2if.V4(),
-				stun:   mstun,
-				stunIP: sif.V4(),
-			}
-			testTwoDevicePing(t, n)
-		})
+		n := &devices{
+			m1:     m1,
+			m1IP:   m1if.V4(),
+			m2:     m2,
+			m2IP:   m2if.V4(),
+			stun:   mstun,
+			stunIP: sif.V4(),
+		}
+		testActiveDiscovery(t, n)
 	})
+
+	t.Run("facing_nats", func(t *testing.T) {
+		mstun := &natlab.Machine{Name: "stun"}
+		m1 := &natlab.Machine{
+			Name:          "m1",
+			PacketHandler: &natlab.Firewall{},
+		}
+		nat1 := &natlab.Machine{
+			Name: "nat1",
+		}
+		m2 := &natlab.Machine{
+			Name:          "m2",
+			PacketHandler: &natlab.Firewall{},
+		}
+		nat2 := &natlab.Machine{
+			Name: "nat2",
+		}
+
+		inet := natlab.NewInternet()
+		lan1 := &natlab.Network{
+			Name:    "lan1",
+			Prefix4: mustPrefix("192.168.0.0/24"),
+		}
+		lan2 := &natlab.Network{
+			Name:    "lan2",
+			Prefix4: mustPrefix("192.168.1.0/24"),
+		}
+
+		sif := mstun.Attach("eth0", inet)
+		nat1WAN := nat1.Attach("wan", inet)
+		nat1LAN := nat1.Attach("lan1", lan1)
+		nat2WAN := nat2.Attach("wan", inet)
+		nat2LAN := nat2.Attach("lan2", lan2)
+		m1if := m1.Attach("eth0", lan1)
+		m2if := m2.Attach("eth0", lan2)
+		lan1.SetDefaultGateway(nat1LAN)
+		lan2.SetDefaultGateway(nat2LAN)
+
+		nat1.PacketHandler = &natlab.SNAT44{
+			Machine:           nat1,
+			ExternalInterface: nat1WAN,
+			Firewall: &natlab.Firewall{
+				TrustedInterface: nat1LAN,
+			},
+		}
+		nat2.PacketHandler = &natlab.SNAT44{
+			Machine:           nat2,
+			ExternalInterface: nat2WAN,
+			Firewall: &natlab.Firewall{
+				TrustedInterface: nat2LAN,
+			},
+		}
+
+		n := &devices{
+			m1:     m1,
+			m1IP:   m1if.V4(),
+			m2:     m2,
+			m2IP:   m2if.V4(),
+			stun:   mstun,
+			stunIP: sif.V4(),
+		}
+		testActiveDiscovery(t, n)
+	})
+}
+
+func mustPrefix(s string) netaddr.IPPrefix {
+	pfx, err := netaddr.ParseIPPrefix(s)
+	if err != nil {
+		panic(err)
+	}
+	return pfx
 }
 
 type devices struct {
@@ -505,6 +699,131 @@ type devices struct {
 	stunIP netaddr.IP
 }
 
+// newPinger starts continuously sending test packets from srcM to
+// dstM, until cleanup is invoked to stop it. Each ping has 1 second
+// to transit the network. It is a test failure to lose a ping.
+func newPinger(t *testing.T, logf logger.Logf, src, dst *magicStack) (cleanup func()) {
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan struct{})
+	one := func() bool {
+		// TODO(danderson): requiring exactly zero packet loss
+		// will probably be too strict for some tests we'd like to
+		// run (e.g. discovery switching to a new path on
+		// failure). Figure out what kind of thing would be
+		// acceptable to test instead of "every ping must
+		// transit".
+		pkt := tuntest.Ping(dst.IP(t).IPAddr().IP, src.IP(t).IPAddr().IP)
+		select {
+		case src.tun.Outbound <- pkt:
+		case <-ctx.Done():
+			return false
+		}
+		select {
+		case <-dst.tun.Inbound:
+			return true
+		case <-time.After(10 * time.Second):
+			// Very generous timeout here because depending on
+			// magicsock setup races, the first handshake might get
+			// eaten by the receiving end (if wireguard-go hasn't been
+			// configured quite yet), so we have to wait for at least
+			// the first retransmit from wireguard before we declare
+			// failure.
+			t.Errorf("timed out waiting for ping to transit")
+			return true
+		case <-ctx.Done():
+			// Try a little bit longer to consume the packet we're
+			// waiting for. This is to deal with shutdown races, where
+			// natlab may still be delivering a packet to us from a
+			// goroutine.
+			select {
+			case <-dst.tun.Inbound:
+			case <-time.After(time.Second):
+			}
+			return false
+		}
+	}
+
+	cleanup = func() {
+		cancel()
+		<-done
+	}
+
+	// Synchronously transit one ping to get things started. This is
+	// nice because it means that newPinger returning means we've
+	// worked through initial connectivity.
+	if !one() {
+		cleanup()
+		return
+	}
+
+	go func() {
+		logf("sending ping stream from %s (%s) to %s (%s)", src, src.IP(t), dst, dst.IP(t))
+		defer close(done)
+		for one() {
+		}
+	}()
+
+	return cleanup
+}
+
+// testActiveDiscovery verifies that two magicStacks tied to the given
+// devices can establish a direct p2p connection with each other. See
+// TestActiveDiscovery for the various configurations of devices that
+// get exercised.
+func testActiveDiscovery(t *testing.T, d *devices) {
+	tstest.PanicOnLog()
+	rc := tstest.NewResourceCheck()
+	defer rc.Assert(t)
+
+	tlogf, setT := makeNestable(t)
+	setT(t)
+
+	start := time.Now()
+	logf := func(msg string, args ...interface{}) {
+		msg = fmt.Sprintf("%s: %s", time.Since(start), msg)
+		tlogf(msg, args...)
+	}
+
+	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
+	defer cleanup()
+
+	m1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap)
+	defer m1.Close()
+	m2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap)
+	defer m2.Close()
+
+	cleanup = meshStacks(logf, []*magicStack{m1, m2})
+	defer cleanup()
+
+	m1IP := m1.IP(t)
+	m2IP := m2.IP(t)
+	logf("IPs: %s %s", m1IP, m2IP)
+
+	cleanup = newPinger(t, logf, m1, m2)
+	defer cleanup()
+
+	// Everything is now up and running, active discovery should find
+	// a direct path between our peers. Wait for it to switch away
+	// from DERP.
+
+	mustDirect := func(m1, m2 *magicStack) {
+		for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) {
+			pst := m1.Status().Peer[m2.Public()]
+			if pst.CurAddr != "" {
+				logf("direct link %s->%s found with addr %s", m1, m2, pst.CurAddr)
+				return
+			}
+			logf("no direct path %s->%s yet, addrs %v", m1, m2, pst.Addrs)
+		}
+		t.Errorf("magicsock did not find a direct path from %s to %s", m1, m2)
+	}
+
+	mustDirect(m1, m2)
+	mustDirect(m2, m1)
+
+	logf("starting cleanup")
+}
+
 func testTwoDevicePing(t *testing.T, d *devices) {
 	tstest.PanicOnLog()
 	rc := tstest.NewResourceCheck()

wgengine/packet/ip.go

@@ -47,12 +47,13 @@ const (
 	// Unknown represents an unknown or unsupported protocol; it's deliberately the zero value.
 	Unknown IPProto = 0x00
 	ICMP    IPProto = 0x01
+	IGMP    IPProto = 0x02
+	ICMPv6  IPProto = 0x3a
 	TCP     IPProto = 0x06
 	UDP     IPProto = 0x11
-	// IPv6 and Fragment are special values. They're not really IPProto values
-	// so we're using the unassigned 0xFE and 0xFF values for them.
+	// Fragment is a special value. It's not really an IPProto value
+	// so we're using the unassigned 0xFF value.
 	// TODO(dmytro): special values should be taken out of here.
-	IPv6     IPProto = 0xFE
 	Fragment IPProto = 0xFF
 )
 
@@ -66,8 +67,6 @@ func (p IPProto) String() string {
 		return "UDP"
 	case TCP:
 		return "TCP"
-	case IPv6:
-		return "IPv6"
 	default:
 		return "Unknown"
 	}

wgengine/packet/packet.go

@@ -30,6 +30,8 @@ var (
 )
 
 // ParsedPacket is a minimal decoding of a packet suitable for use in filters.
+//
+// In general, it only supports IPv4. The IPv6 parsing is very minimal.
 type ParsedPacket struct {
 	// b is the byte buffer that this decodes.
 	b []byte
@@ -41,27 +43,32 @@ type ParsedPacket struct {
 	// This is not the same as len(b) because b can have trailing zeros.
 	length int
 
-	IPProto  IPProto // IP subprotocol (UDP, TCP, etc)
-	SrcIP    IP      // IP source address
-	DstIP    IP      // IP destination address
-	SrcPort  uint16  // TCP/UDP source port
-	DstPort  uint16  // TCP/UDP destination port
-	TCPFlags uint8   // TCP flags (SYN, ACK, etc)
+	IPVersion uint8   // 4, 6, or 0
+	IPProto   IPProto // IP subprotocol (UDP, TCP, etc); the NextHeader field for IPv6
+	SrcIP     IP      // IP source address (not used for IPv6)
+	DstIP     IP      // IP destination address (not used for IPv6)
+	SrcPort   uint16  // TCP/UDP source port
+	DstPort   uint16  // TCP/UDP destination port
+	TCPFlags  uint8   // TCP flags (SYN, ACK, etc)
 }
 
-func (q *ParsedPacket) String() string {
-	switch q.IPProto {
-	case IPv6:
+// NextHeader
+type NextHeader uint8
+
+func (p *ParsedPacket) String() string {
+	if p.IPVersion == 6 {
 		return "IPv6{???}"
+	}
+	switch p.IPProto {
 	case Unknown:
 		return "Unknown{???}"
 	}
 	sb := strbuilder.Get()
-	sb.WriteString(q.IPProto.String())
+	sb.WriteString(p.IPProto.String())
 	sb.WriteByte('{')
-	writeIPPort(sb, q.SrcIP, q.SrcPort)
+	writeIPPort(sb, p.SrcIP, p.SrcPort)
 	sb.WriteString(" > ")
-	writeIPPort(sb, q.DstIP, q.DstPort)
+	writeIPPort(sb, p.DstIP, p.DstPort)
 	sb.WriteByte('}')
 	return sb.String()
 }
@@ -105,20 +112,22 @@ func (q *ParsedPacket) Decode(b []byte) {
 	q.b = b
 
 	if len(b) < ipHeaderLength {
+		q.IPVersion = 0
 		q.IPProto = Unknown
 		return
 	}
 
 	// Check that it's IPv4.
 	// TODO(apenwarr): consider IPv6 support
-	switch (b[0] & 0xF0) >> 4 {
+	q.IPVersion = (b[0] & 0xF0) >> 4
+	switch q.IPVersion {
 	case 4:
 		q.IPProto = IPProto(b[9])
-		// continue
 	case 6:
-		q.IPProto = IPv6
+		q.IPProto = IPProto(b[6]) // "Next Header" field
 		return
 	default:
+		q.IPVersion = 0
 		q.IPProto = Unknown
 		return
 	}

wgengine/packet/packet_test.go

@@ -47,11 +47,12 @@ var icmpRequestDecode = ParsedPacket{
 	dataofs: 24,
 	length:  len(icmpRequestBuffer),
 
-	IPProto: ICMP,
-	SrcIP:   NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:   NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort: 0,
-	DstPort: 0,
+	IPVersion: 4,
+	IPProto:   ICMP,
+	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort:   0,
+	DstPort:   0,
 }
 
 var icmpReplyBuffer = []byte{
@@ -72,11 +73,12 @@ var icmpReplyDecode = ParsedPacket{
 	dataofs: 24,
 	length:  len(icmpReplyBuffer),
 
-	IPProto: ICMP,
-	SrcIP:   NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:   NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort: 0,
-	DstPort: 0,
+	IPVersion: 4,
+	IPProto:   ICMP,
+	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort:   0,
+	DstPort:   0,
 }
 
 // IPv6 Router Solicitation
@@ -90,8 +92,9 @@ var ipv6PacketBuffer = []byte{
 }
 
 var ipv6PacketDecode = ParsedPacket{
-	b:       ipv6PacketBuffer,
-	IPProto: IPv6,
+	b:         ipv6PacketBuffer,
+	IPVersion: 6,
+	IPProto:   ICMPv6,
 }
 
 // This is a malformed IPv4 packet.
@@ -101,8 +104,9 @@ var unknownPacketBuffer = []byte{
 }
 
 var unknownPacketDecode = ParsedPacket{
-	b:       unknownPacketBuffer,
-	IPProto: Unknown,
+	b:         unknownPacketBuffer,
+	IPVersion: 0,
+	IPProto:   Unknown,
 }
 
 var tcpPacketBuffer = []byte{
@@ -125,12 +129,13 @@ var tcpPacketDecode = ParsedPacket{
 	dataofs: 40,
 	length:  len(tcpPacketBuffer),
 
-	IPProto:  TCP,
-	SrcIP:    NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:    NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort:  123,
-	DstPort:  567,
-	TCPFlags: TCPSynAck,
+	IPVersion: 4,
+	IPProto:   TCP,
+	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort:   123,
+	DstPort:   567,
+	TCPFlags:  TCPSynAck,
 }
 
 var udpRequestBuffer = []byte{
@@ -152,11 +157,12 @@ var udpRequestDecode = ParsedPacket{
 	dataofs: 28,
 	length:  len(udpRequestBuffer),
 
-	IPProto: UDP,
-	SrcIP:   NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:   NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort: 123,
-	DstPort: 567,
+	IPVersion: 4,
+	IPProto:   UDP,
+	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort:   123,
+	DstPort:   567,
 }
 
 var udpReplyBuffer = []byte{
@@ -234,7 +240,7 @@ func TestDecode(t *testing.T) {
 			var got ParsedPacket
 			got.Decode(tt.buf)
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("got %v; want %v", got, tt.want)
+				t.Errorf("mismatch\n got: %#v\nwant: %#v", got, tt.want)
 			}
 		})
 	}

wgengine/router/dns_direct.go

@@ -13,6 +13,8 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"os/exec"
+	"runtime"
 	"strings"
 
 	"inet.af/netaddr"
@@ -79,6 +81,25 @@ func dnsReadConfig() (DNSConfig, error) {
 	return config, nil
 }
 
+// isResolvedRunning reports whether systemd-resolved is running on the system,
+// even if it is not managing the system DNS settings.
+func isResolvedRunning() bool {
+	if runtime.GOOS != "linux" {
+		return false
+	}
+
+	// systemd-resolved is never installed without systemd.
+	_, err := exec.LookPath("systemctl")
+	if err != nil {
+		return false
+	}
+
+	// is-active exits with code 3 if the service is not active.
+	err = exec.Command("systemctl", "is-active", "systemd-resolved.service").Run()
+
+	return err == nil
+}
+
 // dnsDirectUp replaces /etc/resolv.conf with a file generated
 // from the given configuration, creating a backup of its old state.
 //
@@ -124,6 +145,10 @@ func dnsDirectUp(config DNSConfig) error {
 		return err
 	}
 
+	if isResolvedRunning() {
+		exec.Command("systemctl", "restart", "systemd-resolved.service").Run() // Best-effort.
+	}
+
 	return nil
 }
 
@@ -147,5 +172,10 @@ func dnsDirectDown() error {
 		return err
 	}
 	os.Remove(tsConf)
+
+	if isResolvedRunning() {
+		exec.Command("systemctl", "restart", "systemd-resolved.service").Run() // Best-effort.
+	}
+
 	return nil
 }

wgengine/router/router_userspace_bsd.go

@@ -153,7 +153,10 @@ func (r *userspaceBSDRouter) Set(cfg *Config) error {
 }
 
 func (r *userspaceBSDRouter) Close() error {
-	cleanup(r.logf, r.tunname)
+	if err := downDNS(r.tunname); err != nil {
+		r.logf("dns down: %v", err)
+	}
+	// No interface cleanup is necessary during normal shutdown.
 	return nil
 }
 
@@ -161,9 +164,12 @@ func cleanup(logf logger.Logf, interfaceName string) {
 	if err := downDNS(interfaceName); err != nil {
 		logf("dns down: %v", err)
 	}
-
-	ifup := []string{"ifconfig", interfaceName, "down"}
+	// If the interface was left behind, ifconfig down will not remove it.
+	// In fact, this will leave a system in a tainted state where starting tailscaled
+	// will result in "interface tailscale0 already exists"
+	// until the defunct interface is ifconfig-destroyed.
+	ifup := []string{"ifconfig", interfaceName, "destroy"}
 	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
-		logf("ifconfig down: %v\n%s", err, out)
+		logf("ifconfig destroy: %v\n%s", err, out)
 	}
 }

wgengine/tsdns/map.go

@@ -0,0 +1,118 @@
+// 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.
+
+package tsdns
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	"inet.af/netaddr"
+)
+
+// Map is all the data Resolver needs to resolve DNS queries within the Tailscale network.
+type Map struct {
+	// nameToIP is a mapping of Tailscale domain names to their IP addresses.
+	// For example, monitoring.tailscale.us -> 100.64.0.1.
+	nameToIP map[string]netaddr.IP
+	// names are the keys of nameToIP in sorted order.
+	names []string
+}
+
+// NewMap returns a new Map with name to address mapping given by nameToIP.
+func NewMap(nameToIP map[string]netaddr.IP) *Map {
+	names := make([]string, 0, len(nameToIP))
+	for name := range nameToIP {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
+	return &Map{
+		nameToIP: nameToIP,
+		names:    names,
+	}
+}
+
+func printSingleNameIP(buf *strings.Builder, name string, ip netaddr.IP) {
+	// Output width is exactly 80 columns.
+	fmt.Fprintf(buf, "%s\t%s\n", name, ip)
+}
+
+func (m *Map) Pretty() string {
+	buf := new(strings.Builder)
+	for _, name := range m.names {
+		printSingleNameIP(buf, name, m.nameToIP[name])
+	}
+	return buf.String()
+}
+
+func (m *Map) PrettyDiffFrom(old *Map) string {
+	var (
+		oldNameToIP map[string]netaddr.IP
+		newNameToIP map[string]netaddr.IP
+		oldNames    []string
+		newNames    []string
+	)
+	if old != nil {
+		oldNameToIP = old.nameToIP
+		oldNames = old.names
+	}
+	if m != nil {
+		newNameToIP = m.nameToIP
+		newNames = m.names
+	}
+
+	buf := new(strings.Builder)
+
+	for len(oldNames) > 0 && len(newNames) > 0 {
+		var name string
+
+		newName, oldName := newNames[0], oldNames[0]
+		switch {
+		case oldName < newName:
+			name = oldName
+			oldNames = oldNames[1:]
+		case oldName > newName:
+			name = newName
+			newNames = newNames[1:]
+		case oldNames[0] == newNames[0]:
+			name = oldNames[0]
+			oldNames = oldNames[1:]
+			newNames = newNames[1:]
+		}
+
+		ipOld, inOld := oldNameToIP[name]
+		ipNew, inNew := newNameToIP[name]
+		switch {
+		case !inOld:
+			buf.WriteByte('+')
+			printSingleNameIP(buf, name, ipNew)
+		case !inNew:
+			buf.WriteByte('-')
+			printSingleNameIP(buf, name, ipOld)
+		case ipOld != ipNew:
+			buf.WriteByte('-')
+			printSingleNameIP(buf, name, ipOld)
+			buf.WriteByte('+')
+			printSingleNameIP(buf, name, ipNew)
+		}
+	}
+
+	for _, name := range oldNames {
+		if _, ok := newNameToIP[name]; !ok {
+			buf.WriteByte('-')
+			printSingleNameIP(buf, name, oldNameToIP[name])
+		}
+	}
+
+	for _, name := range newNames {
+		if _, ok := oldNameToIP[name]; !ok {
+			buf.WriteByte('+')
+			printSingleNameIP(buf, name, newNameToIP[name])
+		}
+	}
+
+	return buf.String()
+}

wgengine/tsdns/map_test.go

@@ -0,0 +1,138 @@
+// 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.
+
+package tsdns
+
+import (
+	"testing"
+
+	"inet.af/netaddr"
+)
+
+func TestPretty(t *testing.T) {
+	tests := []struct {
+		name string
+		dmap *Map
+		want string
+	}{
+		{"empty", NewMap(nil), ""},
+		{
+			"single",
+			NewMap(map[string]netaddr.IP{
+				"hello.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"hello.ipn.dev\t100.101.102.103\n",
+		},
+		{
+			"multiple",
+			NewMap(map[string]netaddr.IP{
+				"test1.domain":     netaddr.IPv4(100, 101, 102, 103),
+				"test2.sub.domain": netaddr.IPv4(100, 99, 9, 1),
+			}),
+			"test1.domain\t100.101.102.103\ntest2.sub.domain\t100.99.9.1\n",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.dmap.Pretty()
+			if tt.want != got {
+				t.Errorf("want %v; got %v", tt.want, got)
+			}
+		})
+	}
+}
+
+func TestPrettyDiffFrom(t *testing.T) {
+	tests := []struct {
+		name string
+		map1 *Map
+		map2 *Map
+		want string
+	}{
+		{
+			"from_empty",
+			nil,
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			"+test1.ipn.dev\t100.101.102.103\n+test2.ipn.dev\t100.103.102.101\n",
+		},
+		{
+			"equal",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"",
+		},
+		{
+			"changed_ip",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test2.ipn.dev": netaddr.IPv4(100, 104, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"-test2.ipn.dev\t100.103.102.101\n+test2.ipn.dev\t100.104.102.101\n",
+		},
+		{
+			"new_domain",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test3.ipn.dev": netaddr.IPv4(100, 105, 106, 107),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"+test3.ipn.dev\t100.105.106.107\n",
+		},
+		{
+			"gone_domain",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"-test2.ipn.dev\t100.103.102.101\n",
+		},
+		{
+			"mixed",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test4.ipn.dev": netaddr.IPv4(100, 107, 106, 105),
+				"test5.ipn.dev": netaddr.IPv4(100, 64, 1, 1),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test2.ipn.dev": netaddr.IPv4(100, 104, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 100, 101, 102),
+				"test3.ipn.dev": netaddr.IPv4(100, 64, 1, 1),
+			}),
+			"-test1.ipn.dev\t100.101.102.103\n+test1.ipn.dev\t100.100.101.102\n" +
+				"-test2.ipn.dev\t100.103.102.101\n+test2.ipn.dev\t100.104.102.101\n" +
+				"+test3.ipn.dev\t100.64.1.1\n-test4.ipn.dev\t100.107.106.105\n-test5.ipn.dev\t100.64.1.1\n",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.map2.PrettyDiffFrom(tt.map1)
+			if tt.want != got {
+				t.Errorf("want %v; got %v", tt.want, got)
+			}
+		})
+	}
+}

wgengine/tsdns/tsdns.go

@@ -45,18 +45,6 @@ var (
 	errNotQuery       = errors.New("not a DNS query")
 )
 
-// Map is all the data Resolver needs to resolve DNS queries within the Tailscale network.
-type Map struct {
-	// domainToIP is a mapping of Tailscale domains to their IP addresses.
-	// For example, monitoring.tailscale.us -> 100.64.0.1.
-	domainToIP map[string]netaddr.IP
-}
-
-// NewMap returns a new Map with domain to address mapping given by domainToIP.
-func NewMap(domainToIP map[string]netaddr.IP) *Map {
-	return &Map{domainToIP: domainToIP}
-}
-
 // Packet represents a DNS payload together with the address of its origin.
 type Packet struct {
 	// Payload is the application layer DNS payload.
@@ -142,8 +130,10 @@ func (r *Resolver) Close() {
 // SetMap sets the resolver's DNS map, taking ownership of it.
 func (r *Resolver) SetMap(m *Map) {
 	r.mu.Lock()
+	oldMap := r.dnsMap
 	r.dnsMap = m
 	r.mu.Unlock()
+	r.logf("map diff:\n%s", m.PrettyDiffFrom(oldMap))
 }
 
 // SetUpstreamNameservers sets the addresses of the resolver's
@@ -189,7 +179,7 @@ func (r *Resolver) Resolve(domain string) (netaddr.IP, dns.RCode, error) {
 		r.mu.RUnlock()
 		return netaddr.IP{}, dns.RCodeServerFailure, errMapNotSet
 	}
-	addr, found := r.dnsMap.domainToIP[domain]
+	addr, found := r.dnsMap.nameToIP[domain]
 	r.mu.RUnlock()
 
 	if !found {

wgengine/tsdns/tsdns_test.go

@@ -23,7 +23,7 @@ var testipv6 = netaddr.IPv6Raw([16]byte{
 })
 
 var dnsMap = &Map{
-	domainToIP: map[string]netaddr.IP{
+	nameToIP: map[string]netaddr.IP{
 		"test1.ipn.dev": testipv4,
 		"test2.ipn.dev": testipv6,
 	},

wgengine/userspace.go

@@ -16,6 +16,7 @@ import (
 	"os"
 	"os/exec"
 	"runtime"
+	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -33,6 +34,7 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
+	"tailscale.com/version"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/magicsock"
 	"tailscale.com/wgengine/monitor"
@@ -560,13 +562,27 @@ func (e *userspaceEngine) pinger(peerKey wgcfg.Key, ips []wgcfg.IP) {
 	p.run(ctx, peerKey, ips, srcIP)
 }
 
-func updateSig(last *string, v interface{}) (changed bool) {
-	sig := deepprint.Hash(v)
-	if *last != sig {
-		*last = sig
-		return true
+var debugTrimWireguard, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_TRIM_WIREGUARD"))
+
+// forceFullWireguardConfig reports whether we should give wireguard
+// our full network map, even for inactive peers
+//
+// TODO(bradfitz): remove this after our 1.0 launch; we don't want to
+// enable wireguard config trimming quite yet because it just landed
+// and we haven't got enough time testing it.
+func forceFullWireguardConfig(numPeers int) bool {
+	// Did the user explicitly enable trimmming via the environment variable knob?
+	if debugTrimWireguard {
+		return false
 	}
-	return false
+	// On iOS with large networks, it's critical, so turn on trimming.
+	// Otherwise we run out of memory from wireguard-go goroutine stacks+buffers.
+	// This will be the default later for all platforms and network sizes.
+	iOS := runtime.GOOS == "darwin" && version.IsMobile()
+	if iOS && numPeers > 50 {
+		return false
+	}
+	return true
 }
 
 // isTrimmablePeer reports whether p is a peer that we can trim out of the
@@ -578,7 +594,10 @@ func updateSig(last *string, v interface{}) (changed bool) {
 // simplicity, have only one IP address (an IPv4 /32), which is the
 // common case for most peers. Subnet router nodes will just always be
 // created in the wireguard-go config.
-func isTrimmablePeer(p *wgcfg.Peer) bool {
+func isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool {
+	if forceFullWireguardConfig(numPeers) {
+		return false
+	}
 	if len(p.AllowedIPs) != 1 || len(p.Endpoints) != 1 {
 		return false
 	}
@@ -680,7 +699,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 
 	for i := range full.Peers {
 		p := &full.Peers[i]
-		if !isTrimmablePeer(p) {
+		if !isTrimmablePeer(p, len(full.Peers)) {
 			min.Peers = append(min.Peers, *p)
 			continue
 		}
@@ -693,7 +712,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 		}
 	}
 
-	if !updateSig(&e.lastEngineSigTrim, min) {
+	if !deepprint.UpdateHash(&e.lastEngineSigTrim, min) {
 		// No changes
 		return nil
 	}
@@ -795,8 +814,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 		routerCfg.Domains = append([]string{magicDNSDomain}, routerCfg.Domains...)
 	}
 
-	engineChanged := updateSig(&e.lastEngineSigFull, cfg)
-	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
+	engineChanged := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
+	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}