From 4472efd12c3f3585cc94f3f938fb5e0bd6beabce Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 9 Feb 2006 18:29:31 +0000 Subject: [PATCH] PIN caching of cards does now work. --- NEWS | 2 + agent/ChangeLog | 10 ++++ agent/agent.h | 2 +- agent/call-scd.c | 78 ++++++++++++++++++++++++---- agent/gpg-agent.c | 2 +- scd/ChangeLog | 14 +++++ scd/app-common.h | 2 + scd/app.c | 127 ++++++++++++++++++++++++++++++++++++++++------ scd/command.c | 49 ++++++++++++++++-- 9 files changed, 256 insertions(+), 30 deletions(-) diff --git a/NEWS b/NEWS index b3b8d05ce..38dee1ec3 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ Noteworthy changes in version 1.9.21 * Support for CardMan 4040 PCMCIA reader. + * Cards are not anymore reseted at the end of a connection. + Noteworthy changes in version 1.9.20 (2005-12-20) ------------------------------------------------- diff --git a/agent/ChangeLog b/agent/ChangeLog index 0aef39487..fbb1a37e0 100644 --- a/agent/ChangeLog +++ b/agent/ChangeLog @@ -1,3 +1,13 @@ +2006-02-09 Werner Koch + + * call-scd.c (struct scd_local_s): New field next_local. + (scd_local_list): New. + (start_scd): Put new local into list. + (agent_reset_scd): Remove it from the list. + (agent_scd_check_aliveness): Here is the actual reason why we need + all this stuff. + (agent_reset_scd): Send the new command RESTART instead of RESET. + 2005-12-16 Werner Koch * minip12.c (cram_octet_string): New diff --git a/agent/agent.h b/agent/agent.h index 0918395ce..1542d6b9f 100644 --- a/agent/agent.h +++ b/agent/agent.h @@ -54,7 +54,7 @@ struct { int batch; /* Batch mode */ const char *homedir; /* Configuration directory name */ - /* Environment setting gathered at program start or hanged using the + /* Environment setting gathered at program start or changed using the Assuan command UPDATESTARTUPTTY. */ char *startup_display; char *startup_ttyname; diff --git a/agent/call-scd.c b/agent/call-scd.c index a883f2733..ff241ce41 100644 --- a/agent/call-scd.c +++ b/agent/call-scd.c @@ -45,6 +45,16 @@ /* Definition of module local data of the CTRL structure. */ struct scd_local_s { + /* We keep a list of all allocated context with a an achnor at + SCD_LOCAL_LIST (see below). */ + struct scd_local_s *next_local; + + /* We need to get back to the ctrl object actually referencing this + structure. This is really an awkward way of enumerint the lcoal + contects. A much cleaner way would be to keep a global list of + ctrl objects to enumerate them. */ + ctrl_t ctrl_backlink; + assuan_context_t ctx; /* NULL or session context for the SCdaemon used with this connection. */ int locked; /* This flag is used to assert proper use of @@ -72,6 +82,10 @@ struct inq_needpin_s }; +/* To keep track of all active SCD contexts, we keep a linked list + anchored at this variable. */ +static struct scd_local_s *scd_local_list; + /* A Mutex used inside the start_scd function. */ static pth_mutex_t start_scd_lock; @@ -202,6 +216,9 @@ start_scd (ctrl_t ctrl) ctrl->scd_local = xtrycalloc (1, sizeof *ctrl->scd_local); if (!ctrl->scd_local) return gpg_error_from_errno (errno); + ctrl->scd_local->ctrl_backlink = ctrl; + ctrl->scd_local->next_local = scd_local_list; + scd_local_list = ctrl->scd_local; } @@ -216,7 +233,7 @@ start_scd (ctrl_t ctrl) if (ctrl->scd_local->ctx) return 0; /* Okay, the context is fine. We used to test for an - alive context here and do an disconnect. How that we + alive context here and do an disconnect. Now 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. */ @@ -404,12 +421,30 @@ agent_scd_check_aliveness (void) 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. */ + /* Okay, scdaemon died. Disconnect the primary connection + now but take care that it won't do another wait. Also + cleanup all other connections and release their + resources. The next use will start a new daemon then. + Due to the use of the START_SCD_LOCAL we are sure that + none of these context are actually in use. */ + struct scd_local_s *sl; + assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1); assuan_disconnect (primary_scd_ctx); + + for (sl=scd_local_list; sl; sl = sl->next_local) + { + if (sl->ctx) + { + if (sl->ctx != primary_scd_ctx) + assuan_disconnect (sl->ctx); + sl->ctx = NULL; + } + } + primary_scd_ctx = NULL; primary_scd_ctx_reusable = 0; + xfree (socket_name); socket_name = NULL; } @@ -422,7 +457,8 @@ agent_scd_check_aliveness (void) -/* Reset the SCD if it has been used. */ +/* Reset the SCD if it has been used. Actually it is not a reset but + a cleanup of resources used by the current connection. */ int agent_reset_scd (ctrl_t ctrl) { @@ -436,16 +472,40 @@ agent_reset_scd (ctrl_t ctrl) reuse. */ if (ctrl->scd_local->ctx == primary_scd_ctx) { - /* 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", + /* Send a RESTART to the SCD. This is required for the + primary connection as a kind of virtual EOF; we don't + have another way to tell it that the next command + should be viewed as if a new connection has been + made. For the non-primary connections this is not + needed as we simply close the socket. We don't check + for an error here because the RESTART may fail for + example if the scdaemon has already been terminated. + Anyway, we need to set the reusable flag to make sure + that the aliveness check can clean it up. */ + assuan_transact (primary_scd_ctx, "RESTART", NULL, NULL, NULL, NULL, NULL, NULL); primary_scd_ctx_reusable = 1; } else assuan_disconnect (ctrl->scd_local->ctx); + ctrl->scd_local->ctx = NULL; + } + + /* Remove the local context from our list and release it. */ + if (!scd_local_list) + BUG (); + else if (scd_local_list == ctrl->scd_local) + scd_local_list = ctrl->scd_local->next_local; + else + { + struct scd_local_s *sl; + + for (sl=scd_local_list; sl->next_local; sl = sl->next_local) + if (sl->next_local == ctrl->scd_local) + break; + if (!sl->next_local) + BUG (); + sl->next_local = ctrl->scd_local->next_local; } xfree (ctrl->scd_local); ctrl->scd_local = NULL; diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c index b6a4f90a6..22bd5589d 100644 --- a/agent/gpg-agent.c +++ b/agent/gpg-agent.c @@ -1320,7 +1320,7 @@ handle_tick (void) 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. */ + the parent is still alive and shutdown if not. */ #ifndef HAVE_W32_SYSTEM if (parent_pid != (pid_t)(-1)) { diff --git a/scd/ChangeLog b/scd/ChangeLog index 5dab2019c..3b850a293 100644 --- a/scd/ChangeLog +++ b/scd/ChangeLog @@ -1,3 +1,17 @@ +2006-02-09 Werner Koch + + * app.c (release_application): Factored code out to .. + (deallocate_app): new function. + (select_application): Introduce new saved application stuff. + (application_notify_card_removed): New. + * command.c (update_card_removed): Call it. + + * app.c (check_application_conflict): New. + * command.c (open_card): Use it here. + (cmd_restart): New command. + + * command.c (cmd_lock): Fixed --wait option to actually terminate. + 2006-02-08 Werner Koch * ccid-driver.c (ccid_get_atr): Read Parameter and select T=1 diff --git a/scd/app-common.h b/scd/app-common.h index 94087f221..d294a5c25 100644 --- a/scd/app-common.h +++ b/scd/app-common.h @@ -129,6 +129,8 @@ size_t app_help_read_length_of_cert (int slot, int fid, size_t *r_certoff); /*-- app.c --*/ void app_dump_state (void); +void application_notify_card_removed (int slot); +gpg_error_t check_application_conflict (ctrl_t ctrl, const char *name); 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.c b/scd/app.c index f27b400b1..fad45ed85 100644 --- a/scd/app.c +++ b/scd/app.c @@ -40,9 +40,15 @@ static struct int initialized; pth_mutex_t lock; app_t app; /* Application context in use or NULL. */ + app_t last_app; /* Last application object used as this slot or NULL. */ } lock_table[10]; + +static void deallocate_app (app_t app); + + + /* 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 @@ -68,6 +74,7 @@ lock_reader (int slot) } lock_table[slot].initialized = 1; lock_table[slot].app = NULL; + lock_table[slot].last_app = NULL; } if (!pth_mutex_acquire (&lock_table[slot].lock, 0, NULL)) @@ -121,13 +128,21 @@ app_dump_state (void) 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 (" app=%p", lock_table[slot].app); + if (lock_table[slot].app->apptype) + log_printf (" type=`%s'", lock_table[slot].app->apptype); + } + if (lock_table[slot].last_app) + { + log_printf (" lastapp=%p", lock_table[slot].last_app); + if (lock_table[slot].last_app->apptype) + log_printf (" type=`%s'", lock_table[slot].last_app->apptype); + } log_printf ("\n"); } } - /* Check wether the application NAME is allowed. This does not mean we have support for it though. */ static int @@ -142,6 +157,46 @@ is_app_allowed (const char *name) } +/* This may be called to tell this module about a removed card. */ +void +application_notify_card_removed (int slot) +{ + if (slot < 0 || slot >= DIM (lock_table)) + return; + + /* Deallocate a saved application for that slot, so that we won't + try to reuse it. */ + if (lock_table[slot].initialized && lock_table[slot].last_app) + { + app_t app = lock_table[slot].last_app; + + lock_table[slot].last_app = NULL; + deallocate_app (app); + } +} + + +/* This fucntion is used by the serialno command to check for an + application conflict which may appear if the serialno command is + used to request a specific application and the connection has + already done a select_application. */ +gpg_error_t +check_application_conflict (ctrl_t ctrl, const char *name) +{ + int slot = ctrl->reader_slot; + app_t app; + + if (slot < 0 || slot >= DIM (lock_table)) + return gpg_error (GPG_ERR_INV_VALUE); + + app = lock_table[slot].initialized ? lock_table[slot].app : NULL; + if (app && app->apptype && name) + if ( ascii_strcasecmp (app->apptype, name)) + return gpg_error (GPG_ERR_CONFLICT); + return 0; +} + + /* If called with NAME as NULL, select the best fitting application and return a context; otherwise select the application with NAME and return a context. SLOT identifies the reader device. Returns @@ -173,6 +228,32 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app) return gpg_error (GPG_ERR_CONFLICT); } + /* If we don't have an app, check whether we have a saved + application for that slot. This is useful so that a card does + not get reset even if only one session is using the card - so the + PIN cache and other cached data are preserved. */ + if (!app && lock_table[slot].initialized && lock_table[slot].last_app) + { + app = lock_table[slot].last_app; + if (!name || (app->apptype && !ascii_strcasecmp (app->apptype, name)) ) + { + /* Yes, we can reuse this application - either the caller + requested an unspecific one or the requested one matches + the saved one. */ + lock_table[slot].app = app; + lock_table[slot].last_app = NULL; + } + else + { + /* No, this saved application can't be used - deallocate it. */ + lock_table[slot].last_app = NULL; + deallocate_app (app); + app = NULL; + } + } + + /* If we can reuse an application, bump the reference count and + return it. */ if (app) { if (app->slot != slot) @@ -183,6 +264,7 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app) return 0; /* Okay: We share that one. */ } + /* Need to allocate a new one. */ app = xtrycalloc (1, sizeof *app); if (!app) { @@ -281,9 +363,25 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app) } +/* Deallocate the application. */ +static void +deallocate_app (app_t app) +{ + if (app->fnc.deinit) + { + app->fnc.deinit (app); + app->fnc.deinit = NULL; + } + + xfree (app->serialno); + xfree (app); +} + /* 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. */ + using reference counting to track the users of the application and + actually deferiing the deallcoation to allow for a later resuse by + a new connection. */ void release_application (app_t app) { @@ -297,20 +395,19 @@ release_application (app_t app) if (--app->ref_count) return; - /* Clear the reference to the application from the lock table. */ + /* Move the reference to the application in 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; + { + if (lock_table[slot].last_app) + deallocate_app (lock_table[slot].last_app); + lock_table[slot].last_app = lock_table[slot].app; + lock_table[slot].app = NULL; + return; + } - /* Deallocate. */ - if (app->fnc.deinit) - { - app->fnc.deinit (app); - app->fnc.deinit = NULL; - } - - xfree (app->serialno); - xfree (app); + log_debug ("application missing in lock table - deallocating anyway\n"); + deallocate_app (app); } diff --git a/scd/command.c b/scd/command.c index d556822a2..1b7a8f67e 100644 --- a/scd/command.c +++ b/scd/command.c @@ -47,7 +47,7 @@ #define set_error(e,t) assuan_set_error (ctx, ASSUAN_ ## e, (t)) -/* Macro to flag a a removed card. */ +/* Macro to flag a removed card. */ #define TEST_CARD_REMOVAL(c,r) \ do { \ int _r = (r); \ @@ -108,6 +108,8 @@ update_card_removed (int slot, int value) if (sl->ctrl_backlink && sl->ctrl_backlink->reader_slot == slot) sl->card_removed = value; + if (value) + application_notify_card_removed (slot); } @@ -142,7 +144,7 @@ do_reset (ctrl_t ctrl, int do_close) struct server_local_s *sl; /* If we are the only session with the reader open we may close - it. If not, do a reset unless the a lock is held on the + it. If not, do a reset unless a lock is held on the reader. */ for (sl=session_list; sl; sl = sl->next_session) if (sl != ctrl->server_local @@ -257,7 +259,12 @@ open_card (ctrl_t ctrl, const char *apptype) return gpg_error (GPG_ERR_LOCKED); if (ctrl->app_ctx) - return 0; /* Already initialized for one specific application. */ + { + /* Already initialized for one specific application. Need to + check that the client didn't requested a specific application + different from the one in use. */ + return check_application_conflict (ctrl, apptype); + } if (ctrl->reader_slot != -1) slot = ctrl->reader_slot; @@ -1201,7 +1208,7 @@ cmd_checkpin (assuan_context_t ctx, char *line) Grant exclusive card access to this session. Note that there is no lock counter used and a second lock from the same session will - get ignore. A single unlock (or RESET) unlocks the session. + be ignored. A single unlock (or RESET) unlocks the session. Return GPG_ERR_LOCKED if another session has locked the reader. If the option --wait is given the command will wait until a @@ -1225,9 +1232,12 @@ cmd_lock (assuan_context_t ctx, char *line) #ifdef USE_GNU_PTH if (rc && has_option (line, "--wait")) { + rc = 0; pth_sleep (1); /* Better implement an event mechanism. However, for card operations this should be sufficient. */ + /* FIXME: Need to check that the connection is still alive. + This can be done by issuing status messages. */ goto retry; } #endif /*USE_GNU_PTH*/ @@ -1293,6 +1303,36 @@ cmd_getinfo (assuan_context_t ctx, char *line) } +/* RESTART + + Restart the current connection; this is a kind of warn reset. It + deletes the context used by this connection but does not send a + RESET to the card. Thus the card itself won't get reset. + + This is used by gpg-agent to reuse a primary pipe connection and + may be used by clients to backup from a conflict in the serial + command; i.e. to select another application. +*/ + +static int +cmd_restart (assuan_context_t ctx, char *line) +{ + ctrl_t ctrl = assuan_get_pointer (ctx); + + if (ctrl->app_ctx) + { + release_application (ctrl->app_ctx); + ctrl->app_ctx = NULL; + } + if (locked_session && ctrl->server_local == locked_session) + { + locked_session = NULL; + log_info ("implicitly unlocking due to RESTART\n"); + } + return 0; +} + + /* Tell the assuan library about our commands */ @@ -1323,6 +1363,7 @@ register_commands (assuan_context_t ctx) { "LOCK", cmd_lock }, { "UNLOCK", cmd_unlock }, { "GETINFO", cmd_getinfo }, + { "RESTART", cmd_restart }, { NULL } }; int i, rc;