Fix not caching RRSIG covering cache NODATA record

During refactoring, a condition that prevented caching RRSIGs for
records that we already have cached NODATA records was changed in an
invalid way.  This was caught later when a cached NODATA(type) +
RRSIG(type) was found in the cache and caused an assertion failure.

Fix and simplify condition that prevents adding such RRSIGs.

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index e7948db4d997b9f2bbed4813b1246d5a785ffd9e..7f2ab9640a68f55a6566395e31f94ffe57980424 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -2600,48 +2600,44 @@ qpcnode_detachnode(dns_dbnode_t **nodep DNS__DB_FLARG) {
 }
 
 static isc_result_t
-expire_ncache_entry(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabtop_t *top,
-                   dns_slabheader_t *newheader, dns_trust_t trust,
-                   dns_rdataset_t *addedrdataset, isc_stdtime_t now,
-                   isc_rwlocktype_t nlocktype,
-                   isc_rwlocktype_t tlocktype DNS__DB_FLARG) {
-       dns_rdatatype_t rdtype = DNS_TYPEPAIR_TYPE(newheader->typepair);
-       dns_rdatatype_t covers = DNS_TYPEPAIR_COVERS(newheader->typepair);
-       dns_typepair_t sigpair = !dns_rdatatype_issig(rdtype)
-                                        ? DNS_SIGTYPEPAIR(rdtype)
-                                        : dns_typepair_none;
+check_ncache_block(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *header,
+                  dns_slabheader_t *newheader, dns_trust_t trust,
+                  dns_rdataset_t *addedrdataset, isc_stdtime_t now,
+                  isc_rwlocktype_t nlocktype,
+                  isc_rwlocktype_t tlocktype DNS__DB_FLARG) {
+       bool block = false;
+
        /*
-        * 1. If we find a cached NXDOMAIN, don't cache anything else
-        *    (dns_typepair_any).
-        *
-        * 2. Don't cache an RRSIG if it covers a type for which we have a
-        *    cached NODATA record.
+        * 1. If we have a cached NXDOMAIN, we won't cache
+        *    anything else here (dns_typepair_any).
+        * 2. If we have a cached NODATA for a given type,
+        *    we won't cache an RRSIG covering the same type.
         */
-       if ((top->typepair == dns_typepair_any) ||
-           (sigpair != dns_rdatatype_none && newheader->typepair == sigpair &&
-            DNS_TYPEPAIR_TYPE(top->typepair) == covers))
+       if (header->typepair == dns_typepair_any) {
+               block = true;
+       } else if (DNS_TYPEPAIR_TYPE(newheader->typepair) ==
+                          dns_rdatatype_rrsig &&
+                  DNS_TYPEPAIR_COVERS(newheader->typepair) ==
+                          DNS_TYPEPAIR_TYPE(header->typepair))
        {
-               dns_slabheader_t *header = first_header(top);
-               if (header == NULL) {
-                       return DNS_R_CONTINUE;
-               }
+               block = true;
+       }
 
-               if (trust < header->trust) {
-                       /*
-                        * The NXDOMAIN/NODATA(QTYPE=ANY) is more trusted.
-                        */
+       if (block) {
+               /*
+                * If the ncache entry causing the block is less trusted
+                * than the new data, evict it from the cache. Otherwise,
+                * bind to it and leave the cache unchanged.
+                */
+               if (trust >= header->trust) {
+                       mark_ancient(header);
+               } else {
                        qpcache_hit(qpdb, header);
                        bindrdataset(qpdb, qpnode, header, now, nlocktype,
                                     tlocktype,
                                     addedrdataset DNS__DB_FLARG_PASS);
                        return DNS_R_UNCHANGED;
                }
-
-               /*
-                * The new rdataset is better.  Expire the ncache entry.
-                */
-               mark_ancient(header);
-               return DNS_R_CONTINUE;
        }
 
        return DNS_R_CONTINUE;
@@ -2707,22 +2703,20 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                                }
                        }
                }
-               if (EXISTS(newheader) && !NEGATIVE(newheader) &&
-                   NEGATIVE(header) && EXISTS(header) && ACTIVE(header, now))
+               if (EXISTS(header) && EXISTS(newheader) && NEGATIVE(header) &&
+                   !NEGATIVE(newheader) && ACTIVE(header, now))
                {
                        /*
-                        * Look for existing active NXDOMAIN or negative
-                        * covered type if we are adding RRSIG.
+                        * There's an existing NXDOMAIN or negative
+                        * covered type in the cache. If it's more
+                        * trusted than the new data, keep it, but
+                        * if not, purge and replace it.
                         */
-                       isc_result_t result = expire_ncache_entry(
-                               qpdb, qpnode, top, newheader, trust,
+                       isc_result_t result = check_ncache_block(
+                               qpdb, qpnode, header, newheader, trust,
                                addedrdataset, now, nlocktype, tlocktype);
                        if (result == DNS_R_UNCHANGED) {
-                               /*
-                                * The existing negative entry is more trusted
-                                * than the new rdataset.
-                                */
-                               return DNS_R_UNCHANGED;
+                               return result;
                        }
                        INSIST(result == DNS_R_CONTINUE);
                }