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