--- /dev/null
+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
+
Code Review / pti / rtp.git / blobdiff