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.

(cherry picked from commit 4096f2713075362b204a88eee1b6bc30e609a328)

diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h
index 8e7b6dd29fbd518a3d84b3017eea8045883a59d3..b461992334aad9eacd2f4a44142b4179942c7a8c 100644 (file)
--- a/lib/dns/include/dns/message.h
+++ b/lib/dns/include/dns/message.h
@@ -119,10 +119,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 8
+#define DNS_EDNSOPTIONS 7 + DNS_EDE_MAX_ERRORS
 
 /*%< EDNS0 extended DNS errors */
 #define DNS_EDE_OTHER               0  /*%< Other Error */
@@ -170,6 +170,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 f095c2decfb796775f347cc7608055f45a3cfcf3..94f2d92ab1a38f01c6c1d188e0f26317e754057e 100644 (file)
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -5029,19 +5029,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_lctx, 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 a51b82b88934c53136e3526b64b4423e01522aa4..63b806d522f5bc6ccf7ffd9e346a1d9925abdb6e 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -225,27 +225,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;
        }
@@ -254,24 +281,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
@@ -1233,11 +1267,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++;
        }
 
@@ -2022,7 +2062,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 0711add8e515046d9cea08959b6741f2f4c12470..e62fd8db07beb67fafe414978d8271dc7c114e0f 100644 (file)
--- 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 29000be3eebfb959043e5e4d2e9b3e93517dcae7..714c8d6c93268153f7f7cfb6c0e2972fa6c11900 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -902,6 +902,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().