From: Evan Hunt <each@isc.org>
Date: Fri, 1 May 2026 19:15:07 +0000 (-0700)
Subject: Check for secure data before caching CD=1 NXDOMAIN
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=a7a90eb9d80aff1be0e975e05f9c92af30e7f537;p=thirdparty%2Fbind9.git

Check for secure data before caching CD=1 NXDOMAIN

An unvalidated NXDOMAIN (e.g. from a CD=1 query) marked every RRset at
the name ancient without checking trust, evicting DNSSEC-validated data.
Keep the cache unchanged when any existing RRset is already secure.

dns_ncache_add() now returns DNS_R_UNCHANGED for the rejected add;
negcache() serves a matching cached negative or the queried type, else
SERVFAIL (never the unrelated RRset the add bound), and rctx_ncache()
forwards it so the fetch fails fast.
---

diff --git a/lib/dns/ncache.c b/lib/dns/ncache.c
index a1670e17e0c..942df43a7f2 100644
--- a/lib/dns/ncache.c
+++ b/lib/dns/ncache.c
@@ -105,7 +105,6 @@ dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 	dns_rdatalist_t ncrdatalist;
 	unsigned char data[65536];
 	unsigned int next = 0;
-	isc_result_t result;
 
 	/*
 	 * Convert the authority data from 'message' into a negative cache
@@ -236,13 +235,8 @@ dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 		ncrdataset.attributes.optout = true;
 	}
 
-	result = dns_db_addrdataset(cache, node, NULL, now, &ncrdataset, 0,
-				    addedrdataset);
-	if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) {
-		return result;
-	}
-
-	return ISC_R_SUCCESS;
+	return dns_db_addrdataset(cache, node, NULL, now, &ncrdataset, 0,
+				  addedrdataset);
 }
 
 isc_result_t
diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 210210747f6..2ced865b833 100644
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -2298,6 +2298,26 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 		trust = newheader->trust;
 	}
 
+	/*
+	 * An unvalidated negative entry covering all types (NXDOMAIN or
+	 * NODATA(QTYPE=ANY)) must not purge secure data. Check for it in a
+	 * separate pass first: evicting as we go and bailing out later would
+	 * destroy lower-trust siblings before we found the secure header.
+	 */
+	if (EXISTS(newheader) && NEGATIVE(newheader) &&
+	    rdtype == dns_rdatatype_any && trust < dns_trust_secure)
+	{
+		DNS_SLABHEADER_FOREACH(header, &qpnode->headers) {
+			if (header->trust >= dns_trust_secure) {
+				qpcache_hit(qpdb, header);
+				bindrdataset(qpdb, qpnode, header, now,
+					     nlocktype, tlocktype,
+					     addedrdataset DNS__DB_FLARG_PASS);
+				return DNS_R_UNCHANGED;
+			}
+		}
+	}
+
 	DNS_SLABHEADER_FOREACH(header, &qpnode->headers) {
 		if (EXISTS(newheader) && NEGATIVE(newheader)) {
 			if (rdtype == dns_rdatatype_any) {
diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index d8ca636d67e..a094eeb430b 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -1082,7 +1082,7 @@ rctx_dispfail(respctx_t *rctx);
 static isc_result_t
 rctx_timedout(respctx_t *rctx);
 
-static void
+static isc_result_t
 rctx_ncache(respctx_t *rctx);
 
 /*%
@@ -6528,10 +6528,34 @@ negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name,
 	result = dns_ncache_add(message, cache, node, covers, now, minttl,
 				maxttl, optout, secure, added);
 
+	/*
+	 * We failed to add the negative cache entry and some rdataset was
+	 * returned.  If we were adding dns_rdatatype_any, this could be of a
+	 * different type than was originally requested.
+	 */
+	if (result == DNS_R_UNCHANGED) {
+		/*
+		 * We got the same negative type that we were adding, everything
+		 * is fine, continue.
+		 */
+		if (NEGATIVE(added) && added->covers == covers) {
+			result = ISC_R_SUCCESS;
+		} else {
+			dns_rdataset_disassociate(added);
+			dns_db_detachnode(&node);
+			return DNS_R_SERVFAIL;
+		}
+	}
+
 	if (added == &rdataset) {
 		dns_rdataset_cleanup(added);
 	}
 
+	if (result != ISC_R_SUCCESS) {
+		dns_db_detachnode(&node);
+		return result;
+	}
+
 	*nodep = node;
 	return result;
 }
@@ -6541,7 +6565,7 @@ negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name,
  * Cache the negatively cacheable parts of the message.  This may
  * also cause work to be queued to the DNSSEC validator.
  */
-static void
+static isc_result_t
 rctx_ncache(respctx_t *rctx) {
 	isc_result_t result = ISC_R_SUCCESS;
 	fetchctx_t *fctx = rctx->fctx;
@@ -6627,6 +6651,7 @@ done:
 	if (result != ISC_R_SUCCESS) {
 		FCTXTRACE3("rctx_ncache complete", result);
 	}
+	return result;
 }
 
 static void
@@ -8101,7 +8126,10 @@ resquery_response_continue(void *arg, isc_result_t result) {
 	/*
 	 * Negative caching
 	 */
-	rctx_ncache(rctx);
+	isc_result_t nresult = rctx_ncache(rctx);
+	if (nresult != ISC_R_SUCCESS) {
+		result = nresult;
+	}
 
 	FCTXTRACE("resquery_response done");
 	rctx_done(rctx, result);