From 5e1df53d05e2142464e0bb5fa81ab0034d1a1864 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 26 Feb 2025 13:47:40 -0800 Subject: [PATCH 01/12] simplify dns_ncache_add() there's no longer any reason to have both dns_ncache_add() and dns_ncache_addoptout(). --- lib/dns/include/dns/ncache.h | 20 +++++++++----------- lib/dns/ncache.c | 34 +++++++--------------------------- lib/dns/resolver.c | 14 ++++---------- 3 files changed, 20 insertions(+), 48 deletions(-) diff --git a/lib/dns/include/dns/ncache.h b/lib/dns/include/dns/ncache.h index 979de11621..ae187668c8 100644 --- a/lib/dns/include/dns/ncache.h +++ b/lib/dns/include/dns/ncache.h @@ -54,26 +54,24 @@ isc_result_t dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl, - dns_ttl_t maxttl, dns_rdataset_t *addedrdataset); -isc_result_t -dns_ncache_addoptout(dns_message_t *message, dns_db_t *cache, - dns_dbnode_t *node, dns_rdatatype_t covers, - isc_stdtime_t now, dns_ttl_t minttl, dns_ttl_t maxttl, - bool optout, dns_rdataset_t *addedrdataset); + dns_ttl_t maxttl, bool optout, bool secure, + dns_rdataset_t *addedrdataset); /*%< * Convert the authority data from 'message' into a negative cache * rdataset, and store it in 'cache' at 'node' with a TTL limited to * 'maxttl'. * - * \li dns_ncache_add produces a negative cache entry with a trust of no - * more than answer - * \li dns_ncache_addoptout produces a negative cache entry which will have - * a trust of secure if all the records that make up the entry are secure. + * \li If 'secure' is true and all the records that make up the entry + * are secure, then dns_ncache_add produces a negative cache entry + * with trust level secure. + * \li If 'secure' is false, the negative cache entry's trust level + * will be capped at answer. * * The 'covers' argument is the RR type whose nonexistence we are caching, * or dns_rdatatype_any when caching a NXDOMAIN response. * - * 'optout' parameter indicates if 'optout' attribute should be set. + * 'optout' parameter indicates if 'optout' attribute should be set. This only + * applies in secure zones; if 'secure' is false, 'optout' is ignored. * * Note: *\li If 'addedrdataset' is not NULL, then it will be attached to the added diff --git a/lib/dns/ncache.c b/lib/dns/ncache.c index 0fa3e7dac3..e3d2ad3341 100644 --- a/lib/dns/ncache.c +++ b/lib/dns/ncache.c @@ -50,12 +50,6 @@ atomic_getuint8(isc_buffer_t *b) { return ret; } -static isc_result_t -addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, - dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl, - dns_ttl_t maxttl, bool optout, bool secure, - dns_rdataset_t *addedrdataset); - static isc_result_t copy_rdataset(dns_rdataset_t *rdataset, isc_buffer_t *buffer) { unsigned int count; @@ -102,25 +96,8 @@ copy_rdataset(dns_rdataset_t *rdataset, isc_buffer_t *buffer) { isc_result_t dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl, - dns_ttl_t maxttl, dns_rdataset_t *addedrdataset) { - return addoptout(message, cache, node, covers, now, minttl, maxttl, - false, false, addedrdataset); -} - -isc_result_t -dns_ncache_addoptout(dns_message_t *message, dns_db_t *cache, - dns_dbnode_t *node, dns_rdatatype_t covers, - isc_stdtime_t now, dns_ttl_t minttl, dns_ttl_t maxttl, - bool optout, dns_rdataset_t *addedrdataset) { - return addoptout(message, cache, node, covers, now, minttl, maxttl, - optout, true, addedrdataset); -} - -static isc_result_t -addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, - dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl, - dns_ttl_t maxttl, bool optout, bool secure, - dns_rdataset_t *addedrdataset) { + dns_ttl_t maxttl, bool optout, bool secure, + dns_rdataset_t *addedrdataset) { isc_buffer_t buffer; isc_region_t r; dns_rdatatype_t type; @@ -135,14 +112,17 @@ addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, /* * Convert the authority data from 'message' into a negative cache * rdataset, and store it in 'cache' at 'node'. + * + * We assume that all data in the authority section has been + * validated by the caller. */ REQUIRE(message != NULL); /* - * We assume that all data in the authority section has been - * validated by the caller. + * If 'secure' is false, ignore 'optout'. */ + optout = optout && secure; /* * Initialize the list. diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index dd61162d9a..fdc919c3fc 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -6341,8 +6341,7 @@ cleanup: } /* - * Do what dns_ncache_addoptout() does, and then compute an appropriate - * eresult. + * Call dns_ncache_add() and then compute an appropriate eresult. */ static isc_result_t ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, @@ -6356,14 +6355,9 @@ ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, dns_rdataset_init(&rdataset); ardataset = &rdataset; } - if (secure) { - result = dns_ncache_addoptout(message, cache, node, covers, now, - minttl, maxttl, optout, - ardataset); - } else { - result = dns_ncache_add(message, cache, node, covers, now, - minttl, maxttl, ardataset); - } + + result = dns_ncache_add(message, cache, node, covers, now, minttl, + maxttl, optout, secure, ardataset); if (result == DNS_R_UNCHANGED || result == ISC_R_SUCCESS) { /* * If the cache now contains a negative entry and we From 5d56df23f2310cebb9fb8c87de3899406ed62ed2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 26 Feb 2025 21:30:43 -0800 Subject: [PATCH 02/12] split out cookie checks from resquery_response_continue() split the code section that handles cookie issues into a separate function for better readablity. --- lib/dns/resolver.c | 151 ++++++++++++++++++++++++++------------------- 1 file changed, 86 insertions(+), 65 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index fdc919c3fc..dbd446e5d6 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -5933,7 +5933,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } /* - * Enforce the configure maximum cache TTL. + * Enforce the configured maximum cache TTL. */ if (rdataset->ttl > res->view->maxcachettl) { rdataset->ttl = res->view->maxcachettl; @@ -5954,7 +5954,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } /* - * Find the SIG for this rdataset, if we have it. + * Find the RRSIG for this rdataset, if we have it. */ ISC_LIST_FOREACH (name->list, sig, link) { if (sig->type == dns_rdatatype_rrsig && @@ -7525,6 +7525,83 @@ cleanup: isc_mem_putanddetach(&rctx->mctx, rctx, sizeof(*rctx)); } +static isc_result_t +rctx_cookiecheck(respctx_t *rctx) { + fetchctx_t *fctx = rctx->fctx; + resquery_t *query = rctx->query; + + /* + * If TSIG signed, sent via TCP, or cookie present, + * no need to continue. + */ + if (dns_message_gettsig(query->rmessage, NULL) != NULL || + query->rmessage->cc_ok || query->rmessage->cc_bad || + (rctx->retryopts & DNS_FETCHOPT_TCP) != 0) + { + return ISC_R_SUCCESS; + } + + /* + * If we've had a cookie from the same server previously, + * retry with TCP. This may be a misconfigured anycast server + * or an attempt to send a spoofed response. + */ + if (dns_adb_getcookie(query->addrinfo, NULL, 0) > CLIENT_COOKIE_SIZE) { + if (isc_log_wouldlog(ISC_LOG_INFO)) { + char addrbuf[ISC_SOCKADDR_FORMATSIZE]; + isc_sockaddr_format(&query->addrinfo->sockaddr, addrbuf, + sizeof(addrbuf)); + isc_log_write(DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, + "missing expected cookie " + "from %s", + addrbuf); + } + rctx->retryopts |= DNS_FETCHOPT_TCP; + rctx->resend = true; + rctx_done(rctx, ISC_R_SUCCESS); + return ISC_R_COMPLETE; + } + + /* + * Retry over TCP if require-cookie is true. + */ + if (fctx->res->view->peers != NULL) { + isc_result_t result; + dns_peer_t *peer = NULL; + bool required = false; + isc_netaddr_t netaddr; + + isc_netaddr_fromsockaddr(&netaddr, &query->addrinfo->sockaddr); + result = dns_peerlist_peerbyaddr(fctx->res->view->peers, + &netaddr, &peer); + if (result == ISC_R_SUCCESS) { + dns_peer_getrequirecookie(peer, &required); + } + if (!required) { + return ISC_R_SUCCESS; + } + + if (isc_log_wouldlog(ISC_LOG_INFO)) { + char addrbuf[ISC_SOCKADDR_FORMATSIZE]; + isc_sockaddr_format(&query->addrinfo->sockaddr, addrbuf, + sizeof(addrbuf)); + isc_log_write(DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, + "missing required " + "cookie from %s", + addrbuf); + } + + rctx->retryopts |= DNS_FETCHOPT_TCP; + rctx->resend = true; + rctx_done(rctx, ISC_R_SUCCESS); + return ISC_R_COMPLETE; + } + + return ISC_R_SUCCESS; +} + static void resquery_response_continue(void *arg, isc_result_t result) { respctx_t *rctx = arg; @@ -7551,72 +7628,16 @@ resquery_response_continue(void *arg, isc_result_t result) { INSIST((query->rmessage->flags & DNS_MESSAGEFLAG_QR) != 0); /* - * If we have had a server cookie and don't get one retry over - * TCP. This may be a misconfigured anycast server or an attempt - * to send a spoofed response. Additionally retry over TCP if - * require-cookie is true and we don't have a got client cookie. - * Skip if we have a valid TSIG. + * Check for cookie issues. */ - if (dns_message_gettsig(query->rmessage, NULL) == NULL && - !query->rmessage->cc_ok && !query->rmessage->cc_bad && - (rctx->retryopts & DNS_FETCHOPT_TCP) == 0) - { - if (dns_adb_getcookie(query->addrinfo, NULL, 0) > - CLIENT_COOKIE_SIZE) - { - if (isc_log_wouldlog(ISC_LOG_INFO)) { - char addrbuf[ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_format(&query->addrinfo->sockaddr, - addrbuf, sizeof(addrbuf)); - isc_log_write(DNS_LOGCATEGORY_RESOLVER, - DNS_LOGMODULE_RESOLVER, - ISC_LOG_INFO, - "missing expected cookie " - "from %s", - addrbuf); - } - rctx->retryopts |= DNS_FETCHOPT_TCP; - rctx->resend = true; - rctx_done(rctx, result); - goto cleanup; - } else if (fctx->res->view->peers != NULL) { - dns_peer_t *peer = NULL; - isc_netaddr_t netaddr; - isc_netaddr_fromsockaddr(&netaddr, - &query->addrinfo->sockaddr); - result = dns_peerlist_peerbyaddr(fctx->res->view->peers, - &netaddr, &peer); - if (result == ISC_R_SUCCESS) { - bool required = false; - result = dns_peer_getrequirecookie(peer, - &required); - if (result == ISC_R_SUCCESS && required) { - if (isc_log_wouldlog(ISC_LOG_INFO)) { - char addrbuf - [ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_format( - &query->addrinfo - ->sockaddr, - addrbuf, - sizeof(addrbuf)); - isc_log_write( - DNS_LOGCATEGORY_RESOLVER, - DNS_LOGMODULE_RESOLVER, - ISC_LOG_INFO, - "missing required " - "cookie " - "from %s", - addrbuf); - } - rctx->retryopts |= DNS_FETCHOPT_TCP; - rctx->resend = true; - rctx_done(rctx, result); - goto cleanup; - } - } - } + result = rctx_cookiecheck(rctx); + if (result == ISC_R_COMPLETE) { + goto cleanup; } + /* + * Check for EDNS issues. + */ rctx_edns(rctx); /* From 7371c4882a97dcc4f423fedc67b32b43183f5668 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 26 Feb 2025 22:06:40 -0800 Subject: [PATCH 03/12] change issecuredomain() functions to bool dns_keytable_issecuredomain() and dns_view_issecuredomain() previously returned a result code to inform the caller of unexpected database failures when looking up names in the keytable and/or NTA table. such failures are not actually possible. both functions now return a simple bool. also, dns_view_issecuredomain() now returns false if view->enablevalidation is false, so the caller no longer has to check for that. --- lib/dns/include/dns/keytable.h | 19 +++-------- lib/dns/include/dns/view.h | 11 ++----- lib/dns/keytable.c | 14 +++----- lib/dns/resolver.c | 38 ++++++---------------- lib/dns/view.c | 24 ++++---------- tests/dns/keytable_test.c | 58 +++++++++++----------------------- 6 files changed, 49 insertions(+), 115 deletions(-) diff --git a/lib/dns/include/dns/keytable.h b/lib/dns/include/dns/keytable.h index 689aab7e9d..9213e99b19 100644 --- a/lib/dns/include/dns/keytable.h +++ b/lib/dns/include/dns/keytable.h @@ -205,9 +205,9 @@ dns_keytable_finddeepestmatch(dns_keytable_t *keytable, const dns_name_t *name, *\li Any other result indicates an error. */ -isc_result_t +bool dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name, - dns_name_t *foundname, bool *wantdnssecp); + dns_name_t *foundname); /*%< * Is 'name' at or beneath a trusted key? * @@ -219,20 +219,11 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name, * *\li 'foundanme' is NULL or is a pointer to an initialized dns_name_t * - *\li '*wantsdnssecp' is a valid bool. - * * Ensures: * - *\li On success, *wantsdnssecp will be true if and only if 'name' - * is at or beneath a trusted key. If 'foundname' is not NULL, then - * it will be updated to contain the name of the closest enclosing - * trust anchor. - * - * Returns: - * - *\li ISC_R_SUCCESS - * - *\li Any other result is an error. + *\li Returns true if and only if 'name' is at or beneath a trusted key. + * If 'foundname' is not NULL, then it will be updated to contain + * the name of the closest enclosing trust anchor. */ isc_result_t diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 9f7c0d43f9..d536cccbd3 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -985,13 +985,12 @@ dns_view_getsecroots(dns_view_t *view, dns_keytable_t **ktp); *\li ISC_R_NOTFOUND */ -isc_result_t +bool dns_view_issecuredomain(dns_view_t *view, const dns_name_t *name, - isc_stdtime_t now, bool checknta, bool *ntap, - bool *secure_domain); + isc_stdtime_t now, bool checknta, bool *ntap); /*%< * Is 'name' at or beneath a trusted key, and not covered by a valid - * negative trust anchor? Put answer in '*secure_domain'. + * negative trust anchor, and DNSSEC validation is enabled? * * If 'checknta' is false, ignore the NTA table in determining * whether this is a secure domain. If 'checknta' is not false, and if @@ -1000,10 +999,6 @@ dns_view_issecuredomain(dns_view_t *view, const dns_name_t *name, * * Requires: * \li 'view' is valid. - * - * Returns: - *\li ISC_R_SUCCESS - *\li Any other value indicates failure */ bool diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 018b4f73e0..54b7a5633f 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -524,13 +524,14 @@ dns_keytable_finddeepestmatch(dns_keytable_t *keytable, const dns_name_t *name, return result; } -isc_result_t +bool dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name, - dns_name_t *foundname, bool *wantdnssecp) { + dns_name_t *foundname) { isc_result_t result; dns_qpread_t qpr; dns_keynode_t *keynode = NULL; void *pval = NULL; + bool secure = false; /* * Is 'name' at or beneath a trusted key? @@ -538,7 +539,6 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name, REQUIRE(VALID_KEYTABLE(keytable)); REQUIRE(dns_name_isabsolute(name)); - REQUIRE(wantdnssecp != NULL); dns_qpmulti_query(keytable->table, &qpr); result = dns_qp_lookup(&qpr, name, DNS_DBNAMESPACE_NORMAL, NULL, NULL, @@ -548,16 +548,12 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name, if (foundname != NULL) { dns_name_copy(&keynode->name, foundname); } - *wantdnssecp = true; - result = ISC_R_SUCCESS; - } else if (result == ISC_R_NOTFOUND) { - *wantdnssecp = false; - result = ISC_R_SUCCESS; + secure = true; } dns_qpread_destroy(keytable->table, &qpr); - return result; + return secure; } static isc_result_t diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index dbd446e5d6..62760a52ea 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2236,9 +2236,9 @@ compute_cc(const resquery_t *query, uint8_t *cookie, const size_t len) { memmove(cookie, digest, CLIENT_COOKIE_SIZE); } -static isc_result_t +static bool issecuredomain(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, - isc_stdtime_t now, bool checknta, bool *ntap, bool *issecure) { + isc_stdtime_t now, bool checknta, bool *ntap) { dns_name_t suffix; unsigned int labels; @@ -2255,8 +2255,7 @@ issecuredomain(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, name = &suffix; } - return dns_view_issecuredomain(view, name, now, checknta, ntap, - issecure); + return dns_view_issecuredomain(view, name, now, checknta, ntap); } static isc_result_t @@ -5846,13 +5845,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, checknta = false; } - if (res->view->enablevalidation) { - result = issecuredomain(res->view, name, fctx->type, now, - checknta, NULL, &secure_domain); - if (result != ISC_R_SUCCESS) { - return result; - } - } + secure_domain = issecuredomain(res->view, name, fctx->type, now, + checknta, NULL); if ((fctx->options & DNS_FETCHOPT_NOCDFLAG) != 0) { valoptions |= DNS_VALIDATOR_NOCDFLAG; @@ -6436,13 +6430,8 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, checknta = false; } - if (fctx->res->view->enablevalidation) { - result = issecuredomain(res->view, name, fctx->type, now, - checknta, NULL, &secure_domain); - if (result != ISC_R_SUCCESS) { - return result; - } - } + secure_domain = issecuredomain(res->view, name, fctx->type, now, + checknta, NULL); if ((fctx->options & DNS_FETCHOPT_NOCDFLAG) != 0) { valoptions |= DNS_VALIDATOR_NOCDFLAG; @@ -9029,7 +9018,6 @@ rctx_ncache(respctx_t *rctx) { */ static isc_result_t rctx_authority_dnssec(respctx_t *rctx) { - isc_result_t result; fetchctx_t *fctx = rctx->fctx; dns_message_t *msg = rctx->query->rmessage; @@ -9112,15 +9100,9 @@ rctx_authority_dnssec(respctx_t *rctx) { if ((fctx->options & DNS_FETCHOPT_NONTA) != 0) { checknta = false; } - if (fctx->res->view->enablevalidation) { - result = issecuredomain( - fctx->res->view, name, - dns_rdatatype_ds, fctx->now, - checknta, NULL, &secure_domain); - if (result != ISC_R_SUCCESS) { - return result; - } - } + secure_domain = issecuredomain( + fctx->res->view, name, dns_rdatatype_ds, + fctx->now, checknta, NULL); if (secure_domain) { rdataset->trust = dns_trust_pending_answer; diff --git a/lib/dns/view.c b/lib/dns/view.c index 4193a43b9e..662eecf3e1 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -1535,41 +1535,31 @@ dns_view_ntacovers(dns_view_t *view, isc_stdtime_t now, const dns_name_t *name, return dns_ntatable_covered(view->ntatable_priv, now, name, anchor); } -isc_result_t +bool dns_view_issecuredomain(dns_view_t *view, const dns_name_t *name, - isc_stdtime_t now, bool checknta, bool *ntap, - bool *secure_domain) { - isc_result_t result; + isc_stdtime_t now, bool checknta, bool *ntap) { bool secure = false; dns_fixedname_t fn; dns_name_t *anchor; REQUIRE(DNS_VIEW_VALID(view)); - if (view->secroots_priv == NULL) { - return ISC_R_NOTFOUND; + if (!view->enablevalidation || view->secroots_priv == NULL) { + return false; } anchor = dns_fixedname_initname(&fn); - - result = dns_keytable_issecuredomain(view->secroots_priv, name, anchor, - &secure); - if (result != ISC_R_SUCCESS) { - return result; - } + secure = dns_keytable_issecuredomain(view->secroots_priv, name, anchor); SET_IF_NOT_NULL(ntap, false); if (checknta && secure && view->ntatable_priv != NULL && dns_ntatable_covered(view->ntatable_priv, now, name, anchor)) { - if (ntap != NULL) { - *ntap = true; - } + SET_IF_NOT_NULL(ntap, true); secure = false; } - *secure_domain = secure; - return ISC_R_SUCCESS; + return secure; } void diff --git a/tests/dns/keytable_test.c b/tests/dns/keytable_test.c index f04c720241..0606334c40 100644 --- a/tests/dns/keytable_test.c +++ b/tests/dns/keytable_test.c @@ -544,7 +544,6 @@ ISC_LOOP_TEST_IMPL(find) { /* check issecuredomain() */ ISC_LOOP_TEST_IMPL(issecuredomain) { - bool issecure; const char **n; const char *names[] = { "example.com", "sub.example.com", "null.example", "sub.null.example", NULL }; @@ -559,22 +558,16 @@ ISC_LOOP_TEST_IMPL(issecuredomain) { * of installing a null key). */ for (n = names; *n != NULL; n++) { - assert_int_equal(dns_keytable_issecuredomain(keytable, - str2name(*n), NULL, - &issecure), - ISC_R_SUCCESS); - assert_true(issecure); + assert_true(dns_keytable_issecuredomain(keytable, str2name(*n), + NULL)); } /* * If the key table has no entry (not even a null one) for a domain or * any of its ancestors, that domain is considered insecure. */ - assert_int_equal(dns_keytable_issecuredomain(keytable, - str2name("example.org"), - NULL, &issecure), - ISC_R_SUCCESS); - assert_false(issecure); + assert_false(dns_keytable_issecuredomain( + keytable, str2name("example.org"), NULL)); destroy_tables(); @@ -604,7 +597,7 @@ ISC_LOOP_TEST_IMPL(dump) { /* check negative trust anchors */ ISC_LOOP_TEST_IMPL(nta) { isc_result_t result; - bool issecure, covered; + bool covered; dns_fixedname_t fn; dns_name_t *keyname = dns_fixedname_name(&fn); unsigned char digest[DNS_DS_BUFFERSIZE]; @@ -636,20 +629,15 @@ ISC_LOOP_TEST_IMPL(nta) { assert_int_equal(result, ISC_R_SUCCESS); /* Should be secure */ - result = dns_view_issecuredomain(myview, - str2name("test.secure.example"), now, - true, &covered, &issecure); - assert_int_equal(result, ISC_R_SUCCESS); + assert_true(dns_view_issecuredomain( + myview, str2name("test.secure.example"), now, true, &covered)); assert_false(covered); - assert_true(issecure); /* Should not be secure */ - result = dns_view_issecuredomain(myview, - str2name("test.insecure.example"), now, - true, &covered, &issecure); - assert_int_equal(result, ISC_R_SUCCESS); + assert_false(dns_view_issecuredomain(myview, + str2name("test.insecure.example"), + now, true, &covered)); assert_true(covered); - assert_false(issecure); /* NTA covered */ covered = dns_view_ntacovers(myview, now, str2name("insecure.example"), @@ -662,38 +650,30 @@ ISC_LOOP_TEST_IMPL(nta) { assert_false(covered); /* As of now + 2, the NTA should be clear */ - result = dns_view_issecuredomain(myview, - str2name("test.insecure.example"), - now + 2, true, &covered, &issecure); - assert_int_equal(result, ISC_R_SUCCESS); + assert_true(dns_view_issecuredomain(myview, + str2name("test.insecure.example"), + now + 2, true, &covered)); assert_false(covered); - assert_true(issecure); /* Now check deletion */ - result = dns_view_issecuredomain(myview, str2name("test.new.example"), - now, true, &covered, &issecure); - assert_int_equal(result, ISC_R_SUCCESS); + assert_true(dns_view_issecuredomain( + myview, str2name("test.new.example"), now, true, &covered)); assert_false(covered); - assert_true(issecure); result = dns_ntatable_add(ntatable, str2name("new.example"), false, now, 3600); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_view_issecuredomain(myview, str2name("test.new.example"), - now, true, &covered, &issecure); - assert_int_equal(result, ISC_R_SUCCESS); + assert_false(dns_view_issecuredomain( + myview, str2name("test.new.example"), now, true, &covered)); assert_true(covered); - assert_false(issecure); result = dns_ntatable_delete(ntatable, str2name("new.example")); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_view_issecuredomain(myview, str2name("test.new.example"), - now, true, &covered, &issecure); - assert_int_equal(result, ISC_R_SUCCESS); + assert_true(dns_view_issecuredomain( + myview, str2name("test.new.example"), now, true, &covered)); assert_false(covered); - assert_true(issecure); isc_loopmgr_shutdown(); From 51a4e00d1da97f34dfb70adc5057ff7da09fe4e8 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 27 Feb 2025 12:43:52 -0800 Subject: [PATCH 04/12] reduce steps for negative caching whenever ncache_adderesult() was called, some preparatory code was run first; this has now been moved into a single function negcache() to reduce code duplication. --- lib/dns/resolver.c | 284 ++++++++++++++++----------------------------- 1 file changed, 101 insertions(+), 183 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 62760a52ea..cdd15f1dc0 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -661,10 +661,9 @@ fctx_minimize_qname(fetchctx_t *fctx); static void fctx_destroy(fetchctx_t *fctx); static isc_result_t -ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, - dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl, - dns_ttl_t maxttl, bool optout, bool secure, - dns_rdataset_t *ardataset, isc_result_t *eresultp); +negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name, + isc_stdtime_t now, bool optout, bool secure, dns_rdataset_t *added, + dns_dbnode_t **nodep, isc_result_t *eresultp); static void validated(void *arg); static void @@ -5006,15 +5005,14 @@ static void clone_results(fetchctx_t *fctx) { dns_fetchresponse_t *hresp = NULL; - FCTXTRACE("clone_results"); + REQUIRE(!ISC_LIST_EMPTY(fctx->resps)); /* * Set up any other resps to have the same data as the first. - * * Caller must be holding the appropriate lock. */ - fctx->cloned = true; + FCTXTRACE("clone_results"); ISC_LIST_FOREACH (fctx->resps, resp, link) { /* This is the head resp; keep a pointer and move on */ @@ -5030,22 +5028,21 @@ clone_results(fetchctx_t *fctx) { dns_db_attach(hresp->db, &resp->db); dns_db_attachnode(hresp->db, hresp->node, &resp->node); - INSIST(hresp->rdataset != NULL); - INSIST(resp->rdataset != NULL); + INSIST(hresp->rdataset != NULL && resp->rdataset != NULL); if (dns_rdataset_isassociated(hresp->rdataset)) { dns_rdataset_clone(hresp->rdataset, resp->rdataset); } - INSIST(!(hresp->sigrdataset == NULL && - resp->sigrdataset != NULL)); - if (hresp->sigrdataset != NULL && - dns_rdataset_isassociated(hresp->sigrdataset) && - resp->sigrdataset != NULL) + INSIST(hresp->sigrdataset != NULL || resp->sigrdataset == NULL); + if (resp->sigrdataset != NULL && hresp->sigrdataset != NULL && + dns_rdataset_isassociated(hresp->sigrdataset)) { dns_rdataset_clone(hresp->sigrdataset, resp->sigrdataset); } } + + fctx->cloned = true; } #define CACHE(r) (((r)->attributes.cache)) @@ -5187,7 +5184,6 @@ validated(void *arg) { isc_result_t eresult = ISC_R_SUCCESS; isc_result_t result = ISC_R_SUCCESS; isc_stdtime_t now; - uint32_t ttl; unsigned int options; dns_fixedname_t fwild; dns_name_t *wild = NULL; @@ -5355,57 +5351,12 @@ validated(void *arg) { } if (negative) { - dns_rdatatype_t covers; FCTXTRACE("nonexistence validation OK"); inc_stats(res, dns_resstatscounter_valnegsuccess); - /* - * Cache DS NXDOMAIN separately to other types. - */ - if (message->rcode == dns_rcode_nxdomain && - fctx->type != dns_rdatatype_ds) - { - covers = dns_rdatatype_any; - } else { - covers = fctx->type; - } - - /* - * Don't report qname minimisation NXDOMAIN errors - * when the result is NXDOMAIN except we have already - * confirmed a higher error. - */ - if (!fctx->force_qmin_warning && - message->rcode == dns_rcode_nxdomain && - (fctx->qmin_warning == DNS_R_NXDOMAIN || - fctx->qmin_warning == DNS_R_NCACHENXDOMAIN)) - { - fctx->qmin_warning = ISC_R_SUCCESS; - } - - result = dns_db_findnode(fctx->cache, val->name, true, &node); - if (result != ISC_R_SUCCESS) { - /* fctx->lock unlocked in noanswer_response */ - goto noanswer_response; - } - - /* - * If we are asking for a SOA record set the cache time - * to zero to facilitate locating the containing zone of - * a arbitrary zone. - */ - ttl = res->view->maxncachettl; - if (fctx->type == dns_rdatatype_soa && - covers == dns_rdatatype_any && res->zero_no_soa_ttl) - { - ttl = 0; - } - - result = ncache_adderesult(message, fctx->cache, node, covers, - now, fctx->res->view->minncachettl, - ttl, val->optout, val->secure, - ardataset, &eresult); + result = negcache(message, fctx, val->name, now, val->optout, + val->secure, ardataset, &node, &eresult); if (result != ISC_R_SUCCESS) { goto noanswer_response; } @@ -5818,8 +5769,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, dns_rdataset_t *addedrdataset = NULL; dns_rdataset_t *ardataset = NULL, *asigrdataset = NULL; dns_rdataset_t *valrdataset = NULL, *valsigrdataset = NULL; - dns_dbnode_t *node = NULL, **anodep = NULL; - dns_db_t **adbp = NULL; + dns_dbnode_t *node = NULL; dns_resolver_t *res = fctx->res; bool need_validation = false; bool secure_domain = false; @@ -5861,12 +5811,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, if (name->attributes.answer && !need_validation) { have_answer = true; resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL) { - adbp = &resp->db; - dns_name_copy(name, resp->foundname); - anodep = &resp->node; - /* * If this is an ANY, SIG or RRSIG query, we're * not going to return any rdatasets, unless we @@ -6286,14 +6231,9 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } } resp->result = eresult; - if (adbp != NULL && *adbp != NULL) { - if (anodep != NULL && *anodep != NULL) { - dns_db_detachnode(*adbp, anodep); - } - dns_db_detach(adbp); - } - dns_db_attach(fctx->cache, adbp); - dns_db_transfernode(fctx->cache, &node, anodep); + dns_name_copy(name, resp->foundname); + dns_db_attach(fctx->cache, &resp->db); + dns_db_transfernode(fctx->cache, &node, &resp->node); clone_results(fctx); } } @@ -6338,77 +6278,104 @@ cleanup: * Call dns_ncache_add() and then compute an appropriate eresult. */ static isc_result_t -ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, - dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t minttl, - dns_ttl_t maxttl, bool optout, bool secure, - dns_rdataset_t *ardataset, isc_result_t *eresultp) { +negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name, + isc_stdtime_t now, bool optout, bool secure, dns_rdataset_t *added, + dns_dbnode_t **nodep, isc_result_t *eresultp) { isc_result_t result; - dns_rdataset_t rdataset; + dns_ttl_t minttl = fctx->res->view->minncachettl; + dns_ttl_t maxttl = fctx->res->view->maxncachettl; + dns_rdatatype_t covers = fctx->type; + dns_db_t *cache = fctx->cache; + dns_dbnode_t *node = NULL; + dns_rdataset_t rdataset = DNS_RDATASET_INIT; - if (ardataset == NULL) { - dns_rdataset_init(&rdataset); - ardataset = &rdataset; + /* Set up a placeholder in case added was NULL */ + if (added == NULL) { + added = &rdataset; + } + + /* + * Cache DS NXDOMAIN separately to other types. + */ + if (message->rcode == dns_rcode_nxdomain && + fctx->type != dns_rdatatype_ds) + { + covers = dns_rdatatype_any; + } + + /* + * If the request was for an SOA record, set the cache time + * to zero to facilitate locating the containing zone of + * an arbitrary zone. + */ + if (fctx->type == dns_rdatatype_soa && covers == dns_rdatatype_any && + fctx->res->zero_no_soa_ttl) + { + maxttl = 0; + } + + /* + * Don't warn about QNAME minimization NXDOMAIN errors + * if the final result is NXDOMAIN anyway. + */ + if (!fctx->force_qmin_warning && message->rcode == dns_rcode_nxdomain && + (fctx->qmin_warning == DNS_R_NXDOMAIN || + fctx->qmin_warning == DNS_R_NCACHENXDOMAIN)) + { + fctx->qmin_warning = ISC_R_SUCCESS; + } + + /* + * Cache the negative entry. + */ + result = dns_db_findnode(fctx->cache, name, true, &node); + if (result != ISC_R_SUCCESS) { + return result; } result = dns_ncache_add(message, cache, node, covers, now, minttl, - maxttl, optout, secure, ardataset); + maxttl, optout, secure, added); if (result == DNS_R_UNCHANGED || result == ISC_R_SUCCESS) { /* - * If the cache now contains a negative entry and we - * care about whether it is DNS_R_NCACHENXDOMAIN or - * DNS_R_NCACHENXRRSET then extract it. + * The cache now either contains the negative entry we + * were adding, or a pre-existing entry (positive or + * negative) that was left alone. Set the event result + * accordingly. */ - if (NEGATIVE(ardataset)) { - /* - * The cache data is a negative cache entry. - */ - if (NXDOMAIN(ardataset)) { - *eresultp = DNS_R_NCACHENXDOMAIN; - } else { - *eresultp = DNS_R_NCACHENXRRSET; - } + if (NXDOMAIN(added)) { + *eresultp = DNS_R_NCACHENXDOMAIN; + } else if (NEGATIVE(added)) { + *eresultp = DNS_R_NCACHENXRRSET; + } else if (added->type == dns_rdatatype_cname) { + *eresultp = DNS_R_CNAME; + } else if (added->type == dns_rdatatype_dname) { + *eresultp = DNS_R_DNAME; } else { - /* - * The attempt to add a negative cache entry - * was rejected. Set *eresultp to reflect - * the type of the dataset being returned. - */ - switch (ardataset->type) { - case dns_rdatatype_cname: - *eresultp = DNS_R_CNAME; - break; - case dns_rdatatype_dname: - *eresultp = DNS_R_DNAME; - break; - default: - *eresultp = ISC_R_SUCCESS; - break; - } + *eresultp = ISC_R_SUCCESS; } result = ISC_R_SUCCESS; } - if (ardataset == &rdataset && dns_rdataset_isassociated(ardataset)) { - dns_rdataset_disassociate(ardataset); + + if (added == &rdataset && dns_rdataset_isassociated(added)) { + dns_rdataset_disassociate(added); } + *nodep = node; return result; } static isc_result_t ncache_message(fetchctx_t *fctx, dns_message_t *message, - dns_adbaddrinfo_t *addrinfo, dns_rdatatype_t covers, - isc_stdtime_t now) { + dns_adbaddrinfo_t *addrinfo, isc_stdtime_t now) { isc_result_t result, eresult = ISC_R_SUCCESS; dns_name_t *name = fctx->name; dns_resolver_t *res = fctx->res; - dns_db_t **adbp = NULL; - dns_dbnode_t *node = NULL, **anodep = NULL; - dns_rdataset_t *ardataset = NULL; + dns_dbnode_t *node = NULL; bool need_validation = false, secure_domain = false; dns_fetchresponse_t *resp = NULL; - uint32_t ttl; unsigned int valoptions = 0; bool checknta = true; + dns_rdataset_t *added = NULL; FCTXTRACE("ncache_message"); @@ -6473,63 +6440,26 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, if (!HAVE_ANSWER(fctx)) { resp = ISC_LIST_HEAD(fctx->resps); if (resp != NULL) { - adbp = &resp->db; - dns_name_copy(name, resp->foundname); - anodep = &resp->node; - ardataset = resp->rdataset; + added = resp->rdataset; } } - result = dns_db_findnode(fctx->cache, name, true, &node); - if (result != ISC_R_SUCCESS) { - goto unlock; - } - - /* - * Don't report qname minimisation NXDOMAIN errors - * when the result is NXDOMAIN except we have already - * confirmed a higher error. - */ - if (!fctx->force_qmin_warning && message->rcode == dns_rcode_nxdomain && - (fctx->qmin_warning == DNS_R_NXDOMAIN || - fctx->qmin_warning == DNS_R_NCACHENXDOMAIN)) - { - fctx->qmin_warning = ISC_R_SUCCESS; - } - - /* - * If we are asking for a SOA record set the cache time - * to zero to facilitate locating the containing zone of - * a arbitrary zone. - */ - ttl = fctx->res->view->maxncachettl; - if (fctx->type == dns_rdatatype_soa && covers == dns_rdatatype_any && - fctx->res->zero_no_soa_ttl) - { - ttl = 0; - } - - result = ncache_adderesult(message, fctx->cache, node, covers, now, - fctx->res->view->minncachettl, ttl, false, - false, ardataset, &eresult); + result = negcache(message, fctx, name, now, false, false, added, &node, + &eresult); if (result != ISC_R_SUCCESS) { goto unlock; } if (!HAVE_ANSWER(fctx)) { FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); - if (resp != NULL) { - resp->result = eresult; - if (adbp != NULL && *adbp != NULL) { - if (anodep != NULL && *anodep != NULL) { - dns_db_detachnode(*adbp, anodep); - } - dns_db_detach(adbp); - } - dns_db_attach(fctx->cache, adbp); - dns_db_transfernode(fctx->cache, &node, anodep); - clone_results(fctx); - } + } + + if (resp != NULL) { + resp->result = eresult; + dns_name_copy(name, resp->foundname); + dns_db_attach(fctx->cache, &resp->db); + dns_db_transfernode(fctx->cache, &node, &resp->node); + clone_results(fctx); } unlock: @@ -8982,29 +8912,17 @@ rctx_authority_negative(respctx_t *rctx) { static void rctx_ncache(respctx_t *rctx) { isc_result_t result; - dns_rdatatype_t covers; fetchctx_t *fctx = rctx->fctx; if (!WANTNCACHE(fctx)) { return; } - /* - * Cache DS NXDOMAIN separately to other types. - */ - if (rctx->query->rmessage->rcode == dns_rcode_nxdomain && - fctx->type != dns_rdatatype_ds) - { - covers = dns_rdatatype_any; - } else { - covers = fctx->type; - } - /* * Cache any negative cache entries in the message. */ result = ncache_message(fctx, rctx->query->rmessage, - rctx->query->addrinfo, covers, rctx->now); + rctx->query->addrinfo, rctx->now); if (result != ISC_R_SUCCESS) { FCTXTRACE3("ncache_message complete", result); } From 7841de08af11fa4ee48628ecbfdf4de38c8ae606 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 27 Feb 2025 17:10:21 -0800 Subject: [PATCH 05/12] add functions to match rdataset types - dns_rdataset_issigtype() returns true if the rdataset is of type RRSIG and covers a specified type - dns_rdataset_matchestype() returns true if the rdataset is of the specified type *or* the RRSIG covering it. --- lib/dns/include/dns/rdataset.h | 27 ++++++++++++++++++++++++ lib/dns/resolver.c | 38 ++++++++++++++-------------------- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/lib/dns/include/dns/rdataset.h b/lib/dns/include/dns/rdataset.h index 7bf6170586..5934d9b0ac 100644 --- a/lib/dns/include/dns/rdataset.h +++ b/lib/dns/include/dns/rdataset.h @@ -673,3 +673,30 @@ dns_rdataset_equals(const dns_rdataset_t *rdataset1, * \li 'rdataset1' is a valid rdataset. * \li 'rdataset2' is a valid rdataset. */ + +/*% + * Returns true if the rdataset is of type 'type', or type RRSIG + * and covers 'type'. + */ +static inline bool +dns_rdataset_matchestype(const dns_rdataset_t *rdataset, + const dns_rdatatype_t type) { + REQUIRE(DNS_RDATASET_VALID(rdataset)); + + return rdataset->type == type || + (rdataset->type == dns_rdatatype_rrsig && + rdataset->covers == type); +} + +/*% + * Returns true if the rdataset is of type 'type', or type RRSIG + * and covers 'type'. + */ +static inline bool +dns_rdataset_issigtype(const dns_rdataset_t *rdataset, + const dns_rdatatype_t type) { + REQUIRE(DNS_RDATASET_VALID(rdataset)); + + return rdataset->type == dns_rdatatype_rrsig && + rdataset->covers == type; +} diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index cdd15f1dc0..d2497ba373 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -5473,13 +5473,14 @@ answer_response: } ISC_LIST_FOREACH (name->list, s, link) { - if (s->type == dns_rdatatype_rrsig && - s->covers == rdataset->type) + if (dns_rdataset_issigtype(sigrdataset, + rdataset->type)) { sigrdataset = s; break; } } + if (sigrdataset == NULL || sigrdataset->trust != dns_trust_secure) { @@ -5674,7 +5675,7 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, * Find the SIG for this rdataset, if we have it. */ ISC_LIST_FOREACH (name->list, sig, link) { - if (sig->type == dns_rdatatype_rrsig && sig->covers == type) { + if (dns_rdataset_issigtype(sig, type)) { sigrdataset = sig; break; } @@ -5751,9 +5752,7 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, if (noqname != NULL) { ISC_LIST_FOREACH (noqname->list, sig, link) { - if (sig->type == dns_rdatatype_rrsig && - sig->covers == found) - { + if (dns_rdataset_issigtype(sig, found)) { *noqnamep = noqname; break; } @@ -5896,9 +5895,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, * Find the RRSIG for this rdataset, if we have it. */ ISC_LIST_FOREACH (name->list, sig, link) { - if (sig->type == dns_rdatatype_rrsig && - sig->covers == rdataset->type) - { + if (dns_rdataset_issigtype(sig, rdataset->type)) { sigrdataset = sig; break; } @@ -5927,14 +5924,13 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, continue; } + /* + * Ignore unrelated non-answer rdatasets that are + * missing signatures. + */ if (sigrdataset == NULL && need_validation && !ANSWER(rdataset)) { - /* - * Ignore unrelated non-answer - * rdatasets that are missing - * signatures. - */ continue; } @@ -6124,9 +6120,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } } if (rdataset->trust == dns_trust_glue && - (rdataset->type == dns_rdatatype_ns || - (rdataset->type == dns_rdatatype_rrsig && - rdataset->covers == dns_rdatatype_ns))) + dns_rdataset_matchestype(rdataset, + dns_rdatatype_ns)) { /* * If the trust level is @@ -8474,9 +8469,7 @@ rctx_answer_match(respctx_t *rctx) { return ISC_R_COMPLETE; } - if (sigrdataset->type != dns_rdatatype_rrsig || - sigrdataset->covers != rctx->type) - { + if (!dns_rdataset_issigtype(sigrdataset, rctx->type)) { continue; } @@ -8622,9 +8615,8 @@ rctx_authority_positive(respctx_t *rctx) { * nothing else. */ ISC_LIST_FOREACH (name->list, rdataset, link) { - if (rdataset->type == dns_rdatatype_ns || - (rdataset->type == dns_rdatatype_rrsig && - rdataset->covers == dns_rdatatype_ns)) + if (dns_rdataset_matchestype(rdataset, + dns_rdatatype_ns)) { name->attributes.cache = true; rdataset->attributes.cache = true; From c23cc105a1e65ae6f409d3bd0ee5e3446604ce99 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 27 Feb 2025 14:28:37 -0800 Subject: [PATCH 06/12] split out some functionality in cache_name() there are now separate functions to check the cacheability of an rdataset or to normalize TTLs, and the code to determine whether validation is necessary has been simplified. --- lib/dns/resolver.c | 467 +++++++++++++++++++++++---------------------- 1 file changed, 235 insertions(+), 232 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index d2497ba373..cd44a0dbdf 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -517,6 +517,8 @@ struct fetchctx { #define FCTX_ATTR_SET(f, a) atomic_fetch_or_release(&(f)->attributes, (a)) #define FCTX_ATTR_CLR(f, a) atomic_fetch_and_release(&(f)->attributes, ~(a)) +#define CHECKNTA(f) (((f)->options & DNS_FETCHOPT_NONTA) == 0) + typedef struct { dns_adbaddrinfo_t *addrinfo; fetchctx_t *fctx; @@ -959,9 +961,10 @@ set_stats(dns_resolver_t *res, isc_statscounter_t counter, uint64_t val) { static isc_result_t valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_t *rdataset, - dns_rdataset_t *sigrdataset, unsigned int valoptions) { + dns_rdataset_t *sigrdataset) { dns_validator_t *validator = NULL; dns_valarg_t *valarg = NULL; + unsigned int valoptions = 0; isc_result_t result; valarg = isc_mem_get(fctx->mctx, sizeof(*valarg)); @@ -971,6 +974,15 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, fetchctx_attach(fctx, &valarg->fctx); + /* Set up validator options */ + if ((fctx->options & DNS_FETCHOPT_NOCDFLAG) != 0) { + valoptions |= DNS_VALIDATOR_NOCDFLAG; + } + + if ((fctx->options & DNS_FETCHOPT_NONTA) != 0) { + valoptions |= DNS_VALIDATOR_NONTA; + } + if (!ISC_LIST_EMPTY(fctx->validators)) { valoptions |= DNS_VALIDATOR_DEFER; } else { @@ -5762,23 +5774,90 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, return result; } +static isc_result_t +check_cacheable(dns_name_t *name, dns_rdataset_t *rdataset, bool fail) { + /* This rdataset isn't marked for caching */ + if (!CACHE(rdataset)) { + return DNS_R_CONTINUE; + } + + /* See if there are any name errors */ + if (CHECKNAMES(rdataset)) { + char namebuf[DNS_NAME_FORMATSIZE]; + char typebuf[DNS_RDATATYPE_FORMATSIZE]; + char classbuf[DNS_RDATATYPE_FORMATSIZE]; + + dns_name_format(name, namebuf, sizeof(namebuf)); + dns_rdatatype_format(rdataset->type, typebuf, sizeof(typebuf)); + dns_rdataclass_format(rdataset->rdclass, classbuf, + sizeof(classbuf)); + isc_log_write(DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, + ISC_LOG_NOTICE, "check-names %s %s/%s/%s", + fail ? "failure" : "warning", namebuf, typebuf, + classbuf); + if (fail) { + if (ANSWER(rdataset)) { + return DNS_R_BADNAME; + } + + return DNS_R_CONTINUE; + } + } + + /* + * We do not attempt to cache or validate glue or out-of-bailiwick + * data - even if there might be some performance benefit to doing + * so - because it makes it simpler and safer. + */ + if (EXTERNAL(rdataset)) { + return DNS_R_CONTINUE; + } + + return ISC_R_SUCCESS; +} + +static void +fixttls(dns_view_t *view, dns_rdataset_t *rdataset, + dns_rdataset_t *sigrdataset) { + /* + * Enforce the configured maximum and minimum cache TTL. + */ + if (rdataset->ttl > view->maxcachettl) { + rdataset->ttl = view->maxcachettl; + } + + if (rdataset->ttl < view->mincachettl) { + rdataset->ttl = view->mincachettl; + } + + /* + * Mark the rdataset as being prefetch eligible. + */ + if (rdataset->ttl >= view->prefetch_eligible) { + rdataset->attributes.prefetch = true; + } + + /* + * Normalize the rdataset and sigrdataset TTLs. + */ + if (sigrdataset != NULL) { + rdataset->ttl = ISC_MIN(rdataset->ttl, sigrdataset->ttl); + sigrdataset->ttl = rdataset->ttl; + } +} + static isc_result_t cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, isc_stdtime_t now) { - dns_rdataset_t *addedrdataset = NULL; + dns_rdataset_t *added = NULL; + dns_rdataset_t *sigrdataset = NULL; dns_rdataset_t *ardataset = NULL, *asigrdataset = NULL; dns_rdataset_t *valrdataset = NULL, *valsigrdataset = NULL; dns_dbnode_t *node = NULL; dns_resolver_t *res = fctx->res; - bool need_validation = false; - bool secure_domain = false; bool have_answer = false; isc_result_t result, eresult = ISC_R_SUCCESS; dns_fetchresponse_t *resp = NULL; - unsigned int options; - bool fail; - unsigned int valoptions = 0; - bool checknta = true; FCTXTRACE("cache_name"); @@ -5789,23 +5868,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * Is DNSSEC validation required for this name? */ - if ((fctx->options & DNS_FETCHOPT_NONTA) != 0) { - valoptions |= DNS_VALIDATOR_NONTA; - checknta = false; - } - - secure_domain = issecuredomain(res->view, name, fctx->type, now, - checknta, NULL); - - if ((fctx->options & DNS_FETCHOPT_NOCDFLAG) != 0) { - valoptions |= DNS_VALIDATOR_NOCDFLAG; - } - - if ((fctx->options & DNS_FETCHOPT_NOVALIDATE) != 0) { - need_validation = false; - } else { - need_validation = secure_domain; - } + bool checknta = ((fctx->options & DNS_FETCHOPT_NONTA) == 0); + bool secure_domain = issecuredomain(res->view, name, fctx->type, now, + checknta, NULL); + bool need_validation = secure_domain && + ((fctx->options & DNS_FETCHOPT_NOVALIDATE) == 0); if (name->attributes.answer && !need_validation) { have_answer = true; @@ -5839,56 +5906,16 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * Cache or validate each cacheable rdataset. */ - fail = ((fctx->res->options & DNS_RESOLVER_CHECKNAMESFAIL) != 0); + bool fail = ((fctx->res->options & DNS_RESOLVER_CHECKNAMESFAIL) != 0); ISC_LIST_FOREACH (name->list, rdataset, link) { - dns_rdataset_t *sigrdataset = NULL; + unsigned int options = 0; - if (!CACHE(rdataset)) { + result = check_cacheable(name, rdataset, fail); + if (result == DNS_R_CONTINUE) { + result = ISC_R_SUCCESS; continue; - } - if (CHECKNAMES(rdataset)) { - char namebuf[DNS_NAME_FORMATSIZE]; - char typebuf[DNS_RDATATYPE_FORMATSIZE]; - char classbuf[DNS_RDATATYPE_FORMATSIZE]; - - dns_name_format(name, namebuf, sizeof(namebuf)); - dns_rdatatype_format(rdataset->type, typebuf, - sizeof(typebuf)); - dns_rdataclass_format(rdataset->rdclass, classbuf, - sizeof(classbuf)); - isc_log_write(DNS_LOGCATEGORY_RESOLVER, - DNS_LOGMODULE_RESOLVER, ISC_LOG_NOTICE, - "check-names %s %s/%s/%s", - fail ? "failure" : "warning", namebuf, - typebuf, classbuf); - if (fail) { - if (ANSWER(rdataset)) { - dns_db_detachnode(fctx->cache, &node); - return DNS_R_BADNAME; - } - continue; - } - } - - /* - * Enforce the configured maximum cache TTL. - */ - if (rdataset->ttl > res->view->maxcachettl) { - rdataset->ttl = res->view->maxcachettl; - } - - /* - * Enforce configured minimum cache TTL. - */ - if (rdataset->ttl < res->view->mincachettl) { - rdataset->ttl = res->view->mincachettl; - } - - /* - * Mark the rdataset as being prefetch eligible. - */ - if (rdataset->ttl >= fctx->res->view->prefetch_eligible) { - rdataset->attributes.prefetch = true; + } else if (result != ISC_R_SUCCESS) { + goto cleanup; } /* @@ -5902,20 +5929,23 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } /* - * If this RRset is in a secure domain, is in bailiwick, - * and is not glue, attempt DNSSEC validation. (We do - * not attempt to validate glue or out-of-bailiwick - * data--even though there might be some performance - * benefit to doing so--because it makes it simpler and - * safer to ensure that records from a secure domain are - * only cached if validated within the context of a - * query to the domain that owns them.) + * Make the TTLs consistent with the configured + * maximum and minimum and with each other. */ - if (secure_domain && rdataset->trust != dns_trust_glue && - !EXTERNAL(rdataset)) - { - dns_trust_t trust; + fixttls(res->view, rdataset, sigrdataset); + /* + * If this RRset is in a secure domain, is in bailiwick, + * and is not glue, attempt DNSSEC validation. + * + * We do not attempt to validate glue or out-of-bailiwick + * data - even though there might be some performance + * benefit to doing so - because it makes it simpler and + * safer to ensure that records from a secure domain are + * only cached if validated within the context of a query + * to the domain that owns them. + */ + if (secure_domain && rdataset->trust != dns_trust_glue) { /* * RRSIGs are validated as part of validating * the type they cover. @@ -5958,107 +5988,13 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, * query, additional should be cached as glue. */ if (rdataset->trust == dns_trust_additional) { - trust = dns_trust_pending_additional; + rdataset->trust = dns_trust_pending_additional; } else { - trust = dns_trust_pending_answer; + rdataset->trust = dns_trust_pending_answer; } - rdataset->trust = trust; if (sigrdataset != NULL) { - sigrdataset->trust = trust; - } - if (!need_validation || !ANSWER(rdataset)) { - options = 0; - if (ANSWER(rdataset) && - rdataset->type != dns_rdatatype_rrsig) - { - isc_result_t tresult; - dns_name_t *noqname = NULL; - tresult = findnoqname( - fctx, message, name, - rdataset->type, &noqname); - if (tresult == ISC_R_SUCCESS && - noqname != NULL) - { - (void)dns_rdataset_addnoqname( - rdataset, noqname); - } - } - if ((fctx->options & DNS_FETCHOPT_PREFETCH) != - 0) - { - options = DNS_DBADD_PREFETCH; - } - if ((fctx->options & DNS_FETCHOPT_NOCACHED) != - 0) - { - options |= DNS_DBADD_FORCE; - } - addedrdataset = ardataset; - result = dns_db_addrdataset( - fctx->cache, node, NULL, now, rdataset, - options, addedrdataset); - if (result == DNS_R_UNCHANGED) { - result = ISC_R_SUCCESS; - if (!need_validation && - ardataset != NULL && - NEGATIVE(ardataset)) - { - /* - * The answer in the - * cache is better than - * the answer we found. - * If it's a negative - * cache entry, we - * must set eresult - * appropriately. - */ - if (NXDOMAIN(ardataset)) { - eresult = - DNS_R_NCACHENXDOMAIN; - } else { - eresult = - DNS_R_NCACHENXRRSET; - } - continue; - } else if (!need_validation && - ardataset != NULL && - sigrdataset != NULL && - !dns_rdataset_equals( - rdataset, ardataset)) - { - /* - * The cache wasn't updated - * because something was - * already there. If the - * data was the same as what - * we were trying to add, - * then sigrdataset might - * still be useful, and we - * should carry on caching - * it. Otherwise, move on. - */ - continue; - } - } - if (result != ISC_R_SUCCESS) { - break; - } - if (sigrdataset != NULL) { - addedrdataset = asigrdataset; - result = dns_db_addrdataset( - fctx->cache, node, NULL, now, - sigrdataset, options, - addedrdataset); - if (result == DNS_R_UNCHANGED) { - result = ISC_R_SUCCESS; - } - if (result != ISC_R_SUCCESS) { - break; - } - } else if (!ANSWER(rdataset)) { - continue; - } + sigrdataset->trust = rdataset->trust; } if (ANSWER(rdataset) && need_validation) { @@ -6088,36 +6024,120 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, result = valcreate( fctx, message, addrinfo, name, rdataset->type, rdataset, - sigrdataset, valoptions); + sigrdataset); } - } else if (CHAINING(rdataset)) { - if (rdataset->type == dns_rdatatype_cname) { - eresult = DNS_R_CNAME; - } else { - INSIST(rdataset->type == - dns_rdatatype_dname); - eresult = DNS_R_DNAME; + } else { + if (ANSWER(rdataset) && + rdataset->type != dns_rdatatype_rrsig) + { + isc_result_t tresult; + dns_name_t *noqname = NULL; + tresult = findnoqname( + fctx, message, name, + rdataset->type, &noqname); + if (tresult == ISC_R_SUCCESS && + noqname != NULL) + { + (void)dns_rdataset_addnoqname( + rdataset, noqname); + } + } + if ((fctx->options & DNS_FETCHOPT_PREFETCH) != + 0) + { + options = DNS_DBADD_PREFETCH; + } + if ((fctx->options & DNS_FETCHOPT_NOCACHED) != + 0) + { + options |= DNS_DBADD_FORCE; + } + added = ardataset; + result = dns_db_addrdataset(fctx->cache, node, + NULL, now, rdataset, + options, added); + if (result == DNS_R_UNCHANGED) { + result = ISC_R_SUCCESS; + if (!need_validation && added != NULL && + NEGATIVE(added)) + { + /* + * The answer in the + * cache is better than + * the answer we found. + * If it's a negative + * cache entry, we + * must set eresult + * appropriately. + */ + if (NXDOMAIN(added)) { + eresult = + DNS_R_NCACHENXDOMAIN; + } else { + eresult = + DNS_R_NCACHENXRRSET; + } + continue; + } else if (!need_validation && + added != NULL && + sigrdataset != NULL && + !dns_rdataset_equals( + rdataset, added)) + { + /* + * The cache wasn't updated + * because something was + * already there. If the + * data was the same as what + * we were trying to add, + * then sigrdataset might + * still be useful, and we + * should carry on caching + * it. Otherwise, move on. + */ + continue; + } + } + if (result != ISC_R_SUCCESS) { + break; + } + if (sigrdataset != NULL) { + added = asigrdataset; + result = dns_db_addrdataset( + fctx->cache, node, NULL, now, + sigrdataset, options, added); + if (result == DNS_R_UNCHANGED) { + result = ISC_R_SUCCESS; + } + if (result != ISC_R_SUCCESS) { + break; + } + } else if (!ANSWER(rdataset)) { + continue; + } + + if (CHAINING(rdataset)) { + if (rdataset->type == + dns_rdatatype_cname) + { + eresult = DNS_R_CNAME; + } else { + INSIST(rdataset->type == + dns_rdatatype_dname); + eresult = DNS_R_DNAME; + } } } - } else if (!EXTERNAL(rdataset)) { + } else { /* * It's OK to cache this rdataset now. */ if (ANSWER(rdataset)) { - addedrdataset = ardataset; + added = ardataset; } else if (ANSWERSIG(rdataset)) { - addedrdataset = asigrdataset; + added = asigrdataset; } else { - addedrdataset = NULL; - } - if (CHAINING(rdataset)) { - if (rdataset->type == dns_rdatatype_cname) { - eresult = DNS_R_CNAME; - } else { - INSIST(rdataset->type == - dns_rdatatype_dname); - eresult = DNS_R_DNAME; - } + added = NULL; } if (rdataset->trust == dns_trust_glue && dns_rdataset_matchestype(rdataset, @@ -6135,8 +6155,6 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } else if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { options = DNS_DBADD_PREFETCH; - } else { - options = 0; } if (ANSWER(rdataset) && @@ -6158,11 +6176,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, */ result = dns_db_addrdataset(fctx->cache, node, NULL, now, rdataset, options, - addedrdataset); + added); if (result == DNS_R_UNCHANGED) { - if (ANSWER(rdataset) && ardataset != NULL && - NEGATIVE(ardataset)) + if (ANSWER(rdataset) && added != NULL && + NEGATIVE(added)) { /* * The answer in the cache is @@ -6171,7 +6189,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, * cache entry, so we must set * eresult appropriately. */ - if (NXDOMAIN(ardataset)) { + if (NXDOMAIN(added)) { eresult = DNS_R_NCACHENXDOMAIN; } else { eresult = DNS_R_NCACHENXRRSET; @@ -6195,7 +6213,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } result = valcreate(fctx, message, addrinfo, name, vtype, - valrdataset, valsigrdataset, valoptions); + valrdataset, valsigrdataset); } if (result == ISC_R_SUCCESS && have_answer) { @@ -6225,6 +6243,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } } } + resp->result = eresult; dns_name_copy(name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); @@ -6233,6 +6252,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } } +cleanup: if (node != NULL) { dns_db_detachnode(fctx->cache, &node); } @@ -6366,18 +6386,13 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name = fctx->name; dns_resolver_t *res = fctx->res; dns_dbnode_t *node = NULL; - bool need_validation = false, secure_domain = false; dns_fetchresponse_t *resp = NULL; - unsigned int valoptions = 0; - bool checknta = true; dns_rdataset_t *added = NULL; FCTXTRACE("ncache_message"); FCTX_ATTR_CLR(fctx, FCTX_ATTR_WANTNCACHE); - POST(need_validation); - /* * XXXMPA remove when we follow cnames and adjust the setting * of FCTX_ATTR_WANTNCACHE in rctx_answer_none(). @@ -6387,23 +6402,11 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, /* * Is DNSSEC validation required for this name? */ - if ((fctx->options & DNS_FETCHOPT_NONTA) != 0) { - valoptions |= DNS_VALIDATOR_NONTA; - checknta = false; - } - - secure_domain = issecuredomain(res->view, name, fctx->type, now, - checknta, NULL); - - if ((fctx->options & DNS_FETCHOPT_NOCDFLAG) != 0) { - valoptions |= DNS_VALIDATOR_NOCDFLAG; - } - - if ((fctx->options & DNS_FETCHOPT_NOVALIDATE) != 0) { - need_validation = false; - } else { - need_validation = secure_domain; - } + bool checknta = ((fctx->options & DNS_FETCHOPT_NONTA) == 0); + bool secure_domain = issecuredomain(res->view, name, fctx->type, now, + checknta, NULL); + bool need_validation = secure_domain && + ((fctx->options & DNS_FETCHOPT_NOVALIDATE) == 0); if (secure_domain) { /* @@ -6421,7 +6424,7 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, * Do negative response validation. */ result = valcreate(fctx, message, addrinfo, name, fctx->type, - NULL, NULL, valoptions); + NULL, NULL); /* * If validation is necessary, return now. Otherwise * continue to process the message, letting the From b940d40635aff79ab176b988c9b0a0c945ecff75 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 1 Mar 2025 15:40:07 -0800 Subject: [PATCH 07/12] set ANSWERSIG flag when processing ANY responses previously, rctx_answer_any() set the ANSWER flag for all rdatasets in the answer section; it now sets ANSWERSIG for RRSIG/SIG rdatasets and ANSWER for everything else. this error didn't cause any harm in the current code, but it could have led to unexpected behavior in the future. --- lib/dns/resolver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index cd44a0dbdf..c565800723 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -8416,7 +8416,11 @@ rctx_answer_any(respctx_t *rctx) { rctx->aname->attributes.cache = true; rctx->aname->attributes.answer = true; - rdataset->attributes.answer = true; + if (dns_rdatatype_issig(rdataset->type)) { + rdataset->attributes.answersig = true; + } else { + rdataset->attributes.answer = true; + } rdataset->attributes.cache = true; rdataset->trust = rctx->trust; } From 83980d76b27923781a3191a6e59b4c1c7afbe36a Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 1 Mar 2025 21:38:34 -0800 Subject: [PATCH 08/12] reduce code duplication around findnoqname() every call to findnoqname() was followed by a call to dns_rdataset_addnoqname(). we can move that call into findnoqname() itself, and simplify the calling functions a bit. --- lib/dns/resolver.c | 75 +++++++++++++++++----------------------------- 1 file changed, 27 insertions(+), 48 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index c565800723..e9e1199f84 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -673,9 +673,9 @@ maybe_cancel_validators(fetchctx_t *fctx); static void add_bad(fetchctx_t *fctx, dns_message_t *rmessage, dns_adbaddrinfo_t *addrinfo, isc_result_t reason, badnstype_t badtype); -static isc_result_t +static void findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, - dns_rdatatype_t type, dns_name_t **noqname); + dns_rdataset_t *rdataset); #define fctx_done_detach(fctxp, result) \ if (fctx__done(*fctxp, result, __func__, __FILE__, __LINE__)) { \ @@ -5391,18 +5391,8 @@ validated(void *arg) { val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER]); RUNTIME_CHECK(result == ISC_R_SUCCESS); } - } else if (val->rdataset->trust == dns_trust_answer && - val->rdataset->type != dns_rdatatype_rrsig) - { - isc_result_t tresult; - dns_name_t *noqname = NULL; - tresult = findnoqname(fctx, message, val->name, - val->rdataset->type, &noqname); - if (tresult == ISC_R_SUCCESS && noqname != NULL) { - tresult = dns_rdataset_addnoqname(val->rdataset, - noqname); - RUNTIME_CHECK(tresult == ISC_R_SUCCESS); - } + } else if (val->rdataset->trust == dns_trust_answer) { + findnoqname(fctx, message, val->name, val->rdataset); } /* @@ -5663,12 +5653,12 @@ fctx_log(void *arg, int level, const char *fmt, ...) { "fctx %p(%s): %s", fctx, fctx->info, msgbuf); } -static isc_result_t +static void findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, - dns_rdatatype_t type, dns_name_t **noqnamep) { + dns_rdataset_t *rdataset) { + isc_result_t result; dns_rdataset_t *sigrdataset = NULL; dns_rdata_rrsig_t rrsig; - isc_result_t result; unsigned int labels; dns_name_t *zonename = NULL; dns_fixedname_t fzonename; @@ -5678,10 +5668,13 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, dns_fixedname_t fnearest; dns_rdatatype_t found = dns_rdatatype_none; dns_name_t *noqname = NULL; + dns_rdatatype_t type = rdataset->type; FCTXTRACE("findnoqname"); - REQUIRE(noqnamep != NULL && *noqnamep == NULL); + if (dns_rdatatype_issig(rdataset->type)) { + return; + } /* * Find the SIG for this rdataset, if we have it. @@ -5694,7 +5687,7 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, } if (sigrdataset == NULL) { - return ISC_R_NOTFOUND; + return; } labels = dns_name_countlabels(name); @@ -5714,7 +5707,7 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, } if (result != ISC_R_SUCCESS) { - return result; + return; } zonename = dns_fixedname_initname(&fzonename); @@ -5765,13 +5758,20 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, if (noqname != NULL) { ISC_LIST_FOREACH (noqname->list, sig, link) { if (dns_rdataset_issigtype(sig, found)) { - *noqnamep = noqname; + sigrdataset = sig; break; } } + if (sigrdataset == NULL) { + noqname = NULL; + } } - return result; + if (result == ISC_R_SUCCESS && noqname != NULL) { + (void)dns_rdataset_addnoqname(rdataset, noqname); + } + + return; } static isc_result_t @@ -6027,20 +6027,9 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, sigrdataset); } } else { - if (ANSWER(rdataset) && - rdataset->type != dns_rdatatype_rrsig) - { - isc_result_t tresult; - dns_name_t *noqname = NULL; - tresult = findnoqname( - fctx, message, name, - rdataset->type, &noqname); - if (tresult == ISC_R_SUCCESS && - noqname != NULL) - { - (void)dns_rdataset_addnoqname( - rdataset, noqname); - } + if (ANSWER(rdataset)) { + findnoqname(fctx, message, name, + rdataset); } if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) @@ -6157,18 +6146,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, options = DNS_DBADD_PREFETCH; } - if (ANSWER(rdataset) && - rdataset->type != dns_rdatatype_rrsig) - { - isc_result_t tresult; - dns_name_t *noqname = NULL; - tresult = findnoqname(fctx, message, name, - rdataset->type, &noqname); - if (tresult == ISC_R_SUCCESS && noqname != NULL) - { - (void)dns_rdataset_addnoqname(rdataset, - noqname); - } + if (ANSWER(rdataset)) { + findnoqname(fctx, message, name, rdataset); } /* From ed56a91d7df46d2ebc6539b3a8da694616b33ed2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 1 Mar 2025 20:04:18 -0800 Subject: [PATCH 09/12] rename and refactor cache_name() and related functions - renamed cache_message() to rctx_cachemessage() - renamed cache_name() to rctx_cachename() - merged ncache_message() into rctx_ncache() - split out a new function, rctx_cacherdataset(), which is called by rctx_cachename() in a loop to process each of the rdatasets associated with the name. --- lib/dns/resolver.c | 754 ++++++++++++++++++++------------------------- 1 file changed, 330 insertions(+), 424 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index e9e1199f84..17de4964f3 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -842,6 +842,9 @@ typedef struct respctx { * response */ dns_rdataset_t *opt; /* OPT rdataset */ + + dns_rdataset_t *vrdataset; + dns_rdataset_t *vsigrdataset; } respctx_t; static void @@ -958,7 +961,7 @@ set_stats(dns_resolver_t *res, isc_statscounter_t counter, uint64_t val) { } } -static isc_result_t +static void valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) { @@ -985,8 +988,6 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, if (!ISC_LIST_EMPTY(fctx->validators)) { valoptions |= DNS_VALIDATOR_DEFER; - } else { - valoptions &= ~DNS_VALIDATOR_DEFER; } result = dns_validator_create( @@ -996,7 +997,6 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, RUNTIME_CHECK(result == ISC_R_SUCCESS); inc_stats(fctx->res, dns_resstatscounter_val); ISC_LIST_APPEND(fctx->validators, validator, link); - return ISC_R_SUCCESS; } static void @@ -2248,8 +2248,8 @@ compute_cc(const resquery_t *query, uint8_t *cookie, const size_t len) { } static bool -issecuredomain(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, - isc_stdtime_t now, bool checknta, bool *ntap) { +issecuredomain(fetchctx_t *fctx, const dns_name_t *name, dns_rdatatype_t type, + isc_stdtime_t now, bool *ntap) { dns_name_t suffix; unsigned int labels; @@ -2266,7 +2266,8 @@ issecuredomain(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, name = &suffix; } - return dns_view_issecuredomain(view, name, now, checknta, ntap); + return dns_view_issecuredomain(fctx->res->view, name, now, + CHECKNTA(fctx), ntap); } static isc_result_t @@ -5184,7 +5185,7 @@ validated(void *arg) { dns_adbaddrinfo_t *addrinfo = NULL; dns_dbnode_t *node = NULL; dns_dbnode_t *nsnode = NULL; - dns_fetchresponse_t *hresp = NULL; + dns_fetchresponse_t *resp = NULL; dns_rdataset_t *ardataset = NULL; dns_rdataset_t *asigrdataset = NULL; dns_resolver_t *res = NULL; @@ -5196,7 +5197,7 @@ validated(void *arg) { isc_result_t eresult = ISC_R_SUCCESS; isc_result_t result = ISC_R_SUCCESS; isc_stdtime_t now; - unsigned int options; + unsigned int options = 0; dns_fixedname_t fwild; dns_name_t *wild = NULL; dns_message_t *message = NULL; @@ -5276,8 +5277,8 @@ validated(void *arg) { * started by a query with cd set) */ - hresp = ISC_LIST_HEAD(fctx->resps); - if (hresp != NULL) { + resp = ISC_LIST_HEAD(fctx->resps); + if (resp != NULL) { if (!negative && !chaining && dns_rdatatype_ismulti(fctx->type)) { /* @@ -5285,8 +5286,8 @@ validated(void *arg) { * will iterate the node. */ } else { - ardataset = hresp->rdataset; - asigrdataset = hresp->sigrdataset; + ardataset = resp->rdataset; + asigrdataset = resp->sigrdataset; } } @@ -5406,21 +5407,18 @@ validated(void *arg) { goto noanswer_response; } - options = 0; if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { options = DNS_DBADD_PREFETCH; } + result = dns_db_addrdataset(fctx->cache, node, NULL, now, val->rdataset, options, ardataset); if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) { goto noanswer_response; } if (ardataset != NULL && NEGATIVE(ardataset)) { - if (NXDOMAIN(ardataset)) { - eresult = DNS_R_NCACHENXDOMAIN; - } else { - eresult = DNS_R_NCACHENXRRSET; - } + eresult = NXDOMAIN(ardataset) ? DNS_R_NCACHENXDOMAIN + : DNS_R_NCACHENXRRSET; } else if (val->sigrdataset != NULL) { result = dns_db_addrdataset(fctx->cache, node, NULL, now, val->sigrdataset, options, @@ -5446,10 +5444,10 @@ validated(void *arg) { if (!ISC_LIST_EMPTY(fctx->validators)) { INSIST(!negative); INSIST(dns_rdatatype_ismulti(fctx->type)); + /* - * Don't send a response yet - we have - * more rdatasets that still need to - * be validated. + * Don't send a response yet - we have more rdatasets + * that still need to be validated. */ dns_db_detachnode(fctx->cache, &node); UNLOCK(&fctx->lock); @@ -5576,6 +5574,10 @@ answer_response: result = ISC_R_SUCCESS; + if (HAVE_ANSWER(fctx) || resp == NULL) { + goto noanswer_response; + } + /* * Respond with an answer, positive or negative, * as opposed to an error. 'node' must be non-NULL. @@ -5583,38 +5585,33 @@ answer_response: FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); - if (hresp != NULL) { - /* - * Negative results must be indicated in val->result. - */ - INSIST(hresp->rdataset != NULL); - if (dns_rdataset_isassociated(hresp->rdataset)) { - if (NEGATIVE(hresp->rdataset)) { - INSIST(eresult == DNS_R_NCACHENXDOMAIN || - eresult == DNS_R_NCACHENXRRSET); - } else if (eresult == ISC_R_SUCCESS && - hresp->rdataset->type != fctx->type) - { - switch (hresp->rdataset->type) { - case dns_rdatatype_cname: - eresult = DNS_R_CNAME; - break; - case dns_rdatatype_dname: - eresult = DNS_R_DNAME; - break; - default: - break; - } + INSIST(resp->rdataset != NULL); + if (dns_rdataset_isassociated(resp->rdataset)) { + if (NEGATIVE(resp->rdataset)) { + INSIST(eresult == DNS_R_NCACHENXDOMAIN || + eresult == DNS_R_NCACHENXRRSET); + } else if (eresult == ISC_R_SUCCESS && + resp->rdataset->type != fctx->type) + { + switch (resp->rdataset->type) { + case dns_rdatatype_cname: + eresult = DNS_R_CNAME; + break; + case dns_rdatatype_dname: + eresult = DNS_R_DNAME; + break; + default: + break; } } - - hresp->result = eresult; - dns_name_copy(val->name, hresp->foundname); - dns_db_attach(fctx->cache, &hresp->db); - dns_db_transfernode(fctx->cache, &node, &hresp->node); - clone_results(fctx); } + resp->result = eresult; + dns_name_copy(val->name, resp->foundname); + dns_db_attach(fctx->cache, &resp->db); + dns_db_transfernode(fctx->cache, &node, &resp->node); + clone_results(fctx); + noanswer_response: if (node != NULL) { dns_db_detachnode(fctx->cache, &node); @@ -5847,19 +5844,203 @@ fixttls(dns_view_t *view, dns_rdataset_t *rdataset, } static isc_result_t -cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, - dns_adbaddrinfo_t *addrinfo, isc_stdtime_t now) { +rctx_cacherdataset(respctx_t *rctx, dns_message_t *message, dns_name_t *name, + dns_dbnode_t *node, dns_rdataset_t *rdataset, + dns_rdataset_t *sigrdataset, bool secure_domain, + bool need_validation) { + isc_result_t result; + fetchctx_t *fctx = rctx->fctx; + resquery_t *query = rctx->query; + dns_rdataset_t *ardataset = NULL, *asigset = NULL; + dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); + unsigned int options = 0; + + if (secure_domain && rdataset->trust != dns_trust_glue) { + /* + * RRSIGs are validated as part of validating + * the type they cover. + */ + if (rdataset->type == dns_rdatatype_rrsig) { + return ISC_R_SUCCESS; + } + + /* + * Ignore unrelated non-answer rdatasets that are + * missing signatures. + */ + if (sigrdataset == NULL && need_validation && !ANSWER(rdataset)) + { + return ISC_R_SUCCESS; + } + + /* + * Mark this rdataset/sigrdataset pair as pending data. + * Track whether it was additional or not. + */ + if (rdataset->trust == dns_trust_additional) { + rdataset->trust = dns_trust_pending_additional; + } else { + rdataset->trust = dns_trust_pending_answer; + } + + if (sigrdataset != NULL) { + sigrdataset->trust = rdataset->trust; + } + + if (ANSWER(rdataset) && need_validation) { + if (!dns_rdatatype_ismulti(fctx->type)) { + /* + * This is The Answer. We will validate + * it, but first we cache the rest of the + * response - it may contain useful keys. + */ + INSIST(rctx->vrdataset == NULL && + rctx->vsigrdataset == NULL); + rctx->vrdataset = rdataset; + rctx->vsigrdataset = sigrdataset; + } else { + /* + * This is one of (potentially) multiple + * answers to an ANY query. To keep things + * simple, we just start the validator + * right away rather than caching first and + * having to remember which rdatasets + * needed validation. + */ + valcreate(fctx, message, query->addrinfo, name, + rdataset->type, rdataset, + sigrdataset); + } + } else { + if (ANSWER(rdataset)) { + /* + * We're not validating, but the client might + * be, so look for the NOQNAME proof. + */ + findnoqname(fctx, message, name, rdataset); + + /* + * If this was not an ANY/RRSIG/SIG query, + * or if it was but we got a CNAME/DNAME, + * then we need to set up rdatasets to + * send back to the caller. + */ + if (!dns_rdatatype_ismulti(fctx->type) || + CHAINING(rdataset)) + { + ardataset = resp->rdataset; + asigset = resp->sigrdataset; + } + } + + if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { + options = DNS_DBADD_PREFETCH; + } + if ((fctx->options & DNS_FETCHOPT_NOCACHED) != 0) { + options |= DNS_DBADD_FORCE; + } + + result = dns_db_addrdataset(fctx->cache, node, NULL, + rctx->now, rdataset, + options, ardataset); + if (result != DNS_R_UNCHANGED && + result != ISC_R_SUCCESS) + { + return result; + } + + if (sigrdataset == NULL) { + return ISC_R_SUCCESS; + } + + if (result == DNS_R_UNCHANGED && !need_validation && + ardataset != NULL && + !dns_rdataset_equals(rdataset, ardataset)) + { + /* + * The cache wasn't updated because + * something was already there. If the data + * was the same as what we were trying to + * add, then sigrdataset might still be + * useful, and we should carry on caching + * it. Otherwise, move on. + */ + return ISC_R_SUCCESS; + } + + result = dns_db_addrdataset(fctx->cache, node, NULL, + rctx->now, sigrdataset, + options, asigset); + if (result != DNS_R_UNCHANGED && + result != ISC_R_SUCCESS) + { + return result; + } + } + + return ISC_R_SUCCESS; + } + + /* + * We're not in a secure domain, or this is glue, + * so we can cache right away. + * + * If this wasn't an ANY/RRSIG/SIG query then send + * an answer back to the caller. + */ dns_rdataset_t *added = NULL; - dns_rdataset_t *sigrdataset = NULL; - dns_rdataset_t *ardataset = NULL, *asigrdataset = NULL; - dns_rdataset_t *valrdataset = NULL, *valsigrdataset = NULL; - dns_dbnode_t *node = NULL; + if (!dns_rdatatype_ismulti(fctx->type)) { + if (ANSWER(rdataset)) { + added = resp->rdataset; + } else if (ANSWERSIG(rdataset)) { + added = resp->sigrdataset; + } + } + + if (rdataset->trust == dns_trust_glue && + dns_rdataset_matchestype(rdataset, dns_rdatatype_ns)) + { + /* + * If the trust level is glue, then we are adding data from + * a referral we got while executing the search algorithm. + * New referral data always takes precedence over the + * existing cache contents. + */ + options = DNS_DBADD_FORCE; + } else if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { + options = DNS_DBADD_PREFETCH; + } + + /* + * Look for the NOQNAME proof. + */ + if (ANSWER(rdataset)) { + findnoqname(fctx, message, name, rdataset); + } + + /* + * Now we can add the rdataset. + */ + result = dns_db_addrdataset(fctx->cache, node, NULL, rctx->now, + rdataset, options, added); + if (result == DNS_R_UNCHANGED) { + result = ISC_R_SUCCESS; + } + + return result; +} + +static isc_result_t +rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { + isc_result_t result; + fetchctx_t *fctx = rctx->fctx; + resquery_t *query = rctx->query; dns_resolver_t *res = fctx->res; - bool have_answer = false; - isc_result_t result, eresult = ISC_R_SUCCESS; + dns_rdataset_t *sigrdataset = NULL; + dns_dbnode_t *node = NULL; dns_fetchresponse_t *resp = NULL; - FCTXTRACE("cache_name"); + FCTXTRACE("rctx_cachename"); /* * The appropriate bucket lock must be held. @@ -5868,33 +6049,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * Is DNSSEC validation required for this name? */ - bool checknta = ((fctx->options & DNS_FETCHOPT_NONTA) == 0); - bool secure_domain = issecuredomain(res->view, name, fctx->type, now, - checknta, NULL); + bool secure_domain = issecuredomain(fctx, name, fctx->type, rctx->now, + NULL); bool need_validation = secure_domain && ((fctx->options & DNS_FETCHOPT_NOVALIDATE) == 0); - if (name->attributes.answer && !need_validation) { - have_answer = true; - resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL) { - /* - * If this is an ANY, SIG or RRSIG query, we're - * not going to return any rdatasets, unless we - * encountered a CNAME or DNAME as "the answer". - * In this case, we're going to return - * DNS_R_CNAME or DNS_R_DNAME and we must set up - * the rdatasets. - */ - if (!dns_rdatatype_ismulti(fctx->type) || - name->attributes.chaining) - { - ardataset = resp->rdataset; - asigrdataset = resp->sigrdataset; - } - } - } - /* * Find or create the cache node. */ @@ -5906,10 +6065,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * Cache or validate each cacheable rdataset. */ - bool fail = ((fctx->res->options & DNS_RESOLVER_CHECKNAMESFAIL) != 0); + bool fail = ((res->options & DNS_RESOLVER_CHECKNAMESFAIL) != 0); ISC_LIST_FOREACH (name->list, rdataset, link) { - unsigned int options = 0; - result = check_cacheable(name, rdataset, fail); if (result == DNS_R_CONTINUE) { result = ISC_R_SUCCESS; @@ -5934,288 +6091,50 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, */ fixttls(res->view, rdataset, sigrdataset); - /* - * If this RRset is in a secure domain, is in bailiwick, - * and is not glue, attempt DNSSEC validation. - * - * We do not attempt to validate glue or out-of-bailiwick - * data - even though there might be some performance - * benefit to doing so - because it makes it simpler and - * safer to ensure that records from a secure domain are - * only cached if validated within the context of a query - * to the domain that owns them. - */ - if (secure_domain && rdataset->trust != dns_trust_glue) { - /* - * RRSIGs are validated as part of validating - * the type they cover. - */ - if (rdataset->type == dns_rdatatype_rrsig) { - continue; - } - - /* - * Ignore unrelated non-answer rdatasets that are - * missing signatures. - */ - if (sigrdataset == NULL && need_validation && - !ANSWER(rdataset)) - { - continue; - } - - /* - * Normalize the rdataset and sigrdataset TTLs. - */ - if (sigrdataset != NULL) { - rdataset->ttl = ISC_MIN(rdataset->ttl, - sigrdataset->ttl); - sigrdataset->ttl = rdataset->ttl; - } - - /* - * Mark the rdataset as being prefetch eligible. - */ - if (rdataset->ttl >= fctx->res->view->prefetch_eligible) - { - rdataset->attributes.prefetch = true; - } - - /* - * Cache this rdataset/sigrdataset pair as - * pending data. Track whether it was - * additional or not. If this was a priming - * query, additional should be cached as glue. - */ - if (rdataset->trust == dns_trust_additional) { - rdataset->trust = dns_trust_pending_additional; - } else { - rdataset->trust = dns_trust_pending_answer; - } - - if (sigrdataset != NULL) { - sigrdataset->trust = rdataset->trust; - } - - if (ANSWER(rdataset) && need_validation) { - if (!dns_rdatatype_ismulti(fctx->type)) { - /* - * This is The Answer. We will - * validate it, but first we - * cache the rest of the - * response - it may contain - * useful keys. - */ - INSIST(valrdataset == NULL && - valsigrdataset == NULL); - valrdataset = rdataset; - valsigrdataset = sigrdataset; - } else { - /* - * This is one of (potentially) - * multiple answers to an ANY - * or SIG query. To keep things - * simple, we just start the - * validator right away rather - * than caching first and - * having to remember which - * rdatasets needed validation. - */ - result = valcreate( - fctx, message, addrinfo, name, - rdataset->type, rdataset, - sigrdataset); - } - } else { - if (ANSWER(rdataset)) { - findnoqname(fctx, message, name, - rdataset); - } - if ((fctx->options & DNS_FETCHOPT_PREFETCH) != - 0) - { - options = DNS_DBADD_PREFETCH; - } - if ((fctx->options & DNS_FETCHOPT_NOCACHED) != - 0) - { - options |= DNS_DBADD_FORCE; - } - added = ardataset; - result = dns_db_addrdataset(fctx->cache, node, - NULL, now, rdataset, - options, added); - if (result == DNS_R_UNCHANGED) { - result = ISC_R_SUCCESS; - if (!need_validation && added != NULL && - NEGATIVE(added)) - { - /* - * The answer in the - * cache is better than - * the answer we found. - * If it's a negative - * cache entry, we - * must set eresult - * appropriately. - */ - if (NXDOMAIN(added)) { - eresult = - DNS_R_NCACHENXDOMAIN; - } else { - eresult = - DNS_R_NCACHENXRRSET; - } - continue; - } else if (!need_validation && - added != NULL && - sigrdataset != NULL && - !dns_rdataset_equals( - rdataset, added)) - { - /* - * The cache wasn't updated - * because something was - * already there. If the - * data was the same as what - * we were trying to add, - * then sigrdataset might - * still be useful, and we - * should carry on caching - * it. Otherwise, move on. - */ - continue; - } - } - if (result != ISC_R_SUCCESS) { - break; - } - if (sigrdataset != NULL) { - added = asigrdataset; - result = dns_db_addrdataset( - fctx->cache, node, NULL, now, - sigrdataset, options, added); - if (result == DNS_R_UNCHANGED) { - result = ISC_R_SUCCESS; - } - if (result != ISC_R_SUCCESS) { - break; - } - } else if (!ANSWER(rdataset)) { - continue; - } - - if (CHAINING(rdataset)) { - if (rdataset->type == - dns_rdatatype_cname) - { - eresult = DNS_R_CNAME; - } else { - INSIST(rdataset->type == - dns_rdatatype_dname); - eresult = DNS_R_DNAME; - } - } - } - } else { - /* - * It's OK to cache this rdataset now. - */ - if (ANSWER(rdataset)) { - added = ardataset; - } else if (ANSWERSIG(rdataset)) { - added = asigrdataset; - } else { - added = NULL; - } - if (rdataset->trust == dns_trust_glue && - dns_rdataset_matchestype(rdataset, - dns_rdatatype_ns)) - { - /* - * If the trust level is - * 'dns_trust_glue' then we are adding - * data from a referral we got while - * executing the search algorithm. New - * referral data always takes precedence - * over the existing cache contents. - */ - options = DNS_DBADD_FORCE; - } else if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) - { - options = DNS_DBADD_PREFETCH; - } - - if (ANSWER(rdataset)) { - findnoqname(fctx, message, name, rdataset); - } - - /* - * Now we can add the rdataset. - */ - result = dns_db_addrdataset(fctx->cache, node, NULL, - now, rdataset, options, - added); - - if (result == DNS_R_UNCHANGED) { - if (ANSWER(rdataset) && added != NULL && - NEGATIVE(added)) - { - /* - * The answer in the cache is - * better than the answer we - * found, and is a negative - * cache entry, so we must set - * eresult appropriately. - */ - if (NXDOMAIN(added)) { - eresult = DNS_R_NCACHENXDOMAIN; - } else { - eresult = DNS_R_NCACHENXRRSET; - } - } - result = ISC_R_SUCCESS; - } else if (result != ISC_R_SUCCESS) { - break; - } + /* Try to cache the rdataset */ + result = rctx_cacherdataset(rctx, message, name, node, rdataset, + sigrdataset, secure_domain, + need_validation); + if (result != ISC_R_SUCCESS) { + goto cleanup; } } - if (valrdataset != NULL) { + if (rctx->vrdataset != NULL) { dns_rdatatype_t vtype = fctx->type; - if (CHAINING(valrdataset)) { - if (valrdataset->type == dns_rdatatype_cname) { - vtype = dns_rdatatype_cname; - } else { - vtype = dns_rdatatype_dname; - } + if (CHAINING(rctx->vrdataset)) { + vtype = rctx->vrdataset->type; + INSIST(dns_rdatatype_isalias(vtype)); } - result = valcreate(fctx, message, addrinfo, name, vtype, - valrdataset, valsigrdataset); + valcreate(fctx, message, query->addrinfo, name, vtype, + rctx->vrdataset, rctx->vsigrdataset); + rctx->vrdataset = NULL; + rctx->vsigrdataset = NULL; } - if (result == ISC_R_SUCCESS && have_answer) { - FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); + if (result == ISC_R_SUCCESS && name->attributes.answer && + !need_validation && !HAVE_ANSWER(fctx)) + { + resp = ISC_LIST_HEAD(fctx->resps); if (resp != NULL) { /* * Negative results must be indicated in * resp->result. */ + resp->result = ISC_R_SUCCESS; if (dns_rdataset_isassociated(resp->rdataset)) { - if (NEGATIVE(resp->rdataset)) { - INSIST(eresult == - DNS_R_NCACHENXDOMAIN || - eresult == DNS_R_NCACHENXRRSET); - } else if (eresult == ISC_R_SUCCESS && - resp->rdataset->type != fctx->type) - { + if (NXDOMAIN(resp->rdataset)) { + resp->result = DNS_R_NCACHENXDOMAIN; + } else if (NEGATIVE(resp->rdataset)) { + resp->result = DNS_R_NCACHENXRRSET; + } else if (resp->rdataset->type != fctx->type) { switch (resp->rdataset->type) { case dns_rdatatype_cname: - eresult = DNS_R_CNAME; + resp->result = DNS_R_CNAME; break; case dns_rdatatype_dname: - eresult = DNS_R_DNAME; + resp->result = DNS_R_DNAME; break; default: break; @@ -6223,12 +6142,13 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } } - resp->result = eresult; dns_name_copy(name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); clone_results(fctx); } + + FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); } cleanup: @@ -6240,22 +6160,24 @@ cleanup: } static isc_result_t -cache_message(fetchctx_t *fctx, dns_message_t *message, - dns_adbaddrinfo_t *addrinfo, isc_stdtime_t now) { - FCTXTRACE("cache_message"); +rctx_cachemessage(respctx_t *rctx) { + isc_result_t result; + fetchctx_t *fctx = rctx->fctx; + resquery_t *query = rctx->query; + dns_message_t *message = query->rmessage; + + FCTXTRACE("rctx_cachemessage"); FCTX_ATTR_CLR(fctx, FCTX_ATTR_WANTCACHE); LOCK(&fctx->lock); - isc_result_t result = ISC_R_SUCCESS; for (dns_section_t section = DNS_SECTION_ANSWER; section <= DNS_SECTION_ADDITIONAL; section++) { MSG_SECTION_FOREACH (message, section, name) { if (name->attributes.cache) { - result = cache_name(fctx, name, message, - addrinfo, now); + result = rctx_cachename(rctx, message, name); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -6358,17 +6280,28 @@ negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name, return result; } -static isc_result_t -ncache_message(fetchctx_t *fctx, dns_message_t *message, - dns_adbaddrinfo_t *addrinfo, isc_stdtime_t now) { - isc_result_t result, eresult = ISC_R_SUCCESS; +/* + * rctx_ncache(): + * Cache the negatively cacheable parts of the message. This may + * also cause work to be queued to the DNSSEC validator. + */ +static void +rctx_ncache(respctx_t *rctx) { + isc_result_t result = ISC_R_SUCCESS; + isc_result_t eresult = ISC_R_SUCCESS; + fetchctx_t *fctx = rctx->fctx; dns_name_t *name = fctx->name; - dns_resolver_t *res = fctx->res; + dns_message_t *message = rctx->query->rmessage; + dns_adbaddrinfo_t *addrinfo = rctx->query->addrinfo; dns_dbnode_t *node = NULL; dns_fetchresponse_t *resp = NULL; dns_rdataset_t *added = NULL; - FCTXTRACE("ncache_message"); + FCTXTRACE("rctx_ncache"); + + if (!WANTNCACHE(fctx)) { + goto done; + } FCTX_ATTR_CLR(fctx, FCTX_ATTR_WANTNCACHE); @@ -6381,15 +6314,16 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, /* * Is DNSSEC validation required for this name? */ - bool checknta = ((fctx->options & DNS_FETCHOPT_NONTA) == 0); - bool secure_domain = issecuredomain(res->view, name, fctx->type, now, - checknta, NULL); + bool secure_domain = issecuredomain(fctx, name, fctx->type, rctx->now, + NULL); bool need_validation = secure_domain && ((fctx->options & DNS_FETCHOPT_NOVALIDATE) == 0); if (secure_domain) { /* - * Mark all rdatasets as pending. + * Mark all rdatasets as pending. (We do this for + * any domain under a trust anchor, regardless + * of whether we're actually validating.) */ MSG_SECTION_FOREACH (message, DNS_SECTION_AUTHORITY, tname) { ISC_LIST_FOREACH (tname->list, trdataset, link) { @@ -6400,37 +6334,31 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, if (need_validation) { /* - * Do negative response validation. + * Start the validator for the negative response. It + * will call validated() on completion; the caching of + * negative answers will be done then. */ - result = valcreate(fctx, message, addrinfo, name, fctx->type, - NULL, NULL); - /* - * If validation is necessary, return now. Otherwise - * continue to process the message, letting the - * validation complete in its own good time. - */ - return result; + valcreate(fctx, message, addrinfo, name, fctx->type, NULL, + NULL); + goto done; } + /* + * Cache the negative answer, and copy it into the fetch response. + */ LOCK(&fctx->lock); - - if (!HAVE_ANSWER(fctx)) { - resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL) { - added = resp->rdataset; - } + resp = ISC_LIST_HEAD(fctx->resps); + if (!HAVE_ANSWER(fctx) && resp != NULL) { + added = resp->rdataset; } - result = negcache(message, fctx, name, now, false, false, added, &node, - &eresult); - if (result != ISC_R_SUCCESS) { + result = negcache(message, fctx, name, rctx->now, false, false, added, + &node, &eresult); + if (result != ISC_R_SUCCESS || HAVE_ANSWER(fctx)) { goto unlock; } - if (!HAVE_ANSWER(fctx)) { - FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); - } - + FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); if (resp != NULL) { resp->result = eresult; dns_name_copy(name, resp->foundname); @@ -6446,7 +6374,10 @@ unlock: dns_db_detachnode(fctx->cache, &node); } - return result; +done: + if (result != ISC_R_SUCCESS) { + FCTXTRACE3("rctx_ncache complete", result); + } } static void @@ -6465,6 +6396,7 @@ mark_related(dns_name_t *name, dns_rdataset_t *rdataset, bool external, } else { rdataset->trust = dns_trust_additional; } + /* * Avoid infinite loops by only marking new rdatasets. */ @@ -7696,10 +7628,9 @@ resquery_response_continue(void *arg, isc_result_t result) { */ if (WANTCACHE(fctx)) { isc_result_t tresult; - tresult = cache_message(fctx, query->rmessage, query->addrinfo, - rctx->now); + tresult = rctx_cachemessage(rctx); if (tresult != ISC_R_SUCCESS) { - FCTXTRACE3("cache_message complete", tresult); + FCTXTRACE3("rctx_cachemessage complete", tresult); rctx_done(rctx, tresult); goto cleanup; } @@ -7794,6 +7725,9 @@ rctx_answer_init(respctx_t *rctx) { rctx->soa_name = NULL; rctx->ds_name = NULL; rctx->found_name = NULL; + + rctx->vrdataset = NULL; + rctx->vsigrdataset = NULL; } /* @@ -8882,30 +8816,6 @@ rctx_authority_negative(respctx_t *rctx) { return ISC_R_SUCCESS; } -/* - * rctx_ncache(): - * Cache the negatively cacheable parts of the message. This may - * also cause work to be queued to the DNSSEC validator. - */ -static void -rctx_ncache(respctx_t *rctx) { - isc_result_t result; - fetchctx_t *fctx = rctx->fctx; - - if (!WANTNCACHE(fctx)) { - return; - } - - /* - * Cache any negative cache entries in the message. - */ - result = ncache_message(fctx, rctx->query->rmessage, - rctx->query->addrinfo, rctx->now); - if (result != ISC_R_SUCCESS) { - FCTXTRACE3("ncache_message complete", result); - } -} - /* * rctx_authority_dnssec(): * @@ -8929,7 +8839,6 @@ rctx_authority_dnssec(respctx_t *rctx) { } ISC_LIST_FOREACH (name->list, rdataset, link) { - bool checknta = true; bool secure_domain = false; dns_rdatatype_t type = rdataset->type; @@ -8993,12 +8902,9 @@ rctx_authority_dnssec(respctx_t *rctx) { name->attributes.cache = true; rdataset->attributes.cache = true; - if ((fctx->options & DNS_FETCHOPT_NONTA) != 0) { - checknta = false; - } - secure_domain = issecuredomain( - fctx->res->view, name, dns_rdatatype_ds, - fctx->now, checknta, NULL); + secure_domain = issecuredomain(fctx, name, + dns_rdatatype_ds, + fctx->now, NULL); if (secure_domain) { rdataset->trust = dns_trust_pending_answer; From 723d167f26f0521cca5d4ead4c4b28320175c2f5 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 1 Mar 2025 22:15:11 -0800 Subject: [PATCH 10/12] further subdivide caching functions rctx_cacherdataset() has been split into two functions: - rctx_cache_secure() starts validation for rdatasets that need it; they are then cached by the validator completion callback validated() - rctx_cache_insecure() caches rdatasets immediately; it is called when validation is disabled or the data to be cached is glue. --- lib/dns/resolver.c | 291 +++++++++++++++++++++++---------------------- 1 file changed, 147 insertions(+), 144 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 17de4964f3..25ac0fa960 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -5844,10 +5844,9 @@ fixttls(dns_view_t *view, dns_rdataset_t *rdataset, } static isc_result_t -rctx_cacherdataset(respctx_t *rctx, dns_message_t *message, dns_name_t *name, - dns_dbnode_t *node, dns_rdataset_t *rdataset, - dns_rdataset_t *sigrdataset, bool secure_domain, - bool need_validation) { +rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, + dns_dbnode_t *node, dns_rdataset_t *rdataset, + dns_rdataset_t *sigrdataset, bool need_validation) { isc_result_t result; fetchctx_t *fctx = rctx->fctx; resquery_t *query = rctx->query; @@ -5855,141 +5854,135 @@ rctx_cacherdataset(respctx_t *rctx, dns_message_t *message, dns_name_t *name, dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); unsigned int options = 0; - if (secure_domain && rdataset->trust != dns_trust_glue) { - /* - * RRSIGs are validated as part of validating - * the type they cover. - */ - if (rdataset->type == dns_rdatatype_rrsig) { - return ISC_R_SUCCESS; - } - - /* - * Ignore unrelated non-answer rdatasets that are - * missing signatures. - */ - if (sigrdataset == NULL && need_validation && !ANSWER(rdataset)) - { - return ISC_R_SUCCESS; - } - - /* - * Mark this rdataset/sigrdataset pair as pending data. - * Track whether it was additional or not. - */ - if (rdataset->trust == dns_trust_additional) { - rdataset->trust = dns_trust_pending_additional; - } else { - rdataset->trust = dns_trust_pending_answer; - } - - if (sigrdataset != NULL) { - sigrdataset->trust = rdataset->trust; - } - - if (ANSWER(rdataset) && need_validation) { - if (!dns_rdatatype_ismulti(fctx->type)) { - /* - * This is The Answer. We will validate - * it, but first we cache the rest of the - * response - it may contain useful keys. - */ - INSIST(rctx->vrdataset == NULL && - rctx->vsigrdataset == NULL); - rctx->vrdataset = rdataset; - rctx->vsigrdataset = sigrdataset; - } else { - /* - * This is one of (potentially) multiple - * answers to an ANY query. To keep things - * simple, we just start the validator - * right away rather than caching first and - * having to remember which rdatasets - * needed validation. - */ - valcreate(fctx, message, query->addrinfo, name, - rdataset->type, rdataset, - sigrdataset); - } - } else { - if (ANSWER(rdataset)) { - /* - * We're not validating, but the client might - * be, so look for the NOQNAME proof. - */ - findnoqname(fctx, message, name, rdataset); - - /* - * If this was not an ANY/RRSIG/SIG query, - * or if it was but we got a CNAME/DNAME, - * then we need to set up rdatasets to - * send back to the caller. - */ - if (!dns_rdatatype_ismulti(fctx->type) || - CHAINING(rdataset)) - { - ardataset = resp->rdataset; - asigset = resp->sigrdataset; - } - } - - if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { - options = DNS_DBADD_PREFETCH; - } - if ((fctx->options & DNS_FETCHOPT_NOCACHED) != 0) { - options |= DNS_DBADD_FORCE; - } - - result = dns_db_addrdataset(fctx->cache, node, NULL, - rctx->now, rdataset, - options, ardataset); - if (result != DNS_R_UNCHANGED && - result != ISC_R_SUCCESS) - { - return result; - } - - if (sigrdataset == NULL) { - return ISC_R_SUCCESS; - } - - if (result == DNS_R_UNCHANGED && !need_validation && - ardataset != NULL && - !dns_rdataset_equals(rdataset, ardataset)) - { - /* - * The cache wasn't updated because - * something was already there. If the data - * was the same as what we were trying to - * add, then sigrdataset might still be - * useful, and we should carry on caching - * it. Otherwise, move on. - */ - return ISC_R_SUCCESS; - } - - result = dns_db_addrdataset(fctx->cache, node, NULL, - rctx->now, sigrdataset, - options, asigset); - if (result != DNS_R_UNCHANGED && - result != ISC_R_SUCCESS) - { - return result; - } - } - + /* + * RRSIGs are validated as part of validating the type they cover. + */ + if (dns_rdatatype_issig(rdataset->type)) { return ISC_R_SUCCESS; } /* - * We're not in a secure domain, or this is glue, - * so we can cache right away. - * - * If this wasn't an ANY/RRSIG/SIG query then send - * an answer back to the caller. + * Ignore unrelated non-answer rdatasets that are missing + * signatures. */ + if (sigrdataset == NULL && need_validation && !ANSWER(rdataset)) { + return ISC_R_SUCCESS; + } + + /* + * Mark this rdataset/sigrdataset pair as "pending". + */ + if (rdataset->trust == dns_trust_additional) { + rdataset->trust = dns_trust_pending_additional; + } else { + rdataset->trust = dns_trust_pending_answer; + } + + if (sigrdataset != NULL) { + sigrdataset->trust = rdataset->trust; + } + + if (ANSWER(rdataset) && need_validation) { + if (!dns_rdatatype_ismulti(fctx->type)) { + /* + * This is The Answer. We will validate it, + * but first we finish caching the rest of the + * response; it may contain useful keys. + */ + INSIST(rctx->vrdataset == NULL && + rctx->vsigrdataset == NULL); + rctx->vrdataset = rdataset; + rctx->vsigrdataset = sigrdataset; + } else { + /* + * This is one of (potentially) multiple answers to + * an ANY query. To keep things simple, we just + * start the validator right away rather than + * caching first and having to remember which + * rdatasets needed validation. + */ + valcreate(fctx, message, query->addrinfo, name, + rdataset->type, rdataset, sigrdataset); + } + } else { + if (ANSWER(rdataset)) { + /* + * We're not validating, but the client might + * be, so look for the NOQNAME proof. + */ + findnoqname(fctx, message, name, rdataset); + + /* + * If this was not an ANY query - or if it was, + * but we got a CNAME/DNAME - then we need to + * set up rdatasets to send back to the caller. + */ + if (resp != NULL && + (!dns_rdatatype_ismulti(fctx->type) || + CHAINING(rdataset))) + { + ardataset = resp->rdataset; + asigset = resp->sigrdataset; + } + } + + if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { + options = DNS_DBADD_PREFETCH; + } + if ((fctx->options & DNS_FETCHOPT_NOCACHED) != 0) { + options |= DNS_DBADD_FORCE; + } + + result = dns_db_addrdataset(fctx->cache, node, NULL, rctx->now, + rdataset, options, ardataset); + if (result != DNS_R_UNCHANGED && result != ISC_R_SUCCESS) { + return result; + } + + if (sigrdataset == NULL) { + return ISC_R_SUCCESS; + } + + if (result == DNS_R_UNCHANGED && !need_validation && + ardataset != NULL && + !dns_rdataset_equals(rdataset, ardataset)) + { + /* + * The cache wasn't updated because something was + * already there. If the data was the same as what + * we were trying to add, then sigrdataset might + * still be useful, and we should carry on caching + * it. Otherwise, move on. + */ + return ISC_R_SUCCESS; + } + + result = dns_db_addrdataset(fctx->cache, node, NULL, rctx->now, + sigrdataset, options, asigset); + if (result != DNS_R_UNCHANGED && result != ISC_R_SUCCESS) { + return result; + } + } + + return ISC_R_SUCCESS; +} + +static isc_result_t +rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, + dns_dbnode_t *node, dns_rdataset_t *rdataset) { + isc_result_t result; + fetchctx_t *fctx = rctx->fctx; + dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); dns_rdataset_t *added = NULL; - if (!dns_rdatatype_ismulti(fctx->type)) { + + /* + * If this was not an ANY query - or if it was, but we got a + * CNAME/DNAME - then we need to set up an rdataset to send + * back to the caller. + */ + if (resp != NULL && + (!dns_rdatatype_ismulti(fctx->type) || CHAINING(rdataset))) + { if (ANSWER(rdataset)) { added = resp->rdataset; } else if (ANSWERSIG(rdataset)) { @@ -5997,15 +5990,16 @@ rctx_cacherdataset(respctx_t *rctx, dns_message_t *message, dns_name_t *name, } } + /* + * If the trust level is glue, then we are adding data from a + * referral we got while executing the search algorithm. New + * referral data always takes precedence over the existing cache + * contents. + */ + unsigned int options = 0; if (rdataset->trust == dns_trust_glue && dns_rdataset_matchestype(rdataset, dns_rdatatype_ns)) { - /* - * If the trust level is glue, then we are adding data from - * a referral we got while executing the search algorithm. - * New referral data always takes precedence over the - * existing cache contents. - */ options = DNS_DBADD_FORCE; } else if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { options = DNS_DBADD_PREFETCH; @@ -6019,7 +6013,7 @@ rctx_cacherdataset(respctx_t *rctx, dns_message_t *message, dns_name_t *name, } /* - * Now we can add the rdataset. + * Cache the rdataset. */ result = dns_db_addrdataset(fctx->cache, node, NULL, rctx->now, rdataset, options, added); @@ -6091,10 +6085,19 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { */ fixttls(res->view, rdataset, sigrdataset); - /* Try to cache the rdataset */ - result = rctx_cacherdataset(rctx, message, name, node, rdataset, - sigrdataset, secure_domain, - need_validation); + /* + * If this is a secure domain and we're not caching + * glue, start validators as needed. Otherwise, cache + * cache now. + */ + if (secure_domain && rdataset->trust != dns_trust_glue) { + result = rctx_cache_secure(rctx, message, name, node, + rdataset, sigrdataset, + need_validation); + } else { + result = rctx_cache_insecure(rctx, message, name, node, + rdataset); + } if (result != ISC_R_SUCCESS) { goto cleanup; } From 9f674c43cfdd9f7ac0f527c63efdd5ce3888b922 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sun, 2 Mar 2025 01:24:08 -0800 Subject: [PATCH 11/12] split out helper functions - fctx_setresult() sets the event result in a fetch response according to the rdataset being returned - DNS_R_NCACHENXDOMAIN or DNS_R_NXRRSET for negative responses, ISC_R_SUCCESS, DNS_R_CNAME, or DNS_R_DNAME for positive ones. - cache_rrset() looks up a node and adds an rdataset. - delete_rrset() looks up a node and removes rdatasets of a specified type and, optionally, the associated signatures. - gettrust() returns the trust level of an rdataset, or dns_trust_none if the rdataset is NULL or not associated. - getrrsig() scans the rdatasets associated with a name for the RRSIG covering a given type. --- lib/dns/resolver.c | 508 +++++++++++++++++++-------------------------- 1 file changed, 210 insertions(+), 298 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 25ac0fa960..fe3b54d1a7 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -665,7 +665,7 @@ fctx_destroy(fetchctx_t *fctx); static isc_result_t negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name, isc_stdtime_t now, bool optout, bool secure, dns_rdataset_t *added, - dns_dbnode_t **nodep, isc_result_t *eresultp); + dns_dbnode_t **nodep); static void validated(void *arg); static void @@ -5175,69 +5175,183 @@ has_000_label(dns_rdataset_t *nsecset) { return false; } +static isc_result_t +fctx_setresult(fetchctx_t *fctx, dns_rdataset_t *rdataset) { + isc_result_t eresult = ISC_R_SUCCESS; + + if (NEGATIVE(rdataset)) { + eresult = NXDOMAIN(rdataset) ? DNS_R_NCACHENXDOMAIN + : DNS_R_NCACHENXRRSET; + } else if (eresult == ISC_R_SUCCESS && rdataset->type != fctx->type) { + switch (rdataset->type) { + case dns_rdatatype_cname: + eresult = DNS_R_CNAME; + break; + case dns_rdatatype_dname: + eresult = DNS_R_DNAME; + break; + default: + break; + } + } + + return eresult; +} + +static inline dns_trust_t +gettrust(dns_rdataset_t *rdataset) { + if (rdataset == NULL || !dns_rdataset_isassociated(rdataset)) { + return dns_trust_none; + } + + return rdataset->trust; +} + +static inline dns_rdataset_t * +getrrsig(dns_name_t *name, dns_rdatatype_t type) { + for (dns_rdataset_t *sig = ISC_LIST_HEAD(name->list); sig != NULL; + sig = ISC_LIST_NEXT(sig, link)) + { + if (dns_rdataset_issigtype(sig, type)) { + return sig; + } + } + + return NULL; +} + +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, + dns_dbnode_t **nodep, dns_rdataset_t *added, + dns_rdataset_t *addedsig) { + isc_result_t result = ISC_R_SUCCESS; + unsigned int options = 0; + dns_dbnode_t *node = NULL; + + if (rdataset == NULL) { + return ISC_R_NOTFOUND; + } + + /* + * If the trust level is glue, we must be caching a referral. + * New referral data always takes precedence over the existing + * cache contents. We also force a cache update if the fctx + * has the _NOCACHED option. + */ + if ((gettrust(rdataset) == dns_trust_glue && + dns_rdataset_matchestype(rdataset, dns_rdatatype_ns)) || + (fctx->options & DNS_FETCHOPT_NOCACHED) != 0) + { + options = DNS_DBADD_FORCE; + } else if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { + options = DNS_DBADD_PREFETCH; + } + + /* + * If the node pointer points to a node, attach to it. + * + * If it points to NULL, find or create the node and pass + * it back to the caller. + * + * If there's no node pointer at all, find the node, but + * detach it before returning. + */ + if (nodep != NULL && *nodep != NULL) { + dns_db_attachnode(fctx->cache, *nodep, &node); + } else { + result = dns_db_findnode(fctx->cache, name, true, &node); + } + + if (result == ISC_R_SUCCESS) { + result = dns_db_addrdataset(fctx->cache, node, NULL, now, + rdataset, options, added); + } + if ((result == ISC_R_SUCCESS || result == DNS_R_UNCHANGED) && + sigrdataset != NULL) + { + result = dns_db_addrdataset(fctx->cache, node, NULL, now, + sigrdataset, options, addedsig); + if (result == DNS_R_UNCHANGED) { + result = ISC_R_SUCCESS; + } + } + + /* + * If we're passing a node that we looked up back to the + * caller, then we don't detach it. + */ + if (nodep != NULL && *nodep == NULL) { + *nodep = node; + } else if (node != NULL) { + dns_db_detachnode(fctx->cache, &node); + } + + return result; +} + +static void +delete_rrset(fetchctx_t *fctx, dns_name_t *name, dns_rdatatype_t type, + bool delrrsig) { + isc_result_t result; + dns_dbnode_t *node = NULL; + + result = dns_db_findnode(fctx->cache, name, false, &node); + if (result == ISC_R_SUCCESS) { + dns_db_deleterdataset(fctx->cache, node, NULL, type, 0); + if (delrrsig) { + dns_db_deleterdataset(fctx->cache, node, NULL, + dns_rdatatype_rrsig, type); + } + } + + if (node != NULL) { + dns_db_detachnode(fctx->cache, &node); + } +} + /* * The validator has finished. */ static void validated(void *arg) { + isc_result_t result = ISC_R_SUCCESS; dns_validator_t *val = (dns_validator_t *)arg; + dns_valarg_t *valarg = val->arg; dns_validator_t *nextval = NULL; dns_adbaddrinfo_t *addrinfo = NULL; dns_dbnode_t *node = NULL; - dns_dbnode_t *nsnode = NULL; dns_fetchresponse_t *resp = NULL; - dns_rdataset_t *ardataset = NULL; - dns_rdataset_t *asigrdataset = NULL; - dns_resolver_t *res = NULL; - dns_valarg_t *valarg = NULL; - fetchctx_t *fctx = NULL; - bool chaining; - bool negative; - bool sentresponse; - isc_result_t eresult = ISC_R_SUCCESS; - isc_result_t result = ISC_R_SUCCESS; - isc_stdtime_t now; - unsigned int options = 0; - dns_fixedname_t fwild; - dns_name_t *wild = NULL; + dns_rdataset_t *ardataset = NULL, *asigrdataset = NULL; dns_message_t *message = NULL; - bool done = false; - - valarg = val->arg; - - REQUIRE(VALID_FCTX(valarg->fctx)); - REQUIRE(!ISC_LIST_EMPTY(valarg->fctx->validators)); + fetchctx_t *fctx = NULL; + dns_resolver_t *res = NULL; + isc_stdtime_t now; + bool sentresponse, done = false; + bool negative = (val->rdataset == NULL); + bool chaining = (val->result == ISC_R_SUCCESS) && !negative && + CHAINING(val->rdataset); fctx = valarg->fctx; valarg->fctx = NULL; + REQUIRE(VALID_FCTX(fctx)); REQUIRE(fctx->tid == isc_tid()); - FCTXTRACE("received validation completion event"); - res = fctx->res; addrinfo = valarg->addrinfo; message = val->message; fctx->vresult = val->result; + FCTXTRACE("received validation completion event"); + LOCK(&fctx->lock); ISC_LIST_UNLINK(fctx->validators, val, link); UNLOCK(&fctx->lock); - /* - * Destroy the validator early so that we can - * destroy the fctx if necessary. Save the wildcard name. - */ - if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { - wild = dns_fixedname_initname(&fwild); - dns_name_copy(dns_fixedname_name(&val->wild), wild); - } - isc_mem_put(fctx->mctx, valarg, sizeof(*valarg)); - negative = (val->rdataset == NULL); - LOCK(&fctx->lock); sentresponse = ((fctx->options & DNS_FETCHOPT_NOVALIDATE) != 0); @@ -5247,30 +5361,11 @@ validated(void *arg) { * events; if so, destroy the fctx. */ if (SHUTTINGDOWN(fctx) && !sentresponse) { - UNLOCK(&fctx->lock); - goto cleanup_fetchctx; + goto unlock; } now = isc_stdtime_now(); - /* - * If chaining, we need to make sure that the right result code - * is returned, and that the rdatasets are bound. - */ - if (val->result == ISC_R_SUCCESS && !negative && - val->rdataset != NULL && CHAINING(val->rdataset)) - { - if (val->rdataset->type == dns_rdatatype_cname) { - eresult = DNS_R_CNAME; - } else { - INSIST(val->rdataset->type == dns_rdatatype_dname); - eresult = DNS_R_DNAME; - } - chaining = true; - } else { - chaining = false; - } - /* * Either we're not shutting down, or we are shutting down but * want to cache the result anyway (if this was a validation @@ -5297,53 +5392,20 @@ validated(void *arg) { fctx->valfail++; fctx->vresult = val->result; if (fctx->vresult != DNS_R_BROKENCHAIN) { - result = ISC_R_NOTFOUND; if (val->rdataset != NULL) { - result = dns_db_findnode(fctx->cache, val->name, - false, &node); + delete_rrset(fctx, val->name, val->type, + (val->sigrdataset != NULL)); } - if (result == ISC_R_SUCCESS) { - (void)dns_db_deleterdataset(fctx->cache, node, - NULL, val->type, 0); - } - if (result == ISC_R_SUCCESS && val->sigrdataset != NULL) - { - (void)dns_db_deleterdataset( - fctx->cache, node, NULL, - dns_rdatatype_rrsig, val->type); - } - if (result == ISC_R_SUCCESS) { - dns_db_detachnode(fctx->cache, &node); - } - } - if (fctx->vresult == DNS_R_BROKENCHAIN && !negative) { + } else if (!negative) { /* * Cache the data as pending for later * validation. */ - result = ISC_R_NOTFOUND; - if (val->rdataset != NULL) { - result = dns_db_findnode(fctx->cache, val->name, - true, &node); - } - if (result == ISC_R_SUCCESS) { - (void)dns_db_addrdataset( - fctx->cache, node, NULL, now, - val->rdataset, 0, NULL); - } - if (result == ISC_R_SUCCESS && val->sigrdataset != NULL) - { - (void)dns_db_addrdataset( - fctx->cache, node, NULL, now, - val->sigrdataset, 0, NULL); - } - if (result == ISC_R_SUCCESS) { - dns_db_detachnode(fctx->cache, &node); - } + cache_rrset(fctx, now, val->name, val->rdataset, + val->sigrdataset, NULL, NULL, NULL); } - result = fctx->vresult; - add_bad(fctx, message, addrinfo, result, badns_validation); + add_bad(fctx, message, addrinfo, result, badns_validation); UNLOCK(&fctx->lock); nextval = ISC_LIST_HEAD(fctx->validators); @@ -5369,16 +5431,15 @@ validated(void *arg) { inc_stats(res, dns_resstatscounter_valnegsuccess); result = negcache(message, fctx, val->name, now, val->optout, - val->secure, ardataset, &node, &eresult); + val->secure, ardataset, &node); if (result != ISC_R_SUCCESS) { goto noanswer_response; } goto answer_response; - } else { - inc_stats(res, dns_resstatscounter_valsuccess); } FCTXTRACE("validation OK"); + inc_stats(res, dns_resstatscounter_valsuccess); if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { result = dns_rdataset_addnoqname( @@ -5392,42 +5453,21 @@ validated(void *arg) { val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER]); RUNTIME_CHECK(result == ISC_R_SUCCESS); } - } else if (val->rdataset->trust == dns_trust_answer) { + } else if (gettrust(val->rdataset) == dns_trust_answer) { findnoqname(fctx, message, val->name, val->rdataset); } /* - * The data was already cached as pending data. - * Re-cache it as secure and bind the cached - * rdatasets to the first event on the fetch - * event list. + * The data was already cached as pending data. Re-cache it as + * secure and bind the cached rdatasets to the first event on the + * fetch event list. */ - result = dns_db_findnode(fctx->cache, val->name, true, &node); + result = cache_rrset(fctx, now, val->name, val->rdataset, + val->sigrdataset, &node, ardataset, asigrdataset); if (result != ISC_R_SUCCESS) { goto noanswer_response; } - if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { - options = DNS_DBADD_PREFETCH; - } - - result = dns_db_addrdataset(fctx->cache, node, NULL, now, val->rdataset, - options, ardataset); - if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) { - goto noanswer_response; - } - if (ardataset != NULL && NEGATIVE(ardataset)) { - eresult = NXDOMAIN(ardataset) ? DNS_R_NCACHENXDOMAIN - : DNS_R_NCACHENXRRSET; - } else if (val->sigrdataset != NULL) { - result = dns_db_addrdataset(fctx->cache, node, NULL, now, - val->sigrdataset, options, - asigrdataset); - if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) { - goto noanswer_response; - } - } - if (sentresponse) { /* * If we only deferred the destroy because we wanted to @@ -5437,8 +5477,7 @@ validated(void *arg) { if (SHUTTINGDOWN(fctx)) { maybe_cancel_validators(fctx); } - UNLOCK(&fctx->lock); - goto cleanup_fetchctx; + goto unlock; } if (!ISC_LIST_EMPTY(fctx->validators)) { @@ -5456,7 +5495,6 @@ validated(void *arg) { } answer_response: - /* * Cache any SOA/NS/NSEC records that happened to be validated. */ @@ -5467,23 +5505,14 @@ answer_response: if ((rdataset->type != dns_rdatatype_ns && rdataset->type != dns_rdatatype_soa && rdataset->type != dns_rdatatype_nsec) || - rdataset->trust != dns_trust_secure) + gettrust(rdataset) != dns_trust_secure) { continue; } - ISC_LIST_FOREACH (name->list, s, link) { - if (dns_rdataset_issigtype(sigrdataset, - rdataset->type)) - { - sigrdataset = s; - break; - } - } + sigrdataset = getrrsig(name, rdataset->type); - if (sigrdataset == NULL || - sigrdataset->trust != dns_trust_secure) - { + if (gettrust(sigrdataset) != dns_trust_secure) { continue; } @@ -5525,21 +5554,11 @@ answer_response: continue; } - result = dns_db_findnode(fctx->cache, name, true, - &nsnode); - if (result != ISC_R_SUCCESS) { - continue; - } - - result = dns_db_addrdataset(fctx->cache, nsnode, NULL, - now, rdataset, 0, NULL); - if (result == ISC_R_SUCCESS) { - result = dns_db_addrdataset( - fctx->cache, nsnode, NULL, now, - sigrdataset, 0, NULL); - } - dns_db_detachnode(fctx->cache, &nsnode); - if (result != ISC_R_SUCCESS) { + result = cache_rrset(fctx, now, name, rdataset, + sigrdataset, NULL, NULL, NULL); + if (result != ISC_R_SUCCESS && + result != DNS_R_UNCHANGED) + { continue; } } @@ -5548,78 +5567,40 @@ answer_response: /* * Add the wild card entry. */ + if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL && - val->rdataset != NULL && dns_rdataset_isassociated(val->rdataset) && - val->rdataset->trust == dns_trust_secure && - val->sigrdataset != NULL && - dns_rdataset_isassociated(val->sigrdataset) && - val->sigrdataset->trust == dns_trust_secure && wild != NULL) + gettrust(val->rdataset) == dns_trust_secure && + gettrust(val->sigrdataset) == dns_trust_secure) { - dns_dbnode_t *wnode = NULL; - - result = dns_db_findnode(fctx->cache, wild, true, &wnode); - if (result == ISC_R_SUCCESS) { - result = dns_db_addrdataset(fctx->cache, wnode, NULL, - now, val->rdataset, 0, - NULL); - } - if (result == ISC_R_SUCCESS) { - (void)dns_db_addrdataset(fctx->cache, wnode, NULL, now, - val->sigrdataset, 0, NULL); - } - if (wnode != NULL) { - dns_db_detachnode(fctx->cache, &wnode); - } - } - - result = ISC_R_SUCCESS; - - if (HAVE_ANSWER(fctx) || resp == NULL) { - goto noanswer_response; + cache_rrset(fctx, now, dns_fixedname_name(&val->wild), + val->rdataset, val->sigrdataset, NULL, NULL, NULL); } /* - * Respond with an answer, positive or negative, - * as opposed to an error. 'node' must be non-NULL. + * Respond with an answer, positive or negative, as + * opposed to an error. */ - FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); - INSIST(resp->rdataset != NULL); - if (dns_rdataset_isassociated(resp->rdataset)) { - if (NEGATIVE(resp->rdataset)) { - INSIST(eresult == DNS_R_NCACHENXDOMAIN || - eresult == DNS_R_NCACHENXRRSET); - } else if (eresult == ISC_R_SUCCESS && - resp->rdataset->type != fctx->type) - { - switch (resp->rdataset->type) { - case dns_rdatatype_cname: - eresult = DNS_R_CNAME; - break; - case dns_rdatatype_dname: - eresult = DNS_R_DNAME; - break; - default: - break; - } - } - } - - resp->result = eresult; + INSIST(resp != NULL && resp->rdataset != NULL); + resp->result = fctx_setresult(fctx, resp->rdataset); dns_name_copy(val->name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); clone_results(fctx); + result = ISC_R_SUCCESS; + noanswer_response: if (node != NULL) { dns_db_detachnode(fctx->cache, &node); } - UNLOCK(&fctx->lock); done = true; +unlock: + UNLOCK(&fctx->lock); + cleanup_fetchctx: if (done) { fctx_done_unref(fctx, result); @@ -5676,13 +5657,7 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, /* * Find the SIG for this rdataset, if we have it. */ - ISC_LIST_FOREACH (name->list, sig, link) { - if (dns_rdataset_issigtype(sig, type)) { - sigrdataset = sig; - break; - } - } - + sigrdataset = getrrsig(name, type); if (sigrdataset == NULL) { return; } @@ -5753,12 +5728,7 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, } if (noqname != NULL) { - ISC_LIST_FOREACH (noqname->list, sig, link) { - if (dns_rdataset_issigtype(sig, found)) { - sigrdataset = sig; - break; - } - } + sigrdataset = getrrsig(noqname, found); if (sigrdataset == NULL) { noqname = NULL; } @@ -5852,7 +5822,6 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, resquery_t *query = rctx->query; dns_rdataset_t *ardataset = NULL, *asigset = NULL; dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); - unsigned int options = 0; /* * RRSIGs are validated as part of validating the type they cover. @@ -5926,15 +5895,13 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, } } - if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { - options = DNS_DBADD_PREFETCH; - } - if ((fctx->options & DNS_FETCHOPT_NOCACHED) != 0) { - options |= DNS_DBADD_FORCE; - } - - result = dns_db_addrdataset(fctx->cache, node, NULL, rctx->now, - rdataset, options, ardataset); + /* + * In this case we cache the rdataset and sigrdataset + * (if any) in two steps, so we can do an extra check + * in-between. + */ + result = cache_rrset(fctx, rctx->now, name, rdataset, NULL, + &node, ardataset, NULL); if (result != DNS_R_UNCHANGED && result != ISC_R_SUCCESS) { return result; } @@ -5957,8 +5924,8 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, return ISC_R_SUCCESS; } - result = dns_db_addrdataset(fctx->cache, node, NULL, rctx->now, - sigrdataset, options, asigset); + result = cache_rrset(fctx, rctx->now, name, sigrdataset, NULL, + &node, asigset, NULL); if (result != DNS_R_UNCHANGED && result != ISC_R_SUCCESS) { return result; } @@ -5990,21 +5957,6 @@ rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, } } - /* - * If the trust level is glue, then we are adding data from a - * referral we got while executing the search algorithm. New - * referral data always takes precedence over the existing cache - * contents. - */ - unsigned int options = 0; - if (rdataset->trust == dns_trust_glue && - dns_rdataset_matchestype(rdataset, dns_rdatatype_ns)) - { - options = DNS_DBADD_FORCE; - } else if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { - options = DNS_DBADD_PREFETCH; - } - /* * Look for the NOQNAME proof. */ @@ -6015,8 +5967,8 @@ rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, /* * Cache the rdataset. */ - result = dns_db_addrdataset(fctx->cache, node, NULL, rctx->now, - rdataset, options, added); + result = cache_rrset(fctx, rctx->now, name, rdataset, NULL, &node, + added, NULL); if (result == DNS_R_UNCHANGED) { result = ISC_R_SUCCESS; } @@ -6072,12 +6024,7 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { /* * Find the RRSIG for this rdataset, if we have it. */ - ISC_LIST_FOREACH (name->list, sig, link) { - if (dns_rdataset_issigtype(sig, rdataset->type)) { - sigrdataset = sig; - break; - } - } + sigrdataset = getrrsig(name, rdataset->type); /* * Make the TTLs consistent with the configured @@ -6090,7 +6037,7 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { * glue, start validators as needed. Otherwise, cache * cache now. */ - if (secure_domain && rdataset->trust != dns_trust_glue) { + if (secure_domain && gettrust(rdataset) != dns_trust_glue) { result = rctx_cache_secure(rctx, message, name, node, rdataset, sigrdataset, need_validation); @@ -6121,30 +6068,13 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { { resp = ISC_LIST_HEAD(fctx->resps); if (resp != NULL) { - /* - * Negative results must be indicated in - * resp->result. - */ - resp->result = ISC_R_SUCCESS; + isc_result_t eresult = ISC_R_SUCCESS; + if (dns_rdataset_isassociated(resp->rdataset)) { - if (NXDOMAIN(resp->rdataset)) { - resp->result = DNS_R_NCACHENXDOMAIN; - } else if (NEGATIVE(resp->rdataset)) { - resp->result = DNS_R_NCACHENXRRSET; - } else if (resp->rdataset->type != fctx->type) { - switch (resp->rdataset->type) { - case dns_rdatatype_cname: - resp->result = DNS_R_CNAME; - break; - case dns_rdatatype_dname: - resp->result = DNS_R_DNAME; - break; - default: - break; - } - } + eresult = fctx_setresult(fctx, resp->rdataset); } + resp->result = eresult; dns_name_copy(name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); @@ -6199,7 +6129,7 @@ cleanup: static isc_result_t negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name, isc_stdtime_t now, bool optout, bool secure, dns_rdataset_t *added, - dns_dbnode_t **nodep, isc_result_t *eresultp) { + dns_dbnode_t **nodep) { isc_result_t result; dns_ttl_t minttl = fctx->res->view->minncachettl; dns_ttl_t maxttl = fctx->res->view->maxncachettl; @@ -6254,24 +6184,7 @@ 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); - if (result == DNS_R_UNCHANGED || result == ISC_R_SUCCESS) { - /* - * The cache now either contains the negative entry we - * were adding, or a pre-existing entry (positive or - * negative) that was left alone. Set the event result - * accordingly. - */ - if (NXDOMAIN(added)) { - *eresultp = DNS_R_NCACHENXDOMAIN; - } else if (NEGATIVE(added)) { - *eresultp = DNS_R_NCACHENXRRSET; - } else if (added->type == dns_rdatatype_cname) { - *eresultp = DNS_R_CNAME; - } else if (added->type == dns_rdatatype_dname) { - *eresultp = DNS_R_DNAME; - } else { - *eresultp = ISC_R_SUCCESS; - } + if (result == DNS_R_UNCHANGED) { result = ISC_R_SUCCESS; } @@ -6291,7 +6204,6 @@ negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name, static void rctx_ncache(respctx_t *rctx) { isc_result_t result = ISC_R_SUCCESS; - isc_result_t eresult = ISC_R_SUCCESS; fetchctx_t *fctx = rctx->fctx; dns_name_t *name = fctx->name; dns_message_t *message = rctx->query->rmessage; @@ -6356,14 +6268,14 @@ rctx_ncache(respctx_t *rctx) { } result = negcache(message, fctx, name, rctx->now, false, false, added, - &node, &eresult); + &node); if (result != ISC_R_SUCCESS || HAVE_ANSWER(fctx)) { goto unlock; } FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); if (resp != NULL) { - resp->result = eresult; + resp->result = fctx_setresult(fctx, resp->rdataset); dns_name_copy(name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); From 5a2938b45231eeb48a708a53caf650854583f211 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 26 Feb 2025 13:59:19 -0800 Subject: [PATCH 12/12] refactor validated() - there was special-case code in validated() to handle the results of a validator started by a CD=1 query. since that never happens, the code has been removed. - the section of code that handles opportunistic caching of validated SOA, NS and NSEC data has been split out to a separate function. - the number of goto statements has been reduced considerably. --- lib/dns/resolver.c | 463 ++++++++++++++++++++++----------------------- 1 file changed, 223 insertions(+), 240 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index fe3b54d1a7..2d40ed2d96 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -675,7 +675,7 @@ add_bad(fetchctx_t *fctx, dns_message_t *rmessage, dns_adbaddrinfo_t *addrinfo, isc_result_t reason, badnstype_t badtype); static void findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, - dns_rdataset_t *rdataset); + dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset); #define fctx_done_detach(fctxp, result) \ if (fctx__done(*fctxp, result, __func__, __FILE__, __LINE__)) { \ @@ -1559,8 +1559,8 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { } /* - * Finalize the EDE context, so it becomes "constant" and assign - * it to all clients. + * Finalize the EDE context so it becomes "constant", and + * assign it to all clients. */ if (resp->edectx != NULL) { dns_ede_copy(resp->edectx, &fctx->edectx); @@ -5310,191 +5310,11 @@ delete_rrset(fetchctx_t *fctx, dns_name_t *name, dns_rdatatype_t type, } } -/* - * The validator has finished. - */ static void -validated(void *arg) { - isc_result_t result = ISC_R_SUCCESS; - dns_validator_t *val = (dns_validator_t *)arg; - dns_valarg_t *valarg = val->arg; - dns_validator_t *nextval = NULL; - dns_adbaddrinfo_t *addrinfo = NULL; - dns_dbnode_t *node = NULL; - dns_fetchresponse_t *resp = NULL; - dns_rdataset_t *ardataset = NULL, *asigrdataset = NULL; - dns_message_t *message = NULL; - fetchctx_t *fctx = NULL; - dns_resolver_t *res = NULL; - isc_stdtime_t now; - bool sentresponse, done = false; - bool negative = (val->rdataset == NULL); - bool chaining = (val->result == ISC_R_SUCCESS) && !negative && - CHAINING(val->rdataset); +fctx_cacheauthority(fetchctx_t *fctx, dns_message_t *message, + isc_stdtime_t now) { + isc_result_t result; - fctx = valarg->fctx; - valarg->fctx = NULL; - - REQUIRE(VALID_FCTX(fctx)); - REQUIRE(fctx->tid == isc_tid()); - - res = fctx->res; - addrinfo = valarg->addrinfo; - - message = val->message; - fctx->vresult = val->result; - - FCTXTRACE("received validation completion event"); - - LOCK(&fctx->lock); - ISC_LIST_UNLINK(fctx->validators, val, link); - UNLOCK(&fctx->lock); - - isc_mem_put(fctx->mctx, valarg, sizeof(*valarg)); - - LOCK(&fctx->lock); - sentresponse = ((fctx->options & DNS_FETCHOPT_NOVALIDATE) != 0); - - /* - * If shutting down, ignore the results. Check to see if we're - * done waiting for validator completions and ADB pending - * events; if so, destroy the fctx. - */ - if (SHUTTINGDOWN(fctx) && !sentresponse) { - goto unlock; - } - - now = isc_stdtime_now(); - - /* - * Either we're not shutting down, or we are shutting down but - * want to cache the result anyway (if this was a validation - * started by a query with cd set) - */ - - resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL) { - if (!negative && !chaining && dns_rdatatype_ismulti(fctx->type)) - { - /* - * Don't bind rdatasets; the caller - * will iterate the node. - */ - } else { - ardataset = resp->rdataset; - asigrdataset = resp->sigrdataset; - } - } - - if (val->result != ISC_R_SUCCESS) { - FCTXTRACE("validation failed"); - inc_stats(res, dns_resstatscounter_valfail); - fctx->valfail++; - fctx->vresult = val->result; - if (fctx->vresult != DNS_R_BROKENCHAIN) { - if (val->rdataset != NULL) { - delete_rrset(fctx, val->name, val->type, - (val->sigrdataset != NULL)); - } - } else if (!negative) { - /* - * Cache the data as pending for later - * validation. - */ - cache_rrset(fctx, now, val->name, val->rdataset, - val->sigrdataset, NULL, NULL, NULL); - } - - add_bad(fctx, message, addrinfo, result, badns_validation); - UNLOCK(&fctx->lock); - - nextval = ISC_LIST_HEAD(fctx->validators); - if (nextval != NULL) { - dns_validator_send(nextval); - goto cleanup_fetchctx; - } else if (sentresponse) { - done = true; - goto cleanup_fetchctx; - } else if (result == DNS_R_BROKENCHAIN) { - done = true; - goto cleanup_fetchctx; - } else { - fctx_try(fctx, true); - goto cleanup_fetchctx; - } - UNREACHABLE(); - } - - if (negative) { - FCTXTRACE("nonexistence validation OK"); - - inc_stats(res, dns_resstatscounter_valnegsuccess); - - result = negcache(message, fctx, val->name, now, val->optout, - val->secure, ardataset, &node); - if (result != ISC_R_SUCCESS) { - goto noanswer_response; - } - goto answer_response; - } - - FCTXTRACE("validation OK"); - inc_stats(res, dns_resstatscounter_valsuccess); - - if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { - result = dns_rdataset_addnoqname( - val->rdataset, val->proofs[DNS_VALIDATOR_NOQNAMEPROOF]); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - INSIST(val->sigrdataset != NULL); - val->sigrdataset->ttl = val->rdataset->ttl; - if (val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER] != NULL) { - result = dns_rdataset_addclosest( - val->rdataset, - val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER]); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - } - } else if (gettrust(val->rdataset) == dns_trust_answer) { - findnoqname(fctx, message, val->name, val->rdataset); - } - - /* - * The data was already cached as pending data. Re-cache it as - * secure and bind the cached rdatasets to the first event on the - * fetch event list. - */ - result = cache_rrset(fctx, now, val->name, val->rdataset, - val->sigrdataset, &node, ardataset, asigrdataset); - if (result != ISC_R_SUCCESS) { - goto noanswer_response; - } - - if (sentresponse) { - /* - * If we only deferred the destroy because we wanted to - * cache the data, destroy now. - */ - dns_db_detachnode(fctx->cache, &node); - if (SHUTTINGDOWN(fctx)) { - maybe_cancel_validators(fctx); - } - goto unlock; - } - - if (!ISC_LIST_EMPTY(fctx->validators)) { - INSIST(!negative); - INSIST(dns_rdatatype_ismulti(fctx->type)); - - /* - * Don't send a response yet - we have more rdatasets - * that still need to be validated. - */ - dns_db_detachnode(fctx->cache, &node); - UNLOCK(&fctx->lock); - dns_validator_send(ISC_LIST_HEAD(fctx->validators)); - goto cleanup_fetchctx; - } - -answer_response: /* * Cache any SOA/NS/NSEC records that happened to be validated. */ @@ -5511,7 +5331,6 @@ answer_response: } sigrdataset = getrrsig(name, rdataset->type); - if (gettrust(sigrdataset) != dns_trust_secure) { continue; } @@ -5563,11 +5382,178 @@ answer_response: } } } +} + +/* + * The validator has finished. + */ +static void +validated(void *arg) { + isc_result_t result = ISC_R_SUCCESS; + dns_validator_t *val = (dns_validator_t *)arg; + dns_valarg_t *valarg = val->arg; + dns_validator_t *nextval = NULL; + dns_adbaddrinfo_t *addrinfo = NULL; + dns_dbnode_t *node = NULL; + dns_fetchresponse_t *resp = NULL; + dns_rdataset_t *ardataset = NULL, *asigrdataset = NULL; + dns_message_t *message = NULL; + fetchctx_t *fctx = NULL; + dns_resolver_t *res = NULL; + isc_stdtime_t now; + bool done = false; + bool negative = (val->rdataset == NULL); + bool chaining = (val->result == ISC_R_SUCCESS) && !negative && + CHAINING(val->rdataset); + + fctx = valarg->fctx; + valarg->fctx = NULL; + + REQUIRE(VALID_FCTX(fctx)); + REQUIRE(fctx->tid == isc_tid()); + + res = fctx->res; + addrinfo = valarg->addrinfo; + + message = val->message; + fctx->vresult = val->result; + + FCTXTRACE("received validation completion event"); + + isc_mem_put(fctx->mctx, valarg, sizeof(*valarg)); + + LOCK(&fctx->lock); + + ISC_LIST_UNLINK(fctx->validators, val, link); + + if (SHUTTINGDOWN(fctx)) { + goto cleanup; + } + + now = isc_stdtime_now(); + + /* Validation failed. */ + if (val->result != ISC_R_SUCCESS) { + FCTXTRACE("validation failed"); + inc_stats(res, dns_resstatscounter_valfail); + fctx->valfail++; + fctx->vresult = val->result; + if (fctx->vresult != DNS_R_BROKENCHAIN) { + if (val->rdataset != NULL) { + delete_rrset(fctx, val->name, val->type, + val->sigrdataset != NULL); + } + } else if (!negative) { + /* + * Cache the data as pending for later validation. + */ + cache_rrset(fctx, now, val->name, val->rdataset, + val->sigrdataset, NULL, NULL, NULL); + } + + add_bad(fctx, message, addrinfo, result, badns_validation); + + /* Start the next validator if there is one. */ + nextval = ISC_LIST_HEAD(fctx->validators); + if (nextval != NULL) { + goto cleanup; + } + + /* A broken trust chain isn't recoverable. */ + if (result == DNS_R_BROKENCHAIN) { + done = true; + goto cleanup; + } + + /* + * Some other error, we can try again. We have to + * unlock the fctx before calling fctx_try(). + */ + UNLOCK(&fctx->lock); + fctx_try(fctx, true); + goto cleanup_unlocked; + } /* - * Add the wild card entry. + * For non-ANY responses, and all negative and chaining responses, + * we pass an rdataset back to the caller. Otherwise the caller + * iterates the node. */ + resp = ISC_LIST_HEAD(fctx->resps); + if (resp != NULL && + (negative || chaining || !dns_rdatatype_ismulti(fctx->type))) + { + ardataset = resp->rdataset; + asigrdataset = resp->sigrdataset; + } + /* + * Validator proved nonexistence. + */ + if (negative) { + FCTXTRACE("nonexistence validation OK"); + inc_stats(res, dns_resstatscounter_valnegsuccess); + + result = negcache(message, fctx, val->name, now, val->optout, + val->secure, ardataset, &node); + if (result != ISC_R_SUCCESS) { + done = true; + goto cleanup; + } + goto answer_response; + } + + /* + * Validator proved a positive answer. + */ + FCTXTRACE("validation OK"); + inc_stats(res, dns_resstatscounter_valsuccess); + + if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { + result = dns_rdataset_addnoqname( + val->rdataset, val->proofs[DNS_VALIDATOR_NOQNAMEPROOF]); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + INSIST(val->sigrdataset != NULL); + val->sigrdataset->ttl = val->rdataset->ttl; + if (val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER] != NULL) { + result = dns_rdataset_addclosest( + val->rdataset, + val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER]); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + } + } else if (gettrust(val->rdataset) == dns_trust_answer) { + findnoqname(fctx, message, val->name, val->rdataset, + val->sigrdataset); + } + + /* + * The data was already cached as pending. Re-cache it as secure. + */ + result = cache_rrset(fctx, now, val->name, val->rdataset, + val->sigrdataset, &node, ardataset, asigrdataset); + if (result != ISC_R_SUCCESS) { + done = true; + goto cleanup; + } + + /* + * If this was an ANY query, we might have more rdatasets + * needing to be validated before we can respond. + */ + if (!ISC_LIST_EMPTY(fctx->validators)) { + INSIST(!negative); + INSIST(dns_rdatatype_ismulti(fctx->type)); + + nextval = ISC_LIST_HEAD(fctx->validators); + goto cleanup; + } + +answer_response: + fctx_cacheauthority(fctx, message, now); + + /* + * Cache the wild card entry. + */ if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL && gettrust(val->rdataset) == dns_trust_secure && gettrust(val->sigrdataset) == dns_trust_secure) @@ -5577,31 +5563,33 @@ answer_response: } /* - * Respond with an answer, positive or negative, as - * opposed to an error. + * We're responding with an answer, positive or negative, + * not an error. */ - FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); + if (resp != NULL) { + FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); - INSIST(resp != NULL && resp->rdataset != NULL); - resp->result = fctx_setresult(fctx, resp->rdataset); - dns_name_copy(val->name, resp->foundname); - dns_db_attach(fctx->cache, &resp->db); - dns_db_transfernode(fctx->cache, &node, &resp->node); - clone_results(fctx); - - result = ISC_R_SUCCESS; - -noanswer_response: - if (node != NULL) { - dns_db_detachnode(fctx->cache, &node); + resp->result = fctx_setresult(fctx, resp->rdataset); + dns_name_copy(val->name, resp->foundname); + dns_db_attach(fctx->cache, &resp->db); + dns_db_transfernode(fctx->cache, &node, &resp->node); + clone_results(fctx); } done = true; -unlock: +cleanup: UNLOCK(&fctx->lock); +cleanup_unlocked: + + if (node != NULL) { + dns_db_detachnode(fctx->cache, &node); + } + + if (nextval != NULL) { + dns_validator_send(nextval); + } -cleanup_fetchctx: if (done) { fctx_done_unref(fctx, result); } @@ -5633,9 +5621,8 @@ fctx_log(void *arg, int level, const char *fmt, ...) { static void findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, - dns_rdataset_t *rdataset) { + dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) { isc_result_t result; - dns_rdataset_t *sigrdataset = NULL; dns_rdata_rrsig_t rrsig; unsigned int labels; dns_name_t *zonename = NULL; @@ -5650,15 +5637,7 @@ findnoqname(fetchctx_t *fctx, dns_message_t *message, dns_name_t *name, FCTXTRACE("findnoqname"); - if (dns_rdatatype_issig(rdataset->type)) { - return; - } - - /* - * Find the SIG for this rdataset, if we have it. - */ - sigrdataset = getrrsig(name, type); - if (sigrdataset == NULL) { + if (dns_rdatatype_issig(rdataset->type) || sigrdataset == NULL) { return; } @@ -5804,9 +5783,7 @@ fixttls(dns_view_t *view, dns_rdataset_t *rdataset, rdataset->attributes.prefetch = true; } - /* - * Normalize the rdataset and sigrdataset TTLs. - */ + /* Normalize the rdataset and sigrdataset TTLs */ if (sigrdataset != NULL) { rdataset->ttl = ISC_MIN(rdataset->ttl, sigrdataset->ttl); sigrdataset->ttl = rdataset->ttl; @@ -5879,7 +5856,7 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, * We're not validating, but the client might * be, so look for the NOQNAME proof. */ - findnoqname(fctx, message, name, rdataset); + findnoqname(fctx, message, name, rdataset, sigrdataset); /* * If this was not an ANY query - or if it was, @@ -5936,7 +5913,8 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, static isc_result_t rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, - dns_dbnode_t *node, dns_rdataset_t *rdataset) { + dns_dbnode_t *node, dns_rdataset_t *rdataset, + dns_rdataset_t *sigrdataset) { isc_result_t result; fetchctx_t *fctx = rctx->fctx; dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); @@ -5961,7 +5939,7 @@ rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, * Look for the NOQNAME proof. */ if (ANSWER(rdataset)) { - findnoqname(fctx, message, name, rdataset); + findnoqname(fctx, message, name, rdataset, sigrdataset); } /* @@ -5984,7 +5962,6 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { dns_resolver_t *res = fctx->res; dns_rdataset_t *sigrdataset = NULL; dns_dbnode_t *node = NULL; - dns_fetchresponse_t *resp = NULL; FCTXTRACE("rctx_cachename"); @@ -6021,35 +5998,38 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { goto cleanup; } - /* - * Find the RRSIG for this rdataset, if we have it. - */ + /* Find the signature for this rdataset */ sigrdataset = getrrsig(name, rdataset->type); /* - * Make the TTLs consistent with the configured - * maximum and minimum and with each other. + * Make the TTL consistent with the configured + * maximum and minimum */ fixttls(res->view, rdataset, sigrdataset); - /* - * If this is a secure domain and we're not caching - * glue, start validators as needed. Otherwise, cache - * cache now. - */ if (secure_domain && gettrust(rdataset) != dns_trust_glue) { + /* + * If this is a secure domain and the rdataset + * isn't glue, start a validator. The data will + * be cached when the validator finishes. + */ result = rctx_cache_secure(rctx, message, name, node, rdataset, sigrdataset, need_validation); } else { + /* Insecure domain or glue: cache the data now. */ result = rctx_cache_insecure(rctx, message, name, node, - rdataset); + rdataset, sigrdataset); } if (result != ISC_R_SUCCESS) { goto cleanup; } } + /* + * If there was a delayed validation set up in + * rctx_cache_secure(), run it now. + */ if (rctx->vrdataset != NULL) { dns_rdatatype_t vtype = fctx->type; if (CHAINING(rctx->vrdataset)) { @@ -6061,12 +6041,15 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { rctx->vrdataset, rctx->vsigrdataset); rctx->vrdataset = NULL; rctx->vsigrdataset = NULL; + goto cleanup; } - if (result == ISC_R_SUCCESS && name->attributes.answer && - !need_validation && !HAVE_ANSWER(fctx)) - { - resp = ISC_LIST_HEAD(fctx->resps); + /* + * We're not validating and have an answer ready; pass + * it back to the caller. + */ + if (!need_validation && name->attributes.answer && !HAVE_ANSWER(fctx)) { + dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); if (resp != NULL) { isc_result_t eresult = ISC_R_SUCCESS; @@ -6079,9 +6062,9 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); clone_results(fctx); - } - FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); + FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); + } } cleanup: