net/dns: always offer MagicDNS records at 100.100.100.100.

Fixes #1886.

Signed-off-by: David Anderson <danderson@tailscale.com>
(cherry picked from commit 6690f86ef4a664aca0fa4449b413b71b01348ad3)

ipn/ipnlocal/local.go

@@ -1683,7 +1683,42 @@ func (b *LocalBackend) authReconfig() {
 
 	var dcfg dns.Config
 
-	// If CorpDNS is false, dcfg remains the zero value.
+	// Populate MagicDNS records. We do this unconditionally so that
+	// quad-100 can always respond to MagicDNS queries, even if the OS
+	// isn't configured to make MagicDNS resolution truly
+	// magic. Details in
+	// https://github.com/tailscale/tailscale/issues/1886.
+	set := func(name string, addrs []netaddr.IPPrefix) {
+		if len(addrs) == 0 || name == "" {
+			return
+		}
+		fqdn, err := dnsname.ToFQDN(name)
+		if err != nil {
+			return // TODO: propagate error?
+		}
+		var ips []netaddr.IP
+		for _, addr := range addrs {
+			// Remove IPv6 addresses for now, as we don't
+			// guarantee that the peer node actually can speak
+			// IPv6 correctly.
+			//
+			// https://github.com/tailscale/tailscale/issues/1152
+			// tracks adding the right capability reporting to
+			// enable AAAA in MagicDNS.
+			if addr.IP.Is6() {
+				continue
+			}
+			ips = append(ips, addr.IP)
+		}
+		dcfg.Hosts[fqdn] = ips
+	}
+	dcfg.AuthoritativeSuffixes = magicDNSRootDomains(nm)
+	dcfg.Hosts = map[dnsname.FQDN][]netaddr.IP{}
+	set(nm.Name, nm.Addresses)
+	for _, peer := range nm.Peers {
+		set(peer.Name, peer.Addresses)
+	}
+
 	if uc.CorpDNS {
 		addDefault := func(resolvers []tailcfg.DNSResolver) {
 			for _, resolver := range resolvers {
@@ -1721,36 +1756,9 @@ func (b *LocalBackend) authReconfig() {
 			}
 			dcfg.SearchDomains = append(dcfg.SearchDomains, fqdn)
 		}
-		set := func(name string, addrs []netaddr.IPPrefix) {
-			if len(addrs) == 0 || name == "" {
-				return
-			}
-			fqdn, err := dnsname.ToFQDN(name)
-			if err != nil {
-				return // TODO: propagate error?
-			}
-			var ips []netaddr.IP
-			for _, addr := range addrs {
-				// Remove IPv6 addresses for now, as we don't
-				// guarantee that the peer node actually can speak
-				// IPv6 correctly.
-				//
-				// https://github.com/tailscale/tailscale/issues/1152
-				// tracks adding the right capability reporting to
-				// enable AAAA in MagicDNS.
-				if addr.IP.Is6() {
-					continue
-				}
-				ips = append(ips, addr.IP)
-			}
-			dcfg.Hosts[fqdn] = ips
-		}
 		if nm.DNS.Proxied { // actually means "enable MagicDNS"
-			dcfg.AuthoritativeSuffixes = magicDNSRootDomains(nm)
+			for _, dom := range dcfg.AuthoritativeSuffixes {
-			dcfg.Hosts = map[dnsname.FQDN][]netaddr.IP{}
+				dcfg.Routes[dom] = []netaddr.IPPort{netaddr.IPPort{IP: tsaddr.TailscaleServiceIP(), Port: 53}}
-			set(nm.Name, nm.Addresses)
-			for _, peer := range nm.Peers {
-				set(peer.Name, peer.Addresses)
 			}
 		}
 

net/dns/config.go

@@ -28,8 +28,11 @@ type Config struct {
 	SearchDomains []dnsname.FQDN
 	// Hosts maps DNS FQDNs to their IPs, which can be a mix of IPv4
 	// and IPv6.
-	// Queries matching entries in Hosts are resolved locally without
+	// Queries matching entries in Hosts are resolved locally by
-	// recursing off-machine.
+	// 100.100.100.100 without leaving the machine.
+	// Adding an entry to Hosts merely creates the record. If you want
+	// it to resolve, you also need to add appropriate routes to
+	// Routes.
 	Hosts map[dnsname.FQDN][]netaddr.IP
 	// AuthoritativeSuffixes is a list of fully-qualified DNS suffixes
 	// for which the in-process Tailscale resolver is authoritative.
@@ -42,7 +45,7 @@ type Config struct {
 // needsAnyResolvers reports whether c requires a resolver to be set
 // at the OS level.
 func (c Config) needsOSResolver() bool {
-	return c.hasDefaultResolvers() || c.hasRoutes() || c.hasHosts()
+	return c.hasDefaultResolvers() || c.hasRoutes()
 }
 
 func (c Config) hasRoutes() bool {
@@ -52,7 +55,7 @@ func (c Config) hasRoutes() bool {
 // hasDefaultResolversOnly reports whether the only resolvers in c are
 // DefaultResolvers.
 func (c Config) hasDefaultResolversOnly() bool {
-	return c.hasDefaultResolvers() && !c.hasRoutes() && !c.hasHosts()
+	return c.hasDefaultResolvers() && !c.hasRoutes()
 }
 
 func (c Config) hasDefaultResolvers() bool {
@@ -76,31 +79,11 @@ func (c Config) singleResolverSet() []netaddr.IPPort {
 	return first
 }
 
-// hasHosts reports whether c requires resolution of MagicDNS hosts or
+// matchDomains returns the list of match suffixes needed by Routes.
-// domains.
-func (c Config) hasHosts() bool {
-	return len(c.Hosts) > 0 || len(c.AuthoritativeSuffixes) > 0
-}
-
-// matchDomains returns the list of match suffixes needed by Routes,
-// AuthoritativeSuffixes. Hosts is not considered as we assume that
-// they're covered by AuthoritativeSuffixes for now.
 func (c Config) matchDomains() []dnsname.FQDN {
-	ret := make([]dnsname.FQDN, 0, len(c.Routes)+len(c.AuthoritativeSuffixes))
+	ret := make([]dnsname.FQDN, 0, len(c.Routes))
-	seen := map[dnsname.FQDN]bool{}
-	for _, suffix := range c.AuthoritativeSuffixes {
-		if seen[suffix] {
-			continue
-		}
-		ret = append(ret, suffix)
-		seen[suffix] = true
-	}
 	for suffix := range c.Routes {
-		if seen[suffix] {
-			continue
-		}
 		ret = append(ret, suffix)
-		seen[suffix] = true
 	}
 	sort.Slice(ret, func(i, j int) bool {
 		return ret[i].WithTrailingDot() < ret[j].WithTrailingDot()

net/dns/manager.go

@@ -6,7 +6,6 @@
 
 import (
 	"runtime"
-	"strings"
 	"time"
 
 	"inet.af/netaddr"
@@ -75,40 +74,51 @@ func (m *Manager) Set(cfg Config) error {
 
 // compileConfig converts cfg into a quad-100 resolver configuration
 // and an OS-level configuration.
-func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
+func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig, err error) {
+	authDomains := make(map[dnsname.FQDN]bool, len(cfg.AuthoritativeSuffixes))
+	for _, dom := range cfg.AuthoritativeSuffixes {
+		authDomains[dom] = true
+	}
+	addRoutes := func() {
+		for suffix, resolvers := range cfg.Routes {
+			// Don't add resolver routes for authoritative domains,
+			// since they're meant to be authoritatively handled
+			// internally.
+			if authDomains[suffix] {
+				continue
+			}
+			rcfg.Routes[suffix] = resolvers
+		}
+	}
+
+	// The internal resolver always gets MagicDNS hosts and
+	// authoritative suffixes, even if we don't propagate MagicDNS to
+	// the OS.
+	rcfg.Hosts = cfg.Hosts
+	rcfg.LocalDomains = cfg.AuthoritativeSuffixes
+	// Similarly, the OS always gets search paths.
+	ocfg.SearchDomains = cfg.SearchDomains
+
 	// Deal with trivial configs first.
 	switch {
 	case !cfg.needsOSResolver():
 		// Set search domains, but nothing else. This also covers the
 		// case where cfg is entirely zero, in which case these
 		// configs clear all Tailscale DNS settings.
-		return resolver.Config{}, OSConfig{
+		return rcfg, ocfg, nil
-			SearchDomains: cfg.SearchDomains,
-		}, nil
 	case cfg.hasDefaultResolversOnly():
 		// Trivial CorpDNS configuration, just override the OS
 		// resolver.
-		return resolver.Config{}, OSConfig{
+		ocfg.Nameservers = toIPsOnly(cfg.DefaultResolvers)
-			Nameservers:   toIPsOnly(cfg.DefaultResolvers),
+		return rcfg, ocfg, nil
-			SearchDomains: cfg.SearchDomains,
-		}, nil
 	case cfg.hasDefaultResolvers():
 		// Default resolvers plus other stuff always ends up proxying
 		// through quad-100.
-		rcfg := resolver.Config{
+		rcfg.Routes = map[dnsname.FQDN][]netaddr.IPPort{
-			Routes: map[dnsname.FQDN][]netaddr.IPPort{
+			".": cfg.DefaultResolvers,
-				".": cfg.DefaultResolvers,
-			},
-			Hosts:        cfg.Hosts,
-			LocalDomains: cfg.AuthoritativeSuffixes,
-		}
-		for suffix, resolvers := range cfg.Routes {
-			rcfg.Routes[suffix] = resolvers
-		}
-		ocfg := OSConfig{
-			Nameservers:   []netaddr.IP{tsaddr.TailscaleServiceIP()},
-			SearchDomains: cfg.SearchDomains,
 		}
+		addRoutes()
+		ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}
 		return rcfg, ocfg, nil
 	}
 
@@ -116,8 +126,6 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 	// configurations. The possible cases don't return directly any
 	// more, because as a final step we have to handle the case where
 	// the OS can't do split DNS.
-	var rcfg resolver.Config
-	var ocfg OSConfig
 
 	// Workaround for
 	// https://github.com/tailscale/corp/issues/1662. Even though
@@ -135,35 +143,20 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 	// This bool is used in a couple of places below to implement this
 	// workaround.
 	isWindows := runtime.GOOS == "windows"
-
+	if cfg.singleResolverSet() != nil && m.os.SupportsSplitDNS() && !isWindows {
-	// The windows check is for
-	// . See also below
-	// for further routing workarounds there.
-	if !cfg.hasHosts() && cfg.singleResolverSet() != nil && m.os.SupportsSplitDNS() && !isWindows {
 		// Split DNS configuration requested, where all split domains
 		// go to the same resolvers. We can let the OS do it.
-		return resolver.Config{}, OSConfig{
+		ocfg.Nameservers = toIPsOnly(cfg.singleResolverSet())
-			Nameservers:   toIPsOnly(cfg.singleResolverSet()),
+		ocfg.MatchDomains = cfg.matchDomains()
-			SearchDomains: cfg.SearchDomains,
+		return rcfg, ocfg, nil
-			MatchDomains:  cfg.matchDomains(),
-		}, nil
 	}
 
 	// Split DNS configuration with either multiple upstream routes,
 	// or routes + MagicDNS, or just MagicDNS, or on an OS that cannot
 	// split-DNS. Install a split config pointing at quad-100.
-	rcfg = resolver.Config{
+	rcfg.Routes = map[dnsname.FQDN][]netaddr.IPPort{}
-		Hosts:        cfg.Hosts,
+	addRoutes()
-		LocalDomains: cfg.AuthoritativeSuffixes,
+	ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}
-		Routes:       map[dnsname.FQDN][]netaddr.IPPort{},
-	}
-	for suffix, resolvers := range cfg.Routes {
-		rcfg.Routes[suffix] = resolvers
-	}
-	ocfg = OSConfig{
-		Nameservers:   []netaddr.IP{tsaddr.TailscaleServiceIP()},
-		SearchDomains: cfg.SearchDomains,
-	}
 
 	// If the OS can't do native split-dns, read out the underlying
 	// resolver config and blend it into our config.
@@ -173,28 +166,7 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 	if !m.os.SupportsSplitDNS() || isWindows {
 		bcfg, err := m.os.GetBaseConfig()
 		if err != nil {
-			// Temporary hack to make OSes where split-DNS isn't fully
+			return resolver.Config{}, OSConfig{}, err
-			// implemented yet not completely crap out, but instead
-			// fall back to quad-9 as a hardcoded "backup resolver".
-			//
-			// This codepath currently only triggers when opted into
-			// the split-DNS feature server side, and when at least
-			// one search domain is something within tailscale.com, so
-			// we don't accidentally leak unstable user DNS queries to
-			// quad-9 if we accidentally go down this codepath.
-			canUseHack := false
-			for _, dom := range cfg.SearchDomains {
-				if strings.HasSuffix(dom.WithoutTrailingDot(), ".tailscale.com") {
-					canUseHack = true
-					break
-				}
-			}
-			if !canUseHack {
-				return resolver.Config{}, OSConfig{}, err
-			}
-			bcfg = OSConfig{
-				Nameservers: []netaddr.IP{netaddr.IPv4(9, 9, 9, 9)},
-			}
 		}
 		rcfg.Routes["."] = toIPPorts(bcfg.Nameservers)
 		ocfg.SearchDomains = append(ocfg.SearchDomains, bcfg.SearchDomains...)

net/dns/manager_test.go

@@ -76,6 +76,22 @@ func TestManager(t *testing.T) {
 				SearchDomains: fqdns("tailscale.com", "universe.tf"),
 			},
 		},
+		{
+			// Regression test for https://github.com/tailscale/tailscale/issues/1886
+			name: "hosts-only",
+			in: Config{
+				Hosts: hosts(
+					"dave.ts.com.", "1.2.3.4",
+					"bradfitz.ts.com.", "2.3.4.5"),
+				AuthoritativeSuffixes: fqdns("ts.com"),
+			},
+			rs: resolver.Config{
+				Hosts: hosts(
+					"dave.ts.com.", "1.2.3.4",
+					"bradfitz.ts.com.", "2.3.4.5"),
+				LocalDomains: fqdns("ts.com"),
+			},
+		},
 		{
 			name: "corp",
 			in: Config{
@@ -104,6 +120,7 @@ func TestManager(t *testing.T) {
 			in: Config{
 				DefaultResolvers: mustIPPs("1.1.1.1:53", "9.9.9.9:53"),
 				SearchDomains:    fqdns("tailscale.com", "universe.tf"),
+				Routes:           upstreams("ts.com", "100.100.100.100:53"),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
@@ -126,6 +143,7 @@ func TestManager(t *testing.T) {
 			in: Config{
 				DefaultResolvers: mustIPPs("1.1.1.1:53", "9.9.9.9:53"),
 				SearchDomains:    fqdns("tailscale.com", "universe.tf"),
+				Routes:           upstreams("ts.com", "100.100.100.100:53"),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
@@ -261,6 +279,7 @@ func TestManager(t *testing.T) {
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
+				Routes:                upstreams("ts.com", "100.100.100.100:53"),
 				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains:         fqdns("tailscale.com", "universe.tf"),
 			},
@@ -286,6 +305,7 @@ func TestManager(t *testing.T) {
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
+				Routes:                upstreams("ts.com", "100.100.100.100:53"),
 				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains:         fqdns("tailscale.com", "universe.tf"),
 			},
@@ -333,7 +353,9 @@ func TestManager(t *testing.T) {
 		{
 			name: "routes-magic-split",
 			in: Config{
-				Routes: upstreams("corp.com", "2.2.2.2:53"),
+				Routes: upstreams(
+					"corp.com", "2.2.2.2:53",
+					"ts.com", "100.100.100.100:53"),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),