From a67a6df3423001cec449d57fe453a815b9257cf7 Mon Sep 17 00:00:00 2001 From: Felix Kaechele Date: Thu, 17 Oct 2024 00:32:49 -0400 Subject: [PATCH] Fix external block detection on NXDOMAIN replies from authoritative upstreams Only consider NXDOMAIN replies as indication of external blocking if both the RA and AA bit are unset. Signed-off-by: Felix Kaechele --- src/dnsmasq/forward.c | 6 +++--- src/dnsmasq_interface.c | 17 +++++++++-------- src/dnsmasq_interface.h | 4 ++-- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index c2824d855..b1678152f 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -783,7 +783,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server } } - FTL_header_analysis(header->hb4, server, daemon->log_display_id); + FTL_header_analysis(header, server, daemon->log_display_id); /* RFC 4035 sect 4.6 para 3 */ if (!is_sign && !option_bool(OPT_DNSSEC_PROXY)) @@ -1213,7 +1213,7 @@ void reply_query(int fd, time_t now) server = daemon->serverarray[c]; - FTL_header_analysis(header->hb4, server, daemon->log_display_id); + FTL_header_analysis(header, server, daemon->log_display_id); if (RCODE(header) != REFUSED) daemon->serverarray[first]->last_server = c; @@ -2173,7 +2173,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si unsigned char *packet = NULL; struct dns_header *new_header = NULL; - FTL_header_analysis(header->hb4, server, daemon->log_display_id); + FTL_header_analysis(header, server, daemon->log_display_id); while (1) { diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 9eec8cdaa..cecd41e1c 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -82,7 +82,7 @@ static char *get_ptrname(struct in_addr *addr); static const char *check_dnsmasq_name(const char *name); // Static blocking metadata -static bool adbit = false, rabit = false; +static bool aabit = false, adbit = false, rabit = false; static const char *blockingreason = ""; static enum reply_type force_next_DNS_reply = REPLY_UNKNOWN; static enum query_status cacheStatus = QUERY_UNKNOWN; @@ -2876,7 +2876,7 @@ int _FTL_check_reply(const unsigned int rcode, const unsigned short flags, const int id, const char* file, const int line) { ednsData *edns = getEDNS(); - // Check if RA bit is unset in DNS header and rcode is NXDOMAIN + // Check if RA and AA bits are unset in DNS header and rcode is NXDOMAIN // If the response code (rcode) is NXDOMAIN, we may be seeing a response from // an externally blocked query. As they are not always accompany a necessary // SOA record, they are not getting added to our cache and, therefore, @@ -2886,8 +2886,8 @@ int _FTL_check_reply(const unsigned int rcode, const unsigned short flags, // Alternatively, we also consider EDE15 as a blocking reason. if(addr == NULL) { - // RA bit is not set and rcode is NXDOMAIN - if(!rabit && rcode == NXDOMAIN) + // RA and AA bits are not set and rcode is NXDOMAIN + if(!rabit && !aabit && rcode == NXDOMAIN) { FTL_blocked_upstream_by_header(QUERY_EXTERNAL_BLOCKED_NXRA, id, file, line); @@ -2923,17 +2923,18 @@ int _FTL_check_reply(const unsigned int rcode, const unsigned short flags, return 0; } -void _FTL_header_analysis(const unsigned char header4, const struct server *server, +void _FTL_header_analysis(struct dns_header *header, const struct server *server, const int id, const char* file, const int line) { // Analyze DNS header bits // Check if AD bit is set in DNS header - adbit = header4 & HB4_AD; + adbit = header->hb4 & HB4_AD; - // Check if RA bit is set in DNS header. We do it here as it is it is + // Check if RA and AA bit is set in DNS header. We do it here as it is it is // forced by dnsmasq shortly after calling FTL_header_analysis() - rabit = header4 & HB4_RA; + rabit = header->hb4 & HB4_RA; + aabit = header->hb3 & HB3_AA; // Store server which sent this reply (if applicable) if(server) diff --git a/src/dnsmasq_interface.h b/src/dnsmasq_interface.h index 4004183f8..b6c043211 100644 --- a/src/dnsmasq_interface.h +++ b/src/dnsmasq_interface.h @@ -27,8 +27,8 @@ void _FTL_iface(struct irec *recviface, const union all_addr *addr, const sa_fam #define FTL_new_query(flags, name, addr, arg, qtype, id, proto) _FTL_new_query(flags, name, addr, arg, qtype, id, proto, __FILE__, __LINE__) bool _FTL_new_query(const unsigned int flags, const char *name, union mysockaddr *addr, char *arg, const unsigned short qtype, const int id, enum protocol proto, const char* file, const int line); -#define FTL_header_analysis(header4, server, id) _FTL_header_analysis(header4, server, id, __FILE__, __LINE__) -void _FTL_header_analysis(const unsigned char header4, const struct server *server, const int id, const char* file, const int line); +#define FTL_header_analysis(header, server, id) _FTL_header_analysis(header, server, id, __FILE__, __LINE__) +void _FTL_header_analysis(struct dns_header *header, const struct server *server, const int id, const char* file, const int line); #define FTL_check_reply(rcode, flags, addr, id) _FTL_check_reply(rcode, flags, addr, id, __FILE__, __LINE__) int _FTL_check_reply(const unsigned int rcode, const unsigned short flags, const union all_addr *addr, const int id, const char* file, const int line);