Fix log message fields in wrong order in some auth replies.

Let's give the poor programmers a chance.

CHANGELOG

@@ -82,6 +82,21 @@ version 2.91
 	to queries which arrive a dnsmasq already carrying an EDNS client
 	subnet.
 
+	Handle DS queries to auth zones. When dnsmasq is configured to
+	act as an authoritative server and has an authoritative zone
+	configured, and recieves a query for that zone _as_forwarder_
+	it answers the query directly rather than forwarding it. This
+	doesn't affect the answer, but it saves dnsmasq forwarding the
+	query to the recusor upstream, whch then bounces it back to dnsmasq
+	in auth mode. The exception should be when the query is for the root
+	of zone, for a DS RR. The answer to that has to come from the parent,
+	via the recursor, and will typically be a proof-of-nonexistence
+	since dnsmasq doesn't support signed zones. This patch suppresses
+	local answers and forces forwarding to the upstream recursor for such
+	queries. It stops breakage when a DNSSEC validating client makes
+	queries to dnsmasq acting as forwarder for a zone for which it is
+	authoritative.
+
 	
 version 2.90
 	Fix reversion in --rev-server introduced in 2.88 which

Makefile

@@ -71,8 +71,8 @@ nft_cflags =    `echo $(COPTS) | $(top)/bld/pkg-wrapper HAVE_NFTSET $(PKG_CONFIG
 nft_libs =      `echo $(COPTS) | $(top)/bld/pkg-wrapper HAVE_NFTSET $(PKG_CONFIG) --libs libnftables`
 version =       -DVERSION='\"`$(top)/bld/get-version $(top)`\"'
 
-sum?=$(shell $(CC) -DDNSMASQ_COMPILE_OPTS $(COPTS) -E $(top)/$(SRC)/dnsmasq.h | ( md5sum 2>/dev/null || md5 ) | cut -f 1 -d ' ')
+sum?=$(shell echo $(CC) -DDNSMASQ_COMPILE_FLAGS="$(CFLAGS)" -DDNSMASQ_COMPILE_OPTS $(COPTS) -E $(top)/$(SRC)/dnsmasq.h | ( md5sum 2>/dev/null || md5 ) | cut -f 1 -d ' ')
-sum!=$(CC) -DDNSMASQ_COMPILE_OPTS $(COPTS) -E $(top)/$(SRC)/dnsmasq.h | ( md5sum 2>/dev/null || md5 ) | cut -f 1 -d ' '
+sum!=echo $(CC) -DDNSMASQ_COMPILE_FLAGS="$(CFLAGS)" -DDNSMASQ_COMPILE_OPTS $(COPTS) -E $(top)/$(SRC)/dnsmasq.h | ( md5sum 2>/dev/null || md5 ) | cut -f 1 -d ' '
 copts_conf = .copts_$(sum)
 
 objs = cache.o rfc1035.o util.o option.o forward.o network.o \

src/auth.c

@@ -513,7 +513,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 		 nxdomain = 0;
 		 if ((crecp->flags & flag) && (local_query || filter_zone(zone, flag, &(crecp->addr))))
 		   {
-		     log_query(crecp->flags, name, &crecp->addr, record_source(crecp->uid), 0);
+		     log_query(crecp->flags & ~F_REVERSE, name, &crecp->addr, record_source(crecp->uid), 0);
 		     if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, 
 					     daemon->auth_ttl, NULL, qtype, C_IN, 
 					     qtype == T_A ? "4" : "6", &crecp->addr))

src/config.h

@@ -362,6 +362,11 @@ HAVE_SOCKADDR_SA_LEN
 #define HAVE_INOTIFY
 #endif
 
+/* This never compiles code, it's only used by the makefile to fingerprint builds. */
+#ifdef DNSMASQ_COMPILE_FLAGS
+static char *compile_flags = DNSMASQ_COMPILE_FLAGS;
+#endif
+
 /* Define a string indicating which options are in use.
    DNSMASQ_COMPILE_OPTS is only defined in dnsmasq.c */
 

src/forward.c

@@ -822,10 +822,6 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header,
 
   daemon->log_display_id = forward->frec_src.log_id;
     
-  /* We've had a reply already, which we're validating. Ignore this duplicate */
-  if (forward->blocking_query || (forward->flags & FREC_GONE_TO_TCP))
-    return;
-
   /* Find the original query that started it all.... */
   for (orig = forward; orig->dependent; orig = orig->dependent);
   
@@ -1153,17 +1149,19 @@ void reply_query(int fd, time_t now)
       check_for_ignored_address(header, n))
     return;
 
-  if ((RCODE(header) == REFUSED || RCODE(header) == SERVFAIL) && forward->forwardall == 0)
-    /* for broken servers, attempt to send to another one. */
-    {
 #ifdef HAVE_DNSSEC
       /* The query MAY have got a good answer, and be awaiting
 	 the results of further queries, in which case
-	 The Stash contains something else and we don't need to retry anyway. */
+	 the stash contains something else and we don't need to retry anyway.
+	 We may also have already got a truncated reply, and be in the process
+	 of doing the query by TCP so can ignore further, probably truncated, UDP answers. */
       if (forward->blocking_query || (forward->flags & FREC_GONE_TO_TCP))
 	return;
 #endif
       
+  if ((RCODE(header) == REFUSED || RCODE(header) == SERVFAIL) && forward->forwardall == 0)
+    /* for broken servers, attempt to send to another one. */
+    {
       /* Get the saved query back. */
       blockdata_retrieve(forward->stash, forward->stash_len, (void *)header);
       
@@ -1693,15 +1691,27 @@ void receive_query(struct listener *listen, time_t now)
 			   &source_addr, auth_dns ? "auth" : "query", type);
       
 #ifdef HAVE_AUTH
-      /* find queries for zones we're authoritative for, and answer them directly */
+      /* Find queries for zones we're authoritative for, and answer them directly.
+	 The exception to this is DS queries for the zone route. They
+	 have to come from the parent zone. Since dnsmasq's auth server
+	 can't do DNSSEC, the zone will be unsigned, and anything using
+	 dnsmasq as a forwarder and doing validation will be expecting to
+	 see the proof of non-existence from the parent. */
       if (!auth_dns && !option_bool(OPT_LOCALISE))
 	for (zone = daemon->auth_zones; zone; zone = zone->next)
-	  if (in_zone(zone, daemon->namebuff, NULL))
+	  {
+	    char *cut;
+	    
+	    if (in_zone(zone, daemon->namebuff, &cut))
+	      {
+		if (type != T_DS || cut)
 		  {
 		    auth_dns = 1;
 		    local_auth = 1;
+		  }
 		break;
 	      }
+	  }
 #endif
       
 #ifdef HAVE_LOOP
@@ -2327,15 +2337,27 @@ unsigned char *tcp_request(int confd, time_t now,
 				   &peer_addr, auth_dns ? "auth" : "query", qtype);
 	      
 #ifdef HAVE_AUTH
-	      /* find queries for zones we're authoritative for, and answer them directly */
+	      /* Find queries for zones we're authoritative for, and answer them directly.
+		 The exception to this is DS queries for the zone route. They
+		 have to come from the parent zone. Since dnsmasq's auth server
+		 can't do DNSSEC, the zone will be unsigned, and anything using
+		 dnsmasq as a forwarder and doing validation will be expecting to
+		 see the proof of non-existence from the parent. */
 	      if (!auth_dns && !option_bool(OPT_LOCALISE))
 		for (zone = daemon->auth_zones; zone; zone = zone->next)
-		  if (in_zone(zone, daemon->namebuff, NULL))
+		  {
+		    char *cut;
+		    
+		    if (in_zone(zone, daemon->namebuff, &cut))
+		      {
+			if (qtype != T_DS || cut)
 			  {
 			    auth_dns = 1;
 			    local_auth = 1;
+			  }
 			break;
 		      }
+		  }
 #endif
 	      
 	      norebind = domain_no_rebind(daemon->namebuff);

src/rfc1035.c

@@ -1559,7 +1559,7 @@ static unsigned long crec_ttl(struct crec *crecp, time_t now)
     return daemon->max_ttl;
 }
 
-static int cache_validated(const struct crec *crecp)
+static int cache_not_validated(const struct crec *crecp)
 {
   return (option_bool(OPT_DNSSEC_VALID) && !(crecp->flags & F_DNSSECOK));
 }
@@ -1660,7 +1660,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
 	
 	/* If the client asked for DNSSEC  don't use cached data. */
 	if ((crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)) ||
-	    (rd_bit && (!do_bit || cache_validated(crecp))))
+	    (rd_bit && (!do_bit || cache_not_validated(crecp))))
 	  {
 	    if (crecp->flags & F_CONFIG || qtype == T_CNAME)
 	      ans = 1;
@@ -1819,7 +1819,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
 		 the zone is unsigned, which implies that we're doing
 		 validation. */
 	      if ((crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)) ||
-		  (rd_bit && (!do_bit || cache_validated(crecp)) ))
+		  (rd_bit && (!do_bit || cache_not_validated(crecp)) ))
 		{
 		  do 
 		    { 
@@ -1975,7 +1975,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
 	      
 	      /* If the client asked for DNSSEC  don't use cached data. */
 	      if ((crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)) ||
-		  (rd_bit && (!do_bit || cache_validated(crecp)) ))
+		  (rd_bit && (!do_bit || cache_not_validated(crecp)) ))
 		do
 		  { 
 		    int stale_flag = 0;
@@ -2157,8 +2157,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
       
       if (!ans)
 	{
-	  if ((crecp = cache_find_by_name(NULL, name, now, F_RR | F_NXDOMAIN)) &&
+	  if ((crecp = cache_find_by_name(NULL, name, now, F_RR | F_NXDOMAIN)) && rd_bit)
-	      rd_bit && (!do_bit || cache_validated(crecp)))
 	    do
 	      {
 		int flags = crecp->flags;
@@ -2169,7 +2168,8 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
 		else
 		  rrtype = crecp->addr.rrdata.rrtype;
 		
-		if ((flags & F_NXDOMAIN) || rrtype == qtype)
+		if (((flags & F_NXDOMAIN) || rrtype == qtype) &&
+		    (!do_bit || cache_not_validated(crecp)))
 		  {
 		    char *rrdata = NULL;
 		    unsigned short rrlen = 0;
@@ -2232,14 +2232,15 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
 	}
       
       
-      if (qtype != T_ANY && !ans && rr_on_list(daemon->filter_rr, qtype))
+      if (qtype != T_ANY && !ans && rr_on_list(daemon->filter_rr, qtype) && !do_bit)
 	{
 	  /* We don't have a cached answer and when we get an answer from upstream we're going to
 	     filter it anyway. If we have a cached answer for the domain for another RRtype then
 	     that may be enough to tell us if the answer should be NODATA and save the round trip.
 	     Cached NXDOMAIN has already been handled, so here we look for any record for the domain,
 	     since its existence allows us to return a NODATA answer. Note that we never set the AD flag,
-	     since we didn't authenticate the record. */
+	     since we didn't authenticate the record; this doesn't work if we want auth data, so
+	     don't use this shortcut in that case. */
 	  
 	  if (cache_find_by_name(NULL, name, now, F_IPV4 | F_IPV6 | F_RR | F_CNAME))
 	    {