Merge pull request #7 from pontem-network/feature/sign_raw_msg

Support for non-UTF8 message signature. Fixes.

src/bcs/types.h

@@ -121,7 +121,13 @@ typedef struct {
     fixed_bytes_t *args;
 } script_payload_t;
 
-typedef enum { TX_RAW = 0, TX_RAW_WITH_DATA = 1, TX_MESSAGE = 2, TX_UNDEFINED = 1000 } tx_variant_t;
+typedef enum {
+    TX_RAW = 0,
+    TX_RAW_WITH_DATA = 1,
+    TX_MESSAGE = 2,
+    TX_RAW_MESSAGE = 3,
+    TX_UNDEFINED = 1000
+} tx_variant_t;
 
 typedef enum {
     PAYLOAD_SCRIPT = 0,

src/transaction/deserialize.c

@@ -24,6 +24,10 @@ parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx) {
         case TX_RAW:
             return tx_raw_deserialize(buf, tx);
         case TX_RAW_WITH_DATA:
+            break;
+        case TX_RAW_MESSAGE:
+            break;  // Since the raw message is processed before display without direct transaction
+                    // buffer reads, null-termination concerns are mitigated.
         case TX_MESSAGE:
             // To make sure the message is a null-terminated string
             if (buf->size == MAX_TRANSACTION_LEN && buf->ptr[MAX_TRANSACTION_LEN - 1] != 0) {
@@ -107,7 +111,6 @@ parser_status_e tx_raw_deserialize(buffer_t *buf, transaction_t *tx) {
 }
 
 parser_status_e tx_variant_deserialize(buffer_t *buf, transaction_t *tx) {
-    parser_status_e status = TX_VARIANT_UNDEFINED_ERROR;
     if (buf->offset != 0) {
         return TX_VARIANT_READ_ERROR;
     }
@@ -126,17 +129,16 @@ parser_status_e tx_variant_deserialize(buffer_t *buf, transaction_t *tx) {
             tx->tx_variant = TX_RAW;
             return PARSING_OK;
         }
-    } else {
-        status = HASHED_PREFIX_READ_ERROR;
     }
 
-    if (transaction_utils_check_encoding(buf->ptr, buf->size)) {
-        buf->offset = 0;
-        tx->tx_variant = TX_MESSAGE;
-        return PARSING_OK;
-    }
+    // Not a transaction prefix, so we reset the offer to consider the full message
+    buf->offset = 0;
 
-    return status;
+    // Try to display the message as UTF8 if possible
+    tx->tx_variant =
+        transaction_utils_check_encoding(buf->ptr, buf->size) ? TX_MESSAGE : TX_RAW_MESSAGE;
+
+    return PARSING_OK;
 }
 
 parser_status_e entry_function_payload_deserialize(buffer_t *buf, transaction_t *tx) {

src/ui/bagl_display.c

@@ -24,6 +24,7 @@
 #include "ux.h"
 #include "glyphs.h"
 #include "io.h"
+#include "format.h"
 
 #include "bagl_display.h"
 #include "display.h"
@@ -207,6 +208,13 @@ UX_STEP_NOCB(ux_display_short_msg_step,
                  .title = "Message",
                  .text = g_struct,
              });
+// Step with title/text for message in raw form
+UX_STEP_NOCB(ux_display_raw_msg_step,
+             bnnn_paging,
+             {
+                 .title = "Raw message",
+                 .text = g_struct,
+             });
 // Step with title/text for transaction type
 UX_STEP_NOCB(ux_display_tx_type_step,
              bnnn_paging,
@@ -289,6 +297,18 @@ UX_FLOW(ux_display_short_message_flow, SEQUENCE_SHORT_MESSAGE);
 // preceding screen : warning icon + "Blind Signing"
 UX_FLOW(ux_display_blind_short_message_flow, &ux_display_blind_warn_step, SEQUENCE_SHORT_MESSAGE);
 
+// FLOW to display message information in raw form:
+// #1 screen : eye icon + "Review Message"
+// #2 screen : display raw message
+// #3 screen : approve button
+// #4 screen : reject button
+#define SEQUENCE_RAW_MESSAGE                                                         \
+    &ux_display_review_msg_step, &ux_display_raw_msg_step, &ux_display_approve_step, \
+        &ux_display_reject_step
+UX_FLOW(ux_display_raw_message_flow, SEQUENCE_RAW_MESSAGE);
+// preceding screen : warning icon + "Blind Signing"
+UX_FLOW(ux_display_blind_raw_message_flow, &ux_display_blind_warn_step, SEQUENCE_RAW_MESSAGE);
+
 // FLOW to display entry_function transaction information:
 // #1 screen : warning icon + "Blind Signing"
 // #2 screen : eye icon + "Review Transaction"
@@ -390,6 +410,30 @@ int ui_display_message() {
     return 0;
 }
 
+int ui_display_raw_message() {
+    memset(g_struct, 0, sizeof(g_struct));
+    const bool short_enough = sizeof(g_struct) >= 2 * G_context.tx_info.raw_tx_len + 1;
+    if (short_enough) {
+        format_hex(G_context.tx_info.raw_tx,
+                   G_context.tx_info.raw_tx_len,
+                   g_struct,
+                   sizeof(g_struct));
+    } else {
+        const size_t cropped_bytes_len = (sizeof(g_struct) - sizeof(DOTS)) / 2;
+        format_hex(G_context.tx_info.raw_tx, cropped_bytes_len, g_struct, sizeof(g_struct));
+        strncpy(g_struct + cropped_bytes_len * 2, DOTS, sizeof(DOTS));
+    }
+    PRINTF("Message: %s\n", g_struct);
+
+    if (short_enough) {
+        ui_flow_display(ux_display_raw_message_flow);
+    } else {
+        ui_flow_verified_display(ux_display_blind_raw_message_flow);
+    }
+
+    return 0;
+}
+
 int ui_display_entry_function() {
     const int ret = ui_prepare_entry_function();
     if (ret == UI_PREPARED) {

src/ui/common_display.c

@@ -87,6 +87,8 @@ int ui_prepare_transaction() {
 
     if (transaction->tx_variant == TX_MESSAGE) {
         return ui_display_message();
+    } else if (transaction->tx_variant == TX_RAW_MESSAGE) {
+        return ui_display_raw_message();
     } else if (transaction->tx_variant != TX_UNDEFINED) {
         uint64_t gas_fee_value = transaction->gas_unit_price * transaction->max_gas_amount;
         memset(g_gas_fee, 0, sizeof(g_gas_fee));

src/ui/display.h

@@ -29,6 +29,7 @@ int ui_display_transaction(void);
 int ui_prepare_transaction(void);
 
 int ui_display_message(void);
+int ui_display_raw_message(void);
 
 int ui_display_entry_function(void);
 int ui_prepare_entry_function(void);

src/ui/nbgl_display_message.c

@@ -21,6 +21,7 @@
 #include <string.h>   // memset
 
 #include "os.h"
+#include "format.h"
 #include "glyphs.h"
 #include "nbgl_use_case.h"
 
@@ -32,6 +33,8 @@
 #include "action/validate.h"
 #include "../common/user_format.h"
 
+#define DOTS "[...]"
+
 static void confirm_message_rejection(void) {
     validate_transaction(false);
     nbgl_useCaseStatus("Message rejected", false, ui_menu_main);
@@ -69,6 +72,21 @@ static void review_message_continue(void) {
     nbgl_useCaseStaticReview(&pairList, &infoLongPress, "Reject message", review_choice);
 }
 
+static void review_raw_message_continue(void) {
+    pairs[0].item = "Raw message";
+    pairs[0].value = g_struct;
+
+    pairList.nbMaxLinesForValue = 0;
+    pairList.nbPairs = 1;
+    pairList.pairs = pairs;
+
+    infoLongPress.icon = &C_Message_64px;
+    infoLongPress.text = "Sign message";
+    infoLongPress.longPressText = "Hold to sign";
+
+    nbgl_useCaseStaticReview(&pairList, &infoLongPress, "Reject message", review_choice);
+}
+
 int ui_display_message() {
     if (is_str_interrupted((const char *) G_context.tx_info.raw_tx, G_context.tx_info.raw_tx_len)) {
         nbgl_useCaseReviewVerify(&C_Message_64px,
@@ -89,4 +107,37 @@ int ui_display_message() {
     return 0;
 }
 
+int ui_display_raw_message() {
+    memset(g_struct, 0, sizeof(g_struct));
+    const bool short_enough = sizeof(g_struct) >= 2 * G_context.tx_info.raw_tx_len + 1;
+    if (short_enough) {
+        format_hex(G_context.tx_info.raw_tx,
+                   G_context.tx_info.raw_tx_len,
+                   g_struct,
+                   sizeof(g_struct));
+    } else {
+        const size_t cropped_bytes_len = (sizeof(g_struct) - sizeof(DOTS)) / 2;
+        format_hex(G_context.tx_info.raw_tx, cropped_bytes_len, g_struct, sizeof(g_struct));
+        strncpy(g_struct + cropped_bytes_len * 2, DOTS, sizeof(DOTS));
+    }
+
+    if (!short_enough) {
+        nbgl_useCaseReviewVerify(&C_Message_64px,
+                                 "Review message",
+                                 NULL,
+                                 "Reject message",
+                                 review_raw_message_continue,
+                                 ask_message_rejection_confirmation);
+    } else {
+        nbgl_useCaseReviewStart(&C_Message_64px,
+                                "Review message",
+                                NULL,
+                                "Reject message",
+                                review_raw_message_continue,
+                                ask_message_rejection_confirmation);
+    }
+
+    return 0;
+}
+
 #endif

tests/test_sign_cmd.py

@@ -3,11 +3,35 @@
 from application_client.aptos_command_sender import AptosCommandSender, Errors
 from application_client.aptos_response_unpacker import unpack_get_public_key_response, unpack_sign_tx_response
 from ragger.error import ExceptionRAPDU
-from ragger.navigator import NavInsID
+from ragger.navigator import NavInsID, NavIns
 from utils import ROOT_SCREENSHOT_PATH, check_signature_validity
 
 # In this tests we check the behavior of the device when asked to sign a transaction
 
+# This fixture is used to disable the blind signing after a test that enabled it
+@pytest.fixture
+def disable_blind_signing(firmware, backend, navigator):
+    yield
+
+    if firmware.device.startswith("nano"):
+        backend.right_click()
+        backend.both_click()
+        backend.right_click()
+        backend.both_click()
+        backend.left_click()
+        backend.both_click()
+        backend.right_click()
+        backend.both_click()
+    else:
+        instructions = [
+            NavInsID.USE_CASE_HOME_SETTINGS,
+            NavInsID.USE_CASE_SETTINGS_NEXT,
+            NavIns(NavInsID.TOUCH, (200, 113)),
+            NavInsID.USE_CASE_CHOICE_REJECT,
+            NavInsID.USE_CASE_SETTINGS_MULTI_PAGE_EXIT
+        ]
+        navigator.navigate(instructions, screen_change_before_first_instruction=False)
+
 
 # In this test we send to the device a transaction to sign and validate it on screen
 # The transaction is short and will be sent in one chunk
@@ -53,7 +77,7 @@ def test_sign_tx_short_tx(firmware, backend, navigator, test_name):
 # In this test we send to the device a transaction to sign and validate it on screen
 # The transaction will be sent in multiple chunks
 # Also, this transaction has a request for blind signing activation
-def test_blind_sign_tx_long_tx(firmware, backend, navigator, test_name):
+def test_blind_sign_tx_long_tx(firmware, backend, navigator, test_name, disable_blind_signing):
     # Use the app interface instead of raw interface
     client = AptosCommandSender(backend)
     path: str = "m/44'/637'/1'/0'/0'"
@@ -169,3 +193,90 @@ def test_sign_tx_short_msg(firmware, backend, navigator, test_name):
     response = client.get_async_response().data
     _, sig, _ = unpack_sign_tx_response(response)
     assert check_signature_validity(public_key, sig, message)
+
+# In this test we send to the device a message to sign and validate it on screen
+# We will ensure that the displayed information is correct by using screenshots comparison
+def test_sign_short_raw_msg(firmware, backend, navigator, test_name):
+    # Use the app interface instead of raw interface
+    client = AptosCommandSender(backend)
+    # The path used for this entire test
+    path: str = "m/44'/637'/1'/0'/0'"
+
+    # First we need to get the public key of the device in order to build the transaction
+    rapdu = client.get_public_key(path=path)
+    _, public_key, _, _ = unpack_get_public_key_response(rapdu.data)
+
+    # Create the mes that will be sent to the device for signing
+    message = bytes.fromhex("01020304ff")
+
+    # Send the sign device instruction.
+    # As it requires on-screen validation, the function is asynchronous.
+    # It will yield the result when the navigation is done
+    with client.sign_tx(path=path, transaction=message):
+        # Validate the on-screen request by performing the navigation appropriate for this device
+        if firmware.device.startswith("nano"):
+            navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
+                                                      [NavInsID.BOTH_CLICK],
+                                                      "Approve",
+                                                      ROOT_SCREENSHOT_PATH,
+                                                      test_name)
+        else:
+            navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP,
+                                                      [NavInsID.USE_CASE_REVIEW_CONFIRM,
+                                                       NavInsID.USE_CASE_STATUS_DISMISS],
+                                                      "Hold to sign",
+                                                      ROOT_SCREENSHOT_PATH,
+                                                      test_name)
+
+    # The device as yielded the result, parse it and ensure that the signature is correct
+    response = client.get_async_response().data
+    _, sig, _ = unpack_sign_tx_response(response)
+    assert check_signature_validity(public_key, sig, message)
+
+# In this test we send to the device a message to sign and validate it on screen
+# We will ensure that the displayed information is correct by using screenshots comparison
+def test_sign_long_raw_msg(firmware, backend, navigator, test_name, disable_blind_signing):
+    # Use the app interface instead of raw interface
+    client = AptosCommandSender(backend)
+    # The path used for this entire test
+    path: str = "m/44'/637'/1'/0'/0'"
+
+    # First we need to get the public key of the device in order to build the transaction
+    rapdu = client.get_public_key(path=path)
+    _, public_key, _, _ = unpack_get_public_key_response(rapdu.data)
+
+    # Create the mes that will be sent to the device for signing
+    message = bytes.fromhex("bc6f6693bddc1a9fec9e674a461eaa00b193094c6fc0d3b382a599c37e1aaa7618eff2c96a3586876082c4594c50c50d7dde1b0000000000000002190d44266241744264b964a37b8f09863167a12d3e70cda39376cfb4e3561e120a736372697074735f76320473776170030700000000000000000000000000000000000000000000000000000000000000010a6170746f735f636f696e094170746f73436f696e000743417434fd869edee76cca2a4d2301e528a1551b1d719b75c350c3c97d15b8b905636f696e7304555344540007190d44266241744264b964a37b8f09863167a12d3e70cda39376cfb4e3561e12066375727665730c556e636f7272656c6174656400020800e1f5050000000008decbb30000000000480000000000000064000000000000008a9ba4640000000002")
+
+    with client.sign_tx(path=path, transaction=message):
+        if firmware.device.startswith("nano"):
+            navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
+                                                        [NavInsID.BOTH_CLICK],
+                                                        "Allow",
+                                                        ROOT_SCREENSHOT_PATH,
+                                                        test_name + "/part0",
+                                                        screen_change_after_last_instruction=False)
+            navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
+                                                        [NavInsID.BOTH_CLICK],
+                                                        "Approve",
+                                                        ROOT_SCREENSHOT_PATH,
+                                                        test_name + "/part1")
+        else:
+            navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP,
+                                                        [NavInsID.USE_CASE_CHOICE_CONFIRM,
+                                                        NavInsID.USE_CASE_STATUS_DISMISS],
+                                                        "Enable blind signing",
+                                                        ROOT_SCREENSHOT_PATH,
+                                                        test_name + "/part0",
+                                                        screen_change_after_last_instruction=False)
+            navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP,
+                                                        [NavInsID.USE_CASE_REVIEW_CONFIRM,
+                                                        NavInsID.USE_CASE_STATUS_DISMISS],
+                                                        "Hold to sign",
+                                                        ROOT_SCREENSHOT_PATH,
+                                                        test_name + "/part1")
+
+    # The device as yielded the result, parse it and ensure that the signature is correct
+    response = client.get_async_response().data
+    _, sig, _ = unpack_sign_tx_response(response)
+    assert check_signature_validity(public_key, sig, message)