From 05e1dc22f0feef8d5af7c8bbb0c0a5129f2c0b05 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Sat, 21 May 2005 18:49:00 +0000 Subject: [PATCH] * call-scd.c (start_scd): Don't test for an alive scdaemon here. (agent_scd_check_aliveness): New. * gpg-agent.c (handle_tick): Test for an alive scdaemon. (handle_signal): Print thread info on SIGUSR1. * scdaemon.c (handle_signal): Print thread info on SIGUSR1. --- NEWS | 5 +++ TODO | 15 +++++++++ agent/ChangeLog | 7 ++++ agent/agent.h | 1 + agent/call-scd.c | 81 ++++++++++++++++++++++++++++++++++------------- agent/gpg-agent.c | 8 ++++- scd/ChangeLog | 4 +++ scd/app-openpgp.c | 2 +- scd/scdaemon.c | 3 +- 9 files changed, 101 insertions(+), 25 deletions(-) diff --git a/NEWS b/NEWS index c25bbe08e..79a74cbe4 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,11 @@ Noteworthy changes in version 1.9.17 ------------------------------------------------- + * gpg-connect-agent has now features to handle Assuan INQUIRE + commands. + + * Internal changes for OpenPGP cards. New Assuan command WRITEKEY. + * GNU Pth is now a hard requirement. * [scdaemon] Support for OpenSC has been removed. Instead a new and diff --git a/TODO b/TODO index 6e8951f03..fe10e9f77 100644 --- a/TODO +++ b/TODO @@ -67,6 +67,11 @@ might want to have an agent context for each service request * sm/export.c ** Return an error code or a status info per user ID. +* scd/tlv.c + The parse_sexp fucntion should not go into this file. Check whether + we can change all S-expression handling code to make use of this + function. + * tests ** Makefile.am We use printf(1) to setup the library path, this is not portable. @@ -89,3 +94,13 @@ might want to have an agent context for each service request This means we can't reread a configuration ** No card status notifications. + + +* IMPORTANT: + Check that the PIN cache is cleared after failed card operations. + After receiving a HUP gpg-agent should set a flag to kill scdaemon + as soon as possible, w/o that scdaemon will continue running as a + zombie and gpg-agent won't be able to fire up a new one. + Implement an scd/agent option to wait for a card. + + diff --git a/agent/ChangeLog b/agent/ChangeLog index 015b0b6d8..6c271c8e2 100644 --- a/agent/ChangeLog +++ b/agent/ChangeLog @@ -1,3 +1,10 @@ +2005-05-21 Werner Koch + + * call-scd.c (start_scd): Don't test for an alive scdaemon here. + (agent_scd_check_aliveness): New. + * gpg-agent.c (handle_tick): Test for an alive scdaemon. + (handle_signal): Print thread info on SIGUSR1. + 2005-05-20 Werner Koch * protect-tool.c: New option --canonical. diff --git a/agent/agent.h b/agent/agent.h index 6ab65eeba..7e4f555e8 100644 --- a/agent/agent.h +++ b/agent/agent.h @@ -247,6 +247,7 @@ int divert_generic_cmd (ctrl_t ctrl, /*-- call-scd.c --*/ void initialize_module_call_scd (void); +void agent_scd_check_aliveness (void); int agent_reset_scd (ctrl_t ctrl); int agent_card_learn (ctrl_t ctrl, void (*kpinfo_cb)(void*, const char *), diff --git a/agent/call-scd.c b/agent/call-scd.c index fc81e2fa3..617ef0d48 100644 --- a/agent/call-scd.c +++ b/agent/call-scd.c @@ -185,26 +185,15 @@ start_scd (ctrl_t ctrl) } ctrl->scd_local->locked++; - /* If we already have a context, we better do a sanity check now to - see whether it has accidently died. This avoids annoying - timeouts and hung connections. */ if (ctrl->scd_local->ctx) - { - pid_t pid; -#ifndef HAVE_W32_SYSTEM - pid = assuan_get_pid (ctrl->scd_local->ctx); - if (pid != (pid_t)(-1) && pid - && ((rc=waitpid (pid, NULL, WNOHANG))==-1 || (rc == pid)) ) - { - assuan_disconnect (ctrl->scd_local->ctx); - ctrl->scd_local->ctx = NULL; - } - else -#endif - return 0; /* Okay, the context is fine. */ - } + return 0; /* Okay, the context is fine. We used to test for an + alive context here and do an disconnect. How that we + have a ticker function to check for it, it is easier + not to check here but to let the connection run on an + error instead. */ - /* We need to protect the lowwing code. */ + + /* We need to protect the following code. */ if (!pth_mutex_acquire (&start_scd_lock, 0, NULL)) { log_error ("failed to acquire the start_scd lock: %s\n", @@ -350,6 +339,50 @@ start_scd (ctrl_t ctrl) } +/* Check whether the Scdaemon is still alive and clean it up if not. */ +void +agent_scd_check_aliveness (void) +{ + pid_t pid; + int rc; + + /* We can do so only if there is no more active primary connection. + With an active primary connection, this is all no problem because + with the end of gpg-agent's session a disconnect is send and the + this function will be used at a later time. */ + if (!primary_scd_ctx || !primary_scd_ctx_reusable) + return; + + if (!pth_mutex_acquire (&start_scd_lock, 0, NULL)) + { + log_error ("failed to acquire the start_scd lock while" + " doing an aliveness check: %s\n", + strerror (errno)); + return; + } + + if (primary_scd_ctx && primary_scd_ctx_reusable) + { + pid = assuan_get_pid (primary_scd_ctx); + if (pid != (pid_t)(-1) && pid + && ((rc=waitpid (pid, NULL, WNOHANG))==-1 || (rc == pid)) ) + { + /* Okay, scdaemon died. Disconnect the primary connection now + but take care that it won't do another wait. */ + assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1); + assuan_disconnect (primary_scd_ctx); + primary_scd_ctx = NULL; + primary_scd_ctx_reusable = 0; + xfree (socket_name); + socket_name = NULL; + } + } + + if (!pth_mutex_release (&start_scd_lock)) + log_error ("failed to release the start_scd lock while" + " doing the aliveness check: %s\n", strerror (errno)); +} + /* Reset the SCD if it has been used. */ int @@ -359,15 +392,19 @@ agent_reset_scd (ctrl_t ctrl) { if (ctrl->scd_local->ctx) { - /* We can't disconnect the primary context becuase libassuan + /* We can't disconnect the primary context because libassuan does a waitpid on it and thus the system would hang. Instead we send a reset and keep that connection for reuse. */ if (ctrl->scd_local->ctx == primary_scd_ctx) { - if (!assuan_transact (primary_scd_ctx, "RESET", - NULL, NULL, NULL, NULL, NULL, NULL)) - primary_scd_ctx_reusable = 1; + /* The RESET may fail for example if the scdaemon has + already been terminated. We need to set the reusable + flag anyway to make sure that the aliveness check can + clean it up. */ + assuan_transact (primary_scd_ctx, "RESET", + NULL, NULL, NULL, NULL, NULL, NULL); + primary_scd_ctx_reusable = 1; } else assuan_disconnect (ctrl->scd_local->ctx); diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c index 4ac995c26..e3e952906 100644 --- a/agent/gpg-agent.c +++ b/agent/gpg-agent.c @@ -1270,6 +1270,11 @@ create_directories (void) static void handle_tick (void) { + /* Check whether the scdaemon has dies and cleanup in this case. */ + agent_scd_check_aliveness (); + + /* If we are running as a child of another process, check whether + the parent is still alive and shutdwon if now. */ #ifndef HAVE_W32_SYSTEM if (parent_pid != (pid_t)(-1)) { @@ -1301,7 +1306,8 @@ handle_signal (int signo) break; case SIGUSR1: - log_info ("SIGUSR1 received - no action defined\n"); + log_info ("SIGUSR1 received - printing internal information:\n"); + pth_ctrl (PTH_CTRL_DUMPSTATE, log_get_stream ()); break; case SIGUSR2: diff --git a/scd/ChangeLog b/scd/ChangeLog index c64fbec7e..feeaabfce 100644 --- a/scd/ChangeLog +++ b/scd/ChangeLog @@ -1,3 +1,7 @@ +2005-05-21 Werner Koch + + * scdaemon.c (handle_signal): Print thread info on SIGUSR1. + 2005-05-20 Werner Koch * ccid-driver.c: Replaced macro DEBUG_T1 by a new debug level. diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c index 16ebd34c8..1165ec683 100644 --- a/scd/app-openpgp.c +++ b/scd/app-openpgp.c @@ -1634,7 +1634,7 @@ do_writekey (app_t app, ctrl_t ctrl, log_info ("protected-private-key passed to writekey\n"); else if (toklen == 20 && !memcmp ("shadowed-private-key", tok, toklen)) log_info ("shadowed-private-key passed to writekey\n"); - err = gpg_error (GPG_ERR_BAD_KEY); + err = gpg_error (GPG_ERR_BAD_SECKEY); goto leave; } if ((err = parse_sexp (&buf, &buflen, &depth, &tok, &toklen))) diff --git a/scd/scdaemon.c b/scd/scdaemon.c index 1110d9d76..488a4853b 100644 --- a/scd/scdaemon.c +++ b/scd/scdaemon.c @@ -807,7 +807,8 @@ handle_signal (int signo) break; case SIGUSR1: - log_info ("SIGUSR1 received - no action defined\n"); + log_info ("SIGUSR1 received - printing internal information:\n"); + pth_ctrl (PTH_CTRL_DUMPSTATE, log_get_stream ()); break; case SIGUSR2: