diff --git a/core/dns.go b/core/dns.go index 52cfd2b..112a4bd 100644 --- a/core/dns.go +++ b/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, diff --git a/core/dns_test.go b/core/dns_test.go new file mode 100644 index 0000000..3f967d1 --- /dev/null +++ b/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) + } + } + }) + } +} diff --git a/core/utils.go b/core/utils.go index 8573124..ef2ea78 100644 --- a/core/utils.go +++ b/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]