From: kkhaike <kkhaike@gmail.com>
Date: Sat, 16 Dec 2023 10:08:16 +0000 (+0800)
Subject: net: prevent unintended retries upon receiving an empty answer response from the... 
X-Git-Tag: go1.23rc1~1199
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=aaf8e844edaa7cbd09d0fdeb9bd0b7458cbfb466;p=gostls13.git

net: prevent unintended retries upon receiving an empty answer response from the DNS server.

CL https://golang.org/cl/37879 migrates DNS message parsing to the golang.org/x/net/dns/dnsmessage package. However, during the modification of the "lame referral" error check introduced by CL https://golang.org/cl/22428, a condition was overlooked. This omission results in unexpected retries when a DNS server returns an empty response (not an invalid response, but one that includes an additional section).

Fixes #57697
Fixes #64783

Change-Id: I203896aa2902c305569005c1712fd2f9f13a9b6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/550435
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Dexter Ouyang <kkhaike@gmail.com>
---

diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go
index ec067acbd0..bef285e413 100644
--- a/src/net/dnsclient_unix.go
+++ b/src/net/dnsclient_unix.go
@@ -211,7 +211,7 @@ func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Que
 
 // checkHeader performs basic sanity checks on the header.
 func checkHeader(p *dnsmessage.Parser, h dnsmessage.Header) error {
-	rcode := extractExtendedRCode(*p, h)
+	rcode, hasAdd := extractExtendedRCode(*p, h)
 
 	if rcode == dnsmessage.RCodeNameError {
 		return errNoSuchHost
@@ -224,7 +224,7 @@ func checkHeader(p *dnsmessage.Parser, h dnsmessage.Header) error {
 
 	// libresolv continues to the next server when it receives
 	// an invalid referral response. See golang.org/issue/15434.
-	if rcode == dnsmessage.RCodeSuccess && !h.Authoritative && !h.RecursionAvailable && err == dnsmessage.ErrSectionDone {
+	if rcode == dnsmessage.RCodeSuccess && !h.Authoritative && !h.RecursionAvailable && err == dnsmessage.ErrSectionDone && !hasAdd {
 		return errLameReferral
 	}
 
@@ -263,16 +263,19 @@ func skipToAnswer(p *dnsmessage.Parser, qtype dnsmessage.Type) error {
 
 // extractExtendedRCode extracts the extended RCode from the OPT resource (EDNS(0))
 // If an OPT record is not found, the RCode from the hdr is returned.
-func extractExtendedRCode(p dnsmessage.Parser, hdr dnsmessage.Header) dnsmessage.RCode {
+// Another return value indicates whether an additional resource was found.
+func extractExtendedRCode(p dnsmessage.Parser, hdr dnsmessage.Header) (dnsmessage.RCode, bool) {
 	p.SkipAllAnswers()
 	p.SkipAllAuthorities()
+	hasAdd := false
 	for {
 		ahdr, err := p.AdditionalHeader()
+		hasAdd = hasAdd || err != dnsmessage.ErrSectionDone
 		if err != nil {
-			return hdr.RCode
+			return hdr.RCode, hasAdd
 		}
 		if ahdr.Type == dnsmessage.TypeOPT {
-			return ahdr.ExtendedRCode(hdr.RCode)
+			return ahdr.ExtendedRCode(hdr.RCode), hasAdd
 		}
 		p.SkipAdditional()
 	}
diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index dfc9773a66..0fad9e94ba 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -803,13 +803,25 @@ func TestIgnoreLameReferrals(t *testing.T) {
 					},
 				}
 			}
+		} else if s == "192.0.2.1:53" {
+			if q.Questions[0].Type == dnsmessage.TypeA && strings.HasPrefix(q.Questions[0].Name.String(), "empty.com.") {
+				var edns0Hdr dnsmessage.ResourceHeader
+				edns0Hdr.SetEDNS0(maxDNSPacketSize, dnsmessage.RCodeSuccess, false)
+
+				r.Additionals = []dnsmessage.Resource{
+					{
+						Header: edns0Hdr,
+						Body:   &dnsmessage.OPTResource{},
+					},
+				}
+			}
 		}
 
 		return r, nil
 	}}
 	r := Resolver{PreferGo: true, Dial: fake.DialContext}
 
-	addrs, err := r.LookupIPAddr(context.Background(), "www.golang.org")
+	addrs, err := r.LookupIP(context.Background(), "ip4", "www.golang.org")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -821,6 +833,15 @@ func TestIgnoreLameReferrals(t *testing.T) {
 	if got, want := addrs[0].String(), "192.0.2.1"; got != want {
 		t.Fatalf("got address %v, want %v", got, want)
 	}
+
+	_, err = r.LookupIP(context.Background(), "ip4", "empty.com")
+	de, ok := err.(*DNSError)
+	if !ok {
+		t.Fatalf("err = %#v; wanted a *net.DNSError", err)
+	}
+	if de.Err != errNoSuchHost.Error() {
+		t.Fatalf("Err = %#v; wanted %q", de.Err, errNoSuchHost.Error())
+	}
 }
 
 func BenchmarkGoLookupIP(b *testing.B) {