From: Colin Vidal <colin@isc.org>
Date: Tue, 14 Jan 2025 16:13:42 +0000 (+0100)
Subject: add support for multiple EDE
X-Git-Tag: v9.21.5~38^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4096f2713075362b204a88eee1b6bc30e609a328;p=thirdparty%2Fbind9.git

add support for multiple EDE

Extended DNS error mechanism (EDE) enables to have several EDE raised
during a DNS resolution (typically, a DNSSEC query will do multiple
fetches which each of them can have an error). Add support to up to 3
EDE errors in an DNS response. If duplicates occur (two EDEs with the
same code, the extra text is not compared), only the first one will be
part of the DNS answer.

Because the maximum number of EDE is statically fixed, `ns_client_t`
object own a static vector of `DNS_DE_MAX_ERRORS` (instead of a linked
list, for instance). The array can be fully filled (all slots point to
an allocated `dns_ednsopt_t` object) or partially filled (or
empty). In such case, the first NULL slot means there is no more EDE
objects.
---

diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h
index c7ebf49591c..e8ea327ba0c 100644
--- a/lib/dns/include/dns/message.h
+++ b/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
- * will set only one per message (for now).
+ * options we know about. Extended DNS Errors may occur multiple times, see
+ * 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)
 
diff --git a/lib/dns/message.c b/lib/dns/message.c
index c7441071a8b..28b3de2387a 100644
--- a/lib/dns/message.c
+++ b/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);
-
-		if (len >= DNS_EDE_EXTRATEXT_LEN) {
-			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';
+		ede->extra_text = isc_mem_allocate(mctx,
+						   strlen(extra_text) + 1);
+		strcpy(ede->extra_text, extra_text);
 	}
 
 	ISC_LIST_APPEND(*list, ede, link);
diff --git a/lib/ns/client.c b/lib/ns/client.c
index b1502dee48b..a330f663cb4 100644
--- a/lib/ns/client.c
+++ b/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) {
-		return;
+	for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) {
+		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];
-	isc_buffer_t buf;
-	uint16_t len = sizeof(uint16_t);
+	const uint16_t codelen = sizeof(code);
+	uint16_t textlen = 0;
+	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),
+				      "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),
-			      "already have ede, ignoring %u %s", code,
+			      "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) {
-			isc_buffer_putstr(&buf, text);
-			len += (uint16_t)(strlen(text));
-		} else {
+		textlen = strlen(text);
+
+		if (textlen > DNS_EDE_EXTRATEXT_LEN) {
 			ns_client_log(client, NS_LOGCATEGORY_CLIENT,
-				      NS_LOGMODULE_CLIENT, ISC_LOG_WARNING,
-				      "ede extra-text too long, ignoring");
+				      NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1),
+				      "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));
-	client->ede->code = DNS_OPT_EDE;
-	client->ede->length = len;
-	client->ede->value = isc_mem_get(client->manager->mctx, len);
-	memmove(client->ede->value, ede, len);
+	ede = isc_mem_get(client->manager->mctx, codelen + textlen);
+
+	memcpy(ede, &code, sizeof(code));
+	if (textlen > 0) {
+		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].value = client->ede->value;
+		ednsopts[count].length = ede->length;
+		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.
diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h
index 2c516de3b87..96f10665e10 100644
--- a/lib/ns/include/ns/client.h
+++ b/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 */
diff --git a/lib/ns/query.c b/lib/ns/query.c
index 8464e782d9f..ff75978b13e 100644
--- a/lib/ns/query.c
+++ b/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().