From 5eaf898fcb51d88736c0c4b8e15bbd8d621cd66f Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Mon, 18 Dec 2023 18:01:07 -0500 Subject: [PATCH] libnetwork: write ServFail if DNS reply msg is bad If the resolver's DNSBackend returns a name that cannot be marshaled into a well-formed DNS message, the resolver will only discover this when it attempts to write the reply message and it fails with an error. No reply message is sent, leaving the client to wait out its timeout and the user in the dark about what went wrong. When writing the intended reply message fails, retry once with a ServFail response to inform the client and user that the DNS query was not resolved due to a problem with to the resolver, not the network. Signed-off-by: Cory Snider --- libnetwork/resolver.go | 11 ++++++++++- libnetwork/resolver_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/libnetwork/resolver.go b/libnetwork/resolver.go index 9c84ddbcfb..9df2154499 100644 --- a/libnetwork/resolver.go +++ b/libnetwork/resolver.go @@ -378,9 +378,18 @@ func (r *Resolver) serveDNS(w dns.ResponseWriter, query *dns.Msg) { reply := func(msg *dns.Msg) { if err = w.WriteMsg(msg); err != nil { - r.log(ctx).WithError(err).Errorf("[resolver] failed to write response") + r.log(ctx).WithError(err).Error("[resolver] failed to write response") span.RecordError(err) span.SetStatus(codes.Error, "WriteMsg failed") + // Make a best-effort attempt to send a failure response to the + // client so it doesn't have to wait for a timeout if the failure + // has to do with the content of msg rather than the connection. + if msg.Rcode != dns.RcodeServerFailure { + if err := w.WriteMsg(new(dns.Msg).SetRcode(query, dns.RcodeServerFailure)); err != nil { + r.log(ctx).WithError(err).Error("[resolver] writing ServFail response also failed") + span.RecordError(err) + } + } } } diff --git a/libnetwork/resolver_test.go b/libnetwork/resolver_test.go index 3a7f0b9aac..74889a3127 100644 --- a/libnetwork/resolver_test.go +++ b/libnetwork/resolver_test.go @@ -86,7 +86,7 @@ func checkDNSAnswersCount(t *testing.T, m *dns.Msg, expected int) { func checkDNSResponseCode(t *testing.T, m *dns.Msg, expected int) { t.Helper() if m.MsgHdr.Rcode != expected { - t.Fatalf("Expected DNS response code: %d. Found: %d", expected, m.MsgHdr.Rcode) + t.Fatalf("Expected DNS response code: %d (%s). Found: %d (%s)", expected, dns.RcodeToString[expected], m.MsgHdr.Rcode, dns.RcodeToString[m.MsgHdr.Rcode]) } } @@ -359,3 +359,26 @@ func TestProxyNXDOMAIN(t *testing.T) { assert.Assert(t, is.Len(resp.Ns, 1)) assert.Equal(t, resp.Ns[0].String(), mockSOA.String()) } + +type ptrDNSBackend struct { + noopDNSBackend + zone map[string]string +} + +func (b *ptrDNSBackend) ResolveIP(_ context.Context, name string) string { + return b.zone[name] +} + +// Regression test for https://github.com/moby/moby/issues/46928 +func TestInvalidReverseDNS(t *testing.T) { + rsv := NewResolver("", false, &ptrDNSBackend{zone: map[string]string{"4.3.2.1": "sixtyfourcharslong9012345678901234567890123456789012345678901234"}}) + rsv.logger = testLogger(t) + + w := &tstwriter{} + q := new(dns.Msg).SetQuestion("4.3.2.1.in-addr.arpa.", dns.TypePTR) + rsv.serveDNS(w, q) + resp := w.GetResponse() + checkNonNullResponse(t, resp) + t.Log("Response: ", resp.String()) + checkDNSResponseCode(t, resp, dns.RcodeServerFailure) +}