From 588a7c34bb6cdffa8ec40d1e7fda047c5da38a61 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 19 Mar 2009 07:09:31 +0000 Subject: [PATCH] Make sure not to leak file descriptors if running gpg-agent with a command. Restore the signal mask to solve a problem in Mono. --- ChangeLog | 4 ++ NEWS | 3 + agent/ChangeLog | 5 ++ agent/gpg-agent.c | 51 ++++++++++++++ common/ChangeLog | 10 +++ common/Makefile.am | 3 +- common/exechelp.c | 166 +++++++++++++++++++++++++++++++++++++++++---- common/exechelp.h | 21 +++++- configure.ac | 2 +- scd/ChangeLog | 2 + scd/apdu.c | 13 +--- 11 files changed, 252 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index cba8be582..33d014c2e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2009-03-18 Werner Koch + + * configure.ac: Test for getrlimit. + 2009-03-03 Werner Koch Release 2.0.11. diff --git a/NEWS b/NEWS index bfa343bb4..ea1763cde 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,9 @@ Noteworthy changes in version 2.0.12 * New command "KEYINFO" for GPG_AGENT. GPGSM now also returns information about smartcards. + * Make sure not to leak file descriptors if running gpg-agent with a + command. Restore the signal mask to solve a problem in Mono. + Noteworthy changes in version 2.0.11 (2009-03-03) ------------------------------------------------- diff --git a/agent/ChangeLog b/agent/ChangeLog index 25aa478e8..526c2d649 100644 --- a/agent/ChangeLog +++ b/agent/ChangeLog @@ -1,3 +1,8 @@ +2009-03-19 Werner Koch + + * gpg-agent.c (main): Save signal mask and open fds. Restore mask + and close all fds prior to the exec. Fixes bug#1013. + 2009-03-17 Werner Koch * command.c (cmd_get_passphrase): Break repeat loop on error. diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c index 6cb08409e..e5372c4f9 100644 --- a/agent/gpg-agent.c +++ b/agent/gpg-agent.c @@ -47,6 +47,7 @@ #include "sysutils.h" #include "setenv.h" #include "gc-opt-flags.h" +#include "exechelp.h" enum cmd_and_opt_values @@ -197,6 +198,17 @@ static ARGPARSE_OPTS opts[] = { #define TIMERTICK_INTERVAL (2) /* Seconds. */ #endif + +/* The list of open file descriptors at startup. Note that this list + has been allocated using the standard malloc. */ +static int *startup_fd_list; + +/* The signal mask at startup and a flag telling whether it is valid. */ +#ifdef HAVE_SIGPROCMASK +static sigset_t startup_signal_mask; +static int startup_signal_mask_valid; +#endif + /* Flag to indicate that a shutdown was requested. */ static int shutdown_pending; @@ -530,6 +542,16 @@ main (int argc, char **argv ) const char *env_file_name = NULL; + /* Before we do anything else we save the list of currently open + file descriptors and the signal mask. This info is required to + do the exec call properly. */ + startup_fd_list = get_all_open_fds (); +#ifdef HAVE_SIGPROCMASK + if (!sigprocmask (SIG_UNBLOCK, NULL, &startup_signal_mask)) + startup_signal_mask_valid = 1; +#endif /*HAVE_SIGPROCMASK*/ + + /* Set program name etc. */ set_strusage (my_strusage); gcry_control (GCRYCTL_SUSPEND_SECMEM_WARN); /* Please note that we may running SUID(ROOT), so be very CAREFUL @@ -961,6 +983,27 @@ main (int argc, char **argv ) close (fd); + /* Note that we used a standard fork so that Pth runs in + both the parent and the child. The pth_fork would + terminate Pth in the child but that is not the way we + want it. Thus we use a plain fork and terminate Pth here + in the parent. The pth_kill may or may not work reliable + but it should not harm to call it. Because Pth fiddles + with the signal mask the signal mask might not be correct + right now and thus we restore it. That is not strictly + necessary but some programs falsely assume a cleared + signal mask. */ +#ifdef HAVE_SIGPROCMASK + if (startup_signal_mask_valid) + { + if (sigprocmask (SIG_SETMASK, &startup_signal_mask, NULL)) + log_error ("error restoring signal mask: %s\n", + strerror (errno)); + } + else + log_info ("no saved signal mask\n"); +#endif /*HAVE_SIGPROCMASK*/ + /* Create the info string: :: */ if (asprintf (&infostr, "GPG_AGENT_INFO=%s:%lu:1", socket_name, (ulong)pid ) < 0) @@ -1039,6 +1082,14 @@ main (int argc, char **argv ) kill (pid, SIGTERM ); exit (1); } + + /* Close all the file descriptors except the standard + ones and those open at startup. We explicitly don't + close 0,1,2 in case something went wrong collecting + them at startup. */ + close_all_fds (3, startup_fd_list); + + /* Run the command. */ execvp (argv[0], argv); log_error ("failed to run the command: %s\n", strerror (errno)); kill (pid, SIGTERM); diff --git a/common/ChangeLog b/common/ChangeLog index 986041f59..1cf4f50fc 100644 --- a/common/ChangeLog +++ b/common/ChangeLog @@ -1,3 +1,13 @@ +2009-03-18 Werner Koch + + * exechelp.c: Include sys/resource.h and sys/stat.h. + (get_max_open_fds): New. + (do_exec): Use it. + (get_all_open_fds): New. + (close_all_fds): New. + (do_exec): Use close_all_fds. + * t-exechelp.c: New. + 2009-03-13 David Shaw * http.c (do_parse_uri): Properly handle IPv6 literal addresses as diff --git a/common/Makefile.am b/common/Makefile.am index d1a2d4822..72c7d179f 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -108,7 +108,7 @@ status-codes.h: Makefile mkstrtable.awk exstatus.awk status.h # # Module tests # -module_tests = t-convert t-percent t-gettime t-sysutils t-sexputil +module_tests = t-convert t-percent t-gettime t-sysutils t-sexputil t-exechelp module_maint_tests = t-helpfile t-b64 t_common_ldadd = libcommon.a ../jnlib/libjnlib.a ../gl/libgnu.a \ @@ -121,6 +121,7 @@ t_sysutils_LDADD = $(t_common_ldadd) t_helpfile_LDADD = $(t_common_ldadd) t_sexputil_LDADD = $(t_common_ldadd) t_b64_LDADD = $(t_common_ldadd) +t_exechelp_LDADD = $(t_common_ldadd) diff --git a/common/exechelp.c b/common/exechelp.c index 4da3c9787..3d1136609 100644 --- a/common/exechelp.c +++ b/common/exechelp.c @@ -1,5 +1,5 @@ /* exechelp.c - fork and exec helpers - * Copyright (C) 2004, 2007, 2008 Free Software Foundation, Inc. + * Copyright (C) 2004, 2007, 2008, 2009 Free Software Foundation, Inc. * * This file is part of GnuPG. * @@ -40,6 +40,16 @@ #include #endif +#ifdef HAVE_GETRLIMIT +#include +#include +#endif /*HAVE_GETRLIMIT*/ + +#ifdef HAVE_STAT +# include +#endif + + #include "util.h" #include "i18n.h" #include "exechelp.h" @@ -48,12 +58,6 @@ #define DEBUG_W32_SPAWN 1 -#ifdef _POSIX_OPEN_MAX -#define MAX_OPEN_FDS _POSIX_OPEN_MAX -#else -#define MAX_OPEN_FDS 20 -#endif - /* We have the usual problem here: Some modules are linked against pth and some are not. However we want to use pth_fork and pth_waitpid here. Using a weak symbol works but is not portable - we should @@ -85,6 +89,145 @@ #endif +/* Return the maximum number of currently allowed open file + descriptors. Only useful on POSIX systems but returns a value on + other systems too. */ +int +get_max_fds (void) +{ + int max_fds = -1; +#ifdef HAVE_GETRLIMIT + struct rlimit rl; + +# ifdef RLIMIT_NOFILE + if (!getrlimit (RLIMIT_NOFILE, &rl)) + max_fds = rl.rlim_max; +# endif + +# ifdef RLIMIT_OFILE + if (max_fds == -1 && !getrlimit (RLIMIT_OFILE, &rl)) + max_fds = rl.rlim_max; + +# endif +#endif /*HAVE_GETRLIMIT*/ + +#ifdef _SC_OPEN_MAX + if (max_fds == -1) + { + long int scres = sysconf (_SC_OPEN_MAX); + if (scres >= 0) + max_fds = scres; + } +#endif + +#ifdef _POSIX_OPEN_MAX + if (max_fds == -1) + max_fds = _POSIX_OPEN_MAX; +#endif + +#ifdef OPEN_MAX + if (max_fds == -1) + max_fds = OPEN_MAX; +#endif + + if (max_fds == -1) + max_fds = 256; /* Arbitrary limit. */ + + return max_fds; +} + + +/* Close all file descriptors starting with descriptor FIRST. If + EXCEPT is not NULL, it is expected to be a list of file descriptors + which shall not be closed. This list shall be sorted in ascending + order with the end marked by -1. */ +void +close_all_fds (int first, int *except) +{ + int max_fd = get_max_fds (); + int fd, i, except_start; + + if (except) + { + except_start = 0; + for (fd=first; fd < max_fd; fd++) + { + for (i=except_start; except[i] != -1; i++) + { + if (except[i] == fd) + { + /* If we found the descriptor in the exception list + we can start the next compare run at the next + index because the exception list is ordered. */ + except_start = i + 1; + break; + } + } + if (except[i] == -1) + close (fd); + } + } + else + { + for (fd=first; fd < max_fd; fd++) + close (fd); + } + + errno = 0; +} + + +/* Returns an array with all currently open file descriptors. The end + of the array is marked by -1. The caller needs to release this + array using the *standard free* and not with xfree. This allow the + use of this fucntion right at startup even before libgcrypt has + been initialized. Returns NULL on error and sets ERRNO + accordingly. */ +int * +get_all_open_fds (void) +{ + int *array; + size_t narray; + int fd, max_fd, idx; +#ifndef HAVE_STAT + array = calloc (1, sizeof *array); + if (array) + array[0] = -1; +#else /*HAVE_STAT*/ + struct stat statbuf; + + max_fd = get_max_fds (); + narray = 32; /* If you change this change also t-exechelp.c. */ + array = calloc (narray, sizeof *array); + if (!array) + return NULL; + + /* Note: The list we return is ordered. */ + for (idx=0, fd=0; fd < max_fd; fd++) + if (!(fstat (fd, &statbuf) == -1 && errno == EBADF)) + { + if (idx+1 >= narray) + { + int *tmp; + + narray += (narray < 256)? 32:256; + tmp = realloc (array, narray * sizeof *array); + if (!tmp) + { + free (array); + return NULL; + } + array = tmp; + } + array[idx++] = fd; + } + array[idx] = -1; +#endif /*HAVE_STAT*/ + return array; +} + + + #ifdef HAVE_W32_SYSTEM /* Helper function to build_w32_commandline. */ static char * @@ -216,7 +359,7 @@ do_exec (const char *pgmname, const char *argv[], void (*preexec)(void) ) { char **arg_list; - int n, i, j; + int i, j; int fds[3]; fds[0] = fd_in; @@ -259,12 +402,7 @@ do_exec (const char *pgmname, const char *argv[], } /* Close all other files. */ - n = sysconf (_SC_OPEN_MAX); - if (n < 0) - n = MAX_OPEN_FDS; - for (i=3; i < n; i++) - close(i); - errno = 0; + close_all_fds (3, NULL); if (preexec) preexec (); diff --git a/common/exechelp.h b/common/exechelp.h index 0ad956f7a..0efee294c 100644 --- a/common/exechelp.h +++ b/common/exechelp.h @@ -1,5 +1,5 @@ /* exechelp.h - Definitions for the fork and exec helpers - * Copyright (C) 2004 Free Software Foundation, Inc. + * Copyright (C) 2004, 2009 Free Software Foundation, Inc. * * This file is part of GnuPG. * @@ -20,6 +20,25 @@ #ifndef GNUPG_COMMON_EXECHELP_H #define GNUPG_COMMON_EXECHELP_H +/* Return the maximum number of currently allowed file descriptors. + Only useful on POSIX systems. */ +int get_max_fds (void); + + +/* Close all file descriptors starting with descriptor FIRST. If + EXCEPT is not NULL, it is expected to be a list of file descriptors + which are not to close. This list shall be sorted in ascending + order with its end marked by -1. */ +void close_all_fds (int first, int *except); + + +/* Returns an array with all currently open file descriptors. The end + of the array is marked by -1. The caller needs to release this + array using the *standard free* and not with xfree. This allow the + use of this fucntion right at startup even before libgcrypt has + been initialized. Returns NULL on error and sets ERRNO accordingly. */ +int *get_all_open_fds (void); + /* Portable function to create a pipe. Under Windows the write end is inheritable. */ diff --git a/configure.ac b/configure.ac index 0b04c5d10..0dd26cdbe 100644 --- a/configure.ac +++ b/configure.ac @@ -1059,7 +1059,7 @@ AC_FUNC_FORK AC_CHECK_FUNCS([strerror strlwr tcgetattr mmap]) AC_CHECK_FUNCS([strcasecmp strncasecmp ctermid times gmtime_r]) AC_CHECK_FUNCS([unsetenv getpwnam getpwuid fcntl ftruncate]) -AC_CHECK_FUNCS([gettimeofday getrusage setrlimit clock_gettime]) +AC_CHECK_FUNCS([gettimeofday getrusage getrlimit setrlimit clock_gettime]) AC_CHECK_FUNCS([atexit raise getpagesize strftime nl_langinfo setlocale]) AC_CHECK_FUNCS([waitpid wait4 sigaction sigprocmask pipe stat getaddrinfo]) AC_CHECK_FUNCS([ttyname rand ftello]) diff --git a/scd/ChangeLog b/scd/ChangeLog index 6ae7da589..28fd31e17 100644 --- a/scd/ChangeLog +++ b/scd/ChangeLog @@ -1,5 +1,7 @@ 2009-03-18 Werner Koch + * apdu.c (open_pcsc_reader_wrapped): Use close_all_fds. + * command.c (cmd_learn): Add option --keypairinfo. * app.c (app_write_learn_status): Add arg FLAGS. * app-common.h (struct app_ctx_s): Add arg FLAGS to LEARN_STATUS. diff --git a/scd/apdu.c b/scd/apdu.c index bbd7ea801..dfddd3f72 100644 --- a/scd/apdu.c +++ b/scd/apdu.c @@ -57,6 +57,7 @@ #include "cardglue.h" #else /* GNUPG_MAJOR_VERSION != 1 */ #include "scdaemon.h" +#include "exechelp.h" #endif /* GNUPG_MAJOR_VERSION != 1 */ #include "apdu.h" @@ -81,11 +82,6 @@ #define DLSTDCALL #endif -#ifdef _POSIX_OPEN_MAX -#define MAX_OPEN_FDS _POSIX_OPEN_MAX -#else -#define MAX_OPEN_FDS 20 -#endif /* Helper to pass parameters related to keypad based operations. */ struct pininfo_s @@ -1653,12 +1649,7 @@ open_pcsc_reader_wrapped (const char *portstr) log_fatal ("dup2 stderr failed: %s\n", strerror (errno)); /* Close all other files. */ - n = sysconf (_SC_OPEN_MAX); - if (n < 0) - n = MAX_OPEN_FDS; - for (i=3; i < n; i++) - close(i); - errno = 0; + close_all_fds (3, NULL); execl (wrapperpgm, "pcsc-wrapper",