scripts/installer: support all distros and oses

As well function entirely headlessly. This was used to test the work
done in #1922. The only thing it fails on is Debian bullseye, which will
need more work due to it being prerelase. More work is required there
but that is out of scope for now.

Signed-off-by: Christine Dodrill <me@christine.website>

.github/ISSUE_TEMPLATE/bug_report.md

@@ -2,36 +2,7 @@
 name: Bug report
 about: Create a bug report
 title: ''
-labels: ''
+labels: 'needs-triage'
 assignees: ''
 
 ---
-
-<!-- Please note, this template is for definite bugs, not requests for
-support. If you need help with Tailscale, please email
-support@tailscale.com. We don't provide support via Github issues. -->
-
-**Describe the bug**
-A clear and concise description of what the bug is.
-
-**To Reproduce**
-Steps to reproduce the behavior:
-1. Go to '...'
-2. Click on '....'
-3. Scroll down to '....'
-4. See error
-
-**Expected behavior**
-A clear and concise description of what you expected to happen.
-
-**Screenshots**
-If applicable, add screenshots to help explain your problem.
-
-**Version information:**
- - Device: [e.g. iPhone X, laptop]
- - OS: [e.g. Windows, MacOS]
- - OS version: [e.g. Windows 10, Ubuntu 18.04]
- - Tailscale version: [e.g. 0.95-0]
-
-**Additional context**
-Add any other context about the problem here.

.github/ISSUE_TEMPLATE/feature_request.md

@@ -2,25 +2,6 @@
 name: Feature request
 about: Suggest an idea for this project
 title: ''
-labels: ''
+labels: 'needs-triage'
 assignees: ''
-
 ---
-
-**Is your feature request related to a problem? Please describe.**
-
-A clear and concise description of what the problem is. Ex. I'm always
-frustrated when [...]
-
-**Describe the solution you'd like**
-
-A clear and concise description of what you want to happen.
-
-**Describe alternatives you've considered**
-
-A clear and concise description of any alternative solutions or
-features you've considered.
-
-**Additional context**
-
-Add any other context or screenshots about the feature request here.

VERSION.txt

@@ -1 +1 @@
-1.7.0
+1.9.0

cmd/tailscale/cli/cli_test.go

@@ -16,126 +16,91 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
-	"tailscale.com/types/logger"
 	"tailscale.com/types/preftype"
 )
 
+// geese is a collection of gooses. It need not be complete.
+// But it should include anything handled specially (e.g. linux, windows)
+// and at least one thing that's not (darwin, freebsd).
+var geese = []string{"linux", "darwin", "windows", "freebsd"}
+
 // Test that checkForAccidentalSettingReverts's updateMaskedPrefsFromUpFlag can handle
 // all flags. This will panic if a new flag creeps in that's unhandled.
+//
+// Also, issue 1880: advertise-exit-node was being ignored. Verify that all flags cause an edit.
 func TestUpdateMaskedPrefsFromUpFlag(t *testing.T) {
-	mp := new(ipn.MaskedPrefs)
-	upFlagSet.VisitAll(func(f *flag.Flag) {
-		updateMaskedPrefsFromUpFlag(mp, f.Name)
-	})
+	for _, goos := range geese {
+		var upArgs upArgsT
+		fs := newUpFlagSet(goos, &upArgs)
+		fs.VisitAll(func(f *flag.Flag) {
+			mp := new(ipn.MaskedPrefs)
+			updateMaskedPrefsFromUpFlag(mp, f.Name)
+			got := mp.Pretty()
+			wantEmpty := preflessFlag(f.Name)
+			isEmpty := got == "MaskedPrefs{}"
+			if isEmpty != wantEmpty {
+				t.Errorf("flag %q created MaskedPrefs %s; want empty=%v", f.Name, got, wantEmpty)
+			}
+		})
+	}
 }
 
 func TestCheckForAccidentalSettingReverts(t *testing.T) {
-	f := func(flags ...string) map[string]bool {
-		m := make(map[string]bool)
-		for _, f := range flags {
-			m[f] = true
-		}
-		return m
-	}
 	tests := []struct {
 		name     string
-		flagSet  map[string]bool
+		flags    []string // argv to be parsed by FlagSet
 		curPrefs *ipn.Prefs
-		curUser  string // os.Getenv("USER") on the client side
-		goos     string // empty means "linux"
-		mp       *ipn.MaskedPrefs
-		want     string
+
+		curExitNodeIP netaddr.IP
+		curUser       string // os.Getenv("USER") on the client side
+		goos          string // empty means "linux"
+
+		want string
 	}{
 		{
-			name:    "bare_up_means_up",
-			flagSet: f(),
+			name:  "bare_up_means_up",
+			flags: []string{},
 			curPrefs: &ipn.Prefs{
 				ControlURL:  ipn.DefaultControlURL,
 				WantRunning: false,
 				Hostname:    "foo",
 			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					WantRunning: true,
-				},
-				WantRunningSet: true,
-			},
 			want: "",
 		},
 		{
-			name:    "losing_hostname",
-			flagSet: f("accept-dns"),
+			name:  "losing_hostname",
+			flags: []string{"--accept-dns"},
 			curPrefs: &ipn.Prefs{
-				ControlURL:  ipn.DefaultControlURL,
-				WantRunning: false,
-				Hostname:    "foo",
-				CorpDNS:     true,
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:  ipn.DefaultControlURL,
-					WantRunning: true,
-					CorpDNS:     true,
-				},
-				ControlURLSet:  true,
-				WantRunningSet: true,
-				CorpDNSSet:     true,
+				ControlURL:       ipn.DefaultControlURL,
+				WantRunning:      false,
+				Hostname:         "foo",
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+				AllowSingleHosts: true,
 			},
 			want: accidentalUpPrefix + " --accept-dns --hostname=foo",
 		},
 		{
-			name:    "hostname_changing_explicitly",
-			flagSet: f("hostname"),
+			name:  "hostname_changing_explicitly",
+			flags: []string{"--hostname=bar"},
 			curPrefs: &ipn.Prefs{
-				ControlURL:  ipn.DefaultControlURL,
-				WantRunning: false,
-				Hostname:    "foo",
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:  ipn.DefaultControlURL,
-					WantRunning: true,
-					Hostname:    "bar",
-				},
-				ControlURLSet:  true,
-				WantRunningSet: true,
-				HostnameSet:    true,
+				ControlURL:       ipn.DefaultControlURL,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+				AllowSingleHosts: true,
+				Hostname:         "foo",
 			},
 			want: "",
 		},
 		{
-			name:    "hostname_changing_empty_explicitly",
-			flagSet: f("hostname"),
+			name:  "hostname_changing_empty_explicitly",
+			flags: []string{"--hostname="},
 			curPrefs: &ipn.Prefs{
-				ControlURL:  ipn.DefaultControlURL,
-				WantRunning: false,
-				Hostname:    "foo",
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:  ipn.DefaultControlURL,
-					WantRunning: true,
-					Hostname:    "",
-				},
-				ControlURLSet:  true,
-				WantRunningSet: true,
-				HostnameSet:    true,
-			},
-			want: "",
-		},
-		{
-			name:    "empty_slice_equals_nil_slice",
-			flagSet: f("hostname"),
-			curPrefs: &ipn.Prefs{
-				ControlURL:      ipn.DefaultControlURL,
-				AdvertiseRoutes: []netaddr.IPPrefix{},
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:      ipn.DefaultControlURL,
-					AdvertiseRoutes: nil,
-				},
-				ControlURLSet: true,
+				ControlURL:       ipn.DefaultControlURL,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+				AllowSingleHosts: true,
+				Hostname:         "foo",
 			},
 			want: "",
 		},
@@ -143,228 +108,174 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			// Issue 1725: "tailscale up --authkey=..." (or other non-empty flags) works from
 			// a fresh server's initial prefs.
 			name:     "up_with_default_prefs",
-			flagSet:  f("authkey"),
+			flags:    []string{"--authkey=foosdlkfjskdljf"},
 			curPrefs: ipn.NewPrefs(),
-			mp: &ipn.MaskedPrefs{
-				Prefs:          *defaultPrefsFromUpArgs(t, "linux"),
-				WantRunningSet: true,
-			},
-			want: "",
+			want:     "",
 		},
 		{
-			name:    "implicit_operator_change",
-			flagSet: f("hostname"),
+			name:  "implicit_operator_change",
+			flags: []string{"--hostname=foo"},
 			curPrefs: &ipn.Prefs{
-				ControlURL:   ipn.DefaultControlURL,
-				OperatorUser: "alice",
+				ControlURL:       ipn.DefaultControlURL,
+				OperatorUser:     "alice",
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
 			},
 			curUser: "eve",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-				},
-				ControlURLSet: true,
-			},
-			want: accidentalUpPrefix + " --hostname= --operator=alice",
+			want:    accidentalUpPrefix + " --hostname=foo --operator=alice",
 		},
 		{
-			name:    "implicit_operator_matches_shell_user",
-			flagSet: f("hostname"),
+			name:  "implicit_operator_matches_shell_user",
+			flags: []string{"--hostname=foo"},
 			curPrefs: &ipn.Prefs{
-				ControlURL:   ipn.DefaultControlURL,
-				OperatorUser: "alice",
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+				OperatorUser:     "alice",
 			},
 			curUser: "alice",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-				},
-				ControlURLSet: true,
-			},
-			want: "",
+			want:    "",
 		},
 		{
-			name:    "error_advertised_routes_exit_node_removed",
-			flagSet: f("advertise-routes"),
+			name:  "error_advertised_routes_exit_node_removed",
+			flags: []string{"--advertise-routes=10.0.42.0/24"},
 			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
 				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"),
+			name:  "advertised_routes_exit_node_removed_explicit",
+			flags: []string{"--advertise-routes=10.0.42.0/24", "--advertise-exit-node=false"},
 			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
 				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"),
+			name:  "advertised_routes_includes_the_0_routes", // but no --advertise-exit-node
+			flags: []string{"--advertise-routes=11.1.43.0/24,0.0.0.0/0,::/0"},
 			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
 				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:  "advertise_exit_node", // Issue 1859
+			flags: []string{"--advertise-exit-node"},
+			curPrefs: &ipn.Prefs{
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
 			},
 			want: "",
 		},
 		{
-			name:    "advertised_routes_includes_only_one_0_route", // and no --advertise-exit-node
-			flagSet: f("advertise-routes"),
+			name:  "advertise_exit_node_over_existing_routes",
+			flags: []string{"--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("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:    "advertise_exit_node", // Issue 1859
-			flagSet: f("advertise-exit-node"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("0.0.0.0/0"),
-						netaddr.MustParseIPPrefix("::/0"),
-					},
-				},
-				// Note: without setting "AdvertiseRoutesSet", as
-				// updateMaskedPrefsFromUpFlag doesn't set that.
-			},
-			want: "",
-		},
-		{
-			name:    "advertise_exit_node_over_existing_routes",
-			flagSet: f("advertise-exit-node"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+
 				AdvertiseRoutes: []netaddr.IPPrefix{
 					netaddr.MustParseIPPrefix("1.2.0.0/16"),
 				},
 			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("0.0.0.0/0"),
-						netaddr.MustParseIPPrefix("::/0"),
-					},
-				},
-				// Note: without setting "AdvertiseRoutesSet", as
-				// updateMaskedPrefsFromUpFlag doesn't set that.
-			},
 			want: accidentalUpPrefix + " --advertise-exit-node --advertise-routes=1.2.0.0/16",
 		},
 		{
-			name:    "advertise_exit_node_over_existing_routes_and_exit_node",
-			flagSet: f("advertise-exit-node"),
+			name:  "advertise_exit_node_over_existing_routes_and_exit_node",
+			flags: []string{"--advertise-exit-node"},
 			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
 				AdvertiseRoutes: []netaddr.IPPrefix{
 					netaddr.MustParseIPPrefix("0.0.0.0/0"),
 					netaddr.MustParseIPPrefix("::/0"),
 					netaddr.MustParseIPPrefix("1.2.0.0/16"),
 				},
 			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("0.0.0.0/0"),
-						netaddr.MustParseIPPrefix("::/0"),
-					},
-				},
-				// Note: without setting "AdvertiseRoutesSet", as
-				// updateMaskedPrefsFromUpFlag doesn't set that.
-			},
 			want: accidentalUpPrefix + " --advertise-exit-node --advertise-routes=1.2.0.0/16",
 		},
 		{
-			name:    "exit_node_clearing", // Issue 1777
-			flagSet: f("exit-node"),
+			name:  "exit_node_clearing", // Issue 1777
+			flags: []string{"--exit-node="},
 			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+
 				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"),
+			name:  "remove_all_implicit",
+			flags: []string{"--force-reauth"},
 			curPrefs: &ipn.Prefs{
 				WantRunning:      true,
 				ControlURL:       ipn.DefaultControlURL,
 				RouteAll:         true,
 				AllowSingleHosts: false,
 				ExitNodeIP:       netaddr.MustParseIP("100.64.5.6"),
-				CorpDNS:          true,
+				CorpDNS:          false,
+				ShieldsUp:        true,
+				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
+				Hostname:         "myhostname",
+				ForceDaemon:      true,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("10.0.0.0/16"),
+					netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					netaddr.MustParseIPPrefix("::/0"),
+				},
+				NetfilterMode: preftype.NetfilterNoDivert,
+				OperatorUser:  "alice",
+			},
+			curUser: "eve",
+			want:    accidentalUpPrefix + " --force-reauth --accept-dns=false --accept-routes --advertise-exit-node --advertise-routes=10.0.0.0/16 --advertise-tags=tag:foo,tag:bar --exit-node=100.64.5.6 --host-routes=false --hostname=myhostname --netfilter-mode=nodivert --operator=alice --shields-up",
+		},
+		{
+			name:  "remove_all_implicit_except_hostname",
+			flags: []string{"--hostname=newhostname"},
+			curPrefs: &ipn.Prefs{
+				WantRunning:      true,
+				ControlURL:       ipn.DefaultControlURL,
+				RouteAll:         true,
+				AllowSingleHosts: false,
+				ExitNodeIP:       netaddr.MustParseIP("100.64.5.6"),
+				CorpDNS:          false,
 				ShieldsUp:        true,
 				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
 				Hostname:         "myhostname",
@@ -376,102 +287,121 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 				OperatorUser:  "alice",
 			},
 			curUser: "eve",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:  ipn.DefaultControlURL,
-					WantRunning: true,
-				},
-			},
-			want: accidentalUpPrefix + " --force-reauth --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",
+			want:    accidentalUpPrefix + " --hostname=newhostname --accept-dns=false --accept-routes --advertise-routes=10.0.0.0/16 --advertise-tags=tag:foo,tag:bar --exit-node=100.64.5.6 --host-routes=false --netfilter-mode=nodivert --operator=alice --shields-up",
 		},
 		{
-			name:    "remove_all_implicit_except_hostname",
-			flagSet: f("hostname"),
+			name:  "loggedout_is_implicit",
+			flags: []string{"--hostname=foo"},
 			curPrefs: &ipn.Prefs{
-				WantRunning:      true,
 				ControlURL:       ipn.DefaultControlURL,
-				RouteAll:         true,
-				AllowSingleHosts: false,
-				ExitNodeIP:       netaddr.MustParseIP("100.64.5.6"),
+				LoggedOut:        true,
+				AllowSingleHosts: true,
 				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",
+				NetfilterMode:    preftype.NetfilterOn,
 			},
-			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",
-		},
-		{
-			name:    "loggedout_is_implicit",
-			flagSet: f("advertise-exit-node"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-				LoggedOut:  true,
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("0.0.0.0/0"),
-					},
-				},
-				AdvertiseRoutesSet: true,
-			},
-			// not an error. LoggedOut is implicit.
-			want: "",
+			want: "", // not an error. LoggedOut is implicit.
 		},
 		{
 			// Test that a pre-1.8 version of Tailscale with bogus NoSNAT pref
 			// values is able to enable exit nodes without warnings.
-			name:    "make_windows_exit_node",
-			flagSet: f("advertise-exit-node"),
+			name:  "make_windows_exit_node",
+			flags: []string{"--advertise-exit-node"},
 			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-				NoSNAT:     true, // assume this no-op accidental pre-1.8 value
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+
+				// And assume this no-op accidental pre-1.8 value:
+				NoSNAT: true,
 			},
 			goos: "windows",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("192.168.0.0/16"),
-					},
-				},
-				AdvertiseRoutesSet: true,
-			},
 			want: "", // not an error
 		},
 		{
-			name:    "ignore_netfilter_change_non_linux",
-			flagSet: f("accept-dns"),
+			name:  "ignore_netfilter_change_non_linux",
+			flags: []string{"--accept-dns"},
 			curPrefs: &ipn.Prefs{
-				ControlURL:    ipn.DefaultControlURL,
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+
 				NetfilterMode: preftype.NetfilterNoDivert, // we never had this bug, but pretend it got set non-zero on Windows somehow
 			},
 			goos: "windows",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					CorpDNS:    false,
-				},
-				CorpDNSSet: true,
-			},
 			want: "", // not an error
 		},
+		{
+			name:  "operator_losing_routes_step1", // https://twitter.com/EXPbits/status/1390418145047887877
+			flags: []string{"--operator=expbits"},
+			curPrefs: &ipn.Prefs{
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					netaddr.MustParseIPPrefix("::/0"),
+					netaddr.MustParseIPPrefix("1.2.0.0/16"),
+				},
+			},
+			want: accidentalUpPrefix + " --operator=expbits --advertise-exit-node --advertise-routes=1.2.0.0/16",
+		},
+		{
+			name:  "operator_losing_routes_step2", // https://twitter.com/EXPbits/status/1390418145047887877
+			flags: []string{"--operator=expbits", "--advertise-routes=1.2.0.0/16"},
+			curPrefs: &ipn.Prefs{
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+				AdvertiseRoutes: []netaddr.IPPrefix{
+					netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					netaddr.MustParseIPPrefix("::/0"),
+					netaddr.MustParseIPPrefix("1.2.0.0/16"),
+				},
+			},
+			want: accidentalUpPrefix + " --advertise-routes=1.2.0.0/16 --operator=expbits --advertise-exit-node",
+		},
+		{
+			name:  "errors_preserve_explicit_flags",
+			flags: []string{"--reset", "--force-reauth=false", "--authkey=secretrand"},
+			curPrefs: &ipn.Prefs{
+				ControlURL:       ipn.DefaultControlURL,
+				WantRunning:      false,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+				AllowSingleHosts: true,
+
+				Hostname: "foo",
+			},
+			want: accidentalUpPrefix + " --authkey=secretrand --force-reauth=false --reset --hostname=foo",
+		},
+		{
+			name:  "error_exit_node_omit_with_ip_pref",
+			flags: []string{"--hostname=foo"},
+			curPrefs: &ipn.Prefs{
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+
+				ExitNodeIP: netaddr.MustParseIP("100.64.5.4"),
+			},
+			want: accidentalUpPrefix + " --hostname=foo --exit-node=100.64.5.4",
+		},
+		{
+			name:          "error_exit_node_omit_with_id_pref",
+			flags:         []string{"--hostname=foo"},
+			curExitNodeIP: netaddr.MustParseIP("100.64.5.7"),
+			curPrefs: &ipn.Prefs{
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
+
+				ExitNodeID: "some_stable_id",
+			},
+			want: accidentalUpPrefix + " --hostname=foo --exit-node=100.64.5.7",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -479,8 +409,19 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			if tt.goos != "" {
 				goos = tt.goos
 			}
+			var upArgs upArgsT
+			flagSet := newUpFlagSet(goos, &upArgs)
+			flagSet.Parse(tt.flags)
+			newPrefs, err := prefsFromUpArgs(upArgs, t.Logf, new(ipnstate.Status), goos)
+			if err != nil {
+				t.Fatal(err)
+			}
+			applyImplicitPrefs(newPrefs, tt.curPrefs, tt.curUser)
 			var got string
-			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp, goos, tt.curUser); err != nil {
+			if err := checkForAccidentalSettingReverts(flagSet, tt.curPrefs, newPrefs, upCheckEnv{
+				goos:          goos,
+				curExitNodeIP: tt.curExitNodeIP,
+			}); err != nil {
 				got = err.Error()
 			}
 			if strings.TrimSpace(got) != tt.want {
@@ -490,16 +431,6 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 	}
 }
 
-func defaultPrefsFromUpArgs(t testing.TB, goos string) *ipn.Prefs {
-	upArgs := upArgsFromOSArgs(goos)
-	prefs, err := prefsFromUpArgs(upArgs, logger.Discard, new(ipnstate.Status), "linux")
-	if err != nil {
-		t.Fatalf("defaultPrefsFromUpArgs: %v", err)
-	}
-	prefs.WantRunning = true
-	return prefs
-}
-
 func upArgsFromOSArgs(goos string, flagArgs ...string) (args upArgsT) {
 	fs := newUpFlagSet(goos, &args)
 	fs.Parse(flagArgs) // populates args
@@ -575,7 +506,7 @@ func TestPrefsFromUpArgs(t *testing.T) {
 			args: upArgsT{
 				exitNodeIP: "foo",
 			},
-			wantErr: `invalid IP address "foo" for --exit-node: unable to parse IP`,
+			wantErr: `invalid IP address "foo" for --exit-node: ParseIP("foo"): unable to parse IP`,
 		},
 		{
 			name: "error_exit_node_allow_lan_without_exit_node",
@@ -695,10 +626,17 @@ func TestPrefsFromUpArgs(t *testing.T) {
 }
 
 func TestPrefFlagMapping(t *testing.T) {
+	prefHasFlag := map[string]bool{}
+	for _, pv := range prefsOfFlag {
+		for _, pref := range pv {
+			prefHasFlag[pref] = true
+		}
+	}
+
 	prefType := reflect.TypeOf(ipn.Prefs{})
 	for i := 0; i < prefType.NumField(); i++ {
 		prefName := prefType.Field(i).Name
-		if _, ok := flagForPref[prefName]; ok {
+		if prefHasFlag[prefName] {
 			continue
 		}
 		switch prefName {
@@ -716,3 +654,15 @@ func TestPrefFlagMapping(t *testing.T) {
 		t.Errorf("unexpected new ipn.Pref field %q is not handled by up.go (see addPrefFlagMapping and checkForAccidentalSettingReverts)", prefName)
 	}
 }
+
+func TestFlagAppliesToOS(t *testing.T) {
+	for _, goos := range geese {
+		var upArgs upArgsT
+		fs := newUpFlagSet(goos, &upArgs)
+		fs.VisitAll(func(f *flag.Flag) {
+			if !flagAppliesToOS(f.Name, goos) {
+				t.Errorf("flagAppliesToOS(%q, %q) = false but found in %s set", f.Name, goos, goos)
+			}
+		})
+	}
+}

cmd/tailscale/cli/ping.go

@@ -139,6 +139,9 @@ func runPing(ctx context.Context, args []string) error {
 			if !anyPong {
 				return errors.New("no reply")
 			}
+			if pingArgs.untilDirect {
+				return errors.New("direct connection not established")
+			}
 			return nil
 		}
 	}

cmd/tailscale/cli/up.go

@@ -13,7 +13,6 @@ import (
 	"reflect"
 	"runtime"
 	"sort"
-	"strconv"
 	"strings"
 	"sync"
 
@@ -295,17 +294,13 @@ func runUp(ctx context.Context, args []string) error {
 		return err
 	}
 
-	flagSet := map[string]bool{}
-	mp := new(ipn.MaskedPrefs)
-	mp.WantRunningSet = true
-	mp.Prefs = *prefs
-	upFlagSet.Visit(func(f *flag.Flag) {
-		updateMaskedPrefsFromUpFlag(mp, f.Name)
-		flagSet[f.Name] = true
-	})
-
 	if !upArgs.reset {
-		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp, runtime.GOOS, os.Getenv("USER")); err != nil {
+		applyImplicitPrefs(prefs, curPrefs, os.Getenv("USER"))
+
+		if err := checkForAccidentalSettingReverts(upFlagSet, curPrefs, prefs, upCheckEnv{
+			goos:          runtime.GOOS,
+			curExitNodeIP: exitNodeIP(prefs, st),
+		}); err != nil {
 			fatalf("%s", err)
 		}
 	}
@@ -317,7 +312,7 @@ func runUp(ctx context.Context, args []string) error {
 
 	// If we're already running and none of the flags require a
 	// restart, we can just do an EditPrefs call and change the
-	// prefs at runtime (e.g. changing hostname, changinged
+	// prefs at runtime (e.g. changing hostname, changing
 	// advertised tags, routes, etc)
 	justEdit := st.BackendState == ipn.Running.String() &&
 		!upArgs.forceReauth &&
@@ -325,6 +320,13 @@ func runUp(ctx context.Context, args []string) error {
 		upArgs.authKey == "" &&
 		!controlURLChanged
 	if justEdit {
+		mp := new(ipn.MaskedPrefs)
+		mp.WantRunningSet = true
+		mp.Prefs = *prefs
+		upFlagSet.Visit(func(f *flag.Flag) {
+			updateMaskedPrefsFromUpFlag(mp, f.Name)
+		})
+
 		_, err := tailscale.EditPrefs(ctx, mp)
 		return err
 	}
@@ -332,7 +334,7 @@ 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 &&
+	simpleUp := upFlagSet.NFlag() == 0 &&
 		curPrefs.Persist != nil &&
 		curPrefs.Persist.LoginName != "" &&
 		st.BackendState != ipn.NeedsLogin.String()
@@ -464,14 +466,20 @@ func runUp(ctx context.Context, args []string) error {
 }
 
 var (
-	flagForPref = map[string]string{} // "ExitNodeIP" => "exit-node"
-	prefsOfFlag = map[string][]string{}
+	prefsOfFlag = map[string][]string{} // "exit-node" => ExitNodeIP, ExitNodeID
 )
 
 func init() {
+	// Both these have the same ipn.Pref:
+	addPrefFlagMapping("advertise-exit-node", "AdvertiseRoutes")
+	addPrefFlagMapping("advertise-routes", "AdvertiseRoutes")
+
+	// And this flag has two ipn.Prefs:
+	addPrefFlagMapping("exit-node", "ExitNodeIP", "ExitNodeID")
+
+	// The rest are 1:1:
 	addPrefFlagMapping("accept-dns", "CorpDNS")
 	addPrefFlagMapping("accept-routes", "RouteAll")
-	addPrefFlagMapping("advertise-routes", "AdvertiseRoutes")
 	addPrefFlagMapping("advertise-tags", "AdvertiseTags")
 	addPrefFlagMapping("host-routes", "AllowSingleHosts")
 	addPrefFlagMapping("hostname", "Hostname")
@@ -479,7 +487,6 @@ func init() {
 	addPrefFlagMapping("netfilter-mode", "NetfilterMode")
 	addPrefFlagMapping("shields-up", "ShieldsUp")
 	addPrefFlagMapping("snat-subnet-routes", "NoSNAT")
-	addPrefFlagMapping("exit-node", "ExitNodeIP", "ExitNodeID")
 	addPrefFlagMapping("exit-node-allow-lan-access", "ExitNodeAllowLANAccess")
 	addPrefFlagMapping("unattended", "ForceDaemon")
 	addPrefFlagMapping("operator", "OperatorUser")
@@ -489,8 +496,6 @@ func addPrefFlagMapping(flagName string, prefNames ...string) {
 	prefsOfFlag[flagName] = prefNames
 	prefType := reflect.TypeOf(ipn.Prefs{})
 	for _, pref := range prefNames {
-		flagForPref[pref] = flagName
-
 		// Crash at runtime if there's a typo in the prefName.
 		if _, ok := prefType.FieldByName(pref); !ok {
 			panic(fmt.Sprintf("invalid ipn.Prefs field %q", pref))
@@ -498,21 +503,27 @@ func addPrefFlagMapping(flagName string, prefNames ...string) {
 	}
 }
 
+// preflessFlag reports whether flagName is a flag that doesn't
+// correspond to an ipn.Pref.
+func preflessFlag(flagName string) bool {
+	switch flagName {
+	case "authkey", "force-reauth", "reset":
+		return true
+	}
+	return false
+}
+
 func updateMaskedPrefsFromUpFlag(mp *ipn.MaskedPrefs, flagName string) {
+	if preflessFlag(flagName) {
+		return
+	}
 	if prefs, ok := prefsOfFlag[flagName]; ok {
 		for _, pref := range prefs {
 			reflect.ValueOf(mp).Elem().FieldByName(pref + "Set").SetBool(true)
 		}
 		return
 	}
-	switch flagName {
-	case "authkey", "force-reauth", "reset":
-		// Not pref-related flags.
-	case "advertise-exit-node":
-		// This pref is a shorthand for advertise-routes.
-	default:
-		panic(fmt.Sprintf("internal error: unhandled flag %q", flagName))
-	}
+	panic(fmt.Sprintf("internal error: unhandled flag %q", flagName))
 }
 
 const accidentalUpPrefix = "Error: changing settings via 'tailscale up' requires mentioning all\n" +
@@ -521,9 +532,16 @@ const accidentalUpPrefix = "Error: changing settings via 'tailscale up' requires
 	"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.
+// upCheckEnv are extra parameters describing the environment as
+// needed by checkForAccidentalSettingReverts and friends.
+type upCheckEnv struct {
+	goos          string
+	curExitNodeIP netaddr.IP
+}
+
+// checkForAccidentalSettingReverts (the "up checker") checks for
+// people running "tailscale up" with a subset of the flags they
+// originally ran it with.
 //
 // For example, in Tailscale 1.6 and prior, a user might've advertised
 // a tag, but later tried to change just one other setting and forgot
@@ -535,169 +553,171 @@ const accidentalUpPrefix = "Error: changing settings via 'tailscale up' requires
 //
 // 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, goos, 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.
-		return nil
-	}
+func checkForAccidentalSettingReverts(flagSet *flag.FlagSet, curPrefs, newPrefs *ipn.Prefs, env upCheckEnv) error {
 	if curPrefs.ControlURL == "" {
 		// Don't validate things on initial "up" before a control URL has been set.
 		return nil
 	}
-	curWithExplicitEdits := curPrefs.Clone()
-	curWithExplicitEdits.ApplyEdits(mp)
 
-	prefType := reflect.TypeOf(ipn.Prefs{})
+	flagIsSet := map[string]bool{}
+	flagSet.Visit(func(f *flag.Flag) {
+		flagIsSet[f.Name] = true
+	})
 
-	// Explicit values (current + explicit edit):
-	ev := reflect.ValueOf(curWithExplicitEdits).Elem()
-	// Implicit values (what we'd get if we replaced everything with flag defaults):
-	iv := reflect.ValueOf(&mp.Prefs).Elem()
+	if len(flagIsSet) == 0 {
+		// A bare "tailscale up" is a special case to just
+		// mean bringing the network up without any changes.
+		return nil
+	}
+
+	// flagsCur is what flags we'd need to use to keep the exact
+	// settings as-is.
+	flagsCur := prefsToFlags(env, curPrefs)
+	flagsNew := prefsToFlags(env, newPrefs)
 
 	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
-		// Persist is a legacy field used for storing keys, which
-		// probably should never have been part of Prefs. It's
-		// likely to migrate elsewhere eventually.
-		if prefName == "Persist" {
+	for flagName := range flagsCur {
+		valCur, valNew := flagsCur[flagName], flagsNew[flagName]
+		if flagIsSet[flagName] {
 			continue
 		}
-		// LoggedOut is a preference, but running the "up" command
-		// always implies that the user now prefers LoggedOut->false.
-		if prefName == "LoggedOut" {
+		if reflect.DeepEqual(valCur, valNew) {
 			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
-		}
-
-		if prefName == "AdvertiseRoutes" &&
-			(len(curPrefs.AdvertiseRoutes) == 0 ||
-				hasExitNodeRoutes(curPrefs.AdvertiseRoutes) && len(curPrefs.AdvertiseRoutes) == 2) &&
-			hasExitNodeRoutes(mp.Prefs.AdvertiseRoutes) &&
-			len(mp.Prefs.AdvertiseRoutes) == 2 &&
-			flagSet["advertise-exit-node"] {
-			continue
-		}
-
-		// Get explicit value and implicit value
-		ex, im := ev.Field(i), iv.Field(i)
-		switch ex.Kind() {
-		case reflect.String, reflect.Slice:
-			if ex.Kind() == reflect.Slice && ex.Len() == 0 && im.Len() == 0 {
-				// Treat nil and non-nil empty slices as equivalent.
-				continue
-			}
-		}
-		exi, imi := ex.Interface(), im.Interface()
-
-		if reflect.DeepEqual(exi, imi) {
-			continue
-		}
-		switch flagName {
-		case "operator":
-			if imi == "" && exi == curUser {
-				// Don't require setting operator if the current user matches
-				// the configured operator.
-				continue
-			}
-		case "snat-subnet-routes", "netfilter-mode":
-			if goos != "linux" {
-				// Issue 1833: we used to accidentally set the NoSNAT
-				// pref for non-Linux nodes. It only affects Linux, so
-				// ignore it if it changes. Likewise, ignore
-				// Linux-only netfilter-mode on non-Linux.
-				continue
-			}
-		}
-		switch flagName {
-		case "":
-			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 prefName == "ExitNodeIP" {
-				missing = append(missing, fmtFlagValueArg("exit-node", fmtSettingVal(exi)))
-			}
-		case "advertise-routes":
-			routes := withoutExitNodes(exi.([]netaddr.IPPrefix))
-			missing = append(missing, fmtFlagValueArg("advertise-routes", fmtSettingVal(routes)))
-		default:
-			missing = append(missing, fmtFlagValueArg(flagName, fmtSettingVal(exi)))
-		}
+		missing = append(missing, fmtFlagValueArg(flagName, valCur))
 	}
 	if len(missing) == 0 {
 		return nil
 	}
+	sort.Strings(missing)
+
+	// Compute the stringification of the explicitly provided args in flagSet
+	// to prepend to the command to run.
+	var explicit []string
+	flagSet.Visit(func(f *flag.Flag) {
+		type isBool interface {
+			IsBoolFlag() bool
+		}
+		if ib, ok := f.Value.(isBool); ok && ib.IsBoolFlag() {
+			if f.Value.String() == "false" {
+				explicit = append(explicit, "--"+f.Name+"=false")
+			} else {
+				explicit = append(explicit, "--"+f.Name)
+			}
+		} else {
+			explicit = append(explicit, fmtFlagValueArg(f.Name, f.Value.String()))
+		}
+	})
+
 	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)))
-		} else {
-			fmt.Fprintf(&sb, " --%s", flagName)
-		}
-	}
-	for _, a := range missing {
+	for _, a := range append(explicit, 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
+// applyImplicitPrefs mutates prefs to add implicit preferences. Currently
+// this is just the operator user, which only needs to be set if it doesn't
+// match the current user.
+//
+// curUser is os.Getenv("USER"). It's pulled out for testability.
+func applyImplicitPrefs(prefs, oldPrefs *ipn.Prefs, curUser string) {
+	if prefs.OperatorUser == "" && oldPrefs.OperatorUser == curUser {
+		prefs.OperatorUser = oldPrefs.OperatorUser
+	}
+}
+
+func flagAppliesToOS(flag, goos string) bool {
+	switch flag {
+	case "netfilter-mode", "snat-subnet-routes":
+		return goos == "linux"
+	case "unattended":
+		return goos == "windows"
+	}
+	return true
+}
+
+func prefsToFlags(env upCheckEnv, prefs *ipn.Prefs) (flagVal map[string]interface{}) {
+	ret := make(map[string]interface{})
+
+	exitNodeIPStr := func() string {
+		if !prefs.ExitNodeIP.IsZero() {
+			return prefs.ExitNodeIP.String()
+		}
+		if prefs.ExitNodeID.IsZero() || env.curExitNodeIP.IsZero() {
+			return ""
+		}
+		return env.curExitNodeIP.String()
+	}
+
+	fs := newUpFlagSet(env.goos, new(upArgsT) /* dummy */)
+	fs.VisitAll(func(f *flag.Flag) {
+		if preflessFlag(f.Name) {
+			return
+		}
+		set := func(v interface{}) {
+			if flagAppliesToOS(f.Name, env.goos) {
+				ret[f.Name] = v
+			} else {
+				ret[f.Name] = nil
+			}
+		}
+		switch f.Name {
+		default:
+			panic(fmt.Sprintf("unhandled flag %q", f.Name))
+		case "login-server":
+			set(prefs.ControlURL)
+		case "accept-routes":
+			set(prefs.RouteAll)
+		case "host-routes":
+			set(prefs.AllowSingleHosts)
+		case "accept-dns":
+			set(prefs.CorpDNS)
+		case "shields-up":
+			set(prefs.ShieldsUp)
+		case "exit-node":
+			set(exitNodeIPStr())
+		case "exit-node-allow-lan-access":
+			set(prefs.ExitNodeAllowLANAccess)
+		case "advertise-tags":
+			set(strings.Join(prefs.AdvertiseTags, ","))
+		case "hostname":
+			set(prefs.Hostname)
+		case "operator":
+			set(prefs.OperatorUser)
+		case "advertise-routes":
+			var sb strings.Builder
+			for i, r := range withoutExitNodes(prefs.AdvertiseRoutes) {
+				if i > 0 {
+					sb.WriteByte(',')
+				}
+				sb.WriteString(r.String())
+			}
+			set(sb.String())
+		case "advertise-exit-node":
+			set(hasExitNodeRoutes(prefs.AdvertiseRoutes))
+		case "snat-subnet-routes":
+			set(!prefs.NoSNAT)
+		case "netfilter-mode":
+			set(prefs.NetfilterMode.String())
+		case "unattended":
+			set(prefs.ForceDaemon)
+		}
+	})
+	return ret
+}
+
+func fmtFlagValueArg(flagName string, val interface{}) string {
+	if val == true {
 		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:
-		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)
+	return fmt.Sprintf("--%s=%v", flagName, shellquote.Join(fmt.Sprint(val)))
 }
 
 func hasExitNodeRoutes(rr []netaddr.IPPrefix) bool {
@@ -729,3 +749,27 @@ func withoutExitNodes(rr []netaddr.IPPrefix) []netaddr.IPPrefix {
 	}
 	return out
 }
+
+// exitNodeIP returns the exit node IP from p, using st to map
+// it from its ID form to an IP address if needed.
+func exitNodeIP(p *ipn.Prefs, st *ipnstate.Status) (ip netaddr.IP) {
+	if p == nil {
+		return
+	}
+	if !p.ExitNodeIP.IsZero() {
+		return p.ExitNodeIP
+	}
+	id := p.ExitNodeID
+	if id.IsZero() {
+		return
+	}
+	for _, p := range st.Peer {
+		if p.ID == id {
+			if len(p.TailscaleIPs) > 0 {
+				return p.TailscaleIPs[0]
+			}
+			break
+		}
+	}
+	return
+}

cmd/tailscaled/depaware.txt

@@ -78,7 +78,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/derp/derpmap                                   from tailscale.com/cmd/tailscaled+
         tailscale.com/disco                                          from tailscale.com/derp+
         tailscale.com/health                                         from tailscale.com/control/controlclient+
-        tailscale.com/internal/deepprint                             from tailscale.com/ipn/ipnlocal+
+        tailscale.com/internal/deephash                              from tailscale.com/ipn/ipnlocal+
         tailscale.com/ipn                                            from tailscale.com/ipn/ipnserver+
         tailscale.com/ipn/ipnlocal                                   from tailscale.com/ipn/ipnserver+
         tailscale.com/ipn/ipnserver                                  from tailscale.com/cmd/tailscaled

control/controlclient/direct.go

@@ -460,10 +460,10 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 			request.NodeKey.ShortString())
 		return true, "", nil
 	}
-	if persist.Provider == "" {
+	if resp.Login.Provider != "" {
 		persist.Provider = resp.Login.Provider
 	}
-	if persist.LoginName == "" {
+	if resp.Login.LoginName != "" {
 		persist.LoginName = resp.Login.LoginName
 	}
 

go.mod

@@ -26,23 +26,24 @@ require (
 	github.com/peterbourgon/ff/v2 v2.0.0
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027
-	github.com/tailscale/wireguard-go v0.0.0-20210429195722-6cd106ab1339
+	github.com/tailscale/wireguard-go v0.0.0-20210510192616-d1aa5623121d
 	github.com/tcnksm/go-httpstat v0.2.0
 	github.com/toqueteos/webbrowser v1.2.0
 	go4.org/mem v0.0.0-20201119185036-c04c5a6ff174
 	golang.org/x/crypto v0.0.0-20210317152858-513c2a44f670
-	golang.org/x/net v0.0.0-20210226172049-e18ecbb05110
+	golang.org/x/net v0.0.0-20210510120150-4163338589ed
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
-	golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57
+	golang.org/x/sys v0.0.0-20210510120138-977fb7262007
 	golang.org/x/term v0.0.0-20210317153231-de623e64d2a6
 	golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba
-	golang.org/x/tools v0.0.0-20201211185031-d93e913c1a58
+	golang.org/x/tools v0.1.0
 	golang.zx2c4.com/wireguard/windows v0.1.2-0.20201113162609-9b85be97fdf8
 	gopkg.in/yaml.v2 v2.2.8 // indirect
 	honnef.co/go/tools v0.1.0
-	inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44
+	inet.af/netaddr v0.0.0-20210511181906-37180328850c
 	inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22
 	inet.af/peercred v0.0.0-20210302202138-56e694897155
+	inet.af/wf v0.0.0-20210424212123-eaa011a774a4
 	rsc.io/goversion v1.2.0
 )
 

go.sum

@@ -23,6 +23,7 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dvyukov/go-fuzz v0.0.0-20201127111758-49e582c6c23d/go.mod h1:11Gm+ccJnvAhCNLlf5+cS9KjtbaD5I5zaZpFMsTHWTw=
+github.com/dvyukov/go-fuzz v0.0.0-20210103155950-6a8e9d1f2415/go.mod h1:11Gm+ccJnvAhCNLlf5+cS9KjtbaD5I5zaZpFMsTHWTw=
 github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjrss6TZTdY2VfCqZPbv5k3iBFa2ZQ=
 github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
 github.com/frankban/quicktest v1.12.1 h1:P6vQcHwZYgVGIpUzKB5DXzkEeYJppJOStPLuh9aB89c=
@@ -44,7 +45,6 @@ github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M=
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
@@ -70,7 +70,6 @@ github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:C
 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=
-github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
 github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
@@ -108,6 +107,7 @@ github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3/go.mod h1:85jBQOZwp
 github.com/pelletier/go-toml v1.6.0/go.mod h1:5N711Q9dKgbdkxHL+MEfF31hpT7l0S0s/t2kKREewys=
 github.com/peterbourgon/ff/v2 v2.0.0 h1:lx0oYI5qr/FU1xnpNhQ+EZM04gKgn46jyYvGEEqBBbY=
 github.com/peterbourgon/ff/v2 v2.0.0/go.mod h1:xjwr+t+SjWm4L46fcj/D+Ap+6ME7+HqFzaP22pP5Ggk=
+github.com/peterbourgon/ff/v3 v3.0.0/go.mod h1:UILIFjRH5a/ar8TjXYLTkIvSvekZqPm5Eb/qbGk6CT0=
 github.com/pkg/diff v0.0.0-20200914180035-5b29258ca4f7 h1:+/+DxvQaYifJ+grD4klzrS5y+KJXldn/2YTl5JG+vZ8=
 github.com/pkg/diff v0.0.0-20200914180035-5b29258ca4f7/go.mod h1:zO8QMzTeZd5cpnIkz/Gn6iK0jDfGicM1nynOkkPIl28=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
@@ -127,6 +127,10 @@ github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027 h1:lK99QQdH3yBW
 github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027/go.mod h1:p9lPsd+cx33L3H9nNoecRRxPssFKUwwI50I3pZ0yT+8=
 github.com/tailscale/wireguard-go v0.0.0-20210429195722-6cd106ab1339 h1:OjLaZ57xeWJUUBAJN5KmsgjsaUABTZhcvgO/lKtZ8sQ=
 github.com/tailscale/wireguard-go v0.0.0-20210429195722-6cd106ab1339/go.mod h1:ys4yUmhKncXy1jWP34qUHKipRjl322VVhxoh1Rkfo7c=
+github.com/tailscale/wireguard-go v0.0.0-20210510175647-030c638da3df h1:ekBw6cxmDhXf9YxTmMZh7SPwUh9rnRRnaoX7HFiGobc=
+github.com/tailscale/wireguard-go v0.0.0-20210510175647-030c638da3df/go.mod h1:ys4yUmhKncXy1jWP34qUHKipRjl322VVhxoh1Rkfo7c=
+github.com/tailscale/wireguard-go v0.0.0-20210510192616-d1aa5623121d h1:qJSz1zlpuPLmfACtnj+tAH4g3iasJMBW8dpeFm5f4wg=
+github.com/tailscale/wireguard-go v0.0.0-20210510192616-d1aa5623121d/go.mod h1:ys4yUmhKncXy1jWP34qUHKipRjl322VVhxoh1Rkfo7c=
 github.com/tcnksm/go-httpstat v0.2.0 h1:rP7T5e5U2HfmOBmZzGgGZjBQ5/GluWUylujl0tJ04I0=
 github.com/tcnksm/go-httpstat v0.2.0/go.mod h1:s3JVJFtQxtBEBC9dwcdTTXS9xFnM3SXAZwPG41aurT8=
 github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9rrstGQ=
@@ -173,8 +177,9 @@ golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwY
 golang.org/x/net v0.0.0-20201216054612-986b41b23924/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20210119194325-5f4716e94777/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
-golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 h1:qWPm9rbaAMKs8Bq/9LRpbMqxWRVUAQwMI9fVrssnTfw=
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
+golang.org/x/net v0.0.0-20210510120150-4163338589ed h1:p9UgmWI9wKpfYmgaV/IZKGdXc5qEK45tDwwwDyjS26I=
+golang.org/x/net v0.0.0-20210510120150-4163338589ed/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -198,29 +203,35 @@ golang.org/x/sys v0.0.0-20201118182958-a01c418693c7/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201218084310-7d0127a74742/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210110051926-789bb1bd4061/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210123111255-9b0068b26619/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210216163648-f7da38b97c65/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210301091718-77cc2087c03b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210316164454-77fc1eacc6aa/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57 h1:F5Gozwx4I1xtr/sr/8CFbb57iKi3297KFs0QDbGN60A=
 golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
+golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210317153231-de623e64d2a6 h1:EC6+IGYTjPpRfv9a2b/6Puw0W+hLtAhkV1tPsXhutqs=
 golang.org/x/term v0.0.0-20210317153231-de623e64d2a6/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
-golang.org/x/text v0.3.4 h1:0YWbFKbhXG/wIiuHDSKpS0Iy7FSA+u45VtBMfQcFTTc=
 golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
+golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba h1:O8mE0/t419eoIwhTFpKVkHiTs/Igowgfkj25AcZrtiE=
 golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191216052735-49a3e744a425/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.0.0-20200609164405-eb789aa7ce50/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
-golang.org/x/tools v0.0.0-20201211185031-d93e913c1a58 h1:1Bs6RVeBFtLZ8Yi1Hk07DiOqzvwLD/4hln4iahvFlag=
 golang.org/x/tools v0.0.0-20201211185031-d93e913c1a58/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
+golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY=
+golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
@@ -242,11 +253,16 @@ gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 honnef.co/go/tools v0.1.0 h1:AWNL1W1i7f0wNZ8VwOKNJ0sliKvOF/adn0EHenfUh+c=
 honnef.co/go/tools v0.1.0/go.mod h1:XtegFAyX/PfluP4921rXU5IkjkqBCDnUq4W8VCIoKvM=
-inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44 h1:p7fX77zWzZMuNdJUhniBsmN1OvFOrW9SOtvgnzqUZX4=
 inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44/go.mod h1:I2i9ONCXRZDnG1+7O8fSuYzjcPxHQXrIfzD/IkR87x4=
+inet.af/netaddr v0.0.0-20210508014949-da1c2a70a83d h1:9tuJMxDV7THGfXWirKBD/v9rbsBC21bHd2eEYsYuIek=
+inet.af/netaddr v0.0.0-20210508014949-da1c2a70a83d/go.mod h1:z0nx+Dh+7N7CC8V5ayHtHGpZpxLQZZxkIaaz6HN65Ls=
+inet.af/netaddr v0.0.0-20210511181906-37180328850c h1:rzDy/tC8LjEdN94+i0Bu22tTo/qE9cvhKyfD0HMU0NU=
+inet.af/netaddr v0.0.0-20210511181906-37180328850c/go.mod h1:z0nx+Dh+7N7CC8V5ayHtHGpZpxLQZZxkIaaz6HN65Ls=
 inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22 h1:DNtszwGa6w76qlIr+PbPEnlBJdiRV8SaxeigOy0q1gg=
 inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22/go.mod h1:GVx+5OZtbG4TVOW5ilmyRZAZXr1cNwfqUEkTOtWK0PM=
 inet.af/peercred v0.0.0-20210302202138-56e694897155 h1:KojYNEYqDkZ2O3LdyTstR1l13L3ePKTIEM2h7ONkfkE=
 inet.af/peercred v0.0.0-20210302202138-56e694897155/go.mod h1:FjawnflS/udxX+SvpsMgZfdqx2aykOlkISeAsADi5IU=
+inet.af/wf v0.0.0-20210424212123-eaa011a774a4 h1:g1VVXY1xRKoO17aKY3g9KeJxDW0lGx1n2Y+WPSWkOL8=
+inet.af/wf v0.0.0-20210424212123-eaa011a774a4/go.mod h1:56/0QVlZ4NmPRh1QuU2OfrKqjSgt5P39R534gD2JMpQ=
 rsc.io/goversion v1.2.0 h1:SPn+NLTiAG7w30IRK/DKp1BjvpWabYgxlLp/+kx5J8w=
 rsc.io/goversion v1.2.0/go.mod h1:Eih9y/uIBS3ulggl7KNJ09xGSLcuNaLgmvvqa07sgfo=

internal/deephash/deephash.go

@@ -0,0 +1,174 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package deephash hashes a Go value recursively, in a predictable
+// order, without looping.
+package deephash
+
+import (
+	"bufio"
+	"crypto/sha256"
+	"fmt"
+	"reflect"
+
+	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/wgkey"
+)
+
+func Hash(v ...interface{}) string {
+	h := sha256.New()
+	// 64 matches the chunk size in crypto/sha256/sha256.go
+	b := bufio.NewWriterSize(h, 64)
+	Print(b, v)
+	b.Flush()
+	return fmt.Sprintf("%x", h.Sum(nil))
+}
+
+// UpdateHash sets last to the hash of v and reports whether its value changed.
+func UpdateHash(last *string, v ...interface{}) (changed bool) {
+	sig := Hash(v)
+	if *last != sig {
+		*last = sig
+		return true
+	}
+	return false
+}
+
+func Print(w *bufio.Writer, v ...interface{}) {
+	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
+}
+
+var (
+	netaddrIPType       = reflect.TypeOf(netaddr.IP{})
+	netaddrIPPrefix     = reflect.TypeOf(netaddr.IPPrefix{})
+	wgkeyKeyType        = reflect.TypeOf(wgkey.Key{})
+	wgkeyPrivateType    = reflect.TypeOf(wgkey.Private{})
+	tailcfgDiscoKeyType = reflect.TypeOf(tailcfg.DiscoKey{})
+)
+
+func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
+	if !v.IsValid() {
+		return
+	}
+
+	// Special case some common types.
+	if v.CanInterface() {
+		switch v.Type() {
+		case netaddrIPType:
+			var b []byte
+			var err error
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*netaddr.IP)
+				b, err = x.MarshalText()
+			} else {
+				x := v.Interface().(netaddr.IP)
+				b, err = x.MarshalText()
+			}
+			if err == nil {
+				w.Write(b)
+				return
+			}
+		case netaddrIPPrefix:
+			var b []byte
+			var err error
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*netaddr.IPPrefix)
+				b, err = x.MarshalText()
+			} else {
+				x := v.Interface().(netaddr.IPPrefix)
+				b, err = x.MarshalText()
+			}
+			if err == nil {
+				w.Write(b)
+				return
+			}
+		case wgkeyKeyType:
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*wgkey.Key)
+				w.Write(x[:])
+			} else {
+				x := v.Interface().(wgkey.Key)
+				w.Write(x[:])
+			}
+			return
+		case wgkeyPrivateType:
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*wgkey.Private)
+				w.Write(x[:])
+			} else {
+				x := v.Interface().(wgkey.Private)
+				w.Write(x[:])
+			}
+			return
+		case tailcfgDiscoKeyType:
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*tailcfg.DiscoKey)
+				w.Write(x[:])
+			} else {
+				x := v.Interface().(tailcfg.DiscoKey)
+				w.Write(x[:])
+			}
+			return
+		}
+	}
+
+	// Generic handling.
+	switch v.Kind() {
+	default:
+		panic(fmt.Sprintf("unhandled kind %v for type %v", v.Kind(), v.Type()))
+	case reflect.Ptr:
+		ptr := v.Pointer()
+		if visited[ptr] {
+			return
+		}
+		visited[ptr] = true
+		print(w, v.Elem(), visited)
+		return
+	case reflect.Struct:
+		w.WriteString("struct{\n")
+		for i, n := 0, v.NumField(); i < n; i++ {
+			fmt.Fprintf(w, " [%d]: ", i)
+			print(w, v.Field(i), visited)
+			w.WriteString("\n")
+		}
+		w.WriteString("}\n")
+	case reflect.Slice, reflect.Array:
+		if v.Type().Elem().Kind() == reflect.Uint8 && v.CanInterface() {
+			fmt.Fprintf(w, "%q", v.Interface())
+			return
+		}
+		fmt.Fprintf(w, "[%d]{\n", v.Len())
+		for i, ln := 0, v.Len(); i < ln; i++ {
+			fmt.Fprintf(w, " [%d]: ", i)
+			print(w, v.Index(i), visited)
+			w.WriteString("\n")
+		}
+		w.WriteString("}\n")
+	case reflect.Interface:
+		print(w, v.Elem(), visited)
+	case reflect.Map:
+		sm := newSortedMap(v)
+		fmt.Fprintf(w, "map[%d]{\n", len(sm.Key))
+		for i, k := range sm.Key {
+			print(w, k, visited)
+			w.WriteString(": ")
+			print(w, sm.Value[i], visited)
+			w.WriteString("\n")
+		}
+		w.WriteString("}\n")
+	case reflect.String:
+		w.WriteString(v.String())
+	case reflect.Bool:
+		fmt.Fprintf(w, "%v", v.Bool())
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		fmt.Fprintf(w, "%v", v.Int())
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+		fmt.Fprintf(w, "%v", v.Uint())
+	case reflect.Float32, reflect.Float64:
+		fmt.Fprintf(w, "%v", v.Float())
+	case reflect.Complex64, reflect.Complex128:
+		fmt.Fprintf(w, "%v", v.Complex())
+	}
+}

internal/deephash/deephash_test.go

@@ -2,13 +2,14 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package deepprint
+package deephash
 
 import (
-	"bytes"
 	"testing"
 
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
+	"tailscale.com/util/dnsname"
 	"tailscale.com/wgengine/router"
 	"tailscale.com/wgengine/wgcfg"
 )
@@ -18,10 +19,6 @@ func TestDeepPrint(t *testing.T) {
 	// Mostly we're just testing that we don't panic on handled types.
 	v := getVal()
 
-	var buf bytes.Buffer
-	Print(&buf, v)
-	t.Logf("Got: %s", buf.Bytes())
-
 	hash1 := Hash(v)
 	t.Logf("hash: %v", hash1)
 	for i := 0; i < 20; i++ {
@@ -39,7 +36,9 @@ func getVal() []interface{} {
 			Addresses: []netaddr.IPPrefix{{Bits: 5, IP: netaddr.IPFrom16([16]byte{3: 3})}},
 			Peers: []wgcfg.Peer{
 				{
-					Endpoints: "foo:5",
+					Endpoints: wgcfg.Endpoints{
+						IPPorts: wgcfg.NewIPPortSet(netaddr.MustParseIPPort("42.42.42.42:5")),
+					},
 				},
 			},
 		},
@@ -49,16 +48,25 @@ func getVal() []interface{} {
 				netaddr.MustParseIPPrefix("1234::/64"),
 			},
 		},
-		map[string]string{
-			"key1": "val1",
-			"key2": "val2",
-			"key3": "val3",
-			"key4": "val4",
-			"key5": "val5",
-			"key6": "val6",
-			"key7": "val7",
-			"key8": "val8",
-			"key9": "val9",
+		map[dnsname.FQDN][]netaddr.IP{
+			dnsname.FQDN("a."): {netaddr.MustParseIP("1.2.3.4"), netaddr.MustParseIP("4.3.2.1")},
+			dnsname.FQDN("b."): {netaddr.MustParseIP("8.8.8.8"), netaddr.MustParseIP("9.9.9.9")},
+		},
+		map[dnsname.FQDN][]netaddr.IPPort{
+			dnsname.FQDN("a."): {netaddr.MustParseIPPort("1.2.3.4:11"), netaddr.MustParseIPPort("4.3.2.1:22")},
+			dnsname.FQDN("b."): {netaddr.MustParseIPPort("8.8.8.8:11"), netaddr.MustParseIPPort("9.9.9.9:22")},
+		},
+		map[tailcfg.DiscoKey]bool{
+			{1: 1}: true,
+			{1: 2}: false,
 		},
 	}
 }
+
+func BenchmarkHash(b *testing.B) {
+	b.ReportAllocs()
+	v := getVal()
+	for i := 0; i < b.N; i++ {
+		Hash(v)
+	}
+}

internal/deephash/fmtsort.go

@@ -10,7 +10,7 @@
 
 // This is a slightly modified fork of Go's src/internal/fmtsort/sort.go
 
-package deepprint
+package deephash
 
 import (
 	"reflect"

internal/deepprint/deepprint.go

@@ -1,103 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Package deepprint walks a Go value recursively, in a predictable
-// order, without looping, and prints each value out to a given
-// Writer, which is assumed to be a hash.Hash, as this package doesn't
-// format things nicely.
-//
-// This is intended as a lighter version of go-spew, etc. We don't need its
-// features when our writer is just a hash.
-package deepprint
-
-import (
-	"crypto/sha256"
-	"fmt"
-	"io"
-	"reflect"
-)
-
-func Hash(v ...interface{}) string {
-	h := sha256.New()
-	Print(h, v)
-	return fmt.Sprintf("%x", h.Sum(nil))
-}
-
-// UpdateHash sets last to the hash of v and reports whether its value changed.
-func UpdateHash(last *string, v ...interface{}) (changed bool) {
-	sig := Hash(v)
-	if *last != sig {
-		*last = sig
-		return true
-	}
-	return false
-}
-
-func Print(w io.Writer, v ...interface{}) {
-	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
-}
-
-func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
-	if !v.IsValid() {
-		return
-	}
-	switch v.Kind() {
-	default:
-		panic(fmt.Sprintf("unhandled kind %v for type %v", v.Kind(), v.Type()))
-	case reflect.Ptr:
-		ptr := v.Pointer()
-		if visited[ptr] {
-			return
-		}
-		visited[ptr] = true
-		print(w, v.Elem(), visited)
-		return
-	case reflect.Struct:
-		fmt.Fprintf(w, "struct{\n")
-		t := v.Type()
-		for i, n := 0, v.NumField(); i < n; i++ {
-			sf := t.Field(i)
-			fmt.Fprintf(w, "%s: ", sf.Name)
-			print(w, v.Field(i), visited)
-			fmt.Fprintf(w, "\n")
-		}
-	case reflect.Slice, reflect.Array:
-		if v.Type().Elem().Kind() == reflect.Uint8 && v.CanInterface() {
-			fmt.Fprintf(w, "%q", v.Interface())
-			return
-		}
-		fmt.Fprintf(w, "[%d]{\n", v.Len())
-		for i, ln := 0, v.Len(); i < ln; i++ {
-			fmt.Fprintf(w, " [%d]: ", i)
-			print(w, v.Index(i), visited)
-			fmt.Fprintf(w, "\n")
-		}
-		fmt.Fprintf(w, "}\n")
-	case reflect.Interface:
-		print(w, v.Elem(), visited)
-	case reflect.Map:
-		sm := newSortedMap(v)
-		fmt.Fprintf(w, "map[%d]{\n", len(sm.Key))
-		for i, k := range sm.Key {
-			print(w, k, visited)
-			fmt.Fprintf(w, ": ")
-			print(w, sm.Value[i], visited)
-			fmt.Fprintf(w, "\n")
-		}
-		fmt.Fprintf(w, "}\n")
-
-	case reflect.String:
-		fmt.Fprintf(w, "%s", v.String())
-	case reflect.Bool:
-		fmt.Fprintf(w, "%v", v.Bool())
-	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-		fmt.Fprintf(w, "%v", v.Int())
-	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
-		fmt.Fprintf(w, "%v", v.Uint())
-	case reflect.Float32, reflect.Float64:
-		fmt.Fprintf(w, "%v", v.Float())
-	case reflect.Complex64, reflect.Complex128:
-		fmt.Fprintf(w, "%v", v.Complex())
-	}
-}

ipn/ipnlocal/local.go

@@ -29,7 +29,7 @@ import (
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/control/controlclient"
 	"tailscale.com/health"
-	"tailscale.com/internal/deepprint"
+	"tailscale.com/internal/deephash"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/ipn/policy"
@@ -326,6 +326,9 @@ func (b *LocalBackend) updateStatus(sb *ipnstate.StatusBuilder, extraLocked func
 		}
 	})
 	sb.MutateSelfStatus(func(ss *ipnstate.PeerStatus) {
+		if b.netMap != nil && b.netMap.SelfNode != nil {
+			ss.ID = b.netMap.SelfNode.StableID
+		}
 		for _, pln := range b.peerAPIListeners {
 			ss.PeerAPIURL = append(ss.PeerAPIURL, pln.urlStr)
 		}
@@ -365,6 +368,7 @@ func (b *LocalBackend) populatePeerStatusLocked(sb *ipnstate.StatusBuilder) {
 		}
 		sb.AddPeer(key.Public(p.Key), &ipnstate.PeerStatus{
 			InNetworkMap:       true,
+			ID:                 p.StableID,
 			UserID:             p.User,
 			TailAddrDeprecated: tailAddr4,
 			TailscaleIPs:       tailscaleIPs,
@@ -430,14 +434,15 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		}
 		return
 	}
-	if st.LoginFinished != nil {
+
+	b.mu.Lock()
+	wasBlocked := b.blocked
+	b.mu.Unlock()
+
+	if st.LoginFinished != nil && wasBlocked {
 		// Auth completed, unblock the engine
 		b.blockEngineUpdates(false)
 		b.authReconfig()
-		b.EditPrefs(&ipn.MaskedPrefs{
-			LoggedOutSet: true,
-			Prefs:        ipn.Prefs{LoggedOut: false},
-		})
 		b.send(ipn.Notify{LoginFinished: &empty.Message{}})
 	}
 
@@ -476,11 +481,15 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		b.authURL = st.URL
 		b.authURLSticky = st.URL
 	}
-	if b.state == ipn.NeedsLogin {
-		if !b.prefs.WantRunning {
+	if wasBlocked && st.LoginFinished != nil {
+		// Interactive login finished successfully (URL visited).
+		// After an interactive login, the user always wants
+		// WantRunning.
+		if !b.prefs.WantRunning || b.prefs.LoggedOut {
 			prefsChanged = true
 		}
 		b.prefs.WantRunning = true
+		b.prefs.LoggedOut = false
 	}
 	// Prefs will be written out; this is not safe unless locked or cloned.
 	if prefsChanged {
@@ -562,10 +571,18 @@ func (b *LocalBackend) findExitNodeIDLocked(nm *netmap.NetworkMap) (prefsChanged
 func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) {
 	if err != nil {
 		b.logf("wgengine status error: %v", err)
+
+		b.statusLock.Lock()
+		b.statusChanged.Broadcast()
+		b.statusLock.Unlock()
 		return
 	}
 	if s == nil {
 		b.logf("[unexpected] non-error wgengine update with status=nil: %v", s)
+
+		b.statusLock.Lock()
+		b.statusChanged.Broadcast()
+		b.statusLock.Unlock()
 		return
 	}
 
@@ -899,7 +916,7 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs)
 	localNets := localNetsB.IPSet()
 	logNets := logNetsB.IPSet()
 
-	changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp)
+	changed := deephash.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp)
 	if !changed {
 		return
 	}
@@ -2109,8 +2126,8 @@ func (b *LocalBackend) enterState(newState ipn.State) {
 	if oldState == newState {
 		return
 	}
-	b.logf("Switching ipn state %v -> %v (WantRunning=%v)",
-		oldState, newState, prefs.WantRunning)
+	b.logf("Switching ipn state %v -> %v (WantRunning=%v, nm=%v)",
+		oldState, newState, prefs.WantRunning, netMap != nil)
 	health.SetIPNState(newState.String(), prefs.WantRunning)
 	b.send(ipn.Notify{State: &newState})
 
@@ -2166,13 +2183,14 @@ func (b *LocalBackend) nextState() ipn.State {
 		cc          = b.cc
 		netMap      = b.netMap
 		state       = b.state
+		blocked     = b.blocked
 		wantRunning = b.prefs.WantRunning
 		loggedOut   = b.prefs.LoggedOut
 	)
 	b.mu.Unlock()
 
 	switch {
-	case !wantRunning && !loggedOut && b.hasNodeKey():
+	case !wantRunning && !loggedOut && !blocked && b.hasNodeKey():
 		return ipn.Stopped
 	case netMap == nil:
 		if cc.AuthCantContinue() || loggedOut {

ipn/ipnlocal/state_test.go

@@ -368,13 +368,11 @@ func TestStateMachine(t *testing.T) {
 	{
 		c.Assert(cc.getCalls(), qt.DeepEquals, []string{"Login"})
 		notifies.drain(0)
-		// BUG: this should immediately set WantRunning to true.
-		// Users don't log in if they don't want to also connect.
-		// (Generally, we're inconsistent about who is supposed to
-		// update Prefs at what time. But the overall philosophy is:
-		// update it when the user's intent changes. This is clearly
-		// at the time the user *requests* Login, not at the time
-		// the login finishes.)
+		// Note: WantRunning isn't true yet. It'll switch to true
+		// after a successful login finishes.
+		// (This behaviour is needed so that b.Login() won't
+		// start connecting to an old account right away, if one
+		// exists when you launch another login.)
 	}
 
 	// Attempted non-interactive login with no key; indicate that
@@ -384,18 +382,16 @@ func TestStateMachine(t *testing.T) {
 	url1 := "http://localhost:1/1"
 	cc.send(nil, url1, false, nil)
 	{
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		c.Assert(cc.getCalls(), qt.DeepEquals, []string{})
 
 		// ...but backend eats that notification, because the user
 		// didn't explicitly request interactive login yet, and
 		// we're already in NeedsLogin state.
 		nn := notifies.drain(1)
 
-		// Trying to log in automatically sets WantRunning.
-		// BUG: that should have happened right after Login().
 		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
 		c.Assert(nn[0].Prefs.LoggedOut, qt.IsFalse)
-		c.Assert(nn[0].Prefs.WantRunning, qt.IsTrue)
+		c.Assert(nn[0].Prefs.WantRunning, qt.IsFalse)
 	}
 
 	// Now we'll try an interactive login.
@@ -451,11 +447,12 @@ func TestStateMachine(t *testing.T) {
 	// same time.
 	// The backend should propagate this upward for the UI.
 	t.Logf("\n\nLoginFinished")
-	notifies.expect(2)
+	notifies.expect(3)
 	cc.setAuthBlocked(false)
+	cc.persist.LoginName = "user1"
 	cc.send(nil, "", true, &netmap.NetworkMap{})
 	{
-		nn := notifies.drain(2)
+		nn := notifies.drain(3)
 		// BUG: still too soon for UpdateEndpoints.
 		//
 		// Arguably it makes sense to unpause now, since the machine
@@ -468,15 +465,12 @@ func TestStateMachine(t *testing.T) {
 		// it's visible in the logs)
 		c.Assert([]string{"unpause", "UpdateEndpoints"}, qt.DeepEquals, cc.getCalls())
 		c.Assert(nn[0].LoginFinished, qt.Not(qt.IsNil))
-		c.Assert(nn[1].State, qt.Not(qt.IsNil))
-		c.Assert(ipn.NeedsMachineAuth, qt.Equals, *nn[1].State)
+		c.Assert(nn[1].Prefs, qt.Not(qt.IsNil))
+		c.Assert(nn[2].State, qt.Not(qt.IsNil))
+		c.Assert(nn[1].Prefs.Persist.LoginName, qt.Equals, "user1")
+		c.Assert(ipn.NeedsMachineAuth, qt.Equals, *nn[2].State)
 	}
 
-	// TODO: check that the logged-in username propagates from control
-	// through to the UI notifications. I think it's used as a hint
-	// for future logins, to pre-fill the username box? Not really sure
-	// how it works.
-
 	// Pretend that the administrator has authorized our machine.
 	t.Logf("\n\nMachineAuthorized")
 	notifies.expect(1)
@@ -581,77 +575,72 @@ func TestStateMachine(t *testing.T) {
 
 	// Let's make the logout succeed.
 	t.Logf("\n\nLogout (async) - succeed")
-	notifies.expect(1)
+	notifies.expect(0)
 	cc.setAuthBlocked(true)
 	cc.send(nil, "", false, nil)
 	{
-		nn := notifies.drain(1)
+		notifies.drain(0)
 		c.Assert(cc.getCalls(), qt.HasLen, 0)
-		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
-		c.Assert(nn[0].Prefs.LoggedOut, qt.IsTrue)
-		// BUG: WantRunning should be false after manual logout.
-		c.Assert(nn[0].Prefs.WantRunning, qt.IsTrue)
+		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
+		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
 
 	// A second logout should do nothing, since the prefs haven't changed.
 	t.Logf("\n\nLogout2 (async)")
-	notifies.expect(1)
+	notifies.expect(0)
 	b.Logout()
 	{
-		nn := notifies.drain(1)
+		notifies.drain(0)
 		// BUG: the backend has already called StartLogout, and we're
 		// still logged out. So it shouldn't call it again.
 		c.Assert([]string{"StartLogout"}, qt.DeepEquals, cc.getCalls())
-		// BUG: Prefs should not change here. Already logged out.
-		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
-		c.Assert(nn[0].Prefs.LoggedOut, qt.IsTrue)
-		c.Assert(nn[0].Prefs.WantRunning, qt.IsFalse)
+		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
+		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
 
 	// Let's acknowledge the second logout too.
 	t.Logf("\n\nLogout2 (async) - succeed")
-	notifies.expect(1)
+	notifies.expect(0)
 	cc.setAuthBlocked(true)
 	cc.send(nil, "", false, nil)
 	{
-		nn := notifies.drain(1)
+		notifies.drain(0)
 		c.Assert(cc.getCalls(), qt.HasLen, 0)
-		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
-		c.Assert(nn[0].Prefs.LoggedOut, qt.IsTrue)
-		// BUG: second logout shouldn't cause WantRunning->true !!
-		c.Assert(nn[0].Prefs.WantRunning, qt.IsTrue)
+		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
+		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
 
 	// Try the synchronous logout feature.
 	t.Logf("\n\nLogout3 (sync)")
-	notifies.expect(1)
+	notifies.expect(0)
 	b.LogoutSync(context.Background())
 	// NOTE: This returns as soon as cc.Logout() returns, which is okay
 	// I guess, since that's supposed to be synchronous.
 	{
-		nn := notifies.drain(1)
+		notifies.drain(0)
 		c.Assert([]string{"Logout"}, qt.DeepEquals, cc.getCalls())
-		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
-		c.Assert(nn[0].Prefs.LoggedOut, qt.IsTrue)
-		c.Assert(nn[0].Prefs.WantRunning, qt.IsFalse)
+		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
+		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
 
 	// Generate the third logout event.
 	t.Logf("\n\nLogout3 (sync) - succeed")
-	notifies.expect(1)
+	notifies.expect(0)
 	cc.setAuthBlocked(true)
 	cc.send(nil, "", false, nil)
 	{
-		nn := notifies.drain(1)
+		notifies.drain(0)
 		c.Assert(cc.getCalls(), qt.HasLen, 0)
-		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
-		c.Assert(nn[0].Prefs.LoggedOut, qt.IsTrue)
-		// BUG: third logout shouldn't cause WantRunning->true !!
-		c.Assert(nn[0].Prefs.WantRunning, qt.IsTrue)
+		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
+		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
 
@@ -669,10 +658,6 @@ func TestStateMachine(t *testing.T) {
 	// happens if the user exits and restarts while logged out.
 	// Note that it's explicitly okay to call b.Start() over and over
 	// again, every time the frontend reconnects.
-	//
-	// BUG: WantRunning is true here (because of the bug above).
-	//  We'll have to adjust the following test's expectations if we
-	//  fix that.
 
 	// TODO: test user switching between statekeys.
 
@@ -691,7 +676,7 @@ func TestStateMachine(t *testing.T) {
 		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
 		c.Assert(nn[1].State, qt.Not(qt.IsNil))
 		c.Assert(nn[0].Prefs.LoggedOut, qt.IsTrue)
-		c.Assert(nn[0].Prefs.WantRunning, qt.IsTrue)
+		c.Assert(nn[0].Prefs.WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, *nn[1].State)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
@@ -703,16 +688,20 @@ func TestStateMachine(t *testing.T) {
 	t.Logf("\n\nLoginFinished3")
 	notifies.expect(3)
 	cc.setAuthBlocked(false)
+	cc.persist.LoginName = "user2"
 	cc.send(nil, "", true, &netmap.NetworkMap{
 		MachineStatus: tailcfg.MachineAuthorized,
 	})
 	{
 		nn := notifies.drain(3)
 		c.Assert([]string{"unpause"}, qt.DeepEquals, cc.getCalls())
-		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
-		c.Assert(nn[1].LoginFinished, qt.Not(qt.IsNil))
+		c.Assert(nn[0].LoginFinished, qt.Not(qt.IsNil))
+		c.Assert(nn[1].Prefs, qt.Not(qt.IsNil))
 		c.Assert(nn[2].State, qt.Not(qt.IsNil))
-		c.Assert(nn[0].Prefs.LoggedOut, qt.IsFalse)
+		// Prefs after finishing the login, so LoginName updated.
+		c.Assert(nn[1].Prefs.Persist.LoginName, qt.Equals, "user2")
+		c.Assert(nn[1].Prefs.LoggedOut, qt.IsFalse)
+		c.Assert(nn[1].Prefs.WantRunning, qt.IsTrue)
 		c.Assert(ipn.Starting, qt.Equals, *nn[2].State)
 	}
 
@@ -773,6 +762,63 @@ func TestStateMachine(t *testing.T) {
 		c.Assert(ipn.Starting, qt.Equals, *nn[0].State)
 	}
 
+	// Disconnect.
+	t.Logf("\n\nStop")
+	notifies.expect(2)
+	b.EditPrefs(&ipn.MaskedPrefs{
+		WantRunningSet: true,
+		Prefs:          ipn.Prefs{WantRunning: false},
+	})
+	{
+		nn := notifies.drain(2)
+		c.Assert([]string{"unpause"}, qt.DeepEquals, cc.getCalls())
+		// BUG: I would expect Prefs to change first, and state after.
+		c.Assert(nn[0].State, qt.Not(qt.IsNil))
+		c.Assert(nn[1].Prefs, qt.Not(qt.IsNil))
+		c.Assert(ipn.Stopped, qt.Equals, *nn[0].State)
+	}
+
+	// We want to try logging in as a different user, while Stopped.
+	// First, start the login process (without logging out first).
+	t.Logf("\n\nLoginDifferent")
+	notifies.expect(2)
+	b.StartLoginInteractive()
+	url3 := "http://localhost:1/3"
+	cc.send(nil, url3, false, nil)
+	{
+		nn := notifies.drain(2)
+		// It might seem like WantRunning should switch to true here,
+		// but that would be risky since we already have a valid
+		// user account. It might try to reconnect to the old account
+		// before the new one is ready. So no change yet.
+		c.Assert([]string{"Login", "unpause"}, qt.DeepEquals, cc.getCalls())
+		c.Assert(nn[0].BrowseToURL, qt.Not(qt.IsNil))
+		c.Assert(nn[1].State, qt.Not(qt.IsNil))
+		c.Assert(*nn[0].BrowseToURL, qt.Equals, url3)
+		c.Assert(ipn.NeedsLogin, qt.Equals, *nn[1].State)
+	}
+
+	// Now, let's say the interactive login completed, using a different
+	// user account than before.
+	t.Logf("\n\nLoginDifferent URL visited")
+	notifies.expect(3)
+	cc.persist.LoginName = "user3"
+	cc.send(nil, "", true, &netmap.NetworkMap{
+		MachineStatus: tailcfg.MachineAuthorized,
+	})
+	{
+		nn := notifies.drain(3)
+		c.Assert([]string{"unpause"}, qt.DeepEquals, cc.getCalls())
+		c.Assert(nn[0].LoginFinished, qt.Not(qt.IsNil))
+		c.Assert(nn[1].Prefs, qt.Not(qt.IsNil))
+		c.Assert(nn[2].State, qt.Not(qt.IsNil))
+		// Prefs after finishing the login, so LoginName updated.
+		c.Assert(nn[1].Prefs.Persist.LoginName, qt.Equals, "user3")
+		c.Assert(nn[1].Prefs.LoggedOut, qt.IsFalse)
+		c.Assert(nn[1].Prefs.WantRunning, qt.IsTrue)
+		c.Assert(ipn.Starting, qt.Equals, *nn[2].State)
+	}
+
 	// The last test case is the most common one: restarting when both
 	// logged in and WantRunning.
 	t.Logf("\n\nStart5")
@@ -793,17 +839,18 @@ func TestStateMachine(t *testing.T) {
 
 	// Control server accepts our valid key from before.
 	t.Logf("\n\nLoginFinished5")
-	notifies.expect(2)
+	notifies.expect(1)
 	cc.setAuthBlocked(false)
 	cc.send(nil, "", true, &netmap.NetworkMap{
 		MachineStatus: tailcfg.MachineAuthorized,
 	})
 	{
-		nn := notifies.drain(2)
+		nn := notifies.drain(1)
 		c.Assert([]string{"unpause"}, qt.DeepEquals, cc.getCalls())
-		c.Assert(nn[0].LoginFinished, qt.Not(qt.IsNil))
-		c.Assert(nn[1].State, qt.Not(qt.IsNil))
-		c.Assert(ipn.Starting, qt.Equals, *nn[1].State)
+		// NOTE: No LoginFinished message since no interactive
+		// login was needed.
+		c.Assert(nn[0].State, qt.Not(qt.IsNil))
+		c.Assert(ipn.Starting, qt.Equals, *nn[0].State)
 		// NOTE: No prefs change this time. WantRunning stays true.
 		// We were in Starting in the first place, so that doesn't
 		// change either.

ipn/ipnserver/server.go

@@ -6,7 +6,9 @@ package ipnserver
 
 import (
 	"bufio"
+	"bytes"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -251,8 +253,7 @@ func (s *server) serveConn(ctx context.Context, c net.Conn, logf logger.Logf) {
 			return
 		}
 		defer c.Close()
-		serverToClient := func(b []byte) { ipn.WriteMsg(c, b) }
-		bs := ipn.NewBackendServer(logf, nil, serverToClient)
+		bs := ipn.NewBackendServer(logf, nil, jsonNotifier(c, s.logf))
 		_, occupied := err.(inUseOtherUserError)
 		if occupied {
 			bs.SendInUseOtherUserErrorMessage(err.Error())
@@ -567,7 +568,9 @@ func (s *server) setServerModeUserLocked() {
 	}
 }
 
-func (s *server) writeToClients(b []byte) {
+var jsonEscapedZero = []byte(`\u0000`)
+
+func (s *server) writeToClients(n ipn.Notify) {
 	inServerMode := s.b.InServerMode()
 
 	s.mu.Lock()
@@ -584,8 +587,17 @@ func (s *server) writeToClients(b []byte) {
 		}
 	}
 
-	for c := range s.clients {
-		ipn.WriteMsg(c, b)
+	if len(s.clients) == 0 {
+		// Common case (at least on busy servers): nobody
+		// connected (no GUI, etc), so return before
+		// serializing JSON.
+		return
+	}
+
+	if b, ok := marshalNotify(n, s.logf); ok {
+		for c := range s.clients {
+			ipn.WriteMsg(c, b)
+		}
 	}
 }
 
@@ -671,8 +683,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 			errMsg := err.Error()
 			go func() {
 				defer c.Close()
-				serverToClient := func(b []byte) { ipn.WriteMsg(c, b) }
-				bs := ipn.NewBackendServer(logf, nil, serverToClient)
+				bs := ipn.NewBackendServer(logf, nil, jsonNotifier(c, logf))
 				bs.SendErrorMessage(errMsg)
 				time.Sleep(time.Second)
 			}()
@@ -962,3 +973,25 @@ func peerPid(entries []netstat.Entry, la, ra netaddr.IPPort) int {
 	}
 	return 0
 }
+
+// jsonNotifier returns a notify-writer func that writes ipn.Notify
+// messages to w.
+func jsonNotifier(w io.Writer, logf logger.Logf) func(ipn.Notify) {
+	return func(n ipn.Notify) {
+		if b, ok := marshalNotify(n, logf); ok {
+			ipn.WriteMsg(w, b)
+		}
+	}
+}
+
+func marshalNotify(n ipn.Notify, logf logger.Logf) (b []byte, ok bool) {
+	b, err := json.Marshal(n)
+	if err != nil {
+		logf("ipnserver: [unexpected] error serializing JSON: %v", err)
+		return nil, false
+	}
+	if bytes.Contains(b, jsonEscapedZero) {
+		logf("[unexpected] zero byte in BackendServer.send notify message: %q", b)
+	}
+	return b, true
+}

ipn/ipnstate/ipnstate.go

@@ -65,6 +65,7 @@ type PeerStatusLite struct {
 }
 
 type PeerStatus struct {
+	ID        tailcfg.StableNodeID
 	PublicKey key.Public
 	HostName  string // HostInfo's Hostname (not a DNS name or necessarily unique)
 	DNSName   string
@@ -203,6 +204,9 @@ func (sb *StatusBuilder) AddPeer(peer key.Public, st *PeerStatus) {
 		return
 	}
 
+	if v := st.ID; v != "" {
+		e.ID = v
+	}
 	if v := st.HostName; v != "" {
 		e.HostName = v
 	}

ipn/message.go

@@ -88,9 +88,9 @@ type Command struct {
 
 type BackendServer struct {
 	logf          logger.Logf
-	b             Backend              // the Backend we are serving up
-	sendNotifyMsg func(jsonMsg []byte) // send a notification message
-	GotQuit       bool                 // a Quit command was received
+	b             Backend      // the Backend we are serving up
+	sendNotifyMsg func(Notify) // send a notification message
+	GotQuit       bool         // a Quit command was received
 }
 
 // NewBackendServer creates a new BackendServer using b.
@@ -98,7 +98,7 @@ type BackendServer struct {
 // 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 {
+func NewBackendServer(logf logger.Logf, b Backend, sendNotifyMsg func(Notify)) *BackendServer {
 	bs := &BackendServer{
 		logf:          logf,
 		b:             b,
@@ -115,14 +115,7 @@ func (bs *BackendServer) send(n Notify) {
 		return
 	}
 	n.Version = version.Long
-	b, err := json.Marshal(n)
-	if err != nil {
-		log.Fatalf("Failed json.Marshal(notify): %v\n%#v", err, n)
-	}
-	if bytes.Contains(b, jsonEscapedZero) {
-		log.Printf("[unexpected] zero byte in BackendServer.send notify message: %q", b)
-	}
-	bs.sendNotifyMsg(b)
+	bs.sendNotifyMsg(n)
 }
 
 func (bs *BackendServer) SendErrorMessage(msg string) {

ipn/message_test.go

@@ -7,6 +7,7 @@ package ipn
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"testing"
 	"time"
 
@@ -74,7 +75,11 @@ func TestClientServer(t *testing.T) {
 			bc.GotNotifyMsg(b)
 		}
 	}()
-	serverToClient := func(b []byte) {
+	serverToClient := func(n Notify) {
+		b, err := json.Marshal(n)
+		if err != nil {
+			panic(err.Error())
+		}
 		serverToClientCh <- append([]byte{}, b...)
 	}
 	clientToServer := func(b []byte) {

net/dns/manager_linux.go

@@ -57,12 +57,12 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 		}
 		if err := dbusPing("org.freedesktop.NetworkManager", "/org/freedesktop/NetworkManager/DnsManager"); err != nil {
 			dbg("nm", "no")
-			return newResolvedManager(logf)
+			return newResolvedManager(logf, interfaceName)
 		}
 		dbg("nm", "yes")
 		if err := nmIsUsingResolved(); err != nil {
 			dbg("nm-resolved", "no")
-			return newResolvedManager(logf)
+			return newResolvedManager(logf, interfaceName)
 		}
 		dbg("nm-resolved", "yes")
 
@@ -90,7 +90,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 			return newNMManager(interfaceName)
 		}
 		dbg("nm-old", "no")
-		return newResolvedManager(logf)
+		return newResolvedManager(logf, interfaceName)
 	case "resolvconf":
 		dbg("rc", "resolvconf")
 		if err := resolvconfSourceIsNM(bs); err == nil {

net/dns/nm.go

@@ -175,9 +175,12 @@ func (m *nmManager) trySet(ctx context.Context, config OSConfig) error {
 		search = append(search, "~.")
 	}
 
-	general := settings["connection"]
-	general["llmnr"] = dbus.MakeVariant(0)
-	general["mdns"] = dbus.MakeVariant(0)
+	// Ideally we would like to disable LLMNR and mdns on the
+	// interface here, but older NetworkManagers don't understand
+	// those settings and choke on them, so we don't. Both LLMNR and
+	// mdns will fail since tailscale0 doesn't do multicast, so it's
+	// effectively fine. We used to try and enforce LLMNR and mdns
+	// settings here, but that led to #1870.
 
 	ipv4Map := settings["ipv4"]
 	ipv4Map["dns"] = dbus.MakeVariant(dnsv4)
@@ -247,7 +250,7 @@ func (m *nmManager) trySet(ctx context.Context, config OSConfig) error {
 	}
 
 	if call := device.CallWithContext(ctx, "org.freedesktop.NetworkManager.Device.Reapply", 0, settings, version, uint32(0)); call.Err != nil {
-		return fmt.Errorf("reapply: %w", err)
+		return fmt.Errorf("reapply: %w", call.Err)
 	}
 
 	return nil

net/dns/resolved.go

@@ -12,11 +12,11 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 
 	"github.com/godbus/dbus/v5"
 	"golang.org/x/sys/unix"
 	"inet.af/netaddr"
-	"tailscale.com/net/interfaces"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/dnsname"
 )
@@ -85,17 +85,24 @@ func isResolvedActive() bool {
 // resolvedManager uses the systemd-resolved DBus API.
 type resolvedManager struct {
 	logf     logger.Logf
+	ifidx    int
 	resolved dbus.BusObject
 }
 
-func newResolvedManager(logf logger.Logf) (*resolvedManager, error) {
+func newResolvedManager(logf logger.Logf, interfaceName string) (*resolvedManager, error) {
 	conn, err := dbus.SystemBus()
 	if err != nil {
 		return nil, err
 	}
 
+	iface, err := net.InterfaceByName(interfaceName)
+	if err != nil {
+		return nil, err
+	}
+
 	return &resolvedManager{
 		logf:     logf,
+		ifidx:    iface.Index,
 		resolved: conn.Object("org.freedesktop.resolve1", dbus.ObjectPath("/org/freedesktop/resolve1")),
 	}, nil
 }
@@ -105,16 +112,6 @@ func (m *resolvedManager) SetDNS(config OSConfig) error {
 	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
 	defer cancel()
 
-	// In principle, we could persist this in the manager struct
-	// if we knew that interface indices are persistent. This does not seem to be the case.
-	_, iface, err := interfaces.Tailscale()
-	if err != nil {
-		return fmt.Errorf("getting interface index: %w", err)
-	}
-	if iface == nil {
-		return errNotReady
-	}
-
 	var linkNameservers = make([]resolvedLinkNameserver, len(config.Nameservers))
 	for i, server := range config.Nameservers {
 		ip := server.As16()
@@ -131,9 +128,9 @@ func (m *resolvedManager) SetDNS(config OSConfig) error {
 		}
 	}
 
-	err = m.resolved.CallWithContext(
+	err := m.resolved.CallWithContext(
 		ctx, "org.freedesktop.resolve1.Manager.SetLinkDNS", 0,
-		iface.Index, linkNameservers,
+		m.ifidx, linkNameservers,
 	).Store()
 	if err != nil {
 		return fmt.Errorf("setLinkDNS: %w", err)
@@ -174,13 +171,13 @@ func (m *resolvedManager) SetDNS(config OSConfig) error {
 
 	err = m.resolved.CallWithContext(
 		ctx, "org.freedesktop.resolve1.Manager.SetLinkDomains", 0,
-		iface.Index, linkDomains,
+		m.ifidx, linkDomains,
 	).Store()
 	if err != nil {
 		return fmt.Errorf("setLinkDomains: %w", err)
 	}
 
-	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkDefaultRoute", 0, iface.Index, len(config.MatchDomains) == 0); call.Err != nil {
+	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkDefaultRoute", 0, m.ifidx, len(config.MatchDomains) == 0); call.Err != nil {
 		return fmt.Errorf("setLinkDefaultRoute: %w", err)
 	}
 
@@ -189,22 +186,22 @@ func (m *resolvedManager) SetDNS(config OSConfig) error {
 	// or something).
 
 	// Disable LLMNR, we don't do multicast.
-	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkLLMNR", 0, iface.Index, "no"); call.Err != nil {
+	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkLLMNR", 0, m.ifidx, "no"); call.Err != nil {
 		m.logf("[v1] failed to disable LLMNR: %v", call.Err)
 	}
 
 	// Disable mdns.
-	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkMulticastDNS", 0, iface.Index, "no"); call.Err != nil {
+	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkMulticastDNS", 0, m.ifidx, "no"); call.Err != nil {
 		m.logf("[v1] failed to disable mdns: %v", call.Err)
 	}
 
 	// We don't support dnssec consistently right now, force it off to
 	// avoid partial failures when we split DNS internally.
-	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkDNSSEC", 0, iface.Index, "no"); call.Err != nil {
+	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkDNSSEC", 0, m.ifidx, "no"); call.Err != nil {
 		m.logf("[v1] failed to disable DNSSEC: %v", call.Err)
 	}
 
-	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkDNSOverTLS", 0, iface.Index, "no"); call.Err != nil {
+	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.SetLinkDNSOverTLS", 0, m.ifidx, "no"); call.Err != nil {
 		m.logf("[v1] failed to disable DoT: %v", call.Err)
 	}
 
@@ -227,15 +224,7 @@ func (m *resolvedManager) Close() error {
 	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
 	defer cancel()
 
-	_, iface, err := interfaces.Tailscale()
-	if err != nil {
-		return fmt.Errorf("getting interface index: %w", err)
-	}
-	if iface == nil {
-		return errNotReady
-	}
-
-	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.RevertLink", 0, iface.Index); call.Err != nil {
+	if call := m.resolved.CallWithContext(ctx, "org.freedesktop.resolve1.Manager.RevertLink", 0, m.ifidx); call.Err != nil {
 		return fmt.Errorf("RevertLink: %w", call.Err)
 	}
 

net/interfaces/interfaces_darwin.go

@@ -10,7 +10,6 @@ import (
 	"log"
 	"net"
 	"syscall"
-	"time"
 
 	"golang.org/x/net/route"
 	"golang.org/x/sys/unix"
@@ -29,32 +28,9 @@ 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
+// fetchRoutingTable calls route.FetchRIB, fetching NET_RT_DUMP2.
 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)
-		}
-	}
+	return route.FetchRIB(syscall.AF_UNSPEC, syscall.NET_RT_DUMP2, 0)
 }
 
 func DefaultRouteInterfaceIndex() (int, error) {

scripts/installer.sh

@@ -0,0 +1,414 @@
+#!/bin/sh
+# 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.
+#
+# This script detects the current operating system, and installs
+# Tailscale according to that OS's conventions.
+
+set -eu
+
+RELEASE="${TAILSCALE_RELEASE:-stable}"
+
+# All the code is wrapped in a main function that gets called at the
+# bottom of the file, so that a truncated partial download doesn't end
+# up executing half a script.
+main() {
+	# Step 1: detect the current linux distro, version, and packaging system.
+	#
+	# We rely on a combination of 'uname' and /etc/os-release to find
+	# an OS name and version, and from there work out what
+	# installation method we should be using.
+	#
+	# The end result of this step is that the following three
+	# variables are populated, if detection was successful.
+	OS=""
+	VERSION=""
+	PACKAGETYPE=""
+
+	if [ -f /etc/os-release ]; then
+		# /etc/os-release populates a number of shell variables. We care about the following:
+		#  - ID: the short name of the OS (e.g. "debian", "freebsd")
+		#  - VERSION_ID: the numeric release version for the OS, if any (e.g. "18.04")
+		#  - VERSION_CODENAME: the codename of the OS release, if any (e.g. "buster")
+		. /etc/os-release
+		case "$ID" in
+			ubuntu)
+				OS="$ID"
+				VERSION="$VERSION_CODENAME"
+				PACKAGETYPE="apt"
+				;;
+			debian)
+				OS="$ID"
+				VERSION="$VERSION_CODENAME"
+				PACKAGETYPE="apt"
+				;;
+			raspbian)
+				OS="$ID"
+				VERSION="$VERSION_CODENAME"
+				PACKAGETYPE="apt"
+				;;
+			centos)
+				OS="$ID"
+				VERSION="$VERSION_ID"
+				PACKAGETYPE="dnf"
+				if [ "$VERSION" = "7" ]; then
+					PACKAGETYPE="yum"
+				fi
+				;;
+			rhel)
+				OS="$ID"
+				VERSION="$(echo "$VERSION_ID" | cut -f1 -d.)"
+				PACKAGETYPE="dnf"
+				;;
+			fedora)
+				OS="$ID"
+				VERSION=""
+				PACKAGETYPE="dnf"
+				;;
+			amzn)
+				OS="amazon-linux"
+				VERSION="$VERSION_ID"
+				PACKAGETYPE="yum"
+				;;
+			opensuse-leap)
+				OS="opensuse"
+				VERSION="leap/$VERSION_ID"
+				PACKAGETYPE="zypper"
+				;;
+			opensuse-tumbleweed)
+				OS="opensuse"
+				VERSION="tumbleweed"
+				PACKAGETYPE="zypper"
+				;;
+ 			arch)
+				OS="$ID"
+				VERSION="" # rolling release
+				PACKAGETYPE="pacman"
+				;;
+			manjaro)
+				OS="$ID"
+				VERSION="" # rolling release
+				PACKAGETYPE="pacman"
+				;;
+			alpine)
+				OS="$ID"
+				VERSION="$(echo $PRETTY_NAME | cut -d' ' -f3)"
+				PACKAGETYPE="apk"
+				;;
+			nixos)
+				echo "Please add Tailscale to your NixOS configuration directly:"
+				echo
+				echo "services.tailscale.enable = true;"
+				exit 1
+				;;
+			void)
+				OS="$ID"
+				VERSION="" # rolling release
+				PACKAGETYPE="xbps"
+				;;
+			gentoo)
+				OS="$ID"
+				VERSION="" # rolling release
+				PACKAGETYPE="emerge"
+				;;
+			freebsd)
+				OS="$ID"
+				VERSION="$(echo "$VERSION_ID" | cut -f1 -d.)"
+				PACKAGETYPE="pkg"
+				;;
+			# TODO: wsl?
+			# TODO: synology? qnap?
+		esac
+	fi
+
+	# If we failed to detect something through os-release, consult
+	# uname and try to infer things from that.
+	if [ -z "$OS" ]; then
+		if type uname >/dev/null 2>&1; then
+			case "$(uname)" in
+				FreeBSD)
+					# FreeBSD before 12.2 doesn't have
+					# /etc/os-release, so we wouldn't have found it in
+					# the os-release probing above.
+					OS="freebsd"
+					VERSION="$(freebsd-version | cut -f1 -d.)"
+					PACKAGETYPE="pkg"
+					;;
+				OpenBSD)
+					OS="openbsd"
+					VERSION="$(uname -r)"
+					PACKAGETYPE=""
+					;;
+				Darwin)
+					OS="macos"
+					VERSION="$(sw_vers -productVersion | cut -f1-2 -d.)"
+					PACKAGETYPE="appstore"
+					;;
+				Linux)
+					OS="other-linux"
+					VERSION=""
+					PACKAGETYPE=""
+					;;
+			esac
+		fi
+	fi
+
+	# Step 2: having detected an OS we support, is it one of the
+	# versions we support?
+	OS_UNSUPPORTED=
+	case "$OS" in
+		ubuntu)
+			if [ "$VERSION" != "xenial" ] && \
+			   [ "$VERSION" != "bionic" ] && \
+			   [ "$VERSION" != "eoan" ] && \
+			   [ "$VERSION" != "focal" ] && \
+			   [ "$VERSION" != "groovy" ] && \
+			   [ "$VERSION" != "hirsute" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+		;;
+		debian)
+			if [ "$VERSION" != "stretch" ] && \
+			   [ "$VERSION" != "buster" ] && \
+			   [ "$VERSION" != "bullseye" ] && \
+			   [ "$VERSION" != "sid" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+		;;
+		raspbian)
+			if [ "$VERSION" != "buster" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+		;;
+		fedora)
+			# We support every fedora release currently in use.
+			# No checking needed.
+		;;
+		centos)
+			if [ "$VERSION" != "7" ] && \
+			   [ "$VERSION" != "8" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+		;;
+		rhel)
+			if [ "$VERSION" != "8" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+		;;
+		amazon-linux)
+			if [ "$VERSION" != "2" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+		;;
+		opensuse)
+			if [ "$VERSION" != "leap/15.1" ] && \
+			   [ "$VERSION" != "leap/15.2" ] && \
+			   [ "$VERSION" != "tumbleweed" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+			;;
+		arch)
+			# Rolling release, no version checking needed.
+			;;
+		manjaro)
+			# Rolling release, no version checking needed.
+			;;
+		alpine)
+			if [ "$VERSION" != "edge" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+			;;
+		void)
+			# Rolling release, no version checking needed.
+			;;
+		gentoo)
+			# Rolling release, no version checking needed.
+			;;
+		freebsd)
+			if [ "$VERSION" != "12" ] && \
+			   [ "$VERSION" != "13" ]
+			then
+				OS_UNSUPPORTED=1
+			fi
+			;;
+		openbsd)
+			OS_UNSUPPORTED=1
+			;;
+		macos)
+			# We delegate macOS installation to the app store, it will
+			# perform version checks for us.
+			;;
+		other-linux)
+			OS_UNSUPPORTED=1
+			;;
+		*)
+			OS_UNSUPPORTED=1
+			;;
+	esac
+	if [ "$OS_UNSUPPORTED" = "1" ]; then
+		case "$OS" in
+			other-linux)
+				echo "Couldn't determine what kind of Linux is running."
+				echo "You could try the static binaries at:"
+				echo "https://pkgs.tailscale.com/stable/#static"
+				;;
+			"")
+				echo "Couldn't determine what operating system you're running."
+				;;
+			*)
+				echo "$OS $VERSION isn't supported by this script yet."
+				;;
+		esac
+		echo
+		echo "If you'd like us to support your system better, please email support@tailscale.com"
+		echo "and tell us what OS you're running."
+		echo
+		echo "Please include the following information we gathered from your system:"
+		echo
+		echo "OS=$OS"
+		echo "VERSION=$VERSION"
+		echo "PACKAGETYPE=$PACKAGETYPE"
+		if type uname >/dev/null 2>&1; then
+			echo "UNAME=$(uname -a)"
+		else
+			echo "UNAME="
+		fi
+		echo
+		if [ -f /etc/os-release ]; then
+			cat /etc/os-release
+		else
+			echo "No /etc/os-release"
+		fi
+		exit 1
+	fi
+
+	# Step 3: work out if we can run privileged commands, and if so,
+	# how.
+	CAN_ROOT=
+	SUDO=
+	if [ "$(id -u)" = 0 ]; then
+		CAN_ROOT=1
+		SUDO=""
+	elif type sudo >/dev/null; then
+		CAN_ROOT=1
+		SUDO="sudo"
+	elif type doas >/dev/null; then
+		CAN_ROOT=1
+		SUDO="doas"
+	fi
+	if [ "$CAN_ROOT" != "1" ]; then
+		echo "This installer needs to run commands as root."
+		echo "We tried looking for 'sudo' and 'doas', but couldn't find them."
+		echo "Either re-run this script as root, or set up sudo/doas."
+		exit 1
+	fi
+
+
+	# Step 4: run the installation.
+	echo "Installing Tailscale for $OS $VERSION, using method $PACKAGETYPE"
+	case "$PACKAGETYPE" in
+		apt)
+			# Ideally we want to use curl, but on some installs we
+			# only have wget. Detect and use what's available.
+			CURL=
+			if type curl >/dev/null; then
+				CURL="curl -fsSL"
+			elif type wget >/dev/null; then
+				CURL="wget -q -O-"
+			fi
+			if [ -z "$CURL" ]; then
+				echo "The installer needs either curl or wget to download files."
+				echo "Please install either curl or wget to proceed."
+				exit 1
+			fi
+
+			# TODO: use newfangled per-repo signature scheme
+			set -x
+			$CURL "https://pkgs.tailscale.com/$RELEASE/$OS/$VERSION.gpg" | $SUDO apt-key add -
+			$CURL "https://pkgs.tailscale.com/$RELEASE/$OS/$VERSION.list" | $SUDO tee /etc/apt/sources.list.d/tailscale.list
+			$SUDO apt-get update
+			$SUDO apt-get install tailscale
+			set +x
+		;;
+		yum)
+			set -x
+			$SUDO yum install -y yum-utils
+			$SUDO yum-config-manager --add-repo "https://pkgs.tailscale.com/$RELEASE/$OS/$VERSION/tailscale.repo"
+			$SUDO yum install -y tailscale
+			$SUDO systemctl enable --now tailscaled
+			set +x
+		;;
+		dnf)
+			set -x
+			$SUDO dnf config-manager --add-repo "https://pkgs.tailscale.com/$RELEASE/$OS/$VERSION/tailscale.repo"
+			$SUDO dnf install -y tailscale
+			$SUDO systemctl enable --now tailscaled
+			set +x
+		;;
+		zypper)
+			set -x
+			$SUDO rpm --import https://pkgs.tailscale.com/$RELEASE/$OS/$VERSION/repo.gpg
+			$SUDO zypper ar -g -r "https://pkgs.tailscale.com/$RELEASE/$OS/$VERSION/tailscale.repo"
+			$SUDO zypper ref -r tailscale-stable
+			$SUDO zypper in -y tailscale
+			$SUDO systemctl enable --now tailscaled
+			set +x
+			;;
+		pacman)
+			set -x
+			$SUDO pacman -S --noconfirm tailscale
+			$SUDO systemctl enable --now tailscaled
+			set +x
+			;;
+		apk)
+			set -x
+			$SUDO apk add tailscale
+			$SUDO rc-update add tailscale
+			$SUDO service tailscale start
+			set +x
+			;;
+		xbps)
+			set -x
+			$SUDO xbps-install tailscale
+			set +x
+			;;
+		emerge)
+			set -x
+			$SUDO emerge net-vpn/tailscale
+			set +x
+			;;
+		appstore)
+			set -x
+			open "https://apps.apple.com/us/app/tailscale/id1475387142"
+			set +x
+			;;
+		pkg)
+			set -x
+			$SUDO pkg install -y tailscale
+			set +x
+    ;;
+		*)
+			echo "unexpected: unknown package type $PACKAGETYPE"
+			exit 1
+			;;
+	esac
+
+	echo "Installation complete! Log in to start using Tailscale by running:"
+	echo
+	if [ -z "$SUDO" ]; then
+		echo "tailscale up"
+	else
+		echo "$SUDO tailscale up"
+	fi
+}
+
+main

tailcfg/tailcfg.go

@@ -7,7 +7,7 @@ package tailcfg
 //go:generate go run tailscale.com/cmd/cloner --type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,DNSResolver,RegisterResponse --clonefunc=true --output=tailcfg_clone.go
 
 import (
-	"bytes"
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"reflect"
@@ -679,7 +679,7 @@ func (et EndpointType) String() string {
 
 // Endpoint is an endpoint IPPort and an associated type.
 // It doesn't currently go over the wire as is but is instead
-// broken up into two parallel slices in MapReqeust, for compatibility
+// broken up into two parallel slices in MapRequest, for compatibility
 // reasons. But this type is used in the codebase.
 type Endpoint struct {
 	Addr netaddr.IPPort
@@ -1027,9 +1027,10 @@ func (k MachineKey) HexString() string                { return fmt.Sprintf("%x",
 func (k *MachineKey) UnmarshalText(text []byte) error { return keyUnmarshalText(k[:], "mkey:", text) }
 
 func keyMarshalText(prefix string, k [32]byte) []byte {
-	buf := bytes.NewBuffer(make([]byte, 0, len(prefix)+64))
-	fmt.Fprintf(buf, "%s%x", prefix, k[:])
-	return buf.Bytes()
+	buf := make([]byte, len(prefix)+64)
+	copy(buf, prefix)
+	hex.Encode(buf[len(prefix):], k[:])
+	return buf
 }
 
 func keyUnmarshalText(dst []byte, prefix string, text []byte) error {

tailcfg/tailcfg_test.go

@@ -518,3 +518,13 @@ func TestEndpointTypeMarshal(t *testing.T) {
 		t.Errorf("got %s; want %s", got, want)
 	}
 }
+
+var sinkBytes []byte
+
+func BenchmarkKeyMarshalText(b *testing.B) {
+	b.ReportAllocs()
+	var k [32]byte
+	for i := 0; i < b.N; i++ {
+		sinkBytes = keyMarshalText("prefix", k)
+	}
+}

tsnet/example/tshello/tshello.go

@@ -0,0 +1,43 @@
+// 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.
+
+// The tshello server demonstrates how to use Tailscale as a library.
+package main
+
+import (
+	"fmt"
+	"html"
+	"log"
+	"net/http"
+	"strings"
+
+	"tailscale.com/tsnet"
+)
+
+func main() {
+	s := new(tsnet.Server)
+	ln, err := s.Listen("tcp", ":80")
+	if err != nil {
+		log.Fatal(err)
+	}
+	log.Fatal(http.Serve(ln, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		who, ok := s.WhoIs(r.RemoteAddr)
+		if !ok {
+			http.Error(w, "WhoIs failed", 500)
+			return
+		}
+		fmt.Fprintf(w, "<html><body><h1>Hello, world!</h1>\n")
+		fmt.Fprintf(w, "<p>You are <b>%s</b> from <b>%s</b> (%s)</p>",
+			html.EscapeString(who.UserProfile.LoginName),
+			html.EscapeString(firstLabel(who.Node.ComputedName)),
+			r.RemoteAddr)
+	})))
+}
+
+func firstLabel(s string) string {
+	if i := strings.Index(s, "."); i != -1 {
+		return s[:i]
+	}
+	return s
+}

tsnet/tsnet.go

@@ -0,0 +1,274 @@
+// 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 tsnet provides Tailscale as a library.
+//
+// It is an experimental work in progress.
+package tsnet
+
+import (
+	"errors"
+	"fmt"
+	"log"
+	"net"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"sync"
+	"time"
+
+	"inet.af/netaddr"
+	"tailscale.com/client/tailscale/apitype"
+	"tailscale.com/control/controlclient"
+	"tailscale.com/ipn"
+	"tailscale.com/ipn/ipnlocal"
+	"tailscale.com/smallzstd"
+	"tailscale.com/types/logger"
+	"tailscale.com/wgengine"
+	"tailscale.com/wgengine/monitor"
+	"tailscale.com/wgengine/netstack"
+)
+
+// Server is an embedded Tailscale server.
+//
+// Its exported fields may be changed until the first call to Listen.
+type Server struct {
+	// Dir specifies the name of the directory to use for
+	// state. If empty, a directory is selected automatically
+	// under os.UserConfigDir (https://golang.org/pkg/os/#UserConfigDir).
+	// based on the name of the binary.
+	Dir string
+
+	// Hostname is the hostname to present to the control server.
+	// If empty, the binary name is used.l
+	Hostname string
+
+	// Logf, if non-nil, specifies the logger to use. By default,
+	// log.Printf is used.
+	Logf logger.Logf
+
+	initOnce sync.Once
+	initErr  error
+	lb       *ipnlocal.LocalBackend
+	// the state directory
+	dir      string
+	hostname string
+
+	mu        sync.Mutex
+	listeners map[listenKey]*listener
+}
+
+// WhoIs reports the node and user who owns the node with the given
+// address. The addr may be an ip:port (as from an
+// http.Request.RemoteAddr) or just an IP address.
+func (s *Server) WhoIs(addr string) (w *apitype.WhoIsResponse, ok bool) {
+	ipp, err := netaddr.ParseIPPort(addr)
+	if err != nil {
+		ip, err := netaddr.ParseIP(addr)
+		if err != nil {
+			return nil, false
+		}
+		ipp.IP = ip
+	}
+	n, up, ok := s.lb.WhoIs(ipp)
+	if !ok {
+		return nil, false
+	}
+	return &apitype.WhoIsResponse{
+		Node:        n,
+		UserProfile: &up,
+	}, true
+}
+
+func (s *Server) doInit() {
+	if err := s.start(); err != nil {
+		s.initErr = fmt.Errorf("tsnet: %w", err)
+	}
+}
+
+func (s *Server) start() error {
+	if v, _ := strconv.ParseBool(os.Getenv("TAILSCALE_USE_WIP_CODE")); !v {
+		return errors.New("code disabled without environment variable TAILSCALE_USE_WIP_CODE set true")
+	}
+
+	exe, err := os.Executable()
+	if err != nil {
+		return err
+	}
+	prog := strings.TrimSuffix(strings.ToLower(filepath.Base(exe)), ".exe")
+
+	s.hostname = s.Hostname
+	if s.hostname == "" {
+		s.hostname = prog
+	}
+
+	s.dir = s.Dir
+	if s.dir == "" {
+		confDir, err := os.UserConfigDir()
+		if err != nil {
+			return err
+		}
+		s.dir = filepath.Join(confDir, "tslib-"+prog)
+		if err := os.MkdirAll(s.dir, 0700); err != nil {
+			return err
+		}
+	}
+	if fi, err := os.Stat(s.dir); err != nil {
+		return err
+	} else if !fi.IsDir() {
+		return fmt.Errorf("%v is not a directory", s.dir)
+	}
+
+	logf := s.Logf
+	if logf == nil {
+		logf = log.Printf
+	}
+
+	// TODO(bradfitz): start logtail? don't use filch, perhaps?
+	// only upload plumbed Logf?
+
+	linkMon, err := monitor.New(logf)
+	if err != nil {
+		return err
+	}
+
+	eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{
+		ListenPort:  0,
+		LinkMonitor: linkMon,
+	})
+	if err != nil {
+		return err
+	}
+
+	tunDev, magicConn, ok := eng.(wgengine.InternalsGetter).GetInternals()
+	if !ok {
+		return fmt.Errorf("%T is not a wgengine.InternalsGetter", eng)
+	}
+
+	ns, err := netstack.Create(logf, tunDev, eng, magicConn, false)
+	if err != nil {
+		return fmt.Errorf("netstack.Create: %w", err)
+	}
+	ns.ForwardTCPIn = s.forwardTCP
+	if err := ns.Start(); err != nil {
+		return fmt.Errorf("failed to start netstack: %w", err)
+	}
+
+	statePath := filepath.Join(s.dir, "tailscaled.state")
+	store, err := ipn.NewFileStore(statePath)
+	if err != nil {
+		return err
+	}
+	logid := "tslib-TODO"
+
+	lb, err := ipnlocal.NewLocalBackend(logf, logid, store, eng)
+	if err != nil {
+		return fmt.Errorf("NewLocalBackend: %v", err)
+	}
+	s.lb = lb
+	lb.SetDecompressor(func() (controlclient.Decompressor, error) {
+		return smallzstd.NewDecoder(nil)
+	})
+	prefs := ipn.NewPrefs()
+	prefs.Hostname = s.hostname
+	prefs.WantRunning = true
+	err = lb.Start(ipn.Options{
+		StateKey:    ipn.GlobalDaemonStateKey,
+		UpdatePrefs: prefs,
+	})
+	if err != nil {
+		return fmt.Errorf("starting backend: %w", err)
+	}
+	if os.Getenv("TS_LOGIN") == "1" {
+		s.lb.StartLoginInteractive()
+	}
+	return nil
+}
+
+func (s *Server) forwardTCP(c net.Conn, port uint16) {
+	s.mu.Lock()
+	ln, ok := s.listeners[listenKey{"tcp", "", fmt.Sprint(port)}]
+	s.mu.Unlock()
+	if !ok {
+		c.Close()
+		return
+	}
+	t := time.NewTimer(time.Second)
+	defer t.Stop()
+	select {
+	case ln.conn <- c:
+	case <-t.C:
+		c.Close()
+	}
+}
+
+func (s *Server) Listen(network, addr string) (net.Listener, error) {
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil {
+		return nil, fmt.Errorf("tsnet: %w", err)
+	}
+
+	s.initOnce.Do(s.doInit)
+	if s.initErr != nil {
+		return nil, s.initErr
+	}
+
+	key := listenKey{network, host, port}
+	ln := &listener{
+		s:    s,
+		key:  key,
+		addr: addr,
+
+		conn: make(chan net.Conn),
+	}
+	s.mu.Lock()
+	if s.listeners == nil {
+		s.listeners = map[listenKey]*listener{}
+	}
+	if _, ok := s.listeners[key]; ok {
+		s.mu.Unlock()
+		return nil, fmt.Errorf("tsnet: listener already open for %s, %s", network, addr)
+	}
+	s.listeners[key] = ln
+	s.mu.Unlock()
+	return ln, nil
+}
+
+type listenKey struct {
+	network string
+	host    string
+	port    string
+}
+
+type listener struct {
+	s    *Server
+	key  listenKey
+	addr string
+	conn chan net.Conn
+}
+
+func (ln *listener) Accept() (net.Conn, error) {
+	c, ok := <-ln.conn
+	if !ok {
+		return nil, fmt.Errorf("tsnet: %w", net.ErrClosed)
+	}
+	return c, nil
+}
+
+func (ln *listener) Addr() net.Addr { return addr{ln} }
+func (ln *listener) Close() error {
+	ln.s.mu.Lock()
+	defer ln.s.mu.Unlock()
+	if v, ok := ln.s.listeners[ln.key]; ok && v == ln {
+		delete(ln.s.listeners, ln.key)
+		close(ln.conn)
+	}
+	return nil
+}
+
+type addr struct{ ln *listener }
+
+func (a addr) Network() string { return a.ln.key.network }
+func (a addr) String() string  { return a.ln.addr }

tstest/integration/integration_test.go

@@ -10,6 +10,8 @@ import (
 	crand "crypto/rand"
 	"crypto/tls"
 	"encoding/json"
+	"errors"
+	"flag"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -21,6 +23,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"strings"
 	"sync"
@@ -41,8 +44,11 @@ import (
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/nettype"
+	"tailscale.com/version"
 )
 
+var verbose = flag.Bool("verbose", false, "verbose debug logs")
+
 var mainError atomic.Value // of error
 
 func TestMain(m *testing.M) {
@@ -57,11 +63,8 @@ func TestMain(m *testing.M) {
 	os.Exit(0)
 }
 
-func TestIntegration(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("not tested/working on Windows yet")
-	}
-
+func TestOneNodeUp_NoAuth(t *testing.T) {
+	t.Parallel()
 	bins := buildTestBinaries(t)
 
 	env := newTestEnv(t, bins)
@@ -69,8 +72,8 @@ func TestIntegration(t *testing.T) {
 
 	n1 := newTestNode(t, env)
 
-	dcmd := n1.StartDaemon(t)
-	defer dcmd.Process.Kill()
+	d1 := n1.StartDaemon(t)
+	defer d1.Kill()
 
 	n1.AwaitListening(t)
 
@@ -87,44 +90,110 @@ func TestIntegration(t *testing.T) {
 		t.Error(err)
 	}
 
-	t.Logf("Running up --login-server=%s ...", env.ControlServer.URL)
-	if err := n1.Tailscale("up", "--login-server="+env.ControlServer.URL).Run(); err != nil {
-		t.Fatalf("up: %v", err)
-	}
+	n1.MustUp()
 
 	if d, _ := time.ParseDuration(os.Getenv("TS_POST_UP_SLEEP")); d > 0 {
 		t.Logf("Sleeping for %v to give 'up' time to misbehave (https://github.com/tailscale/tailscale/issues/1840) ...", d)
 		time.Sleep(d)
 	}
 
-	var ip string
-	if err := tstest.WaitFor(20*time.Second, func() error {
-		out, err := n1.Tailscale("ip").Output()
-		if err != nil {
-			return err
+	t.Logf("Got IP: %v", n1.AwaitIP(t))
+	n1.AwaitRunning(t)
+
+	d1.MustCleanShutdown(t)
+
+	t.Logf("number of HTTP logcatcher requests: %v", env.LogCatcher.numRequests())
+}
+
+func TestOneNodeUp_Auth(t *testing.T) {
+	t.Parallel()
+	bins := buildTestBinaries(t)
+
+	env := newTestEnv(t, bins)
+	defer env.Close()
+	env.Control.RequireAuth = true
+
+	n1 := newTestNode(t, env)
+	d1 := n1.StartDaemon(t)
+	defer d1.Kill()
+
+	n1.AwaitListening(t)
+
+	st := n1.MustStatus(t)
+	t.Logf("Status: %s", st.BackendState)
+
+	t.Logf("Running up --login-server=%s ...", env.ControlServer.URL)
+
+	cmd := n1.Tailscale("up", "--login-server="+env.ControlServer.URL)
+	var authCountAtomic int32
+	cmd.Stdout = &authURLParserWriter{fn: func(urlStr string) error {
+		if env.Control.CompleteAuth(urlStr) {
+			atomic.AddInt32(&authCountAtomic, 1)
+			t.Logf("completed auth path %s", urlStr)
+			return nil
+		}
+		err := fmt.Errorf("Failed to complete auth path to %q", urlStr)
+		t.Log(err)
+		return err
+	}}
+	cmd.Stderr = cmd.Stdout
+	if err := cmd.Run(); err != nil {
+		t.Fatalf("up: %v", err)
+	}
+	t.Logf("Got IP: %v", n1.AwaitIP(t))
+
+	n1.AwaitRunning(t)
+
+	if n := atomic.LoadInt32(&authCountAtomic); n != 1 {
+		t.Errorf("Auth URLs completed = %d; want 1", n)
+	}
+
+	d1.MustCleanShutdown(t)
+
+}
+
+func TestTwoNodes(t *testing.T) {
+	t.Parallel()
+	bins := buildTestBinaries(t)
+
+	env := newTestEnv(t, bins)
+	defer env.Close()
+
+	// Create two nodes:
+	n1 := newTestNode(t, env)
+	d1 := n1.StartDaemon(t)
+	defer d1.Kill()
+
+	n2 := newTestNode(t, env)
+	d2 := n2.StartDaemon(t)
+	defer d2.Kill()
+
+	n1.AwaitListening(t)
+	n2.AwaitListening(t)
+	n1.MustUp()
+	n2.MustUp()
+	n1.AwaitRunning(t)
+	n2.AwaitRunning(t)
+
+	if err := tstest.WaitFor(2*time.Second, func() error {
+		st := n1.MustStatus(t)
+		if len(st.Peer) == 0 {
+			return errors.New("no peers")
+		}
+		if len(st.Peer) > 1 {
+			return fmt.Errorf("got %d peers; want 1", len(st.Peer))
+		}
+		peer := st.Peer[st.Peers()[0]]
+		if peer.ID == st.Self.ID {
+			return errors.New("peer is self")
 		}
-		ip = string(out)
 		return nil
 	}); err != nil {
 		t.Error(err)
 	}
-	t.Logf("Got IP: %v", ip)
 
-	dcmd.Process.Signal(os.Interrupt)
-
-	ps, err := dcmd.Process.Wait()
-	if err != nil {
-		t.Fatalf("tailscaled Wait: %v", err)
-	}
-	if ps.ExitCode() != 0 {
-		t.Errorf("tailscaled ExitCode = %d; want 0", ps.ExitCode())
-	}
-
-	t.Logf("number of HTTP logcatcher requests: %v", env.LogCatcher.numRequests())
-	if err := env.TrafficTrap.Err(); err != nil {
-		t.Errorf("traffic trap: %v", err)
-		t.Logf("logs: %s", env.LogCatcher.logsString())
-	}
+	d1.MustCleanShutdown(t)
+	d2.MustCleanShutdown(t)
 }
 
 // testBinaries are the paths to a tailscaled and tailscale binary.
@@ -139,16 +208,18 @@ type testBinaries struct {
 // if they fail to compile.
 func buildTestBinaries(t testing.TB) *testBinaries {
 	td := t.TempDir()
+	build(t, td, "tailscale.com/cmd/tailscaled", "tailscale.com/cmd/tailscale")
 	return &testBinaries{
 		dir:    td,
-		daemon: build(t, td, "tailscale.com/cmd/tailscaled"),
-		cli:    build(t, td, "tailscale.com/cmd/tailscale"),
+		daemon: filepath.Join(td, "tailscaled"+exe()),
+		cli:    filepath.Join(td, "tailscale"+exe()),
 	}
 }
 
 // testEnv contains the test environment (set of servers) used by one
 // or more nodes.
 type testEnv struct {
+	t        testing.TB
 	Binaries *testBinaries
 
 	LogCatcher       *logCatcher
@@ -168,6 +239,9 @@ type testEnv struct {
 //
 // Call Close to shut everything down.
 func newTestEnv(t testing.TB, bins *testBinaries) *testEnv {
+	if runtime.GOOS == "windows" {
+		t.Skip("not tested/working on Windows yet")
+	}
 	derpMap, derpShutdown := runDERPAndStun(t, logger.Discard)
 	logc := new(logCatcher)
 	control := &testcontrol.Server{
@@ -175,6 +249,7 @@ func newTestEnv(t testing.TB, bins *testBinaries) *testEnv {
 	}
 	trafficTrap := new(trafficTrap)
 	e := &testEnv{
+		t:                 t,
 		Binaries:          bins,
 		LogCatcher:        logc,
 		LogCatcherServer:  httptest.NewServer(logc),
@@ -184,10 +259,16 @@ func newTestEnv(t testing.TB, bins *testBinaries) *testEnv {
 		TrafficTrapServer: httptest.NewServer(trafficTrap),
 		derpShutdown:      derpShutdown,
 	}
+	e.Control.BaseURL = e.ControlServer.URL
 	return e
 }
 
 func (e *testEnv) Close() error {
+	if err := e.TrafficTrap.Err(); err != nil {
+		e.t.Errorf("traffic trap: %v", err)
+		e.t.Logf("logs: %s", e.LogCatcher.logsString())
+	}
+
 	e.LogCatcherServer.Close()
 	e.TrafficTrapServer.Close()
 	e.ControlServer.Close()
@@ -218,9 +299,28 @@ func newTestNode(t *testing.T, env *testEnv) *testNode {
 	}
 }
 
+type Daemon struct {
+	Process *os.Process
+}
+
+func (d *Daemon) Kill() {
+	d.Process.Kill()
+}
+
+func (d *Daemon) MustCleanShutdown(t testing.TB) {
+	d.Process.Signal(os.Interrupt)
+	ps, err := d.Process.Wait()
+	if err != nil {
+		t.Fatalf("tailscaled Wait: %v", err)
+	}
+	if ps.ExitCode() != 0 {
+		t.Errorf("tailscaled ExitCode = %d; want 0", ps.ExitCode())
+	}
+}
+
 // StartDaemon starts the node's tailscaled, failing if it fails to
 // start.
-func (n *testNode) StartDaemon(t testing.TB) *exec.Cmd {
+func (n *testNode) StartDaemon(t testing.TB) *Daemon {
 	cmd := exec.Command(n.env.Binaries.daemon,
 		"--tun=userspace-networking",
 		"--state="+n.stateFile,
@@ -234,7 +334,17 @@ func (n *testNode) StartDaemon(t testing.TB) *exec.Cmd {
 	if err := cmd.Start(); err != nil {
 		t.Fatalf("starting tailscaled: %v", err)
 	}
-	return cmd
+	return &Daemon{
+		Process: cmd.Process,
+	}
+}
+
+func (n *testNode) MustUp() {
+	t := n.env.t
+	t.Logf("Running up --login-server=%s ...", n.env.ControlServer.URL)
+	if err := n.Tailscale("up", "--login-server="+n.env.ControlServer.URL).Run(); err != nil {
+		t.Fatalf("up: %v", err)
+	}
 }
 
 // AwaitListening waits for the tailscaled to be serving local clients
@@ -252,6 +362,40 @@ func (n *testNode) AwaitListening(t testing.TB) {
 	}
 }
 
+func (n *testNode) AwaitIP(t testing.TB) (ips string) {
+	t.Helper()
+	if err := tstest.WaitFor(20*time.Second, func() error {
+		out, err := n.Tailscale("ip").Output()
+		if err != nil {
+			return err
+		}
+		ips = string(out)
+		return nil
+	}); err != nil {
+		t.Fatalf("awaiting an IP address: %v", err)
+	}
+	if ips == "" {
+		t.Fatalf("returned IP address was blank")
+	}
+	return ips
+}
+
+func (n *testNode) AwaitRunning(t testing.TB) {
+	t.Helper()
+	if err := tstest.WaitFor(20*time.Second, func() error {
+		st, err := n.Status()
+		if err != nil {
+			return err
+		}
+		if st.BackendState != "Running" {
+			return fmt.Errorf("in state %q", st.BackendState)
+		}
+		return nil
+	}); err != nil {
+		t.Fatalf("failure/timeout waiting for transition to Running status: %v", err)
+	}
+}
+
 // Tailscale returns a command that runs the tailscale CLI with the provided arguments.
 // It does not start the process.
 func (n *testNode) Tailscale(arg ...string) *exec.Cmd {
@@ -261,15 +405,23 @@ func (n *testNode) Tailscale(arg ...string) *exec.Cmd {
 	return cmd
 }
 
-func (n *testNode) MustStatus(tb testing.TB) *ipnstate.Status {
-	tb.Helper()
+func (n *testNode) Status() (*ipnstate.Status, error) {
 	out, err := n.Tailscale("status", "--json").CombinedOutput()
 	if err != nil {
-		tb.Fatalf("getting status: %v, %s", err, out)
+		return nil, fmt.Errorf("running tailscale status: %v, %s", err, out)
 	}
 	st := new(ipnstate.Status)
 	if err := json.Unmarshal(out, st); err != nil {
-		tb.Fatalf("parsing status json: %v, from: %s", err, out)
+		return nil, fmt.Errorf("decoding tailscale status JSON: %w", err)
+	}
+	return st, nil
+}
+
+func (n *testNode) MustStatus(tb testing.TB) *ipnstate.Status {
+	tb.Helper()
+	st, err := n.Status()
+	if err != nil {
+		tb.Fatal(err)
 	}
 	return st
 }
@@ -291,21 +443,44 @@ func findGo(t testing.TB) string {
 	} else if !fi.Mode().IsRegular() {
 		t.Fatalf("%v is unexpected %v", goBin, fi.Mode())
 	}
-	t.Logf("using go binary %v", goBin)
 	return goBin
 }
 
-func build(t testing.TB, outDir, target string) string {
-	exe := ""
-	if runtime.GOOS == "windows" {
-		exe = ".exe"
+// buildMu limits our use of "go build" to one at a time, so we don't
+// fight Go's built-in caching trying to do the same build concurrently.
+var buildMu sync.Mutex
+
+func build(t testing.TB, outDir string, targets ...string) {
+	buildMu.Lock()
+	defer buildMu.Unlock()
+
+	t0 := time.Now()
+	defer func() { t.Logf("built %s in %v", targets, time.Since(t0).Round(time.Millisecond)) }()
+
+	goBin := findGo(t)
+	cmd := exec.Command(goBin, "install")
+	if version.IsRace() {
+		cmd.Args = append(cmd.Args, "-race")
 	}
-	bin := filepath.Join(outDir, path.Base(target)) + exe
-	errOut, err := exec.Command(findGo(t), "build", "-o", bin, target).CombinedOutput()
-	if err != nil {
-		t.Fatalf("failed to build %v: %v, %s", target, err, errOut)
+	cmd.Args = append(cmd.Args, targets...)
+	cmd.Env = append(os.Environ(), "GOARCH="+runtime.GOARCH, "GOBIN="+outDir)
+	errOut, err := cmd.CombinedOutput()
+	if err == nil {
+		return
 	}
-	return bin
+	if strings.Contains(string(errOut), "when GOBIN is set") {
+		// Fallback slow path for cross-compiled binaries.
+		for _, target := range targets {
+			outFile := filepath.Join(outDir, path.Base(target)+exe())
+			cmd := exec.Command(goBin, "build", "-o", outFile, target)
+			cmd.Env = append(os.Environ(), "GOARCH="+runtime.GOARCH)
+			if errOut, err := cmd.CombinedOutput(); err != nil {
+				t.Fatalf("failed to build %v with %v: %v, %s", target, goBin, err, errOut)
+			}
+		}
+		return
+	}
+	t.Fatalf("failed to build %v with %v: %v, %s", targets, goBin, err, errOut)
 }
 
 // logCatcher is a minimal logcatcher for the logtail upload client.
@@ -378,6 +553,9 @@ func (lc *logCatcher) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	} else {
 		for _, ent := range jreq {
 			fmt.Fprintf(&lc.buf, "%s\n", strings.TrimSpace(ent.Text))
+			if *verbose {
+				fmt.Fprintf(os.Stderr, "%s\n", strings.TrimSpace(ent.Text))
+			}
 		}
 	}
 	w.WriteHeader(200) // must have no content, but not a 204
@@ -454,3 +632,23 @@ func runDERPAndStun(t testing.TB, logf logger.Logf) (derpMap *tailcfg.DERPMap, c
 
 	return m, cleanup
 }
+
+type authURLParserWriter struct {
+	buf bytes.Buffer
+	fn  func(urlStr string) error
+}
+
+var authURLRx = regexp.MustCompile(`(https?://\S+/auth/\S+)`)
+
+func (w *authURLParserWriter) Write(p []byte) (n int, err error) {
+	n, err = w.buf.Write(p)
+	m := authURLRx.FindSubmatch(w.buf.Bytes())
+	if m != nil {
+		urlStr := string(m[1])
+		w.buf.Reset() // so it's not matched again
+		if err := w.fn(urlStr); err != nil {
+			return 0, err
+		}
+	}
+	return n, err
+}

tstest/integration/testcontrol/testcontrol.go

@@ -17,6 +17,8 @@ import (
 	"log"
 	"math/rand"
 	"net/http"
+	"net/url"
+	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -34,19 +36,43 @@ import (
 // Server is a control plane server. Its zero value is ready for use.
 // Everything is stored in-memory in one tailnet.
 type Server struct {
-	Logf    logger.Logf      // nil means to use the log package
-	DERPMap *tailcfg.DERPMap // nil means to use prod DERP map
+	Logf        logger.Logf      // nil means to use the log package
+	DERPMap     *tailcfg.DERPMap // nil means to use prod DERP map
+	RequireAuth bool
+	BaseURL     string // must be set to e.g. "http://127.0.0.1:1234" with no trailing URL
+	Verbose     bool
 
 	initMuxOnce sync.Once
 	mux         *http.ServeMux
 
-	mu      sync.Mutex
-	pubKey  wgkey.Key
-	privKey wgkey.Private
-	nodes   map[tailcfg.NodeKey]*tailcfg.Node
-	users   map[tailcfg.NodeKey]*tailcfg.User
-	logins  map[tailcfg.NodeKey]*tailcfg.Login
-	updates map[tailcfg.NodeID]chan updateType
+	mu            sync.Mutex
+	pubKey        wgkey.Key
+	privKey       wgkey.Private
+	nodes         map[tailcfg.NodeKey]*tailcfg.Node
+	users         map[tailcfg.NodeKey]*tailcfg.User
+	logins        map[tailcfg.NodeKey]*tailcfg.Login
+	updates       map[tailcfg.NodeID]chan updateType
+	authPath      map[string]*AuthPath
+	nodeKeyAuthed map[tailcfg.NodeKey]bool // key => true once authenticated
+}
+
+type AuthPath struct {
+	nodeKey tailcfg.NodeKey
+
+	closeOnce sync.Once
+	ch        chan struct{}
+	success   bool
+}
+
+func (ap *AuthPath) completeSuccessfully() {
+	ap.success = true
+	close(ap.ch)
+}
+
+// CompleteSuccessfully completes the login path successfully, as if
+// the user did the whole auth dance.
+func (ap *AuthPath) CompleteSuccessfully() {
+	ap.closeOnce.Do(ap.completeSuccessfully)
 }
 
 func (s *Server) logf(format string, a ...interface{}) {
@@ -142,6 +168,18 @@ func (s *Server) Node(nodeKey tailcfg.NodeKey) *tailcfg.Node {
 	return s.nodes[nodeKey].Clone()
 }
 
+func (s *Server) AllNodes() (nodes []*tailcfg.Node) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	for _, n := range s.nodes {
+		nodes = append(nodes, n.Clone())
+	}
+	sort.Slice(nodes, func(i, j int) bool {
+		return nodes[i].StableID < nodes[j].StableID
+	})
+	return nodes
+}
+
 func (s *Server) getUser(nodeKey tailcfg.NodeKey) (*tailcfg.User, *tailcfg.Login) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -178,6 +216,56 @@ func (s *Server) getUser(nodeKey tailcfg.NodeKey) (*tailcfg.User, *tailcfg.Login
 	return user, login
 }
 
+// authPathDone returns a close-only struct that's closed when the
+// authPath ("/auth/XXXXXX") has authenticated.
+func (s *Server) authPathDone(authPath string) <-chan struct{} {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if a, ok := s.authPath[authPath]; ok {
+		return a.ch
+	}
+	return nil
+}
+
+func (s *Server) addAuthPath(authPath string, nodeKey tailcfg.NodeKey) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.authPath == nil {
+		s.authPath = map[string]*AuthPath{}
+	}
+	s.authPath[authPath] = &AuthPath{
+		ch:      make(chan struct{}),
+		nodeKey: nodeKey,
+	}
+}
+
+// CompleteAuth marks the provided path or URL (containing
+// "/auth/...")  as successfully authenticated, unblocking any
+// requests blocked on that in serveRegister.
+func (s *Server) CompleteAuth(authPathOrURL string) bool {
+	i := strings.Index(authPathOrURL, "/auth/")
+	if i == -1 {
+		return false
+	}
+	authPath := authPathOrURL[i:]
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	ap, ok := s.authPath[authPath]
+	if !ok {
+		return false
+	}
+	if ap.nodeKey.IsZero() {
+		panic("zero AuthPath.NodeKey")
+	}
+	if s.nodeKeyAuthed == nil {
+		s.nodeKeyAuthed = map[tailcfg.NodeKey]bool{}
+	}
+	s.nodeKeyAuthed[ap.nodeKey] = true
+	ap.CompleteSuccessfully()
+	return true
+}
+
 func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey tailcfg.MachineKey) {
 	var req tailcfg.RegisterRequest
 	if err := s.decode(mkey, r.Body, &req); err != nil {
@@ -189,28 +277,65 @@ func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey tail
 	if req.NodeKey.IsZero() {
 		panic("serveRegister: request has zero node key")
 	}
+	if s.Verbose {
+		j, _ := json.MarshalIndent(req, "", "\t")
+		log.Printf("Got %T: %s", req, j)
+	}
+
+	// If this is a followup request, wait until interactive followup URL visit complete.
+	if req.Followup != "" {
+		followupURL, err := url.Parse(req.Followup)
+		if err != nil {
+			panic(err)
+		}
+		doneCh := s.authPathDone(followupURL.Path)
+		select {
+		case <-r.Context().Done():
+			return
+		case <-doneCh:
+		}
+		// TODO(bradfitz): support a side test API to mark an
+		// auth as failued so we can send an error response in
+		// some follow-ups? For now all are successes.
+	}
 
 	user, login := s.getUser(req.NodeKey)
 	s.mu.Lock()
 	if s.nodes == nil {
 		s.nodes = map[tailcfg.NodeKey]*tailcfg.Node{}
 	}
+
+	machineAuthorized := true // TODO: add Server.RequireMachineAuth
+
 	s.nodes[req.NodeKey] = &tailcfg.Node{
 		ID:                tailcfg.NodeID(user.ID),
 		StableID:          tailcfg.StableNodeID(fmt.Sprintf("TESTCTRL%08x", int(user.ID))),
 		User:              user.ID,
 		Machine:           mkey,
 		Key:               req.NodeKey,
-		MachineAuthorized: true,
+		MachineAuthorized: machineAuthorized,
+	}
+	requireAuth := s.RequireAuth
+	if requireAuth && s.nodeKeyAuthed[req.NodeKey] {
+		requireAuth = false
 	}
 	s.mu.Unlock()
 
+	authURL := ""
+	if requireAuth {
+		randHex := make([]byte, 10)
+		crand.Read(randHex)
+		authPath := fmt.Sprintf("/auth/%x", randHex)
+		s.addAuthPath(authPath, req.NodeKey)
+		authURL = s.BaseURL + authPath
+	}
+
 	res, err := s.encode(mkey, false, tailcfg.RegisterResponse{
 		User:              *user,
 		Login:             *login,
 		NodeKeyExpired:    false,
-		MachineAuthorized: true,
-		AuthURL:           "", // all good; TODO(bradfitz): add ways to not start all good.
+		MachineAuthorized: machineAuthorized,
+		AuthURL:           authURL,
 	})
 	if err != nil {
 		go panic(fmt.Sprintf("serveRegister: encode: %v", err))
@@ -254,6 +379,21 @@ func sendUpdate(dst chan<- updateType, updateType updateType) {
 	}
 }
 
+func (s *Server) UpdateNode(n *tailcfg.Node) (peersToUpdate []tailcfg.NodeID) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if n.Key.IsZero() {
+		panic("zero nodekey")
+	}
+	s.nodes[n.Key] = n.Clone()
+	for _, n2 := range s.nodes {
+		if n.ID != n2.ID {
+			peersToUpdate = append(peersToUpdate, n2.ID)
+		}
+	}
+	return peersToUpdate
+}
+
 func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey tailcfg.MachineKey) {
 	ctx := r.Context()
 
@@ -279,10 +419,8 @@ func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey tailcfg.M
 	if !req.ReadOnly {
 		endpoints := filterInvalidIPv6Endpoints(req.Endpoints)
 		node.Endpoints = endpoints
-		// TODO: more
-		// TODO: register node,
-		//s.UpdateEndpoint(mkey, req.NodeKey,
-		// XXX
+		node.DiscoKey = req.DiscoKey
+		peersToUpdate = s.UpdateNode(node)
 	}
 
 	nodeID := node.ID
@@ -389,6 +527,12 @@ func (s *Server) MapResponse(req *tailcfg.MapRequest) (res *tailcfg.MapResponse,
 		CollectServices: "true",
 		PacketFilter:    tailcfg.FilterAllowAll,
 	}
+	for _, p := range s.AllNodes() {
+		if p.StableID != node.StableID {
+			res.Peers = append(res.Peers, p)
+		}
+	}
+
 	res.Node.Addresses = []netaddr.IPPrefix{
 		netaddr.MustParseIPPrefix(fmt.Sprintf("100.64.%d.%d/32", uint8(node.ID>>8), uint8(node.ID))),
 	}

types/wgkey/key.go

@@ -78,8 +78,16 @@ func (k Key) HexString() string { return hex.EncodeToString(k[:]) }
 func (k Key) Equal(k2 Key) bool { return subtle.ConstantTimeCompare(k[:], k2[:]) == 1 }
 
 func (k *Key) ShortString() string {
-	long := k.Base64()
-	return "[" + long[0:5] + "]"
+	// The goal here is to generate "[" + base64.StdEncoding.EncodeToString(k[:])[:5] + "]".
+	// Since we only care about the first 5 characters, it suffices to encode the first 4 bytes of k.
+	// Encoding those 4 bytes requires 8 bytes.
+	// Make dst have size 9, to fit the leading '[' plus those 8 bytes.
+	// We slice the unused ones away at the end.
+	dst := make([]byte, 9)
+	dst[0] = '['
+	base64.StdEncoding.Encode(dst[1:], k[:4])
+	dst[6] = ']'
+	return string(dst[:7])
 }
 
 func (k *Key) IsZero() bool {
@@ -106,11 +114,10 @@ func (k *Key) UnmarshalJSON(b []byte) error {
 		return errors.New("wgkey.Key: UnmarshalJSON not given a string")
 	}
 	b = b[1 : len(b)-1]
-	key, err := ParseHex(string(b))
-	if err != nil {
-		return fmt.Errorf("wgkey.Key: UnmarshalJSON: %v", err)
+	if len(b) != 2*Size {
+		return fmt.Errorf("wgkey.Key: UnmarshalJSON input wrong size: %d", len(b))
 	}
-	copy(k[:], key[:])
+	hex.Decode(k[:], b)
 	return nil
 }
 

types/wgkey/key_test.go

@@ -156,3 +156,28 @@ func BenchmarkMarshalJSON(b *testing.B) {
 		}
 	}
 }
+
+func BenchmarkUnmarshalJSON(b *testing.B) {
+	b.ReportAllocs()
+	var k Key
+	buf, err := k.MarshalJSON()
+	if err != nil {
+		b.Fatal(err)
+	}
+	for i := 0; i < b.N; i++ {
+		err := k.UnmarshalJSON(buf)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+var sinkString string
+
+func BenchmarkShortString(b *testing.B) {
+	b.ReportAllocs()
+	var k Key
+	for i := 0; i < b.N; i++ {
+		sinkString = k.ShortString()
+	}
+}

util/dnsname/dnsname.go

@@ -24,13 +24,16 @@ func ToFQDN(s string) (FQDN, error) {
 	if isValidFQDN(s) {
 		return FQDN(s), nil
 	}
-	if len(s) == 0 {
+	if len(s) == 0 || s == "." {
 		return FQDN("."), nil
 	}
 
 	if s[len(s)-1] == '.' {
 		s = s[:len(s)-1]
 	}
+	if s[0] == '.' {
+		s = s[1:]
+	}
 	if len(s) > maxNameLength {
 		return "", fmt.Errorf("%q is too long to be a DNS name", s)
 	}

util/dnsname/dnsname_test.go

@@ -20,11 +20,12 @@ func TestFQDN(t *testing.T) {
 		{".", ".", false, 0},
 		{"foo.com", "foo.com.", false, 2},
 		{"foo.com.", "foo.com.", false, 2},
+		{".foo.com.", "foo.com.", false, 2},
+		{".foo.com", "foo.com.", false, 2},
 		{"com", "com.", false, 1},
 		{"www.tailscale.com", "www.tailscale.com.", false, 3},
 		{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com", "", true, 0},
 		{strings.Repeat("aaaaa.", 60) + "com", "", true, 0},
-		{".com", "", true, 0},
 		{"foo..com", "", true, 0},
 	}
 

version/race.go

@@ -0,0 +1,11 @@
+// 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 race
+
+package version
+
+// IsRace reports whether the current binary was built with the Go
+// race detector enabled.
+func IsRace() bool { return true }

version/race_off.go

@@ -0,0 +1,11 @@
+// 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 !race
+
+package version
+
+// IsRace reports whether the current binary was built with the Go
+// race detector enabled.
+func IsRace() bool { return false }

wf/firewall.go

@@ -0,0 +1,510 @@
+// 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 wf
+
+import (
+	"fmt"
+	"os"
+
+	"golang.org/x/sys/windows"
+	"inet.af/netaddr"
+	"inet.af/wf"
+)
+
+// Known addresses.
+var (
+	linkLocalRange           = netaddr.MustParseIPPrefix("ff80::/10")
+	linkLocalDHCPMulticast   = netaddr.MustParseIP("ff02::1:2")
+	siteLocalDHCPMulticast   = netaddr.MustParseIP("ff05::1:3")
+	linkLocalRouterMulticast = netaddr.MustParseIP("ff02::2")
+)
+
+type direction int
+
+const (
+	directionInbound direction = iota
+	directionOutbound
+	directionBoth
+)
+
+type protocol int
+
+const (
+	protocolV4 protocol = iota
+	protocolV6
+	protocolAll
+)
+
+// getLayers returns the wf.LayerIDs where the rules should be added based
+// on the protocol and direction.
+func (p protocol) getLayers(d direction) []wf.LayerID {
+	var layers []wf.LayerID
+	if p == protocolAll || p == protocolV4 {
+		if d == directionBoth || d == directionInbound {
+			layers = append(layers, wf.LayerALEAuthRecvAcceptV4)
+		}
+		if d == directionBoth || d == directionOutbound {
+			layers = append(layers, wf.LayerALEAuthConnectV4)
+		}
+	}
+	if p == protocolAll || p == protocolV6 {
+		if d == directionBoth || d == directionInbound {
+			layers = append(layers, wf.LayerALEAuthRecvAcceptV6)
+		}
+		if d == directionBoth || d == directionOutbound {
+			layers = append(layers, wf.LayerALEAuthConnectV6)
+		}
+	}
+	return layers
+}
+
+func ruleName(action wf.Action, l wf.LayerID, name string) string {
+	switch l {
+	case wf.LayerALEAuthConnectV4:
+		return fmt.Sprintf("%s outbound %s (IPv4)", action, name)
+	case wf.LayerALEAuthConnectV6:
+		return fmt.Sprintf("%s outbound %s (IPv6)", action, name)
+	case wf.LayerALEAuthRecvAcceptV4:
+		return fmt.Sprintf("%s inbound %s (IPv4)", action, name)
+	case wf.LayerALEAuthRecvAcceptV6:
+		return fmt.Sprintf("%s inbound %s (IPv6)", action, name)
+	}
+	return ""
+}
+
+// Firewall uses the Windows Filtering Platform to implement a network firewall.
+type Firewall struct {
+	luid       uint64
+	providerID wf.ProviderID
+	sublayerID wf.SublayerID
+	session    *wf.Session
+
+	permittedRoutes map[netaddr.IPPrefix][]*wf.Rule
+}
+
+// New returns a new Firewall for the provdied interface ID.
+func New(luid uint64) (*Firewall, error) {
+	session, err := wf.New(&wf.Options{
+		Name:    "Tailscale firewall",
+		Dynamic: true,
+	})
+	if err != nil {
+		return nil, err
+	}
+	wguid, err := windows.GenerateGUID()
+	if err != nil {
+		return nil, err
+	}
+	providerID := wf.ProviderID(wguid)
+	if err := session.AddProvider(&wf.Provider{
+		ID:   providerID,
+		Name: "Tailscale provider",
+	}); err != nil {
+		return nil, err
+	}
+	wguid, err = windows.GenerateGUID()
+	if err != nil {
+		return nil, err
+	}
+	sublayerID := wf.SublayerID(wguid)
+	if err := session.AddSublayer(&wf.Sublayer{
+		ID:     sublayerID,
+		Name:   "Tailscale permissive and blocking filters",
+		Weight: 0,
+	}); err != nil {
+		return nil, err
+	}
+	f := &Firewall{
+		luid:            luid,
+		session:         session,
+		providerID:      providerID,
+		sublayerID:      sublayerID,
+		permittedRoutes: make(map[netaddr.IPPrefix][]*wf.Rule),
+	}
+	if err := f.enable(); err != nil {
+		return nil, err
+	}
+	return f, nil
+}
+
+type weight uint64
+
+const (
+	weightTailscaleTraffic weight = 15
+	weightKnownTraffic     weight = 12
+	weightCatchAll         weight = 0
+)
+
+func (f *Firewall) enable() error {
+	if err := f.permitTailscaleService(weightTailscaleTraffic); err != nil {
+		return fmt.Errorf("permitTailscaleService failed: %w", err)
+	}
+
+	if err := f.permitTunInterface(weightTailscaleTraffic); err != nil {
+		return fmt.Errorf("permitTunInterface failed: %w", err)
+	}
+
+	if err := f.permitDNS(weightTailscaleTraffic); err != nil {
+		return fmt.Errorf("permitDNS failed: %w", err)
+	}
+
+	if err := f.permitLoopback(weightKnownTraffic); err != nil {
+		return fmt.Errorf("permitLoopback failed: %w", err)
+	}
+
+	if err := f.permitDHCPv4(weightKnownTraffic); err != nil {
+		return fmt.Errorf("permitDHCPv4 failed: %w", err)
+	}
+
+	if err := f.permitDHCPv6(weightKnownTraffic); err != nil {
+		return fmt.Errorf("permitDHCPv6 failed: %w", err)
+	}
+
+	if err := f.permitNDP(weightKnownTraffic); err != nil {
+		return fmt.Errorf("permitNDP failed: %w", err)
+	}
+
+	/* TODO: actually evaluate if this does anything and if we need this. It's layer 2; our other rules are layer 3.
+	 *  In other words, if somebody complains, try enabling it. For now, keep it off.
+	 * TODO(maisem): implement this.
+	err = permitHyperV(session, baseObjects, weightKnownTraffic)
+	if err != nil {
+		return wrapErr(err)
+	}
+	*/
+
+	if err := f.blockAll(weightCatchAll); err != nil {
+		return fmt.Errorf("blockAll failed: %w", err)
+	}
+	return nil
+}
+
+// UpdatedPermittedRoutes adds rules to allow incoming and outgoing connections
+// from the provided prefixes. It will also remove rules for routes that were
+// previously added but have been removed.
+func (f *Firewall) UpdatePermittedRoutes(newRoutes []netaddr.IPPrefix) error {
+	var routesToAdd []netaddr.IPPrefix
+	routeMap := make(map[netaddr.IPPrefix]bool)
+	for _, r := range newRoutes {
+		routeMap[r] = true
+		if _, ok := f.permittedRoutes[r]; !ok {
+			routesToAdd = append(routesToAdd, r)
+		}
+	}
+	var routesToRemove []netaddr.IPPrefix
+	for r := range f.permittedRoutes {
+		if !routeMap[r] {
+			routesToRemove = append(routesToRemove, r)
+		}
+	}
+	for _, r := range routesToRemove {
+		for _, rule := range f.permittedRoutes[r] {
+			if err := f.session.DeleteRule(rule.ID); err != nil {
+				return err
+			}
+		}
+		delete(f.permittedRoutes, r)
+	}
+	for _, r := range routesToAdd {
+		conditions := []*wf.Match{
+			{
+				Field: wf.FieldIPRemoteAddress,
+				Op:    wf.MatchTypeEqual,
+				Value: r,
+			},
+		}
+		var p protocol
+		if r.IP.Is4() {
+			p = protocolV4
+		} else {
+			p = protocolV6
+		}
+		rules, err := f.addRules("local route", weightKnownTraffic, conditions, wf.ActionPermit, p, directionBoth)
+		if err != nil {
+			return err
+		}
+		f.permittedRoutes[r] = rules
+	}
+	return nil
+}
+
+func (f *Firewall) newRule(name string, w weight, layer wf.LayerID, conditions []*wf.Match, action wf.Action) (*wf.Rule, error) {
+	id, err := windows.GenerateGUID()
+	if err != nil {
+		return nil, err
+	}
+	return &wf.Rule{
+		Name:       ruleName(action, layer, name),
+		ID:         wf.RuleID(id),
+		Provider:   f.providerID,
+		Sublayer:   f.sublayerID,
+		Layer:      layer,
+		Weight:     uint64(w),
+		Conditions: conditions,
+		Action:     action,
+	}, nil
+}
+
+func (f *Firewall) addRules(name string, w weight, conditions []*wf.Match, action wf.Action, p protocol, d direction) ([]*wf.Rule, error) {
+	var rules []*wf.Rule
+	for _, l := range p.getLayers(d) {
+		r, err := f.newRule(name, w, l, conditions, action)
+		if err != nil {
+			return nil, err
+		}
+		if err := f.session.AddRule(r); err != nil {
+			return nil, err
+		}
+		rules = append(rules, r)
+	}
+	return rules, nil
+}
+
+func (f *Firewall) blockAll(w weight) error {
+	_, err := f.addRules("all", w, nil, wf.ActionBlock, protocolAll, directionBoth)
+	return err
+}
+
+func (f *Firewall) permitNDP(w weight) error {
+	// These are aliased according to:
+	// https://social.msdn.microsoft.com/Forums/azure/en-US/eb2aa3cd-5f1c-4461-af86-61e7d43ccc23/filtering-icmp-by-type-code?forum=wfp
+	fieldICMPType := wf.FieldIPLocalPort
+	fieldICMPCode := wf.FieldIPRemotePort
+
+	var icmpConditions = func(t, c uint16, remoteAddress interface{}) []*wf.Match {
+		conditions := []*wf.Match{
+			{
+				Field: wf.FieldIPProtocol,
+				Op:    wf.MatchTypeEqual,
+				Value: wf.IPProtoICMPV6,
+			},
+			{
+				Field: fieldICMPType,
+				Op:    wf.MatchTypeEqual,
+				Value: t,
+			},
+			{
+				Field: fieldICMPCode,
+				Op:    wf.MatchTypeEqual,
+				Value: c,
+			},
+		}
+		if remoteAddress != nil {
+			conditions = append(conditions, &wf.Match{
+				Field: wf.FieldIPRemoteAddress,
+				Op:    wf.MatchTypeEqual,
+				Value: linkLocalRouterMulticast,
+			})
+		}
+		return conditions
+	}
+	/* TODO: actually handle the hop limit somehow! The rules should vaguely be:
+	 *  - icmpv6 133: must be outgoing, dst must be FF02::2/128, hop limit must be 255
+	 *  - icmpv6 134: must be incoming, src must be FE80::/10, hop limit must be 255
+	 *  - icmpv6 135: either incoming or outgoing, hop limit must be 255
+	 *  - icmpv6 136: either incoming or outgoing, hop limit must be 255
+	 *  - icmpv6 137: must be incoming, src must be FE80::/10, hop limit must be 255
+	 */
+
+	//
+	// Router Solicitation Message
+	// ICMP type 133, code 0. Outgoing.
+	//
+	conditions := icmpConditions(133, 0, linkLocalRouterMulticast)
+	if _, err := f.addRules("NDP type 133", w, conditions, wf.ActionPermit, protocolV6, directionOutbound); err != nil {
+		return err
+	}
+
+	//
+	// Router Advertisement Message
+	// ICMP type 134, code 0. Incoming.
+	//
+	conditions = icmpConditions(134, 0, linkLocalRange)
+	if _, err := f.addRules("NDP type 134", w, conditions, wf.ActionPermit, protocolV6, directionInbound); err != nil {
+		return err
+	}
+
+	//
+	// Neighbor Solicitation Message
+	// ICMP type 135, code 0. Bi-directional.
+	//
+	conditions = icmpConditions(135, 0, nil)
+	if _, err := f.addRules("NDP type 135", w, conditions, wf.ActionPermit, protocolV6, directionBoth); err != nil {
+		return err
+	}
+
+	//
+	// Neighbor Advertisement Message
+	// ICMP type 136, code 0. Bi-directional.
+	//
+	conditions = icmpConditions(136, 0, nil)
+	if _, err := f.addRules("NDP type 136", w, conditions, wf.ActionPermit, protocolV6, directionBoth); err != nil {
+		return err
+	}
+
+	//
+	// Redirect Message
+	// ICMP type 137, code 0. Incoming.
+	//
+	conditions = icmpConditions(137, 0, linkLocalRange)
+	if _, err := f.addRules("NDP type 137", w, conditions, wf.ActionPermit, protocolV6, directionInbound); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (f *Firewall) permitDHCPv6(w weight) error {
+	var dhcpConditions = func(remoteAddrs ...interface{}) []*wf.Match {
+		conditions := []*wf.Match{
+			{
+				Field: wf.FieldIPProtocol,
+				Op:    wf.MatchTypeEqual,
+				Value: wf.IPProtoUDP,
+			},
+			{
+				Field: wf.FieldIPLocalAddress,
+				Op:    wf.MatchTypeEqual,
+				Value: linkLocalRange,
+			},
+			{
+				Field: wf.FieldIPLocalPort,
+				Op:    wf.MatchTypeEqual,
+				Value: uint16(546),
+			},
+			{
+				Field: wf.FieldIPRemotePort,
+				Op:    wf.MatchTypeEqual,
+				Value: uint16(547),
+			},
+		}
+		for _, a := range remoteAddrs {
+			conditions = append(conditions, &wf.Match{
+				Field: wf.FieldIPRemoteAddress,
+				Op:    wf.MatchTypeEqual,
+				Value: a,
+			})
+		}
+		return conditions
+	}
+	conditions := dhcpConditions(linkLocalDHCPMulticast, siteLocalDHCPMulticast)
+	if _, err := f.addRules("DHCP request", w, conditions, wf.ActionPermit, protocolV6, directionOutbound); err != nil {
+		return err
+	}
+	conditions = dhcpConditions(linkLocalRange)
+	if _, err := f.addRules("DHCP response", w, conditions, wf.ActionPermit, protocolV6, directionInbound); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (f *Firewall) permitDHCPv4(w weight) error {
+	var dhcpConditions = func(remoteAddrs ...interface{}) []*wf.Match {
+		conditions := []*wf.Match{
+			{
+				Field: wf.FieldIPProtocol,
+				Op:    wf.MatchTypeEqual,
+				Value: wf.IPProtoUDP,
+			},
+			{
+				Field: wf.FieldIPLocalPort,
+				Op:    wf.MatchTypeEqual,
+				Value: uint16(68),
+			},
+			{
+				Field: wf.FieldIPRemotePort,
+				Op:    wf.MatchTypeEqual,
+				Value: uint16(67),
+			},
+		}
+		for _, a := range remoteAddrs {
+			conditions = append(conditions, &wf.Match{
+				Field: wf.FieldIPRemoteAddress,
+				Op:    wf.MatchTypeEqual,
+				Value: a,
+			})
+		}
+		return conditions
+	}
+	conditions := dhcpConditions(netaddr.IPv4(255, 255, 255, 255))
+	if _, err := f.addRules("DHCP request", w, conditions, wf.ActionPermit, protocolV4, directionOutbound); err != nil {
+		return err
+	}
+
+	conditions = dhcpConditions()
+	if _, err := f.addRules("DHCP response", w, conditions, wf.ActionPermit, protocolV4, directionInbound); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (f *Firewall) permitTunInterface(w weight) error {
+	condition := []*wf.Match{
+		{
+			Field: wf.FieldIPLocalInterface,
+			Op:    wf.MatchTypeEqual,
+			Value: f.luid,
+		},
+	}
+	_, err := f.addRules("on TUN", w, condition, wf.ActionPermit, protocolAll, directionBoth)
+	return err
+}
+
+func (f *Firewall) permitLoopback(w weight) error {
+	condition := []*wf.Match{
+		{
+			Field: wf.FieldFlags,
+			Op:    wf.MatchTypeEqual,
+			Value: wf.ConditionFlagIsLoopback,
+		},
+	}
+	_, err := f.addRules("on loopback", w, condition, wf.ActionPermit, protocolAll, directionBoth)
+	return err
+}
+
+func (f *Firewall) permitDNS(w weight) error {
+	conditions := []*wf.Match{
+		{
+			Field: wf.FieldIPRemotePort,
+			Op:    wf.MatchTypeEqual,
+			Value: uint16(53),
+		},
+		// Repeat the condition type for logical OR.
+		{
+			Field: wf.FieldIPProtocol,
+			Op:    wf.MatchTypeEqual,
+			Value: wf.IPProtoUDP,
+		},
+		{
+			Field: wf.FieldIPProtocol,
+			Op:    wf.MatchTypeEqual,
+			Value: wf.IPProtoTCP,
+		},
+	}
+	_, err := f.addRules("DNS", w, conditions, wf.ActionPermit, protocolAll, directionBoth)
+	return err
+}
+
+func (f *Firewall) permitTailscaleService(w weight) error {
+	currentFile, err := os.Executable()
+	if err != nil {
+		return err
+	}
+
+	appID, err := wf.AppID(currentFile)
+	if err != nil {
+		return fmt.Errorf("could not get app id for %q: %w", currentFile, err)
+	}
+	conditions := []*wf.Match{
+		{
+			Field: wf.FieldALEAppID,
+			Op:    wf.MatchTypeEqual,
+			Value: appID,
+		},
+	}
+	_, err = f.addRules("unrestricted traffic for Tailscale service", w, conditions, wf.ActionPermit, protocolAll, directionBoth)
+	return err
+}

wgengine/bench/bench.go

@@ -80,7 +80,7 @@ func main() {
 
 	// tx=134236 rx=133166 (1070 = 0.80% loss) (1088.9 Mbits/sec)
 	case 101:
-		setupWGTest(logf, traf, Addr1, Addr2)
+		setupWGTest(nil, logf, traf, Addr1, Addr2)
 
 	default:
 		log.Fatalf("provide a valid test number (0..n)")

wgengine/bench/bench_test.go

@@ -43,7 +43,7 @@ func BenchmarkBatchTCP(b *testing.B) {
 
 func BenchmarkWireGuardTest(b *testing.B) {
 	run(b, func(logf logger.Logf, traf *TrafficGen) {
-		setupWGTest(logf, traf, Addr1, Addr2)
+		setupWGTest(b, logf, traf, Addr1, Addr2)
 	})
 }
 

wgengine/bench/trafficgen.go

@@ -180,6 +180,7 @@ func (t *TrafficGen) Generate(b []byte, ofs int) int {
 // GotPacket processes a packet that came back on the receive side.
 func (t *TrafficGen) GotPacket(b []byte, ofs int) {
 	t.mu.Lock()
+	defer t.mu.Unlock()
 
 	s := &t.cur
 	seq := int64(binary.BigEndian.Uint64(
@@ -203,9 +204,6 @@ func (t *TrafficGen) GotPacket(b []byte, ofs int) {
 
 	f := t.onFirstPacket
 	t.onFirstPacket = nil
-
-	t.mu.Unlock()
-
 	if f != nil {
 		f()
 	}

wgengine/bench/wg.go

@@ -5,11 +5,12 @@
 package main
 
 import (
+	"errors"
 	"io"
 	"log"
 	"os"
-	"strings"
 	"sync"
+	"testing"
 
 	"github.com/tailscale/wireguard-go/tun"
 	"inet.af/netaddr"
@@ -25,7 +26,7 @@ import (
 	"tailscale.com/wgengine/wgcfg"
 )
 
-func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
+func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
 	l1 := logger.WithPrefix(logf, "e1: ")
 	k1, err := wgkey.NewPrivate()
 	if err != nil {
@@ -49,6 +50,9 @@ func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
 	if err != nil {
 		log.Fatalf("e1 init: %v", err)
 	}
+	if b != nil {
+		b.Cleanup(e1.Close)
+	}
 
 	l2 := logger.WithPrefix(logf, "e2: ")
 	k2, err := wgkey.NewPrivate()
@@ -73,6 +77,9 @@ func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
 	if err != nil {
 		log.Fatalf("e2 init: %v", err)
 	}
+	if b != nil {
+		b.Cleanup(e2.Close)
+	}
 
 	e1.SetFilter(filter.NewAllowAllForTest(l1))
 	e2.SetFilter(filter.NewAllowAllForTest(l2))
@@ -80,15 +87,25 @@ func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
 	var wait sync.WaitGroup
 	wait.Add(2)
 
+	var e1waitDoneOnce sync.Once
 	e1.SetStatusCallback(func(st *wgengine.Status, err error) {
+		if errors.Is(err, wgengine.ErrEngineClosing) {
+			return
+		}
 		if err != nil {
 			log.Fatalf("e1 status err: %v", err)
 		}
 		logf("e1 status: %v", *st)
 
 		var eps []string
+		var ipps []netaddr.IPPort
 		for _, ep := range st.LocalAddrs {
 			eps = append(eps, ep.Addr.String())
+			ipps = append(ipps, ep.Addr)
+		}
+		endpoint := wgcfg.Endpoints{
+			PublicKey: c1.PrivateKey.Public(),
+			IPPorts:   wgcfg.NewIPPortSet(ipps...),
 		}
 
 		n := tailcfg.Node{
@@ -107,22 +124,32 @@ func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
 		p := wgcfg.Peer{
 			PublicKey:  c1.PrivateKey.Public(),
 			AllowedIPs: []netaddr.IPPrefix{a1},
-			Endpoints:  strings.Join(eps, ","),
+			Endpoints:  endpoint,
 		}
 		c2.Peers = []wgcfg.Peer{p}
 		e2.Reconfig(&c2, &router.Config{}, new(dns.Config))
-		wait.Done()
+		e1waitDoneOnce.Do(wait.Done)
 	})
 
+	var e2waitDoneOnce sync.Once
 	e2.SetStatusCallback(func(st *wgengine.Status, err error) {
+		if errors.Is(err, wgengine.ErrEngineClosing) {
+			return
+		}
 		if err != nil {
 			log.Fatalf("e2 status err: %v", err)
 		}
 		logf("e2 status: %v", *st)
 
 		var eps []string
+		var ipps []netaddr.IPPort
 		for _, ep := range st.LocalAddrs {
 			eps = append(eps, ep.Addr.String())
+			ipps = append(ipps, ep.Addr)
+		}
+		endpoint := wgcfg.Endpoints{
+			PublicKey: c2.PrivateKey.Public(),
+			IPPorts:   wgcfg.NewIPPortSet(ipps...),
 		}
 
 		n := tailcfg.Node{
@@ -141,11 +168,11 @@ func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
 		p := wgcfg.Peer{
 			PublicKey:  c2.PrivateKey.Public(),
 			AllowedIPs: []netaddr.IPPrefix{a2},
-			Endpoints:  strings.Join(eps, ","),
+			Endpoints:  endpoint,
 		}
 		c1.Peers = []wgcfg.Peer{p}
 		e1.Reconfig(&c1, &router.Config{}, new(dns.Config))
-		wait.Done()
+		e2waitDoneOnce.Do(wait.Done)
 	})
 
 	// Not using DERP in this test (for now?).

wgengine/magicsock/legacy.go

@@ -10,7 +10,6 @@ import (
 	"crypto/subtle"
 	"encoding/binary"
 	"errors"
-	"fmt"
 	"hash"
 	"net"
 	"strings"
@@ -27,6 +26,7 @@ import (
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/wgkey"
+	"tailscale.com/wgengine/wgcfg"
 )
 
 var (
@@ -34,7 +34,11 @@ var (
 	errDisabled       = errors.New("magicsock: legacy networking disabled")
 )
 
-func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs string) (conn.Endpoint, error) {
+// createLegacyEndpointLocked creates a new wireguard-go endpoint for a legacy connection.
+// pk is the public key of the remote peer. addrs is the ordered set of addresses for the remote peer.
+// rawDest is the encoded wireguard-go endpoint string. It should be treated as a black box.
+// It is provided so that addrSet.DstToString can return it when requested by wireguard-go.
+func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs wgcfg.IPPortSet, rawDest string) (conn.Endpoint, error) {
 	if c.disableLegacy {
 		return nil, errDisabled
 	}
@@ -43,17 +47,9 @@ func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs string) (conn.End
 		Logf:      c.logf,
 		publicKey: pk,
 		curAddr:   -1,
+		rawdst:    rawDest,
 	}
-
-	if addrs != "" {
-		for _, ep := range strings.Split(addrs, ",") {
-			ipp, err := netaddr.ParseIPPort(ep)
-			if err != nil {
-				return nil, fmt.Errorf("bogus address %q", ep)
-			}
-			a.ipPorts = append(a.ipPorts, ipp)
-		}
-	}
+	a.ipPorts = append(a.ipPorts, addrs.IPPorts()...)
 
 	// If this endpoint is being updated, remember its old set of
 	// endpoints so we can remove any (from c.addrsByUDP) that are
@@ -384,6 +380,9 @@ type addrSet struct {
 	// set to a better one. This is only to suppress some
 	// redundant logs.
 	loggedLogPriMask uint32
+
+	// rawdst is the destination string from/for wireguard-go.
+	rawdst string
 }
 
 // derpID returns this addrSet's home DERP node, or 0 if none is found.
@@ -426,17 +425,7 @@ func (a *addrSet) DstToBytes() []byte {
 	return packIPPort(a.dst())
 }
 func (a *addrSet) DstToString() string {
-	var addrs []string
-	for _, addr := range a.ipPorts {
-		addrs = append(addrs, addr.String())
-	}
-
-	a.mu.Lock()
-	defer a.mu.Unlock()
-	if a.roamAddr != nil {
-		addrs = append(addrs, a.roamAddr.String())
-	}
-	return strings.Join(addrs, ",")
+	return a.rawdst
 }
 func (a *addrSet) DstIP() net.IP {
 	return a.dst().IP.IPAddr().IP // TODO: add netaddr accessor to cut an alloc here?

wgengine/magicsock/magicsock.go

@@ -11,6 +11,7 @@ import (
 	"context"
 	crand "crypto/rand"
 	"encoding/binary"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"hash/fnv"
@@ -27,7 +28,6 @@ import (
 	"time"
 
 	"github.com/tailscale/wireguard-go/conn"
-	"go4.org/mem"
 	"golang.org/x/crypto/nacl/box"
 	"golang.org/x/time/rate"
 	"inet.af/netaddr"
@@ -401,7 +401,7 @@ type Options struct {
 	// and 10 seconds seems like a good trade-off between often
 	// enough and not too often.) The provided func is called
 	// while holding userspaceEngine.wgLock and likely calls
-	// Conn.CreateEndpoint, which acquires Conn.mu. As such, you
+	// Conn.ParseEndpoint, which acquires Conn.mu. As such, you
 	// should not hold Conn.mu while calling it.
 	NoteRecvActivity func(tailcfg.DiscoKey)
 
@@ -1696,7 +1696,7 @@ func (c *Conn) processDERPReadResult(dm derpReadResult, b []byte) (n int, ep con
 		if discoEp == nil && c.noteRecvActivity != nil {
 			didNoteRecvActivity = true
 			c.mu.Unlock()          // release lock before calling noteRecvActivity
-			c.noteRecvActivity(dk) // (calls back into CreateEndpoint)
+			c.noteRecvActivity(dk) // (calls back into ParseEndpoint)
 			// Now require the lock. No invariants need to be rechecked; just
 			// 1-2 map lookups follow that are harmless if, say, the peer has
 			// been deleted during this time.
@@ -1837,7 +1837,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bo
 		// We don't have an active endpoint for this sender but we knew about the node, so
 		// it's an idle endpoint that doesn't yet exist in the wireguard config. We now have
 		// to notify the userspace engine (via noteRecvActivity) so wireguard-go can create
-		// an Endpoint (ultimately calling our CreateEndpoint).
+		// an Endpoint (ultimately calling our ParseEndpoint).
 		c.logf("magicsock: got disco message from idle peer, starting lazy conf for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
 		if c.noteRecvActivity == nil {
 			c.logf("magicsock: [unexpected] have node without endpoint, without c.noteRecvActivity hook")
@@ -1851,7 +1851,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bo
 		// We can't hold Conn.mu while calling noteRecvActivity.
 		// noteRecvActivity acquires userspaceEngine.wgLock (and per our
 		// lock ordering rules: wgLock must come first), and also calls
-		// back into our Conn.CreateEndpoint, which would double-acquire
+		// back into our Conn.ParseEndpoint, which would double-acquire
 		// Conn.mu.
 		c.mu.Unlock()
 		c.noteRecvActivity(sender)
@@ -2736,44 +2736,32 @@ func packIPPort(ua netaddr.IPPort) []byte {
 }
 
 // ParseEndpoint is called by WireGuard to connect to an endpoint.
-//
-// keyAddrs is the 32 byte public key of the peer followed by addrs.
-// Addrs is either:
-//
-//  1) a comma-separated list of UDP ip:ports (the peer doesn't have a discovery key)
-//  2) "<hex-discovery-key>.disco.tailscale:12345", a magic value that means the peer
-//     is running code that supports active discovery, so CreateEndpoint returns
-//     a discoEndpoint.
-func (c *Conn) ParseEndpoint(keyAddrs string) (conn.Endpoint, error) {
-	if len(keyAddrs) < 32 {
-		c.logf("[unexpected] ParseEndpoint keyAddrs too short: %q", keyAddrs)
-		return nil, errors.New("endpoint string too short")
+// endpointStr is a json-serialized wgcfg.Endpoints struct.
+// If those Endpoints contain an active discovery key, ParseEndpoint returns a discoEndpoint.
+// Otherwise it returns a legacy endpoint.
+func (c *Conn) ParseEndpoint(endpointStr string) (conn.Endpoint, error) {
+	var endpoints wgcfg.Endpoints
+	err := json.Unmarshal([]byte(endpointStr), &endpoints)
+	if err != nil {
+		return nil, fmt.Errorf("magicsock: ParseEndpoint: json.Unmarshal failed on %q: %w", endpointStr, err)
 	}
-	var pk key.Public
-	copy(pk[:], keyAddrs)
-	addrs := keyAddrs[len(pk):]
+	pk := key.Public(endpoints.PublicKey)
+	discoKey := endpoints.DiscoKey
+	c.logf("magicsock: ParseEndpoint: key=%s: disco=%s ipps=%s", pk.ShortString(), discoKey.ShortString(), derpStr(endpoints.IPPorts.String()))
+
 	c.mu.Lock()
 	defer c.mu.Unlock()
-
-	c.logf("magicsock: ParseEndpoint: key=%s: %s", pk.ShortString(), derpStr(addrs))
-
-	if !strings.HasSuffix(addrs, wgcfg.EndpointDiscoSuffix) {
-		return c.createLegacyEndpointLocked(pk, addrs)
-	}
-
-	discoHex := strings.TrimSuffix(addrs, wgcfg.EndpointDiscoSuffix)
-	discoKey, err := key.NewPublicFromHexMem(mem.S(discoHex))
-	if err != nil {
-		return nil, fmt.Errorf("magicsock: invalid discokey endpoint %q for %v: %w", addrs, pk.ShortString(), err)
+	if discoKey.IsZero() {
+		return c.createLegacyEndpointLocked(pk, endpoints.IPPorts, endpointStr)
 	}
 	de := &discoEndpoint{
-		c:                  c,
-		publicKey:          tailcfg.NodeKey(pk),        // peer public key (for WireGuard + DERP)
-		discoKey:           tailcfg.DiscoKey(discoKey), // for discovery mesages
-		discoShort:         tailcfg.DiscoKey(discoKey).ShortString(),
-		wgEndpointHostPort: addrs,
-		sentPing:           map[stun.TxID]sentPing{},
-		endpointState:      map[netaddr.IPPort]*endpointState{},
+		c:             c,
+		publicKey:     tailcfg.NodeKey(pk),        // peer public key (for WireGuard + DERP)
+		discoKey:      tailcfg.DiscoKey(discoKey), // for discovery mesages
+		discoShort:    tailcfg.DiscoKey(discoKey).ShortString(),
+		wgEndpoint:    endpointStr,
+		sentPing:      map[stun.TxID]sentPing{},
+		endpointState: map[netaddr.IPPort]*endpointState{},
 	}
 	de.initFakeUDPAddr()
 	de.updateFromNode(c.nodeOfDisco[de.discoKey])
@@ -3110,12 +3098,12 @@ type discoEndpoint struct {
 	numStopAndResetAtomic int64
 
 	// These fields are initialized once and never modified.
-	c                  *Conn
-	publicKey          tailcfg.NodeKey  // peer public key (for WireGuard + DERP)
-	discoKey           tailcfg.DiscoKey // for discovery mesages
-	discoShort         string           // ShortString of discoKey
-	fakeWGAddr         netaddr.IPPort   // the UDP address we tell wireguard-go we're using
-	wgEndpointHostPort string           // string from CreateEndpoint: "<hex-discovery-key>.disco.tailscale:12345"
+	c          *Conn
+	publicKey  tailcfg.NodeKey  // peer public key (for WireGuard + DERP)
+	discoKey   tailcfg.DiscoKey // for discovery mesages
+	discoShort string           // ShortString of discoKey
+	fakeWGAddr netaddr.IPPort   // the UDP address we tell wireguard-go we're using
+	wgEndpoint string           // string from ParseEndpoint, holds a JSON-serialized wgcfg.Endpoints
 
 	// Owned by Conn.mu:
 	lastPingFrom netaddr.IPPort
@@ -3295,7 +3283,7 @@ func (de *discoEndpoint) String() string {
 func (de *discoEndpoint) ClearSrc()           {}
 func (de *discoEndpoint) SrcToString() string { panic("unused") } // unused by wireguard-go
 func (de *discoEndpoint) SrcIP() net.IP       { panic("unused") } // unused by wireguard-go
-func (de *discoEndpoint) DstToString() string { return de.wgEndpointHostPort }
+func (de *discoEndpoint) DstToString() string { return de.wgEndpoint }
 func (de *discoEndpoint) DstIP() net.IP       { panic("unused") }
 func (de *discoEndpoint) DstToBytes() []byte  { return packIPPort(de.fakeWGAddr) }
 

wgengine/magicsock/magicsock_test.go

@@ -10,6 +10,7 @@ import (
 	crand "crypto/rand"
 	"crypto/tls"
 	"encoding/binary"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -167,7 +168,7 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
 	tsTun.SetFilter(filter.NewAllowAllForTest(logf))
 
 	wgLogger := wglog.NewLogger(logf)
-	dev := device.NewDevice(tsTun, conn.Bind(), wgLogger.DeviceLogger, new(device.DeviceOptions))
+	dev := device.NewDevice(tsTun, conn.Bind(), wgLogger.DeviceLogger)
 	dev.Up()
 
 	// Wait for magicsock to connect up to DERP.
@@ -453,7 +454,7 @@ func makeConfigs(t *testing.T, addrs []netaddr.IPPort) []wgcfg.Config {
 		privKeys = append(privKeys, wgkey.Private(privKey))
 
 		addresses = append(addresses, []netaddr.IPPrefix{
-			parseCIDR(t, fmt.Sprintf("1.0.0.%d/32", i+1)),
+			netaddr.MustParseIPPrefix(fmt.Sprintf("1.0.0.%d/32", i+1)),
 		})
 	}
 
@@ -468,10 +469,14 @@ func makeConfigs(t *testing.T, addrs []netaddr.IPPort) []wgcfg.Config {
 			if peerNum == i {
 				continue
 			}
+			publicKey := privKeys[peerNum].Public()
 			peer := wgcfg.Peer{
-				PublicKey:           privKeys[peerNum].Public(),
-				AllowedIPs:          addresses[peerNum],
-				Endpoints:           addr.String(),
+				PublicKey:  publicKey,
+				AllowedIPs: addresses[peerNum],
+				Endpoints: wgcfg.Endpoints{
+					PublicKey: publicKey,
+					IPPorts:   wgcfg.NewIPPortSet(addr),
+				},
 				PersistentKeepalive: 25,
 			}
 			cfg.Peers = append(cfg.Peers, peer)
@@ -481,15 +486,6 @@ func makeConfigs(t *testing.T, addrs []netaddr.IPPort) []wgcfg.Config {
 	return cfgs
 }
 
-func parseCIDR(t *testing.T, addr string) netaddr.IPPrefix {
-	t.Helper()
-	cidr, err := netaddr.ParseIPPrefix(addr)
-	if err != nil {
-		t.Fatal(err)
-	}
-	return cidr
-}
-
 // TestDeviceStartStop exercises the startup and shutdown logic of
 // wireguard-go, which is intimately intertwined with magicsock's own
 // lifecycle. We seem to be good at generating deadlocks here, so if
@@ -513,7 +509,7 @@ func TestDeviceStartStop(t *testing.T) {
 
 	tun := tuntest.NewChannelTUN()
 	wgLogger := wglog.NewLogger(t.Logf)
-	dev := device.NewDevice(tun.TUN(), conn.Bind(), wgLogger.DeviceLogger, new(device.DeviceOptions))
+	dev := device.NewDevice(tun.TUN(), conn.Bind(), wgLogger.DeviceLogger)
 	dev.Up()
 	dev.Close()
 }
@@ -1251,6 +1247,19 @@ func newNonLegacyTestConn(t testing.TB) *Conn {
 	return conn
 }
 
+func makeEndpoint(tb testing.TB, public tailcfg.NodeKey, disco tailcfg.DiscoKey) string {
+	tb.Helper()
+	ep := wgcfg.Endpoints{
+		PublicKey: wgkey.Key(public),
+		DiscoKey:  disco,
+	}
+	buf, err := json.Marshal(ep)
+	if err != nil {
+		tb.Fatal(err)
+	}
+	return string(buf)
+}
+
 // addTestEndpoint sets conn's network map to a single peer expected
 // to receive packets from sendConn (or DERP), and returns that peer's
 // nodekey and discokey.
@@ -1270,7 +1279,7 @@ func addTestEndpoint(tb testing.TB, conn *Conn, sendConn net.PacketConn) (tailcf
 		},
 	})
 	conn.SetPrivateKey(wgkey.Private{0: 1})
-	_, err := conn.ParseEndpoint(string(nodeKey[:]) + "0000000000000000000000000000000000000000000000000000000000000001.disco.tailscale:12345")
+	_, err := conn.ParseEndpoint(makeEndpoint(tb, nodeKey, discoKey))
 	if err != nil {
 		tb.Fatal(err)
 	}
@@ -1444,7 +1453,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) {
 			},
 		},
 	})
-	_, err := conn.ParseEndpoint(string(nodeKey1[:]) + "0000000000000000000000000000000000000000000000000000000000000001.disco.tailscale:12345")
+	_, err := conn.ParseEndpoint(makeEndpoint(t, nodeKey1, discoKey))
 	if err != nil {
 		t.Fatal(err)
 	}

wgengine/netstack/netstack.go

@@ -48,6 +48,12 @@ const debugNetstack = false
 // and implements wgengine.FakeImpl to act as a userspace network
 // stack when Tailscale is running in fake mode.
 type Impl struct {
+	// ForwardTCPIn, if non-nil, handles forwarding an inbound TCP
+	// connection.
+	// TODO(bradfitz): provide mechanism for tsnet to reject a
+	// port other than accepting it and closing it.
+	ForwardTCPIn func(c net.Conn, port uint16)
+
 	ipstack     *stack.Stack
 	linkEP      *channel.Endpoint
 	tundev      *tstun.Wrapper
@@ -441,11 +447,15 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 		r.Complete(true)
 		return
 	}
+	r.Complete(false)
+	c := gonet.NewTCPConn(&wq, ep)
+	if ns.ForwardTCPIn != nil {
+		ns.ForwardTCPIn(c, reqDetails.LocalPort)
+		return
+	}
 	if isTailscaleIP {
 		dialAddr = tcpip.Address(net.ParseIP("127.0.0.1")).To4()
 	}
-	r.Complete(false)
-	c := gonet.NewTCPConn(&wq, ep)
 	ns.forwardTCP(c, &wq, dialAddr, reqDetails.LocalPort)
 }
 

wgengine/userspace.go

@@ -7,12 +7,10 @@ package wgengine
 import (
 	"bufio"
 	"bytes"
-	"context"
 	crand "crypto/rand"
 	"errors"
 	"fmt"
 	"io"
-	"net"
 	"os"
 	"reflect"
 	"runtime"
@@ -28,7 +26,7 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/control/controlclient"
 	"tailscale.com/health"
-	"tailscale.com/internal/deepprint"
+	"tailscale.com/internal/deephash"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/dns"
 	"tailscale.com/net/dns/resolver"
@@ -120,7 +118,6 @@ type userspaceEngine struct {
 	statusCallback      StatusCallback
 	peerSequence        []wgkey.Key
 	endpoints           []tailcfg.Endpoint
-	pingers             map[wgkey.Key]*pinger                // legacy pingers for pre-discovery peers
 	pendOpen            map[flowtrack.Tuple]*pendingOpenFlow // see pendopen.go
 	networkMapCallbacks map[*someHandle]NetworkMapCallback
 	tsIPByIPPort        map[netaddr.IPPort]netaddr.IP          // allows registration of IP:ports as belonging to a certain Tailscale IP for whois lookups
@@ -241,7 +238,6 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 		waitCh:  make(chan struct{}),
 		tundev:  tsTUNDev,
 		router:  conf.Router,
-		pingers: make(map[wgkey.Key]*pinger),
 	}
 	e.isLocalAddr.Store(tsaddr.NewContainsIPFunc(nil))
 
@@ -310,52 +306,6 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 	}
 
 	e.wgLogger = wglog.NewLogger(logf)
-	opts := &device.DeviceOptions{
-		HandshakeDone: func(peerKey device.NoisePublicKey, peer *device.Peer, deviceAllowedIPs *device.AllowedIPs) {
-			// Send an unsolicited status event every time a
-			// handshake completes. This makes sure our UI can
-			// update quickly as soon as it connects to a peer.
-			//
-			// We use a goroutine here to avoid deadlocking
-			// wireguard, since RequestStatus() will call back
-			// into it, and wireguard is what called us to get
-			// here.
-			go e.RequestStatus()
-
-			peerWGKey := wgkey.Key(peerKey)
-			if e.magicConn.PeerHasDiscoKey(tailcfg.NodeKey(peerKey)) {
-				e.logf("wireguard handshake complete for %v", peerWGKey.ShortString())
-				// This is a modern peer with discovery support. No need to send pings.
-				return
-			}
-
-			e.logf("wireguard handshake complete for %v; sending legacy pings", peerWGKey.ShortString())
-
-			// Ping every single-IP that peer routes.
-			// These synthetic packets are used to traverse NATs.
-			var ips []netaddr.IP
-			var allowedIPs []netaddr.IPPrefix
-			deviceAllowedIPs.EntriesForPeer(peer, func(stdIP net.IP, cidr uint) bool {
-				ip, ok := netaddr.FromStdIP(stdIP)
-				if !ok {
-					logf("[unexpected] bad IP from deviceAllowedIPs.EntriesForPeer: %v", stdIP)
-					return true
-				}
-				ipp := netaddr.IPPrefix{IP: ip, Bits: uint8(cidr)}
-				allowedIPs = append(allowedIPs, ipp)
-				if ipp.IsSingleIP() {
-					ips = append(ips, ip)
-				}
-				return true
-			})
-			if len(ips) > 0 {
-				go e.pinger(peerWGKey, ips)
-			} else {
-				logf("[unexpected] peer %s has no single-IP routes: %v", peerWGKey.ShortString(), allowedIPs)
-			}
-		},
-	}
-
 	e.tundev.OnTSMPPongReceived = func(pong packet.TSMPPongReply) {
 		e.mu.Lock()
 		defer e.mu.Unlock()
@@ -368,7 +318,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 
 	// wgdev takes ownership of tundev, will close it when closed.
 	e.logf("Creating wireguard device...")
-	e.wgdev = device.NewDevice(e.tundev, e.magicConn.Bind(), e.wgLogger.DeviceLogger, opts)
+	e.wgdev = device.NewDevice(e.tundev, e.magicConn.Bind(), e.wgLogger.DeviceLogger)
 	closePool.addFunc(e.wgdev.Close)
 	closePool.addFunc(func() {
 		if err := e.magicConn.Close(); err != nil {
@@ -507,132 +457,6 @@ func (e *userspaceEngine) pollResolver() {
 	}
 }
 
-// pinger sends ping packets for a few seconds.
-//
-// These generated packets are used to ensure we trigger the spray logic in
-// the magicsock package for NAT traversal.
-//
-// These are only used with legacy peers (before 0.100.0) that don't
-// have advertised discovery keys.
-type pinger struct {
-	e      *userspaceEngine
-	done   chan struct{} // closed after shutdown (not the ctx.Done() chan)
-	cancel context.CancelFunc
-}
-
-// close cleans up pinger and removes it from the userspaceEngine.pingers map.
-// It cannot be called while p.e.mu is held.
-func (p *pinger) close() {
-	p.cancel()
-	<-p.done
-}
-
-func (p *pinger) run(ctx context.Context, peerKey wgkey.Key, ips []netaddr.IP, srcIP netaddr.IP) {
-	defer func() {
-		p.e.mu.Lock()
-		if p.e.pingers[peerKey] == p {
-			delete(p.e.pingers, peerKey)
-		}
-		p.e.mu.Unlock()
-
-		close(p.done)
-	}()
-
-	header := packet.ICMP4Header{
-		IP4Header: packet.IP4Header{
-			Src: srcIP,
-		},
-		Type: packet.ICMP4EchoRequest,
-		Code: packet.ICMP4NoCode,
-	}
-
-	// sendFreq is slightly longer than sprayFreq in magicsock to ensure
-	// that if these ping packets are the only source of early packets
-	// sent to the peer, that each one will be sprayed.
-	const sendFreq = 300 * time.Millisecond
-	const stopAfter = 3 * time.Second
-
-	start := time.Now()
-	var dstIPs []netaddr.IP
-	for _, ip := range ips {
-		if ip.Is6() {
-			// This code is only used for legacy (pre-discovery)
-			// peers. They're not going to work right with IPv6 on the
-			// overlay anyway, so don't bother trying to make ping
-			// work.
-			continue
-		}
-		dstIPs = append(dstIPs, ip)
-	}
-
-	payload := []byte("magicsock_spray") // no meaning
-
-	header.IPID = 1
-	t := time.NewTicker(sendFreq)
-	defer t.Stop()
-	for {
-		select {
-		case <-ctx.Done():
-			return
-		case <-t.C:
-		}
-		if time.Since(start) > stopAfter {
-			return
-		}
-		for _, dstIP := range dstIPs {
-			header.Dst = dstIP
-			// InjectOutbound take ownership of the packet, so we allocate.
-			b := packet.Generate(&header, payload)
-			p.e.tundev.InjectOutbound(b)
-		}
-		header.IPID++
-	}
-}
-
-// pinger sends ping packets for a few seconds.
-//
-// These generated packets are used to ensure we trigger the spray logic in
-// the magicsock package for NAT traversal.
-//
-// This is only used with legacy peers (before 0.100.0) that don't
-// have advertised discovery keys.
-func (e *userspaceEngine) pinger(peerKey wgkey.Key, ips []netaddr.IP) {
-	e.logf("[v1] generating initial ping traffic to %s (%v)", peerKey.ShortString(), ips)
-	var srcIP netaddr.IP
-
-	e.wgLock.Lock()
-	if len(e.lastCfgFull.Addresses) > 0 {
-		srcIP = e.lastCfgFull.Addresses[0].IP
-	}
-	e.wgLock.Unlock()
-
-	if srcIP.IsZero() {
-		e.logf("generating initial ping traffic: no source IP")
-		return
-	}
-
-	ctx, cancel := context.WithCancel(context.Background())
-	p := &pinger{
-		e:      e,
-		done:   make(chan struct{}),
-		cancel: cancel,
-	}
-
-	e.mu.Lock()
-	if e.closing {
-		e.mu.Unlock()
-		return
-	}
-	oldPinger := e.pingers[peerKey]
-	e.pingers[peerKey] = p
-	e.mu.Unlock()
-
-	if oldPinger != nil {
-		oldPinger.close()
-	}
-	p.run(ctx, peerKey, ips, srcIP)
-}
-
 var (
 	debugTrimWireguardEnv = os.Getenv("TS_DEBUG_TRIM_WIREGUARD")
 	debugTrimWireguard, _ = strconv.ParseBool(debugTrimWireguardEnv)
@@ -675,15 +499,7 @@ func isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool {
 	if forceFullWireguardConfig(numPeers) {
 		return false
 	}
-	if !isSingleEndpoint(p.Endpoints) {
-		return false
-	}
-
-	host, _, err := net.SplitHostPort(p.Endpoints)
-	if err != nil {
-		return false
-	}
-	if !strings.HasSuffix(host, ".disco.tailscale") {
+	if p.Endpoints.DiscoKey.IsZero() {
 		return false
 	}
 
@@ -753,26 +569,6 @@ func (e *userspaceEngine) isActiveSince(dk tailcfg.DiscoKey, ip netaddr.IP, t ti
 	return unixTime >= t.Unix()
 }
 
-// discoKeyFromPeer returns the DiscoKey for a wireguard config's Peer.
-//
-// Invariant: isTrimmablePeer(p) == true, so it should have 1 endpoint with
-// Host of form "<64-hex-digits>.disco.tailscale". If invariant is violated,
-// we return the zero value.
-func discoKeyFromPeer(p *wgcfg.Peer) tailcfg.DiscoKey {
-	if len(p.Endpoints) < 64 {
-		return tailcfg.DiscoKey{}
-	}
-	host, rest := p.Endpoints[:64], p.Endpoints[64:]
-	if !strings.HasPrefix(rest, ".disco.tailscale") {
-		return tailcfg.DiscoKey{}
-	}
-	k, err := key.NewPublicFromHexMem(mem.S(host))
-	if err != nil {
-		return tailcfg.DiscoKey{}
-	}
-	return tailcfg.DiscoKey(k)
-}
-
 // discoChanged are the set of peers whose disco keys have changed, implying they've restarted.
 // If a peer is in this set and was previously in the live wireguard config,
 // it needs to be first removed and then re-added to flush out its wireguard session key.
@@ -820,7 +616,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Publ
 			}
 			continue
 		}
-		dk := discoKeyFromPeer(p)
+		dk := p.Endpoints.DiscoKey
 		trackDisco = append(trackDisco, dk)
 		recentlyActive := false
 		for _, cidr := range p.AllowedIPs {
@@ -837,7 +633,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Publ
 		}
 	}
 
-	if !deepprint.UpdateHash(&e.lastEngineSigTrim, min, trimmedDisco, trackDisco, trackIPs) {
+	if !deephash.UpdateHash(&e.lastEngineSigTrim, min, trimmedDisco, trackDisco, trackIPs) {
 		// No changes
 		return nil
 	}
@@ -958,8 +754,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	}
 	e.mu.Unlock()
 
-	engineChanged := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
-	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg, dnsCfg)
+	engineChanged := deephash.UpdateHash(&e.lastEngineSigFull, cfg)
+	routerChanged := deephash.UpdateHash(&e.lastRouterSig, routerCfg, dnsCfg)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}
@@ -970,26 +766,26 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	// and a second time with it.
 	discoChanged := make(map[key.Public]bool)
 	{
-		prevEP := make(map[key.Public]string)
+		prevEP := make(map[key.Public]tailcfg.DiscoKey)
 		for i := range e.lastCfgFull.Peers {
-			if p := &e.lastCfgFull.Peers[i]; isSingleEndpoint(p.Endpoints) {
-				prevEP[key.Public(p.PublicKey)] = p.Endpoints
+			if p := &e.lastCfgFull.Peers[i]; !p.Endpoints.DiscoKey.IsZero() {
+				prevEP[key.Public(p.PublicKey)] = p.Endpoints.DiscoKey
 			}
 		}
 		for i := range cfg.Peers {
 			p := &cfg.Peers[i]
-			if !isSingleEndpoint(p.Endpoints) {
+			if p.Endpoints.DiscoKey.IsZero() {
 				continue
 			}
 			pub := key.Public(p.PublicKey)
-			if old, ok := prevEP[pub]; ok && old != p.Endpoints {
+			if old, ok := prevEP[pub]; ok && old != p.Endpoints.DiscoKey {
 				discoChanged[pub] = true
 				e.logf("wgengine: Reconfig: %s changed from %q to %q", pub.ShortString(), old, p.Endpoints)
 			}
 		}
 	}
 
-	e.lastCfgFull = cfg.Copy()
+	e.lastCfgFull = *cfg.Clone()
 
 	// Tell magicsock about the new (or initial) private key
 	// (which is needed by DERP) before wgdev gets it, as wgdev
@@ -1026,11 +822,6 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	return nil
 }
 
-// isSingleEndpoint reports whether endpoints contains exactly one host:port pair.
-func isSingleEndpoint(s string) bool {
-	return s != "" && !strings.Contains(s, ",")
-}
-
 func (e *userspaceEngine) GetFilter() *filter.Filter {
 	return e.tundev.GetFilter()
 }
@@ -1053,6 +844,8 @@ func (e *userspaceEngine) getStatusCallback() StatusCallback {
 
 var singleNewline = []byte{'\n'}
 
+var ErrEngineClosing = errors.New("engine closing; no status")
+
 func (e *userspaceEngine) getStatus() (*Status, error) {
 	// Grab derpConns before acquiring wgLock to not violate lock ordering;
 	// the DERPs method acquires magicsock.Conn.mu.
@@ -1066,7 +859,7 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 	closing := e.closing
 	e.mu.Unlock()
 	if closing {
-		return nil, errors.New("engine closing; no status")
+		return nil, ErrEngineClosing
 	}
 
 	if e.wgdev == nil {
@@ -1213,17 +1006,12 @@ func (e *userspaceEngine) RequestStatus() {
 }
 
 func (e *userspaceEngine) Close() {
-	var pingers []*pinger
-
 	e.mu.Lock()
 	if e.closing {
 		e.mu.Unlock()
 		return
 	}
 	e.closing = true
-	for _, pinger := range e.pingers {
-		pingers = append(pingers, pinger)
-	}
 	e.mu.Unlock()
 
 	r := bufio.NewReader(strings.NewReader(""))
@@ -1237,13 +1025,6 @@ func (e *userspaceEngine) Close() {
 	e.router.Close()
 	e.wgdev.Close()
 	e.tundev.Close()
-
-	// Shut down pingers after tundev is closed (by e.wgdev.Close) so the
-	// synchronous close does not get stuck on InjectOutbound.
-	for _, pinger := range pingers {
-		pinger.close()
-	}
-
 	close(e.waitCh)
 }
 

wgengine/userspace_test.go

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

wgengine/wgcfg/clone.go

@@ -0,0 +1,101 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Code generated by tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet; DO NOT EDIT.
+
+package wgcfg
+
+import (
+	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/wgkey"
+)
+
+// Clone makes a deep copy of Config.
+// The result aliases no memory with the original.
+func (src *Config) Clone() *Config {
+	if src == nil {
+		return nil
+	}
+	dst := new(Config)
+	*dst = *src
+	dst.Addresses = append(src.Addresses[:0:0], src.Addresses...)
+	dst.DNS = append(src.DNS[:0:0], src.DNS...)
+	dst.Peers = make([]Peer, len(src.Peers))
+	for i := range dst.Peers {
+		dst.Peers[i] = *src.Peers[i].Clone()
+	}
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with command:
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+var _ConfigNeedsRegeneration = Config(struct {
+	Name       string
+	PrivateKey wgkey.Private
+	Addresses  []netaddr.IPPrefix
+	MTU        uint16
+	DNS        []netaddr.IP
+	Peers      []Peer
+}{})
+
+// Clone makes a deep copy of Peer.
+// The result aliases no memory with the original.
+func (src *Peer) Clone() *Peer {
+	if src == nil {
+		return nil
+	}
+	dst := new(Peer)
+	*dst = *src
+	dst.AllowedIPs = append(src.AllowedIPs[:0:0], src.AllowedIPs...)
+	dst.Endpoints = *src.Endpoints.Clone()
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with command:
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+var _PeerNeedsRegeneration = Peer(struct {
+	PublicKey           wgkey.Key
+	AllowedIPs          []netaddr.IPPrefix
+	Endpoints           Endpoints
+	PersistentKeepalive uint16
+}{})
+
+// Clone makes a deep copy of Endpoints.
+// The result aliases no memory with the original.
+func (src *Endpoints) Clone() *Endpoints {
+	if src == nil {
+		return nil
+	}
+	dst := new(Endpoints)
+	*dst = *src
+	dst.IPPorts = *src.IPPorts.Clone()
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with command:
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+var _EndpointsNeedsRegeneration = Endpoints(struct {
+	PublicKey wgkey.Key
+	DiscoKey  tailcfg.DiscoKey
+	IPPorts   IPPortSet
+}{})
+
+// Clone makes a deep copy of IPPortSet.
+// The result aliases no memory with the original.
+func (src *IPPortSet) Clone() *IPPortSet {
+	if src == nil {
+		return nil
+	}
+	dst := new(IPPortSet)
+	*dst = *src
+	dst.ipp = append(src.ipp[:0:0], src.ipp...)
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with command:
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+var _IPPortSetNeedsRegeneration = IPPortSet(struct {
+	ipp []netaddr.IPPort
+}{})

wgengine/wgcfg/config.go

@@ -6,14 +6,15 @@
 package wgcfg
 
 import (
+	"encoding/json"
+	"strings"
+
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/wgkey"
 )
 
-// EndpointDiscoSuffix is appended to the hex representation of a peer's discovery key
-// and is then the sole wireguard endpoint for peers with a non-zero discovery key.
-// This form is then recognize by magicsock's CreateEndpoint.
-const EndpointDiscoSuffix = ".disco.tailscale:12345"
+//go:generate go run tailscale.com/cmd/cloner -type=Config,Peer,Endpoints,IPPortSet -output=clone.go
 
 // Config is a WireGuard configuration.
 // It only supports the set of things Tailscale uses.
@@ -29,36 +30,90 @@ type Config struct {
 type Peer struct {
 	PublicKey           wgkey.Key
 	AllowedIPs          []netaddr.IPPrefix
-	Endpoints           string // comma-separated host/port pairs: "1.2.3.4:56,[::]:80"
+	Endpoints           Endpoints
 	PersistentKeepalive uint16
 }
 
-// Copy makes a deep copy of Config.
-// The result aliases no memory with the original.
-func (cfg Config) Copy() Config {
-	res := cfg
-	if res.Addresses != nil {
-		res.Addresses = append([]netaddr.IPPrefix{}, res.Addresses...)
-	}
-	if res.DNS != nil {
-		res.DNS = append([]netaddr.IP{}, res.DNS...)
-	}
-	peers := make([]Peer, 0, len(res.Peers))
-	for _, peer := range res.Peers {
-		peers = append(peers, peer.Copy())
-	}
-	res.Peers = peers
-	return res
+// Endpoints represents the routes to reach a remote node.
+// It is serialized and provided to wireguard-go as a conn.Endpoint.
+type Endpoints struct {
+	// PublicKey is the public key for the remote node.
+	PublicKey wgkey.Key `json:"pk"`
+	// DiscoKey is the disco key associated with the remote node.
+	DiscoKey tailcfg.DiscoKey `json:"dk,omitempty"`
+	// IPPorts is a set of possible ip+ports the remote node can be reached at.
+	// This is used only for legacy connections to pre-disco (pre-0.100) peers.
+	IPPorts IPPortSet `json:"ipp,omitempty"`
 }
 
-// Copy makes a deep copy of Peer.
-// The result aliases no memory with the original.
-func (peer Peer) Copy() Peer {
-	res := peer
-	if res.AllowedIPs != nil {
-		res.AllowedIPs = append([]netaddr.IPPrefix{}, res.AllowedIPs...)
+func (e Endpoints) Equal(f Endpoints) bool {
+	if e.PublicKey != f.PublicKey {
+		return false
 	}
-	return res
+	if e.DiscoKey != f.DiscoKey {
+		return false
+	}
+	return e.IPPorts.EqualUnordered(f.IPPorts)
+}
+
+// IPPortSet is an immutable slice of netaddr.IPPorts.
+type IPPortSet struct {
+	ipp []netaddr.IPPort
+}
+
+// NewIPPortSet returns an IPPortSet containing the ports in ipp.
+func NewIPPortSet(ipps ...netaddr.IPPort) IPPortSet {
+	return IPPortSet{ipp: append(ipps[:0:0], ipps...)}
+}
+
+// String returns a comma-separated list of all IPPorts in s.
+func (s IPPortSet) String() string {
+	buf := new(strings.Builder)
+	for i, ipp := range s.ipp {
+		if i > 0 {
+			buf.WriteByte(',')
+		}
+		buf.WriteString(ipp.String())
+	}
+	return buf.String()
+}
+
+// IPPorts returns a slice of netaddr.IPPorts containing the IPPorts in s.
+func (s IPPortSet) IPPorts() []netaddr.IPPort {
+	return append(s.ipp[:0:0], s.ipp...)
+}
+
+// EqualUnordered reports whether s and t contain the same IPPorts, regardless of order.
+func (s IPPortSet) EqualUnordered(t IPPortSet) bool {
+	if len(s.ipp) != len(t.ipp) {
+		return false
+	}
+	// Check whether the endpoints are the same, regardless of order.
+	ipps := make(map[netaddr.IPPort]int, len(s.ipp))
+	for _, ipp := range s.ipp {
+		ipps[ipp]++
+	}
+	for _, ipp := range t.ipp {
+		ipps[ipp]--
+	}
+	for _, n := range ipps {
+		if n != 0 {
+			return false
+		}
+	}
+	return true
+}
+
+// MarshalJSON marshals s into JSON.
+// It is necessary so that IPPortSet's fields can be unexported, to guarantee immutability.
+func (s IPPortSet) MarshalJSON() ([]byte, error) {
+	return json.Marshal(s.ipp)
+}
+
+// UnmarshalJSON unmarshals s from JSON.
+// It is necessary so that IPPortSet's fields can be unexported, to guarantee immutability.
+func (s *IPPortSet) UnmarshalJSON(b []byte) error {
+	return json.Unmarshal(b, &s.ipp)
 }
 
 // PeerWithKey returns the Peer with key k and reports whether it was found.

wgengine/wgcfg/device.go

@@ -20,13 +20,14 @@ func DeviceConfig(d *device.Device) (*Config, error) {
 		w.Close()
 	}()
 	cfg, err := FromUAPI(r)
+	// Prefer errors from IpcGetOperation.
+	if setErr := <-errc; setErr != nil {
+		return nil, setErr
+	}
+	// Check FromUAPI error.
 	if err != nil {
 		return nil, err
 	}
-	if err := <-errc; err != nil {
-		return nil, err
-	}
-
 	sort.Slice(cfg.Peers, func(i, j int) bool {
 		return cfg.Peers[i].PublicKey.LessThan(&cfg.Peers[j].PublicKey)
 	})
@@ -47,15 +48,17 @@ func ReconfigDevice(d *device.Device, cfg *Config, logf logger.Logf) (err error)
 	}
 
 	r, w := io.Pipe()
-	errc := make(chan error)
+	errc := make(chan error, 1)
 	go func() {
 		errc <- d.IpcSetOperation(r)
+		w.Close()
 	}()
 
 	err = cfg.ToUAPI(w, prev)
-	if err != nil {
-		return err
-	}
 	w.Close()
-	return <-errc
+	// Prefer errors from IpcSetOperation.
+	if setErr := <-errc; setErr != nil {
+		return setErr
+	}
+	return err // err (if any) from cfg.ToUAPI
 }

wgengine/wgcfg/device_test.go

@@ -8,7 +8,9 @@ import (
 	"bufio"
 	"bytes"
 	"io"
+	"net"
 	"os"
+	"reflect"
 	"sort"
 	"strings"
 	"sync"
@@ -56,8 +58,8 @@ func TestDeviceConfig(t *testing.T) {
 		}},
 	}
 
-	device1 := device.NewDevice(newNilTun(), conn.NewDefaultBind(), device.NewLogger(device.LogLevelError, "device1"))
-	device2 := device.NewDevice(newNilTun(), conn.NewDefaultBind(), device.NewLogger(device.LogLevelError, "device2"))
+	device1 := device.NewDevice(newNilTun(), new(noopBind), device.NewLogger(device.LogLevelError, "device1"))
+	device2 := device.NewDevice(newNilTun(), new(noopBind), device.NewLogger(device.LogLevelError, "device2"))
 	defer device1.Close()
 	defer device2.Close()
 
@@ -89,7 +91,7 @@ func TestDeviceConfig(t *testing.T) {
 				t.Errorf("on error, could not IpcGetOperation: %v", err)
 			}
 			w.Flush()
-			t.Errorf("cfg:\n%s\n---- want:\n%s\n---- uapi:\n%s", gotStr, wantStr, buf.String())
+			t.Errorf("config mismatch:\n---- got:\n%s\n---- want:\n%s\n---- uapi:\n%s", gotStr, wantStr, buf.String())
 		}
 	}
 
@@ -126,7 +128,7 @@ func TestDeviceConfig(t *testing.T) {
 	})
 
 	t.Run("device1 modify peer", func(t *testing.T) {
-		cfg1.Peers[0].Endpoints = "1.2.3.4:12345"
+		cfg1.Peers[0].Endpoints.IPPorts = NewIPPortSet(netaddr.MustParseIPPort("1.2.3.4:12345"))
 		if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil {
 			t.Fatal(err)
 		}
@@ -134,7 +136,7 @@ func TestDeviceConfig(t *testing.T) {
 	})
 
 	t.Run("device1 replace endpoint", func(t *testing.T) {
-		cfg1.Peers[0].Endpoints = "1.1.1.1:123"
+		cfg1.Peers[0].Endpoints.IPPorts = NewIPPortSet(netaddr.MustParseIPPort("1.1.1.1:123"))
 		if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil {
 			t.Fatal(err)
 		}
@@ -175,7 +177,7 @@ func TestDeviceConfig(t *testing.T) {
 		}
 		peersEqual := func(p, q Peer) bool {
 			return p.PublicKey == q.PublicKey && p.PersistentKeepalive == q.PersistentKeepalive &&
-				p.Endpoints == q.Endpoints && cidrsEqual(p.AllowedIPs, q.AllowedIPs)
+				reflect.DeepEqual(p.Endpoints, q.Endpoints) && cidrsEqual(p.AllowedIPs, q.AllowedIPs)
 		}
 		if !peersEqual(peer0(origCfg), peer0(newCfg)) {
 			t.Error("reconfig modified old peer")
@@ -237,3 +239,26 @@ func (t *nilTun) Close() error {
 	close(t.closed)
 	return nil
 }
+
+// A noopBind is a conn.Bind that does no actual binding work.
+type noopBind struct{}
+
+func (noopBind) Open(port uint16) (fns []conn.ReceiveFunc, actualPort uint16, err error) {
+	return nil, 1, nil
+}
+func (noopBind) Close() error                          { return nil }
+func (noopBind) SetMark(mark uint32) error             { return nil }
+func (noopBind) Send(b []byte, ep conn.Endpoint) error { return nil }
+func (noopBind) ParseEndpoint(s string) (conn.Endpoint, error) {
+	return dummyEndpoint(s), nil
+}
+
+// A dummyEndpoint is a string holding the endpoint destination.
+type dummyEndpoint string
+
+func (e dummyEndpoint) ClearSrc()           {}
+func (e dummyEndpoint) SrcToString() string { return "" }
+func (e dummyEndpoint) DstToString() string { return string(e) }
+func (e dummyEndpoint) DstToBytes() []byte  { return nil }
+func (e dummyEndpoint) DstIP() net.IP       { return nil }
+func (dummyEndpoint) SrcIP() net.IP         { return nil }

wgengine/wgcfg/nmcfg/nmcfg.go

@@ -8,8 +8,6 @@ package nmcfg
 import (
 	"bytes"
 	"fmt"
-	"net"
-	"strconv"
 	"strings"
 
 	"inet.af/netaddr"
@@ -79,17 +77,19 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 			cpeer.PersistentKeepalive = 25 // seconds
 		}
 
-		if !peer.DiscoKey.IsZero() {
-			cpeer.Endpoints = fmt.Sprintf("%x.disco.tailscale:12345", peer.DiscoKey[:])
-		} else {
-			if err := appendEndpoint(cpeer, peer.DERP); err != nil {
+		cpeer.Endpoints = wgcfg.Endpoints{PublicKey: wgkey.Key(peer.Key), DiscoKey: peer.DiscoKey}
+		if peer.DiscoKey.IsZero() {
+			// Legacy connection. Add IP+port endpoints.
+			var ipps []netaddr.IPPort
+			if err := appendEndpoint(cpeer, &ipps, peer.DERP); err != nil {
 				return nil, err
 			}
 			for _, ep := range peer.Endpoints {
-				if err := appendEndpoint(cpeer, ep); err != nil {
+				if err := appendEndpoint(cpeer, &ipps, ep); err != nil {
 					return nil, err
 				}
 			}
+			cpeer.Endpoints.IPPorts = wgcfg.NewIPPortSet(ipps...)
 		}
 		didExitNodeWarn := false
 		for _, allowedIP := range peer.AllowedIPs {
@@ -136,21 +136,14 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 	return cfg, nil
 }
 
-func appendEndpoint(peer *wgcfg.Peer, epStr string) error {
+func appendEndpoint(peer *wgcfg.Peer, ipps *[]netaddr.IPPort, epStr string) error {
 	if epStr == "" {
 		return nil
 	}
-	_, port, err := net.SplitHostPort(epStr)
+	ipp, err := netaddr.ParseIPPort(epStr)
 	if err != nil {
 		return fmt.Errorf("malformed endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
 	}
-	_, err = strconv.ParseUint(port, 10, 16)
-	if err != nil {
-		return fmt.Errorf("invalid port in endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
-	}
-	if peer.Endpoints != "" {
-		peer.Endpoints += ","
-	}
-	peer.Endpoints += epStr
+	*ipps = append(*ipps, ipp)
 	return nil
 }

wgengine/wgcfg/parser.go

@@ -7,6 +7,7 @@ package wgcfg
 import (
 	"bufio"
 	"encoding/hex"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net"
@@ -26,21 +27,6 @@ func (e *ParseError) Error() string {
 	return fmt.Sprintf("%s: %q", e.why, e.offender)
 }
 
-func validateEndpoints(s string) error {
-	if s == "" {
-		// Otherwise strings.Split of the empty string produces [""].
-		return nil
-	}
-	vals := strings.Split(s, ",")
-	for _, val := range vals {
-		_, _, err := parseEndpoint(val)
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func parseEndpoint(s string) (host string, port uint16, err error) {
 	i := strings.LastIndexByte(s, ':')
 	if i < 0 {
@@ -103,6 +89,7 @@ func FromUAPI(r io.Reader) (*Config, error) {
 		}
 		key := parts[0]
 		value := parts[1]
+		valueBytes := scanner.Bytes()[len(key)+1:]
 
 		if key == "public_key" {
 			if deviceConfig {
@@ -121,7 +108,7 @@ func FromUAPI(r io.Reader) (*Config, error) {
 		if deviceConfig {
 			err = cfg.handleDeviceLine(key, value)
 		} else {
-			err = cfg.handlePeerLine(peer, key, value)
+			err = cfg.handlePeerLine(peer, key, value, valueBytes)
 		}
 		if err != nil {
 			return nil, err
@@ -165,14 +152,13 @@ func (cfg *Config) handlePublicKeyLine(value string) (*Peer, error) {
 	return peer, nil
 }
 
-func (cfg *Config) handlePeerLine(peer *Peer, key, value string) error {
+func (cfg *Config) handlePeerLine(peer *Peer, key, value string, valueBytes []byte) error {
 	switch key {
 	case "endpoint":
-		err := validateEndpoints(value)
+		err := json.Unmarshal(valueBytes, &peer.Endpoints)
 		if err != nil {
 			return err
 		}
-		peer.Endpoints = value
 	case "persistent_keepalive_interval":
 		n, err := strconv.ParseUint(value, 10, 16)
 		if err != nil {

wgengine/wgcfg/parser_test.go

@@ -53,21 +53,3 @@ func TestParseEndpoint(t *testing.T) {
 		t.Error("Error was expected")
 	}
 }
-
-func TestValidateEndpoints(t *testing.T) {
-	tests := []struct {
-		in   string
-		want error
-	}{
-		{"", nil},
-		{"1.2.3.4:5", nil},
-		{"1.2.3.4:5,6.7.8.9:10", nil},
-		{",", &ParseError{why: "Missing port from endpoint", offender: ""}},
-	}
-	for _, tt := range tests {
-		got := validateEndpoints(tt.in)
-		if !reflect.DeepEqual(got, tt.want) {
-			t.Errorf("%q = %#v (%s); want %#v (%s)", tt.in, got, got, tt.want, tt.want)
-		}
-	}
-}

wgengine/wgcfg/writer.go

@@ -5,11 +5,10 @@
 package wgcfg
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
-	"sort"
 	"strconv"
-	"strings"
 
 	"inet.af/netaddr"
 	"tailscale.com/types/wgkey"
@@ -53,8 +52,12 @@ func (cfg *Config) ToUAPI(w io.Writer, prev *Config) error {
 		setPeer(p)
 		set("protocol_version", "1")
 
-		if !endpointsEqual(oldPeer.Endpoints, p.Endpoints) {
-			set("endpoint", p.Endpoints)
+		if !oldPeer.Endpoints.Equal(p.Endpoints) {
+			buf, err := json.Marshal(p.Endpoints)
+			if err != nil {
+				return err
+			}
+			set("endpoint", string(buf))
 		}
 
 		// TODO: replace_allowed_ips is expensive.
@@ -90,24 +93,6 @@ func (cfg *Config) ToUAPI(w io.Writer, prev *Config) error {
 	return stickyErr
 }
 
-func endpointsEqual(x, y string) bool {
-	// Cheap comparisons.
-	if x == y {
-		return true
-	}
-	xs := strings.Split(x, ",")
-	ys := strings.Split(y, ",")
-	if len(xs) != len(ys) {
-		return false
-	}
-	// Otherwise, see if they're the same, but out of order.
-	sort.Strings(xs)
-	sort.Strings(ys)
-	x = strings.Join(xs, ",")
-	y = strings.Join(ys, ",")
-	return x == y
-}
-
 func cidrsEqual(x, y []netaddr.IPPrefix) bool {
 	// TODO: re-implement using netaddr.IPSet.Equal.
 	if len(x) != len(y) {

wgengine/wglog/wglog.go

@@ -85,7 +85,7 @@ func (x *Logger) SetPeers(peers []wgcfg.Peer) {
 	// Construct a new peer public key log rewriter.
 	replace := make(map[string]string)
 	for _, peer := range peers {
-		old := "peer(" + wireguardGoString(peer.PublicKey) + ")"
+		old := wireguardGoString(peer.PublicKey)
 		new := peer.PublicKey.ShortString()
 		replace[old] = new
 	}
@@ -94,10 +94,17 @@ func (x *Logger) SetPeers(peers []wgcfg.Peer) {
 
 // wireguardGoString prints p in the same format used by wireguard-go.
 func wireguardGoString(k wgkey.Key) string {
-	base64Key := base64.StdEncoding.EncodeToString(k[:])
-	abbreviatedKey := "invalid"
-	if len(base64Key) == 44 {
-		abbreviatedKey = base64Key[0:4] + "…" + base64Key[39:43]
-	}
-	return abbreviatedKey
+	const prefix = "peer("
+	b := make([]byte, len(prefix)+44)
+	copy(b, prefix)
+	r := b[len(prefix):]
+	base64.StdEncoding.Encode(r, k[:])
+	r = r[4:]
+	copy(r, "…")
+	r = r[len("…"):]
+	copy(r, b[len(prefix)+39:len(prefix)+43])
+	r = r[4:]
+	r[0] = ')'
+	r = r[1:]
+	return string(b[:len(b)-len(r)])
 }

wgengine/wglog/wglog_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"testing"
 
+	"tailscale.com/types/logger"
 	"tailscale.com/types/wgkey"
 	"tailscale.com/wgengine/wgcfg"
 	"tailscale.com/wgengine/wglog"
@@ -70,3 +71,30 @@ func stringer(s string) stringerString {
 type stringerString string
 
 func (s stringerString) String() string { return string(s) }
+
+func BenchmarkSetPeers(b *testing.B) {
+	b.ReportAllocs()
+	x := wglog.NewLogger(logger.Discard)
+	peers := [][]wgcfg.Peer{genPeers(0), genPeers(15), genPeers(16), genPeers(15)}
+	for i := 0; i < b.N; i++ {
+		for _, p := range peers {
+			x.SetPeers(p)
+		}
+	}
+}
+
+func genPeers(n int) []wgcfg.Peer {
+	if n > 32 {
+		panic("too many peers")
+	}
+	if n == 0 {
+		return nil
+	}
+	peers := make([]wgcfg.Peer, n)
+	for i := range peers {
+		var k wgkey.Key
+		k[n] = byte(n)
+		peers[i].PublicKey = k
+	}
+	return peers
+}