fix: usr: The resolver now removes other RRsets at the same name when caching a CNAME

When an RRset is in stale cache, and the authoritative server changes the record type to CNAME, the resolver fails to refresh the stale cache. This has been fixed.

Closes #5302

Merge branch '5302-serve-stale-cname-to-a' into 'main'

See merge request isc-projects/bind9!11758

bin/tests/system/serve_stale/ns1/named.conf.j2

@@ -42,5 +42,6 @@ zone "." {
 zone "stale.test" {
 	type primary;
 	file "stale.test.db";
+	allow-update { any; };
 };
 {% endif %}

bin/tests/system/serve_stale/ns1/named4.conf.in

@@ -34,4 +34,5 @@ zone "." {
 zone "stale.test" {
 	type primary;
 	file "stale.test.db";
+	allow-update { any; };
 };

bin/tests/system/serve_stale/ns3/named.conf.j2

@@ -18,6 +18,7 @@ options {
 	recursion yes;
 	dnssec-validation no;
 	qname-minimization off;
+	prefetch 0;
 
 	stale-answer-enable yes;
 	stale-cache-enable yes;

bin/tests/system/serve_stale/ns3/named1.conf.j2

@@ -19,6 +19,7 @@ options {
 	dump-file "named_dump3.db";
 	stale-cache-enable yes;
 	dnssec-validation no;
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/ns3/named2.conf.j2

@@ -29,6 +29,7 @@ options {
 	max-stale-ttl 3600;
 	resolver-query-timeout 30000; # 30 seconds
 	qname-minimization disabled;
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/ns3/named3.conf.j2

@@ -27,6 +27,7 @@ options {
 	stale-refresh-time 0;
 	max-stale-ttl 3600;
 	resolver-query-timeout 10000; # 10 seconds
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/ns3/named4.conf.j2

@@ -29,6 +29,7 @@ options {
 	resolver-query-timeout 10000; # 10 seconds
 	max-stale-ttl 3600;
 	recursive-clients 10;
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/ns3/named5.conf.j2

@@ -28,6 +28,7 @@ options {
 	stale-refresh-time 4;
 	resolver-query-timeout 10000; # 10 seconds
 	max-stale-ttl 3600;
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/ns3/named6.conf.j2

@@ -25,6 +25,7 @@ options {
 	fetches-per-zone 1 fail;
 	fetches-per-server 1 fail;
 	max-stale-ttl 3600;
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/ns3/named7.conf.j2

@@ -33,6 +33,7 @@ options {
 	fetches-per-zone 1 fail;
 	fetches-per-server 1 fail;
 	max-stale-ttl 3600;
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/ns3/named9.conf.j2

@@ -21,6 +21,7 @@ options {
 	stale-cache-enable yes;
 	stale-answer-ttl 3;
 	stale-answer-client-timeout 0;
+	prefetch 0;
 };
 
 zone "." {

bin/tests/system/serve_stale/tests.sh

@@ -2038,17 +2038,6 @@ if [ $ret != 0 ]; then
 fi
 status=$((status + ret))
 
-n=$((n + 1))
-echo_i "check server is alive or restart ($n)"
-ret=0
-$RNDCCMD 10.53.0.3 status >rndc.out.test$n 2>&1 || ret=1
-if [ $ret != 0 ]; then
-  echo_i "failed"
-  echo_i "restart ns3"
-  start_server --noclean --restart --port ${PORT} serve-stale ns3
-fi
-status=$((status + ret))
-
 #############################################
 # Test for stale-answer-client-timeout 0.   #
 #############################################
@@ -2370,6 +2359,89 @@ grep "cname-b3\.stale\.test\..*3.*IN.*A.*192\.0\.2\.2" dig.out.test$n >/dev/null
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
+# Change CNAME to A.
+n=$((n + 1))
+echo_i "change cname2.stale.test. CNAME to A (stale-answer-client-timeout 0) ($n)"
+ret=0
+(
+  echo zone stale.test.
+  echo server 10.53.0.1 "${PORT}"
+  echo update del cname2.stale.test. CNAME a2.stale.test.
+  echo update add cname2.stale.test. 1 A 192.0.0.2
+  echo send
+) | $NSUPDATE || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
+n=$((n + 1))
+ret=0
+echo_i "check stale cname2.stale.test A comes from cache (stale-answer-client-timeout 0) ($n)"
+nextpart ns3/named.run >/dev/null
+$DIG -p ${PORT} @10.53.0.3 cname2.stale.test A >dig.out.test$n || ret=1
+wait_for_log 5 "cname2.stale.test A stale answer used, an attempt to refresh the RRset" ns3/named.run || ret=1
+grep "status: NOERROR" dig.out.test$n >/dev/null || ret=1
+grep "EDE: 3 (Stale Answer): (stale data prioritized over lookup)" dig.out.test$n >/dev/null || ret=1
+grep "ANSWER: 2," dig.out.test$n >/dev/null || ret=1
+grep "cname2\.stale\.test\..*3.*IN.*CNAME.*a2\.stale\.test\." dig.out.test$n >/dev/null || ret=1
+# We can't reliably test the TTL of the a2.stale.test A record.
+grep "a2\.stale\.test\..*IN.*A.*192\.0\.2\.2" dig.out.test$n >/dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
+n=$((n + 1))
+ret=0
+echo_i "check stale cname2.stale.test A is refreshed (stale-answer-client-timeout 0) ($n)"
+nextpart ns3/named.run >/dev/null
+$DIG -p ${PORT} @10.53.0.3 cname2.stale.test A >dig.out.test$n || ret=1
+grep "status: NOERROR" dig.out.test$n >/dev/null || ret=1
+grep "ANSWER: 1," dig.out.test$n >/dev/null || ret=1
+grep "cname2\.stale\.test\..*IN.*A.*192\.0\.0\.2" dig.out.test$n >/dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
+# Change A to CNAME.
+n=$((n + 1))
+echo_i "change cname2.stale.test. A to CNAME (stale-answer-client-timeout 0) ($n)"
+ret=0
+(
+  echo zone stale.test.
+  echo server 10.53.0.1 "${PORT}"
+  echo update del cname2.stale.test. A 192.0.0.2
+  echo update add cname2.stale.test. 1 CNAME a2.stale.test.
+  echo send
+) | $NSUPDATE || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
+# Allow A record to become stale.
+sleep 1
+
+n=$((n + 1))
+ret=0
+echo_i "check stale cname2.stale.test A comes from cache (stale-answer-client-timeout 0) ($n)"
+nextpart ns3/named.run >/dev/null
+$DIG -p ${PORT} @10.53.0.3 cname2.stale.test A >dig.out.test$n || ret=1
+wait_for_log 5 "cname2.stale.test A stale answer used, an attempt to refresh the RRset" ns3/named.run || ret=1
+grep "status: NOERROR" dig.out.test$n >/dev/null || ret=1
+grep "EDE: 3 (Stale Answer): (stale data prioritized over lookup)" dig.out.test$n >/dev/null || ret=1
+grep "ANSWER: 1," dig.out.test$n >/dev/null || ret=1
+grep "cname2\.stale\.test\..*3.*IN.*A.*192\.0\.0\.2" dig.out.test$n >/dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
+n=$((n + 1))
+ret=0
+echo_i "check stale cname2.stale.test A is refreshed (stale-answer-client-timeout 0) ($n)"
+nextpart ns3/named.run >/dev/null
+$DIG -p ${PORT} @10.53.0.3 cname2.stale.test A >dig.out.test$n || ret=1
+grep "status: NOERROR" dig.out.test$n >/dev/null || ret=1
+grep "ANSWER: 2," dig.out.test$n >/dev/null || ret=1
+# We can't reliably test the TTL of the these records.
+grep "cname2\.stale\.test\..*IN.*CNAME.*a2\.stale\.test\." dig.out.test$n >/dev/null || ret=1
+grep "a2\.stale\.test\..*IN.*A.*192\.0\.2\.2" dig.out.test$n >/dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
 ####################################################################
 # Test for stale-answer-client-timeout 0 and stale-refresh-time 4. #
 ####################################################################

bin/tests/system/serve_stale/tests_sh_serve_stale.py

@@ -22,6 +22,7 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns*/named_dump*",
         "ns*/named.stats*",
         "ns*/root.bk",
+        "ns1/stale.test.db.jnl",
     ]
 )
 

lib/dns/resolver.c

@@ -69,6 +69,7 @@
 #include <dns/rdataclass.h>
 #include <dns/rdatalist.h>
 #include <dns/rdataset.h>
+#include <dns/rdatasetiter.h>
 #include <dns/rdatastruct.h>
 #include <dns/rdatatype.h>
 #include <dns/resolver.h>
@@ -5521,6 +5522,99 @@ getrrsig(dns_name_t *name, dns_rdatatype_t type) {
 	return NULL;
 }
 
+static void
+delete_rrset(fetchctx_t *fctx, dns_name_t *name, dns_rdatatype_t type) {
+	isc_result_t result;
+	dns_dbnode_t *node = NULL;
+
+	result = dns_db_findnode(fctx->cache, name, false, &node);
+	if (result != ISC_R_SUCCESS) {
+		return;
+	}
+
+	dns_db_deleterdataset(fctx->cache, node, NULL, type, 0);
+	dns_db_deleterdataset(fctx->cache, node, NULL, dns_rdatatype_rrsig,
+			      type);
+	dns_db_detachnode(&node);
+}
+
+/*
+ * When caching a CNAME, evict other RRsets at the same owner name,
+ * according to the RFC specifications.
+ *
+ * RFC 1034, 3.6.2: Aliases and canonical names
+ *   If a CNAME RR is present at a node, no other data should be
+ *   present.
+ * RFC 2181, 10.1: CNAME resource records
+ *   An alias name (label of a CNAME record) may,
+ *   if DNSSEC is in use, have SIG, NXT, and KEY RRs, but may have no
+ *   other data.
+ * RFC 2535, 2.3.5: Special Considerations with CNAME
+ * RFC 4034, 3: The RRSIG Resource Record
+ *   Because every authoritative RRset in a zone must be protected by a
+ *   digital signature, RRSIG RRs must be present for names containing a
+ *   CNAME RR.  This is a change to the traditional DNS specification
+ *   [RFC1034], which stated that if a CNAME is present for a name, it is
+ *   the only type allowed at that name.
+ * RFC 4034, 4: The NSEC Resource Record
+ *   Because every authoritative name in a zone must be part of the NSEC
+ *   chain, NSEC RRs must be present for names containing a CNAME RR.
+ *   This is a change to the traditional DNS specification [RFC1034],
+ *   which stated that if a CNAME is present for a name, it is the only
+ *   type allowed at that name.
+ *
+ * So types allowed next to CNAME are: KEY, SIG, NXT, RRSIG, and NSEC.
+ */
+static void
+evict_cname_other(fetchctx_t *fctx, dns_name_t *name) {
+	isc_result_t result;
+	dns_dbnode_t *node = NULL;
+	dns_rdatasetiter_t *rdsiter = NULL;
+
+	result = dns_db_findnode(fctx->cache, name, false, &node);
+	if (result != ISC_R_SUCCESS) {
+		return;
+	}
+
+	result = dns_db_allrdatasets(fctx->cache, node, NULL, 0, 0, &rdsiter);
+	if (result != ISC_R_SUCCESS) {
+		dns_db_detachnode(&node);
+		return;
+	}
+
+	DNS_RDATASETITER_FOREACH(rdsiter) {
+		dns_rdataset_t rdataset = DNS_RDATASET_INIT;
+		dns_rdatasetiter_current(rdsiter, &rdataset);
+		if (rdataset.type == dns_rdatatype_nsec ||
+		    rdataset.type == dns_rdatatype_nxt ||
+		    rdataset.type == dns_rdatatype_key)
+		{
+			/* KEY, NSEC and NXT records are allowed */
+			dns_rdataset_disassociate(&rdataset);
+			continue;
+		}
+		if (dns_rdatatype_issig(rdataset.type)) {
+			/* Signatures will be deleted together below */
+			dns_rdataset_disassociate(&rdataset);
+			continue;
+		}
+		if (rdataset.type == dns_rdatatype_none) {
+			/* Negative type. */
+			dns_rdataset_disassociate(&rdataset);
+			continue;
+		}
+
+		dns_db_deleterdataset(fctx->cache, node, NULL, rdataset.type,
+				      0);
+		dns_db_deleterdataset(fctx->cache, node, NULL,
+				      dns_rdatatype_rrsig, rdataset.type);
+		dns_rdataset_disassociate(&rdataset);
+	}
+
+	dns_rdatasetiter_destroy(&rdsiter);
+	dns_db_detachnode(&node);
+}
+
 static isc_result_t
 cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name,
 	    dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
@@ -5575,6 +5669,21 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name,
 		result = dns_db_findnode(fctx->cache, name, true, &node);
 	}
 
+	/*
+	 * Evict CNAME records, according to the RFC rules (see
+	 * evict_cname_other).
+	 *
+	 * Note that a signature is tied to the type it covers and is deleted
+	 * along with the covered RRset in 'delete_rrset()'.
+	 */
+	if (!dns_rdataset_matchestype(rdataset, dns_rdatatype_cname) &&
+	    !dns_rdataset_matchestype(rdataset, dns_rdatatype_key) &&
+	    !dns_rdataset_matchestype(rdataset, dns_rdatatype_nsec) &&
+	    !dns_rdataset_matchestype(rdataset, dns_rdatatype_nxt))
+	{
+		delete_rrset(fctx, name, dns_rdatatype_cname);
+	}
+
 	if (result == ISC_R_SUCCESS) {
 		result = dns_db_addrdataset(fctx->cache, node, NULL, now,
 					    rdataset, options | equalok, added);
@@ -5611,22 +5720,6 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name,
 	return result;
 }
 
-static void
-delete_rrset(fetchctx_t *fctx, dns_name_t *name, dns_rdatatype_t type) {
-	isc_result_t result;
-	dns_dbnode_t *node = NULL;
-
-	result = dns_db_findnode(fctx->cache, name, false, &node);
-	if (result != ISC_R_SUCCESS) {
-		return;
-	}
-
-	dns_db_deleterdataset(fctx->cache, node, NULL, type, 0);
-	dns_db_deleterdataset(fctx->cache, node, NULL, dns_rdatatype_rrsig,
-			      type);
-	dns_db_detachnode(&node);
-}
-
 static void
 fctx_cacheauthority(fetchctx_t *fctx, dns_message_t *message,
 		    isc_stdtime_t now) {
@@ -6271,6 +6364,14 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) {
 			goto cleanup;
 		}
 
+		/*
+		 * If CNAME, delete other RRsets at the same name
+		 * from the cache.
+		 */
+		if (rdataset->type == dns_rdatatype_cname) {
+			evict_cname_other(fctx, name);
+		}
+
 		/* Find the signature for this rdataset */
 		sigrdataset = getrrsig(name, rdataset->type);