From 87444dc6977b61096127dcdfe87dc6cf2c0167d6 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemensik@redhat.com>

Date: Sun, 16 Apr 2017 20:20:08 +0100

Subject: [PATCH] Capture and log  STDOUT and STDERR output from dhcp-script.

(cherry picked from commit c77fb9d8f09d136fa71bde2469c4fd11cefa6f4a)

Compile-time check on buffer sizes for leasefile parsing code.

(cherry picked from commit bf4e62c19e619f7edf8d03d58d33a5752f190bfd)

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

(cherry picked from commit 3a8b0f6fccf464b1ec6d24c0e00e540ab2b17705)

Tweak logging introduced in 3a8b0f6fccf464b1ec6d24c0e00e540ab2b17705

(cherry picked from commit efff74c1aea14757ce074db28e02671c7f7bb5f5)

Don't die() on failing to parse lease-script output.

(cherry picked from commit 05f76dab89d5b879519a4f45b0cccaa1fc3d162d)

---

 man/dnsmasq.8       |   4 +-

 src/dhcp-common.c   |  16 +++---

 src/dhcp-protocol.h |   4 ++

 src/dnsmasq.c       |   8 +++

 src/dnsmasq.h       |  54 +++++++++---------

 src/helper.c        |  56 +++++++++++++++++-

 src/lease.c         | 159 +++++++++++++++++++++++++++++++---------------------

 src/log.c           |   4 +-

 src/rfc3315.c       |   2 +-

 9 files changed, 202 insertions(+), 105 deletions(-)

diff --git a/man/dnsmasq.8 b/man/dnsmasq.8

index 0521534..97d0a4f 100644

--- a/man/dnsmasq.8

+++ b/man/dnsmasq.8

@@ -1551,8 +1551,8 @@ database.

 All file descriptors are

-closed except stdin, stdout and stderr which are open to /dev/null

-(except in debug mode).

+closed except stdin, which is open to /dev/null, and stdout and stderr which capture output for logging by dnsmasq. 

+(In debug mode, stdio, stdout and stderr file are left as those inherited from the invoker of dnsmasq).

 The script is not invoked concurrently: at most one instance

 of the script is ever running (dnsmasq waits for an instance of script to exit

diff --git a/src/dhcp-common.c b/src/dhcp-common.c

index 08528e8..ecc752b 100644

--- a/src/dhcp-common.c

+++ b/src/dhcp-common.c

@@ -20,11 +20,11 @@

 void dhcp_common_init(void)

 {

-    /* These each hold a DHCP option max size 255

-       and get a terminating zero added */

-  daemon->dhcp_buff = safe_malloc(256);

-  daemon->dhcp_buff2 = safe_malloc(256); 

-  daemon->dhcp_buff3 = safe_malloc(256);

+  /* These each hold a DHCP option max size 255

+     and get a terminating zero added */

+  daemon->dhcp_buff = safe_malloc(DHCP_BUFF_SZ);

+  daemon->dhcp_buff2 = safe_malloc(DHCP_BUFF_SZ); 

+  daemon->dhcp_buff3 = safe_malloc(DHCP_BUFF_SZ);

   /* dhcp_packet is used by v4 and v6, outpacket only by v6 

      sizeof(struct dhcp_packet) is as good an initial size as any,

@@ -855,14 +855,14 @@ void log_context(int family, struct dhcp_context *context)

       if (context->flags & CONTEXT_RA_STATELESS)

 	{

 	  if (context->flags & CONTEXT_TEMPLATE)

-	    strncpy(daemon->dhcp_buff, context->template_interface, 256);

+	    strncpy(daemon->dhcp_buff, context->template_interface, DHCP_BUFF_SZ);

 	  else

 	    strcpy(daemon->dhcp_buff, daemon->addrbuff);

 	}

       else 

 #endif

-	inet_ntop(family, start, daemon->dhcp_buff, 256);

-      inet_ntop(family, end, daemon->dhcp_buff3, 256);

+	inet_ntop(family, start, daemon->dhcp_buff, DHCP_BUFF_SZ);

+      inet_ntop(family, end, daemon->dhcp_buff3, DHCP_BUFF_SZ);

       my_syslog(MS_DHCP | LOG_INFO, 

 		(context->flags & CONTEXT_RA_STATELESS) ? 

 		_("%s stateless on %s%.0s%.0s%s") :

diff --git a/src/dhcp-protocol.h b/src/dhcp-protocol.h

index a31d829..0ea449b 100644

--- a/src/dhcp-protocol.h

+++ b/src/dhcp-protocol.h

@@ -19,6 +19,10 @@

 #define DHCP_CLIENT_ALTPORT 1068

 #define PXE_PORT 4011

+/* These each hold a DHCP option max size 255

+   and get a terminating zero added */

+#define DHCP_BUFF_SZ 256

+

 #define BOOTREQUEST              1

 #define BOOTREPLY                2

 #define DHCP_COOKIE              0x63825363

diff --git a/src/dnsmasq.c b/src/dnsmasq.c

index 045ec53..9cd4052 100644

--- a/src/dnsmasq.c

+++ b/src/dnsmasq.c

@@ -1294,6 +1294,7 @@ static void async_event(int pipe, time_t now)

 		daemon->tcp_pids[i] = 0;

 	break;

+#if defined(HAVE_SCRIPT)	

       case EVENT_KILLED:

 	my_syslog(LOG_WARNING, _("script process killed by signal %d"), ev.data);

 	break;

@@ -1307,12 +1308,19 @@ static void async_event(int pipe, time_t now)

 		  daemon->lease_change_command, strerror(ev.data));

 	break;

+      case EVENT_SCRIPT_LOG:

+	my_syslog(MS_SCRIPT | LOG_DEBUG, "%s", msg ? msg : "");

+        free(msg);

+	msg = NULL;

+	break;

+

 	/* necessary for fatal errors in helper */

       case EVENT_USER_ERR:

       case EVENT_DIE:

       case EVENT_LUA_ERR:

 	fatal_event(&ev, msg);

 	break;

+#endif

       case EVENT_REOPEN:

 	/* Note: this may leave TCP-handling processes with the old file still open.

diff --git a/src/dnsmasq.h b/src/dnsmasq.h

index 1896a64..0cfd3c6 100644

--- a/src/dnsmasq.h

+++ b/src/dnsmasq.h

@@ -145,30 +145,31 @@ struct event_desc {

   int event, data, msg_sz;

 };

-#define EVENT_RELOAD    1

-#define EVENT_DUMP      2

-#define EVENT_ALARM     3

-#define EVENT_TERM      4

-#define EVENT_CHILD     5

-#define EVENT_REOPEN    6

-#define EVENT_EXITED    7

-#define EVENT_KILLED    8

-#define EVENT_EXEC_ERR  9

-#define EVENT_PIPE_ERR  10

-#define EVENT_USER_ERR  11

-#define EVENT_CAP_ERR   12

-#define EVENT_PIDFILE   13

-#define EVENT_HUSER_ERR 14

-#define EVENT_GROUP_ERR 15

-#define EVENT_DIE       16

-#define EVENT_LOG_ERR   17

-#define EVENT_FORK_ERR  18

-#define EVENT_LUA_ERR   19

-#define EVENT_TFTP_ERR  20

-#define EVENT_INIT      21

-#define EVENT_NEWADDR   22

-#define EVENT_NEWROUTE  23

-#define EVENT_TIME_ERR  24

+#define EVENT_RELOAD     1

+#define EVENT_DUMP       2

+#define EVENT_ALARM      3

+#define EVENT_TERM       4

+#define EVENT_CHILD      5

+#define EVENT_REOPEN     6

+#define EVENT_EXITED     7

+#define EVENT_KILLED     8

+#define EVENT_EXEC_ERR   9

+#define EVENT_PIPE_ERR   10

+#define EVENT_USER_ERR   11

+#define EVENT_CAP_ERR    12

+#define EVENT_PIDFILE    13

+#define EVENT_HUSER_ERR  14

+#define EVENT_GROUP_ERR  15

+#define EVENT_DIE        16

+#define EVENT_LOG_ERR    17

+#define EVENT_FORK_ERR   18

+#define EVENT_LUA_ERR    19

+#define EVENT_TFTP_ERR   20

+#define EVENT_INIT       21

+#define EVENT_NEWADDR    22

+#define EVENT_NEWROUTE   23

+#define EVENT_TIME_ERR   24

+#define EVENT_SCRIPT_LOG 25

 /* Exit codes. */

 #define EC_GOOD        0

@@ -242,8 +243,9 @@ struct event_desc {

 /* extra flags for my_syslog, we use a couple of facilities since they are known 

    not to occupy the same bits as priorities, no matter how syslog.h is set up. */

-#define MS_TFTP LOG_USER

-#define MS_DHCP LOG_DAEMON 

+#define MS_TFTP   LOG_USER

+#define MS_DHCP   LOG_DAEMON

+#define MS_SCRIPT LOG_MAIL

 struct all_addr {

   union {

diff --git a/src/helper.c b/src/helper.c

index 9c37e37..de31383 100644

--- a/src/helper.c

+++ b/src/helper.c

@@ -14,6 +14,7 @@

    along with this program.  If not, see <http://www.gnu.org/licenses/>.

 */

+#include <stdio.h>

 #include "dnsmasq.h"

 #ifdef HAVE_SCRIPT

@@ -135,7 +136,7 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)

 	max_fd != STDIN_FILENO && max_fd != pipefd[0] && 

 	max_fd != event_fd && max_fd != err_fd)

       close(max_fd);

-  

+

 #ifdef HAVE_LUASCRIPT

   if (daemon->luascript)

     {

@@ -189,6 +190,7 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)

       unsigned char *buf = (unsigned char *)daemon->namebuff;

       unsigned char *end, *extradata, *alloc_buff = NULL;

       int is6, err = 0;

+      int pipeout[2];

       free(alloc_buff);

@@ -472,16 +474,54 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)

       if (!daemon->lease_change_command)

 	continue;

+      /* Pipe to capture stdout and stderr from script */

+      if (!option_bool(OPT_DEBUG) && pipe(pipeout) == -1)

+	continue;

+      

       /* possible fork errors are all temporary resource problems */

       while ((pid = fork()) == -1 && (errno == EAGAIN || errno == ENOMEM))

 	sleep(2);

       if (pid == -1)

-	continue;

+        {

+	  if (!option_bool(OPT_DEBUG))

+	    {

+	      close(pipeout[0]);

+	      close(pipeout[1]);

+	    }

+	  continue;

+        }

       /* wait for child to complete */

       if (pid != 0)

 	{

+	  if (!option_bool(OPT_DEBUG))

+	    {

+	      FILE *fp;

+	  

+	      close(pipeout[1]);

+	      

+	      /* Read lines sent to stdout/err by the script and pass them back to be logged */

+	      if (!(fp = fdopen(pipeout[0], "r")))

+		close(pipeout[0]);

+	      else

+		{

+		  while (fgets(daemon->packet, daemon->packet_buff_sz, fp))

+		    {

+		      /* do not include new lines, log will append them */

+		      size_t len = strlen(daemon->packet);

+		      if (len > 0)

+			{

+			  --len;

+			  if (daemon->packet[len] == '\n')

+			    daemon->packet[len] = 0;

+			}

+		      send_event(event_fd, EVENT_SCRIPT_LOG, 0, daemon->packet);

+		    }

+		  fclose(fp);

+		}

+	    }

+	  

 	  /* reap our children's children, if necessary */

 	  while (1)

 	    {

@@ -504,6 +544,15 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)

 	  continue;

 	}

+

+      if (!option_bool(OPT_DEBUG))

+	{

+	  /* map stdout/stderr of script to pipeout */

+	  close(pipeout[0]);

+	  dup2(pipeout[1], STDOUT_FILENO);

+	  dup2(pipeout[1], STDERR_FILENO);

+	  close(pipeout[1]);

+	}

       if (data.action != ACTION_TFTP && data.action != ACTION_ARP)

 	{

@@ -579,7 +628,8 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)

 	    hostname = NULL;

 	  my_setenv("DNSMASQ_LOG_DHCP", option_bool(OPT_LOG_OPTS) ? "1" : NULL, &err;;

-    }

+	}

+      

       /* we need to have the event_fd around if exec fails */

       if ((i = fcntl(event_fd, F_GETFD)) != -1)

 	fcntl(event_fd, F_SETFD, i | FD_CLOEXEC);

diff --git a/src/lease.c b/src/lease.c

index 20cac90..64047f9 100644

--- a/src/lease.c

+++ b/src/lease.c

@@ -21,94 +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

+  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

+

+     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.

 #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);

-    }

-  

-  /* client-id max length is 255 which is 255*2 digits + 254 colons 

-     borrow DNS packet buffer which is always larger than 1000 bytes */

-  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 explict 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')

 	      {

@@ -116,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;

@@ -148,7 +123,62 @@ 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;

+

+  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)

+    {

+      if (!read_leases(now, leasestream))

+	my_syslog(MS_DHCP | LOG_ERR, _("failed to parse lease database, invalid line: %s %s %s %s ..."),

+		  daemon->dhcp_buff3, daemon->dhcp_buff2,

+		  daemon->namebuff, daemon->dhcp_buff);

+

+      if (ferror(leasestream))

+	die(_("failed to read lease file %s: %s"), daemon->lease_file, EC_FILE);

+    }

 #ifdef HAVE_SCRIPT

   if (!daemon->lease_stream)

@@ -162,6 +192,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);

 	}

diff --git a/src/log.c b/src/log.c

index 8e66629..5fc860b 100644

--- a/src/log.c

+++ b/src/log.c

@@ -288,7 +288,9 @@ void my_syslog(int priority, const char *format, ...)

     func = "-tftp";

   else if ((LOG_FACMASK & priority) == MS_DHCP)

     func = "-dhcp";

-      

+  else if ((LOG_FACMASK & priority) == MS_SCRIPT)

+    func = "-script";

+	    

 #ifdef LOG_PRI

   priority = LOG_PRI(priority);

 #else

diff --git a/src/rfc3315.c b/src/rfc3315.c

index 3f4d69c..a3715cd 100644

--- a/src/rfc3315.c

+++ b/src/rfc3315.c

@@ -1975,7 +1975,7 @@ static void log6_packet(struct state *state, char *type, struct in6_addr *addr,

   if (addr)

     {

-      inet_ntop(AF_INET6, addr, daemon->dhcp_buff2, 255);

+      inet_ntop(AF_INET6, addr, daemon->dhcp_buff2, DHCP_BUFF_SZ - 1);

       strcat(daemon->dhcp_buff2, " ");

     }

   else

-- 

2.9.3