Improved smartcard robustness.

NEWS

@@ -15,13 +15,16 @@ Noteworthy changes in version 2.0.12 (not released)
  * Changed order of the confirmation questions for root certificates
    and stores negative answers in trustlist.txt.
 
- * Better synchronization of several smartcard sessions.
+ * Better synchronization of concurrent smartcard sessions.
 
- * Support for the Telesec Netkey 3 cards.
+ * Support 2048 bit OpenPGP cards.
+
+ * Support Telesec Netkey 3 cards.
 
  * The gpg-protect-tool now uses gpg-agent via libassuan.  Under
    Windows the Pinentry will now be put into the foreground.
 
+ 
 
 Noteworthy changes in version 2.0.11 (2009-03-03)
 -------------------------------------------------

doc/scdaemon.texi

@@ -670,7 +670,7 @@ command; i.e. to select another application.
 @subsection Send a verbatim APDU to the card.
 
 @example
-  APDU [--atr] [--more] [@var{hexstring}]
+  APDU [--atr] [--more] [--exlen[=@var{n}]] [@var{hexstring}]
 @end example
 
 
@@ -689,6 +689,10 @@ message before any data like this:
 Using the option @code{--more} handles the card status word MORE_DATA
 (61xx) and concatenate all reponses to one block.
 
+Using the option @code{--exlen} the returned APDU may use extended
+length up to N bytes.  If N is not given a default value is used
+(currently 4096).
+
 
 
 @mansect see also

scd/ChangeLog

@@ -1,3 +1,12 @@
+2009-05-13  Werner Koch  <wk@g10code.com>
+
+	* ccid-driver.c (abort_cmd): Add arg SEQNO and change callers.
+	(bulk_in): Retry on seqno mismatch.
+
+	* apdu.c (send_le): Release result_buffer.
+	(apdu_send_direct): Implemend extended length.
+	* command.c (cmd_apdu): Add option "--exlen".
+
 2009-05-11  Werner Koch  <wk@g10code.com>
 
 	* apdu.c (send_le): Replace log_error by log_info.

scd/apdu.c

@@ -2930,7 +2930,11 @@ send_le (int slot, int class, int ins, int p0, int p1,
 #undef SHORT_RESULT_BUFFER_SIZE
 
   if ((sw = lock_slot (slot)))
-    return sw;
+    {
+      xfree (apdu_buffer);
+      xfree (result_buffer);
+      return sw;
+    }
 
   do
     {
@@ -3003,6 +3007,8 @@ send_le (int slot, int class, int ins, int p0, int p1,
           log_info ("apdu_send_simple(%d) failed: %s\n",
                     slot, apdu_strerror (rc));
           unlock_slot (slot);
+          xfree (apdu_buffer);
+          xfree (result_buffer);
           return rc? rc : SW_HOST_INCOMPLETE_CARD_RESPONSE;
         }
       sw = (result[resultlen-2] << 8) | result[resultlen-1];
@@ -3041,6 +3047,7 @@ send_le (int slot, int class, int ins, int p0, int p1,
           if (!*retbuf)
             {
               unlock_slot (slot);
+              xfree (result_buffer);
               return SW_HOST_OUT_OF_CORE;
             }
           *retbuflen = resultlen;
@@ -3060,6 +3067,7 @@ send_le (int slot, int class, int ins, int p0, int p1,
           if (!*retbuf)
             {
               unlock_slot (slot);
+              xfree (result_buffer);
               return SW_HOST_OUT_OF_CORE;
             }
           assert (resultlen < bufsize);
@@ -3091,6 +3099,7 @@ send_le (int slot, int class, int ins, int p0, int p1,
               log_error ("apdu_send_simple(%d) for get response failed: %s\n",
                          slot, apdu_strerror (rc));
               unlock_slot (slot);
+              xfree (result_buffer);
               return rc? rc : SW_HOST_INCOMPLETE_CARD_RESPONSE;
             }
           sw = (result[resultlen-2] << 8) | result[resultlen-1];
@@ -3116,6 +3125,7 @@ send_le (int slot, int class, int ins, int p0, int p1,
                       if (!tmp)
                         {
                           unlock_slot (slot);
+                          xfree (result_buffer);
                           return SW_HOST_OUT_OF_CORE;
                         }
                       p = tmp + (p - *retbuf);
@@ -3142,6 +3152,7 @@ send_le (int slot, int class, int ins, int p0, int p1,
     }
 
   unlock_slot (slot);
+  xfree (result_buffer);
 
   if (DBG_CARD_IO && retbuf && sw == SW_SUCCESS)
     log_printhex ("      dump: ", *retbuf, *retbuflen);
@@ -3231,23 +3242,27 @@ apdu_send_simple_kp (int slot, int class, int ins, int p0, int p1,
    and returns with an APDU including the status word.  With
    HANDLE_MORE set to true this function will handle the MORE DATA
    status and return all APDUs concatenated with one status word at
-   the end.  If EXTENDED_MODE is not 0 command chaining or extended
-   length will be used; see send_le for details.  The function does
-   not return a regular status word but 0 on success.  If the slot is
-   locked, the function returns immediately with an error.  */
+   the end.  If EXTENDED_LENGTH is != 0 extended lengths are allowed
+   with a max. result data length of EXTENDED_LENGTH bytes.  The
+   function does not return a regular status word but 0 on success.
+   If the slot is locked, the function returns immediately with an
+   error.  */
 int
-apdu_send_direct (int slot, int extended_mode,
+apdu_send_direct (int slot, size_t extended_length,
                   const unsigned char *apdudata, size_t apdudatalen,
                   int handle_more,
                   unsigned char **retbuf, size_t *retbuflen)
 {
-#define RESULTLEN 258
-  /* FIXME:  Implement dynamic result buffer and extended Le.  */
-  unsigned char apdu[5+256+1];
-  size_t apdulen;
-  unsigned char result[RESULTLEN+10]; /* 10 extra in case of bugs in
-                                         the driver. */
+#define SHORT_RESULT_BUFFER_SIZE 258
+  unsigned char short_result_buffer[SHORT_RESULT_BUFFER_SIZE+10]; 
+  unsigned char *result_buffer = NULL;
+  size_t result_buffer_size;
+  unsigned char *result;
   size_t resultlen;
+  unsigned char short_apdu_buffer[5+256+10];
+  unsigned char *apdu_buffer = NULL;
+  unsigned char *apdu;
+  size_t apdulen;
   int sw;
   long rc; /* we need a long here due to PC/SC. */
   int class;
@@ -3255,26 +3270,59 @@ apdu_send_direct (int slot, int extended_mode,
   if (slot < 0 || slot >= MAX_READER || !reader_table[slot].used )
     return SW_HOST_NO_DRIVER;
 
-  if (extended_mode)
-    return SW_HOST_NOT_SUPPORTED; /* FIXME. */
+  if (apdudatalen > 65535)
+    return SW_HOST_INV_VALUE;
 
-  if ((sw = trylock_slot (slot)))
-    return sw;
-
-  /* We simply trunctate a too long APDU.  */
-  if (apdudatalen > sizeof apdu)
-    apdudatalen = sizeof apdu;
+  if (apdudatalen > sizeof short_apdu_buffer - 5)
+    {
+      apdu_buffer = xtrymalloc (apdudatalen + 5);
+      if (!apdu_buffer)
+        return SW_HOST_OUT_OF_CORE;
+      apdu = apdu_buffer;
+    }
+  else
+    {
+      apdu = short_apdu_buffer;
+    }
   apdulen = apdudatalen;
   memcpy (apdu, apdudata, apdudatalen);
   class = apdulen? *apdu : 0;
 
-  resultlen = RESULTLEN;
+  if (extended_length >= 256 && extended_length <= 65536)
+    {
+      result_buffer_size = extended_length;
+      result_buffer = xtrymalloc (result_buffer_size + 10);
+      if (!result_buffer)
+        {
+          xfree (apdu_buffer);
+          return SW_HOST_OUT_OF_CORE;
+        }
+      result = result_buffer;
+    }
+  else
+    {
+      result_buffer_size = SHORT_RESULT_BUFFER_SIZE;
+      result = short_result_buffer;
+    }
+#undef SHORT_RESULT_BUFFER_SIZE
+
+  if ((sw = trylock_slot (slot)))
+    {
+      xfree (apdu_buffer);
+      xfree (result_buffer);
+      return sw;
+    }
+
+  resultlen = result_buffer_size;
   rc = send_apdu (slot, apdu, apdulen, result, &resultlen, NULL);
+  xfree (apdu_buffer);
+  apdu_buffer = NULL;
   if (rc || resultlen < 2)
     {
       log_error ("apdu_send_direct(%d) failed: %s\n",
                  slot, apdu_strerror (rc));
       unlock_slot (slot);
+      xfree (result_buffer);
       return rc? rc : SW_HOST_INCOMPLETE_CARD_RESPONSE;
     }
   sw = (result[resultlen-2] << 8) | result[resultlen-1];
@@ -3301,6 +3349,7 @@ apdu_send_direct (int slot, int extended_mode,
           if (!*retbuf)
             {
               unlock_slot (slot);
+              xfree (result_buffer);
               return SW_HOST_OUT_OF_CORE;
             }
           assert (resultlen < bufsize);
@@ -3315,20 +3364,22 @@ apdu_send_direct (int slot, int extended_mode,
           if (DBG_CARD_IO)
             log_debug ("apdu_send_direct(%d): %d more bytes available\n",
                        slot, len);
+          apdu = short_apdu_buffer;
           apdulen = 0;
           apdu[apdulen++] = class;
           apdu[apdulen++] = 0xC0;
           apdu[apdulen++] = 0;
           apdu[apdulen++] = 0;
           apdu[apdulen++] = len;
-          memset (apdu+apdulen, 0, sizeof (apdu) - apdulen);
-          resultlen = RESULTLEN;
+          memset (apdu+apdulen, 0, sizeof (short_apdu_buffer) - apdulen);
+          resultlen = result_buffer_size;
           rc = send_apdu (slot, apdu, apdulen, result, &resultlen, NULL);
           if (rc || resultlen < 2)
             {
               log_error ("apdu_send_direct(%d) for get response failed: %s\n",
                          slot, apdu_strerror (rc));
               unlock_slot (slot);
+              xfree (result_buffer);
               return rc ? rc : SW_HOST_INCOMPLETE_CARD_RESPONSE;
             }
           sw = (result[resultlen-2] << 8) | result[resultlen-1];
@@ -3354,6 +3405,7 @@ apdu_send_direct (int slot, int extended_mode,
                       if (!tmp)
                         {
                           unlock_slot (slot);
+                          xfree (result_buffer);
                           return SW_HOST_OUT_OF_CORE;
                         }
                       p = tmp + (p - *retbuf);
@@ -3386,6 +3438,7 @@ apdu_send_direct (int slot, int extended_mode,
           if (!*retbuf)
             {
               unlock_slot (slot);
+              xfree (result_buffer);
               return SW_HOST_OUT_OF_CORE;
             }
           *retbuflen = resultlen;
@@ -3394,6 +3447,7 @@ apdu_send_direct (int slot, int extended_mode,
     }
 
   unlock_slot (slot);
+  xfree (result_buffer);
 
   /* Append the status word.  Note that we reserved the two extra
      bytes while allocating the buffer.  */
@@ -3407,5 +3461,4 @@ apdu_send_direct (int slot, int extended_mode,
     log_printhex ("      dump: ", *retbuf, *retbuflen);
 
   return 0;
-#undef RESULTLEN
 }

scd/apdu.h

@@ -125,7 +125,7 @@ int apdu_send_le (int slot, int extended_mode,
                   int class, int ins, int p0, int p1,
                   int lc, const char *data, int le,
                   unsigned char **retbuf, size_t *retbuflen);
-int apdu_send_direct (int slot, int extended_mode,
+int apdu_send_direct (int slot, size_t extended_length,
                       const unsigned char *apdudata, size_t apdudatalen,
                       int handle_more,
                       unsigned char **retbuf, size_t *retbuflen);

scd/ccid-driver.c

@@ -271,7 +271,7 @@ static int bulk_out (ccid_driver_t handle, unsigned char *msg, size_t msglen,
 static int bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
                     size_t *nread, int expected_type, int seqno, int timeout,
                     int no_debug);
-static int abort_cmd (ccid_driver_t handle);
+static int abort_cmd (ccid_driver_t handle, int seqno);
 
 /* Convert a little endian stored 4 byte value into an unsigned
    integer. */
@@ -1832,9 +1832,11 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
         {
           rc = errno;
           DEBUGOUT_1 ("usb_bulk_read error: %s\n", strerror (rc));
-          if (rc == EAGAIN && eagain_retries++ < 5)
+          if (rc == EAGAIN && eagain_retries++ < 3)
             {
+#ifndef TEST
               gnupg_sleep (1);
+#endif
               goto retry;
             }
           return CCID_DRIVER_ERR_CARD_IO_ERROR;
@@ -1851,7 +1853,9 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
                       handle->dev_fd, strerror (rc));
           if (rc == EAGAIN && eagain_retries++ < 5)
             {
+#ifndef TEST
               gnupg_sleep (1);
+#endif
               goto retry;
             }
           return CCID_DRIVER_ERR_CARD_IO_ERROR;
@@ -1863,21 +1867,20 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
   if (msglen < 10)
     {
       DEBUGOUT_1 ("bulk-in msg too short (%u)\n", (unsigned int)msglen);
-      abort_cmd (handle);
+      abort_cmd (handle, seqno);
       return CCID_DRIVER_ERR_INV_VALUE;
     }
   if (buffer[5] != 0)    
     {
       DEBUGOUT_1 ("unexpected bulk-in slot (%d)\n", buffer[5]);
-      abort_cmd (handle);
       return CCID_DRIVER_ERR_INV_VALUE;
     }
   if (buffer[6] != seqno)    
     {
       DEBUGOUT_2 ("bulk-in seqno does not match (%d/%d)\n",
                   seqno, buffer[6]);
-      abort_cmd (handle);
-      return CCID_DRIVER_ERR_INV_VALUE;
+      /* Retry until we are synced again.  */
+      goto retry;
     }
 
   /* We need to handle the time extension request before we check that
@@ -1895,7 +1898,7 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
   if (buffer[0] != expected_type)
     {
       DEBUGOUT_1 ("unexpected bulk-in msg type (%02x)\n", buffer[0]);
-      abort_cmd (handle);
+      abort_cmd (handle, seqno);
       return CCID_DRIVER_ERR_INV_VALUE;
     }
 
@@ -1943,11 +1946,10 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
 
 /* Send an abort sequence and wait until everything settled.  */
 static int
-abort_cmd (ccid_driver_t handle)
+abort_cmd (ccid_driver_t handle, int seqno)
 {
   int rc;
   char dummybuf[8];
-  unsigned char seqno;
   unsigned char msg[100];
   size_t msglen;
 
@@ -1957,12 +1959,11 @@ abort_cmd (ccid_driver_t handle)
       rc = CCID_DRIVER_ERR_NOT_SUPPORTED;
     }
   
-  DEBUGOUT ("sending abort sequence\n");
+  seqno &= 0xff;
+  DEBUGOUT_1 ("sending abort sequence for seqno %d\n", seqno);
   /* Send the abort command to the control pipe.  Note that we don't
      need to keep track of sent abort commands because there should
      never be another thread using the same slot concurrently.  */
-  handle->seqno--;  /* Restore the last one sent.  */
-  seqno = (handle->seqno & 0xff);
   rc = usb_control_msg (handle->idev, 
                         0x21,/* bmRequestType: host-to-device,
                                 class specific, to interface.  */
@@ -2039,7 +2040,7 @@ abort_cmd (ccid_driver_t handle)
     }
   while (msg[0] != RDR_to_PC_SlotStatus && msg[5] != 0 && msg[6] != seqno);
 
-  handle->seqno = seqno;
+  handle->seqno = ((seqno + 1) & 0xff);
   DEBUGOUT ("sending abort sequence succeeded\n");
 
   return 0;
@@ -2178,7 +2179,7 @@ ccid_poll (ccid_driver_t handle)
 }
 
 
-/* Note that this fucntion won't return the error codes NO_CARD or
+/* Note that this function won't return the error codes NO_CARD or
    CARD_INACTIVE */
 int 
 ccid_slot_status (ccid_driver_t handle, int *statusbits)
@@ -3298,13 +3299,6 @@ main (int argc, char **argv)
   return 0;
 }
 
-static coid
-gnupg_sleep (int seconds)
-{
-  sleep (seconds);
-}
-
-
 /*
  * Local Variables:
  *  compile-command: "gcc -DTEST -Wall -I/usr/local/include -lusb -g ccid-driver.c"

scd/command.c

@@ -202,7 +202,7 @@ has_option (const char *line, const char *name)
 /* Same as has_option but does only test for the name of the option
    and ignores an argument, i.e. with NAME being "--hash" it would
    return a pointer for "--hash" as well as for "--hash=foo".  If
-   thhere is no such option NULL is returned.  The pointer returned
+   there is no such option NULL is returned.  The pointer returned
    points right behind the option name, this may be an equal sign, Nul
    or a space.  */
 static const char *
@@ -1722,7 +1722,7 @@ cmd_disconnect (assuan_context_t ctx, char *line)
 
 
 
-/* APDU [--atr] [--more] [hexstring]
+/* APDU [--atr] [--more] [--exlen[=N]] [hexstring]
 
    Send an APDU to the current reader.  This command bypasses the high
    level functions and sends the data directly to the card.  HEXSTRING
@@ -1735,8 +1735,11 @@ cmd_disconnect (assuan_context_t ctx, char *line)
      S CARD-ATR 3BFA1300FF813180450031C173C00100009000B1
 
    Using the option --more handles the card status word MORE_DATA
-   (61xx) and concatenate all reponses to one block.
+   (61xx) and concatenates all reponses to one block.
 
+   Using the option "--exlen" the returned APDU may use extended
+   length up to N bytes.  If N is not given a default value is used
+   (currently 4096).
  */
 static int
 cmd_apdu (assuan_context_t ctx, char *line)
@@ -1747,10 +1750,22 @@ cmd_apdu (assuan_context_t ctx, char *line)
   size_t apdulen;
   int with_atr;
   int handle_more;
+  const char *s;
+  size_t exlen;
 
   with_atr = has_option (line, "--atr");
   handle_more = has_option (line, "--more");
 
+  if ((s=has_option_name (line, "--exlen")))
+    {
+      if (*s == '=')
+        exlen = strtoul (s+1, NULL, 0);
+      else
+        exlen = 4096;
+    }
+  else
+    exlen = 0;
+
   line = skip_options (line);
 
   if ( IS_LOCKED (ctrl) )
@@ -1787,7 +1802,8 @@ cmd_apdu (assuan_context_t ctx, char *line)
       unsigned char *result = NULL;
       size_t resultlen;
 
-      rc = apdu_send_direct (ctrl->reader_slot, 0, apdu, apdulen, handle_more,
+      rc = apdu_send_direct (ctrl->reader_slot, exlen,
+                             apdu, apdulen, handle_more,
                              &result, &resultlen);
       if (rc)
         log_error ("apdu_send_direct failed: %s\n", gpg_strerror (rc));