chg: dev: Refactor zone fetch code

There is code duplication between `keyfetch` and `nsfetch`, refactor to allow common code paths to differentiate between them. This is in preparation for support of generalized DNS notifications, that will require fetching DSYNC records.

Merge branch 'matthijs-refactor-zone-fetch' into 'main'

See merge request isc-projects/bind9!11176

lib/dns/include/dns/zonefetch.h

@@ -0,0 +1,151 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+/*! \file dns/zonefetch.h */
+
+#include <isc/mem.h>
+#include <isc/result.h>
+
+#include <dns/db.h>
+#include <dns/name.h>
+#include <dns/rdataset.h>
+#include <dns/zone.h>
+
+/*%
+ * Fetch type; various features can initiate fetching and this enum value
+ * allows common code paths to differentiate between them.
+ *
+ * ZONEFETCHTYPE_COUNT is not actually a zonefetch type and needs to be in
+ * the last position of the enum.
+ */
+typedef enum {
+	ZONEFETCHTYPE_KEY,
+	ZONEFETCHTYPE_NS,
+	ZONEFETCHTYPE_COUNT,
+} dns_zonefetch_type_t;
+
+typedef struct dns_keyfetch  dns_keyfetch_t;
+typedef struct dns_nsfetch   dns_nsfetch_t;
+typedef struct dns_zonefetch dns_zonefetch_t;
+
+/*
+ * Fetch methods.
+ */
+typedef struct dns_zonefetch_methods {
+	void (*start_fetch)(dns_zonefetch_t *fetch);
+	void (*continue_fetch)(dns_zonefetch_t *fetch);
+	void (*cancel_fetch)(dns_zonefetch_t *fetch);
+	void (*cleanup_fetch)(dns_zonefetch_t *fetch);
+	isc_result_t (*done_fetch)(dns_zonefetch_t *fetch,
+				   isc_result_t	    eresult);
+} dns_zonefetch_methods_t;
+
+/*
+ * Fetch contexts.
+ */
+struct dns_keyfetch {
+	dns_rdataset_t keydataset;
+	dns_db_t      *db;
+};
+
+struct dns_nsfetch {
+	dns_name_t pname;
+};
+
+typedef union dns_fetchdata {
+	dns_keyfetch_t keyfetch;
+	dns_nsfetch_t  nsfetch;
+} dns_zonefetch_data_t;
+
+struct dns_zonefetch {
+	/* Fetch context */
+	dns_fetch_t    *fetch;
+	isc_mem_t      *mctx;
+	dns_zone_t     *zone;
+	dns_fixedname_t name;
+
+	/* Query */
+	dns_name_t     *qname;
+	dns_rdatatype_t qtype;
+	unsigned int	options;
+
+	/* Response */
+	dns_rdataset_t rrset;
+	dns_rdataset_t sigset;
+
+	/* Type specific */
+	dns_zonefetch_type_t	fetchtype;
+	dns_zonefetch_data_t	fetchdata;
+	dns_zonefetch_methods_t fetchmethods;
+};
+
+void
+dns_zonefetch_run(void *arg);
+/*%<
+ *      Start a zone fetch. This starts a query for a given qname and qtype, and
+ *	recurses to answer a question. The type of fetch depends on the
+ *	fetchtype.
+ */
+
+void
+dns_zonefetch_done(void *arg);
+/*%<
+ *	Complete a zone fetch. This may trigger follow-up actions that depend on
+ *	the fetch type.
+ */
+
+void
+dns_zonefetch_schedule(dns_zonefetch_t *fetch, dns_name_t *name);
+/*%<
+ *	Schedule a zone fetch, starting at 'name'.  Initializes the rdata sets,
+ *	and sets the starting name to 'name'. Note that the query type is
+ *	determined by the type of zone fetch. This function also increments the
+ *	corresponding zone's ireferences (to be decremented in
+ *	dns_zonefetch_done()).
+ *
+ *	Requires:
+ *		'fetch' is not NULL.
+ *		'name' is not NULL.
+ */
+
+void
+dns_zonefetch_reschedule(dns_zonefetch_t *fetch);
+/*%<
+ *	Reschedule a zone fetch.  Initializes the rdata sets and increments the
+ *	corresponding zone's ireferences (to be decremented in
+ *	dns_zonefetch_done()).
+ *
+ *	Requires:
+ *		'fetch' is not NULL.
+ *		'name' is not NULL.
+ */
+
+isc_result_t
+dns_zonefetch_verify(dns_zonefetch_t *fetch, isc_result_t eresult,
+		     dns_trust_t trust);
+/*%<
+ *      Check a completed zone fetch. This checks the response result,
+ *	if there are records and signatures available, and the	level of trust.
+ *
+ *	Requires:
+ *		'fetch' is not NULL.
+ *
+ *	Returns:
+ *		ISC_R_SUCCESS    - if the completed zone fetch is verified.
+ *		ISC_R_NOTFOUND	 - if no records are found.
+ *		DNS_R_NOVALIDSIG - if no signatures are available, or the trust
+ *				   level is below 'trust'.
+ *	 	eresult		 - error code in case the fetch failed.
+ */

lib/dns/meson.build

@@ -165,6 +165,7 @@ dns_srcset.add(
         'validator.c',
         'view.c',
         'zone.c',
+        'zonefetch.c',
         'zoneverify.c',
         'zt.c',
     ),

lib/dns/zone_p.h

@@ -18,8 +18,8 @@
 /*! \file */
 
 /*%
- *     Types and functions below not be used outside this module and its
- *     associated unit tests.
+ *	Types and functions below meant to be used for internal zone
+ *	modules only, and associated unit tests.
  */
 
 #define UDP_REQUEST_TIMEOUT 5 /*%< 5 seconds */
@@ -185,3 +185,31 @@ dns__zone_idetach_locked(dns_zone_t **zonep);
  *\li   The caller is running in the context of the zone's loop.
  *\li   'zonep' to point to a valid zone, already locked.
  */
+
+isc_refcount_t *
+dns__zone_irefs(dns_zone_t *zone);
+/*%<
+ *	Get the reference count of a zone.
+ *
+ *	Requires:
+ *	\li 'zone' to be a valid zone.
+ */
+
+void
+dns__zone_free(dns_zone_t *zone);
+/*
+ *	Free a zone.  Because we require that there be no more
+ *	outstanding events or references, no locking is necessary.
+ *
+ *	Requires:
+ *	\li 'zone' to be a valid zone, unlocked.
+ */
+
+bool
+dns__zone_free_check(dns_zone_t *zone);
+/*
+ *	Check if a zone is ready to be freed.
+ *
+ *	Requires:
+ *	\li 'zone' to be a valid zone, locked.
+ */

lib/dns/zonefetch.c

@@ -0,0 +1,272 @@
+/*
+ * 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.
+ */
+
+/*! \file */
+
+#include <isc/async.h>
+#include <isc/loop.h>
+
+#include <dns/resolver.h>
+#include <dns/view.h>
+#include <dns/zone.h>
+#include <dns/zonefetch.h>
+
+#include "zone_p.h"
+
+void
+dns_zonefetch_run(void *arg) {
+	dns_zonefetch_t *fetch = (dns_zonefetch_t *)arg;
+	dns_zone_t *zone;
+	dns_view_t *view;
+	isc_loop_t *loop;
+	isc_result_t result;
+	dns_resolver_t *resolver = NULL;
+
+	zone = fetch->zone;
+	if (dns__zone_exiting(zone)) {
+		result = ISC_R_SHUTTINGDOWN;
+		goto cancel;
+	}
+	view = dns_zone_getview(zone);
+	loop = dns_zone_getloop(zone);
+
+	INSIST(view != NULL);
+	INSIST(loop != NULL);
+
+	fetch->fetchmethods.start_fetch(fetch);
+
+	result = dns_view_getresolver(view, &resolver);
+	if (result != ISC_R_SUCCESS) {
+		goto cancel;
+	}
+
+	if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) {
+		char namebuf[DNS_NAME_FORMATSIZE];
+		char typebuf[DNS_RDATATYPE_FORMATSIZE];
+		dns_name_format(fetch->qname, namebuf, sizeof(namebuf));
+		dns_rdatatype_format(fetch->qtype, typebuf, sizeof(typebuf));
+		dns_zone_logc(zone, DNS_LOGCATEGORY_DNSSEC, ISC_LOG_DEBUG(3),
+			      "Do fetch for %s/%s request", namebuf, typebuf);
+	}
+
+	/*
+	 * Use of DNS_FETCHOPT_NOCACHED is essential here.  If it is not
+	 * set and the cache still holds a non-expired, validated version
+	 * of the RRset being queried for by the time the response is
+	 * received, the cached RRset will be passed to dns_zonefetch_done()
+	 * instead of the one received in the response as the latter will
+	 * have a lower trust level due to not being validated until
+	 * dns_zonefetch_done() is called.
+	 */
+	INSIST((fetch->options & DNS_FETCHOPT_NOCACHED) != 0);
+
+	result = dns_resolver_createfetch(
+		resolver, fetch->qname, fetch->qtype, NULL, NULL, NULL, NULL, 0,
+		fetch->options, 0, NULL, NULL, loop, dns_zonefetch_done, fetch,
+		NULL, &fetch->rrset, &fetch->sigset, &fetch->fetch);
+
+	dns_resolver_detach(&resolver);
+
+cancel:
+	if (result == ISC_R_SUCCESS) {
+		return;
+	} else if (result != ISC_R_SHUTTINGDOWN) {
+		char namebuf[DNS_NAME_FORMATSIZE];
+		char typebuf[DNS_RDATATYPE_FORMATSIZE];
+		dns_name_format(fetch->qname, namebuf, sizeof(namebuf));
+		dns_rdatatype_format(fetch->qtype, typebuf, sizeof(typebuf));
+		dns_zone_log(zone, ISC_LOG_WARNING,
+			     "Failed fetch for %s/%s request", namebuf,
+			     typebuf);
+	}
+
+	/*
+	 * Fetch failed, cancel.
+	 */
+	dns__zone_lock(zone);
+
+	dns_name_t *zname = dns_fixedname_name(&fetch->name);
+	isc_mem_t *mctx = dns_zone_getmctx(zone);
+	bool free_needed;
+
+	isc_refcount_decrement(dns__zone_irefs(zone));
+	dns_name_free(zname, mctx);
+
+	fetch->fetchmethods.cancel_fetch(fetch);
+
+	isc_mem_putanddetach(&fetch->mctx, fetch, sizeof(*fetch));
+	free_needed = dns__zone_free_check(zone);
+
+	dns__zone_unlock(zone);
+
+	if (free_needed) {
+		dns__zone_free(zone);
+	}
+}
+
+void
+dns_zonefetch_done(void *arg) {
+	dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg;
+	isc_result_t result = ISC_R_NOMORE;
+	isc_result_t eresult;
+	dns_zonefetch_t *fetch = NULL;
+	dns_zone_t *zone = NULL;
+	dns_view_t *view = NULL;
+	isc_mem_t *mctx = NULL;
+	dns_name_t *zname = NULL;
+	dns_rdataset_t *rrset = NULL;
+	dns_rdataset_t *sigset = NULL;
+
+	INSIST(resp != NULL);
+
+	fetch = resp->arg;
+
+	INSIST(fetch != NULL);
+
+	mctx = fetch->mctx;
+	zone = fetch->zone;
+	zname = dns_fixedname_name(&fetch->name);
+	rrset = &fetch->rrset;
+	sigset = &fetch->sigset;
+	view = dns_zone_getview(zone);
+	eresult = resp->result;
+
+	/* Free resources which are not of interest */
+	if (resp->node != NULL) {
+		dns_db_detachnode(&resp->node);
+	}
+	if (resp->db != NULL) {
+		dns_db_detach(&resp->db);
+	}
+	dns_resolver_destroyfetch(&fetch->fetch);
+
+	dns__zone_lock(zone);
+	if (dns__zone_exiting(zone) || view == NULL) {
+		goto cleanup;
+	}
+
+	result = fetch->fetchmethods.done_fetch(fetch, eresult);
+
+cleanup:
+	isc_refcount_decrement(dns__zone_irefs(zone));
+
+	if (dns_rdataset_isassociated(rrset)) {
+		dns_rdataset_disassociate(rrset);
+	}
+	if (dns_rdataset_isassociated(sigset)) {
+		dns_rdataset_disassociate(sigset);
+	}
+
+	fetch->fetchmethods.cleanup_fetch(fetch);
+
+	dns_resolver_freefresp(&resp);
+
+	if (result == DNS_R_CONTINUE) {
+		dns__zone_unlock(zone);
+		fetch->fetchmethods.continue_fetch(fetch);
+	} else {
+		bool free_needed = false;
+		dns_name_free(zname, mctx);
+		isc_mem_putanddetach(&fetch->mctx, fetch,
+				     sizeof(dns_zonefetch_t));
+		free_needed = dns__zone_free_check(zone);
+
+		dns__zone_unlock(zone);
+
+		if (free_needed) {
+			dns__zone_free(zone);
+		}
+	}
+}
+
+static void
+zonefetch_schedule(dns_zonefetch_t *fetch, dns_name_t *name) {
+	dns_zone_t *zone = fetch->zone;
+
+	isc_refcount_increment0(dns__zone_irefs(zone));
+
+	if (name != NULL) {
+		dns_name_t *fname = dns_fixedname_initname(&fetch->name);
+		dns_name_dup(name, fetch->mctx, fname);
+	}
+
+	dns_rdataset_init(&fetch->rrset);
+	dns_rdataset_init(&fetch->sigset);
+
+	isc_async_run(dns_zone_getloop(zone), dns_zonefetch_run, fetch);
+}
+
+void
+dns_zonefetch_schedule(dns_zonefetch_t *fetch, dns_name_t *name) {
+	REQUIRE(fetch != NULL);
+	REQUIRE(name != NULL);
+
+	zonefetch_schedule(fetch, name);
+}
+
+void
+dns_zonefetch_reschedule(dns_zonefetch_t *fetch) {
+	REQUIRE(fetch != NULL);
+
+	zonefetch_schedule(fetch, NULL);
+}
+
+isc_result_t
+dns_zonefetch_verify(dns_zonefetch_t *fetch, isc_result_t eresult,
+		     dns_trust_t trust) {
+	char namebuf[DNS_NAME_FORMATSIZE];
+	char typebuf[DNS_RDATATYPE_FORMATSIZE];
+	dns_rdataset_t *rrset = NULL;
+	dns_rdataset_t *sigset = NULL;
+
+	REQUIRE(fetch != NULL);
+
+	rrset = &fetch->rrset;
+	sigset = &fetch->sigset;
+	dns_name_format(fetch->qname, namebuf, sizeof(namebuf));
+	dns_rdatatype_format(fetch->qtype, typebuf, sizeof(typebuf));
+
+	if (eresult != ISC_R_SUCCESS) {
+		dns_zone_logc(fetch->zone, DNS_LOGCATEGORY_DNSSEC,
+			      ISC_LOG_WARNING, "Unable to fetch %s/%s: %s",
+			      namebuf, typebuf, isc_result_totext(eresult));
+		return eresult;
+	}
+
+	/* No records found */
+	if (!dns_rdataset_isassociated(rrset)) {
+		dns_zone_logc(fetch->zone, DNS_LOGCATEGORY_DNSSEC,
+			      ISC_LOG_WARNING, "No %s records found for '%s'",
+			      typebuf, namebuf);
+		return ISC_R_NOTFOUND;
+	}
+
+	/* No RRSIGs found */
+	if (!dns_rdataset_isassociated(sigset)) {
+		dns_zone_logc(fetch->zone, DNS_LOGCATEGORY_DNSSEC,
+			      ISC_LOG_WARNING, "No %s RRSIGs found for '%s'",
+			      typebuf, namebuf);
+		return DNS_R_NOVALIDSIG;
+	}
+
+	/* Check trust level */
+	if (rrset->trust < trust) {
+		dns_zone_logc(fetch->zone, DNS_LOGCATEGORY_DNSSEC,
+			      ISC_LOG_WARNING,
+			      "Invalid %s RRset for '%s' trust level %u",
+			      typebuf, namebuf, rrset->trust);
+		return DNS_R_NOVALIDSIG;
+	}
+
+	return ISC_R_SUCCESS;
+}

lib/ns/query.c

@@ -951,7 +951,7 @@ ns_query_init(ns_client_t *client) {
 
 	/*
 	 * This mutex is destroyed when the client is destroyed in
-	 * exit_check().
+	 * ns__client_put_cb().
 	 */
 	isc_mutex_init(&client->query.fetchlock);
 	client->query.redirect.fname =