Fix erroneous "DNSSEC validated" state with non-DNSSEC upstream servers.

When DNSEC validation is enabled, but a query is not validated
because it gets forwarded to a non-DNSEC-capable upstream
server, the rr_status array is not correctly cleared, with
the effect that the answer may be maked as DNSSEC validated
if the immediately preceding query was DNS signed and validated.

CHANGELOG

@@ -72,7 +72,10 @@ version 2.91
         a DHCP relay. This change allows dnsmasq to act as both a
         PXE proxy-DHCP server AND a DHCP relay for the same network.
 
-	
+	Fix erroneous "DNSSEC validated" state with non-DNSSEC
+	upstream servers.  Thanks to Dominik Derigs for the bug report.
+
+
 version 2.90
 	Fix reversion in --rev-server introduced in 2.88 which
 	caused breakage if the prefix length is not exactly divisible

src/forward.c

@@ -906,7 +906,7 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header,
 	  else
 	    status = dnssec_validate_reply(now, header, plen, daemon->namebuff, daemon->keyname, &forward->class, 
 					   !option_bool(OPT_DNSSEC_IGN_NS) && (forward->sentto->flags & SERV_DO_DNSSEC),
-				       NULL, NULL, NULL, &orig->validate_counter);
+					   NULL, NULL, NULL, &orig->validate_counter);
 	  
 	  if (STAT_ISEQUAL(status, STAT_ABANDONED))
 	    log_resource = 1;
@@ -1220,10 +1220,15 @@ void reply_query(int fd, time_t now)
   
   
 #ifdef HAVE_DNSSEC
-  if ((forward->sentto->flags & SERV_DO_DNSSEC) && 
+  if (option_bool(OPT_DNSSEC_VALID))
-      option_bool(OPT_DNSSEC_VALID) &&
+    {
-      !(forward->flags & FREC_CHECKING_DISABLED))
+      /* Clear this in case we don't call dnssec_validate() below */
-    dnssec_validate(forward, header, n, STAT_OK, now);
+      memset(daemon->rr_status, 0, sizeof(*daemon->rr_status) * daemon->rr_status_sz);
+      
+      if ((forward->sentto->flags & SERV_DO_DNSSEC) && 
+	  !(forward->flags & FREC_CHECKING_DISABLED))
+	dnssec_validate(forward, header, n, STAT_OK, now);
+    }
   else
 #endif
     return_reply(now, forward, header, n, STAT_OK); 
@@ -2491,46 +2496,52 @@ unsigned char *tcp_request(int confd, time_t now,
 		  log_query_mysockaddr(F_SERVER | F_FORWARD, daemon->namebuff, &serv->addr, NULL, 0);
 		  
 #ifdef HAVE_DNSSEC
-		  if (option_bool(OPT_DNSSEC_VALID) && !checking_disabled && (master->flags & SERV_DO_DNSSEC))
+		  if (option_bool(OPT_DNSSEC_VALID))
 		    {
-		      int keycount = daemon->limit[LIMIT_WORK]; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */
+		      /* Clear this in case we don't call tcp_key_recurse() below */
-		      int validatecount = daemon->limit[LIMIT_CRYPTO]; 
+		      memset(daemon->rr_status, 0, sizeof(*daemon->rr_status) * daemon->rr_status_sz);
-		      int status = tcp_key_recurse(now, STAT_OK, header, m, 0, daemon->namebuff, daemon->keyname, 
-						   serv, have_mark, mark, &keycount, &validatecount);
-		      char *result, *domain = "result";
 		      
-		      union all_addr a;
+		      if (!checking_disabled && (master->flags & SERV_DO_DNSSEC))
-		      a.log.ede = ede = errflags_to_ede(status);
-		      
-		      if (STAT_ISEQUAL(status, STAT_ABANDONED))
 			{
-			  result = "ABANDONED";
+			  int keycount = daemon->limit[LIMIT_WORK]; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */
-			  status = STAT_BOGUS;
+			  int validatecount = daemon->limit[LIMIT_CRYPTO]; 
-			}
+			  int status = tcp_key_recurse(now, STAT_OK, header, m, 0, daemon->namebuff, daemon->keyname, 
-		      else
+						       serv, have_mark, mark, &keycount, &validatecount);
-			result = (STAT_ISEQUAL(status, STAT_SECURE) ? "SECURE" : (STAT_ISEQUAL(status, STAT_INSECURE) ? "INSECURE" : "BOGUS"));
+			  char *result, *domain = "result";
-		      
-		      if (STAT_ISEQUAL(status, STAT_SECURE))
-			cache_secure = 1;
-		      else if (STAT_ISEQUAL(status, STAT_BOGUS))
-			{
-			  no_cache_dnssec = 1;
-			  bogusanswer = 1;
 			  
-			  if (extract_request(header, m, daemon->namebuff, NULL))
+			  union all_addr a;
-			    domain = daemon->namebuff;
+			  a.log.ede = ede = errflags_to_ede(status);
+			  
+			  if (STAT_ISEQUAL(status, STAT_ABANDONED))
+			    {
+			      result = "ABANDONED";
+			      status = STAT_BOGUS;
+			    }
+			  else
+			    result = (STAT_ISEQUAL(status, STAT_SECURE) ? "SECURE" : (STAT_ISEQUAL(status, STAT_INSECURE) ? "INSECURE" : "BOGUS"));
+			  
+			  if (STAT_ISEQUAL(status, STAT_SECURE))
+			    cache_secure = 1;
+			  else if (STAT_ISEQUAL(status, STAT_BOGUS))
+			    {
+			      no_cache_dnssec = 1;
+			      bogusanswer = 1;
+			      
+			      if (extract_request(header, m, daemon->namebuff, NULL))
+				domain = daemon->namebuff;
+			    }
+			  
+			  log_query(F_SECSTAT, domain, &a, result, 0);
+			  
+			  if ((daemon->limit[LIMIT_CRYPTO] - validatecount) > (int)daemon->metrics[METRIC_CRYPTO_HWM])
+			    daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit[LIMIT_CRYPTO] - validatecount;
+			  
+			  if ((daemon->limit[LIMIT_WORK] - keycount) > (int)daemon->metrics[METRIC_WORK_HWM])
+			    daemon->metrics[METRIC_WORK_HWM] = daemon->limit[LIMIT_WORK] - keycount;
+			  
+			  /* include DNSSEC queries in the limit for a connection. */
+			  query_count += daemon->limit[LIMIT_WORK] - keycount;
 			}
-		      
-		      log_query(F_SECSTAT, domain, &a, result, 0);
-		    
-		      if ((daemon->limit[LIMIT_CRYPTO] - validatecount) > (int)daemon->metrics[METRIC_CRYPTO_HWM])
-			daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit[LIMIT_CRYPTO] - validatecount;
-
-		      if ((daemon->limit[LIMIT_WORK] - keycount) > (int)daemon->metrics[METRIC_WORK_HWM])
-			daemon->metrics[METRIC_WORK_HWM] = daemon->limit[LIMIT_WORK] - keycount;
-
-		      /* include DNSSEC queries in the limit for a connection. */
-		      query_count += daemon->limit[LIMIT_WORK] - keycount;
 		    }
 #endif
 		  

src/rfc1035.c

@@ -706,7 +706,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	      if (aqclass == C_IN && res != 2 && (aqtype == T_CNAME || aqtype == T_PTR))
 		{
 #ifdef HAVE_DNSSEC
-		  if (option_bool(OPT_DNSSEC_VALID) && daemon->rr_status[j] != 0)
+		  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)
@@ -825,7 +825,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
 	    }
 	  
 #ifdef HAVE_DNSSEC
-	  if (option_bool(OPT_DNSSEC_VALID) && daemon->rr_status[j] != 0)
+	  if (option_bool(OPT_DNSSEC_VALID) && j < daemon->rr_status_sz && daemon->rr_status[j] != 0)
 	    {
 	      secflag = F_DNSSECOK;