From 086968c15b5b000f3533ab981ec0201678ca78f3 Mon Sep 17 00:00:00 2001 From: Jonathan Nobels Date: Tue, 10 Feb 2026 09:29:14 -0500 Subject: [PATCH] net/dns, ipn/local: skip health warnings in dns forwarder when accept-dns is false (#18572) fixes tailscale/tailscale#18436 Queries can still make their way to the forwarder when accept-dns is disabled. Since we have not configured the forwarder if --accept-dns is false, this errors out (correctly) but it also generates a persistent health warning. This forwards the Pref setting all the way through the stack to the forwarder so that we can be more judicious about when we decide that the forward path is unintentionally missing, vs simply not configured. Testing: tailscale set --accept-dns=false. (or from the GUI) dig @100.100.100.100 example.com tailscale status No dns related health warnings should be surfaced. Signed-off-by: Jonathan Nobels --- ipn/ipnlocal/dnsconfig_test.go | 20 ++++++++++++------- ipn/ipnlocal/node_backend.go | 5 +++-- ipn/ipnlocal/state_test.go | 35 ++++++++++++++++++++-------------- net/dns/config.go | 4 ++++ net/dns/dns_clone.go | 1 + net/dns/dns_view.go | 6 ++++++ net/dns/manager.go | 1 + net/dns/resolver/forwarder.go | 21 ++++++++++++++++---- net/dns/resolver/tsdns.go | 5 ++++- 9 files changed, 70 insertions(+), 28 deletions(-) diff --git a/ipn/ipnlocal/dnsconfig_test.go b/ipn/ipnlocal/dnsconfig_test.go index ab00b4740..9d30029ff 100644 --- a/ipn/ipnlocal/dnsconfig_test.go +++ b/ipn/ipnlocal/dnsconfig_test.go @@ -219,7 +219,8 @@ func TestDNSConfigForNetmap(t *testing.T) { CorpDNS: true, }, want: &dns.Config{ - Hosts: map[dnsname.FQDN][]netip.Addr{}, + AcceptDNS: true, + Hosts: map[dnsname.FQDN][]netip.Addr{}, Routes: map[dnsname.FQDN][]*dnstype.Resolver{ "0.e.1.a.c.5.1.1.a.7.d.f.ip6.arpa.": nil, "100.100.in-addr.arpa.": nil, @@ -319,7 +320,8 @@ func TestDNSConfigForNetmap(t *testing.T) { CorpDNS: true, }, want: &dns.Config{ - Hosts: map[dnsname.FQDN][]netip.Addr{}, + AcceptDNS: true, + Hosts: map[dnsname.FQDN][]netip.Addr{}, DefaultResolvers: []*dnstype.Resolver{ {Addr: "8.8.8.8"}, }, @@ -342,8 +344,9 @@ func TestDNSConfigForNetmap(t *testing.T) { ExitNodeID: "some-id", }, want: &dns.Config{ - Hosts: map[dnsname.FQDN][]netip.Addr{}, - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + AcceptDNS: true, + Hosts: map[dnsname.FQDN][]netip.Addr{}, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, DefaultResolvers: []*dnstype.Resolver{ {Addr: "8.8.4.4"}, }, @@ -362,8 +365,9 @@ func TestDNSConfigForNetmap(t *testing.T) { CorpDNS: true, }, want: &dns.Config{ - Hosts: map[dnsname.FQDN][]netip.Addr{}, - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + AcceptDNS: true, + Hosts: map[dnsname.FQDN][]netip.Addr{}, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, }, }, { @@ -420,6 +424,7 @@ func TestDNSConfigForNetmap(t *testing.T) { CorpDNS: true, }, want: &dns.Config{ + AcceptDNS: true, Hosts: map[dnsname.FQDN][]netip.Addr{ "a.": ips("100.101.101.101"), "p1.": ips("100.102.0.1"), @@ -466,7 +471,8 @@ func TestDNSConfigForNetmap(t *testing.T) { CorpDNS: true, }, want: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, Hosts: map[dnsname.FQDN][]netip.Addr{ "a.": ips("100.101.101.101"), "p1.": ips("100.102.0.1"), diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index 170dae956..929ef34a4 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -696,8 +696,9 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg. } dcfg := &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: map[dnsname.FQDN][]netip.Addr{}, + AcceptDNS: prefs.CorpDNS(), + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: map[dnsname.FQDN][]netip.Addr{}, } // selfV6Only is whether we only have IPv6 addresses ourselves. diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 97c2c4d8f..ed6ad06ef 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -1300,8 +1300,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { Routes: routesWithQuad100(), }, wantDNSCfg: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node1), + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node1), }, }, { @@ -1356,8 +1357,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { Routes: routesWithQuad100(), }, wantDNSCfg: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node2), + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node2), }, }, { @@ -1404,8 +1406,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { Routes: routesWithQuad100(), }, wantDNSCfg: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node1), + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node1), }, }, { @@ -1436,8 +1439,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { Routes: routesWithQuad100(), }, wantDNSCfg: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node3), + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node3), }, }, { @@ -1500,8 +1504,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { Routes: routesWithQuad100(), }, wantDNSCfg: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node1), + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node1), }, }, { @@ -1529,8 +1534,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { Routes: routesWithQuad100(), }, wantDNSCfg: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node1), + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node1), }, }, { @@ -1560,8 +1566,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { Routes: routesWithQuad100(), }, wantDNSCfg: &dns.Config{ - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node1), + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node1), }, }, { diff --git a/net/dns/config.go b/net/dns/config.go index f776d1af0..47fac83c2 100644 --- a/net/dns/config.go +++ b/net/dns/config.go @@ -26,6 +26,10 @@ import ( // Config is a DNS configuration. type Config struct { + // AcceptDNS true if [Prefs.CorpDNS] is enabled (or --accept-dns=true). + // This should be used for error handling and health reporting + // purposes only. + AcceptDNS bool // DefaultResolvers are the DNS resolvers to use for DNS names // which aren't covered by more specific per-domain routes below. // If empty, the OS's default resolvers (the ones that predate diff --git a/net/dns/dns_clone.go b/net/dns/dns_clone.go index ea5e5299b..291f96ec2 100644 --- a/net/dns/dns_clone.go +++ b/net/dns/dns_clone.go @@ -51,6 +51,7 @@ func (src *Config) Clone() *Config { // A compilation failure here means this code must be regenerated, with the command at the top of this file. var _ConfigCloneNeedsRegeneration = Config(struct { + AcceptDNS bool DefaultResolvers []*dnstype.Resolver Routes map[dnsname.FQDN][]*dnstype.Resolver SearchDomains []dnsname.FQDN diff --git a/net/dns/dns_view.go b/net/dns/dns_view.go index 313621c86..70cb89dca 100644 --- a/net/dns/dns_view.go +++ b/net/dns/dns_view.go @@ -87,6 +87,11 @@ func (v *ConfigView) UnmarshalJSONFrom(dec *jsontext.Decoder) error { return nil } +// AcceptDNS true if [Prefs.CorpDNS] is enabled (or --accept-dns=true). +// This should be used for error handling and health reporting +// purposes only. +func (v ConfigView) AcceptDNS() bool { return v.ж.AcceptDNS } + // DefaultResolvers are the DNS resolvers to use for DNS names // which aren't covered by more specific per-domain routes below. // If empty, the OS's default resolvers (the ones that predate @@ -139,6 +144,7 @@ func (v ConfigView) Equal(v2 ConfigView) bool { return v.ж.Equal(v2.ж) } // A compilation failure here means this code must be regenerated, with the command at the top of this file. var _ConfigViewNeedsRegeneration = Config(struct { + AcceptDNS bool DefaultResolvers []*dnstype.Resolver Routes map[dnsname.FQDN][]*dnstype.Resolver SearchDomains []dnsname.FQDN diff --git a/net/dns/manager.go b/net/dns/manager.go index faca1053c..c05205565 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -292,6 +292,7 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig // the OS. rcfg.Hosts = cfg.Hosts rcfg.SubdomainHosts = cfg.SubdomainHosts + rcfg.AcceptDNS = cfg.AcceptDNS routes := map[dnsname.FQDN][]*dnstype.Resolver{} // assigned conditionally to rcfg.Routes below. var propagateHostsToOS bool for suffix, resolvers := range cfg.Routes { diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index 189911ee2..6fec32d6a 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -323,6 +323,12 @@ type forwarder struct { // /etc/resolv.conf is missing/corrupt, and the peerapi ExitDNS stub // resolver lookup. cloudHostFallback []resolverAndDelay + + // acceptDNS tracks the CorpDNS pref (--accept-dns) + // This lets us skip health warnings if the forwarder receives inbound + // queries directly - but we didn't configure it with any upstream resolvers. + // That's an error, but not a health error if the user has disabled CorpDNS. + acceptDNS bool } func newForwarder(logf logger.Logf, netMon *netmon.Monitor, linkSel ForwardLinkSelector, dialer *tsdial.Dialer, health *health.Tracker, knobs *controlknobs.Knobs) *forwarder { @@ -434,7 +440,7 @@ func cloudResolvers() []resolverAndDelay { // Resolver.SetConfig on reconfig. // // The memory referenced by routesBySuffix should not be modified. -func (f *forwarder) setRoutes(routesBySuffix map[dnsname.FQDN][]*dnstype.Resolver) { +func (f *forwarder) setRoutes(routesBySuffix map[dnsname.FQDN][]*dnstype.Resolver, acceptDNS bool) { routes := make([]route, 0, len(routesBySuffix)) cloudHostFallback := cloudResolvers() @@ -468,6 +474,7 @@ func (f *forwarder) setRoutes(routesBySuffix map[dnsname.FQDN][]*dnstype.Resolve f.mu.Lock() defer f.mu.Unlock() + f.acceptDNS = acceptDNS f.routes = routes f.cloudHostFallback = cloudHostFallback } @@ -1056,7 +1063,9 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo resolvers = f.resolvers(domain) if len(resolvers) == 0 { metricDNSFwdErrorNoUpstream.Add(1) - f.health.SetUnhealthy(dnsForwarderFailing, health.Args{health.ArgDNSServers: ""}) + if f.acceptDNS { + f.health.SetUnhealthy(dnsForwarderFailing, health.Args{health.ArgDNSServers: ""}) + } f.logf("no upstream resolvers set, returning SERVFAIL") res, err := servfailResponse(query) @@ -1156,7 +1165,9 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo for _, rr := range resolvers { resolverAddrs = append(resolverAddrs, rr.name.Addr) } - f.health.SetUnhealthy(dnsForwarderFailing, health.Args{health.ArgDNSServers: strings.Join(resolverAddrs, ",")}) + if f.acceptDNS { + f.health.SetUnhealthy(dnsForwarderFailing, health.Args{health.ArgDNSServers: strings.Join(resolverAddrs, ",")}) + } case responseChan <- res: if f.verboseFwd { f.logf("forwarder response(%d, %v, %d) = %d, %v", fq.txid, typ, len(domain), len(res.bs), firstErr) @@ -1181,7 +1192,9 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo for _, rr := range resolvers { resolverAddrs = append(resolverAddrs, rr.name.Addr) } - f.health.SetUnhealthy(dnsForwarderFailing, health.Args{health.ArgDNSServers: strings.Join(resolverAddrs, ",")}) + if f.acceptDNS { + f.health.SetUnhealthy(dnsForwarderFailing, health.Args{health.ArgDNSServers: strings.Join(resolverAddrs, ",")}) + } return fmt.Errorf("waiting for response or error from %v: %w", resolverAddrs, ctx.Err()) } } diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index 5b44f6c2d..d0601de7b 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -70,6 +70,9 @@ type packet struct { // Else forward the query to the most specific matching entry in Routes. // Else return SERVFAIL. type Config struct { + // True if [Prefs.CorpDNS] is true or --accept-dns=true was specified. + // This should only be used for error handling and health reporting. + AcceptDNS bool // Routes is a map of DNS name suffix to the resolvers to use for // queries within that suffix. // Queries only match the most specific suffix. @@ -279,7 +282,7 @@ func (r *Resolver) SetConfig(cfg Config) error { } } - r.forwarder.setRoutes(cfg.Routes) + r.forwarder.setRoutes(cfg.Routes, cfg.AcceptDNS) r.mu.Lock() defer r.mu.Unlock()