Handle validation when more one key is needed.

3 files changed, 142 insertions, 119 deletions

diff --git a/src/config.h b/src/config.h
index c0e699d..b564b8b 100644
--- a/src/config.h
+++ b/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 */
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index a84ac36..204c104 100644
--- a/src/dnsmasq.h
+++ b/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 */
diff --git a/src/forward.c b/src/forward.c
index 2088f98..5163e87 100644
--- a/src/forward.c
+++ b/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);
 	  
@@ -772,15 +775,31 @@ void reply_query(int fd, int family, time_t now)
 	    status = dnssec_validate_ds(now, header, n, daemon->namebuff, daemon->keyname, forward->class);
 	  else
 	    status = dnssec_validate_reply(now, header, n, daemon->namebuff, daemon->keyname, &forward->class);
-	    
+
 	  /* Can't validate, as we're missing key data. Put this
 	     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;
 	      
-	      if ((new = get_new_frec(now, NULL, 1)))
+	    anotherkey:	      
+	      /* Find the original query that started it all.... */
+	      for (orig = forward; orig->dependent; orig = orig->dependent);
+
+	      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,80 +810,67 @@ 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);
+		  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 */
+		  if (status == STAT_NEED_KEY)
+		    {
+		      new->flags |= FREC_DNSKEY_QUERY; 
+		      nn = dnssec_generate_query(header, ((char *) header) + daemon->packet_buff_sz,
+						 daemon->keyname, forward->class, T_DNSKEY, &server->addr);
+		    }
+		  else 
+		    {
+		      new->flags |= FREC_DS_QUERY;
+		      nn = dnssec_generate_query(header,((char *) header) + daemon->packet_buff_sz,
+						 daemon->keyname, forward->class, T_DS, &server->addr);
+		    }
+		  if ((hash = hash_questions(header, nn, daemon->namebuff)))
+		    memcpy(new->hash, hash, HASH_SIZE);
+		  new->new_id = get_id();
+		  header->id = htons(new->new_id);
+		  /* Save query for retransmission */
+		  new->stash = blockdata_alloc((char *)header, nn);
+		  new->stash_len = nn;
+		  
+		  /* Don't resend this. */
+		  daemon->srv_save = NULL;
 		  
-		  /* Now save reply pending receipt of key data */
-		  if (!(forward->stash = blockdata_alloc((char *)header, n)))
-		    free_frec(new); /* malloc failure, unwind */
+		  if (server->sfd)
+		    fd = server->sfd->fd;
 		  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 */
-		      if (status == STAT_NEED_KEY)
-			{
-			  new->flags |= FREC_DNSKEY_QUERY; 
-			  nn = dnssec_generate_query(header, ((char *) header) + daemon->packet_buff_sz,
-						     daemon->keyname, forward->class, T_DNSKEY, &server->addr);
-			}
-		      else 
+		      fd = -1;
+#ifdef HAVE_IPV6
+		      if (server->addr.sa.sa_family == AF_INET6)
 			{
-			  new->flags |= FREC_DS_QUERY;
-			  nn = dnssec_generate_query(header,((char *) header) + daemon->packet_buff_sz,
-						     daemon->keyname, forward->class, T_DS, &server->addr);
+			  if (new->rfd6 || (new->rfd6 = allocate_rfd(AF_INET6)))
+			    fd = new->rfd6->fd;
 			}
-		      if ((hash = hash_questions(header, nn, daemon->namebuff)))
-			memcpy(new->hash, hash, HASH_SIZE);
-		      new->new_id = get_id();
-		      header->id = htons(new->new_id);
-		      /* Save query for retransmission */
-		      new->stash = blockdata_alloc((char *)header, nn);
-		      new->stash_len = nn;
-
-		      /* Don't resend this. */
-		      daemon->srv_save = NULL;
-	
-		      if (server->sfd)
-			fd = server->sfd->fd;
 		      else
-			{
-			  fd = -1;
-#ifdef HAVE_IPV6
-			  if (server->addr.sa.sa_family == AF_INET6)
-			    {
-			      if (new->rfd6 || (new->rfd6 = allocate_rfd(AF_INET6)))
-				fd = new->rfd6->fd;
-			    }
-			  else
 #endif
-			    {
-			      if (new->rfd4 || (new->rfd4 = allocate_rfd(AF_INET)))
-				fd = new->rfd4->fd;
-			    }
-			}
-		      
-		      if (fd != -1)
 			{
-			  while (sendto(fd, (char *)header, nn, 0, &server->addr.sa, sa_len(&server->addr)) == -1 && retry_send()); 
-			  server->queries++;
+			  if (new->rfd4 || (new->rfd4 = allocate_rfd(AF_INET)))
+			    fd = new->rfd4->fd;
 			}
 		    }
+		  
+		  if (fd != -1)
+		    {
+		      while (sendto(fd, (char *)header, nn, 0, &server->addr.sa, sa_len(&server->addr)) == -1 && retry_send()); 
+		      server->queries++;
+		    }
+		  
+		  return;
 		}
-	     
-	      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,60 +1184,73 @@ 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; 
-  unsigned char *packet = whine_malloc(65536 + MAXDNAME + RRFIXEDSZ + sizeof(u16));
-  unsigned char *payload = &packet[2];
-  struct dns_header *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);
-  
-  *length = htons(n);
+  /* limit the amount of work we do, to avoid cycling forever on loops in the DNS */
+  if (--(*keycount) == 0)
+    return STAT_INSECURE;
   
-  if (!read_write(server->tcpfd, packet, n + 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;
-    }
+  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)
     {
-      n = (c1 << 8) | c2;
+      size_t m; 
+      unsigned char *packet = whine_malloc(65536 + MAXDNAME + RRFIXEDSZ + sizeof(u16));
+      unsigned char *payload = &packet[2];
+      struct dns_header *new_header = (struct dns_header *)payload;
+      u16 *length = (u16 *)packet;
+      unsigned char c1, c2;
+       
+      if (!packet)
+	return STAT_INSECURE;
+
+    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 (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);
+      *length = htons(m);
       
-      if (new_status == STAT_NEED_DS || new_status == STAT_NEED_KEY)
+      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))
+	new_status = STAT_INSECURE;
+      else
 	{
-	  if ((new_status = tcp_key_recurse(now, new_status, class, daemon->keyname, server) == STAT_SECURE))
+	  m = (c1 << 8) | c2;
+	  
+	  if (tcp_key_recurse(now, new_status, new_header, m, class, name, keyname, server, keycount) == STAT_SECURE)
 	    {
-	      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);
+	      /* 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, 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);
+
 	      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);
 
+      free(packet);
+    }
+  
   return new_status;
 }
 #endif
@@ -1454,22 +1478,20 @@ unsigned char *tcp_request(int confd, time_t now,
 #ifdef HAVE_DNSSEC
 		      if (option_bool(OPT_DNSSEC_VALID) && !checking_disabled)
 			{
-			  int class, status;
+			  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 (keycount == 0)
+			    result = "ABANDONED";
+			  else
+			    result = (status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
 			  
-			  status = dnssec_validate_reply(now, header, m, daemon->namebuff, daemon->keyname, &class);
+			  log_query(F_KEYTAG | F_SECSTAT, "result", NULL, 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);
-			    }
-
-			  log_query(F_KEYTAG | F_SECSTAT, "result", NULL, 
-				    status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
-
 			  if (status == STAT_BOGUS)
 			    no_cache_dnssec = 1;
-
+			  
 			  if (status == STAT_SECURE)
 			    cache_secure = 1;
 			}