From: Evan Hunt Date: Fri, 1 Jul 2022 21:23:33 +0000 (-0700) Subject: try other servers when receiving FORMERR X-Git-Tag: v9.19.3~6^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f6abb80746fcb4809619879d3760a7292c5bbcc7;p=thirdparty%2Fbind9.git try other servers when receiving FORMERR previously, when an iterative query returned FORMERR, resolution would be stopped under the assumption that other servers for the same domain would likely have the same capabilities. this assumption is not correct; some domains have been reported for which some but not all servers will return FORMERR to a given query; retrying allows recursion to succeed. --- diff --git a/bin/tests/system/qmin/clean.sh b/bin/tests/system/qmin/clean.sh index 172a4230840..52c38e68bae 100644 --- a/bin/tests/system/qmin/clean.sh +++ b/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* diff --git a/bin/tests/system/qmin/tests.sh b/bin/tests/system/qmin/tests.sh index f2b2d4c3adc..26d3cb8946c 100755 --- a/bin/tests/system/qmin/tests.sh +++ b/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)) diff --git a/bin/tests/system/resolver/ans2/ans.pl b/bin/tests/system/resolver/ans2/ans.pl index dd6775fc5cd..94f645151f1 100644 --- a/bin/tests/system/resolver/ans2/ans.pl +++ b/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")); diff --git a/bin/tests/system/resolver/ans3/ans.pl b/bin/tests/system/resolver/ans3/ans.pl index eb41f5fcb37..dbd10d944d8 100644 --- a/bin/tests/system/resolver/ans3/ans.pl +++ b/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")); } diff --git a/bin/tests/system/resolver/ns4/root.db b/bin/tests/system/resolver/ns4/root.db index 71d90e32e56..df6c29b926d 100644 --- a/bin/tests/system/resolver/ns4/root.db +++ b/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 diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh index 0d649784383..3ff95767561 100755 --- a/bin/tests/system/resolver/tests.sh +++ b/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 diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 7392e2bf6d7..ba258a82feb 100644 --- a/lib/dns/resolver.c +++ b/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