From 8161024176ce3adaf410295f4b6eee738cba0471 Mon Sep 17 00:00:00 2001
From: Andrew Dunham <andrew@du.nham.ca>
Date: Tue, 25 Jun 2024 22:23:19 -0400
Subject: [PATCH] wgengine/magicsock: always set home DERP if no control conn

The logic we added in #11378 would prevent selecting a home DERP if we
have no control connection.

Updates tailscale/corp#18095

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I44bb6ac4393989444e4961b8cfa27dc149a33c6e
---
 wgengine/magicsock/derp.go           | 15 +++++++++++++--
 wgengine/magicsock/magicsock_test.go | 15 +++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go
index 8a1c4d367..1ef63112b 100644
--- a/wgengine/magicsock/derp.go
+++ b/wgengine/magicsock/derp.go
@@ -158,6 +158,10 @@ func (c *Conn) maybeSetNearestDERP(report *netcheck.Report) (preferredDERP int)
 	//
 	// For tests, always assume we're connected to control unless we're
 	// explicitly testing this behaviour.
+	//
+	// Despite the above behaviour, ensure that we set the nearest DERP if
+	// we don't currently have one set; any DERP server is better than
+	// none, even if not connected to control.
 	var connectedToControl bool
 	if testenv.InTest() && !checkControlHealthDuringNearestDERPInTests {
 		connectedToControl = true
@@ -166,8 +170,15 @@ func (c *Conn) maybeSetNearestDERP(report *netcheck.Report) (preferredDERP int)
 	}
 	if !connectedToControl {
 		c.mu.Lock()
-		defer c.mu.Unlock()
-		return c.myDerp
+		myDerp := c.myDerp
+		c.mu.Unlock()
+		if myDerp != 0 {
+			return myDerp
+		}
+
+		// Intentionally fall through; we don't have a current DERP, so
+		// as mentioned above selecting one even if not connected is
+		// strictly better than doing nothing.
 	}
 
 	preferredDERP = report.PreferredDERP
diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index 2df05fcb0..cec05dffc 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -1276,6 +1276,7 @@ func newTestConn(t testing.TB) *Conn {
 
 	conn, err := NewConn(Options{
 		NetMon:                 netMon,
+		HealthTracker:          new(health.Tracker),
 		DisablePortMapper:      true,
 		Logf:                   t.Logf,
 		Port:                   port,
@@ -3128,6 +3129,20 @@ func TestMaybeSetNearestDERP(t *testing.T) {
 			connectedToControl: false,
 			want:               1, // no change
 		},
+		{
+			name:               "not_connected_with_report_derp_and_no_current",
+			old:                0,     // no current DERP
+			reportDERP:         21,    // have new DERP
+			connectedToControl: false, // not connected...
+			want:               21,    // ... but want to change to new DERP
+		},
+		{
+			name:               "not_connected_with_fallback_and_no_current",
+			old:                0,     // no current DERP
+			reportDERP:         0,     // no new DERP
+			connectedToControl: false, // not connected...
+			want:               31,    // ... but we fallback to deterministic value
+		},
 		{
 			name:               "connected_no_derp",
 			old:                1,