From 87444dc6977b61096127dcdfe87dc6cf2c0167d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= 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 . */ +#include #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 " 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 " 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