From 3ae073ab82183075a93e71ed889e0ee4fdb780e6 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.

(cherry picked from commit 141a26f979b4bc959d8e866a295e24f8cf456920)

Simplify preceding fix.

Remove distinction between retry with same QID/SP and

retry for same query with different QID/SP. If the

QID/SP are the same as an existing one, simply retry,

if a new QID/SP is seen, add to the list to be replied to.

(cherry picked from commit 305cb79c5754d5554729b18a2c06fe7ce699687a)

---

 src/forward.c | 105 ++++++++++++++++++++++----------------------------

 1 file changed, 45 insertions(+), 60 deletions(-)

diff --git a/src/forward.c b/src/forward.c

index 637f220..1e9c509 100644

--- a/src/forward.c

+++ b/src/forward.c

@@ -17,9 +17,6 @@

 #include "dnsmasq.h"

 static struct frec *lookup_frec(unsigned short id, int fd, int family, void *hash);

-static struct frec *lookup_frec_by_sender(unsigned short id,

-					  union mysockaddr *addr,

-					  void *hash);

 static struct frec *lookup_frec_by_query(void *hash, unsigned int flags);

 static unsigned short get_id(void);

@@ -260,8 +257,48 @@ 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 */

+  if (!forward && (forward = lookup_frec_by_query(hash, fwd_flags)))

+    {

+      struct frec_src *src;

+

+      for (src = &forward->frec_src; src; src = src->next)

+	if (src->orig_id == ntohs(header->id) && 

+	    sockaddr_isequal(&src->source, udpaddr))

+	  break;

+

+      /* Existing query, but from new source, just add this 

+	 client to the list that will get the reply.*/

+      if (!src)

+	{

+	  /* 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;

+	  

+	  src = daemon->free_frec_src;

+	  daemon->free_frec_src = src->next;

+	  src->next = forward->frec_src.next;

+	  forward->frec_src.next = src;

+	  src->orig_id = ntohs(header->id);

+	  src->source = *udpaddr;

+	  src->dest = *dst_addr;

+	  src->log_id = daemon->log_id;

+	  src->iface = dst_iface;

+	  src->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.

@@ -336,42 +373,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.

-	 

-	 Note that is the EDNS client subnet option is in use, we can't do this,

-	 as the clients (and therefore query EDNS options) will be different

-	 for each query. The EDNS subnet code has checks to avoid

-	 attacks in this case. */

-      if (!option_bool(OPT_CLIENT_SUBNET) && (forward = lookup_frec_by_query(hash, fwd_flags)))

-	{

-	  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);

@@ -380,6 +383,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, 0);

       /* table full - flags == 0, return REFUSED */

@@ -2249,25 +2253,6 @@ static struct frec *lookup_frec(unsigned short id, int fd, int family, void *has

   return NULL;

 }

-static struct frec *lookup_frec_by_sender(unsigned short id,

-					  union mysockaddr *addr,

-					  void *hash)

-{

-  struct frec *f;

-  struct frec_src *src;

-

-  for (f = daemon->frec_list; f; f = f->next)

-    if (f->sentto &&

-	!(f->flags & (FREC_DNSKEY_QUERY | FREC_DS_QUERY)) &&

-	memcmp(hash, f->hash, HASH_SIZE) == 0)

-      for (src = &f->frec_src; src; src = src->next)

-	if (src->orig_id == id && 

-	    sockaddr_isequal(&src->source, addr))

-	  return f;

-  

-  return NULL;

-}

-

 static struct frec *lookup_frec_by_query(void *hash, unsigned int flags)

 {

   struct frec *f;

-- 

2.26.2