From f810fb83d350776c78f3c3004cebb002232db428 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Mon, 1 Oct 2018 14:06:31 +0900 Subject: [PATCH] Allow parallel execution waiting ACK. --- ChangeLog | 15 ++++++++ chopstx | 2 +- src/gnuk.h | 7 ++-- src/openpgp.c | 94 +++++++++++++++++++++++++++----------------------- src/usb-ccid.c | 22 ++++++++---- 5 files changed, 86 insertions(+), 54 deletions(-) diff --git a/ChangeLog b/ChangeLog index f220ea8..61cab23 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2018-10-01 NIIBE Yutaka + + * 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 * src/gnuk.h (NR_DO_UIF_SIG, NR_DO_UIF_DEC, NR_DO_UIF_AUT): New. diff --git a/chopstx b/chopstx index 7dc67d2..0ed2e95 160000 --- a/chopstx +++ b/chopstx @@ -1 +1 @@ -Subproject commit 7dc67d2210fb566bc370f0eea7ce9d9c5744760b +Subproject commit 0ed2e95ea269ca9bbb9db5185577049da7eba1f4 diff --git a/src/gnuk.h b/src/gnuk.h index 4bdc2f9..e2ea0a3 100644 --- a/src/gnuk.h +++ b/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_FINISHED_ACK 4 /* OpenPGP Execution finished, requires ACK */ +#define EV_EXEC_ACK_REQUIRED 2 /* OpenPGPcard Execution ACK required*/ +#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 */ diff --git a/src/openpgp.c b/src/openpgp.c index af17abe..11746bb 100644 --- a/src/openpgp.c +++ b/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 -process_command_apdu (void) +static 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 (); diff --git a/src/usb-ccid.c b/src/usb-ccid.c index 941a227..3d043a3 100644 --- a/src/usb-ccid.c +++ b/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? - 0xff : 1)); + c->ccid_state == CCID_STATE_EXECUTE? 1: 0xff); } 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 {