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>

.github/workflows/staticcheck.yml

@@ -21,6 +21,9 @@ jobs:
     - name: Check out code
       uses: actions/checkout@v1
 
+    - name: Run go vet
+      run: go vet ./...
+
     - name: Print staticcheck version
       run: go run honnef.co/go/tools/cmd/staticcheck -version
 

cmd/cloner/cloner.go

@@ -0,0 +1,264 @@
+// 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.
+
+// Cloner is a tool to automate the creation of a Clone method.
+//
+// The result of the Clone method aliases no memory that can be edited
+// with the original.
+//
+// This tool makes lots of implicit assumptions about the types you feed it.
+// In particular, it can only write relatively "shallow" Clone methods.
+// That is, if a type contains another named struct type, cloner assumes that
+// named type will also have a Clone method.
+package main
+
+import (
+	"bytes"
+	"flag"
+	"fmt"
+	"go/ast"
+	"go/format"
+	"go/token"
+	"go/types"
+	"io/ioutil"
+	"log"
+	"os"
+	"strings"
+
+	"golang.org/x/tools/go/packages"
+)
+
+var (
+	flagTypes     = flag.String("type", "", "comma-separated list of types; required")
+	flagOutput    = flag.String("output", "", "output file; required")
+	flagBuildTags = flag.String("tags", "", "compiler build tags to apply")
+)
+
+func main() {
+	log.SetFlags(0)
+	log.SetPrefix("cloner: ")
+	flag.Parse()
+	if len(*flagTypes) == 0 {
+		flag.Usage()
+		os.Exit(2)
+	}
+	typeNames := strings.Split(*flagTypes, ",")
+
+	cfg := &packages.Config{
+		Mode:  packages.NeedTypes | packages.NeedTypesInfo | packages.NeedSyntax | packages.NeedName,
+		Tests: false,
+	}
+	if *flagBuildTags != "" {
+		cfg.BuildFlags = []string{"-tags=" + *flagBuildTags}
+	}
+	pkgs, err := packages.Load(cfg, ".")
+	if err != nil {
+		log.Fatal(err)
+	}
+	if len(pkgs) != 1 {
+		log.Fatalf("wrong number of packages: %d", len(pkgs))
+	}
+	pkg := pkgs[0]
+	buf := new(bytes.Buffer)
+	imports := make(map[string]struct{})
+	for _, typeName := range typeNames {
+		found := false
+		for _, file := range pkg.Syntax {
+			//var fbuf bytes.Buffer
+			//ast.Fprint(&fbuf, pkg.Fset, file, nil)
+			//fmt.Println(fbuf.String())
+
+			for _, d := range file.Decls {
+				decl, ok := d.(*ast.GenDecl)
+				if !ok || decl.Tok != token.TYPE {
+					continue
+				}
+				for _, s := range decl.Specs {
+					spec, ok := s.(*ast.TypeSpec)
+					if !ok || spec.Name.Name != typeName {
+						continue
+					}
+					typeNameObj := pkg.TypesInfo.Defs[spec.Name]
+					typ, ok := typeNameObj.Type().(*types.Named)
+					if !ok {
+						continue
+					}
+					pkg := typeNameObj.Pkg()
+					gen(buf, imports, typeName, typ, pkg)
+				}
+				found = true
+			}
+		}
+		if !found {
+			log.Fatalf("could not find type %s", typeName)
+		}
+	}
+
+	contents := new(bytes.Buffer)
+	fmt.Fprintf(contents, header, *flagTypes, pkg.Name)
+	fmt.Fprintf(contents, "import (\n")
+	for s := range imports {
+		fmt.Fprintf(contents, "\t%q\n", s)
+	}
+	fmt.Fprintf(contents, ")\n\n")
+	contents.Write(buf.Bytes())
+
+	out, err := format.Source(contents.Bytes())
+	if err != nil {
+		log.Fatalf("%s, in source:\n%s", err, contents.Bytes())
+	}
+
+	output := *flagOutput
+	if output == "" {
+		flag.Usage()
+		os.Exit(2)
+	}
+	if err := ioutil.WriteFile(output, out, 0666); err != nil {
+		log.Fatal(err)
+	}
+}
+
+const header = `// 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 %s; DO NOT EDIT.
+
+package %s
+
+`
+
+func gen(buf *bytes.Buffer, imports map[string]struct{}, name string, typ *types.Named, thisPkg *types.Package) {
+	pkgQual := func(pkg *types.Package) string {
+		if thisPkg == pkg {
+			return ""
+		}
+		imports[pkg.Path()] = struct{}{}
+		return pkg.Name()
+	}
+	importedName := func(t types.Type) string {
+		return types.TypeString(t, pkgQual)
+	}
+
+	switch t := typ.Underlying().(type) {
+	case *types.Struct:
+		_ = t
+		name := typ.Obj().Name()
+		fmt.Fprintf(buf, "// Clone makes a deep copy of %s.\n", name)
+		fmt.Fprintf(buf, "// The result aliases no memory with the original.\n")
+		fmt.Fprintf(buf, "func (src *%s) Clone() *%s {\n", name, name)
+		writef := func(format string, args ...interface{}) {
+			fmt.Fprintf(buf, "\t"+format+"\n", args...)
+		}
+		writef("if src == nil {")
+		writef("\treturn nil")
+		writef("}")
+		writef("dst := new(%s)", name)
+		writef("*dst = *src")
+		for i := 0; i < t.NumFields(); i++ {
+			fname := t.Field(i).Name()
+			ft := t.Field(i).Type()
+			if !containsPointers(ft) {
+				continue
+			}
+			if named, _ := ft.(*types.Named); named != nil && !hasBasicUnderlying(ft) {
+				writef("dst.%s = *src.%s.Clone()", fname, fname)
+				continue
+			}
+			switch ft := ft.Underlying().(type) {
+			case *types.Slice:
+				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 {
+						writef("\tdst.%s[i] = src.%s[i].Clone()", fname, fname)
+					} else {
+						writef("\tdst.%s[i] = *src.%s[i].Clone()", fname, fname)
+					}
+					writef("}")
+				} else {
+					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()) {
+					writef("dst.%s = src.%s.Clone()", fname, fname)
+					continue
+				}
+				n := importedName(ft.Elem())
+				writef("if dst.%s != nil {", fname)
+				writef("\tdst.%s = new(%s)", fname, n)
+				writef("\t*dst.%s = *src.%s", fname, fname)
+				if containsPointers(ft.Elem()) {
+					writef("\t" + `panic("TODO pointers in pointers")`)
+				}
+				writef("}")
+			case *types.Map:
+				writef("if dst.%s != nil {", fname)
+				writef("\tdst.%s = map[%s]%s{}", fname, importedName(ft.Key()), importedName(ft.Elem()))
+				if sliceType, isSlice := ft.Elem().(*types.Slice); isSlice {
+					n := importedName(sliceType.Elem())
+					writef("\tfor k := range src.%s {", fname)
+					// use zero-length slice instead of nil to ensure
+					// the key is always copied.
+					writef("\t\tdst.%s[k] = append([]%s{}, src.%s[k]...)", fname, n, fname)
+					writef("\t}")
+				} else if containsPointers(ft.Elem()) {
+					writef("\t\t" + `panic("TODO map value pointers")`)
+				} else {
+					writef("\tfor k, v := range src.%s {", fname)
+					writef("\t\tdst.%s[k] = v", fname)
+					writef("\t}")
+				}
+				writef("}")
+			case *types.Struct:
+				writef(`panic("TODO struct %s")`, fname)
+			default:
+				writef(`panic(fmt.Sprintf("TODO: %T", ft))`)
+			}
+		}
+		writef("return dst")
+		fmt.Fprintf(buf, "}\n\n")
+	}
+}
+
+func hasBasicUnderlying(typ types.Type) bool {
+	switch typ.Underlying().(type) {
+	case *types.Slice, *types.Map:
+		return true
+	default:
+		return false
+	}
+}
+
+func containsPointers(typ types.Type) bool {
+	switch typ.String() {
+	case "time.Time":
+		// time.Time contains a pointer that does not need copying
+		return false
+	case "inet.af/netaddr.IP":
+		return false
+	}
+	switch ft := typ.Underlying().(type) {
+	case *types.Array:
+		return containsPointers(ft.Elem())
+	case *types.Chan:
+		return true
+	case *types.Interface:
+		return true // a little too broad
+	case *types.Map:
+		return true
+	case *types.Pointer:
+		return true
+	case *types.Slice:
+		return true
+	case *types.Struct:
+		for i := 0; i < ft.NumFields(); i++ {
+			if containsPointers(ft.Field(i).Type()) {
+				return true
+			}
+		}
+	}
+	return false
+}

cmd/derper/derper.go

@@ -185,7 +185,7 @@ func main() {
 		}
 		httpsrv.TLSConfig = certManager.TLSConfig()
 		go func() {
-			err := http.ListenAndServe(":80", certManager.HTTPHandler(tsweb.Port80Handler{mux}))
+			err := http.ListenAndServe(":80", certManager.HTTPHandler(tsweb.Port80Handler{Main: mux}))
 			if err != nil {
 				if err != http.ErrServerClosed {
 					log.Fatal(err)

cmd/tailscale/cli/cli.go

@@ -28,18 +28,22 @@ import (
 // (pty, parent PID), etc.
 func ActLikeCLI() bool {
 	if len(os.Args) < 2 {
-		// TODO: on Windows & Mac, show usage if we're being run with a pty.
 		return false
 	}
 	switch os.Args[1] {
-	case "up", "status", "netcheck":
+	case "up", "status", "netcheck", "version",
+		"-V", "--version", "-h", "--help":
 		return true
 	}
 	return false
 }
 
-// Run runs the CLI. The args do npot include the binary name.
+// Run runs the CLI. The args do not include the binary name.
 func Run(args []string) error {
+	if len(args) == 1 && (args[0] == "-V" || args[0] == "--version") {
+		args = []string{"version"}
+	}
+
 	rootfs := flag.NewFlagSet("tailscale", flag.ExitOnError)
 	rootfs.StringVar(&rootArgs.socket, "socket", paths.DefaultTailscaledSocket(), "path to tailscaled's unix socket")
 
@@ -55,6 +59,7 @@ change in the future.
 			upCmd,
 			netcheckCmd,
 			statusCmd,
+			versionCmd,
 		},
 		FlagSet: rootfs,
 		Exec:    func(context.Context, []string) error { return flag.ErrHelp },

cmd/tailscale/cli/up.go

@@ -48,6 +48,7 @@ specify any flags, options are reset to their default.
 		upf := flag.NewFlagSet("up", flag.ExitOnError)
 		upf.StringVar(&upArgs.server, "login-server", "https://login.tailscale.com", "base URL of control server")
 		upf.BoolVar(&upArgs.acceptRoutes, "accept-routes", false, "accept routes advertised by other Tailscale nodes")
+		upf.BoolVar(&upArgs.acceptDNS, "accept-dns", true, "accept DNS configuration from the admin panel")
 		upf.BoolVar(&upArgs.singleRoutes, "host-routes", true, "install host routes to other Tailscale nodes")
 		upf.BoolVar(&upArgs.shieldsUp, "shields-up", false, "don't allow incoming connections")
 		upf.StringVar(&upArgs.advertiseTags, "advertise-tags", "", "ACL tags to request (comma-separated, e.g. eng,montreal,ssh)")
@@ -69,6 +70,7 @@ specify any flags, options are reset to their default.
 var upArgs struct {
 	server          string
 	acceptRoutes    bool
+	acceptDNS       bool
 	singleRoutes    bool
 	shieldsUp       bool
 	advertiseRoutes string
@@ -97,9 +99,9 @@ func parseIPOrCIDR(s string) (wgcfg.CIDR, bool) {
 		return wgcfg.CIDR{}, false
 	}
 	if ip.Is4() {
-		return wgcfg.CIDR{ip, 32}, true
+		return wgcfg.CIDR{IP: ip, Mask: 32}, true
 	} else {
-		return wgcfg.CIDR{ip, 128}, true
+		return wgcfg.CIDR{IP: ip, Mask: 128}, true
 	}
 }
 
@@ -178,6 +180,7 @@ func runUp(ctx context.Context, args []string) error {
 	prefs.ControlURL = upArgs.server
 	prefs.WantRunning = true
 	prefs.RouteAll = upArgs.acceptRoutes
+	prefs.CorpDNS = upArgs.acceptDNS
 	prefs.AllowSingleHosts = upArgs.singleRoutes
 	prefs.ShieldsUp = upArgs.shieldsUp
 	prefs.AdvertiseRoutes = routes

cmd/tailscale/cli/version.go

@@ -0,0 +1,69 @@
+// 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 cli
+
+import (
+	"context"
+	"flag"
+	"fmt"
+	"log"
+
+	"github.com/peterbourgon/ff/v2/ffcli"
+	"tailscale.com/ipn"
+	"tailscale.com/version"
+)
+
+var versionCmd = &ffcli.Command{
+	Name:       "version",
+	ShortUsage: "version [flags]",
+	ShortHelp:  "Print Tailscale version",
+	FlagSet: (func() *flag.FlagSet {
+		fs := flag.NewFlagSet("version", flag.ExitOnError)
+		fs.BoolVar(&versionArgs.daemon, "daemon", false, "also print local node's daemon version")
+		return fs
+	})(),
+	Exec: runVersion,
+}
+
+var versionArgs struct {
+	daemon bool // also check local node's daemon version
+}
+
+func runVersion(ctx context.Context, args []string) error {
+	if len(args) > 0 {
+		log.Fatalf("too many non-flag arguments: %q", args)
+	}
+	if !versionArgs.daemon {
+		fmt.Println(version.LONG)
+		return nil
+	}
+	fmt.Printf("Client: %s\n", version.LONG)
+
+	c, bc, ctx, cancel := connect(ctx)
+	defer cancel()
+
+	bc.AllowVersionSkew = true
+
+	done := make(chan struct{})
+
+	bc.SetNotifyCallback(func(n ipn.Notify) {
+		if n.ErrMessage != nil {
+			log.Fatal(*n.ErrMessage)
+		}
+		if n.Status != nil {
+			fmt.Printf("Daemon: %s\n", n.Version)
+			close(done)
+		}
+	})
+	go pump(ctx, bc, c)
+
+	bc.RequestStatus()
+	select {
+	case <-done:
+		return nil
+	case <-ctx.Done():
+		return ctx.Err()
+	}
+}

cmd/tailscaled/tailscaled.go

@@ -52,6 +52,16 @@ func defaultTunName() string {
 	return "tailscale0"
 }
 
+var args struct {
+	cleanup    bool
+	fake       bool
+	debug      string
+	tunname    string
+	port       uint16
+	statepath  string
+	socketpath string
+}
+
 func main() {
 	// We aren't very performance sensitive, and the parts that are
 	// performance sensitive (wireguard) try hard not to do any memory
@@ -61,55 +71,78 @@ func main() {
 		debug.SetGCPercent(10)
 	}
 
-	cleanup := getopt.BoolLong("cleanup", 0, "clean up system state and exit")
-	fake := getopt.BoolLong("fake", 0, "fake tunnel+routing instead of tuntap")
-	debug := getopt.StringLong("debug", 0, "", "Address of debug server")
-	tunname := getopt.StringLong("tun", 0, defaultTunName(), "tunnel interface name")
-	listenport := getopt.Uint16Long("port", 'p', magicsock.DefaultPort, "WireGuard port (0=autoselect)")
-	statepath := getopt.StringLong("state", 0, paths.DefaultTailscaledStateFile(), "Path of state file")
-	socketpath := getopt.StringLong("socket", 's', paths.DefaultTailscaledSocket(), "Path of the service unix socket")
+	// Set default values for getopt.
+	args.tunname = defaultTunName()
+	args.port = magicsock.DefaultPort
+	args.statepath = paths.DefaultTailscaledStateFile()
+	args.socketpath = paths.DefaultTailscaledSocket()
 
-	logf := wgengine.RusagePrefixLog(log.Printf)
-	logf = logger.RateLimitedFn(logf, 5*time.Second, 5, 100)
+	getopt.FlagLong(&args.cleanup, "cleanup", 0, "clean up system state and exit")
+	getopt.FlagLong(&args.fake, "fake", 0, "fake tunnel+routing instead of tuntap")
+	getopt.FlagLong(&args.debug, "debug", 0, "address of debug server")
+	getopt.FlagLong(&args.tunname, "tun", 0, "tunnel interface name")
+	getopt.FlagLong(&args.port, "port", 'p', "WireGuard port (0=autoselect)")
+	getopt.FlagLong(&args.statepath, "state", 0, "path of state file")
+	getopt.FlagLong(&args.socketpath, "socket", 's', "path of the service unix socket")
 
 	err := fixconsole.FixConsoleIfNeeded()
 	if err != nil {
-		logf("fixConsoleOutput: %v", err)
+		log.Fatalf("fixConsoleOutput: %v", err)
 	}
-	pol := logpolicy.New("tailnode.log.tailscale.io")
 
 	getopt.Parse()
 	if len(getopt.Args()) > 0 {
 		log.Fatalf("too many non-flag arguments: %#v", getopt.Args()[0])
 	}
 
-	if *cleanup {
-		router.Cleanup(logf, *tunname)
-		return
-	}
-
-	if *statepath == "" {
+	if args.statepath == "" {
 		log.Fatalf("--state is required")
 	}
 
-	if *socketpath == "" && runtime.GOOS != "windows" {
+	if args.socketpath == "" && runtime.GOOS != "windows" {
 		log.Fatalf("--socket is required")
 	}
 
+	if err := run(); err != nil {
+		// No need to log; the func already did
+		os.Exit(1)
+	}
+}
+
+func run() error {
+	var err error
+
+	pol := logpolicy.New("tailnode.log.tailscale.io")
+	defer func() {
+		// Finish uploading logs after closing everything else.
+		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+		defer cancel()
+		pol.Shutdown(ctx)
+	}()
+
+	logf := wgengine.RusagePrefixLog(log.Printf)
+	logf = logger.RateLimitedFn(logf, 5*time.Second, 5, 100)
+
+	if args.cleanup {
+		router.Cleanup(logf, args.tunname)
+		return nil
+	}
+
 	var debugMux *http.ServeMux
-	if *debug != "" {
+	if args.debug != "" {
 		debugMux = newDebugMux()
-		go runDebugServer(debugMux, *debug)
+		go runDebugServer(debugMux, args.debug)
 	}
 
 	var e wgengine.Engine
-	if *fake {
+	if args.fake {
 		e, err = wgengine.NewFakeUserspaceEngine(logf, 0)
 	} else {
-		e, err = wgengine.NewUserspaceEngine(logf, *tunname, *listenport)
+		e, err = wgengine.NewUserspaceEngine(logf, args.tunname, args.port)
 	}
 	if err != nil {
-		log.Fatalf("wgengine.New: %v", err)
+		logf("wgengine.New: %v", err)
+		return err
 	}
 	e = wgengine.NewWatchdog(e)
 
@@ -128,23 +161,22 @@ func main() {
 	}()
 
 	opts := ipnserver.Options{
-		SocketPath:         *socketpath,
+		SocketPath:         args.socketpath,
 		Port:               41112,
-		StatePath:          *statepath,
+		StatePath:          args.statepath,
 		AutostartStateKey:  globalStateKey,
 		LegacyConfigPath:   paths.LegacyConfigPath(),
 		SurviveDisconnects: true,
 		DebugMux:           debugMux,
 	}
-	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), opts, e)
-	if err != nil {
-		log.Fatalf("tailscaled: %v", err)
+	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)
+		return err
 	}
 
-	// Finish uploading logs after closing everything else.
-	ctx, cancel = context.WithTimeout(context.Background(), time.Second)
-	cancel()
-	pol.Shutdown(ctx)
+	return nil
 }
 
 func newDebugMux() *http.ServeMux {

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

@@ -5,12 +5,12 @@
 package controlclient
 
 import (
-	"bytes"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"log"
+	"net"
 	"reflect"
+	"strconv"
 	"strings"
 	"time"
 
@@ -23,13 +23,15 @@ 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
-	Peers         []*tailcfg.Node
+	Peers         []*tailcfg.Node // sorted by Node.ID
 	DNS           []wgcfg.IP
 	DNSDomains    []string
 	Hostinfo      tailcfg.Hostinfo
@@ -54,25 +56,25 @@ type NetworkMap struct {
 	// TODO(crawshaw): Capabilities []tailcfg.Capability
 }
 
-func (n *NetworkMap) Equal(n2 *NetworkMap) bool {
-	// TODO(crawshaw): this is crude, but is an easy way to avoid bugs.
-	b, err := json.Marshal(n)
-	if err != nil {
-		panic(err)
-	}
-	b2, err := json.Marshal(n2)
-	if err != nil {
-		panic(err)
-	}
-	return bytes.Equal(b, b2)
-}
-
 func (nm NetworkMap) String() string {
 	return nm.Concise()
 }
 
 func (nm *NetworkMap) Concise() string {
 	buf := new(strings.Builder)
+
+	nm.printConciseHeader(buf)
+	for _, p := range nm.Peers {
+		printPeerConcise(buf, p)
+	}
+	return buf.String()
+}
+
+// printConciseHeader prints a concise header line representing nm to buf.
+//
+// If this function is changed to access different fields of nm, keep
+// in equalConciseHeader in sync.
+func (nm *NetworkMap) printConciseHeader(buf *strings.Builder) {
 	fmt.Fprintf(buf, "netmap: self: %v auth=%v",
 		nm.NodeKey.ShortString(), nm.MachineStatus)
 	if nm.LocalPort != 0 {
@@ -84,72 +86,116 @@ func (nm *NetworkMap) Concise() string {
 	}
 	fmt.Fprintf(buf, " %v", nm.Addresses)
 	buf.WriteByte('\n')
-	for _, p := range nm.Peers {
-		aip := make([]string, len(p.AllowedIPs))
-		for i, a := range p.AllowedIPs {
-			s := strings.TrimSuffix(fmt.Sprint(a), "/32")
-			aip[i] = s
-		}
+}
 
-		ep := make([]string, len(p.Endpoints))
-		for i, e := range p.Endpoints {
-			// Align vertically on the ':' between IP and port
-			colon := strings.IndexByte(e, ':')
-			spaces := 0
-			for colon > 0 && len(e)+spaces-colon < 6 {
-				spaces++
-				colon--
-			}
-			ep[i] = fmt.Sprintf("%21v", e+strings.Repeat(" ", spaces))
-		}
-
-		derp := p.DERP
-		const derpPrefix = "127.3.3.40:"
-		if strings.HasPrefix(derp, derpPrefix) {
-			derp = "D" + derp[len(derpPrefix):]
-		}
-
-		// Most of the time, aip is just one element, so format the
-		// table to look good in that case. This will also make multi-
-		// subnet nodes stand out visually.
-		fmt.Fprintf(buf, " %v %-2v %-15v : %v\n",
-			p.Key.ShortString(), derp,
-			strings.Join(aip, " "),
-			strings.Join(ep, " "))
+// equalConciseHeader reports whether a and b are equal for the fields
+// used by printConciseHeader.
+func (a *NetworkMap) equalConciseHeader(b *NetworkMap) bool {
+	if a.NodeKey != b.NodeKey ||
+		a.MachineStatus != b.MachineStatus ||
+		a.LocalPort != b.LocalPort ||
+		len(a.Addresses) != len(b.Addresses) {
+		return false
 	}
-	return buf.String()
+	for i, a := range a.Addresses {
+		if b.Addresses[i] != a {
+			return false
+		}
+	}
+	return (a.Debug == nil && b.Debug == nil) || reflect.DeepEqual(a.Debug, b.Debug)
+}
+
+// printPeerConcise appends to buf a line repsenting the peer p.
+//
+// If this function is changed to access different fields of p, keep
+// in nodeConciseEqual in sync.
+func printPeerConcise(buf *strings.Builder, p *tailcfg.Node) {
+	aip := make([]string, len(p.AllowedIPs))
+	for i, a := range p.AllowedIPs {
+		s := strings.TrimSuffix(fmt.Sprint(a), "/32")
+		aip[i] = s
+	}
+
+	ep := make([]string, len(p.Endpoints))
+	for i, e := range p.Endpoints {
+		// Align vertically on the ':' between IP and port
+		colon := strings.IndexByte(e, ':')
+		spaces := 0
+		for colon > 0 && len(e)+spaces-colon < 6 {
+			spaces++
+			colon--
+		}
+		ep[i] = fmt.Sprintf("%21v", e+strings.Repeat(" ", spaces))
+	}
+
+	derp := p.DERP
+	const derpPrefix = "127.3.3.40:"
+	if strings.HasPrefix(derp, derpPrefix) {
+		derp = "D" + derp[len(derpPrefix):]
+	}
+
+	// Most of the time, aip is just one element, so format the
+	// table to look good in that case. This will also make multi-
+	// subnet nodes stand out visually.
+	fmt.Fprintf(buf, " %v %-2v %-15v : %v\n",
+		p.Key.ShortString(), derp,
+		strings.Join(aip, " "),
+		strings.Join(ep, " "))
+}
+
+// nodeConciseEqual reports whether a and b are equal for the fields accessed by printPeerConcise.
+func nodeConciseEqual(a, b *tailcfg.Node) bool {
+	return a.Key == b.Key &&
+		a.DERP == b.DERP &&
+		eqCIDRsIgnoreNil(a.AllowedIPs, b.AllowedIPs) &&
+		eqStringsIgnoreNil(a.Endpoints, b.Endpoints)
 }
 
 func (b *NetworkMap) ConciseDiffFrom(a *NetworkMap) string {
-	if reflect.DeepEqual(a, b) {
-		// Fast path that only does one allocation.
-		return ""
-	}
-	out := []string{}
-	ra := strings.Split(a.Concise(), "\n")
-	rb := strings.Split(b.Concise(), "\n")
+	var diff strings.Builder
 
-	ma := map[string]struct{}{}
-	for _, s := range ra {
-		ma[s] = struct{}{}
+	// See if header (non-peers, "bare") part of the network map changed.
+	// If so, print its diff lines first.
+	if !a.equalConciseHeader(b) {
+		diff.WriteByte('-')
+		a.printConciseHeader(&diff)
+		diff.WriteByte('+')
+		b.printConciseHeader(&diff)
 	}
 
-	mb := map[string]struct{}{}
-	for _, s := range rb {
-		mb[s] = struct{}{}
-	}
-
-	for _, s := range ra {
-		if _, ok := mb[s]; !ok {
-			out = append(out, "-"+s)
+	aps, bps := a.Peers, b.Peers
+	for len(aps) > 0 && len(bps) > 0 {
+		pa, pb := aps[0], bps[0]
+		switch {
+		case pa.ID == pb.ID:
+			if !nodeConciseEqual(pa, pb) {
+				diff.WriteByte('-')
+				printPeerConcise(&diff, pa)
+				diff.WriteByte('+')
+				printPeerConcise(&diff, pb)
+			}
+			aps, bps = aps[1:], bps[1:]
+		case pa.ID > pb.ID:
+			// New peer in b.
+			diff.WriteByte('+')
+			printPeerConcise(&diff, pb)
+			bps = bps[1:]
+		case pb.ID > pa.ID:
+			// Deleted peer in b.
+			diff.WriteByte('-')
+			printPeerConcise(&diff, pa)
+			aps = aps[1:]
 		}
 	}
-	for _, s := range rb {
-		if _, ok := ma[s]; !ok {
-			out = append(out, "+"+s)
-		}
+	for _, pa := range aps {
+		diff.WriteByte('-')
+		printPeerConcise(&diff, pa)
 	}
-	return strings.Join(out, "\n")
+	for _, pb := range bps {
+		diff.WriteByte('+')
+		printPeerConcise(&diff, pb)
+	}
+	return diff.String()
 }
 
 func (nm *NetworkMap) JSON() string {
@@ -160,141 +206,141 @@ func (nm *NetworkMap) JSON() string {
 	return string(b)
 }
 
+// WGConfigFlags is a bitmask of flags to control the behavior of the
+// wireguard configuration generation done by NetMap.WGCfg.
+type WGConfigFlags int
+
 const (
-	UAllowSingleHosts = 1 << iota
-	UAllowSubnetRoutes
-	UAllowDefaultRoute
-	UHackDefaultRoute
-
-	UDefault = 0
+	AllowSingleHosts WGConfigFlags = 1 << iota
+	AllowSubnetRoutes
+	AllowDefaultRoute
+	HackDefaultRoute
 )
 
-// Several programs need to parse these arguments into uflags, so let's
-// centralize it here.
-func UFlagsHelper(uroutes, rroutes, droutes bool) int {
-	uflags := 0
-	if uroutes {
-		uflags |= UAllowSingleHosts
-	}
-	if rroutes {
-		uflags |= UAllowSubnetRoutes
-	}
-	if droutes {
-		uflags |= UAllowDefaultRoute
-	}
-	return uflags
-}
-
 // TODO(bradfitz): UAPI seems to only be used by the old confnode and
 // pingnode; delete this when those are deleted/rewritten?
-func (nm *NetworkMap) UAPI(uflags int, dnsOverride []wgcfg.IP) string {
-	wgcfg, err := nm.WGCfg(log.Printf, uflags, dnsOverride)
+func (nm *NetworkMap) UAPI(flags WGConfigFlags, dnsOverride []wgcfg.IP) string {
+	wgcfg, err := nm.WGCfg(log.Printf, flags, dnsOverride)
 	if err != nil {
-		log.Fatalf("WGCfg() failed unexpectedly: %v\n", err)
+		log.Fatalf("WGCfg() failed unexpectedly: %v", err)
 	}
 	s, err := wgcfg.ToUAPI()
 	if err != nil {
-		log.Fatalf("ToUAPI() failed unexpectedly: %v\n", err)
+		log.Fatalf("ToUAPI() failed unexpectedly: %v", err)
 	}
 	return s
 }
 
-func (nm *NetworkMap) WGCfg(logf logger.Logf, uflags int, dnsOverride []wgcfg.IP) (*wgcfg.Config, error) {
-	s := nm._WireGuardConfig(logf, uflags, dnsOverride, true)
-	return wgcfg.FromWgQuick(s, "tailscale")
-}
-
 // EndpointDiscoSuffix is appended to the hex representation of a peer's discovery key
 // and is then the sole wireguard endpoint for peers with a non-zero discovery key.
 // This form is then recognize by magicsock's CreateEndpoint.
 const EndpointDiscoSuffix = ".disco.tailscale:12345"
 
-func (nm *NetworkMap) _WireGuardConfig(logf logger.Logf, uflags int, dnsOverride []wgcfg.IP, allEndpoints bool) string {
-	buf := new(strings.Builder)
-	fmt.Fprintf(buf, "[Interface]\n")
-	fmt.Fprintf(buf, "PrivateKey = %s\n", base64.StdEncoding.EncodeToString(nm.PrivateKey[:]))
-	if len(nm.Addresses) > 0 {
-		fmt.Fprintf(buf, "Address = ")
-		for i, cidr := range nm.Addresses {
-			if i > 0 {
-				fmt.Fprintf(buf, ", ")
-			}
-			fmt.Fprintf(buf, "%s", cidr)
-		}
-		fmt.Fprintf(buf, "\n")
+// WGCfg returns the NetworkMaps's Wireguard configuration.
+func (nm *NetworkMap) WGCfg(logf logger.Logf, flags WGConfigFlags, dnsOverride []wgcfg.IP) (*wgcfg.Config, error) {
+	cfg := &wgcfg.Config{
+		Name:       "tailscale",
+		PrivateKey: nm.PrivateKey,
+		Addresses:  nm.Addresses,
+		ListenPort: nm.LocalPort,
+		DNS:        append([]wgcfg.IP(nil), dnsOverride...),
+		Peers:      make([]wgcfg.Peer, 0, len(nm.Peers)),
 	}
-	fmt.Fprintf(buf, "ListenPort = %d\n", nm.LocalPort)
-	if len(dnsOverride) > 0 {
-		dnss := []string{}
-		for _, ip := range dnsOverride {
-			dnss = append(dnss, ip.String())
-		}
-		fmt.Fprintf(buf, "DNS = %s\n", strings.Join(dnss, ","))
-	}
-	fmt.Fprintf(buf, "\n")
 
-	for i, peer := range nm.Peers {
+	for _, peer := range nm.Peers {
 		if Debug.OnlyDisco && peer.DiscoKey.IsZero() {
 			continue
 		}
-		if (uflags&UAllowSingleHosts) == 0 && len(peer.AllowedIPs) < 2 {
-			logf("wgcfg: %v skipping a single-host peer.\n", peer.Key.ShortString())
+		if (flags&AllowSingleHosts) == 0 && len(peer.AllowedIPs) < 2 {
+			logf("wgcfg: %v skipping a single-host peer.", peer.Key.ShortString())
 			continue
 		}
-		if i > 0 {
-			fmt.Fprintf(buf, "\n")
+		cfg.Peers = append(cfg.Peers, wgcfg.Peer{
+			PublicKey: wgcfg.Key(peer.Key),
+		})
+		cpeer := &cfg.Peers[len(cfg.Peers)-1]
+		if peer.KeepAlive {
+			cpeer.PersistentKeepalive = 25 // seconds
 		}
-		fmt.Fprintf(buf, "[Peer]\n")
-		fmt.Fprintf(buf, "PublicKey = %s\n", base64.StdEncoding.EncodeToString(peer.Key[:]))
-		var endpoints []string
+
 		if !peer.DiscoKey.IsZero() {
-			fmt.Fprintf(buf, "Endpoint = %x%s\n", peer.DiscoKey[:], EndpointDiscoSuffix)
-		} else {
-			if peer.DERP != "" {
-				endpoints = append(endpoints, peer.DERP)
+			if err := appendEndpoint(cpeer, fmt.Sprintf("%x%s", peer.DiscoKey[:], EndpointDiscoSuffix)); err != nil {
+				return nil, err
 			}
-			endpoints = append(endpoints, peer.Endpoints...)
-			if len(endpoints) > 0 {
-				if len(endpoints) == 1 {
-					fmt.Fprintf(buf, "Endpoint = %s", endpoints[0])
-				} else if allEndpoints {
-					// TODO(apenwarr): This mode is incompatible.
-					// Normal wireguard clients don't know how to
-					// parse it (yet?)
-					fmt.Fprintf(buf, "Endpoint = %s", strings.Join(endpoints, ","))
-				} else {
-					fmt.Fprintf(buf, "Endpoint = %s # other endpoints: %s",
-						endpoints[0],
-						strings.Join(endpoints[1:], ", "))
+			cpeer.Endpoints = []wgcfg.Endpoint{{Host: fmt.Sprintf("%x.disco.tailscale", peer.DiscoKey[:]), Port: 12345}}
+		} else {
+			if err := appendEndpoint(cpeer, peer.DERP); err != nil {
+				return nil, err
+			}
+			for _, ep := range peer.Endpoints {
+				if err := appendEndpoint(cpeer, ep); err != nil {
+					return nil, err
 				}
-				buf.WriteByte('\n')
 			}
 		}
-		var aips []string
 		for _, allowedIP := range peer.AllowedIPs {
-			aip := allowedIP.String()
 			if allowedIP.Mask == 0 {
-				if (uflags & UAllowDefaultRoute) == 0 {
-					logf("wgcfg: %v skipping default route\n", peer.Key.ShortString())
+				if (flags & AllowDefaultRoute) == 0 {
+					logf("wgcfg: %v skipping default route", peer.Key.ShortString())
 					continue
 				}
-				if (uflags & UHackDefaultRoute) != 0 {
-					aip = "10.0.0.0/8"
-					logf("wgcfg: %v converting default route => %v\n", peer.Key.ShortString(), aip)
+				if (flags & HackDefaultRoute) != 0 {
+					allowedIP = wgcfg.CIDR{IP: wgcfg.IPv4(10, 0, 0, 0), Mask: 8}
+					logf("wgcfg: %v converting default route => %v", peer.Key.ShortString(), allowedIP.String())
 				}
 			} else if allowedIP.Mask < 32 {
-				if (uflags & UAllowSubnetRoutes) == 0 {
-					logf("wgcfg: %v skipping subnet route\n", peer.Key.ShortString())
+				if (flags & AllowSubnetRoutes) == 0 {
+					logf("wgcfg: %v skipping subnet route", peer.Key.ShortString())
 					continue
 				}
 			}
-			aips = append(aips, aip)
-		}
-		fmt.Fprintf(buf, "AllowedIPs = %s\n", strings.Join(aips, ", "))
-		if peer.KeepAlive {
-			fmt.Fprintf(buf, "PersistentKeepalive = 25\n")
+			cpeer.AllowedIPs = append(cpeer.AllowedIPs, allowedIP)
 		}
 	}
 
-	return buf.String()
+	return cfg, nil
+}
+
+func appendEndpoint(peer *wgcfg.Peer, epStr string) error {
+	if epStr == "" {
+		return nil
+	}
+	host, port, err := net.SplitHostPort(epStr)
+	if err != nil {
+		return fmt.Errorf("malformed endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
+	}
+	port16, err := strconv.ParseUint(port, 10, 16)
+	if err != nil {
+		return fmt.Errorf("invalid port in endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
+	}
+	peer.Endpoints = append(peer.Endpoints, wgcfg.Endpoint{Host: host, Port: uint16(port16)})
+	return nil
+}
+
+// eqStringsIgnoreNil reports whether a and b have the same length and
+// contents, but ignore whether a or b are nil.
+func eqStringsIgnoreNil(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i, v := range a {
+		if v != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+// eqCIDRsIgnoreNil reports whether a and b have the same length and
+// contents, but ignore whether a or b are nil.
+func eqCIDRsIgnoreNil(a, b []wgcfg.CIDR) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i, v := range a {
+		if v != b[i] {
+			return false
+		}
+	}
+	return true
 }

control/controlclient/netmap_test.go

@@ -10,13 +10,14 @@ import (
 	"tailscale.com/tailcfg"
 )
 
-func TestNetworkMapConcise(t *testing.T) {
-	nodekey := func(b byte) (ret tailcfg.NodeKey) {
-		for i := range ret {
-			ret[i] = b
-		}
-		return
+func testNodeKey(b byte) (ret tailcfg.NodeKey) {
+	for i := range ret {
+		ret[i] = b
 	}
+	return
+}
+
+func TestNetworkMapConcise(t *testing.T) {
 	for _, tt := range []struct {
 		name string
 		nm   *NetworkMap
@@ -25,15 +26,15 @@ func TestNetworkMapConcise(t *testing.T) {
 		{
 			name: "basic",
 			nm: &NetworkMap{
-				NodeKey: nodekey(1),
+				NodeKey: testNodeKey(1),
 				Peers: []*tailcfg.Node{
 					{
-						Key:       nodekey(2),
+						Key:       testNodeKey(2),
 						DERP:      "127.3.3.40:2",
 						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
 					},
 					{
-						Key:       nodekey(3),
+						Key:       testNodeKey(3),
 						DERP:      "127.3.3.40:4",
 						Endpoints: []string{"10.2.0.100:12", "10.1.0.100:12345"},
 					},
@@ -44,7 +45,7 @@ func TestNetworkMapConcise(t *testing.T) {
 		{
 			name: "debug_non_nil",
 			nm: &NetworkMap{
-				NodeKey: nodekey(1),
+				NodeKey: testNodeKey(1),
 				Debug:   &tailcfg.Debug{},
 			},
 			want: "netmap: self: [AQEBA] auth=machine-unknown debug={} []\n",
@@ -52,7 +53,7 @@ func TestNetworkMapConcise(t *testing.T) {
 		{
 			name: "debug_values",
 			nm: &NetworkMap{
-				NodeKey: nodekey(1),
+				NodeKey: testNodeKey(1),
 				Debug:   &tailcfg.Debug{LogHeapPprof: true},
 			},
 			want: "netmap: self: [AQEBA] auth=machine-unknown debug={\"LogHeapPprof\":true} []\n",
@@ -70,3 +71,147 @@ func TestNetworkMapConcise(t *testing.T) {
 		})
 	}
 }
+
+func TestConciseDiffFrom(t *testing.T) {
+	for _, tt := range []struct {
+		name string
+		a, b *NetworkMap
+		want string
+	}{
+		{
+			name: "no_change",
+			a: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			b: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			want: "",
+		},
+		{
+			name: "header_change",
+			a: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			b: &NetworkMap{
+				NodeKey: testNodeKey(2),
+				Peers: []*tailcfg.Node{
+					{
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			want: "-netmap: self: [AQEBA] auth=machine-unknown []\n+netmap: self: [AgICA] auth=machine-unknown []\n",
+		},
+		{
+			name: "peer_add",
+			a: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:        2,
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			b: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:        1,
+						Key:       testNodeKey(1),
+						DERP:      "127.3.3.40:1",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+					{
+						ID:        2,
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+					{
+						ID:        3,
+						Key:       testNodeKey(3),
+						DERP:      "127.3.3.40:3",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			want: "+ [AQEBA] D1                 :    192.168.0.100:12     192.168.0.100:12354\n+ [AwMDA] D3                 :    192.168.0.100:12     192.168.0.100:12354\n",
+		},
+		{
+			name: "peer_remove",
+			a: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:        1,
+						Key:       testNodeKey(1),
+						DERP:      "127.3.3.40:1",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+					{
+						ID:        2,
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+					{
+						ID:        3,
+						Key:       testNodeKey(3),
+						DERP:      "127.3.3.40:3",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			b: &NetworkMap{
+				NodeKey: testNodeKey(1),
+				Peers: []*tailcfg.Node{
+					{
+						ID:        2,
+						Key:       testNodeKey(2),
+						DERP:      "127.3.3.40:2",
+						Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"},
+					},
+				},
+			},
+			want: "- [AQEBA] D1                 :    192.168.0.100:12     192.168.0.100:12354\n- [AwMDA] D3                 :    192.168.0.100:12     192.168.0.100:12354\n",
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			var got string
+			n := int(testing.AllocsPerRun(50, func() {
+				got = tt.b.ConciseDiffFrom(tt.a)
+			}))
+			t.Logf("Allocs = %d", n)
+			if got != tt.want {
+				t.Errorf("Wrong output\n Got: %q\nWant: %q\n## Got (unescaped):\n%s\n## Want (unescaped):\n%s\n", got, tt.want, got, tt.want)
+			}
+		})
+	}
+}

derp/derp_server.go

@@ -39,7 +39,11 @@ const (
 )
 
 const host64bit = (^uint(0) >> 32) & 1 // 1 on 64-bit, 0 on 32-bit
-const pad32bit = 4 - host64bit*4       // 0 on 64-bit, 4 on 32-bit
+
+// pad32bit is 4 on 32-bit machines and 0 on 64-bit.
+// It exists so the Server struct's atomic fields can be aligned to 8
+// byte boundaries. (As tested by GOARCH=386 go test, etc)
+const pad32bit = 4 - host64bit*4 // 0 on 64-bit, 4 on 32-bit
 
 // Server is a DERP server.
 type Server struct {

derp/derphttp/derphttp_client.go

@@ -338,6 +338,9 @@ func (c *Client) dialRegion(ctx context.Context, reg *tailcfg.DERPRegion) (net.C
 	var firstErr error
 	for _, n := range reg.Nodes {
 		if n.STUNOnly {
+			if firstErr == nil {
+				firstErr = fmt.Errorf("no non-STUNOnly nodes for %s", c.targetString(reg))
+			}
 			continue
 		}
 		c, err := c.dialNode(ctx, n)

disco/disco.go

@@ -31,6 +31,8 @@ import (
 // Magic is the 6 byte header of all discovery messages.
 const Magic = "TS💬" // 6 bytes: 0x54 53 f0 9f 92 ac
 
+const keyLen = 32
+
 // NonceLen is the length of the nonces used by nacl secretboxes.
 const NonceLen = 24
 
@@ -46,6 +48,15 @@ const v0 = byte(0)
 
 var errShort = errors.New("short message")
 
+// LooksLikeDiscoWrapper reports whether p looks like it's a packet
+// containing an encrypted disco message.
+func LooksLikeDiscoWrapper(p []byte) bool {
+	if len(p) < len(Magic)+keyLen+NonceLen {
+		return false
+	}
+	return string(p[:len(Magic)]) == Magic
+}
+
 // Parse parses the encrypted part of the message from inside the
 // nacl secretbox.
 func Parse(p []byte) (Message, error) {

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3
 	github.com/peterbourgon/ff/v2 v2.0.0
 	github.com/tailscale/winipcfg-go v0.0.0-20200413171540-609dcf2df55f
-	github.com/tailscale/wireguard-go v0.0.0-20200716032321-dd6c1c8fe14c
+	github.com/tailscale/wireguard-go v0.0.0-20200724155040-d554a2a5e7e1
 	github.com/tcnksm/go-httpstat v0.2.0
 	github.com/toqueteos/webbrowser v1.2.0
 	go4.org/mem v0.0.0-20200706164138-185c595c3ecc
@@ -31,7 +31,8 @@ require (
 	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
 	golang.org/x/sys v0.0.0-20200501052902-10377860bb8e
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0
+	golang.org/x/tools v0.0.0-20191216052735-49a3e744a425
 	honnef.co/go/tools v0.0.1-2020.1.4
-	inet.af/netaddr v0.0.0-20200706235120-1ac1a40fae99
+	inet.af/netaddr v0.0.0-20200718043157-99321d6ad24c
 	rsc.io/goversion v1.2.0
 )

go.sum

@@ -30,7 +30,6 @@ github.com/gliderlabs/ssh v0.2.2 h1:6zsha5zo/TWhRhwqCD3+EarCAgZ2yN28ipRnGPnwkI0=
 github.com/gliderlabs/ssh v0.2.2/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0=
 github.com/go-ole/go-ole v1.2.4 h1:nNBDSCOigTSiarFpYE9J/KtEA1IOW4CNeqT9TQDqCxI=
 github.com/go-ole/go-ole v1.2.4/go.mod h1:XCwSNxSkXRo4vlyPy93sltvi/qJq0jqQhjqQNIwKuxM=
-github.com/godbus/dbus v4.1.0+incompatible h1:WqqLRTsQic3apZUK9qC5sGNfXthmPXzUZ7nQPrNITa4=
 github.com/godbus/dbus/v5 v5.0.3 h1:ZqHaoEF7TBzh4jzPmqVhE/5A1z9of6orkAe5uHoAeME=
 github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e h1:1r7pUrabqp18hOBcwBwiTsbnFeTZHV9eER/QT5JVZxY=
@@ -87,10 +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-20200710044538-9320f191f6b1 h1:zMEeWu/X0l+xFnsbri69miflb3HIKoLwedZHD5xx6Mk=
-github.com/tailscale/wireguard-go v0.0.0-20200710044538-9320f191f6b1/go.mod h1:JPm5cTfu1K+qDFRbiHy0sOlHUylYQbpl356sdYFD8V4=
-github.com/tailscale/wireguard-go v0.0.0-20200716032321-dd6c1c8fe14c h1:45GoTCd7XoVVet8ws6q1p8DBvWz3tDrUZ60030+Y+C4=
-github.com/tailscale/wireguard-go v0.0.0-20200716032321-dd6c1c8fe14c/go.mod h1:JPm5cTfu1K+qDFRbiHy0sOlHUylYQbpl356sdYFD8V4=
+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=
@@ -170,7 +167,7 @@ gopkg.in/yaml.v2 v2.2.7 h1:VUgggvou5XRW9mHwD/yXxIYSMtY0zoKQf/v226p2nyo=
 gopkg.in/yaml.v2 v2.2.7/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK8=
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
-inet.af/netaddr v0.0.0-20200706235120-1ac1a40fae99 h1:+43CBpWlrXThaOxixPS5JXEJZC8zaMCpDu3aKffe0bs=
-inet.af/netaddr v0.0.0-20200706235120-1ac1a40fae99/go.mod h1:qqYzz/2whtrbWJvt+DNWQyvekNN4ePQZcg2xc2/Yjww=
+inet.af/netaddr v0.0.0-20200718043157-99321d6ad24c h1:si3Owrfem175Ry6gKqnh59eOXxDojyBTIHxUKuvK/Eo=
+inet.af/netaddr v0.0.0-20200718043157-99321d6ad24c/go.mod h1:qqYzz/2whtrbWJvt+DNWQyvekNN4ePQZcg2xc2/Yjww=
 rsc.io/goversion v1.2.0 h1:SPn+NLTiAG7w30IRK/DKp1BjvpWabYgxlLp/+kx5J8w=
 rsc.io/goversion v1.2.0/go.mod h1:Eih9y/uIBS3ulggl7KNJ09xGSLcuNaLgmvvqa07sgfo=

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.
@@ -353,13 +365,14 @@ func (b *LocalBackend) Start(opts Options) error {
 	b.serverURL = b.prefs.ControlURL
 	hostinfo.RoutableIPs = append(hostinfo.RoutableIPs, b.prefs.AdvertiseRoutes...)
 	hostinfo.RequestTags = append(hostinfo.RequestTags, b.prefs.AdvertiseTags...)
+	applyPrefsToHostinfo(hostinfo, b.prefs)
 
 	b.notify = opts.Notify
 	b.netMap = nil
 	persist := b.prefs.Persist
 	b.mu.Unlock()
 
-	b.updateFilter(nil)
+	b.updateFilter(nil, nil)
 
 	var discoPublic tailcfg.DiscoKey
 	if controlclient.Debug.Disco {
@@ -423,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
@@ -444,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
@@ -580,7 +640,7 @@ func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath st
 	if err != nil {
 		if errors.Is(err, ErrStateNotExist) {
 			if legacyPath != "" {
-				b.prefs, err = LoadPrefs(legacyPath, true)
+				b.prefs, err = LoadPrefs(legacyPath)
 				if err != nil {
 					b.logf("Failed to load legacy prefs: %v", err)
 					b.prefs = NewPrefs()
@@ -720,39 +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...)
-	if h := new.Hostname; h != "" {
-		newHi.Hostname = h
-	}
+	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 {
@@ -829,20 +895,20 @@ func (b *LocalBackend) authReconfig() {
 		return
 	}
 
-	uflags := controlclient.UDefault
+	var flags controlclient.WGConfigFlags
 	if uc.RouteAll {
-		uflags |= controlclient.UAllowDefaultRoute
+		flags |= controlclient.AllowDefaultRoute
 		// TODO(apenwarr): Make subnet routes a different pref?
-		uflags |= controlclient.UAllowSubnetRoutes
+		flags |= controlclient.AllowSubnetRoutes
 		// TODO(apenwarr): Remove this once we sort out subnet routes.
 		//  Right now default routes are broken in Windows, but
 		//  controlclient doesn't properly send subnet routes. So
 		//  let's convert a default route into a subnet route in order
 		//  to allow experimentation.
-		uflags |= controlclient.UHackDefaultRoute
+		flags |= controlclient.HackDefaultRoute
 	}
 	if uc.AllowSingleHosts {
-		uflags |= controlclient.UAllowSingleHosts
+		flags |= controlclient.AllowSingleHosts
 	}
 
 	dns := nm.DNS
@@ -851,7 +917,7 @@ func (b *LocalBackend) authReconfig() {
 		dns = []wgcfg.IP{}
 		dom = []string{}
 	}
-	cfg, err := nm.WGCfg(b.logf, uflags, dns)
+	cfg, err := nm.WGCfg(b.logf, flags, dns)
 	if err != nil {
 		b.logf("wgcfg: %v", err)
 		return
@@ -861,7 +927,7 @@ func (b *LocalBackend) authReconfig() {
 	if err == wgengine.ErrNoChanges {
 		return
 	}
-	b.logf("authReconfig: ra=%v dns=%v 0x%02x: %v", uc.RouteAll, uc.CorpDNS, uflags, err)
+	b.logf("authReconfig: ra=%v dns=%v 0x%02x: %v", uc.RouteAll, uc.CorpDNS, flags, err)
 }
 
 // routerConfig produces a router.Config from a wireguard config,
@@ -940,6 +1006,18 @@ func wgCIDRToNetaddr(cidrs []wgcfg.CIDR) (ret []netaddr.IPPrefix) {
 	return ret
 }
 
+func applyPrefsToHostinfo(hi *tailcfg.Hostinfo, prefs *Prefs) {
+	if h := prefs.Hostname; h != "" {
+		hi.Hostname = h
+	}
+	if v := prefs.OSVersion; v != "" {
+		hi.OSVersion = v
+	}
+	if m := prefs.DeviceModel; m != "" {
+		hi.DeviceModel = m
+	}
+}
+
 // enterState transitions the backend into newState, updating internal
 // state and propagating events out as needed.
 //

ipn/prefs.go

@@ -54,6 +54,10 @@ type Prefs struct {
 	// Hostname is the hostname to use for identifying the node. If
 	// not set, os.Hostname is used.
 	Hostname string
+	// OSVersion overrides tailcfg.Hostinfo's OSVersion.
+	OSVersion string
+	// DeviceModel overrides tailcfg.Hostinfo's DeviceModel.
+	DeviceModel string
 
 	// NotepadURLs is a debugging setting that opens OAuth URLs in
 	// notepad.exe on Windows, rather than loading them in a browser.
@@ -138,6 +142,8 @@ func (p *Prefs) Equals(p2 *Prefs) bool {
 		p.NoSNAT == p2.NoSNAT &&
 		p.NetfilterMode == p2.NetfilterMode &&
 		p.Hostname == p2.Hostname &&
+		p.OSVersion == p2.OSVersion &&
+		p.DeviceModel == p2.DeviceModel &&
 		compareIPNets(p.AdvertiseRoutes, p2.AdvertiseRoutes) &&
 		compareStrings(p.AdvertiseTags, p2.AdvertiseTags) &&
 		p.Persist.Equals(p2.Persist)
@@ -217,10 +223,9 @@ func (p *Prefs) Clone() *Prefs {
 	return p2
 }
 
-// LoadLegacyPrefs loads a legacy relaynode config file into Prefs
-// with sensible migration defaults set. If enforceDefaults is true,
-// Prefs.RouteAll and Prefs.AllowSingleHosts are forced on.
-func LoadPrefs(filename string, enforceDefaults bool) (*Prefs, error) {
+// LoadPrefs loads a legacy relaynode config file into Prefs
+// with sensible migration defaults set.
+func LoadPrefs(filename string) (*Prefs, error) {
 	data, err := ioutil.ReadFile(filename)
 	if err != nil {
 		return nil, fmt.Errorf("loading prefs from %q: %v", filename, err)

ipn/prefs_test.go

@@ -24,7 +24,7 @@ func fieldsOf(t reflect.Type) (fields []string) {
 func TestPrefsEqual(t *testing.T) {
 	tstest.PanicOnLog()
 
-	prefsHandles := []string{"ControlURL", "RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "ShieldsUp", "AdvertiseTags", "Hostname", "NotepadURLs", "DisableDERP", "AdvertiseRoutes", "NoSNAT", "NetfilterMode", "Persist"}
+	prefsHandles := []string{"ControlURL", "RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "ShieldsUp", "AdvertiseTags", "Hostname", "OSVersion", "DeviceModel", "NotepadURLs", "DisableDERP", "AdvertiseRoutes", "NoSNAT", "NetfilterMode", "Persist"}
 	if have := fieldsOf(reflect.TypeOf(Prefs{})); !reflect.DeepEqual(have, prefsHandles) {
 		t.Errorf("Prefs.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
 			have, prefsHandles)

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

metrics/metrics.go

@@ -40,3 +40,10 @@ func (m *LabelMap) Get(key string) *expvar.Int {
 	m.Add(key, 0)
 	return m.Map.Get(key).(*expvar.Int)
 }
+
+// GetFloat returns a direct pointer to the expvar.Float for key, creating it
+// if necessary.
+func (m *LabelMap) GetFloat(key string) *expvar.Float {
+	m.AddFloat(key, 0.0)
+	return m.Map.Get(key).(*expvar.Float)
+}

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
@@ -1157,7 +1194,7 @@ func (c *Client) nodeAddr(ctx context.Context, n *tailcfg.DERPNode, proto probeP
 		if proto == probeIPv6 && ip.Is4() {
 			return nil
 		}
-		return netaddr.IPPort{ip, uint16(port)}.UDPAddr()
+		return netaddr.IPPort{IP: ip, Port: uint16(port)}.UDPAddr()
 	}
 
 	switch proto {
@@ -1167,7 +1204,7 @@ func (c *Client) nodeAddr(ctx context.Context, n *tailcfg.DERPNode, proto probeP
 			if !ip.Is4() {
 				return nil
 			}
-			return netaddr.IPPort{ip, uint16(port)}.UDPAddr()
+			return netaddr.IPPort{IP: ip, Port: uint16(port)}.UDPAddr()
 		}
 	case probeIPv6:
 		if n.IPv6 != "" {
@@ -1175,7 +1212,7 @@ func (c *Client) nodeAddr(ctx context.Context, n *tailcfg.DERPNode, proto probeP
 			if !ip.Is6() {
 				return nil
 			}
-			return netaddr.IPPort{ip, uint16(port)}.UDPAddr()
+			return netaddr.IPPort{IP: ip, Port: uint16(port)}.UDPAddr()
 		}
 	default:
 		return nil

net/netns/netns_linux.go

@@ -26,7 +26,7 @@ import (
 //
 // Keep this in sync with tailscaleBypassMark in
 // wgengine/router/router_linux.go.
-const tailscaleBypassMark = 0x20000
+const tailscaleBypassMark = 0x80000
 
 // ipRuleOnce is the sync.Once & cached value for ipRuleAvailable.
 var ipRuleOnce struct {

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.

safesocket/unixsocket.go

@@ -11,6 +11,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"net"
 	"os"
@@ -77,6 +78,16 @@ func listen(path string, port uint16) (ln net.Listener, _ uint16, err error) {
 //   * it also picks a random hex string that acts as an auth token
 //   * it then creates a file named "sameuserproof-$PORT-$TOKEN" and leaves
 //     that file descriptor open forever.
+//
+// Then, we do different things depending on whether the user is
+// running cmd/tailscale that they built themselves (running as
+// themselves, outside the App Sandbox), or whether the user is
+// running the CLI via the GUI binary
+// (e.g. /Applications/Tailscale.app/Contents/MacOS/Tailscale <args>),
+// in which case we're running within the App Sandbox.
+//
+// If we're outside the App Sandbox:
+//
 //   * then we come along here, running as the same UID, but outside
 //     of the sandbox, and look for it. We can run lsof on our own processes,
 //     but other users on the system can't.
@@ -86,7 +97,38 @@ func listen(path string, port uint16) (ln net.Listener, _ uint16, err error) {
 //   * server verifies $TOKEN, sends "#IPN\n" if okay.
 //   * server is now protocol switched
 //   * we return the net.Conn and the caller speaks the normal protocol
+//
+// If we're inside the App Sandbox, then TS_MACOS_CLI_SHARED_DIR has
+// been set to our shared directory. We now have to find the most
+// recent "sameuserproof" file (there should only be 1, but previous
+// versions of the macOS app didn't clean them up).
 func connectMacOSAppSandbox() (net.Conn, error) {
+	// Are we running the Tailscale.app GUI binary as a CLI, running within the App Sandbox?
+	if d := os.Getenv("TS_MACOS_CLI_SHARED_DIR"); d != "" {
+		fis, err := ioutil.ReadDir(d)
+		if err != nil {
+			return nil, fmt.Errorf("reading TS_MACOS_CLI_SHARED_DIR: %w", err)
+		}
+		var best os.FileInfo
+		for _, fi := range fis {
+			if !strings.HasPrefix(fi.Name(), "sameuserproof-") || strings.Count(fi.Name(), "-") != 2 {
+				continue
+			}
+			if best == nil || fi.ModTime().After(best.ModTime()) {
+				best = fi
+			}
+		}
+		if best == nil {
+			return nil, fmt.Errorf("no sameuserproof token found in TS_MACOS_CLI_SHARED_DIR %q", d)
+		}
+		f := strings.SplitN(best.Name(), "-", 3)
+		portStr, token := f[1], f[2]
+		return connectMacTCP(portStr, token)
+	}
+
+	// Otherwise, assume we're running the cmd/tailscale binary from outside the
+	// App Sandbox.
+
 	out, err := exec.Command("lsof",
 		"-n",                             // numeric sockets; don't do DNS lookups, etc
 		"-a",                             // logical AND remaining options
@@ -110,22 +152,26 @@ func connectMacOSAppSandbox() (net.Conn, error) {
 			continue
 		}
 		portStr, token := f[0], f[1]
-		c, err := net.Dial("tcp", "localhost:"+portStr)
-		if err != nil {
-			return nil, fmt.Errorf("error dialing IPNExtension: %w", err)
-		}
-		if _, err := io.WriteString(c, token+"\n"); err != nil {
-			return nil, fmt.Errorf("error writing auth token: %w", err)
-		}
-		buf := make([]byte, 5)
-		const authOK = "#IPN\n"
-		if _, err := io.ReadFull(c, buf); err != nil {
-			return nil, fmt.Errorf("error reading from IPNExtension post-auth: %w", err)
-		}
-		if string(buf) != authOK {
-			return nil, fmt.Errorf("invalid response reading from IPNExtension post-auth")
-		}
-		return c, nil
+		return connectMacTCP(portStr, token)
 	}
 	return nil, fmt.Errorf("failed to find Tailscale's IPNExtension process")
 }
+
+func connectMacTCP(portStr, token string) (net.Conn, error) {
+	c, err := net.Dial("tcp", "localhost:"+portStr)
+	if err != nil {
+		return nil, fmt.Errorf("error dialing IPNExtension: %w", err)
+	}
+	if _, err := io.WriteString(c, token+"\n"); err != nil {
+		return nil, fmt.Errorf("error writing auth token: %w", err)
+	}
+	buf := make([]byte, 5)
+	const authOK = "#IPN\n"
+	if _, err := io.ReadFull(c, buf); err != nil {
+		return nil, fmt.Errorf("error reading from IPNExtension post-auth: %w", err)
+	}
+	if string(buf) != authOK {
+		return nil, fmt.Errorf("invalid response reading from IPNExtension post-auth")
+	}
+	return c, nil
+}

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"
@@ -93,18 +95,6 @@ type User struct {
 	// Note: be sure to update Clone when adding new fields
 }
 
-// Clone returns a copy of u that aliases no memory with the original.
-func (u *User) Clone() *User {
-	if u == nil {
-		return nil
-	}
-	u2 := new(User)
-	*u2 = *u
-	u2.Logins = append([]LoginID(nil), u.Logins...)
-	u2.Roles = append([]RoleID(nil), u.Roles...)
-	return u2
-}
-
 type Login struct {
 	_             structs.Incomparable
 	ID            LoginID
@@ -150,23 +140,6 @@ type Node struct {
 	//       require changes to Node.Clone.
 }
 
-// Clone makes a deep copy of Node.
-// The result aliases no memory with the original.
-func (n *Node) Clone() (res *Node) {
-	res = new(Node)
-	*res = *n
-
-	res.Addresses = append([]wgcfg.CIDR{}, res.Addresses...)
-	res.AllowedIPs = append([]wgcfg.CIDR{}, res.AllowedIPs...)
-	res.Endpoints = append([]string{}, res.Endpoints...)
-	if res.LastSeen != nil {
-		lastSeen := *res.LastSeen
-		res.LastSeen = &lastSeen
-	}
-	res.Hostinfo = *res.Hostinfo.Clone()
-	return res
-}
-
 type MachineStatus int
 
 const (
@@ -284,7 +257,10 @@ type Hostinfo struct {
 	FrontendLogID string       // logtail ID of frontend instance
 	BackendLogID  string       // logtail ID of backend instance
 	OS            string       // operating system the client runs on (a version.OS value)
+	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
@@ -398,35 +374,14 @@ func (ni *NetInfo) BasicallyEqual(ni2 *NetInfo) bool {
 		ni.LinkType == ni2.LinkType
 }
 
-func (ni *NetInfo) Clone() (res *NetInfo) {
-	if ni == nil {
-		return nil
-	}
-	res = new(NetInfo)
-	*res = *ni
-	if ni.DERPLatency != nil {
-		res.DERPLatency = map[string]float64{}
-		for k, v := range ni.DERPLatency {
-			res.DERPLatency[k] = v
-		}
-	}
-	return res
-}
-
-// Clone makes a deep copy of Hostinfo.
-// The result aliases no memory with the original.
-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 {
+		return true
+	}
+	if (h == nil) != (h2 == nil) {
+		return false
+	}
 	return reflect.DeepEqual(h, h2)
 }
 
@@ -453,6 +408,8 @@ type RegisterRequest struct {
 
 // Clone makes a deep copy of RegisterRequest.
 // The result aliases no memory with the original.
+//
+// TODO: extend cmd/cloner to generate this method.
 func (req *RegisterRequest) Clone() *RegisterRequest {
 	res := new(RegisterRequest)
 	*res = *req
@@ -636,11 +593,39 @@ func (n *Node) Equal(n2 *Node) bool {
 		n.KeyExpiry.Equal(n2.KeyExpiry) &&
 		n.Machine == n2.Machine &&
 		n.DiscoKey == n2.DiscoKey &&
-		reflect.DeepEqual(n.Addresses, n2.Addresses) &&
-		reflect.DeepEqual(n.AllowedIPs, n2.AllowedIPs) &&
-		reflect.DeepEqual(n.Endpoints, n2.Endpoints) &&
-		reflect.DeepEqual(n.Hostinfo, n2.Hostinfo) &&
+		eqCIDRs(n.Addresses, n2.Addresses) &&
+		eqCIDRs(n.AllowedIPs, n2.AllowedIPs) &&
+		eqStrings(n.Endpoints, n2.Endpoints) &&
+		n.Hostinfo.Equal(&n2.Hostinfo) &&
 		n.Created.Equal(n2.Created) &&
-		reflect.DeepEqual(n.LastSeen, n2.LastSeen) &&
+		eqTimePtr(n.LastSeen, n2.LastSeen) &&
 		n.MachineAuthorized == n2.MachineAuthorized
 }
+
+func eqStrings(a, b []string) bool {
+	if len(a) != len(b) || ((a == nil) != (b == nil)) {
+		return false
+	}
+	for i, v := range a {
+		if v != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+func eqCIDRs(a, b []wgcfg.CIDR) bool {
+	if len(a) != len(b) || ((a == nil) != (b == nil)) {
+		return false
+	}
+	for i, v := range a {
+		if v != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+func eqTimePtr(a, b *time.Time) bool {
+	return ((a == nil) == (b == nil)) && (a == nil || a.Equal(*b))
+}

tailcfg/tailcfg_clone.go

@@ -0,0 +1,75 @@
+// 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 User,Node,Hostinfo,NetInfo; DO NOT EDIT.
+
+package tailcfg
+
+import (
+	"time"
+)
+
+// Clone makes a deep copy of User.
+// The result aliases no memory with the original.
+func (src *User) Clone() *User {
+	if src == nil {
+		return nil
+	}
+	dst := new(User)
+	*dst = *src
+	dst.Logins = append(src.Logins[:0:0], src.Logins...)
+	dst.Roles = append(src.Roles[:0:0], src.Roles...)
+	return dst
+}
+
+// Clone makes a deep copy of Node.
+// The result aliases no memory with the original.
+func (src *Node) Clone() *Node {
+	if src == nil {
+		return nil
+	}
+	dst := new(Node)
+	*dst = *src
+	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)
+		*dst.LastSeen = *src.LastSeen
+	}
+	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 {
+	if src == nil {
+		return nil
+	}
+	dst := new(NetInfo)
+	*dst = *src
+	if dst.DERPLatency != nil {
+		dst.DERPLatency = map[string]float64{}
+		for k, v := range src.DERPLatency {
+			dst.DERPLatency[k] = v
+		}
+	}
+	return dst
+}

tailcfg/tailcfg_test.go

@@ -23,7 +23,8 @@ func fieldsOf(t reflect.Type) (fields []string) {
 
 func TestHostinfoEqual(t *testing.T) {
 	hiHandles := []string{
-		"IPNVersion", "FrontendLogID", "BackendLogID", "OS", "Hostname", "RoutableIPs", "RequestTags", "Services",
+		"IPNVersion", "FrontendLogID", "BackendLogID", "OS", "OSVersion",
+		"DeviceModel", "Hostname", "GoArch", "RoutableIPs", "RequestTags", "Services",
 		"NetInfo",
 	}
 	if have := fieldsOf(reflect.TypeOf(Hostinfo{})); !reflect.DeepEqual(have, hiHandles) {
@@ -389,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")
+			}
+		})
+	}
+}

tempfork/pprof/README.md

@@ -0,0 +1,4 @@
+This is a fork of net/http/pprof that doesn't use init side effects
+and doesn't use html/template (which ends up calling
+reflect.Value.MethodByName, which disables some linker deadcode
+optimizations).

tempfork/pprof/pprof.go

@@ -0,0 +1,301 @@
+// Copyright 2010 The Go 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 pprof serves via its HTTP server runtime profiling data
+// in the format expected by the pprof visualization tool.
+//
+// See Go's net/http/pprof for docs.
+//
+// This is a fork of net/http/pprof that doesn't use init side effects
+// and doesn't use html/template (which ends up calling
+// reflect.Value.MethodByName, which disables some linker deadcode
+// optimizations).
+package pprof
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"html"
+	"io"
+	"net/http"
+	"os"
+	"runtime"
+	"runtime/pprof"
+	"runtime/trace"
+	"sort"
+	"strconv"
+	"strings"
+	"time"
+)
+
+func AddHandlers(mux *http.ServeMux) {
+	mux.HandleFunc("/debug/pprof/", Index)
+	mux.HandleFunc("/debug/pprof/cmdline", Cmdline)
+	mux.HandleFunc("/debug/pprof/profile", Profile)
+	mux.HandleFunc("/debug/pprof/symbol", Symbol)
+	mux.HandleFunc("/debug/pprof/trace", Trace)
+}
+
+// Cmdline responds with the running program's
+// command line, with arguments separated by NUL bytes.
+// The package initialization registers it as /debug/pprof/cmdline.
+func Cmdline(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	fmt.Fprintf(w, strings.Join(os.Args, "\x00"))
+}
+
+func sleep(w http.ResponseWriter, d time.Duration) {
+	var clientGone <-chan bool
+	if cn, ok := w.(http.CloseNotifier); ok {
+		clientGone = cn.CloseNotify()
+	}
+	select {
+	case <-time.After(d):
+	case <-clientGone:
+	}
+}
+
+func durationExceedsWriteTimeout(r *http.Request, seconds float64) bool {
+	srv, ok := r.Context().Value(http.ServerContextKey).(*http.Server)
+	return ok && srv.WriteTimeout != 0 && seconds >= srv.WriteTimeout.Seconds()
+}
+
+func serveError(w http.ResponseWriter, status int, txt string) {
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	w.Header().Set("X-Go-Pprof", "1")
+	w.Header().Del("Content-Disposition")
+	w.WriteHeader(status)
+	fmt.Fprintln(w, txt)
+}
+
+// Profile responds with the pprof-formatted cpu profile.
+// Profiling lasts for duration specified in seconds GET parameter, or for 30 seconds if not specified.
+// The package initialization registers it as /debug/pprof/profile.
+func Profile(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	sec, err := strconv.ParseInt(r.FormValue("seconds"), 10, 64)
+	if sec <= 0 || err != nil {
+		sec = 30
+	}
+
+	if durationExceedsWriteTimeout(r, float64(sec)) {
+		serveError(w, http.StatusBadRequest, "profile duration exceeds server's WriteTimeout")
+		return
+	}
+
+	// Set Content Type assuming StartCPUProfile will work,
+	// because if it does it starts writing.
+	w.Header().Set("Content-Type", "application/octet-stream")
+	w.Header().Set("Content-Disposition", `attachment; filename="profile"`)
+	if err := pprof.StartCPUProfile(w); err != nil {
+		// StartCPUProfile failed, so no writes yet.
+		serveError(w, http.StatusInternalServerError,
+			fmt.Sprintf("Could not enable CPU profiling: %s", err))
+		return
+	}
+	sleep(w, time.Duration(sec)*time.Second)
+	pprof.StopCPUProfile()
+}
+
+// Trace responds with the execution trace in binary form.
+// Tracing lasts for duration specified in seconds GET parameter, or for 1 second if not specified.
+// The package initialization registers it as /debug/pprof/trace.
+func Trace(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	sec, err := strconv.ParseFloat(r.FormValue("seconds"), 64)
+	if sec <= 0 || err != nil {
+		sec = 1
+	}
+
+	if durationExceedsWriteTimeout(r, sec) {
+		serveError(w, http.StatusBadRequest, "profile duration exceeds server's WriteTimeout")
+		return
+	}
+
+	// Set Content Type assuming trace.Start will work,
+	// because if it does it starts writing.
+	w.Header().Set("Content-Type", "application/octet-stream")
+	w.Header().Set("Content-Disposition", `attachment; filename="trace"`)
+	if err := trace.Start(w); err != nil {
+		// trace.Start failed, so no writes yet.
+		serveError(w, http.StatusInternalServerError,
+			fmt.Sprintf("Could not enable tracing: %s", err))
+		return
+	}
+	sleep(w, time.Duration(sec*float64(time.Second)))
+	trace.Stop()
+}
+
+// Symbol looks up the program counters listed in the request,
+// responding with a table mapping program counters to function names.
+// The package initialization registers it as /debug/pprof/symbol.
+func Symbol(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+
+	// We have to read the whole POST body before
+	// writing any output. Buffer the output here.
+	var buf bytes.Buffer
+
+	// We don't know how many symbols we have, but we
+	// do have symbol information. Pprof only cares whether
+	// this number is 0 (no symbols available) or > 0.
+	fmt.Fprintf(&buf, "num_symbols: 1\n")
+
+	var b *bufio.Reader
+	if r.Method == "POST" {
+		b = bufio.NewReader(r.Body)
+	} else {
+		b = bufio.NewReader(strings.NewReader(r.URL.RawQuery))
+	}
+
+	for {
+		word, err := b.ReadSlice('+')
+		if err == nil {
+			word = word[0 : len(word)-1] // trim +
+		}
+		pc, _ := strconv.ParseUint(string(word), 0, 64)
+		if pc != 0 {
+			f := runtime.FuncForPC(uintptr(pc))
+			if f != nil {
+				fmt.Fprintf(&buf, "%#x %s\n", pc, f.Name())
+			}
+		}
+
+		// Wait until here to check for err; the last
+		// symbol will have an err because it doesn't end in +.
+		if err != nil {
+			if err != io.EOF {
+				fmt.Fprintf(&buf, "reading request: %v\n", err)
+			}
+			break
+		}
+	}
+
+	w.Write(buf.Bytes())
+}
+
+// Handler returns an HTTP handler that serves the named profile.
+func Handler(name string) http.Handler {
+	return handler(name)
+}
+
+type handler string
+
+func (name handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	p := pprof.Lookup(string(name))
+	if p == nil {
+		serveError(w, http.StatusNotFound, "Unknown profile")
+		return
+	}
+	gc, _ := strconv.Atoi(r.FormValue("gc"))
+	if name == "heap" && gc > 0 {
+		runtime.GC()
+	}
+	debug, _ := strconv.Atoi(r.FormValue("debug"))
+	if debug != 0 {
+		w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	} else {
+		w.Header().Set("Content-Type", "application/octet-stream")
+		w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, name))
+	}
+	p.WriteTo(w, debug)
+}
+
+var profileDescriptions = map[string]string{
+	"allocs":       "A sampling of all past memory allocations",
+	"block":        "Stack traces that led to blocking on synchronization primitives",
+	"cmdline":      "The command line invocation of the current program",
+	"goroutine":    "Stack traces of all current goroutines",
+	"heap":         "A sampling of memory allocations of live objects. You can specify the gc GET parameter to run GC before taking the heap sample.",
+	"mutex":        "Stack traces of holders of contended mutexes",
+	"profile":      "CPU profile. You can specify the duration in the seconds GET parameter. After you get the profile file, use the go tool pprof command to investigate the profile.",
+	"threadcreate": "Stack traces that led to the creation of new OS threads",
+	"trace":        "A trace of execution of the current program. You can specify the duration in the seconds GET parameter. After you get the trace file, use the go tool trace command to investigate the trace.",
+}
+
+// Index responds with the pprof-formatted profile named by the request.
+// For example, "/debug/pprof/heap" serves the "heap" profile.
+// Index responds to a request for "/debug/pprof/" with an HTML page
+// listing the available profiles.
+func Index(w http.ResponseWriter, r *http.Request) {
+	if strings.HasPrefix(r.URL.Path, "/debug/pprof/") {
+		name := strings.TrimPrefix(r.URL.Path, "/debug/pprof/")
+		if name != "" {
+			handler(name).ServeHTTP(w, r)
+			return
+		}
+	}
+
+	type profile struct {
+		Name  string
+		Href  string
+		Desc  string
+		Count int
+	}
+	var profiles []profile
+	for _, p := range pprof.Profiles() {
+		profiles = append(profiles, profile{
+			Name:  p.Name(),
+			Href:  p.Name() + "?debug=1",
+			Desc:  profileDescriptions[p.Name()],
+			Count: p.Count(),
+		})
+	}
+
+	// Adding other profiles exposed from within this package
+	for _, p := range []string{"cmdline", "profile", "trace"} {
+		profiles = append(profiles, profile{
+			Name: p,
+			Href: p,
+			Desc: profileDescriptions[p],
+		})
+	}
+
+	sort.Slice(profiles, func(i, j int) bool {
+		return profiles[i].Name < profiles[j].Name
+	})
+
+	io.WriteString(w, `<html>
+<head>
+<title>/debug/pprof/</title>
+<style>
+.profile-name{
+	display:inline-block;
+	width:6rem;
+}
+</style>
+</head>
+<body>
+/debug/pprof/<br>
+<br>
+Types of profiles available:
+<table>
+<thead><td>Count</td><td>Profile</td></thead>
+`)
+	for _, p := range profiles {
+		fmt.Fprintf(w, "<tr><td>%d</td><td><a href=%v>%v</td></tr>\n",
+			p.Count, html.EscapeString(p.Href), html.EscapeString(p.Name))
+	}
+	io.WriteString(w, `</table>
+<a href="goroutine?debug=2">full goroutine stack dump</a>
+<br/>
+<p>
+Profile Descriptions:
+<ul>
+`)
+	for _, p := range profiles {
+		fmt.Fprintf(w, "<li><div class=profile-name>%s:</div> %s</li>\n",
+			html.EscapeString(p.Name), html.EscapeString(p.Desc))
+	}
+	io.WriteString(w, `
+</ul>
+</p>
+</body>
+</html>
+`)
+}

tempfork/pprof/pprof_test.go

@@ -0,0 +1,81 @@
+// Copyright 2018 The Go 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 pprof
+
+import (
+	"bytes"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"runtime/pprof"
+	"testing"
+)
+
+// TestDescriptions checks that the profile names under runtime/pprof package
+// have a key in the description map.
+func TestDescriptions(t *testing.T) {
+	for _, p := range pprof.Profiles() {
+		_, ok := profileDescriptions[p.Name()]
+		if ok != true {
+			t.Errorf("%s does not exist in profileDescriptions map\n", p.Name())
+		}
+	}
+}
+
+func TestHandlers(t *testing.T) {
+	testCases := []struct {
+		path               string
+		handler            http.HandlerFunc
+		statusCode         int
+		contentType        string
+		contentDisposition string
+		resp               []byte
+	}{
+		{"/debug/pprof/<script>scripty<script>", Index, http.StatusNotFound, "text/plain; charset=utf-8", "", []byte("Unknown profile\n")},
+		{"/debug/pprof/heap", Index, http.StatusOK, "application/octet-stream", `attachment; filename="heap"`, nil},
+		{"/debug/pprof/heap?debug=1", Index, http.StatusOK, "text/plain; charset=utf-8", "", nil},
+		{"/debug/pprof/cmdline", Cmdline, http.StatusOK, "text/plain; charset=utf-8", "", nil},
+		{"/debug/pprof/profile?seconds=1", Profile, http.StatusOK, "application/octet-stream", `attachment; filename="profile"`, nil},
+		{"/debug/pprof/symbol", Symbol, http.StatusOK, "text/plain; charset=utf-8", "", nil},
+		{"/debug/pprof/trace", Trace, http.StatusOK, "application/octet-stream", `attachment; filename="trace"`, nil},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			req := httptest.NewRequest("GET", "http://example.com"+tc.path, nil)
+			w := httptest.NewRecorder()
+			tc.handler(w, req)
+
+			resp := w.Result()
+			if got, want := resp.StatusCode, tc.statusCode; got != want {
+				t.Errorf("status code: got %d; want %d", got, want)
+			}
+
+			body, err := ioutil.ReadAll(resp.Body)
+			if err != nil {
+				t.Errorf("when reading response body, expected non-nil err; got %v", err)
+			}
+			if got, want := resp.Header.Get("X-Content-Type-Options"), "nosniff"; got != want {
+				t.Errorf("X-Content-Type-Options: got %q; want %q", got, want)
+			}
+			if got, want := resp.Header.Get("Content-Type"), tc.contentType; got != want {
+				t.Errorf("Content-Type: got %q; want %q", got, want)
+			}
+			if got, want := resp.Header.Get("Content-Disposition"), tc.contentDisposition; got != want {
+				t.Errorf("Content-Disposition: got %q; want %q", got, want)
+			}
+
+			if resp.StatusCode == http.StatusOK {
+				return
+			}
+			if got, want := resp.Header.Get("X-Go-Pprof"), "1"; got != want {
+				t.Errorf("X-Go-Pprof: got %q; want %q", got, want)
+			}
+			if !bytes.Equal(body, tc.resp) {
+				t.Errorf("response: got %q; want %q", body, tc.resp)
+			}
+		})
+	}
+
+}

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
 			}
 		}

version/version.go

@@ -7,5 +7,5 @@
 // Package version provides the version that the binary was built at.
 package version
 
-const LONG = "date.20200713"
+const LONG = "date.20200727"
 const SHORT = LONG

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,17 +55,55 @@ 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 {
-	pconnPort    uint16 // the preferred port from opts.Port; 0 means auto
-	pconn4       *RebindingUDPConn
-	pconn6       *RebindingUDPConn // non-nil if IPv6 available
-	epFunc       func(endpoints []string)
-	logf         logger.Logf
-	sendLogLimit *rate.Limiter
-	netChecker   *netcheck.Client
-	idleFunc     func() time.Duration // nil means unknown
+	pconnPort        uint16 // the preferred port from opts.Port; 0 means auto
+	pconn4           *RebindingUDPConn
+	pconn6           *RebindingUDPConn // non-nil if IPv6 available
+	epFunc           func(endpoints []string)
+	logf             logger.Logf
+	sendLogLimit     *rate.Limiter
+	netChecker       *netcheck.Client
+	idleFunc         func() time.Duration   // nil means unknown
+	noteRecvActivity func(tailcfg.DiscoKey) // or nil, see Options.NoteRecvActivity
 
 	// bufferedIPv4From and bufferedIPv4Packet are owned by
 	// ReceiveIPv4, and used when both a DERP and IPv4 packet arrive
@@ -89,6 +127,13 @@ type Conn struct {
 	// ============================================================
 	mu sync.Mutex // guards all following fields
 
+	// canCreateEPUnlocked tracks at one place whether mu is
+	// already held. It's then checked in CreateEndpoint to avoid
+	// double-locking mu and thus deadlocking. mu should be held
+	// while setting this; but can be read without mu held.
+	// TODO(bradfitz): delete this shameful hack; refactor the one use
+	canCreateEPUnlocked syncs.AtomicBool
+
 	started bool // Start was called
 	closed  bool // Close was called
 
@@ -104,8 +149,8 @@ type Conn struct {
 	nodeOfDisco     map[tailcfg.DiscoKey]*tailcfg.Node
 	discoOfNode     map[tailcfg.NodeKey]tailcfg.DiscoKey
 	discoOfAddr     map[netaddr.IPPort]tailcfg.DiscoKey // validated non-DERP paths only
-	endpointOfDisco map[tailcfg.DiscoKey]*discoEndpoint
-	sharedDiscoKey  map[tailcfg.DiscoKey]*[32]byte // nacl/box precomputed key
+	endpointOfDisco map[tailcfg.DiscoKey]*discoEndpoint // those with activity only
+	sharedDiscoKey  map[tailcfg.DiscoKey]*[32]byte      // nacl/box precomputed key
 
 	// addrsByUDP is a map of every remote ip:port to a priority
 	// list of endpoint addresses for a peer.
@@ -130,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
 
@@ -235,6 +281,17 @@ type Options struct {
 	// PacketListener optionally specifies how to create PacketConns.
 	// It's meant for testing.
 	PacketListener nettype.PacketListener
+
+	// NoteRecvActivity, if provided, is a func for magicsock to
+	// call whenever it receives a packet from a a
+	// discovery-capable peer if it's been more than ~10 seconds
+	// since the last one. (10 seconds is somewhat arbitrary; the
+	// sole user just doesn't need or want it called on every
+	// packet, just every minute or two for Wireguard timeouts,
+	// and 10 seconds seems like a good trade-off between often
+	// enough and not too often.) The provided func likely calls
+	// Conn.CreateEndpoint, which acquires Conn.mu.
+	NoteRecvActivity func(tailcfg.DiscoKey)
 }
 
 func (o *Options) logf() logger.Logf {
@@ -282,6 +339,7 @@ func NewConn(opts Options) (*Conn, error) {
 	c.epFunc = opts.endpointsFunc()
 	c.idleFunc = opts.IdleFunc
 	c.packetListener = opts.PacketListener
+	c.noteRecvActivity = opts.NoteRecvActivity
 
 	if err := c.initialBind(); err != nil {
 		return nil, err
@@ -582,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.
 //
@@ -685,10 +741,6 @@ func shouldSprayPacket(b []byte) bool {
 	return false
 }
 
-var logPacketDests, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_LOG_PACKET_DESTS"))
-
-var logDisco, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DISCO"))
-
 const sprayPeriod = 3 * time.Second
 
 // appendDests appends to dsts the destinations that b should be
@@ -913,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.
@@ -986,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 {
@@ -1088,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{}) {
@@ -1134,7 +1184,15 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, d
 			}
 			c.ReSTUN("derp-close")
 			c.logf("magicsock: [%p] derp.Recv(derp-%d): %v", dc, regionID, err)
-			time.Sleep(250 * time.Millisecond)
+
+			// Avoid excessive spinning.
+			// TODO: use a backoff timer, perhaps between 10ms and 500ms?
+			// Don't want to sleep too long. For now 250ms seems fine.
+			select {
+			case <-ctx.Done():
+				return
+			case <-time.After(250 * time.Millisecond):
+			}
 			continue
 		}
 		switch m := msg.(type) {
@@ -1160,7 +1218,12 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, d
 		case <-ctx.Done():
 			return
 		case c.derpRecvCh <- res:
-			<-didCopy
+			select {
+			case <-ctx.Done():
+				return
+			case <-didCopy:
+				continue
+			}
 		}
 	}
 }
@@ -1199,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()
@@ -1287,6 +1353,16 @@ func wgRecvAddr(e conn.Endpoint, ipp netaddr.IPPort, addr *net.UDPAddr) *net.UDP
 	return ipp.UDPAddr()
 }
 
+// noteRecvActivity calls the magicsock.Conn.noteRecvActivity hook if
+// e is a discovery-capable peer.
+//
+// This should be called whenever a packet arrives from e.
+func noteRecvActivity(e conn.Endpoint) {
+	if de, ok := e.(*discoEndpoint); ok {
+		de.onRecvActivity()
+	}
+}
+
 func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, addr *net.UDPAddr, err error) {
 Top:
 	// First, process any buffered packet from earlier.
@@ -1294,6 +1370,7 @@ Top:
 		c.bufferedIPv4From = netaddr.IPPort{}
 		addr = from.UDPAddr()
 		ep := c.findEndpoint(from, addr)
+		noteRecvActivity(ep)
 		return copy(b, c.bufferedIPv4Packet), ep, wgRecvAddr(ep, from, addr), nil
 	}
 
@@ -1306,6 +1383,7 @@ Top:
 	var addrSet *AddrSet
 	var discoEp *discoEndpoint
 	var ipp netaddr.IPPort
+	var didNoteRecvActivity bool
 
 	select {
 	case dm := <-c.derpRecvCh:
@@ -1347,6 +1425,24 @@ Top:
 		c.mu.Lock()
 		if dk, ok := c.discoOfNode[tailcfg.NodeKey(dm.src)]; ok {
 			discoEp = c.endpointOfDisco[dk]
+			// If we know about the node (it's in discoOfNode) but don't know about the
+			// endpoint, that's because it's an idle peer that doesn't yet exist in the
+			// wireguard config. So run the receive hook, if defined, which should
+			// create the wireguard peer.
+			if discoEp == nil && c.noteRecvActivity != nil {
+				didNoteRecvActivity = true
+				c.mu.Unlock()          // release lock before calling noteRecvActivity
+				c.noteRecvActivity(dk) // (calls back into CreateEndpoint)
+				// Now require the lock. No invariants need to be rechecked; just
+				// 1-2 map lookups follow that are harmless if, say, the peer has
+				// been deleted during this time.  In that case we'll treate it as a
+				// legacy pre-disco UDP receive and hand it to wireguard which'll
+				// likely just drop it.
+				c.mu.Lock()
+
+				discoEp = c.endpointOfDisco[dk]
+				c.logf("magicsock: DERP packet received from idle peer %v; created=%v", dm.src.ShortString(), discoEp != nil)
+			}
 		}
 		if discoEp == nil {
 			addrSet = c.addrsByKey[dm.src]
@@ -1385,6 +1481,9 @@ Top:
 	} else {
 		ep = c.findEndpoint(ipp, addr)
 	}
+	if !didNoteRecvActivity {
+		noteRecvActivity(ep)
+	}
 	return n, ep, wgRecvAddr(ep, ipp, addr), nil
 }
 
@@ -1411,11 +1510,24 @@ func (c *Conn) ReceiveIPv6(b []byte) (int, conn.Endpoint, *net.UDPAddr, error) {
 		}
 
 		ep := c.findEndpoint(ipp, addr)
+		noteRecvActivity(ep)
 		return n, ep, wgRecvAddr(ep, ipp, addr), nil
 	}
 }
 
-func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey key.Public, dstDisco tailcfg.DiscoKey, m disco.Message) (sent bool, err error) {
+// discoLogLevel controls the verbosity of discovery log messages.
+type discoLogLevel int
+
+const (
+	// discoLog means that a message should be logged.
+	discoLog discoLogLevel = iota
+
+	// discoVerboseLog means that a message should only be logged
+	// in TS_DEBUG_DISCO mode.
+	discoVerboseLog
+)
+
+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 {
 		c.mu.Unlock()
@@ -1433,9 +1545,11 @@ func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey key.Public, dstDisco
 	c.mu.Unlock()
 
 	pkt = box.SealAfterPrecomputation(pkt, m.AppendMarshal(nil), &nonce, sharedKey)
-	sent, err = c.sendAddr(dst, dstKey, pkt)
+	sent, err = c.sendAddr(dst, key.Public(dstKey), pkt)
 	if sent {
-		c.logf("magicsock: disco: %v->%v (%v, %v) sent %v", c.discoShort, dstDisco.ShortString(), dstKey.ShortString(), derpStr(dst.String()), disco.MessageSummary(m))
+		if logLevel == discoLog || (logLevel == discoVerboseLog && debugDisco) {
+			c.logf("magicsock: disco: %v->%v (%v, %v) sent %v", c.discoShort, dstDisco.ShortString(), dstKey.ShortString(), derpStr(dst.String()), disco.MessageSummary(m))
+		}
 	} else if err == nil {
 		// Can't send. (e.g. no IPv6 locally)
 	} else {
@@ -1470,26 +1584,55 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 	if c.closed {
 		return true
 	}
-	if logDisco {
+	if debugDisco {
 		c.logf("magicsock: disco: got disco-looking frame from %v", sender.ShortString())
 	}
 	if c.discoPrivate.IsZero() {
-		if logDisco {
+		if debugDisco {
 			c.logf("magicsock: disco: ignoring disco-looking frame, no local key")
 		}
 		return false
 	}
 
-	de, ok := c.endpointOfDisco[sender]
+	peerNode, ok := c.nodeOfDisco[sender]
 	if !ok {
-		if logDisco {
-			c.logf("magicsock: disco: ignoring disco-looking frame, don't know about %v", sender.ShortString())
+		if debugDisco {
+			c.logf("magicsock: disco: ignoring disco-looking frame, don't know node for %v", sender.ShortString())
 		}
 		// Returning false keeps passing it down, to WireGuard.
 		// WireGuard will almost surely reject it, but give it a chance.
 		return false
 	}
 
+	de, ok := c.endpointOfDisco[sender]
+	if !ok {
+		// We don't have an active endpoint for this sender but we knew about the node, so
+		// it's an idle endpoint that doesn't yet exist in the wireguard config. We now have
+		// to notify the userspace engine (via noteRecvActivity) so wireguard-go can create
+		// an Endpoint (ultimately calling our CreateEndpoint).
+		if debugDisco {
+			c.logf("magicsock: disco: got message from inactive peer %v", sender.ShortString())
+		}
+		if c.noteRecvActivity == nil {
+			c.logf("magicsock: [unexpected] have node without endpoint, without c.noteRecvActivity hook")
+			return false
+		}
+		// noteRecvActivity calls back into CreateEndpoint, which we can't easily control,
+		// and CreateEndpoint expects to be called with c.mu held, but we hold it here, and
+		// it's too invasive for now to release it here and recheck invariants.  So instead,
+		// use this unfortunate hack: set canCreateEPUnlocked which CreateEndpoint then
+		// checks to conditionally acquire the mutex. I'm so sorry.
+		c.canCreateEPUnlocked.Set(true)
+		c.noteRecvActivity(sender)
+		c.canCreateEPUnlocked.Set(false)
+		de, ok = c.endpointOfDisco[sender]
+		if !ok {
+			c.logf("magicsock: [unexpected] lazy endpoint not created for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
+			return false
+		}
+		c.logf("magicsock: lazy endpoint created via disco message for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
+	}
+
 	// First, do we even know (and thus care) about this sender? If not,
 	// don't bother decrypting it.
 
@@ -1506,7 +1649,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 		// Don't log in normal case. Pass on to wireguard, in case
 		// it's actually a a wireguard packet (super unlikely,
 		// but).
-		if logDisco {
+		if debugDisco {
 			c.logf("magicsock: disco: failed to open naclbox from %v (wrong rcpt?)", sender)
 		}
 		// TODO(bradfitz): add some counter for this that logs rarely
@@ -1514,7 +1657,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 	}
 
 	dm, err := disco.Parse(payload)
-	if logDisco {
+	if debugDisco {
 		c.logf("magicsock: disco: disco.Parse = %T, %v", dm, err)
 	}
 	if err != nil {
@@ -1529,8 +1672,11 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 
 	switch dm := dm.(type) {
 	case *disco.Ping:
-		c.handlePingLocked(dm, de, src)
+		c.handlePingLocked(dm, de, src, sender, peerNode)
 	case *disco.Pong:
+		if de == nil {
+			return true
+		}
 		de.handlePongConnLocked(dm, src)
 	case disco.CallMeMaybe:
 		if src.IP != derpMagicIPAddr {
@@ -1538,24 +1684,43 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 			c.logf("[unexpected] CallMeMaybe packets should only come via DERP")
 			return true
 		}
-		c.logf("magicsock: disco: %v<-%v (%v, %v)  got call-me-maybe", c.discoShort, de.discoShort, de.publicKey.ShortString(), derpStr(src.String()))
-		go de.handleCallMeMaybe()
+		if de != nil {
+			c.logf("magicsock: disco: %v<-%v (%v, %v)  got call-me-maybe", c.discoShort, de.discoShort, de.publicKey.ShortString(), derpStr(src.String()))
+			go de.handleCallMeMaybe()
+		}
 	}
 
 	return true
 }
 
-func (c *Conn) handlePingLocked(dm *disco.Ping, de *discoEndpoint, src netaddr.IPPort) {
-	c.logf("magicsock: disco: %v<-%v (%v, %v)  got ping tx=%x", c.discoShort, de.discoShort, de.publicKey.ShortString(), src, dm.TxID[:6])
+// de may be nil
+func (c *Conn) handlePingLocked(dm *disco.Ping, de *discoEndpoint, src netaddr.IPPort, sender tailcfg.DiscoKey, peerNode *tailcfg.Node) {
+	if peerNode == nil {
+		c.logf("magicsock: disco: [unexpected] ignoring ping from unknown peer Node")
+		return
+	}
+	likelyHeartBeat := de != nil && src == de.lastPingFrom && time.Since(de.lastPingTime) < 5*time.Second
+	var discoShort string
+	if de != nil {
+		discoShort = de.discoShort
+		de.lastPingFrom = src
+		de.lastPingTime = time.Now()
+	} else {
+		discoShort = sender.ShortString()
+	}
+	if !likelyHeartBeat || debugDisco {
+		c.logf("magicsock: disco: %v<-%v (%v, %v)  got ping tx=%x", c.discoShort, discoShort, peerNode.Key.ShortString(), src, dm.TxID[:6])
+	}
 
 	// Remember this route if not present.
-	c.setAddrToDiscoLocked(src, de.discoKey, nil)
+	c.setAddrToDiscoLocked(src, sender, nil)
 
-	pongDst := src
-	go de.sendDiscoMessage(pongDst, &disco.Pong{
+	ipDst := src
+	discoDest := sender
+	go c.sendDiscoMessage(ipDst, peerNode.Key, discoDest, &disco.Pong{
 		TxID: dm.TxID,
 		Src:  src,
-	})
+	}, discoVerboseLog)
 }
 
 // setAddrToDiscoLocked records that newk is at src.
@@ -1654,15 +1819,19 @@ 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() {
+		c.logf("magicsock: SetPrivateKey called (zeroed)")
+		c.closeAllDerpLocked("zero-private-key")
 	} else {
-		c.logf("magicsock: SetPrivateKey called (changed")
+		c.logf("magicsock: SetPrivateKey called (changed)")
+		c.closeAllDerpLocked("new-private-key")
 	}
-	c.closeAllDerpLocked("new-private-key")
 
 	// Key changed. Close existing DERP connections and reconnect to home.
-	if c.myDerp != 0 {
+	if c.myDerp != 0 && !newKey.IsZero() {
 		c.logf("magicsock: private key changed, reconnecting to home derp-%d", c.myDerp)
 		c.goDerpConnect(c.myDerp)
 	}
@@ -1712,11 +1881,25 @@ func (c *Conn) SetDERPMap(dm *tailcfg.DERPMap) {
 		return
 	}
 
-	go c.ReSTUN("derp-map-update")
+	if c.started {
+		go c.ReSTUN("derp-map-update")
+	}
+}
+
+func nodesEqual(x, y []*tailcfg.Node) bool {
+	if len(x) != len(y) {
+		return false
+	}
+	for i := range x {
+		if !x[i].Equal(y[i]) {
+			return false
+		}
+	}
+	return true
 }
 
 // SetNetworkMap is called when the control client gets a new network
-// map from the control server.
+// map from the control server. It must always be non-nil.
 //
 // It should not use the DERPMap field of NetworkMap; that's
 // conditionally sent to SetDERPMap instead.
@@ -1724,7 +1907,7 @@ func (c *Conn) SetNetworkMap(nm *controlclient.NetworkMap) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	if reflect.DeepEqual(nm, c.netMap) {
+	if c.netMap != nil && nodesEqual(c.netMap.Peers, nm.Peers) {
 		return
 	}
 
@@ -1916,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
@@ -2009,6 +2190,21 @@ func (c *Conn) ReSTUN(why string) {
 		return
 	}
 
+	// 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
+	}
+
 	if c.endpointsUpdateActive {
 		if c.wantEndpointsUpdate != why {
 			c.logf("magicsock: ReSTUN: endpoint update active, need another later (%q)", why)
@@ -2039,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
@@ -2069,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
@@ -2098,8 +2294,12 @@ func (c *Conn) Rebind() {
 
 	c.mu.Lock()
 	c.closeAllDerpLocked("rebind")
+	haveKey := !c.privateKey.IsZero()
 	c.mu.Unlock()
-	c.goDerpConnect(c.myDerp)
+
+	if haveKey {
+		c.goDerpConnect(c.myDerp)
+	}
 	c.resetAddrSetStates()
 }
 
@@ -2423,17 +2623,30 @@ func (c *Conn) CreateEndpoint(pubKey [32]byte, addrs string) (conn.Endpoint, err
 		if err != nil {
 			return nil, fmt.Errorf("magicsock: invalid discokey endpoint %q for %v: %w", addrs, pk.ShortString(), err)
 		}
-		c.mu.Lock()
-		defer c.mu.Unlock()
+		if !c.canCreateEPUnlocked.Get() { // sorry
+			c.mu.Lock()
+			defer c.mu.Unlock()
+		}
 		de := &discoEndpoint{
 			c:                  c,
-			publicKey:          pk,                         // peer public key (for WireGuard + DERP)
+			publicKey:          tailcfg.NodeKey(pk),        // peer public key (for WireGuard + DERP)
 			discoKey:           tailcfg.DiscoKey(discoKey), // for discovery mesages
 			discoShort:         tailcfg.DiscoKey(discoKey).ShortString(),
 			wgEndpointHostPort: addrs,
 			sentPing:           map[stun.TxID]sentPing{},
 			endpointState:      map[netaddr.IPPort]*endpointState{},
 		}
+		lastRecvTime := new(int64) // atomic
+		de.onRecvActivity = func() {
+			now := time.Now().Unix()
+			old := atomic.LoadInt64(lastRecvTime)
+			if old == 0 || old <= now-10 {
+				atomic.StoreInt64(lastRecvTime, now)
+				if c.noteRecvActivity != nil {
+					c.noteRecvActivity(de.discoKey)
+				}
+			}
+		}
 		de.initFakeUDPAddr()
 		de.updateFromNode(c.nodeOfDisco[de.discoKey])
 		c.endpointOfDisco[de.discoKey] = de
@@ -2662,14 +2875,25 @@ func (c *Conn) UpdateStatus(sb *ipnstate.StatusBuilder) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	for dk, de := range c.endpointOfDisco {
-		ps := &ipnstate.PeerStatus{InMagicSock: true}
-		if node, ok := c.nodeOfDisco[dk]; ok {
-			ps.Addrs = append(ps.Addrs, node.Endpoints...)
-			ps.Relay = c.derpRegionCodeOfAddrLocked(node.DERP)
+	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)
+			}
 		}
-		de.populatePeerStatus(ps)
-		sb.AddPeer(de.publicKey, ps)
+	}
+
+	for dk, n := range c.nodeOfDisco {
+		ps := &ipnstate.PeerStatus{InMagicSock: true}
+		ps.Addrs = append(ps.Addrs, n.Endpoints...)
+		ps.Relay = c.derpRegionCodeOfAddrLocked(n.DERP)
+		if de, ok := c.endpointOfDisco[dk]; ok {
+			de.populatePeerStatus(ps)
+		}
+		sb.AddPeer(key.Public(n.Key), ps)
 	}
 	// Old-style (pre-disco) peers:
 	for k, as := range c.addrsByKey {
@@ -2699,12 +2923,17 @@ func udpAddrDebugString(ua net.UDPAddr) string {
 type discoEndpoint struct {
 	// These fields are initialized once and never modified.
 	c                  *Conn
-	publicKey          key.Public       // peer public key (for WireGuard + DERP)
+	publicKey          tailcfg.NodeKey  // peer public key (for WireGuard + DERP)
 	discoKey           tailcfg.DiscoKey // for discovery mesages
 	discoShort         string           // ShortString of discoKey
 	fakeWGAddr         netaddr.IPPort   // the UDP address we tell wireguard-go we're using
 	fakeWGAddrStd      *net.UDPAddr     // the *net.UDPAddr form of fakeWGAddr
 	wgEndpointHostPort string           // string from CreateEndpoint: "<hex-discovery-key>.disco.tailscale:12345"
+	onRecvActivity     func()
+
+	// Owned by Conn.mu:
+	lastPingFrom netaddr.IPPort
+	lastPingTime time.Time
 
 	// mu protects all following fields.
 	mu sync.Mutex // Lock ordering: Conn.mu, then discoEndpoint.mu
@@ -2776,9 +3005,10 @@ type pongReply struct {
 }
 
 type sentPing struct {
-	to    netaddr.IPPort
-	at    time.Time
-	timer *time.Timer // timeout timer
+	to      netaddr.IPPort
+	at      time.Time
+	timer   *time.Timer // timeout timer
+	purpose discoPingPurpose
 }
 
 // initFakeUDPAddr populates fakeWGAddr with a globally unique fake UDPAddr.
@@ -2815,7 +3045,7 @@ func (de *discoEndpoint) Addrs() []wgcfg.Endpoint {
 	if err != nil {
 		panic(err)
 	}
-	return []wgcfg.Endpoint{{host, uint16(port)}}
+	return []wgcfg.Endpoint{{Host: host, Port: uint16(port)}}
 }
 
 func (de *discoEndpoint) ClearSrc()           {}
@@ -2868,7 +3098,7 @@ func (de *discoEndpoint) heartbeat() {
 	udpAddr, _ := de.addrForSendLocked(now)
 	if !udpAddr.IsZero() {
 		// We have a preferred path. Ping that every 2 seconds.
-		de.startPingLocked(udpAddr, now)
+		de.startPingLocked(udpAddr, now, pingHeartbeat)
 	}
 
 	if de.wantFullPingLocked(now) {
@@ -2921,10 +3151,10 @@ func (de *discoEndpoint) send(b []byte) error {
 	}
 	var err error
 	if !udpAddr.IsZero() {
-		_, err = de.c.sendAddr(udpAddr, de.publicKey, b)
+		_, err = de.c.sendAddr(udpAddr, key.Public(de.publicKey), b)
 	}
 	if !derpAddr.IsZero() {
-		if ok, _ := de.c.sendAddr(derpAddr, de.publicKey, b); ok && err != nil {
+		if ok, _ := de.c.sendAddr(derpAddr, key.Public(de.publicKey), b); ok && err != nil {
 			// UDP failed but DERP worked, so good enough:
 			return nil
 		}
@@ -2932,6 +3162,19 @@ func (de *discoEndpoint) send(b []byte) error {
 	return err
 }
 
+func (de *discoEndpoint) pingTimeout(txid stun.TxID) {
+	de.mu.Lock()
+	defer de.mu.Unlock()
+	sp, ok := de.sentPing[txid]
+	if !ok {
+		return
+	}
+	if debugDisco || de.bestAddr.IsZero() || time.Now().After(de.trustBestAddrUntil) {
+		de.c.logf("magicsock: disco: timeout waiting for pong %x from %v (%v, %v)", txid[:6], sp.to, de.publicKey.ShortString(), de.discoShort)
+	}
+	de.removeSentPingLocked(txid, sp)
+}
+
 // forgetPing is called by a timer when a ping either fails to send or
 // has taken too long to get a pong reply.
 func (de *discoEndpoint) forgetPing(txid stun.TxID) {
@@ -2953,14 +3196,27 @@ func (de *discoEndpoint) 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 *discoEndpoint) sendDiscoPing(ep netaddr.IPPort, txid stun.TxID) {
-	sent, _ := de.sendDiscoMessage(ep, &disco.Ping{TxID: [12]byte(txid)})
+func (de *discoEndpoint) sendDiscoPing(ep netaddr.IPPort, txid stun.TxID, logLevel discoLogLevel) {
+	sent, _ := de.sendDiscoMessage(ep, &disco.Ping{TxID: [12]byte(txid)}, logLevel)
 	if !sent {
 		de.forgetPing(txid)
 	}
 }
 
-func (de *discoEndpoint) startPingLocked(ep netaddr.IPPort, now time.Time) {
+// discoPingPurpose is the reason why a discovery ping message was sent.
+type discoPingPurpose int
+
+const (
+	// pingDiscovery means that purpose of a ping was to see if a
+	// path was valid.
+	pingDiscovery discoPingPurpose = iota
+
+	// pingHeartbeat means that purpose of a ping was whether a
+	// peer was still there.
+	pingHeartbeat
+)
+
+func (de *discoEndpoint) startPingLocked(ep netaddr.IPPort, now time.Time, purpose discoPingPurpose) {
 	st, ok := de.endpointState[ep]
 	if !ok {
 		// Shouldn't happen. But don't ping an endpoint that's
@@ -2972,14 +3228,16 @@ func (de *discoEndpoint) startPingLocked(ep netaddr.IPPort, now time.Time) {
 
 	txid := stun.NewTxID()
 	de.sentPing[txid] = sentPing{
-		to: ep,
-		at: now,
-		timer: time.AfterFunc(pingTimeoutDuration, func() {
-			de.c.logf("magicsock: disco: timeout waiting for pong %x from %v (%v, %v)", txid[:6], ep, de.publicKey.ShortString(), de.discoShort)
-			de.forgetPing(txid)
-		}),
+		to:      ep,
+		at:      now,
+		timer:   time.AfterFunc(pingTimeoutDuration, func() { de.pingTimeout(txid) }),
+		purpose: purpose,
 	}
-	go de.sendDiscoPing(ep, txid)
+	logLevel := discoLog
+	if purpose == pingHeartbeat {
+		logLevel = discoVerboseLog
+	}
+	go de.sendDiscoPing(ep, txid, logLevel)
 }
 
 func (de *discoEndpoint) sendPingsLocked(now time.Time, sendCallMeMaybe bool) {
@@ -2998,7 +3256,7 @@ func (de *discoEndpoint) sendPingsLocked(now time.Time, sendCallMeMaybe bool) {
 			de.c.logf("magicsock: disco: send, starting discovery for %v (%v)", de.publicKey.ShortString(), de.discoShort)
 		}
 
-		de.startPingLocked(ep, now)
+		de.startPingLocked(ep, now, pingDiscovery)
 	}
 	derpAddr := de.derpAddr
 	if sentAny && sendCallMeMaybe && !derpAddr.IsZero() {
@@ -3007,13 +3265,13 @@ func (de *discoEndpoint) sendPingsLocked(now time.Time, sendCallMeMaybe bool) {
 		// so our firewall ports are probably open and now would be a good time
 		// for them to connect.
 		time.AfterFunc(5*time.Millisecond, func() {
-			de.sendDiscoMessage(derpAddr, disco.CallMeMaybe{})
+			de.sendDiscoMessage(derpAddr, disco.CallMeMaybe{}, discoLog)
 		})
 	}
 }
 
-func (de *discoEndpoint) sendDiscoMessage(dst netaddr.IPPort, dm disco.Message) (sent bool, err error) {
-	return de.c.sendDiscoMessage(dst, de.publicKey, de.discoKey, dm)
+func (de *discoEndpoint) sendDiscoMessage(dst netaddr.IPPort, dm disco.Message, logLevel discoLogLevel) (sent bool, err error) {
+	return de.c.sendDiscoMessage(dst, de.publicKey, de.discoKey, dm, logLevel)
 }
 
 func (de *discoEndpoint) updateFromNode(n *tailcfg.Node) {
@@ -3110,11 +3368,13 @@ func (de *discoEndpoint) handlePongConnLocked(m *disco.Pong, src netaddr.IPPort)
 		pongSrc: m.Src,
 	})
 
-	de.c.logf("magicsock: disco: %v<-%v (%v, %v)  got pong tx=%x latency=%v pong.src=%v%v", de.c.discoShort, de.discoShort, de.publicKey.ShortString(), src, m.TxID[:6], latency.Round(time.Millisecond), m.Src, logger.ArgWriter(func(bw *bufio.Writer) {
-		if sp.to != src {
-			fmt.Fprintf(bw, " ping.to=%v", sp.to)
-		}
-	}))
+	if sp.purpose != pingHeartbeat {
+		de.c.logf("magicsock: disco: %v<-%v (%v, %v)  got pong tx=%x latency=%v pong.src=%v%v", de.c.discoShort, de.discoShort, de.publicKey.ShortString(), src, m.TxID[:6], latency.Round(time.Millisecond), m.Src, logger.ArgWriter(func(bw *bufio.Writer) {
+			if sp.to != src {
+				fmt.Fprintf(bw, " ping.to=%v", sp.to)
+			}
+		}))
+	}
 
 	// Promote this pong response to our current best address if it's lower latency.
 	// TODO(bradfitz): decide how latency vs. preference order affects decision

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"
@@ -60,6 +63,256 @@ func (c *Conn) WaitReady(t *testing.T) {
 	}
 }
 
+func runDERPAndStun(t *testing.T, logf logger.Logf, l nettype.PacketListener, stunIP netaddr.IP) (derpMap *tailcfg.DERPMap, cleanup func()) {
+	var serverPrivateKey key.Private
+	if _, err := crand.Read(serverPrivateKey[:]); err != nil {
+		t.Fatal(err)
+	}
+	d := derp.NewServer(serverPrivateKey, logf)
+
+	httpsrv := httptest.NewUnstartedServer(derphttp.Handler(d))
+	httpsrv.Config.ErrorLog = logger.StdLogger(logf)
+	httpsrv.Config.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
+	httpsrv.StartTLS()
+
+	stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, l)
+
+	m := &tailcfg.DERPMap{
+		Regions: map[int]*tailcfg.DERPRegion{
+			1: &tailcfg.DERPRegion{
+				RegionID:   1,
+				RegionCode: "test",
+				Nodes: []*tailcfg.DERPNode{
+					{
+						Name:         "t1",
+						RegionID:     1,
+						HostName:     "test-node.unused",
+						IPv4:         "127.0.0.1",
+						IPv6:         "none",
+						STUNPort:     stunAddr.Port,
+						DERPTestPort: httpsrv.Listener.Addr().(*net.TCPAddr).Port,
+						STUNTestIP:   stunIP.String(),
+					},
+				},
+			},
+		},
+	}
+
+	cleanup = func() {
+		httpsrv.CloseClientConnections()
+		httpsrv.Close()
+		d.Close()
+		stunCleanup()
+	}
+
+	return m, cleanup
+}
+
+// magicStack is a magicsock, plus all the stuff around it that's
+// necessary to send and receive packets to test e2e wireguard
+// happiness.
+type magicStack struct {
+	privateKey wgcfg.PrivateKey
+	epCh       chan []string       // endpoint updates produced by this peer
+	conn       *Conn               // the magicsock itself
+	tun        *tuntest.ChannelTUN // tuntap device to send/receive packets
+	tsTun      *tstun.TUN          // wrapped tun that implements filtering and wgengine hooks
+	dev        *device.Device      // the wireguard-go Device that connects the previous things
+}
+
+// newMagicStack builds and initializes an idle magicsock and
+// friends. You need to call conn.SetNetworkMap and dev.Reconfig
+// before anything interesting happens.
+func newMagicStack(t *testing.T, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap) *magicStack {
+	t.Helper()
+
+	privateKey, err := wgcfg.NewPrivateKey()
+	if err != nil {
+		t.Fatalf("generating private key: %v", err)
+	}
+
+	epCh := make(chan []string, 100) // arbitrary
+	conn, err := NewConn(Options{
+		Logf:           logf,
+		PacketListener: l,
+		EndpointsFunc: func(eps []string) {
+			epCh <- eps
+		},
+	})
+	if err != nil {
+		t.Fatalf("constructing magicsock: %v", err)
+	}
+	conn.Start()
+	conn.SetDERPMap(derpMap)
+	if err := conn.SetPrivateKey(privateKey); err != nil {
+		t.Fatalf("setting private key in magicsock: %v", err)
+	}
+
+	tun := tuntest.NewChannelTUN()
+	tsTun := tstun.WrapTUN(logf, tun.TUN())
+	tsTun.SetFilter(filter.NewAllowAll([]filter.Net{filter.NetAny}, logf))
+
+	dev := device.NewDevice(tsTun, &device.DeviceOptions{
+		Logger: &device.Logger{
+			Debug: logger.StdLogger(logf),
+			Info:  logger.StdLogger(logf),
+			Error: logger.StdLogger(logf),
+		},
+		CreateEndpoint: conn.CreateEndpoint,
+		CreateBind:     conn.CreateBind,
+		SkipBindUpdate: true,
+	})
+	dev.Up()
+
+	// Wait for magicsock to connect up to DERP.
+	conn.WaitReady(t)
+
+	// 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(100 * time.Millisecond)
+	}
+
+	return &magicStack{
+		privateKey: privateKey,
+		epCh:       epCh,
+		conn:       conn,
+		tun:        tun,
+		tsTun:      tsTun,
+		dev:        dev,
+	}
+}
+
+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()
@@ -85,8 +338,9 @@ func TestNewConn(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer conn.Close()
-	conn.Start()
 	conn.SetDERPMap(stuntest.DERPMapOf(stunAddr.String()))
+	conn.SetPrivateKey(wgcfg.PrivateKey(key.NewPrivate()))
+	conn.Start()
 
 	go func() {
 		var pkt [64 << 10]byte
@@ -243,44 +497,6 @@ func parseCIDR(t *testing.T, addr string) wgcfg.CIDR {
 	return cidr
 }
 
-func runDERP(t *testing.T, logf logger.Logf) (s *derp.Server, addr *net.TCPAddr, cleanupFn func()) {
-	var serverPrivateKey key.Private
-	if _, err := crand.Read(serverPrivateKey[:]); err != nil {
-		t.Fatal(err)
-	}
-
-	s = derp.NewServer(serverPrivateKey, logf)
-
-	httpsrv := httptest.NewUnstartedServer(derphttp.Handler(s))
-	httpsrv.Config.ErrorLog = logger.StdLogger(logf)
-	httpsrv.Config.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
-	httpsrv.StartTLS()
-	logf("DERP server URL: %s", httpsrv.URL)
-
-	cleanupFn = func() {
-		httpsrv.CloseClientConnections()
-		httpsrv.Close()
-		s.Close()
-	}
-
-	return s, httpsrv.Listener.Addr().(*net.TCPAddr), cleanupFn
-}
-
-// devLogger returns a wireguard-go device.Logger that writes
-// wireguard logs to the test logger.
-func devLogger(t *testing.T, prefix string, logfx logger.Logf) *device.Logger {
-	pfx := []interface{}{prefix}
-	logf := func(format string, args ...interface{}) {
-		t.Helper()
-		logfx("%s: "+format, append(pfx, args...)...)
-	}
-	return &device.Logger{
-		Debug: logger.StdLogger(logf),
-		Info:  logger.StdLogger(logf),
-		Error: logger.StdLogger(logf),
-	}
-}
-
 // TestDeviceStartStop exercises the startup and shutdown logic of
 // wireguard-go, which is intimately intertwined with magicsock's own
 // lifecycle. We seem to be good at generating deadlocks here, so if
@@ -304,7 +520,11 @@ func TestDeviceStartStop(t *testing.T) {
 
 	tun := tuntest.NewChannelTUN()
 	dev := device.NewDevice(tun.TUN(), &device.DeviceOptions{
-		Logger:         devLogger(t, "dev", t.Logf),
+		Logger: &device.Logger{
+			Debug: logger.StdLogger(t.Logf),
+			Info:  logger.StdLogger(t.Logf),
+			Error: logger.StdLogger(t.Logf),
+		},
 		CreateEndpoint: conn.CreateEndpoint,
 		CreateBind:     conn.CreateBind,
 		SkipBindUpdate: true,
@@ -336,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 {
@@ -408,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()
@@ -417,121 +833,33 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	// all log using the "current" t.Logf function. Sigh.
 	logf, setT := makeNestable(t)
 
-	derpServer, derpAddr, derpCleanupFn := runDERP(t, logf)
-	defer derpCleanupFn()
+	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
+	defer cleanup()
 
-	stunAddr, stunCleanupFn := stuntest.ServeWithPacketListener(t, d.stun)
-	defer stunCleanupFn()
-
-	derpMap := &tailcfg.DERPMap{
-		Regions: map[int]*tailcfg.DERPRegion{
-			1: &tailcfg.DERPRegion{
-				RegionID:   1,
-				RegionCode: "test",
-				Nodes: []*tailcfg.DERPNode{
-					{
-						Name:         "t1",
-						RegionID:     1,
-						HostName:     "test-node.unused",
-						IPv4:         "127.0.0.1",
-						IPv6:         "none",
-						STUNPort:     stunAddr.Port,
-						DERPTestPort: derpAddr.Port,
-						STUNTestIP:   d.stunIP.String(),
-					},
-				},
-			},
-		},
-	}
-
-	epCh1 := make(chan []string, 16)
-	conn1, err := NewConn(Options{
-		Logf:           logger.WithPrefix(logf, "conn1: "),
-		PacketListener: d.m1,
-		EndpointsFunc: func(eps []string) {
-			epCh1 <- eps
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer conn1.Close()
-	conn1.Start()
-	conn1.SetDERPMap(derpMap)
-
-	epCh2 := make(chan []string, 16)
-	conn2, err := NewConn(Options{
-		Logf:           logger.WithPrefix(logf, "conn2: "),
-		PacketListener: d.m2,
-		EndpointsFunc: func(eps []string) {
-			epCh2 <- eps
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer conn2.Close()
-	conn2.Start()
-	conn2.SetDERPMap(derpMap)
+	m1 := newMagicStack(t, logf, d.m1, derpMap)
+	defer m1.Close()
+	m2 := newMagicStack(t, logf, d.m2, derpMap)
+	defer m2.Close()
 
 	addrs := []netaddr.IPPort{
-		{IP: d.m1IP, Port: conn1.LocalPort()},
-		{IP: d.m2IP, Port: conn2.LocalPort()},
+		{IP: d.m1IP, Port: m1.conn.LocalPort()},
+		{IP: d.m2IP, Port: m2.conn.LocalPort()},
 	}
 	cfgs := makeConfigs(t, addrs)
 
-	if err := conn1.SetPrivateKey(cfgs[0].PrivateKey); err != nil {
+	if err := m1.dev.Reconfig(&cfgs[0]); err != nil {
 		t.Fatal(err)
 	}
-	if err := conn2.SetPrivateKey(cfgs[1].PrivateKey); err != nil {
+	if err := m2.dev.Reconfig(&cfgs[1]); err != nil {
 		t.Fatal(err)
 	}
 
-	//uapi1, _ := cfgs[0].ToUAPI()
-	//logf("cfg0: %v", uapi1)
-	//uapi2, _ := cfgs[1].ToUAPI()
-	//logf("cfg1: %v", uapi2)
-
-	tun1 := tuntest.NewChannelTUN()
-	tstun1 := tstun.WrapTUN(logf, tun1.TUN())
-	tstun1.SetFilter(filter.NewAllowAll([]filter.Net{filter.NetAny}, logf))
-	dev1 := device.NewDevice(tstun1, &device.DeviceOptions{
-		Logger:         devLogger(t, "dev1", logf),
-		CreateEndpoint: conn1.CreateEndpoint,
-		CreateBind:     conn1.CreateBind,
-		SkipBindUpdate: true,
-	})
-	dev1.Up()
-	if err := dev1.Reconfig(&cfgs[0]); err != nil {
-		t.Fatal(err)
-	}
-	defer dev1.Close()
-
-	tun2 := tuntest.NewChannelTUN()
-	tstun2 := tstun.WrapTUN(logf, tun2.TUN())
-	tstun2.SetFilter(filter.NewAllowAll([]filter.Net{filter.NetAny}, logf))
-	dev2 := device.NewDevice(tstun2, &device.DeviceOptions{
-		Logger:         devLogger(t, "dev2", logf),
-		CreateEndpoint: conn2.CreateEndpoint,
-		CreateBind:     conn2.CreateBind,
-		SkipBindUpdate: true,
-	})
-	dev2.Up()
-	defer dev2.Close()
-
-	if err := dev2.Reconfig(&cfgs[1]); err != nil {
-		t.Fatal(err)
-	}
-
-	conn1.WaitReady(t)
-	conn2.WaitReady(t)
-
 	ping1 := func(t *testing.T) {
 		msg2to1 := tuntest.Ping(net.ParseIP("1.0.0.1"), net.ParseIP("1.0.0.2"))
-		tun2.Outbound <- msg2to1
+		m2.tun.Outbound <- msg2to1
 		t.Log("ping1 sent")
 		select {
-		case msgRecv := <-tun1.Inbound:
+		case msgRecv := <-m1.tun.Inbound:
 			if !bytes.Equal(msg2to1, msgRecv) {
 				t.Error("ping did not transit correctly")
 			}
@@ -541,10 +869,10 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	}
 	ping2 := func(t *testing.T) {
 		msg1to2 := tuntest.Ping(net.ParseIP("1.0.0.2"), net.ParseIP("1.0.0.1"))
-		tun1.Outbound <- msg1to2
+		m1.tun.Outbound <- msg1to2
 		t.Log("ping2 sent")
 		select {
-		case msgRecv := <-tun2.Inbound:
+		case msgRecv := <-m2.tun.Inbound:
 			if !bytes.Equal(msg1to2, msgRecv) {
 				t.Error("return ping did not transit correctly")
 			}
@@ -570,12 +898,12 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 		setT(t)
 		defer setT(outerT)
 		msg1to2 := tuntest.Ping(net.ParseIP("1.0.0.2"), net.ParseIP("1.0.0.1"))
-		if err := tstun1.InjectOutbound(msg1to2); err != nil {
+		if err := m1.tsTun.InjectOutbound(msg1to2); err != nil {
 			t.Fatal(err)
 		}
 		t.Log("SendPacket sent")
 		select {
-		case msgRecv := <-tun2.Inbound:
+		case msgRecv := <-m2.tun.Inbound:
 			if !bytes.Equal(msg1to2, msgRecv) {
 				t.Error("return ping did not transit correctly")
 			}
@@ -587,7 +915,7 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	t.Run("no-op dev1 reconfig", func(t *testing.T) {
 		setT(t)
 		defer setT(outerT)
-		if err := dev1.Reconfig(&cfgs[0]); err != nil {
+		if err := m1.dev.Reconfig(&cfgs[0]); err != nil {
 			t.Fatal(err)
 		}
 		ping1(t)
@@ -629,14 +957,14 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 
 		for i := 0; i < count; i++ {
 			b := msg(i)
-			tun1.Outbound <- b
+			m1.tun.Outbound <- b
 			time.Sleep(interPacketGap)
 		}
 
 		for i := 0; i < count; i++ {
 			b := msg(i)
 			select {
-			case msgRecv := <-tun2.Inbound:
+			case msgRecv := <-m2.tun.Inbound:
 				if !bytes.Equal(b, msgRecv) {
 					if strict {
 						t.Errorf("return ping %d did not transit correctly: %s", i, cmp.Diff(b, msgRecv))
@@ -648,7 +976,6 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 				}
 			}
 		}
-
 	}
 
 	t.Run("ping 1.0.0.1 x50", func(t *testing.T) {
@@ -665,29 +992,26 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	ep1 := cfgs[1].Peers[0].Endpoints
 	ep1 = append([]wgcfg.Endpoint{derpEp}, ep1...)
 	cfgs[1].Peers[0].Endpoints = ep1
-	if err := dev1.Reconfig(&cfgs[0]); err != nil {
+	if err := m1.dev.Reconfig(&cfgs[0]); err != nil {
 		t.Fatal(err)
 	}
-	if err := dev2.Reconfig(&cfgs[1]); err != nil {
+	if err := m2.dev.Reconfig(&cfgs[1]); err != nil {
 		t.Fatal(err)
 	}
 
 	t.Run("add DERP", func(t *testing.T) {
 		setT(t)
 		defer setT(outerT)
-		defer func() {
-			logf("DERP vars: %s", derpServer.ExpVar().String())
-		}()
 		pingSeq(t, 20, 0, true)
 	})
 
 	// Disable real route.
 	cfgs[0].Peers[0].Endpoints = []wgcfg.Endpoint{derpEp}
 	cfgs[1].Peers[0].Endpoints = []wgcfg.Endpoint{derpEp}
-	if err := dev1.Reconfig(&cfgs[0]); err != nil {
+	if err := m1.dev.Reconfig(&cfgs[0]); err != nil {
 		t.Fatal(err)
 	}
-	if err := dev2.Reconfig(&cfgs[1]); err != nil {
+	if err := m2.dev.Reconfig(&cfgs[1]); err != nil {
 		t.Fatal(err)
 	}
 	time.Sleep(250 * time.Millisecond) // TODO remove
@@ -696,7 +1020,6 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 		setT(t)
 		defer setT(outerT)
 		defer func() {
-			logf("DERP vars: %s", derpServer.ExpVar().String())
 			if t.Failed() || true {
 				uapi1, _ := cfgs[0].ToUAPI()
 				logf("cfg0: %v", uapi1)
@@ -707,8 +1030,8 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 		pingSeq(t, 20, 0, true)
 	})
 
-	dev1.RemoveAllPeers()
-	dev2.RemoveAllPeers()
+	m1.dev.RemoveAllPeers()
+	m2.dev.RemoveAllPeers()
 
 	// Give one peer a non-DERP endpoint. We expect the other to
 	// accept it via roamAddr.
@@ -716,10 +1039,10 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	if ep2 := cfgs[1].Peers[0].Endpoints; len(ep2) != 1 {
 		t.Errorf("unexpected peer endpoints in dev2: %v", ep2)
 	}
-	if err := dev2.Reconfig(&cfgs[1]); err != nil {
+	if err := m2.dev.Reconfig(&cfgs[1]); err != nil {
 		t.Fatal(err)
 	}
-	if err := dev1.Reconfig(&cfgs[0]); err != nil {
+	if err := m1.dev.Reconfig(&cfgs[0]); err != nil {
 		t.Fatal(err)
 	}
 	// Dear future human debugging a test failure here: this test is
@@ -733,7 +1056,7 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 		defer setT(outerT)
 		pingSeq(t, 50, 700*time.Millisecond, false)
 
-		ep2 := dev2.Config().Peers[0].Endpoints
+		ep2 := m2.dev.Config().Peers[0].Endpoints
 		if len(ep2) != 2 {
 			t.Error("handshake spray failed to find real route")
 		}
@@ -944,7 +1267,12 @@ func TestDiscoMessage(t *testing.T) {
 	peer1Priv := c.discoPrivate
 	c.endpointOfDisco = map[tailcfg.DiscoKey]*discoEndpoint{
 		tailcfg.DiscoKey(peer1Pub): &discoEndpoint{
-			// ...
+			// ... (enough for this test)
+		},
+	}
+	c.nodeOfDisco = map[tailcfg.DiscoKey]*tailcfg.Node{
+		tailcfg.DiscoKey(peer1Pub): &tailcfg.Node{
+			// ... (enough for this test)
 		},
 	}
 

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{
@@ -206,9 +212,11 @@ func TestParsedPacket(t *testing.T) {
 		})
 	}
 
+	var sink string
 	allocs := testing.AllocsPerRun(1000, func() {
-		tests[0].qdecode.String()
+		sink = tests[0].qdecode.String()
 	})
+	_ = sink
 	if allocs != 1 {
 		t.Errorf("allocs = %v; want 1", allocs)
 	}
@@ -232,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/dns_networkmanager.go

@@ -18,7 +18,7 @@ import (
 	"github.com/godbus/dbus/v5"
 )
 
-type nmSettings map[string]map[string]dbus.Variant
+type nmConnectionSettings map[string]map[string]dbus.Variant
 
 // nmIsActive determines if NetworkManager is currently managing system DNS settings.
 func nmIsActive() bool {
@@ -56,23 +56,28 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout)
 	defer cancel()
 
+	// conn is a shared connection whose lifecycle is managed by the dbus package.
+	// We should not interfere with that by closing it.
 	conn, err := dbus.SystemBus()
 	if err != nil {
 		return fmt.Errorf("connecting to system bus: %w", err)
 	}
-	defer conn.Close()
 
 	// This is how we get at the DNS settings:
+	//
 	//               org.freedesktop.NetworkManager
-	//                              ⇩
-	//           org.freedesktop.NetworkManager.Device
-	//              (describes a network interface)
-	//                              ⇩
-	//       org.freedesktop.NetworkManager.Connection.Active
-	//  (active instance of a connection initialized from settings)
-	//                              ⇩
-	//         org.freedesktop.NetworkManager.Connection
-	//                   (connection settings)
+	//                              |
+	//                    [GetDeviceByIpIface]
+	//                              |
+	//                              v
+	//           org.freedesktop.NetworkManager.Device <--------\
+	//              (describes a network interface)             |
+	//                              |                           |
+	//                   [GetAppliedConnection]             [Reapply]
+	//                              |                           |
+	//                              v                           |
+	//          org.freedesktop.NetworkManager.Connection       |
+	//                   (connection settings)            ------/
 	//          contains {dns, dns-priority, dns-search}
 	//
 	// Ref: https://developer.gnome.org/NetworkManager/stable/settings-ipv4.html.
@@ -88,49 +93,20 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 		interfaceName,
 	).Store(&devicePath)
 	if err != nil {
-		return fmt.Errorf("GetDeviceByIpIface: %w", err)
+		return fmt.Errorf("getDeviceByIpIface: %w", err)
 	}
 	device := conn.Object("org.freedesktop.NetworkManager", devicePath)
 
-	var activeConnPath dbus.ObjectPath
+	var (
+		settings nmConnectionSettings
+		version  uint64
+	)
 	err = device.CallWithContext(
-		ctx, "org.freedesktop.DBus.Properties.Get", 0,
-		"org.freedesktop.NetworkManager.Device", "ActiveConnection",
-	).Store(&activeConnPath)
+		ctx, "org.freedesktop.NetworkManager.Device.GetAppliedConnection", 0,
+		uint32(0),
+	).Store(&settings, &version)
 	if err != nil {
-		return fmt.Errorf("getting ActiveConnection: %w", err)
-	}
-	activeConn := conn.Object("org.freedesktop.NetworkManager", activeConnPath)
-
-	var connPath dbus.ObjectPath
-	err = activeConn.CallWithContext(
-		ctx, "org.freedesktop.DBus.Properties.Get", 0,
-		"org.freedesktop.NetworkManager.Connection.Active", "Connection",
-	).Store(&connPath)
-	if err != nil {
-		return fmt.Errorf("getting Connection: %w", err)
-	}
-	connection := conn.Object("org.freedesktop.NetworkManager", connPath)
-
-	// Note: strictly speaking, the following is not safe.
-	//
-	// It appears that the way to update connection settings
-	// in NetworkManager is to get an entire connection settings object,
-	// modify the fields we are interested in, then submit the modified object.
-	//
-	// This is unfortunate: if the network state changes in the meantime
-	// (most relevantly to us, if routes change), we will overwrite those changes.
-	//
-	// That said, fortunately, this should have no real effect, as Tailscale routes
-	// do not seem to show up in NetworkManager at all,
-	// so they are presumably immune from being tampered with.
-
-	var settings nmSettings
-	err = connection.CallWithContext(
-		ctx, "org.freedesktop.NetworkManager.Settings.Connection.GetSettings", 0,
-	).Store(&settings)
-	if err != nil {
-		return fmt.Errorf("getting Settings: %w", err)
+		return fmt.Errorf("getAppliedConnection: %w", err)
 	}
 
 	// Frustratingly, NetworkManager represents IPv4 addresses as uint32s,
@@ -143,7 +119,7 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	for _, ip := range config.Nameservers {
 		b := ip.As16()
 		if ip.Is4() {
-			dnsv4 = append(dnsv4, binary.BigEndian.Uint32(b[12:]))
+			dnsv4 = append(dnsv4, binary.LittleEndian.Uint32(b[12:]))
 		} else {
 			dnsv6 = append(dnsv6, b[:])
 		}
@@ -152,10 +128,15 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	ipv4Map := settings["ipv4"]
 	ipv4Map["dns"] = dbus.MakeVariant(dnsv4)
 	ipv4Map["dns-search"] = dbus.MakeVariant(config.Domains)
-	// dns-priority = -1 ensures that we have priority
-	// over other interfaces, except those exploiting this same trick.
-	// Ref: https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1211110/comments/92.
-	ipv4Map["dns-priority"] = dbus.MakeVariant(-1)
+	// We should only request priority if we have nameservers to set.
+	if len(dnsv4) == 0 {
+		ipv4Map["dns-priority"] = dbus.MakeVariant(100)
+	} else {
+		// dns-priority = -1 ensures that we have priority
+		// over other interfaces, except those exploiting this same trick.
+		// Ref: https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1211110/comments/92.
+		ipv4Map["dns-priority"] = dbus.MakeVariant(-1)
+	}
 	// In principle, we should not need set this to true,
 	// as our interface does not configure any automatic DNS settings (presumably via DHCP).
 	// All the same, better to be safe.
@@ -175,7 +156,11 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	// Finally, set the actual DNS config.
 	ipv6Map["dns"] = dbus.MakeVariant(dnsv6)
 	ipv6Map["dns-search"] = dbus.MakeVariant(config.Domains)
-	ipv6Map["dns-priority"] = dbus.MakeVariant(-1)
+	if len(dnsv6) == 0 {
+		ipv6Map["dns-priority"] = dbus.MakeVariant(100)
+	} else {
+		ipv6Map["dns-priority"] = dbus.MakeVariant(-1)
+	}
 	ipv6Map["ignore-auto-dns"] = dbus.MakeVariant(true)
 
 	// deprecatedProperties are the properties in interface settings
@@ -193,11 +178,12 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 		delete(ipv6Map, property)
 	}
 
-	err = connection.CallWithContext(
-		ctx, "org.freedesktop.NetworkManager.Settings.Connection.UpdateUnsaved", 0, settings,
+	err = device.CallWithContext(
+		ctx, "org.freedesktop.NetworkManager.Device.Reapply", 0,
+		settings, version, uint32(0),
 	).Store()
 	if err != nil {
-		return fmt.Errorf("setting Settings: %w", err)
+		return fmt.Errorf("reapply: %w", err)
 	}
 
 	return nil

wgengine/router/dns_resolvconf.go

@@ -57,13 +57,57 @@ func resolvconfIsActive() bool {
 	return false
 }
 
+// resolvconfImplementation enumerates supported implementations of the resolvconf CLI.
+type resolvconfImplementation uint8
+
+const (
+	// resolvconfOpenresolv is the implementation packaged as "openresolv" on Ubuntu.
+	// It supports exclusive mode and interface metrics.
+	resolvconfOpenresolv resolvconfImplementation = iota
+	// resolvconfLegacy is the implementation by Thomas Hood packaged as "resolvconf" on Ubuntu.
+	// It does not support exclusive mode or interface metrics.
+	resolvconfLegacy
+)
+
+// getResolvconfImplementation returns the implementation of resolvconf
+// that appears to be in use.
+func getResolvconfImplementation() resolvconfImplementation {
+	err := exec.Command("resolvconf", "-v").Run()
+	if err != nil {
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			// Thomas Hood's resolvconf has a minimal flag set
+			// and exits with code 99 when passed an unknown flag.
+			if exitErr.ExitCode() == 99 {
+				return resolvconfLegacy
+			}
+		}
+	}
+	return resolvconfOpenresolv
+}
+
+// resolvconfConfigName is the name of the config submitted to resolvconf.
+// It has this form to match the "tun*" rule in interface-order
+// when running resolvconfLegacy, hopefully placing our config first.
+const resolvconfConfigName = "tun-tailscale.inet"
+
 // dnsResolvconfUp invokes the resolvconf binary to associate
 // the given DNS configuration the Tailscale interface.
 func dnsResolvconfUp(config DNSConfig, interfaceName string) error {
+	implementation := getResolvconfImplementation()
+
 	stdin := new(bytes.Buffer)
 	dnsWriteConfig(stdin, config.Nameservers, config.Domains) // dns_direct.go
 
-	cmd := exec.Command("resolvconf", "-m", "0", "-x", "-a", interfaceName+".inet")
+	var cmd *exec.Cmd
+	switch implementation {
+	case resolvconfOpenresolv:
+		// Request maximal priority (metric 0) and exclusive mode.
+		cmd = exec.Command("resolvconf", "-m", "0", "-x", "-a", resolvconfConfigName)
+	case resolvconfLegacy:
+		// This does not quite give us the desired behavior (queries leak),
+		// but there is nothing else we can do without messing with other interfaces' settings.
+		cmd = exec.Command("resolvconf", "-a", resolvconfConfigName)
+	}
 	cmd.Stdin = stdin
 	out, err := cmd.CombinedOutput()
 	if err != nil {
@@ -75,10 +119,21 @@ func dnsResolvconfUp(config DNSConfig, interfaceName string) error {
 
 // dnsResolvconfDown undoes the action of dnsResolvconfUp.
 func dnsResolvconfDown(interfaceName string) error {
-	cmd := exec.Command("resolvconf", "-f", "-d", interfaceName+".inet")
+	implementation := getResolvconfImplementation()
+
+	var cmd *exec.Cmd
+	switch implementation {
+	case resolvconfOpenresolv:
+		cmd = exec.Command("resolvconf", "-f", "-d", resolvconfConfigName)
+	case resolvconfLegacy:
+		// resolvconfLegacy lacks the -f flag.
+		// Instead, it succeeds even when the config does not exist.
+		cmd = exec.Command("resolvconf", "-d", resolvconfConfigName)
+	}
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("running %s: %s", cmd, out)
 	}
+
 	return nil
 }

wgengine/router/dns_resolved.go

@@ -88,11 +88,12 @@ func dnsResolvedUp(config DNSConfig) error {
 	ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout)
 	defer cancel()
 
+	// conn is a shared connection whose lifecycle is managed by the dbus package.
+	// We should not interfere with that by closing it.
 	conn, err := dbus.SystemBus()
 	if err != nil {
 		return fmt.Errorf("connecting to system bus: %w", err)
 	}
-	defer conn.Close()
 
 	resolved := conn.Object(
 		"org.freedesktop.resolve1",
@@ -155,6 +156,8 @@ func dnsResolvedDown() error {
 	ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout)
 	defer cancel()
 
+	// conn is a shared connection whose lifecycle is managed by the dbus package.
+	// We should not interfere with that by closing it.
 	conn, err := dbus.SystemBus()
 	if err != nil {
 		return fmt.Errorf("connecting to system bus: %w", err)

wgengine/router/router_linux.go

@@ -37,15 +37,31 @@ import (
 const (
 	// Packet is from Tailscale and to a subnet route destination, so
 	// is allowed to be routed through this machine.
-	tailscaleSubnetRouteMark = "0x10000"
+	tailscaleSubnetRouteMark = "0x40000"
 	// Packet was originated by tailscaled itself, and must not be
 	// routed over the Tailscale network.
 	//
 	// Keep this in sync with tailscaleBypassMark in
 	// net/netns/netns_linux.go.
-	tailscaleBypassMark = "0x20000"
+	tailscaleBypassMark = "0x80000"
 )
 
+// tailscaleRouteTable is the routing table number for Tailscale
+// network routes. See addIPRules for the detailed policy routing
+// logic that ends up doing lookups within that table.
+//
+// NOTE(danderson): We chose 52 because those are the digits above the
+// letters "TS" on a qwerty keyboard, and 52 is sufficiently unlikely
+// to be picked by other software.
+//
+// NOTE(danderson): You might wonder why we didn't pick some high
+// table number like 5252, to further avoid the potential for
+// collisions with other software. Unfortunately, Busybox's `ip`
+// implementation believes that table numbers are 8-bit integers, so
+// for maximum compatibility we have to stay in the 0-255 range even
+// though linux itself supports larger numbers.
+const tailscaleRouteTable = "52"
+
 // netfilterRunner abstracts helpers to run netfilter commands. It
 // exists purely to swap out go-iptables for a fake implementation in
 // tests.
@@ -378,7 +394,7 @@ func (r *linuxRouter) addRoute(cidr netaddr.IPPrefix) error {
 		"dev", r.tunname,
 	}
 	if r.ipRuleAvailable {
-		args = append(args, "table", "88")
+		args = append(args, "table", tailscaleRouteTable)
 	}
 	return r.cmd.run(args...)
 }
@@ -393,7 +409,7 @@ func (r *linuxRouter) delRoute(cidr netaddr.IPPrefix) error {
 		"dev", r.tunname,
 	}
 	if r.ipRuleAvailable {
-		args = append(args, "table", "88")
+		args = append(args, "table", tailscaleRouteTable)
 	}
 	return r.cmd.run(args...)
 }
@@ -431,7 +447,7 @@ func (r *linuxRouter) addIPRules() error {
 	// NOTE(apenwarr): This sequence seems complicated, right?
 	// If we could simply have a rule that said "match packets that
 	// *don't* have this fwmark", then we would only need to add one
-	// link to table 88 and we'd be done. Unfortunately, older kernels
+	// link to table 52 and we'd be done. Unfortunately, older kernels
 	// and 'ip rule' implementations (including busybox), don't support
 	// checking for the lack of a fwmark, only the presence. The technique
 	// below works even on very old kernels.
@@ -440,7 +456,7 @@ func (r *linuxRouter) addIPRules() error {
 	// main routing table.
 	rg.Run(
 		"ip", "rule", "add",
-		"pref", "8810",
+		"pref", tailscaleRouteTable+"10",
 		"fwmark", tailscaleBypassMark,
 		"table", "main",
 	)
@@ -448,7 +464,7 @@ func (r *linuxRouter) addIPRules() error {
 	// even though it's been empty on every Linux system I've ever seen.
 	rg.Run(
 		"ip", "rule", "add",
-		"pref", "8830",
+		"pref", tailscaleRouteTable+"30",
 		"fwmark", tailscaleBypassMark,
 		"table", "default",
 	)
@@ -457,23 +473,22 @@ func (r *linuxRouter) addIPRules() error {
 	// to the tailscale routes, because that would create routing loops.
 	rg.Run(
 		"ip", "rule", "add",
-		"pref", "8850",
+		"pref", tailscaleRouteTable+"50",
 		"fwmark", tailscaleBypassMark,
 		"type", "unreachable",
 	)
 	// If we get to this point, capture all packets and send them
-	// through to table 88, the set of tailscale routes.
-	// For apps other than us (ie. with no fwmark set), this is the
-	// first routing table, so it takes precedence over all the others,
-	// ie. VPN routes always beat non-VPN routes.
+	// through to the tailscale route table. For apps other than us
+	// (ie. with no fwmark set), this is the first routing table, so
+	// it takes precedence over all the others, ie. VPN routes always
+	// beat non-VPN routes.
 	//
-	// NOTE(apenwarr): tables >255 are not supported in busybox.
-	// I really wanted to use table 8888 here for symmetry, but no luck
-	// with busybox alas.
+	// NOTE(apenwarr): tables >255 are not supported in busybox, so we
+	// can't use a table number that aligns with the rule preferences.
 	rg.Run(
 		"ip", "rule", "add",
-		"pref", "8888",
-		"table", "88",
+		"pref", tailscaleRouteTable+"70",
+		"table", tailscaleRouteTable,
 	)
 	// If that didn't match, then non-fwmark packets fall through to the
 	// usual rules (pref 32766 and 32767, ie. main and default).
@@ -514,23 +529,23 @@ func (r *linuxRouter) delIPRules() error {
 	// Delete new-style tailscale rules.
 	rg.Run(
 		"ip", "rule", "del",
-		"pref", "8810",
+		"pref", tailscaleRouteTable+"10",
 		"table", "main",
 	)
 	rg.Run(
 		"ip", "rule", "del",
-		"pref", "8830",
+		"pref", tailscaleRouteTable+"30",
 		"table", "default",
 	)
 	rg.Run(
 		"ip", "rule", "del",
-		"pref", "8850",
+		"pref", tailscaleRouteTable+"50",
 		"type", "unreachable",
 	)
 	rg.Run(
 		"ip", "rule", "del",
-		"pref", "8888",
-		"table", "88",
+		"pref", tailscaleRouteTable+"70",
+		"table", tailscaleRouteTable,
 	)
 	return rg.ErrAcc
 }

wgengine/router/router_linux_test.go

@@ -34,10 +34,10 @@ func mustCIDRs(ss ...string) []netaddr.IPPrefix {
 
 func TestRouterStates(t *testing.T) {
 	basic := `
-ip rule add pref 8810 fwmark 0x20000 table main
-ip rule add pref 8830 fwmark 0x20000 table default
-ip rule add pref 8850 fwmark 0x20000 type unreachable
-ip rule add pref 8888 table 88
+ip rule add pref 5210 fwmark 0x80000 table main
+ip rule add pref 5230 fwmark 0x80000 table default
+ip rule add pref 5250 fwmark 0x80000 type unreachable
+ip rule add pref 5270 table 52
 `
 	states := []struct {
 		name string
@@ -71,8 +71,8 @@ ip addr add 100.101.102.103/10 dev tailscale0` + basic,
 			want: `
 up
 ip addr add 100.101.102.103/10 dev tailscale0
-ip route add 100.100.100.100/32 dev tailscale0 table 88
-ip route add 192.168.16.0/24 dev tailscale0 table 88` + basic,
+ip route add 100.100.100.100/32 dev tailscale0 table 52
+ip route add 192.168.16.0/24 dev tailscale0 table 52` + basic,
 		},
 
 		{
@@ -86,8 +86,8 @@ ip route add 192.168.16.0/24 dev tailscale0 table 88` + basic,
 			want: `
 up
 ip addr add 100.101.102.103/10 dev tailscale0
-ip route add 100.100.100.100/32 dev tailscale0 table 88
-ip route add 192.168.16.0/24 dev tailscale0 table 88` + basic,
+ip route add 100.100.100.100/32 dev tailscale0 table 52
+ip route add 192.168.16.0/24 dev tailscale0 table 52` + basic,
 		},
 
 		{
@@ -102,19 +102,19 @@ ip route add 192.168.16.0/24 dev tailscale0 table 88` + basic,
 			want: `
 up
 ip addr add 100.101.102.104/10 dev tailscale0
-ip route add 10.0.0.0/8 dev tailscale0 table 88
-ip route add 100.100.100.100/32 dev tailscale0 table 88` + basic +
+ip route add 10.0.0.0/8 dev tailscale0 table 52
+ip route add 100.100.100.100/32 dev tailscale0 table 52` + basic +
 				`filter/FORWARD -j ts-forward
 filter/INPUT -j ts-input
-filter/ts-forward -i tailscale0 -j MARK --set-mark 0x10000
-filter/ts-forward -m mark --mark 0x10000 -j ACCEPT
+filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000
+filter/ts-forward -m mark --mark 0x40000 -j ACCEPT
 filter/ts-forward -o tailscale0 -s 100.64.0.0/10 -j DROP
 filter/ts-forward -o tailscale0 -j ACCEPT
 filter/ts-input -i lo -s 100.101.102.104 -j ACCEPT
 filter/ts-input ! -i tailscale0 -s 100.115.92.0/23 -j RETURN
 filter/ts-input ! -i tailscale0 -s 100.64.0.0/10 -j DROP
 nat/POSTROUTING -j ts-postrouting
-nat/ts-postrouting -m mark --mark 0x10000 -j MASQUERADE
+nat/ts-postrouting -m mark --mark 0x40000 -j MASQUERADE
 `,
 		},
 		{
@@ -127,12 +127,12 @@ nat/ts-postrouting -m mark --mark 0x10000 -j MASQUERADE
 			want: `
 up
 ip addr add 100.101.102.104/10 dev tailscale0
-ip route add 10.0.0.0/8 dev tailscale0 table 88
-ip route add 100.100.100.100/32 dev tailscale0 table 88` + basic +
+ip route add 10.0.0.0/8 dev tailscale0 table 52
+ip route add 100.100.100.100/32 dev tailscale0 table 52` + basic +
 				`filter/FORWARD -j ts-forward
 filter/INPUT -j ts-input
-filter/ts-forward -i tailscale0 -j MARK --set-mark 0x10000
-filter/ts-forward -m mark --mark 0x10000 -j ACCEPT
+filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000
+filter/ts-forward -m mark --mark 0x40000 -j ACCEPT
 filter/ts-forward -o tailscale0 -s 100.64.0.0/10 -j DROP
 filter/ts-forward -o tailscale0 -j ACCEPT
 filter/ts-input -i lo -s 100.101.102.104 -j ACCEPT
@@ -154,12 +154,12 @@ nat/POSTROUTING -j ts-postrouting
 			want: `
 up
 ip addr add 100.101.102.104/10 dev tailscale0
-ip route add 10.0.0.0/8 dev tailscale0 table 88
-ip route add 100.100.100.100/32 dev tailscale0 table 88` + basic +
+ip route add 10.0.0.0/8 dev tailscale0 table 52
+ip route add 100.100.100.100/32 dev tailscale0 table 52` + basic +
 				`filter/FORWARD -j ts-forward
 filter/INPUT -j ts-input
-filter/ts-forward -i tailscale0 -j MARK --set-mark 0x10000
-filter/ts-forward -m mark --mark 0x10000 -j ACCEPT
+filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000
+filter/ts-forward -m mark --mark 0x40000 -j ACCEPT
 filter/ts-forward -o tailscale0 -s 100.64.0.0/10 -j DROP
 filter/ts-forward -o tailscale0 -j ACCEPT
 filter/ts-input -i lo -s 100.101.102.104 -j ACCEPT
@@ -178,12 +178,12 @@ nat/POSTROUTING -j ts-postrouting
 			want: `
 up
 ip addr add 100.101.102.104/10 dev tailscale0
-ip route add 10.0.0.0/8 dev tailscale0 table 88
-ip route add 100.100.100.100/32 dev tailscale0 table 88` + basic +
+ip route add 10.0.0.0/8 dev tailscale0 table 52
+ip route add 100.100.100.100/32 dev tailscale0 table 52` + basic +
 				`filter/FORWARD -j ts-forward
 filter/INPUT -j ts-input
-filter/ts-forward -i tailscale0 -j MARK --set-mark 0x10000
-filter/ts-forward -m mark --mark 0x10000 -j ACCEPT
+filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000
+filter/ts-forward -m mark --mark 0x40000 -j ACCEPT
 filter/ts-forward -o tailscale0 -s 100.64.0.0/10 -j DROP
 filter/ts-forward -o tailscale0 -j ACCEPT
 filter/ts-input -i lo -s 100.101.102.104 -j ACCEPT
@@ -203,10 +203,10 @@ nat/POSTROUTING -j ts-postrouting
 			want: `
 up
 ip addr add 100.101.102.104/10 dev tailscale0
-ip route add 10.0.0.0/8 dev tailscale0 table 88
-ip route add 100.100.100.100/32 dev tailscale0 table 88` + basic +
-				`filter/ts-forward -i tailscale0 -j MARK --set-mark 0x10000
-filter/ts-forward -m mark --mark 0x10000 -j ACCEPT
+ip route add 10.0.0.0/8 dev tailscale0 table 52
+ip route add 100.100.100.100/32 dev tailscale0 table 52` + basic +
+				`filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000
+filter/ts-forward -m mark --mark 0x40000 -j ACCEPT
 filter/ts-forward -o tailscale0 -s 100.64.0.0/10 -j DROP
 filter/ts-forward -o tailscale0 -j ACCEPT
 filter/ts-input -i lo -s 100.101.102.104 -j ACCEPT
@@ -224,12 +224,12 @@ filter/ts-input ! -i tailscale0 -s 100.64.0.0/10 -j DROP
 			want: `
 up
 ip addr add 100.101.102.104/10 dev tailscale0
-ip route add 10.0.0.0/8 dev tailscale0 table 88
-ip route add 100.100.100.100/32 dev tailscale0 table 88` + basic +
+ip route add 10.0.0.0/8 dev tailscale0 table 52
+ip route add 100.100.100.100/32 dev tailscale0 table 52` + basic +
 				`filter/FORWARD -j ts-forward
 filter/INPUT -j ts-input
-filter/ts-forward -i tailscale0 -j MARK --set-mark 0x10000
-filter/ts-forward -m mark --mark 0x10000 -j ACCEPT
+filter/ts-forward -i tailscale0 -j MARK --set-mark 0x40000
+filter/ts-forward -m mark --mark 0x40000 -j ACCEPT
 filter/ts-forward -o tailscale0 -s 100.64.0.0/10 -j DROP
 filter/ts-forward -o tailscale0 -j ACCEPT
 filter/ts-input -i lo -s 100.101.102.104 -j ACCEPT

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/tstun/tun.go

@@ -45,6 +45,11 @@ var (
 	errOffsetTooSmall = errors.New("offset smaller than PacketStartOffset")
 )
 
+// parsedPacketPool holds a pool of ParsedPacket structs for use in filtering.
+// This is needed because escape analysis cannot see that parsed packets
+// do not escape through {Pre,Post}Filter{In,Out}.
+var parsedPacketPool = sync.Pool{New: func() interface{} { return new(packet.ParsedPacket) }}
+
 // FilterFunc is a packet-filtering function with access to the TUN device.
 // It must not hold onto the packet struct, as its backing storage will be reused.
 type FilterFunc func(*packet.ParsedPacket, *TUN) filter.Response
@@ -61,15 +66,13 @@ type TUN struct {
 	_                  [4]byte // force 64-bit alignment of following field on 32-bit
 	lastActivityAtomic int64   // unix seconds of last send or receive
 
+	destIPActivity atomic.Value // of map[packet.IP]func()
+
 	// buffer stores the oldest unconsumed packet from tdev.
 	// It is made a static buffer in order to avoid allocations.
 	buffer [maxBufferSize]byte
 	// bufferConsumed synchronizes access to buffer (shared by Read and poll).
 	bufferConsumed chan struct{}
-	// parsedPacketPool holds a pool of ParsedPacket structs for use in filtering.
-	// This is needed because escape analysis cannot see that parsed packets
-	// do not escape through {Pre,Post}Filter{In,Out}.
-	parsedPacketPool sync.Pool // of *packet.ParsedPacket
 
 	// closed signals poll (by closing) when the device is closed.
 	closed chan struct{}
@@ -121,10 +124,6 @@ func WrapTUN(logf logger.Logf, tdev tun.Device) *TUN {
 		filterFlags: filter.LogAccepts | filter.LogDrops,
 	}
 
-	tun.parsedPacketPool.New = func() interface{} {
-		return new(packet.ParsedPacket)
-	}
-
 	go tun.poll()
 	// The buffer starts out consumed.
 	tun.bufferConsumed <- struct{}{}
@@ -132,6 +131,14 @@ func WrapTUN(logf logger.Logf, tdev tun.Device) *TUN {
 	return tun
 }
 
+// SetDestIPActivityFuncs sets a map of funcs to run per packet
+// destination (the map keys).
+//
+// The map ownership passes to the TUN. It must be non-nil.
+func (t *TUN) SetDestIPActivityFuncs(m map[packet.IP]func()) {
+	t.destIPActivity.Store(m)
+}
+
 func (t *TUN) Close() error {
 	select {
 	case <-t.closed:
@@ -207,10 +214,7 @@ func (t *TUN) poll() {
 	}
 }
 
-func (t *TUN) filterOut(buf []byte) filter.Response {
-	p := t.parsedPacketPool.Get().(*packet.ParsedPacket)
-	defer t.parsedPacketPool.Put(p)
-	p.Decode(buf)
+func (t *TUN) filterOut(p *packet.ParsedPacket) filter.Response {
 
 	if t.PreFilterOut != nil {
 		if t.PreFilterOut(p, t) == filter.Drop {
@@ -274,8 +278,18 @@ func (t *TUN) Read(buf []byte, offset int) (int, error) {
 		}
 	}
 
+	p := parsedPacketPool.Get().(*packet.ParsedPacket)
+	defer parsedPacketPool.Put(p)
+	p.Decode(buf[offset : offset+n])
+
+	if m, ok := t.destIPActivity.Load().(map[packet.IP]func()); ok {
+		if fn := m[p.DstIP]; fn != nil {
+			fn()
+		}
+	}
+
 	if !t.disableFilter {
-		response := t.filterOut(buf[offset : offset+n])
+		response := t.filterOut(p)
 		if response != filter.Accept {
 			// Wireguard considers read errors fatal; pretend nothing was read
 			return 0, nil
@@ -287,8 +301,8 @@ func (t *TUN) Read(buf []byte, offset int) (int, error) {
 }
 
 func (t *TUN) filterIn(buf []byte) filter.Response {
-	p := t.parsedPacketPool.Get().(*packet.ParsedPacket)
-	defer t.parsedPacketPool.Put(p)
+	p := parsedPacketPool.Get().(*packet.ParsedPacket)
+	defer parsedPacketPool.Put(p)
 	p.Decode(buf)
 
 	if t.PreFilterIn != nil {

wgengine/tstun/tun_test.go

@@ -29,17 +29,24 @@ func udp(src, dst packet.IP, sport, dport uint16) []byte {
 	return packet.Generate(header, []byte("udp_payload"))
 }
 
+func filterNet(ip, mask packet.IP) filter.Net {
+	return filter.Net{IP: ip, Mask: mask}
+}
+
 func nets(ips []packet.IP) []filter.Net {
 	out := make([]filter.Net, 0, len(ips))
 	for _, ip := range ips {
-		out = append(out, filter.Net{ip, filter.Netmask(32)})
+		out = append(out, filterNet(ip, filter.Netmask(32)))
 	}
 	return out
 }
 
 func ippr(ip packet.IP, start, end uint16) []filter.NetPortRange {
 	return []filter.NetPortRange{
-		filter.NetPortRange{filter.Net{ip, filter.Netmask(32)}, filter.PortRange{start, end}},
+		filter.NetPortRange{
+			Net:   filterNet(ip, filter.Netmask(32)),
+			Ports: filter.PortRange{First: start, Last: end},
+		},
 	}
 }
 
@@ -49,7 +56,7 @@ func setfilter(logf logger.Logf, tun *TUN) {
 		{Srcs: nets([]packet.IP{0x01020304}), Dsts: ippr(0x05060708, 98, 98)},
 	}
 	localNets := []filter.Net{
-		{packet.IP(0x01020304), filter.Netmask(16)},
+		filterNet(packet.IP(0x01020304), filter.Netmask(16)),
 	}
 	tun.SetFilter(filter.New(matches, localNets, nil, logf))
 }

wgengine/userspace.go

@@ -8,6 +8,7 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"encoding/binary"
 	"errors"
 	"fmt"
 	"io"
@@ -15,6 +16,7 @@ import (
 	"os"
 	"os/exec"
 	"runtime"
+	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -32,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"
@@ -59,6 +62,15 @@ const (
 // magicDNSDomain is the parent domain for Tailscale nodes.
 const magicDNSDomain = "b.tailscale.net"
 
+// Lazy wireguard-go configuration parameters.
+const (
+	// lazyPeerIdleThreshold is the idle duration after
+	// which we remove a peer from the wireguard configuration.
+	// (This includes peers that have never been idle, which
+	// effectively have infinite idleness)
+	lazyPeerIdleThreshold = 5 * time.Minute
+)
+
 type userspaceEngine struct {
 	logf            logger.Logf
 	reqCh           chan struct{}
@@ -76,10 +88,14 @@ type userspaceEngine struct {
 	// incorrectly sent to us.
 	localAddrs atomic.Value // of map[packet.IP]bool
 
-	wgLock        sync.Mutex // serializes all wgdev operations; see lock order comment below
-	lastEngineSig string
-	lastRouterSig string
-	lastCfg       wgcfg.Config
+	wgLock              sync.Mutex // serializes all wgdev operations; see lock order comment below
+	lastCfgFull         wgcfg.Config
+	lastRouterSig       string // of router.Config
+	lastEngineSigFull   string // of full wireguard config
+	lastEngineSigTrim   string // of trimmed wireguard config
+	recvActivityAt      map[tailcfg.DiscoKey]time.Time
+	sentActivityAt      map[packet.IP]*int64 // value is atomic int64 of unixtime
+	destIPActivityFuncs map[packet.IP]func()
 
 	mu             sync.Mutex // guards following; see lock order comment below
 	closing        bool       // Close was called (even if we're still closing)
@@ -210,10 +226,11 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) {
 		e.RequestStatus()
 	}
 	magicsockOpts := magicsock.Options{
-		Logf:          logf,
-		Port:          conf.ListenPort,
-		EndpointsFunc: endpointsFn,
-		IdleFunc:      e.tundev.IdleDuration,
+		Logf:             logf,
+		Port:             conf.ListenPort,
+		EndpointsFunc:    endpointsFn,
+		IdleFunc:         e.tundev.IdleDuration,
+		NoteRecvActivity: e.noteReceiveActivity,
 	}
 	e.magicConn, err = magicsock.NewConn(magicsockOpts)
 	if err != nil {
@@ -513,8 +530,8 @@ func (e *userspaceEngine) pinger(peerKey wgcfg.Key, ips []wgcfg.IP) {
 	var srcIP packet.IP
 
 	e.wgLock.Lock()
-	if len(e.lastCfg.Addresses) > 0 {
-		srcIP = packet.NewIP(e.lastCfg.Addresses[0].IP.IP())
+	if len(e.lastCfgFull.Addresses) > 0 {
+		srcIP = packet.NewIP(e.lastCfgFull.Addresses[0].IP.IP())
 	}
 	e.wgLock.Unlock()
 
@@ -545,13 +562,222 @@ 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
+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
+	}
+	// 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
+// network map.
+//
+// We can only trim peers that both a) support discovery (because we
+// know who they are when we receive their data and don't need to rely
+// on wireguard-go figuring it out) and b) for implementation
+// 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, numPeers int) bool {
+	if forceFullWireguardConfig(numPeers) {
+		return false
+	}
+	if len(p.AllowedIPs) != 1 || len(p.Endpoints) != 1 {
+		return false
+	}
+	if !strings.HasSuffix(p.Endpoints[0].Host, ".disco.tailscale") {
+		return false
+	}
+	aip := p.AllowedIPs[0]
+	// TODO: IPv6 support, once we support IPv6 within the tunnel. In that case,
+	// len(p.AllowedIPs) probably will be more than 1.
+	if aip.Mask != 32 || !aip.IP.Is4() {
+		return false
+	}
+	return true
+}
+
+// noteReceiveActivity is called by magicsock when a packet has been received
+// by the peer using discovery key dk. Magicsock calls this no more than
+// every 10 seconds for a given peer.
+func (e *userspaceEngine) noteReceiveActivity(dk tailcfg.DiscoKey) {
+	e.wgLock.Lock()
+	defer e.wgLock.Unlock()
+
+	now := time.Now()
+	was, ok := e.recvActivityAt[dk]
+	if !ok {
+		// Not a trimmable peer we care about tracking. (See isTrimmablePeer)
+		return
+	}
+	e.recvActivityAt[dk] = now
+
+	// If the last activity time jumped a bunch (say, at least
+	// half the idle timeout) then see if we need to reprogram
+	// Wireguard. This could probably be just
+	// lazyPeerIdleThreshold without the divide by 2, but
+	// maybeReconfigWireguardLocked is cheap enough to call every
+	// couple minutes (just not on every packet).
+	if was.IsZero() || now.Sub(was) < -lazyPeerIdleThreshold/2 {
+		e.maybeReconfigWireguardLocked()
+	}
+}
+
+// isActiveSince reports whether the peer identified by (dk, ip) has
+// had a packet sent to or received from it since t.
+//
+// e.wgLock must be held.
+func (e *userspaceEngine) isActiveSince(dk tailcfg.DiscoKey, ip wgcfg.IP, t time.Time) bool {
+	if e.recvActivityAt[dk].After(t) {
 		return true
 	}
-	return false
+	pip := packet.IP(binary.BigEndian.Uint32(ip.Addr[12:]))
+	timePtr, ok := e.sentActivityAt[pip]
+	if !ok {
+		return false
+	}
+	unixTime := atomic.LoadInt64(timePtr)
+	return unixTime >= t.Unix()
+}
+
+// discoKeyFromPeer returns the DiscoKey for a wireguard config's Peer.
+//
+// Invariant: isTrimmablePeer(p) == true, so it should have 1 endpoint with
+// Host of form "<64-hex-digits>.disco.tailscale". If invariant is violated,
+// we return the zero value.
+func discoKeyFromPeer(p *wgcfg.Peer) tailcfg.DiscoKey {
+	host := p.Endpoints[0].Host
+	if len(host) < 64 {
+		return tailcfg.DiscoKey{}
+	}
+	k, err := key.NewPublicFromHexMem(mem.S(host[:64]))
+	if err != nil {
+		return tailcfg.DiscoKey{}
+	}
+	return tailcfg.DiscoKey(k)
+}
+
+// e.wgLock must be held.
+func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
+	full := e.lastCfgFull
+
+	// Compute a minimal config to pass to wireguard-go
+	// based on the full config. Prune off all the peers
+	// and only add the active ones back.
+	min := full
+	min.Peers = nil
+
+	// We'll only keep a peer around if it's been active in
+	// the past 5 minutes. That's more than WireGuard's key
+	// rotation time anyway so it's no harm if we remove it
+	// later if it's been inactive.
+	activeCutoff := time.Now().Add(-lazyPeerIdleThreshold)
+
+	// Not all peers can be trimmed from the network map (see
+	// isTrimmablePeer).  For those are are trimmable, keep track
+	// of their DiscoKey and Tailscale IPs.  These are the ones
+	// we'll need to install tracking hooks for to watch their
+	// send/receive activity.
+	trackDisco := make([]tailcfg.DiscoKey, 0, len(full.Peers))
+	trackIPs := make([]wgcfg.IP, 0, len(full.Peers))
+
+	for i := range full.Peers {
+		p := &full.Peers[i]
+		if !isTrimmablePeer(p, len(full.Peers)) {
+			min.Peers = append(min.Peers, *p)
+			continue
+		}
+		tsIP := p.AllowedIPs[0].IP
+		dk := discoKeyFromPeer(p)
+		trackDisco = append(trackDisco, dk)
+		trackIPs = append(trackIPs, tsIP)
+		if e.isActiveSince(dk, tsIP, activeCutoff) {
+			min.Peers = append(min.Peers, *p)
+		}
+	}
+
+	if !deepprint.UpdateHash(&e.lastEngineSigTrim, min) {
+		// No changes
+		return nil
+	}
+
+	e.updateActivityMapsLocked(trackDisco, trackIPs)
+
+	e.logf("wgengine: Reconfig: configuring userspace wireguard config (with %d/%d peers)", len(min.Peers), len(full.Peers))
+	if err := e.wgdev.Reconfig(&min); err != nil {
+		e.logf("wgdev.Reconfig: %v", err)
+		return err
+	}
+	return nil
+}
+
+// updateActivityMapsLocked updates the data structures used for tracking the activity
+// of wireguard peers that we might add/remove dynamically from the real config
+// as given to wireguard-go.
+//
+// e.wgLock must be held.
+func (e *userspaceEngine) updateActivityMapsLocked(trackDisco []tailcfg.DiscoKey, trackIPs []wgcfg.IP) {
+	// Generate the new map of which discokeys we want to track
+	// receive times for.
+	mr := map[tailcfg.DiscoKey]time.Time{} // TODO: only recreate this if set of keys changed
+	for _, dk := range trackDisco {
+		// Preserve old times in the new map, but also
+		// populate map entries for new trackDisco values with
+		// time.Time{} zero values. (Only entries in this map
+		// are tracked, so the Time zero values allow it to be
+		// tracked later)
+		mr[dk] = e.recvActivityAt[dk]
+	}
+	e.recvActivityAt = mr
+
+	oldTime := e.sentActivityAt
+	e.sentActivityAt = make(map[packet.IP]*int64, len(oldTime))
+	oldFunc := e.destIPActivityFuncs
+	e.destIPActivityFuncs = make(map[packet.IP]func(), len(oldFunc))
+
+	for _, wip := range trackIPs {
+		pip := packet.IP(binary.BigEndian.Uint32(wip.Addr[12:]))
+		timePtr := oldTime[pip]
+		if timePtr == nil {
+			timePtr = new(int64)
+		}
+		e.sentActivityAt[pip] = timePtr
+
+		fn := oldFunc[pip]
+		if fn == nil {
+			// This is the func that gets run on every outgoing packet for tracked IPs:
+			fn = func() {
+				now, old := time.Now().Unix(), atomic.LoadInt64(timePtr)
+				if old > now-10 {
+					return
+				}
+				atomic.StoreInt64(timePtr, now)
+				if old == 0 || (now-old) <= 60 {
+					e.wgLock.Lock()
+					defer e.wgLock.Unlock()
+					e.maybeReconfigWireguardLocked()
+				}
+			}
+		}
+		e.destIPActivityFuncs[pip] = fn
+	}
+	e.tundev.SetDestIPActivityFuncs(e.destIPActivityFuncs)
 }
 
 func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config) error {
@@ -588,29 +814,24 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 		routerCfg.Domains = append([]string{magicDNSDomain}, routerCfg.Domains...)
 	}
 
-	engineChanged := updateSig(&e.lastEngineSig, cfg)
-	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
+	engineChanged := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
+	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}
-	e.lastCfg = cfg.Copy()
+	e.lastCfgFull = cfg.Copy()
 
-	if engineChanged {
-		e.logf("wgengine: Reconfig: configuring userspace wireguard config")
-		// Tell magicsock about the new (or initial) private key
-		// (which is needed by DERP) before wgdev gets it, as wgdev
-		// will start trying to handshake, which we want to be able to
-		// go over DERP.
-		if err := e.magicConn.SetPrivateKey(cfg.PrivateKey); err != nil {
-			e.logf("wgengine: Reconfig: SetPrivateKey: %v", err)
-		}
+	// Tell magicsock about the new (or initial) private key
+	// (which is needed by DERP) before wgdev gets it, as wgdev
+	// will start trying to handshake, which we want to be able to
+	// go over DERP.
+	if err := e.magicConn.SetPrivateKey(cfg.PrivateKey); err != nil {
+		e.logf("wgengine: Reconfig: SetPrivateKey: %v", err)
+	}
+	e.magicConn.UpdatePeers(peerSet)
 
-		if err := e.wgdev.Reconfig(cfg); err != nil {
-			e.logf("wgdev.Reconfig: %v", err)
-			return err
-		}
-
-		e.magicConn.UpdatePeers(peerSet)
+	if err := e.maybeReconfigWireguardLocked(); err != nil {
+		return err
 	}
 
 	if routerChanged {
@@ -758,15 +979,9 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 
 	var peers []PeerStatus
 	for _, pk := range e.peerSequence {
-		p := pp[pk]
-		if p == nil {
-			p = &PeerStatus{}
+		if p, ok := pp[pk]; ok { // ignore idle ones not in wireguard-go's config
+			peers = append(peers, *p)
 		}
-		peers = append(peers, *p)
-	}
-
-	if len(pp) != len(e.peerSequence) {
-		e.logf("wg status returned %v peers, expected %v", len(pp), len(e.peerSequence))
 	}
 
 	return &Status{

wgengine/watchdog.go

@@ -6,7 +6,9 @@ package wgengine
 
 import (
 	"log"
+	"os"
 	"runtime/pprof"
+	"strconv"
 	"strings"
 	"time"
 
@@ -24,6 +26,9 @@ import (
 //
 // If they do not, the watchdog crashes the process.
 func NewWatchdog(e Engine) Engine {
+	if v, _ := strconv.ParseBool(os.Getenv("TS_DEBUG_DISABLE_WATCHDOG")); v {
+		return e
+	}
 	return &watchdogEngine{
 		wrap:    e,
 		logf:    log.Printf,