new: usr: Add support for multiple extended DNS errors

Extended DNS error mechanism (EDE) may have several errors raised during a DNS resolution. `named` is now able to add up to three EDE codes in a DNS response. In the case of duplicate error codes, only the first one will be part of the DNS response.

Closes #5085

Merge branch '5085-multiple-ede' into 'main'

See merge request isc-projects/bind9!9952

lib/dns/include/dns/message.h

@@ -125,10 +125,10 @@
 
 /*%<
  * The maximum number of EDNS options we allow to set. Reserve space for the
- * options we know about. Extended DNS Errors may occur multiple times, but we
+ * options we know about. Extended DNS Errors may occur multiple times, see
- * will set only one per message (for now).
+ * DNS_EDE_MAX_ERRORS.
  */
-#define DNS_EDNSOPTIONS 9
+#define DNS_EDNSOPTIONS 8 + DNS_EDE_MAX_ERRORS
 
 /*%< EDNS0 extended DNS errors */
 #define DNS_EDE_OTHER		     0	/*%< Other Error */
@@ -176,6 +176,8 @@ typedef ISC_LIST(dns_ede_t) dns_edelist_t;
  */
 #define DNS_EDE_EXTRATEXT_LEN 64
 
+#define DNS_EDE_MAX_ERRORS 3
+
 #define DNS_MESSAGE_REPLYPRESERVE	 (DNS_MESSAGEFLAG_RD | DNS_MESSAGEFLAG_CD)
 #define DNS_MESSAGEEXTFLAG_REPLYPRESERVE (DNS_MESSAGEEXTFLAG_DO)
 

lib/dns/message.c

@@ -5088,19 +5088,9 @@ dns_ede_append(isc_mem_t *mctx, dns_edelist_t *list, uint16_t info_code,
 	};
 
 	if (extra_text) {
-		size_t len = strlen(extra_text);
+		ede->extra_text = isc_mem_allocate(mctx,
-
+						   strlen(extra_text) + 1);
-		if (len >= DNS_EDE_EXTRATEXT_LEN) {
+		strcpy(ede->extra_text, extra_text);
-			isc_log_write(DNS_LOGCATEGORY_GENERAL,
-				      DNS_LOGMODULE_MESSAGE, ISC_LOG_PRINTTIME,
-				      "truncate EDE code %hu text: %s",
-				      info_code, extra_text);
-			len = DNS_EDE_EXTRATEXT_LEN - 1;
-		}
-
-		ede->extra_text = isc_mem_allocate(mctx, len + 1);
-		strncpy(ede->extra_text, extra_text, len);
-		ede->extra_text[len] = '\0';
 	}
 
 	ISC_LIST_APPEND(*list, ede, link);

lib/ns/client.c

@@ -220,27 +220,54 @@ ns_client_settimeout(ns_client_t *client, unsigned int seconds) {
 
 static void
 client_extendederror_reset(ns_client_t *client) {
-	if (client->ede == NULL) {
+	for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) {
-		return;
+		if (client->ede[i]) {
+			dns_ednsopt_t *ede = client->ede[i];
+
+			isc_mem_put(client->manager->mctx, ede->value,
+				    ede->length);
+			isc_mem_put(client->manager->mctx, ede, sizeof(*ede));
+			client->ede[i] = NULL;
+		}
 	}
-	isc_mem_put(client->manager->mctx, client->ede->value,
-		    client->ede->length);
-	isc_mem_put(client->manager->mctx, client->ede, sizeof(dns_ednsopt_t));
-	client->ede = NULL;
 }
 
 void
 ns_client_extendederror(ns_client_t *client, uint16_t code, const char *text) {
-	unsigned char ede[DNS_EDE_EXTRATEXT_LEN + 2];
+	const uint16_t codelen = sizeof(code);
-	isc_buffer_t buf;
+	uint16_t textlen = 0;
-	uint16_t len = sizeof(uint16_t);
+	size_t pos = 0;
+	unsigned char *ede = NULL;
+	dns_ednsopt_t *edns = NULL;
 
 	REQUIRE(NS_CLIENT_VALID(client));
 
-	if (client->ede != NULL) {
+	/*
+	 * As ede will be directly put in the DNS message we need to make sure
+	 * the code is in big-endian format
+	 */
+	code = htobe16(code);
+
+	for (pos = 0; pos < DNS_EDE_MAX_ERRORS; pos++) {
+		edns = client->ede[pos];
+
+		if (edns == NULL) {
+			break;
+		}
+
+		if (memcmp(&code, edns->value, sizeof(code)) == 0) {
 			ns_client_log(client, NS_LOGCATEGORY_CLIENT,
 				      NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1),
-			      "already have ede, ignoring %u %s", code,
+				      "ignoring duplicate ede %u %s", code,
+				      text == NULL ? "(null)" : text);
+			return;
+		}
+	}
+
+	if (pos >= DNS_EDE_MAX_ERRORS) {
+		ns_client_log(client, NS_LOGCATEGORY_CLIENT,
+			      NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1),
+			      "too many ede, ignoring %u %s", code,
 			      text == NULL ? "(null)" : text);
 		return;
 	}
@@ -249,24 +276,31 @@ ns_client_extendederror(ns_client_t *client, uint16_t code, const char *text) {
 		      ISC_LOG_DEBUG(1), "set ede: info-code %u extra-text %s",
 		      code, text == NULL ? "(null)" : text);
 
-	isc_buffer_init(&buf, ede, sizeof(ede));
-	isc_buffer_putuint16(&buf, code);
 	if (text != NULL && strlen(text) > 0) {
-		if (strlen(text) < DNS_EDE_EXTRATEXT_LEN) {
+		textlen = strlen(text);
-			isc_buffer_putstr(&buf, text);
+
-			len += (uint16_t)(strlen(text));
+		if (textlen > DNS_EDE_EXTRATEXT_LEN) {
-		} else {
 			ns_client_log(client, NS_LOGCATEGORY_CLIENT,
-				      NS_LOGMODULE_CLIENT, ISC_LOG_WARNING,
+				      NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1),
-				      "ede extra-text too long, ignoring");
+				      "truncate EDE code %hu text: %s", code,
+				      text);
+			textlen = DNS_EDE_EXTRATEXT_LEN;
 		}
 	}
 
-	client->ede = isc_mem_get(client->manager->mctx, sizeof(dns_ednsopt_t));
+	ede = isc_mem_get(client->manager->mctx, codelen + textlen);
-	client->ede->code = DNS_OPT_EDE;
+
-	client->ede->length = len;
+	memcpy(ede, &code, sizeof(code));
-	client->ede->value = isc_mem_get(client->manager->mctx, len);
+	if (textlen > 0) {
-	memmove(client->ede->value, ede, len);
+		memcpy(ede + codelen, text, textlen);
+	}
+
+	edns = isc_mem_get(client->manager->mctx, sizeof(*edns));
+	*edns = (dns_ednsopt_t){ .code = DNS_OPT_EDE,
+				 .length = codelen + textlen,
+				 .value = ede };
+
+	client->ede[pos] = edns;
 }
 
 static void
@@ -1231,11 +1265,17 @@ no_nsid:
 		count++;
 	}
 
-	if (client->ede != NULL) {
+	for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) {
+		dns_ednsopt_t *ede = client->ede[i];
+
+		if (ede == NULL) {
+			break;
+		}
+
 		INSIST(count < DNS_EDNSOPTIONS);
 		ednsopts[count].code = DNS_OPT_EDE;
-		ednsopts[count].length = client->ede->length;
+		ednsopts[count].length = ede->length;
-		ednsopts[count].value = client->ede->value;
+		ednsopts[count].value = ede->value;
 		count++;
 	}
 
@@ -2034,7 +2074,6 @@ ns_client_request(isc_nmhandle_t *handle, isc_result_t eresult,
 	}
 
 	client->message->rcode = dns_rcode_noerror;
-	client->ede = NULL;
 
 	/*
 	 * Deal with EDNS.

lib/ns/include/ns/client.h

@@ -180,7 +180,7 @@ struct ns_client {
 	size_t		tcpbuf_size;
 	dns_message_t  *message;
 	dns_rdataset_t *opt;
-	dns_ednsopt_t  *ede;
+	dns_ednsopt_t  *ede[DNS_EDE_MAX_ERRORS];
 	uint16_t	udpsize;
 	uint16_t	extflags;
 	int16_t		ednsversion; /* -1 noedns */

lib/ns/query.c

@@ -899,6 +899,7 @@ ns_query_init(ns_client_t *client) {
 	ISC_LIST_INIT(client->query.namebufs);
 	ISC_LIST_INIT(client->query.activeversions);
 	ISC_LIST_INIT(client->query.freeversions);
+	memset(client->ede, 0, sizeof(dns_ednsopt_t *) * DNS_EDE_MAX_ERRORS);
 	/*
 	 * This mutex is destroyed when the client is destroyed in
 	 * exit_check().

tests/dns/ede_test.c

@@ -56,12 +56,16 @@ ISC_RUN_TEST_IMPL(ede_enqueue_unlink) {
 	assert_string_equal(ede->extra_text, msg1);
 	assert_ptr_not_equal(ede->extra_text, msg1);
 
+	/*
+	 * Even though we limit the length of an EDE message to 64 bytes,
+	 * this is done only at the ns/client.c level (to make sure to cover all
+	 * the flows).
+	 */
 	ede = ISC_LIST_NEXT(ede, link);
 	assert_non_null(ede);
 	assert_int_equal(ede->info_code, 4);
-	assert_string_not_equal(ede->extra_text, msg2);
+	assert_string_equal(ede->extra_text, msg2);
 	assert_ptr_not_equal(ede->extra_text, msg2);
-	assert_int_equal(strlen(ede->extra_text), 63);
 
 	dns_ede_unlinkall(mctx, &list);
 	assert_true(ISC_LIST_EMPTY(list));

tests/ns/Makefile.am

@@ -17,7 +17,8 @@ LDADD +=			\
 check_PROGRAMS =		\
 	notify_test		\
 	plugin_test		\
-	query_test
+	query_test		\
+	client_test
 
 notify_test_SOURCES =		\
 	notify_test.c		\

tests/ns/client_test.c

@@ -0,0 +1,171 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#include <inttypes.h>
+#include <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include <isc/buffer.h>
+#include <isc/list.h>
+#include <isc/net.h>
+#include <isc/timer.h>
+#include <isc/tls.h>
+#include <isc/util.h>
+
+#include <ns/client.h>
+
+#include <tests/isc.h>
+
+typedef struct {
+	uint16_t code;
+	const char *txt;
+} client_tests_ede_expected_t;
+
+static ns_clientmgr_t client_ede_test_dummy_manager;
+
+static ns_client_t *
+client_ede_test_initclient(void) {
+	client_ede_test_dummy_manager.mctx = mctx;
+
+	ns_client_t *client = isc_mem_get(mctx, sizeof(*client));
+	memset(client, 0, sizeof(*client));
+	client->magic = NS_CLIENT_MAGIC;
+	client->manager = &client_ede_test_dummy_manager;
+
+	return client;
+}
+
+static void
+client_ede_test_free(ns_client_t *client) {
+	for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) {
+		dns_ednsopt_t *ede = client->ede[i];
+
+		if (ede) {
+			isc_mem_put(mctx, ede->value, ede->length);
+			isc_mem_put(mctx, ede, sizeof(*ede));
+		}
+		client->ede[i] = NULL;
+	}
+	isc_mem_put(mctx, client, sizeof(*client));
+}
+
+static void
+client_ede_test_equals(const client_tests_ede_expected_t *expected,
+		       size_t expected_count, const ns_client_t *client) {
+	size_t count = 0;
+
+	for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) {
+		dns_ednsopt_t *edns = client->ede[i];
+
+		if (edns == NULL) {
+			break;
+		}
+
+		uint16_t code;
+		const size_t codelen = sizeof(code);
+		const unsigned char *txt;
+
+		assert_in_range(count, 0, expected_count);
+		assert_int_equal(edns->code, DNS_OPT_EDE);
+
+		code = ISC_U8TO16_BE(edns->value);
+		assert_int_equal(code, expected[count].code);
+
+		if (edns->length > codelen) {
+			assert_non_null(expected[count].txt);
+			txt = edns->value + codelen;
+			assert_memory_equal(expected[count].txt, txt,
+					    edns->length - codelen);
+		} else {
+			assert_null(expected[count].txt);
+		}
+
+		count++;
+	}
+	assert_int_equal(count, expected_count);
+}
+
+ISC_RUN_TEST_IMPL(client_ede_test_text_max_count) {
+	ns_client_t *client = client_ede_test_initclient();
+
+	const char *txt1 = "foobar";
+	const char *txt2 = "It's been a long time since I rock-and-rolled"
+			   "Ooh, let me get it back, let me get it back";
+
+	ns_client_extendederror(client, 2, txt1);
+	ns_client_extendederror(client, 22, NULL);
+	ns_client_extendederror(client, 3, txt2);
+
+	const client_tests_ede_expected_t expected[3] = {
+		{ .code = 2, .txt = "foobar" },
+		{ .code = 22, .txt = NULL },
+		{ .code = 3,
+		  .txt = "It's been a long time since I rock-and-rolledOoh, "
+			 "let me get it " }
+	};
+
+	client_ede_test_equals(expected, 3, client);
+	client_ede_test_free(client);
+}
+
+ISC_RUN_TEST_IMPL(client_ede_test_max_count) {
+	ns_client_t *client = client_ede_test_initclient();
+
+	ns_client_extendederror(client, 1, NULL);
+	ns_client_extendederror(client, 22, "two");
+	ns_client_extendederror(client, 3, "three");
+	ns_client_extendederror(client, 4, "four");
+	ns_client_extendederror(client, 5, "five");
+
+	const client_tests_ede_expected_t expected[3] = {
+		{ .code = 1, .txt = NULL },
+		{ .code = 22, .txt = "two" },
+		{ .code = 3, .txt = "three" },
+	};
+
+	client_ede_test_equals(expected, 3, client);
+	client_ede_test_free(client);
+}
+
+ISC_RUN_TEST_IMPL(client_ede_test_duplicates) {
+	ns_client_t *client = client_ede_test_initclient();
+
+	ns_client_extendederror(client, 1, NULL);
+	ns_client_extendederror(client, 1, "two");
+	ns_client_extendederror(client, 1, "three");
+
+	const client_tests_ede_expected_t expected[] = {
+		{ .code = 1, .txt = NULL },
+	};
+
+	client_ede_test_equals(expected, 1, client);
+
+	client_ede_test_free(client);
+}
+
+ISC_TEST_LIST_START
+
+ISC_TEST_ENTRY(client_ede_test_text_max_count)
+ISC_TEST_ENTRY(client_ede_test_max_count)
+ISC_TEST_ENTRY(client_ede_test_duplicates)
+
+ISC_TEST_LIST_END
+
+ISC_TEST_MAIN