Fix missing RRSIGs for "glue" lookups with CD=1

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.

diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h
index 513fb1eafb87158e234604bf7f6bea5ccd699f3b..cc80d5fa7b18c50aa2f9240c084bcf59468650f7 100644 (file)
--- a/lib/dns/include/dns/db.h
+++ b/lib/dns/include/dns/db.h
@@ -315,6 +315,7 @@ enum {
 #define DNS_DBADD_EXACT           0x04
 #define DNS_DBADD_EXACTTTL 0x08
 #define DNS_DBADD_PREFETCH 0x10
+#define DNS_DBADD_EQUALOK  0x20
 /*@}*/
 
 /*%
@@ -1249,6 +1250,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.
@@ -1279,7 +1284,10 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
  *
  * \li #ISC_R_SUCCESS
  * \li #DNS_R_UNCHANGED        The operation did not change anything.
- * \li #DNS_R_NOTEXACT
+ * \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.

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 58699ea7f44acdc20f27c6f4901aa864e9b00138..53be3b4d28329ef3414fdca4c4c04467bf471df6 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -2698,6 +2698,18 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                        bindrdataset(qpdb, qpnode, header, now, nlocktype,
                                     tlocktype,
                                     addedrdataset DNS__DB_FLARG_PASS);
+                       if (ACTIVE(header, now) &&
+                           (options & DNS_DBADD_EQUALOK) != 0 &&
+                           dns_rdataslab_equalx(
+                                   header, newheader, qpdb->common.rdclass,
+                                   DNS_TYPEPAIR_TYPE(header->typepair)))
+                       {
+                               /*
+                                * Updated by caller to ISC_R_SUCCESS after
+                                * cleaning up newheader.
+                                */
+                               return ISC_R_EXISTS;
+                       }
                        return DNS_R_UNCHANGED;
                }
 
@@ -2735,6 +2747,13 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                        bindrdataset(qpdb, qpnode, header, now, nlocktype,
                                     tlocktype,
                                     addedrdataset DNS__DB_FLARG_PASS);
+                       if ((options & DNS_DBADD_EQUALOK) != 0) {
+                               /*
+                                * Updated by caller to ISC_R_SUCCESS after
+                                * cleaning up newheader.
+                                */
+                               return ISC_R_EXISTS;
+                       }
                        return DNS_R_UNCHANGED;
                }
 
@@ -2785,6 +2804,13 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                        bindrdataset(qpdb, qpnode, header, now, nlocktype,
                                     tlocktype,
                                     addedrdataset DNS__DB_FLARG_PASS);
+                       if ((options & DNS_DBADD_EQUALOK) != 0) {
+                               /*
+                                * Updated by caller to ISC_R_SUCCESS after
+                                * cleaning up newheader.
+                                */
+                               return ISC_R_EXISTS;
+                       }
                        return DNS_R_UNCHANGED;
                }
 
@@ -3090,6 +3116,10 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 
        NODE_UNLOCK(nlock, &nlocktype);
 
+       if (result == ISC_R_EXISTS) {
+               result = ISC_R_SUCCESS;
+       }
+
        if (tlocktype != isc_rwlocktype_none) {
                TREE_UNLOCK(&qpdb->tree_lock, &tlocktype);
        }

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index bb57ea144e91e2ebfb6ddbda8378e055fb80116b..4005d2ce71de6fbd9981e89adf998644165673b5 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -5224,7 +5224,7 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name,
            dns_dbnode_t **nodep, dns_rdataset_t *added,
            dns_rdataset_t *addedsig, bool need_validation) {
        isc_result_t result = ISC_R_SUCCESS;
-       unsigned int options = 0;
+       unsigned int options = 0, equalok = 0;
        dns_dbnode_t *node = NULL;
 
        if (rdataset == NULL) {
@@ -5246,6 +5246,17 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name,
                options = DNS_DBADD_PREFETCH;
        }
 
+       /*
+        * 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 && added != NULL) {
+               equalok = DNS_DBADD_EQUALOK;
+       }
+
        /*
         * If the node pointer points to a node, attach to it.
         *
@@ -5263,31 +5274,20 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name,
 
        if (result == ISC_R_SUCCESS) {
                result = dns_db_addrdataset(fctx->cache, node, NULL, now,
-                                           rdataset, options, added);
+                                           rdataset, options | equalok, added);
        }
 
-       if (result == DNS_R_UNCHANGED) {
-               /*
-                * 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.
-                */
-               if (!need_validation && added != NULL &&
-                   !dns_rdataset_equals(rdataset, added))
-               {
-                       /* Skip adding the sigrdataset */
-               } else {
-                       /* Carry on caching the sigrdataset */
-                       result = ISC_R_SUCCESS;
-               }
+       if (equalok == 0 && result == DNS_R_UNCHANGED) {
+               result = ISC_R_SUCCESS;
        }
 
        if (result == ISC_R_SUCCESS && sigrdataset != NULL) {
                result = dns_db_addrdataset(fctx->cache, node, NULL, now,
                                            sigrdataset, options, addedsig);
                if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) {
-                       dns__rdataset_disassociate(added);
+                       if (added != NULL) {
+                               dns__rdataset_disassociate(added);
+                       }
                }
        }