Merge pull request #339 from LedgerHQ/apr/feature/eip191_full_length

EIP-191 UI improvements

.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

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
 

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,

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);

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);
             }

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);

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
+#endif  // HAVE_BAGL

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);
+UX_FLOW(ux_warning_contract_data_flow, &ux_warning_contract_data_step);

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[];

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

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_

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;
 

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();

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();

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) {

src_features/signMessage/cmd_signMessage.c

@@ -1,199 +1,294 @@
 #include <stdbool.h>
-#include "shared_context.h"
+#include <ctype.h>
+#include <string.h>
 #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

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_

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;

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

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};
 

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];

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) {

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];

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) {

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,