From c611eded2272ac79997fb3ce11f2339dc32b53cb Mon Sep 17 00:00:00 2001 From: Roland Knall Date: Mon, 4 Jan 2016 18:32:39 +0100 Subject: extcap: Use stderr to print error message This patch reads out the stderr messages from an extcap utility and displays it to an user. It was tested on Qt but not on GTK, but should work their as well. On Mac OS/X and Windows the child_watch does not behave as it was intended. Therefore in extcap_cleanup, the callbacks are called manually, if and only if, they have not been called already. The reason why it displays two error messages is, that by the time the first one is being displayed, glib has not returned from the spawned process on Linux yet. So there is no way to add the stderr correctly, and putting a handler to stderr into interface_opts will lead to memory errors, cause then the code tries to access memory outside of its protection. Bug: 11892 Change-Id: I2db60dd480fed3e01428b91a705057e4f088bd15 Reviewed-on: https://code.wireshark.org/review/12954 Reviewed-by: Roland Knall Petri-Dish: Roland Knall Tested-by: Petri Dish Buildbot Reviewed-by: Dario Lombardo Reviewed-by: Anders Broman --- capchild/capture_sync.c | 2 +- capture_opts.c | 2 + doc/extcap_example.py | 74 ++++++++++++++++++++++++-- dumpcap.c | 25 +++++++-- extcap.c | 139 +++++++++++++++++++++++++++++++++++++++++++++--- extcap.h | 4 +- extcap_spawn.c | 14 ++--- extcap_spawn.h | 2 +- 8 files changed, 231 insertions(+), 31 deletions(-) diff --git a/capchild/capture_sync.c b/capchild/capture_sync.c index 28c34a5542..5c62e2c437 100644 --- a/capchild/capture_sync.c +++ b/capchild/capture_sync.c @@ -1733,7 +1733,7 @@ sync_pipe_input_cb(gint source, gpointer user_data) #endif #ifdef HAVE_EXTCAP g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "sync_pipe_input_cb: cleaning extcap pipe"); - extcap_if_cleanup(cap_session->capture_opts); + extcap_if_cleanup(cap_session->capture_opts, &primary_msg); #endif capture_input_closed(cap_session, primary_msg); g_free(primary_msg); diff --git a/capture_opts.c b/capture_opts.c index 2eb5b630d1..e014290f35 100644 --- a/capture_opts.c +++ b/capture_opts.c @@ -66,6 +66,7 @@ capture_opts_init(capture_options *capture_opts) capture_opts->default_options.extcap = NULL; capture_opts->default_options.extcap_fifo = NULL; capture_opts->default_options.extcap_args = NULL; + capture_opts->default_options.extcap_userdata = NULL; capture_opts->default_options.extcap_pid = INVALID_EXTCAP_PID; #endif #ifdef CAN_SET_CAPTURE_BUFFER_SIZE @@ -1158,6 +1159,7 @@ collect_ifaces(capture_options *capture_opts) #ifdef HAVE_EXTCAP interface_opts.extcap = g_strdup(device.if_info.extcap); interface_opts.extcap_fifo = NULL; + interface_opts.extcap_userdata = NULL; interface_opts.extcap_args = device.external_cap_args_settings; interface_opts.extcap_pid = INVALID_EXTCAP_PID; if (interface_opts.extcap_args) diff --git a/doc/extcap_example.py b/doc/extcap_example.py index ab9967c489..9007e26bba 100755 --- a/doc/extcap_example.py +++ b/doc/extcap_example.py @@ -40,6 +40,7 @@ other script-based formates beside VBScript """ +from __future__ import print_function import os import sys import signal @@ -52,12 +53,40 @@ from threading import Thread ERROR_USAGE = 0 ERROR_ARG = 1 -ERROR_INTERFACE = 2 +ERROR_INTERFACE = 2 ERROR_FIFO = 3 +ERROR_DELAY = 4 doExit = False globalinterface = 0 +""" +This code has been taken from http://stackoverflow.com/questions/5943249/python-argparse-and-controlling-overriding-the-exit-status-code - originally developed by Rob Cowie http://stackoverflow.com/users/46690/rob-cowie +""" +class ArgumentParser(argparse.ArgumentParser): + def _get_action_from_name(self, name): + """Given a name, get the Action instance registered with this parser. + If only it were made available in the ArgumentError object. It is + passed as it's first arg... + """ + container = self._actions + if name is None: + return None + for action in container: + if '/'.join(action.option_strings) == name: + return action + elif action.metavar == name: + return action + elif action.dest == name: + return action + + def error(self, message): + exc = sys.exc_info()[1] + if exc: + exc.argument = self._get_action_from_name(exc.argument_name) + raise exc + super(ArgumentParser, self).error(message) + def signalHandler(signal, frame): global doExit doExit = True @@ -214,6 +243,23 @@ def extcap_capture(interface, fifo, delay, verify, message, remote, fake_ip): fh.close() +def extcap_close_fifo(interface, fifo): + global doExit + + signal.signal(signal.SIGINT, signalHandler) + signal.signal(signal.SIGTERM , signalHandler) + + tdelay = delay if delay != 0 else 5 + + try: + os.stat(fifo) + except OSError: + doExit = True + print ( "Fifo does not exist!" ) + + fh = open(fifo, 'w+b', 0 ) + fh.close() + #### def usage(): @@ -227,7 +273,7 @@ if __name__ == '__main__': message = "" fake_ip = "" - parser = argparse.ArgumentParser( + parser = ArgumentParser( prog="Extcap Example", description="Extcap example program for python" ) @@ -243,12 +289,26 @@ if __name__ == '__main__': # Interface Arguments parser.add_argument("--verify", help="Demonstrates a verification bool flag", action="store_true" ) - parser.add_argument("--delay", help="Demonstrates an integer variable", type=int, default=0, choices=[0, 1, 2, 3, 4, 5] ) + parser.add_argument("--delay", help="Demonstrates an integer variable", type=int, default=0, choices=[0, 1, 2, 3, 4, 5, 6] ) parser.add_argument("--remote", help="Demonstrates a selector choice", default="if1", choices=["if1", "if2"] ) parser.add_argument("--message", help="Demonstrates string variable", nargs='?', default="" ) parser.add_argument("--fake_ip", help="Add a fake sender IP adress", nargs='?', default="127.0.0.1" ) - args, unknown = parser.parse_known_args() + try: + args, unknown = parser.parse_known_args() + except argparse.ArgumentError, exc: + print( "%s: %s" % ( exc.argument.dest, exc.message ), file=sys.stderr) + fifo_found = 0 + fifo = "" + for arg in sys.argv: + if (arg == "--fifo" or arg == "--extcap-fifo") : + fifo_found = 1 + elif ( fifo_found == 1 ): + fifo = arg + break + extcap_close_fifo("", fifo) + sys.exit(ERROR_ARG) + if ( len(sys.argv) <= 1 ): parser.exit("No arguments given!") @@ -282,6 +342,12 @@ if __name__ == '__main__': elif args.capture: if args.fifo is None: sys.exit(ERROR_FIFO) + # The following code demonstrates error management with extcap + if args.delay > 5: + print("Value for delay [%d] too high" % args.delay, file=sys.stderr) + extcap_close_fifo(interface, args.fifo) + sys.exit(ERROR_DELAY) + extcap_capture(interface, args.fifo, args.delay, args.verify, message, args.remote, fake_ip) else: usage() diff --git a/dumpcap.c b/dumpcap.c index 3d736ebfc0..935e52d9ca 100644 --- a/dumpcap.c +++ b/dumpcap.c @@ -1466,12 +1466,14 @@ cap_pipe_open_live(char *pipename, #else /* _WIN32 */ char *pncopy, *pos; wchar_t *err_str; - interface_options interface_opts; #ifdef HAVE_EXTCAP char* extcap_pipe_name; - gboolean extcap_pipe; #endif #endif +#ifdef HAVE_EXTCAP + gboolean extcap_pipe = FALSE; +#endif + interface_options interface_opts; ssize_t b; int fd = -1, sel_ret; size_t bytes_read; @@ -1498,7 +1500,15 @@ cap_pipe_open_live(char *pipename, return; } } else { + + interface_opts = g_array_index(global_capture_opts.ifaces, interface_options, 0); + #ifndef _WIN32 +#ifdef HAVE_EXTCAP + if ( g_strrstr(interface_opts.name, EXTCAP_PIPE_PREFIX) != NULL ) + extcap_pipe = TRUE; +#endif + if (ws_stat64(pipename, &pipe_stat) < 0) { if (errno == ENOENT || errno == ENOTDIR) pcap_opts->cap_pipe_err = PIPNEXIST; @@ -1585,6 +1595,7 @@ cap_pipe_open_live(char *pipename, } return; } + #else /* _WIN32 */ #define PIPE_STR "\\pipe\\" /* Under Windows, named pipes _must_ have the form @@ -1606,8 +1617,6 @@ cap_pipe_open_live(char *pipename, pcap_opts->cap_pipe_err = PIPNEXIST; return; } - - interface_opts = g_array_index(global_capture_opts.ifaces, interface_options, 0); #ifdef HAVE_EXTCAP extcap_pipe_name = g_strconcat("\\\\.\\pipe\\", EXTCAP_PIPE_PREFIX, NULL); extcap_pipe = strstr(interface_opts.name, extcap_pipe_name) ? TRUE : FALSE; @@ -1678,6 +1687,10 @@ cap_pipe_open_live(char *pipename, b = cap_pipe_read(fd, ((char *)&magic)+bytes_read, sizeof magic-bytes_read, pcap_opts->from_cap_socket); + /* jump messaging, if extcap had an error, stderr will provide the correct message */ + if (extcap_pipe && b <= 0) + goto error; + if (b <= 0) { if (b == 0) g_snprintf(errmsg, errmsgl, "End of file on pipe magic during open."); @@ -1704,6 +1717,10 @@ cap_pipe_open_live(char *pipename, /* We don't have to worry about cap_pipe_read_mtx here */ g_async_queue_push(pcap_opts->cap_pipe_pending_q, pcap_opts->cap_pipe_buf); g_async_queue_pop(pcap_opts->cap_pipe_done_q); + /* jump messaging, if extcap had an error, stderr will provide the correct message */ + if (pcap_opts->cap_pipe_bytes_read <= 0 && extcap_pipe) + goto error; + if (pcap_opts->cap_pipe_bytes_read <= 0) { if (pcap_opts->cap_pipe_bytes_read == 0) g_snprintf(errmsg, errmsgl, "End of file on pipe magic during open."); diff --git a/extcap.c b/extcap.c index 4fbf5d79e0..6087224df4 100644 --- a/extcap.c +++ b/extcap.c @@ -58,6 +58,7 @@ static HANDLE pipe_h = NULL; #endif #define EXTCAP_PREF_SIZE 256 +static void extcap_child_watch_cb(GPid pid, gint status _U_, gpointer user_data); /* internal container, for all the extcap interfaces that have been found. * will be resetted by every call to extcap_interface_list() and is being @@ -643,9 +644,44 @@ extcap_has_configuration(const char * ifname, gboolean is_required) { return found; } -void extcap_if_cleanup(capture_options * capture_opts) { +/* taken from capchild/capture_sync.c */ +static gboolean pipe_data_available(int pipe_fd) { +#ifdef _WIN32 /* PeekNamedPipe */ + HANDLE hPipe = (HANDLE) _get_osfhandle(pipe_fd); + DWORD bytes_avail; + + if (hPipe == INVALID_HANDLE_VALUE) + return FALSE; + + if (! PeekNamedPipe(hPipe, NULL, 0, NULL, &bytes_avail, NULL)) + return FALSE; + + if (bytes_avail > 0) + return TRUE; + return FALSE; +#else /* select */ + fd_set rfds; + struct timeval timeout; + + FD_ZERO(&rfds); + FD_SET(pipe_fd, &rfds); + timeout.tv_sec = 0; + timeout.tv_usec = 0; + + if (select(pipe_fd+1, &rfds, NULL, NULL, &timeout) > 0) + return TRUE; + + return FALSE; +#endif +} + +void extcap_if_cleanup(capture_options * capture_opts, gchar ** errormsg) { interface_options interface_opts; + extcap_userdata * userdata; guint icnt = 0; + gboolean overwrite_exitcode; + gchar * buffer; +#define STDERR_BUFFER_SIZE 1024 for (icnt = 0; icnt < capture_opts->ifaces->len; icnt++) { interface_opts = g_array_index(capture_opts->ifaces, interface_options, @@ -655,6 +691,8 @@ void extcap_if_cleanup(capture_options * capture_opts) { if (interface_opts.if_type != IF_EXTCAP) continue; + overwrite_exitcode = FALSE; + g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Extcap [%s] - Cleaning up fifo: %s; PID: %d", interface_opts.name, interface_opts.extcap_fifo, interface_opts.extcap_pid); @@ -681,6 +719,66 @@ void extcap_if_cleanup(capture_options * capture_opts) { "Extcap [%s] - Closing spawned PID: %d", interface_opts.name, interface_opts.extcap_pid); + userdata = (extcap_userdata *) interface_opts.extcap_userdata; + if ( userdata ) + { + if (userdata->extcap_stderr_rd > 0 && pipe_data_available(userdata->extcap_stderr_rd) ) + { + buffer = (gchar * )g_malloc0(sizeof(gchar) * STDERR_BUFFER_SIZE); +#ifdef _WIN32 + win32_readfrompipe((HANDLE)_get_osfhandle(userdata->extcap_stderr_rd), STDERR_BUFFER_SIZE, buffer); +#else + if (read(userdata->extcap_stderr_rd, buffer, sizeof(gchar) * STDERR_BUFFER_SIZE) <= 0 ) + buffer[0] = '\0'; +#endif + if ( strlen ( buffer) > 0 ) + { + userdata->extcap_stderr = g_strdup_printf("%s", buffer); + userdata->exitcode = 1; + } + g_free(buffer); + } + } + +#ifndef _WIN32 + /* Final child watch may not have been called */ + if ( interface_opts.extcap_child_watch != 0 ) + { + extcap_child_watch_cb(userdata->pid, 0, capture_opts); + /* it will have changed in extcap_child_watch_cb */ + interface_opts = g_array_index(capture_opts->ifaces, interface_options, + icnt); + } +#endif + + if ( userdata->extcap_stderr != NULL ) + overwrite_exitcode = TRUE; + + if ( overwrite_exitcode || userdata->exitcode != 0 ) + { + if ( userdata->extcap_stderr != 0 ) + { + if ( *errormsg == NULL ) + *errormsg = g_strdup_printf("Error by extcap pipe: %s", userdata->extcap_stderr); + else + { + gchar * temp = g_strconcat ( *errormsg, "\nError by extcap pipe: " ,userdata->extcap_stderr, NULL ); + g_free(*errormsg); + *errormsg = temp; + } + g_free (userdata->extcap_stderr ); + } + + userdata->extcap_stderr = NULL; + userdata->exitcode = 0; + } + + if (interface_opts.extcap_child_watch > 0) + { + g_source_remove(interface_opts.extcap_child_watch); + interface_opts.extcap_child_watch = 0; + } + if (interface_opts.extcap_pid != INVALID_EXTCAP_PID) { #ifdef _WIN32 @@ -688,6 +786,9 @@ void extcap_if_cleanup(capture_options * capture_opts) { #endif g_spawn_close_pid(interface_opts.extcap_pid); interface_opts.extcap_pid = INVALID_EXTCAP_PID; + + g_free(interface_opts.extcap_userdata); + interface_opts.extcap_userdata = NULL; } /* Make sure modified interface_opts is saved in capture_opts. */ @@ -713,11 +814,16 @@ extcap_add_arg_and_remove_cb(gpointer key, gpointer value, gpointer data) { return FALSE; } -static void extcap_child_watch_cb(GPid pid, gint status _U_, gpointer user_data) +void extcap_child_watch_cb(GPid pid, gint status _U_, gpointer user_data) { guint i; interface_options interface_opts; - capture_options *capture_opts = (capture_options *)user_data; + GError * error = 0; + extcap_userdata * userdata = NULL; + capture_options * capture_opts = (capture_options *)(user_data); + + if ( capture_opts == NULL || capture_opts->ifaces == NULL || capture_opts->ifaces->len == 0 ) + return; /* Close handle to child process. */ g_spawn_close_pid(pid); @@ -728,7 +834,16 @@ static void extcap_child_watch_cb(GPid pid, gint status _U_, gpointer user_data) interface_opts = g_array_index(capture_opts->ifaces, interface_options, i); if (interface_opts.extcap_pid == pid) { - interface_opts.extcap_pid = INVALID_EXTCAP_PID; + userdata = (extcap_userdata *)interface_opts.extcap_userdata; + if ( userdata != NULL ) + { + interface_opts.extcap_pid = INVALID_EXTCAP_PID; + userdata->exitcode = 0; + if ( ! g_spawn_check_exit_status(status, &error) ) + userdata->exitcode = error->code; + if ( status == 0 && userdata->extcap_stderr != NULL ) + userdata->exitcode = 1; + } g_source_remove(interface_opts.extcap_child_watch); interface_opts.extcap_child_watch = 0; @@ -844,6 +959,7 @@ extcap_init_interfaces(capture_options *capture_opts) { guint i; interface_options interface_opts; + extcap_userdata * userdata; for (i = 0; i < capture_opts->ifaces->len; i++) { @@ -863,22 +979,23 @@ extcap_init_interfaces(capture_options *capture_opts) /* Create extcap call */ args = extcap_prepare_arguments(interface_opts); - interface_opts.extcap_userdata = NULL; + userdata = g_new0(extcap_userdata, 1); - pid = extcap_spawn_async(&interface_opts, args ); + pid = extcap_spawn_async(userdata, args ); g_ptr_array_foreach(args, (GFunc)g_free, NULL); g_ptr_array_free(args, TRUE); if ( pid == INVALID_EXTCAP_PID ) + { + g_free(userdata); continue; + } interface_opts.extcap_pid = pid; interface_opts.extcap_child_watch = g_child_watch_add(pid, extcap_child_watch_cb, (gpointer)capture_opts); - capture_opts->ifaces = g_array_remove_index(capture_opts->ifaces, i); - g_array_insert_val(capture_opts->ifaces, i, interface_opts); #ifdef _WIN32 /* On Windows, wait for extcap to connect to named pipe. @@ -894,7 +1011,13 @@ extcap_init_interfaces(capture_options *capture_opts) { extcap_wait_for_pipe(pipe_h, pid); } + #endif + + interface_opts.extcap_userdata = (gpointer) userdata; + + capture_opts->ifaces = g_array_remove_index(capture_opts->ifaces, i); + g_array_insert_val(capture_opts->ifaces, i, interface_opts); } return TRUE; diff --git a/extcap.h b/extcap.h index d8e94b9fad..254bd9fa3a 100644 --- a/extcap.h +++ b/extcap.h @@ -28,8 +28,6 @@ #include -#include "ws_symbol_export.h" - #ifdef _WIN32 #include #include @@ -102,7 +100,7 @@ extcap_create_pipe(char ** fifo); /* Clean up all if related stuff */ void -extcap_if_cleanup(capture_options * capture_opts _U_); +extcap_if_cleanup(capture_options * capture_opts _U_, gchar ** errormsg); struct preference * extcap_pref_for_argument(const gchar *ifname, struct _extcap_arg * arg); diff --git a/extcap_spawn.c b/extcap_spawn.c index 555958c687..484269370c 100644 --- a/extcap_spawn.c +++ b/extcap_spawn.c @@ -197,7 +197,7 @@ gboolean extcap_spawn_sync ( gchar * dirname, gchar * command, gint argc, gchar return result; } -GPid extcap_spawn_async(interface_options * interface, GPtrArray * args) +GPid extcap_spawn_async(extcap_userdata * userdata, GPtrArray * args) { GPid pid = INVALID_EXTCAP_PID; @@ -221,12 +221,6 @@ GPid extcap_spawn_async(interface_options * interface, GPtrArray * args) const gchar * oldpath = g_getenv("PATH"); gchar * newpath = NULL; -#endif - - extcap_userdata * userdata = NULL; - userdata = (extcap_userdata *) g_malloc0(sizeof(extcap_userdata)); - -#ifdef _WIN32 newpath = g_strdup_printf("%s;%s", g_strescape(get_progfile_dir(), NULL), oldpath); g_setenv("PATH", newpath, TRUE); @@ -279,12 +273,12 @@ GPid extcap_spawn_async(interface_options * interface, GPtrArray * args) g_setenv("PATH", oldpath, TRUE); #else - g_spawn_async(NULL, (gchar **)args->pdata, NULL, (GSpawnFlags) G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, - &pid, NULL); + g_spawn_async_with_pipes(NULL, (gchar **)args->pdata, NULL, + (GSpawnFlags) G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, + &pid, NULL, &userdata->extcap_stdout_rd, &userdata->extcap_stderr_rd, NULL); #endif userdata->pid = pid; - interface->extcap_userdata = userdata; return pid; } diff --git a/extcap_spawn.h b/extcap_spawn.h index b9b2ef491e..16d8a97a58 100644 --- a/extcap_spawn.h +++ b/extcap_spawn.h @@ -44,7 +44,7 @@ typedef struct _extcap_userdata { gboolean extcap_spawn_sync ( gchar * dirname, gchar * command, gint argc, gchar ** argv, gchar ** command_output ); -GPid extcap_spawn_async ( interface_options * interface, GPtrArray * args ); +GPid extcap_spawn_async ( extcap_userdata * userdata, GPtrArray * args ); #ifdef _WIN32 gboolean extcap_wait_for_pipe(HANDLE pipe_h, HANDLE pid); -- cgit v1.2.1