Drop in-domain NS without glue from the delegation set

Pull the dns_message_findname() lookups into cache_delegglue() and
cache_delegglue6() so each helper now owns its glue lookup and returns
the number of addresses cached.  cache_delegns() splits referrals into
two cases: in-domain (the NS name is below the delegation point) and
sibling/in-bailiwick.

An in-domain NS without glue is unresolvable by definition - the
resolver would have to ask the very server it's trying to find.  Log
"missing mandatory glue" at notice level and skip the deleg entirely
rather than leaving an unusable entry in the set.  A new
dns_delegset_freedeleg() undoes a fresh dns_delegset_allocdeleg() so
the rest of the delegation set is preserved.

bin/tests/system/expiredglue/tests_expiredglue.py

@@ -45,11 +45,12 @@ def test_expiredglue(ns4):
     isctest.check.same_data(res3_2, res3)
 
 
-def test_loopdetected(ns4):
+def test_missing_mandatory_glue(ns4):
     msg = isctest.query.create("a.missing.tld.", "A")
     with ns4.watch_log_from_here() as watcher:
         res = isctest.query.udp(msg, ns4.ip)
 
-        # However, this is a valid fetch loop, and named detects it.
-        watcher.wait_for_line("loop detected resolving 'ns.missing.tld/A'")
+        # The NS for missing.tld. is in-domain and has no glue, so
+        # named drops the delegation rather than chasing it.
+        watcher.wait_for_line("missing mandatory glue for ns.missing.tld")
         isctest.check.servfail(res)

lib/dns/deleg.c

@@ -422,6 +422,21 @@ dns_delegset_allocdeleg(dns_delegset_t *delegset, dns_deleg_type_t type,
 	*delegp = deleg;
 }
 
+void
+dns_delegset_freedeleg(dns_delegset_t *delegset, dns_deleg_t **delegp) {
+	REQUIRE(DNS_DELEGSET_VALID(delegset));
+	REQUIRE(delegp != NULL && *delegp != NULL);
+	REQUIRE(ISC_LIST_EMPTY((*delegp)->addresses));
+	REQUIRE(ISC_LIST_EMPTY((*delegp)->names));
+
+	dns_deleg_t *deleg = *delegp;
+	*delegp = NULL;
+
+	ISC_LIST_UNLINK(delegset->delegs, deleg, link);
+
+	isc_mem_put(delegset->mctx, deleg, sizeof(*deleg));
+}
+
 void
 dns_delegset_addaddr(dns_delegset_t *delegset, dns_deleg_t *deleg,
 		     const isc_netaddr_t *addr) {

lib/dns/include/dns/deleg.h

@@ -151,6 +151,12 @@ dns_delegset_allocset(dns_delegdb_t *db, dns_delegset_t **delegsetp);
 void
 dns_delegset_allocdeleg(dns_delegset_t *delegset, dns_deleg_type_t type,
 			dns_deleg_t **delegp);
+/*
+ * Free the deleg struct and remove it from the delegation set. Can't
+ * be used on delegation set already attached in the DB.
+ */
+void
+dns_delegset_freedeleg(dns_delegset_t *delegset, dns_deleg_t **delegp);
 
 /*
  * Add a new IP into a delegation. Can't be used on a delegation from a

lib/dns/resolver.c

@@ -6640,10 +6640,19 @@ name_external(const dns_name_t *name, dns_rdatatype_t type, respctx_t *rctx) {
 	return false;
 }
 
-static void
+static size_t
 cache_delegglue(dns_delegset_t *delegset, dns_deleg_t *deleg, dns_ttl_t *ttl,
-		dns_rdataset_t *rdataset) {
+		respctx_t *rctx, const dns_name_t *nsname) {
+	dns_rdataset_t *rdataset = NULL;
 	size_t naddrs = 0;
+	isc_result_t result;
+
+	result = dns_message_findname(rctx->query->rmessage,
+				      DNS_SECTION_ADDITIONAL, nsname,
+				      dns_rdatatype_a, 0, NULL, &rdataset);
+	if (result != ISC_R_SUCCESS) {
+		return 0;
+	}
 
 	if (rdataset->ttl < *ttl) {
 		*ttl = rdataset->ttl;
@@ -6664,12 +6673,22 @@ cache_delegglue(dns_delegset_t *delegset, dns_deleg_t *deleg, dns_ttl_t *ttl,
 			break;
 		}
 	}
+	return naddrs;
 }
 
-static void
+static size_t
 cache_delegglue6(dns_delegset_t *delegset, dns_deleg_t *deleg, dns_ttl_t *ttl,
-		 dns_rdataset_t *rdataset) {
+		 respctx_t *rctx, const dns_name_t *nsname) {
+	dns_rdataset_t *rdataset = NULL;
 	size_t naddrs = 0;
+	isc_result_t result;
+
+	result = dns_message_findname(rctx->query->rmessage,
+				      DNS_SECTION_ADDITIONAL, nsname,
+				      dns_rdatatype_aaaa, 0, NULL, &rdataset);
+	if (result != ISC_R_SUCCESS) {
+		return 0;
+	}
 
 	if (rdataset->ttl < *ttl) {
 		*ttl = rdataset->ttl;
@@ -6690,6 +6709,7 @@ cache_delegglue6(dns_delegset_t *delegset, dns_deleg_t *deleg, dns_ttl_t *ttl,
 			break;
 		}
 	}
+	return naddrs;
 }
 
 /*
@@ -6713,6 +6733,8 @@ cache_delegns(respctx_t *rctx) {
 	dns_fixedname_t fparent;
 	dns_name_t *parent = dns_fixedname_initname(&fparent);
 	size_t labels;
+	size_t ns_count = 0;
+	size_t max_servers = fctx->res->view->max_delegation_servers;
 	isc_result_t result;
 
 	FCTXTRACE("cache_delegns");
@@ -6730,14 +6752,11 @@ cache_delegns(respctx_t *rctx) {
 		dns_name_getlabelsequence(rctx->ns_name, 1, labels - 1, parent);
 	}
 
-	size_t ns_count = 0;
-	size_t max_servers = fctx->res->view->max_delegation_servers;
-
 	DNS_RDATASET_FOREACH(rctx->ns_rdataset) {
-		dns_rdataset_t *gluerdataset = NULL;
 		dns_rdata_t rdata = DNS_RDATA_INIT;
 		dns_rdata_ns_t ns;
 		dns_deleg_t *deleg = NULL;
+		size_t naddrs = 0;
 
 		if (ns_count >= max_servers) {
 			break;
@@ -6758,32 +6777,41 @@ cache_delegns(respctx_t *rctx) {
 		INSIST(rdata.type == dns_rdatatype_ns);
 		dns_rdata_tostruct(&rdata, &ns, NULL);
 
-		if (labels > 1 && dns_name_issubdomain(&ns.name, parent)) {
-			result = dns_message_findname(rctx->query->rmessage,
-						      DNS_SECTION_ADDITIONAL,
-						      &ns.name, dns_rdatatype_a,
-						      0, NULL, &gluerdataset);
-			if (result == ISC_R_SUCCESS) {
-				cache_delegglue(delegset, deleg, &ttl,
-						gluerdataset);
-				gluerdataset = NULL;
-			}
+		/* in-domain GLUE */
+		if (dns_name_issubdomain(&ns.name, rctx->ns_name)) {
+			naddrs += cache_delegglue(delegset, deleg, &ttl, rctx,
+						  &ns.name);
+			naddrs += cache_delegglue6(delegset, deleg, &ttl, rctx,
+						   &ns.name);
+			if (naddrs == 0) {
+				INSIST(ISC_LIST_EMPTY(deleg->addresses));
+				char namebuf[DNS_NAME_FORMATSIZE];
+				dns_name_format(&ns.name, namebuf,
+						sizeof(namebuf));
 
-			result = dns_message_findname(
-				rctx->query->rmessage, DNS_SECTION_ADDITIONAL,
-				&ns.name, dns_rdatatype_aaaa, 0, NULL,
-				&gluerdataset);
-			if (result == ISC_R_SUCCESS) {
-				cache_delegglue6(delegset, deleg, &ttl,
-						 gluerdataset);
-				gluerdataset = NULL;
+				isc_log_write(DNS_LOGCATEGORY_RESOLVER,
+					      DNS_LOGMODULE_RESOLVER,
+					      ISC_LOG_NOTICE,
+					      "missing mandatory glue for %s",
+					      namebuf);
+				dns_delegset_freedeleg(delegset, &deleg);
 			}
+			continue;
 		}
 
-		if (ISC_LIST_EMPTY(deleg->addresses)) {
+		/* in-bailiwick/sibling GLUE */
+		if (labels > 1 && dns_name_issubdomain(&ns.name, parent)) {
+			naddrs += cache_delegglue(delegset, deleg, &ttl, rctx,
+						  &ns.name);
+			naddrs += cache_delegglue6(delegset, deleg, &ttl, rctx,
+						   &ns.name);
+		}
+
+		if (naddrs == 0) {
+			INSIST(ISC_LIST_EMPTY(deleg->addresses));
 			/*
-			 * There is actually no glues for this NSRRset, so this
-			 * is actually a DNS_DELEGTYPE_NS_NAMES.
+			 * There are actually no glues for this NSRRset,
+			 * so this is actually a DNS_DELEGTYPE_NS_NAMES.
 			 */
 			deleg->type = DNS_DELEGTYPE_NS_NAMES;
 			dns_delegset_addns(delegset, deleg, &ns.name);