Factored out onboard and communication UI modules (#98)

- Factored out onboard operations
- Factored out communication operations
- Removed unused parameters from unlock and pin modules
- Reworked pin module
- Added unit tests for communication and onboard modules
- Added UI communication, onboard and pin tests to the CI

.github/workflows/run-tests.yml

@@ -27,6 +27,13 @@ jobs:
             (cd "$d" && make clean test)
           done
 
+      - name: Ledger UI's tests
+        working-directory: ledger/src/ui/test/
+        run: |
+          for d in communication onboard pin unlock; do
+            (cd "$d" && make clean test)
+          done
+
       - name: Ledger common lib tests
         working-directory: ledger/src/common/test/
         run: |

ledger/src/ui/src/bolos_ux.c

@@ -50,16 +50,11 @@
 #include "defs.h"
 #include "err.h"
 #include "attestation.h"
+#include "communication.h"
 #include "signer_authorization.h"
 #include "memutil.h"
 #include "unlock.h"
 
-// Onboarded with the UI flag
-const unsigned char N_onboarded_ui[1] = {0};
-
-// PIN buffer used for authenticated operations
-unsigned char G_pin_buffer[PIN_BUFFER_LENGTH];
-
 #ifdef OS_IO_SEPROXYHAL
 
 #define ARRAYLEN(array) (sizeof(array) / sizeof(array[0]))
@@ -240,8 +235,8 @@ void io_seproxyhal_display(const bagl_element_t *element) {
 // Signer authorization context shorthand
 #define sigaut_ctx (G_bolos_ux_context.sigaut)
 
-// Pin context shorthand
-#define pin_ctx (G_bolos_ux_context.pin)
+// Onboard context shorthand
+#define onboard_ctx (G_bolos_ux_context.onboard)
 
 // Operation being currently executed
 static unsigned char curr_cmd;
@@ -256,24 +251,16 @@ static void reset_if_starting(unsigned char cmd) {
     // Otherwise we already reset when curr_cmd started.
     if (cmd != curr_cmd) {
         curr_cmd = cmd;
-        explicit_bzero(G_bolos_ux_context.words_buffer,
-                       sizeof(G_bolos_ux_context.words_buffer));
-        explicit_bzero(G_bolos_ux_context.string_buffer,
-                       sizeof(G_bolos_ux_context.string_buffer));
-        G_bolos_ux_context.words_buffer_length = 0;
         reset_attestation(&attestation_ctx);
         reset_signer_authorization(&sigaut_ctx);
-        reset_pin_ctx(&pin_ctx);
+        reset_onboard_ctx(&onboard_ctx);
     }
 }
 
 static void sample_main(void) {
     volatile unsigned int rx = 0;
     volatile unsigned int tx = 0;
     volatile unsigned int flags = 0;
-    volatile unsigned char pin = 0;
-    volatile int i = 0;
-    volatile unsigned char aux;
 
     // Initialize current operation
     curr_cmd = 0; // 0 = no operation being executed
@@ -312,124 +299,39 @@ static void sample_main(void) {
                 switch (APDU_CMD()) {
                 case RSK_SEED_CMD: // Send wordlist
                     reset_if_starting(RSK_META_CMD_UIOP);
-                    pin = APDU_AT(2);
-                    if ((pin >= 0) && ((unsigned int)(pin) <=
-                                       sizeof(G_bolos_ux_context.words_buffer)))
-                        G_bolos_ux_context.words_buffer[pin] = APDU_AT(3);
+                    tx = set_host_seed(rx, &onboard_ctx);
                     THROW(APDU_OK);
                     break;
                 case RSK_PIN_CMD: // Send pin_buffer
                     reset_if_starting(RSK_META_CMD_UIOP);
-                    init_pin_ctx(&pin_ctx, G_pin_buffer);
-                    tx = update_pin_buffer(rx, &pin_ctx);
+                    tx = update_pin_buffer(rx);
                     THROW(APDU_OK);
                     break;
                 case RSK_IS_ONBOARD: // Wheter it's onboarded or not
                     reset_if_starting(RSK_IS_ONBOARD);
-                    uint8_t output_index = CMDPOS;
-                    SET_APDU_AT(output_index++, os_perso_isonboarded());
-                    SET_APDU_AT(output_index++, VERSION_MAJOR);
-                    SET_APDU_AT(output_index++, VERSION_MINOR);
-                    SET_APDU_AT(output_index++, VERSION_PATCH);
-                    tx = 5;
+                    tx = is_onboarded();
                     THROW(APDU_OK);
                     break;
                 case RSK_WIPE: //--- wipe and onboard device ---
                     reset_if_starting(RSK_META_CMD_UIOP);
-
-                    // Reset the onboarding flag to mark onboarding
-                    // hasn't been done just in case something fails
-                    aux = 0;
-                    nvm_write(
-                        (void *)PIC(N_onboarded_ui), (void *)&aux, sizeof(aux));
-
-                    init_pin_ctx(&pin_ctx, G_pin_buffer);
-#ifndef DEBUG_BUILD
-                    if (!is_pin_valid(&pin_ctx)) {
-                        THROW(ERR_INVALID_PIN);
-                    }
-#endif
-                    // Wipe device
-                    os_global_pin_invalidate();
-                    os_perso_wipe();
-                    G_bolos_ux_context.onboarding_kind =
-                        BOLOS_UX_ONBOARDING_NEW_24;
-                    // Generate 32 bytes of random with onboard rng
-                    cx_rng((unsigned char *)G_bolos_ux_context.string_buffer,
-                           HASHSIZE);
-                    // XOR with host-generated 32 bytes random
-                    for (i = 0; i < HASHSIZE; i++) {
-                        G_bolos_ux_context.string_buffer[i] ^=
-                            G_bolos_ux_context.words_buffer[i];
-                    }
-                    // The seed is now in string_buffer, generate the mnemonic
-                    os_memset(G_bolos_ux_context.words_buffer,
-                              0,
-                              sizeof(G_bolos_ux_context.words_buffer));
-                    G_bolos_ux_context.words_buffer_length =
-                        bolos_ux_mnemonic_from_data(
-                            (unsigned char *)G_bolos_ux_context.string_buffer,
-                            SEEDSIZE,
-                            (unsigned char *)G_bolos_ux_context.words_buffer,
-                            sizeof(G_bolos_ux_context.words_buffer));
-                    // Clear the seed
-                    explicit_bzero(G_bolos_ux_context.string_buffer,
-                                   sizeof(G_bolos_ux_context.string_buffer));
-                    // Set seed from mnemonic
-                    os_perso_derive_and_set_seed(
-                        0,
-                        NULL,
-                        0,
-                        NULL,
-                        0,
-                        G_bolos_ux_context.words_buffer,
-                        strlen(G_bolos_ux_context.words_buffer));
-                    // Clear the mnemonic
-                    explicit_bzero(G_bolos_ux_context.words_buffer,
-                                   sizeof(G_bolos_ux_context.words_buffer));
-                    // Set PIN
-                    os_perso_set_pin(
-                        0, GET_PIN(&pin_ctx), GET_PIN_LENGTH(&pin_ctx));
-                    // Finalize onboarding
-                    os_perso_finalize();
-                    os_global_pin_invalidate();
-                    SET_APDU_AT(1, 2);
-                    SET_APDU_AT(2,
-                                os_global_pin_check(GET_PIN(&pin_ctx),
-                                                    GET_PIN_LENGTH(&pin_ctx)));
-                    // Clear pin buffer
-                    explicit_bzero(G_pin_buffer, sizeof(G_pin_buffer));
-                    // Turn the onboarding flag on to mark onboarding
-                    // has been done using the UI
-                    aux = 1;
-                    nvm_write(
-                        (void *)PIC(N_onboarded_ui), (void *)&aux, sizeof(aux));
-                    // Output
-                    tx = 3;
+                    tx = onboard_device(&onboard_ctx);
+                    clear_pin();
                     THROW(APDU_OK);
                     break;
                 case RSK_NEWPIN:
                     reset_if_starting(RSK_META_CMD_UIOP);
-                    init_pin_ctx(&pin_ctx, G_pin_buffer);
-#ifndef DEBUG_BUILD
-                    if (!is_pin_valid(&pin_ctx)) {
-                        THROW(ERR_INVALID_PIN);
-                    }
-#endif
-                    tx = set_device_pin(rx, &pin_ctx);
-                    // Clear pin buffer
-                    explicit_bzero(G_pin_buffer, sizeof(G_pin_buffer));
+                    tx = set_pin();
+                    clear_pin();
                     THROW(APDU_OK);
                     break;
                 case RSK_ECHO_CMD: // echo
                     reset_if_starting(RSK_ECHO_CMD);
-                    tx = rx;
+                    tx = echo(rx);
                     THROW(APDU_OK);
                     break;
                 case RSK_MODE_CMD: // print mode
                     reset_if_starting(RSK_MODE_CMD);
-                    SET_APDU_AT(1, RSK_MODE_BOOTLOADER);
-                    tx = 2;
+                    tx = get_mode();
                     THROW(APDU_OK);
                     break;
                 case INS_ATTESTATION:
@@ -444,14 +346,12 @@ static void sample_main(void) {
                     break;
                 case RSK_RETRIES:
                     reset_if_starting(RSK_RETRIES);
-                    SET_APDU_AT(2, (unsigned char)os_global_pin_retries());
-                    tx = 3;
+                    tx = get_retries();
                     THROW(APDU_OK);
                     break;
                 case RSK_UNLOCK_CMD: // Unlock
                     reset_if_starting(RSK_META_CMD_UIOP);
-                    init_pin_ctx(&pin_ctx, G_pin_buffer);
-                    tx = unlock(rx, &pin_ctx);
+                    tx = unlock();
                     // The pin value will also be used in
                     // BOLOS_UX_CONSENT_APP_ADD command, so we can't wipe the
                     // pin buffer here
@@ -634,11 +534,9 @@ void bolos_ux_main(void) {
                 // PIN is invalidated so we must check it again. The pin value
                 // used here is the same as in RSK_UNLOCK_CMD, so we also
                 // don't have a prepended length byte
-                init_pin_ctx(&pin_ctx, G_pin_buffer);
-                os_global_pin_check(pin_ctx.pin_buffer,
-                                    strlen((const char *)pin_ctx.pin_buffer));
+                unlock_with_pin(false);
                 G_bolos_ux_context.exit_code = BOLOS_UX_OK;
-                explicit_bzero(G_pin_buffer, sizeof(G_pin_buffer));
+                clear_pin();
                 break;
             } else {
                 G_bolos_ux_context.exit_code = BOLOS_UX_CANCEL;

ledger/src/ui/src/bolos_ux.h

@@ -47,6 +47,7 @@
 #include "attestation.h"
 #include "signer_authorization.h"
 #include "pin.h"
+#include "onboard.h"
 
 #ifdef HAVE_BOLOS_UX
 
@@ -127,16 +128,6 @@ typedef struct bolos_ux_context {
 
     unsigned int last_ux_id;
 
-#define BOLOS_UX_ONBOARDING_NEW 1
-#define BOLOS_UX_ONBOARDING_NEW_12 12
-#define BOLOS_UX_ONBOARDING_NEW_18 18
-#define BOLOS_UX_ONBOARDING_NEW_24 24
-#define BOLOS_UX_ONBOARDING_RESTORE 2
-#define BOLOS_UX_ONBOARDING_RESTORE_12 12
-#define BOLOS_UX_ONBOARDING_RESTORE_18 18
-#define BOLOS_UX_ONBOARDING_RESTORE_24 24
-    unsigned int onboarding_kind;
-
     union {
         struct {
             unsigned int onboarding_step;
@@ -145,22 +136,12 @@ typedef struct bolos_ux_context {
             unsigned int onboarding_words_are_valid;
             unsigned int onboarding_step_checked_inc;
             unsigned int onboarding_step_checked;
-
-            unsigned int words_buffer_length;
-            // after an int to make sure it's aligned
-            char string_buffer[MAX(32,
-                                   sizeof(bagl_icon_details_t) +
-                                       BOLOS_APP_ICON_SIZE_B -
-                                       1)]; // to store the seed wholy
-
-            char words_buffer[257]; // 128 of words (215 => hashed to 64, or
-                                    // 128) + HMAC_LENGTH*2 = 256
         };
 
         union {
             att_t attestation;
             sigaut_t sigaut;
-            pin_t pin;
+            onboard_t onboard;
         };
     };
 
@@ -221,13 +202,6 @@ unsigned int bolos_ux_get_word_ptr(unsigned char **word,
                                    unsigned int max_length,
                                    unsigned int word_index);
 
-// passphrase will be prefixed with "MNEMONIC" from BIP39, the passphrase
-// content shall start @ 8
-unsigned int bolos_ux_mnemonic_from_data(unsigned char *in,
-                                         unsigned int inLength,
-                                         unsigned char *out,
-                                         unsigned int outLength);
-
 /**
  * Bolos system app internal UX entry point (could be overriden by a further
  * loaded BOLOS_UX application)

ledger/src/ui/src/communication.c

@@ -0,0 +1,65 @@
+/**
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2021 RSK Labs Ltd
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "apdu.h"
+#include "defs.h"
+#include "os.h"
+#include "communication.h"
+
+/*
+ * Implement the RSK ECHO command.
+ *
+ * @arg[in]  rx          number of received bytes from the Host
+ * @ret                  number of transmited bytes to the host
+ */
+unsigned int echo(unsigned int rx) {
+    return rx;
+}
+
+/*
+ * Implement the RSK MODE command.
+ *
+ * Since this is only ever called from the bootloader, this always returns
+ * RSK_MODE_BOOTLOADER
+ *
+ * @ret number of transmited bytes to the host
+ */
+unsigned int get_mode() {
+    unsigned char output_index = CMDPOS;
+    SET_APDU_AT(output_index++, RSK_MODE_BOOTLOADER);
+    return output_index;
+}
+
+/*
+ * Implement the RSK RETRIES command.
+ *
+ * Returns the current number of pin retries for the device
+ *
+ * @ret number of transmited bytes to the host
+ */
+unsigned int get_retries() {
+    unsigned char output_index = OP;
+    SET_APDU_AT(output_index++, (unsigned char)os_global_pin_retries());
+    return output_index;
+}