Merge pull request #45565 from corhere/fix-dns-servfail

libnetwork: just forward the external DNS response

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())
+}