wgengine/magicsock: bootstrap disco for a re-added peer with no path

UpsertPeer kicks disco when a peer has no trusted path; a DERP-only peer
also solicits callMeMaybe. No-ops for peers with an active trusted path.

wgengine/magicsock/endpoint.go

@@ -969,6 +969,48 @@ func (de *endpoint) noteTxActivityExtTriggerLocked(now mono.Time) {
 	}
 }
 
+// maybeKickDiscoBootstrap starts disco (pinging known endpoints and soliciting
+// callMeMaybe over DERP) for a peer that has no trusted path yet, without
+// waiting for outbound WireGuard traffic.
+//
+// Disco is normally armed only off an [endpoint.send], which calls
+// [endpoint.sendDiscoPingsLocked] and arms the heartbeat when there is no
+// trusted best path (endpoint.go: `!udpAddr.isDirect() ||
+// now.After(de.trustBestAddrUntil)`). A peer added or re-keyed via the delta
+// path ([Conn.UpsertPeer]) with no traffic flowing toward it never reaches that
+// send, so disco never starts: the node keeps answering pings but never
+// initiates, leaving it one-way dead until traffic happens to flow
+//
+//
+// It only fires for peers that genuinely need bootstrapping — disco-capable,
+// not expired, with something to probe (a DERP home or known UDP candidates)
+// and no currently-trusted path. A peer that already has a trusted path is
+// skipped, so this neither spams active sessions nor re-arms a working path.
+func (de *endpoint) maybeKickDiscoBootstrap() {
+	de.mu.Lock()
+	defer de.mu.Unlock()
+
+	if de.isWireguardOnly || de.expired {
+		return
+	}
+	if de.disco.Load() == nil {
+		return
+	}
+	if !de.derpAddr.IsValid() && len(de.endpointState) == 0 {
+		// Nothing to probe toward.
+		return
+	}
+	now := mono.Now()
+	if de.bestAddr.epAddr.ap.IsValid() && now.Before(de.trustBestAddrUntil) {
+		// Already have a trusted path; disco isn't needed.
+		return
+	}
+	// Arm the heartbeat so disco keeps re-driving if the first round doesn't
+	// establish a path, then ping known endpoints and solicit callMeMaybe now.
+	de.noteTxActivityExtTriggerLocked(now)
+	de.sendDiscoPingsLocked(now, true)
+}
+
 // MaxDiscoPingSize is the largest useful ping message size that we
 // can send - the maximum packet size minus the IPv4 and UDP headers.
 var MaxDiscoPingSize = tstun.MaxPacketSize - 20 - 8
@@ -1388,7 +1430,15 @@ func (de *endpoint) sendDiscoPingsLocked(now mono.Time, sendCallMeMaybe bool) {
 		de.startDiscoPingLocked(epAddr{ap: ep}, now, pingDiscovery, 0, nil)
 	}
 	derpAddr := de.derpAddr
-	if sentAny && sendCallMeMaybe && derpAddr.IsValid() {
+	// Send a CallMeMaybe over DERP when we either pinged a UDP candidate
+	// (sentAny) or have no UDP candidates at all but do know a DERP home. The
+	// latter is the bootstrap case for a peer whose endpoints aren't known yet
+	// (e.g. mid-STUN after a relogin): without soliciting the reverse
+	// direction it can never establish a path, leaving the node one-way dead
+	// This is narrower than the reverted #19878:
+	// when UDP candidates exist we still require sentAny, preserving #20088's
+	// anti-spam property.
+	if sendCallMeMaybe && derpAddr.IsValid() && (sentAny || len(de.endpointState) == 0) {
 		// Have our magicsock.Conn figure out its STUN endpoint (if
 		// it doesn't know already) and then send a CallMeMaybe
 		// message to our peer via DERP informing them that we've

wgengine/magicsock/endpoint_test.go

@@ -687,3 +687,103 @@ func Test_endpoint_handlePongConnLocked(t *testing.T) {
 		})
 	}
 }
+
+// TestDERPOnlyPeerSolicitsCallMeMaybe proves a peer reachable only via DERP
+// (no UDP endpoints known yet, e.g. mid-STUN after a relogin) still solicits
+// the reverse direction with callMeMaybe. Without this, such a peer can never
+// bootstrap a path — magicsock_disco_sent_callmemaybe stays frozen and the
+// node is one-way dead — and a control server can strand the client by
+// delivering a re-keyed peer endpoint-less.
+func TestDERPOnlyPeerSolicitsCallMeMaybe(t *testing.T) {
+	conn := newTestConn(t)
+	defer conn.Close()
+
+	de := &endpoint{
+		c:             conn,
+		publicKey:     key.NewNode().Public(),
+		sentPing:      make(map[stun.TxID]sentPing),
+		endpointState: make(map[netip.AddrPort]*endpointState), // no UDP candidates
+		debugUpdates:  ringlog.New[EndpointChange](10),
+	}
+	de.derpAddr = netip.AddrPortFrom(tailcfg.DerpMagicIPAddr, 1) // reachable only via DERP
+	de.updateDiscoKey(key.NewDisco().Public())
+
+	// enqueueCallMeMaybe records onEndpointRefreshed[de] only when the local
+	// endpoints are stale; otherwise it tries to send immediately over DERP,
+	// which the test harness can't do. The harness's startup STUN keeps
+	// refreshing lastEndpointsTime, so backdate it and re-drive the trigger in
+	// a loop: one iteration where the goroutine sees stale endpoints wins and
+	// records the callback, proving the callMeMaybe gate let the DERP-only peer
+	// through. stopPeriodicReSTUNTimerLocked quiets the periodic refresher.
+	conn.mu.Lock()
+	conn.stopPeriodicReSTUNTimerLocked()
+	conn.mu.Unlock()
+
+	var solicited bool
+	for range 200 {
+		conn.mu.Lock()
+		conn.lastEndpointsTime = time.Now().Add(-2 * endpointsFreshEnoughDuration)
+		conn.mu.Unlock()
+
+		de.mu.Lock()
+		de.lastFullPing = 0 // allow re-driving past discoPingInterval
+		de.sendDiscoPingsLocked(mono.Now(), true)
+		de.mu.Unlock()
+
+		conn.mu.Lock()
+		_, solicited = conn.onEndpointRefreshed[de]
+		conn.mu.Unlock()
+		if solicited {
+			break
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	if !solicited {
+		t.Fatal("DERP-only peer with no UDP endpoints did not solicit callMeMaybe")
+	}
+}
+
+// TestUpsertDERPOnlyPeerKicksDisco proves that adding a peer via the delta path
+// (UpsertPeer) with a DERP home but no known UDP endpoints initiates disco
+// solicitation without waiting for outbound WireGuard traffic. Previously disco
+// was only armed off a wireguard endpoint.send(); a re-keyed, endpoint-less peer
+// delivered via UpsertPeer therefore never solicited callMeMaybe and stayed
+// one-way dead until traffic happened to flow.
+func TestUpsertDERPOnlyPeerKicksDisco(t *testing.T) {
+	conn := newTestConn(t)
+	defer conn.Close()
+	conn.SetPrivateKey(key.NewNode())
+
+	nv := (&tailcfg.Node{
+		ID:        1,
+		Key:       key.NewNode().Public(),
+		DiscoKey:  key.NewDisco().Public(),
+		HomeDERP:  1,   // reachable via DERP
+		Endpoints: nil, // but no UDP candidates known yet
+	}).View()
+
+	// The real trigger: nothing else, no manual sendDiscoPingsLocked call.
+	conn.UpsertPeer(nv)
+
+	conn.mu.Lock()
+	de, ok := conn.peerMap.endpointForNodeID(nv.ID())
+	conn.mu.Unlock()
+	if !ok {
+		t.Fatal("UpsertPeer did not create an endpoint")
+	}
+
+	// sendDiscoPingsLocked sets lastFullPing and noteTxActivityExtTriggerLocked
+	// arms heartBeatTimer; both being set proves disco was kicked purely from
+	// the add, with no outbound WireGuard traffic. These reads are deterministic
+	// and independent of DERP/STUN being functional in the test harness.
+	de.mu.Lock()
+	pinged := !de.lastFullPing.IsZero()
+	armed := de.heartBeatTimer != nil
+	de.mu.Unlock()
+	if !pinged {
+		t.Fatal("UpsertPeer of a DERP-only peer did not kick sendDiscoPingsLocked (lastFullPing still zero)")
+	}
+	if !armed {
+		t.Fatal("UpsertPeer of a DERP-only peer did not arm the heartbeat")
+	}
+}

wgengine/magicsock/magicsock.go

@@ -3322,6 +3322,16 @@ func (c *Conn) UpsertPeer(n tailcfg.NodeView) {
 	mak.Set(&c.peersByID, n.ID(), n)
 	c.upsertPeerLocked(n, flags, debugRingBufferSize(len(c.peersByID)))
 
+	// A peer added or re-keyed via this delta path won't have disco armed if no
+	// traffic is flowing toward it yet (arming normally happens off an outbound
+	// endpoint.send). Kick disco here for a peer that lacks a trusted path so it
+	// starts initiating instead of staying one-way dead until traffic flows
+	// maybeKickDiscoBootstrap no-ops for peers that
+	// already have a trusted path.
+	if ep, ok := c.peerMap.endpointForNodeID(n.ID()); ok {
+		ep.maybeKickDiscoBootstrap()
+	}
+
 	var relayUpsert candidatePeerRelay
 	relayQualifies := false
 	if c.relayClientEnabled {