From 4354f44d9c7608cea9e585500ba04cdb263e20d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 15 Jun 2018 09:59:20 +0200 Subject: [PATCH] Do not call exit() upon verify_nodes() errors Replace all fatal(), check_result(), and check_dns_dbiterator_current() calls inside verify_nodes() with zoneverify_log_error() calls and error handling code. Enable verify_nodes() to signal errors to the caller using its return value. Modify the call site of verify_nodes() so that its errors are properly handled. Free all heap elements upon verification context cleanup as a verification error may prevent them from being freed elsewhere. Remove the check_dns_dbiterator_current() macro as it is no longer used anywhere in lib/dns/zoneverify.c. --- lib/dns/zoneverify.c | 98 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 19 deletions(-) diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c index 2f724f5931..7ba9fedeaa 100644 --- a/lib/dns/zoneverify.c +++ b/lib/dns/zoneverify.c @@ -50,10 +50,6 @@ #include #include -#define check_dns_dbiterator_current(result) \ - check_result((result == DNS_R_NEWORIGIN) ? ISC_R_SUCCESS : result, \ - "dns_dbiterator_current()") - typedef struct vctx { isc_mem_t * mctx; dns_zone_t * zone; @@ -920,6 +916,14 @@ free_element(isc_mem_t *mctx, struct nsec3_chain_fixed *e) { isc_mem_put(mctx, e, len); } +static void +free_element_heap(void *element, void *uap) { + struct nsec3_chain_fixed *e = (struct nsec3_chain_fixed *)element; + isc_mem_t *mctx = (isc_mem_t *)uap; + + free_element(mctx, e); +} + static isc_boolean_t checknext(const struct nsec3_chain_fixed *first, const struct nsec3_chain_fixed *e) @@ -1151,7 +1155,9 @@ vctx_destroy(vctx_t *vctx) { if (dns_rdataset_isassociated(&vctx->nsec3paramsigs)) { dns_rdataset_disassociate(&vctx->nsec3paramsigs); } + isc_heap_foreach(vctx->expected_chains, free_element_heap, vctx->mctx); isc_heap_destroy(&vctx->expected_chains); + isc_heap_foreach(vctx->found_chains, free_element_heap, vctx->mctx); isc_heap_destroy(&vctx->found_chains); } @@ -1413,7 +1419,7 @@ determine_active_algorithms(vctx_t *vctx, isc_boolean_t ignore_kskflag, * Check that all the records not yet verified were signed by keys that are * present in the DNSKEY RRset. */ -static void +static isc_result_t verify_nodes(vctx_t *vctx, isc_result_t *vresult) { dns_fixedname_t fname, fnextname, fprevname, fzonecut; dns_name_t *name, *nextname, *prevname, *zonecut; @@ -1430,24 +1436,42 @@ verify_nodes(vctx_t *vctx, isc_result_t *vresult) { zonecut = NULL; result = dns_db_createiterator(vctx->db, DNS_DB_NONSEC3, &dbiter); - check_result(result, "dns_db_createiterator()"); + if (result != ISC_R_SUCCESS) { + zoneverify_log_error(vctx, "dns_db_createiterator(): %s", + isc_result_totext(result)); + return (result); + } result = dns_dbiterator_first(dbiter); - check_result(result, "dns_dbiterator_first()"); + if (result != ISC_R_SUCCESS) { + zoneverify_log_error(vctx, "dns_dbiterator_first(): %s", + isc_result_totext(result)); + goto done; + } while (!done) { isc_boolean_t isdelegation = ISC_FALSE; result = dns_dbiterator_current(dbiter, &node, name); - check_dns_dbiterator_current(result); + if (result != ISC_R_SUCCESS && result != DNS_R_NEWORIGIN) { + zoneverify_log_error(vctx, + "dns_dbiterator_current(): %s", + isc_result_totext(result)); + goto done; + } if (!dns_name_issubdomain(name, vctx->origin)) { check_no_nsec(vctx, name, node); dns_db_detachnode(vctx->db, &node); result = dns_dbiterator_next(dbiter); - if (result == ISC_R_NOMORE) + if (result == ISC_R_NOMORE) { done = ISC_TRUE; - else - check_result(result, "dns_dbiterator_next()"); + } else if (result != ISC_R_SUCCESS) { + zoneverify_log_error( + vctx, + "dns_dbiterator_next(): %s", + isc_result_totext(result)); + goto done; + } continue; } if (is_delegation(vctx, name, node, NULL)) { @@ -1463,7 +1487,16 @@ verify_nodes(vctx_t *vctx, isc_result_t *vresult) { while (result == ISC_R_SUCCESS) { result = dns_dbiterator_current(dbiter, &nextnode, nextname); - check_dns_dbiterator_current(result); + if (result != ISC_R_SUCCESS && + result != DNS_R_NEWORIGIN) + { + zoneverify_log_error( + vctx, + "dns_dbiterator_current(): %s", + isc_result_totext(result)); + dns_db_detachnode(vctx->db, &node); + goto done; + } if (!dns_name_issubdomain(nextname, vctx->origin) || (zonecut != NULL && dns_name_issubdomain(nextname, zonecut))) @@ -1484,9 +1517,14 @@ verify_nodes(vctx_t *vctx, isc_result_t *vresult) { if (result == ISC_R_NOMORE) { done = ISC_TRUE; nextname = vctx->origin; - } else if (result != ISC_R_SUCCESS) - fatal("iterating through the database failed: %s", - isc_result_totext(result)); + } else if (result != ISC_R_SUCCESS) { + zoneverify_log_error(vctx, + "iterating through the database " + "failed: %s", + isc_result_totext(result)); + dns_db_detachnode(vctx->db, &node); + goto done; + } result = verifynode(vctx, name, node, isdelegation, &vctx->keyset, &vctx->nsecset, &vctx->nsec3paramset, nextname); @@ -1509,21 +1547,40 @@ verify_nodes(vctx_t *vctx, isc_result_t *vresult) { dns_dbiterator_destroy(&dbiter); result = dns_db_createiterator(vctx->db, DNS_DB_NSEC3ONLY, &dbiter); - check_result(result, "dns_db_createiterator()"); + if (result != ISC_R_SUCCESS) { + zoneverify_log_error(vctx, "dns_db_createiterator(): %s", + isc_result_totext(result)); + return (result); + } for (result = dns_dbiterator_first(dbiter); result == ISC_R_SUCCESS; result = dns_dbiterator_next(dbiter) ) { result = dns_dbiterator_current(dbiter, &node, name); - check_dns_dbiterator_current(result); + if (result != ISC_R_SUCCESS && result != DNS_R_NEWORIGIN) { + zoneverify_log_error(vctx, + "dns_dbiterator_current(): %s", + isc_result_totext(result)); + goto done; + } result = verifynode(vctx, name, node, ISC_FALSE, &vctx->keyset, NULL, NULL, NULL); - check_result(result, "verifynode"); + if (result != ISC_R_SUCCESS) { + zoneverify_log_error(vctx, "verifynode: %s", + isc_result_totext(result)); + dns_db_detachnode(vctx->db, &node); + goto done; + } record_found(vctx, name, node, &vctx->nsec3paramset); dns_db_detachnode(vctx->db, &node); } + result = ISC_R_SUCCESS; + + done: dns_dbiterator_destroy(&dbiter); + + return (result); } static isc_result_t @@ -1616,7 +1673,10 @@ dns_zoneverify_dnssec(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, determine_active_algorithms(&vctx, ignore_kskflag, keyset_kskonly); - verify_nodes(&vctx, &vresult); + result = verify_nodes(&vctx, &vresult); + if (result != ISC_R_SUCCESS) { + goto done; + } result = verify_nsec3_chains(&vctx, mctx); if (vresult == ISC_R_UNSET)