net/dns: replace AuthoritativeSuffixes with nil Route entries.

This leads to a cleaner separation of intent vs. implementation
(Routes is now the only place specifying who handles DNS requests),
and allows for cleaner expression of a configuration that creates
MagicDNS records without serving them to the OS.

Signed-off-by: David Anderson <danderson@tailscale.com>

ipn/ipnlocal/local.go

@@ -1725,7 +1725,6 @@ func (b *LocalBackend) authReconfig() {
 		}
 		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 {
@@ -1770,8 +1769,8 @@ func (b *LocalBackend) authReconfig() {
 			dcfg.SearchDomains = append(dcfg.SearchDomains, fqdn)
 		}
 		if nm.DNS.Proxied { // actually means "enable MagicDNS"
-			for _, dom := range dcfg.AuthoritativeSuffixes {
+			for _, dom := range magicDNSRootDomains(nm) {
-				dcfg.Routes[dom] = []netaddr.IPPort{netaddr.IPPortFrom(tsaddr.TailscaleServiceIP(), 53)}
+				dcfg.Routes[dom] = nil // resolve internally with dcfg.Hosts
 			}
 		}
 
@@ -1795,7 +1794,7 @@ func (b *LocalBackend) authReconfig() {
 			//
 			// https://github.com/tailscale/tailscale/issues/1713
 			addDefault(nm.DNS.FallbackResolvers)
-		case len(dcfg.Routes) == 0 && len(dcfg.Hosts) == 0 && len(dcfg.AuthoritativeSuffixes) == 0:
+		case len(dcfg.Routes) == 0:
 			// No settings requiring split DNS, no problem.
 		case version.OS() == "android":
 			// We don't support split DNS at all on Android yet.

net/dns/config.go

@@ -22,6 +22,8 @@ type Config struct {
 	// for queries that fall within that suffix.
 	// If a query doesn't match any entry in Routes, the
 	// DefaultResolvers are used.
+	// A Routes entry with no resolvers means the route should be
+	// authoritatively answered using the contents of Hosts.
 	Routes map[dnsname.FQDN][]netaddr.IPPort
 	// SearchDomains are DNS suffixes to try when expanding
 	// single-label queries.
@@ -34,12 +36,6 @@ type Config struct {
 	// 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.
-	// Queries for names within AuthoritativeSuffixes can only be
-	// fulfilled by entries in Hosts. Queries with no match in Hosts
-	// return NXDOMAIN.
-	AuthoritativeSuffixes []dnsname.FQDN
 }
 
 // needsAnyResolvers reports whether c requires a resolver to be set
@@ -66,17 +62,21 @@ func (c Config) hasDefaultResolvers() bool {
 // routes use the same resolvers, or nil if multiple sets of resolvers
 // are specified.
 func (c Config) singleResolverSet() []netaddr.IPPort {
-	var first []netaddr.IPPort
+	var (
+		prev            []netaddr.IPPort
+		prevInitialized bool
+	)
 	for _, resolvers := range c.Routes {
-		if first == nil {
+		if !prevInitialized {
-			first = resolvers
+			prev = resolvers
+			prevInitialized = true
 			continue
 		}
-		if !sameIPPorts(first, resolvers) {
+		if !sameIPPorts(prev, resolvers) {
 			return nil
 		}
 	}
-	return first
+	return prev
 }
 
 // matchDomains returns the list of match suffixes needed by Routes.

net/dns/manager.go

@@ -75,27 +75,18 @@ 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) (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
+	routes := map[dnsname.FQDN][]netaddr.IPPort{} // assigned conditionally to rcfg.Routes below.
+	for suffix, resolvers := range cfg.Routes {
+		if len(resolvers) == 0 {
+			rcfg.LocalDomains = append(rcfg.LocalDomains, suffix)
+		} else {
+			routes[suffix] = resolvers
+		}
+	}
 	// Similarly, the OS always gets search paths.
 	ocfg.SearchDomains = cfg.SearchDomains
 
@@ -114,10 +105,8 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig
 	case cfg.hasDefaultResolvers():
 		// Default resolvers plus other stuff always ends up proxying
 		// through quad-100.
-		rcfg.Routes = map[dnsname.FQDN][]netaddr.IPPort{
+		rcfg.Routes = routes
-			".": cfg.DefaultResolvers,
+		rcfg.Routes["."] = cfg.DefaultResolvers
-		}
-		addRoutes()
 		ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}
 		return rcfg, ocfg, nil
 	}
@@ -154,8 +143,7 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig
 	// 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.Routes = map[dnsname.FQDN][]netaddr.IPPort{}
+	rcfg.Routes = routes
-	addRoutes()
 	ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}
 
 	// If the OS can't do native split-dns, read out the underlying

net/dns/manager_test.go

@@ -83,13 +83,11 @@ func TestManager(t *testing.T) {
 				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"),
 			},
 		},
 		{
@@ -120,11 +118,10 @@ 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"),
+				Routes:           upstreams("ts.com", ""),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
-				AuthoritativeSuffixes: fqdns("ts.com"),
 			},
 			os: OSConfig{
 				Nameservers:   mustIPs("100.100.100.100"),
@@ -143,11 +140,10 @@ 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"),
+				Routes:           upstreams("ts.com", ""),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
-				AuthoritativeSuffixes: fqdns("ts.com"),
 			},
 			split: true,
 			os: OSConfig{
@@ -279,8 +275,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"),
+				Routes:        upstreams("ts.com", ""),
-				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains: fqdns("tailscale.com", "universe.tf"),
 			},
 			bs: OSConfig{
@@ -305,8 +300,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"),
+				Routes:        upstreams("ts.com", ""),
-				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains: fqdns("tailscale.com", "universe.tf"),
 			},
 			split: true,
@@ -325,11 +319,10 @@ func TestManager(t *testing.T) {
 		{
 			name: "routes-magic",
 			in: Config{
-				Routes: upstreams("corp.com", "2.2.2.2:53"),
+				Routes: upstreams("corp.com", "2.2.2.2:53", "ts.com", ""),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
-				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains: fqdns("tailscale.com", "universe.tf"),
 			},
 			bs: OSConfig{
@@ -355,11 +348,10 @@ func TestManager(t *testing.T) {
 			in: Config{
 				Routes: upstreams(
 					"corp.com", "2.2.2.2:53",
-					"ts.com", "100.100.100.100:53"),
+					"ts.com", ""),
 				Hosts: hosts(
 					"dave.ts.com.", "1.2.3.4",
 					"bradfitz.ts.com.", "2.3.4.5"),
-				AuthoritativeSuffixes: fqdns("ts.com"),
 				SearchDomains: fqdns("tailscale.com", "universe.tf"),
 			},
 			split: true,
@@ -451,7 +443,12 @@ func upstreams(strs ...string) (ret map[dnsname.FQDN][]netaddr.IPPort) {
 	var key dnsname.FQDN
 	ret = map[dnsname.FQDN][]netaddr.IPPort{}
 	for _, s := range strs {
-		if ipp, err := netaddr.ParseIPPort(s); err == nil {
+		if s == "" {
+			if key == "" {
+				panic("IPPort provided before suffix")
+			}
+			ret[key] = nil
+		} else if ipp, err := netaddr.ParseIPPort(s); err == nil {
 			if key == "" {
 				panic("IPPort provided before suffix")
 			}