From: Antonio Ojea Date: Fri, 22 Dec 2023 18:15:34 +0000 (+0000) Subject: net: don't retry truncated TCP responses X-Git-Tag: go1.23rc1~1254 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=2dfc5eae2ee177d44f867783e13d7401b9f34a18;p=gostls13.git 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 Change-Id: I1bc2c85f67668765fa60b5c0378c9e1e1756dff2 Reviewed-on: https://go-review.googlesource.com/c/go/+/552418 Reviewed-by: Ian Lance Taylor Reviewed-by: Ian Gudger Auto-Submit: Ian Lance Taylor Reviewed-by: Mateusz Poliwczak Reviewed-by: Mauri de Souza Meneguzzo Reviewed-by: Damien Neil LUCI-TryBot-Result: Go LUCI --- diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go index c291d5eb4f..ec067acbd0 100644 --- 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 0da36303cc..dfc9773a66 100644 --- 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{