From 40c98cd267e392a6de53ab4836eab1882cea5023 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 3 Jun 2026 03:40:07 +0000 Subject: [PATCH] tstest/natlab/vmtest: deflake, de-strictify TestSelfSignedDERPHashPinning The test was asserting that a tailnet ping between two nodes traversed DERP rather than going direct. But that wasn't really the point of the test, and I kept forgetting ways that magicsock could find direct paths and thus break this test. So loosen it. We really just want to see whether DERP worked at all and was used in the process of getting a ping through, whether it was direct or not. And that "tailscale debug derp" worked at all, which was what the bug was about to begin with. No need for all the "must be over DERP" stuff. Updates #15579 Change-Id: I70ca63dc10919efa3d193b7af1d31a4a3b9d3950 Signed-off-by: Brad Fitzpatrick --- tstest/natlab/vmtest/selfsignedderp_test.go | 62 ++++++++++++++------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/tstest/natlab/vmtest/selfsignedderp_test.go b/tstest/natlab/vmtest/selfsignedderp_test.go index e41b4e552..752e04738 100644 --- a/tstest/natlab/vmtest/selfsignedderp_test.go +++ b/tstest/natlab/vmtest/selfsignedderp_test.go @@ -10,25 +10,25 @@ import ( "testing" "time" + "tailscale.com/tailcfg" + "tailscale.com/tstest" "tailscale.com/tstest/natlab/vmtest" "tailscale.com/tstest/natlab/vnet" ) -// hardDualNoEndpoints is hard NAT with both an IPv4 LAN and an IPv6 prefix -// (so the DebugDERPRegion probe exercises both address families against the -// test DERP server) and TS_DEBUG_STRIP_ENDPOINTS=1 set on tailscaled so it -// doesn't announce any direct endpoints to peers. Combined on both nodes, -// that leaves DERP as the only available path for the tailnet ping. The -// home DERP itself is left alone so the sha256-raw verification path is -// still exercised. -func hardDualNoEndpoints(env *vmtest.Env) *vmtest.Node { +// easyAnd6NoEndpoints is easy NAT plus an IPv6 prefix and +// TS_DEBUG_STRIP_ENDPOINTS=1 on tailscaled so peer endpoints from control +// are dropped. That forces the initial peer-to-peer disco bootstrap to +// traverse DERP, since without endpoints from control the nodes have no +// other way to first learn about each other. +func easyAnd6NoEndpoints(env *vmtest.Env) *vmtest.Node { n := env.NumNodes() return env.AddNode(fmt.Sprintf("node-%d", n), env.AddNetwork( fmt.Sprintf("2.%d.%d.%d", n, n, n), // public IP - fmt.Sprintf("10.0.%d.1/24", n), + fmt.Sprintf("192.168.%d.1/24", n), v6cidr(n), - vnet.HardNAT), + vnet.EasyNAT), vnet.TailscaledEnv{Key: "TS_DEBUG_STRIP_ENDPOINTS", Value: "1"}, vmtest.OS(vmtest.Gokrazy)) } @@ -39,12 +39,11 @@ func hardDualNoEndpoints(env *vmtest.Env) *vmtest.Node { // fronting CertName), the two nodes communicate over the resulting tailnet, // and `tailscale debug derp` against the same region succeeds. // -// Both nodes sit behind hard NATs and additionally strip their direct -// endpoints (TS_DEBUG_STRIP_ENDPOINTS=1) so disco cannot find a direct path -// and the tailnet ping must traverse DERP, making the sha256-raw pinning of -// the tailscaled→DERP path part of the assertion. (Stripping endpoints — not -// just relying on hard NAT — is needed because the dual-stack LAN provides a -// non-NATted IPv6 path that the nodes would otherwise discover.) +// Nodes are dual-stack (v4 + v6) so the DebugDERPRegion probe exercises both +// address families against the test DERP server. They additionally strip +// peer endpoints from control so the initial peer-to-peer disco bootstrap +// must traverse DERP; the eventual data path may be direct or DERP, the +// test doesn't care, only that DERP worked end-to-end. // // The debug-derp half is the regression test for the bug fixed in PR #19965: // before that change, [ipn/localapi.serveDebugDERPRegion] passed the raw @@ -52,14 +51,39 @@ func hardDualNoEndpoints(env *vmtest.Env) *vmtest.Node { // failed with a hostname mismatch. func TestSelfSignedDERPHashPinning(t *testing.T) { env := vmtest.New(t, vmtest.SelfSignedDERPCertPinning()) - n1 := hardDualNoEndpoints(env) - n2 := hardDualNoEndpoints(env) + n1 := easyAnd6NoEndpoints(env) + n2 := easyAnd6NoEndpoints(env) env.Start() - if err := env.PingExpect(n1, n2, vmtest.PingRouteDERP, 60*time.Second); err != nil { + // End-to-end ping over the WireGuard tunnel. With peer endpoints + // stripped from control, the only way for the peers to first reach each + // other is via DERP, so a successful tunnel ping proves the + // tailscaled→DERP TLS handshake (and thus sha256-raw cert pinning) + // worked on both ends. + if err := env.Ping(n1, n2, tailcfg.PingTSMP, 60*time.Second); err != nil { t.Fatalf("ping node-0 -> node-1: %v", err) } + // Also verify each node both sent and received data packets over DERP. + // With endpoints stripped from control, the TSMP ping above has no + // direct path available, so the WireGuard packets it generates must + // flow via DERP. These counters never decrease, so once they're + // non-zero we know DERP carried real frames in both directions. + for _, n := range []*vmtest.Node{n1, n2} { + if err := tstest.WaitFor(30*time.Second, func() error { + m := env.ClientMetrics(n) + sent := m["magicsock_send_data_derp"].Value + recv := m["magicsock_recv_data_derp"].Value + if sent == 0 || recv == 0 { + return fmt.Errorf("DERP data packets: sent=%d recv=%d; want both > 0", sent, recv) + } + t.Logf("[%s] DERP data packets: sent=%d recv=%d", n.Name(), sent, recv) + return nil + }); err != nil { + t.Errorf("[%s] %v", n.Name(), err) + } + } + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) defer cancel()