[Dnsmasq-discuss] [PATCH] Regression: dnsmasq replies to forwarded query too early when receiving REFUSED response from upstream server

[Chris Novakovic]

When a query is forwarded to multiple upstream servers, a REFUSED
response from an upstream server triggers an immediate REFUSED response
to the original query from dnsmasq: responses from all other servers are
silently discarded, ignoring the possibility that one of the other
upstream servers might answer the query. This appears to be the case in
both the latest stable dnsmasq version (2.75) and the head of the master
branch in git.

Commit 51967f9807665dae403f1497b827165c5fa1084b (from March 2014)
appears to contain a typo that causes this behaviour: "RCODE(header) !=
REFUSED" was removed from the conditional, instead of "RCODE(header) !=
SERVFAIL". I've attached a patch against git master that fixes the
(presumed) typo and provides the behaviour I'd expect.

Cheers,
Chris
-------------- next part --------------
From 42804dafeadc2a16357f5683c7a1b8111f979241 Mon Sep 17 00:00:00 2001
From: Chris Novakovic <chris at chrisn.me.uk>
Date: Mon, 25 Jan 2016 02:22:12 +0000
Subject: Treat REFUSED (not SERVFAIL) as an unsuccessful upstream response

Commit 51967f9807665dae403f1497b827165c5fa1084b began treating SERVFAIL
as a successful response from an upstream server (thus ignoring future
responses to the query from other upstream servers), but a typo in that
commit means that REFUSED responses are accidentally being treated as
successful instead of SERVFAIL responses.

This commit corrects this typo and provides the behaviour intended by
commit 51967f9: SERVFAIL responses are considered successful (and will
be sent back to the requester), while REFUSED responses are considered
unsuccessful (and dnsmasq will wait for responses from other upstream
servers that haven't responded yet).
---
 src/forward.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/forward.c b/src/forward.c
index 414f988..9b464d3 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -853,7 +853,7 @@ void reply_query(int fd, int family, time_t now)
      we get a good reply from another server. Kill it when we've
      had replies from all to avoid filling the forwarding table when
      everything is broken */
-  if (forward->forwardall == 0 || --forward->forwardall == 1 || RCODE(header) != SERVFAIL)
+  if (forward->forwardall == 0 || --forward->forwardall == 1 || RCODE(header) != REFUSED)
     {
       int check_rebind = 0, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0;
 
-- 
1.8.4