Don't synthesize negative responses with pending NSEC

If a cached record has not yet been validated and has trust
level pending, don't use it to synthesize negative responses.

Fixes: isc-projects/bind9#5872
Fixes: isc-projects/bind9#5887
Fixes: isc-projects/bind9#5977

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 818b5632b4b7d1ac5ade856c0bda15f4e8f494da..840b97d9fda11808fc5fa929ced01451f8149129 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -878,6 +878,14 @@ update_cachestats(qpcache_t *qpdb, isc_result_t result) {
        }
 }
 
+static dns_trust_t
+header_trust(dns_slabheader_t *header) {
+       if (header == NULL) {
+               return dns_trust_none;
+       }
+       return atomic_load_acquire(&header->trust);
+}
+
 static void
 bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header,
             isc_stdtime_t now, isc_rwlocktype_t nlocktype,
@@ -932,7 +940,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header,
                rdataset->covers = DNS_TYPEPAIR_COVERS(header->typepair);
        }
        rdataset->ttl = !ZEROTTL(header) ? header->expire - now : 0;
-       rdataset->trust = atomic_load(&header->trust);
+       rdataset->trust = header_trust(header);
        rdataset->resign = 0;
 
        if (NEGATIVE(header)) {
@@ -1260,7 +1268,7 @@ check_dname(qpcnode_t *node, void *arg DNS__DB_FLARG) {
         */
        find_headers(node, search, dns_rdatatype_dname, &found, &foundsig);
 
-       if (found != NULL && (!DNS_TRUST_PENDING(atomic_load(&found->trust)) ||
+       if (found != NULL && (!DNS_TRUST_PENDING(header_trust(found)) ||
                              (search->options & DNS_DBFIND_PENDINGOK) != 0))
        {
                /*
@@ -1344,7 +1352,9 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name,
 
        find_headers(node, search, dns_rdatatype_nsec, &found, &foundsig);
 
-       if (found != NULL) {
+       if (found != NULL && header_trust(found) == dns_trust_secure &&
+           (foundsig == NULL || header_trust(foundsig) == dns_trust_secure))
+       {
                if (nodep != NULL) {
                        qpcnode_acquire(search->qpdb, node, nlocktype,
                                        isc_rwlocktype_none DNS__DB_FLARG_PASS);
@@ -1369,7 +1379,7 @@ missing_answer(dns_slabheader_t *found, unsigned int options) {
                return true;
        }
 
-       dns_trust_t trust = atomic_load(&found->trust);
+       dns_trust_t trust = header_trust(found);
        return (DNS_TRUST_ADDITIONAL(trust) &&
                (options & DNS_DBFIND_ADDITIONALOK) == 0) ||
               (DNS_TRUST_GLUE(trust) && (options & DNS_DBFIND_GLUEOK) == 0) ||
@@ -1534,7 +1544,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
                empty_node = false;
 
                if (header != NULL && header->noqname != NULL &&
-                   atomic_load(&header->trust) == dns_trust_secure)
+                   header_trust(header) == dns_trust_secure)
                {
                        found_noqname = true;
                }
@@ -2252,7 +2262,7 @@ check_ncache_block(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *header,
                 * than the new data, evict it from the cache. Otherwise,
                 * bind to it and leave the cache unchanged.
                 */
-               if (trust >= header->trust) {
+               if (trust >= header_trust(header)) {
                        header_delete(qpnode, header);
                        return DNS_R_CONTINUE;
                } else {
@@ -2312,7 +2322,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
            rdtype == dns_rdatatype_any && trust < dns_trust_secure)
        {
                DNS_SLABHEADER_FOREACH(header, &qpnode->headers) {
-                       if (header->trust >= dns_trust_secure) {
+                       if (header_trust(header) >= dns_trust_secure) {
                                qpcache_hit(qpdb, header);
                                bindrdataset(qpdb, qpnode, header, now,
                                             nlocktype, tlocktype,
@@ -2426,7 +2436,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                 * data will supersede it below. Unclear what the best
                 * policy is here.
                 */
-               dns_trust_t oldtrust = atomic_load(&oldheader->trust);
+               dns_trust_t oldtrust = header_trust(oldheader);
                if (trust < oldtrust &&
                    (ACTIVE(oldheader, now) || !EXISTS(oldheader)))
                {

diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index 3e10884af011fccfe7e1ed29048700742a3e8d93..02353d878da66e176a7ddb72bf10db6ba0182b57 100644 (file)
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -1175,7 +1175,7 @@ bindrdataset(qpzonedb_t *qpdb, dns_vecheader_t *header,
        rdataset->type = DNS_TYPEPAIR_TYPE(header->typepair);
        rdataset->covers = DNS_TYPEPAIR_COVERS(header->typepair);
        rdataset->ttl = header->ttl;
-       rdataset->trust = atomic_load(&header->trust);
+       rdataset->trust = atomic_load_acquire(&header->trust);
 
        if (OPTOUT(header)) {
                rdataset->attributes.optout = true;
@@ -2261,7 +2261,7 @@ loading_addrdataset(void *arg, const dns_name_t *name, dns_rdataset_t *rdataset,
        dns_vecheader_t *newheader = (dns_vecheader_t *)region.base;
        newheader->ttl = rdataset->ttl;
        newheader->serial = 1;
-       atomic_store(&newheader->trust, rdataset->trust);
+       atomic_store_release(&newheader->trust, rdataset->trust);
 
        dns_vecheader_setownercase(newheader, name);
 

diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c
index f3f970a00689cad4b06d5824cb07ed579ba4b877..4c10ef8579b0bbdb46be0307814bf540aa6d9b7e 100644 (file)
--- a/lib/dns/rdataslab.c
+++ b/lib/dns/rdataslab.c
@@ -839,7 +839,7 @@ rdataset_settrust(dns_rdataset_t *rdataset, dns_trust_t trust) {
        dns_slabheader_t *header = rdataset_getheader(rdataset);
 
        rdataset->trust = trust;
-       atomic_store(&header->trust, trust);
+       atomic_store_release(&header->trust, trust);
 }
 
 static void

diff --git a/lib/dns/rdatavec.c b/lib/dns/rdatavec.c
index 415cd3aaba3c468f8d748bb43473de6ca301ca7d..7436af85738868370206ae8fab251c193c1a246e 100644 (file)
--- a/lib/dns/rdatavec.c
+++ b/lib/dns/rdatavec.c
@@ -981,7 +981,7 @@ rdataset_settrust(dns_rdataset_t *rdataset, dns_trust_t trust) {
        dns_vecheader_t *header = dns_vecheader_getheader(rdataset);
 
        rdataset->trust = trust;
-       atomic_store(&header->trust, trust);
+       atomic_store_release(&header->trust, trust);
 }
 
 static void

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 913b14282a2fcf6b50b5613e77582d5018e1d4fa..70e4c2eb2bd2d823cde7b1b3393f9d7566a21a05 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -9421,13 +9421,14 @@ query_coveringnsec(query_ctx_t *qctx) {
        dns_fixedname_t fsigner;
        dns_fixedname_t fwild;
        dns_name_t *fname = NULL;
-       dns_name_t *namespace = NULL;
+       dns_name_t *namespace = dns_fixedname_initname(&fnamespace);
        dns_name_t *nowild = NULL;
        dns_name_t *signer = NULL;
        dns_name_t *wild = NULL;
-       dns_name_t qname;
+       dns_name_t qname = DNS_NAME_INITEMPTY;
        dns_rdataset_t *soardataset = NULL, *sigsoardataset = NULL;
-       dns_rdataset_t rdataset, sigrdataset;
+       dns_rdataset_t rdataset = DNS_RDATASET_INIT;
+       dns_rdataset_t sigrdataset = DNS_RDATASET_INIT;
        bool done = false;
        bool exists = true, data = true;
        bool redirected = false;
@@ -9437,11 +9438,6 @@ query_coveringnsec(query_ctx_t *qctx) {
 
        CCTRACE(ISC_LOG_DEBUG(3), "query_coveringnsec");
 
-       dns_name_init(&qname);
-       dns_rdataset_init(&rdataset);
-       dns_rdataset_init(&sigrdataset);
-       namespace = dns_fixedname_initname(&fnamespace);
-
        /*
         * Check that the NSEC record is from the correct namespace.
         * For records that belong to the parent zone (i.e. DS),
@@ -9499,15 +9495,20 @@ query_coveringnsec(query_ctx_t *qctx) {
        /*
         * Check that we have the correct NOQNAME NSEC record.
         */
-       result = dns_nsec_noexistnodata(qctx->qtype, qctx->client->query.qname,
-                                       qctx->fname, qctx->rdataset, &exists,
-                                       &data, wild, log_noexistnodata, qctx);
-
-       if (result != ISC_R_SUCCESS || (exists && data)) {
-               goto cleanup;
-       }
-
+       CHECK(dns_nsec_noexistnodata(qctx->qtype, qctx->client->query.qname,
+                                    qctx->fname, qctx->rdataset, &exists,
+                                    &data, wild, log_noexistnodata, qctx));
        if (exists) {
+               /*
+                * If there's data at the name, or the NSEC isn't
+                * validated, we don't synthesize an answer.
+                */
+               if (data || qctx->rdataset->trust != dns_trust_secure ||
+                   qctx->sigrdataset->trust != dns_trust_secure)
+               {
+                       goto cleanup;
+               }
+
                if (qctx->type == dns_rdatatype_any) { /* XXX not yet */
                        goto cleanup;
                }
@@ -9534,6 +9535,12 @@ query_coveringnsec(query_ctx_t *qctx) {
                                     qctx->client->inner.now, &node, fname, &cm,
                                     &ci, soardataset, sigsoardataset));
 
+               if (soardataset->trust != dns_trust_secure ||
+                   sigsoardataset->trust != dns_trust_secure)
+               {
+                       goto cleanup;
+               }
+
                (void)query_synthnodata(qctx, signer, &soardataset,
                                        &sigsoardataset);
                done = true;
@@ -9592,10 +9599,15 @@ query_coveringnsec(query_ctx_t *qctx) {
                if (!dns_name_issubdomain(nowild, namespace)) {
                        goto cleanup;
                }
-               result = dns_nsec_noexistnodata(qctx->qtype, wild, nowild,
-                                               &rdataset, &exists, &data, NULL,
-                                               log_noexistnodata, qctx);
-               if (result != ISC_R_SUCCESS || (exists && data)) {
+               CHECK(dns_nsec_noexistnodata(qctx->qtype, wild, nowild,
+                                            &rdataset, &exists, &data, NULL,
+                                            log_noexistnodata, qctx));
+               /*
+                * If the name exists and contains data, we don't synthesize an
+                * answer. Note that the rdataset trust has been verified to be
+                * secure already.
+                */
+               if (exists && data) {
                        goto cleanup;
                }
                break;
@@ -9653,6 +9665,12 @@ query_coveringnsec(query_ctx_t *qctx) {
                             dboptions, qctx->client->inner.now, &node, fname,
                             &cm, &ci, soardataset, sigsoardataset));
 
+       if (soardataset->trust != dns_trust_secure ||
+           sigsoardataset->trust != dns_trust_secure)
+       {
+               goto cleanup;
+       }
+
        (void)query_synthnxdomainnodata(qctx, exists, nowild, &rdataset,
                                        &sigrdataset, signer, &soardataset,
                                        &sigsoardataset);