Overhaul extract_addresses() function.

The proximate cause for doing this is to fix a bug that causes replies to PTR queries with more than one answer to have the second and subsequent answers ignored. The fix turned into a small re-write which removed a very old hack. When caching reponses which include CNAME records, the cache system stores the CNAME with a link to the record representing the target of the CNAME. This isn't possible for PTR records representing IP addresses since the name stored is the target of the PTR, record and its name is inferred from the address in the cache record. Such cache records have the F_REVERSE flag set. To get around this, long ago, the code which stores such records elided the CNAME entirely, so 4.3.2.1.in-addr.arpa CNAME 18/3.2.1.in-addr.arpa 18/3.2.1.in-addr.arpa PTR myhost.example.com would be stored as 4.3.2.1.in-addr.arpa PTR myhost.example.com and returned from the cache to subsequent requestor in that form. Since that hack was committed, dnsmasq has learned to cache arbitrary RRs. So now we can store the PTR records for all the no-trivial cases. The means the CNAME chains ending in PTR records don't get mangled, and we can store PTR records whose name in not w.x.y.x.in-addr.arpa or the IPv6 equivalent.
2025-12-19 10:18:25 +00:00 · 2025-05-18 17:24:41 +01:00
parent 91b800cc62
commit e48a2af4f5
4 changed files with 82 additions and 141 deletions
--- a/3
+++ b/3
@@ -56,6 +56,9 @@ version 2.92
 	Many thanks to JAXPORT, Jacksonville Port Authority for sponsoring
 	this enhancement to dnsmasq.

+	Fix failure to cache PTR RRs when a reply contains more than one answer.
+	Thanks to Dmitry for spotting this.
+
 	
 version 2.91
 	Fix spurious "resource limit exceeded messages". Thanks to 
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1413,7 +1413,7 @@ unsigned int extract_request(struct dns_header *header, size_t qlen, char *name,
 			     unsigned short *typep, unsigned short *classp);
 void setup_reply(struct dns_header *header, unsigned int flags, int ede);
 int extract_addresses(struct dns_header *header, size_t qlen, char *name,
-		      time_t now, struct ipsets *ipsets, struct ipsets *nftsets, int is_sign,
+		      time_t now, struct ipsets *ipsets, struct ipsets *nftsets,
                      int check_rebind, int no_cache_dnssec, int secure);
 #if defined(HAVE_CONNTRACK) && defined(HAVE_UBUS)
 void report_addresses(struct dns_header *header, size_t len, u32 mark);
--- a/src/forward.c
+++ b/src/forward.c
@@ -819,7 +819,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
 	}
      else
 	{
-	  int rc = extract_addresses(header, n, daemon->namebuff, now, ipsets, nftsets, is_sign, check_rebind, no_cache, cache_secure);
+	  int rc = extract_addresses(header, n, daemon->namebuff, now, ipsets, nftsets, check_rebind, no_cache, cache_secure);

 	  if (rc != 0)
 	    {
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -687,12 +687,12 @@ static int log_txt(char *name, unsigned char *p, const int ardlen, int flag)
   Return 2 if the packet is malformed.
 */
 int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t now, 
-		      struct ipsets *ipsets, struct ipsets *nftsets, int is_sign, int check_rebind,
+		      struct ipsets *ipsets, struct ipsets *nftsets, int check_rebind,
 		      int no_cache_dnssec, int secure)
 {
  unsigned char *p, *p1, *endrr, *namep;
  int j, qtype, qclass, aqtype, aqclass, ardlen, res;
-  unsigned long ttl = 0;
+  unsigned long ttl;
  union all_addr addr;
 #ifdef HAVE_IPSET
  char **ipsets_cur;
@@ -704,13 +704,8 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 #else
  (void)nftsets; /* unused */
 #endif
-  int found = 0, cname_count = CNAME_CHAIN;
-  struct crec *cpp = NULL;
+  int name_encoding, found = 0, ptr = 0;
  int flags = RCODE(header) == NXDOMAIN ? F_NXDOMAIN : 0;
-#ifdef HAVE_DNSSEC
-  int cname_short = 0;
-#endif
-  unsigned long cttl = ULONG_MAX, attl;

  cache_start_insert();

@@ -725,15 +720,14 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
  if (qclass != C_IN)
    return 0;

-  /* PTRs: we chase CNAMEs here, since we have no way to 
-     represent them in the cache. */
-  if (qtype == T_PTR)
+  /* If the PTR record encodes an address, store using a name/address record with F_REVERSE set.
+     Otherwise, it gets stored as an arbitrary RR below. If the query is answerable with
+     a CNAME, also take the arbitrary-RR route, since the cache can't represent a CNAME
+     whose target is stored in a F_REVERSE record. */
+  if (qtype == T_PTR && !(flags & F_NXDOMAIN) && (name_encoding = in_arpa_name_2_addr(name, &addr)))
    { 
-      int insert = 1, name_encoding = in_arpa_name_2_addr(name, &addr);
+      ptr = 1;
      
-      if (!(flags & F_NXDOMAIN))
-	{
-	cname_loop:
      if (!(p1 = skip_questions(header, qlen)))
 	return 2;
      
@@ -745,62 +739,39 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	  
 	  GETSHORT(aqtype, p1); 
 	  GETSHORT(aqclass, p1);
-	      GETLONG(attl, p1);
-	      
-	      if ((daemon->max_ttl != 0) && (attl > daemon->max_ttl) && !is_sign)
-		{
-		  (p1) -= 4;
-		  PUTLONG(daemon->max_ttl, p1);
-		}
+	  p = p1;
+	  GETLONG(ttl, p1);
 	  GETSHORT(ardlen, p1);
 	  endrr = p1+ardlen;
 	  
-	      /* TTL of record is minimum of CNAMES and PTR */
-	      if (attl < cttl)
-		cttl = attl;
-	      
-	      if (aqclass == C_IN && res != 2 && (aqtype == T_CNAME || aqtype == T_PTR))
+	  if (aqclass == C_IN && res == 1 && aqtype == T_PTR)
 	    {
+	      found = 1;
+
+	      if ((daemon->max_ttl != 0) && (ttl > daemon->max_ttl))
+		ttl = daemon->max_ttl;
+	      
 #ifdef HAVE_DNSSEC
 	      if (option_bool(OPT_DNSSEC_VALID) && j < daemon->rr_status_sz && daemon->rr_status[j] != 0)
 		{
-		      /* validated RR anywhere in CNAME chain, don't cache. */
-		      if (cname_short || aqtype == T_CNAME)
-			insert = 0;
-		      
 		  secflag = F_DNSSECOK;
 		  /* limit TTL based on signature. */
-		      if (daemon->rr_status[j] < cttl)
-			cttl = daemon->rr_status[j];
+		  if (daemon->rr_status[j] < ttl)
+		    ttl = daemon->rr_status[j];
 		}
 #endif
 	      
-		  if (aqtype == T_CNAME)
-		    log_query(secflag | F_CNAME | F_FORWARD | F_UPSTREAM, name, NULL, NULL, 0);
+	      PUTLONG(ttl, p);

 	      if (!extract_name(header, qlen, &p1, name, EXTR_NAME_EXTRACT, 0))
 		return 2;
-		  
-		  if (aqtype == T_CNAME)
-		    {
-		      if (!cname_count--)
-			return 0; /* looped CNAMES, we can't cache. */
-#ifdef HAVE_DNSSEC
-		      cname_short = 1;
-#endif
-		      goto cname_loop;
-		    }
-		  
-		  found = 1; 
-		  
-		  if (!name_encoding)
-		    log_query(secflag | F_FORWARD | F_UPSTREAM, name, NULL, NULL, aqtype);
-		  else
-		    {
 	      log_query(name_encoding | secflag | F_REVERSE | F_UPSTREAM, name, &addr, NULL, 0);
-		      if (insert)
-			cache_insert(name, &addr, C_IN, now, cttl, name_encoding | secflag | F_REVERSE);
-		    }
+	      cache_insert(name, &addr, C_IN, now, ttl, name_encoding | secflag | F_REVERSE);
+	      
+	      /* restore query into name */
+	      p1 = namep;
+	      if (!extract_name(header, qlen, &p1, name, EXTR_NAME_EXTRACT, 0))
+		return 2;
 	    }
 	  
 	  p1 = endrr;
@@ -809,39 +780,11 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	}
    }

-      if (!found)
-	{
-	  flags |= F_NEG | (secure ?  F_DNSSECOK : 0);
-
-	  if (name_encoding)
-	    flags |= F_REVERSE | name_encoding;
-
-	  log_query(flags | F_UPSTREAM, name, &addr, NULL, 0);
-
-	  if (name_encoding && !option_bool(OPT_NO_NEG))
-	    {
-	      /* For reverse records, we use the name field to store the SOA name. */
-	      int substring, have_soa = find_soa(header, qlen, name, &substring, &ttl, no_cache_dnssec, now);
-
-	      if (have_soa || daemon->neg_ttl)
-		{
-		  /* If daemon->neg_ttl is set, we can cache even without an SOA. */
-		  if (!have_soa)
-		    {
-		      flags |= F_NO_RR; /* Marks no SOA found. */
-		      ttl = daemon->neg_ttl;
-		    }
-		  
-		  cache_insert(name + substring, &addr, C_IN, now, ttl, flags);
-		}
-	    }
-	}
-    }
-  else
+  if (!ptr || !found)
    {
      /* everything other than PTR */
-      struct crec *newc;
-      int addrlen = 0, insert = 1;
+      struct crec *newc, *cpp = NULL;
+      int cname_count = CNAME_CHAIN, addrlen = 0, insert = 1;
            
      if (qtype == T_A)
 	{
@@ -854,12 +797,12 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	  flags |= F_IPV6;
 	}
      else if (qtype != T_CNAME &&
-	       (qtype == T_SRV || rr_on_list(daemon->cache_rr, qtype) || rr_on_list(daemon->cache_rr, T_ANY)))
+	       (qtype == T_SRV || qtype == T_PTR || rr_on_list(daemon->cache_rr, qtype) || rr_on_list(daemon->cache_rr, T_ANY)))
 	flags |= F_RR;
      else
 	insert = 0; /* NOTE: do not cache data from CNAME queries. */
      
-    cname_loop1:
+    cname_loop:
      if (!(p1 = skip_questions(header, qlen)))
 	return 2;
      
@@ -872,12 +815,8 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	  
 	  GETSHORT(aqtype, p1); 
 	  GETSHORT(aqclass, p1);
-	  GETLONG(attl, p1);
-	  if ((daemon->max_ttl != 0) && (attl > daemon->max_ttl) && !is_sign)
-	    {
-	      (p1) -= 4;
-	      PUTLONG(daemon->max_ttl, p1);
-	    }
+	  p = p1;
+	  GETLONG(ttl, p1);
 	  GETSHORT(ardlen, p1);
 	  endrr = p1+ardlen;

@@ -891,17 +830,22 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	      continue;
 	    }

+	  if ((daemon->max_ttl != 0) && (ttl > daemon->max_ttl))
+	    ttl = daemon->max_ttl;
+	  
 #ifdef HAVE_DNSSEC
 	  if (option_bool(OPT_DNSSEC_VALID) && j < daemon->rr_status_sz && daemon->rr_status[j] != 0)
 	    {
 	      secflag = F_DNSSECOK;
 	      
 	      /* limit TTl based on sig. */
-	      if (daemon->rr_status[j] < attl)
-		attl = daemon->rr_status[j];
+	      if (daemon->rr_status[j] < ttl)
+		ttl = daemon->rr_status[j];
 	    }
 #endif	  

+	  PUTLONG(ttl, p);
+	  
 	  if (aqtype == T_CNAME)
 	    {
 	      if (!cname_count--)
@@ -911,7 +855,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	      
 	      if (insert)
 		{
-		  if ((newc = cache_insert(name, NULL, C_IN, now, attl, F_CNAME | F_FORWARD | secflag)))
+		  if ((newc = cache_insert(name, NULL, C_IN, now, ttl, F_CNAME | F_FORWARD | secflag)))
 		    {
 		      newc->addr.cname.target.cache = NULL;
 		      newc->addr.cname.is_name_ptr = 0; 
@@ -924,16 +868,15 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 		    }
 		  
 		  cpp = newc;
-		  if (attl < cttl)
-		    cttl = attl;
 		}
 	      
+	      /* Set the query to the CNAME target and go again unless the query was just for a CNAME. */
 	      namep = p1;
 	      if (!extract_name(header, qlen, &p1, name, EXTR_NAME_EXTRACT, 0))
 		return 2;
 	      
 	      if (qtype != T_CNAME)
-		goto cname_loop1;
+		goto cname_loop;

 	      found = 1;
 	    }
@@ -1089,7 +1032,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	      
 	      if (insert)
 		{
-		  newc = cache_insert(name, &addr, C_IN, now, attl, flags | F_FORWARD | secflag);
+		  newc = cache_insert(name, &addr, C_IN, now, ttl, flags | F_FORWARD | secflag);
 		  if (newc && cpp)
 		    {
 		      next_uid(newc);
@@ -1668,7 +1611,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
  int rd_bit = (header->hb3 & HB3_RD);
  int count = 255; /* catch loops */

-  /* Suppress cached answers of no_cache set. */
+  /* Suppress cached answers if no_cache set. */
  if (no_cache)
    rd_bit = 0;
  
@@ -1678,11 +1621,9 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
  if (filtered)
    *filtered = 0;
  
-  /* never answer queries with RD unset, to avoid cache snooping. */
  if (ntohs(header->qdcount) != 1 ||
      ntohs(header->ancount) != 0 ||
      ntohs(header->nscount) != 0 ||
-      ntohs(header->qdcount) == 0 ||
      OPCODE(header) != QUERY )
    return 0;
  
@@ -2348,14 +2289,11 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
     
     For FORWARD NEG records, the addr.rrdata.datalen field of the othewise
     empty addr is used to held an offset in to the name which yields the SOA
-     name. For REVERSE NEG records, the otherwise empty name field holds the
-     SOA name. If soa_name has zero length, then no SOA is known. soa_lookup
-     MUST be a neg record here.
-     
+     name.
     If the F_NO_RR flag is set, there was no SOA record supplied with the RR.  */
  if (soa_lookup && !(soa_lookup->flags & F_NO_RR))
    {
-      char *soa_name = soa_lookup->flags & F_REVERSE ? cache_get_name(soa_lookup) : name + soa_lookup->addr.rrdata.datalen;
+      char *soa_name = name + soa_lookup->addr.rrdata.datalen;
      
      crecp = NULL;
      while ((crecp = cache_find_by_name(crecp, soa_name, now, F_RR)))