From f25ab855ba7e3cbf1ae0650bbcfbb24648abf49b Mon Sep 17 00:00:00 2001 From: Andrea Gottardo Date: Wed, 9 Oct 2024 10:05:54 -0700 Subject: [PATCH] ipnlocal: don't configure resolver with an invalid PeerAPI DNS URL Updates tailscale/corp#23782 Once the node key of the current exit node expires, we should stop attempting to forward DNS queries to it, because a connection cannot be established to an expired node. Here we change the logic in `exitNodeCanProxyDNS` so that it no longer attempts to use an invalid URL `/dns-query` as the DNS resolver if an IP is no longer available in the network map for the current exit node. Instead, we return false, and log that this issue occurred. This doesn't fix the problem, but it at least mitigates it by allowing DNS queries in the broken state to go to the local default resolver (e.g. 192.168.1.1) instead of endlessly getting forwarded to an DNS server URL that doesn't exist. Next steps include: - add a user-facing health warning when this verifies, instead of just a log line - understand why re-authenticating the exit node doesn't trigger a refresh of the DNS config. Signed-off-by: Andrea Gottardo --- ipn/ipnlocal/local.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 06dd84831..23961390c 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -4089,7 +4089,7 @@ func (b *LocalBackend) authReconfig() { hasPAC := b.prevIfState.HasPAC() disableSubnetsIfPAC := nm.HasCap(tailcfg.NodeAttrDisableSubnetsIfPAC) userDialUseRoutes := nm.HasCap(tailcfg.NodeAttrUserDialUseRoutes) - dohURL, dohURLOK := exitNodeCanProxyDNS(nm, b.peers, prefs.ExitNodeID()) + dohURL, dohURLOK := exitNodeCanProxyDNS(nm, b.peers, prefs.ExitNodeID(), b.logf) dcfg := dnsConfigForNetmap(nm, b.peers, prefs, b.keyExpired, b.logf, version.OS()) // If the current node is an app connector, ensure the app connector machine is started b.reconfigAppConnectorLocked(nm, prefs) @@ -4307,7 +4307,7 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. // If we're using an exit node and that exit node is new enough (1.19.x+) // to run a DoH DNS proxy, then send all our DNS traffic through it. - if dohURL, ok := exitNodeCanProxyDNS(nm, peers, prefs.ExitNodeID()); ok { + if dohURL, ok := exitNodeCanProxyDNS(nm, peers, prefs.ExitNodeID(), logf); ok { addDefault([]*dnstype.Resolver{{Addr: dohURL}}) return dcfg } @@ -6215,13 +6215,18 @@ func (b *LocalBackend) SetExpirySooner(ctx context.Context, expiry time.Time) er // to exitNodeID's DoH service, if available. // // If exitNodeID is the zero valid, it returns "", false. -func exitNodeCanProxyDNS(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg.NodeView, exitNodeID tailcfg.StableNodeID) (dohURL string, ok bool) { +func exitNodeCanProxyDNS(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg.NodeView, exitNodeID tailcfg.StableNodeID, logf logger.Logf) (dohURL string, ok bool) { if exitNodeID.IsZero() { return "", false } for _, p := range peers { if p.StableID() == exitNodeID && peerCanProxyDNS(p) { - return peerAPIBase(nm, p) + "/dns-query", true + ipPort := peerAPIBase(nm, p) + if ipPort == "" { + logf("exitNodeCanProxyDNS(%v) = false; no ipPort available", p.StableID()) + return "", false + } + return ipPort + "/dns-query", true } } return "", false