Pi-Hole/dnsmasq
1
0
Fork 0
mirror of https://github.com/pi-hole/dnsmasq.git synced 2025-12-19 18:28:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix confusion with log-IDs and DNS retries.

Browse Source 
The IDs logged when --log-queries=extra is in effect
can be wrong in three cases.

1) When query is retried in response to a a SERVFAIL or REFUSED
answer from upstream. In this case the ID of an unrelated query will
appear in the answer log lines.

2) When the same query arrives from two clients. The query is
sent upstream once, as designed, and the result returned to both clients,
as designed, but the reply to the first client gets the log-ID of the
second query in error.

3) When a query arrives, is sent upstream, and the reply comes back,
but the transaction is blocked awaiting a DNSSEC query needed to validate
the reply. If the client retries the query in this state, the blocking
DNSSEC query will be resent, as designed, but that send will be logged with
the ID of the original, currently blocked, query.

Thanks to  Dominik Derigs for his analysis of this problem.
This commit is contained in:
Simon Kelley
2021-12-01 16:34:41 +00:00
parent e3093b532c
commit ed96efd865
1 changed files with 11 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/forward.c
View File

@@ -215,7 +215,11 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
break; break;
if (src) if (src)
old_src = 1; {
old_src = 1;
/* If a query is retried, use the log_id for the retry when logging the answer. */
src->log_id = daemon->log_id;
}
else else
{ {
/* Existing query, but from new source, just add this /* Existing query, but from new source, just add this
@@ -286,6 +290,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
goto reply; goto reply;
/* table full - flags == 0, return REFUSED */ /* table full - flags == 0, return REFUSED */
forward->frec_src.log_id = daemon->log_id;
forward->frec_src.source = *udpaddr; forward->frec_src.source = *udpaddr;
forward->frec_src.orig_id = ntohs(header->id); forward->frec_src.orig_id = ntohs(header->id);
forward->frec_src.dest = *dst_addr; forward->frec_src.dest = *dst_addr;
@@ -329,7 +334,6 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
} }
else else
{ {
/* retry on existing query, from original source. Send to all available servers */
#ifdef HAVE_DNSSEC #ifdef HAVE_DNSSEC
/* If we've already got an answer to this query, but we're awaiting keys for validation, /* If we've already got an answer to this query, but we're awaiting keys for validation,
there's no point retrying the query, retry the key query instead...... */ there's no point retrying the query, retry the key query instead...... */
@@ -340,7 +344,10 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
while (forward->blocking_query) while (forward->blocking_query)
forward = forward->blocking_query; forward = forward->blocking_query;
/* log_id should match previous DNSSEC query. */
daemon->log_display_id = forward->frec_src.log_id;
blockdata_retrieve(forward->stash, forward->stash_len, (void *)header); blockdata_retrieve(forward->stash, forward->stash_len, (void *)header);
plen = forward->stash_len; plen = forward->stash_len;
/* get query for logging. */ /* get query for logging. */
@@ -383,7 +390,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
Note that we can get here EITHER because a client retried, Note that we can get here EITHER because a client retried,
or an upstream server returned REFUSED. The above only or an upstream server returned REFUSED. The above only
applied in the later case. For client retries, applied in the later case. For client retries,
keep tyring the last server.. */ keep trying the last server.. */
if (++start == last) if (++start == last)
{ {
if (old_reply) if (old_reply)
@@ -402,9 +409,6 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
forward->flags |= FREC_TEST_PKTSZ; forward->flags |= FREC_TEST_PKTSZ;
} }
/* If a query is retried, use the log_id for the retry when logging the answer. */
forward->frec_src.log_id = daemon->log_id;
/* We may be resending a DNSSEC query here, for which the below processing is not necessary. */ /* We may be resending a DNSSEC query here, for which the below processing is not necessary. */
if (!is_dnssec) if (!is_dnssec)
{ {