From aa87314e6f393863416e1fa7655efc386f81f36f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 25 Jan 2005 14:18:56 +0000 Subject: [PATCH] (get_cached_data): New arg GET_IMMEDIATE to bypass the cache. Changed all callers. (get_one_do): Bypass the cache if the value would have been read directly for v1.1 cards.It makes things a bit slower but obnly for 1.0 cards and there are not that many cards out in the wild. This is required to fix a caching bug when generating new keys; as a side effect of the retrieval of the the C4 DO from the 6E DO the chaced fingerprint will get updated to the old value and later when signing the generated key the checking of the fingerprint fails becuase it won't match the new one. Thanks to Moritz for analyzing this problem. (verify_chv3): Removed the CHV status reread logic because we won't cache the C4 DO anymore. --- doc/DETAILS | 2 ++ g10/ChangeLog | 16 +++++++++++ g10/apdu.c | 28 ++++++++++--------- g10/app-openpgp.c | 69 +++++++++++++++++++++++------------------------ 4 files changed, 67 insertions(+), 48 deletions(-) diff --git a/doc/DETAILS b/doc/DETAILS index 82450c087..5b7fce189 100644 --- a/doc/DETAILS +++ b/doc/DETAILS @@ -549,6 +549,8 @@ more arguments in future versions. to request a specific card. 2 = Request removal of a card. 3 = Card with serialnumber detected + 4 = No card available. + PLAINTEXT This indicates the format of the plaintext that is about to be diff --git a/g10/ChangeLog b/g10/ChangeLog index 68f96b0fd..9373a8cd5 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,19 @@ +2005-01-25 Werner Koch + + * app-openpgp.c (get_cached_data): New arg GET_IMMEDIATE to bypass + the cache. Changed all callers. + (get_one_do): Bypass the cache if the value would have been read + directly for v1.1 cards.It makes things a bit slower but obnly for + 1.0 cards and there are not that many cards out in the wild. This + is required to fix a caching bug when generating new keys; as a + side effect of the retrieval of the the C4 DO from the 6E DO the + chaced fingerprint will get updated to the old value and later + when signing the generated key the checking of the fingerprint + fails becuase it won't match the new one. Thanks to Moritz for + analyzing this problem. + (verify_chv3): Removed the CHV status reread logic because we + won't cache the C4 DO anymore. + 2005-01-21 David Shaw * keyserver.c (free_keyserver_spec): Fix small leak. diff --git a/g10/apdu.c b/g10/apdu.c index 1f7194c17..040de1461 100644 --- a/g10/apdu.c +++ b/g10/apdu.c @@ -65,11 +65,16 @@ #include "dynload.h" #include "ccid-driver.h" + +/* To to conflicting use of threading libraries we usually can't link + against libpcsclite. Instead we use a wrapper program. */ #ifdef USE_GNU_PTH +#ifndef HAVE_W32_SYSTEM #define NEED_PCSC_WRAPPER 1 #endif +#endif - + #define MAX_READER 4 /* Number of readers we support concurrently. */ @@ -1482,7 +1487,6 @@ open_pcsc_reader (const char *portstr) } strcpy (reader_table[slot].rdrname, portstr? portstr : list); xfree (list); - list = NULL; err = pcsc_connect (reader_table[slot].pcsc.context, reader_table[slot].rdrname, @@ -1496,11 +1500,11 @@ open_pcsc_reader (const char *portstr) { log_error ("pcsc_connect failed: %s (0x%lx)\n", pcsc_error_string (err), err); - - pcsc_release_context (reader_table[slot].pcsc.context); + pcsc_release_context (reader_table[slot].pcsc.context); xfree (reader_table[slot].rdrname); reader_table[slot].rdrname = NULL; reader_table[slot].used = 0; + xfree (list); return -1; } @@ -2717,8 +2721,8 @@ apdu_send_le(int slot, int class, int ins, int p0, int p1, resultlen -= 2; if (DBG_CARD_IO) { - log_debug (" response: sw=%04X datalen=%u\n", - sw, (unsigned int)resultlen); + log_debug (" response: sw=%04X datalen=%d\n", + sw, (unsigned int)resultlen); if ( !retbuf && (sw == SW_SUCCESS || (sw & 0xff00) == SW_MORE_DATA)) log_printhex (" dump: ", result, resultlen); } @@ -2784,8 +2788,8 @@ apdu_send_le(int slot, int class, int ins, int p0, int p1, resultlen -= 2; if (DBG_CARD_IO) { - log_debug (" more: sw=%04X datalen=%u\n", - sw, (unsigned int)resultlen); + log_debug (" more: sw=%04X datalen=%d\n", + sw, (unsigned int)resultlen); if (!retbuf && (sw==SW_SUCCESS || (sw&0xff00)==SW_MORE_DATA)) log_printhex (" dump: ", result, resultlen); } @@ -2918,8 +2922,8 @@ apdu_send_direct (int slot, const unsigned char *apdudata, size_t apdudatalen, resultlen -= 2; if (DBG_CARD_IO) { - log_debug (" response: sw=%04X datalen=%u\n", - sw, (unsigned int)resultlen); + log_debug (" response: sw=%04X datalen=%d\n", + sw, (unsigned int)resultlen); if ( !retbuf && (sw == SW_SUCCESS || (sw & 0xff00) == SW_MORE_DATA)) log_printhex (" dump: ", result, resultlen); } @@ -2971,8 +2975,8 @@ apdu_send_direct (int slot, const unsigned char *apdudata, size_t apdudatalen, resultlen -= 2; if (DBG_CARD_IO) { - log_debug (" more: sw=%04X datalen=%u\n", - sw, (unsigned int)resultlen); + log_debug (" more: sw=%04X datalen=%d\n", + sw, (unsigned int)resultlen); if (!retbuf && (sw==SW_SUCCESS || (sw&0xff00)==SW_MORE_DATA)) log_printhex (" dump: ", result, resultlen); } diff --git a/g10/app-openpgp.c b/g10/app-openpgp.c index 56402b6d2..fca0a98b7 100644 --- a/g10/app-openpgp.c +++ b/g10/app-openpgp.c @@ -1,5 +1,5 @@ /* app-openpgp.c - The OpenPGP card application. - * Copyright (C) 2003, 2004 Free Software Foundation, Inc. + * Copyright (C) 2003, 2004, 2005 Free Software Foundation, Inc. * * This file is part of GnuPG. * @@ -138,10 +138,12 @@ do_deinit (app_t app) /* Wrapper around iso7816_get_data which first tries to get the data - from the cache. */ + from the cache. With GET_IMMEDIATE passed as true, the cache is + bypassed. */ static gpg_error_t get_cached_data (app_t app, int tag, - unsigned char **result, size_t *resultlen) + unsigned char **result, size_t *resultlen, + int get_immediate) { gpg_error_t err; int i; @@ -152,23 +154,25 @@ get_cached_data (app_t app, int tag, *result = NULL; *resultlen = 0; - for (c=app->app_local->cache; c; c = c->next) - if (c->tag == tag) - { - if(c->length) + if (!get_immediate) + { + for (c=app->app_local->cache; c; c = c->next) + if (c->tag == tag) { - p = xtrymalloc (c->length); - if (!p) - return gpg_error (gpg_err_code_from_errno (errno)); - memcpy (p, c->data, c->length); - *result = p; + if(c->length) + { + p = xtrymalloc (c->length); + if (!p) + return gpg_error (gpg_err_code_from_errno (errno)); + memcpy (p, c->data, c->length); + *result = p; + } + + *resultlen = c->length; + + return 0; } - - *resultlen = c->length; - - return 0; - } - + } err = iso7816_get_data (app->slot, tag, &p, &len); if (err) @@ -177,6 +181,9 @@ get_cached_data (app_t app, int tag, *resultlen = len; /* Check whether we should cache this object. */ + if (get_immediate) + return 0; + for (i=0; data_objects[i].tag; i++) if (data_objects[i].tag == tag) { @@ -185,8 +192,7 @@ get_cached_data (app_t app, int tag, break; } - /* No, cache it. */ - + /* Okay, cache it. */ for (c=app->app_local->cache; c; c = c->next) assert (c->tag != tag); @@ -299,7 +305,8 @@ get_one_do (app_t app, int tag, unsigned char **result, size_t *nbytes) if (data_objects[i].tag && data_objects[i].get_from) { rc = get_cached_data (app, data_objects[i].get_from, - &buffer, &buflen); + &buffer, &buflen, + data_objects[i].get_immediate_in_v11); if (!rc) { const unsigned char *s; @@ -320,7 +327,8 @@ get_one_do (app_t app, int tag, unsigned char **result, size_t *nbytes) if (!value) /* Not in a constructed DO, try simple. */ { - rc = get_cached_data (app, tag, &buffer, &buflen); + rc = get_cached_data (app, tag, &buffer, &buflen, + data_objects[i].get_immediate_in_v11); if (!rc) { value = buffer; @@ -426,7 +434,7 @@ count_bits (const unsigned char *a, size_t len) at any time and should be called after changing the login-data DO. Everything up to a LF is considered a mailbox or account name. If - the first LF is follewed by DC4 (0x14) control sequence are + the first LF is followed by DC4 (0x14) control sequence are expected up to the next LF. Control sequences are separated by FS (0x28) and consist of key=value pairs. There is one key defined: @@ -836,8 +844,6 @@ verify_chv3 (app_t app, void *relptr; unsigned char *value; size_t valuelen; - int reread_chv_status; - relptr = get_one_do (app, 0x00C4, &value, &valuelen); if (!relptr || valuelen < 7) @@ -853,13 +859,11 @@ verify_chv3 (app_t app, return gpg_error (GPG_ERR_BAD_PIN); } - reread_chv_status = (value[6] < 3); - log_info(_("%d Admin PIN attempts remaining before card" " is permanently locked\n"), value[6]); xfree (relptr); - /* Note to translators: Do not translate the "|A|" prefix but + /* TRANSLATORS: Do not translate the "|A|" prefix but keep it at the start of the string. We need this elsewhere to get some infos on the string. */ rc = pincb (pincb_arg, _("|A|Admin PIN"), &pinvalue); @@ -886,13 +890,6 @@ verify_chv3 (app_t app, return rc; } app->did_chv3 = 1; - /* If the PIN has been entered wrongly before, we need to flush - the cached value so that the next read correctly reflects the - resetted retry counter. Note that version 1.1 of the specs - allow direct reading of that DO, so that we could actually - flush it in all cases. */ - if (reread_chv_status) - flush_cache_item (app, 0x00C4); } return rc; } @@ -1227,7 +1224,7 @@ compare_fingerprint (app_t app, int keyno, unsigned char *sha1fpr) assert (keyno >= 1 && keyno <= 3); - rc = get_cached_data (app, 0x006E, &buffer, &buflen); + rc = get_cached_data (app, 0x006E, &buffer, &buflen, 0); if (rc) { log_error (_("error reading application data\n"));