Merge branch '3152-retry-on-formerr' into 'main'

try other servers when receiving FORMERR

Closes #3152

See merge request isc-projects/bind9!6510

CHANGES

@@ -1,3 +1,7 @@
+5916.	[bug]		When resolving a name, don't give up immediately if an
+			authoritative server returns FORMERR; try the other
+			servers first. [GL #3152]
+
 5915.	[bug]		Detect missing closing brace (}) and computational
 			overflows in $GENERATE directives. [GL #3429]
 

bin/tests/system/qmin/clean.sh

@@ -13,7 +13,7 @@
 
 rm -f ns*/named.conf
 rm -f */named.memstats
-rm -f */named.run
+rm -f */named.run */named.run.prev
 rm -f dig.out.*
 rm -f ns*/named.lock
 rm -f ans*/query.log*

bin/tests/system/qmin/tests.sh

@@ -272,7 +272,7 @@ $RNDCCMD 10.53.0.7 flush
 n=$((n+1))
 echo_i "information that minimization was unsuccessful for .ugly is logged ($n)"
 ret=0
-grep "success resolving 'icky.icky.icky.ptang.zoop.boing.ugly/A' after disabling qname minimization due to 'FORMERR'" ns7/named.run > /dev/null || ret=1
+wait_for_log 5 "success resolving 'icky.icky.icky.ptang.zoop.boing.ugly/A' after disabling qname minimization" ns7/named.run > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 

bin/tests/system/resolver/ans2/ans.pl

@@ -122,6 +122,8 @@ for (;;) {
 		# Delegation to broken TLD.
 		$packet->push("authority", new Net::DNS::RR("broken 300 NS ns.broken"));
 		$packet->push("additional", new Net::DNS::RR("ns.broken 300 A 10.53.0.4"));
+	} elsif ($qname =~ /\.partial-formerr/) {
+		$packet->header->rcode("FORMERR");
 	} else {
 		# Data for the "bogus referrals" test
 		$packet->push("authority", new Net::DNS::RR("below.www.example.com 300 NS ns.below.www.example.com"));

bin/tests/system/resolver/ans3/ans.pl

@@ -125,6 +125,9 @@ for (;;) {
 		$packet->push("answer",
 			      new Net::DNS::RR($qname .
 				       " 300 A 10.53.0.3"));
+	} elsif ($qname =~ /\.partial-formerr/) {
+		$packet->push("answer",
+			      new Net::DNS::RR($qname .  " 1 A 10.53.0.3"));
 	} else {
 		$packet->push("answer", new Net::DNS::RR("www.example.com 300 A 1.2.3.4"));
 	}

bin/tests/system/resolver/ns4/root.db

@@ -32,3 +32,6 @@ sourcens.		NS	ns.sourcens.
 ns.sourcens.		A	10.53.0.4
 targetns. 		NS	ns.targetns.
 ns.targetns.		A	10.53.0.6
+partial-formerr.	NS	ns.partial-formerr.
+ns.partial-formerr.	A	10.53.0.2
+ns.partial-formerr.	A	10.53.0.3

bin/tests/system/resolver/tests.sh

@@ -862,6 +862,14 @@ grep "status: SERVFAIL" dig.ns5.out.${n} > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
+n=$((n+1))
+echo_i "checking SERVFAIL is not returned if only some authoritative servers return FORMERR ($n)"
+ret=0
+dig_with_opts @10.53.0.5 ns.partial-formerr. a > dig.ns5.out.${n} || ret=1
+grep "status: SERVFAIL" dig.ns5.out.${n} > /dev/null && ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
 n=$((n+1))
 echo_i "check logged command line ($n)"
 ret=0

lib/dns/resolver.c

@@ -9890,25 +9890,13 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) {
 		add_bad_edns(fctx, &query->addrinfo->sockaddr);
 		inc_stats(fctx->res, dns_resstatscounter_edns0fail);
 	} else if (rcode == dns_rcode_formerr) {
-		if (ISFORWARDER(query->addrinfo)) {
-			/*
-			 * This forwarder doesn't understand us,
-			 * but other forwarders might.  Keep trying.
-			 */
-			rctx->broken_server = DNS_R_REMOTEFORMERR;
-			rctx->next_server = true;
-		} else {
-			/*
-			 * The server doesn't understand us.  Since
-			 * all servers for a zone need similar
-			 * capabilities, we assume that we will get
-			 * FORMERR from all servers, and thus we
-			 * cannot make any more progress with this
-			 * fetch.
-			 */
-			log_formerr(fctx, "server sent FORMERR");
-			result = DNS_R_FORMERR;
-		}
+		/*
+		 * The server (or forwarder) doesn't understand us,
+		 * but others might.
+		 */
+		rctx->next_server = true;
+		rctx->broken_server = DNS_R_REMOTEFORMERR;
+		log_formerr(fctx, "server sent FORMERR");
 	} else if (rcode == dns_rcode_badvers) {
 		unsigned int version;
 #if DNS_EDNS_VERSION > 0