Fix issue with stale caching.

After replying with stale data, dnsmasq sends the query upstream to
refresh the cache asynchronously and sometimes sends the wrong packet:
packet length can be wrong, and if an EDE marking stale data is added
to the answer that can end up in the query also. This bug only seems
to cause problems when the usptream server is a DOH/DOT proxy. Thanks
to Justin He for the bug report.

CHANGELOG

@@ -18,6 +18,14 @@ version 2.90
 	Add --no-dhcpv4-interface and --no-dhcpv6-interface for
 	better control over which inetrfaces are providing DHCP service.
 
+	Fix issue with stale caching: After replying with stale data,
+	dnsmasq sends the query upstream to refresh the cache asynchronously
+	and sometimes sends the wrong packet: packet length can be wrong,
+	and if an EDE marking stale data is added to the answer that can
+	end up in the query also. This bug only seems to cause problems
+	when the usptream server is a DOH/DOT proxy. Thanks to Justin He
+	for the bug report.
+	
 	
 version 2.89
         Fix bug introduced in 2.88 (commit fe91134b) which can result

src/forward.c

@@ -1803,8 +1803,12 @@ void receive_query(struct listener *listen, time_t now)
     {
       int stale, filtered;
       int ad_reqd = do_bit;
-      u16 hb3 = header->hb3, hb4 = header->hb4;
       int fd = listen->fd;
+      struct blockdata *saved_question = NULL;
+      
+      /* In case answer is stale */
+      if (daemon->cache_max_expiry != 0)
+	saved_question = blockdata_alloc((char *) header, (size_t)n);
       
       /* RFC 6840 5.7 */
       if (header->hb4 & HB4_AD)
@@ -1847,29 +1851,22 @@ void receive_query(struct listener *listen, time_t now)
 	    daemon->metrics[METRIC_DNS_STALE_ANSWERED]++;
 	}
       
-      if (m == 0 || stale)
+      if (stale && saved_question)
 	{
-	  if (m != 0)
+	  /* We answered with stale cache data, so forward the query anyway to
-	    {
+	     refresh that. Restore saved query. */
-	      size_t plen;
+	  blockdata_retrieve(saved_question, (size_t)n, header);
+	  m = 0;
 	  
-	      /* We answered with stale cache data, so forward the query anyway to
+	  /* We've already answered the client, so don't send it the answer 
-		 refresh that. Restore the query from the answer packet. */
+	     when it comes back. */
-	      pheader = find_pseudoheader(header, (size_t)m, &plen, NULL, NULL, NULL);
+	  fd = -1;
+	}
       
-	      header->hb3 = hb3;
+      blockdata_free(saved_question);
-	      header->hb4 = hb4;
-	      header->ancount = htons(0);
-	      header->nscount = htons(0);
-	      header->arcount = htons(0);
-
-	      m = resize_packet(header, m, pheader, plen);
-
-	      /* We've already answered the client, so don't send it the answer 
-		 when it comes back. */
-	      fd = -1;
-	    }
 
+      if (m == 0)
+	{
 	  if (forward_query(fd, &source_addr, &dst_addr, if_index,
 			    header, (size_t)n,  ((char *) header) + udp_size, now, NULL, ad_reqd, do_bit, 0))
 	    daemon->metrics[METRIC_DNS_QUERIES_FORWARDED]++;
@@ -2074,7 +2071,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si
 unsigned char *tcp_request(int confd, time_t now,
 			   union mysockaddr *local_addr, struct in_addr netmask, int auth_dns)
 {
-  size_t size = 0;
+  size_t size = 0, saved_size = 0;
   int norebind;
 #ifdef HAVE_CONNTRACK
   int is_single_query = 0, allowed = 1;
@@ -2085,6 +2082,7 @@ unsigned char *tcp_request(int confd, time_t now,
   int checking_disabled, do_bit, added_pheader = 0, have_pseudoheader = 0;
   int cacheable, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0;
   size_t m;
+  struct blockdata *saved_question = NULL;
   unsigned short qtype;
   unsigned int gotname;
   /* Max TCP packet + slop + size */
@@ -2104,7 +2102,6 @@ unsigned char *tcp_request(int confd, time_t now,
   int have_mark = 0;
   int first, last, filtered, stale, do_stale = 0;
   unsigned int flags = 0;
-  u16 hb3, hb4;
     
   if (!packet || getpeername(confd, (struct sockaddr *)&peer_addr, &peer_len) == -1)
     return packet;
@@ -2159,35 +2156,25 @@ unsigned char *tcp_request(int confd, time_t now,
     {
       int ede = EDE_UNSET;
 
-      if (query_count == TCP_MAX_QUERIES)
-	return packet;
-
       if (do_stale)
 	{
-	  size_t plen;
-
 	  /* We answered the last query with stale data. Now try and get fresh data.
-	     Restore query from answer. */
+	     Restore saved query */
-	  pheader = find_pseudoheader(header, m, &plen, NULL, NULL, NULL);
+	  if (!saved_question)
+	    break;
 
-	  header->hb3 = hb3;
+	  blockdata_retrieve(saved_question, (size_t)saved_size, header);
-	  header->hb4 = hb4;
+	  size = saved_size;
-	  header->ancount = htons(0);
-	  header->nscount = htons(0);
-	  header->arcount = htons(0);
-	  
-	  size = resize_packet(header, m, pheader, plen);
 	}
       else
 	{
+	  if (query_count == TCP_MAX_QUERIES)
+	    return packet;
+
 	  if (!read_write(confd, &c1, 1, 1) || !read_write(confd, &c2, 1, 1) ||
 	      !(size = c1 << 8 | c2) ||
 	      !read_write(confd, payload, size, 1))
 	    return packet;
-	  
-	  /* for stale-answer processing. */
-	  hb3 = header->hb3;
-	  hb4 = header->hb4;
 	}
       
       if (size < (int)sizeof(struct dns_header))
@@ -2294,10 +2281,20 @@ unsigned char *tcp_request(int confd, time_t now,
 	   if (do_stale)
 	     m = 0;
 	   else
-	     /* m > 0 if answered from cache */
+	     {
-	     m = answer_request(header, ((char *) header) + 65536, (size_t)size, 
+	       if (daemon->cache_max_expiry != 0)
-				dst_addr_4, netmask, now, ad_reqd, do_bit, have_pseudoheader, &stale, &filtered);
+		 {
+		   if (saved_question)
+		     blockdata_free(saved_question);
 		   
+		   saved_question = blockdata_alloc((char *) header, (size_t)size);
+		   saved_size = size;
+		 }
+	       
+	       /* m > 0 if answered from cache */
+	       m = answer_request(header, ((char *) header) + 65536, (size_t)size, 
+				  dst_addr_4, netmask, now, ad_reqd, do_bit, have_pseudoheader, &stale, &filtered);
+	     }
 	  /* Do this by steam now we're not in the select() loop */
 	  check_log_writer(1); 
 	  
@@ -2435,7 +2432,7 @@ unsigned char *tcp_request(int confd, time_t now,
 		m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, 0, NULL, 0, do_bit, 0);
 	    }
 	}
-      else
+      else if (have_pseudoheader)
 	{
 	  ede = EDE_UNSET;
 	  
@@ -2485,6 +2482,9 @@ unsigned char *tcp_request(int confd, time_t now,
       close(confd);
     }
 
+  if (saved_question)
+    blockdata_free(saved_question);
+  
   return packet;
 }