all: make client use server-provided DERP map, add DERP region support

Instead of hard-coding the DERP map (except for cmd/tailscale netcheck
for now), get it from the control server at runtime.

And make the DERP map support multiple nodes per region with clients
picking the first one that's available. (The server will balance the
order presented to clients for load balancing)

This deletes the stunner package, merging it into the netcheck package
instead, to minimize all the config hooks that would've been
required.

Also fix some test flakes & races.

Fixes #387 (Don't hard-code the DERP map)
Updates #388 (Add DERP region support)
Fixes #399 (wgengine: flaky tests)

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

wgengine/magicsock/magicsock.go

@@ -9,7 +9,6 @@ package magicsock
 import (
 	"bytes"
 	"context"
-	"crypto/tls"
 	"encoding/binary"
 	"errors"
 	"fmt"
@@ -17,6 +16,7 @@ import (
 	"math/rand"
 	"net"
 	"os"
+	"reflect"
 	"sort"
 	"strconv"
 	"strings"
@@ -32,7 +32,6 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/derp"
 	"tailscale.com/derp/derphttp"
-	"tailscale.com/derp/derpmap"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/dnscache"
 	"tailscale.com/net/interfaces"
@@ -55,7 +54,6 @@ type Conn struct {
 	epFunc       func(endpoints []string)
 	logf         logger.Logf
 	sendLogLimit *rate.Limiter
-	derps        *derpmap.World
 	netChecker   *netcheck.Client
 
 	// bufferedIPv4From and bufferedIPv4Packet are owned by
@@ -76,7 +74,8 @@ type Conn struct {
 
 	mu sync.Mutex // guards all following fields
 
-	closed bool
+	started bool
+	closed  bool
 
 	endpointsUpdateWaiter *sync.Cond
 	endpointsUpdateActive bool
@@ -104,13 +103,12 @@ type Conn struct {
 	netInfoFunc func(*tailcfg.NetInfo) // nil until set
 	netInfoLast *tailcfg.NetInfo
 
-	wantDerp      bool
-	privateKey    key.Private
-	myDerp        int           // nearest DERP server; 0 means none/unknown
-	derpStarted   chan struct{} // closed on first connection to DERP; for tests
-	activeDerp    map[int]activeDerp
-	prevDerp      map[int]*syncs.WaitGroupChan
-	derpTLSConfig *tls.Config // normally nil; used by tests
+	derpMap     *tailcfg.DERPMap // nil (or zero regions/nodes) means DERP is disabled
+	privateKey  key.Private
+	myDerp      int                // nearest DERP region ID; 0 means none/unknown
+	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
 
 	// derpRoute contains optional alternate routes to use as an
 	// optimization instead of contacting a peer via their home
@@ -196,14 +194,9 @@ type Options struct {
 	// Zero means to pick one automatically.
 	Port uint16
 
-	// DERPs, if non-nil, is used instead of derpmap.Prod.
-	DERPs *derpmap.World
-
 	// EndpointsFunc optionally provides a func to be called when
 	// endpoints change. The called func does not own the slice.
 	EndpointsFunc func(endpoint []string)
-
-	derpTLSConfig *tls.Config // normally nil; used by tests
 }
 
 func (o *Options) logf() logger.Logf {
@@ -220,37 +213,39 @@ func (o *Options) endpointsFunc() func([]string) {
 	return o.EndpointsFunc
 }
 
-// Listen creates a magic Conn listening on opts.Port.
-// As the set of possible endpoints for a Conn changes, the
-// callback opts.EndpointsFunc is called.
-func Listen(opts Options) (*Conn, error) {
+// newConn is the error-free, network-listening-side-effect-free based
+// of NewConn. Mostly for tests.
+func newConn() *Conn {
 	c := &Conn{
-		pconnPort:     opts.Port,
-		logf:          opts.logf(),
-		epFunc:        opts.endpointsFunc(),
-		sendLogLimit:  rate.NewLimiter(rate.Every(1*time.Minute), 1),
-		addrsByUDP:    make(map[netaddr.IPPort]*AddrSet),
-		addrsByKey:    make(map[key.Public]*AddrSet),
-		wantDerp:      true,
-		derpRecvCh:    make(chan derpReadResult),
-		udpRecvCh:     make(chan udpReadResult),
-		derpTLSConfig: opts.derpTLSConfig,
-		derpStarted:   make(chan struct{}),
-		derps:         opts.DERPs,
-		peerLastDerp:  make(map[key.Public]int),
+		sendLogLimit: rate.NewLimiter(rate.Every(1*time.Minute), 1),
+		addrsByUDP:   make(map[netaddr.IPPort]*AddrSet),
+		addrsByKey:   make(map[key.Public]*AddrSet),
+		derpRecvCh:   make(chan derpReadResult),
+		udpRecvCh:    make(chan udpReadResult),
+		derpStarted:  make(chan struct{}),
+		peerLastDerp: make(map[key.Public]int),
 	}
 	c.endpointsUpdateWaiter = sync.NewCond(&c.mu)
+	return c
+}
+
+// NewConn creates a magic Conn listening on opts.Port.
+// As the set of possible endpoints for a Conn changes, the
+// callback opts.EndpointsFunc is called.
+//
+// It doesn't start doing anything until Start is called.
+func NewConn(opts Options) (*Conn, error) {
+	c := newConn()
+	c.pconnPort = opts.Port
+	c.logf = opts.logf()
+	c.epFunc = opts.endpointsFunc()
 
 	if err := c.initialBind(); err != nil {
 		return nil, err
 	}
 
 	c.connCtx, c.connCtxCancel = context.WithCancel(context.Background())
-	if c.derps == nil {
-		c.derps = derpmap.Prod()
-	}
 	c.netChecker = &netcheck.Client{
-		DERP:         c.derps,
 		Logf:         logger.WithPrefix(c.logf, "netcheck: "),
 		GetSTUNConn4: func() netcheck.STUNConn { return c.pconn4 },
 	}
@@ -259,6 +254,18 @@ func Listen(opts Options) (*Conn, error) {
 	}
 
 	c.ignoreSTUNPackets()
+
+	return c, nil
+}
+
+func (c *Conn) Start() {
+	c.mu.Lock()
+	if c.started {
+		panic("duplicate Start call")
+	}
+	c.started = true
+	c.mu.Unlock()
+
 	c.ReSTUN("initial")
 
 	// We assume that LinkChange notifications are plumbed through well
@@ -267,8 +274,6 @@ func Listen(opts Options) (*Conn, error) {
 		go c.periodicReSTUN()
 	}
 	go c.periodicDerpCleanup()
-
-	return c, nil
 }
 
 func (c *Conn) donec() <-chan struct{} { return c.connCtx.Done() }
@@ -278,10 +283,6 @@ func (c *Conn) ignoreSTUNPackets() {
 	c.stunReceiveFunc.Store(func([]byte, *net.UDPAddr) {})
 }
 
-// runs in its own goroutine until ctx is shut down.
-// Whenever c.startEpUpdate receives a value, it starts an
-// STUN endpoint lookup.
-//
 // c.mu must NOT be held.
 func (c *Conn) updateEndpoints(why string) {
 	defer func() {
@@ -326,7 +327,11 @@ func (c *Conn) setEndpoints(endpoints []string) (changed bool) {
 }
 
 func (c *Conn) updateNetInfo(ctx context.Context) (*netcheck.Report, error) {
-	if DisableSTUNForTesting {
+	c.mu.Lock()
+	dm := c.derpMap
+	c.mu.Unlock()
+
+	if DisableSTUNForTesting || dm == nil {
 		return new(netcheck.Report), nil
 	}
 
@@ -336,7 +341,7 @@ func (c *Conn) updateNetInfo(ctx context.Context) (*netcheck.Report, error) {
 	c.stunReceiveFunc.Store(c.netChecker.ReceiveSTUNPacket)
 	defer c.ignoreSTUNPackets()
 
-	report, err := c.netChecker.GetReport(ctx)
+	report, err := c.netChecker.GetReport(ctx, dm)
 	if err != nil {
 		return nil, err
 	}
@@ -346,8 +351,11 @@ func (c *Conn) updateNetInfo(ctx context.Context) (*netcheck.Report, error) {
 		MappingVariesByDestIP: report.MappingVariesByDestIP,
 		HairPinning:           report.HairPinning,
 	}
-	for server, d := range report.DERPLatency {
-		ni.DERPLatency[server] = d.Seconds()
+	for rid, d := range report.RegionV4Latency {
+		ni.DERPLatency[fmt.Sprintf("%d-v4", rid)] = d.Seconds()
+	}
+	for rid, d := range report.RegionV6Latency {
+		ni.DERPLatency[fmt.Sprintf("%d-v6", rid)] = d.Seconds()
 	}
 	ni.WorkingIPv6.Set(report.IPv6)
 	ni.WorkingUDP.Set(report.UDP)
@@ -380,9 +388,12 @@ func (c *Conn) pickDERPFallback() int {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	ids := c.derps.IDs()
+	if !c.wantDerpLocked() {
+		return 0
+	}
+	ids := c.derpMap.RegionIDs()
 	if len(ids) == 0 {
-		// No DERP nodes registered.
+		// No DERP regions in non-nil map.
 		return 0
 	}
 
@@ -458,7 +469,7 @@ func (c *Conn) SetNetInfoCallback(fn func(*tailcfg.NetInfo)) {
 func (c *Conn) setNearestDERP(derpNum int) (wantDERP bool) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if !c.wantDerp {
+	if !c.wantDerpLocked() {
 		c.myDerp = 0
 		return false
 	}
@@ -476,7 +487,7 @@ func (c *Conn) setNearestDERP(derpNum int) (wantDERP bool) {
 
 	// On change, notify all currently connected DERP servers and
 	// start connecting to our home DERP if we are not already.
-	c.logf("magicsock: home is now derp-%v (%v)", derpNum, c.derps.ServerByID(derpNum).Geo)
+	c.logf("magicsock: home is now derp-%v (%v)", derpNum, c.derpMap.Regions[derpNum].RegionCode)
 	for i, ad := range c.activeDerp {
 		go ad.c.NotePreferred(i == c.myDerp)
 	}
@@ -791,11 +802,11 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 	if !addr.IP.Equal(derpMagicIP) {
 		return nil
 	}
-	nodeID := addr.Port
+	regionID := addr.Port
 
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if !c.wantDerp || c.closed {
+	if !c.wantDerpLocked() || c.closed {
 		return nil
 	}
 	if c.privateKey.IsZero() {
@@ -807,10 +818,10 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 	// first. If so, might as well use it. (It's a little
 	// arbitrary whether we use this one vs. the reverse route
 	// below when we have both.)
-	ad, ok := c.activeDerp[nodeID]
+	ad, ok := c.activeDerp[regionID]
 	if ok {
 		*ad.lastWrite = time.Now()
-		c.setPeerLastDerpLocked(peer, nodeID, nodeID)
+		c.setPeerLastDerpLocked(peer, regionID, regionID)
 		return ad.writeCh
 	}
 
@@ -823,7 +834,7 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 	if !peer.IsZero() && debugUseDerpRoute {
 		if r, ok := c.derpRoute[peer]; ok {
 			if ad, ok := c.activeDerp[r.derpID]; ok && ad.c == r.dc {
-				c.setPeerLastDerpLocked(peer, r.derpID, nodeID)
+				c.setPeerLastDerpLocked(peer, r.derpID, regionID)
 				*ad.lastWrite = time.Now()
 				return ad.writeCh
 			}
@@ -834,7 +845,7 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 	if !peer.IsZero() {
 		why = peerShort(peer)
 	}
-	c.logf("magicsock: adding connection to derp-%v for %v", nodeID, why)
+	c.logf("magicsock: adding connection to derp-%v for %v", regionID, why)
 
 	firstDerp := false
 	if c.activeDerp == nil {
@@ -842,22 +853,23 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 		c.activeDerp = make(map[int]activeDerp)
 		c.prevDerp = make(map[int]*syncs.WaitGroupChan)
 	}
-	derpSrv := c.derps.ServerByID(nodeID)
-	if derpSrv == nil || derpSrv.HostHTTPS == "" {
+	if c.derpMap == nil || c.derpMap.Regions[regionID] == nil {
 		return nil
 	}
 
 	// Note that derphttp.NewClient does not dial the server
 	// so it is safe to do under the mu lock.
-	dc, err := derphttp.NewClient(c.privateKey, "https://"+derpSrv.HostHTTPS+"/derp", c.logf)
-	if err != nil {
-		c.logf("magicsock: derphttp.NewClient: node %d, host %q invalid? err: %v", nodeID, derpSrv.HostHTTPS, err)
-		return nil
-	}
+	dc := derphttp.NewRegionClient(c.privateKey, c.logf, func() *tailcfg.DERPRegion {
+		c.mu.Lock()
+		defer c.mu.Unlock()
+		if c.derpMap == nil {
+			return nil
+		}
+		return c.derpMap.Regions[regionID]
+	})
 
-	dc.NotePreferred(c.myDerp == nodeID)
+	dc.NotePreferred(c.myDerp == regionID)
 	dc.DNSCache = dnscache.Get()
-	dc.TLSConfig = c.derpTLSConfig
 
 	ctx, cancel := context.WithCancel(c.connCtx)
 	ch := make(chan derpWriteRequest, bufferedDerpWritesBeforeDrop)
@@ -868,21 +880,21 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 	ad.lastWrite = new(time.Time)
 	*ad.lastWrite = time.Now()
 	ad.createTime = time.Now()
-	c.activeDerp[nodeID] = ad
+	c.activeDerp[regionID] = ad
 	c.logActiveDerpLocked()
-	c.setPeerLastDerpLocked(peer, nodeID, nodeID)
+	c.setPeerLastDerpLocked(peer, regionID, regionID)
 
 	// Build a startGate for the derp reader+writer
 	// goroutines, so they don't start running until any
 	// previous generation is closed.
 	startGate := syncs.ClosedChan()
-	if prev := c.prevDerp[nodeID]; prev != nil {
+	if prev := c.prevDerp[regionID]; prev != nil {
 		startGate = prev.DoneChan()
 	}
 	// And register a WaitGroup(Chan) for this generation.
 	wg := syncs.NewWaitGroupChan()
 	wg.Add(2)
-	c.prevDerp[nodeID] = wg
+	c.prevDerp[regionID] = wg
 
 	if firstDerp {
 		startGate = c.derpStarted
@@ -899,37 +911,37 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 }
 
 // setPeerLastDerpLocked notes that peer is now being written to via
-// provided DERP node nodeID, and that that advertises a DERP home
-// node of homeID.
+// the provided DERP regionID, and that the peer advertises a DERP
+// home region ID of homeID.
 //
 // If there's any change, it logs.
 //
 // c.mu must be held.
-func (c *Conn) setPeerLastDerpLocked(peer key.Public, nodeID, homeID int) {
+func (c *Conn) setPeerLastDerpLocked(peer key.Public, regionID, homeID int) {
 	if peer.IsZero() {
 		return
 	}
 	old := c.peerLastDerp[peer]
-	if old == nodeID {
+	if old == regionID {
 		return
 	}
-	c.peerLastDerp[peer] = nodeID
+	c.peerLastDerp[peer] = regionID
 
 	var newDesc string
 	switch {
-	case nodeID == homeID && nodeID == c.myDerp:
+	case regionID == homeID && regionID == c.myDerp:
 		newDesc = "shared home"
-	case nodeID == homeID:
+	case regionID == homeID:
 		newDesc = "their home"
-	case nodeID == c.myDerp:
+	case regionID == c.myDerp:
 		newDesc = "our home"
-	case nodeID != homeID:
+	case regionID != homeID:
 		newDesc = "alt"
 	}
 	if old == 0 {
-		c.logf("magicsock: derp route for %s set to derp-%d (%s)", peerShort(peer), nodeID, newDesc)
+		c.logf("magicsock: derp route for %s set to derp-%d (%s)", peerShort(peer), regionID, newDesc)
 	} else {
-		c.logf("magicsock: derp route for %s changed from derp-%d => derp-%d (%s)", peerShort(peer), old, nodeID, newDesc)
+		c.logf("magicsock: derp route for %s changed from derp-%d => derp-%d (%s)", peerShort(peer), old, regionID, newDesc)
 	}
 }
 
@@ -1284,18 +1296,27 @@ func (c *Conn) UpdatePeers(newPeers map[key.Public]struct{}) {
 	}
 }
 
-// SetDERPEnabled controls whether DERP is used.
-// New connections have it enabled by default.
-func (c *Conn) SetDERPEnabled(wantDerp bool) {
+// SetDERPMap controls which (if any) DERP servers are used.
+// A nil value means to disable DERP; it's disabled by default.
+func (c *Conn) SetDERPMap(dm *tailcfg.DERPMap) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	c.wantDerp = wantDerp
-	if !wantDerp {
-		c.closeAllDerpLocked("derp-disabled")
+	if reflect.DeepEqual(dm, c.derpMap) {
+		return
 	}
+
+	c.derpMap = dm
+	if dm == nil {
+		c.closeAllDerpLocked("derp-disabled")
+		return
+	}
+
+	go c.ReSTUN("derp-map-update")
 }
 
+func (c *Conn) wantDerpLocked() bool { return c.derpMap != nil }
+
 // c.mu must be held.
 func (c *Conn) closeAllDerpLocked(why string) {
 	if len(c.activeDerp) == 0 {
@@ -1352,7 +1373,7 @@ func (c *Conn) logEndpointChange(endpoints []string, reasons map[string]string)
 }
 
 // c.mu must be held.
-func (c *Conn) foreachActiveDerpSortedLocked(fn func(nodeID int, ad activeDerp)) {
+func (c *Conn) foreachActiveDerpSortedLocked(fn func(regionID int, ad activeDerp)) {
 	if len(c.activeDerp) < 2 {
 		for id, ad := range c.activeDerp {
 			fn(id, ad)
@@ -1473,6 +1494,9 @@ func (c *Conn) periodicDerpCleanup() {
 func (c *Conn) ReSTUN(why string) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
+	if !c.started {
+		panic("call to ReSTUN before Start")
+	}
 	if c.closed {
 		// raced with a shutdown.
 		return

wgengine/magicsock/magicsock_test.go

@@ -27,6 +27,7 @@ import (
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/derp/derpmap"
 	"tailscale.com/stun/stuntest"
+	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
@@ -54,7 +55,7 @@ func (c *Conn) WaitReady(t *testing.T) {
 	}
 }
 
-func TestListen(t *testing.T) {
+func TestNewConn(t *testing.T) {
 	tstest.PanicOnLog()
 	rc := tstest.NewResourceCheck()
 	defer rc.Assert(t)
@@ -70,9 +71,8 @@ func TestListen(t *testing.T) {
 	defer stunCleanupFn()
 
 	port := pickPort(t)
-	conn, err := Listen(Options{
+	conn, err := NewConn(Options{
 		Port:          port,
-		DERPs:         derpmap.NewTestWorld(stunAddr),
 		EndpointsFunc: epFunc,
 		Logf:          t.Logf,
 	})
@@ -80,6 +80,8 @@ func TestListen(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer conn.Close()
+	conn.Start()
+	conn.SetDERPMap(stuntest.DERPMapOf(stunAddr.String()))
 
 	go func() {
 		var pkt [64 << 10]byte
@@ -136,9 +138,8 @@ func TestPickDERPFallback(t *testing.T) {
 	rc := tstest.NewResourceCheck()
 	defer rc.Assert(t)
 
-	c := &Conn{
-		derps: derpmap.Prod(),
-	}
+	c := newConn()
+	c.derpMap = derpmap.Prod()
 	a := c.pickDERPFallback()
 	if a == 0 {
 		t.Fatalf("pickDERPFallback returned 0")
@@ -156,7 +157,8 @@ func TestPickDERPFallback(t *testing.T) {
 	// distribution over nodes works.
 	got := map[int]int{}
 	for i := 0; i < 50; i++ {
-		c = &Conn{derps: derpmap.Prod()}
+		c = newConn()
+		c.derpMap = derpmap.Prod()
 		got[c.pickDERPFallback()]++
 	}
 	t.Logf("distribution: %v", got)
@@ -236,7 +238,7 @@ func parseCIDR(t *testing.T, addr string) wgcfg.CIDR {
 	return cidr
 }
 
-func runDERP(t *testing.T, logf logger.Logf) (s *derp.Server, addr string, cleanupFn func()) {
+func runDERP(t *testing.T, logf logger.Logf) (s *derp.Server, addr *net.TCPAddr, cleanupFn func()) {
 	var serverPrivateKey key.Private
 	if _, err := crand.Read(serverPrivateKey[:]); err != nil {
 		t.Fatal(err)
@@ -250,14 +252,13 @@ func runDERP(t *testing.T, logf logger.Logf) (s *derp.Server, addr string, clean
 	httpsrv.StartTLS()
 	logf("DERP server URL: %s", httpsrv.URL)
 
-	addr = strings.TrimPrefix(httpsrv.URL, "https://")
 	cleanupFn = func() {
 		httpsrv.CloseClientConnections()
 		httpsrv.Close()
 		s.Close()
 	}
 
-	return s, addr, cleanupFn
+	return s, httpsrv.Listener.Addr().(*net.TCPAddr), cleanupFn
 }
 
 // devLogger returns a wireguard-go device.Logger that writes
@@ -286,13 +287,14 @@ func TestDeviceStartStop(t *testing.T) {
 	rc := tstest.NewResourceCheck()
 	defer rc.Assert(t)
 
-	conn, err := Listen(Options{
+	conn, err := NewConn(Options{
 		EndpointsFunc: func(eps []string) {},
 		Logf:          t.Logf,
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
+	conn.Start()
 	defer conn.Close()
 
 	tun := tuntest.NewChannelTUN()
@@ -337,48 +339,58 @@ func TestTwoDevicePing(t *testing.T) {
 	// all log using the "current" t.Logf function. Sigh.
 	logf, setT := makeNestable(t)
 
-	// Wipe default DERP list, add local server.
-	// (Do it now, or derpHost will try to connect to derp1.tailscale.com.)
 	derpServer, derpAddr, derpCleanupFn := runDERP(t, logf)
 	defer derpCleanupFn()
-
 	stunAddr, stunCleanupFn := stuntest.Serve(t)
 	defer stunCleanupFn()
 
-	derps := derpmap.NewTestWorldWith(&derpmap.Server{
-		ID:        1,
-		HostHTTPS: derpAddr,
-		STUN4:     stunAddr,
-		Geo:       "Testopolis",
-	})
+	derpMap := &tailcfg.DERPMap{
+		Regions: map[int]*tailcfg.DERPRegion{
+			1: &tailcfg.DERPRegion{
+				RegionID:   1,
+				RegionCode: "test",
+				Nodes: []*tailcfg.DERPNode{
+					{
+						Name:         "t1",
+						RegionID:     1,
+						HostName:     "test-node.unused",
+						IPv4:         "127.0.0.1",
+						IPv6:         "none",
+						STUNPort:     stunAddr.Port,
+						DERPTestPort: derpAddr.Port,
+					},
+				},
+			},
+		},
+	}
 
 	epCh1 := make(chan []string, 16)
-	conn1, err := Listen(Options{
-		Logf:  logger.WithPrefix(logf, "conn1: "),
-		DERPs: derps,
+	conn1, err := NewConn(Options{
+		Logf: logger.WithPrefix(logf, "conn1: "),
 		EndpointsFunc: func(eps []string) {
 			epCh1 <- eps
 		},
-		derpTLSConfig: &tls.Config{InsecureSkipVerify: true},
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer conn1.Close()
+	conn1.Start()
+	conn1.SetDERPMap(derpMap)
 
 	epCh2 := make(chan []string, 16)
-	conn2, err := Listen(Options{
-		Logf:  logger.WithPrefix(logf, "conn2: "),
-		DERPs: derps,
+	conn2, err := NewConn(Options{
+		Logf: logger.WithPrefix(logf, "conn2: "),
 		EndpointsFunc: func(eps []string) {
 			epCh2 <- eps
 		},
-		derpTLSConfig: &tls.Config{InsecureSkipVerify: true},
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer conn2.Close()
+	conn2.Start()
+	conn2.SetDERPMap(derpMap)
 
 	ports := []uint16{conn1.LocalPort(), conn2.LocalPort()}
 	cfgs := makeConfigs(t, ports)

wgengine/userspace.go

@@ -8,6 +8,7 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -49,7 +50,7 @@ const minimalMTU = 1280
 type userspaceEngine struct {
 	logf      logger.Logf
 	reqCh     chan struct{}
-	waitCh    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
@@ -61,6 +62,7 @@ type userspaceEngine struct {
 	lastCfg      wgcfg.Config
 
 	mu             sync.Mutex // guards following; see lock order comment below
+	closing        bool       // Close was called (even if we're still closing)
 	statusCallback StatusCallback
 	peerSequence   []wgcfg.Key
 	endpoints      []string
@@ -149,7 +151,7 @@ func newUserspaceEngineAdvanced(logf logger.Logf, tundev *tstun.TUN, routerGen R
 		Port:          listenPort,
 		EndpointsFunc: endpointsFn,
 	}
-	e.magicConn, err = magicsock.Listen(magicsockOpts)
+	e.magicConn, err = magicsock.NewConn(magicsockOpts)
 	if err != nil {
 		tundev.Close()
 		return nil, fmt.Errorf("wgengine: %v", err)
@@ -210,6 +212,7 @@ func newUserspaceEngineAdvanced(logf logger.Logf, tundev *tstun.TUN, routerGen R
 	// routers do not Read or Write, but do access native interfaces.
 	e.router, err = routerGen(logf, e.wgdev, e.tundev.Unwrap())
 	if err != nil {
+		e.magicConn.Close()
 		return nil, err
 	}
 
@@ -235,16 +238,19 @@ func newUserspaceEngineAdvanced(logf logger.Logf, tundev *tstun.TUN, routerGen R
 
 	e.wgdev.Up()
 	if err := e.router.Up(); err != nil {
+		e.magicConn.Close()
 		e.wgdev.Close()
 		return nil, err
 	}
 	// TODO(danderson): we should delete this. It's pointless to apply
 	// a no-op settings here.
 	if err := e.router.Set(nil); err != nil {
+		e.magicConn.Close()
 		e.wgdev.Close()
 		return nil, err
 	}
 	e.linkMon.Start()
+	e.magicConn.Start()
 
 	return e, nil
 }
@@ -407,6 +413,13 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 	e.wgLock.Lock()
 	defer e.wgLock.Unlock()
 
+	e.mu.Lock()
+	closing := e.closing
+	e.mu.Unlock()
+	if closing {
+		return nil, errors.New("engine closing; no status")
+	}
+
 	if e.wgdev == nil {
 		// RequestStatus was invoked before the wgengine has
 		// finished initializing. This can happen when wgegine
@@ -553,6 +566,11 @@ func (e *userspaceEngine) RequestStatus() {
 
 func (e *userspaceEngine) Close() {
 	e.mu.Lock()
+	if e.closing {
+		e.mu.Unlock()
+		return
+	}
+	e.closing = true
 	for key, cancel := range e.pingers {
 		delete(e.pingers, key)
 		cancel()
@@ -614,8 +632,8 @@ func (e *userspaceEngine) SetNetInfoCallback(cb NetInfoCallback) {
 	e.magicConn.SetNetInfoCallback(cb)
 }
 
-func (e *userspaceEngine) SetDERPEnabled(v bool) {
-	e.magicConn.SetDERPEnabled(v)
+func (e *userspaceEngine) SetDERPMap(dm *tailcfg.DERPMap) {
+	e.magicConn.SetDERPMap(dm)
 }
 
 func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) {

wgengine/watchdog.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"tailscale.com/ipn/ipnstate"
+	"tailscale.com/tailcfg"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/router"
 )
@@ -88,8 +89,8 @@ func (e *watchdogEngine) RequestStatus() {
 func (e *watchdogEngine) LinkChange(isExpensive bool) {
 	e.watchdog("LinkChange", func() { e.wrap.LinkChange(isExpensive) })
 }
-func (e *watchdogEngine) SetDERPEnabled(v bool) {
-	e.watchdog("SetDERPEnabled", func() { e.wrap.SetDERPEnabled(v) })
+func (e *watchdogEngine) SetDERPMap(m *tailcfg.DERPMap) {
+	e.watchdog("SetDERPMap", func() { e.wrap.SetDERPMap(m) })
 }
 func (e *watchdogEngine) Close() {
 	e.watchdog("Close", e.wrap.Close)

wgengine/wgengine.go

@@ -95,9 +95,10 @@ type Engine interface {
 	// action on.
 	LinkChange(isExpensive bool)
 
-	// SetDERPEnabled controls whether DERP is enabled.
-	// It starts enabled by default.
-	SetDERPEnabled(bool)
+	// SetDERPMap controls which (if any) DERP servers are used.
+	// If nil, DERP is disabled. It starts disabled until a DERP map
+	// is configured.
+	SetDERPMap(*tailcfg.DERPMap)
 
 	// SetNetInfoCallback sets the function to call when a
 	// new NetInfo summary is available.