Improve error handling with shcp-script "init" mode.

CHANGELOG

@@ -94,6 +94,11 @@ version 2.77
 	    for diagnosing unexpected problems in scripts.
 	    Thanks to Petr Mensik for the patch.
 
+	    Generate fatal errors when failing to parse the output
+	    of the dhcp-script in "init" mode. Avoids strange errors
+            when the script accidentally emits error messages.
+	    Thanks to Petr Mensik for the patch.
+
 
 version 2.76
             Include 0.0.0.0/8 in DNS rebind checks. This range 

src/lease.c

@@ -21,101 +21,62 @@
 static struct dhcp_lease *leases = NULL, *old_leases = NULL;
 static int dns_dirty, file_dirty, leases_left;
 
-void lease_init(time_t now)
+static int read_leases(time_t now, FILE *leasestream)
 {
   unsigned long ei;
   struct all_addr addr;
   struct dhcp_lease *lease;
   int clid_len, hw_len, hw_type;
-  FILE *leasestream;
-  
-  leases_left = daemon->dhcp_max;
-  
-  if (option_bool(OPT_LEASE_RO))
-    {
-      /* run "<lease_change_script> init" once to get the
-	 initial state of the database. If leasefile-ro is
-	 set without a script, we just do without any 
-	 lease database. */
-#ifdef HAVE_SCRIPT
-      if (daemon->lease_change_command)
-	{
-	  strcpy(daemon->dhcp_buff, daemon->lease_change_command);
-	  strcat(daemon->dhcp_buff, " init");
-	  leasestream = popen(daemon->dhcp_buff, "r");
-	}
-      else
-#endif
-	{
-          file_dirty = dns_dirty = 0;
-          return;
-        }
+  int items;
+  char *domain = NULL;
+
+  *daemon->dhcp_buff3 = *daemon->dhcp_buff2 = '\0';
+
+  /* client-id max length is 255 which is 255*2 digits + 254 colons
+     borrow DNS packet buffer which is always larger than 1000 bytes
 
-    }
-  else
-    {
-      /* NOTE: need a+ mode to create file if it doesn't exist */
-      leasestream = daemon->lease_stream = fopen(daemon->lease_file, "a+");
-      
-      if (!leasestream)
-	die(_("cannot open or create lease file %s: %s"), daemon->lease_file, EC_FILE);
-      
-      /* a+ mode leaves pointer at end. */
-      rewind(leasestream);
-    }
-  
-  /* client-id max length is 255 which is 255*2 digits + 254 colons 
-     borrow DNS packet buffer which is always larger than 1000 bytes 
-  
      Check various buffers are big enough for the code below */
 
 #if (DHCP_BUFF_SZ < 255) || (MAXDNAME < 64) || (PACKETSZ+MAXDNAME+RRFIXEDSZ  < 764)
-# error Buffer size breakage in leasefile parsing. 
+# error Buffer size breakage in leasefile parsing.
 #endif
 
-  if (leasestream)
-    while (fscanf(leasestream, "%255s %255s", daemon->dhcp_buff3, daemon->dhcp_buff2) == 2)
+    while ((items=fscanf(leasestream, "%255s %255s", daemon->dhcp_buff3, daemon->dhcp_buff2)) == 2)
       {
+	*daemon->namebuff = *daemon->dhcp_buff = *daemon->packet = '\0';
+	hw_len = hw_type = clid_len = 0;
+	
 #ifdef HAVE_DHCP6
 	if (strcmp(daemon->dhcp_buff3, "duid") == 0)
 	  {
 	    daemon->duid_len = parse_hex(daemon->dhcp_buff2, (unsigned char *)daemon->dhcp_buff2, 130, NULL, NULL);
+	    if (daemon->duid_len < 0)
+	      return 0;
 	    daemon->duid = safe_malloc(daemon->duid_len);
 	    memcpy(daemon->duid, daemon->dhcp_buff2, daemon->duid_len);
 	    continue;
 	  }
 #endif
-
-	ei = atol(daemon->dhcp_buff3);
 	
 	if (fscanf(leasestream, " %64s %255s %764s",
 		   daemon->namebuff, daemon->dhcp_buff, daemon->packet) != 3)
-	  break;
+	  return 0;
 	
-	clid_len = 0;
-	if (strcmp(daemon->packet, "*") != 0)
-	  clid_len = parse_hex(daemon->packet, (unsigned char *)daemon->packet, 255, NULL, NULL);
-	
-	if (inet_pton(AF_INET, daemon->namebuff, &addr.addr.addr4) &&
-	    (lease = lease4_allocate(addr.addr.addr4)))
+	if (inet_pton(AF_INET, daemon->namebuff, &addr.addr.addr4))
 	  {
+	    if ((lease = lease4_allocate(addr.addr.addr4)))
+	      domain = get_domain(lease->addr);
+	    
 	    hw_len = parse_hex(daemon->dhcp_buff2, (unsigned char *)daemon->dhcp_buff2, DHCP_CHADDR_MAX, NULL, &hw_type);
 	    /* For backwards compatibility, no explicit MAC address type means ether. */
 	    if (hw_type == 0 && hw_len != 0)
 	      hw_type = ARPHRD_ETHER; 
-
-	    lease_set_hwaddr(lease, (unsigned char *)daemon->dhcp_buff2, (unsigned char *)daemon->packet, 
-			     hw_len, hw_type, clid_len, now, 0);
-	    
-	    if (strcmp(daemon->dhcp_buff, "*") !=  0)
-	      lease_set_hostname(lease, daemon->dhcp_buff, 0, get_domain(lease->addr), NULL);
 	  }
 #ifdef HAVE_DHCP6
 	else if (inet_pton(AF_INET6, daemon->namebuff, &addr.addr.addr6))
 	  {
 	    char *s = daemon->dhcp_buff2;
 	    int lease_type = LEASE_NA;
-	    int iaid;
 
 	    if (s[0] == 'T')
 	      {
@@ -123,23 +84,30 @@ void lease_init(time_t now)
 		s++;
 	      }
 	    
-	    iaid = strtoul(s, NULL, 10);
-	    
 	    if ((lease = lease6_allocate(&addr.addr.addr6, lease_type)))
 	      {
-		lease_set_hwaddr(lease, NULL, (unsigned char *)daemon->packet, 0, 0, clid_len, now, 0);
-		lease_set_iaid(lease, iaid);
-		if (strcmp(daemon->dhcp_buff, "*") !=  0)
-		  lease_set_hostname(lease, daemon->dhcp_buff, 0, get_domain6((struct in6_addr *)lease->hwaddr), NULL);
+		lease_set_iaid(lease, strtoul(s, NULL, 10));
+		domain = get_domain6((struct in6_addr *)lease->hwaddr);
 	      }
 	  }
 #endif
 	else
-	  break;
+	  return 0;
 
 	if (!lease)
 	  die (_("too many stored leases"), NULL, EC_MISC);
-       	
+
+	if (strcmp(daemon->packet, "*") != 0)
+	  clid_len = parse_hex(daemon->packet, (unsigned char *)daemon->packet, 255, NULL, NULL);
+	
+	lease_set_hwaddr(lease, (unsigned char *)daemon->dhcp_buff2, (unsigned char *)daemon->packet, 
+			 hw_len, hw_type, clid_len, now, 0);
+	
+	if (strcmp(daemon->dhcp_buff, "*") !=  0)
+	  lease_set_hostname(lease, daemon->dhcp_buff, 0, domain, NULL);
+
+	ei = atol(daemon->dhcp_buff3);
+
 #ifdef HAVE_BROKEN_RTC
 	if (ei != 0)
 	  lease->expires = (time_t)ei + now;
@@ -155,7 +123,60 @@ void lease_init(time_t now)
 	/* set these correctly: the "old" events are generated later from
 	   the startup synthesised SIGHUP. */
 	lease->flags &= ~(LEASE_NEW | LEASE_CHANGED);
+	
+	*daemon->dhcp_buff3 = *daemon->dhcp_buff2 = '\0';
       }
+    
+    return (items == 0 || items == EOF);
+}
+
+void lease_init(time_t now)
+{
+  FILE *leasestream;
+  int readok = 0;
+
+  leases_left = daemon->dhcp_max;
+
+  if (option_bool(OPT_LEASE_RO))
+    {
+      /* run "<lease_change_script> init" once to get the
+	 initial state of the database. If leasefile-ro is
+	 set without a script, we just do without any
+	 lease database. */
+#ifdef HAVE_SCRIPT
+      if (daemon->lease_change_command)
+	{
+	  strcpy(daemon->dhcp_buff, daemon->lease_change_command);
+	  strcat(daemon->dhcp_buff, " init");
+	  leasestream = popen(daemon->dhcp_buff, "r");
+	}
+      else
+#endif
+	{
+          file_dirty = dns_dirty = 0;
+          return;
+        }
+
+    }
+  else
+    {
+      /* NOTE: need a+ mode to create file if it doesn't exist */
+      leasestream = daemon->lease_stream = fopen(daemon->lease_file, "a+");
+
+      if (!leasestream)
+	die(_("cannot open or create lease file %s: %s"), daemon->lease_file, EC_FILE);
+
+      /* a+ mode leaves pointer at end. */
+      rewind(leasestream);
+    }
+
+  if (leasestream)
+    {
+      readok = read_leases(now, leasestream);
+      if (ferror(leasestream))
+	die(_("failed to read lease file %s: %s"), daemon->lease_file, EC_FILE);
+
+    }
   
 #ifdef HAVE_SCRIPT
   if (!daemon->lease_stream)
@@ -169,6 +190,7 @@ void lease_init(time_t now)
 	    errno = ENOENT;
 	  else if (WEXITSTATUS(rc) == 126)
 	    errno = EACCES;
+
 	  die(_("cannot run lease-init script %s: %s"), daemon->lease_change_command, EC_FILE);
 	}
       
@@ -177,6 +199,17 @@ void lease_init(time_t now)
 	  sprintf(daemon->dhcp_buff, "%d", WEXITSTATUS(rc));
 	  die(_("lease-init script returned exit code %s"), daemon->dhcp_buff, WEXITSTATUS(rc) + EC_INIT_OFFSET);
 	}
+
+      /* Only complain if we stopped reading due to a non-parsed line when running script,
+	 this is expected behaviour when reading from a file, if the file was written with IPv6 data
+	 and we are not compiled to understand that. */
+      if (!readok)
+	{
+	  my_syslog(MS_DHCP | LOG_ERR, _("aborting lease-init script, invalid line: %s %s %s %s ..."),
+		    daemon->dhcp_buff3, daemon->dhcp_buff2,
+		    daemon->namebuff, daemon->dhcp_buff);
+	  die(_("failed to parse lease-init script output"), NULL, EC_FILE);
+	}
     }
 #endif