wgengine/tsdns: return NOERROR instead of NOTIMP for most records

This is what every other DNS resolver I could find does, so tsdns
should do it to. This also helps avoid weird error messages about
non-existent records being unimplemented, and thus fixes #848.

Signed-off-by: Smitty <me@smitop.com>

api.md

@@ -5,28 +5,7 @@ The Tailscale API is a (mostly) RESTful API. Typically, POST bodies should be JS
 # Authentication
 Currently based on {some authentication method}. Visit the [admin panel](https://api.tailscale.com/admin) and navigate to the `Keys` page. Generate an API Key and keep it safe. Provide the key as the user key in basic auth when making calls to Tailscale API endpoints.
 
-# APIs
-
-* **[Devices](#device)**
-  - [GET device](#device-get)
-  - [DELETE device](#device-delete)
-  - Routes
-    - [GET device routes](#device-routes-get)
-    - [POST device routes](#device-routes-post)
-* **[Tailnets](#tailnet)**
-  - ACLs
-    - [GET tailnet ACL](#tailnet-acl-get)
-    - [POST tailnet ACL](#tailnet-acl-post): set ACL for a tailnet
-    - [POST tailnet ACL preview](#tailnet-acl-preview-post): preview rule matches on an ACL for a resource
-  - [Devices](#tailnet-devices)
-    - [GET tailnet devices](#tailnet-devices-get)
-  - [DNS](#tailnet-dns)
-    - [GET tailnet DNS nameservers](#tailnet-dns-nameservers-get)
-    - [POST tailnet DNS nameservers](#tailnet-dns-nameservers-post)
-    - [GET tailnet DNS preferences](#tailnet-dns-preferences-get)
-    - [POST tailnet DNS preferences](#tailnet-dns-preferences-post)
-    - [GET tailnet DNS searchpaths](#tailnet-dns-searchpaths-get)
-    - [POST tailnet DNS searchpaths](#tailnet-dns-searchpaths-post)
+# APIS
 
 ## Device
 <!-- TODO: description about what devices are -->
@@ -37,8 +16,6 @@ To find the deviceID of a particular device, you can use the ["GET /devices"](#g
 Find the device you're looking for and get the "id" field.
 This is your deviceID. 
 
-<a name=device-get></div>
-
 #### `GET /api/v2/device/:deviceid` - lists the details for a device
 Returns the details for the specified device.
 Supply the device of interest in the path using its ID.
@@ -126,7 +103,6 @@ Response
 }
 ```
 
-<a name=device-delete></div>
 
 #### `DELETE /api/v2/device/:deviceID` - deletes the device from its tailnet
 Deletes the provided device from its tailnet. 
@@ -163,8 +139,6 @@ If the device is not owned by your tailnet:
 ```
 
 
-<a name=device-routes-get></div>
-
 #### `GET /api/v2/device/:deviceID/routes` - fetch subnet routes that are advertised and enabled for a device
 
 Retrieves the list of subnet routes that a device is advertising, as well as those that are enabled for it. Enabled routes are not necessarily advertised (e.g. for pre-enabling), and likewise, advertised routes are not necessarily enabled.
@@ -192,8 +166,6 @@ Response
 }
 ```
 
-<a name=device-routes-post></div>
-
 #### `POST /api/v2/device/:deviceID/routes` - set the subnet routes that are enabled for a device
 
 Sets which subnet routes are enabled to be routed by a device by replacing the existing list of subnet routes with the supplied parameters. Routes can be enabled without a device advertising them (e.g. for preauth). Returns a list of enabled subnet routes and a list of advertised subnet routes for a device.
@@ -238,8 +210,7 @@ A tailnet is the name of your Tailscale network.
 You can find it in the top left corner of the [Admin Panel](https://login.tailscale.com/admin) beside the Tailscale logo.
 
 
-`alice@example.com` belongs to the `example.com` tailnet and would use the following format for API calls:
-
+"alice@example.com" belongs to the "example.com" tailnet and would use the following format for API calls:
 ```
 GET /api/v2/tailnet/example.com/...
 curl https://api.tailscale.com/api/v2/tailnet/example.com/...
@@ -247,20 +218,20 @@ curl https://api.tailscale.com/api/v2/tailnet/example.com/...
 
 
 For solo plans, the tailnet is the email you signed up with.
-So `alice@gmail.com` has the tailnet `alice@gmail.com` since `@gmail.com` is a shared email host.
+So "alice@gmail.com" has the tailnet "alice@gmail.com" since @gmail.com is a shared email host.
 Her API calls would have the following format:
 ```
 GET /api/v2/tailnet/alice@gmail.com/...
 curl https://api.tailscale.com/api/v2/tailnet/alice@gmail.com/...
 ```
 
-Tailnets are a top-level resource. ACL is an example of a resource that is tied to a top-level tailnet.
+Tailnets are a top level resource. ACL is an example of a resource that is tied to a top level tailnet.
+
 
 For more information on Tailscale networks/tailnets, click [here](https://tailscale.com/kb/1064/invite-team-members).
 
-### ACL
 
-<a name=tailnet-acl-get></a>
+### ACL
 
 #### `GET /api/v2/tailnet/:tailnet/acl` - fetch ACL for a tailnet
 
@@ -363,8 +334,6 @@ Etag: "e0b2816b418b3f266309d94426ac7668ab3c1fa87798785bf82f1085cc2f6d9c"
 }
 ```
 
-<a name=tailnet-acl-post></a>
-
 #### `POST /api/v2/tailnet/:tailnet/acl` - set ACL for a tailnet
 
 Sets the ACL for the given tailnet. HuJSON and JSON are both accepted inputs. An `If-Match` header can be set to avoid missed updates.
@@ -436,8 +405,6 @@ Response
 }
 ```
 
-<a name=tailnet-acl-preview-post></a>
-
 #### `POST /api/v2/tailnet/:tailnet/acl/preview` - preview rule matches on an ACL for a resource
 Determines what rules match for a user on an ACL without saving the ACL to the server.
 
@@ -482,12 +449,8 @@ Response
 {"matches":[{"users":["*"],"ports":["*:*"],"lineNumber":19}],"user":"user1@example.com"}
 ```
 
-<a name=tailnet-devices></a>
-
 ### Devices
 
-<a name=tailnet-devices-get></a>
-
 #### <a name="getdevices"></a> `GET /api/v2/tailnet/:tailnet/devices` - list the devices for a tailnet
 Lists the devices in a tailnet. 
 Supply the tailnet of interest in the path.
@@ -568,12 +531,9 @@ Response
 }
 ```
 
-<a name=tailnet-dns></a>
 
 ### DNS
 
-<a name=tailnet-dns-nameservers-get></a>
-
 #### `GET /api/v2/tailnet/:tailnet/dns/nameservers` - list the DNS nameservers for a tailnet
 Lists the DNS nameservers for a tailnet. 
 Supply the tailnet of interest in the path.
@@ -596,8 +556,6 @@ Response
 }
 ```
 
-<a name=tailnet-dns-nameservers-post></a>
-
 #### `POST /api/v2/tailnet/:tailnet/dns/nameservers` - replaces the list of DNS nameservers for a tailnet
 Replaces the list of DNS nameservers for the given tailnet with the list supplied by the user. 
 Supply the tailnet of interest in the path.
@@ -650,8 +608,6 @@ Response:
 }
 ```
 
-<a name=tailnet-dns-preferences-get></a>
-
 #### `GET /api/v2/tailnet/:tailnet/dns/preferences` - retrieves the DNS preferences for a tailnet
 Retrieves the DNS preferences that are currently set for the given tailnet.
 Supply the tailnet of interest in the path.
@@ -673,8 +629,6 @@ Response:
 }
 ```
 
-<a name=tailnet-dns-preferences-post></a>
-
 #### `POST /api/v2/tailnet/:tailnet/dns/preferences` - replaces the DNS preferences for a tailnet 
 Replaces the DNS preferences for a tailnet, specifically, the MagicDNS setting.
 Note that MagicDNS is dependent on DNS servers. 
@@ -719,8 +673,6 @@ If there are DNS servers:
 }
 ```
 
-<a name=tailnet-dns-searchpaths-get></a>
-
 #### `GET /api/v2/tailnet/:tailnet/dns/searchpaths` - retrieves the search paths for a tailnet 
 Retrieves the list of search paths that is currently set for the given tailnet. 
 Supply the tailnet of interest in the path.
@@ -743,8 +695,6 @@ Response:
 }
 ```
 
-<a name=tailnet-dns-searchpaths-post></a>
-
 #### `POST /api/v2/tailnet/:tailnet/dns/searchpaths` - replaces the search paths for a tailnet 
 Replaces the list of searchpaths with the list supplied by the user and returns an error otherwise.
 

cmd/tailscale/cli/status.go

@@ -206,9 +206,9 @@ func dnsOrQuoteHostname(st *ipnstate.Status, ps *ipnstate.PeerStatus) string {
 		return ps.DNSName[:i]
 	}
 	if ps.DNSName != "" {
-		return strings.TrimRight(ps.DNSName, ".")
+		return ps.DNSName
 	}
-	return fmt.Sprintf("(%q)", strings.ReplaceAll(ps.SimpleHostName(), " ", "_"))
+	return fmt.Sprintf("- (%q)", ps.SimpleHostName())
 }
 
 func sortKey(ps *ipnstate.PeerStatus) string {

cmd/tailscale/depaware.txt

@@ -24,10 +24,10 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         github.com/tailscale/wireguard-go/ratelimiter                from github.com/tailscale/wireguard-go/device
         github.com/tailscale/wireguard-go/replay                     from github.com/tailscale/wireguard-go/device
         github.com/tailscale/wireguard-go/rwcancel                   from github.com/tailscale/wireguard-go/device+
-        github.com/tailscale/wireguard-go/tai64n                     from github.com/tailscale/wireguard-go/device+
+        github.com/tailscale/wireguard-go/tai64n                     from github.com/tailscale/wireguard-go/device
      💣 github.com/tailscale/wireguard-go/tun                        from github.com/tailscale/wireguard-go/device+
    W 💣 github.com/tailscale/wireguard-go/tun/wintun                 from github.com/tailscale/wireguard-go/tun+
-        github.com/tailscale/wireguard-go/wgcfg                      from github.com/tailscale/wireguard-go/device+
+        github.com/tailscale/wireguard-go/wgcfg                      from github.com/tailscale/wireguard-go/conn+
         github.com/tcnksm/go-httpstat                                from tailscale.com/net/netcheck
         github.com/toqueteos/webbrowser                              from tailscale.com/cmd/tailscale/cli
      💣 go4.org/intern                                               from inet.af/netaddr
@@ -90,7 +90,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/wgengine/tstun                                 from tailscale.com/wgengine
    W 💣 tailscale.com/wgengine/winnet                                from tailscale.com/wgengine/router
         golang.org/x/crypto/blake2b                                  from golang.org/x/crypto/nacl/box
-        golang.org/x/crypto/blake2s                                  from github.com/tailscale/wireguard-go/device+
+        golang.org/x/crypto/blake2s                                  from github.com/tailscale/wireguard-go/device
         golang.org/x/crypto/chacha20                                 from golang.org/x/crypto/chacha20poly1305
         golang.org/x/crypto/chacha20poly1305                         from crypto/tls+
         golang.org/x/crypto/cryptobyte                               from crypto/ecdsa+

cmd/tailscaled/depaware.txt

@@ -28,10 +28,10 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         github.com/tailscale/wireguard-go/ratelimiter                from github.com/tailscale/wireguard-go/device
         github.com/tailscale/wireguard-go/replay                     from github.com/tailscale/wireguard-go/device
         github.com/tailscale/wireguard-go/rwcancel                   from github.com/tailscale/wireguard-go/device+
-        github.com/tailscale/wireguard-go/tai64n                     from github.com/tailscale/wireguard-go/device+
+        github.com/tailscale/wireguard-go/tai64n                     from github.com/tailscale/wireguard-go/device
      💣 github.com/tailscale/wireguard-go/tun                        from github.com/tailscale/wireguard-go/device+
    W 💣 github.com/tailscale/wireguard-go/tun/wintun                 from github.com/tailscale/wireguard-go/tun+
-        github.com/tailscale/wireguard-go/wgcfg                      from github.com/tailscale/wireguard-go/device+
+        github.com/tailscale/wireguard-go/wgcfg                      from github.com/tailscale/wireguard-go/conn+
         github.com/tcnksm/go-httpstat                                from tailscale.com/net/netcheck
      💣 go4.org/intern                                               from inet.af/netaddr
      💣 go4.org/mem                                                  from tailscale.com/control/controlclient+
@@ -131,7 +131,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/wgengine/tstun                                 from tailscale.com/wgengine+
    W 💣 tailscale.com/wgengine/winnet                                from tailscale.com/wgengine/router
         golang.org/x/crypto/blake2b                                  from golang.org/x/crypto/nacl/box
-        golang.org/x/crypto/blake2s                                  from github.com/tailscale/wireguard-go/device+
+        golang.org/x/crypto/blake2s                                  from github.com/tailscale/wireguard-go/device
         golang.org/x/crypto/chacha20                                 from golang.org/x/crypto/chacha20poly1305
         golang.org/x/crypto/chacha20poly1305                         from crypto/tls+
         golang.org/x/crypto/cryptobyte                               from crypto/ecdsa+

control/controlclient/direct.go

@@ -107,17 +107,16 @@ func (p *Persist) Pretty() string {
 
 // Direct is the client that connects to a tailcontrol server for a node.
 type Direct struct {
-	httpc                  *http.Client // HTTP client used to talk to tailcontrol
-	serverURL              string       // URL of the tailcontrol server
-	timeNow                func() time.Time
-	lastPrintMap           time.Time
-	newDecompressor        func() (Decompressor, error)
-	keepAlive              bool
-	logf                   logger.Logf
-	discoPubKey            tailcfg.DiscoKey
-	machinePrivKey         wgkey.Private
-	debugFlags             []string
-	keepSharerAndUserSplit bool
+	httpc           *http.Client // HTTP client used to talk to tailcontrol
+	serverURL       string       // URL of the tailcontrol server
+	timeNow         func() time.Time
+	lastPrintMap    time.Time
+	newDecompressor func() (Decompressor, error)
+	keepAlive       bool
+	logf            logger.Logf
+	discoPubKey     tailcfg.DiscoKey
+	machinePrivKey  wgkey.Private
+	debugFlags      []string
 
 	mu           sync.Mutex // mutex guards the following fields
 	serverKey    wgkey.Key
@@ -145,10 +144,6 @@ type Options struct {
 	Logf              logger.Logf
 	HTTPTestClient    *http.Client // optional HTTP client to use (for tests only)
 	DebugFlags        []string     // debug settings to send to control
-
-	// KeepSharerAndUserSplit controls whether the client
-	// understands Node.Sharer. If false, the Sharer is mapped to the User.
-	KeepSharerAndUserSplit bool
 }
 
 type Decompressor interface {
@@ -195,18 +190,17 @@ func NewDirect(opts Options) (*Direct, error) {
 	}
 
 	c := &Direct{
-		httpc:                  httpc,
-		machinePrivKey:         opts.MachinePrivateKey,
-		serverURL:              opts.ServerURL,
-		timeNow:                opts.TimeNow,
-		logf:                   opts.Logf,
-		newDecompressor:        opts.NewDecompressor,
-		keepAlive:              opts.KeepAlive,
-		persist:                opts.Persist,
-		authKey:                opts.AuthKey,
-		discoPubKey:            opts.DiscoPublicKey,
-		debugFlags:             opts.DebugFlags,
-		keepSharerAndUserSplit: opts.KeepSharerAndUserSplit,
+		httpc:           httpc,
+		machinePrivKey:  opts.MachinePrivateKey,
+		serverURL:       opts.ServerURL,
+		timeNow:         opts.TimeNow,
+		logf:            opts.Logf,
+		newDecompressor: opts.NewDecompressor,
+		keepAlive:       opts.KeepAlive,
+		persist:         opts.Persist,
+		authKey:         opts.AuthKey,
+		discoPubKey:     opts.DiscoPublicKey,
+		debugFlags:      opts.DebugFlags,
 	}
 	if opts.Hostinfo == nil {
 		c.SetHostinfo(NewHostinfo())
@@ -791,12 +785,19 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw
 		}
 		addUserProfile(nm.User)
 		for _, peer := range resp.Peers {
+			// TODO(bradfitz): ideally we'd push down the semantically correct
+			// Nodes with differing User vs Sharer fields, but that means
+			// updating Windows, macOS, and tailscale status to respect all
+			// those fields, but until we have a plan for what the UI should
+			// be later when we treat them differently, it's easier to just
+			// merge it together here. The server will anonymize UserProfile
+			// records of those not in your network and not a sharer, which
+			// will be most of the peer.Users so it'll be rare when a node's
+			// owner-who's-different-from-sharer will have a non-scrubbed
+			// UserProfile: they would've also needed to share a node
+			// themselves.  Until we care, merge the data here.
 			if !peer.Sharer.IsZero() {
-				if c.keepSharerAndUserSplit {
-					addUserProfile(peer.Sharer)
-				} else {
-					peer.User = peer.Sharer
-				}
+				peer.User = peer.Sharer
 			}
 			addUserProfile(peer.User)
 		}

control/controlclient/netmap.go

@@ -298,7 +298,7 @@ func (nm *NetworkMap) WGCfg(logf logger.Logf, flags WGConfigFlags) (*wgcfg.Confi
 			if err := appendEndpoint(cpeer, fmt.Sprintf("%x%s", peer.DiscoKey[:], EndpointDiscoSuffix)); err != nil {
 				return nil, err
 			}
-			cpeer.Endpoints = fmt.Sprintf("%x.disco.tailscale:12345", peer.DiscoKey[:])
+			cpeer.Endpoints = []wgcfg.Endpoint{{Host: fmt.Sprintf("%x.disco.tailscale", peer.DiscoKey[:]), Port: 12345}}
 		} else {
 			if err := appendEndpoint(cpeer, peer.DERP); err != nil {
 				return nil, err
@@ -349,18 +349,15 @@ func appendEndpoint(peer *wgcfg.Peer, epStr string) error {
 	if epStr == "" {
 		return nil
 	}
-	_, port, err := net.SplitHostPort(epStr)
+	host, port, err := net.SplitHostPort(epStr)
 	if err != nil {
 		return fmt.Errorf("malformed endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
 	}
-	_, err = strconv.ParseUint(port, 10, 16)
+	port16, err := strconv.ParseUint(port, 10, 16)
 	if err != nil {
 		return fmt.Errorf("invalid port in endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
 	}
-	if peer.Endpoints != "" {
-		peer.Endpoints += ","
-	}
-	peer.Endpoints += epStr
+	peer.Endpoints = append(peer.Endpoints, wgcfg.Endpoint{Host: host, Port: uint16(port16)})
 	return nil
 }
 

go.mod

@@ -24,7 +24,7 @@ require (
 	github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3
 	github.com/peterbourgon/ff/v2 v2.0.0
 	github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027
-	github.com/tailscale/wireguard-go v0.0.0-20210116013233-4cd297ed5a7d
+	github.com/tailscale/wireguard-go v0.0.0-20210109012254-dc30a1b9415e
 	github.com/tcnksm/go-httpstat v0.2.0
 	github.com/toqueteos/webbrowser v1.2.0
 	go4.org/mem v0.0.0-20201119185036-c04c5a6ff174

go.sum

@@ -288,12 +288,6 @@ github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027 h1:lK99QQdH3yBW
 github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027/go.mod h1:p9lPsd+cx33L3H9nNoecRRxPssFKUwwI50I3pZ0yT+8=
 github.com/tailscale/wireguard-go v0.0.0-20210109012254-dc30a1b9415e h1:ZXbXfVJOhSq4/Gt7TnqwXBPCctzYXkWXo3oQS7LZ40I=
 github.com/tailscale/wireguard-go v0.0.0-20210109012254-dc30a1b9415e/go.mod h1:K/wyv4+3PcdVVTV7szyoiEjJ1nVHonM8cJ2mQwG5Fl8=
-github.com/tailscale/wireguard-go v0.0.0-20210113223737-a6213b5eaf98 h1:khwYPK1eT+4pmEFyCjpf6Br/0JWjdVT3uQ+ILFJPTRo=
-github.com/tailscale/wireguard-go v0.0.0-20210113223737-a6213b5eaf98/go.mod h1:K/wyv4+3PcdVVTV7szyoiEjJ1nVHonM8cJ2mQwG5Fl8=
-github.com/tailscale/wireguard-go v0.0.0-20210114205708-a1377e83f551 h1:hjBVxvVa145kVflAFkVcTr/zwUzBO4SqfSS6xhbcMv8=
-github.com/tailscale/wireguard-go v0.0.0-20210114205708-a1377e83f551/go.mod h1:K/wyv4+3PcdVVTV7szyoiEjJ1nVHonM8cJ2mQwG5Fl8=
-github.com/tailscale/wireguard-go v0.0.0-20210116013233-4cd297ed5a7d h1:8GcGtZ4Ui+lzHm6gOq7s2Oe4ksxkbUYtS/JuoJ2Nce8=
-github.com/tailscale/wireguard-go v0.0.0-20210116013233-4cd297ed5a7d/go.mod h1:K/wyv4+3PcdVVTV7szyoiEjJ1nVHonM8cJ2mQwG5Fl8=
 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_test.go

@@ -41,7 +41,12 @@ func getVal() []interface{} {
 			ListenPort: 5,
 			Peers: []wgcfg.Peer{
 				{
-					Endpoints: "foo:5",
+					Endpoints: []wgcfg.Endpoint{
+						{
+							Host: "foo",
+							Port: 5,
+						},
+					},
 				},
 			},
 		},

ipn/ipnserver/conn_linux.go

@@ -1,49 +0,0 @@
-// Copyright (c) 2021 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
-
-package ipnserver
-
-import (
-	"net"
-
-	"golang.org/x/sys/unix"
-	"tailscale.com/types/logger"
-)
-
-func isReadonlyConn(c net.Conn, logf logger.Logf) (ro bool) {
-	ro = true // conservative default for naked returns below
-	uc, ok := c.(*net.UnixConn)
-	if !ok {
-		logf("unexpected connection type %T", c)
-		return
-	}
-	raw, err := uc.SyscallConn()
-	if err != nil {
-		logf("SyscallConn: %v", err)
-		return
-	}
-
-	var cred *unix.Ucred
-	cerr := raw.Control(func(fd uintptr) {
-		cred, err = unix.GetsockoptUcred(int(fd),
-			unix.SOL_SOCKET,
-			unix.SO_PEERCRED)
-	})
-	if cerr != nil {
-		logf("raw.Control: %v", err)
-		return
-	}
-	if err != nil {
-		logf("raw.Control: %v", err)
-		return
-	}
-	if cred.Uid == 0 {
-		// root is not read-only.
-		return false
-	}
-	logf("non-root connection from %v (read-only)", cred.Uid)
-	return true
-}

ipn/ipnserver/conn_no_ucred.go

@@ -1,27 +0,0 @@
-// Copyright (c) 2021 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
-
-package ipnserver
-
-import (
-	"net"
-
-	"tailscale.com/types/logger"
-)
-
-func isReadonlyConn(c net.Conn, logf logger.Logf) bool {
-	// Windows doesn't need/use this mechanism, at least yet. It
-	// has a different last-user-wins auth model.
-
-	// And on Darwin, we're not using it yet, as the Darwin
-	// tailscaled port isn't yet done, and unix.Ucred and
-	// unix.GetsockoptUcred aren't in x/sys/unix.
-
-	// TODO(bradfitz): OpenBSD and FreeBSD should implement this too.
-	// But their x/sys/unix package is different than Linux, so
-	// I didn't include it for now.
-	return false
-}

ipn/ipnserver/server.go

@@ -268,10 +268,6 @@ func (s *server) serveConn(ctx context.Context, c net.Conn, logf logger.Logf) {
 	defer s.removeAndCloseConn(c)
 	logf("[v1] incoming control connection")
 
-	if isReadonlyConn(c, logf) {
-		ctx = ipn.ReadonlyContextOf(ctx)
-	}
-
 	for ctx.Err() == nil {
 		msg, err := ipn.ReadMsg(br)
 		if err != nil {
@@ -283,7 +279,7 @@ func (s *server) serveConn(ctx context.Context, c net.Conn, logf logger.Logf) {
 			return
 		}
 		s.bsMu.Lock()
-		if err := s.bs.GotCommandMsg(ctx, msg); err != nil {
+		if err := s.bs.GotCommandMsg(msg); err != nil {
 			logf("GotCommandMsg: %v", err)
 		}
 		gotQuit := s.bs.GotQuit
@@ -359,7 +355,7 @@ func (s *server) addConn(c net.Conn, isHTTP bool) (ci connIdentity, err error) {
 		if doReset {
 			s.logf("identity changed; resetting server")
 			s.bsMu.Lock()
-			s.bs.Reset(context.TODO())
+			s.bs.Reset()
 			s.bsMu.Unlock()
 		}
 	}()
@@ -411,7 +407,7 @@ func (s *server) removeAndCloseConn(c net.Conn) {
 		} else {
 			s.logf("client disconnected; stopping server")
 			s.bsMu.Lock()
-			s.bs.Reset(context.TODO())
+			s.bs.Reset()
 			s.bsMu.Unlock()
 		}
 	}
@@ -585,7 +581,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 	server.bs = ipn.NewBackendServer(logf, b, server.writeToClients)
 
 	if opts.AutostartStateKey != "" {
-		server.bs.GotCommand(context.TODO(), &ipn.Command{
+		server.bs.GotCommand(&ipn.Command{
 			Version: version.Long,
 			Start: &ipn.StartArgs{
 				Opts: ipn.Options{

ipn/local.go

@@ -564,7 +564,8 @@ func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap, prefs *Pre
 
 	if shieldsUp {
 		b.logf("netmap packet filter: (shields up)")
-		b.e.SetFilter(filter.NewShieldsUpFilter(b.logf))
+		var prevFilter *filter.Filter // don't reuse old filter state
+		b.e.SetFilter(filter.New(nil, localNets, prevFilter, b.logf))
 	} else {
 		b.logf("netmap packet filter: %v", packetFilter)
 		b.e.SetFilter(filter.New(packetFilter, localNets, b.e.GetFilter(), b.logf))

ipn/message.go

@@ -6,7 +6,6 @@ package ipn
 
 import (
 	"bytes"
-	"context"
 	"encoding/binary"
 	"encoding/json"
 	"errors"
@@ -21,24 +20,6 @@ import (
 	"tailscale.com/version"
 )
 
-type readOnlyContextKey struct{}
-
-// IsReadonlyContext reports whether ctx is a read-only context, as currently used
-// by Unix non-root users running the "tailscale" CLI command. They can run "status",
-// but not much else.
-func IsReadonlyContext(ctx context.Context) bool {
-	return ctx.Value(readOnlyContextKey{}) != nil
-}
-
-// ReadonlyContextOf returns ctx wrapped with a context value that
-// will make IsReadonlyContext reports true.
-func ReadonlyContextOf(ctx context.Context) context.Context {
-	if IsReadonlyContext(ctx) {
-		return ctx
-	}
-	return context.WithValue(ctx, readOnlyContextKey{}, readOnlyContextKey{})
-}
-
 var jsonEscapedZero = []byte(`\u0000`)
 
 type NoArgs struct{}
@@ -130,7 +111,7 @@ func (bs *BackendServer) SendInUseOtherUserErrorMessage(msg string) {
 
 // GotCommandMsg parses the incoming message b as a JSON Command and
 // calls GotCommand with it.
-func (bs *BackendServer) GotCommandMsg(ctx context.Context, b []byte) error {
+func (bs *BackendServer) GotCommandMsg(b []byte) error {
 	cmd := &Command{}
 	if len(b) == 0 {
 		return nil
@@ -138,15 +119,15 @@ func (bs *BackendServer) GotCommandMsg(ctx context.Context, b []byte) error {
 	if err := json.Unmarshal(b, cmd); err != nil {
 		return err
 	}
-	return bs.GotCommand(ctx, cmd)
+	return bs.GotCommand(cmd)
 }
 
-func (bs *BackendServer) GotFakeCommand(ctx context.Context, cmd *Command) error {
+func (bs *BackendServer) GotFakeCommand(cmd *Command) error {
 	cmd.Version = version.Long
-	return bs.GotCommand(ctx, cmd)
+	return bs.GotCommand(cmd)
 }
 
-func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error {
+func (bs *BackendServer) GotCommand(cmd *Command) error {
 	if cmd.Version != version.Long && !cmd.AllowVersionSkew {
 		vs := fmt.Sprintf("GotCommand: Version mismatch! frontend=%#v backend=%#v",
 			cmd.Version, version.Long)
@@ -160,33 +141,12 @@ func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error {
 		})
 		return nil
 	}
-
-	// TODO(bradfitz): finish plumbing context down to all the methods below;
-	// currently we just check for read-only contexts in this method and
-	// then never use contexts again.
-
-	// Actions permitted with a read-only context:
-	if c := cmd.RequestEngineStatus; c != nil {
-		bs.b.RequestEngineStatus()
-		return nil
-	} else if c := cmd.RequestStatus; c != nil {
-		bs.b.RequestStatus()
-		return nil
-	} else if c := cmd.Ping; c != nil {
-		bs.b.Ping(c.IP)
-		return nil
-	}
-
-	if IsReadonlyContext(ctx) {
-		msg := "permission denied"
-		bs.send(Notify{ErrMessage: &msg})
-		return nil
-	}
-
 	if cmd.Quit != nil {
 		bs.GotQuit = true
 		return errors.New("Quit command received")
-	} else if c := cmd.Start; c != nil {
+	}
+
+	if c := cmd.Start; c != nil {
 		opts := c.Opts
 		opts.Notify = bs.send
 		return bs.b.Start(opts)
@@ -205,17 +165,27 @@ func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error {
 	} else if c := cmd.SetWantRunning; c != nil {
 		bs.b.SetWantRunning(*c)
 		return nil
+	} else if c := cmd.RequestEngineStatus; c != nil {
+		bs.b.RequestEngineStatus()
+		return nil
+	} else if c := cmd.RequestStatus; c != nil {
+		bs.b.RequestStatus()
+		return nil
 	} else if c := cmd.FakeExpireAfter; c != nil {
 		bs.b.FakeExpireAfter(c.Duration)
 		return nil
+	} else if c := cmd.Ping; c != nil {
+		bs.b.Ping(c.IP)
+		return nil
+	} else {
+		return fmt.Errorf("BackendServer.Do: no command specified")
 	}
-	return fmt.Errorf("BackendServer.Do: no command specified")
 }
 
-func (bs *BackendServer) Reset(ctx context.Context) error {
+func (bs *BackendServer) Reset() error {
 	// Tell the backend we got a Logout command, which will cause it
 	// to forget all its authentication information.
-	return bs.GotFakeCommand(ctx, &Command{Logout: &NoArgs{}})
+	return bs.GotFakeCommand(&Command{Logout: &NoArgs{}})
 }
 
 type BackendClient struct {

ipn/message_test.go

@@ -6,7 +6,6 @@ package ipn
 
 import (
 	"bytes"
-	"context"
 	"testing"
 	"time"
 
@@ -82,7 +81,7 @@ func TestClientServer(t *testing.T) {
 		serverToClientCh <- append([]byte{}, b...)
 	}
 	clientToServer := func(b []byte) {
-		bs.GotCommandMsg(context.TODO(), b)
+		bs.GotCommandMsg(b)
 	}
 	slogf := func(fmt string, args ...interface{}) {
 		t.Logf("s: "+fmt, args...)

logtail/logtail_test.go

@@ -6,12 +6,8 @@ package logtail
 
 import (
 	"context"
-	"encoding/json"
-	"io"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 	"time"
 )
@@ -27,201 +23,28 @@ func TestFastShutdown(t *testing.T) {
 	l := NewLogger(Config{
 		BaseURL: testServ.URL,
 	}, t.Logf)
-	err := l.Shutdown(ctx)
-	if err != nil {
-		t.Error(err)
-	}
+	l.Shutdown(ctx)
 }
 
-// maximum number of times a test will call l.Write()
-const logLines = 3
-
-type LogtailTestServer struct {
-	srv      *httptest.Server // Log server
-	uploaded chan []byte
-}
-
-func NewLogtailTestHarness(t *testing.T) (*LogtailTestServer, *Logger) {
-	ts := LogtailTestServer{}
-
-	// max channel backlog = 1 "started" + #logLines x "log line" + 1 "closed"
-	ts.uploaded = make(chan []byte, 2+logLines)
-
-	ts.srv = httptest.NewServer(http.HandlerFunc(
+func TestUploadMessages(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	uploads := 0
+	testServ := httptest.NewServer(http.HandlerFunc(
 		func(w http.ResponseWriter, r *http.Request) {
-			body, err := ioutil.ReadAll(r.Body)
-			if err != nil {
-				t.Error("failed to read HTTP request")
-			}
-			ts.uploaded <- body
+			w.Header().Set("Content-Type", "application/json; charset=utf-8")
+			uploads += 1
 		}))
+	defer testServ.Close()
 
-	t.Cleanup(ts.srv.Close)
-
-	l := NewLogger(Config{BaseURL: ts.srv.URL}, t.Logf)
-
-	// There is always an initial "logtail started" message
-	body := <-ts.uploaded
-	if !strings.Contains(string(body), "started") {
-		t.Errorf("unknown start logging statement: %q", string(body))
-	}
-
-	return &ts, l
-}
-
-func TestDrainPendingMessages(t *testing.T) {
-	ts, l := NewLogtailTestHarness(t)
-
-	for i := 0; i < logLines; i++ {
+	l := NewLogger(Config{BaseURL: testServ.URL}, t.Logf)
+	for i := 1; i < 10; i++ {
 		l.Write([]byte("log line"))
 	}
 
-	// all of the "log line" messages usually arrive at once, but poll if needed.
-	body := ""
-	for i := 0; i <= logLines; i++ {
-		body += string(<-ts.uploaded)
-		count := strings.Count(body, "log line")
-		if count == logLines {
-			break
-		}
-		// if we never find count == logLines, the test will eventually time out.
-	}
-
-	err := l.Shutdown(context.Background())
-	if err != nil {
-		t.Error(err)
-	}
-}
-
-func TestEncodeAndUploadMessages(t *testing.T) {
-	ts, l := NewLogtailTestHarness(t)
-
-	tests := []struct {
-		name string
-		log  string
-		want string
-	}{
-		{
-			"plain text",
-			"log line",
-			"log line",
-		},
-		{
-			"simple JSON",
-			`{"text": "log line"}`,
-			"log line",
-		},
-	}
-
-	for _, tt := range tests {
-		io.WriteString(l, tt.log)
-		body := <-ts.uploaded
-
-		data := make(map[string]interface{})
-		err := json.Unmarshal(body, &data)
-		if err != nil {
-			t.Error(err)
-		}
-
-		got := data["text"]
-		if got != tt.want {
-			t.Errorf("%s: got %q; want %q", tt.name, got.(string), tt.want)
-		}
-
-		ltail, ok := data["logtail"]
-		if ok {
-			logtailmap := ltail.(map[string]interface{})
-			_, ok = logtailmap["client_time"]
-			if !ok {
-				t.Errorf("%s: no client_time present", tt.name)
-			}
-		} else {
-			t.Errorf("%s: no logtail map present", tt.name)
-		}
-	}
-
-	err := l.Shutdown(context.Background())
-	if err != nil {
-		t.Error(err)
-	}
-}
-
-func TestEncodeSpecialCases(t *testing.T) {
-	ts, l := NewLogtailTestHarness(t)
-
-	// -------------------------------------------------------------------------
-
-	// JSON log message already contains a logtail field.
-	io.WriteString(l, `{"logtail": "LOGTAIL", "text": "text"}`)
-	body := <-ts.uploaded
-	data := make(map[string]interface{})
-	err := json.Unmarshal(body, &data)
-	if err != nil {
-		t.Error(err)
-	}
-	errorHasLogtail, ok := data["error_has_logtail"]
-	if ok {
-		if errorHasLogtail != "LOGTAIL" {
-			t.Errorf("error_has_logtail: got:%q; want:%q",
-				errorHasLogtail, "LOGTAIL")
-		}
-	} else {
-		t.Errorf("no error_has_logtail field: %v", data)
-	}
-
-	// -------------------------------------------------------------------------
-
-	// special characters
-	io.WriteString(l, "\b\f\n\r\t"+`"\`)
-	bodytext := string(<-ts.uploaded)
-	// json.Unmarshal would unescape the characters, we have to look at the encoded text
-	escaped := strings.Contains(bodytext, `\b\f\n\r\t\"\`)
-	if !escaped {
-		t.Errorf("special characters got %s", bodytext)
-	}
-
-	// -------------------------------------------------------------------------
-
-	// skipClientTime to omit the logtail metadata
-	l.skipClientTime = true
-	io.WriteString(l, "text")
-	body = <-ts.uploaded
-	data = make(map[string]interface{})
-	err = json.Unmarshal(body, &data)
-	if err != nil {
-		t.Error(err)
-	}
-	_, ok = data["logtail"]
-	if ok {
-		t.Errorf("skipClientTime: unexpected logtail map present: %v", data)
-	}
-
-	// -------------------------------------------------------------------------
-
-	// lowMem + long string
-	l.skipClientTime = false
-	l.lowMem = true
-	longStr := strings.Repeat("0", 512)
-	io.WriteString(l, longStr)
-	body = <-ts.uploaded
-	data = make(map[string]interface{})
-	err = json.Unmarshal(body, &data)
-	if err != nil {
-		t.Error(err)
-	}
-	text, ok := data["text"]
-	if !ok {
-		t.Errorf("lowMem: no text %v", data)
-	}
-	if n := len(text.(string)); n > 300 {
-		t.Errorf("lowMem: got %d chars; want <300 chars", n)
-	}
-
-	// -------------------------------------------------------------------------
-
-	err = l.Shutdown(context.Background())
-	if err != nil {
-		t.Error(err)
+	l.Shutdown(ctx)
+	cancel()
+	if uploads == 0 {
+		t.Error("no log uploads")
 	}
 }
 
@@ -252,54 +75,3 @@ func TestLoggerWriteLength(t *testing.T) {
 		t.Errorf("logger.Write wrote %d bytes, expected %d", n, len(inBuf))
 	}
 }
-
-func TestParseAndRemoveLogLevel(t *testing.T) {
-	tests := []struct {
-		log       string
-		wantLevel int
-		wantLog   string
-	}{
-		{
-			"no level",
-			0,
-			"no level",
-		},
-		{
-			"[v1] level 1",
-			1,
-			"level 1",
-		},
-		{
-			"level 1 [v1] ",
-			1,
-			"level 1 ",
-		},
-		{
-			"[v2] level 2",
-			2,
-			"level 2",
-		},
-		{
-			"level [v2] 2",
-			2,
-			"level 2",
-		},
-		{
-			"[v3] no level 3",
-			0,
-			"[v3] no level 3",
-		},
-	}
-
-	for _, tt := range tests {
-		gotLevel, gotLog := parseAndRemoveLogLevel([]byte(tt.log))
-		if gotLevel != tt.wantLevel {
-			t.Errorf("parseAndRemoveLogLevel(%q): got:%d; want %d",
-				tt.log, gotLevel, tt.wantLevel)
-		}
-		if string(gotLog) != tt.wantLog {
-			t.Errorf("parseAndRemoveLogLevel(%q): got:%q; want %q",
-				tt.log, gotLog, tt.wantLog)
-		}
-	}
-}

net/nettest/pipe.go

@@ -60,7 +60,7 @@ func (p *Pipe) Read(b []byte) (n int, err error) {
 	for {
 		p.mu.Lock()
 		closed := p.closed
-		timedout := !p.readTimeout.IsZero() && !time.Now().Before(p.readTimeout)
+		timedout := !p.readTimeout.IsZero() && time.Now().After(p.readTimeout)
 		blocked := p.blocked
 		if !closed && !timedout && len(p.buf) > 0 {
 			n2 := copy(b, p.buf)
@@ -99,7 +99,7 @@ func (p *Pipe) Write(b []byte) (n int, err error) {
 	for {
 		p.mu.Lock()
 		closed := p.closed
-		timedout := !p.writeTimeout.IsZero() && !time.Now().Before(p.writeTimeout)
+		timedout := !p.writeTimeout.IsZero() && time.Now().After(p.writeTimeout)
 		blocked := p.blocked
 		if !closed && !timedout {
 			n2 := len(b)

net/nettest/pipe_test.go

@@ -35,7 +35,7 @@ func TestPipeTimeout(t *testing.T) {
 		p := NewPipe("p1", 1<<16)
 		p.SetWriteDeadline(time.Now().Add(-1 * time.Second))
 		n, err := p.Write([]byte{'h'})
-		if !errors.Is(err, ErrWriteTimeout) || !errors.Is(err, ErrTimeout) {
+		if err == nil || !errors.Is(err, ErrWriteTimeout) || !errors.Is(err, ErrTimeout) {
 			t.Errorf("missing write timeout got err: %v", err)
 		}
 		if n != 0 {
@@ -49,7 +49,7 @@ func TestPipeTimeout(t *testing.T) {
 		p.SetReadDeadline(time.Now().Add(-1 * time.Second))
 		b := make([]byte, 1)
 		n, err := p.Read(b)
-		if !errors.Is(err, ErrReadTimeout) || !errors.Is(err, ErrTimeout) {
+		if err == nil || !errors.Is(err, ErrReadTimeout) || !errors.Is(err, ErrTimeout) {
 			t.Errorf("missing read timeout got err: %v", err)
 		}
 		if n != 0 {
@@ -65,7 +65,7 @@ func TestPipeTimeout(t *testing.T) {
 		if err := p.Block(); err != nil {
 			t.Fatal(err)
 		}
-		if _, err := p.Write([]byte{'h'}); !errors.Is(err, ErrWriteTimeout) {
+		if _, err := p.Write([]byte{'h'}); err == nil || !errors.Is(err, ErrWriteTimeout) {
 			t.Fatalf("want write timeout got: %v", err)
 		}
 	})
@@ -80,10 +80,11 @@ func TestPipeTimeout(t *testing.T) {
 		if err := p.Block(); err != nil {
 			t.Fatal(err)
 		}
-		if _, err := p.Read(b); !errors.Is(err, ErrReadTimeout) {
+		if _, err := p.Read(b); err == nil || !errors.Is(err, ErrReadTimeout) {
 			t.Fatalf("want read timeout got: %v", err)
 		}
 	})
+
 }
 
 func TestLimit(t *testing.T) {
@@ -116,8 +117,4 @@ func TestLimit(t *testing.T) {
 	} else if n != 1 {
 		t.Errorf("Read(%q): n=%d want 1", string(b), n)
 	}
-
-	if err := <-errCh; err != nil {
-		t.Error(err)
-	}
 }

net/packet/header.go

@@ -36,6 +36,9 @@ type Header interface {
 	// purpose of computing length and checksum fields. Marshal
 	// implementations must not allocate memory.
 	Marshal(buf []byte) error
+	// ToResponse transforms the header into one for a response packet.
+	// For instance, this swaps the source and destination IPs.
+	ToResponse()
 }
 
 // Generate generates a new packet with the given Header and

net/packet/ip.go

@@ -24,17 +24,6 @@ const (
 	TCP    IPProto = 0x06
 	UDP    IPProto = 0x11
 
-	// TSMP is the Tailscale Message Protocol (our ICMP-ish
-	// thing), an IP protocol used only between Tailscale nodes
-	// (still encrypted by WireGuard) that communicates why things
-	// failed, etc.
-	//
-	// Proto number 99 is reserved for "any private encryption
-	// scheme". We never accept these from the host OS stack nor
-	// send them to the host network stack. It's only used between
-	// nodes.
-	TSMP IPProto = 99
-
 	// Fragment represents any non-first IP fragment, for which we
 	// don't have the sub-protocol header (and therefore can't
 	// figure out what the sub-protocol is).
@@ -58,8 +47,6 @@ func (p IPProto) String() string {
 		return "UDP"
 	case TCP:
 		return "TCP"
-	case TSMP:
-		return "TSMP"
 	default:
 		return "Unknown"
 	}

net/packet/packet.go

@@ -204,10 +204,6 @@ func (q *Parsed) decode4(b []byte) {
 			q.Dst.Port = binary.BigEndian.Uint16(sub[2:4])
 			q.dataofs = q.subofs + udpHeaderLength
 			return
-		case TSMP:
-			// Inter-tailscale messages.
-			q.dataofs = q.subofs
-			return
 		default:
 			q.IPProto = Unknown
 			return
@@ -295,10 +291,6 @@ func (q *Parsed) decode6(b []byte) {
 		q.Src.Port = binary.BigEndian.Uint16(sub[0:2])
 		q.Dst.Port = binary.BigEndian.Uint16(sub[2:4])
 		q.dataofs = q.subofs + udpHeaderLength
-	case TSMP:
-		// Inter-tailscale messages.
-		q.dataofs = q.subofs
-		return
 	default:
 		q.IPProto = Unknown
 		return

net/packet/packet_test.go

@@ -274,38 +274,7 @@ var igmpPacketDecode = Parsed{
 	Dst:       mustIPPort("224.0.0.251:0"),
 }
 
-var ipv4TSMPBuffer = []byte{
-	// IPv4 header:
-	0x45, 0x00,
-	0x00, 0x1b, // 20 + 7 bytes total
-	0x00, 0x00, // ID
-	0x00, 0x00, // Fragment
-	0x40, // TTL
-	byte(TSMP),
-	0x5f, 0xc3, // header checksum (wrong here)
-	// source IP:
-	0x64, 0x5e, 0x0c, 0x0e,
-	// dest IP:
-	0x64, 0x4a, 0x46, 0x03,
-	byte(TSMPTypeRejectedConn),
-	byte(TCP),
-	byte(RejectedDueToACLs),
-	0x00, 123, // src port
-	0x00, 80, // dst port
-}
-
-var ipv4TSMPDecode = Parsed{
-	b:         ipv4TSMPBuffer,
-	subofs:    20,
-	dataofs:   20,
-	length:    27,
-	IPVersion: 4,
-	IPProto:   TSMP,
-	Src:       mustIPPort("100.94.12.14:0"),
-	Dst:       mustIPPort("100.74.70.3:0"),
-}
-
-func TestParsedString(t *testing.T) {
+func TestParsed(t *testing.T) {
 	tests := []struct {
 		name    string
 		qdecode Parsed
@@ -319,7 +288,6 @@ func TestParsedString(t *testing.T) {
 		{"icmp6", icmp6PacketDecode, "ICMPv6{[fe80::fb57:1dea:9c39:8fb7]:0 > [ff02::2]:0}"},
 		{"igmp", igmpPacketDecode, "IGMP{192.168.1.82:0 > 224.0.0.251:0}"},
 		{"unknown", unknownPacketDecode, "Unknown{???}"},
-		{"ipv4_tsmp", ipv4TSMPDecode, "TSMP{100.94.12.14:0 > 100.74.70.3:0}"},
 	}
 
 	for _, tt := range tests {
@@ -356,7 +324,6 @@ func TestDecode(t *testing.T) {
 		{"igmp", igmpPacketBuffer, igmpPacketDecode},
 		{"unknown", unknownPacketBuffer, unknownPacketDecode},
 		{"invalid4", invalid4RequestBuffer, invalid4RequestDecode},
-		{"ipv4_tsmp", ipv4TSMPBuffer, ipv4TSMPDecode},
 	}
 
 	for _, tt := range tests {
@@ -364,7 +331,7 @@ func TestDecode(t *testing.T) {
 			var got Parsed
 			got.Decode(tt.buf)
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("mismatch\n got: %s %#v\nwant: %s %#v", got.String(), got, tt.want.String(), tt.want)
+				t.Errorf("mismatch\n got: %#v\nwant: %#v", got, tt.want)
 			}
 		})
 	}
@@ -449,16 +416,9 @@ func TestMarshalResponse(t *testing.T) {
 	icmpHeader := icmp4RequestDecode.ICMP4Header()
 	udpHeader := udp4RequestDecode.UDP4Header()
 
-	type HeaderToResponser interface {
-		Header
-		// ToResponse transforms the header into one for a response packet.
-		// For instance, this swaps the source and destination IPs.
-		ToResponse()
-	}
-
 	tests := []struct {
 		name   string
-		header HeaderToResponser
+		header Header
 		want   []byte
 	}{
 		{"icmp", &icmpHeader, icmp4ReplyBuffer},

net/packet/tsmp.go

@@ -1,140 +0,0 @@
-// Copyright (c) 2021 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.
-
-// TSMP is our ICMP-like "Tailscale Message Protocol" for signaling
-// Tailscale-specific messages between nodes. It uses IP protocol 99
-// (reserved for "any private encryption scheme") within
-// Wireguard's normal encryption between peers and never hits the host
-// network stack.
-
-package packet
-
-import (
-	"encoding/binary"
-	"errors"
-	"fmt"
-
-	"inet.af/netaddr"
-	"tailscale.com/net/flowtrack"
-)
-
-// TailscaleRejectedHeader is a TSMP message that says that one
-// Tailscale node has rejected the connection from another. Unlike a
-// TCP RST, this includes a reason.
-//
-// On the wire, after the IP header, it's currently 7 bytes:
-//     * '!'
-//     * IPProto byte (IANA protocol number: TCP or UDP)
-//     * 'A' or 'S' (RejectedDueToACLs, RejectedDueToShieldsUp)
-//     * srcPort big endian uint16
-//     * dstPort big endian uint16
-//
-// In the future it might also accept 16 byte IP flow src/dst IPs
-// after the header, if they're different than the IP-level ones.
-type TailscaleRejectedHeader struct {
-	IPSrc  netaddr.IP            // IPv4 or IPv6 header's src IP
-	IPDst  netaddr.IP            // IPv4 or IPv6 header's dst IP
-	Src    netaddr.IPPort        // rejected flow's src
-	Dst    netaddr.IPPort        // rejected flow's dst
-	Proto  IPProto               // proto that was rejected (TCP or UDP)
-	Reason TailscaleRejectReason // why the connection was rejected
-}
-
-func (rh TailscaleRejectedHeader) Flow() flowtrack.Tuple {
-	return flowtrack.Tuple{Src: rh.Src, Dst: rh.Dst}
-}
-
-func (rh TailscaleRejectedHeader) String() string {
-	return fmt.Sprintf("TSMP-reject-flow{%s %s > %s}: %s", rh.Proto, rh.Src, rh.Dst, rh.Reason)
-}
-
-type TSMPType uint8
-
-const (
-	TSMPTypeRejectedConn TSMPType = '!'
-)
-
-type TailscaleRejectReason byte
-
-const (
-	RejectedDueToACLs      TailscaleRejectReason = 'A'
-	RejectedDueToShieldsUp TailscaleRejectReason = 'S'
-)
-
-func (r TailscaleRejectReason) String() string {
-	switch r {
-	case RejectedDueToACLs:
-		return "acl"
-	case RejectedDueToShieldsUp:
-		return "shields"
-	}
-	return fmt.Sprintf("0x%02x", byte(r))
-}
-
-func (h TailscaleRejectedHeader) Len() int {
-	var ipHeaderLen int
-	if h.IPSrc.Is4() {
-		ipHeaderLen = ip4HeaderLength
-	} else if h.IPSrc.Is6() {
-		ipHeaderLen = ip6HeaderLength
-	}
-	return ipHeaderLen +
-		1 + // TSMPType byte
-		1 + // IPProto byte
-		1 + // TailscaleRejectReason byte
-		2*2 // 2 uint16 ports
-}
-
-func (h TailscaleRejectedHeader) Marshal(buf []byte) error {
-	if len(buf) < h.Len() {
-		return errSmallBuffer
-	}
-	if len(buf) > maxPacketLength {
-		return errLargePacket
-	}
-	if h.Src.IP.Is4() {
-		iph := IP4Header{
-			IPProto: TSMP,
-			Src:     h.IPSrc,
-			Dst:     h.IPDst,
-		}
-		iph.Marshal(buf)
-		buf = buf[ip4HeaderLength:]
-	} else if h.Src.IP.Is6() {
-		iph := IP6Header{
-			IPProto: TSMP,
-			Src:     h.IPSrc,
-			Dst:     h.IPDst,
-		}
-		iph.Marshal(buf)
-		buf = buf[ip6HeaderLength:]
-	} else {
-		return errors.New("bogus src IP")
-	}
-	buf[0] = byte(TSMPTypeRejectedConn)
-	buf[1] = byte(h.Proto)
-	buf[2] = byte(h.Reason)
-	binary.BigEndian.PutUint16(buf[3:5], h.Src.Port)
-	binary.BigEndian.PutUint16(buf[5:7], h.Dst.Port)
-	return nil
-}
-
-// AsTailscaleRejectedHeader parses pp as an incoming rejection
-// connection TSMP message.
-//
-// ok reports whether pp was a valid TSMP rejection packet.
-func (pp *Parsed) AsTailscaleRejectedHeader() (h TailscaleRejectedHeader, ok bool) {
-	p := pp.Payload()
-	if len(p) < 7 || p[0] != byte(TSMPTypeRejectedConn) {
-		return
-	}
-	return TailscaleRejectedHeader{
-		Proto:  IPProto(p[1]),
-		Reason: TailscaleRejectReason(p[2]),
-		IPSrc:  pp.Src.IP,
-		IPDst:  pp.Dst.IP,
-		Src:    netaddr.IPPort{IP: pp.Dst.IP, Port: binary.BigEndian.Uint16(p[3:5])},
-		Dst:    netaddr.IPPort{IP: pp.Src.IP, Port: binary.BigEndian.Uint16(p[5:7])},
-	}, true
-}

net/packet/tsmp_test.go

@@ -1,63 +0,0 @@
-// Copyright (c) 2021 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 packet
-
-import (
-	"testing"
-
-	"inet.af/netaddr"
-)
-
-func TestTailscaleRejectedHeader(t *testing.T) {
-	tests := []struct {
-		h       TailscaleRejectedHeader
-		wantStr string
-	}{
-		{
-			h: TailscaleRejectedHeader{
-				IPSrc:  netaddr.MustParseIP("5.5.5.5"),
-				IPDst:  netaddr.MustParseIP("1.2.3.4"),
-				Src:    netaddr.MustParseIPPort("1.2.3.4:567"),
-				Dst:    netaddr.MustParseIPPort("5.5.5.5:443"),
-				Proto:  TCP,
-				Reason: RejectedDueToACLs,
-			},
-			wantStr: "TSMP-reject-flow{TCP 1.2.3.4:567 > 5.5.5.5:443}: acl",
-		},
-		{
-			h: TailscaleRejectedHeader{
-				IPSrc:  netaddr.MustParseIP("2::2"),
-				IPDst:  netaddr.MustParseIP("1::1"),
-				Src:    netaddr.MustParseIPPort("[1::1]:567"),
-				Dst:    netaddr.MustParseIPPort("[2::2]:443"),
-				Proto:  UDP,
-				Reason: RejectedDueToShieldsUp,
-			},
-			wantStr: "TSMP-reject-flow{UDP [1::1]:567 > [2::2]:443}: shields",
-		},
-	}
-	for i, tt := range tests {
-		gotStr := tt.h.String()
-		if gotStr != tt.wantStr {
-			t.Errorf("%v. String = %q; want %q", i, gotStr, tt.wantStr)
-			continue
-		}
-		pkt := make([]byte, tt.h.Len())
-		tt.h.Marshal(pkt)
-
-		var p Parsed
-		p.Decode(pkt)
-		t.Logf("Parsed: %+v", p)
-		t.Logf("Parsed: %s", p.String())
-		back, ok := p.AsTailscaleRejectedHeader()
-		if !ok {
-			t.Errorf("%v. %q (%02x) didn't parse back", i, gotStr, pkt)
-			continue
-		}
-		if back != tt.h {
-			t.Errorf("%v. %q parsed back as %q", i, tt.h, back)
-		}
-	}
-}

safesocket/unixsocket.go

@@ -64,33 +64,10 @@ func listen(path string, port uint16) (ln net.Listener, _ uint16, err error) {
 	if err != nil {
 		return nil, 0, err
 	}
-	os.Chmod(path, socketPermissionsForOS())
+	os.Chmod(path, 0600)
 	return pipe, 0, err
 }
 
-// socketPermissionsForOS returns the permissions to use for the
-// tailscaled.sock.
-func socketPermissionsForOS() os.FileMode {
-	if runtime.GOOS == "linux" {
-		// On Linux, the ipn/ipnserver package looks at the Unix peer creds
-		// and only permits read-only actions from non-root users, so we want
-		// this opened up wider.
-		//
-		// TODO(bradfitz): unify this all one in place probably, moving some
-		// of ipnserver (which does much of the "safe" bits) here. Maybe
-		// instead of net.Listener, we should return a type that returns
-		// an identity in addition to a net.Conn? (returning a wrapped net.Conn
-		// would surprise downstream callers probably)
-		//
-		// TODO(bradfitz): if OpenBSD and FreeBSD do the equivalent peercreds
-		// stuff that's in ipn/ipnserver/conn_ucred.go, they should also
-		// return 0666 here.
-		return 0666
-	}
-	// Otherwise, root only.
-	return 0600
-}
-
 // connectMacOSAppSandbox connects to the Tailscale Network Extension,
 // which is necessarily running within the macOS App Sandbox.  Our
 // little dance to connect a regular user binary to the sandboxed

tailcfg/tailcfg.go

@@ -531,6 +531,8 @@ type MapRequest struct {
 	// Current DebugFlags values are:
 	//     * "warn-ip-forwarding-off": client is trying to be a subnet
 	//       router but their IP forwarding is broken.
+	//     * "v6-overlay": IPv6 development flag to have control send
+	//       v6 node addrs
 	//     * "minimize-netmap": have control minimize the netmap, removing
 	//       peers that are unreachable per ACLS.
 	DebugFlags []string `json:",omitempty"`

tsweb/tsweb.go

@@ -43,7 +43,7 @@ func registerCommonDebug(mux *http.ServeMux) {
 	expvar.Publish("counter_uptime_sec", expvar.Func(func() interface{} { return int64(Uptime().Seconds()) }))
 	mux.Handle("/debug/pprof/", Protected(http.DefaultServeMux)) // to net/http/pprof
 	mux.Handle("/debug/vars", Protected(http.DefaultServeMux))   // to expvar
-	mux.Handle("/debug/varz", Protected(http.HandlerFunc(VarzHandler)))
+	mux.Handle("/debug/varz", Protected(http.HandlerFunc(varzHandler)))
 	mux.Handle("/debug/gc", Protected(http.HandlerFunc(gcHandler)))
 }
 
@@ -371,7 +371,7 @@ func Error(code int, msg string, err error) HTTPError {
 	return HTTPError{Code: code, Msg: msg, Err: err}
 }
 
-// VarzHandler is an HTTP handler to write expvar values into the
+// varzHandler is an HTTP handler to write expvar values into the
 // prometheus export format:
 //
 //   https://github.com/prometheus/docs/blob/master/content/docs/instrumenting/exposition_formats.md
@@ -388,7 +388,7 @@ func Error(code int, msg string, err error) HTTPError {
 //     is not exported.
 //
 // This will evolve over time, or perhaps be replaced.
-func VarzHandler(w http.ResponseWriter, r *http.Request) {
+func varzHandler(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "text/plain; version=0.0.4")
 
 	var dump func(prefix string, kv expvar.KeyValue)

types/key/key.go

@@ -82,15 +82,6 @@ func (k Private) Public() Public {
 	return Public(pub)
 }
 
-func (k Private) SharedSecret(pub Public) (ss [32]byte) {
-	apk := (*[32]byte)(&pub)
-	ask := (*[32]byte)(&k)
-	//lint:ignore SA1019 Code copied from wireguard-go, we aim for
-	//minimal changes from it.
-	curve25519.ScalarMult(&ss, ask, apk)
-	return ss
-}
-
 // NewPublicFromHexMem parses a public key in its hex form, given in m.
 // The provided m must be exactly 64 bytes in length.
 func NewPublicFromHexMem(m mem.RO) (Public, error) {

types/logger/logger.go

@@ -132,7 +132,7 @@ func RateLimitedFn(logf Logf, f time.Duration, burst int, maxCache int) Logf {
 			logf(format, args...)
 		case warn:
 			// For the warning, log the specific format string
-			logf("[RATE LIMITED] format string \"%s\" (example: \"%s\")", format, fmt.Sprintf(format, args...))
+			logf("[RATE LIMITED] format string \"%s\"", format)
 		}
 	}
 }
@@ -192,27 +192,3 @@ func Filtered(logf Logf, allow func(s string) bool) Logf {
 		logf(format, args...)
 	}
 }
-
-// LogfCloser wraps logf to create a logger that can be closed.
-// Calling close makes all future calls to newLogf into no-ops.
-func LogfCloser(logf Logf) (newLogf Logf, close func()) {
-	var (
-		mu     sync.Mutex
-		closed bool
-	)
-	close = func() {
-		mu.Lock()
-		defer mu.Unlock()
-		closed = true
-	}
-	newLogf = func(msg string, args ...interface{}) {
-		mu.Lock()
-		if closed {
-			mu.Unlock()
-			return
-		}
-		mu.Unlock()
-		logf(msg, args...)
-	}
-	return newLogf, close
-}

types/logger/logger_test.go

@@ -45,8 +45,8 @@ func TestRateLimiter(t *testing.T) {
 		"templated format string no. 0",
 		"boring string with constant formatting (constant)",
 		"templated format string no. 1",
-		"[RATE LIMITED] format string \"boring string with constant formatting %s\" (example: \"boring string with constant formatting (constant)\")",
-		"[RATE LIMITED] format string \"templated format string no. %d\" (example: \"templated format string no. 2\")",
+		"[RATE LIMITED] format string \"boring string with constant formatting %s\"",
+		"[RATE LIMITED] format string \"templated format string no. %d\"",
 		"Make sure this string makes it through the rest (that are blocked) 4",
 		"4 shouldn't get filtered.",
 	}

wgengine/filter/filter.go

@@ -39,8 +39,6 @@ type Filter struct {
 	// to an outbound connection that this node made, even if those
 	// incoming packets don't get accepted by matches above.
 	state *filterState
-
-	shieldsUp bool
 }
 
 // filterState is a state cache of past seen packets.
@@ -56,18 +54,15 @@ const lruMax = 512
 type Response int
 
 const (
-	Drop         Response = iota // do not continue processing packet.
-	DropSilently                 // do not continue processing packet, but also don't log
-	Accept                       // continue processing packet.
-	noVerdict                    // no verdict yet, continue running filter
+	Drop      Response = iota // do not continue processing packet.
+	Accept                    // continue processing packet.
+	noVerdict                 // no verdict yet, continue running filter
 )
 
 func (r Response) String() string {
 	switch r {
 	case Drop:
 		return "Drop"
-	case DropSilently:
-		return "DropSilently"
 	case Accept:
 		return "Accept"
 	case noVerdict:
@@ -77,10 +72,6 @@ func (r Response) String() string {
 	}
 }
 
-func (r Response) IsDrop() bool {
-	return r == Drop || r == DropSilently
-}
-
 // RunFlags controls the filter's debug log verbosity at runtime.
 type RunFlags int
 
@@ -132,12 +123,6 @@ func NewAllowNone(logf logger.Logf) *Filter {
 	return New(nil, nil, nil, logf)
 }
 
-func NewShieldsUpFilter(logf logger.Logf) *Filter {
-	f := New(nil, nil, nil, logf)
-	f.shieldsUp = true
-	return f
-}
-
 // New creates a new packet filter. The filter enforces that incoming
 // packets must be destined to an IP in localNets, and must be allowed
 // by matches. If shareStateWith is non-nil, the returned filter
@@ -268,10 +253,6 @@ func (f *Filter) CheckTCP(srcIP, dstIP netaddr.IP, dstPort uint16) Response {
 	return f.RunIn(pkt, 0)
 }
 
-// ShieldsUp reports whether this is a "shields up" (block everything
-// incoming) filter.
-func (f *Filter) ShieldsUp() bool { return f.shieldsUp }
-
 // RunIn determines whether this node is allowed to receive q from a
 // Tailscale peer.
 func (f *Filter) RunIn(q *packet.Parsed, rf RunFlags) Response {
@@ -358,8 +339,6 @@ func (f *Filter) runIn4(q *packet.Parsed) (r Response, why string) {
 		if f.matches4.match(q) {
 			return Accept, "udp ok"
 		}
-	case packet.TSMP:
-		return Accept, "tsmp ok"
 	default:
 		return Drop, "Unknown proto"
 	}

wgengine/magicsock/legacy.go

@@ -5,24 +5,17 @@
 package magicsock
 
 import (
-	"bytes"
-	"crypto/hmac"
-	"crypto/subtle"
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"hash"
 	"net"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/tailscale/wireguard-go/conn"
-	"github.com/tailscale/wireguard-go/tai64n"
+	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/wgcfg"
-	"golang.org/x/crypto/blake2s"
-	"golang.org/x/crypto/chacha20poly1305"
-	"golang.org/x/crypto/poly1305"
 	"inet.af/netaddr"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/types/key"
@@ -30,16 +23,9 @@ import (
 	"tailscale.com/types/wgkey"
 )
 
-var (
-	errNoDestinations = errors.New("magicsock: no destinations")
-	errDisabled       = errors.New("magicsock: legacy networking disabled")
-)
+var errNoDestinations = errors.New("magicsock: no destinations")
 
 func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs string) (conn.Endpoint, error) {
-	if c.disableLegacy {
-		return nil, errDisabled
-	}
-
 	a := &addrSet{
 		Logf:      c.logf,
 		publicKey: pk,
@@ -84,62 +70,17 @@ func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs string) (conn.End
 	return a, nil
 }
 
-func (c *Conn) findLegacyEndpointLocked(ipp netaddr.IPPort, addr *net.UDPAddr, packet []byte) conn.Endpoint {
-	if c.disableLegacy {
-		return nil
-	}
-
+func (c *Conn) findLegacyEndpointLocked(ipp netaddr.IPPort, addr *net.UDPAddr) conn.Endpoint {
 	// Pre-disco: look up their addrSet.
 	if as, ok := c.addrsByUDP[ipp]; ok {
-		as.updateDst(addr)
 		return as
 	}
 
-	// We don't know who this peer is. It's possible that it's one of
-	// our legitimate peers and they've roamed to an address we don't
-	// know. If this is a handshake packet, we can try to identify the
-	// peer in question.
-	if as := c.peerFromPacketLocked(packet); as != nil {
-		as.updateDst(addr)
-		return as
-	}
-
-	// We have no idea who this is, drop the packet.
-	//
-	// In the past, when this magicsock implementation was the main
-	// one, we tried harder to find a match here: we would pass the
-	// packet into wireguard-go with a "singleEndpoint" implementation
-	// that wrapped the UDPAddr. Then, a patch we added to
-	// wireguard-go would call UpdateDst on that singleEndpoint after
-	// decrypting the packet and identifying the peer (if any),
-	// allowing us to update the relevant addrSet.
-	//
-	// This was a significant out of tree patch to wireguard-go, so we
-	// got rid of it, and instead switched to this logic you're
-	// reading now, which makes a best effort to identify sources for
-	// handshake packets (because they're relatively easy to turn into
-	// a peer public key statelessly), but otherwise drops packets
-	// that come from "roaming" addresses that aren't known to
-	// magicsock.
-	//
-	// The practical consequence of this is that some complex NAT
-	// traversal cases will now fail between a very old Tailscale
-	// client (0.96 and earlier) and a very new Tailscale
-	// client. However, those scenarios were likely also failing on
-	// all-old clients, because the probabilistic NAT opening didn't
-	// work reliably. So, in practice, this simplification means
-	// connectivity looks like this:
-	//
-	//  - old+old client: unchanged
-	//  - old+new client (easy network topology): unchanged
-	//  - old+new client (hard network topology): was bad, now a bit worse
-	//  - new+new client: unchanged
-	//
-	// This degradation is acceptable in that it continues to support
-	// the incremental upgrade of old clients that currently work
-	// well, which is our primary goal for the <100 clients still left
-	// on the oldest pre-DERP versions (as of 2021-01-12).
-	return nil
+	// Pre-disco: the peer that sent this packet has roamed beyond
+	// the knowledge provided by the control server.  If the
+	// packet is valid wireguard will call UpdateDst on the
+	// original endpoint using this addr.
+	return (*singleEndpoint)(addr)
 }
 
 func (c *Conn) resetAddrSetStatesLocked() {
@@ -149,11 +90,17 @@ func (c *Conn) resetAddrSetStatesLocked() {
 	}
 }
 
-func (c *Conn) sendAddrSet(b []byte, as *addrSet) error {
-	if c.disableLegacy {
-		return errDisabled
+func (c *Conn) sendSingleEndpoint(b []byte, se *singleEndpoint) error {
+	addr := (*net.UDPAddr)(se)
+	if addr.IP.Equal(derpMagicIP) {
+		c.logf("magicsock: [unexpected] DERP BUG: attempting to send packet to DERP address %v", addr)
+		return nil
 	}
+	_, err := c.sendUDPStd(addr, b)
+	return err
+}
 
+func (c *Conn) sendAddrSet(b []byte, as *addrSet) error {
 	var addrBuf [8]netaddr.IPPort
 	dsts, roamAddr := as.appendDests(addrBuf[:0], b)
 
@@ -182,71 +129,15 @@ func (c *Conn) sendAddrSet(b []byte, as *addrSet) error {
 	return ret
 }
 
-// peerFromPacketLocked extracts returns the addrSet for the peer who sent
-// packet, if derivable.
-//
-// The derived addrSet is a hint, not a cryptographically strong
-// assertion. The returned value MUST NOT be used for any security
-// critical function. Callers MUST assume that the addrset can be
-// picked by a remote attacker.
-func (c *Conn) peerFromPacketLocked(packet []byte) *addrSet {
-	if len(packet) < 4 {
-		return nil
-	}
-	msgType := binary.LittleEndian.Uint32(packet[:4])
-	if msgType != messageInitiationType {
-		// Can't get peer out of a non-handshake packet.
-		return nil
-	}
-
-	var msg messageInitiation
-	reader := bytes.NewReader(packet)
-	err := binary.Read(reader, binary.LittleEndian, &msg)
-	if err != nil {
-		return nil
-	}
-
-	// Process just enough of the handshake to extract the long-term
-	// peer public key. We don't verify the handshake all the way, so
-	// this may be a spoofed packet. The extracted peer MUST NOT be
-	// used for any security critical function. In our case, we use it
-	// as a hint for roaming addresses.
-	var (
-		pub      = c.privateKey.Public()
-		hash     [blake2s.Size]byte
-		chainKey [blake2s.Size]byte
-		peerPK   key.Public
-		boxKey   [chacha20poly1305.KeySize]byte
-	)
-
-	mixHash(&hash, &initialHash, pub[:])
-	mixHash(&hash, &hash, msg.Ephemeral[:])
-	mixKey(&chainKey, &initialChainKey, msg.Ephemeral[:])
-
-	ss := c.privateKey.SharedSecret(key.Public(msg.Ephemeral))
-	if isZero(ss[:]) {
-		return nil
-	}
-
-	kdf2(&chainKey, &boxKey, chainKey[:], ss[:])
-	aead, _ := chacha20poly1305.New(boxKey[:])
-	_, err = aead.Open(peerPK[:0], zeroNonce[:], msg.Static[:], hash[:])
-	if err != nil {
-		return nil
-	}
-
-	return c.addrsByKey[peerPK]
-}
-
 func shouldSprayPacket(b []byte) bool {
 	if len(b) < 4 {
 		return false
 	}
 	msgType := binary.LittleEndian.Uint32(b[:4])
 	switch msgType {
-	case messageInitiationType,
-		messageResponseType,
-		messageCookieReplyType: // TODO: necessary?
+	case device.MessageInitiationType,
+		device.MessageResponseType,
+		device.MessageCookieReplyType: // TODO: necessary?
 		return true
 	}
 	return false
@@ -434,6 +325,19 @@ func (a *addrSet) dst() netaddr.IPPort {
 	return a.ipPorts[i]
 }
 
+// packUDPAddr packs a UDPAddr in the form wanted by WireGuard.
+func packUDPAddr(ua *net.UDPAddr) []byte {
+	ip := ua.IP.To4()
+	if ip == nil {
+		ip = ua.IP
+	}
+	b := make([]byte, 0, len(ip)+2)
+	b = append(b, ip...)
+	b = append(b, byte(ua.Port))
+	b = append(b, byte(ua.Port>>8))
+	return b
+}
+
 func (a *addrSet) DstToBytes() []byte {
 	return packIPPort(a.dst())
 }
@@ -448,9 +352,7 @@ func (a *addrSet) SrcIP() net.IP       { return nil }
 func (a *addrSet) SrcToString() string { return "" }
 func (a *addrSet) ClearSrc()           {}
 
-// updateDst records receipt of a packet from new. This is used to
-// potentially update the transmit address used for this addrSet.
-func (a *addrSet) updateDst(new *net.UDPAddr) error {
+func (a *addrSet) UpdateDst(new *net.UDPAddr) error {
 	if new.IP.Equal(derpMagicIP) {
 		// Never consider DERP addresses as a viable candidate for
 		// either curAddr or roamAddr. It's only ever a last resort
@@ -578,103 +480,43 @@ func (as *addrSet) populatePeerStatus(ps *ipnstate.PeerStatus) {
 	}
 }
 
-func (a *addrSet) Addrs() string {
-	var addrs []string
+func (a *addrSet) Addrs() []wgcfg.Endpoint {
+	var eps []wgcfg.Endpoint
 	for _, addr := range a.addrs {
-		addrs = append(addrs, addr.String())
+		eps = append(eps, wgcfg.Endpoint{
+			Host: addr.IP.String(),
+			Port: uint16(addr.Port),
+		})
 	}
 
 	a.mu.Lock()
 	defer a.mu.Unlock()
 	if a.roamAddr != nil {
-		addrs = append(addrs, a.roamAddr.String())
+		eps = append(eps, wgcfg.Endpoint{
+			Host: a.roamAddr.IP.String(),
+			Port: uint16(a.roamAddr.Port),
+		})
 	}
-	return strings.Join(addrs, ",")
+	return eps
 }
 
-// Message types copied from wireguard-go/device/noise-protocol.go
-const (
-	messageInitiationType  = 1
-	messageResponseType    = 2
-	messageCookieReplyType = 3
-)
+// singleEndpoint is a wireguard-go/conn.Endpoint used for "roaming
+// addressed" in releases of Tailscale that predate discovery
+// messages. New peers use discoEndpoint.
+type singleEndpoint net.UDPAddr
 
-// Cryptographic constants copied from wireguard-go/device/noise-protocol.go
-var (
-	noiseConstruction = "Noise_IKpsk2_25519_ChaChaPoly_BLAKE2s"
-	wgIdentifier      = "WireGuard v1 zx2c4 Jason@zx2c4.com"
-	initialChainKey   [blake2s.Size]byte
-	initialHash       [blake2s.Size]byte
-	zeroNonce         [chacha20poly1305.NonceSize]byte
-)
-
-func init() {
-	initialChainKey = blake2s.Sum256([]byte(noiseConstruction))
-	mixHash(&initialHash, &initialChainKey, []byte(wgIdentifier))
+func (e *singleEndpoint) ClearSrc()           {}
+func (e *singleEndpoint) DstIP() net.IP       { return (*net.UDPAddr)(e).IP }
+func (e *singleEndpoint) SrcIP() net.IP       { return nil }
+func (e *singleEndpoint) SrcToString() string { return "" }
+func (e *singleEndpoint) DstToString() string { return (*net.UDPAddr)(e).String() }
+func (e *singleEndpoint) DstToBytes() []byte  { return packUDPAddr((*net.UDPAddr)(e)) }
+func (e *singleEndpoint) UpdateDst(dst *net.UDPAddr) error {
+	return fmt.Errorf("magicsock.singleEndpoint(%s).UpdateDst(%s): should never be called", (*net.UDPAddr)(e), dst)
 }
-
-// messageInitiation is the same as wireguard-go's MessageInitiation,
-// from wireguard-go/device/noise-protocol.go.
-type messageInitiation struct {
-	Type      uint32
-	Sender    uint32
-	Ephemeral wgcfg.Key
-	Static    [wgcfg.KeySize + poly1305.TagSize]byte
-	Timestamp [tai64n.TimestampSize + poly1305.TagSize]byte
-	MAC1      [blake2s.Size128]byte
-	MAC2      [blake2s.Size128]byte
-}
-
-func mixKey(dst *[blake2s.Size]byte, c *[blake2s.Size]byte, data []byte) {
-	kdf1(dst, c[:], data)
-}
-
-func mixHash(dst *[blake2s.Size]byte, h *[blake2s.Size]byte, data []byte) {
-	hash, _ := blake2s.New256(nil)
-	hash.Write(h[:])
-	hash.Write(data)
-	hash.Sum(dst[:0])
-	hash.Reset()
-}
-
-func hmac1(sum *[blake2s.Size]byte, key, in0 []byte) {
-	mac := hmac.New(func() hash.Hash {
-		h, _ := blake2s.New256(nil)
-		return h
-	}, key)
-	mac.Write(in0)
-	mac.Sum(sum[:0])
-}
-
-func hmac2(sum *[blake2s.Size]byte, key, in0, in1 []byte) {
-	mac := hmac.New(func() hash.Hash {
-		h, _ := blake2s.New256(nil)
-		return h
-	}, key)
-	mac.Write(in0)
-	mac.Write(in1)
-	mac.Sum(sum[:0])
-}
-
-func kdf1(t0 *[blake2s.Size]byte, key, input []byte) {
-	hmac1(t0, key, input)
-	hmac1(t0, t0[:], []byte{0x1})
-}
-
-func kdf2(t0, t1 *[blake2s.Size]byte, key, input []byte) {
-	var prk [blake2s.Size]byte
-	hmac1(&prk, key, input)
-	hmac1(t0, prk[:], []byte{0x1})
-	hmac2(t1, prk[:], t0[:], []byte{0x2})
-	for i := range prk[:] {
-		prk[i] = 0
-	}
-}
-
-func isZero(val []byte) bool {
-	acc := 1
-	for _, b := range val {
-		acc &= subtle.ConstantTimeByteEq(b, 0)
-	}
-	return acc == 1
+func (e *singleEndpoint) Addrs() []wgcfg.Endpoint {
+	return []wgcfg.Endpoint{{
+		Host: e.IP.String(),
+		Port: uint16(e.Port),
+	}}
 }

wgengine/magicsock/magicsock.go

@@ -28,6 +28,7 @@ import (
 	"time"
 
 	"github.com/tailscale/wireguard-go/conn"
+	"github.com/tailscale/wireguard-go/wgcfg"
 	"go4.org/mem"
 	"golang.org/x/crypto/nacl/box"
 	"golang.org/x/time/rate"
@@ -49,6 +50,7 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/nettype"
 	"tailscale.com/types/opt"
+	"tailscale.com/types/structs"
 	"tailscale.com/types/wgkey"
 	"tailscale.com/version"
 )
@@ -118,7 +120,6 @@ type Conn struct {
 	packetListener   nettype.PacketListener
 	noteRecvActivity func(tailcfg.DiscoKey) // or nil, see Options.NoteRecvActivity
 	simulatedNetwork bool
-	disableLegacy    bool
 
 	// ================================================================
 	// No locking required to access these fields, either because
@@ -127,7 +128,6 @@ type Conn struct {
 
 	connCtx       context.Context // closed on Conn.Close
 	connCtxCancel func()          // closes connCtx
-	donec         <-chan struct{} // connCtx.Done()'s to avoid context.cancelCtx.Done()'s mutex per call
 
 	// pconn4 and pconn6 are the underlying UDP sockets used to
 	// send/receive packets for wireguard and other magicsock
@@ -146,21 +146,21 @@ type Conn struct {
 	// TODO(danderson): now that we have global rate-limiting, is this still useful?
 	sendLogLimit *rate.Limiter
 
+	// bufferedIPv4From and bufferedIPv4Packet are owned by
+	// ReceiveIPv4, and used when both a DERP and IPv4 packet arrive
+	// at the same time. It stores the IPv4 packet for use in the next call.
+	bufferedIPv4From   netaddr.IPPort // if non-zero, then bufferedIPv4Packet is valid
+	bufferedIPv4Packet []byte         // the received packet (reused, owned by ReceiveIPv4)
+
 	// stunReceiveFunc holds the current STUN packet processing func.
 	// Its Loaded value is always non-nil.
 	stunReceiveFunc atomic.Value // of func(p []byte, fromAddr *net.UDPAddr)
 
-	// derpRecvCh is used by ReceiveIPv4 to read DERP messages.
+	// udpRecvCh and derpRecvCh are used by ReceiveIPv4 to multiplex
+	// reads from DERP and the pconn4.
+	udpRecvCh  chan udpReadResult
 	derpRecvCh chan derpReadResult
 
-	// derpRecvCountAtomic is atomically incremented by runDerpReader whenever
-	// a DERP message arrives. It's incremented before runDerpReader is interrupted.
-	derpRecvCountAtomic int64
-	// derpRecvCountLast is used by ReceiveIPv4 to compare against
-	// its last read value of derpRecvCountAtomic to determine
-	// whether a DERP channel read should be done.
-	derpRecvCountLast int64 // owned by ReceiveIPv4
-
 	// ============================================================
 	mu     sync.Mutex // guards all following fields; see userspaceEngine lock ordering rules
 	muCond *sync.Cond
@@ -383,11 +383,6 @@ type Options struct {
 	// triggering macOS and Windows firwall dialog boxes during
 	// "go test").
 	SimulatedNetwork bool
-
-	// DisableLegacyNetworking disables legacy peer handling. When
-	// enabled, only active discovery-aware nodes will be able to
-	// communicate with Conn.
-	DisableLegacyNetworking bool
 }
 
 func (o *Options) logf() logger.Logf {
@@ -415,11 +410,11 @@ func (o *Options) derpActiveFunc() func() {
 // of NewConn. Mostly for tests.
 func newConn() *Conn {
 	c := &Conn{
-		disableLegacy:   true,
 		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),
 		endpointOfDisco: make(map[tailcfg.DiscoKey]*discoEndpoint),
@@ -446,14 +441,12 @@ func NewConn(opts Options) (*Conn, error) {
 	c.packetListener = opts.PacketListener
 	c.noteRecvActivity = opts.NoteRecvActivity
 	c.simulatedNetwork = opts.SimulatedNetwork
-	c.disableLegacy = opts.DisableLegacyNetworking
 
 	if err := c.initialBind(); err != nil {
 		return nil, err
 	}
 
 	c.connCtx, c.connCtxCancel = context.WithCancel(context.Background())
-	c.donec = c.connCtx.Done()
 	c.netChecker = &netcheck.Client{
 		Logf:                logger.WithPrefix(c.logf, "netcheck: "),
 		GetSTUNConn4:        func() netcheck.STUNConn { return c.pconn4 },
@@ -486,6 +479,8 @@ func (c *Conn) Start() {
 	go c.periodicDerpCleanup()
 }
 
+func (c *Conn) donec() <-chan struct{} { return c.connCtx.Done() }
+
 // ignoreSTUNPackets sets a STUN packet processing func that does nothing.
 func (c *Conn) ignoreSTUNPackets() {
 	c.stunReceiveFunc.Store(func([]byte, netaddr.IPPort) {})
@@ -687,16 +682,6 @@ func (c *Conn) callNetInfoCallback(ni *tailcfg.NetInfo) {
 	}
 }
 
-// addValidDiscoPathForTest makes addr a validated disco address for
-// discoKey. It's used in tests to enable receiving of packets from
-// addr without having to spin up the entire active discovery
-// machinery.
-func (c *Conn) addValidDiscoPathForTest(discoKey tailcfg.DiscoKey, addr netaddr.IPPort) {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-	c.discoOfAddr[addr] = discoKey
-}
-
 func (c *Conn) SetNetInfoCallback(fn func(*tailcfg.NetInfo)) {
 	if fn == nil {
 		panic("nil NetInfoCallback")
@@ -1023,6 +1008,8 @@ func (c *Conn) Send(b []byte, ep conn.Endpoint) error {
 		panic(fmt.Sprintf("[unexpected] Endpoint type %T", v))
 	case *discoEndpoint:
 		return v.send(b)
+	case *singleEndpoint:
+		return c.sendSingleEndpoint(b, v)
 	case *addrSet:
 		return c.sendAddrSet(b, v)
 	}
@@ -1097,7 +1084,7 @@ func (c *Conn) sendAddr(addr netaddr.IPPort, pubKey key.Public, b []byte) (sent
 	copy(pkt, b)
 
 	select {
-	case <-c.donec:
+	case <-c.donec():
 		return false, errConnClosed
 	case ch <- derpWriteRequest{addr, pubKey, pkt}:
 		return true, nil
@@ -1377,16 +1364,7 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, d
 			// Ignore.
 			// TODO: handle endpoint notification messages.
 			continue
-
 		}
-		// Before we wake up ReceiveIPv4 with SetReadDeadline,
-		// note that a DERP packet has arrived.  ReceiveIPv4
-		// will read this field to note that its UDP read
-		// error is due to us.
-		atomic.AddInt64(&c.derpRecvCountAtomic, 1)
-		// Cancel the pconn read goroutine.
-		c.pconn4.SetReadDeadline(aLongTimeAgo)
-
 		select {
 		case <-ctx.Done():
 			return
@@ -1440,7 +1418,7 @@ func (c *Conn) runDerpWriter(ctx context.Context, dc *derphttp.Client, ch <-chan
 // Endpoint to find the UDPAddr to return to wireguard anyway, so no
 // benefit unless we can, say, always return the same fake UDPAddr for
 // all packets.
-func (c *Conn) findEndpoint(ipp netaddr.IPPort, addr *net.UDPAddr, packet []byte) conn.Endpoint {
+func (c *Conn) findEndpoint(ipp netaddr.IPPort, addr *net.UDPAddr) conn.Endpoint {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
@@ -1452,16 +1430,71 @@ func (c *Conn) findEndpoint(ipp netaddr.IPPort, addr *net.UDPAddr, packet []byte
 		}
 	}
 
-	if addr == nil {
-		addr = ipp.UDPAddr()
-	}
-	return c.findLegacyEndpointLocked(ipp, addr, packet)
+	return c.findLegacyEndpointLocked(ipp, addr)
+}
+
+type udpReadResult struct {
+	_    structs.Incomparable
+	n    int
+	err  error
+	addr *net.UDPAddr
+	ipp  netaddr.IPPort
 }
 
 // aLongTimeAgo is a non-zero time, far in the past, used for
 // immediate cancellation of network operations.
 var aLongTimeAgo = time.Unix(233431200, 0)
 
+// awaitUDP4 reads a single IPv4 UDP packet (or an error) and sends it
+// to c.udpRecvCh, skipping over (but handling) any STUN replies.
+func (c *Conn) awaitUDP4(b []byte) {
+	for {
+		n, pAddr, err := c.pconn4.ReadFrom(b)
+		if err != nil {
+			select {
+			case c.udpRecvCh <- udpReadResult{err: err}:
+			case <-c.donec():
+			}
+			return
+		}
+		addr := pAddr.(*net.UDPAddr)
+		ipp, ok := netaddr.FromStdAddr(addr.IP, addr.Port, addr.Zone)
+		if !ok {
+			continue
+		}
+		if stun.Is(b[:n]) {
+			c.stunReceiveFunc.Load().(func([]byte, netaddr.IPPort))(b[:n], ipp)
+			continue
+		}
+		if c.handleDiscoMessage(b[:n], ipp) {
+			continue
+		}
+
+		select {
+		case c.udpRecvCh <- udpReadResult{n: n, addr: addr, ipp: ipp}:
+		case <-c.donec():
+		}
+		return
+	}
+}
+
+// wgRecvAddr returns the net.UDPAddr we tell wireguard-go the address
+// from which we received a packet for an endpoint.
+//
+// ipp is required. addr can be optionally provided.
+func wgRecvAddr(e conn.Endpoint, ipp netaddr.IPPort, addr *net.UDPAddr) *net.UDPAddr {
+	if ipp == (netaddr.IPPort{}) {
+		panic("zero ipp")
+	}
+	if de, ok := e.(*discoEndpoint); ok {
+		return de.fakeWGAddrStd
+	}
+	if addr != nil {
+		return addr
+	}
+	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).
@@ -1474,89 +1507,121 @@ func (c *Conn) noteRecvActivityFromEndpoint(e conn.Endpoint) {
 	}
 }
 
-func (c *Conn) ReceiveIPv6(b []byte) (int, conn.Endpoint, error) {
-	if c.pconn6 == nil {
-		return 0, nil, syscall.EAFNOSUPPORT
+func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, addr *net.UDPAddr, err error) {
+Top:
+	// First, process any buffered packet from earlier.
+	if from := c.bufferedIPv4From; from != (netaddr.IPPort{}) {
+		c.bufferedIPv4From = netaddr.IPPort{}
+		addr = from.UDPAddr()
+		ep := c.findEndpoint(from, addr)
+		c.noteRecvActivityFromEndpoint(ep)
+		return copy(b, c.bufferedIPv4Packet), ep, wgRecvAddr(ep, from, addr), nil
 	}
-	for {
-		n, pAddr, err := c.pconn6.ReadFrom(b)
-		if err != nil {
-			return 0, nil, err
-		}
-		if ep, ok := c.receiveIP(b[:n], pAddr.(*net.UDPAddr)); ok {
-			return n, ep, nil
-		}
-	}
-}
 
-func (c *Conn) derpPacketArrived() bool {
-	rc := atomic.LoadInt64(&c.derpRecvCountAtomic)
-	if rc != c.derpRecvCountLast {
-		c.derpRecvCountLast = rc
-		return true
-	}
-	return false
-}
+	go c.awaitUDP4(b)
 
-// ReceiveIPv4 is called by wireguard-go to receive an IPv4 packet.
-// In Tailscale's case, that packet might also arrive via DERP. A DERP packet arrival
-// aborts the pconn4 read deadline to make it fail.
-func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, err error) {
-	for {
-		n, pAddr, err := c.pconn4.ReadFrom(b)
-		if err != nil {
-			// If the pconn4 read failed, the likely reason is a DERP reader received
-			// a packet and interrupted us.
-			// It's possible for ReadFrom to return a non deadline exceeded error
-			// and for there to have also had a DERP packet arrive, but that's fine:
-			// we'll get the same error from ReadFrom later.
-			if c.derpPacketArrived() {
-				c.pconn4.SetReadDeadline(time.Time{}) // restore
-				n, ep, err = c.receiveIPv4DERP(b)
-				if err == errLoopAgain {
-					continue
-				}
-				return n, ep, err
-			}
-			return 0, nil, err
-		}
-		if ep, ok := c.receiveIP(b[:n], pAddr.(*net.UDPAddr)); ok {
-			return n, ep, nil
-		}
-	}
-}
+	// Once the above goroutine has started, it owns b until it writes
+	// to udpRecvCh. The code below must not access b until it's
+	// completed a successful receive on udpRecvCh.
 
-// receiveIP is the shared bits of ReceiveIPv4 and ReceiveIPv6.
-func (c *Conn) receiveIP(b []byte, ua *net.UDPAddr) (ep conn.Endpoint, ok bool) {
-	ipp, ok := netaddr.FromStdAddr(ua.IP, ua.Port, ua.Zone)
-	if !ok {
-		return
-	}
-	if stun.Is(b) {
-		c.stunReceiveFunc.Load().(func([]byte, netaddr.IPPort))(b, ipp)
-		return
-	}
-	if c.handleDiscoMessage(b, ipp) {
-		return
-	}
-	ep = c.findEndpoint(ipp, ua, b)
-	if ep == nil {
-		return
-	}
-	c.noteRecvActivityFromEndpoint(ep)
-	return ep, true
-}
+	var ipp netaddr.IPPort
 
-var errLoopAgain = errors.New("received packet was not a wireguard-go packet or no endpoint found")
-
-// receiveIPv4DERP reads a packet from c.derpRecvCh into b and returns the associated endpoint.
-//
-// If the packet was a disco message or the peer endpoint wasn't
-// found, the returned error is errLoopAgain.
-func (c *Conn) receiveIPv4DERP(b []byte) (n int, ep conn.Endpoint, err error) {
-	var dm derpReadResult
 	select {
-	case <-c.donec:
+	case dm := <-c.derpRecvCh:
+		// Cancel the pconn read goroutine
+		c.pconn4.SetReadDeadline(aLongTimeAgo)
+		// Wait for the UDP-reading goroutine to be done, since it's currently
+		// the owner of the b []byte buffer:
+		select {
+		case um := <-c.udpRecvCh:
+			if um.err != nil {
+				// The normal case. The SetReadDeadline interrupted
+				// the read and we get an error which we now ignore.
+			} else {
+				// The pconn.ReadFrom succeeded and was about to send,
+				// but DERP sent first. So now we have both ready.
+				// Save the UDP packet away for use by the next
+				// ReceiveIPv4 call.
+				c.bufferedIPv4From = um.ipp
+				c.bufferedIPv4Packet = append(c.bufferedIPv4Packet[:0], b[:um.n]...)
+			}
+			c.pconn4.SetReadDeadline(time.Time{})
+		case <-c.donec():
+			return 0, nil, nil, errors.New("Conn closed")
+		}
+		var regionID int
+		n, regionID = dm.n, dm.regionID
+		ncopy := dm.copyBuf(b)
+		if ncopy != n {
+			err = fmt.Errorf("received DERP packet of length %d that's too big for WireGuard ReceiveIPv4 buf size %d", n, ncopy)
+			c.logf("magicsock: %v", err)
+			return 0, nil, nil, err
+		}
+
+		ipp = netaddr.IPPort{IP: derpMagicIPAddr, Port: uint16(regionID)}
+		if c.handleDiscoMessage(b[:n], ipp) {
+			goto Top
+		}
+
+		var (
+			didNoteRecvActivity bool
+			discoEp             *discoEndpoint
+			asEp                *addrSet
+		)
+		c.mu.Lock()
+		if dk, ok := c.discoOfNode[tailcfg.NodeKey(dm.src)]; ok {
+			discoEp = c.endpointOfDisco[dk]
+			// If we know about the node (it's in discoOfNode) but don't know about the
+			// endpoint, that's because it's an idle peer that doesn't yet exist in the
+			// wireguard config. So run the receive hook, if defined, which should
+			// create the wireguard peer.
+			if discoEp == nil && c.noteRecvActivity != nil {
+				didNoteRecvActivity = true
+				c.mu.Unlock()          // release lock before calling noteRecvActivity
+				c.noteRecvActivity(dk) // (calls back into CreateEndpoint)
+				// Now require the lock. No invariants need to be rechecked; just
+				// 1-2 map lookups follow that are harmless if, say, the peer has
+				// been deleted during this time.  In that case we'll treate it as a
+				// legacy pre-disco UDP receive and hand it to wireguard which'll
+				// likely just drop it.
+				c.mu.Lock()
+
+				discoEp = c.endpointOfDisco[dk]
+				c.logf("magicsock: DERP packet received from idle peer %v; created=%v", dm.src.ShortString(), ep != nil)
+			}
+		}
+		asEp = c.addrsByKey[dm.src]
+		c.mu.Unlock()
+
+		if discoEp != nil {
+			ep = discoEp
+		} else if asEp != nil {
+			ep = asEp
+		} else {
+			key := wgkey.Key(dm.src)
+			c.logf("magicsock: DERP packet from unknown key: %s", key.ShortString())
+			// TODO(danderson): after we fail to find a DERP endpoint, we
+			// seem to be falling through to passing the packet to
+			// wireguard with a garbage singleEndpoint. This feels wrong,
+			// should we goto Top above?
+			ep = c.findEndpoint(ipp, addr)
+		}
+
+		if !didNoteRecvActivity {
+			c.noteRecvActivityFromEndpoint(ep)
+		}
+		return n, ep, wgRecvAddr(ep, ipp, addr), nil
+
+	case um := <-c.udpRecvCh:
+		if um.err != nil {
+			return 0, nil, nil, err
+		}
+		n, addr, ipp = um.n, um.addr, um.ipp
+		ep = c.findEndpoint(ipp, addr)
+		c.noteRecvActivityFromEndpoint(ep)
+		return n, ep, wgRecvAddr(ep, ipp, addr), nil
+
+	case <-c.donec():
 		// Socket has been shut down. All the producers of packets
 		// respond to the context cancellation and go away, so we have
 		// to also unblock and return an error, to inform wireguard-go
@@ -1567,72 +1632,36 @@ func (c *Conn) receiveIPv4DERP(b []byte) (n int, ep conn.Endpoint, err error) {
 		// unblocks any concurrent Read()s. wireguard-go itself calls
 		// Clos() on magicsock, and expects ReceiveIPv4 to unblock
 		// with an error so it can clean up.
-		return 0, nil, errors.New("socket closed")
-	case dm = <-c.derpRecvCh:
-		// Below.
+		return 0, nil, nil, errors.New("socket closed")
 	}
+}
 
-	var regionID int
-	n, regionID = dm.n, dm.regionID
-	ncopy := dm.copyBuf(b)
-	if ncopy != n {
-		err = fmt.Errorf("received DERP packet of length %d that's too big for WireGuard ReceiveIPv4 buf size %d", n, ncopy)
-		c.logf("magicsock: %v", err)
-		return 0, nil, err
+func (c *Conn) ReceiveIPv6(b []byte) (int, conn.Endpoint, *net.UDPAddr, error) {
+	if c.pconn6 == nil {
+		return 0, nil, nil, syscall.EAFNOSUPPORT
 	}
-
-	ipp := netaddr.IPPort{IP: derpMagicIPAddr, Port: uint16(regionID)}
-	if c.handleDiscoMessage(b[:n], ipp) {
-		return 0, nil, errLoopAgain
-	}
-
-	var (
-		didNoteRecvActivity bool
-		discoEp             *discoEndpoint
-		asEp                *addrSet
-	)
-	c.mu.Lock()
-	if dk, ok := c.discoOfNode[tailcfg.NodeKey(dm.src)]; ok {
-		discoEp = c.endpointOfDisco[dk]
-		// If we know about the node (it's in discoOfNode) but don't know about the
-		// endpoint, that's because it's an idle peer that doesn't yet exist in the
-		// wireguard config. So run the receive hook, if defined, which should
-		// create the wireguard peer.
-		if discoEp == nil && c.noteRecvActivity != nil {
-			didNoteRecvActivity = true
-			c.mu.Unlock()          // release lock before calling noteRecvActivity
-			c.noteRecvActivity(dk) // (calls back into CreateEndpoint)
-			// Now require the lock. No invariants need to be rechecked; just
-			// 1-2 map lookups follow that are harmless if, say, the peer has
-			// been deleted during this time.
-			c.mu.Lock()
-
-			discoEp = c.endpointOfDisco[dk]
-			c.logf("magicsock: DERP packet received from idle peer %v; created=%v", dm.src.ShortString(), ep != nil)
+	for {
+		n, pAddr, err := c.pconn6.ReadFrom(b)
+		if err != nil {
+			return 0, nil, nil, err
 		}
-	}
-	if !c.disableLegacy {
-		asEp = c.addrsByKey[dm.src]
-	}
-	c.mu.Unlock()
-
-	if discoEp != nil {
-		ep = discoEp
-	} else if asEp != nil {
-		ep = asEp
-	} else {
-		key := wgkey.Key(dm.src)
-		c.logf("magicsock: DERP packet from unknown key: %s", key.ShortString())
-		ep = c.findEndpoint(ipp, nil, b[:n])
-		if ep == nil {
-			return 0, nil, errLoopAgain
+		addr := pAddr.(*net.UDPAddr)
+		ipp, ok := netaddr.FromStdAddr(addr.IP, addr.Port, addr.Zone)
+		if !ok {
+			continue
+		}
+		if stun.Is(b[:n]) {
+			c.stunReceiveFunc.Load().(func([]byte, netaddr.IPPort))(b[:n], ipp)
+			continue
+		}
+		if c.handleDiscoMessage(b[:n], ipp) {
+			continue
 		}
-	}
 
-	if !didNoteRecvActivity {
+		ep := c.findEndpoint(ipp, addr)
 		c.noteRecvActivityFromEndpoint(ep)
+		return n, ep, wgRecvAddr(ep, ipp, addr), nil
 	}
-	return n, ep, nil
 }
 
 // discoLogLevel controls the verbosity of discovery log messages.
@@ -2113,6 +2142,7 @@ func (c *Conn) SetNetworkMap(nm *controlclient.NetworkMap) {
 			delete(c.sharedDiscoKey, dk)
 		}
 	}
+
 }
 
 func (c *Conn) wantDerpLocked() bool { return c.derpMap != nil }
@@ -2218,10 +2248,11 @@ func (c *Conn) LastMark() uint32           { return 0 }
 // Only the first close does anything. Any later closes return nil.
 func (c *Conn) Close() error {
 	c.mu.Lock()
-	defer c.mu.Unlock()
 	if c.closed {
+		c.mu.Unlock()
 		return nil
 	}
+	defer c.mu.Unlock()
 
 	for _, ep := range c.endpointOfDisco {
 		ep.stopAndReset()
@@ -2245,13 +2276,6 @@ func (c *Conn) Close() error {
 	return err
 }
 
-// isClosed reports whether c is closed.
-func (c *Conn) isClosed() bool {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-	return c.closed
-}
-
 func (c *Conn) goroutinesRunningLocked() bool {
 	if c.endpointsUpdateActive {
 		return true
@@ -2324,7 +2348,7 @@ func (c *Conn) periodicReSTUN() {
 	var lastIdleState opt.Bool
 	for {
 		select {
-		case <-c.donec:
+		case <-c.donec():
 			return
 		case <-timer.C:
 			doReSTUN := c.shouldDoPeriodicReSTUN()
@@ -2349,7 +2373,7 @@ func (c *Conn) periodicDerpCleanup() {
 	defer ticker.Stop()
 	for {
 		select {
-		case <-c.donec:
+		case <-c.donec():
 			return
 		case <-ticker.C:
 			c.cleanStaleDerp()
@@ -2789,6 +2813,7 @@ type discoEndpoint struct {
 	discoKey           tailcfg.DiscoKey // for discovery mesages
 	discoShort         string           // ShortString of discoKey
 	fakeWGAddr         netaddr.IPPort   // the UDP address we tell wireguard-go we're using
+	fakeWGAddrStd      *net.UDPAddr     // the *net.UDPAddr form of fakeWGAddr
 	wgEndpointHostPort string           // string from CreateEndpoint: "<hex-discovery-key>.disco.tailscale:12345"
 
 	// Owned by Conn.mu:
@@ -2923,6 +2948,7 @@ func (de *discoEndpoint) initFakeUDPAddr() {
 		IP:   netaddr.IPFrom16(addr),
 		Port: 12345,
 	}
+	de.fakeWGAddrStd = de.fakeWGAddr.UDPAddr()
 }
 
 // isFirstRecvActivityInAwhile notes that receive activity has occured for this
@@ -2945,11 +2971,19 @@ func (de *discoEndpoint) String() string {
 	return fmt.Sprintf("magicsock.discoEndpoint{%v, %v}", de.publicKey.ShortString(), de.discoShort)
 }
 
-func (de *discoEndpoint) Addrs() string {
+func (de *discoEndpoint) Addrs() []wgcfg.Endpoint {
 	// This has to be the same string that was passed to
 	// CreateEndpoint, otherwise Reconfig will end up recreating
 	// Endpoints and losing state over time.
-	return de.wgEndpointHostPort
+	host, portStr, err := net.SplitHostPort(de.wgEndpointHostPort)
+	if err != nil {
+		panic(err)
+	}
+	port, err := strconv.ParseUint(portStr, 10, 16)
+	if err != nil {
+		panic(err)
+	}
+	return []wgcfg.Endpoint{{Host: host, Port: uint16(port)}}
 }
 
 func (de *discoEndpoint) ClearSrc()           {}
@@ -2958,6 +2992,11 @@ func (de *discoEndpoint) SrcIP() net.IP       { panic("unused") } // unused by w
 func (de *discoEndpoint) DstToString() string { return de.wgEndpointHostPort }
 func (de *discoEndpoint) DstIP() net.IP       { panic("unused") }
 func (de *discoEndpoint) DstToBytes() []byte  { return packIPPort(de.fakeWGAddr) }
+func (de *discoEndpoint) UpdateDst(addr *net.UDPAddr) error {
+	// This is called ~per packet (and requiring a mutex acquisition inside wireguard-go).
+	// TODO(bradfitz): make that cheaper and/or remove it. We don't need it.
+	return nil
+}
 
 // addrForSendLocked returns the address(es) that should be used for
 // sending the next packet. Zero, one, or both of UDP address and DERP
@@ -2982,10 +3021,6 @@ func (de *discoEndpoint) heartbeat() {
 
 	de.heartBeatTimer = nil
 
-	if de.c.isClosed() {
-		return
-	}
-
 	if de.lastSend.IsZero() {
 		// Shouldn't happen.
 		return

wgengine/magicsock/magicsock_test.go

@@ -11,6 +11,7 @@ import (
 	"crypto/tls"
 	"encoding/binary"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -20,7 +21,6 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"testing"
 	"time"
 	"unsafe"
@@ -132,7 +132,7 @@ type magicStack struct {
 // newMagicStack builds and initializes an idle magicsock and
 // friends. You need to call conn.SetNetworkMap and dev.Reconfig
 // before anything interesting happens.
-func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap, disableLegacy bool) *magicStack {
+func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap) *magicStack {
 	t.Helper()
 
 	privateKey, err := wgkey.NewPrivate()
@@ -147,8 +147,7 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
 		EndpointsFunc: func(eps []string) {
 			epCh <- eps
 		},
-		SimulatedNetwork:        l != nettype.Std{},
-		DisableLegacyNetworking: disableLegacy,
+		SimulatedNetwork: l != nettype.Std{},
 	})
 	if err != nil {
 		t.Fatalf("constructing magicsock: %v", err)
@@ -164,7 +163,11 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
 	tsTun.SetFilter(filter.NewAllowAllForTest(logf))
 
 	dev := device.NewDevice(tsTun, &device.DeviceOptions{
-		Logger:         wireguardGoLogger(logf),
+		Logger: &device.Logger{
+			Debug: logger.StdLogger(logf),
+			Info:  logger.StdLogger(logf),
+			Error: logger.StdLogger(logf),
+		},
 		CreateEndpoint: conn.CreateEndpoint,
 		CreateBind:     conn.CreateBind,
 		SkipBindUpdate: true,
@@ -337,10 +340,9 @@ func TestNewConn(t *testing.T) {
 
 	port := pickPort(t)
 	conn, err := NewConn(Options{
-		Port:                    port,
-		EndpointsFunc:           epFunc,
-		Logf:                    t.Logf,
-		DisableLegacyNetworking: true,
+		Port:          port,
+		EndpointsFunc: epFunc,
+		Logf:          t.Logf,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -353,7 +355,7 @@ func TestNewConn(t *testing.T) {
 	go func() {
 		var pkt [64 << 10]byte
 		for {
-			_, _, err := conn.ReceiveIPv4(pkt[:])
+			_, _, _, err := conn.ReceiveIPv4(pkt[:])
 			if err != nil {
 				return
 			}
@@ -481,9 +483,12 @@ func makeConfigs(t *testing.T, addrs []netaddr.IPPort) []wgcfg.Config {
 				continue
 			}
 			peer := wgcfg.Peer{
-				PublicKey:           privKeys[peerNum].Public(),
-				AllowedIPs:          addresses[peerNum],
-				Endpoints:           addr.String(),
+				PublicKey:  privKeys[peerNum].Public(),
+				AllowedIPs: addresses[peerNum],
+				Endpoints: []wgcfg.Endpoint{{
+					Host: addr.IP.String(),
+					Port: addr.Port,
+				}},
 				PersistentKeepalive: 25,
 			}
 			cfg.Peers = append(cfg.Peers, peer)
@@ -514,9 +519,8 @@ func TestDeviceStartStop(t *testing.T) {
 	defer rc.Assert(t)
 
 	conn, err := NewConn(Options{
-		EndpointsFunc:           func(eps []string) {},
-		Logf:                    t.Logf,
-		DisableLegacyNetworking: true,
+		EndpointsFunc: func(eps []string) {},
+		Logf:          t.Logf,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -526,7 +530,11 @@ func TestDeviceStartStop(t *testing.T) {
 
 	tun := tuntest.NewChannelTUN()
 	dev := device.NewDevice(tun.TUN(), &device.DeviceOptions{
-		Logger:         wireguardGoLogger(t.Logf),
+		Logger: &device.Logger{
+			Debug: logger.StdLogger(t.Logf),
+			Info:  logger.StdLogger(t.Logf),
+			Error: logger.StdLogger(t.Logf),
+		},
 		CreateEndpoint: conn.CreateEndpoint,
 		CreateBind:     conn.CreateBind,
 		SkipBindUpdate: true,
@@ -535,6 +543,93 @@ func TestDeviceStartStop(t *testing.T) {
 	dev.Close()
 }
 
+// A context used in TestConnClosing() which seeks to test that code which calls
+// Err() to see if a connection is already being closed does not then proceed to
+// try to acquire the mutex, as this would lead to deadlock. When Err() is called
+// this context acquires the lock itself, in order to force a deadlock (and test
+// failure on timeout).
+type testConnClosingContext struct {
+	parent context.Context
+	mu     *sync.Mutex
+}
+
+func (c *testConnClosingContext) Deadline() (deadline time.Time, ok bool) {
+	d, o := c.parent.Deadline()
+	return d, o
+}
+func (c *testConnClosingContext) Done() <-chan struct{} {
+	return c.parent.Done()
+}
+func (c *testConnClosingContext) Err() error {
+	// Deliberately deadlock if anything grabs the lock after checking Err()
+	c.mu.Lock()
+	return errors.New("testConnClosingContext error")
+}
+func (c *testConnClosingContext) Value(key interface{}) interface{} {
+	return c.parent.Value(key)
+}
+func (*testConnClosingContext) String() string {
+	return "testConnClosingContext"
+}
+
+func TestConnClosing(t *testing.T) {
+	privateKey, err := wgkey.NewPrivate()
+	if err != nil {
+		t.Fatalf("generating private key: %v", err)
+	}
+
+	epCh := make(chan []string, 100)
+	conn, err := NewConn(Options{
+		Logf:           t.Logf,
+		PacketListener: nettype.Std{},
+		EndpointsFunc: func(eps []string) {
+			epCh <- eps
+		},
+		SimulatedNetwork: false,
+	})
+	if err != nil {
+		t.Fatalf("constructing magicsock: %v", err)
+	}
+
+	derpMap, cleanup := runDERPAndStun(t, t.Logf, nettype.Std{}, netaddr.IPv4(127, 0, 3, 1))
+	defer cleanup()
+
+	// The point of this test case is to exercise handling in derpWriteChanOfAddr() which
+	// returns early if connCtx.Err() returns non-nil, to avoid a deadlock on conn.mu.
+	// We swap in a context which always returns an error, and deliberately grabs the lock
+	// to cause a deadlock if magicsock.go tries to acquire the lock after calling Err().
+	closingCtx := testConnClosingContext{parent: conn.connCtx, mu: &conn.mu}
+	conn.connCtx = &closingCtx
+	conn.Start()
+
+	conn.SetDERPMap(derpMap)
+	if err := conn.SetPrivateKey(privateKey); err != nil {
+		t.Fatalf("setting private key in magicsock: %v", err)
+	}
+
+	tun := tuntest.NewChannelTUN()
+	tsTun := tstun.WrapTUN(t.Logf, tun.TUN())
+	tsTun.SetFilter(filter.NewAllowAllForTest(t.Logf))
+
+	dev := device.NewDevice(tsTun, &device.DeviceOptions{
+		Logger: &device.Logger{
+			Debug: logger.StdLogger(t.Logf),
+			Info:  logger.StdLogger(t.Logf),
+			Error: logger.StdLogger(t.Logf),
+		},
+		CreateEndpoint: conn.CreateEndpoint,
+		CreateBind:     conn.CreateBind,
+		SkipBindUpdate: true,
+	})
+
+	dev.Up()
+	conn.WaitReady(t)
+
+	// We don't assert any failures within the test itself. If derpWriteChanOfAddr tries to
+	// grab the lock it will deadlock, and conn.WaitReady(t) will call t.Fatal() after timeout.
+	// (verified by deliberately breaking derpWriteChanOfAddr)
+}
+
 // Exercise a code path in sendDiscoMessage if the connection has been closed.
 func TestConnClosed(t *testing.T) {
 	mstun := &natlab.Machine{Name: "stun"}
@@ -554,15 +649,12 @@ func TestConnClosed(t *testing.T) {
 		stunIP: sif.V4(),
 	}
 
-	logf, closeLogf := logger.LogfCloser(t.Logf)
-	defer closeLogf()
-
-	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
+	derpMap, cleanup := runDERPAndStun(t, t.Logf, d.stun, d.stunIP)
 	defer cleanup()
 
-	ms1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap, true)
+	ms1 := newMagicStack(t, logger.WithPrefix(t.Logf, "conn1: "), d.m1, derpMap)
 	defer ms1.Close()
-	ms2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap, true)
+	ms2 := newMagicStack(t, logger.WithPrefix(t.Logf, "conn2: "), d.m2, derpMap)
 	defer ms2.Close()
 
 	cleanup = meshStacks(t.Logf, []*magicStack{ms1, ms2})
@@ -836,20 +928,18 @@ func testActiveDiscovery(t *testing.T, d *devices) {
 	setT(t)
 
 	start := time.Now()
-	wlogf := func(msg string, args ...interface{}) {
+	logf := func(msg string, args ...interface{}) {
 		t.Helper()
 		msg = fmt.Sprintf("%s: %s", time.Since(start).Truncate(time.Microsecond), msg)
 		tlogf(msg, args...)
 	}
-	logf, closeLogf := logger.LogfCloser(wlogf)
-	defer closeLogf()
 
 	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
 	defer cleanup()
 
-	m1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap, true)
+	m1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap)
 	defer m1.Close()
-	m2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap, true)
+	m2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap)
 	defer m2.Close()
 
 	cleanup = meshStacks(logf, []*magicStack{m1, m2})
@@ -901,9 +991,9 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
 	defer cleanup()
 
-	m1 := newMagicStack(t, logf, d.m1, derpMap, false)
+	m1 := newMagicStack(t, logf, d.m1, derpMap)
 	defer m1.Close()
-	m2 := newMagicStack(t, logf, d.m2, derpMap, false)
+	m2 := newMagicStack(t, logf, d.m2, derpMap)
 	defer m2.Close()
 
 	addrs := []netaddr.IPPort{
@@ -1050,12 +1140,12 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	})
 
 	// Add DERP relay.
-	derpEp := "127.3.3.40:1"
+	derpEp := wgcfg.Endpoint{Host: "127.3.3.40", Port: 1}
 	ep0 := cfgs[0].Peers[0].Endpoints
-	ep0 = derpEp + "," + ep0
+	ep0 = append([]wgcfg.Endpoint{derpEp}, ep0...)
 	cfgs[0].Peers[0].Endpoints = ep0
 	ep1 := cfgs[1].Peers[0].Endpoints
-	ep1 = derpEp + "," + ep1
+	ep1 = append([]wgcfg.Endpoint{derpEp}, ep1...)
 	cfgs[1].Peers[0].Endpoints = ep1
 	if err := m1.dev.Reconfig(&cfgs[0]); err != nil {
 		t.Fatal(err)
@@ -1071,8 +1161,8 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	})
 
 	// Disable real route.
-	cfgs[0].Peers[0].Endpoints = derpEp
-	cfgs[1].Peers[0].Endpoints = derpEp
+	cfgs[0].Peers[0].Endpoints = []wgcfg.Endpoint{derpEp}
+	cfgs[1].Peers[0].Endpoints = []wgcfg.Endpoint{derpEp}
 	if err := m1.dev.Reconfig(&cfgs[0]); err != nil {
 		t.Fatal(err)
 	}
@@ -1126,7 +1216,7 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 	})
 }
 
-// TestAddrSet tests addrSet appendDests and updateDst.
+// TestAddrSet tests addrSet appendDests and UpdateDst.
 func TestAddrSet(t *testing.T) {
 	tstest.PanicOnLog()
 	rc := tstest.NewResourceCheck()
@@ -1288,7 +1378,7 @@ func TestAddrSet(t *testing.T) {
 				faket = faket.Add(st.advance)
 
 				if st.updateDst != nil {
-					if err := tt.as.updateDst(st.updateDst); err != nil {
+					if err := tt.as.UpdateDst(st.updateDst); err != nil {
 						t.Fatal(err)
 					}
 					continue
@@ -1382,7 +1472,7 @@ func stringifyConfig(cfg wgcfg.Config) string {
 	return string(j)
 }
 
-func Test32bitAlignment(t *testing.T) {
+func TestDiscoEndpointAlignment(t *testing.T) {
 	var de discoEndpoint
 	off := unsafe.Offsetof(de.lastRecvUnixAtomic)
 	if off%8 != 0 {
@@ -1394,8 +1484,6 @@ func Test32bitAlignment(t *testing.T) {
 	if de.isFirstRecvActivityInAwhile() {
 		t.Error("expected false on second call")
 	}
-	var c Conn
-	atomic.AddInt64(&c.derpRecvCountAtomic, 1)
 }
 
 func BenchmarkReceiveFrom(b *testing.B) {
@@ -1406,7 +1494,6 @@ func BenchmarkReceiveFrom(b *testing.B) {
 		EndpointsFunc: func(eps []string) {
 			b.Logf("endpoints: %q", eps)
 		},
-		DisableLegacyNetworking: true,
 	})
 	if err != nil {
 		b.Fatal(err)
@@ -1419,21 +1506,6 @@ func BenchmarkReceiveFrom(b *testing.B) {
 	}
 	defer sendConn.Close()
 
-	// Give conn just enough state that it'll recognize sendConn as a
-	// valid peer and not fall through to the legacy magicsock
-	// codepath.
-	discoKey := tailcfg.DiscoKey{31: 1}
-	conn.SetNetworkMap(&controlclient.NetworkMap{
-		Peers: []*tailcfg.Node{
-			{
-				DiscoKey:  discoKey,
-				Endpoints: []string{sendConn.LocalAddr().String()},
-			},
-		},
-	})
-	conn.CreateEndpoint([32]byte{1: 1}, "0000000000000000000000000000000000000000000000000000000000000001.disco.tailscale:12345")
-	conn.addValidDiscoPathForTest(discoKey, netaddr.MustParseIPPort(sendConn.LocalAddr().String()))
-
 	var dstAddr net.Addr = conn.pconn4.LocalAddr()
 	sendBuf := make([]byte, 1<<10)
 	for i := range sendBuf {
@@ -1445,12 +1517,13 @@ func BenchmarkReceiveFrom(b *testing.B) {
 		if _, err := sendConn.WriteTo(sendBuf, dstAddr); err != nil {
 			b.Fatalf("WriteTo: %v", err)
 		}
-		n, ep, err := conn.ReceiveIPv4(buf)
+		n, ep, addr, err := conn.ReceiveIPv4(buf)
 		if err != nil {
 			b.Fatal(err)
 		}
 		_ = n
 		_ = ep
+		_ = addr
 	}
 }
 
@@ -1484,19 +1557,3 @@ func BenchmarkReceiveFrom_Native(b *testing.B) {
 		}
 	}
 }
-
-func wireguardGoLogger(logf logger.Logf) *device.Logger {
-	// wireguard-go logs as it starts and stops routines.
-	// Silence those; there are a lot of them, and they're just noise.
-	allowLogf := func(s string) bool {
-		return !strings.Contains(s, "Routine:")
-	}
-	filtered := logger.Filtered(logf, allowLogf)
-	stdLogger := logger.StdLogger(filtered)
-
-	return &device.Logger{
-		Debug: stdLogger,
-		Info:  stdLogger,
-		Error: stdLogger,
-	}
-}

wgengine/netstack/netstack.go

@@ -28,7 +28,6 @@ import (
 	"gvisor.dev/gvisor/pkg/tcpip/transport/udp"
 	"gvisor.dev/gvisor/pkg/waiter"
 	"inet.af/netaddr"
-	"tailscale.com/control/controlclient"
 	"tailscale.com/net/packet"
 	"tailscale.com/types/logger"
 	"tailscale.com/wgengine"
@@ -63,46 +62,7 @@ func Impl(logf logger.Logf, tundev *tstun.TUN, e wgengine.Engine, mc *magicsock.
 		log.Fatal(err)
 	}
 
-	e.AddNetworkMapCallback(func(nm *controlclient.NetworkMap) {
-		oldIPs := make(map[tcpip.Address]bool)
-		for _, ip := range ipstack.AllAddresses()[nicID] {
-			oldIPs[ip.AddressWithPrefix.Address] = true
-		}
-		newIPs := make(map[tcpip.Address]bool)
-		for _, ip := range nm.Addresses {
-			newIPs[tcpip.Address(ip.IPNet().IP)] = true
-		}
-
-		ipsToBeAdded := make(map[tcpip.Address]bool)
-		for ip := range newIPs {
-			if !oldIPs[ip] {
-				ipsToBeAdded[ip] = true
-			}
-		}
-		ipsToBeRemoved := make(map[tcpip.Address]bool)
-		for ip := range oldIPs {
-			if !newIPs[ip] {
-				ipsToBeRemoved[ip] = true
-			}
-		}
-
-		for ip := range ipsToBeRemoved {
-			err := ipstack.RemoveAddress(nicID, ip)
-			if err != nil {
-				logf("netstack: could not deregister IP %s: %v", ip, err)
-			} else {
-				logf("netstack: deregistered IP %s", ip)
-			}
-		}
-		for ip := range ipsToBeAdded {
-			err := ipstack.AddAddress(nicID, ipv4.ProtocolNumber, ip)
-			if err != nil {
-				logf("netstack: could not register IP %s: %v", ip, err)
-			} else {
-				logf("netstack: registered IP %s", ip)
-			}
-		}
-	})
+	ipstack.AddAddress(nicID, ipv4.ProtocolNumber, tcpip.Address(net.ParseIP("100.96.188.101").To4()))
 
 	// Add 0.0.0.0/0 default route.
 	subnet, _ := tcpip.NewSubnet(tcpip.Address(strings.Repeat("\x00", 4)), tcpip.AddressMask(strings.Repeat("\x00", 4)))

wgengine/pendopen.go

@@ -32,47 +32,35 @@ type pendingOpenFlow struct {
 	timer *time.Timer // until giving up on the flow
 }
 
-func (e *userspaceEngine) removeFlow(f flowtrack.Tuple) (removed bool) {
-	e.mu.Lock()
-	defer e.mu.Unlock()
-	of, ok := e.pendOpen[f]
-	if !ok {
-		// Not a tracked flow (likely already removed)
-		return false
-	}
-	of.timer.Stop()
-	delete(e.pendOpen, f)
-	return true
-}
-
 func (e *userspaceEngine) trackOpenPreFilterIn(pp *packet.Parsed, t *tstun.TUN) (res filter.Response) {
 	res = filter.Accept // always
 
-	if pp.IPProto == packet.TSMP {
-		res = filter.DropSilently
-		rh, ok := pp.AsTailscaleRejectedHeader()
-		if !ok {
-			return
-		}
-		if f := rh.Flow(); e.removeFlow(f) {
-			e.logf("open-conn-track: flow %v %v > %v rejected due to %v", rh.Proto, rh.Src, rh.Dst, rh.Reason)
-		}
-		return
-	}
-
 	if pp.IPVersion == 0 ||
 		pp.IPProto != packet.TCP ||
 		pp.TCPFlags&(packet.TCPSyn|packet.TCPRst) == 0 {
 		return
 	}
 
-	// Either a SYN or a RST came back. Remove it in either case.
+	flow := flowtrack.Tuple{Dst: pp.Src, Src: pp.Dst} // src/dst reversed
 
-	f := flowtrack.Tuple{Dst: pp.Src, Src: pp.Dst} // src/dst reversed
-	removed := e.removeFlow(f)
-	if removed && pp.TCPFlags&packet.TCPRst != 0 {
-		e.logf("open-conn-track: flow TCP %v got RST by peer", f)
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	of, ok := e.pendOpen[flow]
+	if !ok {
+		// Not a tracked flow.
+		return
 	}
+	of.timer.Stop()
+	delete(e.pendOpen, flow)
+
+	if pp.TCPFlags&packet.TCPRst != 0 {
+		// TODO(bradfitz): have peer send a IP proto 99 "why"
+		// packet first with details and log that instead
+		// (e.g. ACL prohibited, shields up, etc).
+		e.logf("open-conn-track: flow %v got RST by peer", flow)
+		return
+	}
+
 	return
 }
 

wgengine/router/dns/manager_windows.go

@@ -18,6 +18,7 @@ import (
 const (
 	ipv4RegBase = `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters`
 	ipv6RegBase = `SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters`
+	tsRegBase   = `SOFTWARE\Tailscale IPN`
 )
 
 type windowsManager struct {
@@ -90,6 +91,11 @@ func (m windowsManager) Up(config Config) error {
 		return err
 	}
 
+	newSearchList := strings.Join(config.Domains, ",")
+	if err := setRegistryString(tsRegBase, "SearchList", newSearchList); err != nil {
+		return err
+	}
+
 	// Force DNS re-registration in Active Directory. What we actually
 	// care about is that this command invokes the undocumented hidden
 	// function that forces Windows to notice that adapter settings

wgengine/tsdns/tsdns.go

@@ -201,11 +201,12 @@ func (r *Resolver) Resolve(domain string, tp dns.Type) (netaddr.IP, dns.RCode, e
 			break
 		}
 	}
+	if !anyHasSuffix {
+		return netaddr.IP{}, dns.RCodeRefused, nil
+	}
+
 	addr, found := dnsMap.nameToIP[domain]
 	if !found {
-		if !anyHasSuffix {
-			return netaddr.IP{}, dns.RCodeRefused, nil
-		}
 		return netaddr.IP{}, dns.RCodeNameError, nil
 	}
 

wgengine/tstun/tun.go

@@ -218,8 +218,8 @@ func (t *TUN) poll() {
 func (t *TUN) filterOut(p *packet.Parsed) filter.Response {
 
 	if t.PreFilterOut != nil {
-		if res := t.PreFilterOut(p, t); res.IsDrop() {
-			return res
+		if t.PreFilterOut(p, t) == filter.Drop {
+			return filter.Drop
 		}
 	}
 
@@ -234,8 +234,8 @@ func (t *TUN) filterOut(p *packet.Parsed) filter.Response {
 	}
 
 	if t.PostFilterOut != nil {
-		if res := t.PostFilterOut(p, t); res.IsDrop() {
-			return res
+		if t.PostFilterOut(p, t) == filter.Drop {
+			return filter.Drop
 		}
 	}
 
@@ -264,12 +264,12 @@ func (t *TUN) Read(buf []byte, offset int) (int, error) {
 		return 0, io.EOF
 	case err := <-t.errors:
 		return 0, err
-	case pkt := <-t.outbound:
-		n = copy(buf[offset:], pkt)
+	case packet := <-t.outbound:
+		n = copy(buf[offset:], packet)
 		// t.buffer has a fixed location in memory,
 		// so this is the easiest way to tell when it has been consumed.
-		// &pkt[0] can be used because empty packets do not reach t.outbound.
-		if &pkt[0] == &t.buffer[PacketStartOffset] {
+		// &packet[0] can be used because empty packets do not reach t.outbound.
+		if &packet[0] == &t.buffer[PacketStartOffset] {
 			t.bufferConsumed <- struct{}{}
 		} else {
 			// If the packet is not from t.buffer, then it is an injected packet.
@@ -307,8 +307,8 @@ func (t *TUN) filterIn(buf []byte) filter.Response {
 	p.Decode(buf)
 
 	if t.PreFilterIn != nil {
-		if res := t.PreFilterIn(p, t); res.IsDrop() {
-			return res
+		if t.PreFilterIn(p, t) == filter.Drop {
+			return filter.Drop
 		}
 	}
 
@@ -319,29 +319,6 @@ func (t *TUN) filterIn(buf []byte) filter.Response {
 	}
 
 	if filt.RunIn(p, t.filterFlags) != filter.Accept {
-
-		// Tell them, via TSMP, we're dropping them due to the ACL.
-		// Their host networking stack can translate this into ICMP
-		// or whatnot as required. But notably, their GUI or tailscale CLI
-		// can show them a rejection history with reasons.
-		if p.IPVersion == 4 && p.IPProto == packet.TCP && p.TCPFlags&packet.TCPSyn != 0 {
-			rj := packet.TailscaleRejectedHeader{
-				IPSrc:  p.Dst.IP,
-				IPDst:  p.Src.IP,
-				Src:    p.Src,
-				Dst:    p.Dst,
-				Proto:  p.IPProto,
-				Reason: packet.RejectedDueToACLs,
-			}
-			if filt.ShieldsUp() {
-				rj.Reason = packet.RejectedDueToShieldsUp
-			}
-			pkt := packet.Generate(rj, nil)
-			t.InjectOutbound(pkt)
-
-			// TODO(bradfitz): also send a TCP RST, after the TSMP message.
-		}
-
 		return filter.Drop
 	}
 
@@ -354,15 +331,10 @@ func (t *TUN) filterIn(buf []byte) filter.Response {
 	return filter.Accept
 }
 
-// Write accepts an incoming packet. The packet begins at buf[offset:],
-// like wireguard-go/tun.Device.Write.
 func (t *TUN) Write(buf []byte, offset int) (int, error) {
 	if !t.disableFilter {
-		res := t.filterIn(buf[offset:])
-		if res == filter.DropSilently {
-			return len(buf), nil
-		}
-		if res != filter.Accept {
+		response := t.filterIn(buf[offset:])
+		if response != filter.Accept {
 			return 0, ErrFiltered
 		}
 	}

wgengine/tstun/tun_test.go

@@ -341,22 +341,6 @@ func TestAllocs(t *testing.T) {
 	}
 }
 
-func TestClose(t *testing.T) {
-	ftun, tun := newFakeTUN(t.Logf, false)
-
-	data := udp4("1.2.3.4", "5.6.7.8", 98, 98)
-	_, err := ftun.Write(data, 0)
-	if err != nil {
-		t.Error(err)
-	}
-
-	tun.Close()
-	_, err = ftun.Write(data, 0)
-	if err == nil {
-		t.Error("Expected error from ftun.Write() after Close()")
-	}
-}
-
 func BenchmarkWrite(b *testing.B) {
 	ftun, tun := newFakeTUN(b.Logf, true)
 	defer tun.Close()

wgengine/userspace.go

@@ -111,16 +111,15 @@ type userspaceEngine struct {
 	sentActivityAt      map[netaddr.IP]*int64     // value is atomic int64 of unixtime
 	destIPActivityFuncs map[netaddr.IP]func()
 
-	mu                  sync.Mutex // guards following; see lock order comment below
-	closing             bool       // Close was called (even if we're still closing)
-	statusCallback      StatusCallback
-	linkChangeCallback  func(major bool, newState *interfaces.State)
-	peerSequence        []wgkey.Key
-	endpoints           []string
-	pingers             map[wgkey.Key]*pinger // legacy pingers for pre-discovery peers
-	linkState           *interfaces.State
-	pendOpen            map[flowtrack.Tuple]*pendingOpenFlow // see pendopen.go
-	networkMapCallbacks map[*someHandle]NetworkMapCallback
+	mu                 sync.Mutex // guards following; see lock order comment below
+	closing            bool       // Close was called (even if we're still closing)
+	statusCallback     StatusCallback
+	linkChangeCallback func(major bool, newState *interfaces.State)
+	peerSequence       []wgkey.Key
+	endpoints          []string
+	pingers            map[wgkey.Key]*pinger // legacy pingers for pre-discovery peers
+	linkState          *interfaces.State
+	pendOpen           map[flowtrack.Tuple]*pendingOpenFlow // see pendopen.go
 
 	// Lock ordering: magicsock.Conn.mu, wgLock, then mu.
 }
@@ -282,7 +281,7 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) {
 	// wireguard-go logs as it starts and stops routines.
 	// Silence those; there are a lot of them, and they're just noise.
 	allowLogf := func(s string) bool {
-		return !strings.Contains(s, "Routine:")
+		return !strings.HasPrefix(s, "Routine:")
 	}
 	filtered := logger.Filtered(logf, allowLogf)
 	// flags==0 because logf is already nested in another logger.
@@ -296,7 +295,7 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) {
 
 	opts := &device.DeviceOptions{
 		Logger: &logger,
-		HandshakeDone: func(peerKey device.NoisePublicKey, peer *device.Peer, deviceAllowedIPs *device.AllowedIPs) {
+		HandshakeDone: func(peerKey wgcfg.Key, peer *device.Peer, deviceAllowedIPs *device.AllowedIPs) {
 			// Send an unsolicited status event every time a
 			// handshake completes. This makes sure our UI can
 			// update quickly as soon as it connects to a peer.
@@ -307,14 +306,13 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) {
 			// here.
 			go e.RequestStatus()
 
-			peerWGKey := wgkey.Key(peerKey)
 			if e.magicConn.PeerHasDiscoKey(tailcfg.NodeKey(peerKey)) {
-				e.logf("wireguard handshake complete for %v", peerWGKey.ShortString())
+				e.logf("wireguard handshake complete for %v", peerKey.ShortString())
 				// This is a modern peer with discovery support. No need to send pings.
 				return
 			}
 
-			e.logf("wireguard handshake complete for %v; sending legacy pings", peerWGKey.ShortString())
+			e.logf("wireguard handshake complete for %v; sending legacy pings", peerKey.ShortString())
 
 			// Ping every single-IP that peer routes.
 			// These synthetic packets are used to traverse NATs.
@@ -330,9 +328,9 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) {
 				}
 			}
 			if len(ips) > 0 {
-				go e.pinger(peerWGKey, ips)
+				go e.pinger(wgkey.Key(peerKey), ips)
 			} else {
-				logf("[unexpected] peer %s has no single-IP routes: %v", peerWGKey.ShortString(), allowedIPs)
+				logf("[unexpected] peer %s has no single-IP routes: %v", peerKey.ShortString(), allowedIPs)
 			}
 		},
 		CreateBind:     e.magicConn.CreateBind,
@@ -675,15 +673,10 @@ func isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool {
 	if forceFullWireguardConfig(numPeers) {
 		return false
 	}
-	if !isSingleEndpoint(p.Endpoints) {
+	if len(p.Endpoints) != 1 {
 		return false
 	}
-
-	host, _, err := net.SplitHostPort(p.Endpoints)
-	if err != nil {
-		return false
-	}
-	if !strings.HasSuffix(host, ".disco.tailscale") {
+	if !strings.HasSuffix(p.Endpoints[0].Host, ".disco.tailscale") {
 		return false
 	}
 
@@ -747,14 +740,11 @@ func (e *userspaceEngine) isActiveSince(dk tailcfg.DiscoKey, ip netaddr.IP, t ti
 // Host of form "<64-hex-digits>.disco.tailscale". If invariant is violated,
 // we return the zero value.
 func discoKeyFromPeer(p *wgcfg.Peer) tailcfg.DiscoKey {
-	if len(p.Endpoints) < 64 {
+	host := p.Endpoints[0].Host
+	if len(host) < 64 {
 		return tailcfg.DiscoKey{}
 	}
-	host, rest := p.Endpoints[:64], p.Endpoints[64:]
-	if !strings.HasPrefix(rest, ".disco.tailscale") {
-		return tailcfg.DiscoKey{}
-	}
-	k, err := key.NewPublicFromHexMem(mem.S(host))
+	k, err := key.NewPublicFromHexMem(mem.S(host[:64]))
 	if err != nil {
 		return tailcfg.DiscoKey{}
 	}
@@ -807,14 +797,11 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Publ
 			}
 			continue
 		}
+		tsIP := p.AllowedIPs[0].IP
 		dk := discoKeyFromPeer(p)
 		trackDisco = append(trackDisco, dk)
-		recentlyActive := false
-		for _, cidr := range p.AllowedIPs {
-			trackIPs = append(trackIPs, cidr.IP)
-			recentlyActive = recentlyActive || e.isActiveSince(dk, cidr.IP, activeCutoff)
-		}
-		if recentlyActive {
+		trackIPs = append(trackIPs, tsIP)
+		if e.isActiveSince(dk, tsIP, activeCutoff) {
 			min.Peers = append(min.Peers, *p)
 			if discoChanged[key.Public(p.PublicKey)] {
 				needRemoveStep = true
@@ -958,21 +945,21 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	// and a second time with it.
 	discoChanged := make(map[key.Public]bool)
 	{
-		prevEP := make(map[key.Public]string)
+		prevEP := make(map[key.Public]wgcfg.Endpoint)
 		for i := range e.lastCfgFull.Peers {
-			if p := &e.lastCfgFull.Peers[i]; isSingleEndpoint(p.Endpoints) {
-				prevEP[key.Public(p.PublicKey)] = p.Endpoints
+			if p := &e.lastCfgFull.Peers[i]; len(p.Endpoints) == 1 {
+				prevEP[key.Public(p.PublicKey)] = p.Endpoints[0]
 			}
 		}
 		for i := range cfg.Peers {
 			p := &cfg.Peers[i]
-			if !isSingleEndpoint(p.Endpoints) {
+			if len(p.Endpoints) != 1 {
 				continue
 			}
 			pub := key.Public(p.PublicKey)
-			if old, ok := prevEP[pub]; ok && old != p.Endpoints {
+			if old, ok := prevEP[pub]; ok && old != p.Endpoints[0] {
 				discoChanged[pub] = true
-				e.logf("wgengine: Reconfig: %s changed from %q to %q", pub.ShortString(), old, p.Endpoints)
+				e.logf("wgengine: Reconfig: %s changed from %s to %s", pub.ShortString(), &old, &p.Endpoints[0])
 			}
 		}
 	}
@@ -1017,11 +1004,6 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	return nil
 }
 
-// isSingleEndpoint reports whether endpoints contains exactly one host:port pair.
-func isSingleEndpoint(s string) bool {
-	return s != "" && !strings.Contains(s, ",")
-}
-
 func (e *userspaceEngine) GetFilter() *filter.Filter {
 	return e.tundev.GetFilter()
 }
@@ -1294,21 +1276,6 @@ func (e *userspaceEngine) SetLinkChangeCallback(cb func(major bool, newState *in
 	}
 }
 
-func (e *userspaceEngine) AddNetworkMapCallback(cb NetworkMapCallback) func() {
-	e.mu.Lock()
-	defer e.mu.Unlock()
-	if e.networkMapCallbacks == nil {
-		e.networkMapCallbacks = make(map[*someHandle]NetworkMapCallback)
-	}
-	h := new(someHandle)
-	e.networkMapCallbacks[h] = cb
-	return func() {
-		e.mu.Lock()
-		defer e.mu.Unlock()
-		delete(e.networkMapCallbacks, h)
-	}
-}
-
 func getLinkState() (*interfaces.State, error) {
 	s, err := interfaces.GetState()
 	if s != nil {
@@ -1327,15 +1294,6 @@ func (e *userspaceEngine) SetDERPMap(dm *tailcfg.DERPMap) {
 
 func (e *userspaceEngine) SetNetworkMap(nm *controlclient.NetworkMap) {
 	e.magicConn.SetNetworkMap(nm)
-	e.mu.Lock()
-	callbacks := make([]NetworkMapCallback, 0, 4)
-	for _, fn := range e.networkMapCallbacks {
-		callbacks = append(callbacks, fn)
-	}
-	e.mu.Unlock()
-	for _, fn := range callbacks {
-		fn(nm)
-	}
 }
 
 func (e *userspaceEngine) DiscoPublicKey() tailcfg.DiscoKey {

wgengine/userspace_test.go

@@ -103,7 +103,12 @@ func TestUserspaceEngineReconfig(t *testing.T) {
 					AllowedIPs: []netaddr.IPPrefix{
 						{IP: netaddr.IPv4(100, 100, 99, 1), Bits: 32},
 					},
-					Endpoints: discoHex + ".disco.tailscale:12345",
+					Endpoints: []wgcfg.Endpoint{
+						{
+							Host: discoHex + ".disco.tailscale",
+							Port: 12345,
+						},
+					},
 				},
 			},
 		}

wgengine/watchdog.go

@@ -110,11 +110,6 @@ func (e *watchdogEngine) SetDERPMap(m *tailcfg.DERPMap) {
 func (e *watchdogEngine) SetNetworkMap(nm *controlclient.NetworkMap) {
 	e.watchdog("SetNetworkMap", func() { e.wrap.SetNetworkMap(nm) })
 }
-func (e *watchdogEngine) AddNetworkMapCallback(callback NetworkMapCallback) func() {
-	var fn func()
-	e.watchdog("AddNetworkMapCallback", func() { fn = e.wrap.AddNetworkMapCallback(callback) })
-	return func() { e.watchdog("RemoveNetworkMapCallback", fn) }
-}
 func (e *watchdogEngine) DiscoPublicKey() (k tailcfg.DiscoKey) {
 	e.watchdog("DiscoPublicKey", func() { k = e.wrap.DiscoPublicKey() })
 	return k

wgengine/wgengine.go

@@ -36,7 +36,7 @@ type PeerStatus struct {
 // TODO(bradfitz): remove this, subset of ipnstate? Need to migrate users.
 type Status struct {
 	Peers      []PeerStatus
-	LocalAddrs []string // the set of possible endpoints for the magic conn
+	LocalAddrs []string // TODO(crawshaw): []wgcfg.Endpoint?
 	DERPs      int      // number of active DERP connections
 }
 
@@ -49,15 +49,6 @@ type StatusCallback func(*Status, error)
 // NetInfoCallback is the type used by Engine.SetNetInfoCallback.
 type NetInfoCallback func(*tailcfg.NetInfo)
 
-// NetworkMapCallback is the type used by callbacks that hook
-// into network map updates.
-type NetworkMapCallback func(*controlclient.NetworkMap)
-
-// someHandle is allocated so its pointer address acts as a unique
-// map key handle. (It needs to have non-zero size for Go to guarantee
-// the pointer is unique.)
-type someHandle struct{ _ byte }
-
 // ErrNoChanges is returned by Engine.Reconfig if no changes were made.
 var ErrNoChanges = errors.New("no changes made to Engine config")
 
@@ -123,12 +114,6 @@ type Engine interface {
 	// The network map should only be read from.
 	SetNetworkMap(*controlclient.NetworkMap)
 
-	// AddNetworkMapCallback adds a function to a list of callbacks
-	// that are called when the network map updates. It returns a
-	// function that when called would remove the function from the
-	// list of callbacks.
-	AddNetworkMapCallback(NetworkMapCallback) (removeCallback func())
-
 	// SetNetInfoCallback sets the function to call when a
 	// new NetInfo summary is available.
 	SetNetInfoCallback(NetInfoCallback)