From 6adfc2348be06fae8446fff0f4f698fc791ccebd Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 29 Oct 2025 15:30:46 +0100 Subject: [PATCH 1/6] Export zone functions Make some zone functions available that we are going to need in the zonefetch code. --- lib/dns/zone.c | 43 +++++++++++++++++++++++-------------------- lib/dns/zone_p.h | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index fdba710f47..c054fbd745 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1168,12 +1168,8 @@ clear_keylist(dns_dnsseckeylist_t *list, isc_mem_t *mctx) { } } -/* - * Free a zone. Because we require that there be no more - * outstanding events or references, no locking is necessary. - */ -static void -zone_free(dns_zone_t *zone) { +void +dns__zone_free(dns_zone_t *zone) { REQUIRE(DNS_ZONE_VALID(zone)); REQUIRE(!LOCKED_ZONE(zone)); REQUIRE(zone->timer == NULL); @@ -5882,8 +5878,8 @@ done: return result; } -static bool -exit_check(dns_zone_t *zone) { +bool +dns__zone_free_check(dns_zone_t *zone) { REQUIRE(LOCKED_ZONE(zone)); if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_SHUTDOWN) && @@ -6241,10 +6237,10 @@ dns_zone_idetach(dns_zone_t **zonep) { if (isc_refcount_decrement(&zone->irefs) == 1) { bool free_needed; LOCK_ZONE(zone); - free_needed = exit_check(zone); + free_needed = dns__zone_free_check(zone); UNLOCK_ZONE(zone); if (free_needed) { - zone_free(zone); + dns__zone_free(zone); } } } @@ -6256,6 +6252,13 @@ dns_zone_getmctx(dns_zone_t *zone) { return zone->mctx; } +isc_refcount_t * +dns__zone_irefs(dns_zone_t *zone) { + REQUIRE(DNS_ZONE_VALID(zone)); + + return &zone->irefs; +} + dns_zonemgr_t * dns_zone_getmgr(dns_zone_t *zone) { REQUIRE(DNS_ZONE_VALID(zone)); @@ -14840,12 +14843,12 @@ zone_shutdown(void *arg) { } /* - * We have now canceled everything set the flag to allow exit_check() - * to succeed. We must not unlock between setting this flag and - * calling exit_check(). + * We have now canceled everything set the flag to allow + * dns__zone_free_check() to succeed. We must not unlock between + * setting this flag and calling dns__zone_free_check(). */ DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_SHUTDOWN); - free_needed = exit_check(zone); + free_needed = dns__zone_free_check(zone); /* * If a dump is in progress for the secure zone, defer detaching from * the raw zone as it may prevent the unsigned serial number from being @@ -14876,7 +14879,7 @@ zone_shutdown(void *arg) { dns_zone_idetach(&secure); } if (free_needed) { - zone_free(zone); + dns__zone_free(zone); } } @@ -15075,10 +15078,10 @@ zone__settimer(void *arg) { free: isc_mem_put(zone->mctx, data, sizeof(*data)); isc_refcount_decrement(&zone->irefs); - free_needed = exit_check(zone); + free_needed = dns__zone_free_check(zone); UNLOCK_ZONE(zone); if (free_needed) { - zone_free(zone); + dns__zone_free(zone); } } @@ -17788,10 +17791,10 @@ again: } isc_refcount_decrement(&zone->irefs); - free_needed = exit_check(zone); + free_needed = dns__zone_free_check(zone); UNLOCK_ZONE(zone); if (free_needed) { - zone_free(zone); + dns__zone_free(zone); } } @@ -23723,7 +23726,7 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, * not yet have a database. Prevent that by queueing the event * up if zone->db is NULL. All events queued here are * subsequently processed by receive_secure_db() if it ever gets - * called or simply freed by zone_free() otherwise. + * called or simply freed by dns__zone_free() otherwise. */ ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read); diff --git a/lib/dns/zone_p.h b/lib/dns/zone_p.h index aefa74ea3b..9a7d3481fa 100644 --- a/lib/dns/zone_p.h +++ b/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. + */ From 7ac02d4c047389a84689d9a1cc26803ed74f3b7a Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 29 Oct 2025 14:02:45 +0100 Subject: [PATCH 2/6] Refactor zone fetch code There is code duplication between keyfetch and nsfetch, refactor to allow common code paths to differentiate between them. --- lib/dns/include/dns/zonefetch.h | 103 ++++++ lib/dns/meson.build | 1 + lib/dns/zone.c | 638 +++++++++++++------------------- lib/dns/zonefetch.c | 189 ++++++++++ 4 files changed, 554 insertions(+), 377 deletions(-) create mode 100644 lib/dns/include/dns/zonefetch.h create mode 100644 lib/dns/zonefetch.c diff --git a/lib/dns/include/dns/zonefetch.h b/lib/dns/include/dns/zonefetch.h new file mode 100644 index 0000000000..d0b1df0aaa --- /dev/null +++ b/lib/dns/include/dns/zonefetch.h @@ -0,0 +1,103 @@ +/* + * 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 +#include + +#include +#include +#include +#include + +/*% + * Fetch type; various features can initiate fetching and this enum value + * allows common code paths to differentiate between them + */ +typedef enum { + ZONEFETCHTYPE_KEY, + ZONEFETCHTYPE_NS, +} 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. + */ diff --git a/lib/dns/meson.build b/lib/dns/meson.build index 02e06dabe7..f6628079d6 100644 --- a/lib/dns/meson.build +++ b/lib/dns/meson.build @@ -165,6 +165,7 @@ dns_srcset.add( 'validator.c', 'view.c', 'zone.c', + 'zonefetch.c', 'zoneverify.c', 'zt.c', ), diff --git a/lib/dns/zone.c b/lib/dns/zone.c index c054fbd745..0dd8e63d32 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -92,6 +92,7 @@ #include #include #include +#include #include #include @@ -177,8 +178,6 @@ typedef struct dns_signing dns_signing_t; typedef ISC_LIST(dns_signing_t) dns_signinglist_t; typedef struct dns_nsec3chain dns_nsec3chain_t; typedef ISC_LIST(dns_nsec3chain_t) dns_nsec3chainlist_t; -typedef struct dns_nsfetch dns_nsfetch_t; -typedef struct dns_keyfetch dns_keyfetch_t; typedef struct dns_asyncload dns_asyncload_t; typedef struct dns_include dns_include_t; @@ -760,27 +759,6 @@ struct dns_nsec3chain { * so it can be recovered in the event of a error. */ -struct dns_keyfetch { - isc_mem_t *mctx; - dns_fixedname_t name; - dns_rdataset_t keydataset; - dns_rdataset_t dnskeyset; - dns_rdataset_t dnskeysigset; - dns_zone_t *zone; - dns_db_t *db; - dns_fetch_t *fetch; -}; - -struct dns_nsfetch { - isc_mem_t *mctx; - dns_fixedname_t name; - dns_name_t pname; - dns_rdataset_t nsrrset; - dns_rdataset_t nssigset; - dns_zone_t *zone; - dns_fetch_t *fetch; -}; - /*% * Hold state for an asynchronous load */ @@ -835,6 +813,7 @@ zone_debuglog(dns_zone_t *zone, const char *, int debuglevel, const char *msg, static void dnssec_log(dns_zone_t *zone, int level, const char *fmt, ...) ISC_FORMAT_PRINTF(3, 4); + static void queue_xfrin(dns_zone_t *zone); static isc_result_t @@ -912,8 +891,6 @@ static void checkds_send_tons(dns_checkds_t *checkds); static void checkds_send_toaddr(void *arg); -static void -nsfetch_levelup(dns_nsfetch_t *nsfetch); static isc_result_t zone_dump(dns_zone_t *, bool); static void @@ -10455,26 +10432,30 @@ matchkey(dns_rdataset_t *rdset, dns_rdata_t *rr) { * 1/10 * RRSigExpirationInterval)) */ static isc_stdtime_t -refresh_time(dns_keyfetch_t *kfetch, bool retry) { +refresh_time(dns_zonefetch_t *fetch, bool retry) { isc_result_t result; uint32_t t; - dns_rdataset_t *rdset; + dns_rdataset_t *sigset; dns_rdata_t sigrr = DNS_RDATA_INIT; dns_rdata_sig_t sig; - isc_stdtime_t now = isc_stdtime_now(); + isc_stdtime_t now; - if (dns_rdataset_isassociated(&kfetch->dnskeysigset)) { - rdset = &kfetch->dnskeysigset; + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); + + now = isc_stdtime_now(); + + if (dns_rdataset_isassociated(&fetch->sigset)) { + sigset = &fetch->sigset; } else { return now + dns_zone_mkey_hour; } - result = dns_rdataset_first(rdset); + result = dns_rdataset_first(sigset); if (result != ISC_R_SUCCESS) { return now + dns_zone_mkey_hour; } - dns_rdataset_current(rdset, &sigrr); + dns_rdataset_current(sigset, &sigrr); result = dns_rdata_tostruct(&sigrr, &sig, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); @@ -10523,16 +10504,22 @@ refresh_time(dns_keyfetch_t *kfetch, bool retry) { * hold zone lock. */ static isc_result_t -minimal_update(dns_keyfetch_t *kfetch, dns_dbversion_t *ver, dns_diff_t *diff) { +minimal_update(dns_zonefetch_t *fetch, dns_dbversion_t *ver, dns_diff_t *diff) { + dns_keyfetch_t *kfetch; isc_result_t result; isc_buffer_t keyb; unsigned char key_buf[4096]; dns_rdata_keydata_t keydata; dns_name_t *name; - dns_zone_t *zone = kfetch->zone; - isc_stdtime_t now = isc_stdtime_now(); + dns_zone_t *zone; + isc_stdtime_t now; - name = dns_fixedname_name(&kfetch->name); + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); + + now = isc_stdtime_now(); + zone = fetch->zone; + name = dns_fixedname_name(&fetch->name); + kfetch = &fetch->fetchdata.keyfetch; DNS_RDATASET_FOREACH(&kfetch->keydataset) { dns_rdata_t rdata = DNS_RDATA_INIT; @@ -10550,7 +10537,7 @@ minimal_update(dns_keyfetch_t *kfetch, dns_dbversion_t *ver, dns_diff_t *diff) { if (result != ISC_R_SUCCESS) { goto failure; } - keydata.refresh = refresh_time(kfetch, true); + keydata.refresh = refresh_time(fetch, true); set_refreshkeytimer(zone, &keydata, now, false); dns_rdata_reset(&rdata); @@ -10572,7 +10559,7 @@ failure: * Verify that DNSKEY set is signed by the key specified in 'keydata'. */ static bool -revocable(dns_keyfetch_t *kfetch, dns_rdata_keydata_t *keydata) { +revocable(dns_zonefetch_t *fetch, dns_rdata_keydata_t *keydata) { isc_result_t result; dns_name_t *keyname; isc_mem_t *mctx; @@ -10585,11 +10572,12 @@ revocable(dns_keyfetch_t *kfetch, dns_rdata_keydata_t *keydata) { bool answer = false; dst_algorithm_t algorithm; - REQUIRE(kfetch != NULL && keydata != NULL); - REQUIRE(dns_rdataset_isassociated(&kfetch->dnskeysigset)); + REQUIRE(fetch != NULL && keydata != NULL); + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); + REQUIRE(dns_rdataset_isassociated(&fetch->sigset)); - keyname = dns_fixedname_name(&kfetch->name); - mctx = kfetch->zone->view->mctx; + keyname = dns_fixedname_name(&fetch->name); + mctx = fetch->zone->view->mctx; /* Generate a key from keydata */ isc_buffer_init(&keyb, key_buf, sizeof(key_buf)); @@ -10602,12 +10590,12 @@ revocable(dns_keyfetch_t *kfetch, dns_rdata_keydata_t *keydata) { } /* See if that key generated any of the signatures */ - DNS_RDATASET_FOREACH(&kfetch->dnskeysigset) { + DNS_RDATASET_FOREACH(&fetch->sigset) { dns_rdata_t sigrr = DNS_RDATA_INIT; dns_fixedname_t fixed; dns_fixedname_init(&fixed); - dns_rdataset_current(&kfetch->dnskeysigset, &sigrr); + dns_rdataset_current(&fetch->sigset, &sigrr); result = dns_rdata_tostruct(&sigrr, &sig, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); @@ -10616,11 +10604,11 @@ revocable(dns_keyfetch_t *kfetch, dns_rdata_keydata_t *keydata) { if (dst_key_alg(dstkey) == algorithm && dst_key_rid(dstkey) == sig.keyid) { - result = dns_dnssec_verify(keyname, &kfetch->dnskeyset, + result = dns_dnssec_verify(keyname, &fetch->rrset, dstkey, false, mctx, &sigrr, dns_fixedname_name(&fixed)); - dnssec_log(kfetch->zone, ISC_LOG_DEBUG(3), + dnssec_log(fetch->zone, ISC_LOG_DEBUG(3), "Confirm revoked DNSKEY is self-signed: %s", isc_result_totext(result)); @@ -10635,15 +10623,83 @@ revocable(dns_keyfetch_t *kfetch, dns_rdata_keydata_t *keydata) { return answer; } +/* + * Fetch DNSKEY records at the trust anchor name. + */ +static void +keyfetch_start(dns_zonefetch_t *fetch) { + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); + + fetch->qname = dns_fixedname_name(&fetch->name); + fetch->qtype = dns_rdatatype_dnskey; +} + +static void +keyfetch_continue(dns_zonefetch_t *fetch) { + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); + /* No continue path for keyfetch exists. */ + REQUIRE(0); +} + +static void +keyfetch_cancel(dns_zonefetch_t *fetch) { + dns_keyfetch_t *kfetch; + dns_zone_t *zone; + + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); + + kfetch = &fetch->fetchdata.keyfetch; + zone = fetch->zone; + + INSIST(LOCKED_ZONE(zone)); + + /* + * Error during a key fetch; cancel and retry in an hour. + */ + zone->refreshkeycount--; + + dns_db_detach(&kfetch->db); + dns_rdataset_disassociate(&kfetch->keydataset); + + if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) { + /* Don't really retry if we are exiting */ + isc_time_t timenow, timethen; + char timebuf[80]; + + timenow = isc_time_now(); + DNS_ZONE_TIME_ADD(&timenow, dns_zone_mkey_hour, &timethen); + zone->refreshkeytime = timethen; + zone_settimer(zone, &timenow); + + isc_time_formattimestamp(&zone->refreshkeytime, timebuf, 80); + dnssec_log(zone, ISC_LOG_DEBUG(1), "retry key refresh: %s", + timebuf); + } +} + +static void +keyfetch_cleanup(dns_zonefetch_t *fetch) { + dns_keyfetch_t *kfetch = NULL; + + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); + + kfetch = &fetch->fetchdata.keyfetch; + + dns_db_detach(&kfetch->db); + + if (dns_rdataset_isassociated(&kfetch->keydataset)) { + dns_rdataset_disassociate(&kfetch->keydataset); + } +} + /* * A DNSKEY set has been fetched from the zone apex of a zone whose trust * anchors are being managed; scan the keyset, and update the key zone and the * local trust anchors according to RFC5011. */ -static void -keyfetch_done(void *arg) { - dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg; - isc_result_t result, eresult; +static isc_result_t +keyfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) { + isc_result_t result; dns_keyfetch_t *kfetch = NULL; dns_zone_t *zone = NULL; isc_mem_t *mctx = NULL; @@ -10665,41 +10721,22 @@ keyfetch_done(void *arg) { isc_stdtime_t now; int pending = 0; bool secure = false, initial = false; - bool free_needed; dns_keynode_t *keynode = NULL; dns_rdataset_t *dnskeys = NULL, *dnskeysigs = NULL; dns_rdataset_t *keydataset = NULL, dsset; - INSIST(resp != NULL); + REQUIRE(fetch != NULL); + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_KEY); - kfetch = resp->arg; + kfetch = &fetch->fetchdata.keyfetch; + zone = fetch->zone; + mctx = fetch->mctx; + keyname = dns_fixedname_name(&fetch->name); + dnskeys = &fetch->rrset; + dnskeysigs = &fetch->sigset; - INSIST(kfetch != NULL); - - zone = kfetch->zone; - mctx = kfetch->mctx; - keyname = dns_fixedname_name(&kfetch->name); - dnskeys = &kfetch->dnskeyset; - dnskeysigs = &kfetch->dnskeysigset; keydataset = &kfetch->keydataset; - 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(&kfetch->fetch); - - LOCK_ZONE(zone); - if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING) || zone->view == NULL) { - goto cleanup; - } - now = isc_stdtime_now(); dns_name_format(keyname, namebuf, sizeof(namebuf)); @@ -10726,7 +10763,7 @@ keyfetch_done(void *arg) { dnssec_log(zone, ISC_LOG_WARNING, "Unable to fetch DNSKEY set '%s': %s", namebuf, isc_result_totext(eresult)); - CHECK(minimal_update(kfetch, ver, &diff)); + CHECK(minimal_update(fetch, ver, &diff)); goto done; } @@ -10735,7 +10772,7 @@ keyfetch_done(void *arg) { dnssec_log(zone, ISC_LOG_WARNING, "No DNSKEY RRSIGs found for '%s': %s", namebuf, isc_result_totext(eresult)); - CHECK(minimal_update(kfetch, ver, &diff)); + CHECK(minimal_update(fetch, ver, &diff)); goto done; } @@ -10901,7 +10938,7 @@ anchors_done: keydata.addhd = now + dns_zone_mkey_month; } - keydata.refresh = refresh_time(kfetch, false); + keydata.refresh = refresh_time(fetch, false); } else if (keydata.removehd == 0) { dnssec_log(zone, ISC_LOG_INFO, "Active key %d for zone %s " @@ -10918,7 +10955,7 @@ anchors_done: "from managed keys database", keytag, namebuf); } else { - keydata.refresh = refresh_time(kfetch, false); + keydata.refresh = refresh_time(fetch, false); } if (secure || deletekey) { @@ -11004,7 +11041,7 @@ anchors_done: result = dns_rdata_tostruct(&keydatarr, &keydata, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (revoked && revocable(kfetch, &keydata)) { + if (revoked && revocable(fetch, &keydata)) { if (keydata.addhd > now) { /* * Key wasn't trusted yet, and now @@ -11160,7 +11197,7 @@ anchors_done: if (updatekey) { /* Set refresh timer */ - keydata.refresh = refresh_time(kfetch, false); + keydata.refresh = refresh_time(fetch, false); dns_rdata_reset(&keydatarr); isc_buffer_init(&keyb, key_buf, sizeof(key_buf)); dns_rdata_fromstruct(&keydatarr, zone->rdclass, @@ -11180,7 +11217,7 @@ anchors_done: keydata.addhd = initializing ? now : now + dns_zone_mkey_month; - keydata.refresh = refresh_time(kfetch, false); + keydata.refresh = refresh_time(fetch, false); dns_rdata_reset(&keydatarr); isc_buffer_init(&keyb, key_buf, sizeof(key_buf)); dns_rdata_fromstruct(&keydatarr, zone->rdclass, @@ -11249,128 +11286,20 @@ failure: "DNSSEC validation may be at risk", isc_result_totext(result)); } + dns_diff_clear(&diff); + if (ver != NULL) { dns_db_closeversion(kfetch->db, &ver, commit); } -cleanup: - dns_db_detach(&kfetch->db); - - isc_refcount_decrement(&zone->irefs); - - if (dns_rdataset_isassociated(keydataset)) { - dns_rdataset_disassociate(keydataset); - } - if (dns_rdataset_isassociated(dnskeys)) { - dns_rdataset_disassociate(dnskeys); - } - if (dns_rdataset_isassociated(dnskeysigs)) { - dns_rdataset_disassociate(dnskeysigs); - } - - dns_resolver_freefresp(&resp); - dns_name_free(keyname, mctx); - isc_mem_putanddetach(&kfetch->mctx, kfetch, sizeof(dns_keyfetch_t)); - if (secroots != NULL) { dns_keytable_detach(&secroots); } - free_needed = exit_check(zone); - UNLOCK_ZONE(zone); - - if (free_needed) { - zone_free(zone); - } - INSIST(ver == NULL); -} -static void -retry_keyfetch(dns_keyfetch_t *kfetch, dns_name_t *kname) { - isc_time_t timenow, timethen; - dns_zone_t *zone = kfetch->zone; - bool free_needed; - char namebuf[DNS_NAME_FORMATSIZE]; - - dns_name_format(kname, namebuf, sizeof(namebuf)); - dnssec_log(zone, ISC_LOG_WARNING, - "Failed to create fetch for %s DNSKEY update", namebuf); - - /* - * Error during a key fetch; cancel and retry in an hour. - */ - LOCK_ZONE(zone); - zone->refreshkeycount--; - isc_refcount_decrement(&zone->irefs); - dns_db_detach(&kfetch->db); - dns_rdataset_disassociate(&kfetch->keydataset); - dns_name_free(kname, zone->mctx); - isc_mem_putanddetach(&kfetch->mctx, kfetch, sizeof(*kfetch)); - - if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) { - /* Don't really retry if we are exiting */ - char timebuf[80]; - - timenow = isc_time_now(); - DNS_ZONE_TIME_ADD(&timenow, dns_zone_mkey_hour, &timethen); - zone->refreshkeytime = timethen; - zone_settimer(zone, &timenow); - - isc_time_formattimestamp(&zone->refreshkeytime, timebuf, 80); - dnssec_log(zone, ISC_LOG_DEBUG(1), "retry key refresh: %s", - timebuf); - } - - free_needed = exit_check(zone); - UNLOCK_ZONE(zone); - - if (free_needed) { - zone_free(zone); - } -} - -static void -do_keyfetch(void *arg) { - isc_result_t result; - dns_keyfetch_t *kfetch = (dns_keyfetch_t *)arg; - dns_name_t *kname = dns_fixedname_name(&kfetch->name); - dns_resolver_t *resolver = NULL; - dns_zone_t *zone = kfetch->zone; - unsigned int options = DNS_FETCHOPT_NOVALIDATE | DNS_FETCHOPT_UNSHARED | - DNS_FETCHOPT_NOCACHED; - - if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) { - goto retry; - } - - result = dns_view_getresolver(zone->view, &resolver); - if (result != ISC_R_SUCCESS) { - goto retry; - } - - /* - * 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 keyfetch_done() - * instead of the one received in the response as the latter will - * have a lower trust level due to not being validated until - * keyfetch_done() is called. - */ - result = dns_resolver_createfetch( - resolver, kname, dns_rdatatype_dnskey, NULL, NULL, NULL, NULL, - 0, options, 0, NULL, NULL, zone->loop, keyfetch_done, kfetch, - NULL, &kfetch->dnskeyset, &kfetch->dnskeysigset, - &kfetch->fetch); - - dns_resolver_detach(&resolver); - if (result == ISC_R_SUCCESS) { - return; - } -retry: - retry_keyfetch(kfetch, kname); + return result; } /* @@ -11469,19 +11398,38 @@ zone_refreshkeys(dns_zone_t *zone) { #ifdef ENABLE_AFL if (!dns_fuzzing_resolver) { #endif /* ifdef ENABLE_AFL */ + dns_zonefetch_t *fetch = NULL; dns_keyfetch_t *kfetch = NULL; - kfetch = isc_mem_get(zone->mctx, - sizeof(dns_keyfetch_t)); - *kfetch = (dns_keyfetch_t){ .zone = zone }; - isc_mem_attach(zone->mctx, &kfetch->mctx); + fetch = isc_mem_get(zone->mctx, + sizeof(dns_zonefetch_t)); + *fetch = (dns_zonefetch_t){ + .zone = zone, + .options = DNS_FETCHOPT_NOVALIDATE | + DNS_FETCHOPT_UNSHARED | + DNS_FETCHOPT_NOCACHED, + .fetchtype = ZONEFETCHTYPE_KEY, + .fetchmethods = + (dns_zonefetch_methods_t){ + .start_fetch = keyfetch_start, + .continue_fetch = + keyfetch_continue, + .cancel_fetch = keyfetch_cancel, + .cleanup_fetch = + keyfetch_cleanup, + .done_fetch = keyfetch_done, + }, + }; + isc_mem_attach(zone->mctx, &fetch->mctx); zone->refreshkeycount++; isc_refcount_increment0(&zone->irefs); - kname = dns_fixedname_initname(&kfetch->name); + kname = dns_fixedname_initname(&fetch->name); dns_name_dup(name, zone->mctx, kname); - dns_rdataset_init(&kfetch->dnskeyset); - dns_rdataset_init(&kfetch->dnskeysigset); + dns_rdataset_init(&fetch->rrset); + dns_rdataset_init(&fetch->sigset); + + kfetch = &fetch->fetchdata.keyfetch; dns_rdataset_init(&kfetch->keydataset); dns_rdataset_clone(kdset, &kfetch->keydataset); dns_db_attach(db, &kfetch->db); @@ -11496,7 +11444,7 @@ zone_refreshkeys(dns_zone_t *zone) { namebuf); } - isc_async_run(zone->loop, do_keyfetch, kfetch); + isc_async_run(zone->loop, dns_zonefetch_run, fetch); fetching = true; #ifdef ENABLE_AFL } @@ -21138,52 +21086,100 @@ checkds_send(dns_zone_t *zone) { } } +/* + * Fetch NS records from parent zone. + */ +static void +nsfetch_start(dns_zonefetch_t *fetch) { + dns_nsfetch_t *nsfetch; + unsigned int nlabels = 1; + + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS); + + nsfetch = &fetch->fetchdata.nsfetch; + + /* Derive parent domain. XXXWMM: Check for root domain */ + dns_name_split(&nsfetch->pname, + dns_name_countlabels(&nsfetch->pname) - nlabels, NULL, + &nsfetch->pname); + + fetch->qtype = dns_rdatatype_ns; + fetch->qname = &nsfetch->pname; +} + +/* + * Retry an NS RRset lookup, one level up. In other words, this function should + * be called on an dns_nsfetch structure where the response yielded in a NODATA + * response. This must be because there is an empty non-terminal inbetween the + * child and parent zone. + */ +static void +nsfetch_continue(dns_zonefetch_t *fetch) { + dns_zone_t *zone = fetch->zone; + + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS); + +#ifdef ENABLE_AFL + if (!dns_fuzzing_resolver) { +#endif /* ifdef ENABLE_AFL */ + LOCK_ZONE(zone); + zone->nsfetchcount++; + isc_refcount_increment0(&zone->irefs); + + dns_rdataset_init(&fetch->rrset); + dns_rdataset_init(&fetch->sigset); + if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { + dnssec_log(zone, ISC_LOG_DEBUG(3), + "Creating parent NS fetch in " + "nsfetch_continue()"); + } + isc_async_run(zone->loop, dns_zonefetch_run, fetch); + UNLOCK_ZONE(zone); +#ifdef ENABLE_AFL + } +#endif /* ifdef ENABLE_AFL */ +} + +static void +nsfetch_cancel(dns_zonefetch_t *fetch) { + dns_zone_t *zone; + + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS); + + zone = fetch->zone; + + INSIST(LOCKED_ZONE(zone)); + + zone->nsfetchcount--; +} + +static void +nsfetch_cleanup(dns_zonefetch_t *fetch) { + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS); +} + /* * An NS RRset has been fetched from the parent of a zone whose DS RRset needs * to be checked; scan the RRset and start sending queries to the parental * agents. */ -static void -nsfetch_done(void *arg) { - dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg; - isc_result_t result = ISC_R_NOMORE, eresult; - dns_nsfetch_t *nsfetch = NULL; +static isc_result_t +nsfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) { + dns_nsfetch_t *nsfetch; + isc_result_t result = ISC_R_NOMORE; dns_zone_t *zone = NULL; - isc_mem_t *mctx = NULL; - dns_name_t *zname = NULL; dns_name_t *pname = NULL; char pnamebuf[DNS_NAME_FORMATSIZE]; - bool free_needed, levelup = false; dns_rdataset_t *nsrrset = NULL; dns_rdataset_t *nssigset = NULL; - INSIST(resp != NULL); + REQUIRE(fetch != NULL); + REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS); - nsfetch = resp->arg; - - INSIST(nsfetch != NULL); - - zone = nsfetch->zone; - mctx = nsfetch->mctx; - zname = dns_fixedname_name(&nsfetch->name); + nsfetch = &fetch->fetchdata.nsfetch; + zone = fetch->zone; + nsrrset = &fetch->rrset; pname = &nsfetch->pname; - nsrrset = &nsfetch->nsrrset; - nssigset = &nsfetch->nssigset; - 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(&nsfetch->fetch); - - LOCK_ZONE(zone); - if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING) || zone->view == NULL) { - goto cleanup; - } zone->nsfetchcount--; @@ -21195,8 +21191,7 @@ nsfetch_done(void *arg) { if (eresult == DNS_R_NCACHENXRRSET || eresult == DNS_R_NXRRSET) { dnssec_log(zone, ISC_LOG_DEBUG(3), "NODATA response for NS '%s', level up", pnamebuf); - levelup = true; - goto cleanup; + return DNS_R_CONTINUE; } else if (eresult != ISC_R_SUCCESS) { dnssec_log(zone, ISC_LOG_WARNING, @@ -21290,136 +21285,7 @@ done: isc_result_totext(result)); } -cleanup: - isc_refcount_decrement(&zone->irefs); - - if (dns_rdataset_isassociated(nsrrset)) { - dns_rdataset_disassociate(nsrrset); - } - if (dns_rdataset_isassociated(nssigset)) { - dns_rdataset_disassociate(nssigset); - } - - dns_resolver_freefresp(&resp); - - if (levelup) { - UNLOCK_ZONE(zone); - nsfetch_levelup(nsfetch); - return; - } - - dns_name_free(zname, mctx); - isc_mem_putanddetach(&nsfetch->mctx, nsfetch, sizeof(dns_nsfetch_t)); - - free_needed = exit_check(zone); - UNLOCK_ZONE(zone); - - if (free_needed) { - zone_free(zone); - } -} - -static void -do_nsfetch(void *arg) { - dns_nsfetch_t *nsfetch = (dns_nsfetch_t *)arg; - isc_result_t result; - unsigned int nlabels = 1; - dns_resolver_t *resolver = NULL; - dns_zone_t *zone = nsfetch->zone; - unsigned int options = DNS_FETCHOPT_UNSHARED | DNS_FETCHOPT_NOCACHED; - - if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) { - result = ISC_R_SHUTTINGDOWN; - goto cleanup; - } - - result = dns_view_getresolver(zone->view, &resolver); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - - if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { - char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(&nsfetch->pname, namebuf, sizeof(namebuf)); - dnssec_log(zone, ISC_LOG_DEBUG(3), - "Create fetch for '%s' NS request", namebuf); - } - - /* Derive parent domain. XXXWMM: Check for root domain */ - dns_name_split(&nsfetch->pname, - dns_name_countlabels(&nsfetch->pname) - nlabels, NULL, - &nsfetch->pname); - - /* - * 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 nsfetch_done() - * instead of the one received in the response as the latter will - * have a lower trust level due to not being validated until - * nsfetch_done() is called. - */ - result = dns_resolver_createfetch( - resolver, &nsfetch->pname, dns_rdatatype_ns, NULL, NULL, NULL, - NULL, 0, options, 0, NULL, NULL, zone->loop, nsfetch_done, - nsfetch, NULL, &nsfetch->nsrrset, &nsfetch->nssigset, - &nsfetch->fetch); - - dns_resolver_detach(&resolver); - -cleanup: - if (result != ISC_R_SUCCESS) { - dns_name_t *zname = dns_fixedname_name(&nsfetch->name); - bool free_needed; - char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(&nsfetch->pname, namebuf, sizeof(namebuf)); - dnssec_log(zone, ISC_LOG_WARNING, - "Failed to create fetch for '%s' NS request", - namebuf); - LOCK_ZONE(zone); - zone->nsfetchcount--; - isc_refcount_decrement(&zone->irefs); - - dns_name_free(zname, zone->mctx); - isc_mem_putanddetach(&nsfetch->mctx, nsfetch, sizeof(*nsfetch)); - - free_needed = exit_check(zone); - UNLOCK_ZONE(zone); - if (free_needed) { - zone_free(zone); - } - } -} - -/* - * Retry an NS RRset lookup, one level up. In other words, this function should - * be called on an dns_nsfetch structure where the response yielded in a NODATA - * response. This must be because there is an empty non-terminal inbetween the - * child and parent zone. - */ -static void -nsfetch_levelup(dns_nsfetch_t *nsfetch) { - dns_zone_t *zone = nsfetch->zone; - -#ifdef ENABLE_AFL - if (!dns_fuzzing_resolver) { -#endif /* ifdef ENABLE_AFL */ - LOCK_ZONE(zone); - zone->nsfetchcount++; - isc_refcount_increment0(&zone->irefs); - - dns_rdataset_init(&nsfetch->nsrrset); - dns_rdataset_init(&nsfetch->nssigset); - if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { - dnssec_log(zone, ISC_LOG_DEBUG(3), - "Creating parent NS fetch in " - "nsfetch_levelup()"); - } - isc_async_run(zone->loop, do_nsfetch, nsfetch); - UNLOCK_ZONE(zone); -#ifdef ENABLE_AFL - } -#endif /* ifdef ENABLE_AFL */ + return result; } static void @@ -21475,27 +21341,45 @@ zone_checkds(dns_zone_t *zone) { #ifdef ENABLE_AFL if (!dns_fuzzing_resolver) { #endif /* ifdef ENABLE_AFL */ - dns_nsfetch_t *nsfetch; + dns_zonefetch_t *fetch = NULL; + dns_nsfetch_t *nsfetch = NULL; dns_name_t *name = NULL; - nsfetch = isc_mem_get(zone->mctx, sizeof(dns_nsfetch_t)); - *nsfetch = (dns_nsfetch_t){ .zone = zone }; - isc_mem_attach(zone->mctx, &nsfetch->mctx); + fetch = isc_mem_get(zone->mctx, sizeof(dns_zonefetch_t)); + *fetch = (dns_zonefetch_t){ + .zone = zone, + .options = DNS_FETCHOPT_UNSHARED | + DNS_FETCHOPT_NOCACHED, + .fetchtype = ZONEFETCHTYPE_NS, + .fetchmethods = + (dns_zonefetch_methods_t){ + .start_fetch = nsfetch_start, + .continue_fetch = nsfetch_continue, + .cancel_fetch = nsfetch_cancel, + .cleanup_fetch = nsfetch_cleanup, + .done_fetch = nsfetch_done, + }, + }; + isc_mem_attach(zone->mctx, &fetch->mctx); + LOCK_ZONE(zone); zone->nsfetchcount++; isc_refcount_increment0(&zone->irefs); - name = dns_fixedname_initname(&nsfetch->name); + name = dns_fixedname_initname(&fetch->name); + dns_name_dup(&zone->origin, zone->mctx, name); + dns_rdataset_init(&fetch->rrset); + dns_rdataset_init(&fetch->sigset); + + nsfetch = &fetch->fetchdata.nsfetch; dns_name_init(&nsfetch->pname); dns_name_clone(&zone->origin, &nsfetch->pname); - dns_name_dup(&zone->origin, zone->mctx, name); - dns_rdataset_init(&nsfetch->nsrrset); - dns_rdataset_init(&nsfetch->nssigset); + if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { dnssec_log( zone, ISC_LOG_DEBUG(3), "Creating parent NS fetch in zone_checkds()"); } - isc_async_run(zone->loop, do_nsfetch, nsfetch); + isc_async_run(zone->loop, dns_zonefetch_run, fetch); UNLOCK_ZONE(zone); #ifdef ENABLE_AFL } diff --git a/lib/dns/zonefetch.c b/lib/dns/zonefetch.c new file mode 100644 index 0000000000..988f4a6bfa --- /dev/null +++ b/lib/dns/zonefetch.c @@ -0,0 +1,189 @@ +/* + * 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 + +#include +#include +#include +#include + +#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); + } + } +} From 94a82c2ecb0f7ddcb56a9634ffd6525ecf0a6ba0 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 30 Oct 2025 11:01:04 +0100 Subject: [PATCH 3/6] Count per fetch type Track fetch counts per type in an array, rather than special named variables within the zone structure. --- lib/dns/include/dns/zonefetch.h | 6 +++++- lib/dns/zone.c | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/dns/include/dns/zonefetch.h b/lib/dns/include/dns/zonefetch.h index d0b1df0aaa..41dcb52b10 100644 --- a/lib/dns/include/dns/zonefetch.h +++ b/lib/dns/include/dns/zonefetch.h @@ -25,11 +25,15 @@ /*% * Fetch type; various features can initiate fetching and this enum value - * allows common code paths to differentiate between them + * 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; diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 0dd8e63d32..4b8b893e8d 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -318,7 +318,6 @@ struct dns_zone { isc_time_t refreshkeytime; isc_time_t xfrintime; uint32_t refreshkeyinterval; - uint32_t refreshkeycount; uint32_t refresh; uint32_t retry; uint32_t expire; @@ -343,9 +342,10 @@ struct dns_zone { dns_remote_t parentals; dns_dnsseckeylist_t checkds_ok; dns_checkdstype_t checkdstype; - uint32_t nsfetchcount; uint32_t parent_nscount; + uint32_t fetchcount[ZONEFETCHTYPE_COUNT]; + dns_remote_t alsonotify; dns_notifyctx_t notifyctx; @@ -10656,7 +10656,7 @@ keyfetch_cancel(dns_zonefetch_t *fetch) { /* * Error during a key fetch; cancel and retry in an hour. */ - zone->refreshkeycount--; + zone->fetchcount[ZONEFETCHTYPE_KEY]--; dns_db_detach(&kfetch->db); dns_rdataset_disassociate(&kfetch->keydataset); @@ -10747,8 +10747,8 @@ keyfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) { CHECK(dns_db_newversion(kfetch->db, &ver)); - zone->refreshkeycount--; - alldone = (zone->refreshkeycount == 0); + zone->fetchcount[ZONEFETCHTYPE_KEY]--; + alldone = (zone->fetchcount[ZONEFETCHTYPE_KEY] == 0); if (alldone) { DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_REFRESHING); @@ -11422,7 +11422,7 @@ zone_refreshkeys(dns_zone_t *zone) { }; isc_mem_attach(zone->mctx, &fetch->mctx); - zone->refreshkeycount++; + zone->fetchcount[ZONEFETCHTYPE_KEY]++; isc_refcount_increment0(&zone->irefs); kname = dns_fixedname_initname(&fetch->name); dns_name_dup(name, zone->mctx, kname); @@ -21123,7 +21123,7 @@ nsfetch_continue(dns_zonefetch_t *fetch) { if (!dns_fuzzing_resolver) { #endif /* ifdef ENABLE_AFL */ LOCK_ZONE(zone); - zone->nsfetchcount++; + zone->fetchcount[ZONEFETCHTYPE_NS]++; isc_refcount_increment0(&zone->irefs); dns_rdataset_init(&fetch->rrset); @@ -21150,7 +21150,7 @@ nsfetch_cancel(dns_zonefetch_t *fetch) { INSIST(LOCKED_ZONE(zone)); - zone->nsfetchcount--; + zone->fetchcount[ZONEFETCHTYPE_NS]--; } static void @@ -21181,7 +21181,7 @@ nsfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) { nsrrset = &fetch->rrset; pname = &nsfetch->pname; - zone->nsfetchcount--; + zone->fetchcount[ZONEFETCHTYPE_NS]--; dns_name_format(pname, pnamebuf, sizeof(pnamebuf)); dnssec_log(zone, ISC_LOG_DEBUG(3), @@ -21363,7 +21363,7 @@ zone_checkds(dns_zone_t *zone) { isc_mem_attach(zone->mctx, &fetch->mctx); LOCK_ZONE(zone); - zone->nsfetchcount++; + zone->fetchcount[ZONEFETCHTYPE_NS]++; isc_refcount_increment0(&zone->irefs); name = dns_fixedname_initname(&fetch->name); dns_name_dup(&zone->origin, zone->mctx, name); From 6f70cb7a396e4be9dab24b2ff4e4eeb9f6e93ab5 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 31 Oct 2025 09:04:13 +0100 Subject: [PATCH 4/6] A unified way to verify the zone fetch There is a lot of similarity when checking the completed fetch is legit. Create a new function to verify the fetch and to reduce code duplication. --- lib/dns/include/dns/zonefetch.h | 18 ++++++++++++ lib/dns/zone.c | 48 +++---------------------------- lib/dns/zonefetch.c | 50 +++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 44 deletions(-) diff --git a/lib/dns/include/dns/zonefetch.h b/lib/dns/include/dns/zonefetch.h index 41dcb52b10..01f189b81d 100644 --- a/lib/dns/include/dns/zonefetch.h +++ b/lib/dns/include/dns/zonefetch.h @@ -105,3 +105,21 @@ dns_zonefetch_done(void *arg); * Complete a zone fetch. This may trigger follow-up actions that depend on * the fetch type. */ + +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. + */ diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 4b8b893e8d..cf7b2bed68 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -10758,20 +10758,8 @@ keyfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) { "Returned from key fetch in keyfetch_done() for '%s': %s", namebuf, isc_result_totext(eresult)); - /* Fetch failed */ - if (eresult != ISC_R_SUCCESS || !dns_rdataset_isassociated(dnskeys)) { - dnssec_log(zone, ISC_LOG_WARNING, - "Unable to fetch DNSKEY set '%s': %s", namebuf, - isc_result_totext(eresult)); - CHECK(minimal_update(fetch, ver, &diff)); - goto done; - } - - /* No RRSIGs found */ - if (!dns_rdataset_isassociated(dnskeysigs)) { - dnssec_log(zone, ISC_LOG_WARNING, - "No DNSKEY RRSIGs found for '%s': %s", namebuf, - isc_result_totext(eresult)); + result = dns_zonefetch_verify(fetch, eresult, dns_trust_none); + if (result != ISC_R_SUCCESS) { CHECK(minimal_update(fetch, ver, &diff)); goto done; } @@ -21171,7 +21159,6 @@ nsfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) { dns_name_t *pname = NULL; char pnamebuf[DNS_NAME_FORMATSIZE]; dns_rdataset_t *nsrrset = NULL; - dns_rdataset_t *nssigset = NULL; REQUIRE(fetch != NULL); REQUIRE(fetch->fetchtype == ZONEFETCHTYPE_NS); @@ -21192,37 +21179,10 @@ nsfetch_done(dns_zonefetch_t *fetch, isc_result_t eresult) { dnssec_log(zone, ISC_LOG_DEBUG(3), "NODATA response for NS '%s', level up", pnamebuf); return DNS_R_CONTINUE; - - } else if (eresult != ISC_R_SUCCESS) { - dnssec_log(zone, ISC_LOG_WARNING, - "Unable to fetch NS set '%s': %s", pnamebuf, - isc_result_totext(eresult)); - result = eresult; - goto done; } - /* No NS records found */ - if (!dns_rdataset_isassociated(nsrrset)) { - dnssec_log(zone, ISC_LOG_WARNING, - "No NS records found for '%s'", pnamebuf); - result = ISC_R_NOTFOUND; - goto done; - } - - /* No RRSIGs found */ - if (!dns_rdataset_isassociated(nssigset)) { - dnssec_log(zone, ISC_LOG_WARNING, "No NS RRSIGs found for '%s'", - pnamebuf); - result = DNS_R_NOVALIDSIG; - goto done; - } - - /* Check trust level */ - if (nsrrset->trust < dns_trust_secure) { - dnssec_log(zone, ISC_LOG_WARNING, - "Invalid NS RRset for '%s' trust level %u", pnamebuf, - nsrrset->trust); - result = DNS_R_NOVALIDSIG; + result = dns_zonefetch_verify(fetch, eresult, dns_trust_secure); + if (result != ISC_R_SUCCESS) { goto done; } diff --git a/lib/dns/zonefetch.c b/lib/dns/zonefetch.c index 988f4a6bfa..ea3fa3099c 100644 --- a/lib/dns/zonefetch.c +++ b/lib/dns/zonefetch.c @@ -187,3 +187,53 @@ cleanup: } } } + +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; +} From 77418fedcef39715b7f9e7745555f1f9e2405889 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 4 Nov 2025 17:17:13 +0100 Subject: [PATCH 5/6] Fix comment in lib/ns/query.c While renaming exit_check() to dns__zone_free_check() in lib/dns/zone.c, a dead reference to exit_check() in the comments was found in lib/ns/query.c. --- lib/ns/query.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ns/query.c b/lib/ns/query.c index 2a8f587bbf..8733e0938d 100644 --- a/lib/ns/query.c +++ b/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 = From 868ede012cc21918d4961ccc94b256f12c46902a Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 6 Nov 2025 10:41:50 +0100 Subject: [PATCH 6/6] Schedule a zonefetch Scheduling and rescheduling a zonefetch is also similar. Refactor into zonefetch functions. This also increments and decrements the zone's internal reference counter in the same module, which may be less confusing when reading the code. --- lib/dns/include/dns/zonefetch.h | 26 ++++++++++++++++++++++++++ lib/dns/zone.c | 29 ++++++++--------------------- lib/dns/zonefetch.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/lib/dns/include/dns/zonefetch.h b/lib/dns/include/dns/zonefetch.h index 01f189b81d..2ea5108281 100644 --- a/lib/dns/include/dns/zonefetch.h +++ b/lib/dns/include/dns/zonefetch.h @@ -106,6 +106,32 @@ dns_zonefetch_done(void *arg); * 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); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index cf7b2bed68..a9acedda42 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -11332,7 +11332,7 @@ zone_refreshkeys(dns_zone_t *zone) { result = dns_rriterator_nextrrset(&rrit)) { isc_stdtime_t timer = 0xffffffff; - dns_name_t *name = NULL, *kname = NULL; + dns_name_t *name = NULL; dns_rdataset_t *kdset = NULL; uint32_t ttl; @@ -11411,28 +11411,22 @@ zone_refreshkeys(dns_zone_t *zone) { isc_mem_attach(zone->mctx, &fetch->mctx); zone->fetchcount[ZONEFETCHTYPE_KEY]++; - isc_refcount_increment0(&zone->irefs); - kname = dns_fixedname_initname(&fetch->name); - dns_name_dup(name, zone->mctx, kname); - dns_rdataset_init(&fetch->rrset); - dns_rdataset_init(&fetch->sigset); kfetch = &fetch->fetchdata.keyfetch; dns_rdataset_init(&kfetch->keydataset); dns_rdataset_clone(kdset, &kfetch->keydataset); dns_db_attach(db, &kfetch->db); + dns_zonefetch_schedule(fetch, name); + if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(kname, namebuf, - sizeof(namebuf)); + dns_name_format(name, namebuf, sizeof(namebuf)); dnssec_log(zone, ISC_LOG_DEBUG(3), "Creating key fetch in " "zone_refreshkeys() for '%s'", namebuf); } - - isc_async_run(zone->loop, dns_zonefetch_run, fetch); fetching = true; #ifdef ENABLE_AFL } @@ -21112,16 +21106,14 @@ nsfetch_continue(dns_zonefetch_t *fetch) { #endif /* ifdef ENABLE_AFL */ LOCK_ZONE(zone); zone->fetchcount[ZONEFETCHTYPE_NS]++; - isc_refcount_increment0(&zone->irefs); - dns_rdataset_init(&fetch->rrset); - dns_rdataset_init(&fetch->sigset); + dns_zonefetch_reschedule(fetch); + if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { dnssec_log(zone, ISC_LOG_DEBUG(3), "Creating parent NS fetch in " "nsfetch_continue()"); } - isc_async_run(zone->loop, dns_zonefetch_run, fetch); UNLOCK_ZONE(zone); #ifdef ENABLE_AFL } @@ -21303,7 +21295,6 @@ zone_checkds(dns_zone_t *zone) { #endif /* ifdef ENABLE_AFL */ dns_zonefetch_t *fetch = NULL; dns_nsfetch_t *nsfetch = NULL; - dns_name_t *name = NULL; fetch = isc_mem_get(zone->mctx, sizeof(dns_zonefetch_t)); *fetch = (dns_zonefetch_t){ @@ -21324,22 +21315,18 @@ zone_checkds(dns_zone_t *zone) { LOCK_ZONE(zone); zone->fetchcount[ZONEFETCHTYPE_NS]++; - isc_refcount_increment0(&zone->irefs); - name = dns_fixedname_initname(&fetch->name); - dns_name_dup(&zone->origin, zone->mctx, name); - dns_rdataset_init(&fetch->rrset); - dns_rdataset_init(&fetch->sigset); nsfetch = &fetch->fetchdata.nsfetch; dns_name_init(&nsfetch->pname); dns_name_clone(&zone->origin, &nsfetch->pname); + dns_zonefetch_schedule(fetch, &zone->origin); + if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { dnssec_log( zone, ISC_LOG_DEBUG(3), "Creating parent NS fetch in zone_checkds()"); } - isc_async_run(zone->loop, dns_zonefetch_run, fetch); UNLOCK_ZONE(zone); #ifdef ENABLE_AFL } diff --git a/lib/dns/zonefetch.c b/lib/dns/zonefetch.c index ea3fa3099c..c58cae5183 100644 --- a/lib/dns/zonefetch.c +++ b/lib/dns/zonefetch.c @@ -13,6 +13,7 @@ /*! \file */ +#include #include #include @@ -188,6 +189,38 @@ cleanup: } } +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) {