diff --git a/ipn/ipnlocal/dnsconfig_test.go b/ipn/ipnlocal/dnsconfig_test.go index 217b5457d..a27d1824b 100644 --- a/ipn/ipnlocal/dnsconfig_test.go +++ b/ipn/ipnlocal/dnsconfig_test.go @@ -259,10 +259,10 @@ func TestDNSConfigForNetmap(t *testing.T) { want: &dns.Config{ Hosts: map[dnsname.FQDN][]netaddr.IP{}, DefaultResolvers: []dnstype.Resolver{ - {Addr: "8.8.8.8:53"}, + {Addr: "8.8.8.8"}, }, Routes: map[dnsname.FQDN][]dnstype.Resolver{ - "foo.com.": {{Addr: "1.2.3.4:53"}}, + "foo.com.": {{Addr: "1.2.3.4"}}, }, }, }, @@ -283,7 +283,7 @@ func TestDNSConfigForNetmap(t *testing.T) { Hosts: map[dnsname.FQDN][]netaddr.IP{}, Routes: map[dnsname.FQDN][]dnstype.Resolver{}, DefaultResolvers: []dnstype.Resolver{ - {Addr: "8.8.4.4:53"}, + {Addr: "8.8.4.4"}, }, }, }, diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index cfdbc2809..6ebc55b07 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2207,7 +2207,7 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, prefs *ipn.Prefs, logf logger.Log addDefault := func(resolvers []dnstype.Resolver) { for _, r := range resolvers { - dcfg.DefaultResolvers = append(dcfg.DefaultResolvers, normalizeResolver(r)) + dcfg.DefaultResolvers = append(dcfg.DefaultResolvers, r) } } @@ -2236,7 +2236,7 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, prefs *ipn.Prefs, logf logger.Log dcfg.Routes[fqdn] = make([]dnstype.Resolver, 0, len(resolvers)) for _, r := range resolvers { - dcfg.Routes[fqdn] = append(dcfg.Routes[fqdn], normalizeResolver(r)) + dcfg.Routes[fqdn] = append(dcfg.Routes[fqdn], r) } } @@ -2267,16 +2267,6 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, prefs *ipn.Prefs, logf logger.Log return dcfg } -func normalizeResolver(cfg dnstype.Resolver) dnstype.Resolver { - if ip, err := netaddr.ParseIP(cfg.Addr); err == nil { - // Add 53 here for bare IPs for consistency with previous data type. - return dnstype.Resolver{ - Addr: netaddr.IPPortFrom(ip, 53).String(), - } - } - return cfg -} - // SetVarRoot sets the root directory of Tailscale's writable // storage area . (e.g. "/var/lib/tailscale") // diff --git a/net/dns/config.go b/net/dns/config.go index 27fa8395a..883f4dd30 100644 --- a/net/dns/config.go +++ b/net/dns/config.go @@ -84,10 +84,7 @@ func (c Config) hasDefaultIPResolversOnly() bool { return false } for _, r := range c.DefaultResolvers { - if ipp, err := netaddr.ParseIPPort(r.Addr); err == nil && ipp.Port() == 53 { - continue - } - if _, err := netaddr.ParseIP(r.Addr); err != nil { + if ipp, ok := r.IPPort(); !ok || ipp.Port() != 53 { return false } } diff --git a/net/dns/manager.go b/net/dns/manager.go index 53d1b1210..829a3fa8c 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -174,7 +174,7 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig } var defaultRoutes []dnstype.Resolver for _, ip := range bcfg.Nameservers { - defaultRoutes = append(defaultRoutes, dnstype.ResolverFromIP(ip)) + defaultRoutes = append(defaultRoutes, dnstype.Resolver{Addr: ip.String()}) } rcfg.Routes["."] = defaultRoutes ocfg.SearchDomains = append(ocfg.SearchDomains, bcfg.SearchDomains...) @@ -188,10 +188,8 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig // DoH or custom-port entries with something like hasDefaultIPResolversOnly. func toIPsOnly(resolvers []dnstype.Resolver) (ret []netaddr.IP) { for _, r := range resolvers { - if ipp, err := netaddr.ParseIPPort(r.Addr); err == nil && ipp.Port() == 53 { + if ipp, ok := r.IPPort(); ok && ipp.Port() == 53 { ret = append(ret, ipp.IP()) - } else if ip, err := netaddr.ParseIP(r.Addr); err == nil { - ret = append(ret, ip) } } return ret diff --git a/net/dns/manager_test.go b/net/dns/manager_test.go index 1c5e1714d..ef47571ea 100644 --- a/net/dns/manager_test.go +++ b/net/dns/manager_test.go @@ -96,7 +96,7 @@ func TestManager(t *testing.T) { { name: "corp", in: Config{ - DefaultResolvers: mustRes("1.1.1.1:53", "9.9.9.9:53"), + DefaultResolvers: mustRes("1.1.1.1", "9.9.9.9"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, os: OSConfig{ @@ -107,7 +107,7 @@ func TestManager(t *testing.T) { { name: "corp-split", in: Config{ - DefaultResolvers: mustRes("1.1.1.1:53", "9.9.9.9:53"), + DefaultResolvers: mustRes("1.1.1.1", "9.9.9.9"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, split: true, @@ -119,7 +119,7 @@ func TestManager(t *testing.T) { { name: "corp-magic", in: Config{ - DefaultResolvers: mustRes("1.1.1.1:53", "9.9.9.9:53"), + DefaultResolvers: mustRes("1.1.1.1", "9.9.9.9"), SearchDomains: fqdns("tailscale.com", "universe.tf"), Routes: upstreams("ts.com", ""), Hosts: hosts( @@ -131,7 +131,7 @@ func TestManager(t *testing.T) { SearchDomains: fqdns("tailscale.com", "universe.tf"), }, rs: resolver.Config{ - Routes: upstreams(".", "1.1.1.1:53", "9.9.9.9:53"), + Routes: upstreams(".", "1.1.1.1", "9.9.9.9"), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -141,7 +141,7 @@ func TestManager(t *testing.T) { { name: "corp-magic-split", in: Config{ - DefaultResolvers: mustRes("1.1.1.1:53", "9.9.9.9:53"), + DefaultResolvers: mustRes("1.1.1.1", "9.9.9.9"), SearchDomains: fqdns("tailscale.com", "universe.tf"), Routes: upstreams("ts.com", ""), Hosts: hosts( @@ -154,7 +154,7 @@ func TestManager(t *testing.T) { SearchDomains: fqdns("tailscale.com", "universe.tf"), }, rs: resolver.Config{ - Routes: upstreams(".", "1.1.1.1:53", "9.9.9.9:53"), + Routes: upstreams(".", "1.1.1.1", "9.9.9.9"), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -164,8 +164,8 @@ func TestManager(t *testing.T) { { name: "corp-routes", in: Config{ - DefaultResolvers: mustRes("1.1.1.1:53", "9.9.9.9:53"), - Routes: upstreams("corp.com", "2.2.2.2:53"), + DefaultResolvers: mustRes("1.1.1.1", "9.9.9.9"), + Routes: upstreams("corp.com", "2.2.2.2"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, os: OSConfig{ @@ -174,15 +174,15 @@ func TestManager(t *testing.T) { }, rs: resolver.Config{ Routes: upstreams( - ".", "1.1.1.1:53", "9.9.9.9:53", - "corp.com.", "2.2.2.2:53"), + ".", "1.1.1.1", "9.9.9.9", + "corp.com.", "2.2.2.2"), }, }, { name: "corp-routes-split", in: Config{ - DefaultResolvers: mustRes("1.1.1.1:53", "9.9.9.9:53"), - Routes: upstreams("corp.com", "2.2.2.2:53"), + DefaultResolvers: mustRes("1.1.1.1", "9.9.9.9"), + Routes: upstreams("corp.com", "2.2.2.2"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, split: true, @@ -192,14 +192,14 @@ func TestManager(t *testing.T) { }, rs: resolver.Config{ Routes: upstreams( - ".", "1.1.1.1:53", "9.9.9.9:53", - "corp.com.", "2.2.2.2:53"), + ".", "1.1.1.1", "9.9.9.9", + "corp.com.", "2.2.2.2"), }, }, { name: "routes", in: Config{ - Routes: upstreams("corp.com", "2.2.2.2:53"), + Routes: upstreams("corp.com", "2.2.2.2"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, bs: OSConfig{ @@ -212,14 +212,14 @@ func TestManager(t *testing.T) { }, rs: resolver.Config{ Routes: upstreams( - ".", "8.8.8.8:53", - "corp.com.", "2.2.2.2:53"), + ".", "8.8.8.8", + "corp.com.", "2.2.2.2"), }, }, { name: "routes-split", in: Config{ - Routes: upstreams("corp.com", "2.2.2.2:53"), + Routes: upstreams("corp.com", "2.2.2.2"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, split: true, @@ -233,8 +233,8 @@ func TestManager(t *testing.T) { name: "routes-multi", in: Config{ Routes: upstreams( - "corp.com", "2.2.2.2:53", - "bigco.net", "3.3.3.3:53"), + "corp.com", "2.2.2.2", + "bigco.net", "3.3.3.3"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, bs: OSConfig{ @@ -247,17 +247,17 @@ func TestManager(t *testing.T) { }, rs: resolver.Config{ Routes: upstreams( - ".", "8.8.8.8:53", - "corp.com.", "2.2.2.2:53", - "bigco.net.", "3.3.3.3:53"), + ".", "8.8.8.8", + "corp.com.", "2.2.2.2", + "bigco.net.", "3.3.3.3"), }, }, { name: "routes-multi-split", in: Config{ Routes: upstreams( - "corp.com", "2.2.2.2:53", - "bigco.net", "3.3.3.3:53"), + "corp.com", "2.2.2.2", + "bigco.net", "3.3.3.3"), SearchDomains: fqdns("tailscale.com", "universe.tf"), }, split: true, @@ -268,8 +268,8 @@ func TestManager(t *testing.T) { }, rs: resolver.Config{ Routes: upstreams( - "corp.com.", "2.2.2.2:53", - "bigco.net.", "3.3.3.3:53"), + "corp.com.", "2.2.2.2", + "bigco.net.", "3.3.3.3"), }, }, { @@ -290,7 +290,7 @@ func TestManager(t *testing.T) { SearchDomains: fqdns("tailscale.com", "universe.tf", "coffee.shop"), }, rs: resolver.Config{ - Routes: upstreams(".", "8.8.8.8:53"), + Routes: upstreams(".", "8.8.8.8"), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -322,7 +322,7 @@ func TestManager(t *testing.T) { { name: "routes-magic", in: Config{ - Routes: upstreams("corp.com", "2.2.2.2:53", "ts.com", ""), + Routes: upstreams("corp.com", "2.2.2.2", "ts.com", ""), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -338,8 +338,8 @@ func TestManager(t *testing.T) { }, rs: resolver.Config{ Routes: upstreams( - "corp.com.", "2.2.2.2:53", - ".", "8.8.8.8:53"), + "corp.com.", "2.2.2.2", + ".", "8.8.8.8"), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -350,7 +350,7 @@ func TestManager(t *testing.T) { name: "routes-magic-split", in: Config{ Routes: upstreams( - "corp.com", "2.2.2.2:53", + "corp.com", "2.2.2.2", "ts.com", ""), Hosts: hosts( "dave.ts.com.", "1.2.3.4", @@ -364,7 +364,7 @@ func TestManager(t *testing.T) { MatchDomains: fqdns("corp.com", "ts.com"), }, rs: resolver.Config{ - Routes: upstreams("corp.com.", "2.2.2.2:53"), + Routes: upstreams("corp.com.", "2.2.2.2"), Hosts: hosts( "dave.ts.com.", "1.2.3.4", "bradfitz.ts.com.", "2.3.4.5"), @@ -393,6 +393,14 @@ func TestManager(t *testing.T) { }, } + trIP := cmp.Transformer("ipStr", func(ip netaddr.IP) string { return ip.String() }) + trIPPort := cmp.Transformer("ippStr", func(ipp netaddr.IPPort) string { + if ipp.Port() == 53 { + return ipp.IP().String() + } + return ipp.String() + }) + for _, test := range tests { t.Run(test.name, func(t *testing.T) { f := fakeOSConfigurator{ @@ -405,8 +413,6 @@ func TestManager(t *testing.T) { if err := m.Set(test.in); err != nil { t.Fatalf("m.Set: %v", err) } - trIP := cmp.Transformer("ipStr", func(ip netaddr.IP) string { return ip.String() }) - trIPPort := cmp.Transformer("ippStr", func(ipp netaddr.IPPort) string { return ipp.String() }) if diff := cmp.Diff(f.OSConfig, test.os, trIP, trIPPort, cmpopts.EquateEmpty()); diff != "" { t.Errorf("wrong OSConfig (-got+want)\n%s", diff) } @@ -503,6 +509,11 @@ func upstreams(strs ...string) (ret map[dnsname.FQDN][]dnstype.Resolver) { panic("IPPort provided before suffix") } ret[key] = append(ret[key], dnstype.Resolver{Addr: ipp.String()}) + } else if _, err := netaddr.ParseIP(s); err == nil { + if key == "" { + panic("IPPort provided before suffix") + } + ret[key] = append(ret[key], dnstype.Resolver{Addr: s}) } else if strings.HasPrefix(s, "http") { ret[key] = append(ret[key], dnstype.Resolver{Addr: s}) } else { diff --git a/net/dns/resolver/doh_test.go b/net/dns/resolver/doh_test.go index aed624507..0141b534d 100644 --- a/net/dns/resolver/doh_test.go +++ b/net/dns/resolver/doh_test.go @@ -49,9 +49,9 @@ func TestDoH(t *testing.T) { dohSem: make(chan struct{}, 10), } - for ip := range publicdns.KnownDoH() { - t.Run(ip.String(), func(t *testing.T) { - urlBase, c, ok := f.getKnownDoHClient(ip) + for urlBase := range publicdns.DoHIPsOfBase() { + t.Run(urlBase, func(t *testing.T) { + c, ok := f.getKnownDoHClientForProvider(urlBase) if !ok { t.Fatal("expected DoH") } diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index 2741d6349..8459c28fa 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -25,6 +25,7 @@ import ( dns "golang.org/x/net/dns/dnsmessage" "inet.af/netaddr" + "tailscale.com/envknob" "tailscale.com/hostinfo" "tailscale.com/net/dns/publicdns" "tailscale.com/net/dnscache" @@ -49,6 +50,11 @@ const ( // arbitrary. dohTransportTimeout = 30 * time.Second + // dohTransportTimeout is how much of a head start to give a DoH query + // that was upgraded from a well-known public DNS provider's IP before + // normal UDP mode is attempted as a fallback. + dohHeadStart = 500 * time.Millisecond + // wellKnownHostBackupDelay is how long to artificially delay upstream // DNS queries to the "fallback" DNS server IP for a known provider // (e.g. how long to wait to query Google's 8.8.4.4 after 8.8.8.8). @@ -228,65 +234,55 @@ func (f *forwarder) Close() error { return nil } -// resolversWithDelays maps from a set of DNS server names to a slice of -// a type that included a startDelay. So if resolvers contains e.g. four -// Google DNS IPs (two IPv4 + twoIPv6), this function partition adds -// delays to some. +// resolversWithDelays maps from a set of DNS server names to a slice of a type +// that included a startDelay, upgrading any well-known DoH (DNS-over-HTTP) +// servers in the process, insert a DoH lookup first before UDP fallbacks. func resolversWithDelays(resolvers []dnstype.Resolver) []resolverAndDelay { - type hostAndFam struct { - host string // some arbitrary string representing DNS host (currently the DoH base) - bits uint8 // either 32 or 128 for IPv4 vs IPv6s address family - } + rr := make([]resolverAndDelay, 0, len(resolvers)+2) - // Track how many of each known resolver host are in the list, - // per address family. - total := map[hostAndFam]int{} - - rr := make([]resolverAndDelay, len(resolvers)) + // Add the known DoH ones first, starting immediately. + didDoH := map[string]bool{} for _, r := range resolvers { - if ip, err := netaddr.ParseIP(r.Addr); err == nil { - if host, ok := publicdns.KnownDoH()[ip]; ok { - total[hostAndFam{host, ip.BitLen()}]++ - } + ipp, ok := r.IPPort() + if !ok { + continue } + dohBase, ok := publicdns.KnownDoH()[ipp.IP()] + if !ok || didDoH[dohBase] { + continue + } + didDoH[dohBase] = true + rr = append(rr, resolverAndDelay{name: dnstype.Resolver{Addr: dohBase}}) } + type hostAndFam struct { + host string // some arbitrary string representing DNS host (currently the DoH base) + bits uint8 // either 32 or 128 for IPv4 vs IPv6s address family + } done := map[hostAndFam]int{} - for i, r := range resolvers { + for _, r := range resolvers { + ipp, ok := r.IPPort() + if !ok { + // Pass non-IP ones through unchanged, without delay. + // (e.g. DNS-over-ExitDNS when using an exit node) + rr = append(rr, resolverAndDelay{name: r}) + continue + } + ip := ipp.IP() var startDelay time.Duration - if ip, err := netaddr.ParseIP(r.Addr); err == nil { - if host, ok := publicdns.KnownDoH()[ip]; ok { - key4 := hostAndFam{host, 32} - key6 := hostAndFam{host, 128} - switch { - case ip.Is4(): - if done[key4] > 0 { - startDelay += wellKnownHostBackupDelay - } - case ip.Is6(): - total4 := total[key4] - if total4 >= 2 { - // If we have two IPv4 IPs of the same provider - // already in the set, delay the IPv6 queries - // until halfway through the timeout (so wait - // 2.5 seconds). Even the network is IPv6-only, - // the DoH dialer will fallback to IPv6 - // immediately anyway. - startDelay = responseTimeout / 2 - } else if total4 == 1 { - startDelay += wellKnownHostBackupDelay - } - if done[key6] > 0 { - startDelay += wellKnownHostBackupDelay - } - } - done[hostAndFam{host, ip.BitLen()}]++ + if host, ok := publicdns.KnownDoH()[ip]; ok { + // We already did the DoH query early. These + startDelay = dohHeadStart + key := hostAndFam{host, ip.BitLen()} + if done[key] > 0 { + startDelay += wellKnownHostBackupDelay } + done[key]++ } - rr[i] = resolverAndDelay{ + rr = append(rr, resolverAndDelay{ name: r, startDelay: startDelay, - } + }) } return rr } @@ -334,23 +330,6 @@ func (f *forwarder) packetListener(ip netaddr.IP) (packetListener, error) { return lc, nil } -// getKnownDoHClient returns an HTTP client for a DoH provider (such as Google -// or Cloudflare DNS), as a function of one of its (usually four) IPs. -// -// The provided IP is only used to determine the DoH provider; it is not -// prioritized among the set of IPs that are used by the provider. -func (f *forwarder) getKnownDoHClient(ip netaddr.IP) (urlBase string, c *http.Client, ok bool) { - urlBase, ok = publicdns.KnownDoH()[ip] - if !ok { - return "", nil, false - } - c, ok = f.getKnownDoHClientForProvider(urlBase) - if !ok { - return "", nil, false - } - return urlBase, c, true -} - // getKnownDoHClientForProvider returns an HTTP client for a specific DoH // provider named by its DoH base URL (like "https://dns.google/dns-query"). // @@ -444,34 +423,43 @@ func (f *forwarder) sendDoH(ctx context.Context, urlBase string, c *http.Client, return res, err } +var verboseDNSForward = envknob.Bool("TS_DEBUG_DNS_FORWARD_SEND") + // send sends packet to dst. It is best effort. // // send expects the reply to have the same txid as txidOut. -func (f *forwarder) send(ctx context.Context, fq *forwardQuery, rr resolverAndDelay) ([]byte, error) { +func (f *forwarder) send(ctx context.Context, fq *forwardQuery, rr resolverAndDelay) (ret []byte, err error) { + if verboseDNSForward { + f.logf("forwarder.send(%q) ...", rr.name.Addr) + defer func() { + f.logf("forwarder.send(%q) = %v, %v", rr.name.Addr, len(ret), err) + }() + } if strings.HasPrefix(rr.name.Addr, "http://") { return f.sendDoH(ctx, rr.name.Addr, f.dialer.PeerAPIHTTPClient(), fq.packet) } if strings.HasPrefix(rr.name.Addr, "https://") { + // Only known DoH providers are supported currently. Specifically, we + // only support DoH providers where we can TCP connect to them on port + // 443 at the same IP address they serve normal UDP DNS from (1.1.1.1, + // 8.8.8.8, 9.9.9.9, etc.) That's why OpenDNS and custon DoH providers + // aren't currently supported. There's no backup DNS resolution path for + // them. + urlBase := rr.name.Addr + if hc, ok := f.getKnownDoHClientForProvider(urlBase); ok { + return f.sendDoH(ctx, urlBase, hc, fq.packet) + } metricDNSFwdErrorType.Add(1) - return nil, fmt.Errorf("https:// resolvers not supported yet") + return nil, fmt.Errorf("arbitrary https:// resolvers not supported yet") } if strings.HasPrefix(rr.name.Addr, "tls://") { metricDNSFwdErrorType.Add(1) return nil, fmt.Errorf("tls:// resolvers not supported yet") } - ipp, err := netaddr.ParseIPPort(rr.name.Addr) - if err != nil { - return nil, err - } - - // Upgrade known DNS IPs to DoH (DNS-over-HTTPs). - // All known DoH is over port 53. - if urlBase, dc, ok := f.getKnownDoHClient(ipp.IP()); ok { - res, err := f.sendDoH(ctx, urlBase, dc, fq.packet) - if err == nil || ctx.Err() != nil { - return res, err - } - f.logf("DoH error from %v: %v", ipp.IP(), err) + ipp, ok := rr.name.IPPort() + if !ok { + metricDNSFwdErrorType.Add(1) + return nil, fmt.Errorf("unrecognized resolver type %q", rr.name.Addr) } metricDNSFwdUDP.Add(1) diff --git a/net/dns/resolver/forwarder_test.go b/net/dns/resolver/forwarder_test.go index f90761af6..02d3afb5b 100644 --- a/net/dns/resolver/forwarder_test.go +++ b/net/dns/resolver/forwarder_test.go @@ -7,7 +7,6 @@ package resolver import ( "flag" "fmt" - "net" "reflect" "strings" "testing" @@ -25,11 +24,7 @@ func (rr resolverAndDelay) String() string { func TestResolversWithDelays(t *testing.T) { // query q := func(ss ...string) (ipps []dnstype.Resolver) { - for _, s := range ss { - host, _, err := net.SplitHostPort(s) - if err != nil { - t.Fatal(err) - } + for _, host := range ss { ipps = append(ipps, dnstype.Resolver{Addr: host}) } return @@ -46,12 +41,8 @@ func TestResolversWithDelays(t *testing.T) { panic(fmt.Sprintf("parsing duration in %q: %v", s, err)) } } - host, _, err := net.SplitHostPort(s) - if err != nil { - t.Fatal(err) - } rr = append(rr, resolverAndDelay{ - name: dnstype.Resolver{Addr: host}, + name: dnstype.Resolver{Addr: s}, startDelay: d, }) } @@ -65,28 +56,28 @@ func TestResolversWithDelays(t *testing.T) { }{ { name: "unknown-no-delays", - in: q("1.2.3.4:53", "2.3.4.5:53"), - want: o("1.2.3.4:53", "2.3.4.5:53"), + in: q("1.2.3.4", "2.3.4.5"), + want: o("1.2.3.4", "2.3.4.5"), }, { name: "google-all-ipv4", - in: q("8.8.8.8:53", "8.8.4.4:53"), - want: o("8.8.8.8:53", "8.8.4.4:53+200ms"), + in: q("8.8.8.8", "8.8.4.4"), + want: o("https://dns.google/dns-query", "8.8.8.8+0.5s", "8.8.4.4+0.7s"), }, { name: "google-only-ipv6", - in: q("[2001:4860:4860::8888]:53", "[2001:4860:4860::8844]:53"), - want: o("[2001:4860:4860::8888]:53", "[2001:4860:4860::8844]:53+200ms"), + in: q("2001:4860:4860::8888", "2001:4860:4860::8844"), + want: o("https://dns.google/dns-query", "2001:4860:4860::8888+0.5s", "2001:4860:4860::8844+0.7s"), }, { name: "google-all-four", - in: q("8.8.8.8:53", "8.8.4.4:53", "[2001:4860:4860::8888]:53", "[2001:4860:4860::8844]:53"), - want: o("8.8.8.8:53", "8.8.4.4:53+200ms", "[2001:4860:4860::8888]:53+2.5s", "[2001:4860:4860::8844]:53+2.7s"), + in: q("8.8.8.8", "8.8.4.4", "2001:4860:4860::8888", "2001:4860:4860::8844"), + want: o("https://dns.google/dns-query", "8.8.8.8+0.5s", "8.8.4.4+0.7s", "2001:4860:4860::8888+0.5s", "2001:4860:4860::8844+0.7s"), }, { name: "quad9-one-v4-one-v6", - in: q("9.9.9.9:53", "[2620:fe::fe]:53"), - want: o("9.9.9.9:53", "[2620:fe::fe]:53+200ms"), + in: q("9.9.9.9", "2620:fe::fe"), + want: o("https://dns.quad9.net/dns-query", "9.9.9.9+0.5s", "2620:fe::fe+0.5s"), }, } diff --git a/net/dns/resolver/tsdns_test.go b/net/dns/resolver/tsdns_test.go index 106c979aa..5adec2c60 100644 --- a/net/dns/resolver/tsdns_test.go +++ b/net/dns/resolver/tsdns_test.go @@ -755,6 +755,9 @@ func TestDelegateCollision(t *testing.T) { if err != nil { t.Error(err) } + if len(ans) == 0 { + t.Fatal("no answers") + } var wantType dns.Type switch ans[0].Body.(type) { diff --git a/types/dnstype/dnstype.go b/types/dnstype/dnstype.go index e1f9e9fdb..3c3a19813 100644 --- a/types/dnstype/dnstype.go +++ b/types/dnstype/dnstype.go @@ -12,7 +12,9 @@ import "inet.af/netaddr" // Resolver is the configuration for one DNS resolver. type Resolver struct { // Addr is the address of the DNS resolver, one of: - // - A plain IP address for a "classic" UDP+TCP DNS resolver + // - A plain IP address for a "classic" UDP+TCP DNS resolver. + // This is the common format as sent by the control plane. + // - An IP:port, for tests. // - [TODO] "tls://resolver.com" for DNS over TCP+TLS // - [TODO] "https://resolver.com/query-tmpl" for DNS over HTTPS Addr string `json:",omitempty"` @@ -26,7 +28,20 @@ type Resolver struct { BootstrapResolution []netaddr.IP `json:",omitempty"` } -// ResolverFromIP defines a Resolver for ip on port 53. -func ResolverFromIP(ip netaddr.IP) Resolver { - return Resolver{Addr: netaddr.IPPortFrom(ip, 53).String()} +// IPPort returns r.Addr as an IP address and port if either +// r.Addr is an IP address (the common case) or if r.Addr +// is an IP:port (as done in tests). +func (r *Resolver) IPPort() (ipp netaddr.IPPort, ok bool) { + if r.Addr == "" || r.Addr[0] == 'h' || r.Addr[0] == 't' { + // Fast path to avoid ParseIP error allocation for obviously not IP + // cases. + return + } + if ip, err := netaddr.ParseIP(r.Addr); err == nil { + return netaddr.IPPortFrom(ip, 53), true + } + if ipp, err := netaddr.ParseIPPort(r.Addr); err == nil { + return ipp, true + } + return }