Merge branch '901-empty-any' into 'master'

handle empty ANY query responses

Closes #901

See merge request isc-projects/bind9!1580

CHANGES

@@ -1,3 +1,7 @@
+5169.	[bug]		The presence of certain types in an otherwise
+			empty node could cause a crash while processing a
+			type ANY query. [GL #901]
+
 5168.	[bug]		Do not crash on shutdown when RPZ fails to load.  Also,
 			keep previous version of the database if RPZ fails to
 			load. [GL #813]

bin/tests/system/dnssec/ns3/insecure.example.db

@@ -21,4 +21,5 @@ ns			A	10.53.0.3
 a			A	10.0.0.1
 b			A	10.0.0.2
 d			A	10.0.0.4
+x			DNSKEY	258 3 5 Cg==
 z			A	10.0.0.26

bin/tests/system/dnssec/ns3/secure.example.db.in

@@ -28,6 +28,7 @@ g			A	10.0.0.7
 z			A	10.0.0.26
 a.a.a.a.a.a.a.a.a.a.e	A	10.0.0.27
 x			CNAME	a
+zz			DNSKEY	258 3 5 Cg==
 
 private			NS	ns.private
 ns.private		A	10.53.0.2

bin/tests/system/dnssec/tests.sh

@@ -3635,6 +3635,19 @@ n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
+echo_i "checking DNSSEC records are occluded from ANY in an insecure zone ($n)"
+ret=0
+dig_with_opts any x.insecure.example. @10.53.0.3 > dig.out.ns3.1.test$n || ret=1
+grep "status: NOERROR" dig.out.ns3.1.test$n > /dev/null || ret=1
+grep "ANSWER: 0," dig.out.ns3.1.test$n > /dev/null || ret=1
+dig_with_opts any zz.secure.example. @10.53.0.3 > dig.out.ns3.2.test$n || ret=1
+grep "status: NOERROR" dig.out.ns3.2.test$n > /dev/null || ret=1
+# DNSKEY+RRSIG, NSEC+RRSIG
+grep "ANSWER: 4," dig.out.ns3.2.test$n > /dev/null || ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
 # Note: after this check, ns4 will not be validating any more; do not add any
 # further validation tests employing ns4 below this check.
 echo_i "check that validation defaults to off when dnssec-enable is off ($n)"

doc/arm/notes.xml

@@ -127,7 +127,9 @@
     <itemizedlist>
       <listitem>
         <para>
-          None.
+	  The presence of certain record types in an otherwise empty
+	  node of a zone could trigger an assertion failure while processing
+	  a query of type ANY. [GL #901]
 	</para>
       </listitem>
     </itemizedlist>

lib/ns/query.c

@@ -6727,7 +6727,7 @@ query_addnoqnameproof(query_ctx_t *qctx) {
  */
 static isc_result_t
 query_respond_any(query_ctx_t *qctx) {
-	bool found = false;
+	bool found = false, hidden = false;
 	dns_rdatasetiter_t *rdsiter = NULL;
 	isc_result_t result;
 	dns_rdatatype_t onetype = 0; 	/* type to use for minimal-any */
@@ -6782,11 +6782,11 @@ query_respond_any(query_ctx_t *qctx) {
 		    dns_rdatatype_isdnssec(qctx->rdataset->type))
 		{
 			/*
-			 * The zone is transitioning from insecure
+			 * The zone may be transitioning from insecure
-			 * to secure. Hide the dnssec records from
+			 * to secure. Hide DNSSEC records from ANY queries.
-			 * ANY queries.
 			 */
 			dns_rdataset_disassociate(qctx->rdataset);
+			hidden = true;
 		} else if (qctx->view->minimal_any &&
 			   !TCP(qctx->client) && !WANTDNSSEC(qctx->client) &&
 			   qctx->qtype == dns_rdatatype_any &&
@@ -6808,7 +6808,8 @@ query_respond_any(query_ctx_t *qctx) {
 			    qctx->rdataset->type == qctx->qtype) &&
 			   qctx->rdataset->type != 0)
 		{
-			if (NOQNAME(qctx->rdataset) && WANTDNSSEC(qctx->client))
+			if (NOQNAME(qctx->rdataset) &&
+			    WANTDNSSEC(qctx->client))
 			{
 				qctx->noqname = qctx->rdataset;
 			} else {
@@ -6864,8 +6865,9 @@ query_respond_any(query_ctx_t *qctx) {
 			}
 
 			qctx->rdataset = ns_client_newrdataset(qctx->client);
-			if (qctx->rdataset == NULL)
+			if (qctx->rdataset == NULL) {
 				break;
+			}
 		} else {
 			/*
 			 * We're not interested in this rdataset.
@@ -6886,48 +6888,59 @@ query_respond_any(query_ctx_t *qctx) {
 	}
 
 	if (found) {
+		/*
+		 * Call hook if any answers were found.
+		 * Do this before releasing qctx->fname, in case
+		 * the hook function needs it.
+		 */
 		CALL_HOOK(NS_QUERY_RESPOND_ANY_FOUND, qctx);
-
-		if (qctx->fname != NULL) {
-			dns_message_puttempname(qctx->client->message,
-						&qctx->fname);
-		}
-
-		query_addauth(qctx);
-		return (ns_query_done(qctx));
 	}
 
-	/*
-	 * If we're here, no matching rdatasets were found.  If we were
-	 * searching for RRSIG/SIG, that may be okay, but otherwise
-	 * something's gone wrong.
-	 */
-	INSIST(qctx->qtype == dns_rdatatype_rrsig ||
-	       qctx->qtype == dns_rdatatype_sig);
-
 	if (qctx->fname != NULL) {
-		dns_message_puttempname(qctx->client->message,
+		dns_message_puttempname(qctx->client->message, &qctx->fname);
-					&qctx->fname);
 	}
 
-	if (!qctx->is_zone) {
+	if (found) {
-		qctx->authoritative = false;
+		/*
-		qctx->client->attributes &= ~NS_CLIENTATTR_RA;
+		 * At least one matching rdataset was found
+		 */
 		query_addauth(qctx);
-		return (ns_query_done(qctx));
+	} else if (qctx->qtype == dns_rdatatype_rrsig ||
+		   qctx->qtype == dns_rdatatype_sig)
+	{
+		/*
+		 * No matching rdatasets were found, but we got
+		 * here on a search for RRSIG/SIG, so that's okay.
+		 */
+		if (!qctx->is_zone) {
+			qctx->authoritative = false;
+			qctx->client->attributes &= ~NS_CLIENTATTR_RA;
+			query_addauth(qctx);
+			return (ns_query_done(qctx));
+		}
+
+		if (qctx->qtype == dns_rdatatype_rrsig &&
+		    dns_db_issecure(qctx->db))
+		{
+			char namebuf[DNS_NAME_FORMATSIZE];
+			dns_name_format(qctx->client->query.qname,
+					namebuf, sizeof(namebuf));
+			ns_client_log(qctx->client, DNS_LOGCATEGORY_DNSSEC,
+				      NS_LOGMODULE_QUERY, ISC_LOG_WARNING,
+				      "missing signature for %s", namebuf);
+		}
+
+		qctx->fname = ns_client_newname(qctx->client, qctx->dbuf, &b);
+		return (query_sign_nodata(qctx));
+	} else if (!hidden) {
+		/*
+		 * No matching rdatasets were found and nothing was
+		 * deliberately hidden: something must have gone wrong.
+		 */
+		QUERY_ERROR(qctx, DNS_R_SERVFAIL);
 	}
 
-	if (qctx->qtype == dns_rdatatype_rrsig && dns_db_issecure(qctx->db)) {
+	return (ns_query_done(qctx));
-		char namebuf[DNS_NAME_FORMATSIZE];
-		dns_name_format(qctx->client->query.qname,
-				namebuf, sizeof(namebuf));
-		ns_client_log(qctx->client, DNS_LOGCATEGORY_DNSSEC,
-			      NS_LOGMODULE_QUERY, ISC_LOG_WARNING,
-			      "missing signature for %s", namebuf);
-	}
-
-	qctx->fname = ns_client_newname(qctx->client, qctx->dbuf, &b);
-	return (query_sign_nodata(qctx));
 
  cleanup:
 	return (result);