libnetwork: just forward the external DNS response

Our resolver is just a forwarder for external DNS so it should act like
it. Unless it's a server failure or refusal, take the response at face
value and forward it along to the client. RFC 8020 is only applicable to
caching recursive name servers and our resolver is neither caching nor
recursive.

Signed-off-by: Cory Snider <csnider@mirantis.com>

libnetwork/resolver.go

@@ -449,7 +449,6 @@ func (r *Resolver) dialExtDNS(proto string, server extDNSEntry) (net.Conn, error
 }
 
 func (r *Resolver) forwardExtDNS(proto string, query *dns.Msg) *dns.Msg {
-	queryName, queryType := query.Question[0].Name, query.Question[0].Qtype
 	for _, extDNS := range r.extDNSList {
 		if extDNS.IPStr == "" {
 			break
@@ -477,20 +476,7 @@ func (r *Resolver) forwardExtDNS(proto string, query *dns.Msg) *dns.Msg {
 		case dns.RcodeServerFailure, dns.RcodeRefused:
 			// Server returned FAILURE: continue with the next external DNS server
 			// Server returned REFUSED: this can be a transitional status, so continue with the next external DNS server
-			logrus.Debugf("[resolver] external DNS %s:%s responded with %s for %q", proto, extDNS.IPStr, statusString(resp.Rcode), queryName)
-			continue
-		case dns.RcodeNameError:
-			// Server returned NXDOMAIN. Stop resolution if it's an authoritative answer (see RFC 8020: https://tools.ietf.org/html/rfc8020#section-2)
-			logrus.Debugf("[resolver] external DNS %s:%s responded with %s for %q", proto, extDNS.IPStr, statusString(resp.Rcode), queryName)
-			if resp.Authoritative {
-				break
-			}
-			continue
-		case dns.RcodeSuccess:
-			// All is well
-		default:
-			// Server gave some error. Log the error, and continue with the next external DNS server
-			logrus.Debugf("[resolver] external DNS %s:%s responded with %s (code %d) for %q", proto, extDNS.IPStr, statusString(resp.Rcode), resp.Rcode, queryName)
+			logrus.Debugf("[resolver] external DNS %s:%s returned failure:\n%s", proto, extDNS.IPStr, resp)
 			continue
 		}
 		answers := 0
@@ -509,8 +495,8 @@ func (r *Resolver) forwardExtDNS(proto string, query *dns.Msg) *dns.Msg {
 				r.backend.HandleQueryResp(h.Name, ip)
 			}
 		}
-		if resp.Answer == nil || answers == 0 {
-			logrus.Debugf("[resolver] external DNS %s:%s did not return any %s records for %q", proto, extDNS.IPStr, dns.TypeToString[queryType], queryName)
+		if len(resp.Answer) == 0 {
+			logrus.Debugf("[resolver] external DNS %s:%s returned response with no answers:\n%s", proto, extDNS.IPStr, resp)
 		}
 		resp.Compress = true
 		return resp
@@ -558,10 +544,3 @@ func (r *Resolver) exchange(proto string, extDNS extDNSEntry, query *dns.Msg) *d
 	}
 	return resp
 }
-
-func statusString(responseCode int) string {
-	if s, ok := dns.RcodeToString[responseCode]; ok {
-		return s
-	}
-	return "UNKNOWN"
-}

libnetwork/resolver_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/miekg/dns"
 	"github.com/sirupsen/logrus"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
 )
 
@@ -457,3 +458,60 @@ type badSRVDNSBackend struct{ noopDNSBackend }
 func (badSRVDNSBackend) ResolveService(name string) ([]*net.SRV, []net.IP) {
 	return []*net.SRV{nil, nil, nil}, nil // Mismatched slice lengths
 }
+
+func TestProxyNXDOMAIN(t *testing.T) {
+	mockSOA, err := dns.NewRR(".	86367	IN	SOA	a.root-servers.net. nstld.verisign-grs.com. 2023051800 1800 900 604800 86400\n")
+	assert.NilError(t, err)
+	assert.Assert(t, mockSOA != nil)
+
+	serveStarted := make(chan struct{})
+	srv := &dns.Server{
+		Net:  "udp",
+		Addr: "127.0.0.1:0",
+		Handler: dns.HandlerFunc(func(w dns.ResponseWriter, r *dns.Msg) {
+			msg := new(dns.Msg).SetRcode(r, dns.RcodeNameError)
+			msg.Ns = append(msg.Ns, dns.Copy(mockSOA))
+			w.WriteMsg(msg)
+		}),
+		NotifyStartedFunc: func() { close(serveStarted) },
+	}
+	serveDone := make(chan error, 1)
+	go func() {
+		defer close(serveDone)
+		serveDone <- srv.ListenAndServe()
+	}()
+
+	select {
+	case err := <-serveDone:
+		t.Fatal(err)
+	case <-serveStarted:
+	}
+
+	defer func() {
+		if err := srv.Shutdown(); err != nil {
+			t.Error(err)
+		}
+		<-serveDone
+	}()
+
+	srvAddr := srv.PacketConn.LocalAddr().(*net.UDPAddr)
+	rsv := NewResolver("", true, noopDNSBackend{})
+	rsv.SetExtServers([]extDNSEntry{
+		{IPStr: srvAddr.IP.String(), port: uint16(srvAddr.Port), HostLoopback: true},
+	})
+
+	// The resolver logs lots of valuable info at level debug. Redirect it
+	// to t.Log() so the log spew is emitted only if the test fails.
+	defer redirectLogrusTo(t)()
+
+	w := &tstwriter{localAddr: srv.PacketConn.LocalAddr()}
+	q := new(dns.Msg).SetQuestion("example.net.", dns.TypeA)
+	rsv.serveDNS(w, q)
+	resp := w.GetResponse()
+	checkNonNullResponse(t, resp)
+	t.Log("Response:\n" + resp.String())
+	checkDNSResponseCode(t, resp, dns.RcodeNameError)
+	assert.Assert(t, is.Len(resp.Answer, 0))
+	assert.Assert(t, is.Len(resp.Ns, 1))
+	assert.Equal(t, resp.Ns[0].String(), mockSOA.String())
+}