]> Cypherpunks repositories - gostls13.git/commitdiff
net: don't return DNS query results including the second best records unconditionally
authorMikio Hara <mikioh.mikioh@gmail.com>
Tue, 9 Jun 2015 08:30:00 +0000 (17:30 +0900)
committerMikio Hara <mikioh.mikioh@gmail.com>
Tue, 28 Jul 2015 08:39:26 +0000 (08:39 +0000)
This change prevents DNS query results using domain search list
overtaking results not using the list unconditionally, which only
happens when using builtin DNS stub resolver.

The previous internal lookup function lookup is split into lookup and
goLookupIPOrder for iteration over a set of names: FQDN or absolute
FQDN, with domain label suffixes in search list, without domain label
suffixes, and for concurrent A and AAAA record queries.

Fixes #11081.

Change-Id: I9ff0640f69276e372d97e709b149ed5b153e8601
Reviewed-on: https://go-review.googlesource.com/10836
Reviewed-by: Russ Cox <rsc@golang.org>
src/net/dnsclient.go
src/net/dnsclient_unix.go
src/net/dnsclient_unix_test.go

index e5d0ae039b8663fff6565d1a7667e5ad12f17706..ce48521bc601933a1113890cb13b95e04115f33c 100644 (file)
@@ -41,7 +41,7 @@ func answer(name, server string, dns *dnsMsg, qtype uint16) (cname string, addrs
        addrs = make([]dnsRR, 0, len(dns.answer))
 
        if dns.rcode == dnsRcodeNameError && dns.recursion_available {
-               return "", nil, &DNSError{Err: errNoSuchHost.Error(), Name: name}
+               return "", nil, &DNSError{Err: errNoSuchHost.Error(), Name: name, Server: server}
        }
        if dns.rcode != dnsRcodeSuccess {
                // None of the error codes make sense
index 6b775f713e8d62315e1a9e72dde10f09d8cb3e10..c03c1b1159fdc6c58b5f863b063452f0116fb4c6 100644 (file)
@@ -185,9 +185,9 @@ func tryOneName(cfg *dnsConfig, name string, qtype uint16) (string, []dnsRR, err
                                }
                                continue
                        }
-                       cname, addrs, err := answer(name, server, msg, qtype)
-                       if err == nil || err.(*DNSError).Err == errNoSuchHost.Error() {
-                               return cname, addrs, err
+                       cname, rrs, err := answer(name, server, msg, qtype)
+                       if err == nil || msg.rcode == dnsRcodeSuccess || msg.rcode == dnsRcodeNameError && msg.recursion_available {
+                               return cname, rrs, err
                        }
                        lastErr = err
                }
@@ -299,59 +299,55 @@ func (conf *resolverConfig) releaseSema() {
 
 func lookup(name string, qtype uint16) (cname string, rrs []dnsRR, err error) {
        if !isDomainName(name) {
-               return name, nil, &DNSError{Err: "invalid domain name", Name: name}
+               return "", nil, &DNSError{Err: "invalid domain name", Name: name}
        }
-
        resolvConf.tryUpdate("/etc/resolv.conf")
        resolvConf.mu.RLock()
-       resolv := resolvConf.dnsConfig
+       conf := resolvConf.dnsConfig
        resolvConf.mu.RUnlock()
-
-       // If name is rooted (trailing dot) or has enough dots,
-       // try it by itself first.
-       rooted := len(name) > 0 && name[len(name)-1] == '.'
-       if rooted || count(name, '.') >= resolv.ndots {
-               rname := name
-               if !rooted {
-                       rname += "."
-               }
-               // Can try as ordinary name.
-               cname, rrs, err = tryOneName(resolv, rname, qtype)
-               if rooted || err == nil {
-                       return
-               }
-       }
-
-       // Otherwise, try suffixes.
-       for _, suffix := range resolv.search {
-               rname := name + "." + suffix
-               if rname[len(rname)-1] != '.' {
-                       rname += "."
-               }
-               cname, rrs, err = tryOneName(resolv, rname, qtype)
+       for _, fqdn := range conf.nameList(name) {
+               cname, rrs, err = tryOneName(conf, fqdn, qtype)
                if err == nil {
-                       return
+                       break
                }
        }
-
-       // Last ditch effort: try unsuffixed only if we haven't already,
-       // that is, name is not rooted and has less than ndots dots.
-       if count(name, '.') < resolv.ndots {
-               cname, rrs, err = tryOneName(resolv, name+".", qtype)
-               if err == nil {
-                       return
-               }
-       }
-
-       if e, ok := err.(*DNSError); ok {
+       if err, ok := err.(*DNSError); ok {
                // Show original name passed to lookup, not suffixed one.
                // In general we might have tried many suffixes; showing
                // just one is misleading. See also golang.org/issue/6324.
-               e.Name = name
+               err.Name = name
        }
        return
 }
 
+// nameList returns a list of names for sequential DNS queries.
+func (conf *dnsConfig) nameList(name string) []string {
+       // If name is rooted (trailing dot), try only that name.
+       rooted := len(name) > 0 && name[len(name)-1] == '.'
+       if rooted {
+               return []string{name}
+       }
+       // Build list of search choices.
+       names := make([]string, 0, 1+len(conf.search))
+       // If name has enough dots, try unsuffixed first.
+       if count(name, '.') >= conf.ndots {
+               names = append(names, name+".")
+       }
+       // Try suffixes.
+       for _, suffix := range conf.search {
+               suffixed := name + "." + suffix
+               if suffixed[len(suffixed)-1] != '.' {
+                       suffixed += "."
+               }
+               names = append(names, suffixed)
+       }
+       // Try unsuffixed, if not tried first above.
+       if count(name, '.') < conf.ndots {
+               names = append(names, name+".")
+       }
+       return names
+}
+
 // hostLookupOrder specifies the order of LookupHost lookup strategies.
 // It is basically a simplified representation of nsswitch.conf.
 // "files" means /etc/hosts.
@@ -436,27 +432,44 @@ func goLookupIPOrder(name string, order hostLookupOrder) (addrs []IPAddr, err er
                        return addrs, nil
                }
        }
+       if !isDomainName(name) {
+               return nil, &DNSError{Err: "invalid domain name", Name: name}
+       }
+       resolvConf.tryUpdate("/etc/resolv.conf")
+       resolvConf.mu.RLock()
+       conf := resolvConf.dnsConfig
+       resolvConf.mu.RUnlock()
        type racer struct {
-               qtype uint16
-               rrs   []dnsRR
+               rrs []dnsRR
                error
        }
        lane := make(chan racer, 1)
        qtypes := [...]uint16{dnsTypeA, dnsTypeAAAA}
-       for _, qtype := range qtypes {
-               go func(qtype uint16) {
-                       _, rrs, err := lookup(name, qtype)
-                       lane <- racer{qtype, rrs, err}
-               }(qtype)
-       }
        var lastErr error
-       for range qtypes {
-               racer := <-lane
-               if racer.error != nil {
-                       lastErr = racer.error
-                       continue
+       for _, fqdn := range conf.nameList(name) {
+               for _, qtype := range qtypes {
+                       go func(qtype uint16) {
+                               _, rrs, err := tryOneName(conf, fqdn, qtype)
+                               lane <- racer{rrs, err}
+                       }(qtype)
+               }
+               for range qtypes {
+                       racer := <-lane
+                       if racer.error != nil {
+                               lastErr = racer.error
+                               continue
+                       }
+                       addrs = append(addrs, addrRecordList(racer.rrs)...)
+               }
+               if len(addrs) > 0 {
+                       break
                }
-               addrs = append(addrs, addrRecordList(racer.rrs)...)
+       }
+       if lastErr, ok := lastErr.(*DNSError); ok {
+               // Show original name passed to lookup, not suffixed one.
+               // In general we might have tried many suffixes; showing
+               // just one is misleading. See also golang.org/issue/6324.
+               lastErr.Name = name
        }
        sortByRFC6724(addrs)
        if len(addrs) == 0 {
index c6bfc67abc24d573b16b7a4c143304e399cf0549..a999f8f060726b17e268ae1ca7ce90735386b258 100644 (file)
@@ -224,6 +224,160 @@ func TestUpdateResolvConf(t *testing.T) {
        }
 }
 
+var goLookupIPWithResolverConfigTests = []struct {
+       name  string
+       lines []string // resolver configuration lines
+       error
+       a, aaaa bool // whether response contains A, AAAA-record
+}{
+       // no records, transport timeout
+       {
+               "jgahvsekduiv9bw4b3qhn4ykdfgj0493iohkrjfhdvhjiu4j",
+               []string{
+                       "options timeout:1 attempts:1",
+                       "nameserver 255.255.255.255", // please forgive us for abuse of limited broadcast address
+               },
+               &DNSError{Name: "jgahvsekduiv9bw4b3qhn4ykdfgj0493iohkrjfhdvhjiu4j", Server: "255.255.255.255:53", IsTimeout: true},
+               false, false,
+       },
+
+       // no records, non-existent domain
+       {
+               "jgahvsekduiv9bw4b3qhn4ykdfgj0493iohkrjfhdvhjiu4j",
+               []string{
+                       "options timeout:3 attempts:1",
+                       "nameserver 8.8.8.8",
+               },
+               &DNSError{Name: "jgahvsekduiv9bw4b3qhn4ykdfgj0493iohkrjfhdvhjiu4j", Server: "8.8.8.8:53", IsTimeout: false},
+               false, false,
+       },
+
+       // a few A records, no AAAA records
+       {
+               "ipv4.google.com.",
+               []string{
+                       "nameserver 8.8.8.8",
+                       "nameserver 2001:4860:4860::8888",
+               },
+               nil,
+               true, false,
+       },
+       {
+               "ipv4.google.com",
+               []string{
+                       "domain golang.org",
+                       "nameserver 2001:4860:4860::8888",
+                       "nameserver 8.8.8.8",
+               },
+               nil,
+               true, false,
+       },
+       {
+               "ipv4.google.com",
+               []string{
+                       "search x.golang.org y.golang.org",
+                       "nameserver 2001:4860:4860::8888",
+                       "nameserver 8.8.8.8",
+               },
+               nil,
+               true, false,
+       },
+
+       // no A records, a few AAAA records
+       {
+               "ipv6.google.com.",
+               []string{
+                       "nameserver 2001:4860:4860::8888",
+                       "nameserver 8.8.8.8",
+               },
+               nil,
+               false, true,
+       },
+       {
+               "ipv6.google.com",
+               []string{
+                       "domain golang.org",
+                       "nameserver 8.8.8.8",
+                       "nameserver 2001:4860:4860::8888",
+               },
+               nil,
+               false, true,
+       },
+       {
+               "ipv6.google.com",
+               []string{
+                       "search x.golang.org y.golang.org",
+                       "nameserver 8.8.8.8",
+                       "nameserver 2001:4860:4860::8888",
+               },
+               nil,
+               false, true,
+       },
+
+       // both A and AAAA records
+       {
+               "hostname.as112.net", // see RFC 7534
+               []string{
+                       "domain golang.org",
+                       "nameserver 2001:4860:4860::8888",
+                       "nameserver 8.8.8.8",
+               },
+               nil,
+               true, true,
+       },
+       {
+               "hostname.as112.net", // see RFC 7534
+               []string{
+                       "search x.golang.org y.golang.org",
+                       "nameserver 2001:4860:4860::8888",
+                       "nameserver 8.8.8.8",
+               },
+               nil,
+               true, true,
+       },
+}
+
+func TestGoLookupIPWithResolverConfig(t *testing.T) {
+       if testing.Short() || !*testExternal {
+               t.Skip("avoid external network")
+       }
+
+       conf, err := newResolvConfTest()
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer conf.teardown()
+
+       for _, tt := range goLookupIPWithResolverConfigTests {
+               if err := conf.writeAndUpdate(tt.lines); err != nil {
+                       t.Error(err)
+                       continue
+               }
+               conf.tryUpdate(conf.path)
+               addrs, err := goLookupIP(tt.name)
+               if err != nil {
+                       if err, ok := err.(*DNSError); !ok || (err.Name != tt.error.(*DNSError).Name || err.Server != tt.error.(*DNSError).Server || err.IsTimeout != tt.error.(*DNSError).IsTimeout) {
+                               t.Errorf("got %v; want %v", err, tt.error)
+                       }
+                       continue
+               }
+               if len(addrs) == 0 {
+                       t.Errorf("no records for %s", tt.name)
+               }
+               if !tt.a && !tt.aaaa && len(addrs) > 0 {
+                       t.Errorf("unexpected %v for %s", addrs, tt.name)
+               }
+               for _, addr := range addrs {
+                       if !tt.a && addr.IP.To4() != nil {
+                               t.Errorf("got %v; must not be IPv4 address", addr)
+                       }
+                       if !tt.aaaa && addr.IP.To16() != nil && addr.IP.To4() == nil {
+                               t.Errorf("got %v; must not be IPv6 address", addr)
+                       }
+               }
+       }
+}
+
 func BenchmarkGoLookupIP(b *testing.B) {
        testHookUninstaller.Do(uninstallTestHooks)