Fix external block detection on NXDOMAIN replies from authoritative u…

…pstreams Only consider NXDOMAIN replies as indication of external blocking if both the RA and AA bit are unset. Signed-off-by: Felix Kaechele <[email protected]>
pi-hole · Oct 17, 2024 · a67a6df · a67a6df
1 parent 3c7b356
commit a67a6df
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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);