From 141a26f979b4bc959d8e866a295e24f8cf456920 Mon Sep 17 00:00:00 2001
From: Simon Kelley <simon@thekelleys.org.uk>
Date: Wed, 17 Feb 2021 23:56:32 +0000
Subject: [PATCH] Fix problem with DNS retries in 2.83/2.84.

The new logic in 2.83/2.84 which merges distinct requests for the
same domain causes problems with clients which do retries as distinct
requests (differing IDs and/or source ports.) The retries just get
piggy-backed on the first, failed, request.

The logic is now changed so that distinct requests for repeated
queries still get merged into a single ID/source port, but they now
always trigger a re-try upstream.

Thanks to Nicholas Mu for his analysis.
---
 src/forward.c | 79 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/src/forward.c b/src/forward.c
index 8fb0327..e82e14a 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -278,8 +278,46 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
     fwd_flags |= FREC_DO_QUESTION;
 #endif
   
-  /* may be no servers available. */
-  if (forward || (forward = lookup_frec_by_sender(ntohs(header->id), udpaddr, hash)))
+  /* Check for retry on existing query from same source */
+  if (!forward && (!(forward = lookup_frec_by_sender(ntohs(header->id), udpaddr, hash))))
+    {
+      /* Maybe query from new source, but the same query may be in progress
+	 from another source. If so, just add this client to the
+	 list that will get the reply.*/
+	 
+      if (!option_bool(OPT_ADD_MAC) && !option_bool(OPT_MAC_B64) &&
+	  (forward = lookup_frec_by_query(hash, fwd_flags)))
+	{
+	  struct frec_src *new;
+
+	  /* Note whine_malloc() zeros memory. */
+	  if (!daemon->free_frec_src &&
+	      daemon->frec_src_count < daemon->ftabsize &&
+	      (daemon->free_frec_src = whine_malloc(sizeof(struct frec_src))))
+	    {
+	      daemon->frec_src_count++;
+	      daemon->free_frec_src->next = NULL;
+	    }
+	  
+	  /* If we've been spammed with many duplicates, just drop the query. */
+	  if (!daemon->free_frec_src)
+	    return 0;
+	  
+	  new = daemon->free_frec_src;
+	  daemon->free_frec_src = new->next;
+	  new->next = forward->frec_src.next;
+	  forward->frec_src.next = new;
+	  new->orig_id = ntohs(header->id);
+	  new->source = *udpaddr;
+	  new->dest = *dst_addr;
+	  new->log_id = daemon->log_id;
+	  new->iface = dst_iface;
+	  new->fd = udpfd;
+	}
+    }
+
+  /* retry existing query */
+  if (forward)
     {
       /* If we didn't get an answer advertising a maximal packet in EDNS,
 	 fall back to 1280, which should work everywhere on IPv6.
@@ -350,40 +388,8 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
     }
   else 
     {
-      /* Query from new source, but the same query may be in progress
-	 from another source. If so, just add this client to the
-	 list that will get the reply.*/
-	 
-      if (!option_bool(OPT_ADD_MAC) && !option_bool(OPT_MAC_B64) &&
-	  (forward = lookup_frec_by_query(hash, fwd_flags)))
-	{
-	  /* Note whine_malloc() zeros memory. */
-	  if (!daemon->free_frec_src &&
-	      daemon->frec_src_count < daemon->ftabsize &&
-	      (daemon->free_frec_src = whine_malloc(sizeof(struct frec_src))))
-	    {
-	      daemon->frec_src_count++;
-	      daemon->free_frec_src->next = NULL;
-	    }
-	  
-	  /* If we've been spammed with many duplicates, just drop the query. */
-	  if (daemon->free_frec_src)
-	    {
-	      struct frec_src *new = daemon->free_frec_src;
-	      daemon->free_frec_src = new->next;
-	      new->next = forward->frec_src.next;
-	      forward->frec_src.next = new;
-	      new->orig_id = ntohs(header->id);
-	      new->source = *udpaddr;
-	      new->dest = *dst_addr;
-	      new->log_id = daemon->log_id;
-	      new->iface = dst_iface;
-	      new->fd = udpfd;
-	    }
-	  
-	  return 1;
-	}
-	
+      /* new query */
+
       if (gotname)
 	flags = search_servers(now, &addrp, gotname, daemon->namebuff, &type, &domain, &norebind);
       
@@ -392,6 +398,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
 #endif
       type &= ~SERV_DO_DNSSEC;      
       
+      /* may be no servers available. */
       if (daemon->servers && !flags)
 	forward = get_new_frec(now, NULL, NULL);
       /* table full - flags == 0, return REFUSED */
-- 
1.8.3.1