diff --git a/.github/workflows/lint-workflow.yml b/.github/workflows/lint-workflow.yml index b9ea676..f74416d 100644 --- a/.github/workflows/lint-workflow.yml +++ b/.github/workflows/lint-workflow.yml @@ -19,8 +19,8 @@ jobs: uses: actions/checkout@v2 - name: Lint - uses: DoozyX/clang-format-lint-action@v0.13 + uses: DoozyX/clang-format-lint-action@v0.14 with: source: "./" extensions: "h,c" - clangFormatVersion: 12.0.0 + clangFormatVersion: 12.0.1 diff --git a/CHANGELOG.md b/CHANGELOG.md index f7b3867..f1d6de3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [1.9.20](https://github.com/ledgerhq/app-ethereum/compare/1.9.19...1.9.20) - 2022-XX-XX +### Changed + +- EIP-191 improvements, now lets the user see the entire message by chunks 255 characters (LNX & LNS+), LNS still limited to the first chunk of 99 characters + ## [1.9.19](https://github.com/ledgerhq/app-ethereum/compare/1.9.18...1.9.19) - 2022-06-15 ### Added @@ -18,7 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed -- EIP-191 signatures now show (up to 99 characters on LNS and 255 on LNX & LNS) the actual data contained in the message (clear-signing) +- EIP-191 signatures now show (up to 99 characters on LNS and 255 on LNX & LNS+) the actual data contained in the message (clear-signing) ### Fixed diff --git a/src/apdu_constants.h b/src/apdu_constants.h index 1937075..219b7d0 100644 --- a/src/apdu_constants.h +++ b/src/apdu_constants.h @@ -58,6 +58,9 @@ #define OFFSET_LC 4 #define OFFSET_CDATA 5 +#define ERR_APDU_EMPTY 0x6982 +#define ERR_APDU_SIZE_MISMATCH 0x6983 + void handleGetPublicKey(uint8_t p1, uint8_t p2, const uint8_t *dataBuffer, @@ -88,12 +91,10 @@ void handleGetAppConfiguration(uint8_t p1, uint16_t dataLength, unsigned int *flags, unsigned int *tx); -void handleSignPersonalMessage(uint8_t p1, +bool handleSignPersonalMessage(uint8_t p1, uint8_t p2, - const uint8_t *dataBuffer, - uint16_t dataLength, - unsigned int *flags, - unsigned int *tx); + const uint8_t *const payload, + uint8_t length); void handleSignEIP712Message(uint8_t p1, uint8_t p2, const uint8_t *dataBuffer, diff --git a/src/eth_plugin_handler.c b/src/eth_plugin_handler.c index d0de69c..e5187ca 100644 --- a/src/eth_plugin_handler.c +++ b/src/eth_plugin_handler.c @@ -113,7 +113,7 @@ static bool eth_plugin_perform_init_old_internal(uint8_t *contractAddress, j++) { if (memcmp(init->selector, (const void *) PIC(selectors[j]), SELECTOR_SIZE) == 0) { if ((INTERNAL_ETH_PLUGINS[i].availableCheck == NULL) || - ((PluginAvailableCheck) PIC(INTERNAL_ETH_PLUGINS[i].availableCheck)) ()) { + ((PluginAvailableCheck) PIC(INTERNAL_ETH_PLUGINS[i].availableCheck))()) { strlcpy(dataContext.tokenContext.pluginName, INTERNAL_ETH_PLUGINS[i].alias, PLUGIN_ID_LENGTH); diff --git a/src/main.c b/src/main.c index cf99d6f..2a32e22 100644 --- a/src/main.c +++ b/src/main.c @@ -664,12 +664,11 @@ void handleApdu(unsigned int *flags, unsigned int *tx) { case INS_SIGN_PERSONAL_MESSAGE: memset(tmpCtx.transactionContext.tokenSet, 0, MAX_ITEMS); + *flags |= IO_ASYNCH_REPLY; handleSignPersonalMessage(G_io_apdu_buffer[OFFSET_P1], G_io_apdu_buffer[OFFSET_P2], G_io_apdu_buffer + OFFSET_CDATA, - G_io_apdu_buffer[OFFSET_LC], - flags, - tx); + G_io_apdu_buffer[OFFSET_LC]); break; case INS_SIGN_EIP_712_MESSAGE: @@ -771,9 +770,11 @@ void app_main(void) { // no apdu received, well, reset the session, and reset the // bootloader configuration if (rx == 0) { - THROW(0x6982); + THROW(ERR_APDU_EMPTY); + } + if (rx > OFFSET_LC && rx != (G_io_apdu_buffer[OFFSET_LC] + 5)) { + THROW(ERR_APDU_SIZE_MISMATCH); } - PRINTF("New APDU received:\n%.*H\n", rx, G_io_apdu_buffer); handleApdu(&flags, &tx); } diff --git a/src/ui_callbacks.h b/src/ui_callbacks.h index 46e3665..1616e3e 100644 --- a/src/ui_callbacks.h +++ b/src/ui_callbacks.h @@ -10,8 +10,8 @@ unsigned int io_seproxyhal_touch_tx_ok(const bagl_element_t *e); unsigned int io_seproxyhal_touch_tx_cancel(const bagl_element_t *e); unsigned int io_seproxyhal_touch_address_ok(const bagl_element_t *e); unsigned int io_seproxyhal_touch_address_cancel(const bagl_element_t *e); -unsigned int io_seproxyhal_touch_signMessage_ok(const bagl_element_t *e); -unsigned int io_seproxyhal_touch_signMessage_cancel(const bagl_element_t *e); +unsigned int io_seproxyhal_touch_signMessage_ok(void); +unsigned int io_seproxyhal_touch_signMessage_cancel(void); unsigned int io_seproxyhal_touch_data_ok(const bagl_element_t *e); unsigned int io_seproxyhal_touch_data_cancel(const bagl_element_t *e); unsigned int io_seproxyhal_touch_signMessage712_v0_ok(const bagl_element_t *e); @@ -23,7 +23,6 @@ unsigned int io_seproxyhal_touch_stark_unsafe_sign_ok(const bagl_element_t *e); unsigned int io_seproxyhal_touch_stark_pubkey_ok(const bagl_element_t *e); unsigned int io_seproxyhal_touch_stark_ok(const bagl_element_t *e); -void ui_idle(void); void ui_warning_contract_data(void); void io_seproxyhal_send_status(uint32_t sw); diff --git a/src_bagl/common_ui.c b/src_bagl/common_ui.c index 57be492..e1449f0 100644 --- a/src_bagl/common_ui.c +++ b/src_bagl/common_ui.c @@ -32,10 +32,6 @@ void ui_display_public_key(void) { ux_flow_init(0, ux_display_public_flow, NULL); } -void ui_display_sign(void) { - ux_flow_init(0, ux_sign_flow, NULL); -} - void ui_sign_712_v0(void) { ux_flow_init(0, ux_sign_712_v0_flow, NULL); } @@ -74,4 +70,4 @@ void ui_confirm_parameter(void) { ux_flow_init(0, ux_confirm_parameter_flow, NULL); } -#endif // HAVE_BAGL \ No newline at end of file +#endif // HAVE_BAGL diff --git a/src_bagl/ui_flow.c b/src_bagl/ui_flow.c index d6c0a16..79cedf4 100644 --- a/src_bagl/ui_flow.c +++ b/src_bagl/ui_flow.c @@ -1,5 +1,5 @@ #include "shared_context.h" -#include "ui_callbacks.h" +#include "common_ui.h" void display_settings(const ux_flow_step_t* const start_step); void switch_settings_blind_signing(void); @@ -184,4 +184,4 @@ UX_STEP_CB( #endif // clang-format on -UX_FLOW(ux_warning_contract_data_flow, &ux_warning_contract_data_step); \ No newline at end of file +UX_FLOW(ux_warning_contract_data_flow, &ux_warning_contract_data_step); diff --git a/src_bagl/ui_flow.h b/src_bagl/ui_flow.h index 7b18286..184036b 100644 --- a/src_bagl/ui_flow.h +++ b/src_bagl/ui_flow.h @@ -20,8 +20,6 @@ extern const ux_flow_step_t* const ux_confirm_parameter_flow[]; extern const ux_flow_step_t* const ux_approval_allowance_flow[]; -extern const ux_flow_step_t* const ux_sign_flow[]; - extern const ux_flow_step_t* const ux_sign_712_v0_flow[]; extern const ux_flow_step_t* const ux_display_public_eth2_flow[]; diff --git a/src_bagl/ui_flow_signMessage.c b/src_bagl/ui_flow_signMessage.c index ee8e39e..e4b38fd 100644 --- a/src_bagl/ui_flow_signMessage.c +++ b/src_bagl/ui_flow_signMessage.c @@ -1,9 +1,37 @@ #include "shared_context.h" #include "ui_callbacks.h" +#include "ui_flow_signMessage.h" +#include "sign_message.h" + +static uint8_t ui_pos; + +static void dummy_pre_cb(void) { + if (ui_pos == UI_191_POS_REVIEW) { +#ifdef TARGET_NANOS + skip_rest_of_message(); +#else + question_switcher(); +#endif + } else { + ux_flow_prev(); + ui_pos = UI_191_POS_REVIEW; + } +} + +#ifndef TARGET_NANOS +static void dummy_post_cb(void) { + if (ui_pos == UI_191_POS_QUESTION) { + continue_displaying_message(); + } else // UI_191_END + { + ui_191_switch_to_message_end(); + } +} +#endif // clang-format off UX_STEP_NOCB( - ux_sign_flow_1_step, + ux_191_step_review, pnn, { &C_icon_certificate, @@ -11,25 +39,50 @@ UX_STEP_NOCB( "message", }); UX_STEP_NOCB( - ux_sign_flow_2_step, + ux_191_step_message, bnnn_paging, { .title = "Message", .text = strings.tmp.tmp, }); +UX_STEP_INIT( + ux_191_step_dummy_pre, + NULL, + NULL, + { + dummy_pre_cb(); + }); +#ifndef TARGET_NANOS UX_STEP_CB( - ux_sign_flow_3_step, + ux_191_step_theres_more, + nnn, + skip_rest_of_message(), + { + "Press right to", + "continue message", + "Double-press to skip" + }); +UX_STEP_INIT( + ux_191_step_dummy_post, + NULL, + NULL, + { + dummy_post_cb(); + }); +#endif +UX_STEP_CB( + ux_191_step_sign, pbb, - io_seproxyhal_touch_signMessage_ok(NULL), + io_seproxyhal_touch_signMessage_ok(), { &C_icon_validate_14, "Sign", "message", }); UX_STEP_CB( - ux_sign_flow_4_step, + ux_191_step_cancel, pbb, - io_seproxyhal_touch_signMessage_cancel(NULL), + io_seproxyhal_touch_signMessage_cancel(), { &C_icon_crossmark, "Cancel", @@ -37,8 +90,43 @@ UX_STEP_CB( }); // clang-format on -UX_FLOW(ux_sign_flow, - &ux_sign_flow_1_step, - &ux_sign_flow_2_step, - &ux_sign_flow_3_step, - &ux_sign_flow_4_step); +UX_FLOW(ux_191_flow, + &ux_191_step_review, + &ux_191_step_message, + &ux_191_step_dummy_pre, +#ifndef TARGET_NANOS + &ux_191_step_theres_more, + &ux_191_step_dummy_post, +#endif + &ux_191_step_sign, + &ux_191_step_cancel); + +void ui_191_start(void) { + ux_flow_init(0, ux_191_flow, NULL); + ui_pos = UI_191_POS_REVIEW; +} + +void ui_191_switch_to_message(void) { + ux_flow_init(0, ux_191_flow, &ux_191_step_message); + ui_pos = UI_191_POS_REVIEW; +} + +#ifndef TARGET_NANOS +void ui_191_switch_to_message_end(void) { + // Force it to a value that will make it automatically do a prev() + ui_pos = UI_191_POS_QUESTION; + ux_flow_init(0, ux_191_flow, &ux_191_step_dummy_pre); +} +#endif + +void ui_191_switch_to_sign(void) { + ux_flow_init(0, ux_191_flow, &ux_191_step_sign); + ui_pos = UI_191_POS_END; +} + +#ifndef TARGET_NANOS +void ui_191_switch_to_question(void) { + ux_flow_init(0, ux_191_flow, &ux_191_step_theres_more); + ui_pos = UI_191_POS_QUESTION; +} +#endif diff --git a/src_bagl/ui_flow_signMessage.h b/src_bagl/ui_flow_signMessage.h new file mode 100644 index 0000000..69e047b --- /dev/null +++ b/src_bagl/ui_flow_signMessage.h @@ -0,0 +1,12 @@ +#ifndef UI_FLOW_SIGNMESSAGE_H_ +#define UI_FLOW_SIGNMESSAGE_H_ + +typedef enum { UI_191_POS_REVIEW, UI_191_POS_QUESTION, UI_191_POS_END } e_ui_191_position; + +void ui_191_start(void); +void ui_191_switch_to_message(void); +void ui_191_switch_to_message_end(void); +void ui_191_switch_to_sign(void); +void ui_191_switch_to_question(void); + +#endif // UI_FLOW_SIGNMESSAGE_H_ diff --git a/src_common/network.h b/src_common/network.h index 8a4513a..ade3152 100644 --- a/src_common/network.h +++ b/src_common/network.h @@ -8,8 +8,8 @@ #define MAX_NETWORK_TICKER_LEN 8 typedef struct network_info_s { - const char name[NETWORK_STRING_MAX_SIZE]; - const char ticker[MAX_NETWORK_TICKER_LEN]; + const char *name; + const char *ticker; uint64_t chain_id; } network_info_t; diff --git a/src_features/getEth2PublicKey/ui_common_getEth2PublicKey.c b/src_features/getEth2PublicKey/ui_common_getEth2PublicKey.c index ca10fc3..77b38c0 100644 --- a/src_features/getEth2PublicKey/ui_common_getEth2PublicKey.c +++ b/src_features/getEth2PublicKey/ui_common_getEth2PublicKey.c @@ -2,7 +2,7 @@ #include "shared_context.h" #include "feature_getEth2PublicKey.h" -#include "ui_callbacks.h" +#include "common_ui.h" unsigned int io_seproxyhal_touch_eth2_address_ok(__attribute__((unused)) const bagl_element_t *e) { uint32_t tx = set_result_get_eth2_publicKey(); diff --git a/src_features/getPublicKey/ui_common_getPublicKey.c b/src_features/getPublicKey/ui_common_getPublicKey.c index c7bd3a3..d38724a 100644 --- a/src_features/getPublicKey/ui_common_getPublicKey.c +++ b/src_features/getPublicKey/ui_common_getPublicKey.c @@ -1,6 +1,6 @@ #include "shared_context.h" #include "feature_getPublicKey.h" -#include "ui_callbacks.h" +#include "common_ui.h" unsigned int io_seproxyhal_touch_address_ok(__attribute__((unused)) const bagl_element_t *e) { uint32_t tx = set_result_get_publicKey(); diff --git a/src_features/performPrivacyOperation/ui_common_performPrivacyOperation.c b/src_features/performPrivacyOperation/ui_common_performPrivacyOperation.c index ac045f2..a2cb1c3 100644 --- a/src_features/performPrivacyOperation/ui_common_performPrivacyOperation.c +++ b/src_features/performPrivacyOperation/ui_common_performPrivacyOperation.c @@ -1,6 +1,6 @@ #include "shared_context.h" #include "feature_getPublicKey.h" -#include "ui_callbacks.h" +#include "common_ui.h" #include "feature_performPrivacyOperation.h" unsigned int io_seproxyhal_touch_privacy_ok(__attribute__((unused)) const bagl_element_t *e) { diff --git a/src_features/signMessage/cmd_signMessage.c b/src_features/signMessage/cmd_signMessage.c index 71876e0..4b4008f 100644 --- a/src_features/signMessage/cmd_signMessage.c +++ b/src_features/signMessage/cmd_signMessage.c @@ -1,199 +1,294 @@ #include -#include "shared_context.h" +#include +#include #include "apdu_constants.h" -#include "utils.h" -#include "common_ui.h" +#include "sign_message.h" +#include "ui_flow_signMessage.h" + +static uint8_t processed_size; +static struct { + sign_message_state sign_state : 1; + bool ui_started : 1; +} states; static const char SIGN_MAGIC[] = "\x19" "Ethereum Signed Message:\n"; /** - * Check if a given character is a "special" displayable ASCII character + * Send a response APDU with the given Status Word * - * @param[in] c character we're checking - * @return wether the character is special or not + * @param[in] sw status word */ -static inline bool is_char_special(char c) { - return ((c >= '\b') && (c <= '\r')); +static void apdu_reply(uint16_t sw) { + G_io_apdu_buffer[0] = (sw >> 8) & 0xff; + G_io_apdu_buffer[1] = sw & 0xff; + io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, 2); } /** - * Check if a given data is made of ASCII characters + * Get unprocessed data from last received APDU * - * @param[in] data the input data - * @param[in] the length of the input data - * @return wether the data is fully ASCII or not + * @return pointer to data in APDU buffer */ -static bool is_data_ascii(const uint8_t *const data, size_t length) { - for (uint8_t idx = 0; idx < length; ++idx) { - if (!is_char_special(data[idx]) && ((data[idx] < 0x20) || (data[idx] > 0x7e))) { - return false; - } +static const uint8_t *unprocessed_data(void) { + return &G_io_apdu_buffer[OFFSET_CDATA] + processed_size; +} + +/** + * Get size of unprocessed data from last received APDU + * + * @return size of data in bytes + */ +static size_t unprocessed_length(void) { + return G_io_apdu_buffer[OFFSET_LC] - processed_size; +} + +/** + * Get used space from UI buffer + * + * @return size in bytes + */ +static size_t ui_buffer_length(void) { + return strlen(UI_191_BUFFER); +} + +/** + * Get remaining space from UI buffer + * + * @return size in bytes + */ +static size_t remaining_ui_buffer_length(void) { + // -1 for the ending NULL byte + return (sizeof(UI_191_BUFFER) - 1) - ui_buffer_length(); +} + +/** + * Get free space from UI buffer + * + * @return pointer to the free space + */ +static char *remaining_ui_buffer(void) { + return &UI_191_BUFFER[ui_buffer_length()]; +} + +/** + * Reset the UI buffer + * + * Simply sets its first byte to a NULL character + */ +static void reset_ui_buffer(void) { + UI_191_BUFFER[0] = '\0'; +} + +/** + * Handle the data specific to the first APDU of an EIP-191 signature + * + * @param[in] data the APDU payload + * @param[in] length the payload size + * @return pointer to the start of the start of the message; \ref NULL if it failed + */ +static const uint8_t *first_apdu_data(const uint8_t *data, uint16_t *length) { + if (appState != APP_STATE_IDLE) { + reset_app_context(); + } + appState = APP_STATE_SIGNING_MESSAGE; + data = parseBip32(data, length, &tmpCtx.messageSigningContext.bip32); + if (data == NULL) { + apdu_reply(0x6a80); + return NULL; + } + + if (*length < sizeof(uint32_t)) { + PRINTF("Invalid data\n"); + apdu_reply(0x6a80); + return NULL; + } + + tmpCtx.messageSigningContext.remainingLength = U4BE(data, 0); + data += sizeof(uint32_t); + *length -= sizeof(uint32_t); + + // Initialize message header + length + cx_keccak_init(&global_sha3, 256); + cx_hash((cx_hash_t *) &global_sha3, 0, (uint8_t *) SIGN_MAGIC, sizeof(SIGN_MAGIC) - 1, NULL, 0); + snprintf(strings.tmp.tmp2, + sizeof(strings.tmp.tmp2), + "%u", + tmpCtx.messageSigningContext.remainingLength); + cx_hash((cx_hash_t *) &global_sha3, + 0, + (uint8_t *) strings.tmp.tmp2, + strlen(strings.tmp.tmp2), + NULL, + 0); + reset_ui_buffer(); + states.sign_state = STATE_191_HASH_DISPLAY; + states.ui_started = false; + return data; +} + +/** + * Feed the progressive hash with new data + * + * @param[in] data the new data + * @param[in] length the data length + * @return whether it was successful or not + */ +static bool feed_hash(const uint8_t *const data, const uint8_t length) { + if (length > tmpCtx.messageSigningContext.remainingLength) { + PRINTF("Error: Length mismatch ! (%u > %u)!\n", + length, + tmpCtx.messageSigningContext.remainingLength); + apdu_reply(0x6a80); + return false; + } + cx_hash((cx_hash_t *) &global_sha3, 0, data, length, NULL, 0); + if ((tmpCtx.messageSigningContext.remainingLength -= length) == 0) { + // Finalize hash + cx_hash((cx_hash_t *) &global_sha3, + CX_LAST, + NULL, + 0, + tmpCtx.messageSigningContext.hash, + 32); } return true; } /** - * Initialize value string that will be displayed in the UX STEP - * - * @param[in] if the value is ASCII + * Feed the UI with new data */ -static void init_value_str(bool is_ascii) { - if (is_ascii) { - strings.tmp.tmp[0] = '\0'; // init string as empty - } else { - strcpy(strings.tmp.tmp, "0x"); // will display the hex bytes instead - } -} +static void feed_display(void) { + int c; -/** - * @return Whether the currently stored data is initialized as ASCII or not - */ -static bool is_value_str_ascii() { - return (memcmp(strings.tmp.tmp, "0x", 2) != 0); -} - -/** - * Update the global UI string variable by formatting & appending the new data to it - * - * @param[in] data the input data - * @param[in] length the data length - * @param[in] is_ascii wether the data is ASCII or not - */ -static void feed_value_str(const uint8_t *const data, size_t length, bool is_ascii) { - uint16_t value_strlen = strlen(strings.tmp.tmp); - - if ((value_strlen + 1) < sizeof(strings.tmp.tmp)) { - if (is_ascii) { - uint8_t src_idx = 0; - uint16_t dst_idx = value_strlen; - bool prev_is_special = false; - - while ((src_idx < length) && (dst_idx < sizeof(strings.tmp.tmp))) { - if (prev_is_special) { - if (!is_char_special(data[src_idx])) { - prev_is_special = false; - } - } else { - if (is_char_special(data[src_idx])) { - prev_is_special = true; - strings.tmp.tmp[dst_idx] = ' '; - dst_idx += 1; - } - } - if (!is_char_special(data[src_idx])) { - strings.tmp.tmp[dst_idx] = data[src_idx]; - dst_idx += 1; - } - src_idx += 1; - } - - if (dst_idx < sizeof(strings.tmp.tmp)) { - strings.tmp.tmp[dst_idx] = '\0'; - } else { - const char marker[] = "..."; - - memcpy(strings.tmp.tmp + sizeof(strings.tmp.tmp) - sizeof(marker), - marker, - sizeof(marker)); - } + while ((unprocessed_length() > 0) && (remaining_ui_buffer_length() > 0)) { + c = *(char *) unprocessed_data(); + if (isspace(c)) // to replace all white-space characters as spaces + { + c = ' '; + } + if (isprint(c)) { + sprintf(remaining_ui_buffer(), "%c", (char) c); + processed_size += 1; } else { - // truncate to strings.tmp.tmp 's size - length = MIN(length, (sizeof(strings.tmp.tmp) - value_strlen) / 2); - for (size_t i = 0; i < length; i++) { - snprintf(strings.tmp.tmp + value_strlen + 2 * i, - sizeof(strings.tmp.tmp) - value_strlen - 2 * i, - "%02X", - data[i]); + if (remaining_ui_buffer_length() >= 4) // 4 being the fixed length of \x00 + { + snprintf(remaining_ui_buffer(), remaining_ui_buffer_length(), "\\x%02x", c); + processed_size += 1; + } else { + // fill the rest of the UI buffer spaces, to consider the buffer full + memset(remaining_ui_buffer(), ' ', remaining_ui_buffer_length()); } } } + +#ifdef TARGET_NANOS + if ((remaining_ui_buffer_length() == 0) && (unprocessed_length > 0)) { + sprintf(remaining_ui_buffer() - 3, "..."); + } +#endif + + if ((remaining_ui_buffer_length() == 0) || + (tmpCtx.messageSigningContext.remainingLength == 0)) { + if (!states.ui_started) { + ui_191_start(); + states.ui_started = true; + } else { + ui_191_switch_to_message(); + } + } + + if ((unprocessed_length() == 0) && (tmpCtx.messageSigningContext.remainingLength > 0)) { + apdu_reply(0x9000); + } } -void handleSignPersonalMessage(uint8_t p1, +/** + * EIP-191 APDU handler + * + * @param[in] p1 instruction parameter 1 + * @param[in] p2 instruction parameter 2 + * @param[in] payload received data + * @param[in] length data length + * @return whether the handling of the APDU was successful or not + */ +bool handleSignPersonalMessage(uint8_t p1, uint8_t p2, - const uint8_t *workBuffer, - uint16_t dataLength, - unsigned int *flags, - unsigned int *tx) { - UNUSED(tx); - uint8_t hashMessage[INT256_LENGTH]; + const uint8_t *const payload, + uint8_t length) { + const uint8_t *data = payload; + uint16_t u16_length = length; + (void) p2; + processed_size = 0; if (p1 == P1_FIRST) { - char tmp[11] = {0}; - - if (appState != APP_STATE_IDLE) { - reset_app_context(); + if ((data = first_apdu_data(data, &u16_length)) == NULL) { + return false; } - appState = APP_STATE_SIGNING_MESSAGE; - - workBuffer = parseBip32(workBuffer, &dataLength, &tmpCtx.messageSigningContext.bip32); - - if (workBuffer == NULL) { - THROW(0x6a80); - } - - if (dataLength < sizeof(uint32_t)) { - PRINTF("Invalid data\n"); - THROW(0x6a80); - } - - tmpCtx.messageSigningContext.remainingLength = U4BE(workBuffer, 0); - workBuffer += sizeof(uint32_t); - dataLength -= sizeof(uint32_t); - // Initialize message header + length - cx_keccak_init(&global_sha3, 256); - cx_hash((cx_hash_t *) &global_sha3, - 0, - (uint8_t *) SIGN_MAGIC, - sizeof(SIGN_MAGIC) - 1, - NULL, - 0); - snprintf(tmp, sizeof(tmp), "%u", tmpCtx.messageSigningContext.remainingLength); - cx_hash((cx_hash_t *) &global_sha3, 0, (uint8_t *) tmp, strlen(tmp), NULL, 0); - cx_sha256_init(&tmpContent.sha2); - - init_value_str(is_data_ascii(workBuffer, dataLength)); - + processed_size = data - payload; } else if (p1 != P1_MORE) { - THROW(0x6B00); - } - if (p2 != 0) { - THROW(0x6B00); - } - if ((p1 == P1_MORE) && (appState != APP_STATE_SIGNING_MESSAGE)) { - PRINTF("Signature not initialized\n"); - THROW(0x6985); - } - if (dataLength > tmpCtx.messageSigningContext.remainingLength) { - THROW(0x6A80); + PRINTF("Error: Unexpected P1 (%u)!\n", p1); + apdu_reply(0x6B00); + return false; } - cx_hash((cx_hash_t *) &global_sha3, 0, workBuffer, dataLength, NULL, 0); - cx_hash((cx_hash_t *) &tmpContent.sha2, 0, workBuffer, dataLength, NULL, 0); - tmpCtx.messageSigningContext.remainingLength -= dataLength; - - feed_value_str(workBuffer, dataLength, is_value_str_ascii()); - - if (tmpCtx.messageSigningContext.remainingLength == 0) { - cx_hash((cx_hash_t *) &global_sha3, - CX_LAST, - workBuffer, - 0, - tmpCtx.messageSigningContext.hash, - 32); - cx_hash((cx_hash_t *) &tmpContent.sha2, CX_LAST, workBuffer, 0, hashMessage, 32); + if (!feed_hash(data, u16_length)) { + return false; + } + if (states.sign_state == STATE_191_HASH_DISPLAY) { + feed_display(); + } else // hash only + { + if (tmpCtx.messageSigningContext.remainingLength == 0) { #ifdef NO_CONSENT - io_seproxyhal_touch_signMessage_ok(NULL); -#else // NO_CONSENT - ui_display_sign(); -#endif // NO_CONSENT - - *flags |= IO_ASYNCH_REPLY; + io_seproxyhal_touch_signMessage_ok(); +#else + ui_191_switch_to_sign(); +#endif + } else { + apdu_reply(0x9000); + } + } + return true; +} +#ifndef TARGET_NANOS +/** + * Decide whether to show the question to show more of the message or not + */ +void question_switcher(void) { + if ((states.sign_state == STATE_191_HASH_DISPLAY) && + ((tmpCtx.messageSigningContext.remainingLength > 0) || (unprocessed_length() > 0))) { + ui_191_switch_to_question(); } else { - THROW(0x9000); + // Go to Sign / Cancel + ui_191_switch_to_sign(); } } +#endif + +/** + * The user has decided to skip the rest of the message + */ +void skip_rest_of_message(void) { + states.sign_state = STATE_191_HASH_ONLY; + if (tmpCtx.messageSigningContext.remainingLength > 0) { + apdu_reply(0x9000); + } else { + ui_191_switch_to_sign(); + } +} + +#ifndef TARGET_NANOS +/** + * The user has decided to see the next chunk of the message + */ +void continue_displaying_message(void) { + reset_ui_buffer(); + if (unprocessed_length() > 0) { + feed_display(); + } +} +#endif diff --git a/src_features/signMessage/sign_message.h b/src_features/signMessage/sign_message.h new file mode 100644 index 0000000..6d35e49 --- /dev/null +++ b/src_features/signMessage/sign_message.h @@ -0,0 +1,12 @@ +#ifndef SIGN_MESSAGE_H_ +#define SIGN_MESSAGE_H_ + +#define UI_191_BUFFER strings.tmp.tmp + +typedef enum { STATE_191_HASH_DISPLAY = 0, STATE_191_HASH_ONLY } sign_message_state; + +void question_switcher(void); +void skip_rest_of_message(void); +void continue_displaying_message(void); + +#endif // SIGN_MESSAGE_H_ diff --git a/src_features/signMessage/ui_common_signMessage.c b/src_features/signMessage/ui_common_signMessage.c index b8fc9c3..eb6707d 100644 --- a/src_features/signMessage/ui_common_signMessage.c +++ b/src_features/signMessage/ui_common_signMessage.c @@ -1,8 +1,7 @@ #include "os_io_seproxyhal.h" -#include "shared_context.h" -#include "ui_callbacks.h" +#include "common_ui.h" -unsigned int io_seproxyhal_touch_signMessage_ok(__attribute__((unused)) const bagl_element_t *e) { +unsigned int io_seproxyhal_touch_signMessage_ok(void) { uint8_t privateKeyData[INT256_LENGTH]; uint8_t signature[100]; cx_ecfp_private_key_t privateKey; @@ -46,8 +45,7 @@ unsigned int io_seproxyhal_touch_signMessage_ok(__attribute__((unused)) const ba return 0; // do not redraw the widget } -unsigned int io_seproxyhal_touch_signMessage_cancel(__attribute__((unused)) - const bagl_element_t *e) { +unsigned int io_seproxyhal_touch_signMessage_cancel(void) { reset_app_context(); G_io_apdu_buffer[0] = 0x69; G_io_apdu_buffer[1] = 0x85; diff --git a/src_features/signMessageEIP712/cmd_signMessage712.c b/src_features/signMessageEIP712/cmd_signMessage712.c index 5585d52..847c40a 100644 --- a/src_features/signMessageEIP712/cmd_signMessage712.c +++ b/src_features/signMessageEIP712/cmd_signMessage712.c @@ -27,7 +27,7 @@ void handleSignEIP712Message(uint8_t p1, memmove(tmpCtx.messageSigningContext712.messageHash, workBuffer + 32, 32); #ifdef NO_CONSENT - io_seproxyhal_touch_signMessage_ok(NULL); + io_seproxyhal_touch_signMessage_ok(); #else // NO_CONSENT ui_sign_712_v0(); #endif // NO_CONSENT diff --git a/src_features/signMessageEIP712/ui_common_signMessage712.c b/src_features/signMessageEIP712/ui_common_signMessage712.c index e7dea8f..4d5e266 100644 --- a/src_features/signMessageEIP712/ui_common_signMessage712.c +++ b/src_features/signMessageEIP712/ui_common_signMessage712.c @@ -1,6 +1,6 @@ #include "os_io_seproxyhal.h" #include "shared_context.h" -#include "ui_callbacks.h" +#include "common_ui.h" static const uint8_t EIP_712_MAGIC[] = {0x19, 0x01}; diff --git a/src_features/signTx/ui_common_signTx.c b/src_features/signTx/ui_common_signTx.c index 6c482d0..ef8171f 100644 --- a/src_features/signTx/ui_common_signTx.c +++ b/src_features/signTx/ui_common_signTx.c @@ -1,7 +1,7 @@ #include "os_io_seproxyhal.h" #include "shared_context.h" #include "utils.h" -#include "ui_callbacks.h" +#include "common_ui.h" unsigned int io_seproxyhal_touch_tx_ok(__attribute__((unused)) const bagl_element_t *e) { uint8_t privateKeyData[INT256_LENGTH]; diff --git a/src_features/stark_getPublicKey/ui_common_stark_getPublicKey.c b/src_features/stark_getPublicKey/ui_common_stark_getPublicKey.c index 94a6658..31b9714 100644 --- a/src_features/stark_getPublicKey/ui_common_stark_getPublicKey.c +++ b/src_features/stark_getPublicKey/ui_common_stark_getPublicKey.c @@ -1,7 +1,7 @@ #ifdef HAVE_STARKWARE #include "shared_context.h" -#include "ui_callbacks.h" +#include "common_ui.h" #include "feature_stark_getPublicKey.h" unsigned int io_seproxyhal_touch_stark_pubkey_ok(__attribute__((unused)) const bagl_element_t *e) { diff --git a/src_features/stark_sign/ui_common_stark_sign.c b/src_features/stark_sign/ui_common_stark_sign.c index a44ee99..a45f7f9 100644 --- a/src_features/stark_sign/ui_common_stark_sign.c +++ b/src_features/stark_sign/ui_common_stark_sign.c @@ -3,7 +3,7 @@ #include "os_io_seproxyhal.h" #include "shared_context.h" #include "stark_utils.h" -#include "ui_callbacks.h" +#include "common_ui.h" unsigned int io_seproxyhal_touch_stark_ok(__attribute__((unused)) const bagl_element_t *e) { uint8_t privateKeyData[32]; diff --git a/src_features/stark_unsafe_sign/ui_common_stark_unsafe_sign.c b/src_features/stark_unsafe_sign/ui_common_stark_unsafe_sign.c index 899c59d..4774627 100644 --- a/src_features/stark_unsafe_sign/ui_common_stark_unsafe_sign.c +++ b/src_features/stark_unsafe_sign/ui_common_stark_unsafe_sign.c @@ -3,7 +3,7 @@ #include "os_io_seproxyhal.h" #include "shared_context.h" #include "stark_utils.h" -#include "ui_callbacks.h" +#include "common_ui.h" unsigned int io_seproxyhal_touch_stark_unsafe_sign_ok(__attribute__((unused)) const bagl_element_t *e) { diff --git a/tests/speculos/test_eip191.py b/tests/speculos/old_test_eip191.py similarity index 100% rename from tests/speculos/test_eip191.py rename to tests/speculos/old_test_eip191.py diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00001.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00001.png index ad00cb8..51efae0 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_nonascii/00001.png and b/tests/zemu/snapshots/nanos_eip191_nonascii/00001.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00002.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00002.png index b435297..b4e4df5 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_nonascii/00002.png and b/tests/zemu/snapshots/nanos_eip191_nonascii/00002.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00003.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00003.png index fcd68b2..d95e831 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_nonascii/00003.png and b/tests/zemu/snapshots/nanos_eip191_nonascii/00003.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00004.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00004.png index 251c562..e8c4c8e 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_nonascii/00004.png and b/tests/zemu/snapshots/nanos_eip191_nonascii/00004.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00005.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00005.png index f5c2d67..a259c69 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_nonascii/00005.png and b/tests/zemu/snapshots/nanos_eip191_nonascii/00005.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00006.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00006.png index d55782f..4ce22f6 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_nonascii/00006.png and b/tests/zemu/snapshots/nanos_eip191_nonascii/00006.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00008.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00008.png index ce795f3..d55782f 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_nonascii/00008.png and b/tests/zemu/snapshots/nanos_eip191_nonascii/00008.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00009.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00009.png new file mode 100644 index 0000000..f5c2d67 Binary files /dev/null and b/tests/zemu/snapshots/nanos_eip191_nonascii/00009.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_nonascii/00010.png b/tests/zemu/snapshots/nanos_eip191_nonascii/00010.png new file mode 100644 index 0000000..ce795f3 Binary files /dev/null and b/tests/zemu/snapshots/nanos_eip191_nonascii/00010.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_opensea/00002.png b/tests/zemu/snapshots/nanos_eip191_opensea/00002.png index 0676953..1fd8170 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_opensea/00002.png and b/tests/zemu/snapshots/nanos_eip191_opensea/00002.png differ diff --git a/tests/zemu/snapshots/nanos_eip191_opensea/00005.png b/tests/zemu/snapshots/nanos_eip191_opensea/00005.png index 228f924..1a86e37 100644 Binary files a/tests/zemu/snapshots/nanos_eip191_opensea/00005.png and b/tests/zemu/snapshots/nanos_eip191_opensea/00005.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_nonascii/00001.png b/tests/zemu/snapshots/nanox_eip191_nonascii/00001.png index 93eda35..6a5f8e8 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_nonascii/00001.png and b/tests/zemu/snapshots/nanox_eip191_nonascii/00001.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_nonascii/00002.png b/tests/zemu/snapshots/nanox_eip191_nonascii/00002.png index 873634e..78d7d1d 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_nonascii/00002.png and b/tests/zemu/snapshots/nanox_eip191_nonascii/00002.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00001.png b/tests/zemu/snapshots/nanox_eip191_opensea/00001.png index 802fad5..4f59a8d 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00001.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00001.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00002.png b/tests/zemu/snapshots/nanox_eip191_opensea/00002.png index a3ba739..c094bc6 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00002.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00002.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00003.png b/tests/zemu/snapshots/nanox_eip191_opensea/00003.png index 4601bca..9a3efb7 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00003.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00003.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00004.png b/tests/zemu/snapshots/nanox_eip191_opensea/00004.png index eab9f1d..657edb5 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00004.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00004.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00005.png b/tests/zemu/snapshots/nanox_eip191_opensea/00005.png index 0b4bdbd..1f135ae 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00005.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00005.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00006.png b/tests/zemu/snapshots/nanox_eip191_opensea/00006.png index c9da92b..952e5ca 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00006.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00006.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00007.png b/tests/zemu/snapshots/nanox_eip191_opensea/00007.png index 121cfd5..f29ed00 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00007.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00007.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00008.png b/tests/zemu/snapshots/nanox_eip191_opensea/00008.png index c9da92b..8af3d9c 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00008.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00008.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00009.png b/tests/zemu/snapshots/nanox_eip191_opensea/00009.png index a58590b..c9da92b 100644 Binary files a/tests/zemu/snapshots/nanox_eip191_opensea/00009.png and b/tests/zemu/snapshots/nanox_eip191_opensea/00009.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00010.png b/tests/zemu/snapshots/nanox_eip191_opensea/00010.png new file mode 100644 index 0000000..121cfd5 Binary files /dev/null and b/tests/zemu/snapshots/nanox_eip191_opensea/00010.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00011.png b/tests/zemu/snapshots/nanox_eip191_opensea/00011.png new file mode 100644 index 0000000..c9da92b Binary files /dev/null and b/tests/zemu/snapshots/nanox_eip191_opensea/00011.png differ diff --git a/tests/zemu/snapshots/nanox_eip191_opensea/00012.png b/tests/zemu/snapshots/nanox_eip191_opensea/00012.png new file mode 100644 index 0000000..a58590b Binary files /dev/null and b/tests/zemu/snapshots/nanox_eip191_opensea/00012.png differ diff --git a/tests/zemu/src/eip191.test.js b/tests/zemu/src/eip191.test.js index 2223256..c7f25bd 100644 --- a/tests/zemu/src/eip191.test.js +++ b/tests/zemu/src/eip191.test.js @@ -34,7 +34,7 @@ nano_models.forEach(function(model) { await waitForAppScreen(sim); - const rclicks = (model.letter == 'S') ? 6 : 4; + const rclicks = (model.letter == 'S') ? 8 : 4; await sim.navigateAndCompareSnapshots('.', model.name + '_eip191_nonascii', [rclicks, -1, 0]); await expect(tx).resolves.toEqual({ @@ -54,7 +54,14 @@ nano_models.forEach(function(model) { await waitForAppScreen(sim); - await sim.navigateAndCompareSnapshots('.', model.name + '_eip191_opensea', [7, -1, 0]); + if (model.letter == 'S') + { + await sim.navigateAndCompareSnapshots('.', model.name + '_eip191_opensea', [1, 5, 1, -1, 0]); + } + else + { + await sim.navigateAndCompareSnapshots('.', model.name + '_eip191_opensea', [1, 5, 1, 2, 1, -1, 0]); + } await expect(tx).resolves.toEqual({ "v": 28,