ipnlocal: in Start() fast path, don't forget to send Prefs.

The resulting empty Prefs had AllowSingleHosts=false and Routeall=false, so that on iOS if you did these steps: - Login and leave running - Terminate the frontend - Restart the frontend (fast path restart, missing prefs) - Set WantRunning=false - Set WantRunning=true ...then you would have Tailscale running, but with no routes. You would also accidentally disable the ExitNodeID/IP prefs (symptom: the current exit node setting didn't appear in the UI), but since nothing else worked either, you probably didn't notice. The fix was easy enough. It turns out we already knew about the problem, so this also fixes one of the BUG entries in state_test. Fixes: #1918 (BUG-1) and some as-yet-unreported bugs with exit nodes. Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
ipnlocal: don't assume NeedsLogin immediately after StartLogout().
2021-05-20 03:35:14 -04:00 · 2021-05-20 03:35:14 -04:00 · 2021-05-20 01:08:06 -04:00 · 2021-05-20 01:07:23 -04:00 · 2021-05-20 01:07:23 -04:00 · 2021-05-19 14:22:26 -04:00
21 changed files with 813 additions and 750 deletions
--- a/VERSION.txt
+++ b/VERSION.txt
@@ -1 +1 @@
-1.7.0
+1.8.3
--- a/cmd/tailscale/cli/cli_test.go
+++ b/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
@@ -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)
+			}
+		})
+	}
+}
--- a/cmd/tailscale/cli/up.go
+++ b/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
+}
--- a/control/controlclient/auto.go
+++ b/control/controlclient/auto.go
@@ -576,9 +576,12 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
 	c.logf("[v1] sendStatus: %s: %v", who, state)

 	var p *persist.Persist
-	var fin *empty.Message
+	var loginFin, logoutFin *empty.Message
 	if state == StateAuthenticated {
-		fin = new(empty.Message)
+		loginFin = new(empty.Message)
+	}
+	if state == StateNotAuthenticated {
+		logoutFin = new(empty.Message)
 	}
 	if nm != nil && loggedIn && synced {
 		pp := c.direct.GetPersist()
@@ -589,12 +592,13 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
 		nm = nil
 	}
 	new := Status{
-		LoginFinished: fin,
-		URL:           url,
-		Persist:       p,
-		NetMap:        nm,
-		Hostinfo:      hi,
-		State:         state,
+		LoginFinished:  loginFin,
+		LogoutFinished: logoutFin,
+		URL:            url,
+		Persist:        p,
+		NetMap:         nm,
+		Hostinfo:       hi,
+		State:          state,
 	}
 	if err != nil {
 		new.Err = err.Error()
--- a/control/controlclient/controlclient_test.go
+++ b/control/controlclient/controlclient_test.go
@@ -22,7 +22,7 @@ func fieldsOf(t reflect.Type) (fields []string) {

 func TestStatusEqual(t *testing.T) {
 	// Verify that the Equal method stays in sync with reality
-	equalHandles := []string{"LoginFinished", "Err", "URL", "NetMap", "State", "Persist", "Hostinfo"}
+	equalHandles := []string{"LoginFinished", "LogoutFinished", "Err", "URL", "NetMap", "State", "Persist", "Hostinfo"}
 	if have := fieldsOf(reflect.TypeOf(Status{})); !reflect.DeepEqual(have, equalHandles) {
 		t.Errorf("Status.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
 			have, equalHandles)
--- a/control/controlclient/direct.go
+++ b/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
 	}

--- a/control/controlclient/status.go
+++ b/control/controlclient/status.go
@@ -64,11 +64,12 @@ func (s State) String() string {
 }

 type Status struct {
-	_             structs.Incomparable
-	LoginFinished *empty.Message // nonempty when login finishes
-	Err           string
-	URL           string             // interactive URL to visit to finish logging in
-	NetMap        *netmap.NetworkMap // server-pushed configuration
+	_              structs.Incomparable
+	LoginFinished  *empty.Message // nonempty when login finishes
+	LogoutFinished *empty.Message // nonempty when logout finishes
+	Err            string
+	URL            string             // interactive URL to visit to finish logging in
+	NetMap         *netmap.NetworkMap // server-pushed configuration

 	// The internal state should not be exposed outside this
 	// package, but we have some automated tests elsewhere that need to
@@ -86,6 +87,7 @@ func (s *Status) Equal(s2 *Status) bool {
 	}
 	return s != nil && s2 != nil &&
 		(s.LoginFinished == nil) == (s2.LoginFinished == nil) &&
+		(s.LogoutFinished == nil) == (s2.LogoutFinished == nil) &&
 		s.Err == s2.Err &&
 		s.URL == s2.URL &&
 		reflect.DeepEqual(s.Persist, s2.Persist) &&
--- a/go.mod
+++ b/go.mod
@@ -26,7 +26,7 @@ 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-20210510175647-030c638da3df
 	github.com/tcnksm/go-httpstat v0.2.0
 	github.com/toqueteos/webbrowser v1.2.0
 	go4.org/mem v0.0.0-20201119185036-c04c5a6ff174
--- a/go.sum
+++ b/go.sum
@@ -127,6 +127,8 @@ 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/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=
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -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{}})
 	}

@@ -446,6 +451,13 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 	// Lock b once and do only the things that require locking.
 	b.mu.Lock()

+	if st.LogoutFinished != nil {
+		// Since we're logged out now, our netmap cache is invalid.
+		// Since st.NetMap==nil means "netmap is unchanged", there is
+		// no other way to represent this change.
+		b.setNetMapLocked(nil)
+	}
+
 	prefs := b.prefs
 	stateKey := b.stateKey
 	netMap := b.netMap
@@ -476,11 +488,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 +578,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
 	}

@@ -631,6 +655,12 @@ func (b *LocalBackend) getNewControlClientFunc() clientGen {
 // startIsNoopLocked reports whether a Start call on this LocalBackend
 // with the provided Start Options would be a useless no-op.
 //
+// TODO(apenwarr): we shouldn't need this.
+//  The state machine is now nearly clean enough where it can accept a new
+//  connection while in any state, not just Running, and on any platform.
+//  We'd want to add a few more tests to state_test.go to ensure this continues
+//  to work as expected.
+//
 // b.mu must be held.
 func (b *LocalBackend) startIsNoopLocked(opts ipn.Options) bool {
 	// Options has 5 fields; check all of them:
@@ -684,6 +714,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 		b.send(ipn.Notify{
 			State:         &state,
 			NetMap:        nm,
+			Prefs:         b.prefs,
 			LoginFinished: new(empty.Message),
 		})
 		return nil
@@ -1679,7 +1710,42 @@ func (b *LocalBackend) authReconfig() {

 	var dcfg dns.Config

-	// If CorpDNS is false, dcfg remains the zero value.
+	// Populate MagicDNS records. We do this unconditionally so that
+	// quad-100 can always respond to MagicDNS queries, even if the OS
+	// isn't configured to make MagicDNS resolution truly
+	// magic. Details in
+	// https://github.com/tailscale/tailscale/issues/1886.
+	set := func(name string, addrs []netaddr.IPPrefix) {
+		if len(addrs) == 0 || name == "" {
+			return
+		}
+		fqdn, err := dnsname.ToFQDN(name)
+		if err != nil {
+			return // TODO: propagate error?
+		}
+		var ips []netaddr.IP
+		for _, addr := range addrs {
+			// Remove IPv6 addresses for now, as we don't
+			// guarantee that the peer node actually can speak
+			// IPv6 correctly.
+			//
+			// https://github.com/tailscale/tailscale/issues/1152
+			// tracks adding the right capability reporting to
+			// enable AAAA in MagicDNS.
+			if addr.IP.Is6() {
+				continue
+			}
+			ips = append(ips, addr.IP)
+		}
+		dcfg.Hosts[fqdn] = ips
+	}
+	dcfg.AuthoritativeSuffixes = magicDNSRootDomains(nm)
+	dcfg.Hosts = map[dnsname.FQDN][]netaddr.IP{}
+	set(nm.Name, nm.Addresses)
+	for _, peer := range nm.Peers {
+		set(peer.Name, peer.Addresses)
+	}
+
 	if uc.CorpDNS {
 		addDefault := func(resolvers []tailcfg.DNSResolver) {
 			for _, resolver := range resolvers {
@@ -1717,36 +1783,9 @@ func (b *LocalBackend) authReconfig() {
 			}
 			dcfg.SearchDomains = append(dcfg.SearchDomains, fqdn)
 		}
-		set := func(name string, addrs []netaddr.IPPrefix) {
-			if len(addrs) == 0 || name == "" {
-				return
-			}
-			fqdn, err := dnsname.ToFQDN(name)
-			if err != nil {
-				return // TODO: propagate error?
-			}
-			var ips []netaddr.IP
-			for _, addr := range addrs {
-				// Remove IPv6 addresses for now, as we don't
-				// guarantee that the peer node actually can speak
-				// IPv6 correctly.
-				//
-				// https://github.com/tailscale/tailscale/issues/1152
-				// tracks adding the right capability reporting to
-				// enable AAAA in MagicDNS.
-				if addr.IP.Is6() {
-					continue
-				}
-				ips = append(ips, addr.IP)
-			}
-			dcfg.Hosts[fqdn] = ips
-		}
 		if nm.DNS.Proxied { // actually means "enable MagicDNS"
-			dcfg.AuthoritativeSuffixes = magicDNSRootDomains(nm)
-			dcfg.Hosts = map[dnsname.FQDN][]netaddr.IP{}
-			set(nm.Name, nm.Addresses)
-			for _, peer := range nm.Peers {
-				set(peer.Name, peer.Addresses)
+			for _, dom := range dcfg.AuthoritativeSuffixes {
+				dcfg.Routes[dom] = []netaddr.IPPort{netaddr.IPPort{IP: tsaddr.TailscaleServiceIP(), Port: 53}}
 			}
 		}

@@ -2109,8 +2148,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 +2205,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 {
@@ -2300,7 +2340,6 @@ func (b *LocalBackend) LogoutSync(ctx context.Context) error {
 func (b *LocalBackend) logout(ctx context.Context, sync bool) error {
 	b.mu.Lock()
 	cc := b.cc
-	b.setNetMapLocked(nil)
 	b.mu.Unlock()

 	b.EditPrefs(&ipn.MaskedPrefs{
@@ -2327,10 +2366,6 @@ func (b *LocalBackend) logout(ctx context.Context, sync bool) error {
 		cc.StartLogout()
 	}

-	b.mu.Lock()
-	b.setNetMapLocked(nil)
-	b.mu.Unlock()
-
 	b.stateMachine()
 	return err
 }
--- a/ipn/ipnlocal/state_test.go
+++ b/ipn/ipnlocal/state_test.go
@@ -140,6 +140,8 @@ func (cc *mockControl) send(err error, url string, loginFinished bool, nm *netma
 		}
 		if loginFinished {
 			s.LoginFinished = &empty.Message{}
+		} else if url == "" && err == nil && nm == nil {
+			s.LogoutFinished = &empty.Message{}
 		}
 		cc.statusFunc(s)
 	}
@@ -368,13 +370,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 +384,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 +449,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 +467,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)
@@ -554,10 +550,7 @@ func TestStateMachine(t *testing.T) {
 		c.Assert(nn[0].State, qt.Not(qt.IsNil))
 		c.Assert(nn[0].LoginFinished, qt.Not(qt.IsNil))
 		c.Assert(nn[0].NetMap, qt.Not(qt.IsNil))
-		// BUG: Prefs should be sent too, or the UI could end up in
-		// a bad state. (iOS, the only current user of this feature,
-		// probably wouldn't notice because it happens to not display
-		// any prefs. Maybe exit nodes will look weird?)
+		c.Assert(nn[0].Prefs, qt.Not(qt.IsNil))
 	}

 	// undo the state hack above.
@@ -569,14 +562,13 @@ func TestStateMachine(t *testing.T) {
 	b.Logout()
 	{
 		nn := notifies.drain(2)
-		// BUG: now is not the time to unpause.
-		c.Assert([]string{"unpause", "StartLogout"}, qt.DeepEquals, cc.getCalls())
+		c.Assert([]string{"pause", "StartLogout"}, qt.DeepEquals, cc.getCalls())
 		c.Assert(nn[0].State, qt.Not(qt.IsNil))
 		c.Assert(nn[1].Prefs, qt.Not(qt.IsNil))
-		c.Assert(ipn.NeedsLogin, qt.Equals, *nn[0].State)
+		c.Assert(ipn.Stopped, qt.Equals, *nn[0].State)
 		c.Assert(nn[1].Prefs.LoggedOut, qt.IsTrue)
 		c.Assert(nn[1].Prefs.WantRunning, qt.IsFalse)
-		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
+		c.Assert(ipn.Stopped, qt.Equals, b.State())
 	}

 	// Let's make the logout succeed.
@@ -586,72 +578,69 @@ func TestStateMachine(t *testing.T) {
 	cc.send(nil, "", false, nil)
 	{
 		nn := notifies.drain(1)
-		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([]string{"unpause"}, qt.DeepEquals, cc.getCalls())
+		c.Assert(nn[0].State, qt.Not(qt.IsNil))
+		c.Assert(ipn.NeedsLogin, qt.Equals, *nn[0].State)
+		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.
--- a/ipn/ipnstate/ipnstate.go
+++ b/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
 	}
--- a/ipn/message.go
+++ b/ipn/message.go
@@ -104,7 +104,9 @@ func NewBackendServer(logf logger.Logf, b Backend, sendNotifyMsg func(b []byte))
 		b:             b,
 		sendNotifyMsg: sendNotifyMsg,
 	}
-	if sendNotifyMsg != nil {
+	// b may be nil if the BackendServer is being created just to
+	// encapsulate and send an error message.
+	if sendNotifyMsg != nil && b != nil {
 		b.SetNotifyCallback(bs.send)
 	}
 	return bs
--- a/net/dns/config.go
+++ b/net/dns/config.go
@@ -28,8 +28,11 @@ type Config struct {
 	SearchDomains []dnsname.FQDN
 	// Hosts maps DNS FQDNs to their IPs, which can be a mix of IPv4
 	// and IPv6.
-	// Queries matching entries in Hosts are resolved locally without
-	// recursing off-machine.
+	// Queries matching entries in Hosts are resolved locally by
+	// 100.100.100.100 without leaving the machine.
+	// Adding an entry to Hosts merely creates the record. If you want
+	// it to resolve, you also need to add appropriate routes to
+	// Routes.
 	Hosts map[dnsname.FQDN][]netaddr.IP
 	// AuthoritativeSuffixes is a list of fully-qualified DNS suffixes
 	// for which the in-process Tailscale resolver is authoritative.
@@ -42,7 +45,7 @@ type Config struct {
 // needsAnyResolvers reports whether c requires a resolver to be set
 // at the OS level.
 func (c Config) needsOSResolver() bool {
-	return c.hasDefaultResolvers() || c.hasRoutes() || c.hasHosts()
+	return c.hasDefaultResolvers() || c.hasRoutes()
 }

 func (c Config) hasRoutes() bool {
@@ -52,7 +55,7 @@ func (c Config) hasRoutes() bool {
 // hasDefaultResolversOnly reports whether the only resolvers in c are
 // DefaultResolvers.
 func (c Config) hasDefaultResolversOnly() bool {
-	return c.hasDefaultResolvers() && !c.hasRoutes() && !c.hasHosts()
+	return c.hasDefaultResolvers() && !c.hasRoutes()
 }

 func (c Config) hasDefaultResolvers() bool {
@@ -76,31 +79,11 @@ func (c Config) singleResolverSet() []netaddr.IPPort {
 	return first
 }

-// hasHosts reports whether c requires resolution of MagicDNS hosts or
-// domains.
-func (c Config) hasHosts() bool {
-	return len(c.Hosts) > 0 || len(c.AuthoritativeSuffixes) > 0
-}
-
-// matchDomains returns the list of match suffixes needed by Routes,
-// AuthoritativeSuffixes. Hosts is not considered as we assume that
-// they're covered by AuthoritativeSuffixes for now.
+// matchDomains returns the list of match suffixes needed by Routes.
 func (c Config) matchDomains() []dnsname.FQDN {
-	ret := make([]dnsname.FQDN, 0, len(c.Routes)+len(c.AuthoritativeSuffixes))
-	seen := map[dnsname.FQDN]bool{}
-	for _, suffix := range c.AuthoritativeSuffixes {
-		if seen[suffix] {
-			continue
-		}
-		ret = append(ret, suffix)
-		seen[suffix] = true
-	}
+	ret := make([]dnsname.FQDN, 0, len(c.Routes))
 	for suffix := range c.Routes {
-		if seen[suffix] {
-			continue
-		}
 		ret = append(ret, suffix)
-		seen[suffix] = true
 	}
 	sort.Slice(ret, func(i, j int) bool {
 		return ret[i].WithTrailingDot() < ret[j].WithTrailingDot()
--- a/net/dns/manager.go
+++ b/net/dns/manager.go
@@ -6,7 +6,6 @@ package dns

 import (
 	"runtime"
-	"strings"
 	"time"

 	"inet.af/netaddr"
@@ -75,40 +74,51 @@ func (m *Manager) Set(cfg Config) error {

 // compileConfig converts cfg into a quad-100 resolver configuration
 // and an OS-level configuration.
-func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
+func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig, err error) {
+	authDomains := make(map[dnsname.FQDN]bool, len(cfg.AuthoritativeSuffixes))
+	for _, dom := range cfg.AuthoritativeSuffixes {
+		authDomains[dom] = true
+	}
+	addRoutes := func() {
+		for suffix, resolvers := range cfg.Routes {
+			// Don't add resolver routes for authoritative domains,
+			// since they're meant to be authoritatively handled
+			// internally.
+			if authDomains[suffix] {
+				continue
+			}
+			rcfg.Routes[suffix] = resolvers
+		}
+	}
+
+	// The internal resolver always gets MagicDNS hosts and
+	// authoritative suffixes, even if we don't propagate MagicDNS to
+	// the OS.
+	rcfg.Hosts = cfg.Hosts
+	rcfg.LocalDomains = cfg.AuthoritativeSuffixes
+	// Similarly, the OS always gets search paths.
+	ocfg.SearchDomains = cfg.SearchDomains
+
 	// Deal with trivial configs first.
 	switch {
 	case !cfg.needsOSResolver():
 		// Set search domains, but nothing else. This also covers the
 		// case where cfg is entirely zero, in which case these
 		// configs clear all Tailscale DNS settings.
-		return resolver.Config{}, OSConfig{
-			SearchDomains: cfg.SearchDomains,
-		}, nil
+		return rcfg, ocfg, nil
 	case cfg.hasDefaultResolversOnly():
 		// Trivial CorpDNS configuration, just override the OS
 		// resolver.
-		return resolver.Config{}, OSConfig{
-			Nameservers:   toIPsOnly(cfg.DefaultResolvers),
-			SearchDomains: cfg.SearchDomains,
-		}, nil
+		ocfg.Nameservers = toIPsOnly(cfg.DefaultResolvers)
+		return rcfg, ocfg, nil
 	case cfg.hasDefaultResolvers():
 		// Default resolvers plus other stuff always ends up proxying
 		// through quad-100.
-		rcfg := resolver.Config{
-			Routes: map[dnsname.FQDN][]netaddr.IPPort{
-				".": cfg.DefaultResolvers,
-			},
-			Hosts:        cfg.Hosts,
-			LocalDomains: cfg.AuthoritativeSuffixes,
-		}
-		for suffix, resolvers := range cfg.Routes {
-			rcfg.Routes[suffix] = resolvers
-		}
-		ocfg := OSConfig{
-			Nameservers:   []netaddr.IP{tsaddr.TailscaleServiceIP()},
-			SearchDomains: cfg.SearchDomains,
+		rcfg.Routes = map[dnsname.FQDN][]netaddr.IPPort{
+			".": cfg.DefaultResolvers,
 		}
+		addRoutes()
+		ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}
 		return rcfg, ocfg, nil
 	}

@@ -116,8 +126,6 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 	// configurations. The possible cases don't return directly any
 	// more, because as a final step we have to handle the case where
 	// the OS can't do split DNS.
-	var rcfg resolver.Config
-	var ocfg OSConfig

 	// Workaround for
 	// https://github.com/tailscale/corp/issues/1662. Even though
@@ -135,35 +143,20 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 	// This bool is used in a couple of places below to implement this
 	// workaround.
 	isWindows := runtime.GOOS == "windows"
-
-	// The windows check is for
-	// . See also below
-	// for further routing workarounds there.
-	if !cfg.hasHosts() && cfg.singleResolverSet() != nil && m.os.SupportsSplitDNS() && !isWindows {
+	if cfg.singleResolverSet() != nil && m.os.SupportsSplitDNS() && !isWindows {
 		// Split DNS configuration requested, where all split domains
 		// go to the same resolvers. We can let the OS do it.
-		return resolver.Config{}, OSConfig{
-			Nameservers:   toIPsOnly(cfg.singleResolverSet()),
-			SearchDomains: cfg.SearchDomains,
-			MatchDomains:  cfg.matchDomains(),
-		}, nil
+		ocfg.Nameservers = toIPsOnly(cfg.singleResolverSet())
+		ocfg.MatchDomains = cfg.matchDomains()
+		return rcfg, ocfg, nil
 	}

 	// Split DNS configuration with either multiple upstream routes,
 	// or routes + MagicDNS, or just MagicDNS, or on an OS that cannot
 	// split-DNS. Install a split config pointing at quad-100.
-	rcfg = resolver.Config{
-		Hosts:        cfg.Hosts,
-		LocalDomains: cfg.AuthoritativeSuffixes,
-		Routes:       map[dnsname.FQDN][]netaddr.IPPort{},
-	}
-	for suffix, resolvers := range cfg.Routes {
-		rcfg.Routes[suffix] = resolvers
-	}
-	ocfg = OSConfig{
-		Nameservers:   []netaddr.IP{tsaddr.TailscaleServiceIP()},
-		SearchDomains: cfg.SearchDomains,
-	}
+	rcfg.Routes = map[dnsname.FQDN][]netaddr.IPPort{}
+	addRoutes()
+	ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}

 	// If the OS can't do native split-dns, read out the underlying
 	// resolver config and blend it into our config.
@@ -173,28 +166,7 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 	if !m.os.SupportsSplitDNS() || isWindows {
 		bcfg, err := m.os.GetBaseConfig()
 		if err != nil {
-			// Temporary hack to make OSes where split-DNS isn't fully
-			// implemented yet not completely crap out, but instead
-			// fall back to quad-9 as a hardcoded "backup resolver".
-			//
-			// This codepath currently only triggers when opted into
-			// the split-DNS feature server side, and when at least
-			// one search domain is something within tailscale.com, so
-			// we don't accidentally leak unstable user DNS queries to
-			// quad-9 if we accidentally go down this codepath.
-			canUseHack := false
-			for _, dom := range cfg.SearchDomains {
-				if strings.HasSuffix(dom.WithoutTrailingDot(), ".tailscale.com") {
-					canUseHack = true
-					break
-				}
-			}
-			if !canUseHack {
-				return resolver.Config{}, OSConfig{}, err
-			}
-			bcfg = OSConfig{
-				Nameservers: []netaddr.IP{netaddr.IPv4(9, 9, 9, 9)},
-			}
+			return resolver.Config{}, OSConfig{}, err
 		}
 		rcfg.Routes["."] = toIPPorts(bcfg.Nameservers)
 		ocfg.SearchDomains = append(ocfg.SearchDomains, bcfg.SearchDomains...)
--- a/net/dns/manager_linux.go
+++ b/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 {
--- a/net/dns/manager_test.go
+++ b/net/dns/manager_test.go
@@ -76,6 +76,22 @@ func TestManager(t *testing.T) {
 				SearchDomains: fqdns("tailscale.com", "universe.tf"),
 			},
 		},
+		{
+			// Regression test for https://github.com/tailscale/tailscale/issues/1886
+			name: "hosts-only",
+			in: Config{
+				Hosts: hosts(
+					"dave.ts.com.", "1.2.3.4",
+					"bradfitz.ts.com.", "2.3.4.5"),
+				AuthoritativeSuffixes: fqdns("ts.com"),
+			},
+			rs: resolver.Config{
+				Hosts: hosts(
+					"dave.ts.com.", "1.2.3.4",
+					"bradfitz.ts.com.", "2.3.4.5"),
+				LocalDomains: fqdns("ts.com"),
+			},
+		},
 		{
 			name: "corp",
 			in: Config{
@@ -104,6 +120,7 @@ func TestManager(t *testing.T) {
 			in: Config{
 				DefaultResolvers: mustIPPs("1.1.1.1:53", "9.9.9.9:53"),
 				SearchDomains:    fqdns("tailscale.com", "universe.tf"),
+				Routes:           upstreams("ts.com", "100.100.100.100:53"),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
@@ -126,6 +143,7 @@ func TestManager(t *testing.T) {
 			in: Config{
 				DefaultResolvers: mustIPPs("1.1.1.1:53", "9.9.9.9:53"),
 				SearchDomains:    fqdns("tailscale.com", "universe.tf"),
+				Routes:           upstreams("ts.com", "100.100.100.100:53"),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
@@ -261,6 +279,7 @@ func TestManager(t *testing.T) {
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
+				Routes:                upstreams("ts.com", "100.100.100.100:53"),
 				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains:         fqdns("tailscale.com", "universe.tf"),
 			},
@@ -286,6 +305,7 @@ func TestManager(t *testing.T) {
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
+				Routes:                upstreams("ts.com", "100.100.100.100:53"),
 				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains:         fqdns("tailscale.com", "universe.tf"),
 			},
@@ -333,7 +353,9 @@ func TestManager(t *testing.T) {
 		{
 			name: "routes-magic-split",
 			in: Config{
-				Routes: upstreams("corp.com", "2.2.2.2:53"),
+				Routes: upstreams(
+					"corp.com", "2.2.2.2:53",
+					"ts.com", "100.100.100.100:53"),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
--- a/net/dns/nm.go
+++ b/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
--- a/net/dns/resolved.go
+++ b/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)
 	}

--- a/util/dnsname/dnsname.go
+++ b/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)
 	}
--- a/util/dnsname/dnsname_test.go
+++ b/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},
 	}
Author	SHA1	Message	Date
Avery Pennarun	f308995edf	ipnlocal: in Start() fast path, don't forget to send Prefs. The resulting empty Prefs had AllowSingleHosts=false and Routeall=false, so that on iOS if you did these steps: - Login and leave running - Terminate the frontend - Restart the frontend (fast path restart, missing prefs) - Set WantRunning=false - Set WantRunning=true ...then you would have Tailscale running, but with no routes. You would also accidentally disable the ExitNodeID/IP prefs (symptom: the current exit node setting didn't appear in the UI), but since nothing else worked either, you probably didn't notice. The fix was easy enough. It turns out we already knew about the problem, so this also fixes one of the BUG entries in state_test. Fixes: #1918 (BUG-1) and some as-yet-unreported bugs with exit nodes. Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>	2021-05-20 03:35:14 -04:00
Avery Pennarun	ee19c53063	ipnlocal: don't assume NeedsLogin immediately after StartLogout(). Previously, there was no server round trip required to log out, so when you asked ipnlocal to Logout(), it could clear the netmap immediately and switch to NeedsLogin state. In v1.8, we added a true Logout operation. ipn.Logout() would trigger an async cc.StartLogout() and also immediately switch to NeedsLogin. Unfortunately, some frontends would see NeedsLogin and immediately trigger a new StartInteractiveLogin() operation, before the controlclient auth state machine actually acted on the Logout command, thus accidentally invalidating the entire logout operation, retaining the netmap, and violating the user's expectations. Instead, add a new LogoutFinished signal from controlclient (paralleling LoginFinished) and, upon starting a logout, don't update the ipn state machine until it's received. Updates: #1918 (BUG-2) Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>	2021-05-20 03:35:14 -04:00
Avery Pennarun	d9facab159	ipnlocal: fix switching users while logged in + Stopped. This code path is very tricky since it was originally designed for the "re-authenticate to refresh my keys" use case, which didn't want to lose the original session even if the refresh cycle failed. This is why it acts differently from the Logout(); Login(); case. Maybe that's too fancy, considering that it probably never quite worked at all, for switching between users without logging out first. But it works now. This was more invasive than I hoped, but the necessary fixes actually removed several other suspicious BUG: lines from state_test.go, so I'm pretty confident this is a significant net improvement. Fixes tailscale/corp#1756. Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>	2021-05-20 01:08:06 -04:00
Avery Pennarun	30869d0f40	controlclient: update Persist.LoginName when it changes. Well, that was anticlimactic. Fixes tailscale/corp#461. Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>	2021-05-20 01:07:23 -04:00
Avery Pennarun	3a426a40b6	ipnlocal: fix deadlock in RequestEngineStatusAndWait() error path. If the engine was shutting down from a previous session (e.closing=true), it would return an error code when trying to get status. In that case, ipnlocal would never unblock any callers that were waiting on the status. Not sure if this ever happened in real life, but I accidentally triggered it while writing a test. Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>	2021-05-20 01:07:23 -04:00
David Crawshaw	256db54c9d	ipn: allow b to be nil in NewBackendServer A couple of code paths in ipnserver use a NewBackendServer with a nil backend just to call the callback with an encapsulated error message. This covers a panic case seen in logs. For #1920 Signed-off-by: David Crawshaw <crawshaw@tailscale.com>	2021-05-19 14:22:26 -04:00
David Anderson	f22185cae1	net/dns: always offer MagicDNS records at 100.100.100.100. Fixes #1886. Signed-off-by: David Anderson <danderson@tailscale.com> (cherry picked from commit `6690f86ef4`)	2021-05-18 14:20:47 -07:00
David Anderson	d0e86b08c9	VERSION.txt: this is v1.8.3 Signed-off-by: David Anderson <dave@natulte.net>	2021-05-10 16:39:53 -07:00
David Anderson	38a83e937f	net/dns: don't use interfaces.Tailscale to find the tailscale interface index. interfaces.Tailscale only returns an interface if it has at least one Tailscale IP assigned to it. In the resolved DNS manager, when we're called upon to tear down DNS config, the interface no longer has IPs. Instead, look up the interface index on construction and reuse it throughout the daemon lifecycle. Fixes #1892. Signed-off-by: David Anderson <dave@natulte.net> (cherry picked from commit `cfde997699`)	2021-05-10 15:27:36 -07:00
David Anderson	04d015abbf	VERSION.txt: this is v1.8.2 Signed-off-by: David Anderson <dave@natulte.net>	2021-05-10 13:35:04 -07:00
David Anderson	fdc48ff96f	Revert "net/dns: set IPv4 auto mode in NM, so it lets us set DNS." This reverts commit `7d16c8228b`. I have no idea how I ended up here. The bug I was fixing with this change fails to reproduce on Ubuntu 18.04 now, and this change definitely does break 20.04, 20.10, and Debian Buster. So, until we can reliably reproduce the problem this was meant to fix, reverting. Part of #1875 Signed-off-by: David Anderson <dave@natulte.net> (cherry picked from commit `3c8e230ee1`)	2021-05-10 13:08:10 -07:00
David Anderson	b308f6ad21	util/dnsname: normalize leading dots in ToFQDN. Fixes #1888. Signed-off-by: David Anderson <dave@natulte.net> (cherry picked from commit `dc32b4695c`)	2021-05-10 13:07:51 -07:00
Josh Bleecher Snyder	b8bd18b053	go.mod: pull in wintun alignment fix from upstream wireguard-go `6cd106ab13...030c638da3` Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>	2021-05-10 11:10:59 -07:00
Brad Fitzpatrick	87c804b481	cmd/tailscale: rewrite the "up" checker, fix bugs The old way was way too fragile and had felt like it had more special cases than normal cases. (see #1874, #1860, #1834, etc) It became very obvious the old algorithm didn't work when we made the output be pretty and try to show the user the command they need to run in `5ecc7c7200` for #1746) The new algorithm is to map the prefs (current and new) back to flags and then compare flags. This nicely handles the OS-specific flags and the n:1 and 1:n flag:pref cases. No change in the existing already-massive test suite, except some ordering differences (the missing items are now sorted), but some new tests are added for behavior that was broken before. In particular, it now: * preserves non-pref boolean flags set to false, and preserves exit node IPs (mapping them back from the ExitNodeID pref, as well as ExitNodeIP), * doesn't ignore --advertise-exit-node when doing an EditPrefs call (#1880) * doesn't lose the --operator on the non-EditPrefs paths (e.g. with --force-reauth, or when the backend was not in state Running). Fixes #1880 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> (cherry picked from commit `5190435d6e`)	2021-05-07 09:32:33 -07:00
Brad Fitzpatrick	d39db05b7c	ipn/{ipnlocal,ipnstate}: add PeerStatus.ID stable ID to status --json output Needed for the "up checker" to map back from exit node stable IDs (the ipn.Prefs.ExitNodeID) back to an IP address in error messages. But also previously requested so people can use it to then make API calls. The upcoming "tailscale admin" subcommand will probably need it too. Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> (cherry picked from commit `e72ed3fcc2`)	2021-05-07 09:32:28 -07:00
David Anderson	fbfdcc56ed	VERSION.txt: this is v1.8.1 Signed-off-by: David Anderson <danderson@tailscale.com>	2021-05-06 16:18:29 -07:00
David Anderson	f2a38320f8	net/dns: log the correct error when NM Reapply fails. Found while debugging #1870. Signed-off-by: David Anderson <danderson@tailscale.com> (cherry picked from commit `5bd38b10b4`)	2021-05-06 16:04:27 -07:00
David Anderson	71bcaae29e	net/dns: set IPv4 auto mode in NM, so it lets us set DNS. Part of #1870. Signed-off-by: David Anderson <danderson@tailscale.com> (cherry picked from commit `7d16c8228b`)	2021-05-06 16:04:23 -07:00
David Anderson	bf7c50dbdd	net/dns: don't try to configure LLMNR or mdns in NetworkManager. Fixes #1870. Signed-off-by: David Anderson <danderson@tailscale.com> (cherry picked from commit `77e2375501`)	2021-05-06 16:04:18 -07:00
Brad Fitzpatrick	ff4c2dbec9	cmd/tailscale: fix another up warning with exit nodes The --advertise-routes and --advertise-exit-node flags both mutating one pref is the gift that keeps on giving. I need to rewrite the this up warning code to first map prefs back to flag values and then just compare flags instead of comparing prefs, but this is the minimal fix for now. This also includes work on the tests, to make them easier to write (and more accurate), by letting you write the flag args directly and have that parse into the upArgs/MaskedPrefs directly, the same as the code, rather than them being possibly out of sync being written by hand. Fixes https://twitter.com/EXPbits/status/1390418145047887877 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> (cherry picked from commit `e78e26b6fb`)	2021-05-06 15:51:25 -07:00
David Crawshaw	e7899afbf6	VERSION.txt: this is v1.8.0 Signed-off-by: David Crawshaw <crawshaw@tailscale.com>	2021-05-06 07:08:25 -07:00
@@ -1 +1 @@
 .7.0
 .8.3