chg: usr: When forwarding, query with CD=0 first

Previously, when queries were forwarded to a remote resolver, the CD (checking disabled) bit was used, which could lead to bogus data being retrieved that might have been corrected if validation had been permitted. The CD bit is now only used as a fallback if an initial query without CD fails. See #5132.

Merge branch '5132-cd-retry' into 'main'

See merge request isc-projects/bind9!10024

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/ns4/named5.conf.in

@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0.  If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+// NS4
+
+options {
+	query-source address 10.53.0.4;
+	notify-source 10.53.0.4;
+	transfer-source 10.53.0.4;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.4; };
+	listen-on-v6 { none; };
+	recursion yes;
+	dnssec-validation yes;
+	minimal-responses no;
+
+};
+
+# Note: This is deliberately wrong! The bind.keys file contains
+# the real DNS root key, so it won't work with the local toy
+# root zones used in the tests. This is to test a forwarder
+# talking to a resolver with a misconfigured trust anchor.
+include "../../../../../bind.keys";
+
+key rndc_key {
+	secret "1234abcd8765";
+	algorithm @DEFAULT_HMAC@;
+};
+
+controls {
+	inet 10.53.0.4 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+zone "." {
+	type hint;
+	file "../../_common/root.hint";
+};
+
+zone "corp" {
+	type static-stub;
+	server-addresses { 10.53.0.2; };
+};

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,56 @@ 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
+rndccmd 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i
+# confirm invalid DS produces SERVFAIL in resolver
+$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))
+
+copy_setports ns4/named5.conf.in ns4/named.conf
+rndccmd 10.53.0.4 reconfig 2>&1 | sed 's/^/ns4 /' | cat_i
+sleep 3
+
+echo_i "checking forwarder CD behavior (forward server with bad trust anchor) ($n)"
+ret=0
+# confirm invalid trust anchor produces SERVFAIL in resolver
+$DIG +tcp +dnssec -p "$PORT" @10.53.0.4 a.secure.example >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 a.secure.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

@@ -131,6 +131,11 @@ enum {
 						 * possible. */
 	DNS_FETCHOPT_QMINFETCH = 1 << 16,	/*%< Qmin fetch */
 	DNS_FETCHOPT_WANTZONEVERSION = 1 << 17, /*%< Request ZONEVERSION */
+	DNS_FETCHOPT_TRYCD = 1 << 18,		/*%< 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

@@ -2345,7 +2345,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;
@@ -2396,24 +2395,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;
 	}
 
 	/*
@@ -7788,6 +7777,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 ||
@@ -10006,6 +9997,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)
 	{
@@ -10100,6 +10093,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;