From 41356227f24801510f9eb173ffe4ddd4768587db Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 18 May 2023 14:10:44 -0400 Subject: [PATCH] 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 --- libnetwork/resolver.go | 27 ++--------------- libnetwork/resolver_test.go | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/libnetwork/resolver.go b/libnetwork/resolver.go index e8e6944039..1304e95df2 100644 --- a/libnetwork/resolver.go +++ b/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" -} diff --git a/libnetwork/resolver_test.go b/libnetwork/resolver_test.go index e782de6e2a..4637a4298e 100644 --- a/libnetwork/resolver_test.go +++ b/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()) +}