]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
split out some functionality in cache_name()
authorEvan Hunt <each@isc.org>
Thu, 27 Feb 2025 22:28:37 +0000 (14:28 -0800)
committerOndřej Surý <ondrej@isc.org>
Tue, 5 Aug 2025 10:16:36 +0000 (12:16 +0200)
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

index d2497ba373b329f4c27bca60e3a61b322470fb80..cd44a0dbdf9dc861edcf94c7559716be4acc3217 100644 (file)
@@ -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;
                }
 
                /*
@@ -5901,21 +5928,24 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                        }
                }
 
+               /*
+                * Make the TTLs consistent with the configured
+                * maximum and minimum and with each other.
+                */
+               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
+                * 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.)
+                * only cached if validated within the context of a query
+                * to the domain that owns them.
                 */
-               if (secure_domain && rdataset->trust != dns_trust_glue &&
-                   !EXTERNAL(rdataset))
-               {
-                       dns_trust_t trust;
-
+               if (secure_domain && rdataset->trust != dns_trust_glue) {
                        /*
                         * RRSIGs are validated as part of validating
                         * the type they cover.
@@ -5958,17 +5988,45 @@ 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;
+                               sigrdataset->trust = rdataset->trust;
                        }
-                       if (!need_validation || !ANSWER(rdataset)) {
-                               options = 0;
+
+                       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) &&
                                    rdataset->type != dns_rdatatype_rrsig)
                                {
@@ -5994,15 +6052,14 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                {
                                        options |= DNS_DBADD_FORCE;
                                }
-                               addedrdataset = ardataset;
-                               result = dns_db_addrdataset(
-                                       fctx->cache, node, NULL, now, rdataset,
-                                       options, addedrdataset);
+                               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 &&
-                                           ardataset != NULL &&
-                                           NEGATIVE(ardataset))
+                                       if (!need_validation && added != NULL &&
+                                           NEGATIVE(added))
                                        {
                                                /*
                                                 * The answer in the
@@ -6013,7 +6070,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                                 * must set eresult
                                                 * appropriately.
                                                 */
-                                               if (NXDOMAIN(ardataset)) {
+                                               if (NXDOMAIN(added)) {
                                                        eresult =
                                                                DNS_R_NCACHENXDOMAIN;
                                                } else {
@@ -6022,10 +6079,10 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                                }
                                                continue;
                                        } else if (!need_validation &&
-                                                  ardataset != NULL &&
+                                                  added != NULL &&
                                                   sigrdataset != NULL &&
                                                   !dns_rdataset_equals(
-                                                          rdataset, ardataset))
+                                                          rdataset, added))
                                        {
                                                /*
                                                 * The cache wasn't updated
@@ -6045,11 +6102,10 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                        break;
                                }
                                if (sigrdataset != NULL) {
-                                       addedrdataset = asigrdataset;
+                                       added = asigrdataset;
                                        result = dns_db_addrdataset(
                                                fctx->cache, node, NULL, now,
-                                               sigrdataset, options,
-                                               addedrdataset);
+                                               sigrdataset, options, added);
                                        if (result == DNS_R_UNCHANGED) {
                                                result = ISC_R_SUCCESS;
                                        }
@@ -6059,65 +6115,29 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                } else if (!ANSWER(rdataset)) {
                                        continue;
                                }
-                       }
 
-                       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, valoptions);
-                               }
-                       } 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;
+                               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