net: don't retry truncated TCP responses

UDP messages may be truncated:

https://www.rfc-editor.org/rfc/rfc1035#section-4.2.1

> Messages carried by UDP are restricted to 512 bytes (not counting
> the IP or UDP headers). Longer messages are truncated and the TC
> bit is set in the header.

However, TCP also have a size limitation of 65535 bytes

https://www.rfc-editor.org/rfc/rfc1035#section-4.2.2

> The message is prefixed with a two byte length field which gives
the message length, excluding the two byte length field.

These limitations makes that the maximum possible number of A records
per RRSet is ~ 4090.

There are environments like Kubernetes that may have larger number of
records (5000+) that does not fit in a single message. In this cases,
the DNS server sets the Truncated bit on the message to indicate that
it could not send the full answer despite is using TCP.

We should only retry when the TC bit is set and the connection is UDP,
otherwise, we'll never being able to get an answer and the client will
receive an errNoAnswerFromDNSServer, that is a different behavior than
the existing in the glibc resolver, that returns all the existing
addresses in the TCP truncated response.

Fixes #64896

Signed-off-by: Antonio Ojea <aojea@google.com>
Change-Id: I1bc2c85f67668765fa60b5c0378c9e1e1756dff2
Reviewed-on: https://go-review.googlesource.com/c/go/+/552418
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Gudger <ian@iangudger.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Mateusz Poliwczak <mpoliwczak34@gmail.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go
index c291d5eb4f20e01e7f92ebad90b54b12338c737d..ec067acbd040b5542bc2aad10dffd8f093e5efbe 100644 (file)
--- a/src/net/dnsclient_unix.go
+++ b/src/net/dnsclient_unix.go
@@ -194,7 +194,14 @@ func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Que
                if err := p.SkipQuestion(); err != dnsmessage.ErrSectionDone {
                        return dnsmessage.Parser{}, dnsmessage.Header{}, errInvalidDNSResponse
                }
-               if h.Truncated { // see RFC 5966
+               // RFC 5966 indicates that when a client receives a UDP response with
+               // the TC flag set, it should take the TC flag as an indication that it
+               // should retry over TCP instead.
+               // The case when the TC flag is set in a TCP response is not well specified,
+               // so this implements the glibc resolver behavior, returning the existing
+               // dns response instead of returning a "errNoAnswerFromDNSServer" error.
+               // See go.dev/issue/64896
+               if h.Truncated && network == "udp" {
                        continue
                }
                return p, h, nil

diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index 0da36303cc8887cfbd70f10b0b55f7624cc24fc7..dfc9773a66c5e1a07427d83b4d7908bcc5bf9ab5 100644 (file)
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -94,6 +94,61 @@ func TestDNSTransportFallback(t *testing.T) {
        }
 }
 
+func TestDNSTransportNoFallbackOnTCP(t *testing.T) {
+       fake := fakeDNSServer{
+               rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
+                       r := dnsmessage.Message{
+                               Header: dnsmessage.Header{
+                                       ID:        q.Header.ID,
+                                       Response:  true,
+                                       RCode:     dnsmessage.RCodeSuccess,
+                                       Truncated: true,
+                               },
+                               Questions: q.Questions,
+                       }
+                       if n == "tcp" {
+                               r.Answers = []dnsmessage.Resource{
+                                       {
+                                               Header: dnsmessage.ResourceHeader{
+                                                       Name:   q.Questions[0].Name,
+                                                       Type:   dnsmessage.TypeA,
+                                                       Class:  dnsmessage.ClassINET,
+                                                       Length: 4,
+                                               },
+                                               Body: &dnsmessage.AResource{
+                                                       A: TestAddr,
+                                               },
+                                       },
+                               }
+                       }
+                       return r, nil
+               },
+       }
+       r := Resolver{PreferGo: true, Dial: fake.DialContext}
+       for _, tt := range dnsTransportFallbackTests {
+               ctx, cancel := context.WithCancel(context.Background())
+               defer cancel()
+               p, h, err := r.exchange(ctx, tt.server, tt.question, time.Second, useUDPOrTCP, false)
+               if err != nil {
+                       t.Error(err)
+                       continue
+               }
+               if h.RCode != tt.rcode {
+                       t.Errorf("got %v from %v; want %v", h.RCode, tt.server, tt.rcode)
+                       continue
+               }
+               a, err := p.AllAnswers()
+               if err != nil {
+                       t.Errorf("unexpected error %v getting all answers from %v", err, tt.server)
+                       continue
+               }
+               if len(a) != 1 {
+                       t.Errorf("got %d answers from %v; want 1", len(a), tt.server)
+                       continue
+               }
+       }
+}
+
 // See RFC 6761 for further information about the reserved, pseudo
 // domain names.
 var specialDomainNameTests = []struct {
@@ -1775,6 +1830,53 @@ func TestDNSUseTCP(t *testing.T) {
        }
 }
 
+func TestDNSUseTCPTruncated(t *testing.T) {
+       fake := fakeDNSServer{
+               rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
+                       r := dnsmessage.Message{
+                               Header: dnsmessage.Header{
+                                       ID:        q.Header.ID,
+                                       Response:  true,
+                                       RCode:     dnsmessage.RCodeSuccess,
+                                       Truncated: true,
+                               },
+                               Questions: q.Questions,
+                               Answers: []dnsmessage.Resource{
+                                       {
+                                               Header: dnsmessage.ResourceHeader{
+                                                       Name:   q.Questions[0].Name,
+                                                       Type:   dnsmessage.TypeA,
+                                                       Class:  dnsmessage.ClassINET,
+                                                       Length: 4,
+                                               },
+                                               Body: &dnsmessage.AResource{
+                                                       A: TestAddr,
+                                               },
+                                       },
+                               },
+                       }
+                       if n == "udp" {
+                               t.Fatal("udp protocol was used instead of tcp")
+                       }
+                       return r, nil
+               },
+       }
+       r := Resolver{PreferGo: true, Dial: fake.DialContext}
+       ctx, cancel := context.WithCancel(context.Background())
+       defer cancel()
+       p, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second, useTCPOnly, false)
+       if err != nil {
+               t.Fatal("exchange failed:", err)
+       }
+       a, err := p.AllAnswers()
+       if err != nil {
+               t.Fatalf("unexpected error %v getting all answers", err)
+       }
+       if len(a) != 1 {
+               t.Fatalf("got %d answers; want 1", len(a))
+       }
+}
+
 // Issue 34660: PTR response with non-PTR answers should ignore non-PTR
 func TestPTRandNonPTR(t *testing.T) {
        fake := fakeDNSServer{