1
0
mirror of https://github.com/golang/go synced 2024-11-26 17:56:55 -07:00

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>
This commit is contained in:
Antonio Ojea 2023-12-22 18:15:34 +00:00 committed by Gopher Robot
parent 69f1290fcb
commit 2dfc5eae2e
2 changed files with 110 additions and 1 deletions

View File

@ -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

View File

@ -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{