diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 821dedd00..c4ae286c0 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1699,7 +1699,41 @@ 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.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 { @@ -1737,36 +1771,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) - dcfg.Hosts = map[dnsname.FQDN][]netaddr.IP{} - set(nm.Name, nm.Addresses) - for _, peer := range nm.Peers { - set(peer.Name, peer.Addresses) + for _, dom := range magicDNSRootDomains(nm) { + dcfg.Routes[dom] = nil // resolve internally with dcfg.Hosts } } @@ -1790,7 +1797,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. diff --git a/net/dns/config.go b/net/dns/config.go index 0a6fb3481..dc0b73440 100644 --- a/net/dns/config.go +++ b/net/dns/config.go @@ -22,27 +22,26 @@ 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. 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 - // recursing off-machine. + // Queries matching entries in Hosts are resolved locally by + // 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. - // 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 // 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 +51,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 { @@ -63,44 +62,28 @@ 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 { - first = resolvers + if !prevInitialized { + prev = resolvers + prevInitialized = true continue } - if !sameIPPorts(first, resolvers) { + if !sameIPPorts(prev, resolvers) { return nil } } - return first + return prev } -// hasHosts reports whether c requires resolution of MagicDNS hosts or -// 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. +// matchDomains returns the list of match suffixes needed by Routes. func (c Config) matchDomains() []dnsname.FQDN { - ret := make([]dnsname.FQDN, 0, len(c.Routes)+len(c.AuthoritativeSuffixes)) - seen := map[dnsname.FQDN]bool{} - for _, suffix := range c.AuthoritativeSuffixes { - if seen[suffix] { - continue - } - ret = append(ret, suffix) - seen[suffix] = true - } + ret := make([]dnsname.FQDN, 0, len(c.Routes)) 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() diff --git a/net/dns/manager.go b/net/dns/manager.go index d7335c584..db34475c7 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -6,7 +6,6 @@ import ( "runtime" - "strings" "time" "inet.af/netaddr" @@ -75,40 +74,40 @@ 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) { + // The internal resolver always gets MagicDNS hosts and + // authoritative suffixes, even if we don't propagate MagicDNS to + // the OS. + rcfg.Hosts = cfg.Hosts + 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 + // 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{ - SearchDomains: cfg.SearchDomains, - }, nil + return rcfg, ocfg, nil case cfg.hasDefaultResolversOnly(): // Trivial CorpDNS configuration, just override the OS // resolver. - return resolver.Config{}, OSConfig{ - Nameservers: toIPsOnly(cfg.DefaultResolvers), - SearchDomains: cfg.SearchDomains, - }, nil + ocfg.Nameservers = toIPsOnly(cfg.DefaultResolvers) + return rcfg, ocfg, nil case cfg.hasDefaultResolvers(): // Default resolvers plus other stuff always ends up proxying // through quad-100. - rcfg := resolver.Config{ - Routes: map[dnsname.FQDN][]netaddr.IPPort{ - ".": 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, - } + rcfg.Routes = routes + rcfg.Routes["."] = cfg.DefaultResolvers + ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()} return rcfg, ocfg, nil } @@ -116,8 +115,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 +132,19 @@ 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" - - // The windows check is for - // . See also below - // for further routing workarounds there. - if !cfg.hasHosts() && cfg.singleResolverSet() != nil && m.os.SupportsSplitDNS() && !isWindows { + if 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{ - Nameservers: toIPsOnly(cfg.singleResolverSet()), - SearchDomains: cfg.SearchDomains, - MatchDomains: cfg.matchDomains(), - }, nil + ocfg.Nameservers = toIPsOnly(cfg.singleResolverSet()) + ocfg.MatchDomains = cfg.matchDomains() + return rcfg, ocfg, 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{ - Hosts: cfg.Hosts, - LocalDomains: cfg.AuthoritativeSuffixes, - 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, - } + rcfg.Routes = routes + ocfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()} // If the OS can't do native split-dns, read out the underlying // resolver config and blend it into our config. @@ -173,28 +154,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 - // 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)}, - } + return resolver.Config{}, OSConfig{}, err } rcfg.Routes["."] = toIPPorts(bcfg.Nameservers) ocfg.SearchDomains = append(ocfg.SearchDomains, bcfg.SearchDomains...) diff --git a/net/dns/manager_test.go b/net/dns/manager_test.go index 142c31691..8835f2aec 100644 --- a/net/dns/manager_test.go +++ b/net/dns/manager_test.go @@ -76,6 +76,20 @@ 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"), + }, + rs: resolver.Config{ + Hosts: hosts( + "dave.ts.com.", "1.2.3.4", + "bradfitz.ts.com.", "2.3.4.5"), + }, + }, { name: "corp", in: Config{ @@ -104,10 +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", ""), 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"), @@ -126,10 +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", ""), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), - AuthoritativeSuffixes: fqdns("ts.com"), }, split: true, os: OSConfig{ @@ -261,8 +275,8 @@ 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"), - SearchDomains: fqdns("tailscale.com", "universe.tf"), + Routes: upstreams("ts.com", ""), + SearchDomains: fqdns("tailscale.com", "universe.tf"), }, bs: OSConfig{ Nameservers: mustIPs("8.8.8.8"), @@ -286,8 +300,8 @@ 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"), - SearchDomains: fqdns("tailscale.com", "universe.tf"), + Routes: upstreams("ts.com", ""), + SearchDomains: fqdns("tailscale.com", "universe.tf"), }, split: true, os: OSConfig{ @@ -305,12 +319,11 @@ 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"), + SearchDomains: fqdns("tailscale.com", "universe.tf"), }, bs: OSConfig{ Nameservers: mustIPs("8.8.8.8"), @@ -333,12 +346,13 @@ 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", ""), 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"), + SearchDomains: fqdns("tailscale.com", "universe.tf"), }, split: true, os: OSConfig{ @@ -429,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") }