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