From 30e837f31aec59570bb174767166b1046b36d520 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= <michal@isc.org>
Date: Fri, 15 Jun 2018 09:59:20 +0200
Subject: [PATCH] Do not call exit() upon verifyset() errors

Replace all check_result() and fprintf() calls inside verifyset() with
zoneverify_log_error() calls and error handling code.  Enable
verifyset() to signal errors to the caller using its return value.

Modify the call site of verifyset() so that its errors are properly
handled.

Define buffer sizes using named constants rather than plain integers.
---
 lib/dns/zoneverify.c | 51 ++++++++++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c
index e29646cdbd..07ccc05915 100644
--- a/lib/dns/zoneverify.c
+++ b/lib/dns/zoneverify.c
@@ -723,14 +723,14 @@ verifynsec3s(const vctx_t *vctx, dns_name_t *name,
 	return (result);
 }
 
-static void
+static isc_result_t
 verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, dns_name_t *name,
 	  dns_dbnode_t *node, dns_rdataset_t *keyrdataset)
 {
 	unsigned char set_algorithms[256];
 	char namebuf[DNS_NAME_FORMATSIZE];
-	char algbuf[80];
-	char typebuf[80];
+	char algbuf[DNS_SECALG_FORMATSIZE];
+	char typebuf[DNS_RDATATYPE_FORMATSIZE];
 	dns_rdataset_t sigrdataset;
 	dns_rdatasetiter_t *rdsiter = NULL;
 	isc_result_t result;
@@ -738,7 +738,11 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, dns_name_t *name,
 
 	dns_rdataset_init(&sigrdataset);
 	result = dns_db_allrdatasets(vctx->db, node, vctx->ver, 0, &rdsiter);
-	check_result(result, "dns_db_allrdatasets()");
+	if (result != ISC_R_SUCCESS) {
+		zoneverify_log_error(vctx, "dns_db_allrdatasets(): %s",
+				     isc_result_totext(result));
+		return (result);
+	}
 	for (result = dns_rdatasetiter_first(rdsiter);
 	     result == ISC_R_SUCCESS;
 	     result = dns_rdatasetiter_next(rdsiter)) {
@@ -751,12 +755,13 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, dns_name_t *name,
 	if (result != ISC_R_SUCCESS) {
 		dns_name_format(name, namebuf, sizeof(namebuf));
 		dns_rdatatype_format(rdataset->type, typebuf, sizeof(typebuf));
-		fprintf(stderr, "No signatures for %s/%s\n", namebuf, typebuf);
+		zoneverify_log_error(vctx, "No signatures for %s/%s",
+				     namebuf, typebuf);
 		for (i = 0; i < 256; i++)
 			if (vctx->act_algorithms[i] != 0)
 				vctx->bad_algorithms[i] = 1;
-		dns_rdatasetiter_destroy(&rdsiter);
-		return;
+		result = ISC_R_SUCCESS;
+		goto done;
 	}
 
 	memset(set_algorithms, 0, sizeof(set_algorithms));
@@ -773,8 +778,10 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, dns_name_t *name,
 			dns_name_format(name, namebuf, sizeof(namebuf));
 			dns_rdatatype_format(rdataset->type, typebuf,
 					     sizeof(typebuf));
-			fprintf(stderr, "TTL mismatch for %s %s keytag %u\n",
-				namebuf, typebuf, sig.keyid);
+			zoneverify_log_error(vctx,
+					     "TTL mismatch for "
+					     "%s %s keytag %u",
+					     namebuf, typebuf, sig.keyid);
 			continue;
 		}
 		if ((set_algorithms[sig.algorithm] != 0) ||
@@ -783,7 +790,8 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, dns_name_t *name,
 		if (goodsig(vctx, &rdata, name, keyrdataset, rdataset))
 			set_algorithms[sig.algorithm] = 1;
 	}
-	dns_rdatasetiter_destroy(&rdsiter);
+	result = ISC_R_SUCCESS;
+
 	if (memcmp(set_algorithms, vctx->act_algorithms,
 		   sizeof(set_algorithms))) {
 		dns_name_format(name, namebuf, sizeof(namebuf));
@@ -792,12 +800,21 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, dns_name_t *name,
 			if ((vctx->act_algorithms[i] != 0) &&
 			    (set_algorithms[i] == 0)) {
 				dns_secalg_format(i, algbuf, sizeof(algbuf));
-				fprintf(stderr, "No correct %s signature for "
-					"%s %s\n", algbuf, namebuf, typebuf);
+				zoneverify_log_error(vctx,
+						     "No correct %s signature "
+						     "for %s %s",
+						     algbuf, namebuf, typebuf);
 				vctx->bad_algorithms[i] = 1;
 			}
 	}
-	dns_rdataset_disassociate(&sigrdataset);
+
+ done:
+	if (dns_rdataset_isassociated(&sigrdataset)) {
+		dns_rdataset_disassociate(&sigrdataset);
+	}
+	dns_rdatasetiter_destroy(&rdsiter);
+
+	return (result);
 }
 
 static isc_result_t
@@ -835,7 +852,13 @@ verifynode(vctx_t *vctx, dns_name_t *name, dns_dbnode_t *node,
 		    rdataset.type != dns_rdatatype_dnskey &&
 		    (!delegation || rdataset.type == dns_rdatatype_ds ||
 		     rdataset.type == dns_rdatatype_nsec)) {
-			verifyset(vctx, &rdataset, name, node, keyrdataset);
+			result = verifyset(vctx, &rdataset, name, node,
+					   keyrdataset);
+			if (result != ISC_R_SUCCESS) {
+				dns_rdataset_disassociate(&rdataset);
+				dns_rdatasetiter_destroy(&rdsiter);
+				return (result);
+			}
 			dns_nsec_setbit(types, rdataset.type, 1);
 			if (rdataset.type > maxtype)
 				maxtype = rdataset.type;