Handle validation when more one key is needed.

src/config.h

@@ -19,6 +19,7 @@
 #define CHILD_LIFETIME 150 /* secs 'till terminated (RFC1035 suggests > 120s) */
 #define EDNS_PKTSZ 4096 /* default max EDNS.0 UDP packet from RFC5625 */
 #define KEYBLOCK_LEN 35 /* choose to mininise fragmentation when storing DNSSEC keys */
+#define DNSSEC_WORK 50 /* Max number of queries to validate one question */
 #define TIMEOUT 10 /* drop UDP queries after TIMEOUT seconds */
 #define FORWARD_TEST 50 /* try all servers every 50 queries */
 #define FORWARD_TIME 20 /* or 20 seconds */

src/dnsmasq.h

@@ -560,7 +560,7 @@ struct frec {
   time_t time;
   unsigned char *hash[HASH_SIZE];
 #ifdef HAVE_DNSSEC 
-  int class;
+  int class, work_counter;
   struct blockdata *stash; /* Saved reply, whilst we validate */
   size_t stash_len;
   struct frec *dependent; /* Query awaiting internally-generated DNSKEY or DS query */

src/forward.c

@@ -331,6 +331,9 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
 	    forward->flags |= FREC_NOREBIND;
 	  if (header->hb4 & HB4_CD)
 	    forward->flags |= FREC_CHECKING_DISABLED;
+#ifdef HAVE_DNSSEC
+	  forward->work_counter = DNSSEC_WORK;
+#endif
 
 	  header->id = htons(forward->new_id);
 	  
@@ -777,10 +780,26 @@ void reply_query(int fd, int family, time_t now)
 	     answer aside, whilst we get that. */     
 	  if (status == STAT_NEED_DS || status == STAT_NEED_KEY)
 	    {
-	      struct frec *new;
+	      struct frec *new, *orig;
+	      
+	      /* Free any saved query */
+	      if (forward->stash)
+		blockdata_free(forward->stash);
+	      
+	      /* Now save reply pending receipt of key data */
+	      if (!(forward->stash = blockdata_alloc((char *)header, n)))
+		return;
+	      forward->stash_len = n;
+	      
+	    anotherkey:	      
+	      /* Find the original query that started it all.... */
+	      for (orig = forward; orig->dependent; orig = orig->dependent);
 
-	      if ((new = get_new_frec(now, NULL, 1)))
+	      if (--orig->work_counter == 0 || !(new = get_new_frec(now, NULL, 1)))
+		status = STAT_INSECURE;
+	      else
 		{
+		  int fd;
 		  struct frec *next = new->next;
 		  *new = *forward; /* copy everything, then overwrite */
 		  new->next = next;
@@ -791,19 +810,6 @@ void reply_query(int fd, int family, time_t now)
 #endif
 		  new->flags &= ~(FREC_DNSKEY_QUERY | FREC_DS_QUERY);
 		  
-		  /* Free any saved query */
-		  if (forward->stash)
-		    blockdata_free(forward->stash);
-		  
-		  /* Now save reply pending receipt of key data */
-		  if (!(forward->stash = blockdata_alloc((char *)header, n)))
-		    free_frec(new); /* malloc failure, unwind */
-		  else
-		    {
-		      int fd;
-		      
-		      forward->stash_len = n;
-		      
 		  new->dependent = forward; /* to find query awaiting new one. */
 		  forward->blocking_query = new; /* for garbage cleaning */
 		  /* validate routines leave name of required record in daemon->keyname */
@@ -854,17 +860,17 @@ void reply_query(int fd, int family, time_t now)
 		      while (sendto(fd, (char *)header, nn, 0, &server->addr.sa, sa_len(&server->addr)) == -1 && retry_send()); 
 		      server->queries++;
 		    }
-		    }
-		}
 		  
 		  return;
 		}
+	    }
 	  
 	  /* Ok, we reached far enough up the chain-of-trust that we can validate something.
 	     Now wind back down, pulling back answers which wouldn't previously validate
-	     and validate them with the new data. Failure to find needed data here is an internal error.
-	     Once we get to the original answer (FREC_DNSSEC_QUERY not set) and it validates,
-	     return it to the original requestor. */
+	     and validate them with the new data. Note that if an answer needs multiple
+	     keys to validate, we may find another key is needed, in which case we set off
+	     down another branch of the tree. Once we get to the original answer 
+	     (FREC_DNSSEC_QUERY not set) and it validates, return it to the original requestor. */
 	  while (forward->dependent)
 	    {
 	      struct frec *prev = forward->dependent;
@@ -884,18 +890,23 @@ void reply_query(int fd, int family, time_t now)
 		    status = dnssec_validate_reply(now, header, n, daemon->namebuff, daemon->keyname, &forward->class);	
 		  
 		  if (status == STAT_NEED_DS || status == STAT_NEED_KEY)
-		    {
-		      my_syslog(LOG_ERR, _("Unexpected missing data for DNSSEC validation"));
-		      status = STAT_INSECURE;
-		    }
+		    goto anotherkey;
 		}
 	    }
 	  
 	  if (status == STAT_TRUNCATED)
 	    header->hb3 |= HB3_TC;
 	  else
-	    log_query(F_KEYTAG | F_SECSTAT, "result", NULL, 
-		      status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
+	    {
+	      char *result;
+	      
+	      if (forward->work_counter == 0)
+		result = "ABANDONED";
+	      else
+		result = (status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
+	      
+	      log_query(F_KEYTAG | F_SECSTAT, "result", NULL, result);
+	    }
 	  
 	  no_cache_dnssec = 0;
 	  
@@ -1173,59 +1184,72 @@ void receive_query(struct listener *listen, time_t now)
 }
 
 #ifdef HAVE_DNSSEC
-static int tcp_key_recurse(time_t now, int status, int class, char *keyname, struct server *server)
+static int tcp_key_recurse(time_t now, int status, struct dns_header *header, size_t n, 
+			   int class, char *name, char *keyname, struct server *server, int *keycount)
 {
   /* Recurse up the key heirarchy */
-  size_t n; 
+  int new_status;
+
+  /* limit the amount of work we do, to avoid cycling forever on loops in the DNS */
+  if (--(*keycount) == 0)
+    return STAT_INSECURE;
+  
+  if (status == STAT_NEED_KEY)
+    new_status = dnssec_validate_by_ds(now, header, n, name, keyname, class);
+  else if (status == STAT_NEED_DS)
+    new_status = dnssec_validate_ds(now, header, n, name, keyname, class);
+  else
+    new_status = dnssec_validate_reply(now, header, n, name, keyname, &class);
+  
+  /* Can't validate because we need a key/DS whose name now in keyname.
+     Make query for same, and recurse to validate */
+  if (new_status == STAT_NEED_DS || new_status == STAT_NEED_KEY)
+    {
+      size_t m; 
       unsigned char *packet = whine_malloc(65536 + MAXDNAME + RRFIXEDSZ + sizeof(u16));
       unsigned char *payload = &packet[2];
-  struct dns_header *header = (struct dns_header *)payload;
+      struct dns_header *new_header = (struct dns_header *)payload;
       u16 *length = (u16 *)packet;
-  int new_status;
       unsigned char c1, c2;
        
-  n = dnssec_generate_query(header, ((char *) header) + 65536, keyname, class, 
-			    status == STAT_NEED_KEY ? T_DNSKEY : T_DS, &server->addr);
+      if (!packet)
+	return STAT_INSECURE;
 
-  *length = htons(n);
+    another_tcp_key:
+      m = dnssec_generate_query(new_header, ((char *) new_header) + 65536, keyname, class, 
+				new_status == STAT_NEED_KEY ? T_DNSKEY : T_DS, &server->addr);
       
-  if (!read_write(server->tcpfd, packet, n + sizeof(u16), 0) ||
+      *length = htons(m);
+      
+      if (!read_write(server->tcpfd, packet, m + sizeof(u16), 0) ||
 	  !read_write(server->tcpfd, &c1, 1, 1) ||
 	  !read_write(server->tcpfd, &c2, 1, 1) ||
 	  !read_write(server->tcpfd, payload, (c1 << 8) | c2, 1))
-    {
-      close(server->tcpfd);
-      server->tcpfd = -1;
 	new_status = STAT_INSECURE;
-    }
       else
 	{
-      n = (c1 << 8) | c2;
-      
-      if (status == STAT_NEED_KEY)
-	new_status = dnssec_validate_by_ds(now, header, n, daemon->namebuff, daemon->keyname, class);
-      else
-	new_status = dnssec_validate_ds(now, header, n, daemon->namebuff, daemon->keyname, class);
+	  m = (c1 << 8) | c2;
 	  
-      if (new_status == STAT_NEED_DS || new_status == STAT_NEED_KEY)
-	{
-	  if ((new_status = tcp_key_recurse(now, new_status, class, daemon->keyname, server) == STAT_SECURE))
+	  if (tcp_key_recurse(now, new_status, new_header, m, class, name, keyname, server, keycount) == STAT_SECURE)
 	    {
+	      /* Reached a validated record, now try again at this level.
+		 Note that we may get ANOTHER NEED_* if an answer needs more than one key.
+		 If so, go round again. */
+	      
 	      if (status == STAT_NEED_KEY)
-		new_status = dnssec_validate_by_ds(now, header, n, daemon->namebuff, daemon->keyname, class);
+		new_status = dnssec_validate_by_ds(now, header, n, name, keyname, class);
+	      else if (status == STAT_NEED_DS)
+		new_status = dnssec_validate_ds(now, header, n, name, keyname, class);
 	      else
-		new_status = dnssec_validate_ds(now, header, n, daemon->namebuff, daemon->keyname, class);
+		new_status = dnssec_validate_reply(now, header, n, name, keyname, &class);
 
 	      if (new_status == STAT_NEED_DS || new_status == STAT_NEED_KEY)
-		{
-		  my_syslog(LOG_ERR, _("Unexpected missing data for DNSSEC validation"));
-		  status = STAT_INSECURE;
-		}
-	    }
+		goto another_tcp_key;
 	    }
 	}
 
       free(packet);
+    }
   
   return new_status;
 }
@@ -1454,18 +1478,16 @@ unsigned char *tcp_request(int confd, time_t now,
 #ifdef HAVE_DNSSEC
 		      if (option_bool(OPT_DNSSEC_VALID) && !checking_disabled)
 			{
-			  int class, status;
-			  
-			  status = dnssec_validate_reply(now, header, m, daemon->namebuff, daemon->keyname, &class);
+			  int keycount = DNSSEC_WORK; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */
+			  int status = tcp_key_recurse(now, STAT_TRUNCATED, header, m, 0, daemon->namebuff, daemon->keyname, last_server, &keycount);
+			  char *result;
 
-			  if (status == STAT_NEED_DS || status == STAT_NEED_KEY)
-			    {
-			      if ((status = tcp_key_recurse(now, status, class, daemon->keyname, last_server)) == STAT_SECURE)
-				status = dnssec_validate_reply(now, header, m, daemon->namebuff, daemon->keyname, &class);
-			    }
+			  if (keycount == 0)
+			    result = "ABANDONED";
+			  else
+			    result = (status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
 			  
-			  log_query(F_KEYTAG | F_SECSTAT, "result", NULL, 
-				    status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
+			  log_query(F_KEYTAG | F_SECSTAT, "result", NULL, result);
 			  
 			  if (status == STAT_BOGUS)
 			    no_cache_dnssec = 1;