(get_cached_data): New arg GET_IMMEDIATE to bypass
authorWerner Koch <wk@gnupg.org>
Tue, 25 Jan 2005 14:18:56 +0000 (14:18 +0000)
committerWerner Koch <wk@gnupg.org>
Tue, 25 Jan 2005 14:18:56 +0000 (14:18 +0000)
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
g10/ChangeLog
g10/apdu.c
g10/app-openpgp.c

index 82450c0..5b7fce1 100644 (file)
@@ -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 <format> <timestamp>
         This indicates the format of the plaintext that is about to be
index 68f96b0..9373a8c 100644 (file)
@@ -1,3 +1,19 @@
+2005-01-25  Werner Koch  <wk@g10code.com>
+
+       * 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  <dshaw@grover.jabberwocky.com>
 
        * keyserver.c (free_keyserver_spec): Fix small leak.
index 1f7194c..040de14 100644 (file)
 #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);
             }
index 56402b6..fca0a98 100644 (file)
@@ -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"));