fix: connectivity check fails on split-DNS destinations at startup

The diagnostics never got the split-DNS fix from 20097a0. That commit
  taught the dial path to resolve through the tailnet's own resolver
  (resolveDialAddr) but left getPeerFromRules on resolveAddr, so the two
  disagreed about how to look a destination up: connecting through a
  split-DNS name worked while the startup check called it unresolvable.

  Both paths now share resolveHostToIP, which honors MagicDNS, split-DNS
  routes and the DoH fallback.

  The check also resolved once at startup and cached the result for the
  process lifetime. tsnet reports Running before the netmap's DNS config
  reaches its resolver, and accept-routes is only applied after Up()
  returns, so a split-DNS name can fail for the first few seconds and
  resolve fine after. That transient failure dropped the peer permanently
  -- and when every rule failed, the goroutine returned and diagnostics
  never ran at all. Peers are re-resolved every round now, with a warm-up
  retry so the first report waits for DNS rather than racing it.

  Destinations outside the tailnet are no longer reported as failures.
  mc.lxns.net resolves fine but belongs to no peer, which is not an error,
  just not something to ping. errNotTailnetPeer separates "cannot resolve"
  from "resolved, not a peer"; only the former is retried or warned about.
  Unresolved rules now log their tag and dst, which the old message
  omitted entirely.

  Also fixed:

    * NormalizeDstAddrWithSuffix passed "host:port" to resolveAddr, so
      every existence check failed on the stray colon. Fixing that made
      the pass wait on cold-start DNS and delayed the listeners by ~5s,
      so it is now bounded by normalizeDNSBudget.

    * Peer lookup matched any AllowedIPs prefix containing the address.
      An exit node advertises 0.0.0.0/0, which contains everything, so a
      tailnet with an exit node picked the wrong peer at random depending
      on map iteration order. Default routes are skipped, the most
      specific route wins, ties break deterministically.

    * peer.AllowedIPs is a nillable pointer, dereferenced unguarded.

core/dns.go

@@ -46,51 +46,113 @@ func GetMagicDNSSuffixFromStatus(st *ipnstate.Status) (string, error) {
 	return suffix, nil
 }
 
-// addr(ip or domain) to tailscale ip
-// check the address is in the tailscale network
+// errNotTailnetPeer reports that a destination resolved successfully but the
+// resulting IP is not carried by any tailnet peer — an ordinary public address.
+// It is distinct from a resolution failure: retrying will not change the answer.
+var errNotTailnetPeer = errors.New("address is not reachable through a tailnet peer")
+
+// resolveAddr maps a destination host (an IP literal or a domain) to the
+// tailnet address of the peer that carries it, so the peer can be pinged for
+// connectivity diagnostics. Names are resolved through the same tailnet-aware
+// path the dial code uses (see resolveHostToIP), so MagicDNS and split-DNS
+// destinations behave identically in both.
 func resolveAddr(ctx context.Context, srv *tsnet.Server, addr string) (*netip.Addr, error) {
-	lc, err := srv.LocalClient()
+	ip, err := netip.ParseAddr(addr)
 	if err != nil {
-		return nil, err
+		ip, err = resolveHostToIP(ctx, srv, addr)
+		if err != nil {
+			return nil, err
+		}
 	}
-	stat, err := lc.Status(ctx)
+
+	stat, err := getCachedStatus(ctx, srv)
 	if err != nil {
 		return nil, err
 	}
 
-	if ip, err := netip.ParseAddr(addr); err == nil {
-		for _, peer := range stat.Peer {
-			for _, ipRange := range peer.AllowedIPs.All() {
-				if ipRange.Contains(ip) {
-					return &peer.TailscaleIPs[0], nil
-				}
+	peer, ok := peerCarryingIP(stat, ip)
+	if !ok {
+		return nil, fmt.Errorf("%w: %s (%s)", errNotTailnetPeer, addr, ip)
+	}
+	return &peer, nil
+}
+
+// peerCarryingIP returns the tailnet address of the peer that ip belongs to,
+// either because it is the peer's own address or because the peer advertises a
+// route covering it.
+func peerCarryingIP(stat *ipnstate.Status, ip netip.Addr) (netip.Addr, bool) {
+	for _, peer := range stat.Peer {
+		for _, peerIP := range peer.TailscaleIPs {
+			if peerIP == ip {
+				return peer.TailscaleIPs[0], true
 			}
 		}
+	}
+
+	// Otherwise the subnet router advertising the most specific route wins.
+	// Default routes are skipped: an exit node advertises 0.0.0.0/0, which
+	// contains every address and would otherwise shadow the real owner at
+	// random, since Go's map iteration order is unspecified. Ties are broken by
+	// the lowest tailnet address so repeated calls agree with each other.
+	bestBits := -1
+	var best netip.Addr
+	for _, peer := range stat.Peer {
+		if peer.AllowedIPs == nil || peer.AllowedIPs.IsNil() || len(peer.TailscaleIPs) == 0 {
+			continue
+		}
+		for _, route := range peer.AllowedIPs.All() {
+			if route.Bits() == 0 || !route.Contains(ip) {
+				continue
+			}
+			candidate := peer.TailscaleIPs[0]
+			if route.Bits() > bestBits || (route.Bits() == bestBits && candidate.Compare(best) < 0) {
+				bestBits, best = route.Bits(), candidate
+			}
+		}
+	}
+	return best, bestBits >= 0
+}
+
+// resolveHostToIP resolves a bare hostname to an address using the tailnet's
+// own resolver, falling back to DNS-over-HTTPS. Both the dial path and the
+// connectivity diagnostics go through here so they share one view of DNS.
+//
+// A bare single-label name additionally gets the MagicDNS suffix appended so
+// short tailnet hostnames still resolve; a name that already contains a dot (an
+// FQDN, including split-DNS suffixes) is queried as-is.
+func resolveHostToIP(ctx context.Context, srv *tsnet.Server, host string) (netip.Addr, error) {
+	candidates := []string{host}
+	if suffix, ok := GetMagicDNSSuffix(); ok && !strings.Contains(host, ".") {
+		candidates = append(candidates, host+"."+suffix)
+	}
+
+	var lastErr error
+	if dnsMgr, ok := srv.Sys().DNSManager.GetOK(); ok {
+		for _, name := range candidates {
+			ip, err := resolveHostViaResolver(ctx, dnsMgr, name)
+			if err != nil {
+				lastErr = err
+				continue
+			}
+			return ip, nil
+		}
 	} else {
-		suffix, ok := GetMagicDNSSuffix()
-		if ok {
-			if !strings.HasSuffix(addr, suffix) {
-				dnsMgr, ok := srv.Sys().DNSManager.GetOK()
-				if !ok {
-					return nil, errors.New("DNS manager not available")
-				}
-				ipaddr, err := resolveHostViaResolver(ctx, dnsMgr, addr)
-				if err != nil {
-					return nil, err
-				}
-				return resolveAddr(ctx, srv, ipaddr.String())
-			}
-		}
-		// addr is tailscale domain, resolve it
-		for _, peer := range stat.Peer {
-			dnsName := strings.TrimSuffix(peer.DNSName, ".")
-			if dnsName == addr {
-				return &peer.TailscaleIPs[0], nil
-			}
+		lastErr = errors.New("DNS manager not available")
+	}
+
+	// Fallback: resolve public names via DNS-over-HTTPS when the tailnet
+	// resolver couldn't (no working system DNS on the host, or a name outside
+	// the tailnet's split-DNS routes). Only the original host is queried — DoH
+	// can't resolve tailnet-internal MagicDNS names.
+	if dohEnabled() {
+		if ip, derr := resolveHostViaDoH(ctx, host); derr == nil {
+			return ip, nil
+		} else {
+			lastErr = fmt.Errorf("tailnet dns: %v; doh: %w", lastErr, derr)
 		}
 	}
 
-	return nil, errors.New(fmt.Sprintf("addr '%s' not found in tsnet", addr))
+	return netip.Addr{}, fmt.Errorf("resolve %q: %w", host, lastErr)
 }
 
 // resolveDialAddr resolves the host portion of a "host:port" destination to a
@@ -117,42 +179,11 @@ func resolveDialAddr(ctx context.Context, srv *tsnet.Server, addr string) (strin
 		return addr, nil // already ip:port, nothing to resolve
 	}
 
-	// Names to try, in order. A bare single-label name additionally gets the
-	// MagicDNS suffix appended so short tailnet hostnames still resolve; a name
-	// that already contains a dot (an FQDN, including split-DNS suffixes) is
-	// queried as-is.
-	candidates := []string{host}
-	if suffix, ok := GetMagicDNSSuffix(); ok && !strings.Contains(host, ".") {
-		candidates = append(candidates, host+"."+suffix)
+	ip, err := resolveHostToIP(ctx, srv, host)
+	if err != nil {
+		return addr, err
 	}
-
-	var lastErr error
-	if dnsMgr, ok := srv.Sys().DNSManager.GetOK(); ok {
-		for _, name := range candidates {
-			ip, err := resolveHostViaResolver(ctx, dnsMgr, name)
-			if err != nil {
-				lastErr = err
-				continue
-			}
-			return net.JoinHostPort(ip.String(), port), nil
-		}
-	} else {
-		lastErr = errors.New("DNS manager not available")
-	}
-
-	// Fallback: resolve public names via DNS-over-HTTPS when the tailnet
-	// resolver couldn't (no working system DNS on the host, or a name outside
-	// the tailnet's split-DNS routes). Only the original host is queried — DoH
-	// can't resolve tailnet-internal MagicDNS names.
-	if dohEnabled() {
-		if ip, derr := resolveHostViaDoH(ctx, host); derr == nil {
-			return net.JoinHostPort(ip.String(), port), nil
-		} else {
-			lastErr = fmt.Errorf("tailnet dns: %v; doh: %w", lastErr, derr)
-		}
-	}
-
-	return addr, fmt.Errorf("resolve %q: %w", host, lastErr)
+	return net.JoinHostPort(ip.String(), port), nil
 }
 
 // dnsExchange sends a single DNS question and returns the first address answer,

core/dns_test.go

@@ -0,0 +1,135 @@
+package core
+
+import (
+	"net/netip"
+	"testing"
+
+	"tailscale.com/ipn/ipnstate"
+	"tailscale.com/types/key"
+	"tailscale.com/types/views"
+)
+
+// peerStatus builds a PeerStatus with the given tailnet address and advertised
+// routes. Passing no routes leaves AllowedIPs nil, as it is for peers that
+// advertise nothing.
+func peerStatus(tailIP string, routes ...string) *ipnstate.PeerStatus {
+	ps := &ipnstate.PeerStatus{
+		TailscaleIPs: []netip.Addr{netip.MustParseAddr(tailIP)},
+	}
+	if len(routes) > 0 {
+		prefixes := make([]netip.Prefix, 0, len(routes))
+		for _, r := range routes {
+			prefixes = append(prefixes, netip.MustParsePrefix(r))
+		}
+		s := views.SliceOf(prefixes)
+		ps.AllowedIPs = &s
+	}
+	return ps
+}
+
+func statusWithPeers(peers ...*ipnstate.PeerStatus) *ipnstate.Status {
+	st := &ipnstate.Status{Peer: make(map[key.NodePublic]*ipnstate.PeerStatus, len(peers))}
+	for _, p := range peers {
+		st.Peer[key.NewNode().Public()] = p
+	}
+	return st
+}
+
+func TestPeerCarryingIP(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		peers []*ipnstate.PeerStatus
+		ip    string
+		want  string // "" means no peer expected
+	}{
+		{
+			name:  "peer's own address",
+			peers: []*ipnstate.PeerStatus{peerStatus("100.64.0.1", "100.64.0.1/32")},
+			ip:    "100.64.0.1",
+			want:  "100.64.0.1",
+		},
+		{
+			name: "subnet router carries a LAN address",
+			peers: []*ipnstate.PeerStatus{
+				peerStatus("100.64.0.2", "100.64.0.2/32", "10.0.0.0/24"),
+				peerStatus("100.64.0.3", "100.64.0.3/32"),
+			},
+			ip:   "10.0.0.7",
+			want: "100.64.0.2",
+		},
+		{
+			// An exit node advertises 0.0.0.0/0, which Contains every address.
+			// Matching it would pick a peer at random out of map iteration order.
+			name: "exit node does not shadow the real subnet router",
+			peers: []*ipnstate.PeerStatus{
+				peerStatus("100.64.0.9", "0.0.0.0/0", "::/0"),
+				peerStatus("100.64.0.2", "10.0.0.0/24"),
+			},
+			ip:   "10.0.0.7",
+			want: "100.64.0.2",
+		},
+		{
+			name: "most specific route wins",
+			peers: []*ipnstate.PeerStatus{
+				peerStatus("100.64.0.4", "10.0.0.0/8"),
+				peerStatus("100.64.0.5", "10.0.0.0/24"),
+			},
+			ip:   "10.0.0.7",
+			want: "100.64.0.5",
+		},
+		{
+			name: "equal routes break the tie deterministically",
+			peers: []*ipnstate.PeerStatus{
+				peerStatus("100.64.0.8", "10.0.0.0/24"),
+				peerStatus("100.64.0.6", "10.0.0.0/24"),
+			},
+			ip:   "10.0.0.7",
+			want: "100.64.0.6",
+		},
+		{
+			name:  "public address belongs to no peer",
+			peers: []*ipnstate.PeerStatus{peerStatus("100.64.0.1", "10.0.0.0/24")},
+			ip:    "1.1.1.1",
+			want:  "",
+		},
+		{
+			name:  "peer without AllowedIPs is skipped, not dereferenced",
+			peers: []*ipnstate.PeerStatus{peerStatus("100.64.0.1")},
+			ip:    "10.0.0.7",
+			want:  "",
+		},
+		{
+			name:  "exit node alone still does not match",
+			peers: []*ipnstate.PeerStatus{peerStatus("100.64.0.9", "0.0.0.0/0")},
+			ip:    "1.1.1.1",
+			want:  "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			st := statusWithPeers(tt.peers...)
+			// Run repeatedly: map iteration order is unspecified, so a result
+			// that depends on it shows up as a flake here.
+			for range 20 {
+				got, ok := peerCarryingIP(st, netip.MustParseAddr(tt.ip))
+				if tt.want == "" {
+					if ok {
+						t.Fatalf("peerCarryingIP() = %v, true; want no match", got)
+					}
+					continue
+				}
+				if !ok {
+					t.Fatalf("peerCarryingIP() = _, false; want %s", tt.want)
+				}
+				if got.String() != tt.want {
+					t.Fatalf("peerCarryingIP() = %s, want %s", got, tt.want)
+				}
+			}
+		})
+	}
+}

core/utils.go

@@ -45,8 +45,16 @@ func StartTimeWatchDog(ctx context.Context, logger *slog.Logger) <-chan struct{}
 	return ch
 }
 
-func getPeerFromRules(ctx context.Context, srv *tsnet.Server, rules map[string][]ConnectRule, logger *slog.Logger) ([]netip.Addr, error) {
+// getPeerFromRules maps every connect rule's destination onto the tailnet peer
+// that carries it. Alongside the peers it reports how many rules could not be
+// resolved at all; those are retryable, unlike destinations that resolve to an
+// address outside the tailnet (an ordinary public host), which are skipped for
+// good. warn selects whether unresolved rules are logged as warnings — during
+// startup the tailnet resolver may not have its split-DNS routes yet, so the
+// first few rounds stay quiet.
+func getPeerFromRules(ctx context.Context, srv *tsnet.Server, rules map[string][]ConnectRule, logger *slog.Logger, warn bool) ([]netip.Addr, int) {
 	peerSet := make(map[netip.Addr]struct{})
+	unresolved := 0
 
 	for tag, rrs := range rules {
 		for _, rule := range rrs {
@@ -61,7 +69,18 @@ func getPeerFromRules(ctx context.Context, srv *tsnet.Server, rules map[string][
 			addr, err := resolveAddr(ctx, srv, ap)
 
 			if err != nil {
-				logger.Warn("failed to resolve address", "err", err)
+				if errors.Is(err, errNotTailnetPeer) {
+					logger.Debug("destination is outside the tailnet, skipping diagnostics",
+						"tag", tag, "dst", rule.DstAddr, "err", err)
+					continue
+				}
+				unresolved++
+				if warn {
+					logger.Warn("failed to resolve address", "tag", tag, "dst", rule.DstAddr, "err", err)
+				} else {
+					logger.Debug("failed to resolve address (tailnet DNS may still be settling)",
+						"tag", tag, "dst", rule.DstAddr, "err", err)
+				}
 				continue
 			}
 			logger.Debug("address found", "dst_addr", rule.DstAddr, "tag", tag, "address", addr)
@@ -73,7 +92,7 @@ func getPeerFromRules(ctx context.Context, srv *tsnet.Server, rules map[string][
 	for peer := range peerSet {
 		result = append(result, peer)
 	}
-	return result, nil
+	return result, unresolved
 }
 
 func peerConnectivityLogic(ctx context.Context, lc *local.Client, relativePeers []netip.Addr, logger *slog.Logger) {
@@ -117,16 +136,19 @@ func peerConnectivityLogic(ctx context.Context, lc *local.Client, relativePeers
 	}
 }
 
-func StartPeerConnectivityDiagnostics(ctx context.Context, logger *slog.Logger, srv *tsnet.Server, rules map[string][]ConnectRule) {
-	relativePeers, err := getPeerFromRules(ctx, srv, rules, logger)
-	if err != nil {
-		return
-	}
-	logger.Debug("Peers loaded", "count", len(relativePeers))
+const (
+	// peerDiagInterval is how often connectivity to each peer is re-checked.
+	peerDiagInterval = 120 * time.Second
+	// A tsnet server reports Running before the netmap's DNS configuration has
+	// been programmed into its resolver, and accept-routes is only applied once
+	// the server is up — so at startup a split-DNS destination can briefly fail
+	// to resolve even though it resolves fine moments later. Retry a handful of
+	// times before reporting anything as broken.
+	peerDiagWarmupTries = 6
+	peerDiagWarmupDelay = 2 * time.Second
+)
 
-	if len(relativePeers) == 0 {
-		return
-	}
+func StartPeerConnectivityDiagnostics(ctx context.Context, logger *slog.Logger, srv *tsnet.Server, rules map[string][]ConnectRule) {
 	go func() {
 		lc, err := srv.LocalClient()
 		if err != nil {
@@ -134,18 +156,42 @@ func StartPeerConnectivityDiagnostics(ctx context.Context, logger *slog.Logger,
 			return
 		}
 
-		ticker := time.NewTicker(120 * time.Second)
+		// Warm-up: keep retrying while destinations are still unresolvable, and
+		// only escalate to a warning on the final attempt.
+		var peers []netip.Addr
+		for try := 1; ; try++ {
+			last := try >= peerDiagWarmupTries
+			var unresolved int
+			peers, unresolved = getPeerFromRules(ctx, srv, rules, logger, last)
+			if unresolved == 0 || last {
+				break
+			}
+			logger.Debug("waiting for tailnet DNS before diagnosing peers",
+				"unresolved", unresolved, "attempt", try)
+			select {
+			case <-ctx.Done():
+				return
+			case <-time.After(peerDiagWarmupDelay):
+			}
+		}
+		logger.Debug("Peers loaded", "count", len(peers))
+
+		ticker := time.NewTicker(peerDiagInterval)
 		defer ticker.Stop()
 
-		peerConnectivityLogic(ctx, lc, relativePeers, logger) // execute now
-
 		for {
+			peerConnectivityLogic(ctx, lc, peers, logger)
+
 			select {
 			case <-ctx.Done():
 				return
 			case <-ticker.C:
-				peerConnectivityLogic(ctx, lc, relativePeers, logger)
 			}
+
+			// Re-resolve every round: destinations that failed at startup
+			// recover on their own, and split-DNS records may point elsewhere
+			// than they did two minutes ago.
+			peers, _ = getPeerFromRules(ctx, srv, rules, logger, true)
 		}
 	}()
 }
@@ -174,11 +220,13 @@ func NormalizeDstAddrWithSuffix(ctx context.Context, srv *tsnet.Server, dst stri
 		return dst, false, nil
 	}
 
-	normalized := net.JoinHostPort(host+"."+suffix, port)
+	qualified := host + "." + suffix
+	normalized := net.JoinHostPort(qualified, port)
 
-	// check domain exists before use
+	// check domain exists before use. resolveAddr takes a bare host — passing
+	// the "host:port" form made every lookup here fail on the stray colon.
 	if strings.Contains(host, ".") {
-		_, err = resolveAddr(ctx, srv, normalized)
+		_, err = resolveAddr(ctx, srv, qualified)
 		if err != nil {
 			return dst, false, nil
 		}
@@ -187,7 +235,18 @@ func NormalizeDstAddrWithSuffix(ctx context.Context, srv *tsnet.Server, dst stri
 	return normalized, true, nil
 }
 
+// normalizeDNSBudget caps how long the whole normalization pass may spend
+// waiting on DNS. It runs before the connectors start listening, and on a cold
+// start the tailnet resolver needs a few seconds before it answers — without a
+// bound the listeners would not come up until then. A name that cannot be
+// checked in time simply keeps its configured form, which is the same
+// conclusion the check reaches for anything that is not a MagicDNS name.
+const normalizeDNSBudget = 2 * time.Second
+
 func NormalizeConnectRulesDstAddr(ctx context.Context, srv *tsnet.Server, rules map[string][]ConnectRule, logger *slog.Logger) {
+	ctx, cancel := context.WithTimeout(ctx, normalizeDNSBudget)
+	defer cancel()
+
 	for tag, rrs := range rules {
 		for i := range rrs {
 			rule := &rrs[i]