- Fix for #411, #439, #469: Reset the DNS message ID when moving queries

  between TCP streams.
- Refactor for uniform way to produce random DNS message IDs.

diff --git a/doc/Changelog b/doc/Changelog
index 6fd42f92d21d41813fc5b7a700a8566ce3938faa..c410c188099a1bf56dd6db378490c92cbe2b054a 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,8 @@
+19 May 2021: George
+       - Fix for #411, #439, #469: Reset the DNS message ID when moving queries
+         between TCP streams.
+       - Refactor for uniform way to produce random DNS message IDs.
+
 17 May 2021: Wouter
        - Fix #489: Compile using MSYS2 MinGW 64-bit.
 

diff --git a/services/authzone.c b/services/authzone.c
index 196fe6693d958277d6e6ffb3a447769544d3eeb2..9f9b08f7de03f77bb0497b076836c69a1f16488c 100644 (file)
--- a/services/authzone.c
+++ b/services/authzone.c
@@ -5442,7 +5442,7 @@ xfr_transfer_init_fetch(struct auth_xfer* xfr, struct module_env* env)
        /* perform AXFR/IXFR */
        /* set the packet to be written */
        /* create new ID */
-       xfr->task_transfer->id = (uint16_t)(ub_random(env->rnd)&0xffff);
+       xfr->task_transfer->id = GET_RANDOM_ID(env->rnd);
        xfr_create_ixfr_packet(xfr, env->scratch_buffer,
                xfr->task_transfer->id, master);
 
@@ -6292,7 +6292,7 @@ xfr_probe_send_probe(struct auth_xfer* xfr, struct module_env* env,
        /* create new ID for new probes, but not on timeout retries,
         * this means we'll accept replies to previous retries to same ip */
        if(timeout == AUTH_PROBE_TIMEOUT)
-               xfr->task_probe->id = (uint16_t)(ub_random(env->rnd)&0xffff);
+               xfr->task_probe->id = GET_RANDOM_ID(env->rnd);
        xfr_create_soa_probe_packet(xfr, env->scratch_buffer, 
                xfr->task_probe->id);
        /* we need to remove the cp if we have a different ip4/ip6 type now */

diff --git a/services/outside_network.c b/services/outside_network.c
index 9b09aa3609560929a09707ec498c517df5203484..af14f8622d4fb8adcc0ce53205133a8528b70dfc 100644 (file)
--- a/services/outside_network.c
+++ b/services/outside_network.c
@@ -94,6 +94,10 @@ static void waiting_list_remove(struct outside_network* outnet,
 static void reuse_tcp_remove_tree_list(struct outside_network* outnet,
        struct reuse_tcp* reuse);
 
+/** select a DNS ID for a TCP stream */
+static uint16_t tcp_select_id(struct outside_network* outnet,
+       struct reuse_tcp* reuse);
+
 int 
 pending_cmp(const void* key1, const void* key2)
 {
@@ -406,9 +410,18 @@ static void reuse_write_wait_push_back(struct reuse_tcp* reuse,
 void
 reuse_tree_by_id_insert(struct reuse_tcp* reuse, struct waiting_tcp* w)
 {
+#ifdef UNBOUND_DEBUG
+       rbnode_type* added;
+#endif
        log_assert(w->id_node.key == NULL);
        w->id_node.key = w;
+#ifdef UNBOUND_DEBUG
+       added =
+#else
+       (void)
+#endif
        rbtree_insert(&reuse->tree_by_id, &w->id_node);
+       log_assert(added);  /* should have been added */
 }
 
 /** find element in tree by id */
@@ -752,6 +765,9 @@ use_free_buffer(struct outside_network* outnet)
                w->on_tcp_waiting_list = 0;
                reuse = reuse_tcp_find(outnet, &w->addr, w->addrlen,
                        w->ssl_upstream);
+               /* re-select an ID when moving to a new TCP buffer */
+               w->id = tcp_select_id(outnet, reuse);
+               LDNS_ID_SET(w->pkt, w->id);
                if(reuse) {
                        log_reuse_tcp(VERB_CLIENT, "use free buffer for waiting tcp: "
                                "found reuse", reuse);
@@ -830,8 +846,17 @@ outnet_add_tcp_waiting(struct outside_network* outnet, struct waiting_tcp* w)
 static void
 reuse_tree_by_id_delete(struct reuse_tcp* reuse, struct waiting_tcp* w)
 {
+#ifdef UNBOUND_DEBUG
+       rbnode_type* rem;
+#endif
        log_assert(w->id_node.key != NULL);
+#ifdef UNBOUND_DEBUG
+       rem =
+#else
+       (void)
+#endif
        rbtree_delete(&reuse->tree_by_id, w);
+       log_assert(rem);  /* should have been there */
        w->id_node.key = NULL;
 }
 
@@ -1788,14 +1813,14 @@ select_id(struct outside_network* outnet, struct pending* pend,
        sldns_buffer* packet)
 {
        int id_tries = 0;
-       pend->id = ((unsigned)ub_random(outnet->rnd)>>8) & 0xffff;
+       pend->id = GET_RANDOM_ID(outnet->rnd);
        LDNS_ID_SET(sldns_buffer_begin(packet), pend->id);
 
        /* insert in tree */
        pend->node.key = pend;
        while(!rbtree_insert(outnet->pending, &pend->node)) {
                /* change ID to avoid collision */
-               pend->id = ((unsigned)ub_random(outnet->rnd)>>8) & 0xffff;
+               pend->id = GET_RANDOM_ID(outnet->rnd);
                LDNS_ID_SET(sldns_buffer_begin(packet), pend->id);
                id_tries++;
                if(id_tries == MAX_ID_RETRY) {
@@ -2088,6 +2113,14 @@ reuse_tcp_close_oldest(struct outside_network* outnet)
        reuse_cb_and_decommission(outnet, pend, NETEVENT_CLOSED);
 }
 
+static uint16_t
+tcp_select_id(struct outside_network* outnet, struct reuse_tcp* reuse)
+{
+       if(reuse)
+               return reuse_tcp_select_id(reuse, outnet);
+       return GET_RANDOM_ID(outnet->rnd);
+}
+
 /** find spare ID value for reuse tcp stream.  That is random and also does
  * not collide with an existing query ID that is in use or waiting */
 uint16_t
@@ -2101,13 +2134,13 @@ reuse_tcp_select_id(struct reuse_tcp* reuse, struct outside_network* outnet)
 
        /* make really sure the tree is not empty */
        if(reuse->tree_by_id.count == 0) {
-               id = ((unsigned)ub_random(outnet->rnd)>>8) & 0xffff;
+               id = GET_RANDOM_ID(outnet->rnd);
                return id;
        }
 
        /* try to find random empty spots by picking them */
        for(i = 0; i<try_random; i++) {
-               id = ((unsigned)ub_random(outnet->rnd)>>8) & 0xffff;
+               id = GET_RANDOM_ID(outnet->rnd);
                if(!reuse_tcp_by_id_find(reuse, id)) {
                        return id;
                }
@@ -2205,9 +2238,7 @@ pending_tcp_query(struct serviced_query* sq, sldns_buffer* packet,
        w->pkt = (uint8_t*)w + sizeof(struct waiting_tcp);
        w->pkt_len = sldns_buffer_limit(packet);
        memmove(w->pkt, sldns_buffer_begin(packet), w->pkt_len);
-       if(reuse)
-               w->id = reuse_tcp_select_id(reuse, sq->outnet);
-       else    w->id = ((unsigned)ub_random(sq->outnet->rnd)>>8) & 0xffff;
+       w->id = tcp_select_id(sq->outnet, reuse);
        LDNS_ID_SET(w->pkt, w->id);
        memcpy(&w->addr, &sq->addr, sq->addrlen);
        w->addrlen = sq->addrlen;

diff --git a/util/net_help.h b/util/net_help.h
index 114f4cdb5885c099259fc772ef76c7baaa7f3c51..79835270c4772517053e7e60703c609c972020ba 100644 (file)
--- a/util/net_help.h
+++ b/util/net_help.h
@@ -42,6 +42,7 @@
 #ifndef NET_HELP_H
 #define NET_HELP_H
 #include "util/log.h"
+#include "util/random.h"
 struct sock_list;
 struct regional;
 struct config_strlist;
@@ -92,6 +93,9 @@ extern uint16_t EDNS_ADVERTISED_SIZE;
 /** DNSKEY secure entry point, KSK flag */
 #define DNSKEY_BIT_SEP 0x0001
 
+/** return a random 16-bit number given a random source */
+#define GET_RANDOM_ID(rnd) (((unsigned)ub_random(rnd)>>8) & 0xffff)
+
 /** minimal responses when positive answer */
 extern int MINIMAL_RESPONSES;