cmd/cloner, tailcfg: fix nil vs len 0 issues, add tests, use for Hostinfo

Also use go:generate and https://golang.org/s/generatedcode header style.

cmd/tailscaled/tailscaled.go

@@ -149,16 +149,11 @@ func run() error {
 	ctx, cancel := context.WithCancel(context.Background())
 	// Exit gracefully by cancelling the ipnserver context in most common cases:
 	// interrupted from the TTY or killed by a service manager.
-	interrupt := make(chan os.Signal, 1)
-	signal.Notify(interrupt, syscall.SIGINT, syscall.SIGTERM)
-	// SIGPIPE sometimes gets generated when CLIs disconnect from
-	// tailscaled. The default action is to terminate the process, we
-	// want to keep running.
-	signal.Ignore(syscall.SIGPIPE)
 	go func() {
+		interrupt := make(chan os.Signal, 1)
+		signal.Notify(interrupt, syscall.SIGINT, syscall.SIGTERM)
 		select {
-		case s := <-interrupt:
-			logf("tailscaled got signal %v; shutting down", s)
+		case <-interrupt:
 			cancel()
 		case <-ctx.Done():
 			// continue
@@ -174,7 +169,7 @@ func run() error {
 		SurviveDisconnects: true,
 		DebugMux:           debugMux,
 	}
-	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), ipnserver.FixedEngine(e), opts)
+	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), opts, e)
 	// Cancelation is not an error: it is the only way to stop ipnserver.
 	if err != nil && err != context.Canceled {
 		logf("ipnserver.Run: %v", err)

cmd/tailscaled/tailscaled.service

@@ -3,6 +3,8 @@ Description=Tailscale node agent
 Documentation=https://tailscale.com/kb/
 Wants=network-pre.target
 After=network-pre.target
+StartLimitIntervalSec=0
+StartLimitBurst=0
 
 [Service]
 EnvironmentFile=/etc/default/tailscaled

control/controlclient/auto.go

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

control/controlclient/controlclient_test.go

@@ -70,10 +70,3 @@ 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,8 +4,6 @@
 
 package controlclient
 
-//go:generate go run tailscale.com/cmd/cloner -type=Persist -output=direct_clone.go
-
 import (
 	"bytes"
 	"context"
@@ -21,7 +19,6 @@ import (
 	"net/url"
 	"os"
 	"reflect"
-	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -30,7 +27,6 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"golang.org/x/crypto/nacl/box"
 	"golang.org/x/oauth2"
-	"inet.af/netaddr"
 	"tailscale.com/log/logheap"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/tlsdial"
@@ -169,20 +165,12 @@ 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,
 	}
 }
 
@@ -596,19 +584,17 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 		}
 		if resp.KeepAlive {
 			vlogf("netmap: got keep-alive")
-		} else {
-			vlogf("netmap: got new map")
-		}
-		select {
-		case timeoutReset <- struct{}{}:
-			vlogf("netmap: sent timer reset")
-		case <-ctx.Done():
-			c.logf("netmap: not resetting timer; context done: %v", ctx.Err())
-			return ctx.Err()
-		}
-		if resp.KeepAlive {
+			select {
+			case timeoutReset <- struct{}{}:
+				vlogf("netmap: sent keep-alive timer reset")
+			case <-ctx.Done():
+				c.logf("netmap: not resetting timer for keep-alive due to: %v", ctx.Err())
+				return ctx.Err()
+			}
 			continue
 		}
+		vlogf("netmap: got new map")
+
 		if resp.DERPMap != nil {
 			vlogf("netmap: new map contains DERP map")
 			lastDERPMap = resp.DERPMap
@@ -633,7 +619,6 @@ 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,
@@ -641,7 +626,8 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 			UserProfiles: make(map[tailcfg.UserID]tailcfg.UserProfile),
 			Domain:       resp.Domain,
 			Roles:        resp.Roles,
-			DNS:          resp.DNSConfig,
+			DNS:          resp.DNS,
+			DNSDomains:   resp.SearchPaths,
 			Hostinfo:     resp.Node.Hostinfo,
 			PacketFilter: c.parsePacketFilter(resp.PacketFilter),
 			DERPMap:      lastDERPMap,
@@ -655,15 +641,6 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 		} else {
 			nm.MachineStatus = tailcfg.MachineUnauthorized
 		}
-		if len(resp.DNS) > 0 {
-			nm.DNS.Nameservers = wgIPToNetaddr(resp.DNS)
-		}
-		if len(resp.SearchPaths) > 0 {
-			nm.DNS.Domains = resp.SearchPaths
-		}
-		if Debug.ProxyDNS {
-			nm.DNS.Proxied = true
-		}
 
 		// Printing the netmap can be extremely verbose, but is very
 		// handy for debugging. Let's limit how often we do it.
@@ -803,24 +780,12 @@ func loadServerKey(ctx context.Context, httpc *http.Client, serverURL string) (w
 	return key, nil
 }
 
-func wgIPToNetaddr(ips []wgcfg.IP) (ret []netaddr.IP) {
-	for _, ip := range ips {
-		nip, ok := netaddr.FromStdIP(ip.IP())
-		if !ok {
-			panic(fmt.Sprintf("conversion of %s from wgcfg to netaddr IP failed", ip))
-		}
-		ret = append(ret, nip.Unmap())
-	}
-	return ret
-}
-
 // Debug contains temporary internal-only debug knobs.
 // They're unexported to not draw attention to them.
 var Debug = initDebug()
 
 type debug struct {
 	NetMap     bool
-	ProxyDNS   bool
 	OnlyDisco  bool
 	Disco      bool
 	ForceDisco bool // ask control server to not filter out our disco key
@@ -829,7 +794,6 @@ type debug struct {
 func initDebug() debug {
 	d := debug{
 		NetMap:     envBool("TS_DEBUG_NETMAP"),
-		ProxyDNS:   envBool("TS_DEBUG_PROXY_DNS"),
 		OnlyDisco:  os.Getenv("TS_DEBUG_USE_DISCO") == "only",
 		ForceDisco: os.Getenv("TS_DEBUG_USE_DISCO") == "only" || envBool("TS_DEBUG_USE_DISCO"),
 	}

control/controlclient/direct_clone.go

@@ -1,20 +0,0 @@
-// 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

@@ -1,87 +0,0 @@
-// 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

@@ -1,26 +0,0 @@
-// 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

@@ -7,6 +7,7 @@ package controlclient
 import (
 	"encoding/json"
 	"fmt"
+	"log"
 	"net"
 	"reflect"
 	"strconv"
@@ -22,16 +23,15 @@ import (
 type NetworkMap struct {
 	// Core networking
 
-	NodeKey    tailcfg.NodeKey
-	PrivateKey wgcfg.PrivateKey
-	Expiry     time.Time
-	// Name is the DNS name assigned to this node.
-	Name          string
+	NodeKey       tailcfg.NodeKey
+	PrivateKey    wgcfg.PrivateKey
+	Expiry        time.Time
 	Addresses     []wgcfg.CIDR
 	LocalPort     uint16 // used for debugging
 	MachineStatus tailcfg.MachineStatus
 	Peers         []*tailcfg.Node // sorted by Node.ID
-	DNS           tailcfg.DNSConfig
+	DNS           []wgcfg.IP
+	DNSDomains    []string
 	Hostinfo      tailcfg.Hostinfo
 	PacketFilter  filter.Matches
 
@@ -131,18 +131,12 @@ func printPeerConcise(buf *strings.Builder, p *tailcfg.Node) {
 	if strings.HasPrefix(derp, derpPrefix) {
 		derp = "D" + derp[len(derpPrefix):]
 	}
-	var discoShort string
-	if !p.DiscoKey.IsZero() {
-		discoShort = p.DiscoKey.ShortString() + " "
-	}
 
 	// 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 %s%-2v %-15v : %v\n",
-		p.Key.ShortString(),
-		discoShort,
-		derp,
+	fmt.Fprintf(buf, " %v %-2v %-15v : %v\n",
+		p.Key.ShortString(), derp,
 		strings.Join(aip, " "),
 		strings.Join(ep, " "))
 }
@@ -151,7 +145,6 @@ func printPeerConcise(buf *strings.Builder, p *tailcfg.Node) {
 func nodeConciseEqual(a, b *tailcfg.Node) bool {
 	return a.Key == b.Key &&
 		a.DERP == b.DERP &&
-		a.DiscoKey == b.DiscoKey &&
 		eqCIDRsIgnoreNil(a.AllowedIPs, b.AllowedIPs) &&
 		eqStringsIgnoreNil(a.Endpoints, b.Endpoints)
 }
@@ -222,18 +215,33 @@ const (
 	HackDefaultRoute
 )
 
+// 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(flags WGConfigFlags, dnsOverride []wgcfg.IP) string {
+	wgcfg, err := nm.WGCfg(log.Printf, flags, dnsOverride)
+	if err != nil {
+		log.Fatalf("WGCfg() failed unexpectedly: %v", err)
+	}
+	s, err := wgcfg.ToUAPI()
+	if err != nil {
+		log.Fatalf("ToUAPI() failed unexpectedly: %v", err)
+	}
+	return s
+}
+
 // 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"
 
 // WGCfg returns the NetworkMaps's Wireguard configuration.
-func (nm *NetworkMap) WGCfg(logf logger.Logf, flags WGConfigFlags) (*wgcfg.Config, error) {
+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)),
 	}
 

control/controlclient/netmap_test.go

@@ -5,10 +5,8 @@
 package controlclient
 
 import (
-	"encoding/hex"
 	"testing"
 
-	"github.com/tailscale/wireguard-go/wgcfg"
 	"tailscale.com/tailcfg"
 )
 
@@ -19,15 +17,6 @@ func testNodeKey(b byte) (ret tailcfg.NodeKey) {
 	return
 }
 
-func testDiscoKey(hexPrefix string) (ret tailcfg.DiscoKey) {
-	b, err := hex.DecodeString(hexPrefix)
-	if err != nil {
-		panic(err)
-	}
-	copy(ret[:], b)
-	return
-}
-
 func TestNetworkMapConcise(t *testing.T) {
 	for _, tt := range []struct {
 		name string
@@ -213,62 +202,6 @@ func TestConciseDiffFrom(t *testing.T) {
 			},
 			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_port_change",
-			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", "1.1.1.1:1"},
-					},
-				},
-			},
-			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", "1.1.1.1:2"},
-					},
-				},
-			},
-			want: "- [AgICA] D2                 :    192.168.0.100:12             1.1.1.1:1  \n+ [AgICA] D2                 :    192.168.0.100:12             1.1.1.1:2  \n",
-		},
-		{
-			name: "disco_key_only_change",
-			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:41641", "1.1.1.1:41641"},
-						DiscoKey:   testDiscoKey("f00f00f00f"),
-						AllowedIPs: []wgcfg.CIDR{{IP: wgcfg.IPv4(100, 102, 103, 104), Mask: 32}},
-					},
-				},
-			},
-			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:41641", "1.1.1.1:41641"},
-						DiscoKey:   testDiscoKey("ba4ba4ba4b"),
-						AllowedIPs: []wgcfg.CIDR{{IP: wgcfg.IPv4(100, 102, 103, 104), Mask: 32}},
-					},
-				},
-			},
-			want: "- [AgICA] d:f00f00f00f000000 D2 100.102.103.104 :   192.168.0.100:41641         1.1.1.1:41641\n+ [AgICA] d:ba4ba4ba4b000000 D2 100.102.103.104 :   192.168.0.100:41641         1.1.1.1:41641\n",
-		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			var got string

derp/derp_server.go

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

go.sum

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

internal/deepprint/deepprint.go

@@ -18,23 +18,13 @@ 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))
 }
 
-// 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{}) {
+func Print(w io.Writer, v interface{}) {
 	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
 }
 

internal/deepprint/deepprint_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"inet.af/netaddr"
 	"tailscale.com/wgengine/router"
-	"tailscale.com/wgengine/router/dns"
 )
 
 func TestDeepPrint(t *testing.T) {
@@ -51,7 +50,7 @@ func getVal() []interface{} {
 			},
 		},
 		&router.Config{
-			DNS: dns.Config{
+			DNSConfig: router.DNSConfig{
 				Nameservers: []netaddr.IP{netaddr.IPv4(8, 8, 8, 8)},
 				Domains:     []string{"tailscale.net"},
 			},

ipn/ipnserver/server.go

@@ -69,6 +69,10 @@ 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
@@ -148,9 +152,7 @@ func (s *server) writeToClients(b []byte) {
 	}
 }
 
-// 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 {
+func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wgengine.Engine) error {
 	runDone := make(chan struct{})
 	defer close(runDone)
 
@@ -177,38 +179,25 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 
 	bo := backoff.NewBackoff("ipnserver", logf)
 
-	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)
+	if opts.ErrorMessage != "" {
 		for i := 1; ctx.Err() == nil; i++ {
-			c, err := listen.Accept()
+			s, err := listen.Accept()
 			if err != nil {
 				logf("%d: Accept: %v", i, err)
 				bo.BackOff(ctx, err)
 				continue
 			}
-			logf("%d: trying getEngine again...", i)
-			eng, err = getEngine()
-			if err == nil {
-				logf("%d: GetEngine worked; exiting failure loop", i)
-				unservedConn = c
-				break
+			serverToClient := func(b []byte) {
+				ipn.WriteMsg(s, b)
 			}
-			logf("%d: getEngine failed again: %v", i, err)
-			errMsg := err.Error()
 			go func() {
-				defer c.Close()
-				serverToClient := func(b []byte) { ipn.WriteMsg(c, b) }
+				defer s.Close()
 				bs := ipn.NewBackendServer(logf, nil, serverToClient)
-				bs.SendErrorMessage(errMsg)
-				time.Sleep(time.Second)
+				bs.SendErrorMessage(opts.ErrorMessage)
+				s.Read(make([]byte, 1))
 			}()
 		}
-		if err := ctx.Err(); err != nil {
-			return err
-		}
+		return ctx.Err()
 	}
 
 	var store ipn.StateStore
@@ -221,7 +210,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 		store = &ipn.MemoryStore{}
 	}
 
-	b, err := ipn.NewLocalBackend(logf, logid, store, eng)
+	b, err := ipn.NewLocalBackend(logf, logid, store, e)
 	if err != nil {
 		return fmt.Errorf("NewLocalBackend: %v", err)
 	}
@@ -254,14 +243,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 	}
 
 	for i := 1; ctx.Err() == nil; i++ {
-		var c net.Conn
-		var err error
-		if unservedConn != nil {
-			c = unservedConn
-			unservedConn = nil
-		} else {
-			c, err = listen.Accept()
-		}
+		c, err := listen.Accept()
 		if err != nil {
 			if ctx.Err() == nil {
 				logf("ipnserver: Accept: %v", err)
@@ -389,8 +371,3 @@ 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", ipnserver.FixedEngine(eng), opts)
+	err = ipnserver.Run(ctx, logTriggerTestf, "dummy_logid", opts, eng)
 	t.Logf("ipnserver.Run = %v", err)
 }

ipn/ipnstate/ipnstate.go

@@ -18,7 +18,6 @@ import (
 	"sync"
 	"time"
 
-	"inet.af/netaddr"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 )
@@ -26,7 +25,6 @@ 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
 }
@@ -111,18 +109,6 @@ 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.
@@ -232,12 +218,6 @@ 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,10 +16,8 @@ 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/net/tsaddr"
 	"tailscale.com/portlist"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/empty"
@@ -29,7 +27,6 @@ import (
 	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/router"
-	"tailscale.com/wgengine/router/dns"
 	"tailscale.com/wgengine/tsdns"
 )
 
@@ -54,10 +51,12 @@ type LocalBackend struct {
 	backendLogID    string
 	portpoll        *portlist.Poller // may be nil
 	portpollOnce    sync.Once
-	serverURL       string // tailcontrol URL
 	newDecompressor func() (controlclient.Decompressor, error)
 
-	filterHash string
+	// TODO: these fields are accessed unsafely by concurrent
+	// goroutines. They need to be protected.
+	serverURL       string // tailcontrol URL
+	lastFilterPrint time.Time
 
 	// The mutex protects the following elements.
 	mu       sync.Mutex
@@ -187,56 +186,21 @@ 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 {
-		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()
-	}
+		persist := *st.Persist // copy
 
-	b.mu.Unlock()
+		b.mu.Lock()
+		b.prefs.Persist = &persist
+		prefs := b.prefs.Clone()
+		stateKey := b.stateKey
+		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)
@@ -245,41 +209,63 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		b.send(Notify{Prefs: prefs})
 	}
 	if st.NetMap != nil {
-		if netMap != nil {
-			diff := st.NetMap.ConciseDiffFrom(netMap)
+		// 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 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.updateFilter(st.NetMap, prefs)
-		b.e.SetNetworkMap(st.NetMap)
-
-		if !dnsMapsEqual(st.NetMap, netMap) {
+		b.send(Notify{NetMap: st.NetMap})
+		// There is nothing to update if the map hasn't changed.
+		if changed {
+			b.updateFilter(st.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.
@@ -374,7 +360,7 @@ func (b *LocalBackend) Start(opts Options) error {
 	persist := b.prefs.Persist
 	b.mu.Unlock()
 
-	b.updateFilter(nil, nil)
+	b.updateFilter(nil)
 
 	var discoPublic tailcfg.DiscoKey
 	if controlclient.Debug.Disco {
@@ -438,121 +424,65 @@ 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, prefs *Prefs) {
-	// NOTE(danderson): keep change detection as the first thing in
-	// this function. Don't try to optimize by returning early, more
-	// likely than not you'll just end up breaking the change
-	// detection and end up with the wrong filter installed. This is
-	// quite hard to debug, so save yourself the trouble.
-	var (
-		haveNetmap   = netMap != nil
-		addrs        []wgcfg.CIDR
-		packetFilter filter.Matches
-		advRoutes    []wgcfg.CIDR
-		shieldsUp    = prefs == nil || prefs.ShieldsUp // Be conservative when not ready
-	)
-	if haveNetmap {
-		addrs = netMap.Addresses
-		packetFilter = netMap.PacketFilter
-	}
-	if prefs != nil {
-		advRoutes = prefs.AdvertiseRoutes
-	}
-
-	changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, advRoutes, shieldsUp)
-	if !changed {
-		return
-	}
-
-	if !haveNetmap {
+func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
+	if netMap == nil {
+		// Not configured yet, block everything
 		b.logf("netmap packet filter: (not ready yet)")
 		b.e.SetFilter(filter.NewAllowNone(b.logf))
 		return
 	}
 
+	b.mu.Lock()
+	advRoutes := b.prefs.AdvertiseRoutes
+	b.mu.Unlock()
 	localNets := wgCIDRsToFilter(netMap.Addresses, advRoutes)
 
-	if shieldsUp {
+	if b.shieldsAreUp() {
+		// Shields up, block everything
 		b.logf("netmap packet filter: (shields up)")
 		var prevFilter *filter.Filter // don't reuse old filter state
 		b.e.SetFilter(filter.New(filter.Matches{}, localNets, prevFilter, b.logf))
+		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: %v", packetFilter)
-		b.e.SetFilter(filter.New(packetFilter, localNets, b.e.GetFilter(), b.logf))
+		b.logf("netmap packet filter: (length %d)", len(netMap.PacketFilter))
 	}
-}
-
-// 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
-	}
-	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
+	b.e.SetFilter(filter.New(netMap.PacketFilter, localNets, b.e.GetFilter(), b.logf))
 }
 
 // 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
 	}
 
-	nameToIP := make(map[string]netaddr.IP)
-	set := func(name string, addrs []wgcfg.CIDR) {
+	domainToIP := make(map[string]netaddr.IP)
+	set := func(hostname string, addrs []wgcfg.CIDR) {
 		if len(addrs) == 0 {
 			return
 		}
-		nameToIP[name] = netaddr.IPFrom16(addrs[0].IP.Addr)
+		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)
 	}
 
 	for _, peer := range netMap.Peers {
-		set(peer.Name, peer.Addresses)
+		set(peer.Hostinfo.Hostname, peer.Addresses)
 	}
-	set(netMap.Name, netMap.Addresses)
+	set(netMap.Hostinfo.Hostname, netMap.Addresses)
 
-	dnsMap := tsdns.NewMap(nameToIP)
-	// map diff will be logged in tsdns.Resolver.SetMap.
-	b.e.SetDNSMap(dnsMap)
+	b.e.SetDNSMap(tsdns.NewMap(domainToIP))
 }
 
 // readPoller is a goroutine that receives service lists from
@@ -791,45 +721,37 @@ 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
-	// We do this to avoid holding the lock while doing everything else.
-	new = b.prefs.Clone()
-
+	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)
+		}
+	}
 	oldHi := b.hostinfo
 	newHi := oldHi.Clone()
 	newHi.RoutableIPs = append([]wgcfg.CIDR(nil), b.prefs.AdvertiseRoutes...)
 	applyPrefsToHostinfo(newHi, new)
 	b.hostinfo = newHi
 	hostInfoChanged := !oldHi.Equal(newHi)
-
 	b.mu.Unlock()
 
-	if stateKey != "" {
-		if err := b.store.WriteState(stateKey, new.ToBytes()); err != nil {
-			b.logf("Failed to save new controlclient state: %v", err)
-		}
-	}
-
 	b.logf("SetPrefs: %v", new.Pretty())
 
 	if old.ShieldsUp != new.ShieldsUp || hostInfoChanged {
 		b.doSetHostinfoFilterServices(newHi)
 	}
 
-	b.updateFilter(netMap, new)
+	b.updateFilter(b.netMap)
+	// TODO(dmytro): when Prefs gain an EnableTailscaleDNS toggle, updateDNSMap here.
 
 	turnDERPOff := new.DisableDERP && !old.DisableDERP
 	turnDERPOn := !new.DisableDERP && old.DisableDERP
 	if turnDERPOff {
 		b.e.SetDERPMap(nil)
-	} else if turnDERPOn && netMap != nil {
-		b.e.SetDERPMap(netMap.DERPMap)
+	} else if turnDERPOn && b.netMap != nil {
+		b.e.SetDERPMap(b.netMap.DERPMap)
 	}
 
 	if old.WantRunning != new.WantRunning {
@@ -922,71 +844,28 @@ func (b *LocalBackend) authReconfig() {
 		flags |= controlclient.AllowSingleHosts
 	}
 
-	cfg, err := nm.WGCfg(b.logf, flags)
+	dns := nm.DNS
+	dom := nm.DNSDomains
+	if !uc.CorpDNS {
+		dns = []wgcfg.IP{}
+		dom = []string{}
+	}
+	cfg, err := nm.WGCfg(b.logf, flags, dns)
 	if err != nil {
 		b.logf("wgcfg: %v", err)
 		return
 	}
 
-	rcfg := routerConfig(cfg, uc)
-
-	// If CorpDNS is false, rcfg.DNS remains the zero value.
-	if uc.CorpDNS {
-		domains := nm.DNS.Domains
-		proxied := nm.DNS.Proxied
-		if proxied {
-			if len(nm.DNS.Nameservers) == 0 {
-				b.logf("[unexpected] dns proxied but no nameservers")
-				proxied = false
-			} else {
-				domains = append(domains, domainsForProxying(nm)...)
-			}
-		}
-		rcfg.DNS = dns.Config{
-			Nameservers: nm.DNS.Nameservers,
-			Domains:     domains,
-			PerDomain:   nm.DNS.PerDomain,
-			Proxied:     proxied,
-		}
-	}
-
-	err = b.e.Reconfig(cfg, rcfg)
+	err = b.e.Reconfig(cfg, routerConfig(cfg, uc, dom))
 	if err == wgengine.ErrNoChanges {
 		return
 	}
 	b.logf("authReconfig: ra=%v dns=%v 0x%02x: %v", uc.RouteAll, uc.CorpDNS, flags, err)
 }
 
-// domainsForProxying produces a list of search domains for proxied DNS.
-func domainsForProxying(nm *controlclient.NetworkMap) []string {
-	var domains []string
-	if idx := strings.IndexByte(nm.Name, '.'); idx != -1 {
-		domains = append(domains, nm.Name[idx+1:])
-	}
-	for _, peer := range nm.Peers {
-		idx := strings.IndexByte(peer.Name, '.')
-		if idx == -1 {
-			continue
-		}
-		domain := peer.Name[idx+1:]
-		seen := false
-		// In theory this makes the function O(n^2) worst case,
-		// but in practice we expect domains to contain very few elements
-		// (only one until invitations are introduced).
-		for _, seenDomain := range domains {
-			if domain == seenDomain {
-				seen = true
-			}
-		}
-		if !seen {
-			domains = append(domains, domain)
-		}
-	}
-	return domains
-}
-
-// routerConfig produces a router.Config from a wireguard config and IPN prefs.
-func routerConfig(cfg *wgcfg.Config, prefs *Prefs) *router.Config {
+// routerConfig produces a router.Config from a wireguard config,
+// IPN prefs, and the dnsDomains pulled from control's network map.
+func routerConfig(cfg *wgcfg.Config, prefs *Prefs, dnsDomains []string) *router.Config {
 	var addrs []wgcfg.CIDR
 	for _, addr := range cfg.Addresses {
 		addrs = append(addrs, wgcfg.CIDR{
@@ -1000,14 +879,20 @@ func routerConfig(cfg *wgcfg.Config, prefs *Prefs) *router.Config {
 		SubnetRoutes:     wgCIDRToNetaddr(prefs.AdvertiseRoutes),
 		SNATSubnetRoutes: !prefs.NoSNAT,
 		NetfilterMode:    prefs.NetfilterMode,
+		DNSConfig: router.DNSConfig{
+			Nameservers: wgIPToNetaddr(cfg.DNS),
+			Domains:     dnsDomains,
+		},
 	}
 
 	for _, peer := range cfg.Peers {
 		rs.Routes = append(rs.Routes, wgCIDRToNetaddr(peer.AllowedIPs)...)
 	}
 
+	// The Tailscale DNS IP.
+	// TODO(dmytro): make this configurable.
 	rs.Routes = append(rs.Routes, netaddr.IPPrefix{
-		IP:   tsaddr.TailscaleServiceIP(),
+		IP:   netaddr.IPv4(100, 100, 100, 100),
 		Bits: 32,
 	})
 
@@ -1031,6 +916,17 @@ func wgCIDRsToFilter(cidrLists ...[]wgcfg.CIDR) (ret []filter.Net) {
 	return ret
 }
 
+func wgIPToNetaddr(ips []wgcfg.IP) (ret []netaddr.IP) {
+	for _, ip := range ips {
+		nip, ok := netaddr.FromStdIP(ip.IP())
+		if !ok {
+			panic(fmt.Sprintf("conversion of %s from wgcfg to netaddr IP failed", ip))
+		}
+		ret = append(ret, nip.Unmap())
+	}
+	return ret
+}
+
 func wgCIDRToNetaddr(cidrs []wgcfg.CIDR) (ret []netaddr.IPPrefix) {
 	for _, cidr := range cidrs {
 		ncidr, ok := netaddr.FromStdIPNet(cidr.IPNet())

logpolicy/logpolicy.go

@@ -31,9 +31,7 @@ import (
 	"tailscale.com/logtail/filch"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/tlsdial"
-	"tailscale.com/paths"
 	"tailscale.com/smallzstd"
-	"tailscale.com/types/logger"
 	"tailscale.com/version"
 )
 
@@ -56,7 +54,7 @@ type Policy struct {
 func (c *Config) ToBytes() []byte {
 	data, err := json.MarshalIndent(c, "", "\t")
 	if err != nil {
-		log.Fatalf("logpolicy.Config marshal: %v", err)
+		log.Fatalf("logpolicy.Config marshal: %v\n", err)
 	}
 	return data
 }
@@ -103,36 +101,21 @@ func (l logWriter) Write(buf []byte) (int, error) {
 
 // logsDir returns the directory to use for log configuration and
 // buffer storage.
-func logsDir(logf logger.Logf) string {
-	// STATE_DIRECTORY is set by systemd 240+ but we support older
-	// systems-d. For example, Ubuntu 18.04 (Bionic Beaver) is 237.
+func logsDir() string {
 	systemdStateDir := os.Getenv("STATE_DIRECTORY")
 	if systemdStateDir != "" {
-		logf("logpolicy: using $STATE_DIRECTORY, %q", systemdStateDir)
 		return systemdStateDir
 	}
 
-	// Default to e.g. /var/lib/tailscale or /var/db/tailscale on Unix.
-	if d := paths.DefaultTailscaledStateFile(); d != "" {
-		d = filepath.Dir(d) // directory of e.g. "/var/lib/tailscale/tailscaled.state"
-		if err := os.MkdirAll(d, 0700); err == nil {
-			logf("logpolicy: using system state directory %q", d)
-			return d
-		}
-	}
-
 	cacheDir, err := os.UserCacheDir()
 	if err == nil {
-		d := filepath.Join(cacheDir, "Tailscale")
-		logf("logpolicy: using UserCacheDir, %q", d)
-		return d
+		return filepath.Join(cacheDir, "Tailscale")
 	}
 
 	// Use the current working directory, unless we're being run by a
 	// service manager that sets it to /.
 	wd, err := os.Getwd()
 	if err == nil && wd != "/" {
-		logf("logpolicy: using current directory, %q", wd)
 		return wd
 	}
 
@@ -143,7 +126,6 @@ func logsDir(logf logger.Logf) string {
 	if err != nil {
 		panic("no safe place found to store log state")
 	}
-	logf("logpolicy: using temp directory, %q", tmp)
 	return tmp
 }
 
@@ -167,14 +149,6 @@ func runningUnderSystemd() bool {
 // moved from whereever it does exist, into dir. Leftover logs state
 // in / and $CACHE_DIRECTORY is deleted.
 func tryFixLogStateLocation(dir, cmdname string) {
-	switch runtime.GOOS {
-	case "linux", "freebsd", "openbsd":
-		// These are the OSes where we might have written stuff into
-		// root. Others use different logic to find the logs storage
-		// dir.
-	default:
-		return
-	}
 	if cmdname == "" {
 		log.Printf("[unexpected] no cmdname given to tryFixLogStateLocation, please file a bug at https://github.com/tailscale/tailscale")
 		return
@@ -189,6 +163,14 @@ func tryFixLogStateLocation(dir, cmdname string) {
 		// Only root could have written log configs to weird places.
 		return
 	}
+	switch runtime.GOOS {
+	case "linux", "freebsd", "openbsd":
+		// These are the OSes where we might have written stuff into
+		// root. Others use different logic to find the logs storage
+		// dir.
+	default:
+		return
+	}
 
 	// We stored logs in 2 incorrect places: either /, or CACHE_DIR
 	// (aka /var/cache/tailscale). We want to move files into the
@@ -321,28 +303,23 @@ func New(collection string) *Policy {
 	}
 	console := log.New(stderrWriter{}, "", lflags)
 
-	var earlyErrBuf bytes.Buffer
-	earlyLogf := func(format string, a ...interface{}) {
-		fmt.Fprintf(&earlyErrBuf, format, a...)
-		earlyErrBuf.WriteByte('\n')
+	dir := logsDir()
+
+	if runtime.GOOS != "windows" { // version.CmdName call was blowing some Windows stack limit via goversion DLL loading
+		tryFixLogStateLocation(dir, version.CmdName())
 	}
 
-	dir := logsDir(earlyLogf)
-
-	cmdName := version.CmdName()
-	tryFixLogStateLocation(dir, cmdName)
-
-	cfgPath := filepath.Join(dir, fmt.Sprintf("%s.log.conf", cmdName))
+	cfgPath := filepath.Join(dir, fmt.Sprintf("%s.log.conf", version.CmdName()))
 	var oldc *Config
 	data, err := ioutil.ReadFile(cfgPath)
 	if err != nil {
-		earlyLogf("logpolicy.Read %v: %v", cfgPath, err)
+		log.Printf("logpolicy.Read %v: %v\n", cfgPath, err)
 		oldc = &Config{}
 		oldc.Collection = collection
 	} else {
 		oldc, err = ConfigFromBytes(data)
 		if err != nil {
-			earlyLogf("logpolicy.Config unmarshal: %v", err)
+			log.Printf("logpolicy.Config unmarshal: %v\n", err)
 			oldc = &Config{}
 		}
 	}
@@ -364,7 +341,7 @@ func New(collection string) *Policy {
 	newc.PublicID = newc.PrivateID.Public()
 	if newc != *oldc {
 		if err := newc.save(cfgPath); err != nil {
-			earlyLogf("logpolicy.Config.Save: %v", err)
+			log.Printf("logpolicy.Config.Save: %v\n", err)
 		}
 	}
 
@@ -382,7 +359,7 @@ func New(collection string) *Policy {
 		HTTPC: &http.Client{Transport: newLogtailTransport(logtail.DefaultHost)},
 	}
 
-	filchBuf, filchErr := filch.New(filepath.Join(dir, cmdName), filch.Options{})
+	filchBuf, filchErr := filch.New(filepath.Join(dir, version.CmdName()), filch.Options{})
 	if filchBuf != nil {
 		c.Buffer = filchBuf
 	}
@@ -390,17 +367,14 @@ func New(collection string) *Policy {
 	log.SetFlags(0) // other logflags are set on console, not here
 	log.SetOutput(lw)
 
-	log.Printf("Program starting: v%v, Go %v: %#v",
+	log.Printf("Program starting: v%v, Go %v: %#v\n",
 		version.LONG,
 		strings.TrimPrefix(runtime.Version(), "go"),
 		os.Args)
-	log.Printf("LogID: %v", newc.PublicID)
+	log.Printf("LogID: %v\n", newc.PublicID)
 	if filchErr != nil {
 		log.Printf("filch failed: %v", filchErr)
 	}
-	if earlyErrBuf.Len() != 0 {
-		log.Printf("%s", earlyErrBuf.Bytes())
-	}
 
 	return &Policy{
 		Logtail:  lw,
@@ -419,7 +393,7 @@ func (p *Policy) Close() {
 // log upload if it can be done before ctx is canceled.
 func (p *Policy) Shutdown(ctx context.Context) error {
 	if p.Logtail != nil {
-		log.Printf("flushing log.")
+		log.Printf("flushing log.\n")
 		return p.Logtail.Shutdown(ctx)
 	}
 	return nil

logtail/logtail.go

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

logtail/logtail_test.go

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

net/interfaces/interfaces_darwin.go

@@ -10,7 +10,6 @@ import (
 	"go4.org/mem"
 	"inet.af/netaddr"
 	"tailscale.com/util/lineread"
-	"tailscale.com/version"
 )
 
 func init() {
@@ -33,13 +32,6 @@ default            link#14            UCSI         utun2
 
 */
 func likelyHomeRouterIPDarwin() (ret netaddr.IP, ok bool) {
-	if version.IsMobile() {
-		// Don't try to do subprocesses on iOS. Ends up with log spam like:
-		// kernel: "Sandbox: IPNExtension(86580) deny(1) process-fork"
-		// TODO(bradfitz): let our iOS app register a func with this package
-		// and have it call into C/Swift to get the routing table.
-		return ret, false
-	}
 	cmd := exec.Command("/usr/sbin/netstat", "-r", "-n", "-f", "inet")
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {

net/interfaces/interfaces_linux.go

@@ -5,14 +5,8 @@
 package interfaces
 
 import (
-	"bytes"
-	"log"
-	"os/exec"
-	"runtime"
-
 	"go4.org/mem"
 	"inet.af/netaddr"
-	"tailscale.com/syncs"
 	"tailscale.com/util/lineread"
 )
 
@@ -20,8 +14,6 @@ func init() {
 	likelyHomeRouterIP = likelyHomeRouterIPLinux
 }
 
-var procNetRouteErr syncs.AtomicBool
-
 /*
 Parse 10.0.0.1 out of:
 
@@ -31,17 +23,9 @@ 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
-	err := lineread.File("/proc/net/route", func(line []byte) error {
+	lineread.File("/proc/net/route", func(line []byte) error {
 		lineNum++
 		if lineNum == 1 {
 			// Skip header line.
@@ -71,47 +55,5 @@ 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,9 +18,7 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"os"
 	"sort"
-	"strconv"
 	"sync"
 	"time"
 
@@ -38,45 +36,6 @@ 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
@@ -180,7 +139,7 @@ func (c *Client) logf(format string, a ...interface{}) {
 }
 
 func (c *Client) vlogf(format string, a ...interface{}) {
-	if c.Verbose || debugNetcheck {
+	if c.Verbose {
 		c.logf(format, a...)
 	}
 }
@@ -211,8 +170,6 @@ 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()
@@ -373,7 +330,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 = defaultActiveRetransmitTime
+				prevLatency = 200 * time.Millisecond
 			}
 			delay := time.Duration(try) * prevLatency
 			if do4 {
@@ -396,12 +353,16 @@ 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) * defaultInitialRetransmitTime
+			delay := time.Duration(try) * initialSTUNTimeout
 			if ifState.HaveV4 && nodeMight4(n) {
 				p4 = append(p4, probe{delay: delay, node: n.Name, proto: probeIPv4})
 			}
@@ -557,7 +518,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(hairpinCheckTimeout, func() { close(rs.hairTimeout) })
+	time.AfterFunc(500*time.Millisecond, func() { close(rs.hairTimeout) })
 }
 
 func (rs *reportState) waitHairCheck(ctx context.Context) {
@@ -578,7 +539,6 @@ 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 {
@@ -689,7 +649,7 @@ func (rs *reportState) probePortMapServices() {
 	}
 	defer uc.Close()
 	tempPort := uc.LocalAddr().(*net.UDPAddr).Port
-	uc.SetReadDeadline(time.Now().Add(portMapServiceProbeTimeout))
+	uc.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
 
 	// Send request packets for all three protocols.
 	uc.WriteTo(uPnPPacket, port1900)
@@ -767,10 +727,15 @@ 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, overallProbeTimeout)
+	ctx, cancel := context.WithTimeout(ctx, overallTimeout)
 	defer cancel()
 
 	if dm == nil {
@@ -879,7 +844,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
 		}(probeSet)
 	}
 
-	stunTimer := time.NewTimer(stunProbeTimeout)
+	stunTimer := time.NewTimer(stunTimeout)
 	defer stunTimer.Stop()
 
 	select {
@@ -892,9 +857,7 @@ 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.
@@ -949,7 +912,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), overallProbeTimeout)
+	ctx, cancel := context.WithTimeout(httpstat.WithHTTPStat(ctx, &result), 5*time.Second)
 	defer cancel()
 
 	var ip netaddr.IP

net/tsaddr/tsaddr.go

@@ -32,15 +32,6 @@ func CGNATRange() netaddr.IPPrefix {
 
 var cgnatRange oncePrefix
 
-// TailscaleServiceIP returns the listen address of services
-// provided by Tailscale itself such as the Magic DNS proxy.
-func TailscaleServiceIP() netaddr.IP {
-	serviceIP.Do(func() { mustIP(&serviceIP.v, "100.100.100.100") })
-	return serviceIP.v
-}
-
-var serviceIP onceIP
-
 // IsTailscaleIP reports whether ip is an IP address in a range that
 // Tailscale assigns from.
 func IsTailscaleIP(ip netaddr.IP) bool {
@@ -59,16 +50,3 @@ type oncePrefix struct {
 	sync.Once
 	v netaddr.IPPrefix
 }
-
-func mustIP(v *netaddr.IP, ip string) {
-	var err error
-	*v, err = netaddr.ParseIP(ip)
-	if err != nil {
-		panic(err)
-	}
-}
-
-type onceIP struct {
-	sync.Once
-	v netaddr.IP
-}

portlist/portlist_linux.go

@@ -10,15 +10,12 @@ 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.
@@ -29,30 +26,13 @@ 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)
 		}
@@ -116,18 +96,7 @@ func addProcesses(pl []Port) ([]Port, error) {
 	}
 
 	err := foreachPID(func(pid string) error {
-		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)
+		fdDir, err := os.Open(fmt.Sprintf("/proc/%s/fd", pid))
 		if err != nil {
 			// Can't open fd list for this pid. Maybe
 			// don't have access. Ignore it.

tailcfg/tailcfg.go

@@ -17,7 +17,6 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"go4.org/mem"
 	"golang.org/x/oauth2"
-	"inet.af/netaddr"
 	"tailscale.com/types/key"
 	"tailscale.com/types/opt"
 	"tailscale.com/types/structs"
@@ -261,7 +260,6 @@ type Hostinfo struct {
 	OSVersion     string       // operating system version, with optional distro prefix ("Debian 10.4", "Windows 10 Pro 10.0.19041")
 	DeviceModel   string       // mobile phone model ("Pixel 3a", "iPhone 11 Pro")
 	Hostname      string       // name of the host the client runs on
-	GoArch        string       // the host's GOARCH value (of the running binary)
 	RoutableIPs   []wgcfg.CIDR `json:",omitempty"` // set of IP ranges this client can route
 	RequestTags   []string     `json:",omitempty"` // set of ACL tags this node wants to claim
 	Services      []Service    `json:",omitempty"` // services advertised by this machine
@@ -493,31 +491,15 @@ var FilterAllowAll = []FilterRule{
 	},
 }
 
-// DNSConfig is the DNS configuration.
-type DNSConfig struct {
-	Nameservers []netaddr.IP `json:",omitempty"`
-	Domains     []string     `json:",omitempty"`
-	PerDomain   bool
-	Proxied     bool
-}
-
 type MapResponse struct {
 	KeepAlive bool // if set, all other fields are ignored
 
 	// Networking
-	Node    *Node
-	Peers   []*Node
-	DERPMap *DERPMap
-
-	// DNS is the same as DNSConfig.Nameservers.
-	//
-	// TODO(dmytro): should be sent in DNSConfig.Nameservers once clients have updated.
-	DNS []wgcfg.IP
-	// SearchPaths are the same as DNSConfig.Domains.
-	//
-	// TODO(dmytro): should be sent in DNSConfig.Domains once clients have updated.
+	Node        *Node
+	Peers       []*Node
+	DNS         []wgcfg.IP
 	SearchPaths []string
-	DNSConfig   DNSConfig
+	DERPMap     *DERPMap
 
 	// ACLs
 	Domain       string
@@ -613,7 +595,6 @@ func (n *Node) Equal(n2 *Node) bool {
 		eqCIDRs(n.Addresses, n2.Addresses) &&
 		eqCIDRs(n.AllowedIPs, n2.AllowedIPs) &&
 		eqStrings(n.Endpoints, n2.Endpoints) &&
-		n.DERP == n2.DERP &&
 		n.Hostinfo.Equal(&n2.Hostinfo) &&
 		n.Created.Equal(n2.Created) &&
 		eqTimePtr(n.LastSeen, n2.LastSeen) &&

tailcfg/tailcfg_test.go

@@ -24,7 +24,7 @@ func fieldsOf(t reflect.Type) (fields []string) {
 func TestHostinfoEqual(t *testing.T) {
 	hiHandles := []string{
 		"IPNVersion", "FrontendLogID", "BackendLogID", "OS", "OSVersion",
-		"DeviceModel", "Hostname", "GoArch", "RoutableIPs", "RequestTags", "Services",
+		"DeviceModel", "Hostname", "RoutableIPs", "RequestTags", "Services",
 		"NetInfo",
 	}
 	if have := fieldsOf(reflect.TypeOf(Hostinfo{})); !reflect.DeepEqual(have, hiHandles) {
@@ -315,11 +315,6 @@ func TestNodeEqual(t *testing.T) {
 			&Node{LastSeen: &now},
 			true,
 		},
-		{
-			&Node{DERP: "foo"},
-			&Node{DERP: "bar"},
-			false,
-		},
 	}
 	for i, tt := range tests {
 		got := tt.a.Equal(tt.b)

tstest/natlab/natlab.go

@@ -93,10 +93,9 @@ func mustPrefix(s string) netaddr.IPPrefix {
 // NewInternet returns a network that simulates the internet.
 func NewInternet() *Network {
 	return &Network{
-		Name: "internet",
-		// easily recognizable internett-y addresses
-		Prefix4: mustPrefix("1.0.0.0/24"),
-		Prefix6: mustPrefix("1111::/64"),
+		Name:    "internet",
+		Prefix4: mustPrefix("203.0.113.0/24"), // documentation netblock that looks Internet-y
+		Prefix6: mustPrefix("fc00:52::/64"),
 	}
 }
 
@@ -185,17 +184,6 @@ 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

@@ -9,7 +9,6 @@ package version
 import (
 	"os"
 	"path"
-	"path/filepath"
 	"strings"
 
 	"rsc.io/goversion/version"
@@ -23,27 +22,22 @@ func CmdName() string {
 	if err != nil {
 		return "cmd"
 	}
-
-	// fallbackName, the lowercase basename of the executable, is what we return if
-	// we can't find the Go module metadata embedded in the file.
-	fallbackName := filepath.Base(strings.TrimSuffix(strings.ToLower(e), ".exe"))
-
 	var ret string
 	v, err := version.ReadExe(e)
 	if err != nil {
-		return fallbackName
-	}
-	// 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") {
-			goPkg := strings.TrimPrefix(line, "path\t") // like "tailscale.com/cmd/tailscale"
-			ret = path.Base(goPkg)                      // goPkg is always forward slashes; use path, not filepath
-			break
+		ret = strings.TrimSuffix(strings.ToLower(e), ".exe")
+	} 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"))
+				break
+			}
 		}
 	}
 	if ret == "" {
-		return fallbackName
+		return "cmd"
 	}
 	return ret
 }

version/mkversion.sh

@@ -5,107 +5,89 @@ set -eu
 mode=$1
 describe=$2
 
-# Git describe output overall looks like
-# MAJOR.MINOR.PATCH-NUMCOMMITS-GITHASH. Depending on the tag being
-# described and the state of the repo, ver can be missing PATCH,
-# NUMCOMMITS, or both.
-#
-# Valid values look like: 1.2.3-1234-abcdef, 0.98-1234-abcdef,
-# 1.0.0-abcdef, 0.99-abcdef.
-ver="${describe#v}"
-stem="${ver%%-*}" # Just the semver-ish bit e.g. 1.2.3, 0.98
-suffix="${ver#$stem}" # The rest e.g. -23-abcdef, -abcdef
+long() {
+    ver="${describe#v}"
+    stem="${ver%%-*}"
+    case "$stem" in
+        *.*.*)
+            # Full SemVer, nothing to do.
+            semver="${stem}"
+        ;;
+        *.*)
+            # Old style major.minor, add a .0
+            semver="${stem}.0"
+            ;;
+        *)
+            echo "Unparseable version $stem" >&2
+            exit 1
+            ;;
+    esac
+    suffix="${ver#$stem}"
+    case "$suffix" in
+        -*-*)
+            # Has a change count in addition to the commit hash.
+        ;;
+        -*)
+            # Missing change count, add one.
+            suffix="-0${suffix}"
+            ;;
+        *)
+            echo "Unexpected version suffix" >&2
+            exit 1
+    esac
+    echo "${semver}${suffix}"
+}
 
-# Normalize the stem into a full major.minor.patch semver. We might
-# not use all those pieces depending on what kind of version we're
-# making, but it's good to have them all on hand.
-case "$stem" in
-    *.*.*)
-        # Full SemVer, nothing to do
-        stem="$stem"
-        ;;
-    *.*)
-        # Old style major.minor, add a .0
-        stem="${stem}.0"
-        ;;
-    *)
-        echo "Unparseable version $stem" >&2
-        exit 1
-        ;;
-esac
-major=$(echo "$stem" | cut -f1 -d.)
-minor=$(echo "$stem" | cut -f2 -d.)
-patch=$(echo "$stem" | cut -f3 -d.)
+short() {
+    ver="$(long)"
+    case "$ver" in
+        *-*-*)
+            echo "${ver%-*}"
+            ;;
+        *-*)
+            echo "$ver"
+            ;;
+        *)
+            echo "Long version in invalid format" >&2
+            exit 1
+            ;;
+    esac
+}
 
-# Extract the change count and git ID from the suffix.
-case "$suffix" in
-    -*-*)
-        # Has both a change count and a commit hash.
-        changecount=$(echo "$suffix" | cut -f2 -d-)
-        githash=$(echo "$suffix" | cut -f3 -d-)
-        ;;
-    -*)
-        # Git hash only, change count is zero.
-        changecount="0"
-        githash=$(echo "$suffix" | cut -f2 -d-)
-        ;;
-    *)
-        echo "Unparseable version suffix $suffix" >&2
-        exit 1
-        ;;
-esac
+xcode() {
+    ver=$(short | sed -e 's/-/./')
+    major=$(echo "$ver" | cut -f1 -d.)
+    minor=$(echo "$ver" | cut -f2 -d.)
+    patch=$(echo "$ver" | cut -f3 -d.)
+    changecount=$(echo "$ver" | cut -f4 -d.)
 
-# Validate that the version data makes sense. Rules:
-#  - Odd number minors are unstable. Patch must be 0, and gets
-#    replaced by changecount.
-#  - Even number minors are stable. Changecount must be 0, and
-#    gets removed.
-#
-# After this section, we only use major/minor/patch, which have been
-# tweaked as needed.
-if expr "$minor" : "[0-9]*[13579]$" >/dev/null; then
-    # Unstable
-    if [ "$patch" != "0" ]; then
-        # This is a fatal error, because a non-zero patch number
-        # indicates that we created an unstable git tag in violation
-        # of our versioning policy, and we want to blow up loudly to
-        # get that fixed.
-        echo "Unstable release $describe has a non-zero patch number, which is not allowed" >&2
-        exit 1
-    fi
-    patch="$changecount"
-else
-    # Stable
-    if [ "$changecount" != "0" ]; then
-        # This is a commit that's sitting between two stable
-        # releases. We never want to release such a commit to the
-        # pbulic, but it's useful to be able to build it for
-        # debugging. Just force the version to 0.0.0, so that we're
-        # forced to rely on the git commit hash.
-        major=0
-        minor=0
-        patch=0
-    fi
-fi
+    # Apple version numbers must be major.minor.patch. We have 4 fields
+    # because we need major.minor.patch for go module compatibility, and
+    # changecount for automatic version numbering of unstable builds. To
+    # resolve this, for Apple builds, we combine changecount into patch:
+    patch=$((patch*10000 + changecount))
 
-case "$1" in
+    # CFBundleShortVersionString: the "short name" used in the App Store.
+    # eg. 0.92.98
+    echo "VERSION_NAME = $major.$minor.$patch"
+    # CFBundleVersion: the build number. Needs to be 3 numeric sections
+    # that increment for each release according to SemVer rules.
+    #
+    # We start counting at 100 because we submitted using raw build
+    # numbers before, and Apple doesn't let you start over.
+    # e.g. 0.98.3-123 -> 100.98.3123
+    major=$((major + 100))
+    echo "VERSION_ID = $major.$minor.$patch"
+}
+
+case "$mode" in
     long)
-        echo "${major}.${minor}.${patch}-${githash}"
-        ;;
+        long
+    ;;
     short)
-        echo "${major}.${minor}.${patch}"
-        ;;
+        short
+    ;;
     xcode)
-        # CFBundleShortVersionString: the "short name" used in the App
-        # Store.  eg. 0.92.98
-        echo "VERSION_NAME = ${major}.${minor}.${patch}"
-        # CFBundleVersion: the build number. Needs to be 3 numeric
-        # sections that increment for each release according to SemVer
-        # rules.
-        #
-        # We start counting at 100 because we submitted using raw
-        # build numbers before, and Apple doesn't let you start over.
-        # e.g. 0.98.3 -> 100.98.3
-        echo "VERSION_ID = $((major + 100)).${minor}.${patch}"        
-        ;;
+        xcode
+    ;;
 esac

version/mkversion_test.go

@@ -15,60 +15,42 @@ func xcode(short, long string) string {
 	return fmt.Sprintf("VERSION_NAME = %s\nVERSION_ID = %s", short, long)
 }
 
-func mkversion(t *testing.T, mode, in string) (string, bool) {
+func mkversion(t *testing.T, mode, in string) string {
 	t.Helper()
 	bs, err := exec.Command("./mkversion.sh", mode, in).CombinedOutput()
 	if err != nil {
 		t.Logf("mkversion.sh output: %s", string(bs))
-		return "", false
+		t.Fatalf("mkversion.sh %s %s: %v", mode, in, err)
 	}
-	return strings.TrimSpace(string(bs)), true
+	return strings.TrimSpace(string(bs))
 }
 
 func TestMkversion(t *testing.T) {
 	tests := []struct {
 		in    string
-		ok    bool
 		long  string
 		short string
 		xcode string
 	}{
-		{"v0.98-abcdef", true, "0.98.0-abcdef", "0.98.0", xcode("0.98.0", "100.98.0")},
-		{"v0.98.1-abcdef", true, "0.98.1-abcdef", "0.98.1", xcode("0.98.1", "100.98.1")},
-		{"v1.1.0-37-abcdef", true, "1.1.37-abcdef", "1.1.37", xcode("1.1.37", "101.1.37")},
-		{"v1.2.9-abcdef", true, "1.2.9-abcdef", "1.2.9", xcode("1.2.9", "101.2.9")},
-		{"v1.2.9-0-abcdef", true, "1.2.9-abcdef", "1.2.9", xcode("1.2.9", "101.2.9")},
-		{"v1.15.0-129-abcdef", true, "1.15.129-abcdef", "1.15.129", xcode("1.15.129", "101.15.129")},
-
-		{"v0.98-123-abcdef", true, "0.0.0-abcdef", "0.0.0", xcode("0.0.0", "100.0.0")},
-		{"v1.0.0-37-abcdef", true, "0.0.0-abcdef", "0.0.0", xcode("0.0.0", "100.0.0")},
-
-		{"v0.99.5-0-abcdef", false, "", "", ""},   // unstable, patch not allowed
-		{"v0.99.5-123-abcdef", false, "", "", ""}, // unstable, patch not allowed
-		{"v1-abcdef", false, "", "", ""},          // bad semver
-		{"v1.0", false, "", "", ""},               // missing suffix
+		{"v0.98-abcdef", "0.98.0-0-abcdef", "0.98.0-0", xcode("0.98.0", "100.98.0")},
+		{"v0.98-123-abcdef", "0.98.0-123-abcdef", "0.98.0-123", xcode("0.98.123", "100.98.123")},
+		{"v0.99.5-123-abcdef", "0.99.5-123-abcdef", "0.99.5-123", xcode("0.99.50123", "100.99.50123")},
+		{"v0.99.5-123-abcdef", "0.99.5-123-abcdef", "0.99.5-123", xcode("0.99.50123", "100.99.50123")},
+		{"v2.3-0-abcdef", "2.3.0-0-abcdef", "2.3.0-0", xcode("2.3.0", "102.3.0")},
+		{"1.2.3-4-abcdef", "1.2.3-4-abcdef", "1.2.3-4", xcode("1.2.30004", "101.2.30004")},
 	}
 
 	for _, test := range tests {
-		gotlong, longOK := mkversion(t, "long", test.in)
-		if longOK != test.ok {
-			t.Errorf("mkversion.sh long %q ok=%v, want %v", test.in, longOK, test.ok)
-		}
-		gotshort, shortOK := mkversion(t, "short", test.in)
-		if shortOK != test.ok {
-			t.Errorf("mkversion.sh short %q ok=%v, want %v", test.in, shortOK, test.ok)
-		}
-		gotxcode, xcodeOK := mkversion(t, "xcode", test.in)
-		if xcodeOK != test.ok {
-			t.Errorf("mkversion.sh xcode %q ok=%v, want %v", test.in, xcodeOK, test.ok)
-		}
-		if longOK && gotlong != test.long {
+		gotlong := mkversion(t, "long", test.in)
+		gotshort := mkversion(t, "short", test.in)
+		gotxcode := mkversion(t, "xcode", test.in)
+		if gotlong != test.long {
 			t.Errorf("mkversion.sh long %q: got %q, want %q", test.in, gotlong, test.long)
 		}
-		if shortOK && gotshort != test.short {
+		if gotshort != test.short {
 			t.Errorf("mkversion.sh short %q: got %q, want %q", test.in, gotshort, test.short)
 		}
-		if xcodeOK && gotxcode != test.xcode {
+		if gotxcode != test.xcode {
 			t.Errorf("mkversion.sh xcode %q: got %q, want %q", test.in, gotxcode, test.xcode)
 		}
 	}

wgengine/filter/filter.go

@@ -6,7 +6,6 @@
 package filter
 
 import (
-	"fmt"
 	"sync"
 	"time"
 
@@ -137,13 +136,9 @@ 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, dir direction, r Response, why string) {
+func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, r Response, why string) {
 	var verdict string
 
-	if r == Drop && omitDropLogging(q, dir) {
-		return
-	}
-
 	if r == Drop && (runflags&LogDrops) != 0 && dropBucket.Allow() {
 		verdict = "Drop"
 		runflags &= HexdumpDrops
@@ -162,28 +157,26 @@ func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, dir dir
 
 // RunIn determines whether this node is allowed to receive q from a Tailscale peer.
 func (f *Filter) RunIn(q *packet.ParsedPacket, rf RunFlags) Response {
-	dir := in
-	r := f.pre(q, rf, dir)
+	r := f.pre(q, rf)
 	if r == Accept || r == Drop {
 		// already logged
 		return r
 	}
 
 	r, why := f.runIn(q)
-	f.logRateLimit(rf, q, dir, r, why)
+	f.logRateLimit(rf, q, 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 {
-	dir := out
-	r := f.pre(q, rf, dir)
+	r := f.pre(q, rf)
 	if r == Drop || r == Accept {
 		// already logged
 		return r
 	}
 	r, why := f.runOut(q)
-	f.logRateLimit(rf, q, dir, r, why)
+	f.logRateLimit(rf, q, r, why)
 	return r
 }
 
@@ -195,11 +188,6 @@ 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() {
@@ -259,106 +247,30 @@ func (f *Filter) runOut(q *packet.ParsedPacket) (r Response, why string) {
 	return Accept, "ok out"
 }
 
-// direction is whether a packet was flowing in to this machine, or flowing out.
-type direction int
-
-const (
-	in direction = iota
-	out
-)
-
-func (d direction) String() string {
-	switch d {
-	case in:
-		return "in"
-	case out:
-		return "out"
-	default:
-		return fmt.Sprintf("[??dir=%d]", int(d))
-	}
-}
-
-func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags, dir direction) Response {
+func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags) Response {
 	if len(q.Buffer()) == 0 {
 		// wireguard keepalive packet, always permit.
 		return Accept
 	}
 	if len(q.Buffer()) < 20 {
-		f.logRateLimit(rf, q, dir, Drop, "too short")
+		f.logRateLimit(rf, q, 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, dir, Drop, "unknown")
+		f.logRateLimit(rf, q, Drop, "unknown")
+		return Drop
+	case packet.IPv6:
+		f.logRateLimit(rf, q, Drop, "ipv6")
 		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, dir, Accept, "fragment")
+		f.logRateLimit(rf, q, Accept, "fragment")
 		return Accept
 	}
 
 	return noVerdict
 }
-
-const (
-	// ipv6AllRoutersLinkLocal is ff02::2 (All link-local routers)
-	ipv6AllRoutersLinkLocal = "\xff\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02"
-	// ipv6AllMLDv2CapableRouters is ff02::16 (All MLDv2-capable routers)
-	ipv6AllMLDv2CapableRouters = "\xff\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x16"
-)
-
-// 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 omitDropLogging(p *packet.ParsedPacket, dir direction) bool {
-	b := p.Buffer()
-	switch dir {
-	case out:
-		switch p.IPVersion {
-		case 4:
-			// ParsedPacket.Decode zeros out ParsedPacket.IPProtocol for protocols
-			// it doesn't know about, so parse it out ourselves if needed.
-			ipProto := p.IPProto
-			if ipProto == 0 && len(b) > 8 {
-				ipProto = packet.IPProto(b[9])
-			}
-			// Omit logging about outgoing IGMP.
-			if ipProto == packet.IGMP {
-				return true
-			}
-		case 6:
-			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
-				}
-			}
-			if string(dst) == ipv6AllMLDv2CapableRouters {
-				return true
-			}
-			// Actually, just catch all multicast.
-			if dst[0] == 0xff {
-				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

@@ -6,9 +6,7 @@ package filter
 
 import (
 	"encoding/binary"
-	"encoding/hex"
 	"encoding/json"
-	"strings"
 	"testing"
 
 	"tailscale.com/types/logger"
@@ -221,7 +219,7 @@ func TestPreFilter(t *testing.T) {
 	for _, testPacket := range packets {
 		p := &ParsedPacket{}
 		p.Decode(testPacket.b)
-		got := f.pre(p, LogDrops|LogAccepts, in)
+		got := f.pre(p, LogDrops|LogAccepts)
 		if got != testPacket.want {
 			t.Errorf("%q got=%v want=%v packet:\n%s", testPacket.desc, got, testPacket.want, packet.Hexdump(testPacket.b))
 		}
@@ -300,63 +298,3 @@ func rawdefault(proto packet.IPProto, trimLength int) []byte {
 	port := uint16(53)
 	return rawpacket(proto, ip, ip, port, port, trimLength)
 }
-
-func parseHexPkt(t *testing.T, h string) *packet.ParsedPacket {
-	t.Helper()
-	b, err := hex.DecodeString(strings.ReplaceAll(h, " ", ""))
-	if err != nil {
-		t.Fatalf("failed to read hex %q: %v", h, err)
-	}
-	p := new(packet.ParsedPacket)
-	p.Decode(b)
-	return p
-}
-
-func TestOmitDropLogging(t *testing.T) {
-	tests := []struct {
-		name string
-		pkt  *packet.ParsedPacket
-		dir  direction
-		want bool
-	}{
-		{
-			name: "v4_tcp_out",
-			pkt:  &packet.ParsedPacket{IPVersion: 4, IPProto: packet.TCP},
-			dir:  out,
-			want: false,
-		},
-		{
-			name: "v6_icmp_out", // as seen on Linux
-			pkt:  parseHexPkt(t, "60 00 00 00 00 00 3a 00   fe800000000000000000000000000000 ff020000000000000000000000000002"),
-			dir:  out,
-			want: true,
-		},
-		{
-			name: "v6_to_MLDv2_capable_routers", // as seen on Windows
-			pkt:  parseHexPkt(t, "60 00 00 00 00 24 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ff 02 00 00 00 00 00 00 00 00 00 00 00 00 00 16 3a 00 05 02 00 00 01 00 8f 00 6e 80 00 00 00 01 04 00 00 00 ff 02 00 00 00 00 00 00 00 00 00 00 00 00 00 0c"),
-			dir:  out,
-			want: true,
-		},
-		{
-			name: "v4_igmp_out", // on Windows, from https://github.com/tailscale/tailscale/issues/618
-			pkt:  parseHexPkt(t, "46 00 00 30 37 3a 00 00 01 02 10 0e a9 fe 53 6b e0 00 00 16 94 04 00 00 22 00 14 05 00 00 00 02 04 00 00 00 e0 00 00 fb 04 00 00 00 e0 00 00 fc"),
-			dir:  out,
-			want: true,
-		},
-		{
-			name: "v6_udp_multicast",
-			pkt:  parseHexPkt(t, "60 00 00 00 00 00 11 00  fe800000000000007dc6bc04499262a3 ff120000000000000000000000008384"),
-			dir:  out,
-			want: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := omitDropLogging(tt.pkt, tt.dir)
-			if got != tt.want {
-				t.Errorf("got %v; want %v\npacket: %#v\n%s", got, tt.want, tt.pkt, packet.Hexdump(tt.pkt.Buffer()))
-			}
-		})
-	}
-}

wgengine/magicsock/magicsock.go

@@ -55,43 +55,6 @@ 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 {
@@ -125,12 +88,19 @@ type Conn struct {
 	packetListener nettype.PacketListener
 
 	// ============================================================
-	mu     sync.Mutex // guards all following fields; see userspaceEngine lock ordering rules
-	muCond *sync.Cond
+	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
 
+	endpointsUpdateWaiter *sync.Cond
 	endpointsUpdateActive bool
 	wantEndpointsUpdate   string // true if non-empty; string is reason
 	lastEndpoints         []string
@@ -168,9 +138,8 @@ 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 & cleaner Close
+	derpStarted chan struct{}      // closed on first connection to DERP; for tests
 	activeDerp  map[int]activeDerp // DERP regionID -> connection to a node in that region
 	prevDerp    map[int]*syncs.WaitGroupChan
 
@@ -282,10 +251,8 @@ type Options struct {
 	// 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 is called while
-	// holding userspaceEngine.wgLock and likely calls
-	// Conn.CreateEndpoint, which acquires Conn.mu. As such, you should
-	// not hold
+	// enough and not too often.) The provided func likely calls
+	// Conn.CreateEndpoint, which acquires Conn.mu.
 	NoteRecvActivity func(tailcfg.DiscoKey)
 }
 
@@ -318,7 +285,7 @@ func newConn() *Conn {
 		sharedDiscoKey:  make(map[tailcfg.DiscoKey]*[32]byte),
 		discoOfAddr:     make(map[netaddr.IPPort]tailcfg.DiscoKey),
 	}
-	c.muCond = sync.NewCond(&c.mu)
+	c.endpointsUpdateWaiter = sync.NewCond(&c.mu)
 	return c
 }
 
@@ -390,7 +357,7 @@ func (c *Conn) updateEndpoints(why string) {
 			go c.updateEndpoints(why)
 		} else {
 			c.endpointsUpdateActive = false
-			c.muCond.Broadcast()
+			c.endpointsUpdateWaiter.Broadcast()
 		}
 
 	}()
@@ -635,6 +602,8 @@ 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.
 //
@@ -736,6 +705,10 @@ func shouldSprayPacket(b []byte) bool {
 	return false
 }
 
+var logPacketDests, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_LOG_PACKET_DESTS"))
+
+var debugDisco, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DISCO"))
+
 const sprayPeriod = 3 * time.Second
 
 // appendDests appends to dsts the destinations that b should be
@@ -960,6 +933,11 @@ 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.
@@ -1028,11 +1006,6 @@ 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 {
@@ -1074,7 +1047,6 @@ func (c *Conn) derpWriteChanOfAddr(addr netaddr.IPPort, peer key.Public) chan<-
 		go func() {
 			dc.Connect(ctx)
 			close(c.derpStarted)
-			c.muCond.Broadcast()
 		}()
 	}
 
@@ -1136,6 +1108,8 @@ 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{}) {
@@ -1258,11 +1232,8 @@ 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; 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.
+// wireguard-go transport packets; IIRC wireguard-go only uses this
+// Endpoint for the relatively rare non-data packets.
 func (c *Conn) findEndpoint(ipp netaddr.IPPort, addr *net.UDPAddr) conn.Endpoint {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -1349,15 +1320,13 @@ func wgRecvAddr(e conn.Endpoint, ipp netaddr.IPPort, addr *net.UDPAddr) *net.UDP
 	return ipp.UDPAddr()
 }
 
-// noteRecvActivityFromEndpoint calls the c.noteRecvActivity hook if
-// e is a discovery-capable peer and this is the first receive activity
-// it's got in awhile (in last 10 seconds).
+// 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 (c *Conn) noteRecvActivityFromEndpoint(e conn.Endpoint) {
-	de, ok := e.(*discoEndpoint)
-	if ok && c.noteRecvActivity != nil && de.isFirstRecvActivityInAwhile() {
-		c.noteRecvActivity(de.discoKey)
+func noteRecvActivity(e conn.Endpoint) {
+	if de, ok := e.(*discoEndpoint); ok {
+		de.onRecvActivity()
 	}
 }
 
@@ -1368,7 +1337,7 @@ Top:
 		c.bufferedIPv4From = netaddr.IPPort{}
 		addr = from.UDPAddr()
 		ep := c.findEndpoint(from, addr)
-		c.noteRecvActivityFromEndpoint(ep)
+		noteRecvActivity(ep)
 		return copy(b, c.bufferedIPv4Packet), ep, wgRecvAddr(ep, from, addr), nil
 	}
 
@@ -1480,7 +1449,7 @@ Top:
 		ep = c.findEndpoint(ipp, addr)
 	}
 	if !didNoteRecvActivity {
-		c.noteRecvActivityFromEndpoint(ep)
+		noteRecvActivity(ep)
 	}
 	return n, ep, wgRecvAddr(ep, ipp, addr), nil
 }
@@ -1508,7 +1477,7 @@ func (c *Conn) ReceiveIPv6(b []byte) (int, conn.Endpoint, *net.UDPAddr, error) {
 		}
 
 		ep := c.findEndpoint(ipp, addr)
-		c.noteRecvActivityFromEndpoint(ep)
+		noteRecvActivity(ep)
 		return n, ep, wgRecvAddr(ep, ipp, addr), nil
 	}
 }
@@ -1529,7 +1498,7 @@ func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey tailcfg.NodeKey, dstD
 	c.mu.Lock()
 	if c.closed {
 		c.mu.Unlock()
-		return false, errConnClosed
+		return false, errClosed
 	}
 	var nonce [disco.NonceLen]byte
 	if _, err := crand.Read(nonce[:]); err != nil {
@@ -1585,10 +1554,6 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 	if debugDisco {
 		c.logf("magicsock: disco: got disco-looking frame from %v", sender.ShortString())
 	}
-	if c.privateKey.IsZero() {
-		// Ignore disco messages when we're stopped.
-		return false
-	}
 	if c.discoPrivate.IsZero() {
 		if debugDisco {
 			c.logf("magicsock: disco: ignoring disco-looking frame, no local key")
@@ -1606,9 +1571,8 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 		return false
 	}
 
-	needsRecvActivityCall := false
-	de, endpointFound0 := c.endpointOfDisco[sender]
-	if !endpointFound0 {
+	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
@@ -1620,37 +1584,20 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 			c.logf("magicsock: [unexpected] have node without endpoint, without c.noteRecvActivity hook")
 			return false
 		}
-		needsRecvActivityCall = true
-	} else {
-		needsRecvActivityCall = de.isFirstRecvActivityInAwhile()
-	}
-	if needsRecvActivityCall && c.noteRecvActivity != nil {
-		// We can't hold Conn.mu while calling noteRecvActivity.
-		// noteRecvActivity acquires userspaceEngine.wgLock (and per our
-		// lock ordering rules: wgLock must come first), and also calls
-		// back into our Conn.CreateEndpoint, which would double-acquire
-		// Conn.mu.
-		c.mu.Unlock()
+		// 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.mu.Lock() // re-acquire
-
-		// Now, recheck invariants that might've changed while we'd
-		// released the lock, which isn't much:
-		if c.closed || c.privateKey.IsZero() {
-			return true
-		}
+		c.canCreateEPUnlocked.Set(false)
 		de, ok = c.endpointOfDisco[sender]
 		if !ok {
-			if _, ok := c.nodeOfDisco[sender]; !ok {
-				// They just disappeared while we'd released the lock.
-				return false
-			}
 			c.logf("magicsock: [unexpected] lazy endpoint not created for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
 			return false
 		}
-		if !endpointFound0 {
-			c.logf("magicsock: lazy endpoint created via disco message for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
-		}
+		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,
@@ -1839,11 +1786,8 @@ func (c *Conn) SetPrivateKey(privateKey wgcfg.PrivateKey) error {
 	c.privateKey = newKey
 
 	if oldKey.IsZero() {
-		c.everHadKey = true
 		c.logf("magicsock: SetPrivateKey called (init)")
-		if c.started {
-			go c.ReSTUN("set-private-key")
-		}
+		go c.ReSTUN("set-private-key")
 	} else if newKey.IsZero() {
 		c.logf("magicsock: SetPrivateKey called (zeroed)")
 		c.closeAllDerpLocked("zero-private-key")
@@ -1858,12 +1802,6 @@ func (c *Conn) SetPrivateKey(privateKey wgcfg.PrivateKey) error {
 		c.goDerpConnect(c.myDerp)
 	}
 
-	if newKey.IsZero() {
-		for _, de := range c.endpointOfDisco {
-			de.stopAndReset()
-		}
-	}
-
 	return nil
 }
 
@@ -1965,6 +1903,7 @@ func (c *Conn) SetNetworkMap(nm *controlclient.NetworkMap) {
 				c.nodeOfDisco[n.DiscoKey] = n
 				if old, ok := c.discoOfNode[n.Key]; ok && old != n.DiscoKey {
 					c.logf("magicsock: node %s changed discovery key from %x to %x", n.Key.ShortString(), old[:8], n.DiscoKey[:8])
+					// TODO: reset AddrSet states, reset wireguard session key, etc.
 				}
 				c.discoOfNode[n.Key] = n.DiscoKey
 			}
@@ -1977,7 +1916,7 @@ func (c *Conn) SetNetworkMap(nm *controlclient.NetworkMap) {
 	// Clean c.endpointOfDisco for discovery keys that are no longer present.
 	for dk, de := range c.endpointOfDisco {
 		if _, ok := c.nodeOfDisco[dk]; !ok {
-			de.stopAndReset()
+			de.cleanup()
 			delete(c.endpointOfDisco, dk)
 			delete(c.sharedDiscoKey, dk)
 		}
@@ -2095,7 +2034,7 @@ func (c *Conn) Close() error {
 	defer c.mu.Unlock()
 
 	for _, ep := range c.endpointOfDisco {
-		ep.stopAndReset()
+		ep.cleanup()
 	}
 
 	c.closed = true
@@ -2105,21 +2044,6 @@ func (c *Conn) Close() error {
 		c.pconn6.Close()
 	}
 	err := c.pconn4.Close()
-
-	// Wait on goroutines updating right at the end, once everything is
-	// already closed. We want everything else in the Conn to be
-	// consistently in the closed state before we release mu to wait
-	// on the endpoint updater & derphttp.Connect.
-	for c.goroutinesRunningLocked() {
-		c.muCond.Wait()
-	}
-	return err
-}
-
-func (c *Conn) goroutinesRunningLocked() bool {
-	if c.endpointsUpdateActive {
-		return true
-	}
 	// The goroutine running dc.Connect in derpWriteChanOfAddr may linger
 	// and appear to leak, as observed in https://github.com/tailscale/tailscale/issues/554.
 	// This is despite the underlying context being cancelled by connCtxCancel above.
@@ -2129,16 +2053,20 @@ func (c *Conn) goroutinesRunningLocked() bool {
 	// on the first run, it sets firstDerp := true and spawns the aforementioned goroutine.
 	// To detect this, we check activeDerp, which is initialized to non-nil on the first run.
 	if c.activeDerp != nil {
-		select {
-		case <-c.derpStarted:
-			break
-		default:
-			return true
-		}
+		<-c.derpStarted
 	}
-	return false
+	// Wait on endpoints updating right at the end, once everything is
+	// already closed. We want everything else in the Conn to be
+	// consistently in the closed state before we release mu to wait
+	// on the endpoint updater.
+	for c.endpointsUpdateActive {
+		c.endpointsUpdateWaiter.Wait()
+	}
+	return err
 }
 
+var debugReSTUNStopOnIdle, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_RESTUN_STOP_ON_IDLE"))
+
 func maxIdleBeforeSTUNShutdown() time.Duration {
 	if debugReSTUNStopOnIdle {
 		return time.Minute
@@ -2229,19 +2157,8 @@ func (c *Conn) ReSTUN(why string) {
 		// raced with a shutdown.
 		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)
+	if c.privateKey.IsZero() {
+		c.logf("magicsock: ReSTUN(%q) ignored; no private key", why)
 		return
 	}
 
@@ -2275,7 +2192,7 @@ func (c *Conn) listenPacket(ctx context.Context, network, addr string) (net.Pack
 
 func (c *Conn) bind1(ruc **RebindingUDPConn, which string) error {
 	host := ""
-	if inTest() {
+	if v, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST")); v {
 		host = "127.0.0.1"
 	}
 	var pc net.PacketConn
@@ -2305,7 +2222,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 inTest() {
+	if v, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST")); v {
 		host = "127.0.0.1"
 	}
 	listenCtx := context.Background() // unused without DNS name to resolve
@@ -2654,9 +2571,6 @@ func (c *Conn) CreateBind(uint16) (conn.Bind, uint16, error) {
 //
 
 func (c *Conn) CreateEndpoint(pubKey [32]byte, addrs string) (conn.Endpoint, error) {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
 	pk := key.Public(pubKey)
 	c.logf("magicsock: CreateEndpoint: key=%s: %s", pk.ShortString(), derpStr(addrs))
 
@@ -2666,6 +2580,10 @@ 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)
 		}
+		if !c.canCreateEPUnlocked.Get() { // sorry
+			c.mu.Lock()
+			defer c.mu.Unlock()
+		}
 		de := &discoEndpoint{
 			c:                  c,
 			publicKey:          tailcfg.NodeKey(pk),        // peer public key (for WireGuard + DERP)
@@ -2675,6 +2593,17 @@ func (c *Conn) CreateEndpoint(pubKey [32]byte, addrs string) (conn.Endpoint, err
 			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
@@ -2698,6 +2627,9 @@ func (c *Conn) CreateEndpoint(pubKey [32]byte, addrs string) (conn.Endpoint, err
 		}
 	}
 
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
 	// If this endpoint is being updated, remember its old set of
 	// endpoints so we can remove any (from c.addrsByUDP) that are
 	// not in the new set.
@@ -2900,17 +2832,6 @@ func (c *Conn) UpdateStatus(sb *ipnstate.StatusBuilder) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	if c.netMap != nil {
-		for _, addr := range c.netMap.Addresses {
-			if (addr.IP.Is4() && addr.Mask != 32) || (addr.IP.Is6() && addr.Mask != 128) {
-				continue
-			}
-			if ip, ok := netaddr.FromStdIP(addr.IP.IP()); ok {
-				sb.AddTailscaleIP(ip)
-			}
-		}
-	}
-
 	for dk, n := range c.nodeOfDisco {
 		ps := &ipnstate.PeerStatus{InMagicSock: true}
 		ps.Addrs = append(ps.Addrs, n.Endpoints...)
@@ -2946,9 +2867,6 @@ func udpAddrDebugString(ua net.UDPAddr) string {
 // discoEndpoint is a wireguard/conn.Endpoint for new-style peers that
 // advertise a DiscoKey and participate in active discovery.
 type discoEndpoint struct {
-	// atomically accessed; declared first for alignment reasons
-	lastRecvUnixAtomic int64
-
 	// These fields are initialized once and never modified.
 	c                  *Conn
 	publicKey          tailcfg.NodeKey  // peer public key (for WireGuard + DERP)
@@ -2957,6 +2875,7 @@ type discoEndpoint struct {
 	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
@@ -3053,19 +2972,6 @@ func (de *discoEndpoint) initFakeUDPAddr() {
 	de.fakeWGAddrStd = de.fakeWGAddr.UDPAddr()
 }
 
-// isFirstRecvActivityInAwhile notes that receive activity has occured for this
-// endpoint and reports whether it's been at least 10 seconds since the last
-// receive activity (including having never received from this peer before).
-func (de *discoEndpoint) isFirstRecvActivityInAwhile() bool {
-	now := time.Now().Unix()
-	old := atomic.LoadInt64(&de.lastRecvUnixAtomic)
-	if old <= now-10 {
-		atomic.StoreInt64(&de.lastRecvUnixAtomic, now)
-		return true
-	}
-	return false
-}
-
 // String exists purely so wireguard-go internals can log.Printf("%v")
 // its internal conn.Endpoints and we don't end up with data races
 // from fmt (via log) reading mutex fields and such.
@@ -3478,27 +3384,14 @@ func (de *discoEndpoint) populatePeerStatus(ps *ipnstate.PeerStatus) {
 	}
 }
 
-// stopAndReset stops timers associated with de and resets its state back to zero.
-// It's called when a discovery endpoint is no longer present in the NetworkMap,
-// or when magicsock is transition from running to stopped state (via SetPrivateKey(zero))
-func (de *discoEndpoint) stopAndReset() {
+// cleanup is called when a discovery endpoint is no longer present in the NetworkMap.
+// This is where we can do cleanup such as closing goroutines or canceling timers.
+func (de *discoEndpoint) cleanup() {
 	de.mu.Lock()
 	defer de.mu.Unlock()
 
 	de.c.logf("magicsock: doing cleanup for discovery key %x", de.discoKey[:])
 
-	// Zero these fields so if the user re-starts the network, the discovery
-	// state isn't a mix of before & after two sessions.
-	de.lastSend = time.Time{}
-	de.lastFullPing = time.Time{}
-	de.bestAddr = netaddr.IPPort{}
-	de.bestAddrLatency = 0
-	de.bestAddrAt = time.Time{}
-	de.trustBestAddrUntil = time.Time{}
-	for _, es := range de.endpointState {
-		es.lastPing = time.Time{}
-	}
-
 	for txid, sp := range de.sentPing {
 		de.removeSentPingLocked(txid, sp)
 	}
@@ -3550,3 +3443,5 @@ type ippCacheKey struct {
 
 // derpStr replaces DERP IPs in s with "derp-".
 func derpStr(s string) string { return strings.ReplaceAll(s, "127.3.3.40:", "derp-") }
+
+var errClosed = errors.New("conn is closed")

wgengine/magicsock/magicsock_test.go

@@ -6,23 +6,19 @@ package magicsock
 
 import (
 	"bytes"
-	"context"
 	crand "crypto/rand"
 	"crypto/tls"
 	"encoding/binary"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net"
 	"net/http"
 	"net/http/httptest"
 	"os"
-	"strconv"
 	"strings"
 	"sync"
 	"testing"
 	"time"
-	"unsafe"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/tailscale/wireguard-go/device"
@@ -30,11 +26,9 @@ 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"
@@ -72,6 +66,11 @@ func runDERPAndStun(t *testing.T, logf logger.Logf, l nettype.PacketListener, st
 		t.Fatal(err)
 	}
 	d := derp.NewServer(serverPrivateKey, logf)
+	if l != (nettype.Std{}) {
+		// When using virtual networking, only allow DERP to forward
+		// discovery traffic, not actual packets.
+		d.OnlyDisco = true
+	}
 
 	httpsrv := httptest.NewUnstartedServer(derphttp.Handler(d))
 	httpsrv.Config.ErrorLog = logger.StdLogger(logf)
@@ -173,7 +172,7 @@ func newMagicStack(t *testing.T, logf logger.Logf, l nettype.PacketListener, der
 	// Wait for first endpoint update to be available
 	deadline := time.Now().Add(2 * time.Second)
 	for len(epCh) == 0 && time.Now().Before(deadline) {
-		time.Sleep(100 * time.Millisecond)
+		time.Sleep(10 * time.Millisecond)
 	}
 
 	return &magicStack{
@@ -186,136 +185,11 @@ func newMagicStack(t *testing.T, logf logger.Logf, l nettype.PacketListener, der
 	}
 }
 
-func (s *magicStack) String() string {
-	pub := s.Public()
-	return pub.ShortString()
-}
-
 func (s *magicStack) Close() {
 	s.dev.Close()
 	s.conn.Close()
 }
 
-func (s *magicStack) Public() key.Public {
-	return key.Public(s.privateKey.Public())
-}
-
-func (s *magicStack) Status() *ipnstate.Status {
-	var sb ipnstate.StatusBuilder
-	s.conn.UpdateStatus(&sb)
-	return sb.Status()
-}
-
-// IP returns the Tailscale IP address assigned to this magicStack.
-//
-// Something external needs to provide a NetworkMap and WireGuard
-// configs to the magicStack in order for it to acquire an IP
-// address. See meshStacks for one possible source of netmaps and IPs.
-func (s *magicStack) IP(t *testing.T) netaddr.IP {
-	for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) {
-		st := s.Status()
-		if len(st.TailscaleIPs) > 0 {
-			return st.TailscaleIPs[0]
-		}
-	}
-	t.Fatal("timed out waiting for magicstack to get an IP assigned")
-	panic("unreachable") // compiler doesn't know t.Fatal panics
-}
-
-// meshStacks monitors epCh on all given ms, and plumbs network maps
-// and WireGuard configs into everyone to form a full mesh that has up
-// to date endpoint info. Think of it as an extremely stripped down
-// and purpose-built Tailscale control plane.
-//
-// meshStacks only supports disco connections, not legacy logic.
-func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
-	ctx, cancel := context.WithCancel(context.Background())
-
-	// Serialize all reconfigurations globally, just to keep things
-	// simpler.
-	var (
-		mu  sync.Mutex
-		eps = make([][]string, len(ms))
-	)
-
-	buildNetmapLocked := func(myIdx int) *controlclient.NetworkMap {
-		me := ms[myIdx]
-		nm := &controlclient.NetworkMap{
-			PrivateKey: me.privateKey,
-			NodeKey:    tailcfg.NodeKey(me.privateKey.Public()),
-			Addresses:  []wgcfg.CIDR{{IP: wgcfg.IPv4(1, 0, 0, byte(myIdx+1)), Mask: 32}},
-		}
-		for i, peer := range ms {
-			if i == myIdx {
-				continue
-			}
-			addrs := []wgcfg.CIDR{{IP: wgcfg.IPv4(1, 0, 0, byte(i+1)), Mask: 32}}
-			peer := &tailcfg.Node{
-				ID:         tailcfg.NodeID(i + 1),
-				Name:       fmt.Sprintf("node%d", i+1),
-				Key:        tailcfg.NodeKey(peer.privateKey.Public()),
-				DiscoKey:   peer.conn.DiscoPublicKey(),
-				Addresses:  addrs,
-				AllowedIPs: addrs,
-				Endpoints:  eps[i],
-				DERP:       "127.3.3.40:1",
-			}
-			nm.Peers = append(nm.Peers, peer)
-		}
-
-		return nm
-	}
-
-	updateEps := func(idx int, newEps []string) {
-		mu.Lock()
-		defer mu.Unlock()
-
-		eps[idx] = newEps
-
-		for i, m := range ms {
-			netmap := buildNetmapLocked(i)
-			m.conn.SetNetworkMap(netmap)
-			peerSet := make(map[key.Public]struct{}, len(netmap.Peers))
-			for _, peer := range netmap.Peers {
-				peerSet[key.Public(peer.Key)] = struct{}{}
-			}
-			m.conn.UpdatePeers(peerSet)
-			wg, err := netmap.WGCfg(logf, controlclient.AllowSingleHosts)
-			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()
@@ -559,136 +433,65 @@ func makeNestable(t *testing.T) (logf logger.Logf, setT func(t *testing.T)) {
 }
 
 func TestTwoDevicePing(t *testing.T) {
-	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)
-
+	t.Run("real", func(t *testing.T) {
+		l, ip := nettype.Std{}, netaddr.IPv4(127, 0, 0, 1)
 		n := &devices{
-			m1:     m1,
-			m1IP:   m1if.V4(),
-			m2:     m2,
-			m2IP:   m2if.V4(),
-			stun:   mstun,
-			stunIP: sif.V4(),
+			m1:     l,
+			m1IP:   ip,
+			m2:     l,
+			m2IP:   ip,
+			stun:   l,
+			stunIP: ip,
 		}
-		testActiveDiscovery(t, n)
+		testTwoDevicePing(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)
 
-	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)
+			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 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)
+		})
 	})
-
-	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 {
@@ -702,135 +505,6 @@ 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) {
-		lastLog := time.Now().Add(-time.Minute)
-		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
-			}
-			if now := time.Now(); now.Sub(lastLog) > time.Second {
-				logf("no direct path %s->%s yet, addrs %v", m1, m2, pst.Addrs)
-				lastLog = now
-			}
-		}
-		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()
@@ -930,7 +604,7 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	})
 
 	// TODO: Remove this once the following tests are reliable.
-	if run, _ := strconv.ParseBool(os.Getenv("RUN_CURSED_TESTS")); !run {
+	if os.Getenv("RUN_CURSED_TESTS") == "" {
 		t.Skip("skipping following tests because RUN_CURSED_TESTS is not set.")
 	}
 
@@ -1028,8 +702,10 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 		defer setT(outerT)
 		defer func() {
 			if t.Failed() || true {
-				logf("cfg0: %v", stringifyConfig(cfgs[0]))
-				logf("cfg1: %v", stringifyConfig(cfgs[1]))
+				uapi1, _ := cfgs[0].ToUAPI()
+				logf("cfg0: %v", uapi1)
+				uapi2, _ := cfgs[1].ToUAPI()
+				logf("cfg1: %v", uapi2)
 			}
 		}()
 		pingSeq(t, 20, 0, true)
@@ -1267,7 +943,6 @@ func initAddrSet(as *AddrSet) {
 func TestDiscoMessage(t *testing.T) {
 	c := newConn()
 	c.logf = t.Logf
-	c.privateKey = key.NewPrivate()
 
 	peer1Pub := c.DiscoPublicKey()
 	peer1Priv := c.discoPrivate
@@ -1315,25 +990,3 @@ func TestDiscoStringLogRace(t *testing.T) {
 	}()
 	wg.Wait()
 }
-
-func stringifyConfig(cfg wgcfg.Config) string {
-	j, err := json.Marshal(cfg)
-	if err != nil {
-		panic(err)
-	}
-	return string(j)
-}
-
-func TestDiscoEndpointAlignment(t *testing.T) {
-	var de discoEndpoint
-	off := unsafe.Offsetof(de.lastRecvUnixAtomic)
-	if off%8 != 0 {
-		t.Fatalf("lastRecvUnixAtomic is not 8-byte aligned")
-	}
-	if !de.isFirstRecvActivityInAwhile() { // verify this doesn't panic on 32-bit
-		t.Error("expected true")
-	}
-	if de.isFirstRecvActivityInAwhile() {
-		t.Error("expected false on second call")
-	}
-}

wgengine/monitor/monitor_linux.go

@@ -79,7 +79,7 @@ func (c *nlConn) Receive() (message, error) {
 	case unix.RTM_NEWADDR, unix.RTM_DELADDR:
 		var rmsg rtnetlink.AddressMessage
 		if err := rmsg.UnmarshalBinary(msg.Data); err != nil {
-			c.logf("failed to parse type %v: %v", msg.Header.Type, err)
+			c.logf("failed to parse type 0x%x: %v", msg.Header.Type, err)
 			return unspecifiedMessage{}, nil
 		}
 		return &newAddrMessage{
@@ -99,18 +99,8 @@ func (c *nlConn) Receive() (message, error) {
 			Dst:     netaddrIP(rmsg.Attributes.Dst),
 			Gateway: netaddrIP(rmsg.Attributes.Gateway),
 		}, nil
-	case unix.RTM_DELROUTE:
-		var rmsg rtnetlink.RouteMessage
-		if err := rmsg.UnmarshalBinary(msg.Data); err != nil {
-			c.logf("RTM_DELROUTE: failed to parse: %v", err)
-			return unspecifiedMessage{}, nil
-		}
-		// Just log it for now, but don't bubble it up.
-		// (Debugging https://github.com/tailscale/tailscale/issues/643)
-		c.logf("RTM_DELROUTE: %+v", rmsg)
-		return unspecifiedMessage{}, nil
 	default:
-		c.logf("unhandled netlink msg type %+v, %q", msg.Header, msg.Data)
+		c.logf("unhandled netlink msg type 0x%x: %+v, %q", msg.Header.Type, msg.Header, msg.Data)
 		return unspecifiedMessage{}, nil
 	}
 }

wgengine/packet/ip.go

@@ -47,13 +47,12 @@ 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
-	// Fragment is a special value. It's not really an IPProto value
-	// so we're using the unassigned 0xFF value.
+	// IPv6 and Fragment are special values. They're not really IPProto values
+	// so we're using the unassigned 0xFE and 0xFF values for them.
 	// TODO(dmytro): special values should be taken out of here.
+	IPv6     IPProto = 0xFE
 	Fragment IPProto = 0xFF
 )
 
@@ -67,6 +66,8 @@ func (p IPProto) String() string {
 		return "UDP"
 	case TCP:
 		return "TCP"
+	case IPv6:
+		return "IPv6"
 	default:
 		return "Unknown"
 	}

wgengine/packet/packet.go

@@ -30,8 +30,6 @@ 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
@@ -43,32 +41,27 @@ type ParsedPacket struct {
 	// This is not the same as len(b) because b can have trailing zeros.
 	length int
 
-	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)
+	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)
 }
 
-// NextHeader
-type NextHeader uint8
-
-func (p *ParsedPacket) String() string {
-	if p.IPVersion == 6 {
-		return fmt.Sprintf("IPv6{Proto=%d}", p.IPProto)
-	}
-	switch p.IPProto {
+func (q *ParsedPacket) String() string {
+	switch q.IPProto {
+	case IPv6:
+		return "IPv6{???}"
 	case Unknown:
 		return "Unknown{???}"
 	}
 	sb := strbuilder.Get()
-	sb.WriteString(p.IPProto.String())
+	sb.WriteString(q.IPProto.String())
 	sb.WriteByte('{')
-	writeIPPort(sb, p.SrcIP, p.SrcPort)
+	writeIPPort(sb, q.SrcIP, q.SrcPort)
 	sb.WriteString(" > ")
-	writeIPPort(sb, p.DstIP, p.DstPort)
+	writeIPPort(sb, q.DstIP, q.DstPort)
 	sb.WriteByte('}')
 	return sb.String()
 }
@@ -112,22 +105,20 @@ 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
-	q.IPVersion = (b[0] & 0xF0) >> 4
-	switch q.IPVersion {
+	switch (b[0] & 0xF0) >> 4 {
 	case 4:
 		q.IPProto = IPProto(b[9])
+		// continue
 	case 6:
-		q.IPProto = IPProto(b[6]) // "Next Header" field
+		q.IPProto = IPv6
 		return
 	default:
-		q.IPVersion = 0
 		q.IPProto = Unknown
 		return
 	}

wgengine/packet/packet_test.go

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

wgengine/router/dns.go

@@ -0,0 +1,74 @@
+// 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 router
+
+import (
+	"inet.af/netaddr"
+)
+
+// DNSConfig is the subset of Config that contains DNS parameters.
+type DNSConfig struct {
+	// Nameservers are the IP addresses of the nameservers to use.
+	Nameservers []netaddr.IP
+	// Domains are the search domains to use.
+	Domains []string
+}
+
+// EquivalentTo determines whether its argument and receiver
+// represent equivalent DNS configurations (then DNS reconfig is a no-op).
+func (lhs DNSConfig) EquivalentTo(rhs DNSConfig) bool {
+	if len(lhs.Nameservers) != len(rhs.Nameservers) {
+		return false
+	}
+
+	if len(lhs.Domains) != len(rhs.Domains) {
+		return false
+	}
+
+	// With how we perform resolution order shouldn't matter,
+	// but it is unlikely that we will encounter different orders.
+	for i, server := range lhs.Nameservers {
+		if rhs.Nameservers[i] != server {
+			return false
+		}
+	}
+
+	for i, domain := range lhs.Domains {
+		if rhs.Domains[i] != domain {
+			return false
+		}
+	}
+
+	return true
+}
+
+// dnsMode determines how DNS settings are managed.
+type dnsMode uint8
+
+const (
+	// dnsDirect indicates that /etc/resolv.conf is edited directly.
+	dnsDirect dnsMode = iota
+	// dnsResolvconf indicates that a resolvconf binary is used.
+	dnsResolvconf
+	// dnsNetworkManager indicates that the NetworkManaer DBus API is used.
+	dnsNetworkManager
+	// dnsResolved indicates that the systemd-resolved DBus API is used.
+	dnsResolved
+)
+
+func (m dnsMode) String() string {
+	switch m {
+	case dnsDirect:
+		return "direct"
+	case dnsResolvconf:
+		return "resolvconf"
+	case dnsNetworkManager:
+		return "networkmanager"
+	case dnsResolved:
+		return "resolved"
+	default:
+		return "???"
+	}
+}

wgengine/router/dns/config.go

@@ -1,75 +0,0 @@
-// 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 dns
-
-import (
-	"inet.af/netaddr"
-
-	"tailscale.com/types/logger"
-)
-
-// Config is the set of parameters that uniquely determine
-// the state to which a manager should bring system DNS settings.
-type Config struct {
-	// Nameservers are the IP addresses of the nameservers to use.
-	Nameservers []netaddr.IP
-	// Domains are the search domains to use.
-	Domains []string
-	// PerDomain indicates whether it is preferred to use Nameservers
-	// only for DNS queries for subdomains of Domains.
-	// Note that Nameservers may still be applied to all queries
-	// if the manager does not support per-domain settings.
-	PerDomain bool
-	// Proxied indicates whether DNS requests are proxied through a tsdns.Resolver.
-	Proxied bool
-}
-
-// Equal determines whether its argument and receiver
-// represent equivalent DNS configurations (then DNS reconfig is a no-op).
-func (lhs Config) Equal(rhs Config) bool {
-	if lhs.Proxied != rhs.Proxied || lhs.PerDomain != rhs.PerDomain {
-		return false
-	}
-
-	if len(lhs.Nameservers) != len(rhs.Nameservers) {
-		return false
-	}
-
-	if len(lhs.Domains) != len(rhs.Domains) {
-		return false
-	}
-
-	// With how we perform resolution order shouldn't matter,
-	// but it is unlikely that we will encounter different orders.
-	for i, server := range lhs.Nameservers {
-		if rhs.Nameservers[i] != server {
-			return false
-		}
-	}
-
-	// The order of domains, on the other hand, is significant.
-	for i, domain := range lhs.Domains {
-		if rhs.Domains[i] != domain {
-			return false
-		}
-	}
-
-	return true
-}
-
-// ManagerConfig is the set of parameters from which
-// a manager implementation is chosen and initialized.
-type ManagerConfig struct {
-	// logf is the logger for the manager to use.
-	Logf logger.Logf
-	// InterfaceNAme is the name of the interface with which DNS settings should be associated.
-	InterfaceName string
-	// Cleanup indicates that the manager is created for cleanup only.
-	// A no-op manager will be instantiated if the system needs no cleanup.
-	Cleanup bool
-	// PerDomain indicates that a manager capable of per-domain configuration is preferred.
-	// Certain managers are per-domain only; they will not be considered if this is false.
-	PerDomain bool
-}

wgengine/router/dns/manager.go

@@ -1,94 +0,0 @@
-// 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 dns
-
-import (
-	"time"
-
-	"tailscale.com/types/logger"
-)
-
-// reconfigTimeout is the time interval within which Manager.{Up,Down} should complete.
-//
-// This is particularly useful because certain conditions can cause indefinite hangs
-// (such as improper dbus auth followed by contextless dbus.Object.Call).
-// Such operations should be wrapped in a timeout context.
-const reconfigTimeout = time.Second
-
-type managerImpl interface {
-	// Up updates system DNS settings to match the given configuration.
-	Up(Config) error
-	// Down undoes the effects of Up.
-	// It is idempotent and performs no action if Up has never been called.
-	Down() error
-}
-
-// Manager manages system DNS settings.
-type Manager struct {
-	logf logger.Logf
-
-	impl managerImpl
-
-	config  Config
-	mconfig ManagerConfig
-}
-
-// NewManagers created a new manager from the given config.
-func NewManager(mconfig ManagerConfig) *Manager {
-	mconfig.Logf = logger.WithPrefix(mconfig.Logf, "dns: ")
-	m := &Manager{
-		logf: mconfig.Logf,
-		impl: newManager(mconfig),
-
-		config:  Config{PerDomain: mconfig.PerDomain},
-		mconfig: mconfig,
-	}
-
-	m.logf("using %T", m.impl)
-	return m
-}
-
-func (m *Manager) Set(config Config) error {
-	if config.Equal(m.config) {
-		return nil
-	}
-
-	m.logf("Set: %+v", config)
-
-	if len(config.Nameservers) == 0 {
-		err := m.impl.Down()
-		// If we save the config, we will not retry next time. Only do this on success.
-		if err == nil {
-			m.config = config
-		}
-		return err
-	}
-
-	// Switching to and from per-domain mode may require a change of manager.
-	if config.PerDomain != m.config.PerDomain {
-		if err := m.impl.Down(); err != nil {
-			return err
-		}
-		m.mconfig.PerDomain = config.PerDomain
-		m.impl = newManager(m.mconfig)
-		m.logf("switched to %T", m.impl)
-	}
-
-	err := m.impl.Up(config)
-	// If we save the config, we will not retry next time. Only do this on success.
-	if err == nil {
-		m.config = config
-	}
-
-	return err
-}
-
-func (m *Manager) Up() error {
-	return m.impl.Up(m.config)
-}
-
-func (m *Manager) Down() error {
-	return m.impl.Down()
-}

wgengine/router/dns/manager_default.go

@@ -1,14 +0,0 @@
-// 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,!freebsd,!openbsd,!windows
-
-package dns
-
-func newManager(mconfig ManagerConfig) managerImpl {
-	// TODO(dmytro): on darwin, we should use a macOS-specific method such as scutil.
-	// This is currently not implemented. Editing /etc/resolv.conf does not work,
-	// as most applications use the system resolver, which disregards it.
-	return newNoopManager(mconfig)
-}

wgengine/router/dns/manager_freebsd.go

@@ -1,14 +0,0 @@
-// 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 dns
-
-func newManager(mconfig ManagerConfig) managerImpl {
-	switch {
-	case isResolvconfActive():
-		return newResolvconfManager(mconfig)
-	default:
-		return newDirectManager(mconfig)
-	}
-}

wgengine/router/dns/manager_linux.go

@@ -1,27 +0,0 @@
-// 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 dns
-
-func newManager(mconfig ManagerConfig) managerImpl {
-	switch {
-	// systemd-resolved should only activate per-domain.
-	case isResolvedActive() && mconfig.PerDomain:
-		if mconfig.Cleanup {
-			return newNoopManager(mconfig)
-		} else {
-			return newResolvedManager(mconfig)
-		}
-	case isNMActive():
-		if mconfig.Cleanup {
-			return newNoopManager(mconfig)
-		} else {
-			return newNMManager(mconfig)
-		}
-	case isResolvconfActive():
-		return newResolvconfManager(mconfig)
-	default:
-		return newDirectManager(mconfig)
-	}
-}

wgengine/router/dns/manager_openbsd.go

@@ -1,9 +0,0 @@
-// 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 dns
-
-func newManager(mconfig ManagerConfig) managerImpl {
-	return newDirectManager(mconfig)
-}

wgengine/router/dns/manager_windows.go

@@ -1,83 +0,0 @@
-// 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 dns
-
-import (
-	"fmt"
-	"strings"
-
-	"github.com/tailscale/wireguard-go/tun"
-	"golang.org/x/sys/windows/registry"
-	"tailscale.com/types/logger"
-)
-
-type windowsManager struct {
-	logf logger.Logf
-	guid string
-}
-
-func newManager(mconfig ManagerConfig) managerImpl {
-	return windowsManager{
-		logf: mconfig.Logf,
-		guid: tun.WintunGUID,
-	}
-}
-
-func setRegistry(path, nameservers, domains string) error {
-	key, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.READ|registry.SET_VALUE)
-	if err != nil {
-		return fmt.Errorf("opening %s: %w", path, err)
-	}
-	defer key.Close()
-
-	err = key.SetStringValue("NameServer", nameservers)
-	if err != nil {
-		return fmt.Errorf("setting %s/NameServer: %w", path, err)
-	}
-
-	err = key.SetStringValue("Domain", domains)
-	if err != nil {
-		return fmt.Errorf("setting %s/Domain: %w", path, err)
-	}
-
-	return nil
-}
-
-func (m windowsManager) Up(config Config) error {
-	var ipsv4 []string
-	var ipsv6 []string
-	for _, ip := range config.Nameservers {
-		if ip.Is4() {
-			ipsv4 = append(ipsv4, ip.String())
-		} else {
-			ipsv6 = append(ipsv6, ip.String())
-		}
-	}
-	nsv4 := strings.Join(ipsv4, ",")
-	nsv6 := strings.Join(ipsv6, ",")
-
-	var domains string
-	if len(config.Domains) > 0 {
-		if len(config.Domains) > 1 {
-			m.logf("only a single search domain is supported")
-		}
-		domains = config.Domains[0]
-	}
-
-	v4Path := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + m.guid
-	if err := setRegistry(v4Path, nsv4, domains); err != nil {
-		return err
-	}
-	v6Path := `SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters\Interfaces\` + m.guid
-	if err := setRegistry(v6Path, nsv6, domains); err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func (m windowsManager) Down() error {
-	return m.Up(Config{Nameservers: nil, Domains: nil})
-}

wgengine/router/dns/noop.go

@@ -1,17 +0,0 @@
-// 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 dns
-
-type noopManager struct{}
-
-// Up implements managerImpl.
-func (m noopManager) Up(Config) error { return nil }
-
-// Down implements managerImpl.
-func (m noopManager) Down() error { return nil }
-
-func newNoopManager(mconfig ManagerConfig) managerImpl {
-	return noopManager{}
-}

wgengine/router/dns_direct.go

@@ -4,7 +4,7 @@
 
 // +build linux freebsd openbsd
 
-package dns
+package router
 
 import (
 	"bufio"
@@ -13,8 +13,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"os/exec"
-	"runtime"
 	"strings"
 
 	"inet.af/netaddr"
@@ -27,8 +25,8 @@ const (
 	resolvConf = "/etc/resolv.conf"
 )
 
-// writeResolvConf writes DNS configuration in resolv.conf format to the given writer.
-func writeResolvConf(w io.Writer, servers []netaddr.IP, domains []string) {
+// dnsWriteConfig writes DNS configuration in resolv.conf format to the given writer.
+func dnsWriteConfig(w io.Writer, servers []netaddr.IP, domains []string) {
 	io.WriteString(w, "# resolv.conf(5) file generated by tailscale\n")
 	io.WriteString(w, "# DO NOT EDIT THIS FILE BY HAND -- CHANGES WILL BE OVERWRITTEN\n\n")
 	for _, ns := range servers {
@@ -46,9 +44,9 @@ func writeResolvConf(w io.Writer, servers []netaddr.IP, domains []string) {
 	}
 }
 
-// readResolvConf reads DNS configuration from /etc/resolv.conf.
-func readResolvConf() (Config, error) {
-	var config Config
+// dnsReadConfig reads DNS configuration from /etc/resolv.conf.
+func dnsReadConfig() (DNSConfig, error) {
+	var config DNSConfig
 
 	f, err := os.Open("/etc/resolv.conf")
 	if err != nil {
@@ -81,43 +79,17 @@ func readResolvConf() (Config, 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
-}
-
-// directManager is a managerImpl which replaces /etc/resolv.conf with a file
-// generated from the given configuration, creating a backup of its old state.
+// dnsDirectUp replaces /etc/resolv.conf with a file generated
+// from the given configuration, creating a backup of its old state.
 //
 // This way of configuring DNS is precarious, since it does not react
 // to the disappearance of the Tailscale interface.
-// The caller must call Down before program shutdown
-// or as cleanup if the program terminates unexpectedly.
-type directManager struct{}
-
-func newDirectManager(mconfig ManagerConfig) managerImpl {
-	return directManager{}
-}
-
-// Up implements managerImpl.
-func (m directManager) Up(config Config) error {
+// The caller must call dnsDirectDown before program shutdown
+// and ensure that router.Cleanup is run if the program terminates unexpectedly.
+func dnsDirectUp(config DNSConfig) error {
 	// Write the tsConf file.
 	buf := new(bytes.Buffer)
-	writeResolvConf(buf, config.Nameservers, config.Domains)
+	dnsWriteConfig(buf, config.Nameservers, config.Domains)
 	if err := atomicfile.WriteFile(tsConf, buf.Bytes(), 0644); err != nil {
 		return err
 	}
@@ -152,15 +124,12 @@ func (m directManager) Up(config Config) error {
 		return err
 	}
 
-	if isResolvedRunning() {
-		exec.Command("systemctl", "restart", "systemd-resolved.service").Run() // Best-effort.
-	}
-
 	return nil
 }
 
-// Down implements managerImpl.
-func (m directManager) Down() error {
+// dnsDirectDown restores /etc/resolv.conf to its state before dnsDirectUp.
+// It is idempotent and behaves correctly even if dnsDirectUp has never been run.
+func dnsDirectDown() error {
 	if _, err := os.Stat(backupConf); err != nil {
 		// If the backup file does not exist, then Up never ran successfully.
 		if os.IsNotExist(err) {
@@ -178,10 +147,5 @@ func (m directManager) Down() 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

@@ -4,7 +4,7 @@
 
 // +build linux
 
-package dns
+package router
 
 import (
 	"bufio"
@@ -20,8 +20,8 @@ import (
 
 type nmConnectionSettings map[string]map[string]dbus.Variant
 
-// isNMActive determines if NetworkManager is currently managing system DNS settings.
-func isNMActive() bool {
+// nmIsActive determines if NetworkManager is currently managing system DNS settings.
+func nmIsActive() bool {
 	// This is somewhat tricky because NetworkManager supports a number
 	// of DNS configuration modes. In all cases, we expect it to be installed
 	// and /etc/resolv.conf to contain a mention of NetworkManager in the comments.
@@ -50,20 +50,10 @@ func isNMActive() bool {
 	return false
 }
 
-// nmManager uses the NetworkManager DBus API.
-type nmManager struct {
-	interfaceName string
-}
-
-func newNMManager(mconfig ManagerConfig) managerImpl {
-	return nmManager{
-		interfaceName: mconfig.InterfaceName,
-	}
-}
-
-// Up implements managerImpl.
-func (m nmManager) Up(config Config) error {
-	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
+// dnsNetworkManagerUp updates the DNS config for the Tailscale interface
+// through the NetworkManager DBus API.
+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.
@@ -100,7 +90,7 @@ func (m nmManager) Up(config Config) error {
 	var devicePath dbus.ObjectPath
 	err = nm.CallWithContext(
 		ctx, "org.freedesktop.NetworkManager.GetDeviceByIpIface", 0,
-		m.interfaceName,
+		interfaceName,
 	).Store(&devicePath)
 	if err != nil {
 		return fmt.Errorf("getDeviceByIpIface: %w", err)
@@ -199,7 +189,7 @@ func (m nmManager) Up(config Config) error {
 	return nil
 }
 
-// Down implements managerImpl.
-func (m nmManager) Down() error {
-	return m.Up(Config{Nameservers: nil, Domains: nil})
+// dnsNetworkManagerDown undoes the changes made by dnsNetworkManagerUp.
+func dnsNetworkManagerDown(interfaceName string) error {
+	return dnsNetworkManagerUp(DNSConfig{Nameservers: nil, Domains: nil}, interfaceName)
 }

wgengine/router/dns_resolvconf.go

@@ -4,7 +4,7 @@
 
 // +build linux freebsd
 
-package dns
+package router
 
 import (
 	"bufio"
@@ -14,10 +14,10 @@ import (
 	"os/exec"
 )
 
-// isResolvconfActive indicates whether the system appears to be using resolvconf.
-// If this is true, then directManager should be avoided:
+// resolvconfIsActive indicates whether the system appears to be using resolvconf.
+// If this is true, then dnsManualUp should be avoided:
 // resolvconf has exclusive ownership of /etc/resolv.conf.
-func isResolvconfActive() bool {
+func resolvconfIsActive() bool {
 	// Sanity-check first: if there is no resolvconf binary, then this is fruitless.
 	//
 	// However, this binary may be a shim like the one systemd-resolved provides.
@@ -57,31 +57,21 @@ func isResolvconfActive() bool {
 	return false
 }
 
-// resolvconfImpl enumerates supported implementations of the resolvconf CLI.
-type resolvconfImpl uint8
+// 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 resolvconfImpl = iota
+	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
 )
 
-func (impl resolvconfImpl) String() string {
-	switch impl {
-	case resolvconfOpenresolv:
-		return "openresolv"
-	case resolvconfLegacy:
-		return "legacy"
-	default:
-		return "unknown"
-	}
-}
-
-// getResolvconfImpl returns the implementation of resolvconf that appears to be in use.
-func getResolvconfImpl() resolvconfImpl {
+// 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 {
@@ -95,31 +85,21 @@ func getResolvconfImpl() resolvconfImpl {
 	return resolvconfOpenresolv
 }
 
-type resolvconfManager struct {
-	impl resolvconfImpl
-}
-
-func newResolvconfManager(mconfig ManagerConfig) managerImpl {
-	impl := getResolvconfImpl()
-	mconfig.Logf("resolvconf implementation is %s", impl)
-
-	return resolvconfManager{
-		impl: impl,
-	}
-}
-
 // 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"
 
-// Up implements managerImpl.
-func (m resolvconfManager) Up(config Config) error {
+// 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)
-	writeResolvConf(stdin, config.Nameservers, config.Domains) // dns_direct.go
+	dnsWriteConfig(stdin, config.Nameservers, config.Domains) // dns_direct.go
 
 	var cmd *exec.Cmd
-	switch m.impl {
+	switch implementation {
 	case resolvconfOpenresolv:
 		// Request maximal priority (metric 0) and exclusive mode.
 		cmd = exec.Command("resolvconf", "-m", "0", "-x", "-a", resolvconfConfigName)
@@ -137,10 +117,12 @@ func (m resolvconfManager) Up(config Config) error {
 	return nil
 }
 
-// Down implements managerImpl.
-func (m resolvconfManager) Down() error {
+// dnsResolvconfDown undoes the action of dnsResolvconfUp.
+func dnsResolvconfDown(interfaceName string) error {
+	implementation := getResolvconfImplementation()
+
 	var cmd *exec.Cmd
-	switch m.impl {
+	switch implementation {
 	case resolvconfOpenresolv:
 		cmd = exec.Command("resolvconf", "-f", "-d", resolvconfConfigName)
 	case resolvconfLegacy:

wgengine/router/dns_resolved.go

@@ -4,13 +4,14 @@
 
 // +build linux
 
-package dns
+package router
 
 import (
 	"context"
 	"errors"
 	"fmt"
 	"os/exec"
+	"time"
 
 	"github.com/godbus/dbus/v5"
 	"golang.org/x/sys/unix"
@@ -22,7 +23,7 @@ import (
 //
 // We only consider resolved to be the system resolver if the stub resolver is;
 // that is, if this address is the sole nameserver in /etc/resolved.conf.
-// In other cases, resolved may be managing the system DNS configuration directly.
+// In other cases, resolved may still be managing the system DNS configuration directly.
 // Then the nameserver list will be a concatenation of those for all
 // the interfaces that register their interest in being a default resolver with
 //   SetLinkDomains([]{{"~.", true}, ...})
@@ -35,6 +36,13 @@ import (
 // this address is, in fact, hard-coded into resolved.
 var resolvedListenAddr = netaddr.IPv4(127, 0, 0, 53)
 
+// dnsReconfigTimeout is the timeout for DNS reconfiguration.
+//
+// This is useful because certain conditions can cause indefinite hangs
+// (such as improper dbus auth followed by contextless dbus.Object.Call).
+// Such operations should be wrapped in a timeout context.
+const dnsReconfigTimeout = time.Second
+
 var errNotReady = errors.New("interface not ready")
 
 type resolvedLinkNameserver struct {
@@ -47,8 +55,8 @@ type resolvedLinkDomain struct {
 	RoutingOnly bool
 }
 
-// isResolvedActive determines if resolved is currently managing system DNS settings.
-func isResolvedActive() bool {
+// resolvedIsActive determines if resolved is currently managing system DNS settings.
+func resolvedIsActive() bool {
 	// systemd-resolved is never installed without systemd.
 	_, err := exec.LookPath("systemctl")
 	if err != nil {
@@ -61,7 +69,7 @@ func isResolvedActive() bool {
 		return false
 	}
 
-	config, err := readResolvConf()
+	config, err := dnsReadConfig()
 	if err != nil {
 		return false
 	}
@@ -74,16 +82,10 @@ func isResolvedActive() bool {
 	return false
 }
 
-// resolvedManager uses the systemd-resolved DBus API.
-type resolvedManager struct{}
-
-func newResolvedManager(mconfig ManagerConfig) managerImpl {
-	return resolvedManager{}
-}
-
-// Up implements managerImpl.
-func (m resolvedManager) Up(config Config) error {
-	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
+// dnsResolvedUp sets the DNS parameters for the Tailscale interface
+// to given nameservers and search domains using the resolved DBus API.
+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.
@@ -98,8 +100,6 @@ func (m resolvedManager) Up(config Config) error {
 		dbus.ObjectPath("/org/freedesktop/resolve1"),
 	)
 
-	// In principle, we could persist this in the manager struct
-	// if we knew that interface indices are persistent. This does not seem to be the case.
 	_, iface, err := interfaces.Tailscale()
 	if err != nil {
 		return fmt.Errorf("getting interface index: %w", err)
@@ -129,7 +129,7 @@ func (m resolvedManager) Up(config Config) error {
 		iface.Index, linkNameservers,
 	).Store()
 	if err != nil {
-		return fmt.Errorf("setLinkDNS: %w", err)
+		return fmt.Errorf("SetLinkDNS: %w", err)
 	}
 
 	var linkDomains = make([]resolvedLinkDomain, len(config.Domains))
@@ -145,15 +145,15 @@ func (m resolvedManager) Up(config Config) error {
 		iface.Index, linkDomains,
 	).Store()
 	if err != nil {
-		return fmt.Errorf("setLinkDomains: %w", err)
+		return fmt.Errorf("SetLinkDomains: %w", err)
 	}
 
 	return nil
 }
 
-// Down implements managerImpl.
-func (m resolvedManager) Down() error {
-	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
+// dnsResolvedDown undoes the changes made by dnsResolvedUp.
+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.

wgengine/router/ifconfig_windows.go

@@ -21,6 +21,7 @@ import (
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun"
 	"golang.org/x/sys/windows"
+	"golang.org/x/sys/windows/registry"
 	"tailscale.com/wgengine/winnet"
 )
 
@@ -116,13 +117,6 @@ func monitorDefaultRoutes(device *device.Device, autoMTU bool, tun *tun.NativeTu
 				return err
 			}
 			iface.NlMtu = mtu - 80
-			// If the TUN device was created with a smaller MTU,
-			// though, such as 1280, we don't want to go bigger than
-			// configured. (See the comment on minimalMTU in the
-			// wgengine package.)
-			if min, err := tun.MTU(); err == nil && min < int(iface.NlMtu) {
-				iface.NlMtu = uint32(min)
-			}
 			if iface.NlMtu < 576 {
 				iface.NlMtu = 576
 			}
@@ -163,6 +157,28 @@ func monitorDefaultRoutes(device *device.Device, autoMTU bool, tun *tun.NativeTu
 	return cb, nil
 }
 
+func setDNSDomains(g windows.GUID, dnsDomains []string) {
+	gs := g.String()
+	log.Printf("setDNSDomains(%v) guid=%v\n", dnsDomains, gs)
+	p := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + gs
+	key, err := registry.OpenKey(registry.LOCAL_MACHINE, p, registry.READ|registry.SET_VALUE)
+	if err != nil {
+		log.Printf("setDNSDomains(%v): open: %v\n", p, err)
+		return
+	}
+	defer key.Close()
+
+	// Windows only supports a single per-interface DNS domain.
+	dom := ""
+	if len(dnsDomains) > 0 {
+		dom = dnsDomains[0]
+	}
+	err = key.SetStringValue("Domain", dom)
+	if err != nil {
+		log.Printf("setDNSDomains(%v): SetStringValue: %v\n", p, err)
+	}
+}
+
 func setFirewall(ifcGUID *windows.GUID) (bool, error) {
 	c := ole.Connection{}
 	err := c.Initialize()
@@ -246,6 +262,8 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error {
 		}
 	}()
 
+	setDNSDomains(guid, cfg.Domains)
+
 	routes := []winipcfg.RouteData{}
 	var firstGateway4 *net.IP
 	var firstGateway6 *net.IP
@@ -340,6 +358,16 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error {
 		errAcc = err
 	}
 
+	var dnsIPs []net.IP
+	for _, ip := range cfg.Nameservers {
+		dnsIPs = append(dnsIPs, ip.IPAddr().IP)
+	}
+	err = iface.SetDNS(dnsIPs)
+	if err != nil && errAcc == nil {
+		log.Printf("setdns: %v\n", err)
+		errAcc = err
+	}
+
 	ipif, err := iface.GetIpInterface(winipcfg.AF_INET)
 	if err != nil {
 		log.Printf("getipif: %v\n", err)

wgengine/router/router.go

@@ -11,7 +11,6 @@ import (
 	"github.com/tailscale/wireguard-go/tun"
 	"inet.af/netaddr"
 	"tailscale.com/types/logger"
-	"tailscale.com/wgengine/router/dns"
 )
 
 // Router is responsible for managing the system network stack.
@@ -41,15 +40,6 @@ func New(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (Router, err
 // in case the Tailscale daemon terminated without closing the router.
 // No other state needs to be instantiated before this runs.
 func Cleanup(logf logger.Logf, interfaceName string) {
-	mconfig := dns.ManagerConfig{
-		Logf:          logf,
-		InterfaceName: interfaceName,
-		Cleanup:       true,
-	}
-	dns := dns.NewManager(mconfig)
-	if err := dns.Down(); err != nil {
-		logf("dns down: %v", err)
-	}
 	cleanup(logf, interfaceName)
 }
 
@@ -82,7 +72,7 @@ type Config struct {
 	LocalAddrs []netaddr.IPPrefix
 	Routes     []netaddr.IPPrefix // routes to point into the Tailscale interface
 
-	DNS dns.Config
+	DNSConfig
 
 	// Linux-only things below, ignored on other platforms.
 

wgengine/router/router_darwin.go

@@ -10,10 +10,55 @@ import (
 	"tailscale.com/types/logger"
 )
 
-func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (Router, error) {
-	return newUserspaceBSDRouter(logf, wgdev, tundev)
+type darwinRouter struct {
+	logf    logger.Logf
+	tunname string
+	Router
 }
 
-func cleanup(logger.Logf, string) {
-	// Nothing to do.
+func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (Router, error) {
+	tunname, err := tundev.Name()
+	if err != nil {
+		return nil, err
+	}
+
+	userspaceRouter, err := newUserspaceBSDRouter(logf, nil, tundev)
+	if err != nil {
+		return nil, err
+	}
+
+	return &darwinRouter{
+		logf:    logf,
+		tunname: tunname,
+		Router:  userspaceRouter,
+	}, nil
+}
+
+func (r *darwinRouter) Set(cfg *Config) error {
+	if cfg == nil {
+		cfg = &shutdownConfig
+	}
+
+	if SetRoutesFunc != nil {
+		return SetRoutesFunc(cfg)
+	}
+
+	return r.Router.Set(cfg)
+}
+
+func (r *darwinRouter) Up() error {
+	if SetRoutesFunc != nil {
+		return nil // bringing up the tunnel is handled externally
+	}
+	return r.Router.Up()
+}
+
+func upDNS(config DNSConfig, interfaceName string) error {
+	// Handled by IPNExtension
+	return nil
+}
+
+func downDNS(interfaceName string) error {
+	// Handled by IPNExtension
+	return nil
 }

wgengine/router/router_darwin_support.go

@@ -0,0 +1,23 @@
+// 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 router
+
+// SetRoutesFunc applies the given router settings to the OS network
+// stack. cfg is guaranteed to be non-nil.
+//
+// This is logically part of the router_darwin.go implementation, and
+// should not be used on other platforms.
+//
+// The code to reconfigure the network stack on MacOS and iOS is in
+// the non-open `ipn-go-bridge` package, which bridges between the Go
+// and Swift pieces of the application. The ipn-go-bridge sets
+// SetRoutesFunc at startup.
+//
+// So why isn't this in router_darwin.go? Because in the non-oss
+// repository, we build ipn-go-bridge when developing on Linux as well
+// as MacOS, so that we don't have to wait until the Mac CI to
+// discover that we broke it. So this one definition needs to exist in
+// both the darwin and linux builds. Hence this file and build tag.
+var SetRoutesFunc func(cfg *Config) error

wgengine/router/router_default.go

@@ -16,6 +16,6 @@ func newUserspaceRouter(logf logger.Logf, tunname string, dev *device.Device, tu
 	return NewFakeRouter(logf, tunname, dev, tuntap, netChanged)
 }
 
-func cleanup(logf logger.Logf, interfaceName string) {
-	// Nothing to do here.
+func cleanup() error {
+	return nil
 }

wgengine/router/router_freebsd.go

@@ -5,6 +5,8 @@
 package router
 
 import (
+	"fmt"
+
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun"
 	"tailscale.com/types/logger"
@@ -19,13 +21,34 @@ func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (
 	return newUserspaceBSDRouter(logf, nil, tundev)
 }
 
-func cleanup(logf logger.Logf, interfaceName string) {
-	// 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 destroy: %v\n%s", err, out)
+func upDNS(config DNSConfig, interfaceName string) error {
+	if len(config.Nameservers) == 0 {
+		return downDNS(interfaceName)
 	}
+
+	if resolvconfIsActive() {
+		if err := dnsResolvconfUp(config, interfaceName); err != nil {
+			return fmt.Errorf("resolvconf: %w")
+		}
+		return nil
+	}
+
+	if err := dnsDirectUp(config); err != nil {
+		return fmt.Errorf("direct: %w")
+	}
+	return nil
+}
+
+func downDNS(interfaceName string) error {
+	if resolvconfIsActive() {
+		if err := dnsResolvconfDown(interfaceName); err != nil {
+			return fmt.Errorf("resolvconf: %w")
+		}
+		return nil
+	}
+
+	if err := dnsDirectDown(); err != nil {
+		return fmt.Errorf("direct: %w")
+	}
+	return nil
 }

wgengine/router/router_linux.go

@@ -15,7 +15,6 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/types/logger"
-	"tailscale.com/wgengine/router/dns"
 )
 
 // The following bits are added to packet marks for Tailscale use.
@@ -85,7 +84,8 @@ type linuxRouter struct {
 	snatSubnetRoutes bool
 	netfilterMode    NetfilterMode
 
-	dns *dns.Manager
+	dnsMode   dnsMode
+	dnsConfig DNSConfig
 
 	ipt4 netfilterRunner
 	cmd  commandRunner
@@ -109,11 +109,6 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, netfilter netf
 	_, err := exec.Command("ip", "rule").Output()
 	ipRuleAvailable := (err == nil)
 
-	mconfig := dns.ManagerConfig{
-		Logf:          logf,
-		InterfaceName: tunname,
-	}
-
 	return &linuxRouter{
 		logf:            logf,
 		ipRuleAvailable: ipRuleAvailable,
@@ -121,7 +116,6 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, netfilter netf
 		netfilterMode:   NetfilterOff,
 		ipt4:            netfilter,
 		cmd:             cmd,
-		dns:             dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -139,12 +133,26 @@ func (r *linuxRouter) Up() error {
 		return err
 	}
 
+	switch {
+	// TODO(dmytro): enable resolved when per-domain resolvers are desired.
+	case resolvedIsActive():
+		r.dnsMode = dnsDirect
+	//	r.dnsMode = dnsResolved
+	case nmIsActive():
+		r.dnsMode = dnsNetworkManager
+	case resolvconfIsActive():
+		r.dnsMode = dnsResolvconf
+	default:
+		r.dnsMode = dnsDirect
+	}
+	r.logf("dns mode: %v", r.dnsMode)
+
 	return nil
 }
 
 func (r *linuxRouter) Close() error {
-	if err := r.dns.Down(); err != nil {
-		return fmt.Errorf("dns down: %v", err)
+	if err := r.downDNS(); err != nil {
+		return err
 	}
 	if err := r.downInterface(); err != nil {
 		return err
@@ -198,8 +206,12 @@ func (r *linuxRouter) Set(cfg *Config) error {
 	}
 	r.snatSubnetRoutes = cfg.SNATSubnetRoutes
 
-	if err := r.dns.Set(cfg.DNS); err != nil {
-		return fmt.Errorf("dns set: %v", err)
+	if !r.dnsConfig.EquivalentTo(cfg.DNSConfig) {
+		if err := r.upDNS(cfg.DNSConfig); err != nil {
+			r.logf("dns up: %v", err)
+		} else {
+			r.dnsConfig = cfg.DNSConfig
+		}
 	}
 
 	return nil
@@ -843,6 +855,68 @@ func normalizeCIDR(cidr netaddr.IPPrefix) string {
 	return fmt.Sprintf("%s/%d", nip, cidr.Bits)
 }
 
-func cleanup(logf logger.Logf, interfaceName string) {
-	// TODO(dmytro): clean up iptables.
+// upDNS updates the system DNS configuration to the given one.
+func (r *linuxRouter) upDNS(config DNSConfig) error {
+	if len(config.Nameservers) == 0 {
+		return r.downDNS()
+	}
+
+	switch r.dnsMode {
+	case dnsResolved:
+		if err := dnsResolvedUp(config); err != nil {
+			return fmt.Errorf("resolved: %w", err)
+		}
+	case dnsResolvconf:
+		if err := dnsResolvconfUp(config, r.tunname); err != nil {
+			return fmt.Errorf("resolvconf: %w", err)
+		}
+	case dnsNetworkManager:
+		if err := dnsNetworkManagerUp(config, r.tunname); err != nil {
+			return fmt.Errorf("network manager: %w", err)
+		}
+	case dnsDirect:
+		if err := dnsDirectUp(config); err != nil {
+			return fmt.Errorf("direct: %w", err)
+		}
+	}
+	return nil
+}
+
+// downDNS restores system DNS configuration to its state before upDNS.
+// It is idempotent (in particular, it does nothing if upDNS was never run).
+func (r *linuxRouter) downDNS() error {
+	switch r.dnsMode {
+	case dnsResolved:
+		if err := dnsResolvedDown(); err != nil {
+			return fmt.Errorf("resolved: %w", err)
+		}
+	case dnsResolvconf:
+		if err := dnsResolvconfDown(r.tunname); err != nil {
+			return fmt.Errorf("resolvconf: %w", err)
+		}
+	case dnsNetworkManager:
+		if err := dnsNetworkManagerDown(r.tunname); err != nil {
+			return fmt.Errorf("network manager: %w", err)
+		}
+	case dnsDirect:
+		if err := dnsDirectDown(); err != nil {
+			return fmt.Errorf("direct: %w", err)
+		}
+	}
+	return nil
+}
+
+func cleanup(logf logger.Logf, interfaceName string) {
+	// Note: we need not do anything for dnsResolved,
+	// as its settings are interface-bound and get cleaned up for us.
+	switch {
+	case resolvconfIsActive():
+		if err := dnsResolvconfDown(interfaceName); err != nil {
+			logf("down down: resolvconf: %v", err)
+		}
+	default:
+		if err := dnsDirectDown(); err != nil {
+			logf("dns down: direct: %v", err)
+		}
+	}
 }

wgengine/router/router_openbsd.go

@@ -14,7 +14,6 @@ import (
 	"github.com/tailscale/wireguard-go/tun"
 	"inet.af/netaddr"
 	"tailscale.com/types/logger"
-	"tailscale.com/wgengine/router/dns"
 )
 
 // For now this router only supports the WireGuard userspace implementation.
@@ -27,7 +26,7 @@ type openbsdRouter struct {
 	local   netaddr.IPPrefix
 	routes  map[netaddr.IPPrefix]struct{}
 
-	dns *dns.Manager
+	dnsConfig DNSConfig
 }
 
 func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (Router, error) {
@@ -35,16 +34,9 @@ func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (
 	if err != nil {
 		return nil, err
 	}
-
-	mconfig := dns.ManagerConfig{
-		Logf:          logf,
-		InterfaceName: tunname,
-	}
-
 	return &openbsdRouter{
 		logf:    logf,
 		tunname: tunname,
-		dns:     dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -70,10 +62,7 @@ func (r *openbsdRouter) Set(cfg *Config) error {
 	}
 
 	// TODO: support configuring multiple local addrs on interface.
-	if len(cfg.LocalAddrs) == 0 {
-		return nil
-	}
-	if len(cfg.LocalAddrs) > 1 {
+	if len(cfg.LocalAddrs) != 1 {
 		return errors.New("freebsd doesn't support setting multiple local addrs yet")
 	}
 	localAddr := cfg.LocalAddrs[0]
@@ -166,22 +155,26 @@ func (r *openbsdRouter) Set(cfg *Config) error {
 	r.local = localAddr
 	r.routes = newRoutes
 
-	if err := r.dns.Set(cfg.DNS); err != nil {
-		errq = fmt.Errorf("dns set: %v", err)
+	if !r.dnsConfig.EquivalentTo(cfg.DNSConfig) {
+		if err := dnsDirectUp(cfg.DNSConfig); err != nil {
+			errq = fmt.Errorf("dns up: direct: %v", err)
+		} else {
+			r.dnsConfig = cfg.DNSConfig
+		}
 	}
 
 	return errq
 }
 
 func (r *openbsdRouter) Close() error {
-	if err := r.dns.Down(); err != nil {
-		return fmt.Errorf("dns down: %v", err)
-	}
 	cleanup(r.logf, r.tunname)
 	return nil
 }
 
 func cleanup(logf logger.Logf, interfaceName string) {
+	if err := dnsDirectDown(); err != nil {
+		logf("dns down: direct: %v", err)
+	}
 	out, err := cmd("ifconfig", interfaceName, "down").CombinedOutput()
 	if err != nil {
 		logf("ifconfig down: %v\n%s", err, out)

wgengine/router/router_userspace_bsd.go

@@ -16,7 +16,6 @@ import (
 	"github.com/tailscale/wireguard-go/tun"
 	"inet.af/netaddr"
 	"tailscale.com/types/logger"
-	"tailscale.com/wgengine/router/dns"
 )
 
 type userspaceBSDRouter struct {
@@ -25,7 +24,7 @@ type userspaceBSDRouter struct {
 	local   netaddr.IPPrefix
 	routes  map[netaddr.IPPrefix]struct{}
 
-	dns *dns.Manager
+	dnsConfig DNSConfig
 }
 
 func newUserspaceBSDRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (Router, error) {
@@ -33,16 +32,9 @@ func newUserspaceBSDRouter(logf logger.Logf, _ *device.Device, tundev tun.Device
 	if err != nil {
 		return nil, err
 	}
-
-	mconfig := dns.ManagerConfig{
-		Logf:          logf,
-		InterfaceName: tunname,
-	}
-
 	return &userspaceBSDRouter{
 		logf:    logf,
 		tunname: tunname,
-		dns:     dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -149,17 +141,29 @@ func (r *userspaceBSDRouter) Set(cfg *Config) error {
 	r.local = localAddr
 	r.routes = newRoutes
 
-	if err := r.dns.Set(cfg.DNS); err != nil {
-		errq = fmt.Errorf("dns set: %v", err)
+	if !r.dnsConfig.EquivalentTo(cfg.DNSConfig) {
+		if err := upDNS(cfg.DNSConfig, r.tunname); err != nil {
+			errq = fmt.Errorf("dns up: %v", err)
+		} else {
+			r.dnsConfig = cfg.DNSConfig
+		}
 	}
 
 	return errq
 }
 
 func (r *userspaceBSDRouter) Close() error {
-	if err := r.dns.Down(); err != nil {
-		r.logf("dns down: %v", err)
-	}
-	// No interface cleanup is necessary during normal shutdown.
+	cleanup(r.logf, r.tunname)
 	return nil
 }
+
+func cleanup(logf logger.Logf, interfaceName string) {
+	if err := downDNS(interfaceName); err != nil {
+		logf("dns down: %v", err)
+	}
+
+	ifup := []string{"ifconfig", interfaceName, "down"}
+	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
+		logf("ifconfig down: %v\n%s", err, out)
+	}
+}

wgengine/router/router_windows.go

@@ -5,14 +5,12 @@
 package router
 
 import (
-	"fmt"
 	"log"
 
 	winipcfg "github.com/tailscale/winipcfg-go"
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun"
 	"tailscale.com/types/logger"
-	"tailscale.com/wgengine/router/dns"
 )
 
 type winRouter struct {
@@ -21,7 +19,6 @@ type winRouter struct {
 	nativeTun           *tun.NativeTun
 	wgdev               *device.Device
 	routeChangeCallback *winipcfg.RouteChangeCallback
-	dns                 *dns.Manager
 }
 
 func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (Router, error) {
@@ -29,20 +26,11 @@ func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Devic
 	if err != nil {
 		return nil, err
 	}
-
-	nativeTun := tundev.(*tun.NativeTun)
-	guid := nativeTun.GUID().String()
-	mconfig := dns.ManagerConfig{
-		Logf:          logf,
-		InterfaceName: guid,
-	}
-
 	return &winRouter{
 		logf:      logf,
 		wgdev:     wgdev,
 		tunname:   tunname,
-		nativeTun: nativeTun,
-		dns:       dns.NewManager(mconfig),
+		nativeTun: tundev.(*tun.NativeTun),
 	}, nil
 }
 
@@ -67,18 +55,10 @@ func (r *winRouter) Set(cfg *Config) error {
 		r.logf("ConfigureInterface: %v\n", err)
 		return err
 	}
-
-	if err := r.dns.Set(cfg.DNS); err != nil {
-		return fmt.Errorf("dns set: %w", err)
-	}
-
 	return nil
 }
 
 func (r *winRouter) Close() error {
-	if err := r.dns.Down(); err != nil {
-		return fmt.Errorf("dns down: %w", err)
-	}
 	if r.routeChangeCallback != nil {
 		r.routeChangeCallback.Unregister()
 	}
@@ -86,5 +66,5 @@ func (r *winRouter) Close() error {
 }
 
 func cleanup(logf logger.Logf, interfaceName string) {
-	// Nothing to do here.
+	// DNS is interface-bound, so nothing to do here.
 }

wgengine/tsdns/map.go

@@ -1,118 +0,0 @@
-// 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

@@ -1,138 +0,0 @@
-// 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

@@ -40,12 +40,23 @@ var ErrClosed = errors.New("closed")
 var (
 	errAllFailed      = errors.New("all upstream nameservers failed")
 	errFullQueue      = errors.New("request queue full")
-	errNoNameservers  = errors.New("no upstream nameservers set")
 	errMapNotSet      = errors.New("domain map not set")
 	errNotImplemented = errors.New("query type not implemented")
 	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.
@@ -131,10 +142,8 @@ 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
@@ -180,7 +189,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.nameToIP[domain]
+	addr, found := r.dnsMap.domainToIP[domain]
 	r.mu.RUnlock()
 
 	if !found {
@@ -258,7 +267,7 @@ func (r *Resolver) delegate(query []byte) ([]byte, error) {
 	r.mu.RUnlock()
 
 	if len(nameservers) == 0 {
-		return nil, errNoNameservers
+		return nil, errAllFailed
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), delegateTimeout)

wgengine/tsdns/tsdns_test.go

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

wgengine/userspace.go

@@ -13,11 +13,9 @@ import (
 	"fmt"
 	"io"
 	"log"
-	"net"
 	"os"
 	"os/exec"
 	"runtime"
-	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -32,11 +30,9 @@ import (
 	"tailscale.com/internal/deepprint"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/interfaces"
-	"tailscale.com/net/tsaddr"
 	"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"
@@ -71,31 +67,19 @@ const (
 	// (This includes peers that have never been idle, which
 	// effectively have infinite idleness)
 	lazyPeerIdleThreshold = 5 * time.Minute
-
-	// packetSendTimeUpdateFrequency controls how often we record
-	// the time that we wrote a packet to an IP address.
-	packetSendTimeUpdateFrequency = 10 * time.Second
-
-	// packetSendRecheckWireguardThreshold controls how long we can go
-	// between packet sends to an IP before checking to see
-	// whether this IP address needs to be added back to the
-	// Wireguard peer oconfig.
-	packetSendRecheckWireguardThreshold = 1 * time.Minute
 )
 
 type userspaceEngine struct {
-	logf      logger.Logf
-	reqCh     chan struct{}
-	waitCh    chan struct{} // chan is closed when first Close call completes; contrast with closing bool
-	timeNow   func() time.Time
-	tundev    *tstun.TUN
-	wgdev     *device.Device
-	router    router.Router
-	resolver  *tsdns.Resolver
-	magicConn *magicsock.Conn
-	linkMon   *monitor.Mon
-
-	testMaybeReconfigHook func() // for tests; if non-nil, fires if maybeReconfigWireguardLocked called
+	logf            logger.Logf
+	reqCh           chan struct{}
+	waitCh          chan struct{} // chan is closed when first Close call completes; contrast with closing bool
+	tundev          *tstun.TUN
+	wgdev           *device.Device
+	router          router.Router
+	resolver        *tsdns.Resolver
+	useTailscaleDNS bool
+	magicConn       *magicsock.Conn
+	linkMon         *monitor.Mon
 
 	// localAddrs is the set of IP addresses assigned to the local
 	// tunnel interface. It's used to reflect local packets
@@ -119,7 +103,7 @@ type userspaceEngine struct {
 	pingers        map[wgcfg.Key]*pinger // legacy pingers for pre-discovery peers
 	linkState      *interfaces.State
 
-	// Lock ordering: magicsock.Conn.mu, wgLock, then mu.
+	// Lock ordering: wgLock, then mu.
 }
 
 // RouterGen is the signature for a function that creates a
@@ -135,9 +119,13 @@ type EngineConfig struct {
 	RouterGen RouterGen
 	// ListenPort is the port on which the engine will listen.
 	ListenPort uint16
-	// Fake determines whether this engine is running in fake mode,
-	// which disables such features as DNS configuration and unrestricted ICMP Echo responses.
-	Fake bool
+	// EchoRespondToAll determines whether ICMP Echo requests incoming from Tailscale peers
+	// will be intercepted and responded to, regardless of the source host.
+	EchoRespondToAll bool
+	// UseTailscaleDNS determines whether DNS requests for names of the form <mynode>.<mydomain>.<root>
+	// directed to the designated Taislcale DNS address (see wgengine/tsdns)
+	// will be intercepted and resolved by a tsdns.Resolver.
+	UseTailscaleDNS bool
 }
 
 type Loggify struct {
@@ -152,11 +140,11 @@ func (l *Loggify) Write(b []byte) (int, error) {
 func NewFakeUserspaceEngine(logf logger.Logf, listenPort uint16) (Engine, error) {
 	logf("Starting userspace wireguard engine (FAKE tuntap device).")
 	conf := EngineConfig{
-		Logf:       logf,
-		TUN:        tstun.NewFakeTUN(),
-		RouterGen:  router.NewFake,
-		ListenPort: listenPort,
-		Fake:       true,
+		Logf:             logf,
+		TUN:              tstun.NewFakeTUN(),
+		RouterGen:        router.NewFake,
+		ListenPort:       listenPort,
+		EchoRespondToAll: true,
 	}
 	return NewUserspaceEngineAdvanced(conf)
 }
@@ -183,6 +171,8 @@ func NewUserspaceEngine(logf logger.Logf, tunname string, listenPort uint16) (En
 		TUN:        tun,
 		RouterGen:  router.New,
 		ListenPort: listenPort,
+		// TODO(dmytro): plumb this down.
+		UseTailscaleDNS: true,
 	}
 
 	e, err := NewUserspaceEngineAdvanced(conf)
@@ -202,19 +192,19 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) {
 	logf := conf.Logf
 
 	e := &userspaceEngine{
-		timeNow:  time.Now,
-		logf:     logf,
-		reqCh:    make(chan struct{}, 1),
-		waitCh:   make(chan struct{}),
-		tundev:   tstun.WrapTUN(logf, conf.TUN),
-		resolver: tsdns.NewResolver(logf, magicDNSDomain),
-		pingers:  make(map[wgcfg.Key]*pinger),
+		logf:            logf,
+		reqCh:           make(chan struct{}, 1),
+		waitCh:          make(chan struct{}),
+		tundev:          tstun.WrapTUN(logf, conf.TUN),
+		resolver:        tsdns.NewResolver(logf, magicDNSDomain),
+		useTailscaleDNS: conf.UseTailscaleDNS,
+		pingers:         make(map[wgcfg.Key]*pinger),
 	}
 	e.localAddrs.Store(map[packet.IP]bool{})
 	e.linkState, _ = getLinkState()
 
 	// Respond to all pings only in fake mode.
-	if conf.Fake {
+	if conf.EchoRespondToAll {
 		e.tundev.PostFilterIn = echoRespondToAll
 	}
 	e.tundev.PreFilterOut = e.handleLocalPackets
@@ -374,9 +364,11 @@ func echoRespondToAll(p *packet.ParsedPacket, t *tstun.TUN) filter.Response {
 // tailscaled directly. Other packets are allowed to proceed into the
 // main ACL filter.
 func (e *userspaceEngine) handleLocalPackets(p *packet.ParsedPacket, t *tstun.TUN) filter.Response {
-	if verdict := e.handleDNS(p, t); verdict == filter.Drop {
-		// local DNS handled the packet.
-		return filter.Drop
+	if e.useTailscaleDNS {
+		if verdict := e.handleDNS(p, t); verdict == filter.Drop {
+			// local DNS handled the packet.
+			return filter.Drop
+		}
 	}
 
 	if runtime.GOOS == "darwin" && e.isLocalAddr(p.DstIP) {
@@ -405,7 +397,7 @@ func (e *userspaceEngine) isLocalAddr(ip packet.IP) bool {
 func (e *userspaceEngine) handleDNS(p *packet.ParsedPacket, t *tstun.TUN) filter.Response {
 	if p.DstIP == magicDNSIP && p.DstPort == magicDNSPort && p.IPProto == packet.UDP {
 		request := tsdns.Packet{
-			Payload: append([]byte(nil), p.Payload()...),
+			Payload: p.Payload(),
 			Addr:    netaddr.IPPort{IP: p.SrcIP.Netaddr(), Port: p.SrcPort},
 		}
 		err := e.resolver.EnqueueRequest(request)
@@ -568,27 +560,13 @@ func (e *userspaceEngine) pinger(peerKey wgcfg.Key, ips []wgcfg.IP) {
 	p.run(ctx, peerKey, ips, srcIP)
 }
 
-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
+func updateSig(last *string, v interface{}) (changed bool) {
+	sig := deepprint.Hash(v)
+	if *last != sig {
+		*last = sig
+		return true
 	}
-	// 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
+	return false
 }
 
 // isTrimmablePeer reports whether p is a peer that we can trim out of the
@@ -600,10 +578,7 @@ func forceFullWireguardConfig(numPeers int) bool {
 // simplicity, have only one IP address (an IPv4 /32), which is the
 // common case for most peers. Subnet router nodes will just always be
 // created in the wireguard-go config.
-func isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool {
-	if forceFullWireguardConfig(numPeers) {
-		return false
-	}
+func isTrimmablePeer(p *wgcfg.Peer) bool {
 	if len(p.AllowedIPs) != 1 || len(p.Endpoints) != 1 {
 		return false
 	}
@@ -626,12 +601,12 @@ 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
 	}
-	now := e.timeNow()
 	e.recvActivityAt[dk] = now
 
 	// If the last activity time jumped a bunch (say, at least
@@ -640,7 +615,7 @@ func (e *userspaceEngine) noteReceiveActivity(dk tailcfg.DiscoKey) {
 	// 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 {
+	if was.IsZero() || now.Sub(was) < -lazyPeerIdleThreshold/2 {
 		e.maybeReconfigWireguardLocked()
 	}
 }
@@ -681,11 +656,6 @@ func discoKeyFromPeer(p *wgcfg.Peer) tailcfg.DiscoKey {
 
 // e.wgLock must be held.
 func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
-	if hook := e.testMaybeReconfigHook; hook != nil {
-		hook()
-		return nil
-	}
-
 	full := e.lastCfgFull
 
 	// Compute a minimal config to pass to wireguard-go
@@ -698,7 +668,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 	// 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 := e.timeNow().Add(-lazyPeerIdleThreshold)
+	activeCutoff := time.Now().Add(-lazyPeerIdleThreshold)
 
 	// Not all peers can be trimmed from the network map (see
 	// isTrimmablePeer).  For those are are trimmable, keep track
@@ -710,7 +680,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 
 	for i := range full.Peers {
 		p := &full.Peers[i]
-		if !isTrimmablePeer(p, len(full.Peers)) {
+		if !isTrimmablePeer(p) {
 			min.Peers = append(min.Peers, *p)
 			continue
 		}
@@ -723,7 +693,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 		}
 	}
 
-	if !deepprint.UpdateHash(&e.lastEngineSigTrim, min) {
+	if !updateSig(&e.lastEngineSigTrim, min) {
 		// No changes
 		return nil
 	}
@@ -774,19 +744,12 @@ func (e *userspaceEngine) updateActivityMapsLocked(trackDisco []tailcfg.DiscoKey
 		if fn == nil {
 			// This is the func that gets run on every outgoing packet for tracked IPs:
 			fn = func() {
-				now := e.timeNow().Unix()
-				old := atomic.LoadInt64(timePtr)
-
-				// How long's it been since we last sent a packet?
-				// For our first packet, old is Unix epoch time 0 (1970).
-				elapsedSec := now - old
-
-				if elapsedSec >= int64(packetSendTimeUpdateFrequency/time.Second) {
-					atomic.StoreInt64(timePtr, now)
+				now, old := time.Now().Unix(), atomic.LoadInt64(timePtr)
+				if old > now-10 {
+					return
 				}
-				// On a big jump, assume we might no longer be in the wireguard
-				// config and go check.
-				if elapsedSec >= int64(packetSendRecheckWireguardThreshold/time.Second) {
+				atomic.StoreInt64(timePtr, now)
+				if old == 0 || (now-old) <= 60 {
 					e.wgLock.Lock()
 					defer e.wgLock.Unlock()
 					e.maybeReconfigWireguardLocked()
@@ -825,8 +788,15 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	}
 	e.mu.Unlock()
 
-	engineChanged := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
-	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg)
+	// If the only nameserver is quad 100 (Magic DNS), set up the resolver appropriately.
+	if len(routerCfg.Nameservers) == 1 && routerCfg.Nameservers[0] == packet.IP(magicDNSIP).Netaddr() {
+		// TODO(dmytro): plumb dnsReadConfig here instead of hardcoding this.
+		e.resolver.SetNameservers([]string{"8.8.8.8:53"})
+		routerCfg.Domains = append([]string{magicDNSDomain}, routerCfg.Domains...)
+	}
+
+	engineChanged := updateSig(&e.lastEngineSigFull, cfg)
+	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}
@@ -846,15 +816,6 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	}
 
 	if routerChanged {
-		if routerCfg.DNS.Proxied {
-			ips := routerCfg.DNS.Nameservers
-			nameservers := make([]string, len(ips))
-			for i, ip := range ips {
-				nameservers[i] = net.JoinHostPort(ip.String(), "53")
-			}
-			e.resolver.SetNameservers(nameservers)
-			routerCfg.DNS.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}
-		}
 		e.logf("wgengine: Reconfig: configuring router")
 		if err := e.router.Set(routerCfg); err != nil {
 			return err
@@ -892,11 +853,6 @@ func (e *userspaceEngine) getStatusCallback() StatusCallback {
 // TODO: this function returns an error but it's always nil, and when
 // there's actually a problem it just calls log.Fatal. Why?
 func (e *userspaceEngine) getStatus() (*Status, error) {
-	// Grab derpConns before acquiring wgLock to not violate lock ordering;
-	// the DERPs method acquires magicsock.Conn.mu.
-	// (See comment in userspaceEngine's declaration.)
-	derpConns := e.magicConn.DERPs()
-
 	e.wgLock.Lock()
 	defer e.wgLock.Unlock()
 
@@ -1012,7 +968,7 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 	return &Status{
 		LocalAddrs: append([]string(nil), e.endpoints...),
 		Peers:      peers,
-		DERPs:      derpConns,
+		DERPs:      e.magicConn.DERPs(),
 	}, nil
 }
 

wgengine/userspace_test.go

@@ -1,88 +0,0 @@
-// 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 wgengine
-
-import (
-	"bytes"
-	"fmt"
-	"testing"
-	"time"
-
-	"tailscale.com/tailcfg"
-	"tailscale.com/types/key"
-	"tailscale.com/wgengine/tstun"
-)
-
-func TestNoteReceiveActivity(t *testing.T) {
-	now := time.Unix(1, 0)
-	tick := func(d time.Duration) { now = now.Add(d) }
-	var logBuf bytes.Buffer
-
-	confc := make(chan bool, 1)
-	gotConf := func() bool {
-		select {
-		case <-confc:
-			return true
-		default:
-			return false
-		}
-	}
-	e := &userspaceEngine{
-		timeNow:        func() time.Time { return now },
-		recvActivityAt: map[tailcfg.DiscoKey]time.Time{},
-		logf: func(format string, a ...interface{}) {
-			fmt.Fprintf(&logBuf, format, a...)
-		},
-		tundev:                new(tstun.TUN),
-		testMaybeReconfigHook: func() { confc <- true },
-	}
-	ra := e.recvActivityAt
-
-	dk := tailcfg.DiscoKey(key.NewPrivate().Public())
-
-	// Activity on an untracked key should do nothing.
-	e.noteReceiveActivity(dk)
-	if len(ra) != 0 {
-		t.Fatalf("unexpected growth in map: now has %d keys; want 0", len(ra))
-	}
-	if logBuf.Len() != 0 {
-		t.Fatalf("unexpected log write (and thus activity): %s", logBuf.Bytes())
-	}
-
-	// Now track it and expect updates.
-	ra[dk] = time.Time{}
-	e.noteReceiveActivity(dk)
-	if len(ra) != 1 {
-		t.Fatalf("unexpected growth in map: now has %d keys; want 1", len(ra))
-	}
-	if got := ra[dk]; got != now {
-		t.Fatalf("time in map = %v; want %v", got, now)
-	}
-	if !gotConf() {
-		t.Fatalf("didn't get expected reconfig")
-	}
-
-	// With updates 1 second apart, don't expect a reconfig.
-	for i := 0; i < 300; i++ {
-		tick(time.Second)
-		e.noteReceiveActivity(dk)
-		if len(ra) != 1 {
-			t.Fatalf("map len = %d; want 1", len(ra))
-		}
-		if got := ra[dk]; got != now {
-			t.Fatalf("time in map = %v; want %v", got, now)
-		}
-		if gotConf() {
-			t.Fatalf("unexpected reconfig")
-		}
-	}
-
-	// But if there's a big jump it should get an update.
-	tick(3 * time.Minute)
-	e.noteReceiveActivity(dk)
-	if !gotConf() {
-		t.Fatalf("expected config")
-	}
-}