]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Fix missing RRSIGs for "glue" lookups with CD=1
authorMark Andrews <marka@isc.org>
Wed, 10 Sep 2025 06:18:41 +0000 (16:18 +1000)
committerMark Andrews <marka@isc.org>
Wed, 10 Sep 2025 07:08:52 +0000 (17:08 +1000)
The code to test whether to store the RRSIGs on DNS_R_UNCHANGED
with CD=1 was failing because the comparison methods of the two
rdatatset instances were not compatible.  Move the testing into
dns_db_addrdataset(), and request it by setting the DNS_ADD_EQUALOK
option.  If the option is set and the old and new rrsets compare
as equal, dns_db_addrdataset() returns ISC_R_SUCCESS instead of
DNS_R_UNCHANGED.

(cherry picked from commit b954a1df43e6e6e5ff60f1da1240ece644b7e190)

lib/dns/include/dns/db.h
lib/dns/qpcache.c
lib/dns/rbtdb.c
lib/dns/resolver.c

index 254b7d38e507ca6e62cd7d8d3bef8ab687ce9011..081c8ae8345ee8463b38f8177adfe9aeacc534f5 100644 (file)
@@ -302,6 +302,7 @@ enum {
 #define DNS_DBADD_EXACT           0x04
 #define DNS_DBADD_EXACTTTL 0x08
 #define DNS_DBADD_PREFETCH 0x10
+#define DNS_DBADD_EQUALOK  0x20
 /*@}*/
 
 /*%
@@ -1248,6 +1249,10 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
  *     the old and new rdata sets.  If #DNS_DBADD_EXACTTTL is set then both
  *     the old and new rdata sets must have the same ttl.
  *
+ * \li If the #DNS_DBADD_EQUALOK option is set, and the database is not
+ *     changed, compare the old and new rdatasets; if they are equal,
+ *     return #ISC_R_SUCCESS instead of #DNS_R_UNCHANGED.
+ *
  * \li The 'now' field is ignored if 'db' is a zone database.  If 'db' is
  *     a cache database, then the added rdataset will expire no later than
  *     now + rdataset->ttl.
@@ -1277,8 +1282,12 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
  * Returns:
  *
  * \li #ISC_R_SUCCESS
- * \li #DNS_R_UNCHANGED                        The operation did not change
- * anything. \li       #ISC_R_NOMEMORY \li     #DNS_R_NOTEXACT
+ * \li #DNS_R_UNCHANGED        The operation did not change anything.
+ * \li #ISC_R_NOMEMORY
+ * \li #DNS_R_NOTEXACT         The TTL didn't match and #DNS_DBADD_EXACTTTL
+ *                             was set, or there was a partial overlap
+ *                             between the old and new rdatasets and
+ *                             DNS_DBADD_EXACT was set.
  *
  * \li Other results are possible, depending upon the database
  *     implementation used.
index c631f27231dceb8cd35397b1eedf9d20e20df078..2ef95b9cf465c50a27001eecff9430484eec42c4 100644 (file)
@@ -2993,13 +2993,23 @@ find_header:
                 */
                if (trust < header->trust && (ACTIVE(header, now) || header_nx))
                {
-                       dns_slabheader_destroy(&newheader);
-                       if (addedrdataset != NULL) {
-                               bindrdataset(qpdb, qpnode, header, now,
-                                            nlocktype, tlocktype,
-                                            addedrdataset DNS__DB_FLARG_PASS);
+                       isc_result_t result = DNS_R_UNCHANGED;
+                       bindrdataset(qpdb, qpnode, header, now, nlocktype,
+                                    tlocktype,
+                                    addedrdataset DNS__DB_FLARG_PASS);
+                       if (ACTIVE(header, now) &&
+                           (options & DNS_DBADD_EQUALOK) != 0 &&
+                           dns_rdataslab_equalx(
+                                   (unsigned char *)header,
+                                   (unsigned char *)newheader,
+                                   (unsigned int)(sizeof(*newheader)),
+                                   qpdb->common.rdclass,
+                                   (dns_rdatatype_t)header->type))
+                       {
+                               result = ISC_R_SUCCESS;
                        }
-                       return DNS_R_UNCHANGED;
+                       dns_slabheader_destroy(&newheader);
+                       return result;
                }
 
                /*
index 4cb3b9cd47337b480cea94b254a6ceb33f33ed1b..105f5f169304438199413ee5c3b4bf40e211e3ac 100644 (file)
@@ -2729,14 +2729,24 @@ find_header:
                if (rbtversion == NULL && trust < header->trust &&
                    (ACTIVE(header, now) || header_nx))
                {
-                       dns_slabheader_destroy(&newheader);
-                       if (addedrdataset != NULL) {
-                               dns__rbtdb_bindrdataset(
-                                       rbtdb, rbtnode, header, now,
-                                       isc_rwlocktype_write,
-                                       addedrdataset DNS__DB_FLARG_PASS);
+                       result = DNS_R_UNCHANGED;
+                       dns__rbtdb_bindrdataset(
+                               rbtdb, rbtnode, header, now,
+                               isc_rwlocktype_write,
+                               addedrdataset DNS__DB_FLARG_PASS);
+                       if (ACTIVE(header, now) &&
+                           (options & DNS_DBADD_EQUALOK) != 0 &&
+                           dns_rdataslab_equalx(
+                                   (unsigned char *)header,
+                                   (unsigned char *)newheader,
+                                   (unsigned int)(sizeof(*newheader)),
+                                   rbtdb->common.rdclass,
+                                   (dns_rdatatype_t)header->type))
+                       {
+                               result = ISC_R_SUCCESS;
                        }
-                       return DNS_R_UNCHANGED;
+                       dns_slabheader_destroy(&newheader);
+                       return result;
                }
 
                /*
index bcac380e8fa95ce6767ed996066fb5ff835a9056..7163b54a8b8f76aff80a8beb28ac3f8e9ca54ab5 100644 (file)
@@ -5877,7 +5877,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
        bool have_answer = false;
        isc_result_t result, eresult = ISC_R_SUCCESS;
        dns_fetchresponse_t *resp = NULL;
-       unsigned int options;
+       unsigned int options = 0, equalok = 0;
        bool fail;
        unsigned int valoptions = 0;
        bool checknta = true;
@@ -6089,6 +6089,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                        }
                        if (!need_validation || !ANSWER(rdataset)) {
                                options = 0;
+                               equalok = 0;
                                if (ANSWER(rdataset) &&
                                    rdataset->type != dns_rdatatype_rrsig)
                                {
@@ -6114,10 +6115,23 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                {
                                        options |= DNS_DBADD_FORCE;
                                }
+                               /*
+                                * If we're validating and passing the added
+                                * rdataset back to the caller, then we ask
+                                * dns_db_addrdataset() to compare the old and
+                                * new rdatasets whenever the result would
+                                * normally have been DNS_R_UNCHANGED, and to
+                                * return ISC_R_SUCCESS if they compare equal.
+                                * This allows us to continue and cache RRSIGs
+                                * in that case.
+                                */
+                               if (!need_validation && ardataset != NULL) {
+                                       equalok = DNS_DBADD_EQUALOK;
+                               }
                                addedrdataset = ardataset;
                                result = dns_db_addrdataset(
                                        fctx->cache, node, NULL, now, rdataset,
-                                       options, addedrdataset);
+                                       options | equalok, addedrdataset);
                                if (result == DNS_R_UNCHANGED) {
                                        result = ISC_R_SUCCESS;
                                        if (!need_validation &&
@@ -6141,25 +6155,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                                                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.
-                                                */
+                                       }
+                                       if (equalok) {
                                                continue;
                                        }
+                                       result = ISC_R_SUCCESS;
                                }
                                if (result != ISC_R_SUCCESS) {
                                        break;