From 339397ab74882fe1f2a450f824d582093407009a Mon Sep 17 00:00:00 2001
From: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
Date: Thu, 6 Jul 2023 11:11:21 -0700
Subject: [PATCH] wgengine/magicsock: remove noV4/noV6 check in
 addrForSendWireGuardLocked

This change removes the noV4/noV6 check from addrForSendWireGuardLocked.

On Android, the client panics when reaching	`rand.Intn()`, likely due to
the candidates list being containing no candidates. The suspicion is
that the `noV4` and the `noV6` are both being triggered causing the
loop to continue.

Updates tailscale/corp#12938
Updates #7826

Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>
---
 wgengine/magicsock/magicsock.go      | 15 ++++++-------
 wgengine/magicsock/magicsock_test.go | 32 ----------------------------
 2 files changed, 6 insertions(+), 41 deletions(-)

diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index dea8b2d97..2136bdb1d 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -30,6 +30,7 @@ import (
 
 	"github.com/tailscale/wireguard-go/conn"
 	"go4.org/mem"
+	"golang.org/x/exp/maps"
 	"golang.org/x/net/ipv4"
 	"golang.org/x/net/ipv6"
 	"tailscale.com/control/controlclient"
@@ -4409,16 +4410,12 @@ func (de *endpoint) addrForWireGuardSendLocked(now mono.Time) (udpAddr netip.Add
 		return udpAddr, false
 	}
 
-	candidates := make([]netip.AddrPort, 0, len(de.endpointState))
-	for ipp := range de.endpointState {
-		if ipp.Addr().Is4() && de.c.noV4.Load() {
-			continue
-		}
-		if ipp.Addr().Is6() && de.c.noV6.Load() {
-			continue
-		}
-		candidates = append(candidates, ipp)
+	candidates := maps.Keys(de.endpointState)
+	if len(candidates) == 0 {
+		de.c.logf("magicsock: addrForSendWireguardLocked: [unexpected] no candidates available for endpoint")
+		return udpAddr, false
 	}
+
 	// Randomly select an address to use until we retrieve latency information
 	// and give it a short trustBestAddrUntil time so we avoid flapping between
 	// addresses while waiting on latency information to be populated.
diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index 78e5bb232..ff3a25ab4 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -2809,36 +2809,6 @@ func TestAddrForSendLockedForWireGuardOnly(t *testing.T) {
 			},
 			want: netip.MustParseAddrPort("[2345:0425:2CA1:0000:0000:0567:5673:23b5]:222"),
 		},
-		{
-			name:       "choose IPv4 when IPv6 is not useable",
-			sendWGPing: false,
-			noV6:       true,
-			ep: []endpointDetails{
-				{
-					addrPort: netip.MustParseAddrPort("1.1.1.1:111"),
-					latency:  100 * time.Millisecond,
-				},
-				{
-					addrPort: netip.MustParseAddrPort("[1::1]:567"),
-				},
-			},
-			want: netip.MustParseAddrPort("1.1.1.1:111"),
-		},
-		{
-			name:       "choose IPv6 when IPv4 is not useable",
-			sendWGPing: false,
-			noV4:       true,
-			ep: []endpointDetails{
-				{
-					addrPort: netip.MustParseAddrPort("1.1.1.1:111"),
-				},
-				{
-					addrPort: netip.MustParseAddrPort("[1::1]:567"),
-					latency:  100 * time.Millisecond,
-				},
-			},
-			want: netip.MustParseAddrPort("[1::1]:567"),
-		},
 		{
 			name:       "choose IPv6 address when latency is the same for v4 and v6",
 			sendWGPing: true,
@@ -2865,8 +2835,6 @@ func TestAddrForSendLockedForWireGuardOnly(t *testing.T) {
 				noV6: atomic.Bool{},
 			},
 		}
-		endpoint.c.noV4.Store(test.noV4)
-		endpoint.c.noV6.Store(test.noV6)
 
 		for _, epd := range test.ep {
 			endpoint.endpointState[epd.addrPort] = &endpointState{}