From fe8dcd4aab08d1009a4cd80c64d42c9bfe83b2eb Mon Sep 17 00:00:00 2001
From: Willem Toorop <willem@nlnetlabs.nl>
Date: Mon, 5 Sep 2022 12:24:00 +0200
Subject: [PATCH] Produce call_reporting for errors too

---
 src/const-info.c             |  2 ++
 src/dict.c                   |  6 ++++-
 src/general.c                |  2 ++
 src/getdns/getdns_extra.h.in | 10 ++++++++
 src/stub.c                   | 49 +++++++++++++++++++++++++-----------
 src/util-internal.c          | 45 +++++++++++++++++++--------------
 6 files changed, 79 insertions(+), 35 deletions(-)

diff --git a/src/const-info.c b/src/const-info.c
index 3c1bc073..14dfe8e6 100644
--- a/src/const-info.c
+++ b/src/const-info.c
@@ -109,6 +109,7 @@ static struct const_info consts_info[] = {
 	{     902, "GETDNS_RESPSTATUS_ALL_TIMEOUT", GETDNS_RESPSTATUS_ALL_TIMEOUT_TEXT },
 	{     903, "GETDNS_RESPSTATUS_NO_SECURE_ANSWERS", GETDNS_RESPSTATUS_NO_SECURE_ANSWERS_TEXT },
 	{     904, "GETDNS_RESPSTATUS_ALL_BOGUS_ANSWERS", GETDNS_RESPSTATUS_ALL_BOGUS_ANSWERS_TEXT },
+	{     950, "GETDNS_RESPSTATUS_ALL_ERRED", GETDNS_RESPSTATUS_ALL_ERRED_TEXT },
 	{    1000, "GETDNS_EXTENSION_TRUE", GETDNS_EXTENSION_TRUE_TEXT },
 	{    1001, "GETDNS_EXTENSION_FALSE", GETDNS_EXTENSION_FALSE_TEXT },
 	{    1100, "GETDNS_BAD_DNS_CNAME_IN_TARGET", GETDNS_BAD_DNS_CNAME_IN_TARGET_TEXT },
@@ -272,6 +273,7 @@ static struct const_name_info consts_name_info[] = {
 	{ "GETDNS_RESOLUTION_RECURSING", 521 },
 	{ "GETDNS_RESOLUTION_STUB", 520 },
 	{ "GETDNS_RESPSTATUS_ALL_BOGUS_ANSWERS", 904 },
+	{ "GETDNS_RESPSTATUS_ALL_ERRED", 950 },
 	{ "GETDNS_RESPSTATUS_ALL_TIMEOUT", 902 },
 	{ "GETDNS_RESPSTATUS_GOOD", 900 },
 	{ "GETDNS_RESPSTATUS_NO_NAME", 901 },
diff --git a/src/dict.c b/src/dict.c
index 9caee781..14430aa5 100644
--- a/src/dict.c
+++ b/src/dict.c
@@ -658,13 +658,17 @@ getdns_return_t
 getdns_dict_util_set_string(getdns_dict *dict,
     const char *name, const char *value)
 {
+	static const char *nill_value = "<nil>";
 	getdns_item    *item;
 	getdns_bindata *newbindata;
 	getdns_return_t r;
 
-	if (!dict || !name || !value)
+	if (!dict || !name)
 		return GETDNS_RETURN_INVALID_PARAMETER;
 
+	if (!value)
+		value = nill_value;
+
 	if (!(newbindata = _getdns_bindata_copy(
 	    &dict->mf, strlen(value) + 1, (uint8_t *)value)))
 		return GETDNS_RETURN_MEMORY_ERROR;
diff --git a/src/general.c b/src/general.c
index 66964585..a941d948 100644
--- a/src/general.c
+++ b/src/general.c
@@ -63,6 +63,8 @@ void _getdns_call_user_callback(getdns_dns_req *dnsreq, getdns_dict *response)
 #if defined(REQ_DEBUG) && REQ_DEBUG
 	debug_req(__FUNC__, *dnsreq->netreqs);
 #endif
+	if (!response && dnsreq->return_call_reporting)
+		response = _getdns_create_getdns_response(dnsreq);
 	if (dnsreq->user_callback) {
 		dnsreq->context->processing = 1;
 		dnsreq->user_callback(dnsreq->context,
diff --git a/src/getdns/getdns_extra.h.in b/src/getdns/getdns_extra.h.in
index 223b1de4..b6c58645 100644
--- a/src/getdns/getdns_extra.h.in
+++ b/src/getdns/getdns_extra.h.in
@@ -119,6 +119,16 @@ extern "C" {
 /** @}
   */
 
+/**
+ * \addtogroup respstatus Status Codes for responses and texts
+ *  @{
+ */
+
+#define GETDNS_RESPSTATUS_ALL_ERRED 950
+#define GETDNS_RESPSTATUS_ALL_ERRED_TEXT "All queries for the name erred"
+
+/** @}
+  */
 
 /**
  * \defgroup versions Version values
diff --git a/src/stub.c b/src/stub.c
index af801ba0..545f8994 100644
--- a/src/stub.c
+++ b/src/stub.c
@@ -1779,13 +1779,42 @@ upstream_read_cb(void *userarg)
 	}
 }
 
+static void
+netreq_equip_tls_debug_info(getdns_network_req *netreq)
+{
+	_getdns_tls_x509 *cert;
+
+	if (!netreq || !netreq->upstream)
+		return;
+	netreq->debug_tls_auth_status = netreq->upstream->tls_auth_state;
+	if (!netreq->upstream->tls_obj)
+		return;
+	if (netreq->debug_tls_peer_cert.data) {
+		GETDNS_FREE( netreq->owner->my_mf
+		           , netreq->debug_tls_peer_cert.data);
+		netreq->debug_tls_peer_cert.data = NULL;
+		netreq->debug_tls_peer_cert.size = 0;
+	}
+	if ((cert = _getdns_tls_connection_get_peer_certificate(
+			&netreq->owner->my_mf, netreq->upstream->tls_obj))) {
+		_getdns_tls_x509_to_der( &netreq->owner->my_mf, cert
+		                       , &netreq->debug_tls_peer_cert);
+		_getdns_tls_x509_free(&netreq->owner->my_mf, cert);
+	}
+	netreq->debug_tls_version = _getdns_tls_connection_get_version(
+						netreq->upstream->tls_obj);
+	netreq->debug_pkix_auth   = _getdns_tls_connection_get_pkix_auth(
+						netreq->upstream->tls_obj);
+	netreq->debug_pin_auth    = _getdns_tls_connection_get_pin_auth(
+						netreq->upstream->tls_obj);
+}
+
 static void
 upstream_write_cb(void *userarg)
 {
 	getdns_upstream *upstream = (getdns_upstream *)userarg;
 	getdns_network_req *netreq = upstream->write_queue;
 	int q;
-	_getdns_tls_x509 *cert;
 
 	if (!netreq) {
 		GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event);
@@ -1834,6 +1863,8 @@ upstream_write_cb(void *userarg)
 		    "%-40s : Conn closed: %s - *Failure*\n",
 		    upstream->addr_str,
 		    (upstream->transport == GETDNS_TRANSPORT_TLS ? "TLS" : "TCP"));
+		if (netreq->owner->return_call_reporting)
+			netreq_equip_tls_debug_info(netreq);
 		if (fallback_on_write(netreq) == STUB_TCP_ERROR) {
 			/* TODO: Need new state to report transport unavailable*/
 			_getdns_netreq_change_state(netreq, NET_REQ_ERRORED);
@@ -1844,20 +1875,8 @@ upstream_write_cb(void *userarg)
 	default:
 		/* Unqueue the netreq from the write_queue */
 		remove_from_write_queue(upstream, netreq);
-
-		if (netreq->owner->return_call_reporting &&
-		    netreq->upstream->tls_obj) {
-			if (netreq->debug_tls_peer_cert.data == NULL &&
-			    (cert = _getdns_tls_connection_get_peer_certificate(&upstream->upstreams->mf, netreq->upstream->tls_obj))) {
-				_getdns_tls_x509_to_der(&upstream->upstreams->mf, cert, &netreq->debug_tls_peer_cert);
-				_getdns_tls_x509_free(&upstream->upstreams->mf, cert);
-			}
-			netreq->debug_tls_version = _getdns_tls_connection_get_version(netreq->upstream->tls_obj);
-			netreq->debug_pkix_auth = _getdns_tls_connection_get_pkix_auth(netreq->upstream->tls_obj);
-			netreq->debug_pin_auth = _getdns_tls_connection_get_pin_auth(netreq->upstream->tls_obj);
-		}
-		/* Need this because auth status is reset on connection close */
-		netreq->debug_tls_auth_status = netreq->upstream->tls_auth_state;
+		if (netreq->owner->return_call_reporting)
+			netreq_equip_tls_debug_info(netreq);
 		upstream->queries_sent++;
 
 		/* Empty write_queue?, then deschedule upstream write_cb */
diff --git a/src/util-internal.c b/src/util-internal.c
index f009d1cc..8ebdc9e2 100644
--- a/src/util-internal.c
+++ b/src/util-internal.c
@@ -846,9 +846,11 @@ _getdns_create_call_reporting_dict(
 	                       , netreq->request_type ) ||
 
 	    /* Safe, because uint32_t facilitates RRT's of almost 50 days*/
-	    getdns_dict_set_int(netreq_debug, "run_time/ms",
-		    (uint32_t)(( netreq->debug_end_time
-	                       - netreq->debug_start_time)/1000))) {
+	    getdns_dict_set_int(netreq_debug, "run_time/ms", 
+		      (uint32_t) ( netreq->debug_end_time
+	                         - netreq->debug_start_time) >= 1000
+		    ? (uint32_t)(( netreq->debug_end_time                         
+                                 - netreq->debug_start_time) /  1000) : 0 )) {
 
 		getdns_dict_destroy(netreq_debug);
 		return NULL;
@@ -1177,7 +1179,7 @@ _getdns_create_getdns_response(getdns_dns_req *completed_request)
 	int rrsigs_in_answer = 0;
 	getdns_dict *reply;
 	getdns_bindata *canonical_name = NULL;
-	int nreplies = 0, nanswers = 0;
+	int nreplies = 0, nerred = 0, ntimedout = 0, nanswers = 0;
 	int nsecure = 0, ninsecure = 0, nindeterminate = 0, nbogus = 0;
 	getdns_dict   *netreq_debug;
 	_srvs srvs = { 0, 0, NULL };
@@ -1237,8 +1239,7 @@ _getdns_create_getdns_response(getdns_dns_req *completed_request)
 		getdns_bindata *tmp_ipv4_address;
 		getdns_bindata *tmp_ipv6_address;
 
-		if (call_reporting && (  netreq->response_len
-		                      || netreq->state == NET_REQ_TIMED_OUT)) {
+		if (call_reporting) {
 			if (!(netreq_debug =
 			   _getdns_create_call_reporting_dict(context,netreq)))
 				goto error;
@@ -1249,6 +1250,11 @@ _getdns_create_getdns_response(getdns_dns_req *completed_request)
 
 			netreq_debug = NULL;
 		}
+		switch (netreq->state) {
+		case NET_REQ_TIMED_OUT: ntimedout++; break;
+		case NET_REQ_ERRORED  : nerred++   ; break;
+		default               :              break;
+		}
 		if (! netreq->response_len)
 			continue;
 
@@ -1383,19 +1389,20 @@ _getdns_create_getdns_response(getdns_dns_req *completed_request)
 		GETDNS_FREE(context->mf, srvs.rrs);
 	}
 	if (getdns_dict_set_int(result, GETDNS_STR_KEY_STATUS,
-	    completed_request->request_timed_out ||
-	    nreplies == 0   ? GETDNS_RESPSTATUS_ALL_TIMEOUT :
-	    (  completed_request->dnssec
-	    && nsecure == 0 && nindeterminate ) > 0
-	                    ? GETDNS_RESPSTATUS_NO_SECURE_ANSWERS :
-	    (  completed_request->dnssec_return_only_secure
-	    && nsecure == 0 && ninsecure ) > 0
-	                    ? GETDNS_RESPSTATUS_NO_SECURE_ANSWERS :
-	    (  completed_request->dnssec_return_only_secure
-	    || completed_request->dnssec ) && nsecure == 0 && nbogus > 0
-	                    ? GETDNS_RESPSTATUS_ALL_BOGUS_ANSWERS :
-	    nanswers == 0   ? GETDNS_RESPSTATUS_NO_NAME
-	                    : GETDNS_RESPSTATUS_GOOD))
+	      completed_request->request_timed_out || nreplies == 0
+	    ? ( nerred > ntimedout ? GETDNS_RESPSTATUS_ALL_ERRED
+	                           : GETDNS_RESPSTATUS_ALL_TIMEOUT )
+	    : completed_request->dnssec && nsecure == 0 && nindeterminate > 0
+	    ? GETDNS_RESPSTATUS_NO_SECURE_ANSWERS
+	    : completed_request->dnssec_return_only_secure && nsecure == 0
+	                                                   && ninsecure > 0
+	    ? GETDNS_RESPSTATUS_NO_SECURE_ANSWERS
+	    : (  completed_request->dnssec_return_only_secure
+	      || completed_request->dnssec ) && nsecure == 0 && nbogus > 0
+	    ? GETDNS_RESPSTATUS_ALL_BOGUS_ANSWERS
+	    : nanswers == 0
+	    ? GETDNS_RESPSTATUS_NO_NAME
+	    : GETDNS_RESPSTATUS_GOOD))
 		goto error_free_result;
 
 	return result;