From 122392e0b352507cabb9e982208d35d2e56902e0 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 31 Oct 2018 22:24:02 +0000 Subject: [PATCH] Revert 68f6312d4bae30b78daafcd6f51dc441b8685b1e The above is intended to increase robustness, but actually does the opposite. The problem is that by ignoring SERVFAIL messages and hoping for a better answer from another of the servers we've forwarded to, we become vulnerable in the case that one or more of the configured servers is down or not responding. Consider the case that a domain is indeed BOGUS, and we've send the query to n servers. With 68f6312d4bae30b78daafcd6f51dc441b8685b1e we ignore the first n-1 SERVFAIL replies, and only return the final n'th answer to the client. Now, if one of the servers we are forwarding to is down, then we won't get all n replies, and the client will never get an answer! This is a far more likely scenario than a temporary SERVFAIL from only one of a set of notionally identical servers, so, on the ground of robustness, we have to believe any SERVFAIL answers we get, and return them to the client. The client could be using the same recursive servers we are, so it should, in theory, retry on SERVFAIL anyway. --- src/forward.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/forward.c b/src/forward.c index 84bed20..df33b8e 100644 --- a/src/forward.c +++ b/src/forward.c @@ -957,8 +957,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) != REFUSED && 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;