From f1dac8851d02a0cb63fc7379ee74692856d0cf39 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 3 Jun 2005 13:57:24 +0000 Subject: [PATCH] * command.c (cmd_updatestartuptty): New. * gpg-agent.c: New option --write-env-file. * gpg-agent.c (handle_connections): Make sure that the signals we are handling are not blocked.Block signals while creating new threads. * estream.c: Use HAVE_CONFIG_H and not USE_CONFIG_H! (es_func_fd_read, es_func_fd_write): Protect against EINTR. * gpg-agent.texi (Agent UPDATESTARTUPTTY): New. * scdaemon.c (handle_connections): Make sure that the signals we are handling are not blocked.Block signals while creating new threads. (handle_connections): Include the file descriptor into the name of the thread. --- ChangeLog | 5 ++ NEWS | 3 + TODO | 7 ++ agent/ChangeLog | 17 +++++ agent/agent.h | 14 ++-- agent/call-scd.c | 30 ++++++++ agent/command.c | 34 +++++++++ agent/gpg-agent.c | 104 ++++++++++++++++++++++------ common/ChangeLog | 6 ++ common/estream.c | 10 ++- configure.ac | 3 + doc/ChangeLog | 4 ++ doc/gpg-agent.texi | 60 +++++++++++++--- scd/ChangeLog | 29 ++++++++ scd/app-common.h | 6 ++ scd/app-openpgp.c | 2 +- scd/app.c | 167 ++++++++++++++++++++++++++++++++------------- scd/command.c | 6 +- scd/scdaemon.c | 33 +++++++-- scd/scdaemon.h | 23 +++++-- 20 files changed, 459 insertions(+), 104 deletions(-) diff --git a/ChangeLog b/ChangeLog index fbd9ad79d..f7efcee89 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2005-06-02 Werner Koch + + * configure.ac (HAVE_PTH): Define as alias for USE_GNU_PTH. It is + used by common/estream.c. + 2005-06-01 Werner Koch * configure.ac (gl_INIT): Add gnulib stuff. diff --git a/NEWS b/NEWS index 79a74cbe4..e28f1284a 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,9 @@ Noteworthy changes in version 1.9.17 does allows only signing using TCOS cards but we are going to enhance it to match all the old capabilities. + * [gpg-agent] New option --rite-env-file and Assuan command + UPDATESTARTUPTTY. + Noteworthy changes in version 1.9.16 (2005-04-21) ------------------------------------------------- diff --git a/TODO b/TODO index 74763a71f..5f1b57a0f 100644 --- a/TODO +++ b/TODO @@ -72,6 +72,13 @@ might want to have an agent context for each service request we can change all S-expression handling code to make use of this function. +* scd +** Application context vs. reader slot + We have 2 concurrent method of tracking whether a read is in use: + Using the session_list in command.c and the lock_table in app.c. IT + would be better to do this just at one place. First we need to see + how we can support cards with multiple applications. + * tests ** Makefile.am We use printf(1) to setup the library path, this is not portable. diff --git a/agent/ChangeLog b/agent/ChangeLog index 9c57ad43e..9621e5de0 100644 --- a/agent/ChangeLog +++ b/agent/ChangeLog @@ -1,3 +1,20 @@ +2005-06-03 Werner Koch + + * command.c (cmd_updatestartuptty): New. + + * gpg-agent.c: New option --write-env-file. + + * gpg-agent.c (handle_connections): Make sure that the signals we + are handling are not blocked.Block signals while creating new + threads. + +2005-06-02 Werner Koch + + * call-scd.c (agent_scd_dump_state, dump_mutex_state): New. + * gpg-agent.c (handle_signal): Print it on SIGUSR1. + (handle_connections): Include the file descriptor into the + threadnames. + 2005-06-01 Werner Koch * gpg-agent.c: Include setenv.h. diff --git a/agent/agent.h b/agent/agent.h index a667c0d46..51e66abee 100644 --- a/agent/agent.h +++ b/agent/agent.h @@ -54,12 +54,13 @@ struct { int batch; /* Batch mode */ const char *homedir; /* Configuration directory name */ - /* Environment setting gathred at program start. */ - const char *startup_display; - const char *startup_ttyname; - const char *startup_ttytype; - const char *startup_lc_ctype; - const char *startup_lc_messages; + /* Environment setting gathered at program start or hanged using the + Assuan command UPDATESTARTUPTTY. */ + char *startup_display; + char *startup_ttyname; + char *startup_ttytype; + char *startup_lc_ctype; + char *startup_lc_messages; const char *pinentry_program; /* Filename of the program to start as @@ -248,6 +249,7 @@ int divert_generic_cmd (ctrl_t ctrl, /*-- call-scd.c --*/ void initialize_module_call_scd (void); +void agent_scd_dump_state (void); void agent_scd_check_aliveness (void); int agent_reset_scd (ctrl_t ctrl); int agent_card_learn (ctrl_t ctrl, diff --git a/agent/call-scd.c b/agent/call-scd.c index 78e28fe97..00c9df2a7 100644 --- a/agent/call-scd.c +++ b/agent/call-scd.c @@ -116,6 +116,35 @@ initialize_module_call_scd (void) } +static void +dump_mutex_state (pth_mutex_t *m) +{ + if (!(m->mx_state & PTH_MUTEX_INITIALIZED)) + log_printf ("not_initialized"); + else if (!(m->mx_state & PTH_MUTEX_LOCKED)) + log_printf ("not_locked"); + else + log_printf ("locked tid=0x%lx count=%lu", (long)m->mx_owner, m->mx_count); +} + + +/* This function may be called to print infromation pertaining to the + current state of this module to the log. */ +void +agent_scd_dump_state (void) +{ + log_info ("agent_scd_dump_state: scd_lock="); + dump_mutex_state (&start_scd_lock); + log_printf ("\n"); + log_info ("agent_scd_dump_state: primary_scd_ctx=%p pid=%ld reusable=%d\n", + primary_scd_ctx, + (long)assuan_get_pid (primary_scd_ctx), + primary_scd_ctx_reusable); + if (socket_name) + log_info ("agent_scd_dump_state: socket=`%s'\n", socket_name); +} + + /* The unlock_scd function shall be called after having accessed the SCD. It is currently not very useful but gives an opportunity to keep track of connections currently calling SCD. Note that the @@ -384,6 +413,7 @@ agent_scd_check_aliveness (void) } + /* Reset the SCD if it has been used. */ int agent_reset_scd (ctrl_t ctrl) diff --git a/agent/command.c b/agent/command.c index 8af159f6d..56167118d 100644 --- a/agent/command.c +++ b/agent/command.c @@ -867,6 +867,39 @@ cmd_scd (ASSUAN_CONTEXT ctx, char *line) } + +/* UPDATESTARTUPTTY + + Set startup TTY and X DISPLAY variables to the values of this + session. This command is useful to pull future pinentries to + another screen. It is only required because there is no way in the + ssh-agent protocol to convey this information. */ +static int +cmd_updatestartuptty (assuan_context_t ctx, char *line) +{ + ctrl_t ctrl = assuan_get_pointer (ctx); + + xfree (opt.startup_display); opt.startup_display = NULL; + xfree (opt.startup_ttyname); opt.startup_ttyname = NULL; + xfree (opt.startup_ttytype); opt.startup_ttytype = NULL; + xfree (opt.startup_lc_ctype); opt.startup_lc_ctype = NULL; + xfree (opt.startup_lc_messages); opt.startup_lc_messages = NULL; + + if (ctrl->display) + opt.startup_display = xtrystrdup (ctrl->display); + if (ctrl->ttyname) + opt.startup_ttyname = xtrystrdup (ctrl->ttyname); + if (ctrl->ttytype) + opt.startup_ttytype = xtrystrdup (ctrl->ttytype); + if (ctrl->lc_ctype) + opt.startup_lc_ctype = xtrystrdup (ctrl->lc_ctype); + if (ctrl->lc_messages) + opt.startup_lc_messages = xtrystrdup (ctrl->lc_messages); + + return 0; +} + + static int option_handler (ASSUAN_CONTEXT ctx, const char *key, const char *value) @@ -957,6 +990,7 @@ register_commands (ASSUAN_CONTEXT ctx) { "INPUT", NULL }, { "OUTPUT", NULL }, { "SCD", cmd_scd }, + { "UPDATESTARTUPTTY", cmd_updatestartuptty }, { NULL } }; int i, rc; diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c index 3537b07f0..90b071d5e 100644 --- a/agent/gpg-agent.c +++ b/agent/gpg-agent.c @@ -93,7 +93,8 @@ enum cmd_and_opt_values oKeepTTY, oKeepDISPLAY, oSSHSupport, - oDisableScdaemon + oDisableScdaemon, + oWriteEnvFile }; @@ -147,6 +148,8 @@ static ARGPARSE_OPTS opts[] = { { oAllowPresetPassphrase, "allow-preset-passphrase", 0, N_("allow presetting passphrase")}, { oSSHSupport, "enable-ssh-support", 0, N_("enable ssh-agent emulation") }, + { oWriteEnvFile, "write-env-file", 2, + N_("|FILE|write environment settings also to FILE")}, {0} }; @@ -438,6 +441,7 @@ main (int argc, char **argv ) int gpgconf_list = 0; int standard_socket = 0; gpg_error_t err; + const char *env_file_name = NULL; set_strusage (my_strusage); gcry_control (GCRYCTL_SUSPEND_SECMEM_WARN); @@ -501,7 +505,7 @@ main (int argc, char **argv ) opt.startup_ttytype = getenv ("TERM"); if (opt.startup_ttytype) opt.startup_ttytype = xstrdup (opt.startup_ttytype); - /* Fixme: Neen to use the locale fucntion here. */ + /* Fixme: Better use the locale function here. */ opt.startup_lc_ctype = getenv ("LC_CTYPE"); if (opt.startup_lc_ctype) opt.startup_lc_ctype = xstrdup (opt.startup_lc_ctype); @@ -619,6 +623,7 @@ main (int argc, char **argv ) case oKeepDISPLAY: opt.keep_display = 1; break; case oSSHSupport: opt.ssh_support = 1; break; + case oWriteEnvFile: env_file_name = pargs.r.ret_str; break; default : pargs.err = configfp? 1:2; break; } @@ -855,6 +860,29 @@ main (int argc, char **argv ) if (opt.ssh_support) *socket_name_ssh = 0; + if (env_file_name) + { + FILE *fp; + + fp = fopen (env_file_name, "w"); + if (!fp) + log_error (_("error creating `%s': %s\n"), + env_file_name, strerror (errno)); + else + { + fputs (infostr, fp); + putc ('\n', fp); + if (opt.ssh_support) + { + fputs (infostr_ssh_sock, fp); + putc ('\n', fp); + fputs (infostr_ssh_pid, fp); + putc ('\n', fp); + } + fclose (fp); + } + } + if (argc) { /* Run the program given on the commandline. */ @@ -1273,7 +1301,7 @@ create_directories (void) static void handle_tick (void) { - /* Check whether the scdaemon has dies and cleanup in this case. */ + /* Check whether the scdaemon has died and cleanup in this case. */ agent_scd_check_aliveness (); /* If we are running as a child of another process, check whether @@ -1311,6 +1339,7 @@ handle_signal (int signo) case SIGUSR1: log_info ("SIGUSR1 received - printing internal information:\n"); pth_ctrl (PTH_CTRL_DUMPSTATE, log_get_stream ()); + agent_scd_dump_state (); break; case SIGUSR2: @@ -1353,7 +1382,8 @@ start_connection_thread (void *arg) int fd = (int)arg; if (opt.verbose) - log_info (_("handler for fd %d started\n"), fd); + log_info (_("handler 0x%lx for fd %d started\n"), + (long)pth_self (), fd); /* FIXME: Move this housekeeping into a ticker function. Calling it for each connection should work but won't work anymore if our @@ -1362,7 +1392,8 @@ start_connection_thread (void *arg) start_command_handler (-1, fd); if (opt.verbose) - log_info (_("handler for fd %d terminated\n"), fd); + log_info (_("handler 0x%lx for fd %d terminated\n"), + (long)pth_self (), fd); return NULL; } @@ -1375,13 +1406,15 @@ start_connection_thread_ssh (void *arg) int fd = (int)arg; if (opt.verbose) - log_info (_("ssh handler for fd %d started\n"), fd); + log_info (_("ssh handler 0x%lx for fd %d started\n"), + (long)pth_self (), fd); agent_trustlist_housekeeping (); start_command_handler_ssh (fd); if (opt.verbose) - log_info (_("ssh handler for fd %d terminated\n"), fd); + log_info (_("ssh handler 0x%lx for fd %d terminated\n"), + (long)pth_self (), fd); return NULL; } @@ -1405,15 +1438,17 @@ handle_connections (int listen_fd, int listen_fd_ssh) tattr = pth_attr_new(); pth_attr_set (tattr, PTH_ATTR_JOINABLE, 0); pth_attr_set (tattr, PTH_ATTR_STACK_SIZE, 256*1024); - pth_attr_set (tattr, PTH_ATTR_NAME, "gpg-agent"); #ifndef HAVE_W32_SYSTEM /* fixme */ + /* Make sure that the signals we are going to handle are not blocked + and create an event object for them. */ sigemptyset (&sigs ); sigaddset (&sigs, SIGHUP); sigaddset (&sigs, SIGUSR1); sigaddset (&sigs, SIGUSR2); sigaddset (&sigs, SIGINT); sigaddset (&sigs, SIGTERM); + pth_sigmask (SIG_UNBLOCK, &sigs, NULL); ev = pth_event (PTH_EVENT_SIGS, &sigs, &signo); #else ev = NULL; @@ -1427,6 +1462,8 @@ handle_connections (int listen_fd, int listen_fd_ssh) for (;;) { + sigset_t oldsigs; + if (shutdown_pending) { if (pth_ctrl (PTH_CTRL_GETTHREADS) == 1) @@ -1488,6 +1525,12 @@ handle_connections (int listen_fd, int listen_fd_ssh) handle_tick (); } + + /* We now might create new threads and because we don't want any + signals - we are handling here - to be delivered to a new + thread. Thus we need to block those signals. */ + pth_sigmask (SIG_BLOCK, &sigs, &oldsigs); + if (FD_ISSET (listen_fd, &read_fdset)) { plen = sizeof paddr; @@ -1496,12 +1539,20 @@ handle_connections (int listen_fd, int listen_fd_ssh) { log_error ("accept failed: %s\n", strerror (errno)); } - else if (!pth_spawn (tattr, start_connection_thread, (void*)fd)) - { - log_error ("error spawning connection handler: %s\n", - strerror (errno) ); - close (fd); - } + else + { + char threadname[50]; + snprintf (threadname, sizeof threadname-1, + "conn fd=%d (gpg)", fd); + threadname[sizeof threadname -1] = 0; + pth_attr_set (tattr, PTH_ATTR_NAME, threadname); + if (!pth_spawn (tattr, start_connection_thread, (void*)fd)) + { + log_error ("error spawning connection handler: %s\n", + strerror (errno) ); + close (fd); + } + } fd = -1; } @@ -1513,14 +1564,27 @@ handle_connections (int listen_fd, int listen_fd_ssh) { log_error ("accept failed for ssh: %s\n", strerror (errno)); } - else if (!pth_spawn (tattr, start_connection_thread_ssh, (void*)fd)) - { - log_error ("error spawning ssh connection handler: %s\n", - strerror (errno) ); - close (fd); - } + else + { + char threadname[50]; + snprintf (threadname, sizeof threadname-1, + "conn fd=%d (ssh)", fd); + threadname[sizeof threadname -1] = 0; + pth_attr_set (tattr, PTH_ATTR_NAME, threadname); + + if (!pth_spawn (tattr, start_connection_thread_ssh, (void*)fd)) + { + log_error ("error spawning ssh connection handler: %s\n", + strerror (errno) ); + close (fd); + } + } fd = -1; } + + /* Restore the signal mask. */ + pth_sigmask (SIG_SETMASK, &oldsigs, NULL); + } pth_event_free (ev, PTH_FREE_ALL); diff --git a/common/ChangeLog b/common/ChangeLog index fccc71d49..08fb06775 100644 --- a/common/ChangeLog +++ b/common/ChangeLog @@ -1,3 +1,9 @@ +2005-06-03 Werner Koch + + * estream.c: Use HAVE_CONFIG_H and not USE_CONFIG_H! + (es_func_fd_read, es_func_fd_write): Protect against EINTR. + + 2005-06-01 Werner Koch * Makefile.am (AM_CPPFLAGS): Added. diff --git a/common/estream.c b/common/estream.c index 00cb749e8..bf5b02001 100644 --- a/common/estream.c +++ b/common/estream.c @@ -22,7 +22,7 @@ # include #endif -#ifdef USE_CONFIG_H +#ifdef HAVE_CONFIG_H # include #endif @@ -597,7 +597,9 @@ es_func_fd_read (void *cookie, char *buffer, size_t size) estream_cookie_fd_t file_cookie = cookie; ssize_t bytes_read; - bytes_read = ESTREAM_SYS_READ (file_cookie->fd, buffer, size); + do + bytes_read = ESTREAM_SYS_READ (file_cookie->fd, buffer, size); + while (bytes_read == -1 && errno == EINTR); return bytes_read; } @@ -610,7 +612,9 @@ es_func_fd_write (void *cookie, const char *buffer, size_t size) estream_cookie_fd_t file_cookie = cookie; ssize_t bytes_written; - bytes_written = ESTREAM_SYS_WRITE (file_cookie->fd, buffer, size); + do + bytes_written = ESTREAM_SYS_WRITE (file_cookie->fd, buffer, size); + while (bytes_written == -1 && errno == EINTR); return bytes_written; } diff --git a/configure.ac b/configure.ac index c1a3d77ea..17465c520 100644 --- a/configure.ac +++ b/configure.ac @@ -531,6 +531,8 @@ if test "$have_w32_system" = no; then PTH_LIBS="$PTH_LIBS `$PTH_CONFIG --libs`" AC_DEFINE(USE_GNU_PTH, 1, [Defined if the GNU Portable Thread Library should be used]) + AC_DEFINE(HAVE_PTH, 1, + [Defined if the GNU Pth is available]) fi fi else @@ -538,6 +540,7 @@ else PTH_CFLAGS="" PTH_LIBS="" AC_DEFINE(USE_GNU_PTH, 1) + AC_DEFINE(HAVE_PTH, 1) fi AC_SUBST(PTH_CFLAGS) AC_SUBST(PTH_LIBS) diff --git a/doc/ChangeLog b/doc/ChangeLog index 25840a5b1..f353bdf03 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,7 @@ +2005-06-03 Werner Koch + + * gpg-agent.texi (Agent UPDATESTARTUPTTY): New. + 2005-05-17 Werner Koch * gpg-agent.texi (Agent Options): Removed --disable-pth. diff --git a/doc/gpg-agent.texi b/doc/gpg-agent.texi index fa005c3b7..5e8c19468 100644 --- a/doc/gpg-agent.texi +++ b/doc/gpg-agent.texi @@ -25,11 +25,11 @@ eval `gpg-agent --daemon` @noindent If you don't use an X server, you can also put this into your regular startup file @code{~/.profile} or @code{.bash_profile}. It is best not -to run multiple instance of the @command{gpg-agent}, so you should make sure that -only is running: @command{gpg-agent} uses an environment variable to inform -clients about the communication parameters. You can write the -content of this environment variable to a file so that you can test for -a running agent. This short script may do the job: +to run multiple instance of the @command{gpg-agent}, so you should make +sure that only one is running: @command{gpg-agent} uses an environment +variable to inform clients about the communication parameters. You can +write the content of this environment variable to a file so that you can +test for a running agent. This short script may do the job: @smallexample if test -f $HOME/.gpg-agent-info && \ @@ -42,6 +42,9 @@ else fi @end smallexample +The new option @option{--write-env-file} may be used instead. + + @noindent You should always add the following lines to your @code{.bashrc} or whatever initialization file is used for all shell invocations: @@ -243,6 +246,21 @@ shell respective the C-shell . The default ist to guess it based on the environment variable @code{SHELL} which is in almost all cases sufficient. +@item --write-env-file @var{file} +@opindex write-env-file +Often it is required to connect to the agent from a process not being an +inferior of @command{gpg-agent} and thus the environment variable with +the socket name is not available. To help setting up those variables in +other sessions, this option may be used to write the information into +@var{file}. The format is suitable to be evaluated by a Bourne shell +like in this simple example: + +@example +eval `cat @var{file}` +eval `cut -d= -f 1 < @var{file} | xargs echo export` +@end example + + @item --no-grab @opindex no-grab Tell the pinentryo not to grab the keyboard and mouse. This option @@ -353,12 +371,19 @@ directory. Once, a key has been added to the gpg-agent this way, the gpg-agent will be ready to use the key. -Note: in case the gpg-agent receives a signature request, the user -might need to be prompted for a passphrase, which is necessary for -decrypting the stored key. Since the ssh-agent protocol does not -contain a mechanism for telling the agent on which display/terminal it -is running, gpg-agent's ssh-support will use the TTY or X display where -gpg-agent has been started. +Note: in case the gpg-agent receives a signature request, the user might +need to be prompted for a passphrase, which is necessary for decrypting +the stored key. Since the ssh-agent protocol does not contain a +mechanism for telling the agent on which display/terminal it is running, +gpg-agent's ssh-support will use the TTY or X display where gpg-agent +has been started. To switch this display to the current one, the +follwing command may be used: + +@smallexample +echo UPDATESTARTUPTTY | gpg-connect-agent +@end smallexample + + @end table @@ -544,6 +569,7 @@ secret keys. * Agent HAVEKEY:: Check whether a key is available * Agent LEARN:: Register a smartcard * Agent PASSWD:: Change a Passphrase +* Agent UPDATESTARTUPTTY:: Change the Standard Display @end menu @node Agent PKDECRYPT @@ -944,4 +970,16 @@ This command is used to interactively change the passphrase of the key indentified by the hex string @var{keygrip}. +@node Agent UPDATESTARTUPTTY +@subsection Change the standard display + +@example + UPDATESTARTUPTTY +@end example + +Set the startup TTY and X-DISPLAY variables to the values of this +session. This command is useful to direct future pinentry invocations +to another screen. It is only required because there is no way in the +ssh-agent protocol to convey this information. + diff --git a/scd/ChangeLog b/scd/ChangeLog index 136ed5618..da433e2f8 100644 --- a/scd/ChangeLog +++ b/scd/ChangeLog @@ -1,3 +1,32 @@ +2005-06-03 Werner Koch + + * scdaemon.c (handle_connections): Make sure that the signals we + are handling are not blocked.Block signals while creating new + threads. + (handle_connections): Include the file descriptor into the name of + the thread. + +2005-06-02 Werner Koch + + * app.c (app_dump_state, dump_mutex_state): New. + * scdaemon.c (handle_signal): Print it on SIGUSR1. + + * app-openpgp.c (do_writekey): Typo fix. + + * command.c (open_card): Check for locked state even if an + application context is available. + + * app-common.h: Add REF_COUNT field. + * app.c (release_application, select_application): Implement + reference counting to share the context beween connections. + + * app.c (lock_reader, unlock_reader): Take SLOT instead of APP as + argument. Changed all callers. + (select_application): Unlock the reader on error. This should fix + the hangs I noticed last week. + + * scdaemon.h: Removed card_ctx_t cruft. + 2005-06-01 Werner Koch * scdaemon.c: Include mkdtemp.h. diff --git a/scd/app-common.h b/scd/app-common.h index 812736ece..94087f221 100644 --- a/scd/app-common.h +++ b/scd/app-common.h @@ -39,6 +39,11 @@ struct app_ctx_s { function pointers may be used. Note that for unsupported operations the particular function pointer is set to NULL */ + + int ref_count; /* Number of connections currently using this + application context. fixme: We might want to + merg this witghn INITIALIZED above. */ + int slot; /* Used reader. */ /* If this is used by GnuPG 1.4 we need to know the assuan context @@ -123,6 +128,7 @@ size_t app_help_read_length_of_cert (int slot, int fid, size_t *r_certoff); /*-- app.c --*/ +void app_dump_state (void); gpg_error_t select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app); void release_application (app_t app); diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c index 14483869b..1ff096138 100644 --- a/scd/app-openpgp.c +++ b/scd/app-openpgp.c @@ -1745,7 +1745,7 @@ do_writekey (app_t app, ctrl_t ctrl, nbits = rsa_e? count_bits (rsa_e, rsa_e_len) : 0; if (nbits < 2 || nbits > 32) { - log_error (_("RSA public exponent missing or largerr than %d bits\n"), + log_error (_("RSA public exponent missing or larger than %d bits\n"), 32); err = gpg_error (GPG_ERR_BAD_SECKEY); goto leave; diff --git a/scd/app.c b/scd/app.c index f2c427f5b..2c8c915d7 100644 --- a/scd/app.c +++ b/scd/app.c @@ -39,25 +39,24 @@ static struct { int initialized; pth_mutex_t lock; + app_t app; /* Application context in use or NULL. */ } lock_table[10]; -/* Lock the reader associated with the APP context. This function - shall be used right before calling any of the actual application - functions to serialize access to the reader. We do this always - even if the reader is not actually used. This allows an actual - application to assume that it never shares a reader (while - performing one command). Returns 0 on success; only then the - unlock_reader function must be called after returning from the - handler. */ +/* Lock the reader SLOT. This function shall be used right before + calling any of the actual application functions to serialize access + to the reader. We do this always even if the reader is not + actually used. This allows an actual connection to assume that it + never shares a reader (while performing one command). Returns 0 on + success; only then the unlock_reader function must be called after + returning from the handler. */ static gpg_error_t -lock_reader (app_t app) +lock_reader (int slot) { gpg_error_t err; - int slot = app->slot; if (slot < 0 || slot >= DIM (lock_table)) - return gpg_error (app->slot<0? GPG_ERR_INV_VALUE : GPG_ERR_RESOURCE_LIMIT); + return gpg_error (slot<0? GPG_ERR_INV_VALUE : GPG_ERR_RESOURCE_LIMIT); if (!lock_table[slot].initialized) { @@ -68,6 +67,7 @@ lock_reader (app_t app) return err; } lock_table[slot].initialized = 1; + lock_table[slot].app = NULL; } if (!pth_mutex_acquire (&lock_table[slot].lock, 0, NULL)) @@ -83,10 +83,8 @@ lock_reader (app_t app) /* Release a lock on the reader. See lock_reader(). */ static void -unlock_reader (app_t app) +unlock_reader (int slot) { - int slot = app->slot; - if (slot < 0 || slot >= DIM (lock_table) || !lock_table[slot].initialized) log_bug ("unlock_reader called for invalid slot %d\n", slot); @@ -96,6 +94,39 @@ unlock_reader (app_t app) slot, strerror (errno)); } + + +static void +dump_mutex_state (pth_mutex_t *m) +{ + if (!(m->mx_state & PTH_MUTEX_INITIALIZED)) + log_printf ("not_initialized"); + else if (!(m->mx_state & PTH_MUTEX_LOCKED)) + log_printf ("not_locked"); + else + log_printf ("locked tid=0x%lx count=%lu", (long)m->mx_owner, m->mx_count); +} + + +/* This function may be called to print information pertaining to the + current state of this module to the log. */ +void +app_dump_state (void) +{ + int slot; + + for (slot=0; slot < DIM (lock_table); slot++) + if (lock_table[slot].initialized) + { + log_info ("app_dump_state: slot=%d lock=", slot); + dump_mutex_state (&lock_table[slot].lock); + if (lock_table[slot].app) + log_printf (" app=%p type=`%s'", + lock_table[slot].app, lock_table[slot].app->apptype); + log_printf ("\n"); + } +} + /* Check wether the application NAME is allowed. This does not mean we have support for it though. */ @@ -120,23 +151,48 @@ gpg_error_t select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app) { gpg_error_t err; - app_t app; + app_t app = NULL; unsigned char *result = NULL; size_t resultlen; *r_app = NULL; + + err = lock_reader (slot); + if (err) + return err; + + /* First check whether we already have an application to share. */ + app = lock_table[slot].initialized ? lock_table[slot].app : NULL; + if (app && name) + if (!app->apptype || ascii_strcasecmp (app->apptype, name)) + { + unlock_reader (slot); + if (app->apptype) + log_info ("application `%s' in use by reader %d - can't switch\n", + app->apptype, slot); + return gpg_error (GPG_ERR_CONFLICT); + } + + if (app) + { + if (app->slot != slot) + log_bug ("slot mismatch %d/%d\n", app->slot, slot); + app->ref_count++; + *r_app = app; + unlock_reader (slot); + return 0; /* Okay: We share that one. */ + } + app = xtrycalloc (1, sizeof *app); if (!app) { err = gpg_error_from_errno (errno); log_info ("error allocating context: %s\n", gpg_strerror (err)); + unlock_reader (slot); return err; } app->slot = slot; - err = lock_reader (app); - if (err) - return err; /* Fixme: We should now first check whether a card is at all present. */ @@ -162,7 +218,7 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app) have some test cards with such an invalid encoding and therefore I use this ugly workaround to return something I can further experiment with. */ - log_debug ("enabling BMI testcard workaround\n"); + log_info ("enabling BMI testcard workaround\n"); n--; } @@ -212,22 +268,41 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app) log_info ("no supported card application found: %s\n", gpg_strerror (err)); xfree (app); + unlock_reader (slot); return err; } app->initialized = 1; - unlock_reader (app); + app->ref_count = 1; + lock_table[slot].app = app; *r_app = app; + unlock_reader (slot); return 0; } +/* Free the resources associated with the application APP. APP is + allowed to be NULL in which case this is a no-op. Note that we are + using reference counting to track the users of the application. */ void release_application (app_t app) { + int slot; + if (!app) return; + if (app->ref_count < 1) + log_bug ("trying to release an already released context\n"); + if (--app->ref_count) + return; + + /* Clear the reference to the application from the lock table. */ + for (slot = 0; slot < DIM (lock_table); slot++) + if (lock_table[slot].initialized && lock_table[slot].app == app) + lock_table[slot].app = NULL; + + /* Deallocate. */ if (app->fnc.deinit) { app->fnc.deinit (app); @@ -320,11 +395,11 @@ app_write_learn_status (app_t app, CTRL ctrl) if (app->apptype) send_status_info (ctrl, "APPTYPE", app->apptype, strlen (app->apptype), NULL, 0); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.learn_status (app, ctrl); - unlock_reader (app); + unlock_reader (app->slot); return err; } @@ -345,11 +420,11 @@ app_readcert (app_t app, const char *certid, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.readcert) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.readcert (app, certid, cert, certlen); - unlock_reader (app); + unlock_reader (app->slot); return err; } @@ -377,11 +452,11 @@ app_readkey (app_t app, const char *keyid, unsigned char **pk, size_t *pklen) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.readkey) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err= app->fnc.readkey (app, keyid, pk, pklen); - unlock_reader (app); + unlock_reader (app->slot); return err; } @@ -419,11 +494,11 @@ app_getattr (app_t app, CTRL ctrl, const char *name) if (!app->fnc.getattr) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.getattr (app, ctrl, name); - unlock_reader (app); + unlock_reader (app->slot); return err; } @@ -442,11 +517,11 @@ app_setattr (app_t app, const char *name, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.setattr) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.setattr (app, name, pincb, pincb_arg, value, valuelen); - unlock_reader (app); + unlock_reader (app->slot); return err; } @@ -468,14 +543,14 @@ app_sign (app_t app, const char *keyidstr, int hashalgo, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.sign) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.sign (app, keyidstr, hashalgo, pincb, pincb_arg, indata, indatalen, outdata, outdatalen); - unlock_reader (app); + unlock_reader (app->slot); if (opt.verbose) log_info ("operation sign result: %s\n", gpg_strerror (err)); return err; @@ -500,14 +575,14 @@ app_auth (app_t app, const char *keyidstr, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.auth) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.auth (app, keyidstr, pincb, pincb_arg, indata, indatalen, outdata, outdatalen); - unlock_reader (app); + unlock_reader (app->slot); if (opt.verbose) log_info ("operation auth result: %s\n", gpg_strerror (err)); return err; @@ -532,14 +607,14 @@ app_decipher (app_t app, const char *keyidstr, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.decipher) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.decipher (app, keyidstr, pincb, pincb_arg, indata, indatalen, outdata, outdatalen); - unlock_reader (app); + unlock_reader (app->slot); if (opt.verbose) log_info ("operation decipher result: %s\n", gpg_strerror (err)); return err; @@ -562,12 +637,12 @@ app_writekey (app_t app, ctrl_t ctrl, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.writekey) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.writekey (app, ctrl, keyidstr, flags, pincb, pincb_arg, keydata, keydatalen); - unlock_reader (app); + unlock_reader (app->slot); if (opt.verbose) log_info ("operation writekey result: %s\n", gpg_strerror (err)); return err; @@ -589,11 +664,11 @@ app_genkey (app_t app, CTRL ctrl, const char *keynostr, unsigned int flags, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.genkey) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.genkey (app, ctrl, keynostr, flags, pincb, pincb_arg); - unlock_reader (app); + unlock_reader (app->slot); if (opt.verbose) log_info ("operation genkey result: %s\n", gpg_strerror (err)); return err; @@ -612,11 +687,11 @@ app_get_challenge (app_t app, size_t nbytes, unsigned char *buffer) return gpg_error (GPG_ERR_INV_VALUE); if (!app->initialized) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = iso7816_get_challenge (app->slot, nbytes, buffer); - unlock_reader (app); + unlock_reader (app->slot); return err; } @@ -636,12 +711,12 @@ app_change_pin (app_t app, CTRL ctrl, const char *chvnostr, int reset_mode, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.change_pin) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.change_pin (app, ctrl, chvnostr, reset_mode, pincb, pincb_arg); - unlock_reader (app); + unlock_reader (app->slot); if (opt.verbose) log_info ("operation change_pin result: %s\n", gpg_strerror (err)); return err; @@ -664,11 +739,11 @@ app_check_pin (app_t app, const char *keyidstr, return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.check_pin) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); - err = lock_reader (app); + err = lock_reader (app->slot); if (err) return err; err = app->fnc.check_pin (app, keyidstr, pincb, pincb_arg); - unlock_reader (app); + unlock_reader (app->slot); if (opt.verbose) log_info ("operation check_pin result: %s\n", gpg_strerror (err)); return err; diff --git a/scd/command.c b/scd/command.c index 738b1f003..287f8c921 100644 --- a/scd/command.c +++ b/scd/command.c @@ -253,12 +253,12 @@ open_card (ctrl_t ctrl, const char *apptype) if (ctrl->server_local->card_removed) return map_to_assuan_status (gpg_error (GPG_ERR_CARD_REMOVED)); - if (ctrl->app_ctx) - return 0; /* Already initialized for one specific application. */ - if ( IS_LOCKED (ctrl) ) return gpg_error (GPG_ERR_LOCKED); + if (ctrl->app_ctx) + return 0; /* Already initialized for one specific application. */ + if (ctrl->reader_slot != -1) slot = ctrl->reader_slot; else diff --git a/scd/scdaemon.c b/scd/scdaemon.c index 341719b1e..5b5e09176 100644 --- a/scd/scdaemon.c +++ b/scd/scdaemon.c @@ -809,6 +809,7 @@ handle_signal (int signo) case SIGUSR1: log_info ("SIGUSR1 received - printing internal information:\n"); pth_ctrl (PTH_CTRL_DUMPSTATE, log_get_stream ()); + app_dump_state (); break; case SIGUSR2: @@ -1013,7 +1014,6 @@ handle_connections (int listen_fd) tattr = pth_attr_new(); pth_attr_set (tattr, PTH_ATTR_JOINABLE, 0); pth_attr_set (tattr, PTH_ATTR_STACK_SIZE, 512*1024); - pth_attr_set (tattr, PTH_ATTR_NAME, "scd-connections"); #ifndef HAVE_W32_SYSTEM /* fixme */ sigemptyset (&sigs ); @@ -1022,6 +1022,7 @@ handle_connections (int listen_fd) sigaddset (&sigs, SIGUSR2); sigaddset (&sigs, SIGINT); sigaddset (&sigs, SIGTERM); + pth_sigmask (SIG_UNBLOCK, &sigs, NULL); ev = pth_event (PTH_EVENT_SIGS, &sigs, &signo); #else ev = NULL; @@ -1034,6 +1035,8 @@ handle_connections (int listen_fd) for (;;) { + sigset_t oldsigs; + if (shutdown_pending) { if (pth_ctrl (PTH_CTRL_GETTHREADS) == 1) @@ -1093,6 +1096,11 @@ handle_connections (int listen_fd) handle_tick (); } + /* We now might create new threads and because we don't want any + signals - we are handling here - to be delivered to a new + thread. Thus we need to block those signals. */ + pth_sigmask (SIG_BLOCK, &sigs, &oldsigs); + if (listen_fd != -1 && FD_ISSET (listen_fd, &read_fdset)) { plen = sizeof paddr; @@ -1101,15 +1109,26 @@ handle_connections (int listen_fd) { log_error ("accept failed: %s\n", strerror (errno)); } - else if (!pth_spawn (tattr, start_connection_thread, (void*)fd)) - { - log_error ("error spawning connection handler: %s\n", - strerror (errno) ); - close (fd); - } + else + { + char threadname[50]; + snprintf (threadname, sizeof threadname-1, "conn fd=%d", fd); + threadname[sizeof threadname -1] = 0; + pth_attr_set (tattr, PTH_ATTR_NAME, threadname); + + if (!pth_spawn (tattr, start_connection_thread, (void*)fd)) + { + log_error ("error spawning connection handler: %s\n", + strerror (errno) ); + close (fd); + } + } fd = -1; } + /* Restore the signal mask. */ + pth_sigmask (SIG_SETMASK, &oldsigs, NULL); + } pth_event_free (ev, PTH_FREE_ALL); diff --git a/scd/scdaemon.h b/scd/scdaemon.h index eaa9abd35..54566b6ad 100644 --- a/scd/scdaemon.h +++ b/scd/scdaemon.h @@ -77,19 +77,28 @@ struct { #define DBG_CARD_IO (opt.debug & DBG_CARD_IO_VALUE) struct server_local_s; -struct card_ctx_s; struct app_ctx_s; -struct server_control_s { +struct server_control_s +{ + /* Local data of the server; used only in command.c. */ struct server_local_s *server_local; - int reader_slot; /* Slot of the open reader or -1 if not open. */ - struct card_ctx_s *card_ctx; + + /* Slot of the open reader or -1 if not open. */ + int reader_slot; + + /* The application context used with this connection or NULL if none + associated. Note that this is shared with the other connections: + All connections accessing the same reader are using the same + application context. */ struct app_ctx_s *app_ctx; - struct { + + /* Helper to store the value we are going to sign */ + struct + { unsigned char *value; int valuelen; - } in_data; /* helper to store the value we are going to sign */ - + } in_data; }; typedef struct server_control_s *CTRL;