Allow parallel execution waiting ACK.

ChangeLog

@@ -1,3 +1,18 @@
+2018-10-01  NIIBE Yutaka  <gniibe@fsij.org>
+
+	* src/gnuk.h (EV_EXEC_ACK_REQUIRED): New.
+	(EV_EXEC_FINISHED_ACK): Remove.
+	(CCID_STATE_CONFIRM_ACK): Remove.
+	(CCID_STATE_ACK_REQUIRED_0, CCID_STATE_ACK_REQUIRED_1): New.
+	* src/openpgp.c (cmd_pso): Send EV_EXEC_ACK_REQUIRED, if needed.
+	(cmd_internal_authenticate): Likewise.
+	(process_command_apdu): No return value.
+	(openpgp_card_thread): Always send EV_EXEC_FINISHED.
+	* src/usb-ccid.c (ccid_send_data_block_time_extension): Follow the
+	change of state.
+	(ccid_handle_data, ccid_handle_timeout): Likewise.
+	(ccid_thread): Handle EV_EXEC_ACK_REQUIRED.
+
 2018-09-27  NIIBE Yutaka  <gniibe@fsij.org>
 
 	* src/gnuk.h (NR_DO_UIF_SIG, NR_DO_UIF_DEC, NR_DO_UIF_AUT): New.

src/gnuk.h

@@ -25,8 +25,8 @@ void ccid_card_change_signal (int how);
 
 /* CCID thread */
 #define EV_RX_DATA_READY      1 /* USB Rx data available  */
-#define EV_EXEC_FINISHED      2 /* OpenPGP Execution finished */
+#define EV_EXEC_ACK_REQUIRED  2 /* OpenPGPcard Execution ACK required*/
-#define EV_EXEC_FINISHED_ACK  4 /* OpenPGP Execution finished, requires ACK */
+#define EV_EXEC_FINISHED      4 /* OpenPGPcard Execution finished */
 #define EV_TX_FINISHED        8 /* CCID Tx finished  */
 #define EV_CARD_CHANGE       16
 
@@ -55,7 +55,8 @@ enum ccid_state {
   CCID_STATE_START,		/* Initial */
   CCID_STATE_WAIT,		/* Waiting APDU */
   CCID_STATE_EXECUTE,		/* Executing command */
-  CCID_STATE_CONFIRM_ACK,	/* Execution finished, waiting user's ACK */
+  CCID_STATE_ACK_REQUIRED_0,	/* Ack required (executing)*/
+  CCID_STATE_ACK_REQUIRED_1,	/* Waiting user's ACK (execution finished) */
   CCID_STATE_RECEIVE,		/* APDU Received Partially */
   CCID_STATE_SEND,		/* APDU Sent Partially */
 

src/openpgp.c

@@ -145,7 +145,7 @@ get_pinpad_input (int msg_code)
 #endif
 
 static void
-cmd_verify (void)
+cmd_verify (struct eventflag *ccid_comm)
 {
   int len;
   uint8_t p1 = P1 (apdu);
@@ -153,6 +153,7 @@ cmd_verify (void)
   int r;
   const uint8_t *pw;
 
+  (void)ccid_comm;
   DEBUG_INFO (" - VERIFY\r\n");
   DEBUG_BYTE (p2);
 
@@ -275,7 +276,7 @@ gpg_change_keystring (int who_old, const uint8_t *old_ks,
 }
 
 static void
-cmd_change_password (void)
+cmd_change_password (struct eventflag *ccid_comm)
 {
   uint8_t old_ks[KEYSTRING_MD_SIZE];
   uint8_t new_ks0[KEYSTRING_SIZE];
@@ -295,6 +296,7 @@ cmd_change_password (void)
   int salt_len;
   const uint8_t *ks_pw3;
 
+  (void)ccid_comm;
   DEBUG_INFO ("Change PW\r\n");
   DEBUG_BYTE (who);
 
@@ -547,7 +549,7 @@ s2k (const unsigned char *salt, size_t slen,
 
 
 static void
-cmd_reset_user_password (void)
+cmd_reset_user_password (struct eventflag *ccid_comm)
 {
   uint8_t p1 = P1 (apdu);
   int len;
@@ -561,6 +563,7 @@ cmd_reset_user_password (void)
   const uint8_t *salt;
   int salt_len;
 
+  (void)ccid_comm;
   DEBUG_INFO ("Reset PW1\r\n");
   DEBUG_BYTE (p1);
 
@@ -695,12 +698,13 @@ cmd_reset_user_password (void)
 }
 
 static void
-cmd_put_data (void)
+cmd_put_data (struct eventflag *ccid_comm)
 {
   uint8_t *data;
   uint16_t tag;
   int len;
 
+  (void)ccid_comm;
   DEBUG_INFO (" - PUT DATA\r\n");
 
   tag = ((P1 (apdu)<<8) | P2 (apdu));
@@ -710,8 +714,9 @@ cmd_put_data (void)
 }
 
 static void
-cmd_pgp_gakp (void)
+cmd_pgp_gakp (struct eventflag *ccid_comm)
 {
+  (void)ccid_comm;
   DEBUG_INFO (" - Generate Asymmetric Key Pair\r\n");
   DEBUG_BYTE (P1 (apdu));
 
@@ -745,12 +750,13 @@ gpg_get_firmware_update_key (uint8_t keyno)
 #endif
 
 static void
-cmd_read_binary (void)
+cmd_read_binary (struct eventflag *ccid_comm)
 {
   int is_short_EF = (P1 (apdu) & 0x80) != 0;
   uint8_t file_id;
   uint16_t offset;
 
+  (void)ccid_comm;
   DEBUG_INFO (" - Read binary\r\n");
 
   if (is_short_EF)
@@ -821,8 +827,9 @@ cmd_read_binary (void)
 }
 
 static void
-cmd_select_file (void)
+cmd_select_file (struct eventflag *ccid_comm)
 {
+  (void)ccid_comm;
   if (P1 (apdu) == 4)	/* Selection by DF name */
     {
       DEBUG_INFO (" - select DF by name\r\n");
@@ -891,10 +898,11 @@ cmd_select_file (void)
 }
 
 static void
-cmd_get_data (void)
+cmd_get_data (struct eventflag *ccid_comm)
 {
   uint16_t tag = ((P1 (apdu)<<8) | P2 (apdu));
 
+  (void)ccid_comm;
   DEBUG_INFO (" - Get Data\r\n");
 
   gpg_do_get_data (tag, 0);
@@ -909,7 +917,7 @@ cmd_get_data (void)
 #define ECC_CIPHER_DO_HEADER_SIZE 7
 
 static void
-cmd_pso (void)
+cmd_pso (struct eventflag *ccid_comm)
 {
   int len = apdu.cmd_apdu_data_len;
   int r = -1;
@@ -936,6 +944,11 @@ cmd_pso (void)
 	  return;
 	}
 
+#ifdef ACKBTN_SUPPORT
+      if (gpg_do_get_uif (GPG_KEY_FOR_SIGNING))
+	eventflag_signal (ccid_comm, EV_EXEC_ACK_REQUIRED);
+#endif
+
       if (attr == ALGO_RSA2K || attr == ALGO_RSA4K)
 	{
 	  /* Check size of digestInfo */
@@ -1027,6 +1040,11 @@ cmd_pso (void)
 	  return;
 	}
 
+#ifdef ACKBTN_SUPPORT
+      if (gpg_do_get_uif (GPG_KEY_FOR_DECRYPTION))
+	eventflag_signal (ccid_comm, EV_EXEC_ACK_REQUIRED);
+#endif
+
       if (attr == ALGO_RSA2K || attr == ALGO_RSA4K)
 	{
 	  /* Skip padding 0x00 */
@@ -1103,7 +1121,7 @@ cmd_pso (void)
 
 #define MAX_RSA_DIGEST_INFO_LEN 102 /* 40% */
 static void
-cmd_internal_authenticate (void)
+cmd_internal_authenticate (struct eventflag *ccid_comm)
 {
   int attr = gpg_get_algo_attr (GPG_KEY_FOR_AUTHENTICATION);
   int pubkey_len = gpg_get_algo_attr_key_size (GPG_KEY_FOR_AUTHENTICATION,
@@ -1133,6 +1151,11 @@ cmd_internal_authenticate (void)
       return;
     }
 
+#ifdef ACKBTN_SUPPORT
+  if (gpg_do_get_uif (GPG_KEY_FOR_AUTHENTICATION))
+    eventflag_signal (ccid_comm, EV_EXEC_ACK_REQUIRED);
+#endif
+
   if (attr == ALGO_RSA2K || attr == ALGO_RSA4K)
     {
       if (len > MAX_RSA_DIGEST_INFO_LEN)
@@ -1309,10 +1332,11 @@ modify_binary (uint8_t op, uint8_t p1, uint8_t p2, int len)
 
 #if defined(CERTDO_SUPPORT)
 static void
-cmd_update_binary (void)
+cmd_update_binary (struct eventflag *ccid_comm)
 {
   int len = apdu.cmd_apdu_data_len;
 
+  (void)ccid_comm;
   DEBUG_INFO (" - UPDATE BINARY\r\n");
   modify_binary (MBD_OPRATION_UPDATE, P1 (apdu), P2 (apdu), len);
   DEBUG_INFO ("UPDATE BINARY done.\r\n");
@@ -1321,10 +1345,11 @@ cmd_update_binary (void)
 
 
 static void
-cmd_write_binary (void)
+cmd_write_binary (struct eventflag *ccid_comm)
 {
   int len = apdu.cmd_apdu_data_len;
 
+  (void)ccid_comm;
   DEBUG_INFO (" - WRITE BINARY\r\n");
   modify_binary (MBD_OPRATION_WRITE, P1 (apdu), P2 (apdu), len);
   DEBUG_INFO ("WRITE BINARY done.\r\n");
@@ -1333,7 +1358,7 @@ cmd_write_binary (void)
 
 #ifdef FLASH_UPGRADE_SUPPORT
 static void
-cmd_external_authenticate (void)
+cmd_external_authenticate (struct eventflag *ccid_comm)
 {
   const uint8_t *pubkey;
   const uint8_t *signature = apdu.cmd_apdu_data;
@@ -1341,6 +1366,7 @@ cmd_external_authenticate (void)
   uint8_t keyno = P2 (apdu);
   int r;
 
+  (void)ccid_comm;
   DEBUG_INFO (" - EXTERNAL AUTHENTICATE\r\n");
 
   if (keyno >= 4)
@@ -1376,10 +1402,11 @@ cmd_external_authenticate (void)
 #endif
 
 static void
-cmd_get_challenge (void)
+cmd_get_challenge (struct eventflag *ccid_comm)
 {
   int len = apdu.expected_res_size;
 
+  (void)ccid_comm;
   DEBUG_INFO (" - GET CHALLENGE\r\n");
 
   if (len > CHALLENGE_LEN)
@@ -1404,8 +1431,9 @@ cmd_get_challenge (void)
 
 #ifdef LIFE_CYCLE_MANAGEMENT_SUPPORT
 static void
-cmd_activate_file (void)
+cmd_activate_file (struct eventflag *ccid_comm)
 {
+  (void)ccid_comm;
   if (file_selection != FILE_CARD_TERMINATED)
     {
       GPG_NO_RECORD ();
@@ -1418,13 +1446,13 @@ cmd_activate_file (void)
 }
 
 static void
-cmd_terminate_df (void)
+cmd_terminate_df (struct eventflag *ccid_comm)
 {
   const uint8_t *ks_pw3;
-
   uint8_t p1 = P1 (apdu);
   uint8_t p2 = P2 (apdu);
 
+  (void)ccid_comm;
   if (file_selection != FILE_DF_OPENPGP)
     {
       GPG_NO_RECORD ();
@@ -1468,7 +1496,7 @@ cmd_terminate_df (void)
 struct command
 {
   uint8_t command;
-  void (*cmd_handler) (void);
+  void (*cmd_handler) (struct eventflag *ccid_comm);
 };
 
 const struct command cmds[] = {
@@ -1501,14 +1529,11 @@ const struct command cmds[] = {
 };
 #define NUM_CMDS ((int)(sizeof (cmds) / sizeof (struct command)))
 
-static int
+static void
-process_command_apdu (void)
+process_command_apdu (struct eventflag *ccid_comm)
 {
   int i;
   uint8_t cmd = INS (apdu);
-#ifdef ACKBTN_SUPPORT
-  uint8_t was_signing = (P1 (apdu) == 0x9e && P2 (apdu) == 0x9a);
-#endif
 
   for (i = 0; i < NUM_CMDS; i++)
     if (cmds[i].command == cmd)
@@ -1529,7 +1554,7 @@ process_command_apdu (void)
       else
 	{
 	  chopstx_setcancelstate (1);
-	  cmds[i].cmd_handler ();
+	  cmds[i].cmd_handler (ccid_comm);
 	  chopstx_setcancelstate (0);
 	}
     }
@@ -1539,22 +1564,6 @@ process_command_apdu (void)
       DEBUG_BYTE (cmd);
       GPG_NO_INS ();
     }
-
-#ifdef ACKBTN_SUPPORT
-  if (cmd == INS_PSO)
-    {
-      if (was_signing)
-	return gpg_do_get_uif (GPG_KEY_FOR_SIGNING);
-      else
-	return gpg_do_get_uif (GPG_KEY_FOR_DECRYPTION);
-    }
-  else if (cmd == INS_INTERNAL_AUTHENTICATE)
-    return gpg_do_get_uif (GPG_KEY_FOR_AUTHENTICATION);
-  else
-    return 0;
-#else
-  return 0;
-#endif
 }
 
 void *
@@ -1572,7 +1581,6 @@ openpgp_card_thread (void *arg)
       int len, pw_len, newpw_len;
 #endif
       eventmask_t m = eventflag_wait (openpgp_comm);
-      int r = 0;
 
       DEBUG_INFO ("GPG!: ");
 
@@ -1660,10 +1668,10 @@ openpgp_card_thread (void *arg)
 	break;
 
       led_blink (LED_START_COMMAND);
-      r = process_command_apdu ();
+      process_command_apdu (ccid_comm);
       led_blink (LED_FINISH_COMMAND);
     done:
-      eventflag_signal (ccid_comm, r? EV_EXEC_FINISHED_ACK : EV_EXEC_FINISHED);
+      eventflag_signal (ccid_comm, EV_EXEC_FINISHED);
     }
 
   gpg_fini ();

src/usb-ccid.c

@@ -1081,8 +1081,7 @@ static void
 ccid_send_data_block_time_extension (struct ccid *c)
 {
   ccid_send_data_block_internal (c, CCID_CMD_STATUS_TIMEEXT,
-				 (c->ccid_state == CCID_STATE_CONFIRM_ACK?
+				 c->ccid_state == CCID_STATE_EXECUTE? 1: 0xff);
-				  0xff : 1));
 }
 
 static void
@@ -1467,7 +1466,8 @@ ccid_handle_data (struct ccid *c)
 	}
       break;
     case CCID_STATE_EXECUTE:
-    case CCID_STATE_CONFIRM_ACK:
+    case CCID_STATE_ACK_REQUIRED_0:
+    case CCID_STATE_ACK_REQUIRED_1:
       if (c->ccid_header.msg_type == CCID_POWER_OFF)
 	next_state = ccid_power_off (c);
       else if (c->ccid_header.msg_type == CCID_SLOT_STATUS)
@@ -1496,7 +1496,8 @@ ccid_handle_timeout (struct ccid *c)
   switch (c->ccid_state)
     {
     case CCID_STATE_EXECUTE:
-    case CCID_STATE_CONFIRM_ACK:
+    case CCID_STATE_ACK_REQUIRED_0:
+    case CCID_STATE_ACK_REQUIRED_1:
       ccid_send_data_block_time_extension (c);
       break;
     default:
@@ -1775,8 +1776,11 @@ ccid_thread (void *arg)
 	  ackbtn_disable ();
 	  chopstx_intr_done (&ack_intr);
 	  led_blink (LED_FINISH_COMMAND);
-	  if (c->ccid_state == CCID_STATE_CONFIRM_ACK)
+	  if (c->ccid_state == CCID_STATE_ACK_REQUIRED_1)
 	    goto exec_done;
+
+	  c->ccid_state = CCID_STATE_EXECUTE;
+	  continue;
 	}
 #endif
 
@@ -1839,17 +1843,21 @@ ccid_thread (void *arg)
 		c->ccid_state = CCID_STATE_WAIT;
 	      }
 	  }
+#ifdef ACKBTN_SUPPORT
+	else if (c->ccid_state == CCID_STATE_ACK_REQUIRED_0)
+	  c->ccid_state = CCID_STATE_ACK_REQUIRED_1;
+#endif
 	else
 	  {
 	    DEBUG_INFO ("ERR05\r\n");
 	  }
 #ifdef ACKBTN_SUPPORT
-      else if (m == EV_EXEC_FINISHED_ACK)
+      else if (m == EV_EXEC_ACK_REQUIRED)
 	if (c->ccid_state == CCID_STATE_EXECUTE)
 	  {
 	    ackbtn_enable ();
 	    led_blink (LED_WAIT_FOR_BUTTON);
-	    c->ccid_state = CCID_STATE_CONFIRM_ACK;
+	    c->ccid_state = CCID_STATE_ACK_REQUIRED_0;
 	  }
 	else
 	  {