cmd/tailscale: make the new 'up' errors prettier and more helpful

Fixes #1746

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

cmd/tailscale/cli/cli_test.go

@@ -40,6 +40,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 		name     string
 		flagSet  map[string]bool
 		curPrefs *ipn.Prefs
+		curUser  string // os.Getenv("USER") on the client side
 		mp       *ipn.MaskedPrefs
 		want     string
 	}{
@@ -78,7 +79,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 				WantRunningSet: true,
 				CorpDNSSet:     true,
 			},
-			want: `'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; --hostname is not specified but its default value of "" differs from current value "foo"`,
+			want: accidentalUpPrefix + " --accept-dns --hostname=foo",
 		},
 		{
 			name:    "hostname_changing_explicitly",
@@ -148,14 +149,213 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			},
 			want: "",
 		},
+		{
+			name:    "implicit_operator_change",
+			flagSet: f("hostname"),
+			curPrefs: &ipn.Prefs{
+				ControlURL:   ipn.DefaultControlURL,
+				OperatorUser: "alice",
+			},
+			curUser: "eve",
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+				},
+				ControlURLSet: true,
+			},
+			want: accidentalUpPrefix + " --hostname= --operator=alice",
+		},
+		{
+			name:    "implicit_operator_matches_shell_user",
+			flagSet: f("hostname"),
+			curPrefs: &ipn.Prefs{
+				ControlURL:   ipn.DefaultControlURL,
+				OperatorUser: "alice",
+			},
+			curUser: "alice",
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+				},
+				ControlURLSet: true,
+			},
+			want: "",
+		},
+		{
+			name:    "error_advertised_routes_exit_node_removed",
+			flagSet: f("advertise-routes"),
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("10.0.42.0/24"),
+					netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					netaddr.MustParseIPPrefix("::/0"),
+				},
+			},
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					AdvertiseRoutes: []netaddr.IPPrefix{
+						netaddr.MustParseIPPrefix("10.0.42.0/24"),
+					},
+				},
+				AdvertiseRoutesSet: true,
+			},
+			want: accidentalUpPrefix + " --advertise-routes=10.0.42.0/24 --advertise-exit-node",
+		},
+		{
+			name:    "advertised_routes_exit_node_removed",
+			flagSet: f("advertise-routes", "advertise-exit-node"),
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("10.0.42.0/24"),
+					netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					netaddr.MustParseIPPrefix("::/0"),
+				},
+			},
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					AdvertiseRoutes: []netaddr.IPPrefix{
+						netaddr.MustParseIPPrefix("10.0.42.0/24"),
+					},
+				},
+				AdvertiseRoutesSet: true,
+			},
+			want: "",
+		},
+		{
+			name:    "advertised_routes_includes_the_0_routes", // but no --advertise-exit-node
+			flagSet: f("advertise-routes"),
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("10.0.42.0/24"),
+					netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					netaddr.MustParseIPPrefix("::/0"),
+				},
+			},
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					AdvertiseRoutes: []netaddr.IPPrefix{
+						netaddr.MustParseIPPrefix("11.1.43.0/24"),
+						netaddr.MustParseIPPrefix("0.0.0.0/0"),
+						netaddr.MustParseIPPrefix("::/0"),
+					},
+				},
+				AdvertiseRoutesSet: true,
+			},
+			want: "",
+		},
+		{
+			name:    "advertised_routes_includes_only_one_0_route", // and no --advertise-exit-node
+			flagSet: f("advertise-routes"),
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("10.0.42.0/24"),
+					netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					netaddr.MustParseIPPrefix("::/0"),
+				},
+			},
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					AdvertiseRoutes: []netaddr.IPPrefix{
+						netaddr.MustParseIPPrefix("11.1.43.0/24"),
+						netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					},
+				},
+				AdvertiseRoutesSet: true,
+			},
+			want: accidentalUpPrefix + " --advertise-routes=11.1.43.0/24,0.0.0.0/0 --advertise-exit-node",
+		},
+		{
+			name:    "exit_node_clearing", // Issue 1777
+			flagSet: f("exit-node"),
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				ExitNodeID: "fooID",
+			},
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					ExitNodeIP: netaddr.IP{},
+				},
+				ExitNodeIPSet: true,
+			},
+			want: "",
+		},
+		{
+			name:    "remove_all_implicit",
+			flagSet: f("force-reauth"),
+			curPrefs: &ipn.Prefs{
+				WantRunning:      true,
+				ControlURL:       ipn.DefaultControlURL,
+				RouteAll:         true,
+				AllowSingleHosts: false,
+				ExitNodeIP:       netaddr.MustParseIP("100.64.5.6"),
+				CorpDNS:          true,
+				ShieldsUp:        true,
+				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
+				Hostname:         "myhostname",
+				ForceDaemon:      true,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("10.0.0.0/16"),
+				},
+				NetfilterMode: preftype.NetfilterNoDivert,
+				OperatorUser:  "alice",
+			},
+			curUser: "eve",
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL:  ipn.DefaultControlURL,
+					WantRunning: true,
+				},
+			},
+			want: accidentalUpPrefix + " --accept-routes --exit-node=100.64.5.6 --accept-dns --shields-up --advertise-tags=tag:foo,tag:bar --hostname=myhostname --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
+		},
+		{
+			name:    "remove_all_implicit_except_hostname",
+			flagSet: f("hostname"),
+			curPrefs: &ipn.Prefs{
+				WantRunning:      true,
+				ControlURL:       ipn.DefaultControlURL,
+				RouteAll:         true,
+				AllowSingleHosts: false,
+				ExitNodeIP:       netaddr.MustParseIP("100.64.5.6"),
+				CorpDNS:          true,
+				ShieldsUp:        true,
+				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
+				Hostname:         "myhostname",
+				ForceDaemon:      true,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("10.0.0.0/16"),
+				},
+				NetfilterMode: preftype.NetfilterNoDivert,
+				OperatorUser:  "alice",
+			},
+			curUser: "eve",
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL:  ipn.DefaultControlURL,
+					WantRunning: true,
+					Hostname:    "newhostname",
+				},
+				HostnameSet: true,
+			},
+			want: accidentalUpPrefix + " --hostname=newhostname --accept-routes --exit-node=100.64.5.6 --accept-dns --shields-up --advertise-tags=tag:foo,tag:bar --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			var got string
-			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp); err != nil {
+			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp, tt.curUser); err != nil {
 				got = err.Error()
 			}
-			if got != tt.want {
+			if strings.TrimSpace(got) != tt.want {
 				t.Errorf("unexpected result\n got: %s\nwant: %s\n", got, tt.want)
 			}
 		})

cmd/tailscale/cli/push.go

@@ -16,6 +16,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"path/filepath"
 	"strconv"
 	"time"
 	"unicode/utf8"
@@ -61,12 +62,13 @@ func runPush(ctx context.Context, args []string) error {
 		return err
 	}
 
-	peerAPIBase, lastSeen, err := discoverPeerAPIBase(ctx, ip)
+	peerAPIBase, lastSeen, isOffline, err := discoverPeerAPIBase(ctx, ip)
 	if err != nil {
 		return err
 	}
-
-	if !lastSeen.IsZero() && time.Since(lastSeen) > lastSeenOld {
+	if isOffline {
+		fmt.Fprintf(os.Stderr, "# warning: %s is offline\n", hostOrIP)
+	} else if !lastSeen.IsZero() && time.Since(lastSeen) > lastSeenOld {
 		fmt.Fprintf(os.Stderr, "# warning: %s last seen %v ago\n", hostOrIP, time.Since(lastSeen).Round(time.Minute))
 	}
 
@@ -97,7 +99,7 @@ func runPush(ctx context.Context, args []string) error {
 		contentLength = fi.Size()
 		fileContents = io.LimitReader(f, contentLength)
 		if name == "" {
-			name = fileArg
+			name = filepath.Base(fileArg)
 		}
 
 		if slow, _ := strconv.ParseBool(os.Getenv("TS_DEBUG_SLOW_PUSH")); slow {
@@ -126,14 +128,14 @@ func runPush(ctx context.Context, args []string) error {
 	return errors.New(res.Status)
 }
 
-func discoverPeerAPIBase(ctx context.Context, ipStr string) (base string, lastSeen time.Time, err error) {
+func discoverPeerAPIBase(ctx context.Context, ipStr string) (base string, lastSeen time.Time, isOffline bool, err error) {
 	ip, err := netaddr.ParseIP(ipStr)
 	if err != nil {
-		return "", time.Time{}, err
+		return "", time.Time{}, false, err
 	}
 	fts, err := tailscale.FileTargets(ctx)
 	if err != nil {
-		return "", time.Time{}, err
+		return "", time.Time{}, false, err
 	}
 	for _, ft := range fts {
 		n := ft.Node
@@ -144,10 +146,11 @@ func discoverPeerAPIBase(ctx context.Context, ipStr string) (base string, lastSe
 			if n.LastSeen != nil {
 				lastSeen = *n.LastSeen
 			}
-			return ft.PeerAPIURL, lastSeen, nil
+			isOffline = n.Online != nil && !*n.Online
+			return ft.PeerAPIURL, lastSeen, isOffline, nil
 		}
 	}
-	return "", time.Time{}, errors.New("target seems to be running an old Tailscale version")
+	return "", time.Time{}, false, errors.New("target seems to be running an old Tailscale version")
 }
 
 const maxSniff = 4 << 20
@@ -204,15 +207,22 @@ func runPushTargets(ctx context.Context, args []string) error {
 	}
 	for _, ft := range fts {
 		n := ft.Node
-		var ago string
-		if n.LastSeen == nil {
-			ago = "\tnode never seen"
-		} else {
-			if d := time.Since(*n.LastSeen); d > lastSeenOld {
-				ago = fmt.Sprintf("\tlast seen %v ago", d.Round(time.Minute))
+		var detail string
+		if n.Online != nil {
+			if !*n.Online {
+				detail = "offline"
 			}
+		} else {
+			detail = "unknown-status"
 		}
-		fmt.Printf("%s\t%s%s\n", n.Addresses[0].IP, n.ComputedName, ago)
+		if detail != "" && n.LastSeen != nil {
+			d := time.Since(*n.LastSeen)
+			detail += fmt.Sprintf("; last seen %v ago", d.Round(time.Minute))
+		}
+		if detail != "" {
+			detail = "\t" + detail
+		}
+		fmt.Printf("%s\t%s%s\n", n.Addresses[0].IP, n.ComputedName, detail)
 	}
 	return nil
 }

cmd/tailscale/cli/up.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/go-multierror/multierror"
+	shellquote "github.com/kballard/go-shellquote"
 	"github.com/peterbourgon/ff/v2/ffcli"
 	"inet.af/netaddr"
 	"tailscale.com/client/tailscale"
@@ -245,6 +245,23 @@ func runUp(ctx context.Context, args []string) error {
 	if err != nil {
 		fatalf("can't fetch status from tailscaled: %v", err)
 	}
+	origAuthURL := st.AuthURL
+
+	// printAuthURL reports whether we should print out the
+	// provided auth URL from an IPN notify.
+	printAuthURL := func(url string) bool {
+		if upArgs.authKey != "" {
+			// Issue 1755: when using an authkey, don't
+			// show an authURL that might still be pending
+			// from a previous non-completed interactive
+			// login.
+			return false
+		}
+		if upArgs.forceReauth && url == origAuthURL {
+			return false
+		}
+		return true
+	}
 
 	if distro.Get() == distro.Synology {
 		notSupported := "not yet supported on Synology; see https://github.com/tailscale/tailscale/issues/451"
@@ -285,7 +302,7 @@ func runUp(ctx context.Context, args []string) error {
 	})
 
 	if !upArgs.reset {
-		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp); err != nil {
+		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp, os.Getenv("USER")); err != nil {
 			fatalf("%s", err)
 		}
 	}
@@ -312,7 +329,10 @@ func runUp(ctx context.Context, args []string) error {
 	// simpleUp is whether we're running a simple "tailscale up"
 	// to transition to running from a previously-logged-in but
 	// down state, without changing any settings.
-	simpleUp := len(flagSet) == 0 && curPrefs.Persist != nil && curPrefs.Persist.LoginName != ""
+	simpleUp := len(flagSet) == 0 &&
+		curPrefs.Persist != nil &&
+		curPrefs.Persist.LoginName != "" &&
+		st.BackendState != ipn.NeedsLogin.String()
 
 	// At this point we need to subscribe to the IPN bus to watch
 	// for state transitions and possible need to authenticate.
@@ -367,7 +387,7 @@ func runUp(ctx context.Context, args []string) error {
 				cancel()
 			}
 		}
-		if url := n.BrowseToURL; url != nil {
+		if url := n.BrowseToURL; url != nil && printAuthURL(*url) {
 			printed = true
 			fmt.Fprintf(os.Stderr, "\nTo authenticate, visit:\n\n\t%s\n\n", *url)
 		}
@@ -418,7 +438,9 @@ func runUp(ctx context.Context, args []string) error {
 		}
 
 		bc.Start(opts)
-		startLoginInteractive()
+		if upArgs.forceReauth {
+			startLoginInteractive()
+		}
 	}
 
 	select {
@@ -450,7 +472,7 @@ func init() {
 	addPrefFlagMapping("netfilter-mode", "NetfilterMode")
 	addPrefFlagMapping("shields-up", "ShieldsUp")
 	addPrefFlagMapping("snat-subnet-routes", "NoSNAT")
-	addPrefFlagMapping("exit-node", "ExitNodeIP", "ExitNodeIP")
+	addPrefFlagMapping("exit-node", "ExitNodeIP", "ExitNodeID")
 	addPrefFlagMapping("exit-node-allow-lan-access", "ExitNodeAllowLANAccess")
 	addPrefFlagMapping("unattended", "ForceDaemon")
 	addPrefFlagMapping("operator", "OperatorUser")
@@ -486,6 +508,12 @@ func updateMaskedPrefsFromUpFlag(mp *ipn.MaskedPrefs, flagName string) {
 	}
 }
 
+const accidentalUpPrefix = "Error: changing settings via 'tailscale up' requires mentioning all\n" +
+	"non-default flags. To proceed, either re-run your command with --reset or\n" +
+	"specify use the command below to explicitly mention the current value of\n" +
+	"all non-default settings:\n\n" +
+	"\ttailscale up"
+
 // checkForAccidentalSettingReverts checks for people running
 // "tailscale up" with a subset of the flags they originally ran it
 // with.
@@ -500,7 +528,7 @@ func updateMaskedPrefsFromUpFlag(mp *ipn.MaskedPrefs, flagName string) {
 //
 // mp is the mask of settings actually set, where mp.Prefs is the new
 // preferences to set, including any values set from implicit flags.
-func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Prefs, mp *ipn.MaskedPrefs) error {
+func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Prefs, mp *ipn.MaskedPrefs, curUser string) error {
 	if len(flagSet) == 0 {
 		// A bare "tailscale up" is a special case to just
 		// mean bringing the network up without any changes.
@@ -519,15 +547,29 @@ func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Pre
 	ev := reflect.ValueOf(curWithExplicitEdits).Elem()
 	// Implicit values (what we'd get if we replaced everything with flag defaults):
 	iv := reflect.ValueOf(&mp.Prefs).Elem()
-	var errs []error
-	var didExitNodeErr bool
+
+	var missing []string
+	flagExplicitValue := map[string]interface{}{} // e.g. "accept-dns" => true (from flagSet)
 	for i := 0; i < prefType.NumField(); i++ {
 		prefName := prefType.Field(i).Name
 		if prefName == "Persist" {
 			continue
 		}
 		flagName, hasFlag := flagForPref[prefName]
+
+		// Special case for advertise-exit-node; which is a
+		// flag but doesn't have a corresponding pref.  The
+		// flag augments advertise-routes, so we have to infer
+		// the imaginary pref's current value from the routes.
+		if prefName == "AdvertiseRoutes" &&
+			hasExitNodeRoutes(curPrefs.AdvertiseRoutes) &&
+			!hasExitNodeRoutes(curWithExplicitEdits.AdvertiseRoutes) &&
+			!flagSet["advertise-exit-node"] {
+			missing = append(missing, "--advertise-exit-node")
+		}
+
 		if hasFlag && flagSet[flagName] {
+			flagExplicitValue[flagName] = ev.Field(i).Interface()
 			continue
 		}
 		// Get explicit value and implicit value
@@ -540,33 +582,93 @@ func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Pre
 			}
 		}
 		exi, imi := ex.Interface(), im.Interface()
+
 		if reflect.DeepEqual(exi, imi) {
 			continue
 		}
+		if flagName == "operator" && imi == "" && exi == curUser {
+			// Don't require setting operator if the current user matches
+			// the configured operator.
+			continue
+		}
 		switch flagName {
 		case "":
-			errs = append(errs, fmt.Errorf("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; this command would change the value of flagless pref %q", prefName))
+			return fmt.Errorf("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; this command would change the value of flagless pref %q", prefName)
 		case "exit-node":
-			if !didExitNodeErr {
-				didExitNodeErr = true
-				errs = append(errs, errors.New("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; --exit-node is not specified but an exit node is currently configured"))
+			if prefName == "ExitNodeIP" {
+				missing = append(missing, fmtFlagValueArg("exit-node", fmtSettingVal(exi)))
 			}
 		default:
-			errs = append(errs, fmt.Errorf("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; --%s is not specified but its default value of %v differs from current value %v",
-				flagName, fmtSettingVal(imi), fmtSettingVal(exi)))
+			missing = append(missing, fmtFlagValueArg(flagName, fmtSettingVal(exi)))
 		}
 	}
-	return multierror.New(errs)
+	if len(missing) == 0 {
+		return nil
+	}
+	var sb strings.Builder
+	sb.WriteString(accidentalUpPrefix)
+
+	var flagSetSorted []string
+	for f := range flagSet {
+		flagSetSorted = append(flagSetSorted, f)
+	}
+	sort.Strings(flagSetSorted)
+	for _, flagName := range flagSetSorted {
+		if ev, ok := flagExplicitValue[flagName]; ok {
+			fmt.Fprintf(&sb, " %s", fmtFlagValueArg(flagName, fmtSettingVal(ev)))
+		}
+	}
+	for _, a := range missing {
+		fmt.Fprintf(&sb, " %s", a)
+	}
+	sb.WriteString("\n\n")
+	return errors.New(sb.String())
+}
+
+func fmtFlagValueArg(flagName, val string) string {
+	if val == "true" {
+		// TODO: check flagName's type to see if its Pref is of type bool
+		return "--" + flagName
+	}
+	if val == "" {
+		return "--" + flagName + "="
+	}
+	return fmt.Sprintf("--%s=%v", flagName, shellquote.Join(val))
 }
 
 func fmtSettingVal(v interface{}) string {
 	switch v := v.(type) {
 	case bool:
 		return strconv.FormatBool(v)
-	case string, preftype.NetfilterMode:
-		return fmt.Sprintf("%q", v)
+	case string:
+		return v
+	case preftype.NetfilterMode:
+		return v.String()
 	case []string:
 		return strings.Join(v, ",")
+	case []netaddr.IPPrefix:
+		var sb strings.Builder
+		for i, r := range v {
+			if i > 0 {
+				sb.WriteByte(',')
+			}
+			sb.WriteString(r.String())
+		}
+		return sb.String()
 	}
 	return fmt.Sprint(v)
 }
+
+func hasExitNodeRoutes(rr []netaddr.IPPrefix) bool {
+	var v4, v6 bool
+	for _, r := range rr {
+		if r.Bits == 0 {
+			if r.IP.Is4() {
+				v4 = true
+			} else if r.IP.Is6() {
+				v6 = true
+			}
+		}
+	}
+	return v4 && v6
+}

cmd/tailscale/depaware.txt

@@ -2,7 +2,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
 
    W 💣 github.com/alexbrainman/sspi                                 from github.com/alexbrainman/sspi/negotiate
    W 💣 github.com/alexbrainman/sspi/negotiate                       from tailscale.com/net/tshttpproxy
-        github.com/go-multierror/multierror                          from tailscale.com/cmd/tailscale/cli
+        github.com/kballard/go-shellquote                            from tailscale.com/cmd/tailscale/cli
         github.com/peterbourgon/ff/v2                                from github.com/peterbourgon/ff/v2/ffcli
         github.com/peterbourgon/ff/v2/ffcli                          from tailscale.com/cmd/tailscale/cli
         github.com/tcnksm/go-httpstat                                from tailscale.com/net/netcheck

cmd/tailscaled/depaware.txt

@@ -131,9 +131,11 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/types/strbuilder                               from tailscale.com/net/packet
         tailscale.com/types/structs                                  from tailscale.com/control/controlclient+
         tailscale.com/types/wgkey                                    from tailscale.com/control/controlclient+
+   L    tailscale.com/util/cmpver                                    from tailscale.com/net/dns
         tailscale.com/util/dnsname                                   from tailscale.com/ipn/ipnstate+
   LW    tailscale.com/util/endian                                    from tailscale.com/net/netns+
    L    tailscale.com/util/lineread                                  from tailscale.com/control/controlclient+
+        tailscale.com/util/osshare                                   from tailscale.com/cmd/tailscaled+
         tailscale.com/util/pidowner                                  from tailscale.com/ipn/ipnserver
         tailscale.com/util/racebuild                                 from tailscale.com/logpolicy
         tailscale.com/util/systemd                                   from tailscale.com/control/controlclient+

cmd/tailscaled/install_windows.go

@@ -16,6 +16,7 @@ import (
 	"golang.org/x/sys/windows/svc/mgr"
 	"tailscale.com/logtail/backoff"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/osshare"
 )
 
 func init() {
@@ -79,6 +80,9 @@ func installSystemDaemonWindows(args []string) (err error) {
 }
 
 func uninstallSystemDaemonWindows(args []string) (ret error) {
+	// Remove file sharing from Windows shell (noop in non-windows)
+	osshare.SetFileSharingEnabled(false, logger.Discard)
+
 	m, err := mgr.Connect()
 	if err != nil {
 		return fmt.Errorf("failed to connect to Windows service manager: %v", err)

cmd/tailscaled/tailscaled.go

@@ -40,6 +40,7 @@ import (
 	"tailscale.com/types/flagtype"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
+	"tailscale.com/util/osshare"
 	"tailscale.com/version"
 	"tailscale.com/version/distro"
 	"tailscale.com/wgengine"
@@ -160,7 +161,12 @@ func main() {
 		log.Fatalf("--socket is required")
 	}
 
-	if err := run(); err != nil {
+	err := run()
+
+	// Remove file sharing from Windows shell (noop in non-windows)
+	osshare.SetFileSharingEnabled(false, logger.Discard)
+
+	if err != nil {
 		// No need to log; the func already did
 		os.Exit(1)
 	}

control/controlclient/auto.go

@@ -597,6 +597,9 @@ func (c *Client) mapRoutine() {
 }
 
 func (c *Client) AuthCantContinue() bool {
+	if c == nil {
+		return true
+	}
 	c.mu.Lock()
 	defer c.mu.Unlock()
 

control/controlclient/direct.go

@@ -413,7 +413,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 
 		// Don't log the common error types. Signatures are not usually enabled,
 		// so these are expected.
-		if err != errCertificateNotConfigured && err != errNoCertStore {
+		if !errors.Is(err, errCertificateNotConfigured) && !errors.Is(err, errNoCertStore) {
 			c.logf("RegisterReq sign error: %v", err)
 		}
 	}
@@ -570,6 +570,11 @@ func (c *Direct) SendLiteMapUpdate(ctx context.Context) error {
 	return c.sendMapRequest(ctx, 1, nil)
 }
 
+// If we go more than pollTimeout without hearing from the server,
+// end the long poll. We should be receiving a keep alive ping
+// every minute.
+const pollTimeout = 120 * time.Second
+
 // cb nil means to omit peers.
 func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netmap.NetworkMap)) error {
 	c.mu.Lock()
@@ -694,10 +699,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 		return nil
 	}
 
-	// If we go more than pollTimeout without hearing from the server,
-	// end the long poll. We should be receiving a keep alive ping
-	// every minute.
-	const pollTimeout = 120 * time.Second
 	timeout := time.NewTimer(pollTimeout)
 	timeoutReset := make(chan struct{})
 	pollDone := make(chan struct{})
@@ -795,6 +796,11 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 			}
 			setControlAtomic(&controlUseDERPRoute, resp.Debug.DERPRoute)
 			setControlAtomic(&controlTrimWGConfig, resp.Debug.TrimWGConfig)
+			if sleep := time.Duration(resp.Debug.SleepSeconds * float64(time.Second)); sleep > 0 {
+				if err := sleepAsRequested(ctx, c.logf, timeoutReset, sleep); err != nil {
+					return err
+				}
+			}
 		}
 
 		nm := sess.netmapForResponse(&resp)
@@ -1181,3 +1187,34 @@ func answerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest) {
 		logf("answerPing complete to %v (after %v)", pr.URL, d)
 	}
 }
+
+func sleepAsRequested(ctx context.Context, logf logger.Logf, timeoutReset chan<- struct{}, d time.Duration) error {
+	const maxSleep = 5 * time.Minute
+	if d > maxSleep {
+		logf("sleeping for %v, capped from server-requested %v ...", maxSleep, d)
+		d = maxSleep
+	} else {
+		logf("sleeping for server-requested %v ...", d)
+	}
+
+	ticker := time.NewTicker(pollTimeout / 2)
+	defer ticker.Stop()
+	timer := time.NewTimer(d)
+	defer timer.Stop()
+	for {
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-timer.C:
+			return nil
+		case <-ticker.C:
+			select {
+			case timeoutReset <- struct{}{}:
+			case <-timer.C:
+				return nil
+			case <-ctx.Done():
+				return ctx.Err()
+			}
+		}
+	}
+}

derp/derpmap/derpmap.go

@@ -83,6 +83,9 @@ func Prod() *tailcfg.DERPMap {
 			10: derpRegion(10, "sea", "Seattle",
 				derpNode("a", "137.220.36.168", "2001:19f0:8001:2d9:5400:2ff:feef:bbb1"),
 			),
+			11: derpRegion(11, "sao", "São Paulo",
+				derpNode("a", "18.230.97.74", "2600:1f1e:ee4:5611:ec5c:1736:d43b:a454"),
+			),
 		},
 	}
 }

go.mod

@@ -15,6 +15,7 @@ require (
 	github.com/google/go-cmp v0.5.4
 	github.com/goreleaser/nfpm v1.1.10
 	github.com/jsimonetti/rtnetlink v0.0.0-20210212075122-66c871082f2b
+	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
 	github.com/klauspost/compress v1.10.10
 	github.com/kr/pty v1.1.8
 	github.com/mdlayher/netlink v1.3.2

go.sum

@@ -61,6 +61,8 @@ github.com/jsimonetti/rtnetlink v0.0.0-20201220180245-69540ac93943/go.mod h1:z4c
 github.com/jsimonetti/rtnetlink v0.0.0-20210122163228-8d122574c736/go.mod h1:ZXpIyOK59ZnN7J0BV99cZUPmsqDRZ3eq5X+st7u/oSA=
 github.com/jsimonetti/rtnetlink v0.0.0-20210212075122-66c871082f2b h1:c3NTyLNozICy8B4mlMXemD3z/gXgQzVXZS/HqT+i3do=
 github.com/jsimonetti/rtnetlink v0.0.0-20210212075122-66c871082f2b/go.mod h1:8w9Rh8m+aHZIG69YPGGem1i5VzoyRC8nw2kA8B+ik5U=
+github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
+github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/klauspost/compress v1.10.10 h1:a/y8CglcM7gLGYmlbP/stPE5sR3hbhFRUjCBfd/0B3I=
 github.com/klauspost/compress v1.10.10/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=

health/health.go

@@ -36,6 +36,10 @@ var (
 	ipnState                string
 	ipnWantRunning          bool
 	anyInterfaceUp          = true // until told otherwise
+
+	ReceiveIPv4 = ReceiveFuncState{name: "IPv4"}
+	ReceiveIPv6 = ReceiveFuncState{name: "IPv6"}
+	ReceiveDERP = ReceiveFuncState{name: "DERP"}
 )
 
 // Subsystem is the name of a subsystem whose health can be monitored.
@@ -213,6 +217,68 @@ func SetAnyInterfaceUp(up bool) {
 	selfCheckLocked()
 }
 
+// ReceiveFuncState tracks the state of a wireguard-go conn.ReceiveFunc.
+type ReceiveFuncState struct {
+	// name is a mnemonic for the receive func, used in error messages.
+	name string
+	// started indicates whether magicsock.connBind.Open
+	// has requested that wireguard-go start its receive func
+	// goroutine (without a corresponding connBind.Close).
+	started bool
+	// running models whether wireguard-go's receive func
+	// goroutine is actually running. We cannot easily introspect that,
+	// so it is based on our knowledge of wireguard-go's internals.
+	running bool
+}
+
+// err returns the error state (if any) that s represents.
+func (s ReceiveFuncState) err() error {
+	// Possible states:
+	//   | started | running | notes
+	//   | ------- | ------- | -----
+	//   | true    | true    | normal operation
+	//   | true    | false   | we prematurely returned a permanent error from this receive func
+	//   | false   | true    | we have told package health that we're closing the bind, but the receive funcs haven't closed yet (transient)
+	//   | false   | false   | not running
+
+	// The problematic case is started && !running.
+	// If that happens, wireguard-go will no longer request packets,
+	// and we'll lose an entire communication channel.
+	if s.started && !s.running {
+		return fmt.Errorf("receive%s started but not running", s.name)
+	}
+	return nil
+}
+
+// Open tells r that connBind.Open has requested wireguard-go open a conn.Bind that includes r.
+func (r *ReceiveFuncState) Open() {
+	mu.Lock()
+	defer mu.Unlock()
+	r.started = true
+	r.running = true
+	selfCheckLocked()
+}
+
+// Stop tells r that we have returned a permanent error to wireguard-go.
+// wireguard-go's receive func goroutine for r will soon stop.
+func (r *ReceiveFuncState) Stop() {
+	mu.Lock()
+	defer mu.Unlock()
+	r.running = false
+	selfCheckLocked()
+}
+
+// Close tells r that connBind.Close has requested wireguard-go close the bind for r.
+// This will stop the corresponding receive func goroutine.
+// Close must be called before actually closing the underlying connection,
+// to avoid a small window of false positives.
+func (r *ReceiveFuncState) Close() {
+	mu.Lock()
+	defer mu.Unlock()
+	r.started = false
+	selfCheckLocked()
+}
+
 func timerSelfCheck() {
 	mu.Lock()
 	defer mu.Unlock()
@@ -263,6 +329,11 @@ func overallErrorLocked() error {
 	_ = lastMapRequestHeard
 
 	var errs []error
+	for _, recv := range []ReceiveFuncState{ReceiveIPv4, ReceiveIPv6, ReceiveDERP} {
+		if err := recv.err(); err != nil {
+			errs = append(errs, err)
+		}
+	}
 	for sys, err := range sysErr {
 		if err == nil || sys == SysOverall {
 			continue

ipn/backend.go

@@ -55,17 +55,21 @@ type EngineStatus struct {
 // that they have not changed.
 // They are JSON-encoded on the wire, despite the lack of struct tags.
 type Notify struct {
-	_             structs.Incomparable
-	Version       string             // version number of IPN backend
-	ErrMessage    *string            // critical error message, if any; for InUseOtherUser, the details
-	LoginFinished *empty.Message     // event: non-nil when login process succeeded
-	State         *State             // current IPN state has changed
-	Prefs         *Prefs             // preferences were changed
-	NetMap        *netmap.NetworkMap // new netmap received
-	Engine        *EngineStatus      // wireguard engine stats
-	BrowseToURL   *string            // UI should open a browser right now
-	BackendLogID  *string            // public logtail id used by backend
-	PingResult    *ipnstate.PingResult
+	_       structs.Incomparable
+	Version string // version number of IPN backend
+
+	// ErrMessage, if non-nil, contains a critical error message.
+	// For State InUseOtherUser, ErrMessage is not critical and just contains the details.
+	ErrMessage *string
+
+	LoginFinished *empty.Message       // non-nil when/if the login process succeeded
+	State         *State               // if non-nil, the new or current IPN state
+	Prefs         *Prefs               // if non-nil, the new or current preferences
+	NetMap        *netmap.NetworkMap   // if non-nil, the new or current netmap
+	Engine        *EngineStatus        // if non-nil, the new or urrent wireguard stats
+	BrowseToURL   *string              // if non-nil, UI should open a browser right now
+	BackendLogID  *string              // if non-nil, the public logtail ID used by backend
+	PingResult    *ipnstate.PingResult // if non-nil, a ping response arrived
 
 	// FilesWaiting if non-nil means that files are buffered in
 	// the Tailscale daemon and ready for local transfer to the

ipn/ipnlocal/local.go

@@ -46,6 +46,7 @@ import (
 	"tailscale.com/types/persist"
 	"tailscale.com/types/wgkey"
 	"tailscale.com/util/dnsname"
+	"tailscale.com/util/osshare"
 	"tailscale.com/util/systemd"
 	"tailscale.com/version"
 	"tailscale.com/wgengine"
@@ -105,6 +106,7 @@ type LocalBackend struct {
 	inServerMode   bool
 	machinePrivKey wgkey.Private
 	state          ipn.State
+	capFileSharing bool // whether netMap contains the file sharing capability
 	// hostinfo is mutated in-place while mu is held.
 	hostinfo *tailcfg.Hostinfo
 	// netMap is not mutated in-place once set.
@@ -114,7 +116,8 @@ type LocalBackend struct {
 	engineStatus     ipn.EngineStatus
 	endpoints        []tailcfg.Endpoint
 	blocked          bool
-	authURL          string
+	authURL          string // cleared on Notify
+	authURLSticky    string // not cleared on Notify
 	interact         bool
 	prevIfState      *interfaces.State
 	peerAPIServer    *peerAPIServer // or nil
@@ -144,6 +147,8 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, e wge
 		panic("ipn.NewLocalBackend: wgengine must not be nil")
 	}
 
+	osshare.SetFileSharingEnabled(false, logf)
+
 	// Default filter blocks everything and logs nothing, until Start() is called.
 	e.SetFilter(filter.NewAllowNone(logf, &netaddr.IPSet{}))
 
@@ -310,7 +315,7 @@ func (b *LocalBackend) updateStatus(sb *ipnstate.StatusBuilder, extraLocked func
 	sb.MutateStatus(func(s *ipnstate.Status) {
 		s.Version = version.Long
 		s.BackendState = b.state.String()
-		s.AuthURL = b.authURL
+		s.AuthURL = b.authURLSticky
 		if b.netMap != nil {
 			s.MagicDNSSuffix = b.netMap.MagicDNSSuffix()
 		}
@@ -460,6 +465,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 	}
 	if st.URL != "" {
 		b.authURL = st.URL
+		b.authURLSticky = st.URL
 	}
 	if b.state == ipn.NeedsLogin {
 		if !b.prefs.WantRunning {
@@ -588,6 +594,24 @@ func (b *LocalBackend) SetHTTPTestClient(c *http.Client) {
 	b.httpTestClient = c
 }
 
+// startIsNoopLocked reports whether a Start call on this LocalBackend
+// with the provided Start Options would be a useless no-op.
+//
+// b.mu must be held.
+func (b *LocalBackend) startIsNoopLocked(opts ipn.Options) bool {
+	// Options has 4 fields; check all of them:
+	//   * FrontendLogID
+	//   * StateKey
+	//   * Prefs
+	//   * AuthKey
+	return b.state == ipn.Running &&
+		b.hostinfo != nil &&
+		b.hostinfo.FrontendLogID == opts.FrontendLogID &&
+		b.stateKey == opts.StateKey &&
+		opts.Prefs == nil &&
+		opts.AuthKey == ""
+}
+
 // Start applies the configuration specified in opts, and starts the
 // state machine.
 //
@@ -603,18 +627,37 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 		return errors.New("no state key or prefs provided")
 	}
 
+	defer b.stateMachine()
 	if opts.Prefs != nil {
 		b.logf("Start: %v", opts.Prefs.Pretty())
 	} else {
 		b.logf("Start")
 	}
 
+	b.mu.Lock()
+
+	// The iOS client sends a "Start" whenever its UI screen comes
+	// up, just because it wants a netmap. That should be fixed,
+	// but meanwhile we can make Start cheaper here for such a
+	// case and not restart the world (which takes a few seconds).
+	// Instead, just send a notify with the state that iOS needs.
+	if b.startIsNoopLocked(opts) {
+		b.logf("Start: already running; sending notify")
+		nm := b.netMap
+		state := b.state
+		b.mu.Unlock()
+		b.send(ipn.Notify{
+			State:         &state,
+			NetMap:        nm,
+			LoginFinished: new(empty.Message),
+		})
+		return nil
+	}
+
 	hostinfo := controlclient.NewHostinfo()
 	hostinfo.BackendLogID = b.backendLogID
 	hostinfo.FrontendLogID = opts.FrontendLogID
 
-	b.mu.Lock()
-
 	if b.cc != nil {
 		// TODO(apenwarr): avoid the need to reinit controlclient.
 		// This will trigger a full relogin/reconfigure cycle every
@@ -1030,7 +1073,7 @@ func (b *LocalBackend) popBrowserAuthNow() {
 	b.mu.Lock()
 	url := b.authURL
 	b.interact = false
-	b.authURL = ""
+	b.authURL = "" // but NOT clearing authURLSticky
 	b.mu.Unlock()
 
 	b.logf("popBrowserAuthNow: url=%v", url != "")
@@ -1647,15 +1690,21 @@ func (b *LocalBackend) authReconfig() {
 		switch {
 		case len(dcfg.DefaultResolvers) != 0:
 			// Default resolvers already set.
-		case len(dcfg.Routes) == 0 && len(dcfg.Hosts) == 0 && len(dcfg.AuthoritativeSuffixes) == 0:
-			// No settings requiring split DNS, no problem.
-		case (version.OS() == "iOS" || version.OS() == "macOS") && !uc.ExitNodeID.IsZero():
-			// On Apple OSes, if your NetworkExtension provides a
-			// default route, underlying primary resolvers are
-			// automatically removed, so we MUST provide a set of
-			// resolvers capable of resolving the entire world.
+		case !uc.ExitNodeID.IsZero():
+			// When using exit nodes, it's very likely the LAN
+			// resolvers will become unreachable. So, force use of the
+			// fallback resolvers until we implement DNS forwarding to
+			// exit nodes.
+			//
+			// This is especially important on Apple OSes, where
+			// adding the default route to the tunnel interface makes
+			// it "primary", and we MUST provide VPN-sourced DNS
+			// settings or we break all DNS resolution.
+			//
 			// https://github.com/tailscale/tailscale/issues/1713
 			addDefault(nm.DNS.FallbackResolvers)
+		case len(dcfg.Routes) == 0 && len(dcfg.Hosts) == 0 && len(dcfg.AuthoritativeSuffixes) == 0:
+			// No settings requiring split DNS, no problem.
 		case version.OS() == "android":
 			// We don't support split DNS at all on Android yet.
 			addDefault(nm.DNS.FallbackResolvers)
@@ -1716,6 +1765,20 @@ func (b *LocalBackend) fileRootLocked(uid tailcfg.UserID) string {
 	return dir
 }
 
+// closePeerAPIListenersLocked closes any existing peer API listeners
+// and clears out the peer API server state.
+//
+// It does not kick off any Hostinfo update with new services.
+//
+// b.mu must be held.
+func (b *LocalBackend) closePeerAPIListenersLocked() {
+	b.peerAPIServer = nil
+	for _, pln := range b.peerAPIListeners {
+		pln.Close()
+	}
+	b.peerAPIListeners = nil
+}
+
 func (b *LocalBackend) initPeerAPIListener() {
 	b.mu.Lock()
 	defer b.mu.Unlock()
@@ -1734,11 +1797,7 @@ func (b *LocalBackend) initPeerAPIListener() {
 		}
 	}
 
-	b.peerAPIServer = nil
-	for _, pln := range b.peerAPIListeners {
-		pln.Close()
-	}
-	b.peerAPIListeners = nil
+	b.closePeerAPIListenersLocked()
 
 	selfNode := b.netMap.SelfNode
 	if len(b.netMap.Addresses) == 0 || selfNode == nil {
@@ -1957,20 +2016,27 @@ func applyPrefsToHostinfo(hi *tailcfg.Hostinfo, prefs *ipn.Prefs) {
 // happen".
 func (b *LocalBackend) enterState(newState ipn.State) {
 	b.mu.Lock()
-	state := b.state
+	oldState := b.state
 	b.state = newState
 	prefs := b.prefs
 	cc := b.cc
 	networkUp := b.prevIfState.AnyInterfaceUp()
 	activeLogin := b.activeLogin
 	authURL := b.authURL
+	if newState == ipn.Running {
+		b.authURL = ""
+		b.authURLSticky = ""
+	} else if oldState == ipn.Running {
+		// Transitioning away from running.
+		b.closePeerAPIListenersLocked()
+	}
 	b.mu.Unlock()
 
-	if state == newState {
+	if oldState == newState {
 		return
 	}
 	b.logf("Switching ipn state %v -> %v (WantRunning=%v)",
-		state, newState, prefs.WantRunning)
+		oldState, newState, prefs.WantRunning)
 	health.SetIPNState(newState.String(), prefs.WantRunning)
 	b.send(ipn.Notify{State: &newState})
 
@@ -2115,6 +2181,7 @@ func (b *LocalBackend) ResetForClientDisconnect() {
 	b.setNetMapLocked(nil)
 	b.prefs = new(ipn.Prefs)
 	b.authURL = ""
+	b.authURLSticky = ""
 	b.activeLogin = ""
 }
 
@@ -2193,6 +2260,17 @@ func (b *LocalBackend) setNetInfo(ni *tailcfg.NetInfo) {
 	cc.SetNetInfo(ni)
 }
 
+func hasCapability(nm *netmap.NetworkMap, cap string) bool {
+	if nm != nil && nm.SelfNode != nil {
+		for _, c := range nm.SelfNode.Capabilities {
+			if c == cap {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 	var login string
 	if nm != nil {
@@ -2207,6 +2285,13 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 		b.activeLogin = login
 	}
 
+	// Determine if file sharing is enabled
+	fs := hasCapability(nm, tailcfg.CapabilityFileSharing)
+	if fs != b.capFileSharing {
+		osshare.SetFileSharingEnabled(fs, b.logf)
+	}
+	b.capFileSharing = fs
+
 	if nm == nil {
 		b.nodeByAddr = nil
 		return
@@ -2315,20 +2400,7 @@ func (b *LocalBackend) OpenFile(name string) (rc io.ReadCloser, size int64, err
 func (b *LocalBackend) hasCapFileSharing() bool {
 	b.mu.Lock()
 	defer b.mu.Unlock()
-	return b.hasCapFileSharingLocked()
-}
-
-func (b *LocalBackend) hasCapFileSharingLocked() bool {
-	nm := b.netMap
-	if nm == nil || nm.SelfNode == nil {
-		return false
-	}
-	for _, c := range nm.SelfNode.Capabilities {
-		if c == tailcfg.CapabilityFileSharing {
-			return true
-		}
-	}
-	return false
+	return b.capFileSharing
 }
 
 // FileTargets lists nodes that the current node can send files to.
@@ -2337,7 +2409,7 @@ func (b *LocalBackend) FileTargets() ([]*apitype.FileTarget, error) {
 
 	b.mu.Lock()
 	defer b.mu.Unlock()
-	if !b.hasCapFileSharingLocked() {
+	if !b.capFileSharing {
 		return nil, errors.New("file sharing not enabled by Tailscale admin")
 	}
 	nm := b.netMap

ipn/ipnlocal/local_test.go

@@ -5,14 +5,20 @@
 package ipnlocal
 
 import (
+	"bytes"
+	"fmt"
+	"net/http"
 	"reflect"
+	"sync"
 	"testing"
 
 	"inet.af/netaddr"
+	"tailscale.com/ipn"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/netmap"
+	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/wgcfg"
 )
 
@@ -419,3 +425,48 @@ func TestPeerAPIBase(t *testing.T) {
 		})
 	}
 }
+
+type panicOnUseTransport struct{}
+
+func (panicOnUseTransport) RoundTrip(*http.Request) (*http.Response, error) {
+	panic("unexpected HTTP request")
+}
+
+var nl = []byte("\n")
+
+func TestStartsInNeedsLoginState(t *testing.T) {
+	var (
+		mu     sync.Mutex
+		logBuf bytes.Buffer
+	)
+	logf := func(format string, a ...interface{}) {
+		mu.Lock()
+		defer mu.Unlock()
+		fmt.Fprintf(&logBuf, format, a...)
+		if !bytes.HasSuffix(logBuf.Bytes(), nl) {
+			logBuf.Write(nl)
+		}
+	}
+	store := new(ipn.MemoryStore)
+	eng, err := wgengine.NewFakeUserspaceEngine(logf, 0)
+	if err != nil {
+		t.Fatalf("NewFakeUserspaceEngine: %v", err)
+	}
+	lb, err := NewLocalBackend(logf, "logid", store, eng)
+	if err != nil {
+		t.Fatalf("NewLocalBackend: %v", err)
+	}
+
+	lb.SetHTTPTestClient(&http.Client{
+		Transport: panicOnUseTransport{}, // validate we don't send HTTP requests
+	})
+
+	if err := lb.Start(ipn.Options{
+		StateKey: ipn.GlobalDaemonStateKey,
+	}); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	if st := lb.State(); st != ipn.NeedsLogin {
+		t.Errorf("State = %v; want NeedsLogin", st)
+	}
+}

ipn/ipnlocal/peerapi.go

@@ -11,6 +11,7 @@ import (
 	"hash/crc32"
 	"html"
 	"io"
+	"io/fs"
 	"net"
 	"net/http"
 	"net/url"
@@ -18,6 +19,7 @@ import (
 	"path"
 	"path/filepath"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -28,6 +30,7 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/ipn"
+	"tailscale.com/logtail/backoff"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/syncs"
 	"tailscale.com/tailcfg"
@@ -47,10 +50,24 @@ type peerAPIServer struct {
 	// download directory (as *.partial files), rather than making
 	// the frontend retrieve it over localapi HTTP and write it
 	// somewhere itself. This is used on GUI macOS version.
+	// In directFileMode, the peerapi doesn't do the final rename
+	// from "foo.jpg.partial" to "foo.jpg".
 	directFileMode bool
 }
 
-const partialSuffix = ".partial"
+const (
+	// partialSuffix is the suffix appened to files while they're
+	// still in the process of being transferred.
+	partialSuffix = ".partial"
+
+	// deletedSuffix is the suffix for a deleted marker file
+	// that's placed next to a file (without the suffix) that we
+	// tried to delete, but Windows wouldn't let us. These are
+	// only written on Windows (and in tests), but they're not
+	// permitted to be uploaded directly on any platform, like
+	// partial files.
+	deletedSuffix = ".deleted"
+)
 
 func validFilenameRune(r rune) bool {
 	switch r {
@@ -83,6 +100,7 @@ func (s *peerAPIServer) diskPath(baseName string) (fullPath string, ok bool) {
 	clean := path.Clean(baseName)
 	if clean != baseName ||
 		clean == "." || clean == ".." ||
+		strings.HasSuffix(clean, deletedSuffix) ||
 		strings.HasSuffix(clean, partialSuffix) {
 		return "", false
 	}
@@ -116,11 +134,28 @@ func (s *peerAPIServer) hasFilesWaiting() bool {
 	for {
 		des, err := f.ReadDir(10)
 		for _, de := range des {
-			if strings.HasSuffix(de.Name(), partialSuffix) {
+			name := de.Name()
+			if strings.HasSuffix(name, partialSuffix) {
+				continue
+			}
+			if strings.HasSuffix(name, deletedSuffix) { // for Windows + tests
+				// After we're done looping over files, then try
+				// to delete this file. Don't do it proactively,
+				// as the OS may return "foo.jpg.deleted" before "foo.jpg"
+				// and we don't want to delete the ".deleted" file before
+				// enumerating to the "foo.jpg" file.
+				defer tryDeleteAgain(filepath.Join(s.rootDir, strings.TrimSuffix(name, deletedSuffix)))
 				continue
 			}
 			if de.Type().IsRegular() {
-				return true
+				_, err := os.Stat(filepath.Join(s.rootDir, name+deletedSuffix))
+				if os.IsNotExist(err) {
+					return true
+				}
+				if err == nil {
+					tryDeleteAgain(filepath.Join(s.rootDir, name))
+					continue
+				}
 			}
 		}
 		if err == io.EOF {
@@ -133,6 +168,12 @@ func (s *peerAPIServer) hasFilesWaiting() bool {
 	return false
 }
 
+// WaitingFiles returns the list of files that have been sent by a
+// peer that are waiting in the buffered "pick up" directory owned by
+// the Tailscale daemon.
+//
+// As a side effect, it also does any lazy deletion of files as
+// required by Windows.
 func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 	if s.rootDir == "" {
 		return nil, errors.New("peerapi disabled; no storage configured")
@@ -145,6 +186,7 @@ func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 		return nil, err
 	}
 	defer f.Close()
+	var deleted map[string]bool // "foo.jpg" => true (if "foo.jpg.deleted" exists)
 	for {
 		des, err := f.ReadDir(10)
 		for _, de := range des {
@@ -152,6 +194,13 @@ func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 			if strings.HasSuffix(name, partialSuffix) {
 				continue
 			}
+			if strings.HasSuffix(name, deletedSuffix) { // for Windows + tests
+				if deleted == nil {
+					deleted = map[string]bool{}
+				}
+				deleted[strings.TrimSuffix(name, deletedSuffix)] = true
+				continue
+			}
 			if de.Type().IsRegular() {
 				fi, err := de.Info()
 				if err != nil {
@@ -170,9 +219,41 @@ func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 			return nil, err
 		}
 	}
+	if len(deleted) > 0 {
+		// Filter out any return values "foo.jpg" where a
+		// "foo.jpg.deleted" marker file exists on disk.
+		all := ret
+		ret = ret[:0]
+		for _, wf := range all {
+			if !deleted[wf.Name] {
+				ret = append(ret, wf)
+			}
+		}
+		// And do some opportunistic deleting while we're here.
+		// Maybe Windows is done virus scanning the file we tried
+		// to delete a long time ago and will let us delete it now.
+		for name := range deleted {
+			tryDeleteAgain(filepath.Join(s.rootDir, name))
+		}
+	}
+	sort.Slice(ret, func(i, j int) bool { return ret[i].Name < ret[j].Name })
 	return ret, nil
 }
 
+// tryDeleteAgain tries to delete path (and path+deletedSuffix) after
+// it failed earlier.  This happens on Windows when various anti-virus
+// tools hook into filesystem operations and have the file open still
+// while we're trying to delete it. In that case we instead mark it as
+// deleted (writing a "foo.jpg.deleted" marker file), but then we
+// later try to clean them up.
+//
+// fullPath is the full path to the file without the deleted suffix.
+func tryDeleteAgain(fullPath string) {
+	if err := os.Remove(fullPath); err == nil || os.IsNotExist(err) {
+		os.Remove(fullPath + deletedSuffix)
+	}
+}
+
 func (s *peerAPIServer) DeleteFile(baseName string) error {
 	if s.rootDir == "" {
 		return errors.New("peerapi disabled; no storage configured")
@@ -184,11 +265,55 @@ func (s *peerAPIServer) DeleteFile(baseName string) error {
 	if !ok {
 		return errors.New("bad filename")
 	}
-	err := os.Remove(path)
-	if err != nil && !os.IsNotExist(err) {
+	var bo *backoff.Backoff
+	logf := s.b.logf
+	t0 := time.Now()
+	for {
+		err := os.Remove(path)
+		if err != nil && !os.IsNotExist(err) {
+			err = redactErr(err)
+			// Put a retry loop around deletes on Windows. Windows
+			// file descriptor closes are effectively asynchronous,
+			// as a bunch of hooks run on/after close, and we can't
+			// necessarily delete the file for a while after close,
+			// as we need to wait for everybody to be done with
+			// it. (on Windows, unlike Unix, a file can't be deleted
+			// if it's open anywhere)
+			// So try a few times but ultimately just leave a
+			// "foo.jpg.deleted" marker file to note that it's
+			// deleted and we clean it up later.
+			if runtime.GOOS == "windows" {
+				if bo == nil {
+					bo = backoff.NewBackoff("delete-retry", logf, 1*time.Second)
+				}
+				if time.Since(t0) < 5*time.Second {
+					bo.BackOff(context.Background(), err)
+					continue
+				}
+				if err := redactErr(touchFile(path + deletedSuffix)); err != nil {
+					logf("peerapi: failed to leave deleted marker: %v", err)
+				}
+			}
+			logf("peerapi: failed to DeleteFile: %v", err)
+			return err
+		}
+		return nil
+	}
+}
+
+func redactErr(err error) error {
+	if pe, ok := err.(*os.PathError); ok {
+		pe.Path = "redacted"
+	}
+	return err
+}
+
+func touchFile(path string) error {
+	f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0666)
+	if err != nil {
 		return err
 	}
-	return nil
+	return f.Close()
 }
 
 func (s *peerAPIServer) OpenFile(baseName string) (rc io.ReadCloser, size int64, err error) {
@@ -202,6 +327,10 @@ func (s *peerAPIServer) OpenFile(baseName string) (rc io.ReadCloser, size int64,
 	if !ok {
 		return nil, 0, errors.New("bad filename")
 	}
+	if fi, err := os.Stat(path + deletedSuffix); err == nil && fi.Mode().IsRegular() {
+		tryDeleteAgain(path)
+		return nil, 0, &fs.PathError{Op: "open", Path: path, Err: fs.ErrNotExist}
+	}
 	f, err := os.Open(path)
 	if err != nil {
 		return nil, 0, err
@@ -367,6 +496,10 @@ func (h *peerAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		h.handlePeerPut(w, r)
 		return
 	}
+	if r.URL.Path == "/v0/goroutines" {
+		h.handleServeGoroutines(w, r)
+		return
+	}
 	who := h.peerUser.DisplayName
 	fmt.Fprintf(w, `<html>
 <meta name="viewport" content="width=device-width, initial-scale=1">
@@ -479,19 +612,18 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "bad filename", 400)
 		return
 	}
-	if h.ps.directFileMode {
-		dstFile += partialSuffix
-	}
-	f, err := os.Create(dstFile)
+	// TODO(bradfitz): prevent same filename being sent by two peers at once
+	partialFile := dstFile + partialSuffix
+	f, err := os.Create(partialFile)
 	if err != nil {
-		h.logf("put Create error: %v", err)
+		h.logf("put Create error: %v", redactErr(err))
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	var success bool
 	defer func() {
 		if !success {
-			os.Remove(dstFile)
+			os.Remove(partialFile)
 		}
 	}()
 	var finalSize int64
@@ -505,7 +637,7 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 			ph:      h,
 		}
 		if h.ps.directFileMode {
-			inFile.partialPath = dstFile
+			inFile.partialPath = partialFile
 		}
 		h.ps.b.registerIncomingFile(inFile, true)
 		defer h.ps.b.registerIncomingFile(inFile, false)
@@ -527,6 +659,13 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 		if inFile != nil { // non-zero length; TODO: notify even for zero length
 			inFile.markAndNotifyDone()
 		}
+	} else {
+		if err := os.Rename(partialFile, dstFile); err != nil {
+			err = redactErr(err)
+			h.logf("put final rename: %v", err)
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
 	}
 
 	h.logf("put of %s from %v/%v", approxSize(finalSize), h.remoteAddr.IP, h.peerNode.ComputedName)
@@ -546,5 +685,21 @@ func approxSize(n int64) string {
 	if n <= 1<<20 {
 		return "<=1MB"
 	}
-	return fmt.Sprintf("~%dMB", n/1<<20)
+	return fmt.Sprintf("~%dMB", n>>20)
+}
+
+func (h *peerAPIHandler) handleServeGoroutines(w http.ResponseWriter, r *http.Request) {
+	if !h.isSelf {
+		http.Error(w, "not owner", http.StatusForbidden)
+		return
+	}
+	var buf []byte
+	for size := 4 << 10; size <= 2<<20; size *= 2 {
+		buf = make([]byte, size)
+		buf = buf[:runtime.Stack(buf, true)]
+		if len(buf) < size {
+			break
+		}
+	}
+	w.Write(buf)
 }

ipn/ipnlocal/peerapi_test.go

@@ -10,15 +10,16 @@ import (
 	"io"
 	"io/fs"
 	"io/ioutil"
+	"math/rand"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
 	"tailscale.com/tailcfg"
-	"tailscale.com/types/netmap"
 )
 
 type peerAPITestEnv struct {
@@ -102,7 +103,7 @@ func hexAll(v string) string {
 	return sb.String()
 }
 
-func TestHandlePeerPut(t *testing.T) {
+func TestHandlePeerAPI(t *testing.T) {
 	tests := []struct {
 		name       string
 		isSelf     bool // the peer sending the request is owned by us
@@ -133,6 +134,21 @@ func TestHandlePeerPut(t *testing.T) {
 				bodyNotContains("You are the owner of this node."),
 			),
 		},
+		{
+			name:   "peer_api_goroutines_deny",
+			isSelf: false,
+			req:    httptest.NewRequest("GET", "/v0/goroutines", nil),
+			checks: checks(httpStatus(403)),
+		},
+		{
+			name:   "peer_api_goroutines",
+			isSelf: true,
+			req:    httptest.NewRequest("GET", "/v0/goroutines", nil),
+			checks: checks(
+				httpStatus(200),
+				bodyContains("ServeHTTP"),
+			),
+		},
 		{
 			name:       "reject_non_owner_put",
 			isSelf:     false,
@@ -220,6 +236,16 @@ func TestHandlePeerPut(t *testing.T) {
 				bodyContains("bad filename"),
 			),
 		},
+		{
+			name:       "bad_filename_deleted",
+			isSelf:     true,
+			capSharing: true,
+			req:        httptest.NewRequest("PUT", "/v0/put/foo.deleted", nil),
+			checks: checks(
+				httpStatus(400),
+				bodyContains("bad filename"),
+			),
+		},
 		{
 			name:       "bad_filename_dot",
 			isSelf:     true,
@@ -375,18 +401,10 @@ func TestHandlePeerPut(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var caps []string
-			if tt.capSharing {
-				caps = append(caps, tailcfg.CapabilityFileSharing)
-			}
 			var e peerAPITestEnv
 			lb := &LocalBackend{
-				netMap: &netmap.NetworkMap{
-					SelfNode: &tailcfg.Node{
-						Capabilities: caps,
-					},
-				},
-				logf: e.logf,
+				logf:           e.logf,
+				capFileSharing: tt.capSharing,
 			}
 			e.ph = &peerAPIHandler{
 				isSelf: tt.isSelf,
@@ -422,3 +440,134 @@ func TestHandlePeerPut(t *testing.T) {
 		})
 	}
 }
+
+// Windows likes to hold on to file descriptors for some indeterminate
+// amount of time after you close them and not let you delete them for
+// a bit. So test that we work around that sufficiently.
+func TestFileDeleteRace(t *testing.T) {
+	dir := t.TempDir()
+	ps := &peerAPIServer{
+		b: &LocalBackend{
+			logf:           t.Logf,
+			capFileSharing: true,
+		},
+		rootDir: dir,
+	}
+	ph := &peerAPIHandler{
+		isSelf: true,
+		peerNode: &tailcfg.Node{
+			ComputedName: "some-peer-name",
+		},
+		ps: ps,
+	}
+	buf := make([]byte, 2<<20)
+	for i := 0; i < 30; i++ {
+		rr := httptest.NewRecorder()
+		ph.ServeHTTP(rr, httptest.NewRequest("PUT", "/v0/put/foo.txt", bytes.NewReader(buf[:rand.Intn(len(buf))])))
+		if res := rr.Result(); res.StatusCode != 200 {
+			t.Fatal(res.Status)
+		}
+		wfs, err := ps.WaitingFiles()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(wfs) != 1 {
+			t.Fatalf("waiting files = %d; want 1", len(wfs))
+		}
+
+		if err := ps.DeleteFile("foo.txt"); err != nil {
+			t.Fatal(err)
+		}
+		wfs, err = ps.WaitingFiles()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(wfs) != 0 {
+			t.Fatalf("waiting files = %d; want 0", len(wfs))
+		}
+	}
+}
+
+// Tests "foo.jpg.deleted" marks (for Windows).
+func TestDeletedMarkers(t *testing.T) {
+	dir := t.TempDir()
+	ps := &peerAPIServer{
+		b: &LocalBackend{
+			logf:           t.Logf,
+			capFileSharing: true,
+		},
+		rootDir: dir,
+	}
+
+	nothingWaiting := func() {
+		t.Helper()
+		ps.knownEmpty.Set(false)
+		if ps.hasFilesWaiting() {
+			t.Fatal("unexpected files waiting")
+		}
+	}
+	touch := func(base string) {
+		t.Helper()
+		if err := touchFile(filepath.Join(dir, base)); err != nil {
+			t.Fatal(err)
+		}
+	}
+	wantEmptyTempDir := func() {
+		t.Helper()
+		if fis, err := ioutil.ReadDir(dir); err != nil {
+			t.Fatal(err)
+		} else if len(fis) > 0 && runtime.GOOS != "windows" {
+			for _, fi := range fis {
+				t.Errorf("unexpected file in tempdir: %q", fi.Name())
+			}
+		}
+	}
+
+	nothingWaiting()
+	wantEmptyTempDir()
+
+	touch("foo.jpg.deleted")
+	nothingWaiting()
+	wantEmptyTempDir()
+
+	touch("foo.jpg.deleted")
+	touch("foo.jpg")
+	nothingWaiting()
+	wantEmptyTempDir()
+
+	touch("foo.jpg.deleted")
+	touch("foo.jpg")
+	wf, err := ps.WaitingFiles()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(wf) != 0 {
+		t.Fatalf("WaitingFiles = %d; want 0", len(wf))
+	}
+	wantEmptyTempDir()
+
+	touch("foo.jpg.deleted")
+	touch("foo.jpg")
+	if rc, _, err := ps.OpenFile("foo.jpg"); err == nil {
+		rc.Close()
+		t.Fatal("unexpected foo.jpg open")
+	}
+	wantEmptyTempDir()
+
+	// And verify basics still work in non-deleted cases.
+	touch("foo.jpg")
+	touch("bar.jpg.deleted")
+	if wf, err := ps.WaitingFiles(); err != nil {
+		t.Error(err)
+	} else if len(wf) != 1 {
+		t.Errorf("WaitingFiles = %d; want 1", len(wf))
+	} else if wf[0].Name != "foo.jpg" {
+		t.Errorf("unexpected waiting file %+v", wf[0])
+	}
+	if rc, _, err := ps.OpenFile("foo.jpg"); err != nil {
+		t.Fatal(err)
+	} else {
+		rc.Close()
+	}
+
+}

ipn/message.go

@@ -93,17 +93,27 @@ type BackendServer struct {
 	GotQuit       bool                 // a Quit command was received
 }
 
+// NewBackendServer creates a new BackendServer using b.
+//
+// If sendNotifyMsg is non-nil, it additionally sets the Backend's
+// notification callback to call the func with ipn.Notify messages in
+// JSON form. If nil, it does not change the notification callback.
 func NewBackendServer(logf logger.Logf, b Backend, sendNotifyMsg func(b []byte)) *BackendServer {
 	bs := &BackendServer{
 		logf:          logf,
 		b:             b,
 		sendNotifyMsg: sendNotifyMsg,
 	}
-	b.SetNotifyCallback(bs.send)
+	if sendNotifyMsg != nil {
+		b.SetNotifyCallback(bs.send)
+	}
 	return bs
 }
 
 func (bs *BackendServer) send(n Notify) {
+	if bs.sendNotifyMsg == nil {
+		return
+	}
 	n.Version = version.Long
 	b, err := json.Marshal(n)
 	if err != nil {

ipn/message_test.go

@@ -87,6 +87,8 @@ func TestClientServer(t *testing.T) {
 		t.Logf("c: "+fmt, args...)
 	}
 	bs = NewBackendServer(slogf, b, serverToClient)
+	// Verify that this doesn't break bs's callback:
+	NewBackendServer(slogf, b, nil)
 	bc = NewBackendClient(clogf, clientToServer)
 
 	ch := make(chan Notify, 256)

net/dns/manager_linux.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/godbus/dbus/v5"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/cmpver"
 )
 
 type kv struct {
@@ -64,18 +65,54 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 			return newResolvedManager(logf)
 		}
 		dbg("nm-resolved", "yes")
-		return newNMManager(interfaceName)
+
+		// Version of NetworkManager before 1.26.6 programmed resolved
+		// incorrectly, such that NM's settings would always take
+		// precedence over other settings set by other resolved
+		// clients.
+		//
+		// If we're dealing with such a version, we have to set our
+		// DNS settings through NM to have them take.
+		//
+		// However, versions 1.26.6 later both fixed the resolved
+		// programming issue _and_ started ignoring DNS settings for
+		// "unmanaged" interfaces - meaning NM 1.26.6 and later
+		// actively ignore DNS configuration we give it. So, for those
+		// NM versions, we can and must use resolved directly.
+		old, err := nmVersionOlderThan("1.26.6")
+		if err != nil {
+			// Failed to figure out NM's version, can't make a correct
+			// decision.
+			return nil, fmt.Errorf("checking NetworkManager version: %v", err)
+		}
+		if old {
+			dbg("nm-old", "yes")
+			return newNMManager(interfaceName)
+		}
+		dbg("nm-old", "no")
+		return newResolvedManager(logf)
 	case "resolvconf":
 		dbg("rc", "resolvconf")
 		if err := resolvconfSourceIsNM(bs); err == nil {
 			dbg("src-is-nm", "yes")
 			if err := dbusPing("org.freedesktop.NetworkManager", "/org/freedesktop/NetworkManager/DnsManager"); err == nil {
 				dbg("nm", "yes")
-				return newNMManager(interfaceName)
+				old, err := nmVersionOlderThan("1.26.6")
+				if err != nil {
+					return nil, fmt.Errorf("checking NetworkManager version: %v", err)
+				}
+				if old {
+					dbg("nm-old", "yes")
+					return newNMManager(interfaceName)
+				} else {
+					dbg("nm-old", "no")
+				}
+			} else {
+				dbg("nm", "no")
 			}
-			dbg("nm", "no")
+		} else {
+			dbg("src-is-nm", "no")
 		}
-		dbg("src-is-nm", "no")
 		if _, err := exec.LookPath("resolvconf"); err != nil {
 			dbg("resolvconf", "no")
 			return newDirectManager()
@@ -89,7 +126,16 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 			return newDirectManager()
 		}
 		dbg("nm", "yes")
-		return newNMManager(interfaceName)
+		old, err := nmVersionOlderThan("1.26.6")
+		if err != nil {
+			return nil, fmt.Errorf("checking NetworkManager version: %v", err)
+		}
+		if old {
+			dbg("nm-old", "yes")
+			return newNMManager(interfaceName)
+		}
+		dbg("nm-old", "no")
+		return newDirectManager()
 	default:
 		dbg("rc", "unknown")
 		return newDirectManager()
@@ -135,6 +181,27 @@ func resolvconfSourceIsNM(resolvDotConf []byte) error {
 	return nil
 }
 
+func nmVersionOlderThan(want string) (bool, error) {
+	conn, err := dbus.SystemBus()
+	if err != nil {
+		// DBus probably not running.
+		return false, err
+	}
+
+	nm := conn.Object("org.freedesktop.NetworkManager", dbus.ObjectPath("/org/freedesktop/NetworkManager"))
+	v, err := nm.GetProperty("org.freedesktop.NetworkManager.Version")
+	if err != nil {
+		return false, err
+	}
+
+	version, ok := v.Value().(string)
+	if !ok {
+		return false, fmt.Errorf("unexpected type %T for NM version", v.Value())
+	}
+
+	return cmpver.Compare(version, want) < 0, nil
+}
+
 func nmIsUsingResolved() error {
 	conn, err := dbus.SystemBus()
 	if err != nil {

net/dns/resolver/forwarder.go

@@ -13,7 +13,6 @@ import (
 	"hash/crc32"
 	"math/rand"
 	"net"
-	"os"
 	"sync"
 	"time"
 
@@ -39,8 +38,6 @@ const (
 
 var errNoUpstreams = errors.New("upstream nameservers not set")
 
-var aLongTimeAgo = time.Unix(0, 1)
-
 type forwardingRecord struct {
 	src       netaddr.IPPort
 	createdAt time.Time
@@ -303,8 +300,6 @@ type fwdConn struct {
 	// logf allows a fwdConn to log.
 	logf logger.Logf
 
-	// wg tracks the number of outstanding conn.Read and conn.Write calls.
-	wg sync.WaitGroup
 	// change allows calls to read to block until a the network connection has been replaced.
 	change *sync.Cond
 
@@ -352,15 +347,12 @@ func (c *fwdConn) send(packet []byte, dst netaddr.IPPort) {
 		}
 		c.mu.Unlock()
 
-		a := dst.UDPAddr()
-		c.wg.Add(1)
-		_, err := conn.WriteTo(packet, a)
-		c.wg.Done()
+		_, err := conn.WriteTo(packet, dst.UDPAddr())
 		if err == nil {
 			// Success
 			return
 		}
-		if errors.Is(err, os.ErrDeadlineExceeded) {
+		if errors.Is(err, net.ErrClosed) {
 			// We intentionally closed this connection.
 			// It has been replaced by a new connection. Try again.
 			continue
@@ -429,14 +421,12 @@ func (c *fwdConn) read(out []byte) int {
 		}
 		c.mu.Unlock()
 
-		c.wg.Add(1)
 		n, _, err := conn.ReadFrom(out)
-		c.wg.Done()
 		if err == nil {
 			// Success.
 			return n
 		}
-		if errors.Is(err, os.ErrDeadlineExceeded) {
+		if errors.Is(err, net.ErrClosed) {
 			// We intentionally closed this connection.
 			// It has been replaced by a new connection. Try again.
 			continue
@@ -468,10 +458,7 @@ func (c *fwdConn) closeConnLocked() {
 	if c.conn == nil {
 		return
 	}
-	// Unblock all readers/writers, wait for them, close the connection.
-	c.conn.SetDeadline(aLongTimeAgo)
-	c.wg.Wait()
-	c.conn.Close()
+	c.conn.Close() // unblocks all readers/writers, they'll pick up the next connection.
 	c.conn = nil
 }
 

net/interfaces/interfaces_darwin.go

@@ -10,6 +10,7 @@ import (
 	"log"
 	"net"
 	"syscall"
+	"time"
 
 	"golang.org/x/net/route"
 	"golang.org/x/sys/unix"
@@ -28,6 +29,34 @@ func DefaultRouteInterface() (string, error) {
 	return iface.Name, nil
 }
 
+// fetchRoutingTable is a retry loop around route.FetchRIB, fetching NET_RT_DUMP2.
+//
+// The retry loop is due to a bug in the BSDs (or Go?). See
+// https://github.com/tailscale/tailscale/issues/1345
+func fetchRoutingTable() (rib []byte, err error) {
+	fails := 0
+	for {
+		rib, err := route.FetchRIB(syscall.AF_UNSPEC, syscall.NET_RT_DUMP2, 0)
+		if err == nil {
+			return rib, nil
+		}
+		fails++
+		if fails < 10 {
+			// Empirically, 1 retry is enough. In a long
+			// stress test while toggling wifi on & off, I
+			// only saw a few occurrences of 2 and one 3.
+			// So 10 should be more plenty.
+			if fails > 5 {
+				time.Sleep(5 * time.Millisecond)
+			}
+			continue
+		}
+		if err != nil {
+			return nil, fmt.Errorf("route.FetchRIB: %w", err)
+		}
+	}
+}
+
 func DefaultRouteInterfaceIndex() (int, error) {
 	// $ netstat -nr
 	// Routing tables
@@ -43,7 +72,7 @@ func DefaultRouteInterfaceIndex() (int, error) {
 	// c       RTF_PRCLONING    Protocol-specified generate new routes on use
 	// I       RTF_IFSCOPE      Route is associated with an interface scope
 
-	rib, err := route.FetchRIB(syscall.AF_UNSPEC, syscall.NET_RT_DUMP2, 0)
+	rib, err := fetchRoutingTable()
 	if err != nil {
 		return 0, fmt.Errorf("route.FetchRIB: %w", err)
 	}
@@ -83,7 +112,7 @@ func init() {
 }
 
 func likelyHomeRouterIPDarwinFetchRIB() (ret netaddr.IP, ok bool) {
-	rib, err := route.FetchRIB(syscall.AF_UNSPEC, syscall.NET_RT_DUMP2, 0)
+	rib, err := fetchRoutingTable()
 	if err != nil {
 		log.Printf("routerIP/FetchRIB: %v", err)
 		return ret, false

net/interfaces/interfaces_darwin_test.go

@@ -83,4 +83,14 @@ func likelyHomeRouterIPDarwinExec() (ret netaddr.IP, ok bool) {
 	return ret, !ret.IsZero()
 }
 
+func TestFetchRoutingTable(t *testing.T) {
+	// Issue 1345: this used to be flaky on darwin.
+	for i := 0; i < 20; i++ {
+		_, err := fetchRoutingTable()
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
 var errStopReadingNetstatTable = errors.New("found private gateway")

net/tstun/wrap.go

@@ -90,7 +90,7 @@ type Wrapper struct {
 	// to discard an empty packet instead of sending it through t.outbound.
 	outbound chan []byte
 
-	// fitler stores the currently active package filter
+	// filter atomically stores the currently active packet filter
 	filter atomic.Value // of *filter.Filter
 	// filterFlags control the verbosity of logging packet drops/accepts.
 	filterFlags filter.RunFlags

tailcfg/tailcfg.go

@@ -41,7 +41,8 @@ import (
 //    16: 2021-04-15: client understands Node.Online, MapResponse.OnlineChange
 //    17: 2021-04-18: MapResponse.Domain empty means unchanged
 //    18: 2021-04-19: MapResponse.Node nil means unchanged (all fields now omitempty)
-const CurrentMapRequestVersion = 18
+//    19: 2021-04-21: MapResponse.Debug.SleepSeconds
+const CurrentMapRequestVersion = 19
 
 type StableID string
 
@@ -1012,6 +1013,12 @@ type Debug struct {
 	// GoroutineDumpURL, if non-empty, requests that the client do
 	// a one-time dump of its active goroutines to the given URL.
 	GoroutineDumpURL string `json:",omitempty"`
+
+	// SleepSeconds requests that the client sleep for the
+	// provided number of seconds.
+	// The client can (and should) limit the value (such as 5
+	// minutes).
+	SleepSeconds float64 `json:",omitempty"`
 }
 
 func (k MachineKey) String() string                   { return fmt.Sprintf("mkey:%x", k[:]) }

util/cmpver/version.go

@@ -0,0 +1,96 @@
+// 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 cmpver implements a variant of debian version number
+// comparison.
+//
+// A version is a string consisting of alternating non-numeric and
+// numeric fields. When comparing two versions, each one is broken
+// down into its respective fields, and the fields are compared
+// pairwise. The comparison is lexicographic for non-numeric fields,
+// numeric for numeric fields. The first non-equal field pair
+// determines the ordering of the two versions.
+//
+// This comparison scheme is a simplified version of Debian's version
+// number comparisons. Debian differs in a few details of
+// lexicographical field comparison, where certain characters have
+// special meaning and ordering. We don't need that, because Tailscale
+// version numbers don't need it.
+package cmpver
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+	"unicode"
+)
+
+// Compare returns an integer comparing two strings as version
+// numbers. The result will be 0 if v1==v2, -1 if v1 < v2, and +1 if
+// v1 > v2.
+func Compare(v1, v2 string) int {
+	notNumber := func(r rune) bool { return !unicode.IsNumber(r) }
+
+	var (
+		f1, f2 string
+		n1, n2 uint64
+		err    error
+	)
+	for v1 != "" || v2 != "" {
+		// Compare the non-numeric character run lexicographically.
+		f1, v1 = splitPrefixFunc(v1, notNumber)
+		f2, v2 = splitPrefixFunc(v2, notNumber)
+
+		if res := strings.Compare(f1, f2); res != 0 {
+			return res
+		}
+
+		// Compare the numeric character run numerically.
+		f1, v1 = splitPrefixFunc(v1, unicode.IsNumber)
+		f2, v2 = splitPrefixFunc(v2, unicode.IsNumber)
+
+		// ParseUint refuses to parse empty strings, which would only
+		// happen if we reached end-of-string. We follow the Debian
+		// convention that empty strings mean zero, because
+		// empirically that produces reasonable-feeling comparison
+		// behavior.
+		n1 = 0
+		if f1 != "" {
+			n1, err = strconv.ParseUint(f1, 10, 64)
+			if err != nil {
+				panic(fmt.Sprintf("all-number string %q didn't parse as string: %s", f1, err))
+			}
+		}
+
+		n2 = 0
+		if f2 != "" {
+			n2, err = strconv.ParseUint(f2, 10, 64)
+			if err != nil {
+				panic(fmt.Sprintf("all-number string %q didn't parse as string: %s", f2, err))
+			}
+		}
+
+		switch {
+		case n1 == n2:
+		case n1 < n2:
+			return -1
+		case n1 > n2:
+			return 1
+		}
+	}
+
+	// Only way to reach here is if v1 and v2 run out of fields
+	// simultaneously - i.e. exactly equal versions.
+	return 0
+}
+
+// splitPrefixFunc splits s at the first rune where f(rune) is false.
+func splitPrefixFunc(s string, f func(rune) bool) (string, string) {
+	for i, r := range s {
+		if !f(r) {
+			return s[:i], s[i:]
+		}
+	}
+	return s, s[:0]
+}

util/cmpver/version_test.go

@@ -0,0 +1,106 @@
+// 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 cmpver
+
+import "testing"
+
+func TestCompare(t *testing.T) {
+	tests := []struct {
+		name   string
+		v1, v2 string
+		want   int
+	}{
+		{
+			name: "both empty",
+			want: 0,
+		},
+		{
+			name: "v1 empty",
+			v2:   "1.2.3",
+			want: -1,
+		},
+		{
+			name: "v2 empty",
+			v1:   "1.2.3",
+			want: 1,
+		},
+
+		{
+			name: "semver major",
+			v1:   "2.0.0",
+			v2:   "1.9.9",
+			want: 1,
+		},
+		{
+			name: "semver major",
+			v1:   "2.0.0",
+			v2:   "1.9.9",
+			want: 1,
+		},
+		{
+			name: "semver minor",
+			v1:   "1.9.0",
+			v2:   "1.8.9",
+			want: 1,
+		},
+		{
+			name: "semver patch",
+			v1:   "1.9.9",
+			v2:   "1.9.8",
+			want: 1,
+		},
+		{
+			name: "semver equal",
+			v1:   "1.9.8",
+			v2:   "1.9.8",
+			want: 0,
+		},
+
+		{
+			name: "tailscale major",
+			v1:   "1.0-0",
+			v2:   "0.97-105",
+			want: 1,
+		},
+		{
+			name: "tailscale minor",
+			v1:   "0.98-0",
+			v2:   "0.97-105",
+			want: 1,
+		},
+		{
+			name: "tailscale patch",
+			v1:   "0.97-120",
+			v2:   "0.97-105",
+			want: 1,
+		},
+		{
+			name: "tailscale equal",
+			v1:   "0.97-105",
+			v2:   "0.97-105",
+			want: 0,
+		},
+		{
+			name: "tailscale weird extra field",
+			v1:   "0.96.1-0", // more fields == larger
+			v2:   "0.96-105",
+			want: 1,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := Compare(test.v1, test.v2)
+			if got != test.want {
+				t.Errorf("Compare(%v, %v) = %v, want %v", test.v1, test.v2, got, test.want)
+			}
+			// Reversing the comparison should reverse the outcome.
+			got2 := Compare(test.v2, test.v1)
+			if got2 != -test.want {
+				t.Errorf("Compare(%v, %v) = %v, want %v", test.v2, test.v1, got2, -test.want)
+			}
+		})
+	}
+}

util/osshare/filesharingstatus_noop.go

@@ -0,0 +1,13 @@
+// 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 !windows
+
+package osshare
+
+import (
+	"tailscale.com/types/logger"
+)
+
+func SetFileSharingEnabled(enabled bool, logf logger.Logf) {}

util/osshare/filesharingstatus_windows.go

@@ -0,0 +1,107 @@
+// 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 windows
+
+package osshare
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"sync"
+
+	"golang.org/x/sys/windows/registry"
+	"tailscale.com/types/logger"
+)
+
+const (
+	sendFileShellKey = `*\shell\tailscale`
+)
+
+var ipnExePath struct {
+	sync.Mutex
+	cache string // absolute path of tailscale-ipn.exe, populated lazily on first use
+}
+
+func getIpnExePath(logf logger.Logf) string {
+	ipnExePath.Lock()
+	defer ipnExePath.Unlock()
+
+	if ipnExePath.cache != "" {
+		return ipnExePath.cache
+	}
+
+	// Find the absolute path of tailscale-ipn.exe assuming that it's in the same
+	// directory as this executable (tailscaled.exe).
+	p, err := os.Executable()
+	if err != nil {
+		logf("os.Executable error: %v", err)
+		return ""
+	}
+	if p, err = filepath.EvalSymlinks(p); err != nil {
+		logf("filepath.EvalSymlinks error: %v", err)
+		return ""
+	}
+	p = filepath.Join(filepath.Dir(p), "tailscale-ipn.exe")
+	if p, err = filepath.Abs(p); err != nil {
+		logf("filepath.Abs error: %v", err)
+		return ""
+	}
+	ipnExePath.cache = p
+
+	return p
+}
+
+// SetFileSharingEnabled adds/removes "Send with Tailscale" from the Windows shell menu.
+func SetFileSharingEnabled(enabled bool, logf logger.Logf) {
+	logf = logger.WithPrefix(logf, fmt.Sprintf("SetFileSharingEnabled(%v) error: ", enabled))
+	if enabled {
+		enableFileSharing(logf)
+	} else {
+		disableFileSharing(logf)
+	}
+}
+
+func enableFileSharing(logf logger.Logf) {
+	path := getIpnExePath(logf)
+	if path == "" {
+		return
+	}
+
+	k, _, err := registry.CreateKey(registry.CLASSES_ROOT, sendFileShellKey, registry.WRITE)
+	if err != nil {
+		logf("failed to create HKEY_CLASSES_ROOT\\%s reg key: %v", sendFileShellKey, err)
+		return
+	}
+	defer k.Close()
+	if err := k.SetStringValue("", "Send with Tailscale..."); err != nil {
+		logf("k.SetStringValue error: %v", err)
+		return
+	}
+	if err := k.SetStringValue("Icon", path+",0"); err != nil {
+		logf("k.SetStringValue error: %v", err)
+		return
+	}
+	c, _, err := registry.CreateKey(k, "command", registry.WRITE)
+	if err != nil {
+		logf("failed to create HKEY_CLASSES_ROOT\\%s\\command reg key: %v", sendFileShellKey, err)
+		return
+	}
+	defer c.Close()
+	if err := c.SetStringValue("", "\""+path+"\" /push \"%1\""); err != nil {
+		logf("c.SetStringValue error: %v", err)
+	}
+}
+
+func disableFileSharing(logf logger.Logf) {
+	if err := registry.DeleteKey(registry.CLASSES_ROOT, sendFileShellKey+"\\command"); err != nil &&
+		err != registry.ErrNotExist {
+		logf("registry.DeleteKey error: %v\n", err)
+		return
+	}
+	if err := registry.DeleteKey(registry.CLASSES_ROOT, sendFileShellKey); err != nil && err != registry.ErrNotExist {
+		logf("registry.DeleteKey error: %v\n", err)
+	}
+}

wgengine/bench/bench.go

@@ -0,0 +1,398 @@
+// 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.
+
+// Create two wgengine instances and pass data through them, measuring
+// throughput, latency, and packet loss.
+package main
+
+import (
+	"bufio"
+	"io"
+	"log"
+	"net"
+	"net/http"
+	"net/http/pprof"
+	"os"
+	"strconv"
+	"time"
+
+	"inet.af/netaddr"
+	"tailscale.com/types/logger"
+)
+
+const PayloadSize = 1000
+const ICMPMinSize = 24
+
+var Addr1 = netaddr.MustParseIPPrefix("100.64.1.1/32")
+var Addr2 = netaddr.MustParseIPPrefix("100.64.1.2/32")
+
+func main() {
+	var logf logger.Logf = log.Printf
+	log.SetFlags(0)
+
+	debugMux := newDebugMux()
+	go runDebugServer(debugMux, "0.0.0.0:8999")
+
+	mode, err := strconv.Atoi(os.Args[1])
+	if err != nil {
+		log.Fatalf("%q: %v", os.Args[1], err)
+	}
+
+	traf := NewTrafficGen(nil)
+
+	// Sample test results below are using GOMAXPROCS=2 (for some
+	// tests, including wireguard-go, higher GOMAXPROCS goes slower)
+	// on apenwarr's old Linux box:
+	//   Intel(R) Core(TM) i7-4785T CPU @ 2.20GHz
+	// My 2019 Mac Mini is about 20% faster on most tests.
+
+	switch mode {
+	// tx=8786325 rx=8786326 (0 = 0.00% loss) (70768.7 Mbits/sec)
+	case 1:
+		setupTrivialNoAllocTest(logf, traf)
+
+	// tx=6476293 rx=6476293 (0 = 0.00% loss) (52249.7 Mbits/sec)
+	case 2:
+		setupTrivialTest(logf, traf)
+
+	// tx=1957974 rx=1958379 (0 = 0.00% loss) (15939.8 Mbits/sec)
+	case 11:
+		setupBlockingChannelTest(logf, traf)
+
+	// tx=728621 rx=701825 (26620 = 3.65% loss) (5525.2 Mbits/sec)
+	// (much faster on macOS??)
+	case 12:
+		setupNonblockingChannelTest(logf, traf)
+
+	// tx=1024260 rx=941098 (83334 = 8.14% loss) (7516.6 Mbits/sec)
+	// (much faster on macOS??)
+	case 13:
+		setupDoubleChannelTest(logf, traf)
+
+	// tx=265468 rx=263189 (2279 = 0.86% loss) (2162.0 Mbits/sec)
+	case 21:
+		setupUDPTest(logf, traf)
+
+	// tx=1493580 rx=1493580 (0 = 0.00% loss) (12210.4 Mbits/sec)
+	case 31:
+		setupBatchTCPTest(logf, traf)
+
+	// tx=134236 rx=133166 (1070 = 0.80% loss) (1088.9 Mbits/sec)
+	case 101:
+		setupWGTest(logf, traf, Addr1, Addr2)
+
+	default:
+		log.Fatalf("provide a valid test number (0..n)")
+	}
+
+	logf("initialized ok.")
+	traf.Start(Addr1.IP, Addr2.IP, PayloadSize+ICMPMinSize, 0)
+
+	var cur, prev Snapshot
+	var pps int64
+	i := 0
+	for {
+		i += 1
+		time.Sleep(10 * time.Millisecond)
+
+		if (i % 100) == 0 {
+			prev = cur
+			cur = traf.Snap()
+			d := cur.Sub(prev)
+
+			if prev.WhenNsec == 0 {
+				logf("tx=%-6d rx=%-6d", d.TxPackets, d.RxPackets)
+			} else {
+				logf("%v @%7d pkt/s", d, pps)
+			}
+		}
+
+		pps = traf.Adjust()
+	}
+}
+
+func newDebugMux() *http.ServeMux {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/debug/pprof/", pprof.Index)
+	mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
+	mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
+	mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
+	mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
+	return mux
+}
+
+func runDebugServer(mux *http.ServeMux, addr string) {
+	srv := &http.Server{
+		Addr:    addr,
+		Handler: mux,
+	}
+	if err := srv.ListenAndServe(); err != nil {
+		log.Fatal(err)
+	}
+}
+
+// The absolute minimal test of the traffic generator: have it fill
+// a packet buffer, then absorb it again. Zero packet loss.
+func setupTrivialNoAllocTest(logf logger.Logf, traf *TrafficGen) {
+	go func() {
+		b := make([]byte, 1600)
+		for {
+			n := traf.Generate(b, 16)
+			if n == 0 {
+				break
+			}
+			traf.GotPacket(b[0:n+16], 16)
+		}
+	}()
+}
+
+// Almost the same, but this time allocate a fresh buffer each time
+// through the loop. Still zero packet loss. Runs about 2/3 as fast for me.
+func setupTrivialTest(logf logger.Logf, traf *TrafficGen) {
+	go func() {
+		for {
+			b := make([]byte, 1600)
+			n := traf.Generate(b, 16)
+			if n == 0 {
+				break
+			}
+			traf.GotPacket(b[0:n+16], 16)
+		}
+	}()
+}
+
+// Pass packets through a blocking channel between sender and receiver.
+// Still zero packet loss since the sender stops when the channel is full.
+// Max speed depends on channel length (I'm not sure why).
+func setupBlockingChannelTest(logf logger.Logf, traf *TrafficGen) {
+	ch := make(chan []byte, 1000)
+
+	go func() {
+		// transmitter
+		for {
+			b := make([]byte, 1600)
+			n := traf.Generate(b, 16)
+			if n == 0 {
+				close(ch)
+				break
+			}
+			ch <- b[0 : n+16]
+		}
+	}()
+
+	go func() {
+		// receiver
+		for b := range ch {
+			traf.GotPacket(b, 16)
+		}
+	}()
+}
+
+// Same as setupBlockingChannelTest, but now we drop packets whenever the
+// channel is full. Max speed is about the same as the above test, but
+// now with nonzero packet loss.
+func setupNonblockingChannelTest(logf logger.Logf, traf *TrafficGen) {
+	ch := make(chan []byte, 1000)
+
+	go func() {
+		// transmitter
+		for {
+			b := make([]byte, 1600)
+			n := traf.Generate(b, 16)
+			if n == 0 {
+				close(ch)
+				break
+			}
+			select {
+			case ch <- b[0 : n+16]:
+			default:
+			}
+		}
+	}()
+
+	go func() {
+		// receiver
+		for b := range ch {
+			traf.GotPacket(b, 16)
+		}
+	}()
+}
+
+// Same as above, but at an intermediate blocking channel and goroutine
+// to make things a little more like wireguard-go. Roughly 20% slower than
+// the single-channel verison.
+func setupDoubleChannelTest(logf logger.Logf, traf *TrafficGen) {
+	ch := make(chan []byte, 1000)
+	ch2 := make(chan []byte, 1000)
+
+	go func() {
+		// transmitter
+		for {
+			b := make([]byte, 1600)
+			n := traf.Generate(b, 16)
+			if n == 0 {
+				close(ch)
+				break
+			}
+			select {
+			case ch <- b[0 : n+16]:
+			default:
+			}
+		}
+	}()
+
+	go func() {
+		// intermediary
+		for b := range ch {
+			ch2 <- b
+		}
+		close(ch2)
+	}()
+
+	go func() {
+		// receiver
+		for b := range ch2 {
+			traf.GotPacket(b, 16)
+		}
+	}()
+}
+
+// Instead of a channel, pass packets through a UDP socket.
+func setupUDPTest(logf logger.Logf, traf *TrafficGen) {
+	la, err := net.ResolveUDPAddr("udp", ":0")
+	if err != nil {
+		log.Fatalf("resolve: %v", err)
+	}
+
+	s1, err := net.ListenUDP("udp", la)
+	if err != nil {
+		log.Fatalf("listen1: %v", err)
+	}
+	s2, err := net.ListenUDP("udp", la)
+	if err != nil {
+		log.Fatalf("listen2: %v", err)
+	}
+
+	a2 := s2.LocalAddr()
+
+	// On macOS (but not Linux), you can't transmit to 0.0.0.0:port,
+	// which is what returns from .LocalAddr() above. We have to
+	// force it to localhost instead.
+	a2.(*net.UDPAddr).IP = net.ParseIP("127.0.0.1")
+
+	s1.SetWriteBuffer(1024 * 1024)
+	s2.SetReadBuffer(1024 * 1024)
+
+	go func() {
+		// transmitter
+		b := make([]byte, 1600)
+		for {
+			n := traf.Generate(b, 16)
+			if n == 0 {
+				break
+			}
+			s1.WriteTo(b[16:n+16], a2)
+		}
+	}()
+
+	go func() {
+		// receiver
+		b := make([]byte, 1600)
+		for traf.Running() {
+			// Use ReadFrom instead of Read, to be more like
+			// how wireguard-go does it, even though we're not
+			// going to actually look at the address.
+			n, _, err := s2.ReadFrom(b)
+			if err != nil {
+				log.Fatalf("s2.Read: %v", err)
+			}
+			traf.GotPacket(b[:n], 0)
+		}
+	}()
+}
+
+// Instead of a channel, pass packets through a TCP socket.
+// TCP is a single stream, so we can amortize one syscall across
+// multiple packets. 10x amortization seems to make it go ~10x faster,
+// as expected, getting us close to the speed of the channel tests above.
+// There's also zero packet loss.
+func setupBatchTCPTest(logf logger.Logf, traf *TrafficGen) {
+	sl, err := net.Listen("tcp", ":0")
+	if err != nil {
+		log.Fatalf("listen: %v", err)
+	}
+
+	s1, err := net.Dial("tcp", sl.Addr().String())
+	if err != nil {
+		log.Fatalf("dial: %v", err)
+	}
+
+	s2, err := sl.Accept()
+	if err != nil {
+		log.Fatalf("accept: %v", err)
+	}
+
+	s1.(*net.TCPConn).SetWriteBuffer(1024 * 1024)
+	s2.(*net.TCPConn).SetReadBuffer(1024 * 1024)
+
+	ch := make(chan int)
+
+	go func() {
+		// transmitter
+
+		bs1 := bufio.NewWriterSize(s1, 1024*1024)
+
+		b := make([]byte, 1600)
+		i := 0
+		for {
+			i += 1
+			n := traf.Generate(b, 16)
+			if n == 0 {
+				break
+			}
+			if i == 1 {
+				ch <- n
+			}
+			bs1.Write(b[16 : n+16])
+
+			// TODO: this is a pretty half-baked batching
+			// function, which we'd never want to employ in
+			// a real-life program.
+			//
+			// In real life, we'd probably want to flush
+			// immediately when there are no more packets to
+			// generate, and queue up only if we fall behind.
+			//
+			// In our case however, we just want to see the
+			// technical benefits of batching 10 syscalls
+			// into 1, so a fixed ratio makes more sense.
+			if (i % 10) == 0 {
+				bs1.Flush()
+			}
+		}
+	}()
+
+	go func() {
+		// receiver
+
+		bs2 := bufio.NewReaderSize(s2, 1024*1024)
+
+		// Find out the packet size (we happen to know they're
+		// all the same size)
+		packetSize := <-ch
+
+		b := make([]byte, packetSize)
+		for traf.Running() {
+			// TODO: can't use ReadFrom() here, which is
+			// unfair compared to UDP. (ReadFrom for UDP
+			// apparently allocates memory per packet, which
+			// this test does not.)
+			n, err := io.ReadFull(bs2, b)
+			if err != nil {
+				log.Fatalf("s2.Read: %v", err)
+			}
+			traf.GotPacket(b[:n], 0)
+		}
+	}()
+}

wgengine/bench/bench_test.go

@@ -0,0 +1,108 @@
+// 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.
+
+// Create two wgengine instances and pass data through them, measuring
+// throughput, latency, and packet loss.
+package main
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"tailscale.com/types/logger"
+)
+
+func BenchmarkTrivialNoAlloc(b *testing.B) {
+	run(b, setupTrivialNoAllocTest)
+}
+func BenchmarkTrivial(b *testing.B) {
+	run(b, setupTrivialTest)
+}
+
+func BenchmarkBlockingChannel(b *testing.B) {
+	run(b, setupBlockingChannelTest)
+}
+
+func BenchmarkNonblockingChannel(b *testing.B) {
+	run(b, setupNonblockingChannelTest)
+}
+
+func BenchmarkDoubleChannel(b *testing.B) {
+	run(b, setupDoubleChannelTest)
+}
+
+func BenchmarkUDP(b *testing.B) {
+	run(b, setupUDPTest)
+}
+
+func BenchmarkBatchTCP(b *testing.B) {
+	run(b, setupBatchTCPTest)
+}
+
+func BenchmarkWireGuardTest(b *testing.B) {
+	run(b, func(logf logger.Logf, traf *TrafficGen) {
+		setupWGTest(logf, traf, Addr1, Addr2)
+	})
+}
+
+type SetupFunc func(logger.Logf, *TrafficGen)
+
+func run(b *testing.B, setup SetupFunc) {
+	sizes := []int{
+		ICMPMinSize + 8,
+		ICMPMinSize + 100,
+		ICMPMinSize + 1000,
+	}
+
+	for _, size := range sizes {
+		b.Run(fmt.Sprintf("%d", size), func(b *testing.B) {
+			runOnce(b, setup, size)
+		})
+	}
+}
+
+func runOnce(b *testing.B, setup SetupFunc, payload int) {
+	b.StopTimer()
+	b.ReportAllocs()
+
+	var logf logger.Logf = b.Logf
+	if !testing.Verbose() {
+		logf = logger.Discard
+	}
+
+	traf := NewTrafficGen(b.StartTimer)
+	setup(logf, traf)
+
+	logf("initialized. (n=%v)", b.N)
+	b.SetBytes(int64(payload))
+
+	traf.Start(Addr1.IP, Addr2.IP, payload, int64(b.N))
+
+	var cur, prev Snapshot
+	var pps int64
+	i := 0
+	for traf.Running() {
+		i += 1
+		time.Sleep(10 * time.Millisecond)
+
+		if (i % 100) == 0 {
+			prev = cur
+			cur = traf.Snap()
+			d := cur.Sub(prev)
+
+			if prev.WhenNsec != 0 {
+				logf("%v @%7d pkt/sec", d, pps)
+			}
+		}
+
+		pps = traf.Adjust()
+	}
+
+	cur = traf.Snap()
+	d := cur.Sub(prev)
+	loss := float64(d.LostPackets) / float64(d.RxPackets)
+
+	b.ReportMetric(loss*100, "%lost")
+}

wgengine/bench/trafficgen.go

@@ -0,0 +1,262 @@
+// 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 main
+
+import (
+	"encoding/binary"
+	"fmt"
+	"log"
+	"sync"
+	"time"
+
+	"inet.af/netaddr"
+	"tailscale.com/net/packet"
+	"tailscale.com/types/ipproto"
+)
+
+type Snapshot struct {
+	WhenNsec int64 // current time
+	timeAcc  int64 // accumulated time (+NSecPerTx per transmit)
+
+	LastSeqTx    int64 // last sequence number sent
+	LastSeqRx    int64 // last sequence number received
+	TotalLost    int64 // packets out-of-order or lost so far
+	TotalOOO     int64 // packets out-of-order so far
+	TotalBytesRx int64 // total bytes received so far
+}
+
+type Delta struct {
+	DurationNsec int64
+	TxPackets    int64
+	RxPackets    int64
+	LostPackets  int64
+	OOOPackets   int64
+	Bytes        int64
+}
+
+func (b Snapshot) Sub(a Snapshot) Delta {
+	return Delta{
+		DurationNsec: b.WhenNsec - a.WhenNsec,
+		TxPackets:    b.LastSeqTx - a.LastSeqTx,
+		RxPackets: (b.LastSeqRx - a.LastSeqRx) -
+			(b.TotalLost - a.TotalLost) +
+			(b.TotalOOO - a.TotalOOO),
+		LostPackets: b.TotalLost - a.TotalLost,
+		OOOPackets:  b.TotalOOO - a.TotalOOO,
+		Bytes:       b.TotalBytesRx - a.TotalBytesRx,
+	}
+}
+
+func (d Delta) String() string {
+	return fmt.Sprintf("tx=%-6d rx=%-4d (%6d = %.1f%% loss) (%d OOO) (%4.1f Mbit/s)",
+		d.TxPackets, d.RxPackets, d.LostPackets,
+		float64(d.LostPackets)*100/float64(d.TxPackets),
+		d.OOOPackets,
+		float64(d.Bytes)*8*1e9/float64(d.DurationNsec)/1e6)
+}
+
+type TrafficGen struct {
+	mu        sync.Mutex
+	cur, prev Snapshot // snapshots used for rate control
+	buf       []byte   // pre-generated packet buffer
+	done      bool     // true if the test has completed
+
+	onFirstPacket func() // function to call on first received packet
+
+	// maxPackets is the max packets to receive (not send) before
+	// ending the test. If it's zero, the test runs forever.
+	maxPackets int64
+
+	// nsPerPacket is the target average nanoseconds between packets.
+	// It's initially zero, which means transmit as fast as the
+	// caller wants to go.
+	nsPerPacket int64
+
+	// ppsHistory is the observed packets-per-second from recent
+	// samples.
+	ppsHistory [5]int64
+}
+
+// NewTrafficGen creates a new, initially locked, TrafficGen.
+// Until Start() is called, Generate() will block forever.
+func NewTrafficGen(onFirstPacket func()) *TrafficGen {
+	t := TrafficGen{
+		onFirstPacket: onFirstPacket,
+	}
+
+	// initially locked, until first Start()
+	t.mu.Lock()
+
+	return &t
+}
+
+// Start starts the traffic generator. It assumes mu is already locked,
+// and unlocks it.
+func (t *TrafficGen) Start(src, dst netaddr.IP, bytesPerPacket int, maxPackets int64) {
+	h12 := packet.ICMP4Header{
+		IP4Header: packet.IP4Header{
+			IPProto: ipproto.ICMPv4,
+			IPID:    0,
+			Src:     src,
+			Dst:     dst,
+		},
+		Type: packet.ICMP4EchoRequest,
+		Code: packet.ICMP4NoCode,
+	}
+
+	// ensure there's room for ICMP header plus sequence number
+	if bytesPerPacket < ICMPMinSize+8 {
+		log.Fatalf("bytesPerPacket must be > 24+8")
+	}
+
+	t.maxPackets = maxPackets
+
+	payload := make([]byte, bytesPerPacket-ICMPMinSize)
+	t.buf = packet.Generate(h12, payload)
+
+	t.mu.Unlock()
+}
+
+func (t *TrafficGen) Snap() Snapshot {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	t.cur.WhenNsec = time.Now().UnixNano()
+	return t.cur
+}
+
+func (t *TrafficGen) Running() bool {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	return !t.done
+}
+
+// Generate produces the next packet in the sequence. It sleeps if
+// it's too soon for the next packet to be sent.
+//
+// The generated packet is placed into buf at offset ofs, for compatibility
+// with the wireguard-go conventions.
+//
+// The return value is the number of bytes generated in the packet, or 0
+// if the test has finished running.
+func (t *TrafficGen) Generate(b []byte, ofs int) int {
+	t.mu.Lock()
+
+	now := time.Now().UnixNano()
+	if t.nsPerPacket == 0 || t.cur.timeAcc == 0 {
+		t.cur.timeAcc = now - 1
+	}
+	if t.cur.timeAcc >= now {
+		// too soon
+		t.mu.Unlock()
+		time.Sleep(time.Duration(t.cur.timeAcc-now) * time.Nanosecond)
+		t.mu.Lock()
+
+		now = t.cur.timeAcc
+	}
+	if t.done {
+		t.mu.Unlock()
+		return 0
+	}
+
+	t.cur.timeAcc += t.nsPerPacket
+	t.cur.LastSeqTx += 1
+	t.cur.WhenNsec = now
+	seq := t.cur.LastSeqTx
+
+	t.mu.Unlock()
+
+	copy(b[ofs:], t.buf)
+	binary.BigEndian.PutUint64(
+		b[ofs+ICMPMinSize:ofs+ICMPMinSize+8],
+		uint64(seq))
+
+	return len(t.buf)
+}
+
+// GotPacket processes a packet that came back on the receive side.
+func (t *TrafficGen) GotPacket(b []byte, ofs int) {
+	t.mu.Lock()
+
+	s := &t.cur
+	seq := int64(binary.BigEndian.Uint64(
+		b[ofs+ICMPMinSize : ofs+ICMPMinSize+8]))
+	if seq > s.LastSeqRx {
+		if s.LastSeqRx > 0 {
+			// only count lost packets after the very first
+			// successful one.
+			s.TotalLost += seq - s.LastSeqRx - 1
+		}
+		s.LastSeqRx = seq
+	} else {
+		s.TotalOOO += 1
+	}
+
+	// +1 packet since we only start counting after the first one
+	if t.maxPackets > 0 && s.LastSeqRx >= t.maxPackets+1 {
+		t.done = true
+	}
+	s.TotalBytesRx += int64(len(b) - ofs)
+
+	f := t.onFirstPacket
+	t.onFirstPacket = nil
+
+	t.mu.Unlock()
+
+	if f != nil {
+		f()
+	}
+}
+
+// Adjust tunes the transmit rate based on the received packets.
+// The goal is to converge on the fastest transmit rate that still has
+// minimal packet loss. Returns the new target rate in packets/sec.
+//
+// We need to play this guessing game in order to balance out tx and rx
+// rates when there's a lossy network between them. Otherwise we can end
+// up using 99% of the CPU to blast out transmitted packets and leaving only
+// 1% to receive them, leading to a misleading throughput calculation.
+//
+// Call this function multiple times per second.
+func (t *TrafficGen) Adjust() (pps int64) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	d := t.cur.Sub(t.prev)
+
+	// don't adjust rate until the first full period *after* receiving
+	// the first packet. This skips any handshake time in the underlying
+	// transport.
+	if t.prev.LastSeqRx == 0 || d.DurationNsec == 0 {
+		t.prev = t.cur
+		return 0 // no estimate yet, continue at max speed
+	}
+
+	pps = int64(d.RxPackets) * 1e9 / int64(d.DurationNsec)
+
+	// We use a rate selection algorithm based loosely on TCP BBR.
+	// Basically, we set the transmit rate to be a bit higher than
+	// the best observed transmit rate in the last several time
+	// periods. This guarantees some packet loss, but should converge
+	// quickly on a rate near the sustainable maximum.
+	bestPPS := pps
+	for _, p := range t.ppsHistory {
+		if p > bestPPS {
+			bestPPS = p
+		}
+	}
+	if pps > 0 && t.prev.WhenNsec > 0 {
+		copy(t.ppsHistory[1:], t.ppsHistory[0:len(t.ppsHistory)-1])
+		t.ppsHistory[0] = pps
+	}
+	if bestPPS > 0 {
+		pps = bestPPS * 103 / 100
+		t.nsPerPacket = int64(1e9 / pps)
+	}
+	t.prev = t.cur
+
+	return pps
+}

wgengine/bench/wg.go

@@ -0,0 +1,205 @@
+// 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 main
+
+import (
+	"io"
+	"log"
+	"os"
+	"strings"
+	"sync"
+
+	"github.com/tailscale/wireguard-go/tun"
+	"inet.af/netaddr"
+
+	"tailscale.com/net/dns"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/logger"
+	"tailscale.com/types/netmap"
+	"tailscale.com/types/wgkey"
+	"tailscale.com/wgengine"
+	"tailscale.com/wgengine/filter"
+	"tailscale.com/wgengine/router"
+	"tailscale.com/wgengine/wgcfg"
+)
+
+func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
+	l1 := logger.WithPrefix(logf, "e1: ")
+	k1, err := wgcfg.NewPrivateKey()
+	if err != nil {
+		log.Fatalf("e1 NewPrivateKey: %v", err)
+	}
+	c1 := wgcfg.Config{
+		Name:       "e1",
+		PrivateKey: k1,
+		Addresses:  []netaddr.IPPrefix{a1},
+	}
+	t1 := &sourceTun{
+		logf: logger.WithPrefix(logf, "tun1: "),
+		traf: traf,
+	}
+	e1, err := wgengine.NewUserspaceEngine(l1, wgengine.Config{
+		Router:      router.NewFake(l1),
+		LinkMonitor: nil,
+		ListenPort:  0,
+		Tun:         t1,
+	})
+	if err != nil {
+		log.Fatalf("e1 init: %v", err)
+	}
+
+	l2 := logger.WithPrefix(logf, "e2: ")
+	k2, err := wgcfg.NewPrivateKey()
+	if err != nil {
+		log.Fatalf("e2 NewPrivateKey: %v", err)
+	}
+	c2 := wgcfg.Config{
+		Name:       "e2",
+		PrivateKey: k2,
+		Addresses:  []netaddr.IPPrefix{a2},
+	}
+	t2 := &sinkTun{
+		logf: logger.WithPrefix(logf, "tun2: "),
+		traf: traf,
+	}
+	e2, err := wgengine.NewUserspaceEngine(l2, wgengine.Config{
+		Router:      router.NewFake(l2),
+		LinkMonitor: nil,
+		ListenPort:  0,
+		Tun:         t2,
+	})
+	if err != nil {
+		log.Fatalf("e2 init: %v", err)
+	}
+
+	e1.SetFilter(filter.NewAllowAllForTest(l1))
+	e2.SetFilter(filter.NewAllowAllForTest(l2))
+
+	var wait sync.WaitGroup
+	wait.Add(2)
+
+	e1.SetStatusCallback(func(st *wgengine.Status, err error) {
+		if err != nil {
+			log.Fatalf("e1 status err: %v", err)
+		}
+		logf("e1 status: %v", *st)
+
+		var eps []string
+		for _, ep := range st.LocalAddrs {
+			eps = append(eps, ep.Addr.String())
+		}
+
+		n := tailcfg.Node{
+			ID:         tailcfg.NodeID(0),
+			Name:       "n1",
+			Addresses:  []netaddr.IPPrefix{a1},
+			AllowedIPs: []netaddr.IPPrefix{a1},
+			Endpoints:  eps,
+		}
+		e2.SetNetworkMap(&netmap.NetworkMap{
+			NodeKey:    tailcfg.NodeKey(k2),
+			PrivateKey: wgkey.Private(k2),
+			Peers:      []*tailcfg.Node{&n},
+		})
+
+		p := wgcfg.Peer{
+			PublicKey:  c1.PrivateKey.Public(),
+			AllowedIPs: []netaddr.IPPrefix{a1},
+			Endpoints:  strings.Join(eps, ","),
+		}
+		c2.Peers = []wgcfg.Peer{p}
+		e2.Reconfig(&c2, &router.Config{}, new(dns.Config))
+		wait.Done()
+	})
+
+	e2.SetStatusCallback(func(st *wgengine.Status, err error) {
+		if err != nil {
+			log.Fatalf("e2 status err: %v", err)
+		}
+		logf("e2 status: %v", *st)
+
+		var eps []string
+		for _, ep := range st.LocalAddrs {
+			eps = append(eps, ep.Addr.String())
+		}
+
+		n := tailcfg.Node{
+			ID:         tailcfg.NodeID(0),
+			Name:       "n2",
+			Addresses:  []netaddr.IPPrefix{a2},
+			AllowedIPs: []netaddr.IPPrefix{a2},
+			Endpoints:  eps,
+		}
+		e1.SetNetworkMap(&netmap.NetworkMap{
+			NodeKey:    tailcfg.NodeKey(k1),
+			PrivateKey: wgkey.Private(k1),
+			Peers:      []*tailcfg.Node{&n},
+		})
+
+		p := wgcfg.Peer{
+			PublicKey:  c2.PrivateKey.Public(),
+			AllowedIPs: []netaddr.IPPrefix{a2},
+			Endpoints:  strings.Join(eps, ","),
+		}
+		c1.Peers = []wgcfg.Peer{p}
+		e1.Reconfig(&c1, &router.Config{}, new(dns.Config))
+		wait.Done()
+	})
+
+	// Not using DERP in this test (for now?).
+	e1.SetDERPMap(&tailcfg.DERPMap{})
+	e2.SetDERPMap(&tailcfg.DERPMap{})
+
+	wait.Wait()
+}
+
+type sourceTun struct {
+	logf logger.Logf
+	traf *TrafficGen
+}
+
+func (t *sourceTun) Close() error           { return nil }
+func (t *sourceTun) Events() chan tun.Event { return nil }
+func (t *sourceTun) File() *os.File         { return nil }
+func (t *sourceTun) Flush() error           { return nil }
+func (t *sourceTun) MTU() (int, error)      { return 1500, nil }
+func (t *sourceTun) Name() (string, error)  { return "source", nil }
+
+func (t *sourceTun) Write(b []byte, ofs int) (int, error) {
+	// Discard all writes
+	return len(b) - ofs, nil
+}
+
+func (t *sourceTun) Read(b []byte, ofs int) (int, error) {
+	// Continually generate "input" packets
+	n := t.traf.Generate(b, ofs)
+	if n == 0 {
+		return 0, io.EOF
+	}
+	return n, nil
+}
+
+type sinkTun struct {
+	logf logger.Logf
+	traf *TrafficGen
+}
+
+func (t *sinkTun) Close() error           { return nil }
+func (t *sinkTun) Events() chan tun.Event { return nil }
+func (t *sinkTun) File() *os.File         { return nil }
+func (t *sinkTun) Flush() error           { return nil }
+func (t *sinkTun) MTU() (int, error)      { return 1500, nil }
+func (t *sinkTun) Name() (string, error)  { return "sink", nil }
+
+func (t *sinkTun) Read(b []byte, ofs int) (int, error) {
+	// Never returns
+	select {}
+}
+
+func (t *sinkTun) Write(b []byte, ofs int) (int, error) {
+	// Count packets, but discard them
+	t.traf.GotPacket(b, ofs)
+	return len(b) - ofs, nil
+}

wgengine/magicsock/magicsock.go

@@ -1584,6 +1584,9 @@ func (c *Conn) receiveIPv6(b []byte) (int, conn.Endpoint, error) {
 	for {
 		n, ipp, err := c.pconn6.ReadFromNetaddr(b)
 		if err != nil {
+			if isPermanentNetError(err) {
+				health.ReceiveIPv6.Stop()
+			}
 			return 0, nil, err
 		}
 		if ep, ok := c.receiveIP(b[:n], ipp, &c.ippEndpoint6); ok {
@@ -1597,6 +1600,9 @@ func (c *Conn) receiveIPv4(b []byte) (n int, ep conn.Endpoint, err error) {
 	for {
 		n, ipp, err := c.pconn4.ReadFromNetaddr(b)
 		if err != nil {
+			if isPermanentNetError(err) {
+				health.ReceiveIPv4.Stop()
+			}
 			return 0, nil, err
 		}
 		if ep, ok := c.receiveIP(b[:n], ipp, &c.ippEndpoint4); ok {
@@ -1657,6 +1663,7 @@ func (c *connBind) receiveDERP(b []byte) (n int, ep conn.Endpoint, err error) {
 		}
 		return n, ep, nil
 	}
+	health.ReceiveDERP.Stop()
 	return 0, nil, net.ErrClosed
 }
 
@@ -1727,6 +1734,18 @@ func (c *Conn) processDERPReadResult(dm derpReadResult, b []byte) (n int, ep con
 	return n, ep
 }
 
+// isPermanentNetError reports whether err is permanent.
+// It matches an equivalent check in wireguard-go's RoutineReceiveIncoming.
+func isPermanentNetError(err error) bool {
+	// Once this module requires Go 1.17, the comparison to net.ErrClosed can be removed.
+	// See https://github.com/golang/go/issues/45357.
+	if errors.Is(err, net.ErrClosed) {
+		return true
+	}
+	neterr, ok := err.(net.Error)
+	return ok && !neterr.Temporary()
+}
+
 // discoLogLevel controls the verbosity of discovery log messages.
 type discoLogLevel int
 
@@ -2411,8 +2430,11 @@ func (c *connBind) Open(ignoredPort uint16) ([]conn.ReceiveFunc, uint16, error)
 	}
 	c.closed = false
 	fns := []conn.ReceiveFunc{c.receiveIPv4, c.receiveDERP}
+	health.ReceiveIPv4.Open()
+	health.ReceiveDERP.Open()
 	if c.pconn6 != nil {
 		fns = append(fns, c.receiveIPv6)
+		health.ReceiveIPv6.Open()
 	}
 	// TODO: Combine receiveIPv4 and receiveIPv6 and receiveIP into a single
 	// closure that closes over a *RebindingUDPConn?
@@ -2434,12 +2456,15 @@ func (c *connBind) Close() error {
 	}
 	c.closed = true
 	// Unblock all outstanding receives.
+	health.ReceiveIPv4.Close()
 	c.pconn4.Close()
 	if c.pconn6 != nil {
+		health.ReceiveIPv6.Close()
 		c.pconn6.Close()
 	}
 	// Send an empty read result to unblock receiveDERP,
 	// which will then check connBind.Closed.
+	health.ReceiveDERP.Close()
 	c.derpRecvCh <- derpReadResult{}
 	return nil
 }

wgengine/netstack/netstack.go

@@ -393,12 +393,10 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons
 }
 
 func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
-	if debugNetstack {
-		// Kinda ugly:
-		// ForwarderRequest: &{{{{0 0}}} 0xc0001c30b0 0xc0004c3d40 {1240 6 true 826109390 0 true}
-		ns.logf("[v2] ForwarderRequest: %v", r)
-	}
 	reqDetails := r.ID()
+	if debugNetstack {
+		ns.logf("[v2] TCP ForwarderRequest: %s", stringifyTEI(reqDetails))
+	}
 	dialAddr := reqDetails.LocalAddress
 	dialNetAddr, _ := netaddr.FromStdIP(net.IP(dialAddr))
 	isTailscaleIP := tsaddr.IsTailscaleIP(dialNetAddr)
@@ -472,11 +470,14 @@ func (ns *Impl) forwardTCP(client *gonet.TCPConn, wq *waiter.Queue, dialAddr tcp
 }
 
 func (ns *Impl) acceptUDP(r *udp.ForwarderRequest) {
-	ns.logf("[v2] UDP ForwarderRequest: %v", r)
+	reqDetails := r.ID()
+	if debugNetstack {
+		ns.logf("[v2] UDP ForwarderRequest: %v", stringifyTEI(reqDetails))
+	}
 	var wq waiter.Queue
 	ep, err := r.CreateEndpoint(&wq)
 	if err != nil {
-		ns.logf("Could not create endpoint, exiting")
+		ns.logf("acceptUDP: could not create endpoint: %v", err)
 		return
 	}
 	localAddr, err := ep.GetLocalAddress()
@@ -563,3 +564,9 @@ func startPacketCopy(ctx context.Context, cancel context.CancelFunc, dst net.Pac
 		}
 	}()
 }
+
+func stringifyTEI(tei stack.TransportEndpointID) string {
+	localHostPort := net.JoinHostPort(tei.LocalAddress.String(), strconv.Itoa(int(tei.LocalPort)))
+	remoteHostPort := net.JoinHostPort(tei.RemoteAddress.String(), strconv.Itoa(int(tei.RemotePort)))
+	return fmt.Sprintf("%s -> %s", remoteHostPort, localHostPort)
+}

wgengine/pendopen.go

@@ -190,6 +190,25 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) {
 		return
 	}
 	if ps == nil {
+		onlyZeroRoute := true // whether peerForIP returned n only because its /0 route matched
+		for _, r := range n.AllowedIPs {
+			if r.Bits != 0 && r.Contains(flow.Dst.IP) {
+				onlyZeroRoute = false
+				break
+			}
+		}
+		if onlyZeroRoute {
+			// This node was returned by peerForIP because
+			// its exit node /0 route(s) matched, but this
+			// might not be the exit node that's currently
+			// selected.  Rather than log misleading
+			// errors, just don't log at all for now.
+			// TODO(bradfitz): update this code to be
+			// exit-node-aware and make peerForIP return
+			// the node of the currently selected exit
+			// node.
+			return
+		}
 		e.logf("open-conn-track: timeout opening %v; target node %v in netmap but unknown to wireguard", flow, n.Key.ShortString())
 		return
 	}

wgengine/router/ifconfig_windows.go

@@ -713,22 +713,68 @@ func getInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses, family winipcfg.Addres
 	return
 }
 
-// isSingleRouteInPrefixes reports whether r is a single-address
-// prefix that appears in pfxs.
-func isSingleRouteInPrefixes(r net.IPNet, pfxs []netaddr.IPPrefix) bool {
-	rr, ok := netaddr.FromStdIPNet(&r)
-	if !ok {
-		return false
+func getAllInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses) ([]*winipcfg.RouteData, error) {
+	routes4, err := getInterfaceRoutes(ifc, windows.AF_INET)
+	if err != nil {
+		return nil, err
 	}
-	if !rr.IsSingleIP() {
-		return false
+
+	routes6, err := getInterfaceRoutes(ifc, windows.AF_INET6)
+	if err != nil {
+		// TODO: what if v6 unavailable?
+		return nil, err
 	}
-	for _, pfx := range pfxs {
-		if pfx == rr {
-			return true
+	rd := make([]*winipcfg.RouteData, 0, len(routes4)+len(routes6))
+	for _, r := range routes4 {
+		rd = append(rd, &winipcfg.RouteData{
+			Destination: r.DestinationPrefix.IPNet(),
+			NextHop:     r.NextHop.IP(),
+			Metric:      r.Metric,
+		})
+	}
+	for _, r := range routes6 {
+		rd = append(rd, &winipcfg.RouteData{
+			Destination: r.DestinationPrefix.IPNet(),
+			NextHop:     r.NextHop.IP(),
+			Metric:      r.Metric,
+		})
+	}
+	return rd, nil
+}
+
+// filterRoutes removes routes that have been added by Windows and should not
+// be managed by us.
+func filterRoutes(routes []*winipcfg.RouteData, dontDelete []netaddr.IPPrefix) []*winipcfg.RouteData {
+	ddm := make(map[netaddr.IPPrefix]bool)
+	for _, dd := range dontDelete {
+		// See issue 1448: we don't want to touch the routes added
+		// by Windows for our interface addresses.
+		ddm[dd] = true
+	}
+	for _, r := range routes {
+		// We don't want to touch broadcast routes that Windows adds.
+		nr, ok := netaddr.FromStdIPNet(&r.Destination)
+		if !ok {
+			continue
 		}
+		if nr.IsSingleIP() {
+			continue
+		}
+		lastIP := nr.Range().To
+		ddm[netaddr.IPPrefix{
+			IP:   lastIP,
+			Bits: lastIP.BitLen(),
+		}] = true
 	}
-	return false
+	filtered := make([]*winipcfg.RouteData, 0, len(routes))
+	for _, r := range routes {
+		rr, ok := netaddr.FromStdIPNet(&r.Destination)
+		if ok && ddm[rr] {
+			continue
+		}
+		filtered = append(filtered, r)
+	}
+	return filtered
 }
 
 // syncRoutes incrementally sets multiples routes on an interface.
@@ -736,42 +782,11 @@ func isSingleRouteInPrefixes(r net.IPNet, pfxs []netaddr.IPPrefix) bool {
 // dontDelete is a list of interface address routes that the
 // synchronization logic should never delete.
 func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*winipcfg.RouteData, dontDelete []netaddr.IPPrefix) error {
-	routes4, err := getInterfaceRoutes(ifc, windows.AF_INET)
+	existingRoutes, err := getAllInterfaceRoutes(ifc)
 	if err != nil {
 		return err
 	}
-
-	routes6, err := getInterfaceRoutes(ifc, windows.AF_INET6)
-	if err != nil {
-		// TODO: what if v6 unavailable?
-		return err
-	}
-
-	got := make([]*winipcfg.RouteData, 0, len(routes4))
-	for _, r := range routes4 {
-		if isSingleRouteInPrefixes(r.DestinationPrefix.IPNet(), dontDelete) {
-			// See issue 1448: we don't want to touch the routes added
-			// by Windows for our interface addresses.
-			continue
-		}
-		got = append(got, &winipcfg.RouteData{
-			Destination: r.DestinationPrefix.IPNet(),
-			NextHop:     r.NextHop.IP(),
-			Metric:      r.Metric,
-		})
-	}
-	for _, r := range routes6 {
-		if isSingleRouteInPrefixes(r.DestinationPrefix.IPNet(), dontDelete) {
-			// See issue 1448: we don't want to touch the routes added
-			// by Windows for our interface addresses.
-			continue
-		}
-		got = append(got, &winipcfg.RouteData{
-			Destination: r.DestinationPrefix.IPNet(),
-			NextHop:     r.NextHop.IP(),
-			Metric:      r.Metric,
-		})
-	}
+	got := filterRoutes(existingRoutes, dontDelete)
 
 	add, del := deltaRouteData(got, want)
 
@@ -783,6 +798,7 @@ func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*winipcfg.RouteData, do
 			if dstStr == "169.254.255.255/32" {
 				// Issue 785. Ignore these routes
 				// failing to delete. Harmless.
+				// TODO(maisem): do we still need this?
 				continue
 			}
 			errs = append(errs, fmt.Errorf("deleting route %v: %w", dstStr, err))

wgengine/router/ifconfig_windows_test.go

@@ -193,6 +193,14 @@ func TestDeltaNets(t *testing.T) {
 	}
 }
 
+func formatRouteData(rds []*winipcfg.RouteData) string {
+	var b strings.Builder
+	for _, rd := range rds {
+		b.WriteString(fmt.Sprintf("%+v", rd))
+	}
+	return b.String()
+}
+
 func equalRouteDatas(a, b []*winipcfg.RouteData) bool {
 	if len(a) != len(b) {
 		return false
@@ -205,6 +213,43 @@ func equalRouteDatas(a, b []*winipcfg.RouteData) bool {
 	return true
 }
 
+func TestFilterRoutes(t *testing.T) {
+	var h0 net.IP
+	in := []*winipcfg.RouteData{
+		// LinkLocal and Loopback routes.
+		{*ipnet4("169.254.0.0", 16), h0, 1},
+		{*ipnet4("169.254.255.255", 32), h0, 1},
+		{*ipnet4("127.0.0.0", 8), h0, 1},
+		{*ipnet4("127.255.255.255", 32), h0, 1},
+		// Local LAN routes.
+		{*ipnet4("192.168.0.0", 24), h0, 1},
+		{*ipnet4("192.168.0.255", 32), h0, 1},
+		{*ipnet4("192.168.1.0", 25), h0, 1},
+		{*ipnet4("192.168.1.127", 32), h0, 1},
+		// Some random other route.
+		{*ipnet4("192.168.2.23", 32), h0, 1},
+		// Our own tailscale address.
+		{*ipnet4("100.100.100.100", 32), h0, 1},
+		// Other tailscale addresses.
+		{*ipnet4("100.100.100.101", 32), h0, 1},
+		{*ipnet4("100.100.100.102", 32), h0, 1},
+	}
+	want := []*winipcfg.RouteData{
+		{*ipnet4("169.254.0.0", 16), h0, 1},
+		{*ipnet4("127.0.0.0", 8), h0, 1},
+		{*ipnet4("192.168.0.0", 24), h0, 1},
+		{*ipnet4("192.168.1.0", 25), h0, 1},
+		{*ipnet4("192.168.2.23", 32), h0, 1},
+		{*ipnet4("100.100.100.101", 32), h0, 1},
+		{*ipnet4("100.100.100.102", 32), h0, 1},
+	}
+
+	got := filterRoutes(in, mustCIDRs("100.100.100.100/32"))
+	if !equalRouteDatas(got, want) {
+		t.Errorf("\ngot:  %v\n  want: %v\n", formatRouteData(got), formatRouteData(want))
+	}
+}
+
 func TestDeltaRouteData(t *testing.T) {
 	var h0 net.IP
 	h1 := net.ParseIP("99.99.99.99")
@@ -232,9 +277,9 @@ func TestDeltaRouteData(t *testing.T) {
 	}
 
 	if !equalRouteDatas(add, wantAdd) {
-		t.Errorf("add:\n   got: %v\n  want: %v\n", add, wantAdd)
+		t.Errorf("add:\n   got: %v\n  want: %v\n", formatRouteData(add), formatRouteData(wantAdd))
 	}
 	if !equalRouteDatas(del, wantDel) {
-		t.Errorf("del:\n   got: %v\n  want: %v\n", del, wantDel)
+		t.Errorf("del:\n   got: %v\n  want: %v\n", formatRouteData(del), formatRouteData(wantDel))
 	}
 }

wgengine/router/router_linux_test.go

@@ -20,22 +20,6 @@ import (
 	"inet.af/netaddr"
 )
 
-func mustCIDR(s string) netaddr.IPPrefix {
-	pfx, err := netaddr.ParseIPPrefix(s)
-	if err != nil {
-		panic(err)
-	}
-	return pfx
-}
-
-func mustCIDRs(ss ...string) []netaddr.IPPrefix {
-	var ret []netaddr.IPPrefix
-	for _, s := range ss {
-		ret = append(ret, mustCIDR(s))
-	}
-	return ret
-}
-
 func TestRouterStates(t *testing.T) {
 	basic := `
 ip rule add -4 pref 5210 fwmark 0x80000 table main

wgengine/router/router_test.go

@@ -0,0 +1,15 @@
+// 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 router
+
+import "inet.af/netaddr"
+
+func mustCIDRs(ss ...string) []netaddr.IPPrefix {
+	var ret []netaddr.IPPrefix
+	for _, s := range ss {
+		ret = append(ret, netaddr.MustParseIPPrefix(s))
+	}
+	return ret
+}

wgengine/wgcfg/nmcfg/nmcfg.go

@@ -6,6 +6,7 @@
 package nmcfg
 
 import (
+	"bytes"
 	"fmt"
 	"net"
 	"strconv"
@@ -60,6 +61,11 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 		Peers:      make([]wgcfg.Peer, 0, len(nm.Peers)),
 	}
 
+	// Logging buffers
+	skippedUnselected := new(bytes.Buffer)
+	skippedIPs := new(bytes.Buffer)
+	skippedSubnets := new(bytes.Buffer)
+
 	for _, peer := range nm.Peers {
 		if controlclient.Debug.OnlyDisco && peer.DiscoKey.IsZero() {
 			continue
@@ -92,16 +98,23 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 					continue
 				}
 				didExitNodeWarn = true
-				logf("[v1] wgcfg: skipping unselected default route from %q (%v)", nodeDebugName(peer), peer.Key.ShortString())
+				if skippedUnselected.Len() > 0 {
+					skippedUnselected.WriteString(", ")
+				}
+				fmt.Fprintf(skippedUnselected, "%q (%v)", nodeDebugName(peer), peer.Key.ShortString())
 				continue
 			} else if allowedIP.IsSingleIP() && tsaddr.IsTailscaleIP(allowedIP.IP) && (flags&netmap.AllowSingleHosts) == 0 {
-				logf("[v1] wgcfg: skipping node IP %v from %q (%v)",
-					allowedIP.IP, nodeDebugName(peer), peer.Key.ShortString())
+				if skippedIPs.Len() > 0 {
+					skippedIPs.WriteString(", ")
+				}
+				fmt.Fprintf(skippedIPs, "%v from %q (%v)", allowedIP.IP, nodeDebugName(peer), peer.Key.ShortString())
 				continue
 			} else if cidrIsSubnet(peer, allowedIP) {
 				if (flags & netmap.AllowSubnetRoutes) == 0 {
-					logf("[v1] wgcfg: not accepting subnet route %v from %q (%v)",
-						allowedIP, nodeDebugName(peer), peer.Key.ShortString())
+					if skippedSubnets.Len() > 0 {
+						skippedSubnets.WriteString(", ")
+					}
+					fmt.Fprintf(skippedSubnets, "%v from %q (%v)", allowedIP, nodeDebugName(peer), peer.Key.ShortString())
 					continue
 				}
 			}
@@ -109,6 +122,16 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 		}
 	}
 
+	if skippedUnselected.Len() > 0 {
+		logf("[v1] wgcfg: skipped unselected default routes from: %s", skippedUnselected.Bytes())
+	}
+	if skippedIPs.Len() > 0 {
+		logf("[v1] wgcfg: skipped node IPs: %s", skippedIPs)
+	}
+	if skippedSubnets.Len() > 0 {
+		logf("[v1] wgcfg: did not accept subnet routes: %s", skippedSubnets)
+	}
+
 	return cfg, nil
 }
 

wgengine/wglog/wglog.go

@@ -31,7 +31,7 @@ func NewLogger(logf logger.Logf) *Logger {
 
 	wrapper := func(format string, args ...interface{}) {
 		msg := fmt.Sprintf(format, args...)
-		if strings.Contains(msg, "Routine:") {
+		if strings.Contains(msg, "Routine:") && !strings.Contains(msg, "receive incoming") {
 			// wireguard-go logs as it starts and stops routines.
 			// Drop those; there are a lot of them, and they're just noise.
 			return