VERSION.txt: this is v1.8.1

Signed-off-by: David Anderson <danderson@tailscale.com>

VERSION.txt

@@ -1 +1 @@
-1.7.0
+1.8.1

cmd/tailscale/cli/cli_test.go

@@ -38,48 +38,42 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 		return m
 	}
 	tests := []struct {
-		name     string
-		flagSet  map[string]bool
+		name string
+
+		// flags, if non-nil populates flagSet and mp.
+		flags []string // if non-nil,
+
+		// flagSet and mp are required if flags is nil.
+		// flagSet is the set of flags that were explicitly set in flags.
+		// mp is the mutation to apply to server.
+		flagSet map[string]bool
+		mp      *ipn.MaskedPrefs
+
 		curPrefs *ipn.Prefs
 		curUser  string // os.Getenv("USER") on the client side
 		goos     string // empty means "linux"
-		mp       *ipn.MaskedPrefs
 		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",
 		},
@@ -184,47 +178,35 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			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",
+			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: "",
 		},
 		{
@@ -252,91 +234,44 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			want: "",
 		},
 		{
-			name:    "advertised_routes_includes_only_one_0_route", // and no --advertise-exit-node
-			flagSet: f("advertise-routes"),
+			name:  "advertise_exit_node", // Issue 1859
+			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.
+				ControlURL:       ipn.DefaultControlURL,
+				AllowSingleHosts: true,
+				CorpDNS:          true,
+				NetfilterMode:    preftype.NetfilterOn,
 			},
 			want: "",
 		},
 		{
-			name:    "advertise_exit_node_over_existing_routes",
-			flagSet: f("advertise-exit-node"),
+			name:  "advertise_exit_node_over_existing_routes",
+			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("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",
 		},
 		{
@@ -356,15 +291,15 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			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",
@@ -376,24 +311,18 @@ 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 + " --force-reauth --accept-routes --host-routes=false --exit-node=100.64.5.6 --accept-dns=false --shields-up --advertise-tags=tag:foo,tag:bar --hostname=myhostname --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
 		},
 		{
-			name:    "remove_all_implicit_except_hostname",
-			flagSet: f("hostname"),
+			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:          true,
+				CorpDNS:          false,
 				ShieldsUp:        true,
 				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
 				Hostname:         "myhostname",
@@ -405,15 +334,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 				OperatorUser:  "alice",
 			},
 			curUser: "eve",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:  ipn.DefaultControlURL,
-					WantRunning: true,
-					Hostname:    "newhostname",
-				},
-				HostnameSet: true,
-			},
-			want: accidentalUpPrefix + " --hostname=newhostname --accept-routes --exit-node=100.64.5.6 --accept-dns --shields-up --advertise-tags=tag:foo,tag:bar --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
+			want:    accidentalUpPrefix + " --hostname=newhostname --accept-routes --host-routes=false --exit-node=100.64.5.6 --accept-dns=false --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",
@@ -472,6 +393,38 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			},
 			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-routes=1.2.0.0/16 --advertise-exit-node",
+		},
+		{
+			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",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -479,8 +432,23 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			if tt.goos != "" {
 				goos = tt.goos
 			}
+			flagSet := tt.flagSet
+			mp := tt.mp
+			if tt.flags != nil {
+				if tt.flagSet != nil || tt.mp != nil {
+					t.Fatal("flags and flagSet/mp are mutually exclusive")
+				}
+				var upArgs upArgsT
+				fs := newUpFlagSet(goos, &upArgs)
+				fs.Parse(tt.flags)
+				prefs, err := prefsFromUpArgs(upArgs, t.Logf, new(ipnstate.Status), goos)
+				if err != nil {
+					t.Fatal(err)
+				}
+				flagSet, mp = flagSetAndMaskedPrefs(prefs, fs)
+			}
 			var got string
-			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp, goos, tt.curUser); err != nil {
+			if err := checkForAccidentalSettingReverts(flagSet, tt.curPrefs, mp, goos, tt.curUser); err != nil {
 				got = err.Error()
 			}
 			if strings.TrimSpace(got) != tt.want {

cmd/tailscale/cli/up.go

@@ -295,14 +295,7 @@ 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
-	})
+	flagSet, mp := flagSetAndMaskedPrefs(prefs, upFlagSet)
 
 	if !upArgs.reset {
 		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp, runtime.GOOS, os.Getenv("USER")); err != nil {
@@ -498,6 +491,18 @@ func addPrefFlagMapping(flagName string, prefNames ...string) {
 	}
 }
 
+func flagSetAndMaskedPrefs(prefs *ipn.Prefs, fs *flag.FlagSet) (flagSetMap map[string]bool, mp *ipn.MaskedPrefs) {
+	flagSetMap = map[string]bool{}
+	mp = new(ipn.MaskedPrefs)
+	mp.WantRunningSet = true
+	mp.Prefs = *prefs
+	fs.Visit(func(f *flag.Flag) {
+		updateMaskedPrefsFromUpFlag(mp, f.Name)
+		flagSetMap[f.Name] = true
+	})
+	return flagSetMap, mp
+}
+
 func updateMaskedPrefsFromUpFlag(mp *ipn.MaskedPrefs, flagName string) {
 	if prefs, ok := prefsOfFlag[flagName]; ok {
 		for _, pref := range prefs {
@@ -635,8 +640,12 @@ func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Pre
 				missing = append(missing, fmtFlagValueArg("exit-node", fmtSettingVal(exi)))
 			}
 		case "advertise-routes":
+			hadExitNode := hasExitNodeRoutes(exi.([]netaddr.IPPrefix))
 			routes := withoutExitNodes(exi.([]netaddr.IPPrefix))
 			missing = append(missing, fmtFlagValueArg("advertise-routes", fmtSettingVal(routes)))
+			if hadExitNode && !flagSet["advertise-exit-node"] {
+				missing = append(missing, "--advertise-exit-node")
+			}
 		default:
 			missing = append(missing, fmtFlagValueArg(flagName, fmtSettingVal(exi)))
 		}

net/dns/nm.go

@@ -175,11 +175,33 @@ 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"]
+	// In some versions of NM, you can only provide DNS configuration
+	// if the interface is in "auto" or "manual" mode (defaults to
+	// "disabled").
+	//
+	// In "manual" mode we'd have to also give NM all our IP and
+	// routing configuration, which we don't want to do. So instead,
+	// we set the ipv4 mode to "auto", and then tell it to ignore any
+	// information it might get from DHCP (which is none, since we
+	// don't do DHCP on tailscale0). This is the documented way to
+	// provide only DNS configuration to NetworkManager.
+	//
+	// Some versions of NM don't need this, and will accept DNS
+	// configuration even for disabled interfaces, but there's no harm
+	// in telling those versions to do DHCP and ignore all results,
+	// hence why we have a single codepath here.
+	ipv4Map["method"] = dbus.MakeVariant("auto")
+	ipv4Map["ignore-auto-routes"] = dbus.MakeVariant(true)
+	ipv4Map["ignore-auto-dns"] = dbus.MakeVariant(true)
+	ipv4Map["never-default"] = dbus.MakeVariant(true)
 	ipv4Map["dns"] = dbus.MakeVariant(dnsv4)
 	ipv4Map["dns-search"] = dbus.MakeVariant(search)
 	// We should only request priority if we have nameservers to set.
@@ -199,18 +221,9 @@ func (m *nmManager) trySet(ctx context.Context, config OSConfig) error {
 	}
 
 	ipv6Map := settings["ipv6"]
-	// In IPv6 settings, you're only allowed to provide additional
-	// static DNS settings in "auto" (SLAAC) or "manual" mode. In
-	// "manual" mode you also have to specify IP addresses, so we use
-	// "auto".
-	//
-	// NM actually documents that to set just DNS servers, you should
-	// use "auto" mode and then set ignore auto routes and DNS, which
-	// basically means "autoconfigure but ignore any autoconfiguration
-	// results you might get". As a safety, we also say that
-	// NetworkManager should never try to make us the default route
-	// (none of its business anyway, we handle our own default
-	// routing).
+	// Same logic as above for IPv4 - request autoconfiguration, tell
+	// NM to ignore all autoconfiguration results, and provide manual
+	// DNS settings.
 	ipv6Map["method"] = dbus.MakeVariant("auto")
 	if len(addrs6) > 0 {
 		ipv6Map["address-data"] = dbus.MakeVariant(addrs6)
@@ -247,7 +260,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