From 5c24f0ed803a0f60d3a05f148f3e20f99f3d00d7 Mon Sep 17 00:00:00 2001
From: Alex Chan <alexc@tailscale.com>
Date: Mon, 15 Sep 2025 06:53:41 +0100
Subject: [PATCH] wgengine/magicsock: send a valid payload in
 TestNetworkDownSendErrors

This test ostensibly checks whether we record an error metric if a packet
is dropped because the network is down, but the network connectivity is
irrelevant -- the send error is actually because the arguments to Send()
are invalid:

    RebindingUDPConn.WriteWireGuardBatchTo:
    [unexpected] offset (0) != Geneve header length (8)

This patch changes the test so we try to send a valid packet, and we
verify this by sending it once before taking the network down.  The new
error is:

    magicsock: network down

which is what we're trying to test.

We then test sending an invalid payload as a separate test case.

Updates tailscale/corp#22075

Signed-off-by: Alex Chan <alexc@tailscale.com>
---
 wgengine/magicsock/magicsock_test.go | 83 +++++++++++++++++++++++-----
 1 file changed, 69 insertions(+), 14 deletions(-)

diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index 5774432d5..bb5922c8c 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -3131,34 +3131,89 @@ func TestMaybeRebindOnError(t *testing.T) {
 	})
 }
 
-func TestNetworkDownSendErrors(t *testing.T) {
+func newTestConnAndRegistry(t *testing.T) (*Conn, *usermetric.Registry, func()) {
+	t.Helper()
 	bus := eventbus.New()
-	defer bus.Close()
-
 	netMon := must.Get(netmon.New(bus, t.Logf))
-	defer netMon.Close()
 
 	reg := new(usermetric.Registry)
+
 	conn := must.Get(NewConn(Options{
 		DisablePortMapper: true,
 		Logf:              t.Logf,
 		NetMon:            netMon,
-		Metrics:           reg,
 		EventBus:          bus,
+		Metrics:           reg,
 	}))
-	defer conn.Close()
 
-	conn.SetNetworkUp(false)
-	if err := conn.Send([][]byte{{00}}, &lazyEndpoint{}, 0); err == nil {
-		t.Error("expected error, got nil")
-	}
-	resp := httptest.NewRecorder()
-	reg.Handler(resp, new(http.Request))
-	if !strings.Contains(resp.Body.String(), `tailscaled_outbound_dropped_packets_total{reason="error"} 1`) {
-		t.Errorf("expected NetworkDown to increment packet dropped metric; got %q", resp.Body.String())
+	return conn, reg, func() {
+		bus.Close()
+		netMon.Close()
+		conn.Close()
 	}
 }
 
+func TestNetworkSendErrors(t *testing.T) {
+	t.Run("network-down", func(t *testing.T) {
+		// TODO(alexc): This test case fails on Windows because it never
+		// successfully sends the first packet:
+		//
+		// 	   expected successful Send, got err: "write udp4 0.0.0.0:57516->127.0.0.1:9999:
+		// 	   wsasendto: The requested address is not valid in its context."
+		//
+		// It would be nice to run this test on Windows, but I was already
+		// on a side quest and it was unclear if this test has ever worked
+		// correctly on Windows.
+		if runtime.GOOS == "windows" {
+			t.Skipf("skipping on %s", runtime.GOOS)
+		}
+
+		conn, reg, close := newTestConnAndRegistry(t)
+		defer close()
+
+		buffs := [][]byte{{00, 00, 00, 00, 00, 00, 00, 00}}
+		ep := &lazyEndpoint{
+			src: epAddr{ap: netip.MustParseAddrPort("127.0.0.1:9999")},
+		}
+		offset := 8
+
+		// Check this is a valid payload to send when the network is up
+		conn.SetNetworkUp(true)
+		if err := conn.Send(buffs, ep, offset); err != nil {
+			t.Errorf("expected successful Send, got err: %q", err)
+		}
+
+		// Now we know the payload would be sent if the network is up,
+		// send it again when the network is down
+		conn.SetNetworkUp(false)
+		err := conn.Send(buffs, ep, offset)
+		if err == nil {
+			t.Error("expected error, got nil")
+		}
+		resp := httptest.NewRecorder()
+		reg.Handler(resp, new(http.Request))
+		if !strings.Contains(resp.Body.String(), `tailscaled_outbound_dropped_packets_total{reason="error"} 1`) {
+			t.Errorf("expected NetworkDown to increment packet dropped metric; got %q", resp.Body.String())
+		}
+	})
+
+	t.Run("invalid-payload", func(t *testing.T) {
+		conn, reg, close := newTestConnAndRegistry(t)
+		defer close()
+
+		conn.SetNetworkUp(false)
+		err := conn.Send([][]byte{{00}}, &lazyEndpoint{}, 0)
+		if err == nil {
+			t.Error("expected error, got nil")
+		}
+		resp := httptest.NewRecorder()
+		reg.Handler(resp, new(http.Request))
+		if !strings.Contains(resp.Body.String(), `tailscaled_outbound_dropped_packets_total{reason="error"} 1`) {
+			t.Errorf("expected invalid payload to increment packet dropped metric; got %q", resp.Body.String())
+		}
+	})
+}
+
 func Test_packetLooksLike(t *testing.T) {
 	discoPub := key.DiscoPublicFromRaw32(mem.B([]byte{1: 1, 30: 30, 31: 31}))
 	nakedDisco := make([]byte, 0, 512)