From: Witold Kręcicki <wpk@isc.org>
Date: Tue, 23 Oct 2018 11:45:30 +0000 (+0200)
Subject: Set result to SERVFAIL if upstream responded with FORMERR
X-Git-Tag: v9.13.4~106^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b5c9a8caadafb4295aaf7040eee42ea0f1aeb8f6;p=thirdparty%2Fbind9.git

Set result to SERVFAIL if upstream responded with FORMERR

Commit ba912435427cf884fdc1ca26743eba6c00439106 causes the resolver to
respond to a client query with FORMERR when all upstream queries sent to
the servers authoritative for QNAME elicit FORMERR responses.  This
happens because resolver code returns DNS_R_FORMERR in such a case and
dns_result_torcode() acts as a pass-through for all arguments which are
already a valid RCODE.

The correct RCODE to set in the response returned to the client in the
case described above is SERVFAIL.  Make sure this happens by overriding
the RCODE in query_gotanswer(), on the grounds that any format errors in
the client query itself should be caught long before execution reaches
that point.  This change should not reduce query error logging accuracy
as the resolver code itself reports the exact reason for returning a
DNS_R_FORMERR result using log_formerr().
---

diff --git a/bin/tests/system/resolver/ans8/ans.pl b/bin/tests/system/resolver/ans8/ans.pl
index a019e6cf5c5..f3bf33fa6de 100644
--- a/bin/tests/system/resolver/ans8/ans.pl
+++ b/bin/tests/system/resolver/ans8/ans.pl
@@ -74,6 +74,9 @@ sub handleUDP {
 	} elsif ($qname eq "ns.no-questions") {
 		$packet->push("answer", new Net::DNS::RR($qname . " 300 A 10.53.0.8"));
 		return $packet->data;
+	} elsif ($qname =~ /\.formerr-to-all$/) {
+		$packet->header->rcode("FORMERR");
+		return $packet->data;
 	}
 
 	# don't use Net::DNS to construct the header only reply as early
diff --git a/bin/tests/system/resolver/ns4/root.db b/bin/tests/system/resolver/ns4/root.db
index 721765d1be7..43e4bea4a60 100644
--- a/bin/tests/system/resolver/ns4/root.db
+++ b/bin/tests/system/resolver/ns4/root.db
@@ -24,3 +24,5 @@ example.net.		NS	ns.example.net.
 ns.example.net.		A	10.53.0.6
 no-questions.		NS	ns.no-questions.
 ns.no-questions.	A	10.53.0.8
+formerr-to-all.		NS	ns.formerr-to-all.
+ns.formerr-to-all.	A	10.53.0.8
diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh
index 2bea476f1be..75ab5c06622 100755
--- a/bin/tests/system/resolver/tests.sh
+++ b/bin/tests/system/resolver/tests.sh
@@ -788,5 +788,13 @@ grep "1.2.3.4" dig.ns5.out.${n} > /dev/null && ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo_i "checking SERVFAIL is returned when all authoritative servers return FORMERR ($n)"
+ret=0
+$DIG $DIGOPTS @10.53.0.5 ns.formerr-to-all. 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=`expr $status + $ret`
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1
diff --git a/lib/ns/query.c b/lib/ns/query.c
index e7ccb99586d..a1fc0d3a947 100644
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -6737,6 +6737,10 @@ query_gotanswer(query_ctx_t *qctx, isc_result_t result) {
 	case DNS_R_DNAME:
 		return (query_dname(qctx));
 
+	case DNS_R_FORMERR:
+		QUERY_ERROR(qctx, DNS_R_SERVFAIL);
+		return (query_done(qctx));
+
 	default:
 		/*
 		 * Something has gone wrong.