dns_rdataset_addnoqname() could find unsigned NSEC/NSEC3

The dns_rdatalist addnoqname() implementation searches for the first
NSEC or NSEC3 record in a message, then for the first RRSIG covering
that type in the same message.  Previously, if no RRSIG for the type was
found, the function accepted the unsigned record. Now, it will instead
continue searching until an NSEC or NSEC3 that does have a matching
signature is found.

When this function is called from validated() in resolver.c, a
non-success return code is now treated as an error instead of triggering
an assertion failure.

Fixes: isc-projects/bind9#5985

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 72ad867e3c1a26d1ac1f61ac1255dfd2fe7bcb3d..fc5ec56300cfea26f7cbd0bfe2071d570dedb4cc 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -2638,8 +2638,7 @@ addnoqname(isc_mem_t *mctx, dns_slabheader_t *newheader, uint32_t maxrrperset,
        dns_rdataset_t neg = DNS_RDATASET_INIT, negsig = DNS_RDATASET_INIT;
        isc_region_t r1 = { .base = NULL }, r2 = { .base = NULL };
 
-       result = dns_rdataset_getnoqname(rdataset, &name, &neg, &negsig);
-       RUNTIME_CHECK(result == ISC_R_SUCCESS);
+       CHECK(dns_rdataset_getnoqname(rdataset, &name, &neg, &negsig));
 
        CHECK(dns_rdataslab_fromrdataset(&neg, mctx, &r1, maxrrperset));
 
@@ -2666,8 +2665,8 @@ cleanup:
                        dns_slabheader_detach(&header);
                }
        }
-       dns_rdataset_disassociate(&neg);
-       dns_rdataset_disassociate(&negsig);
+       dns_rdataset_cleanup(&neg);
+       dns_rdataset_cleanup(&negsig);
 
        return result;
 }

diff --git a/lib/dns/rdatalist.c b/lib/dns/rdatalist.c
index 6882d913dd960add695fe5fc224c9caa3532205d..487ebd570af5ae8a5d30f8c6ceff40f64b5f9d11 100644 (file)
--- a/lib/dns/rdatalist.c
+++ b/lib/dns/rdatalist.c
@@ -178,26 +178,24 @@ dns__rdatalist_addnoqname(dns_rdataset_t *rdataset, dns_name_t *name) {
        REQUIRE(rdataset != NULL);
 
        ISC_LIST_FOREACH(name->list, rdset, link) {
-               if (rdset->rdclass != rdataset->rdclass) {
+               if (rdset->rdclass != rdataset->rdclass ||
+                   !dns_rdatatype_isnsec(rdset->type))
+               {
                        continue;
                }
-               if (dns_rdatatype_isnsec(rdset->type)) {
-                       neg = rdset;
-               }
-       }
-       if (neg == NULL) {
-               return ISC_R_NOTFOUND;
-       }
 
-       ISC_LIST_FOREACH(name->list, rdset, link) {
-               if (rdset->type == dns_rdatatype_rrsig &&
-                   rdset->covers == neg->type)
-               {
-                       negsig = rdset;
+               ISC_LIST_FOREACH(name->list, sigset, link) {
+                       if (sigset->type == dns_rdatatype_rrsig &&
+                           sigset->covers == rdset->type)
+                       {
+                               neg = rdset;
+                               negsig = sigset;
+                               break;
+                       }
                }
        }
 
-       if (negsig == NULL) {
+       if (neg == NULL || negsig == NULL) {
                return ISC_R_NOTFOUND;
        }
 
@@ -214,6 +212,7 @@ dns__rdatalist_addnoqname(dns_rdataset_t *rdataset, dns_name_t *name) {
        rdataset->ttl = neg->ttl = negsig->ttl = ttl;
        rdataset->attributes.noqname = true;
        rdataset->rdlist.noqname = name;
+
        return ISC_R_SUCCESS;
 }
 

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index b3bc301017f23fdf4c2168b0e7c28a0d160be687..6aa03303e84a354045693ab945935ecf8f687d61 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -5975,9 +5975,9 @@ validated(void *arg) {
        inc_stats(res, dns_resstatscounter_valsuccess);
 
        if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) {
-               result = dns_rdataset_addnoqname(
-                       val->rdataset, val->proofs[DNS_VALIDATOR_NOQNAMEPROOF]);
-               RUNTIME_CHECK(result == ISC_R_SUCCESS);
+               CHECK(dns_rdataset_addnoqname(
+                       val->rdataset,
+                       val->proofs[DNS_VALIDATOR_NOQNAMEPROOF]));
                INSIST(val->sigrdataset != NULL);
                val->sigrdataset->ttl = val->rdataset->ttl;
                if (val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER] != NULL) {

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 519d164f1f8651d5238a91c62f88f8728476b8dc..74a0cc081e97c09ba4dac0d133289b61522bc1bf 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -7280,8 +7280,7 @@ query_addnoqnameproof(query_ctx_t *qctx) {
        neg = ns_client_newrdataset(client);
        negsig = ns_client_newrdataset(client);
 
-       result = dns_rdataset_getnoqname(qctx->noqname, fname, neg, negsig);
-       RUNTIME_CHECK(result == ISC_R_SUCCESS);
+       CHECK(dns_rdataset_getnoqname(qctx->noqname, fname, neg, negsig));
 
        query_addrrset(qctx, &fname, &neg, &negsig, dbuf,
                       DNS_SECTION_AUTHORITY);