From 2404c0ffad916ab92b4530e38a71b682ab956789 Mon Sep 17 00:00:00 2001
From: David Anderson <danderson@tailscale.com>
Date: Wed, 24 Feb 2021 20:05:23 -0800
Subject: [PATCH] ipn/ipnlocal: only filter out default routes when computing
 the local wg config.

UIs need to see the full unedited netmap in order to know what exit nodes they
can offer to the user.

Signed-off-by: David Anderson <danderson@tailscale.com>
---
 ipn/ipnlocal/local.go                | 56 ++++++++++------------------
 wgengine/magicsock/magicsock_test.go |  2 +-
 wgengine/wgcfg/nmcfg/nmcfg.go        |  7 +++-
 3 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index ac6dca473..e3cd5a27e 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -310,7 +310,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		prefsChanged = true
 	}
 	if st.NetMap != nil {
-		if b.keepOneExitNodeLocked(st.NetMap) {
+		if b.findExitNodeID(st.NetMap) {
 			prefsChanged = true
 		}
 		b.setNetMapLocked(st.NetMap)
@@ -371,51 +371,35 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 	b.authReconfig()
 }
 
-// keepOneExitNodeLocked edits nm to retain only the default
-// routes provided by the exit node specified in b.prefs. It returns
-// whether prefs was mutated as part of the process, due to an exit
-// node IP being converted into a node ID.
-func (b *LocalBackend) keepOneExitNodeLocked(nm *netmap.NetworkMap) (prefsChanged bool) {
+// findExitNodeID updates b.prefs to reference an exit node by ID,
+// rather than by IP. It returns whether prefs was mutated.
+func (b *LocalBackend) findExitNodeID(nm *netmap.NetworkMap) (prefsChanged bool) {
 	// If we have a desired IP on file, try to find the corresponding
 	// node.
-	if !b.prefs.ExitNodeIP.IsZero() {
-		// IP takes precedence over ID, so if both are set, clear ID.
-		if b.prefs.ExitNodeID != "" {
-			b.prefs.ExitNodeID = ""
-			prefsChanged = true
-		}
+	if b.prefs.ExitNodeIP.IsZero() {
+		return false
+	}
 
-	peerLoop:
-		for _, peer := range nm.Peers {
-			for _, addr := range peer.Addresses {
-				if !addr.IsSingleIP() || addr.IP != b.prefs.ExitNodeIP {
-					continue
-				}
-				// Found the node being referenced, upgrade prefs to
-				// reference it directly for next time.
-				b.prefs.ExitNodeID = peer.StableID
-				b.prefs.ExitNodeIP = netaddr.IP{}
-				prefsChanged = true
-				break peerLoop
-			}
-		}
+	// IP takes precedence over ID, so if both are set, clear ID.
+	if b.prefs.ExitNodeID != "" {
+		b.prefs.ExitNodeID = ""
+		prefsChanged = true
 	}
 
-	// At this point, we have a node ID if the requested node is in
-	// the netmap. If not, the ID will be empty, and we'll strip out
-	// all default routes.
 	for _, peer := range nm.Peers {
-		out := peer.AllowedIPs[:0]
-		for _, allowedIP := range peer.AllowedIPs {
-			if allowedIP.Bits == 0 && peer.StableID != b.prefs.ExitNodeID {
+		for _, addr := range peer.Addresses {
+			if !addr.IsSingleIP() || addr.IP != b.prefs.ExitNodeIP {
 				continue
 			}
-			out = append(out, allowedIP)
+			// Found the node being referenced, upgrade prefs to
+			// reference it directly for next time.
+			b.prefs.ExitNodeID = peer.StableID
+			b.prefs.ExitNodeIP = netaddr.IP{}
+			return true
 		}
-		peer.AllowedIPs = out
 	}
 
-	return prefsChanged
+	return false
 }
 
 // setWgengineStatus is the callback by the wireguard engine whenever it posts a new status.
@@ -1274,7 +1258,7 @@ func (b *LocalBackend) authReconfig() {
 		}
 	}
 
-	cfg, err := nmcfg.WGCfg(nm, b.logf, flags)
+	cfg, err := nmcfg.WGCfg(nm, b.logf, flags, uc.ExitNodeID)
 	if err != nil {
 		b.logf("wgcfg: %v", err)
 		return
diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index 54f11845f..914533a73 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -297,7 +297,7 @@ func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
 				peerSet[key.Public(peer.Key)] = struct{}{}
 			}
 			m.conn.UpdatePeers(peerSet)
-			wg, err := nmcfg.WGCfg(nm, logf, netmap.AllowSingleHosts)
+			wg, err := nmcfg.WGCfg(nm, logf, netmap.AllowSingleHosts, "")
 			if err != nil {
 				// We're too far from the *testing.T to be graceful,
 				// blow up. Shouldn't happen anyway.
diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go
index 36dc065c8..57053ea78 100644
--- a/wgengine/wgcfg/nmcfg/nmcfg.go
+++ b/wgengine/wgcfg/nmcfg/nmcfg.go
@@ -52,7 +52,7 @@ func cidrIsSubnet(node *tailcfg.Node, cidr netaddr.IPPrefix) bool {
 }
 
 // WGCfg returns the NetworkMaps's Wireguard configuration.
-func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags) (*wgcfg.Config, error) {
+func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags, exitNode tailcfg.StableNodeID) (*wgcfg.Config, error) {
 	cfg := &wgcfg.Config{
 		Name:       "tailscale",
 		PrivateKey: wgcfg.PrivateKey(nm.PrivateKey),
@@ -89,7 +89,10 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags)
 			}
 		}
 		for _, allowedIP := range peer.AllowedIPs {
-			if allowedIP.IsSingleIP() && tsaddr.IsTailscaleIP(allowedIP.IP) && (flags&netmap.AllowSingleHosts) == 0 {
+			if allowedIP.Bits == 0 && peer.StableID != exitNode {
+				logf("[v1] wgcfg: skipping unselected default route from %q (%v)", nodeDebugName(peer), peer.Key.ShortString())
+				continue
+			} else if allowedIP.IsSingleIP() && tsaddr.IsTailscaleIP(allowedIP.IP) && (flags&netmap.AllowSingleHosts) == 0 {
 				logf("[v1] wgcfg: skipping node IP %v from %q (%v)",
 					allowedIP.IP, nodeDebugName(peer), peer.Key.ShortString())
 				continue