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 <jonathan@tailscale.com>
main
Jonathan Nobels 2 months ago committed by GitHub
parent e4008d1994
commit 086968c15b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 20
      ipn/ipnlocal/dnsconfig_test.go
  2. 5
      ipn/ipnlocal/node_backend.go
  3. 35
      ipn/ipnlocal/state_test.go
  4. 4
      net/dns/config.go
  5. 1
      net/dns/dns_clone.go
  6. 6
      net/dns/dns_view.go
  7. 1
      net/dns/manager.go
  8. 21
      net/dns/resolver/forwarder.go
  9. 5
      net/dns/resolver/tsdns.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"),

@ -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.

@ -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),
},
},
{

@ -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

@ -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

@ -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

@ -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 {

@ -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())
}
}

@ -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()

Loading…
Cancel
Save