on/off support
authorNIIBE Yutaka <gniibe@fsij.org>
Wed, 8 Dec 2010 05:10:30 +0000 (14:10 +0900)
committerNIIBE Yutaka <gniibe@fsij.org>
Wed, 8 Dec 2010 05:10:30 +0000 (14:10 +0900)
ChangeLog
doc/NOTES
src/ac.c
src/gnuk.h
src/main.c
src/openpgp-do.c
src/openpgp.c
src/usb-icc.c
src/usb_desc.c

index 28f6aa3..0c5011e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2010-12-08  NIIBE Yutaka  <gniibe@fsij.org>
+
+       * src/main.c (main): Don't create GPGThread here.
+       * src/usb-icc.c (icc_power_on): But create here, when requested.
+       (icc_power_off): Terminate GPGThread.
+       * src/openpgp.c (gpg_init, gpg_fini): New.
+       (GPGthread): Check chThdShouldTerminate.  Call gpg_init and gpg_fini.
+
 2010-12-07  NIIBE Yutaka  <gniibe@fsij.org>
 
        USB CCID/ICC implementation changes.
index 4fc4825..86626db 100644 (file)
--- a/doc/NOTES
+++ b/doc/NOTES
@@ -1,17 +1,28 @@
 USB communication
 =================
 
-  * No command chaining, but extended APDU and extended Lc and Le
+  * No command chaining, but extended APDU and extended Lc and Le.
+    I think that this keep the code simple.
 
-  * dwMaxCCIDMessageLength: 64
+  * Once, the value of dwMaxCCIDMessageLength was 64 and we supported
+    ICC block chaining, so that we could not handle multple Bulk
+    transactions.
+
+  * Now, the value of dwMaxCCIDMessageLength is 320, that's the size
+    of header of ICC block plus size of maximum APDU (by 64
+    granularity).  Still, some ccid implementation sends ICC block
+    using chaining (unfortunately), so we keep the code of ICC block
+    chaining.
 
 
 OpenPGP card protocol implementation
 ====================================
 
-I try to follow "no clear password(s)" policy.
-After key import, keystrings are also removed.
-But because of this, it is not that easy to overwrite key(s).
+I try to follow "no clear password(s)" policy, even if it is on
+protected flash memory.  Futher, keystrings for user and reset code
+are removed after key imports.  Because of this, replacing keys
+are not possible without password information.  Thus, replacing
+existing keys are not supported.
 
 
 How a private key is stored
index 4326061..4dd7357 100644 (file)
--- a/src/ac.c
+++ b/src/ac.c
@@ -243,3 +243,10 @@ verify_admin (const uint8_t *pw, int pw_len)
   auth_status |= AC_ADMIN_AUTHORIZED;
   return 1;
 }
+
+void
+ac_fini (void)
+{
+  auth_status = AC_NONE_AUTHORIZED;
+  memset (keystring_md_pw3, 0, KEYSTRING_MD_SIZE);
+}
index 02bee69..4078b28 100644 (file)
@@ -25,12 +25,6 @@ extern void *memcpy (void *dest, const void *src, size_t n);
 extern void *memset (void *s, int c, size_t n);
 extern int memcmp (const void *s1, const void *s2, size_t n);
 
-/*
- * Interface between ICC<-->GPG
- */
-extern Thread *icc_thread;
-extern Thread *gpg_thread;
-
 #define EV_EXEC_FINISHED ((eventmask_t)2)       /* GPG Execution finished */
 
 /* maximum cmd apdu data is key import 22+4+128+128 (proc_key_import) */
@@ -79,6 +73,7 @@ extern int verify_admin_0 (const uint8_t *pw, int buf_len, int pw_len_known);
 
 extern void ac_reset_pso_cds (void);
 extern void ac_reset_pso_other (void);
+extern void ac_fini (void);
 
 
 extern void write_res_apdu (const uint8_t *p, int len,
index 57bb856..70319e8 100644 (file)
@@ -155,9 +155,6 @@ _write (const char *s, int size)
 static WORKING_AREA(waUSBthread, 128);
 extern msg_t USBthread (void *arg);
 
-static WORKING_AREA(waGPGthread, 128*16);
-extern msg_t GPGthread (void *arg);
-
 Thread *blinker_thread;
 /*
  * LEDs blinks.
@@ -176,16 +173,12 @@ main (int argc, char **argv)
   eventmask_t m;
   uint8_t led_state = 0;
   int count = 0;
-  const uint8_t *flash_data_start;
 
   (void)argc;
   (void)argv;
 
   blinker_thread = chThdSelf ();
 
-  flash_data_start = flash_init ();
-  gpg_data_scan (flash_data_start);
-
   usb_lld_init ();
   USB_Init();
 
@@ -199,7 +192,6 @@ main (int argc, char **argv)
 #endif
 
   chThdCreateStatic (waUSBthread, sizeof(waUSBthread), NORMALPRIO, USBthread, NULL);
-  chThdCreateStatic (waGPGthread, sizeof(waGPGthread), NORMALPRIO, GPGthread, NULL);
 
   while (1)
     {
@@ -225,9 +217,9 @@ main (int argc, char **argv)
       if (bDeviceState == CONFIGURED && (count % 100) == 0)
        {
          DEBUG_SHORT (count / 100);
-         _write ("\r\nThis is ChibiOS 2.0.2 on Olimex STM32-H103.\r\n"
+         _write ("\r\nThis is ChibiOS 2.0.8 on STM32.\r\n"
                  "Testing USB driver.\n\n"
-                 "Hello world\r\n\r\n", 47+21+15);
+                 "Hello world\r\n\r\n", 35+21+15);
        }
 #endif
     }
index b0af770..aa0cb04 100644 (file)
@@ -78,7 +78,7 @@ gpg_increment_digital_signature_counter (void)
 }
 
 #define PASSWORD_ERRORS_MAX 3  /* >= errors, it will be locked */
-const uint8_t *pw_err_counter_p[3];
+static const uint8_t *pw_err_counter_p[3];
 
 static int
 gpg_get_pw_err_counter (uint8_t which)
@@ -836,14 +836,14 @@ proc_key_import (const uint8_t *data, int len)
     return 1;
 }
 
-static const uint16_t const cmp_ch_data[] = {
+static const uint16_t cmp_ch_data[] = {
   3,
   GPG_DO_NAME,
   GPG_DO_LANGUAGE,
   GPG_DO_SEX,
 };
 
-static const uint16_t const cmp_app_data[] = {
+static const uint16_t cmp_app_data[] = {
   10,
   GPG_DO_AID,
   GPG_DO_HIST_BYTES,
@@ -854,7 +854,7 @@ static const uint16_t const cmp_app_data[] = {
   GPG_DO_FP_ALL, GPG_DO_CAFP_ALL, GPG_DO_KGTIME_ALL
 };
 
-static const uint16_t const cmp_ss_temp[] = { 1, GPG_DO_DS_COUNT };
+static const uint16_t cmp_ss_temp[] = { 1, GPG_DO_DS_COUNT };
 
 static const struct do_table_entry
 gpg_do_table[] = {
index c5ffebc..a7ed79a 100644 (file)
@@ -75,7 +75,24 @@ write_res_apdu (const uint8_t *p, int len, uint8_t sw1, uint8_t sw2)
 #define FILE_EF_DIR    3
 #define FILE_EF_SERIAL 4
 
-static uint8_t file_selection = FILE_NONE;
+static uint8_t file_selection;
+
+static void
+gpg_init (void)
+{
+  const uint8_t *flash_data_start;
+
+  file_selection = FILE_NONE;
+  flash_data_start = flash_init ();
+  gpg_data_scan (flash_data_start);
+}
+
+static void
+gpg_fini (void)
+{
+  ac_fini ();
+  memset ((void *)kd, 0, sizeof (struct key_data)*3);
+}
 
 static void
 cmd_verify (void)
@@ -714,17 +731,16 @@ process_command_apdu (void)
     }
 }
 
-Thread *gpg_thread;
-
 msg_t
 GPGthread (void *arg)
 {
-  (void)arg;
+  Thread *icc_thread = (Thread *)arg;
+
+  gpg_init ();
 
-  gpg_thread = chThdSelf ();
   chEvtClear (ALL_EVENTS);
 
-  while (1)
+  while (!chThdShouldTerminate ())
     {
       eventmask_t m;
 
@@ -738,5 +754,6 @@ GPGthread (void *arg)
       chEvtSignal (icc_thread, EV_EXEC_FINISHED);
     }
 
+  gpg_fini ();
   return 0;
 }
index 2f69cdc..1b2ca56 100644 (file)
@@ -75,7 +75,7 @@ struct icc_header {
   uint16_t param;
 } __attribute__((packed));
 
-static int icc_data_size;
+int icc_data_size;
 
 /*
  * USB-ICC communication could be considered "half duplex".
@@ -113,7 +113,7 @@ static uint8_t *icc_chain_p;
  */
 static int icc_tx_size;
 
-Thread *icc_thread;
+static Thread *icc_thread;
 
 #define EV_RX_DATA_READY (eventmask_t)1  /* USB Rx data available  */
 /* EV_EXEC_FINISHED == 2 */
@@ -292,12 +292,21 @@ icc_error (int offset)
   SetEPTxValid (ENDP1);
 }
 
+static Thread *gpg_thread;
+static WORKING_AREA(waGPGthread, 128*16);
+extern msg_t GPGthread (void *arg);
+
+
 /* Send back ATR (Answer To Reset) */
 enum icc_state
 icc_power_on (void)
 {
   int size_atr;
 
+  if (gpg_thread == NULL)
+    gpg_thread = chThdCreateStatic (waGPGthread, sizeof(waGPGthread),
+                                   NORMALPRIO, GPGthread, (void *)icc_thread);
+
   size_atr = sizeof (ATR);
   icc_buffer[0] = ICC_DATA_BLOCK_RET;
   icc_buffer[1] = size_atr;
@@ -359,6 +368,13 @@ icc_send_status (void)
 enum icc_state
 icc_power_off (void)
 {
+  if (gpg_thread)
+    {
+      chThdTerminate (gpg_thread);
+      chThdWait (gpg_thread);
+      gpg_thread = NULL;
+    }
+
   icc_send_status ();
   DEBUG_INFO ("OFF\r\n");
   return ICC_STATE_START;
@@ -524,7 +540,12 @@ icc_handle_data (void)
        }
       break;
     case ICC_STATE_RECEIVE:
-      if (icc_header->msg_type == ICC_SLOT_STATUS)
+      if (icc_header->msg_type == ICC_POWER_OFF)
+       {
+         icc_chain_p = NULL;
+         next_state = icc_power_off ();
+       }
+      else if (icc_header->msg_type == ICC_SLOT_STATUS)
        icc_send_status ();
       else if (icc_header->msg_type == ICC_XFR_BLOCK)
        {
@@ -557,10 +578,7 @@ icc_handle_data (void)
       break;
     case ICC_STATE_EXECUTE:
       if (icc_header->msg_type == ICC_POWER_OFF)
-       {
-         /* XXX: Kill GPG thread */
-         next_state = icc_power_off ();
-       }
+       next_state = icc_power_off ();
       else if (icc_header->msg_type == ICC_SLOT_STATUS)
        icc_send_status ();
       else
index decdaf1..3005ec7 100644 (file)
@@ -79,6 +79,13 @@ static const uint8_t gnukConfigDescriptor[] = {
   0xfe, 0, 0, 0,         /* dwMaxIFSD:  */
   0, 0, 0, 0,            /* dwSynchProtocols: FIXED VALUE */
   0, 0, 0, 0,            /* dwMechanical: FIXED VALUE */
+  /*
+   * According to Specification for USB ICCD (revision 1.0),
+   * dwFeatures should be 0x00040840.
+   *
+   * It is different now for better interaction to GPG's in-stock
+   * ccid-driver.
+   */
 #ifdef DEBUG
   0x82, 0x04, 0x04, 0x00, /* dwFeatures:
                           *  Short and extended APDU level: 0x40000