Check for secure data before caching CD=1 NXDOMAIN

An unvalidated NXDOMAIN (e.g. from a CD=1 query) marked every RRset at
the name ancient without checking trust, evicting DNSSEC-validated data.
Keep the cache unchanged when any existing RRset is already secure.

dns_ncache_add() now returns DNS_R_UNCHANGED for the rejected add;
negcache() serves a matching cached negative or the queried type, else
SERVFAIL (never the unrelated RRset the add bound), and rctx_ncache()
forwards it so the fetch fails fast.

diff --git a/lib/dns/ncache.c b/lib/dns/ncache.c
index a1670e17e0c58ae7a5c48ab9702e6db91af61032..942df43a7f26f1c2f803a43f848ee4d839dbffd4 100644 (file)
--- a/lib/dns/ncache.c
+++ b/lib/dns/ncache.c
@@ -105,7 +105,6 @@ dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
        dns_rdatalist_t ncrdatalist;
        unsigned char data[65536];
        unsigned int next = 0;
-       isc_result_t result;
 
        /*
         * Convert the authority data from 'message' into a negative cache
@@ -236,13 +235,8 @@ dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
                ncrdataset.attributes.optout = true;
        }
 
-       result = dns_db_addrdataset(cache, node, NULL, now, &ncrdataset, 0,
-                                   addedrdataset);
-       if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) {
-               return result;
-       }
-
-       return ISC_R_SUCCESS;
+       return dns_db_addrdataset(cache, node, NULL, now, &ncrdataset, 0,
+                                 addedrdataset);
 }
 
 isc_result_t

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 210210747f68cc998363bf7c57588a334c1c6b23..2ced865b8338a8ce8a75e494e9893434e92c3d99 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -2298,6 +2298,26 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                trust = newheader->trust;
        }
 
+       /*
+        * An unvalidated negative entry covering all types (NXDOMAIN or
+        * NODATA(QTYPE=ANY)) must not purge secure data. Check for it in a
+        * separate pass first: evicting as we go and bailing out later would
+        * destroy lower-trust siblings before we found the secure header.
+        */
+       if (EXISTS(newheader) && NEGATIVE(newheader) &&
+           rdtype == dns_rdatatype_any && trust < dns_trust_secure)
+       {
+               DNS_SLABHEADER_FOREACH(header, &qpnode->headers) {
+                       if (header->trust >= dns_trust_secure) {
+                               qpcache_hit(qpdb, header);
+                               bindrdataset(qpdb, qpnode, header, now,
+                                            nlocktype, tlocktype,
+                                            addedrdataset DNS__DB_FLARG_PASS);
+                               return DNS_R_UNCHANGED;
+                       }
+               }
+       }
+
        DNS_SLABHEADER_FOREACH(header, &qpnode->headers) {
                if (EXISTS(newheader) && NEGATIVE(newheader)) {
                        if (rdtype == dns_rdatatype_any) {

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index d8ca636d67eca9d5282b41919caae10d06edf034..a094eeb430bfd10b799802c867705e8a1d36037a 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -1082,7 +1082,7 @@ rctx_dispfail(respctx_t *rctx);
 static isc_result_t
 rctx_timedout(respctx_t *rctx);
 
-static void
+static isc_result_t
 rctx_ncache(respctx_t *rctx);
 
 /*%
@@ -6528,10 +6528,34 @@ 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);
 
+       /*
+        * We failed to add the negative cache entry and some rdataset was
+        * returned.  If we were adding dns_rdatatype_any, this could be of a
+        * different type than was originally requested.
+        */
+       if (result == DNS_R_UNCHANGED) {
+               /*
+                * We got the same negative type that we were adding, everything
+                * is fine, continue.
+                */
+               if (NEGATIVE(added) && added->covers == covers) {
+                       result = ISC_R_SUCCESS;
+               } else {
+                       dns_rdataset_disassociate(added);
+                       dns_db_detachnode(&node);
+                       return DNS_R_SERVFAIL;
+               }
+       }
+
        if (added == &rdataset) {
                dns_rdataset_cleanup(added);
        }
 
+       if (result != ISC_R_SUCCESS) {
+               dns_db_detachnode(&node);
+               return result;
+       }
+
        *nodep = node;
        return result;
 }
@@ -6541,7 +6565,7 @@ negcache(dns_message_t *message, fetchctx_t *fctx, const dns_name_t *name,
  * Cache the negatively cacheable parts of the message.  This may
  * also cause work to be queued to the DNSSEC validator.
  */
-static void
+static isc_result_t
 rctx_ncache(respctx_t *rctx) {
        isc_result_t result = ISC_R_SUCCESS;
        fetchctx_t *fctx = rctx->fctx;
@@ -6627,6 +6651,7 @@ done:
        if (result != ISC_R_SUCCESS) {
                FCTXTRACE3("rctx_ncache complete", result);
        }
+       return result;
 }
 
 static void
@@ -8101,7 +8126,10 @@ resquery_response_continue(void *arg, isc_result_t result) {
        /*
         * Negative caching
         */
-       rctx_ncache(rctx);
+       isc_result_t nresult = rctx_ncache(rctx);
+       if (nresult != ISC_R_SUCCESS) {
+               result = nresult;
+       }
 
        FCTXTRACE("resquery_response done");
        rctx_done(rctx, result);