From 3bdcfa719305618aca2c339d402205a0b4f6d5b2 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Fri, 18 Sep 2020 07:44:01 -0700
Subject: [PATCH] ipn: remove DisableDERP pref

We depend on DERP for NAT traversal now[0] so disabling it entirely can't
work.

What we'll do instead in the future is let people specify
alternate/additional DERP servers. And perhaps in the future we could
also add a pref for nodes to say when they expect to never need/want
to use DERP for data (but allow it for NAT traversal communication).

But this isn't the right pref and it doesn't work, so delete it.

Fixes #318

[0] https://tailscale.com/blog/how-nat-traversal-works/
---
 cmd/tailscale/cli/up.go |  3 ---
 ipn/local.go            | 15 ++-------------
 ipn/prefs.go            |  8 ++------
 ipn/prefs_test.go       |  2 +-
 4 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go
index 9b57a573a..a1b9a65fe 100644
--- a/cmd/tailscale/cli/up.go
+++ b/cmd/tailscale/cli/up.go
@@ -60,7 +60,6 @@
 		upf.StringVar(&upArgs.advertiseTags, "advertise-tags", "", "ACL tags to request (comma-separated, e.g. eng,montreal,ssh)")
 		upf.StringVar(&upArgs.authKey, "authkey", "", "node authorization key")
 		upf.StringVar(&upArgs.hostname, "hostname", "", "hostname to use instead of the one provided by the OS")
-		upf.BoolVar(&upArgs.enableDERP, "enable-derp", true, "enable the use of DERP servers")
 		if runtime.GOOS == "linux" || isBSD(runtime.GOOS) || version.OS() == "macOS" {
 			upf.StringVar(&upArgs.advertiseRoutes, "advertise-routes", "", "routes to advertise to other nodes (comma-separated, e.g. 10.0.0.0/8,192.168.0.0/24)")
 		}
@@ -89,7 +88,6 @@ func defaultNetfilterMode() string {
 	forceReauth     bool
 	advertiseRoutes string
 	advertiseTags   string
-	enableDERP      bool
 	snat            bool
 	netfilterMode   string
 	authKey         string
@@ -216,7 +214,6 @@ func runUp(ctx context.Context, args []string) error {
 	prefs.AdvertiseRoutes = routes
 	prefs.AdvertiseTags = tags
 	prefs.NoSNAT = !upArgs.snat
-	prefs.DisableDERP = !upArgs.enableDERP
 	prefs.Hostname = upArgs.hostname
 	if runtime.GOOS == "linux" {
 		switch upArgs.netfilterMode {
diff --git a/ipn/local.go b/ipn/local.go
index c981e66ff..4306eb263 100644
--- a/ipn/local.go
+++ b/ipn/local.go
@@ -273,17 +273,10 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 
 		b.updateFilter(st.NetMap, prefs)
 		b.e.SetNetworkMap(st.NetMap)
-
 		if !dnsMapsEqual(st.NetMap, netMap) {
 			b.updateDNSMap(st.NetMap)
 		}
-
-		disableDERP := prefs != nil && prefs.DisableDERP
-		if disableDERP {
-			b.e.SetDERPMap(nil)
-		} else {
-			b.e.SetDERPMap(st.NetMap.DERPMap)
-		}
+		b.e.SetDERPMap(st.NetMap.DERPMap)
 
 		b.send(Notify{NetMap: st.NetMap})
 	}
@@ -869,11 +862,7 @@ func (b *LocalBackend) SetPrefs(new *Prefs) {
 
 	b.updateFilter(netMap, new)
 
-	turnDERPOff := new.DisableDERP && !old.DisableDERP
-	turnDERPOn := !new.DisableDERP && old.DisableDERP
-	if turnDERPOff {
-		b.e.SetDERPMap(nil)
-	} else if turnDERPOn && netMap != nil {
+	if netMap != nil {
 		b.e.SetDERPMap(netMap.DERPMap)
 	}
 
diff --git a/ipn/prefs.go b/ipn/prefs.go
index 5febba4e9..e3c306aad 100644
--- a/ipn/prefs.go
+++ b/ipn/prefs.go
@@ -67,9 +67,6 @@ type Prefs struct {
 	// users narrow it down a bit.
 	NotepadURLs bool
 
-	// DisableDERP prevents DERP from being used.
-	DisableDERP bool
-
 	// The following block of options only have an effect on Linux.
 
 	// AdvertiseRoutes specifies CIDR prefixes to advertise into the
@@ -109,9 +106,9 @@ func (p *Prefs) Pretty() string {
 	} else {
 		pp = "Persist=nil"
 	}
-	return fmt.Sprintf("Prefs{ra=%v mesh=%v dns=%v want=%v notepad=%v derp=%v shields=%v routes=%v snat=%v nf=%v %v}",
+	return fmt.Sprintf("Prefs{ra=%v mesh=%v dns=%v want=%v notepad=%v shields=%v routes=%v snat=%v nf=%v %v}",
 		p.RouteAll, p.AllowSingleHosts, p.CorpDNS, p.WantRunning,
-		p.NotepadURLs, !p.DisableDERP, p.ShieldsUp, p.AdvertiseRoutes, !p.NoSNAT, p.NetfilterMode, pp)
+		p.NotepadURLs, p.ShieldsUp, p.AdvertiseRoutes, !p.NoSNAT, p.NetfilterMode, pp)
 }
 
 func (p *Prefs) ToBytes() []byte {
@@ -137,7 +134,6 @@ func (p *Prefs) Equals(p2 *Prefs) bool {
 		p.CorpDNS == p2.CorpDNS &&
 		p.WantRunning == p2.WantRunning &&
 		p.NotepadURLs == p2.NotepadURLs &&
-		p.DisableDERP == p2.DisableDERP &&
 		p.ShieldsUp == p2.ShieldsUp &&
 		p.NoSNAT == p2.NoSNAT &&
 		p.NetfilterMode == p2.NetfilterMode &&
diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go
index 4f75095dc..a11bd7531 100644
--- a/ipn/prefs_test.go
+++ b/ipn/prefs_test.go
@@ -24,7 +24,7 @@ func fieldsOf(t reflect.Type) (fields []string) {
 func TestPrefsEqual(t *testing.T) {
 	tstest.PanicOnLog()
 
-	prefsHandles := []string{"ControlURL", "RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "ShieldsUp", "AdvertiseTags", "Hostname", "OSVersion", "DeviceModel", "NotepadURLs", "DisableDERP", "AdvertiseRoutes", "NoSNAT", "NetfilterMode", "Persist"}
+	prefsHandles := []string{"ControlURL", "RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "ShieldsUp", "AdvertiseTags", "Hostname", "OSVersion", "DeviceModel", "NotepadURLs", "AdvertiseRoutes", "NoSNAT", "NetfilterMode", "Persist"}
 	if have := fieldsOf(reflect.TypeOf(Prefs{})); !reflect.DeepEqual(have, prefsHandles) {
 		t.Errorf("Prefs.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
 			have, prefsHandles)