when forwarding, try with CD=0 first

when sending a query to a forwarder for a name within a secure domain,
the first query is now sent with CD=0. when the forwarder itself
is validating, this will give it a chance to detect bogus data and
replace it with valid data before answering. this reduces our chances
of being stuck with data that can't be validated.

if the forwarder returns SERVFAIL to the initial query, the query
will be repeated with CD=1, to allow for the possibility that the
forwarder's validator is faulty or that the bogus answer is covered
by an NTA.

note: previously, CD=1 was only sent when the query name was in a
secure domain. today, validating servers have a trust anchor at the
root by default, so virtually all queries are in a secure domain.
therefore, the code has been simplified.  as long as validation is
enabled, any forward query that receives a SERVFAIL response will be
retried with CD=1.

bin/tests/system/dnssec/ns2/example.db.in

@@ -67,6 +67,10 @@ ns.bogus		A	10.53.0.3
 badds			NS	ns.badds
 ns.badds		A	10.53.0.3
 
+; A subdomain with a corrupt DS, but locally trusted by the forwarder
+localkey		NS	ns.localkey
+ns.localkey		A	10.53.0.3
+
 ; A dynamic secure subdomain
 dynamic			NS	dynamic
 dynamic			A	10.53.0.3

bin/tests/system/dnssec/ns2/sign.sh

@@ -59,7 +59,7 @@ zonefile=example.db
 
 # Get the DS records for the "example." zone.
 for subdomain in digest-alg-unsupported ds-unsupported secure badds \
-  bogus dynamic keyless nsec3 optout \
+  bogus localkey dynamic keyless nsec3 optout \
   nsec3-unknown optout-unknown multiple rsasha256 rsasha512 \
   kskonly update-nsec3 auto-nsec auto-nsec3 secure.below-cname \
   ttlpatch split-dnssec split-smart expired expiring upper lower \

bin/tests/system/dnssec/ns3/named.conf.in

@@ -103,6 +103,12 @@ zone "badds.example" {
 	allow-update { any; };
 };
 
+zone "localkey.example" {
+	type primary;
+	file "localkey.example.db.signed";
+	allow-update { any; };
+};
+
 zone "dynamic.example" {
 	type primary;
 	file "dynamic.example.db.signed";

bin/tests/system/dnssec/ns3/sign.sh

@@ -620,6 +620,21 @@ cat "$infile" "$keyname.key" >"$zonefile"
 "$SIGNER" -P -o "$zone" "$zonefile" >/dev/null
 sed -e 's/bogus/badds/g' <dsset-bogus.example. >dsset-badds.example.
 
+#
+# Same as badds, but locally trusted by the forwarder
+#
+zone=localkey.example.
+infile=bogus.example.db.in
+zonefile=localkey.example.db
+
+keyname=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone")
+
+cat "$infile" "$keyname.key" >"$zonefile"
+
+"$SIGNER" -P -o "$zone" "$zonefile" >/dev/null
+sed -e 's/bogus/localkey/g' <dsset-bogus.example. >dsset-localkey.example.
+keyfile_to_static_keys $keyname >../ns9/trusted-localkey.conf
+
 #
 # A zone with future signatures.
 #

bin/tests/system/dnssec/ns9/named.conf.in

@@ -38,3 +38,4 @@ controls {
 };
 
 include "trusted.conf";
+include "trusted-localkey.conf";

bin/tests/system/dnssec/tests.sh

@@ -4723,5 +4723,36 @@ n=$((n + 1))
 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
+echo_i "checking validating forwarder behavior with mismatching NS ($n)"
+ret=0
+rndccmd 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i
+$DIG +tcp +cd -p "$PORT" -t ns inconsistent @10.53.0.9 >dig.out.ns9.test$n.1 || ret=1
+grep "ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1" dig.out.ns9.test$n.1 >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n.1 >/dev/null && ret=1
+$DIG +tcp +cd +dnssec -p "$PORT" -t ns inconsistent @10.53.0.9 >dig.out.ns9.test$n.2 || ret=1
+grep "ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1" dig.out.ns9.test$n.2 >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n.2 >/dev/null && ret=1
+$DIG +tcp +dnssec -p "$PORT" -t ns inconsistent @10.53.0.9 >dig.out.ns9.test$n.3 || ret=1
+grep "ANSWER: 3, AUTHORITY: 0, ADDITIONAL: 1" dig.out.ns9.test$n.3 >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n.3 >/dev/null || ret=1
+n=$((n + 1))
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
+echo_i "checking forwarder CD behavior (DS mismatch and local trust anchor) ($n)"
+ret=0
+# confirm invalid DS produces SERVFAIL in forwarder
+$DIG +tcp +dnssec -p "$PORT" @10.53.0.4 localkey.example soa >dig.out.ns4.test$n || ret=1
+grep "status: SERVFAIL" dig.out.ns4.test$n >/dev/null || ret=1
+# check that lookup using forwarder succeeds and that SERVFAIL was received
+nextpart ns9/named.run >/dev/null
+$DIG +tcp +dnssec -p "$PORT" @10.53.0.9 localkey.example soa >dig.out.ns9.test$n || ret=1
+grep "status: NOERROR" dig.out.ns9.test$n >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n >/dev/null || ret=1
+nextpart ns9/named.run | grep 'status: SERVFAIL' >/dev/null || ret=1
+n=$((n + 1))
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

bin/tests/system/dnssec/tests_sh_dnssec.py

@@ -104,6 +104,7 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns3/future.example.db",
         "ns3/keyless.example.db",
         "ns3/kskonly.example.db",
+        "ns3/localkey.example.db",
         "ns3/lower.example.db",
         "ns3/managed-future.example.db",
         "ns3/multiple.example.db",
@@ -152,10 +153,10 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns4/named_dump.db",
         "ns4/named_dump.db.*",
         "ns5/revoked.conf",
-        "ns5/trusted.conf",
         "ns6/optout-tld.db",
         "ns7/split-rrsig.db",
         "ns7/split-rrsig.db.unsplit",
+        "ns9/trusted-localkey.conf",
         "signer/example.db",
         "signer/example.db.after",
         "signer/example.db.before",

lib/dns/include/dns/resolver.h

@@ -130,6 +130,11 @@ enum {
 	DNS_FETCHOPT_NOFORWARD = 1 << 15,      /*%< Do not use forwarders if
 						* possible. */
 	DNS_FETCHOPT_QMINFETCH = 1 << 16,      /*%< Qmin fetch */
+	DNS_FETCHOPT_TRYCD = 1 << 17,	       /*%< Send the first query
+						* to a forwader with
+						* CD=0, but retry with CD=1
+						* if it returns SERVFAIL.
+						*/
 
 	/*% EDNS version bits: */
 	DNS_FETCHOPT_EDNSVERSIONSET = 1 << 23,

lib/dns/resolver.c

@@ -2344,7 +2344,6 @@ resquery_send(resquery_t *query) {
 	dns_peer_t *peer = NULL;
 	dns_compress_t cctx;
 	bool useedns;
-	bool secure_domain;
 	bool tcp = ((query->options & DNS_FETCHOPT_TCP) != 0);
 	dns_ednsopt_t ednsopts[DNS_EDNSOPTIONS];
 	unsigned int ednsopt = 0;
@@ -2395,24 +2394,14 @@ resquery_send(resquery_t *query) {
 	 */
 	if ((query->options & DNS_FETCHOPT_NOCDFLAG) != 0) {
 		/* Do nothing */
-	} else if ((query->options & DNS_FETCHOPT_NOVALIDATE) != 0) {
+	} else if ((query->options & DNS_FETCHOPT_NOVALIDATE) != 0 ||
+		   (query->options & DNS_FETCHOPT_TRYCD) != 0)
+	{
 		fctx->qmessage->flags |= DNS_MESSAGEFLAG_CD;
 	} else if (res->view->enablevalidation &&
 		   ((fctx->qmessage->flags & DNS_MESSAGEFLAG_RD) != 0))
 	{
-		bool checknta = ((query->options & DNS_FETCHOPT_NONTA) == 0);
-		bool ntacovered = false;
-		result = issecuredomain(res->view, fctx->name, fctx->type,
-					isc_time_seconds(&query->start),
-					checknta, &ntacovered, &secure_domain);
-		if (result != ISC_R_SUCCESS) {
-			secure_domain = false;
-		}
-		if (secure_domain ||
-		    (ISFORWARDER(query->addrinfo) && ntacovered))
-		{
-			fctx->qmessage->flags |= DNS_MESSAGEFLAG_CD;
-		}
+		query->options |= DNS_FETCHOPT_TRYCD;
 	}
 
 	/*
@@ -7658,6 +7647,8 @@ resquery_response_continue(void *arg, isc_result_t result) {
 	fetchctx_t *fctx = rctx->fctx;
 	resquery_t *query = rctx->query;
 
+	QTRACE("response_continue");
+
 	if (result != ISC_R_SUCCESS) {
 		FCTXTRACE3("signature check failed", result);
 		if (result == DNS_R_UNEXPECTEDTSIG ||
@@ -9855,6 +9846,8 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) {
 	char code[64];
 	dns_rcode_t rcode = rctx->query->rmessage->rcode;
 
+	QTRACE("rctx_badserver");
+
 	if (rcode == dns_rcode_noerror || rcode == dns_rcode_yxdomain ||
 	    rcode == dns_rcode_nxdomain)
 	{
@@ -9949,6 +9942,16 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) {
 		}
 		query->addrinfo->flags |= FCTX_ADDRINFO_BADCOOKIE;
 		rctx->resend = true;
+	} else if (ISFORWARDER(query->addrinfo) &&
+		   query->rmessage->rcode == dns_rcode_servfail &&
+		   (query->options & DNS_FETCHOPT_TRYCD) != 0)
+	{
+		/*
+		 * We got a SERVFAIL from a forwarder with
+		 * CD=0; try again with CD=1.
+		 */
+		rctx->retryopts |= DNS_FETCHOPT_TRYCD;
+		rctx->resend = true;
 	} else {
 		rctx->broken_server = DNS_R_UNEXPECTEDRCODE;
 		rctx->next_server = true;